[PATCH v3 0/2] ARM: OMAP4+: Fix gpmc_fck clock

[PATCH v3 0/2] ARM: OMAP4+: Fix gpmc_fck clock

[Florian Vaussard]

Hello,

Trying to get my SMSC9221 working on OMAP4 with DT,
I faced a misconfigured gpmc_fck (dummy clock set to 0)
resulting in serveral division-by-zero, misconfigured
timings and driver lost in the La La Land.

To solve this, patch 1 removes gpmc_fck from the dummy
clocks, and patch 2 adds the gpmc_fck DT node and
reference it from the gpmc node.

*Note*: For DRA7, there is no DTS node for the GPMC, so I
was unable to set the corresponding clock. And without
a public TRM, I cannot do much more.

Tested on DuoVero/Parlor (OMAP4430) with SMSC9221 (DTS
was posted on the OMAP ML [1]).

Regards,
Florian

[1] http://thread.gmane.org/gmane.linux.ports.arm.omap/110801
---
Since v2:
- Added OMAP5 and DRA7
Since v1:
- Removed the gpmc_fck clock node, and reference directly l3_ick

Florian Vaussard (2):
  CLK: TI: OMAP4/5/DRA7: Remove gpmc_fck from dummy clocks
  ARM: DTS: OMAP4/5: Use l3_ick for the gpmc node

 arch/arm/boot/dts/omap4.dtsi | 2 ++
 arch/arm/boot/dts/omap5.dtsi | 2 ++
 drivers/clk/ti/clk-44xx.c    | 1 -
 drivers/clk/ti/clk-54xx.c    | 1 -
 drivers/clk/ti/clk-7xx.c     | 1 -
 5 files changed, 4 insertions(+), 3 deletions(-)

-- 
1.8.1.2

[PATCH v3 1/2] CLK: TI: OMAP4/5/DRA7: Remove gpmc_fck from dummy clocks

[Florian Vaussard]

When arch/arm/mach-omap2/gpmc.c calls clk_get(..., "fck"), it will
get a dummy clock and try to use it. As the rate is configured to zero,
this will result in several divisions by zero, and misconfigured
timings, with devices on the bus being lost in the La La Land.

It is better to remove gpmc_fck from the dummy clocks, so that gpmc.c
can fail gracefully.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 drivers/clk/ti/clk-44xx.c | 1 -
 drivers/clk/ti/clk-54xx.c | 1 -
 drivers/clk/ti/clk-7xx.c  | 1 -
 3 files changed, 3 deletions(-)

diff --git a/drivers/clk/ti/clk-44xx.c b/drivers/clk/ti/clk-44xx.c
index ae00218..02517a8 100644
--- a/drivers/clk/ti/clk-44xx.c
+++ b/drivers/clk/ti/clk-44xx.c
@@ -222,7 +222,6 @@ static struct ti_dt_clk omap44xx_clks[] = {
 	DT_CLK(NULL, "auxclk5_src_ck", "auxclk5_src_ck"),
 	DT_CLK(NULL, "auxclk5_ck", "auxclk5_ck"),
 	DT_CLK(NULL, "auxclkreq5_ck", "auxclkreq5_ck"),
-	DT_CLK("50000000.gpmc", "fck", "dummy_ck"),
 	DT_CLK("omap_i2c.1", "ick", "dummy_ck"),
 	DT_CLK("omap_i2c.2", "ick", "dummy_ck"),
 	DT_CLK("omap_i2c.3", "ick", "dummy_ck"),
diff --git a/drivers/clk/ti/clk-54xx.c b/drivers/clk/ti/clk-54xx.c
index 0ef9f58..08f3d1b 100644
--- a/drivers/clk/ti/clk-54xx.c
+++ b/drivers/clk/ti/clk-54xx.c
@@ -182,7 +182,6 @@ static struct ti_dt_clk omap54xx_clks[] = {
 	DT_CLK(NULL, "auxclk3_src_ck", "auxclk3_src_ck"),
 	DT_CLK(NULL, "auxclk3_ck", "auxclk3_ck"),
 	DT_CLK(NULL, "auxclkreq3_ck", "auxclkreq3_ck"),
-	DT_CLK(NULL, "gpmc_ck", "dummy_ck"),
 	DT_CLK("omap_i2c.1", "ick", "dummy_ck"),
 	DT_CLK("omap_i2c.2", "ick", "dummy_ck"),
 	DT_CLK("omap_i2c.3", "ick", "dummy_ck"),
diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
index 9977653..f7e4073 100644
--- a/drivers/clk/ti/clk-7xx.c
+++ b/drivers/clk/ti/clk-7xx.c
@@ -262,7 +262,6 @@ static struct ti_dt_clk dra7xx_clks[] = {
 	DT_CLK(NULL, "vip1_gclk_mux", "vip1_gclk_mux"),
 	DT_CLK(NULL, "vip2_gclk_mux", "vip2_gclk_mux"),
 	DT_CLK(NULL, "vip3_gclk_mux", "vip3_gclk_mux"),
-	DT_CLK(NULL, "gpmc_ck", "dummy_ck"),
 	DT_CLK("omap_i2c.1", "ick", "dummy_ck"),
 	DT_CLK("omap_i2c.2", "ick", "dummy_ck"),
 	DT_CLK("omap_i2c.3", "ick", "dummy_ck"),
-- 
1.8.1.2

Re: [PATCH v3 1/2] CLK: TI: OMAP4/5/DRA7: Remove gpmc_fck from dummy clocks

[Mike Turquette]

Quoting Florian Vaussard (2014-02-26 02:38:08)
> When arch/arm/mach-omap2/gpmc.c calls clk_get(..., "fck"), it will
> get a dummy clock and try to use it. As the rate is configured to zero,
> this will result in several divisions by zero, and misconfigured
> timings, with devices on the bus being lost in the La La Land.
> 
> It is better to remove gpmc_fck from the dummy clocks, so that gpmc.c
> can fail gracefully.
> 
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>

Looks good to me.

Regards,
Mike

> ---
>  drivers/clk/ti/clk-44xx.c | 1 -
>  drivers/clk/ti/clk-54xx.c | 1 -
>  drivers/clk/ti/clk-7xx.c  | 1 -
>  3 files changed, 3 deletions(-)
> 
> diff --git a/drivers/clk/ti/clk-44xx.c b/drivers/clk/ti/clk-44xx.c
> index ae00218..02517a8 100644
> --- a/drivers/clk/ti/clk-44xx.c
> +++ b/drivers/clk/ti/clk-44xx.c
> @@ -222,7 +222,6 @@ static struct ti_dt_clk omap44xx_clks[] = {
>         DT_CLK(NULL, "auxclk5_src_ck", "auxclk5_src_ck"),
>         DT_CLK(NULL, "auxclk5_ck", "auxclk5_ck"),
>         DT_CLK(NULL, "auxclkreq5_ck", "auxclkreq5_ck"),
> -       DT_CLK("50000000.gpmc", "fck", "dummy_ck"),
>         DT_CLK("omap_i2c.1", "ick", "dummy_ck"),
>         DT_CLK("omap_i2c.2", "ick", "dummy_ck"),
>         DT_CLK("omap_i2c.3", "ick", "dummy_ck"),
> diff --git a/drivers/clk/ti/clk-54xx.c b/drivers/clk/ti/clk-54xx.c
> index 0ef9f58..08f3d1b 100644
> --- a/drivers/clk/ti/clk-54xx.c
> +++ b/drivers/clk/ti/clk-54xx.c
> @@ -182,7 +182,6 @@ static struct ti_dt_clk omap54xx_clks[] = {
>         DT_CLK(NULL, "auxclk3_src_ck", "auxclk3_src_ck"),
>         DT_CLK(NULL, "auxclk3_ck", "auxclk3_ck"),
>         DT_CLK(NULL, "auxclkreq3_ck", "auxclkreq3_ck"),
> -       DT_CLK(NULL, "gpmc_ck", "dummy_ck"),
>         DT_CLK("omap_i2c.1", "ick", "dummy_ck"),
>         DT_CLK("omap_i2c.2", "ick", "dummy_ck"),
>         DT_CLK("omap_i2c.3", "ick", "dummy_ck"),
> diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
> index 9977653..f7e4073 100644
> --- a/drivers/clk/ti/clk-7xx.c
> +++ b/drivers/clk/ti/clk-7xx.c
> @@ -262,7 +262,6 @@ static struct ti_dt_clk dra7xx_clks[] = {
>         DT_CLK(NULL, "vip1_gclk_mux", "vip1_gclk_mux"),
>         DT_CLK(NULL, "vip2_gclk_mux", "vip2_gclk_mux"),
>         DT_CLK(NULL, "vip3_gclk_mux", "vip3_gclk_mux"),
> -       DT_CLK(NULL, "gpmc_ck", "dummy_ck"),
>         DT_CLK("omap_i2c.1", "ick", "dummy_ck"),
>         DT_CLK("omap_i2c.2", "ick", "dummy_ck"),
>         DT_CLK("omap_i2c.3", "ick", "dummy_ck"),
> -- 
> 1.8.1.2
> 

Re: [PATCH v3 1/2] CLK: TI: OMAP4/5/DRA7: Remove gpmc_fck from dummy clocks

[Tony Lindgren]

* Mike Turquette <mturquette@linaro.org> [140226 12:43]:
> Quoting Florian Vaussard (2014-02-26 02:38:08)
> > When arch/arm/mach-omap2/gpmc.c calls clk_get(..., "fck"), it will
> > get a dummy clock and try to use it. As the rate is configured to zero,
> > this will result in several divisions by zero, and misconfigured
> > timings, with devices on the bus being lost in the La La Land.
> > 
> > It is better to remove gpmc_fck from the dummy clocks, so that gpmc.c
> > can fail gracefully.
> > 
> > Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> 
> Looks good to me.

Can that be upgraded to an ack then?

Regards,

Tony
 
> > ---
> >  drivers/clk/ti/clk-44xx.c | 1 -
> >  drivers/clk/ti/clk-54xx.c | 1 -
> >  drivers/clk/ti/clk-7xx.c  | 1 -
> >  3 files changed, 3 deletions(-)
> > 
> > diff --git a/drivers/clk/ti/clk-44xx.c b/drivers/clk/ti/clk-44xx.c
> > index ae00218..02517a8 100644
> > --- a/drivers/clk/ti/clk-44xx.c
> > +++ b/drivers/clk/ti/clk-44xx.c
> > @@ -222,7 +222,6 @@ static struct ti_dt_clk omap44xx_clks[] = {
> >         DT_CLK(NULL, "auxclk5_src_ck", "auxclk5_src_ck"),
> >         DT_CLK(NULL, "auxclk5_ck", "auxclk5_ck"),
> >         DT_CLK(NULL, "auxclkreq5_ck", "auxclkreq5_ck"),
> > -       DT_CLK("50000000.gpmc", "fck", "dummy_ck"),
> >         DT_CLK("omap_i2c.1", "ick", "dummy_ck"),
> >         DT_CLK("omap_i2c.2", "ick", "dummy_ck"),
> >         DT_CLK("omap_i2c.3", "ick", "dummy_ck"),
> > diff --git a/drivers/clk/ti/clk-54xx.c b/drivers/clk/ti/clk-54xx.c
> > index 0ef9f58..08f3d1b 100644
> > --- a/drivers/clk/ti/clk-54xx.c
> > +++ b/drivers/clk/ti/clk-54xx.c
> > @@ -182,7 +182,6 @@ static struct ti_dt_clk omap54xx_clks[] = {
> >         DT_CLK(NULL, "auxclk3_src_ck", "auxclk3_src_ck"),
> >         DT_CLK(NULL, "auxclk3_ck", "auxclk3_ck"),
> >         DT_CLK(NULL, "auxclkreq3_ck", "auxclkreq3_ck"),
> > -       DT_CLK(NULL, "gpmc_ck", "dummy_ck"),
> >         DT_CLK("omap_i2c.1", "ick", "dummy_ck"),
> >         DT_CLK("omap_i2c.2", "ick", "dummy_ck"),
> >         DT_CLK("omap_i2c.3", "ick", "dummy_ck"),
> > diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
> > index 9977653..f7e4073 100644
> > --- a/drivers/clk/ti/clk-7xx.c
> > +++ b/drivers/clk/ti/clk-7xx.c
> > @@ -262,7 +262,6 @@ static struct ti_dt_clk dra7xx_clks[] = {
> >         DT_CLK(NULL, "vip1_gclk_mux", "vip1_gclk_mux"),
> >         DT_CLK(NULL, "vip2_gclk_mux", "vip2_gclk_mux"),
> >         DT_CLK(NULL, "vip3_gclk_mux", "vip3_gclk_mux"),
> > -       DT_CLK(NULL, "gpmc_ck", "dummy_ck"),
> >         DT_CLK("omap_i2c.1", "ick", "dummy_ck"),
> >         DT_CLK("omap_i2c.2", "ick", "dummy_ck"),
> >         DT_CLK("omap_i2c.3", "ick", "dummy_ck"),
> > -- 
> > 1.8.1.2
> > 

Re: [PATCH v3 1/2] CLK: TI: OMAP4/5/DRA7: Remove gpmc_fck from dummy clocks

[Tero Kristo]

On 02/28/2014 07:28 PM, Tony Lindgren wrote:
> * Mike Turquette <mturquette@linaro.org> [140226 12:43]:
>> Quoting Florian Vaussard (2014-02-26 02:38:08)
>>> When arch/arm/mach-omap2/gpmc.c calls clk_get(..., "fck"), it will
>>> get a dummy clock and try to use it. As the rate is configured to zero,
>>> this will result in several divisions by zero, and misconfigured
>>> timings, with devices on the bus being lost in the La La Land.
>>>
>>> It is better to remove gpmc_fck from the dummy clocks, so that gpmc.c
>>> can fail gracefully.
>>>
>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
>>
>> Looks good to me.
>
> Can that be upgraded to an ack then?

Looks good to me also.

Acked-by: Tero Kristo <t-kristo@ti.com>

>
> Regards,
>
> Tony
>
>>> ---
>>>   drivers/clk/ti/clk-44xx.c | 1 -
>>>   drivers/clk/ti/clk-54xx.c | 1 -
>>>   drivers/clk/ti/clk-7xx.c  | 1 -
>>>   3 files changed, 3 deletions(-)
>>>
>>> diff --git a/drivers/clk/ti/clk-44xx.c b/drivers/clk/ti/clk-44xx.c
>>> index ae00218..02517a8 100644
>>> --- a/drivers/clk/ti/clk-44xx.c
>>> +++ b/drivers/clk/ti/clk-44xx.c
>>> @@ -222,7 +222,6 @@ static struct ti_dt_clk omap44xx_clks[] = {
>>>          DT_CLK(NULL, "auxclk5_src_ck", "auxclk5_src_ck"),
>>>          DT_CLK(NULL, "auxclk5_ck", "auxclk5_ck"),
>>>          DT_CLK(NULL, "auxclkreq5_ck", "auxclkreq5_ck"),
>>> -       DT_CLK("50000000.gpmc", "fck", "dummy_ck"),
>>>          DT_CLK("omap_i2c.1", "ick", "dummy_ck"),
>>>          DT_CLK("omap_i2c.2", "ick", "dummy_ck"),
>>>          DT_CLK("omap_i2c.3", "ick", "dummy_ck"),
>>> diff --git a/drivers/clk/ti/clk-54xx.c b/drivers/clk/ti/clk-54xx.c
>>> index 0ef9f58..08f3d1b 100644
>>> --- a/drivers/clk/ti/clk-54xx.c
>>> +++ b/drivers/clk/ti/clk-54xx.c
>>> @@ -182,7 +182,6 @@ static struct ti_dt_clk omap54xx_clks[] = {
>>>          DT_CLK(NULL, "auxclk3_src_ck", "auxclk3_src_ck"),
>>>          DT_CLK(NULL, "auxclk3_ck", "auxclk3_ck"),
>>>          DT_CLK(NULL, "auxclkreq3_ck", "auxclkreq3_ck"),
>>> -       DT_CLK(NULL, "gpmc_ck", "dummy_ck"),
>>>          DT_CLK("omap_i2c.1", "ick", "dummy_ck"),
>>>          DT_CLK("omap_i2c.2", "ick", "dummy_ck"),
>>>          DT_CLK("omap_i2c.3", "ick", "dummy_ck"),
>>> diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
>>> index 9977653..f7e4073 100644
>>> --- a/drivers/clk/ti/clk-7xx.c
>>> +++ b/drivers/clk/ti/clk-7xx.c
>>> @@ -262,7 +262,6 @@ static struct ti_dt_clk dra7xx_clks[] = {
>>>          DT_CLK(NULL, "vip1_gclk_mux", "vip1_gclk_mux"),
>>>          DT_CLK(NULL, "vip2_gclk_mux", "vip2_gclk_mux"),
>>>          DT_CLK(NULL, "vip3_gclk_mux", "vip3_gclk_mux"),
>>> -       DT_CLK(NULL, "gpmc_ck", "dummy_ck"),
>>>          DT_CLK("omap_i2c.1", "ick", "dummy_ck"),
>>>          DT_CLK("omap_i2c.2", "ick", "dummy_ck"),
>>>          DT_CLK("omap_i2c.3", "ick", "dummy_ck"),
>>> --
>>> 1.8.1.2
>>>

[PATCH v3 2/2] ARM: DTS: OMAP4/5: Use l3_ick for the gpmc node

[Florian Vaussard]

The GPMC clock is derived from l3_ick. The simplest solution is
to reference directly l3_ick to provide the GPMC fck in order to
get correct timings. The real management of the clock is left to
hwmod.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 arch/arm/boot/dts/omap4.dtsi | 2 ++
 arch/arm/boot/dts/omap5.dtsi | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 756cd34..381c0cc 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -275,6 +275,8 @@
 			gpmc,num-waitpins = <4>;
 			ti,hwmods = "gpmc";
 			ti,no-idle-on-init;
+			clocks = <&l3_div_ck>;
+			clock-names = "fck";
 		};
 
 		uart1: serial@4806a000 {
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index a72813a..7a16647 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -302,6 +302,8 @@
 			gpmc,num-cs = <8>;
 			gpmc,num-waitpins = <4>;
 			ti,hwmods = "gpmc";
+			clocks = <&l3_iclk_div>;
+			clock-names = "fck";
 		};
 
 		i2c1: i2c@48070000 {
-- 
1.8.1.2

Re: [PATCH v3 0/2] ARM: OMAP4+: Fix gpmc_fck clock

[Florian Vaussard]

Hi,

On 02/26/2014 11:38 AM, Florian Vaussard wrote:
> Hello,
> 
> Trying to get my SMSC9221 working on OMAP4 with DT,
> I faced a misconfigured gpmc_fck (dummy clock set to 0)
> resulting in serveral division-by-zero, misconfigured
> timings and driver lost in the La La Land.
> 
> To solve this, patch 1 removes gpmc_fck from the dummy
> clocks, and patch 2 adds the gpmc_fck DT node and
> reference it from the gpmc node.
> 
> *Note*: For DRA7, there is no DTS node for the GPMC, so I
> was unable to set the corresponding clock. And without
> a public TRM, I cannot do much more.
> 
> Tested on DuoVero/Parlor (OMAP4430) with SMSC9221 (DTS
> was posted on the OMAP ML [1]).
> 
> Regards,
> Florian
> 
> [1] http://thread.gmane.org/gmane.linux.ports.arm.omap/110801
> ---
> Since v2:
> - Added OMAP5 and DRA7
> Since v1:
> - Removed the gpmc_fck clock node, and reference directly l3_ick
> 
> Florian Vaussard (2):
>   CLK: TI: OMAP4/5/DRA7: Remove gpmc_fck from dummy clocks
>   ARM: DTS: OMAP4/5: Use l3_ick for the gpmc node
> 

Any other comments on these two patches? If it is not going into 3.15
before the merge window, it will have to go through the -rc cycle as the
Duovero/Parlor .dts will be upstream.

Regards,
Florian

>  arch/arm/boot/dts/omap4.dtsi | 2 ++
>  arch/arm/boot/dts/omap5.dtsi | 2 ++
>  drivers/clk/ti/clk-44xx.c    | 1 -
>  drivers/clk/ti/clk-54xx.c    | 1 -
>  drivers/clk/ti/clk-7xx.c     | 1 -
>  5 files changed, 4 insertions(+), 3 deletions(-)
> 

Re: [PATCH v3 0/2] ARM: OMAP4+: Fix gpmc_fck clock

[Tony Lindgren]

* Florian Vaussard <florian.vaussard@epfl.ch> [140313 01:36]:
> Hi,
> 
> On 02/26/2014 11:38 AM, Florian Vaussard wrote:
> > Hello,
> > 
> > Trying to get my SMSC9221 working on OMAP4 with DT,
> > I faced a misconfigured gpmc_fck (dummy clock set to 0)
> > resulting in serveral division-by-zero, misconfigured
> > timings and driver lost in the La La Land.
> > 
> > To solve this, patch 1 removes gpmc_fck from the dummy
> > clocks, and patch 2 adds the gpmc_fck DT node and
> > reference it from the gpmc node.
> > 
> > *Note*: For DRA7, there is no DTS node for the GPMC, so I
> > was unable to set the corresponding clock. And without
> > a public TRM, I cannot do much more.
> > 
> > Tested on DuoVero/Parlor (OMAP4430) with SMSC9221 (DTS
> > was posted on the OMAP ML [1]).
> > 
> > Regards,
> > Florian
> > 
> > [1] http://thread.gmane.org/gmane.linux.ports.arm.omap/110801
> > ---
> > Since v2:
> > - Added OMAP5 and DRA7
> > Since v1:
> > - Removed the gpmc_fck clock node, and reference directly l3_ick
> > 
> > Florian Vaussard (2):
> >   CLK: TI: OMAP4/5/DRA7: Remove gpmc_fck from dummy clocks
> >   ARM: DTS: OMAP4/5: Use l3_ick for the gpmc node
> > 
> 
> Any other comments on these two patches? If it is not going into 3.15
> before the merge window, it will have to go through the -rc cycle as the
> Duovero/Parlor .dts will be upstream.

Applying both into omap-for-v3.15/fixes with Cc: stable v3.14+.

Regards,

Tony
 
> >  arch/arm/boot/dts/omap4.dtsi | 2 ++
> >  arch/arm/boot/dts/omap5.dtsi | 2 ++
> >  drivers/clk/ti/clk-44xx.c    | 1 -
> >  drivers/clk/ti/clk-54xx.c    | 1 -
> >  drivers/clk/ti/clk-7xx.c     | 1 -
> >  5 files changed, 4 insertions(+), 3 deletions(-)
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html