* [PATCH v2] stmmac: CSR clock configuration fix
@ 2016-12-22 12:38 Joao Pinto
2016-12-22 15:42 ` Phil Reid
2016-12-22 16:21 ` David Miller
0 siblings, 2 replies; 8+ messages in thread
From: Joao Pinto @ 2016-12-22 12:38 UTC (permalink / raw)
To: peppe.cavallaro, davem, seraphin.bonnaffe
Cc: hock.leong.kweh, niklas.cassel, pavel, linux-kernel, netdev,
Joao Pinto
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 <jpinto@synopsys.com>
---
changes v1->v2 (David Miller)
- DWMAC100 and DWMAC1000 csr clocks masks should also be fixed for the patch
to make sense
drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 8 ++++----
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index b21d03f..94223c8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -539,7 +539,7 @@ struct mac_device_info *dwmac1000_setup(void __iomem *ioaddr, int mcbins,
mac->mii.reg_shift = 6;
mac->mii.reg_mask = 0x000007C0;
mac->mii.clk_csr_shift = 2;
- mac->mii.clk_csr_mask = 0xF;
+ mac->mii.clk_csr_mask = GENMASK(4, 2);
/* Get and dump the chip ID */
*synopsys_id = stmmac_get_synopsys_id(hwid);
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
index a1d582f..8a40e69 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
@@ -197,7 +197,7 @@ struct mac_device_info *dwmac100_setup(void __iomem *ioaddr, int *synopsys_id)
mac->mii.reg_shift = 6;
mac->mii.reg_mask = 0x000007C0;
mac->mii.clk_csr_shift = 2;
- mac->mii.clk_csr_mask = 0xF;
+ mac->mii.clk_csr_mask = GENMASK(4, 2);
/* Synopsys Id is not available on old chips */
*synopsys_id = 0;
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] stmmac: CSR clock configuration fix
2016-12-22 12:38 [PATCH v2] stmmac: CSR clock configuration fix Joao Pinto
@ 2016-12-22 15:42 ` Phil Reid
2016-12-22 15:47 ` Joao Pinto
2016-12-22 16:21 ` David Miller
1 sibling, 1 reply; 8+ messages in thread
From: Phil Reid @ 2016-12-22 15:42 UTC (permalink / raw)
To: Joao Pinto, peppe.cavallaro, davem, seraphin.bonnaffe
Cc: hock.leong.kweh, niklas.cassel, pavel, linux-kernel, netdev
G'day Joao,
On 22/12/2016 20:38, Joao Pinto wrote:
> 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 <jpinto@synopsys.com>
> ---
> changes v1->v2 (David Miller)
> - DWMAC100 and DWMAC1000 csr clocks masks should also be fixed for the patch
> to make sense
>
> drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 2 +-
> drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c | 2 +-
> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 8 ++++----
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> index b21d03f..94223c8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> @@ -539,7 +539,7 @@ struct mac_device_info *dwmac1000_setup(void __iomem *ioaddr, int mcbins,
> mac->mii.reg_shift = 6;
> mac->mii.reg_mask = 0x000007C0;
> mac->mii.clk_csr_shift = 2;
> - mac->mii.clk_csr_mask = 0xF;
> + mac->mii.clk_csr_mask = GENMASK(4, 2);
Should this not be GENMASK(5,2)
>
> /* Get and dump the chip ID */
> *synopsys_id = stmmac_get_synopsys_id(hwid);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
> index a1d582f..8a40e69 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
> @@ -197,7 +197,7 @@ struct mac_device_info *dwmac100_setup(void __iomem *ioaddr, int *synopsys_id)
> mac->mii.reg_shift = 6;
> mac->mii.reg_mask = 0x000007C0;
> mac->mii.clk_csr_shift = 2;
> - mac->mii.clk_csr_mask = 0xF;
> + mac->mii.clk_csr_mask = GENMASK(4, 2);
same as above?
>
> /* Synopsys Id is not available on old chips */
> *synopsys_id = 0;
> 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;
>
>
--
Regards
Phil Reid
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] stmmac: CSR clock configuration fix
2016-12-22 15:42 ` Phil Reid
@ 2016-12-22 15:47 ` Joao Pinto
2016-12-22 16:57 ` Phil Reid
0 siblings, 1 reply; 8+ messages in thread
From: Joao Pinto @ 2016-12-22 15:47 UTC (permalink / raw)
To: Phil Reid, Joao Pinto, peppe.cavallaro, davem, seraphin.bonnaffe
Cc: hock.leong.kweh, niklas.cassel, pavel, linux-kernel, netdev
Hello Phil,
Às 3:42 PM de 12/22/2016, Phil Reid escreveu:
> G'day Joao,
>
> On 22/12/2016 20:38, Joao Pinto wrote:
>> 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 <jpinto@synopsys.com>
>> ---
>> changes v1->v2 (David Miller)
>> - DWMAC100 and DWMAC1000 csr clocks masks should also be fixed for the patch
>> to make sense
>>
>> drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 2 +-
>> drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c | 2 +-
>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 8 ++++----
>> 3 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>> b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>> index b21d03f..94223c8 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>> @@ -539,7 +539,7 @@ struct mac_device_info *dwmac1000_setup(void __iomem
>> *ioaddr, int mcbins,
>> mac->mii.reg_shift = 6;
>> mac->mii.reg_mask = 0x000007C0;
>> mac->mii.clk_csr_shift = 2;
>> - mac->mii.clk_csr_mask = 0xF;
>> + mac->mii.clk_csr_mask = GENMASK(4, 2);
>
> Should this not be GENMASK(5,2)
According to Universal MAC databook (valid for MAC100 and MAC1000), we have:
Bits: 4:2
000 60-100 MHz clk_csr_i/42
001 100-150 MHz clk_csr_i/62
010 20-35 MHz clk_csr_i/16
011 35-60 MHz clk_csr_i/26
100 150-250 MHz clk_csr_i/102
101 250-300 MHz clk_csr_i/124
110, 111 Reserved
So only bits 2, 3 and 4 should be masked.
Thanks.
>
>>
>> /* Get and dump the chip ID */
>> *synopsys_id = stmmac_get_synopsys_id(hwid);
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
>> b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
>> index a1d582f..8a40e69 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
>> @@ -197,7 +197,7 @@ struct mac_device_info *dwmac100_setup(void __iomem
>> *ioaddr, int *synopsys_id)
>> mac->mii.reg_shift = 6;
>> mac->mii.reg_mask = 0x000007C0;
>> mac->mii.clk_csr_shift = 2;
>> - mac->mii.clk_csr_mask = 0xF;
>> + mac->mii.clk_csr_mask = GENMASK(4, 2);
> same as above?
>
>>
>> /* Synopsys Id is not available on old chips */
>> *synopsys_id = 0;
>> 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;
>>
>>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] stmmac: CSR clock configuration fix
2016-12-22 15:47 ` Joao Pinto
@ 2016-12-22 16:57 ` Phil Reid
2016-12-22 17:06 ` Joao Pinto
0 siblings, 1 reply; 8+ messages in thread
From: Phil Reid @ 2016-12-22 16:57 UTC (permalink / raw)
To: Joao Pinto, peppe.cavallaro, davem, seraphin.bonnaffe
Cc: hock.leong.kweh, niklas.cassel, pavel, linux-kernel, netdev
On 22/12/2016 23:47, Joao Pinto wrote:
>
> Hello Phil,
>
> Às 3:42 PM de 12/22/2016, Phil Reid escreveu:
>> G'day Joao,
>>
>> On 22/12/2016 20:38, Joao Pinto wrote:
>>> 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 <jpinto@synopsys.com>
>>> ---
>>> changes v1->v2 (David Miller)
>>> - DWMAC100 and DWMAC1000 csr clocks masks should also be fixed for the patch
>>> to make sense
>>>
>>> drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 2 +-
>>> drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c | 2 +-
>>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 8 ++++----
>>> 3 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>> index b21d03f..94223c8 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>> @@ -539,7 +539,7 @@ struct mac_device_info *dwmac1000_setup(void __iomem
>>> *ioaddr, int mcbins,
>>> mac->mii.reg_shift = 6;
>>> mac->mii.reg_mask = 0x000007C0;
>>> mac->mii.clk_csr_shift = 2;
>>> - mac->mii.clk_csr_mask = 0xF;
>>> + mac->mii.clk_csr_mask = GENMASK(4, 2);
>>
>> Should this not be GENMASK(5,2)
>
> According to Universal MAC databook (valid for MAC100 and MAC1000), we have:
>
> Bits: 4:2
> 000 60-100 MHz clk_csr_i/42
> 001 100-150 MHz clk_csr_i/62
> 010 20-35 MHz clk_csr_i/16
> 011 35-60 MHz clk_csr_i/26
> 100 150-250 MHz clk_csr_i/102
> 101 250-300 MHz clk_csr_i/124
> 110, 111 Reserved
>
> So only bits 2, 3 and 4 should be masked.
>
> Thanks.
>
But this is a change in behaviour from what was there isn't.
Previous mask was 4 bits. now it's 3.
And for example the altera socfgpa implementation in the cyclone V has valid values
for values 0x8-0xf, using bit 5:2.
--
Regards
Phil Reid
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] stmmac: CSR clock configuration fix
2016-12-22 16:57 ` Phil Reid
@ 2016-12-22 17:06 ` Joao Pinto
2016-12-23 1:09 ` Phil Reid
0 siblings, 1 reply; 8+ messages in thread
From: Joao Pinto @ 2016-12-22 17:06 UTC (permalink / raw)
To: Phil Reid, Joao Pinto, peppe.cavallaro, davem, seraphin.bonnaffe
Cc: hock.leong.kweh, niklas.cassel, pavel, linux-kernel, netdev
Às 4:57 PM de 12/22/2016, Phil Reid escreveu:
> On 22/12/2016 23:47, Joao Pinto wrote:
>>
>> Hello Phil,
>>
>> Às 3:42 PM de 12/22/2016, Phil Reid escreveu:
>>> G'day Joao,
>>>
>>> On 22/12/2016 20:38, Joao Pinto wrote:
>>>> 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 <jpinto@synopsys.com>
>>>> ---
>>>> changes v1->v2 (David Miller)
>>>> - DWMAC100 and DWMAC1000 csr clocks masks should also be fixed for the patch
>>>> to make sense
>>>>
>>>> drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 2 +-
>>>> drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c | 2 +-
>>>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 8 ++++----
>>>> 3 files changed, 6 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>>> index b21d03f..94223c8 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>>> @@ -539,7 +539,7 @@ struct mac_device_info *dwmac1000_setup(void __iomem
>>>> *ioaddr, int mcbins,
>>>> mac->mii.reg_shift = 6;
>>>> mac->mii.reg_mask = 0x000007C0;
>>>> mac->mii.clk_csr_shift = 2;
>>>> - mac->mii.clk_csr_mask = 0xF;
>>>> + mac->mii.clk_csr_mask = GENMASK(4, 2);
>>>
>>> Should this not be GENMASK(5,2)
>>
>> According to Universal MAC databook (valid for MAC100 and MAC1000), we have:
>>
>> Bits: 4:2
>> 000 60-100 MHz clk_csr_i/42
>> 001 100-150 MHz clk_csr_i/62
>> 010 20-35 MHz clk_csr_i/16
>> 011 35-60 MHz clk_csr_i/26
>> 100 150-250 MHz clk_csr_i/102
>> 101 250-300 MHz clk_csr_i/124
>> 110, 111 Reserved
>>
>> So only bits 2, 3 and 4 should be masked.
>>
>> Thanks.
>>
> But this is a change in behaviour from what was there isn't.
> Previous mask was 4 bits. now it's 3.
>
> And for example the altera socfgpa implementation in the cyclone V has valid values
> for values 0x8-0xf, using bit 5:2.
According to the databook, bit 5 is reserved and RO. When reserved, it is
possible to customize. Is that the case? If there is hardware using the 5th bit
we can put it GENMASK(5, 2) with no problem.
>
>
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] stmmac: CSR clock configuration fix
2016-12-22 17:06 ` Joao Pinto
@ 2016-12-23 1:09 ` Phil Reid
2016-12-23 10:09 ` Joao Pinto
0 siblings, 1 reply; 8+ messages in thread
From: Phil Reid @ 2016-12-23 1:09 UTC (permalink / raw)
To: Joao Pinto, peppe.cavallaro, davem, seraphin.bonnaffe
Cc: hock.leong.kweh, niklas.cassel, pavel, linux-kernel, netdev
G'day Joao,
On 23/12/2016 01:06, Joao Pinto wrote:
> Às 4:57 PM de 12/22/2016, Phil Reid escreveu:
>> On 22/12/2016 23:47, Joao Pinto wrote:
>>>
>>> Hello Phil,
>>>
>>> Às 3:42 PM de 12/22/2016, Phil Reid escreveu:
>>>> G'day Joao,
>>>>
>>>> On 22/12/2016 20:38, Joao Pinto wrote:
>>>>> 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 <jpinto@synopsys.com>
>>>>> ---
>>>>> changes v1->v2 (David Miller)
>>>>> - DWMAC100 and DWMAC1000 csr clocks masks should also be fixed for the patch
>>>>> to make sense
>>>>>
>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 2 +-
>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c | 2 +-
>>>>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 8 ++++----
>>>>> 3 files changed, 6 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>>>> index b21d03f..94223c8 100644
>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>>>> @@ -539,7 +539,7 @@ struct mac_device_info *dwmac1000_setup(void __iomem
>>>>> *ioaddr, int mcbins,
>>>>> mac->mii.reg_shift = 6;
>>>>> mac->mii.reg_mask = 0x000007C0;
>>>>> mac->mii.clk_csr_shift = 2;
>>>>> - mac->mii.clk_csr_mask = 0xF;
>>>>> + mac->mii.clk_csr_mask = GENMASK(4, 2);
>>>>
>>>> Should this not be GENMASK(5,2)
>>>
>>> According to Universal MAC databook (valid for MAC100 and MAC1000), we have:
>>>
>>> Bits: 4:2
>>> 000 60-100 MHz clk_csr_i/42
>>> 001 100-150 MHz clk_csr_i/62
>>> 010 20-35 MHz clk_csr_i/16
>>> 011 35-60 MHz clk_csr_i/26
>>> 100 150-250 MHz clk_csr_i/102
>>> 101 250-300 MHz clk_csr_i/124
>>> 110, 111 Reserved
>>>
>>> So only bits 2, 3 and 4 should be masked.
>>>
>>> Thanks.
>>>
>> But this is a change in behaviour from what was there isn't.
>> Previous mask was 4 bits. now it's 3.
>>
>> And for example the altera socfgpa implementation in the cyclone V has valid values
>> for values 0x8-0xf, using bit 5:2.
>
> According to the databook, bit 5 is reserved and RO. When reserved, it is
> possible to customize. Is that the case? If there is hardware using the 5th bit
> we can put it GENMASK(5, 2) with no problem.
>
I've also checked the Aria 10 documentation and bit 5 is also RW.
The following options are documented and supported
1000: CSR clock/4
1001: CSR clock/6
1010: CSR clock/8
1011: CSR clock/10
1100: CSR clock/12
1101: CSR clock/14
1110: CSR clock/16
1111: CSR clock/18
They do mention that these values will probably be outside the IEEE 802.3 specified range.
But there's at least a couple of cores out there where GENMASK(5,2) is valid.
Can't say if anyone is using that setting thou.
--
Regards
Phil Reid
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] stmmac: CSR clock configuration fix
2016-12-23 1:09 ` Phil Reid
@ 2016-12-23 10:09 ` Joao Pinto
0 siblings, 0 replies; 8+ messages in thread
From: Joao Pinto @ 2016-12-23 10:09 UTC (permalink / raw)
To: Phil Reid, Joao Pinto, peppe.cavallaro, davem, seraphin.bonnaffe
Cc: hock.leong.kweh, niklas.cassel, pavel, linux-kernel, netdev
Hello Phil,
Às 1:09 AM de 12/23/2016, Phil Reid escreveu:
> G'day Joao,
> On 23/12/2016 01:06, Joao Pinto wrote:
>> Às 4:57 PM de 12/22/2016, Phil Reid escreveu:
>>> On 22/12/2016 23:47, Joao Pinto wrote:
>>>>
>>>> Hello Phil,
>>>>
>>>> Às 3:42 PM de 12/22/2016, Phil Reid escreveu:
>>>>> G'day Joao,
>>>>>
>>>>> On 22/12/2016 20:38, Joao Pinto wrote:
>>>>>> 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 <jpinto@synopsys.com>
>>>>>> ---
>>>>>> changes v1->v2 (David Miller)
>>>>>> - DWMAC100 and DWMAC1000 csr clocks masks should also be fixed for the patch
>>>>>> to make sense
>>>>>>
>>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 2 +-
>>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c | 2 +-
>>>>>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 8 ++++----
>>>>>> 3 files changed, 6 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>>>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>>>>> index b21d03f..94223c8 100644
>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>>>>> @@ -539,7 +539,7 @@ struct mac_device_info *dwmac1000_setup(void __iomem
>>>>>> *ioaddr, int mcbins,
>>>>>> mac->mii.reg_shift = 6;
>>>>>> mac->mii.reg_mask = 0x000007C0;
>>>>>> mac->mii.clk_csr_shift = 2;
>>>>>> - mac->mii.clk_csr_mask = 0xF;
>>>>>> + mac->mii.clk_csr_mask = GENMASK(4, 2);
>>>>>
>>>>> Should this not be GENMASK(5,2)
>>>>
>>>> According to Universal MAC databook (valid for MAC100 and MAC1000), we have:
>>>>
>>>> Bits: 4:2
>>>> 000 60-100 MHz clk_csr_i/42
>>>> 001 100-150 MHz clk_csr_i/62
>>>> 010 20-35 MHz clk_csr_i/16
>>>> 011 35-60 MHz clk_csr_i/26
>>>> 100 150-250 MHz clk_csr_i/102
>>>> 101 250-300 MHz clk_csr_i/124
>>>> 110, 111 Reserved
>>>>
>>>> So only bits 2, 3 and 4 should be masked.
>>>>
>>>> Thanks.
>>>>
>>> But this is a change in behaviour from what was there isn't.
>>> Previous mask was 4 bits. now it's 3.
>>>
>>> And for example the altera socfgpa implementation in the cyclone V has valid
>>> values
>>> for values 0x8-0xf, using bit 5:2.
>>
>> According to the databook, bit 5 is reserved and RO. When reserved, it is
>> possible to customize. Is that the case? If there is hardware using the 5th bit
>> we can put it GENMASK(5, 2) with no problem.
>>
> I've also checked the Aria 10 documentation and bit 5 is also RW.
> The following options are documented and supported
> 1000: CSR clock/4
> 1001: CSR clock/6
> 1010: CSR clock/8
> 1011: CSR clock/10
> 1100: CSR clock/12
> 1101: CSR clock/14
> 1110: CSR clock/16
> 1111: CSR clock/18
> They do mention that these values will probably be outside the IEEE 802.3
> specified range.
>
> But there's at least a couple of cores out there where GENMASK(5,2) is valid.
> Can't say if anyone is using that setting thou.
Thanks for checking it! Ok, it seems like they are using the reserved bit 5. No
problem, I am going to change the patch and put the mask from 2 to 5. Thanks for
your help!
Joao
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] stmmac: CSR clock configuration fix
2016-12-22 12:38 [PATCH v2] stmmac: CSR clock configuration fix Joao Pinto
2016-12-22 15:42 ` Phil Reid
@ 2016-12-22 16:21 ` David Miller
1 sibling, 0 replies; 8+ messages in thread
From: David Miller @ 2016-12-22 16:21 UTC (permalink / raw)
To: Joao.Pinto
Cc: peppe.cavallaro, seraphin.bonnaffe, hock.leong.kweh,
niklas.cassel, pavel, linux-kernel, netdev
From: Joao Pinto <Joao.Pinto@synopsys.com>
Date: Thu, 22 Dec 2016 12:38:00 +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 <jpinto@synopsys.com>
Applied.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-12-23 10:09 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-22 12:38 [PATCH v2] stmmac: CSR clock configuration fix Joao Pinto
2016-12-22 15:42 ` Phil Reid
2016-12-22 15:47 ` Joao Pinto
2016-12-22 16:57 ` Phil Reid
2016-12-22 17:06 ` Joao Pinto
2016-12-23 1:09 ` Phil Reid
2016-12-23 10:09 ` Joao Pinto
2016-12-22 16:21 ` David Miller
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).