[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aS3GzJljbfp2xJmW@shell.armlinux.org.uk>
Date: Mon, 1 Dec 2025 16:48:12 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Jan Petrous <jan.petrous@....nxp.com>, s32@....com,
Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
netdev@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linaro-s32@...aro.org
Subject: Re: [PATCH 1/4] net: stmmac: s32: use the syscon interface
PHY_INTF_SEL_RGMII
On Mon, Dec 01, 2025 at 04:08:20PM +0300, Dan Carpenter wrote:
> On the s32 chipset the GMAC_0_CTRL_STS register is in GPR region.
> Originally, accessing this register was done in a sort of ad-hoc way,
> but we want to use the syscon interface to do it.
>
> This is a little bit uglier because we to maintain backwards compatibility
> to the old device trees so we have to support both ways to access this
> register.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org>
> ---
> .../net/ethernet/stmicro/stmmac/dwmac-s32.c | 23 +++++++++++++++----
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c
> index 5a485ee98fa7..20de761b7d28 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c
> @@ -11,12 +11,14 @@
> #include <linux/device.h>
> #include <linux/ethtool.h>
> #include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/of_mdio.h>
> #include <linux/of_address.h>
> #include <linux/phy.h>
> #include <linux/phylink.h>
> #include <linux/platform_device.h>
> +#include <linux/regmap.h>
> #include <linux/stmmac.h>
>
> #include "stmmac_platform.h"
> @@ -32,6 +34,8 @@
> struct s32_priv_data {
> void __iomem *ioaddr;
> void __iomem *ctrl_sts;
> + struct regmap *sts_regmap;
> + unsigned int sts_offset;
> struct device *dev;
> phy_interface_t *intf_mode;
> struct clk *tx_clk;
> @@ -40,7 +44,10 @@ struct s32_priv_data {
>
> static int s32_gmac_write_phy_intf_select(struct s32_priv_data *gmac)
> {
> - writel(S32_PHY_INTF_SEL_RGMII, gmac->ctrl_sts);
> + if (gmac->ctrl_sts)
> + writel(S32_PHY_INTF_SEL_RGMII, gmac->ctrl_sts);
> + else
> + regmap_write(gmac->sts_regmap, gmac->sts_offset, PHY_INTF_SEL_RGMII);
Sorry, but even if that regmap_write() is targetting the exact same
register, these are not identical.
S32_PHY_INTF_SEL_RGMII, which is a S32-specific value, takes the value 2.
PHY_INTF_SEL_RGMII is the dwmac specific value, and takes the value 1.
If this targets the same register, then by writing PHY_INTF_SEL_RGMII,
you are in effect writing the equivalent of S32_PHY_INTF_SEL_SGMII to
it. This seems like a bug.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists