* [PATCH v2 net-next] ravb: Avoid unsupported internal delay mode for R-Car E3/D3
@ 2019-04-10 11:45 Simon Horman
2019-04-10 12:17 ` Wolfram Sang
2019-04-10 16:19 ` Andrew Lunn
0 siblings, 2 replies; 11+ messages in thread
From: Simon Horman @ 2019-04-10 11:45 UTC (permalink / raw)
To: David Miller, Sergei Shtylyov
Cc: Magnus Damm, netdev, linux-renesas-soc, Yoshihiro Shimoda,
Wolfram Sang, Simon Horman
According to the R-Car Gen3 Hardware Manual Rev 1.50 of Nov 30, 2018, the
TX clock internal delay mode isn't supported on R-Car E3 (r8a77990) or D3
(r8a77995). And by extension it is also not supported by RZ/G2E (r9a774c0).
This matches all ES versions of the affected SoCs as it is
not clear if this problem will be resolved in newer chips.
This can be revisited, as necessary.
This patch does not error-out if PHY_INTERFACE_MODE_RGMII_ID or
PHY_INTERFACE_MODE_RGMII_TXID are used on SoCs where TX clock delay
mode is not supported as there is a risk of introducing a regression
when used in conjunction with older DT blobs present in the field.
Based on work by Kazuya Mizuguchi.
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
v2
* Dropped RFC designation
* Also correct RZ/G2E (r8a774c0)
* Remove revision (ES version) portion of soc match
as it is not clear this problem will be resolved in newer chips.
* Refer to user manual v1.50 rather than errata for v1.00 in changelog
* Note absence of error handling in Changelog
---
drivers/net/ethernet/renesas/ravb_main.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 4f648394e645..99a12bf06d35 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1969,6 +1969,13 @@ static void ravb_set_config_mode(struct net_device *ndev)
}
}
+static const struct soc_device_attribute ravb_delay_mode_quirk_match[] = {
+ { .soc_id = "r8a774c0" },
+ { .soc_id = "r8a77990" },
+ { .soc_id = "r8a77995" },
+ { /* sentinel */ }
+};
+
/* Set tx and rx clock internal delay modes */
static void ravb_set_delay_mode(struct net_device *ndev)
{
@@ -1979,8 +1986,9 @@ static void ravb_set_delay_mode(struct net_device *ndev)
priv->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID)
set |= APSR_DM_RDM;
- if (priv->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
- priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID)
+ if ((priv->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
+ priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) &&
+ !soc_device_match(ravb_delay_mode_quirk_match))
set |= APSR_DM_TDM;
ravb_modify(ndev, APSR, APSR_DM, set);
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 net-next] ravb: Avoid unsupported internal delay mode for R-Car E3/D3
2019-04-10 11:45 [PATCH v2 net-next] ravb: Avoid unsupported internal delay mode for R-Car E3/D3 Simon Horman
@ 2019-04-10 12:17 ` Wolfram Sang
2019-04-10 16:19 ` Andrew Lunn
1 sibling, 0 replies; 11+ messages in thread
From: Wolfram Sang @ 2019-04-10 12:17 UTC (permalink / raw)
To: Simon Horman
Cc: David Miller, Sergei Shtylyov, Magnus Damm, netdev,
linux-renesas-soc, Yoshihiro Shimoda, Wolfram Sang
[-- Attachment #1: Type: text/plain, Size: 920 bytes --]
On Wed, Apr 10, 2019 at 01:45:02PM +0200, Simon Horman wrote:
> According to the R-Car Gen3 Hardware Manual Rev 1.50 of Nov 30, 2018, the
> TX clock internal delay mode isn't supported on R-Car E3 (r8a77990) or D3
> (r8a77995). And by extension it is also not supported by RZ/G2E (r9a774c0).
>
> This matches all ES versions of the affected SoCs as it is
> not clear if this problem will be resolved in newer chips.
> This can be revisited, as necessary.
>
> This patch does not error-out if PHY_INTERFACE_MODE_RGMII_ID or
> PHY_INTERFACE_MODE_RGMII_TXID are used on SoCs where TX clock delay
> mode is not supported as there is a risk of introducing a regression
> when used in conjunction with older DT blobs present in the field.
>
> Based on work by Kazuya Mizuguchi.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 net-next] ravb: Avoid unsupported internal delay mode for R-Car E3/D3
2019-04-10 11:45 [PATCH v2 net-next] ravb: Avoid unsupported internal delay mode for R-Car E3/D3 Simon Horman
2019-04-10 12:17 ` Wolfram Sang
@ 2019-04-10 16:19 ` Andrew Lunn
2019-04-10 17:17 ` Sergei Shtylyov
1 sibling, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2019-04-10 16:19 UTC (permalink / raw)
To: Simon Horman
Cc: David Miller, Sergei Shtylyov, Magnus Damm, netdev,
linux-renesas-soc, Yoshihiro Shimoda, Wolfram Sang
On Wed, Apr 10, 2019 at 01:45:02PM +0200, Simon Horman wrote:
> According to the R-Car Gen3 Hardware Manual Rev 1.50 of Nov 30, 2018, the
> TX clock internal delay mode isn't supported on R-Car E3 (r8a77990) or D3
> (r8a77995). And by extension it is also not supported by RZ/G2E (r9a774c0).
>
> This matches all ES versions of the affected SoCs as it is
> not clear if this problem will be resolved in newer chips.
> This can be revisited, as necessary.
>
> This patch does not error-out if PHY_INTERFACE_MODE_RGMII_ID or
> PHY_INTERFACE_MODE_RGMII_TXID are used on SoCs where TX clock delay
> mode is not supported as there is a risk of introducing a regression
> when used in conjunction with older DT blobs present in the field.
Hi Simon
I think it should at least WARN_ON(). Such blobs are broken, even if
they do kind of work.
Andrew
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 net-next] ravb: Avoid unsupported internal delay mode for R-Car E3/D3
2019-04-10 16:19 ` Andrew Lunn
@ 2019-04-10 17:17 ` Sergei Shtylyov
2019-04-11 8:40 ` Simon Horman
0 siblings, 1 reply; 11+ messages in thread
From: Sergei Shtylyov @ 2019-04-10 17:17 UTC (permalink / raw)
To: Andrew Lunn, Simon Horman
Cc: David Miller, Magnus Damm, netdev, linux-renesas-soc,
Yoshihiro Shimoda, Wolfram Sang
On 04/10/2019 07:19 PM, Andrew Lunn wrote:
>> According to the R-Car Gen3 Hardware Manual Rev 1.50 of Nov 30, 2018, the
>> TX clock internal delay mode isn't supported on R-Car E3 (r8a77990) or D3
>> (r8a77995). And by extension it is also not supported by RZ/G2E (r9a774c0).
>>
>> This matches all ES versions of the affected SoCs as it is
>> not clear if this problem will be resolved in newer chips.
>> This can be revisited, as necessary.
>>
>> This patch does not error-out if PHY_INTERFACE_MODE_RGMII_ID or
>> PHY_INTERFACE_MODE_RGMII_TXID are used on SoCs where TX clock delay
>> mode is not supported as there is a risk of introducing a regression
>> when used in conjunction with older DT blobs present in the field.
>
> Hi Simon
>
> I think it should at least WARN_ON(). Such blobs are broken, even if
> they do kind of work.
Good idea! Simon, third time's a charm? :-)
> Andrew
MBR, Sergei
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 net-next] ravb: Avoid unsupported internal delay mode for R-Car E3/D3
2019-04-10 17:17 ` Sergei Shtylyov
@ 2019-04-11 8:40 ` Simon Horman
2019-04-11 9:20 ` Simon Horman
0 siblings, 1 reply; 11+ messages in thread
From: Simon Horman @ 2019-04-11 8:40 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Andrew Lunn, David Miller, Magnus Damm, netdev, linux-renesas-soc,
Yoshihiro Shimoda, Wolfram Sang
On Wed, Apr 10, 2019 at 08:17:16PM +0300, Sergei Shtylyov wrote:
> On 04/10/2019 07:19 PM, Andrew Lunn wrote:
>
> >> According to the R-Car Gen3 Hardware Manual Rev 1.50 of Nov 30, 2018, the
> >> TX clock internal delay mode isn't supported on R-Car E3 (r8a77990) or D3
> >> (r8a77995). And by extension it is also not supported by RZ/G2E (r9a774c0).
> >>
> >> This matches all ES versions of the affected SoCs as it is
> >> not clear if this problem will be resolved in newer chips.
> >> This can be revisited, as necessary.
> >>
> >> This patch does not error-out if PHY_INTERFACE_MODE_RGMII_ID or
> >> PHY_INTERFACE_MODE_RGMII_TXID are used on SoCs where TX clock delay
> >> mode is not supported as there is a risk of introducing a regression
> >> when used in conjunction with older DT blobs present in the field.
> >
> > Hi Simon
> >
> > I think it should at least WARN_ON(). Such blobs are broken, even if
> > they do kind of work.
>
> Good idea! Simon, third time's a charm? :-)
Sure, can do.
>
> > Andrew
>
> MBR, Sergei
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 net-next] ravb: Avoid unsupported internal delay mode for R-Car E3/D3
2019-04-11 8:40 ` Simon Horman
@ 2019-04-11 9:20 ` Simon Horman
2019-04-11 13:33 ` Andrew Lunn
0 siblings, 1 reply; 11+ messages in thread
From: Simon Horman @ 2019-04-11 9:20 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Andrew Lunn, David Miller, Magnus Damm, netdev, linux-renesas-soc,
Yoshihiro Shimoda, Wolfram Sang
On Thu, Apr 11, 2019 at 10:40:27AM +0200, Simon Horman wrote:
> On Wed, Apr 10, 2019 at 08:17:16PM +0300, Sergei Shtylyov wrote:
> > On 04/10/2019 07:19 PM, Andrew Lunn wrote:
> >
> > >> According to the R-Car Gen3 Hardware Manual Rev 1.50 of Nov 30, 2018, the
> > >> TX clock internal delay mode isn't supported on R-Car E3 (r8a77990) or D3
> > >> (r8a77995). And by extension it is also not supported by RZ/G2E (r9a774c0).
> > >>
> > >> This matches all ES versions of the affected SoCs as it is
> > >> not clear if this problem will be resolved in newer chips.
> > >> This can be revisited, as necessary.
> > >>
> > >> This patch does not error-out if PHY_INTERFACE_MODE_RGMII_ID or
> > >> PHY_INTERFACE_MODE_RGMII_TXID are used on SoCs where TX clock delay
> > >> mode is not supported as there is a risk of introducing a regression
> > >> when used in conjunction with older DT blobs present in the field.
> > >
> > > Hi Simon
> > >
> > > I think it should at least WARN_ON(). Such blobs are broken, even if
> > > they do kind of work.
> >
> > Good idea! Simon, third time's a charm? :-)
>
> Sure, can do.
How about something like this?
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1980,8 +1987,14 @@ static void ravb_set_delay_mode(struct net_device *ndev)
set |= APSR_DM_RDM;
if (priv->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
- priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID)
- set |= APSR_DM_TDM;
+ priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) {
+ if (soc_device_match(ravb_delay_mode_quirk_match))
+ dev_warn(ndev->dev.parent,
+ "phy-mode %s requires TX clock internal delay mode which is not supported by this hardwre revision",
+ phy_modes(priv->phy_interface));
+ else
+ set |= APSR_DM_TDM;
+ }
ravb_modify(ndev, APSR, APSR_DM, set);
}
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 net-next] ravb: Avoid unsupported internal delay mode for R-Car E3/D3
2019-04-11 9:20 ` Simon Horman
@ 2019-04-11 13:33 ` Andrew Lunn
2019-04-11 16:36 ` Sergei Shtylyov
0 siblings, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2019-04-11 13:33 UTC (permalink / raw)
To: Simon Horman
Cc: Sergei Shtylyov, David Miller, Magnus Damm, netdev,
linux-renesas-soc, Yoshihiro Shimoda, Wolfram Sang
On Thu, Apr 11, 2019 at 11:20:57AM +0200, Simon Horman wrote:
> On Thu, Apr 11, 2019 at 10:40:27AM +0200, Simon Horman wrote:
> > On Wed, Apr 10, 2019 at 08:17:16PM +0300, Sergei Shtylyov wrote:
> > > On 04/10/2019 07:19 PM, Andrew Lunn wrote:
> > >
> > > >> According to the R-Car Gen3 Hardware Manual Rev 1.50 of Nov 30, 2018, the
> > > >> TX clock internal delay mode isn't supported on R-Car E3 (r8a77990) or D3
> > > >> (r8a77995). And by extension it is also not supported by RZ/G2E (r9a774c0).
> > > >>
> > > >> This matches all ES versions of the affected SoCs as it is
> > > >> not clear if this problem will be resolved in newer chips.
> > > >> This can be revisited, as necessary.
> > > >>
> > > >> This patch does not error-out if PHY_INTERFACE_MODE_RGMII_ID or
> > > >> PHY_INTERFACE_MODE_RGMII_TXID are used on SoCs where TX clock delay
> > > >> mode is not supported as there is a risk of introducing a regression
> > > >> when used in conjunction with older DT blobs present in the field.
> > > >
> > > > Hi Simon
> > > >
> > > > I think it should at least WARN_ON(). Such blobs are broken, even if
> > > > they do kind of work.
> > >
> > > Good idea! Simon, third time's a charm? :-)
> >
> > Sure, can do.
>
> How about something like this?
>
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -1980,8 +1987,14 @@ static void ravb_set_delay_mode(struct net_device *ndev)
> set |= APSR_DM_RDM;
>
> if (priv->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID)
> - set |= APSR_DM_TDM;
> + priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> + if (soc_device_match(ravb_delay_mode_quirk_match))
> + dev_warn(ndev->dev.parent,
> + "phy-mode %s requires TX clock internal delay mode which is not supported by this hardwre revision",
> + phy_modes(priv->phy_interface));
Hi Simon
The point of the warning is to tell users they should upgrade their DT
blob to one that is not broken. So i think the message should say
this. Also, we want users to notice this, which is why i said
WARN_ON(). Something big so it gets noticed.
Andrew
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 net-next] ravb: Avoid unsupported internal delay mode for R-Car E3/D3
2019-04-11 13:33 ` Andrew Lunn
@ 2019-04-11 16:36 ` Sergei Shtylyov
2019-04-15 11:45 ` Simon Horman
0 siblings, 1 reply; 11+ messages in thread
From: Sergei Shtylyov @ 2019-04-11 16:36 UTC (permalink / raw)
To: Andrew Lunn, Simon Horman
Cc: David Miller, Magnus Damm, netdev, linux-renesas-soc,
Yoshihiro Shimoda, Wolfram Sang
On 04/11/2019 04:33 PM, Andrew Lunn wrote:
>>>>>> According to the R-Car Gen3 Hardware Manual Rev 1.50 of Nov 30, 2018, the
>>>>>> TX clock internal delay mode isn't supported on R-Car E3 (r8a77990) or D3
>>>>>> (r8a77995). And by extension it is also not supported by RZ/G2E (r9a774c0).
>>>>>>
>>>>>> This matches all ES versions of the affected SoCs as it is
>>>>>> not clear if this problem will be resolved in newer chips.
>>>>>> This can be revisited, as necessary.
>>>>>>
>>>>>> This patch does not error-out if PHY_INTERFACE_MODE_RGMII_ID or
>>>>>> PHY_INTERFACE_MODE_RGMII_TXID are used on SoCs where TX clock delay
>>>>>> mode is not supported as there is a risk of introducing a regression
>>>>>> when used in conjunction with older DT blobs present in the field.
>>>>>
>>>>> Hi Simon
>>>>>
>>>>> I think it should at least WARN_ON(). Such blobs are broken, even if
>>>>> they do kind of work.
>>>>
>>>> Good idea! Simon, third time's a charm? :-)
>>>
>>> Sure, can do.
>>
>> How about something like this?
>>
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -1980,8 +1987,14 @@ static void ravb_set_delay_mode(struct net_device *ndev)
>> set |= APSR_DM_RDM;
>>
>> if (priv->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
>> - priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID)
>> - set |= APSR_DM_TDM;
>> + priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) {
>> + if (soc_device_match(ravb_delay_mode_quirk_match))
>> + dev_warn(ndev->dev.parent,
>> + "phy-mode %s requires TX clock internal delay mode which is not supported by this hardwre revision",
>> + phy_modes(priv->phy_interface));
>
> Hi Simon
>
> The point of the warning is to tell users they should upgrade their DT
> blob to one that is not broken. So i think the message should say
> this. Also, we want users to notice this, which is why i said
> WARN_ON(). Something big so it gets noticed.
I agree in general but I guess you meant WARN() -- WARN_ON() doesn't take
a string arg...
> Andrew
MBR, Sergei
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 net-next] ravb: Avoid unsupported internal delay mode for R-Car E3/D3
2019-04-11 16:36 ` Sergei Shtylyov
@ 2019-04-15 11:45 ` Simon Horman
2019-04-15 16:49 ` Andrew Lunn
2019-04-15 17:10 ` Sergei Shtylyov
0 siblings, 2 replies; 11+ messages in thread
From: Simon Horman @ 2019-04-15 11:45 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Andrew Lunn, David Miller, Magnus Damm, netdev, linux-renesas-soc,
Yoshihiro Shimoda, Wolfram Sang
On Thu, Apr 11, 2019 at 07:36:35PM +0300, Sergei Shtylyov wrote:
> On 04/11/2019 04:33 PM, Andrew Lunn wrote:
>
> >>>>>> According to the R-Car Gen3 Hardware Manual Rev 1.50 of Nov 30, 2018, the
> >>>>>> TX clock internal delay mode isn't supported on R-Car E3 (r8a77990) or D3
> >>>>>> (r8a77995). And by extension it is also not supported by RZ/G2E (r9a774c0).
> >>>>>>
> >>>>>> This matches all ES versions of the affected SoCs as it is
> >>>>>> not clear if this problem will be resolved in newer chips.
> >>>>>> This can be revisited, as necessary.
> >>>>>>
> >>>>>> This patch does not error-out if PHY_INTERFACE_MODE_RGMII_ID or
> >>>>>> PHY_INTERFACE_MODE_RGMII_TXID are used on SoCs where TX clock delay
> >>>>>> mode is not supported as there is a risk of introducing a regression
> >>>>>> when used in conjunction with older DT blobs present in the field.
> >>>>>
> >>>>> Hi Simon
> >>>>>
> >>>>> I think it should at least WARN_ON(). Such blobs are broken, even if
> >>>>> they do kind of work.
> >>>>
> >>>> Good idea! Simon, third time's a charm? :-)
> >>>
> >>> Sure, can do.
> >>
> >> How about something like this?
> >>
> >> --- a/drivers/net/ethernet/renesas/ravb_main.c
> >> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> >> @@ -1980,8 +1987,14 @@ static void ravb_set_delay_mode(struct net_device *ndev)
> >> set |= APSR_DM_RDM;
> >>
> >> if (priv->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
> >> - priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID)
> >> - set |= APSR_DM_TDM;
> >> + priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> >> + if (soc_device_match(ravb_delay_mode_quirk_match))
> >> + dev_warn(ndev->dev.parent,
> >> + "phy-mode %s requires TX clock internal delay mode which is not supported by this hardwre revision",
> >> + phy_modes(priv->phy_interface));
> >
> > Hi Simon
> >
> > The point of the warning is to tell users they should upgrade their DT
> > blob to one that is not broken. So i think the message should say
> > this. Also, we want users to notice this, which is why i said
> > WARN_ON(). Something big so it gets noticed.
>
> I agree in general but I guess you meant WARN() -- WARN_ON() doesn't take
> a string arg...
Thanks, how about this?
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 4f648394e645..9618c4881c83 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1980,8 +1987,12 @@ static void ravb_set_delay_mode(struct net_device *ndev)
set |= APSR_DM_RDM;
if (priv->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
- priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID)
- set |= APSR_DM_TDM;
+ priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) {
+ if (!WARN(soc_device_match(ravb_delay_mode_quirk_match),
+ "phy-mode %s requires TX clock internal delay mode which is not supported by this hardware revision. Please update device tree",
+ phy_modes(priv->phy_interface)))
+ set |= APSR_DM_TDM;
+ }
ravb_modify(ndev, APSR, APSR_DM, set);
}
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 net-next] ravb: Avoid unsupported internal delay mode for R-Car E3/D3
2019-04-15 11:45 ` Simon Horman
@ 2019-04-15 16:49 ` Andrew Lunn
2019-04-15 17:10 ` Sergei Shtylyov
1 sibling, 0 replies; 11+ messages in thread
From: Andrew Lunn @ 2019-04-15 16:49 UTC (permalink / raw)
To: Simon Horman
Cc: Sergei Shtylyov, David Miller, Magnus Damm, netdev,
linux-renesas-soc, Yoshihiro Shimoda, Wolfram Sang
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 4f648394e645..9618c4881c83 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -1980,8 +1987,12 @@ static void ravb_set_delay_mode(struct net_device *ndev)
> set |= APSR_DM_RDM;
>
> if (priv->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID)
> - set |= APSR_DM_TDM;
> + priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> + if (!WARN(soc_device_match(ravb_delay_mode_quirk_match),
> + "phy-mode %s requires TX clock internal delay mode which is not supported by this hardware revision. Please update device tree",
> + phy_modes(priv->phy_interface)))
> + set |= APSR_DM_TDM;
> + }
>
> ravb_modify(ndev, APSR, APSR_DM, set);
> }
Hi Simon
This looks good. Please add my:
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
To the real patch.
Andrew
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 net-next] ravb: Avoid unsupported internal delay mode for R-Car E3/D3
2019-04-15 11:45 ` Simon Horman
2019-04-15 16:49 ` Andrew Lunn
@ 2019-04-15 17:10 ` Sergei Shtylyov
1 sibling, 0 replies; 11+ messages in thread
From: Sergei Shtylyov @ 2019-04-15 17:10 UTC (permalink / raw)
To: Simon Horman
Cc: Andrew Lunn, David Miller, Magnus Damm, netdev, linux-renesas-soc,
Yoshihiro Shimoda, Wolfram Sang
On 04/15/2019 02:45 PM, Simon Horman wrote:
>>>>>>>> According to the R-Car Gen3 Hardware Manual Rev 1.50 of Nov 30, 2018, the
>>>>>>>> TX clock internal delay mode isn't supported on R-Car E3 (r8a77990) or D3
>>>>>>>> (r8a77995). And by extension it is also not supported by RZ/G2E (r9a774c0).
>>>>>>>>
>>>>>>>> This matches all ES versions of the affected SoCs as it is
>>>>>>>> not clear if this problem will be resolved in newer chips.
>>>>>>>> This can be revisited, as necessary.
>>>>>>>>
>>>>>>>> This patch does not error-out if PHY_INTERFACE_MODE_RGMII_ID or
>>>>>>>> PHY_INTERFACE_MODE_RGMII_TXID are used on SoCs where TX clock delay
>>>>>>>> mode is not supported as there is a risk of introducing a regression
>>>>>>>> when used in conjunction with older DT blobs present in the field.
>>>>>>>
>>>>>>> Hi Simon
>>>>>>>
>>>>>>> I think it should at least WARN_ON(). Such blobs are broken, even if
>>>>>>> they do kind of work.
>>>>>>
>>>>>> Good idea! Simon, third time's a charm? :-)
>>>>>
>>>>> Sure, can do.
>>>>
>>>> How about something like this?
>>>>
>>>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>>>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>>>> @@ -1980,8 +1987,14 @@ static void ravb_set_delay_mode(struct net_device *ndev)
>>>> set |= APSR_DM_RDM;
>>>>
>>>> if (priv->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
>>>> - priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID)
>>>> - set |= APSR_DM_TDM;
>>>> + priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) {
>>>> + if (soc_device_match(ravb_delay_mode_quirk_match))
>>>> + dev_warn(ndev->dev.parent,
>>>> + "phy-mode %s requires TX clock internal delay mode which is not supported by this hardwre revision",
>>>> + phy_modes(priv->phy_interface));
>>>
>>> Hi Simon
>>>
>>> The point of the warning is to tell users they should upgrade their DT
>>> blob to one that is not broken. So i think the message should say
>>> this. Also, we want users to notice this, which is why i said
>>> WARN_ON(). Something big so it gets noticed.
>>
>> I agree in general but I guess you meant WARN() -- WARN_ON() doesn't take
>> a string arg...
>
> Thanks, how about this?
>
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 4f648394e645..9618c4881c83 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -1980,8 +1987,12 @@ static void ravb_set_delay_mode(struct net_device *ndev)
> set |= APSR_DM_RDM;
>
> if (priv->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
> - priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID)
> - set |= APSR_DM_TDM;
> + priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> + if (!WARN(soc_device_match(ravb_delay_mode_quirk_match),
> + "phy-mode %s requires TX clock internal delay mode which is not supported by this hardware revision. Please update device tree",
> + phy_modes(priv->phy_interface)))
> + set |= APSR_DM_TDM;
> + }
>
> ravb_modify(ndev, APSR, APSR_DM, set);
> }
Looks fine, thanx! :-)
MBR, Sergei
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-04-15 17:10 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-10 11:45 [PATCH v2 net-next] ravb: Avoid unsupported internal delay mode for R-Car E3/D3 Simon Horman
2019-04-10 12:17 ` Wolfram Sang
2019-04-10 16:19 ` Andrew Lunn
2019-04-10 17:17 ` Sergei Shtylyov
2019-04-11 8:40 ` Simon Horman
2019-04-11 9:20 ` Simon Horman
2019-04-11 13:33 ` Andrew Lunn
2019-04-11 16:36 ` Sergei Shtylyov
2019-04-15 11:45 ` Simon Horman
2019-04-15 16:49 ` Andrew Lunn
2019-04-15 17:10 ` Sergei Shtylyov
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).