Probe SJA1105 DSA children as platform sub-devices

[PATCH v2 net-next 04/15] net: dsa: sja1105: prepare regmap for passing to child devices

Posted by Vladimir Oltean 2 weeks, 4 days ago

Prepare a single regmap covering the entire SPI address space of the
SJA1105 and SJA1110 switches which can be given to MDIO buses, XPCS,
irqchip drivers etc.

This regmap is address-zero-based (can access the entire switch address
space) and child devices are supposed to access their respective memory
region with the help of struct resource (IORESOURCE_REG, to be precise).

Nothing is currently done with the regmap, it is just allocated and
added to the device's devres list, so it doesn't need to be freed.

Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v1->v2:
- s/sja1105_create_regmap()/devm_sja1105_create_regmap()/ for clarity in
  sja1105_probe() that teardown is not needed.
- drop unnecessary priv->regmap

 drivers/net/dsa/sja1105/sja1105.h      |  3 ++
 drivers/net/dsa/sja1105/sja1105_main.c |  6 +++
 drivers/net/dsa/sja1105/sja1105_spi.c  | 55 ++++++++++++++++++++++++++
 3 files changed, 64 insertions(+)

diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h
index dceb96ae9c83..8d4c0c8df326 100644
--- a/drivers/net/dsa/sja1105/sja1105.h
+++ b/drivers/net/dsa/sja1105/sja1105.h
@@ -11,6 +11,8 @@
 #include <linux/dsa/8021q.h>
 #include <net/dsa.h>
 #include <linux/mutex.h>
+#include <linux/regmap.h>
+
 #include "sja1105_static_config.h"
 
 #define SJA1105ET_FDB_BIN_SIZE		4
@@ -338,6 +340,7 @@ int static_config_buf_prepare_for_upload(struct sja1105_private *priv,
 int sja1105_static_config_upload(struct sja1105_private *priv);
 int sja1105_inhibit_tx(const struct sja1105_private *priv,
 		       unsigned long port_bitmap, bool tx_inhibited);
+int devm_sja1105_create_regmap(struct sja1105_private *priv);
 
 extern const struct sja1105_info sja1105e_info;
 extern const struct sja1105_info sja1105t_info;
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 2a4a0fe20dae..e9e091cf8998 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -3291,6 +3291,12 @@ static int sja1105_probe(struct spi_device *spi)
 
 	priv->info = of_device_get_match_data(dev);
 
+	rc = devm_sja1105_create_regmap(priv);
+	if (rc < 0) {
+		dev_err(dev, "Failed to create regmap: %pe\n", ERR_PTR(rc));
+		return rc;
+	}
+
 	/* Detect hardware device */
 	rc = sja1105_check_device_id(priv);
 	if (rc < 0) {
diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
index 834b5c1b4db0..856a751de53a 100644
--- a/drivers/net/dsa/sja1105/sja1105_spi.c
+++ b/drivers/net/dsa/sja1105/sja1105_spi.c
@@ -408,6 +408,61 @@ int sja1105_static_config_upload(struct sja1105_private *priv)
 	return rc;
 }
 
+static int sja1105_regmap_bus_reg_read(void *ctx, unsigned int reg,
+				       unsigned int *val)
+{
+	struct sja1105_private *priv = ctx;
+	u32 tmp;
+	int rc;
+
+	rc = sja1105_xfer_u32(priv, SPI_READ, SJA1110_SPI_ADDR(reg), &tmp,
+			      NULL);
+	if (rc)
+		return rc;
+
+	*val = tmp;
+
+	return 0;
+}
+
+static int sja1105_regmap_bus_reg_write(void *ctx, unsigned int reg,
+					unsigned int val)
+{
+	struct sja1105_private *priv = ctx;
+	u32 tmp = val;
+
+	return sja1105_xfer_u32(priv, SPI_WRITE, SJA1110_SPI_ADDR(reg), &tmp,
+				NULL);
+}
+
+/* The primary purpose of this is to pass it to child devices,
+ * not to abstract SPI access for the main driver.
+ */
+int devm_sja1105_create_regmap(struct sja1105_private *priv)
+{
+	static const struct regmap_bus sja1105_regmap_bus = {
+		.reg_read = sja1105_regmap_bus_reg_read,
+		.reg_write = sja1105_regmap_bus_reg_write,
+		.reg_format_endian_default = REGMAP_ENDIAN_NATIVE,
+		.val_format_endian_default = REGMAP_ENDIAN_NATIVE,
+	};
+	static const struct regmap_config regmap_config = {
+		.name = "regs",
+		.reg_bits = 32,
+		.val_bits = 32,
+		.reg_stride = 4,
+	};
+	struct device *dev = &priv->spidev->dev;
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init(dev, &sja1105_regmap_bus, priv,
+				  &regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	return 0;
+}
+
 static const struct sja1105_regs sja1105et_regs = {
 	.device_id = 0x0,
 	.prod_id = 0x100BC3,
-- 
2.34.1

Re: [PATCH v2 net-next 04/15] net: dsa: sja1105: prepare regmap for passing to child devices

Posted by Andy Shevchenko 2 weeks, 4 days ago

On Thu, Jan 22, 2026 at 12:56:43PM +0200, Vladimir Oltean wrote:
> Prepare a single regmap covering the entire SPI address space of the
> SJA1105 and SJA1110 switches which can be given to MDIO buses, XPCS,
> irqchip drivers etc.
> 
> This regmap is address-zero-based (can access the entire switch address
> space) and child devices are supposed to access their respective memory
> region with the help of struct resource (IORESOURCE_REG, to be precise).
> 
> Nothing is currently done with the regmap, it is just allocated and
> added to the device's devres list, so it doesn't need to be freed.

...

> --- a/drivers/net/dsa/sja1105/sja1105.h
> +++ b/drivers/net/dsa/sja1105/sja1105.h

>  #include <linux/dsa/8021q.h>
>  #include <net/dsa.h>
>  #include <linux/mutex.h>
> +#include <linux/regmap.h>
> +
>  #include "sja1105_static_config.h"

It looks to me that somebody missed grouping above (not this change) and it's
better to have it done:

#include <linux/dsa/8021q.h>
#include <linux/mutex.h>
#include <linux/regmap.h>

#include <net/dsa.h>

#include "sja1105_static_config.h"

...

> +	rc = devm_sja1105_create_regmap(priv);
> +	if (rc < 0) {
> +		dev_err(dev, "Failed to create regmap: %pe\n", ERR_PTR(rc));
> +		return rc;
> +	}

Hmm... Perhaps return dev_err_probe(...); ?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 net-next 04/15] net: dsa: sja1105: prepare regmap for passing to child devices

Posted by Vladimir Oltean 2 weeks, 4 days ago

On Thu, Jan 22, 2026 at 02:23:18PM +0200, Andy Shevchenko wrote:
> On Thu, Jan 22, 2026 at 12:56:43PM +0200, Vladimir Oltean wrote:
> > Prepare a single regmap covering the entire SPI address space of the
> > SJA1105 and SJA1110 switches which can be given to MDIO buses, XPCS,
> > irqchip drivers etc.
> > 
> > This regmap is address-zero-based (can access the entire switch address
> > space) and child devices are supposed to access their respective memory
> > region with the help of struct resource (IORESOURCE_REG, to be precise).
> > 
> > Nothing is currently done with the regmap, it is just allocated and
> > added to the device's devres list, so it doesn't need to be freed.
> 
> ...
> 
> > --- a/drivers/net/dsa/sja1105/sja1105.h
> > +++ b/drivers/net/dsa/sja1105/sja1105.h
> 
> >  #include <linux/dsa/8021q.h>
> >  #include <net/dsa.h>
> >  #include <linux/mutex.h>
> > +#include <linux/regmap.h>
> > +
> >  #include "sja1105_static_config.h"
> 
> It looks to me that somebody missed grouping above (not this change) and it's
> better to have it done:
> 
> #include <linux/dsa/8021q.h>
> #include <linux/mutex.h>
> #include <linux/regmap.h>
> 
> #include <net/dsa.h>
> 
> #include "sja1105_static_config.h"

That somebody would be me, but I don't consider this a relevant change
for this patch set.

> ...
> 
> > +	rc = devm_sja1105_create_regmap(priv);
> > +	if (rc < 0) {
> > +		dev_err(dev, "Failed to create regmap: %pe\n", ERR_PTR(rc));
> > +		return rc;
> > +	}
> 
> Hmm... Perhaps return dev_err_probe(...); ?

I never understood the point of dev_err_probe() when you know the return
code can never be -EPROBE_DEFER.

> -- 
> With Best Regards,
> Andy Shevchenko
> 
>

Re: [PATCH v2 net-next 04/15] net: dsa: sja1105: prepare regmap for passing to child devices

Posted by Andy Shevchenko 2 weeks, 4 days ago

On Thu, Jan 22, 2026 at 03:42:45PM +0200, Vladimir Oltean wrote:
> On Thu, Jan 22, 2026 at 02:23:18PM +0200, Andy Shevchenko wrote:
> > On Thu, Jan 22, 2026 at 12:56:43PM +0200, Vladimir Oltean wrote:

...

> > > --- a/drivers/net/dsa/sja1105/sja1105.h
> > > +++ b/drivers/net/dsa/sja1105/sja1105.h
> > 
> > >  #include <linux/dsa/8021q.h>
> > >  #include <net/dsa.h>
> > >  #include <linux/mutex.h>
> > > +#include <linux/regmap.h>
> > > +
> > >  #include "sja1105_static_config.h"
> > 
> > It looks to me that somebody missed grouping above (not this change) and it's
> > better to have it done:
> > 
> > #include <linux/dsa/8021q.h>
> > #include <linux/mutex.h>
> > #include <linux/regmap.h>
> > 
> > #include <net/dsa.h>
> > 
> > #include "sja1105_static_config.h"
> 
> That somebody would be me, but I don't consider this a relevant change
> for this patch set.

While it's true, this patch already adds blank line which suggests that the
author (would be you :-) has something like grouping in mind. Otherwise just
place a new one before net/* which might be more logical in accordance with
your justification (of not doing grouping here).

But I'm not going to fight to death for this :-) Your choice!

...

> > > +	rc = devm_sja1105_create_regmap(priv);
> > > +	if (rc < 0) {
> > > +		dev_err(dev, "Failed to create regmap: %pe\n", ERR_PTR(rc));
> > > +		return rc;
> > > +	}
> > 
> > Hmm... Perhaps return dev_err_probe(...); ?
> 
> I never understood the point of dev_err_probe() when you know the return
> code can never be -EPROBE_DEFER.

i) Smaller code; ii) no need to care about: a) deferred probe cases;
b) -ENOMEM cases.  I see only benefits here by using it.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 net-next 04/15] net: dsa: sja1105: prepare regmap for passing to child devices

Posted by Russell King (Oracle) 2 weeks, 4 days ago

On Thu, Jan 22, 2026 at 04:54:11PM +0200, Andy Shevchenko wrote:
> On Thu, Jan 22, 2026 at 03:42:45PM +0200, Vladimir Oltean wrote:
> > I never understood the point of dev_err_probe() when you know the return
> > code can never be -EPROBE_DEFER.
> 
> i) Smaller code; ii) no need to care about: a) deferred probe cases;
> b) -ENOMEM cases.  I see only benefits here by using it.

Isn't it also used for /sys/kernel/debug/devices_deferred to report to
the user ?

E.g.

supply-voltage-monitor  iio_hwmon: Failed to get channels

produced by drivers/hwmon/iio_hwmon.c::iio_hwmon_probe():

        channels = devm_iio_channel_get_all(dev);
        if (IS_ERR(channels)) {
                ret = PTR_ERR(channels);
                if (ret == -ENODEV)
                        ret = -EPROBE_DEFER;
                return dev_err_probe(dev, ret,
                                     "Failed to get channels\n");
        }


-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v2 net-next 04/15] net: dsa: sja1105: prepare regmap for passing to child devices

Posted by Andy Shevchenko 2 weeks, 4 days ago

On Thu, Jan 22, 2026 at 04:17:59PM +0000, Russell King (Oracle) wrote:
> On Thu, Jan 22, 2026 at 04:54:11PM +0200, Andy Shevchenko wrote:
> > On Thu, Jan 22, 2026 at 03:42:45PM +0200, Vladimir Oltean wrote:
> > > I never understood the point of dev_err_probe() when you know the return
> > > code can never be -EPROBE_DEFER.
> > 
> > i) Smaller code; ii) no need to care about: a) deferred probe cases;
> > b) -ENOMEM cases.  I see only benefits here by using it.
> 
> Isn't it also used for /sys/kernel/debug/devices_deferred to report to
> the user ?
> 
> E.g.
> 
> supply-voltage-monitor  iio_hwmon: Failed to get channels
> 
> produced by drivers/hwmon/iio_hwmon.c::iio_hwmon_probe():
> 
>         channels = devm_iio_channel_get_all(dev);
>         if (IS_ERR(channels)) {
>                 ret = PTR_ERR(channels);
>                 if (ret == -ENODEV)
>                         ret = -EPROBE_DEFER;
>                 return dev_err_probe(dev, ret,
>                                      "Failed to get channels\n");
>         }

Yes, that's what I imply by "deferred probe cases".

-- 
With Best Regards,
Andy Shevchenko