| Message ID | 20191109142017.10851-1-grygorii.strashko@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | clk: ti: dra7: fix parent for gmac_clkctrl | expand |
* Grygorii Strashko <grygorii.strashko@ti.com> [191109 14:21]: > The parent clk for gmac clk ctrl has to be gmac_main_clk (125MHz) instead > of dpll_gmac_ck (1GHz). This is caused incorrect CPSW MDIO operation. > Hence, fix it. > > Fixes: commit dffa9051d546 ('clk: ti: dra7: add new clkctrl data') > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> Hmm is there a mux for the source though? In that case using assigned-clocks and assigned-clock-parents in addition to a related clk-7xx.c fix would be needed. Regards, Tony > --- > drivers/clk/ti/clk-7xx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c > index 9dd6185a4b4e..66e4b2b9ec60 100644 > --- a/drivers/clk/ti/clk-7xx.c > +++ b/drivers/clk/ti/clk-7xx.c > @@ -405,7 +405,7 @@ static const struct omap_clkctrl_bit_data dra7_gmac_bit_data[] __initconst = { > }; > > static const struct omap_clkctrl_reg_data dra7_gmac_clkctrl_regs[] __initconst = { > - { DRA7_GMAC_GMAC_CLKCTRL, dra7_gmac_bit_data, CLKF_SW_SUP, "dpll_gmac_ck" }, > + { DRA7_GMAC_GMAC_CLKCTRL, dra7_gmac_bit_data, CLKF_SW_SUP, "gmac_main_clk" }, > { 0 }, > }; > > -- > 2.17.1 >
On 11/11/2019 19:12, Tony Lindgren wrote: > * Grygorii Strashko <grygorii.strashko@ti.com> [191109 14:21]: >> The parent clk for gmac clk ctrl has to be gmac_main_clk (125MHz) instead >> of dpll_gmac_ck (1GHz). This is caused incorrect CPSW MDIO operation. >> Hence, fix it. >> >> Fixes: commit dffa9051d546 ('clk: ti: dra7: add new clkctrl data') >> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > > Hmm is there a mux for the source though? Not sure what do you mean here :( fck clock for CPSW and MDIO is "gmac_main_clk" which is 125MHz and that what need to be passed to drivers and enabled through the clock tree. The TI specific PM is handled by gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 0 which required sysc programming and child modules dosn't need to even know that. So, this patch is simply correct clock tree for dra7: dpll_gmac_ck -> .... -> gmac_main_clk -> gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 0 Seems MDIO dt also need to be fixed to use: gmac_main_clk as "fck". I'll try and send patch. By the way, the patch follows am3/am4 (am4_cpsw_125mhz_clkctrl_regs, am3_cpsw_125mhz_clkctrl_regs) > > In that case using assigned-clocks and assigned-clock-parents in > addition to a related clk-7xx.c fix would be needed. > > Regards, > > Tony > >> --- >> drivers/clk/ti/clk-7xx.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c >> index 9dd6185a4b4e..66e4b2b9ec60 100644 >> --- a/drivers/clk/ti/clk-7xx.c >> +++ b/drivers/clk/ti/clk-7xx.c >> @@ -405,7 +405,7 @@ static const struct omap_clkctrl_bit_data dra7_gmac_bit_data[] __initconst = { >> }; >> >> static const struct omap_clkctrl_reg_data dra7_gmac_clkctrl_regs[] __initconst = { >> - { DRA7_GMAC_GMAC_CLKCTRL, dra7_gmac_bit_data, CLKF_SW_SUP, "dpll_gmac_ck" }, >> + { DRA7_GMAC_GMAC_CLKCTRL, dra7_gmac_bit_data, CLKF_SW_SUP, "gmac_main_clk" }, >> { 0 }, >> }; >> >> -- >> 2.17.1 >> -- Best regards, grygorii
* Grygorii Strashko <grygorii.strashko@ti.com> [191113 10:02]: > > > On 11/11/2019 19:12, Tony Lindgren wrote: > > * Grygorii Strashko <grygorii.strashko@ti.com> [191109 14:21]: > > > The parent clk for gmac clk ctrl has to be gmac_main_clk (125MHz) instead > > > of dpll_gmac_ck (1GHz). This is caused incorrect CPSW MDIO operation. > > > Hence, fix it. > > > > > > Fixes: commit dffa9051d546 ('clk: ti: dra7: add new clkctrl data') > > > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > > > > Hmm is there a mux for the source though? > > Not sure what do you mean here :( > > fck clock for CPSW and MDIO is "gmac_main_clk" which is 125MHz and > that what need to be passed to drivers and enabled through the clock tree. > The TI specific PM is handled by gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 0 > which required sysc programming and child modules dosn't need to even know that. OK > So, this patch is simply correct clock tree for dra7: > dpll_gmac_ck -> .... -> gmac_main_clk -> gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 0 So I guess there's no mux clock for DRA7_GMAC_GMAC_CLKCTRL 0. What I meant is maybe check also that no top level mux is needed similar to what we have for these configured with assigned-clocks: $ git grep -C3 assigned-clock arch/arm/boot/dts/dra7* > Seems MDIO dt also need to be fixed to use: > gmac_main_clk as "fck". I'll try and send patch. OK > By the way, the patch follows am3/am4 (am4_cpsw_125mhz_clkctrl_regs, > am3_cpsw_125mhz_clkctrl_regs) OK Tony
On 13/11/2019 17:23, Tony Lindgren wrote: > * Grygorii Strashko <grygorii.strashko@ti.com> [191113 10:02]: >> >> >> On 11/11/2019 19:12, Tony Lindgren wrote: >>> * Grygorii Strashko <grygorii.strashko@ti.com> [191109 14:21]: >>>> The parent clk for gmac clk ctrl has to be gmac_main_clk (125MHz) instead >>>> of dpll_gmac_ck (1GHz). This is caused incorrect CPSW MDIO operation. >>>> Hence, fix it. >>>> >>>> Fixes: commit dffa9051d546 ('clk: ti: dra7: add new clkctrl data') >>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> >>> >>> Hmm is there a mux for the source though? >> >> Not sure what do you mean here :( >> >> fck clock for CPSW and MDIO is "gmac_main_clk" which is 125MHz and >> that what need to be passed to drivers and enabled through the clock tree. >> The TI specific PM is handled by gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 0 >> which required sysc programming and child modules dosn't need to even know that. > > OK > >> So, this patch is simply correct clock tree for dra7: >> dpll_gmac_ck -> .... -> gmac_main_clk -> gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 0 > > So I guess there's no mux clock for DRA7_GMAC_GMAC_CLKCTRL 0. > > What I meant is maybe check also that no top level mux is needed > similar to what we have for these configured with assigned-clocks: > > $ git grep -C3 assigned-clock arch/arm/boot/dts/dra7* No. No muxes here. [...] -- Best regards, grygorii
* Grygorii Strashko <grygorii.strashko@ti.com> [191113 15:36]: > > > On 13/11/2019 17:23, Tony Lindgren wrote: > > * Grygorii Strashko <grygorii.strashko@ti.com> [191113 10:02]: > > > > > > > > > On 11/11/2019 19:12, Tony Lindgren wrote: > > > > * Grygorii Strashko <grygorii.strashko@ti.com> [191109 14:21]: > > > > > The parent clk for gmac clk ctrl has to be gmac_main_clk (125MHz) instead > > > > > of dpll_gmac_ck (1GHz). This is caused incorrect CPSW MDIO operation. > > > > > Hence, fix it. > > > > > > > > > > Fixes: commit dffa9051d546 ('clk: ti: dra7: add new clkctrl data') > > > > > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > > > > > > > > Hmm is there a mux for the source though? > > > > > > Not sure what do you mean here :( > > > > > > fck clock for CPSW and MDIO is "gmac_main_clk" which is 125MHz and > > > that what need to be passed to drivers and enabled through the clock tree. > > > The TI specific PM is handled by gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 0 > > > which required sysc programming and child modules dosn't need to even know that. > > > > OK > > > > > So, this patch is simply correct clock tree for dra7: > > > dpll_gmac_ck -> .... -> gmac_main_clk -> gmac_clkctrl DRA7_GMAC_GMAC_CLKCTRL 0 > > > > So I guess there's no mux clock for DRA7_GMAC_GMAC_CLKCTRL 0. > > > > What I meant is maybe check also that no top level mux is needed > > similar to what we have for these configured with assigned-clocks: > > > > $ git grep -C3 assigned-clock arch/arm/boot/dts/dra7* > > No. No muxes here. OK thanks for checking. Tony
Quoting Grygorii Strashko (2019-11-09 06:20:17) > The parent clk for gmac clk ctrl has to be gmac_main_clk (125MHz) instead > of dpll_gmac_ck (1GHz). This is caused incorrect CPSW MDIO operation. > Hence, fix it. > > Fixes: commit dffa9051d546 ('clk: ti: dra7: add new clkctrl data') There shouldn't be 'commit' in a Fixes: line.
diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c index 9dd6185a4b4e..66e4b2b9ec60 100644 --- a/drivers/clk/ti/clk-7xx.c +++ b/drivers/clk/ti/clk-7xx.c @@ -405,7 +405,7 @@ static const struct omap_clkctrl_bit_data dra7_gmac_bit_data[] __initconst = { }; static const struct omap_clkctrl_reg_data dra7_gmac_clkctrl_regs[] __initconst = { - { DRA7_GMAC_GMAC_CLKCTRL, dra7_gmac_bit_data, CLKF_SW_SUP, "dpll_gmac_ck" }, + { DRA7_GMAC_GMAC_CLKCTRL, dra7_gmac_bit_data, CLKF_SW_SUP, "gmac_main_clk" }, { 0 }, };
The parent clk for gmac clk ctrl has to be gmac_main_clk (125MHz) instead of dpll_gmac_ck (1GHz). This is caused incorrect CPSW MDIO operation. Hence, fix it. Fixes: commit dffa9051d546 ('clk: ti: dra7: add new clkctrl data') Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> --- drivers/clk/ti/clk-7xx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.17.1