[PATCH v3] net: netsec: Ignore 'phy-mode' on SynQuacer in DT mode

Mark Brown posted 1 patch 2 years, 1 month ago
drivers/net/ethernet/socionext/netsec.c | 11 +++++++++++
1 file changed, 11 insertions(+)
[PATCH v3] net: netsec: Ignore 'phy-mode' on SynQuacer in DT mode
Posted by Mark Brown 2 years, 1 month ago
As documented in acd7aaf51b20 ("netsec: ignore 'phy-mode' device
property on ACPI systems") the SocioNext SynQuacer platform ships with
firmware defining the PHY mode as RGMII even though the physical
configuration of the PHY is for TX and RX delays.  Since bbc4d71d63549bc
("net: phy: realtek: fix rtl8211e rx/tx delay config") this has caused
misconfiguration of the PHY, rendering the network unusable.

This was worked around for ACPI by ignoring the phy-mode property but
the system is also used with DT.  For DT instead if we're running on a
SynQuacer force a working PHY mode, as well as the standard EDK2
firmware with DT there are also some of these systems that use u-boot
and might not initialise the PHY if not netbooting.  Newer firmware
imagaes for at least EDK2 are available from Linaro so print a warning
when doing this.

Fixes: 533dd11a12f6 ("net: socionext: Add Synquacer NetSec driver")
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Changes in v3:
- Typo fixes.
- Link to v2: https://lore.kernel.org/r/20230728-synquacer-net-v2-1-aea4d4f32b26@kernel.org

Changes in v2:
- Unlike ACPI force what appears to be the correct mode, there are
  u-boot firmwares which might not configure the PHY.
- Link to v1: https://lore.kernel.org/r/20230727-synquacer-net-v1-1-4d7f5c4cc8d9@kernel.org
---
 drivers/net/ethernet/socionext/netsec.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index 2d7347b71c41..0dcd6a568b06 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -1851,6 +1851,17 @@ static int netsec_of_probe(struct platform_device *pdev,
 		return err;
 	}
 
+	/*
+	 * SynQuacer is physically configured with TX and RX delays
+	 * but the standard firmware claimed otherwise for a long
+	 * time, ignore it.
+	 */
+	if (of_machine_is_compatible("socionext,developer-box") &&
+	    priv->phy_interface != PHY_INTERFACE_MODE_RGMII_ID) {
+		dev_warn(&pdev->dev, "Outdated firmware reports incorrect PHY mode, overriding\n");
+		priv->phy_interface = PHY_INTERFACE_MODE_RGMII_ID;
+	}
+
 	priv->phy_np = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
 	if (!priv->phy_np) {
 		dev_err(&pdev->dev, "missing required property 'phy-handle'\n");

---
base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4
change-id: 20230727-synquacer-net-e241f34baceb

Best regards,
-- 
Mark Brown <broonie@kernel.org>
Re: [PATCH v3] net: netsec: Ignore 'phy-mode' on SynQuacer in DT mode
Posted by patchwork-bot+netdevbpf@kernel.org 2 years, 1 month ago
Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 31 Jul 2023 11:48:32 +0100 you wrote:
> As documented in acd7aaf51b20 ("netsec: ignore 'phy-mode' device
> property on ACPI systems") the SocioNext SynQuacer platform ships with
> firmware defining the PHY mode as RGMII even though the physical
> configuration of the PHY is for TX and RX delays.  Since bbc4d71d63549bc
> ("net: phy: realtek: fix rtl8211e rx/tx delay config") this has caused
> misconfiguration of the PHY, rendering the network unusable.
> 
> [...]

Here is the summary with links:
  - [v3] net: netsec: Ignore 'phy-mode' on SynQuacer in DT mode
    https://git.kernel.org/netdev/net/c/f3bb7759a924

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH v3] net: netsec: Ignore 'phy-mode' on SynQuacer in DT mode
Posted by Andrew Lunn 2 years, 1 month ago
On Mon, Jul 31, 2023 at 11:48:32AM +0100, Mark Brown wrote:
> As documented in acd7aaf51b20 ("netsec: ignore 'phy-mode' device
> property on ACPI systems") the SocioNext SynQuacer platform ships with
> firmware defining the PHY mode as RGMII even though the physical
> configuration of the PHY is for TX and RX delays.  Since bbc4d71d63549bc
> ("net: phy: realtek: fix rtl8211e rx/tx delay config") this has caused
> misconfiguration of the PHY, rendering the network unusable.
> 
> This was worked around for ACPI by ignoring the phy-mode property but
> the system is also used with DT.  For DT instead if we're running on a
> SynQuacer force a working PHY mode, as well as the standard EDK2
> firmware with DT there are also some of these systems that use u-boot
> and might not initialise the PHY if not netbooting.  Newer firmware
> imagaes for at least EDK2 are available from Linaro so print a warning
> when doing this.
> 
> Fixes: 533dd11a12f6 ("net: socionext: Add Synquacer NetSec driver")
> Signed-off-by: Mark Brown <broonie@kernel.org>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Re: [PATCH v3] net: netsec: Ignore 'phy-mode' on SynQuacer in DT mode
Posted by Ard Biesheuvel 2 years, 1 month ago
On Mon, 31 Jul 2023 at 12:48, Mark Brown <broonie@kernel.org> wrote:
>
> As documented in acd7aaf51b20 ("netsec: ignore 'phy-mode' device
> property on ACPI systems") the SocioNext SynQuacer platform ships with
> firmware defining the PHY mode as RGMII even though the physical
> configuration of the PHY is for TX and RX delays.  Since bbc4d71d63549bc
> ("net: phy: realtek: fix rtl8211e rx/tx delay config") this has caused
> misconfiguration of the PHY, rendering the network unusable.
>
> This was worked around for ACPI by ignoring the phy-mode property but
> the system is also used with DT.  For DT instead if we're running on a
> SynQuacer force a working PHY mode, as well as the standard EDK2
> firmware with DT there are also some of these systems that use u-boot
> and might not initialise the PHY if not netbooting.  Newer firmware
> imagaes for at least EDK2 are available from Linaro so print a warning
> when doing this.
>
> Fixes: 533dd11a12f6 ("net: socionext: Add Synquacer NetSec driver")
> Signed-off-by: Mark Brown <broonie@kernel.org>

Acked-by: Ard Biesheuvel <ardb@kernel.org>

> ---
> Changes in v3:
> - Typo fixes.
> - Link to v2: https://lore.kernel.org/r/20230728-synquacer-net-v2-1-aea4d4f32b26@kernel.org
>
> Changes in v2:
> - Unlike ACPI force what appears to be the correct mode, there are
>   u-boot firmwares which might not configure the PHY.
> - Link to v1: https://lore.kernel.org/r/20230727-synquacer-net-v1-1-4d7f5c4cc8d9@kernel.org
> ---
>  drivers/net/ethernet/socionext/netsec.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> index 2d7347b71c41..0dcd6a568b06 100644
> --- a/drivers/net/ethernet/socionext/netsec.c
> +++ b/drivers/net/ethernet/socionext/netsec.c
> @@ -1851,6 +1851,17 @@ static int netsec_of_probe(struct platform_device *pdev,
>                 return err;
>         }
>
> +       /*
> +        * SynQuacer is physically configured with TX and RX delays
> +        * but the standard firmware claimed otherwise for a long
> +        * time, ignore it.
> +        */
> +       if (of_machine_is_compatible("socionext,developer-box") &&
> +           priv->phy_interface != PHY_INTERFACE_MODE_RGMII_ID) {
> +               dev_warn(&pdev->dev, "Outdated firmware reports incorrect PHY mode, overriding\n");
> +               priv->phy_interface = PHY_INTERFACE_MODE_RGMII_ID;
> +       }
> +
>         priv->phy_np = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
>         if (!priv->phy_np) {
>                 dev_err(&pdev->dev, "missing required property 'phy-handle'\n");
>
> ---
> base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4
> change-id: 20230727-synquacer-net-e241f34baceb
>
> Best regards,
> --
> Mark Brown <broonie@kernel.org>
>
Re: [PATCH v3] net: netsec: Ignore 'phy-mode' on SynQuacer in DT mode
Posted by Ilias Apalodimas 2 years, 1 month ago
On Mon, 31 Jul 2023 at 13:54, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 31 Jul 2023 at 12:48, Mark Brown <broonie@kernel.org> wrote:
> >
> > As documented in acd7aaf51b20 ("netsec: ignore 'phy-mode' device
> > property on ACPI systems") the SocioNext SynQuacer platform ships with
> > firmware defining the PHY mode as RGMII even though the physical
> > configuration of the PHY is for TX and RX delays.  Since bbc4d71d63549bc
> > ("net: phy: realtek: fix rtl8211e rx/tx delay config") this has caused
> > misconfiguration of the PHY, rendering the network unusable.
> >
> > This was worked around for ACPI by ignoring the phy-mode property but
> > the system is also used with DT.  For DT instead if we're running on a
> > SynQuacer force a working PHY mode, as well as the standard EDK2
> > firmware with DT there are also some of these systems that use u-boot
> > and might not initialise the PHY if not netbooting.  Newer firmware
> > imagaes for at least EDK2 are available from Linaro so print a warning
> > when doing this.
> >
> > Fixes: 533dd11a12f6 ("net: socionext: Add Synquacer NetSec driver")
> > Signed-off-by: Mark Brown <broonie@kernel.org>
>
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
>
> > ---
> > Changes in v3:
> > - Typo fixes.
> > - Link to v2: https://lore.kernel.org/r/20230728-synquacer-net-v2-1-aea4d4f32b26@kernel.org
> >
> > Changes in v2:
> > - Unlike ACPI force what appears to be the correct mode, there are
> >   u-boot firmwares which might not configure the PHY.
> > - Link to v1: https://lore.kernel.org/r/20230727-synquacer-net-v1-1-4d7f5c4cc8d9@kernel.org
> > ---
> >  drivers/net/ethernet/socionext/netsec.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> > index 2d7347b71c41..0dcd6a568b06 100644
> > --- a/drivers/net/ethernet/socionext/netsec.c
> > +++ b/drivers/net/ethernet/socionext/netsec.c
> > @@ -1851,6 +1851,17 @@ static int netsec_of_probe(struct platform_device *pdev,
> >                 return err;
> >         }
> >
> > +       /*
> > +        * SynQuacer is physically configured with TX and RX delays
> > +        * but the standard firmware claimed otherwise for a long
> > +        * time, ignore it.
> > +        */
> > +       if (of_machine_is_compatible("socionext,developer-box") &&
> > +           priv->phy_interface != PHY_INTERFACE_MODE_RGMII_ID) {
> > +               dev_warn(&pdev->dev, "Outdated firmware reports incorrect PHY mode, overriding\n");
> > +               priv->phy_interface = PHY_INTERFACE_MODE_RGMII_ID;
> > +       }
> > +
> >         priv->phy_np = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
> >         if (!priv->phy_np) {
> >                 dev_err(&pdev->dev, "missing required property 'phy-handle'\n");
> >
> > ---
> > base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4
> > change-id: 20230727-synquacer-net-e241f34baceb
> >
> > Best regards,
> > --
> > Mark Brown <broonie@kernel.org>
> >

Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>