[PATCH] AT91: SAM9G45 - add a separate clock entry for every single TC block

[PATCH] AT91: SAM9G45 - add a separate clock entry for every single TC block

[Nicolas Ferre]

From: Fabian Godehardt <fg@emlix.com>

Without this patch you will not be able to register the first block
because of the second association call on at91_add_device_tc().

Signed-off-by: Fabian Godehardt <fg@emlix.com>
[nicolas.ferre@atmel.com: change tcb1_clk to fake child clock of tcb0_clk]
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 arch/arm/mach-at91/at91sam9g45.c         |   15 ++++++++++++---
 arch/arm/mach-at91/at91sam9g45_devices.c |    4 ++--
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-at91/at91sam9g45.c b/arch/arm/mach-at91/at91sam9g45.c
index 753c0d3..c67b47f 100644
--- a/arch/arm/mach-at91/at91sam9g45.c
+++ b/arch/arm/mach-at91/at91sam9g45.c
@@ -121,8 +121,8 @@ static struct clk ssc1_clk = {
 	.pmc_mask	= 1 << AT91SAM9G45_ID_SSC1,
 	.type		= CLK_TYPE_PERIPHERAL,
 };
-static struct clk tcb_clk = {
-	.name		= "tcb_clk",
+static struct clk tcb0_clk = {
+	.name		= "tcb0_clk",
 	.pmc_mask	= 1 << AT91SAM9G45_ID_TCB,
 	.type		= CLK_TYPE_PERIPHERAL,
 };
@@ -192,6 +192,14 @@ static struct clk ohci_clk = {
 	.parent		= &uhphs_clk,
 };
 
+/* One additional fake clock for second TC block */
+static struct clk tcb1_clk = {
+	.name		= "tcb1_clk",
+	.pmc_mask	= 0,
+	.type		= CLK_TYPE_PERIPHERAL,
+	.parent		= &tcb0_clk,
+};
+
 static struct clk *periph_clocks[] __initdata = {
 	&pioA_clk,
 	&pioB_clk,
@@ -208,7 +216,7 @@ static struct clk *periph_clocks[] __initdata = {
 	&spi1_clk,
 	&ssc0_clk,
 	&ssc1_clk,
-	&tcb_clk,
+	&tcb0_clk,
 	&pwm_clk,
 	&tsc_clk,
 	&dma_clk,
@@ -221,6 +229,7 @@ static struct clk *periph_clocks[] __initdata = {
 	&mmc1_clk,
 	// irq0
 	&ohci_clk,
+	&tcb1_clk,
 };
 
 /*
diff --git a/arch/arm/mach-at91/at91sam9g45_devices.c b/arch/arm/mach-at91/at91sam9g45_devices.c
index 809114d..4822019 100644
--- a/arch/arm/mach-at91/at91sam9g45_devices.c
+++ b/arch/arm/mach-at91/at91sam9g45_devices.c
@@ -835,9 +835,9 @@ static struct platform_device at91sam9g45_tcb1_device = {
 static void __init at91_add_device_tc(void)
 {
 	/* this chip has one clock and irq for all six TC channels */
-	at91_clock_associate("tcb_clk", &at91sam9g45_tcb0_device.dev, "t0_clk");
+	at91_clock_associate("tcb0_clk", &at91sam9g45_tcb0_device.dev, "t0_clk");
 	platform_device_register(&at91sam9g45_tcb0_device);
-	at91_clock_associate("tcb_clk", &at91sam9g45_tcb1_device.dev, "t0_clk");
+	at91_clock_associate("tcb1_clk", &at91sam9g45_tcb1_device.dev, "t0_clk");
 	platform_device_register(&at91sam9g45_tcb1_device);
 }
 #else
-- 
1.5.6.5

Re: [PATCH] AT91: SAM9G45 - add a separate clock entry for every single TC block

[avictor.za]

> Without this patch you will not be able to register the first block
> because of the second association call on at91_add_device_tc().
>
> Signed-off-by: Fabian Godehardt <fg@emlix.com>
> [nicolas.ferre@atmel.com: change tcb1_clk to fake child clock of tcb0_clk]
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Signed-off-by: Nicolas Ferre

Acked-By: Andrew Victor <linux@maxim.org.za>

Re: [PATCH] AT91: SAM9G45 - add a separate clock entry for every single TC block

[avictor.za]

hi,

> +/* One additional fake clock for second TC block */
> +static struct clk tcb1_clk = {
> +       .name           = "tcb1_clk",
> +       .pmc_mask       = 0,
> +       .type           = CLK_TYPE_PERIPHERAL,
> +       .parent         = &tcb0_clk,
> +};
> +

Looking at this again...  since type is CLK_TYPE_PERIPHERAL, when you
call clk_register() the "parent" is changed to the master clock.

Which means, then later you call clk_enable() the "pmc_mask" is still
0, so 0 gets written (in pmc_periph_mode) to AT91_PMC_PCER.  So the
TCB clock won't be enabled.

Or am I missing something?


Regards,
  Andrew Victor

Re: [PATCH] AT91: SAM9G45 - add a separate clock entry for every single TC block

[Nicolas Ferre]

Le 07/09/2010 21:42, avictor.za@gmail.com :
> hi,
> 
>> +/* One additional fake clock for second TC block */
>> +static struct clk tcb1_clk = {
>> +       .name           = "tcb1_clk",
>> +       .pmc_mask       = 0,
>> +       .type           = CLK_TYPE_PERIPHERAL,
>> +       .parent         = &tcb0_clk,
>> +};
>> +
> 
> Looking at this again...  since type is CLK_TYPE_PERIPHERAL, when you
> call clk_register() the "parent" is changed to the master clock.
> 
> Which means, then later you call clk_enable() the "pmc_mask" is still
> 0, so 0 gets written (in pmc_periph_mode) to AT91_PMC_PCER.  So the
> TCB clock won't be enabled.
> 
> Or am I missing something?

You are absolutely right!

What do you think about this modification of clk_register() function?

--- a/arch/arm/mach-at91/clock.c
+++ b/arch/arm/mach-at91/clock.c
@@ -501,7 +501,8 @@ postcore_initcall(at91_clk_debugfs_init);
 int __init clk_register(struct clk *clk)
 {
        if (clk_is_peripheral(clk)) {
-               clk->parent = &mck;
+               if (!clk->parent)
+                       clk->parent = &mck;
                clk->mode = pmc_periph_mode;
                list_add_tail(&clk->node, &clocks);
        }

It is a very little modification which implements what I had in mind
while creating a kind of "child peripheral" clock.

Best regards,
-- 
Nicolas Ferre

Re: [PATCH] AT91: SAM9G45 - add a separate clock entry for every single TC block

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:00 Wed 08 Sep     , Nicolas Ferre wrote:
> Le 07/09/2010 21:42, avictor.za@gmail.com :
> > hi,
> > 
> >> +/* One additional fake clock for second TC block */
> >> +static struct clk tcb1_clk = {
> >> +       .name           = "tcb1_clk",
> >> +       .pmc_mask       = 0,
> >> +       .type           = CLK_TYPE_PERIPHERAL,
> >> +       .parent         = &tcb0_clk,
> >> +};
> >> +
> > 
> > Looking at this again...  since type is CLK_TYPE_PERIPHERAL, when you
> > call clk_register() the "parent" is changed to the master clock.
> > 
> > Which means, then later you call clk_enable() the "pmc_mask" is still
> > 0, so 0 gets written (in pmc_periph_mode) to AT91_PMC_PCER.  So the
> > TCB clock won't be enabled.
> > 
> > Or am I missing something?
> 
> You are absolutely right!
> 
> What do you think about this modification of clk_register() function?
> 
> --- a/arch/arm/mach-at91/clock.c
> +++ b/arch/arm/mach-at91/clock.c
> @@ -501,7 +501,8 @@ postcore_initcall(at91_clk_debugfs_init);
>  int __init clk_register(struct clk *clk)
>  {
>         if (clk_is_peripheral(clk)) {
> -               clk->parent = &mck;
> +               if (!clk->parent)
> +                       clk->parent = &mck;
>                 clk->mode = pmc_periph_mode;
>                 list_add_tail(&clk->node, &clocks);
>         }
> 
> It is a very little modification which implements what I had in mind
> while creating a kind of "child peripheral" clock.
so so

but until we switch to clkdev it will solve the issue

Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

Best Regards,
J.