Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230727-synquacer-net-v1-1-4d7f5c4cc8d9@kernel.org>
In-Reply-To: <20230727-synquacer-net-v1-1-4d7f5c4cc8d9@kernel.org>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Fri, 28 Jul 2023 07:45:44 +0200
Message-ID: <CAMj1kXH_4OEY58Nb9yGHTDvjfouJHKNVhReo0mMdD_aGWW_WGQ@mail.gmail.com>
Subject: Re: [PATCH] net: netsec: Ignore 'phy-mode' on SynQuacer in DT mode
To:     Mark Brown <broonie@kernel.org>,
        Masahisa Kojima <masahisa.kojima@linaro.org>
Cc:     Jassi Brar <jaswinder.singh@linaro.org>,
        Ilias Apalodimas <ilias.apalodimas@linaro.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

(cc Masahisa)

On Thu, 27 Jul 2023 at 23:52, 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 commits.  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.  Since the firmware used with DT is the
> same (the firmware interface is selectable in the firmware
> configuration) and the firmware configures the PHY prior to running the
> OS we can use the same workaround.
>

Wouldn't this break SynQuacers booting with firmware that lacks a
network driver? (I.e., u-boot?)

I am not sure why, but quite some effort has gone into porting u-boot
to this SoC as well.


> Limit this to the SynQuacer, though practically speaking this is the
> only currently known system using this device.
>
> Fixes: 533dd11a12f6 ("net: socionext: Add Synquacer NetSec driver")
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  drivers/net/ethernet/socionext/netsec.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> index 2d7347b71c41..ae4d336efaa4 100644
> --- a/drivers/net/ethernet/socionext/netsec.c
> +++ b/drivers/net/ethernet/socionext/netsec.c
> @@ -1845,10 +1845,20 @@ static int netsec_of_probe(struct platform_device *pdev,
>  {
>         int err;
>
> -       err = of_get_phy_mode(pdev->dev.of_node, &priv->phy_interface);
> -       if (err) {
> -               dev_err(&pdev->dev, "missing required property 'phy-mode'\n");
> -               return err;
> +       if (of_machine_is_compatible("socionext,developer-box")) {
> +               /*
> +                * SynQuacer reports RGMII but is physically
> +                * configured with TX and RX delays, since the
> +                * firwmare configures the PHY prior to boot just
> +                * ignore the configuration.
> +                */
> +               priv->phy_interface = PHY_INTERFACE_MODE_NA;
> +       } else {
> +               err = of_get_phy_mode(pdev->dev.of_node, &priv->phy_interface);
> +               if (err) {
> +                       dev_err(&pdev->dev, "missing required property 'phy-mode'\n");
> +                       return err;
> +               }
>         }
>
>         priv->phy_np = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
>
> ---
> base-commit: 6eaae198076080886b9e7d57f4ae06fa782f90ef
> change-id: 20230727-synquacer-net-e241f34baceb
>
> Best regards,
> --
> Mark Brown <broonie@kernel.org>
>