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@linaro.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);
dev_dbg(gmac->dev, "PHY mode set to %s\n", phy_modes(*gmac->intf_mode));
@@ -125,10 +132,16 @@ static int s32_dwmac_probe(struct platform_device *pdev)
"dt configuration failed\n");
/* PHY interface mode control reg */
- gmac->ctrl_sts = devm_platform_get_and_ioremap_resource(pdev, 1, NULL);
- if (IS_ERR(gmac->ctrl_sts))
- return dev_err_probe(dev, PTR_ERR(gmac->ctrl_sts),
- "S32CC config region is missing\n");
+ gmac->sts_regmap = syscon_regmap_lookup_by_phandle_args(dev->of_node,
+ "phy-sel", 1, &gmac->sts_offset);
+ if (gmac->sts_regmap == ERR_PTR(-EPROBE_DEFER))
+ return PTR_ERR(gmac->sts_regmap);
+ if (IS_ERR(gmac->sts_regmap)) {
+ gmac->ctrl_sts = devm_platform_get_and_ioremap_resource(pdev, 1, NULL);
+ if (IS_ERR(gmac->ctrl_sts))
+ return dev_err_probe(dev, PTR_ERR(gmac->ctrl_sts),
+ "S32CC config region is missing\n");
+ }
/* tx clock */
gmac->tx_clk = devm_clk_get(&pdev->dev, "tx");
--
2.51.0On 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.
What's benefit by use syscon interface here? syscon have not much
well consided funcitonal abstraction.
Frank
>
> 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@linaro.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);
>
> dev_dbg(gmac->dev, "PHY mode set to %s\n", phy_modes(*gmac->intf_mode));
>
> @@ -125,10 +132,16 @@ static int s32_dwmac_probe(struct platform_device *pdev)
> "dt configuration failed\n");
>
> /* PHY interface mode control reg */
> - gmac->ctrl_sts = devm_platform_get_and_ioremap_resource(pdev, 1, NULL);
> - if (IS_ERR(gmac->ctrl_sts))
> - return dev_err_probe(dev, PTR_ERR(gmac->ctrl_sts),
> - "S32CC config region is missing\n");
> + gmac->sts_regmap = syscon_regmap_lookup_by_phandle_args(dev->of_node,
> + "phy-sel", 1, &gmac->sts_offset);
> + if (gmac->sts_regmap == ERR_PTR(-EPROBE_DEFER))
> + return PTR_ERR(gmac->sts_regmap);
> + if (IS_ERR(gmac->sts_regmap)) {
> + gmac->ctrl_sts = devm_platform_get_and_ioremap_resource(pdev, 1, NULL);
> + if (IS_ERR(gmac->ctrl_sts))
> + return dev_err_probe(dev, PTR_ERR(gmac->ctrl_sts),
> + "S32CC config region is missing\n");
> + }
>
> /* tx clock */
> gmac->tx_clk = devm_clk_get(&pdev->dev, "tx");
> --
> 2.51.0
>
On Mon, Dec 01, 2025 at 05:29:36PM -0500, Frank Li wrote: > 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. > > What's benefit by use syscon interface here? syscon have not much > well consided funcitonal abstraction. > The GPR has a bunch of random registers that aren't really related. On these chips they're just regular MMIO registers, but in other configurations you can only access them using SCMI. It's better to group them together that's how they are in the hardware. Otherwise we'd end up randomly adding a register address to the ethernet device tree entry, but it's nicer to use a phandle to reference the GPR. The only register we're using now is the GMAC_0_CTRL_STS but here is the list of registers in the GPR. From 0x4007C000 0 Software-Triggered Faults (SW_NCF) 4 GMAC Control (GMAC_0_CTRL_STS) 28 CMU Status 1 (CMU_STATUS_REG1) 2C CMUs Status 2 (CMU_STATUS_REG2) 30 FCCU EOUT Override Clear (FCCU_EOUT_OVERRIDE_CLEAR_REG) 38 SRC POR Control (SRC_POR_CTRL_REG) 54 GPR21 (GPR21) 5C GPR23 (GPR23) 60 GPR24 Register (GPR24) CC Debug Control (DEBUG_CONTROL) F0 Timestamp Control (TIMESTAMP_CONTROL_REGISTER) F4 FlexRay OS Tick Input Select (FLEXRAY_OS_TICK_INPUT_SELECT_REG) FC GPR63 Register (GPR63) Then from 0x4007CA00 0 Coherency Enable for PFE Ports (PFE_COH_EN) 4 PFE EMAC Interface Mode (PFE_EMACX_INTF_SEL) 20 PFE EMACX Power Control (PFE_PWR_CTRL) 28 Error Injection on Cortex-M7 AHB and AXI Pipe (CM7_TCM_AHB_SLICE) 2C Error Injection AHBP Gasket Cortex-M7 (ERROR_INJECTION_AHBP_GASKET_CM7) 40 LLCE Subsystem Status (LLCE_STAT) 44 LLCE Power Control (LLCE_CTRL) 48 DDR Urgent Control (DDR_URGENT_CTRL) 4C FTM Global Load Control (FLXTIM_CTRL) 50 FTM LDOK Status (FLXTIM_STAT) 54 Top CMU Status (CMU_STAT) 58 Accelerator NoC No Pending Trans Status (NOC_NOPEND_TRANS) 90 SerDes RD/WD Toggle Control (PCIE_TOGGLE) 94 SerDes Toggle Done Status (PCIE_TOGGLEDONE_STAT) E0 Generic Control 0 (GENCTRL0) E4 Generic Control 1 (GENCTRL1) F0 Generic Status 0 (GENSTAT0) FC Cortex-M7 AXI Parity Error and AHBP Gasket Error Alarm (CM7_AXI_AHBP_GASKET_ERROR_ALARM) From 4007C800 4 GPR01 Register (GPR01) 30 GPR12 Register (GPR12) 58 GPR22 Register (GPR22) 70 GPR28 Register (GPR28) 74 GPR29 Register (GPR29) From 4007CB00 4 WKUP Pad Pullup/Pulldown Select (WKUP_PUS) regards, dan carpenter
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@linaro.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!
On Mon, Dec 01, 2025 at 04:48:12PM +0000, Russell King (Oracle) wrote:
> 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@linaro.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.
>
Yeah. Sorry, I forward ported this, then back ported it, then forward
ported this again and I messed up. :(
regards,
dan carpenter
© 2016 - 2026 Red Hat, Inc.