| Message ID | 20191221110004.9951-1-grygorii.strashko@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] clk: ti: dra7: fix parent for gmac_clkctrl | expand |
On 21/12/2019 13:00, Grygorii Strashko wrote: > 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: 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(-) > > 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" }, I think the gmac clk path is still somehow wrong after this change. This only fixes it partially imo. Looking at TRM, gmac_main_clk is fed from dpll_gmac_x2_h12, but looking at the existing clock data, gmac_main_clk comes out from dpll_gmac_m2_ck. This potentially applies one extra divider to the path which appears wrong. Can you take a look at fixing the DTS side for this also? -Tero -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 17/01/2020 16:05, Tero Kristo wrote: > On 21/12/2019 13:00, Grygorii Strashko wrote: >> 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: 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(-) >> >> 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" }, > > I think the gmac clk path is still somehow wrong after this change. This only fixes it partially imo. > > Looking at TRM, gmac_main_clk is fed from dpll_gmac_x2_h12, No, it seems not. DPLL_GMAC.CLKOUT_M2 -> GMAC_250M_CLK -> 1/2 -> GMAC_MAIN_CLK http://www.ti.com/lit/ug/sprui30f/sprui30f.pdf 3.6.3.12.1 DPLL_GMAC Overview Figure 3-41. CM_CORE_AON Overview (b) Figure 24-185. GMAC_SW Integration but looking at the existing clock data, gmac_main_clk comes out from dpll_gmac_m2_ck. This potentially applies one extra divider to the path which appears wrong. Can you take a look at fixing the DTS side for this also? > > -Tero > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 17/01/2020 16:49, Grygorii Strashko wrote: > > > On 17/01/2020 16:05, Tero Kristo wrote: >> On 21/12/2019 13:00, Grygorii Strashko wrote: >>> 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: 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(-) >>> >>> 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" }, >> >> I think the gmac clk path is still somehow wrong after this change. >> This only fixes it partially imo. >> >> Looking at TRM, gmac_main_clk is fed from dpll_gmac_x2_h12, > > No, it seems not. > DPLL_GMAC.CLKOUT_M2 -> GMAC_250M_CLK -> 1/2 -> GMAC_MAIN_CLK Hmm ok, yeah looking at the GMAC IP details shows how it actually goes, I got confused with the clkdm mapping in 3.6.3.12.1. So no DT changes needed after all and this patch alone is fine. -Tero > > http://www.ti.com/lit/ug/sprui30f/sprui30f.pdf > 3.6.3.12.1 DPLL_GMAC Overview > Figure 3-41. CM_CORE_AON Overview (b) > Figure 24-185. GMAC_SW Integration > > but looking at the existing clock data, gmac_main_clk comes out from > dpll_gmac_m2_ck. This potentially applies one extra divider to the path > which appears wrong. Can you take a look at fixing the DTS side for this > also? >> >> -Tero >> -- > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 21/12/2019 13:00, Grygorii Strashko wrote: > 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: dffa9051d546 ('clk: ti: dra7: add new clkctrl data') > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> Queued up for 5.6, thanks. -Tero > --- > 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 }, > }; > > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
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: 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(-)