[v2] clk: ti: dra7: fix parent for gmac_clkctrl
diff mbox series

Message ID 20191221110004.9951-1-grygorii.strashko@ti.com
State New
Headers show
Series
  • [v2] clk: ti: dra7: fix parent for gmac_clkctrl
Related show

Commit Message

Grygorii Strashko Dec. 21, 2019, 11 a.m. UTC
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(-)

Comments

Tero Kristo Jan. 17, 2020, 2:05 p.m. UTC | #1
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
Grygorii Strashko Jan. 17, 2020, 2:49 p.m. UTC | #2
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
Tero Kristo Jan. 17, 2020, 3:47 p.m. UTC | #3
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
Tero Kristo Jan. 21, 2020, 7:34 a.m. UTC | #4
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

Patch
diff mbox series

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 },
 };