* [PATCH 0/2] ARM: OMAP4: Fix gpmc_fck clock @ 2014-02-17 11:27 Florian Vaussard 2014-02-17 11:27 ` [PATCH 1/2] CLK: TI: OMAP4: Remove gpmc_fck from dummy clocks Florian Vaussard 2014-02-17 11:27 ` [PATCH 2/2] ARM: DTS: OMAP4: Add gpmc_fck clock node Florian Vaussard 0 siblings, 2 replies; 7+ messages in thread From: Florian Vaussard @ 2014-02-17 11:27 UTC (permalink / raw) To: linux-arm-kernel 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. Tested on DuoVero/Parlor (OMAP4430) with SMSC9221. Regards, Florian --- Tero: I hopped to have figured out correctly the clock type and topology based on the TRM, but this is not the easiest part of the OMAP family :) Florian Vaussard (2): CLK: TI: OMAP4: Remove gpmc_fck from dummy clocks ARM: DTS: OMAP4: Add gpmc_fck clock node arch/arm/boot/dts/omap4.dtsi | 2 ++ arch/arm/boot/dts/omap44xx-clocks.dtsi | 6 ++++++ drivers/clk/ti/clk-44xx.c | 1 - 3 files changed, 8 insertions(+), 1 deletion(-) -- 1.8.1.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] CLK: TI: OMAP4: Remove gpmc_fck from dummy clocks 2014-02-17 11:27 [PATCH 0/2] ARM: OMAP4: Fix gpmc_fck clock Florian Vaussard @ 2014-02-17 11:27 ` Florian Vaussard 2014-02-17 11:27 ` [PATCH 2/2] ARM: DTS: OMAP4: Add gpmc_fck clock node Florian Vaussard 1 sibling, 0 replies; 7+ messages in thread From: Florian Vaussard @ 2014-02-17 11:27 UTC (permalink / raw) To: linux-arm-kernel 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 - 1 file changed, 1 deletion(-) 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"), -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] ARM: DTS: OMAP4: Add gpmc_fck clock node 2014-02-17 11:27 [PATCH 0/2] ARM: OMAP4: Fix gpmc_fck clock Florian Vaussard 2014-02-17 11:27 ` [PATCH 1/2] CLK: TI: OMAP4: Remove gpmc_fck from dummy clocks Florian Vaussard @ 2014-02-17 11:27 ` Florian Vaussard 2014-02-17 12:29 ` Tero Kristo 1 sibling, 1 reply; 7+ messages in thread From: Florian Vaussard @ 2014-02-17 11:27 UTC (permalink / raw) To: linux-arm-kernel Add the gpmc_fck clock, derived from l3_ick, and reference it from the GPMC node to get it correctly working. Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> --- arch/arm/boot/dts/omap4.dtsi | 2 ++ arch/arm/boot/dts/omap44xx-clocks.dtsi | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi index d3f8a6e..8a0cc71 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 = <&gpmc_fck>; + clock-names = "fck"; }; uart1: serial at 4806a000 { diff --git a/arch/arm/boot/dts/omap44xx-clocks.dtsi b/arch/arm/boot/dts/omap44xx-clocks.dtsi index c821ff5..ae2c441 100644 --- a/arch/arm/boot/dts/omap44xx-clocks.dtsi +++ b/arch/arm/boot/dts/omap44xx-clocks.dtsi @@ -1036,6 +1036,12 @@ ti,index-power-of-two; }; + gpmc_fck: gpmc_fck { + #clock-cells = <0>; + compatible = "ti,clkdm-gate-clock"; + clocks = <&l3_div_ck>; + }; + gpio2_dbclk: gpio2_dbclk { #clock-cells = <0>; compatible = "ti,gate-clock"; -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] ARM: DTS: OMAP4: Add gpmc_fck clock node 2014-02-17 11:27 ` [PATCH 2/2] ARM: DTS: OMAP4: Add gpmc_fck clock node Florian Vaussard @ 2014-02-17 12:29 ` Tero Kristo 2014-02-17 16:13 ` Florian Vaussard 0 siblings, 1 reply; 7+ messages in thread From: Tero Kristo @ 2014-02-17 12:29 UTC (permalink / raw) To: linux-arm-kernel On 02/17/2014 01:27 PM, Florian Vaussard wrote: > Add the gpmc_fck clock, derived from l3_ick, and reference it from > the GPMC node to get it correctly working. > > Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> > --- > arch/arm/boot/dts/omap4.dtsi | 2 ++ > arch/arm/boot/dts/omap44xx-clocks.dtsi | 6 ++++++ > 2 files changed, 8 insertions(+) > > diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi > index d3f8a6e..8a0cc71 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 = <&gpmc_fck>; > + clock-names = "fck"; > }; > > uart1: serial at 4806a000 { > diff --git a/arch/arm/boot/dts/omap44xx-clocks.dtsi b/arch/arm/boot/dts/omap44xx-clocks.dtsi > index c821ff5..ae2c441 100644 > --- a/arch/arm/boot/dts/omap44xx-clocks.dtsi > +++ b/arch/arm/boot/dts/omap44xx-clocks.dtsi > @@ -1036,6 +1036,12 @@ > ti,index-power-of-two; > }; > > + gpmc_fck: gpmc_fck { > + #clock-cells = <0>; > + compatible = "ti,clkdm-gate-clock"; > + clocks = <&l3_div_ck>; > + }; > + Why not implement a proper gate clock for CM_L3_2_GPMC_CLKCTRL? The approach you have taken looks good to me otherwise. -Tero > gpio2_dbclk: gpio2_dbclk { > #clock-cells = <0>; > compatible = "ti,gate-clock"; > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] ARM: DTS: OMAP4: Add gpmc_fck clock node 2014-02-17 12:29 ` Tero Kristo @ 2014-02-17 16:13 ` Florian Vaussard 2014-02-18 7:27 ` Tero Kristo 0 siblings, 1 reply; 7+ messages in thread From: Florian Vaussard @ 2014-02-17 16:13 UTC (permalink / raw) To: linux-arm-kernel Hi, On 02/17/2014 01:29 PM, Tero Kristo wrote: > On 02/17/2014 01:27 PM, Florian Vaussard wrote: >> Add the gpmc_fck clock, derived from l3_ick, and reference it from >> the GPMC node to get it correctly working. >> >> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> >> --- >> arch/arm/boot/dts/omap4.dtsi | 2 ++ >> arch/arm/boot/dts/omap44xx-clocks.dtsi | 6 ++++++ >> 2 files changed, 8 insertions(+) >> >> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi >> index d3f8a6e..8a0cc71 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 = <&gpmc_fck>; >> + clock-names = "fck"; >> }; >> >> uart1: serial at 4806a000 { >> diff --git a/arch/arm/boot/dts/omap44xx-clocks.dtsi >> b/arch/arm/boot/dts/omap44xx-clocks.dtsi >> index c821ff5..ae2c441 100644 >> --- a/arch/arm/boot/dts/omap44xx-clocks.dtsi >> +++ b/arch/arm/boot/dts/omap44xx-clocks.dtsi >> @@ -1036,6 +1036,12 @@ >> ti,index-power-of-two; >> }; >> >> + gpmc_fck: gpmc_fck { >> + #clock-cells = <0>; >> + compatible = "ti,clkdm-gate-clock"; >> + clocks = <&l3_div_ck>; >> + }; >> + > > Why not implement a proper gate clock for CM_L3_2_GPMC_CLKCTRL? The > approach you have taken looks good to me otherwise. > So something like: gpmc_fck: gpmc_fck { #clock-cells = <0>; compatible = "ti,gate-clock"; clocks = <&l3_div_ck>; reg = <(CM_L3_2_GPMC_CLKCTRL)>; ti,bit-shift = <0>; }; ? I was not sure for gate-clock, as setting the bit will enable the clock only if the corresponding clock domain (CD_L3_2) is enabled as well. Regards, Florian ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] ARM: DTS: OMAP4: Add gpmc_fck clock node 2014-02-17 16:13 ` Florian Vaussard @ 2014-02-18 7:27 ` Tero Kristo 2014-02-19 8:39 ` Florian Vaussard 0 siblings, 1 reply; 7+ messages in thread From: Tero Kristo @ 2014-02-18 7:27 UTC (permalink / raw) To: linux-arm-kernel On 02/17/2014 06:13 PM, Florian Vaussard wrote: > Hi, > > On 02/17/2014 01:29 PM, Tero Kristo wrote: >> On 02/17/2014 01:27 PM, Florian Vaussard wrote: >>> Add the gpmc_fck clock, derived from l3_ick, and reference it from >>> the GPMC node to get it correctly working. >>> >>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> >>> --- >>> arch/arm/boot/dts/omap4.dtsi | 2 ++ >>> arch/arm/boot/dts/omap44xx-clocks.dtsi | 6 ++++++ >>> 2 files changed, 8 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi >>> index d3f8a6e..8a0cc71 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 = <&gpmc_fck>; >>> + clock-names = "fck"; >>> }; >>> >>> uart1: serial at 4806a000 { >>> diff --git a/arch/arm/boot/dts/omap44xx-clocks.dtsi >>> b/arch/arm/boot/dts/omap44xx-clocks.dtsi >>> index c821ff5..ae2c441 100644 >>> --- a/arch/arm/boot/dts/omap44xx-clocks.dtsi >>> +++ b/arch/arm/boot/dts/omap44xx-clocks.dtsi >>> @@ -1036,6 +1036,12 @@ >>> ti,index-power-of-two; >>> }; >>> >>> + gpmc_fck: gpmc_fck { >>> + #clock-cells = <0>; >>> + compatible = "ti,clkdm-gate-clock"; >>> + clocks = <&l3_div_ck>; >>> + }; >>> + >> >> Why not implement a proper gate clock for CM_L3_2_GPMC_CLKCTRL? The >> approach you have taken looks good to me otherwise. >> > > So something like: > > gpmc_fck: gpmc_fck { > #clock-cells = <0>; > compatible = "ti,gate-clock"; > clocks = <&l3_div_ck>; > reg = <(CM_L3_2_GPMC_CLKCTRL)>; > ti,bit-shift = <0>; > }; > > ? I was not sure for gate-clock, as setting the bit will enable the > clock only if the corresponding clock domain (CD_L3_2) is enabled as well. Actually you may want not to add a gate clock for this, as this would result in duplicate control to the same bits... hwmod is already controlling this. Do you need the clkdm control for something? If not, you could just link the gpmc node directly to use l3_div_ck as the provider for its clock rate. Looking at the driver, I think you don't need it for anything else. -Tero ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] ARM: DTS: OMAP4: Add gpmc_fck clock node 2014-02-18 7:27 ` Tero Kristo @ 2014-02-19 8:39 ` Florian Vaussard 0 siblings, 0 replies; 7+ messages in thread From: Florian Vaussard @ 2014-02-19 8:39 UTC (permalink / raw) To: linux-arm-kernel On 02/18/2014 08:27 AM, Tero Kristo wrote: > On 02/17/2014 06:13 PM, Florian Vaussard wrote: >> Hi, >> >> On 02/17/2014 01:29 PM, Tero Kristo wrote: >>> On 02/17/2014 01:27 PM, Florian Vaussard wrote: >>>> Add the gpmc_fck clock, derived from l3_ick, and reference it from >>>> the GPMC node to get it correctly working. >>>> >>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch> >>>> --- >>>> arch/arm/boot/dts/omap4.dtsi | 2 ++ >>>> arch/arm/boot/dts/omap44xx-clocks.dtsi | 6 ++++++ >>>> 2 files changed, 8 insertions(+) >>>> >>>> diff --git a/arch/arm/boot/dts/omap4.dtsi >>>> b/arch/arm/boot/dts/omap4.dtsi >>>> index d3f8a6e..8a0cc71 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 = <&gpmc_fck>; >>>> + clock-names = "fck"; >>>> }; >>>> >>>> uart1: serial at 4806a000 { >>>> diff --git a/arch/arm/boot/dts/omap44xx-clocks.dtsi >>>> b/arch/arm/boot/dts/omap44xx-clocks.dtsi >>>> index c821ff5..ae2c441 100644 >>>> --- a/arch/arm/boot/dts/omap44xx-clocks.dtsi >>>> +++ b/arch/arm/boot/dts/omap44xx-clocks.dtsi >>>> @@ -1036,6 +1036,12 @@ >>>> ti,index-power-of-two; >>>> }; >>>> >>>> + gpmc_fck: gpmc_fck { >>>> + #clock-cells = <0>; >>>> + compatible = "ti,clkdm-gate-clock"; >>>> + clocks = <&l3_div_ck>; >>>> + }; >>>> + >>> >>> Why not implement a proper gate clock for CM_L3_2_GPMC_CLKCTRL? The >>> approach you have taken looks good to me otherwise. >>> >> >> So something like: >> >> gpmc_fck: gpmc_fck { >> #clock-cells = <0>; >> compatible = "ti,gate-clock"; >> clocks = <&l3_div_ck>; >> reg = <(CM_L3_2_GPMC_CLKCTRL)>; >> ti,bit-shift = <0>; >> }; >> >> ? I was not sure for gate-clock, as setting the bit will enable the >> clock only if the corresponding clock domain (CD_L3_2) is enabled as >> well. > > Actually you may want not to add a gate clock for this, as this would > result in duplicate control to the same bits... hwmod is already > controlling this. Do you need the clkdm control for something? If not, > you could just link the gpmc node directly to use l3_div_ck as the > provider for its clock rate. Looking at the driver, I think you don't > need it for anything else. > I missed the hwmod bit. So your solution seems the most reasonable for now. I will send a v2 shortly. Thanks for your help. Regards, Florian ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-02-19 8:39 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-02-17 11:27 [PATCH 0/2] ARM: OMAP4: Fix gpmc_fck clock Florian Vaussard 2014-02-17 11:27 ` [PATCH 1/2] CLK: TI: OMAP4: Remove gpmc_fck from dummy clocks Florian Vaussard 2014-02-17 11:27 ` [PATCH 2/2] ARM: DTS: OMAP4: Add gpmc_fck clock node Florian Vaussard 2014-02-17 12:29 ` Tero Kristo 2014-02-17 16:13 ` Florian Vaussard 2014-02-18 7:27 ` Tero Kristo 2014-02-19 8:39 ` Florian Vaussard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).