When testing stmmac with my QoS reference design I checked a problem in the
CSR clock configuration that was impossibilitating the phy discovery, since
every read operation returned 0x0000ffff. This patch fixes the issue.
Signed-off-by: Joao Pinto <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index 23322fd..fda01f7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -81,8 +81,8 @@ static int stmmac_mdio_read(struct mii_bus *bus, int phyaddr, int phyreg)
value |= (phyaddr << priv->hw->mii.addr_shift)
& priv->hw->mii.addr_mask;
value |= (phyreg << priv->hw->mii.reg_shift) & priv->hw->mii.reg_mask;
- value |= (priv->clk_csr & priv->hw->mii.clk_csr_mask)
- << priv->hw->mii.clk_csr_shift;
+ value |= (priv->clk_csr << priv->hw->mii.clk_csr_shift)
+ & priv->hw->mii.clk_csr_mask;
if (priv->plat->has_gmac4)
value |= MII_GMAC4_READ;
@@ -122,8 +122,8 @@ static int stmmac_mdio_write(struct mii_bus *bus, int phyaddr, int phyreg,
& priv->hw->mii.addr_mask;
value |= (phyreg << priv->hw->mii.reg_shift) & priv->hw->mii.reg_mask;
- value |= ((priv->clk_csr & priv->hw->mii.clk_csr_mask)
- << priv->hw->mii.clk_csr_shift);
+ value |= (priv->clk_csr << priv->hw->mii.clk_csr_shift)
+ & priv->hw->mii.clk_csr_mask;
if (priv->plat->has_gmac4)
value |= MII_GMAC4_WRITE;
--
2.9.3
From: Joao Pinto <[email protected]>
Date: Tue, 20 Dec 2016 11:21:47 +0000
> When testing stmmac with my QoS reference design I checked a problem in the
> CSR clock configuration that was impossibilitating the phy discovery, since
> every read operation returned 0x0000ffff. This patch fixes the issue.
>
> Signed-off-by: Joao Pinto <[email protected]>
This isn't enough.
It looks like various parts of this driver set the mask field
differently.
dwmac1000_core.c and dwmac100_core.c set the mask to be the low bits.
But dwmac4_core.c uses GENMASK(11, 8) which means the mask is a value
which is shifted up already.
So your patch will break chips driven by dwmac4_core.c.
In order for your change to be correct you must consolidate all of
these various pieces to use the same convention.
?s 6:21 PM de 12/21/2016, David Miller escreveu:
> From: Joao Pinto <[email protected]>
> Date: Tue, 20 Dec 2016 11:21:47 +0000
>
>> When testing stmmac with my QoS reference design I checked a problem in the
>> CSR clock configuration that was impossibilitating the phy discovery, since
>> every read operation returned 0x0000ffff. This patch fixes the issue.
>>
>> Signed-off-by: Joao Pinto <[email protected]>
>
> This isn't enough.
>
> It looks like various parts of this driver set the mask field
> differently.
>
> dwmac1000_core.c and dwmac100_core.c set the mask to be the low bits.
>
> But dwmac4_core.c uses GENMASK(11, 8) which means the mask is a value
> which is shifted up already.
>
> So your patch will break chips driven by dwmac4_core.c.
I am using a GMAC4 reference design to test the patches. The clock configuration
as is, does not work, resulting in the phy discovery failure. By applying this
patch I am able to set the clock value properly.
I am going to check in the Databook of GMAC4 and older versions in order to
justify better.
>
> In order for your change to be correct you must consolidate all of
> these various pieces to use the same convention.
>
Thanks.
?s 12:23 PM de 12/22/2016, Joao Pinto escreveu:
>
> Hi David,
>
> ?s 10:15 AM de 12/22/2016, Joao Pinto escreveu:
>> ?s 6:21 PM de 12/21/2016, David Miller escreveu:
>>> From: Joao Pinto <[email protected]>
>>> Date: Tue, 20 Dec 2016 11:21:47 +0000
>>>
>>>> When testing stmmac with my QoS reference design I checked a problem in the
>>>> CSR clock configuration that was impossibilitating the phy discovery, since
>>>> every read operation returned 0x0000ffff. This patch fixes the issue.
>>>>
>>>> Signed-off-by: Joao Pinto <[email protected]>
>>>
>>> This isn't enough.
>>>
>>> It looks like various parts of this driver set the mask field
>>> differently.
>>>
>>> dwmac1000_core.c and dwmac100_core.c set the mask to be the low bits.
>>>
>>> But dwmac4_core.c uses GENMASK(11, 8) which means the mask is a value
>>> which is shifted up already.
>>>
>>> So your patch will break chips driven by dwmac4_core.c.
>>
>> I am using a GMAC4 reference design to test the patches. The clock configuration
>> as is, does not work, resulting in the phy discovery failure. By applying this
>> patch I am able to set the clock value properly.
>>
>> I am going to check in the Databook of GMAC4 and older versions in order to
>> justify better.
>
> So from the 4.20 Databook:
>
> Bits
> 11:8 CR parameter
> 0000: CSR clock = 60-100 MHz; MDC clock = CSR
> 0001: CSR clock = 100-150 MHz; MDC clock = CSR
> 0010: CSR clock = 20-35 MHz; MDC clock = CSR
> 0011: CSR clock = 35-60 MHz; MDC clock = CSR
> 0100: CSR clock = 150-250 MHz; MDC clock = CSR
> 0101: CSR clock = 250-300 MHz; MDC clock = CSR
> 0110, 0111: Reserved
>
> For example, if you want configure the CSR clock to 250-300MHZ (my case), you
> will set the parameter CR to 0x5. The current mechanism simply messes the value.
> With this fix, the 0x5 is shifted to 11:8 like the databook specifies and
> applies a GENMASK(11:8) on top, causing the reset of every bit outside the 11:8
> which is an assurance.
>
> For older cores like 4.10 and 4.00 the logic is the same. The information was
> confirmed by R&D.
>
> Thanks.
Bu checking DWMAC100 and DWMAC1000 I understand your concern now. I am going to
change their masks also in order to put it right. V2 comming soon.
Thanks.
>
>>
>>>
>>> In order for your change to be correct you must consolidate all of
>>> these various pieces to use the same convention.
>>>
>>
>> Thanks.
>>
>
Hi David,
?s 10:15 AM de 12/22/2016, Joao Pinto escreveu:
> ?s 6:21 PM de 12/21/2016, David Miller escreveu:
>> From: Joao Pinto <[email protected]>
>> Date: Tue, 20 Dec 2016 11:21:47 +0000
>>
>>> When testing stmmac with my QoS reference design I checked a problem in the
>>> CSR clock configuration that was impossibilitating the phy discovery, since
>>> every read operation returned 0x0000ffff. This patch fixes the issue.
>>>
>>> Signed-off-by: Joao Pinto <[email protected]>
>>
>> This isn't enough.
>>
>> It looks like various parts of this driver set the mask field
>> differently.
>>
>> dwmac1000_core.c and dwmac100_core.c set the mask to be the low bits.
>>
>> But dwmac4_core.c uses GENMASK(11, 8) which means the mask is a value
>> which is shifted up already.
>>
>> So your patch will break chips driven by dwmac4_core.c.
>
> I am using a GMAC4 reference design to test the patches. The clock configuration
> as is, does not work, resulting in the phy discovery failure. By applying this
> patch I am able to set the clock value properly.
>
> I am going to check in the Databook of GMAC4 and older versions in order to
> justify better.
So from the 4.20 Databook:
Bits
11:8 CR parameter
0000: CSR clock = 60-100 MHz; MDC clock = CSR
0001: CSR clock = 100-150 MHz; MDC clock = CSR
0010: CSR clock = 20-35 MHz; MDC clock = CSR
0011: CSR clock = 35-60 MHz; MDC clock = CSR
0100: CSR clock = 150-250 MHz; MDC clock = CSR
0101: CSR clock = 250-300 MHz; MDC clock = CSR
0110, 0111: Reserved
For example, if you want configure the CSR clock to 250-300MHZ (my case), you
will set the parameter CR to 0x5. The current mechanism simply messes the value.
With this fix, the 0x5 is shifted to 11:8 like the databook specifies and
applies a GENMASK(11:8) on top, causing the reset of every bit outside the 11:8
which is an assurance.
For older cores like 4.10 and 4.00 the logic is the same. The information was
confirmed by R&D.
Thanks.
>
>>
>> In order for your change to be correct you must consolidate all of
>> these various pieces to use the same convention.
>>
>
> Thanks.
>