[PATCH] i2c-mux-pca954x: use OF match table
From: Alvin Šipraga
Allow the mux driver to match against device tree compatible strings.
Signed-off-by: Alvin Šipraga
---
drivers/i2c/muxes/i2c-mux-pca954x.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c
b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 6c21b92860f0..fef95fa57eb7 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -255,9 +255,23 @@ static int pca954x_probe(struct device *dev)
return ret;
}
+static const struct of_device_id pca954x_dt_ids[] = {
+ { .compatible = "nxp,pca9540", .data = (const void *)pca_9540 },
+ { .compatible = "nxp,pca9542", .data = (const void *)pca_9542 },
+ { .compatible = "nxp,pca9543", .data = (const void *)pca_9543 },
+ { .compatible = "nxp,pca9544", .data = (const void *)pca_9544 },
+ { .compatible = "nxp,pca9545", .data = (const void *)pca_9545 },
+ { .compatible = "nxp,pca9546", .data = (const void *)pca_9546 },
+ { .compatible = "nxp,pca9547", .data = (const void *)pca_9547 },
+ { .compatible = "nxp,pca9548", .data = (const void *)pca_9548 },
+ {}
+};
+MODULE_DEVICE_TABLE(of, pca954x_of_match);
+
static struct driver pca954x_driver = {
.name = "pca954x",
.probe = pca954x_probe,
+ .of_compatible = DRV_OF_COMPAT(pca954x_dt_ids),
.id_table = pca954x_id,
};
device_i2c_driver(pca954x_driver);
---
base-commit: cd2f24268402207a20467ddc41be9865a73ca4f8
change-id: 20231222-pca954x-of-dd6da5a25622
[PATCH 1/4] net: fec_imx: reverse registration order of mdiobus and edev
From: Alvin Šipraga This is necessary so that on systems with MDIO-connected Etheret switches, DSA can find the master edev during switch registration. Otherwise the switch setup will fail. Signed-off-by: Alvin Šipraga --- drivers/net/fec_imx.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/net/fec_imx.c b/drivers/net/fec_imx.c index 203a2a8aa191..75a65962820b 100644 --- a/drivers/net/fec_imx.c +++ b/drivers/net/fec_imx.c @@ -895,18 +895,18 @@ static int fec_probe(struct device *dev) fec->miibus.priv = fec; fec->miibus.parent = dev; - ret = mdiobus_register(>miibus); + ret = eth_register(edev); if (ret) goto free_receive_packets; - ret = eth_register(edev); + ret = mdiobus_register(>miibus); if (ret) - goto unregister_mdio; + goto unregister_eth; return 0; -unregister_mdio: - mdiobus_unregister(>miibus); +unregister_eth: + eth_unregister(edev); free_receive_packets: fec_free_receive_packets(fec, FEC_RBD_NUM, FEC_MAX_PKT_SIZE); free_xbd: -- 2.43.0
[PATCH 0/4] fix barebox support for MDIO-connected Realtek Ethernet switches
While bringing up a new board with an MDIO-connected RTL8365MB, I had to make some changes to barebox to get network boot to work. This small series addresses all of the problems I encountered along the way. Signed-off-by: Alvin Šipraga --- Alvin Šipraga (4): net: fec_imx: reverse registration order of mdiobus and edev net: dsa: realtek: fix support for MDIO-connected switches net: dsa: realtek: unify ds_ops net: mdio_bus: associate OF nodes with their devices drivers/net/fec_imx.c | 10 ++-- drivers/net/phy/mdio_bus.c | 4 ++ drivers/net/realtek-dsa/realtek-mdio.c | 105 + drivers/net/realtek-dsa/realtek-smi.c | 2 +- drivers/net/realtek-dsa/realtek.h | 3 +- drivers/net/realtek-dsa/rtl8365mb.c| 24 +--- drivers/net/realtek-dsa/rtl8366rb.c| 23 +--- 7 files changed, 97 insertions(+), 74 deletions(-) --- base-commit: 934cb2146e5b34eb45c81e3f9bc9b63c95701e43 change-id: 20231222-realtek-mdio-fix-2672b2ef4ae6
[PATCH 4/4] net: mdio_bus: associate OF nodes with their devices
From: Alvin Šipraga barebox deep-probe will walk the device tree to ensure dependent devices have been probed. In so doing, it uses the device_node::dev pointer to check whether a given node has a device; if not, a device is created on demand. The behaviour is recursive, so parent nodes without an associated device will also have devices created on their behalf. The recursion stops when a parent with a device is found. Weird behaviour can ensure if, when devices with a device_node are registered, the device_node::dev field is not populated. This patch addresses a niche, benign, but also noisy error observed as a result of this behaviour. One mostly harmless consequence is that spurious devices can get created when a suitable one already exists. In my case, I have an MDIO-connected Ethernet switch with an internal MDIO bus and some internal PHYs. With deep-probe, but without these changes, the devinfo output looks as follows: `-- 30be.ether...@30be.of `-- eth0 `-- miibus0 `-- mdio0-dev1d `-- 0x-0x003f ( 64 Bytes): /dev/mdio0-phy1d `-- 30be.ethernet@30be:mdio:ethernet-switch@1d:ports:p...@6.of `-- 30be.ethernet@30be:mdio:ethernet-switch@1d:ports:p...@0.of `-- eth1 `-- 30be.ethernet@30be:mdio:ethernet-switch@1d:ports:p...@1.of `-- eth2 `-- 30be.ethernet@30be:mdio:ethernet-switch@1d:ports:p...@2.of `-- eth3 `-- 30be.ethernet@30be:mdio:ethernet-switch@1d:ports:p...@3.of `-- eth4 `-- miibus1 `-- mdio1-phy00 `-- 0x-0x003f ( 64 Bytes): /dev/mdio1-phy00 `-- mdio1-phy01 `-- 0x-0x003f ( 64 Bytes): /dev/mdio1-phy01 `-- mdio1-phy02 `-- 0x-0x003f ( 64 Bytes): /dev/mdio1-phy02 `-- mdio1-phy03 `-- 0x-0x003f ( 64 Bytes): /dev/mdio1-phy03 `-- 30be.ethernet@30be:mdio.of `-- 30be.ethernet@30be:mdio:ethernet-swi...@1d.of `-- 30be.ethernet@30be:mdio:ethernet-switch@1d:mdio.of Notice the last three devices with generic names; they are spurious creations of the deep-probe algorithm. In fact, the devices have already been created: real devspurious dev miibus0 30be.ethernet@30be:mdio.of mdio0-devid1d 30be.ethernet@30be:mdio:ethernet-swi...@1d.of miibus1 30be.ethernet@30be:mdio:ethernet-switch@1d:mdio.of The only reason there aren't even more spurious devices created is that deep-probe stopped at 30be.ether...@30be.of, a platform device whose device_node had its dev field populated correctly. But these so-called real devices are all created by mdio_bus, which only links one way, i.e. sets device::of_node, but not device_node::dev. This issue probably goes unnoticed on most boards because, while the call to of_device_ensure_probed() returns -ENODEV on the bottom listed node, the code in phy.c doesn't check the return code, and the real devices have already been probed, so no harm is done. I observed much more surprising behaviour on my board because the switch I am using, an RTL8365MB, has multiple possible management interfaces, for which barebox has two drivers: realtek-smi and realtek-mdio (for SMI- and MDIO-connected switches, respectively). The compatible strings are the same, "realtek,rtl8365mb", but the bus types are different: the former is a platform driver, the latter a phy (read: mdio) driver. In my bootloader I have both drivers built in, because some of my boards use SMI while others use MDIO. When I would run the command 'boot bnet', bringing up my network interface, DSA would call phy_device_connect() to connect my edev with its corresponding phy, and a deep-probe would kick off because the phy's parent's device_node::dev field was not populated. And during the creation of the spurious device for my (already probed) Ethernet switch, the platform code would find a compatible string match with realtek-smi and try to probe the device with that driver: Booting entry 'bnet' ERROR: gpiolib: _gpio_request: gpio-233 (30be.ethernet@30be:mdio:ethernet-swi...@1d.of-reset) status -16 ERROR: realtek-smi 30be.ethernet@30be:mdio:ethernet-swi...@1d.of: error Device or resource busy: failed to get RESET GPIO ERROR: realtek-smi 30be.ethernet@30be:mdio:ethernet-swi...@1d.of: probe failed: Device or resource busy As stated above, my switch is connected over MDIO, not SMI, so I really should not be seeing any "realtek-smi" in my log. Nevertheless, because phy.c doesn't check for errors, the errors are benign and my network boot runs just fine. Dumping the stack around the above error print in the realtek-smi driver reveals exactly why things are going wrong:
[PATCH 3/4] net: dsa: realtek: unify ds_ops
From: Alvin Šipraga
Now that neither interface driver requires .phy_read or .phy_write, the
ops are the same and some code can be deleted. No functional change.
Signed-off-by: Alvin Šipraga
---
drivers/net/realtek-dsa/realtek-mdio.c | 2 +-
drivers/net/realtek-dsa/realtek-smi.c | 2 +-
drivers/net/realtek-dsa/realtek.h | 3 +--
drivers/net/realtek-dsa/rtl8365mb.c| 11 ++-
drivers/net/realtek-dsa/rtl8366rb.c| 10 ++
5 files changed, 7 insertions(+), 21 deletions(-)
diff --git a/drivers/net/realtek-dsa/realtek-mdio.c
b/drivers/net/realtek-dsa/realtek-mdio.c
index 2b8661b95677..4fc2295b1b5f 100644
--- a/drivers/net/realtek-dsa/realtek-mdio.c
+++ b/drivers/net/realtek-dsa/realtek-mdio.c
@@ -236,7 +236,7 @@ static int realtek_mdio_probe(struct phy_device *mdiodev)
priv->ds->dev = dev;
priv->ds->num_ports = priv->num_ports;
priv->ds->priv = priv;
- priv->ds->ops = var->ds_ops_mdio;
+ priv->ds->ops = var->ds_ops;
ret = realtek_dsa_init_tagger(priv);
if (ret)
diff --git a/drivers/net/realtek-dsa/realtek-smi.c
b/drivers/net/realtek-dsa/realtek-smi.c
index f93024ace516..da150dbc5d8b 100644
--- a/drivers/net/realtek-dsa/realtek-smi.c
+++ b/drivers/net/realtek-dsa/realtek-smi.c
@@ -450,7 +450,7 @@ static int realtek_smi_probe(struct device *dev)
priv->ds->dev = dev;
priv->ds->num_ports = priv->num_ports;
priv->ds->priv = priv;
- priv->ds->ops = var->ds_ops_smi;
+ priv->ds->ops = var->ds_ops;
ret = realtek_dsa_init_tagger(priv);
if (ret)
diff --git a/drivers/net/realtek-dsa/realtek.h
b/drivers/net/realtek-dsa/realtek.h
index ac84b18cdd14..dbca9494627a 100644
--- a/drivers/net/realtek-dsa/realtek.h
+++ b/drivers/net/realtek-dsa/realtek.h
@@ -69,8 +69,7 @@ struct realtek_ops {
};
struct realtek_variant {
- const struct dsa_switch_ops *ds_ops_smi;
- const struct dsa_switch_ops *ds_ops_mdio;
+ const struct dsa_switch_ops *ds_ops;
const struct realtek_ops *ops;
unsigned int clk_delay;
u8 cmd_read;
diff --git a/drivers/net/realtek-dsa/rtl8365mb.c
b/drivers/net/realtek-dsa/rtl8365mb.c
index 0f4c471715d1..588998235827 100644
--- a/drivers/net/realtek-dsa/rtl8365mb.c
+++ b/drivers/net/realtek-dsa/rtl8365mb.c
@@ -1225,13 +1225,7 @@ static int rtl8365mb_detect(struct realtek_priv *priv)
return 0;
}
-static const struct dsa_switch_ops rtl8365mb_switch_ops_smi = {
- .port_pre_enable = rtl8365mb_phylink_mac_config,
- .port_disable = rtl8365mb_phylink_mac_link_down,
- .port_enable = rtl8365mb_phylink_mac_link_up,
-};
-
-static const struct dsa_switch_ops rtl8365mb_switch_ops_mdio = {
+static const struct dsa_switch_ops rtl8365mb_switch_ops = {
.port_pre_enable = rtl8365mb_phylink_mac_config,
.port_disable = rtl8365mb_phylink_mac_link_down,
.port_enable = rtl8365mb_phylink_mac_link_up,
@@ -1247,8 +1241,7 @@ static const struct realtek_ops rtl8365mb_ops = {
};
const struct realtek_variant rtl8365mb_variant = {
- .ds_ops_smi = _switch_ops_smi,
- .ds_ops_mdio = _switch_ops_mdio,
+ .ds_ops = _switch_ops,
.ops = _ops,
.clk_delay = 10,
.cmd_read = 0xb9,
diff --git a/drivers/net/realtek-dsa/rtl8366rb.c
b/drivers/net/realtek-dsa/rtl8366rb.c
index 6fd2b1852159..35028d319ecd 100644
--- a/drivers/net/realtek-dsa/rtl8366rb.c
+++ b/drivers/net/realtek-dsa/rtl8366rb.c
@@ -1079,12 +1079,7 @@ static int rtl8366rb_detect(struct realtek_priv *priv)
return rtl8366rb_reset_chip(priv);
}
-static const struct dsa_switch_ops rtl8366rb_switch_ops_smi = {
- .port_enable = rtl8366rb_port_enable,
- .port_disable = rtl8366rb_port_disable,
-};
-
-static const struct dsa_switch_ops rtl8366rb_switch_ops_mdio = {
+static const struct dsa_switch_ops rtl8366rb_switch_ops = {
.port_enable = rtl8366rb_port_enable,
.port_disable = rtl8366rb_port_disable,
};
@@ -1098,8 +1093,7 @@ static const struct realtek_ops rtl8366rb_ops = {
};
const struct realtek_variant rtl8366rb_variant = {
- .ds_ops_smi = _switch_ops_smi,
- .ds_ops_mdio = _switch_ops_mdio,
+ .ds_ops = _switch_ops,
.ops = _ops,
.clk_delay = 10,
.cmd_read = 0xa9,
--
2.43.0
[PATCH 2/4] net: dsa: realtek: fix support for MDIO-connected switches
From: Alvin Šipraga
DSA offers drivers the option to have the core register an MDIO bus,
intended for platforms which do not have a specific OF node for the MDIO
bus or corresponding phy-handle properties on the switch port nodes.
This logic works OK, but is incidentally broken in barebox because the
dsa_switch::phys_mii_mask field is not being set. That means all reads
and writes to the internal PHYs are dropped before anything goes out on
the bus.
Notwithstanding that barebox issue, there is an ongoing discussion [1]
on the Linux mailing list as to the virtues (or lack thereof) of
realtek-mdio opting into this core functionality to begin with. The fact
is that, for SMI-connected switches, realtek-smi absolutely expects an
MDIO bus node. And the device tree bindings strongly (albeit not
strictly) assert that this node for both MDIO and SMI connected
switches. In the specific case of barebox, the driver likely has even
fewer users, and realtek-mdio is currently broken, so there is nothing
to break by strictly enforcing the presence of an MDIO node.
The same discussion [1] centers around an effort to actually unify
large, common parts of the SMI and MDIO drivers, and ditching the
asymmetric MDIO bus registration would make this effort even easier.
To that end, add a little bit more code in order to bury this particular
issue once and for all in barebox. Further cleanup can be done to reduce
the overall quantity of code, probably following whatever ends up in
Linux.
A slice dependency is also added to the parent MDIO bus to prevent the
PHY status polling from interleaving MDIO accesses during switch
management.
Link: https://lore.kernel.org/all/20231221174746.hylsmr3f7g5byrsi@skbuf/ [1]
Fixes: f9f31a9a21e4 ("net: dsa: add Realtek (rtl8365mb/rtl8366rb) switch
support")
Signed-off-by: Alvin Šipraga
---
drivers/net/realtek-dsa/realtek-mdio.c | 103 ++---
drivers/net/realtek-dsa/rtl8365mb.c| 13 -
drivers/net/realtek-dsa/rtl8366rb.c| 13 -
3 files changed, 81 insertions(+), 48 deletions(-)
diff --git a/drivers/net/realtek-dsa/realtek-mdio.c
b/drivers/net/realtek-dsa/realtek-mdio.c
index 463757711198..2b8661b95677 100644
--- a/drivers/net/realtek-dsa/realtek-mdio.c
+++ b/drivers/net/realtek-dsa/realtek-mdio.c
@@ -47,22 +47,27 @@ static int realtek_mdio_write(void *ctx, u32 reg, u32 val)
struct mii_bus *bus = priv->bus;
int ret;
- ret = bus->write(bus, priv->mdio_addr, REALTEK_MDIO_CTRL0_REG,
REALTEK_MDIO_ADDR_OP);
+ ret = mdiobus_write(bus, priv->mdio_addr, REALTEK_MDIO_CTRL0_REG,
+ REALTEK_MDIO_ADDR_OP);
if (ret)
- goto out_unlock;
+ return ret;
- ret = bus->write(bus, priv->mdio_addr, REALTEK_MDIO_ADDRESS_REG, reg);
+ ret = mdiobus_write(bus, priv->mdio_addr, REALTEK_MDIO_ADDRESS_REG,
+ reg);
if (ret)
- goto out_unlock;
+ return ret;
- ret = bus->write(bus, priv->mdio_addr, REALTEK_MDIO_DATA_WRITE_REG,
val);
+ ret = mdiobus_write(bus, priv->mdio_addr, REALTEK_MDIO_DATA_WRITE_REG,
+ val);
if (ret)
- goto out_unlock;
+ return ret;
- ret = bus->write(bus, priv->mdio_addr, REALTEK_MDIO_CTRL1_REG,
REALTEK_MDIO_WRITE_OP);
+ ret = mdiobus_write(bus, priv->mdio_addr, REALTEK_MDIO_CTRL1_REG,
+ REALTEK_MDIO_WRITE_OP);
+ if (ret)
+ return ret;
-out_unlock:
- return ret;
+ return 0;
}
static int realtek_mdio_read(void *ctx, u32 reg, u32 *val)
@@ -71,26 +76,43 @@ static int realtek_mdio_read(void *ctx, u32 reg, u32 *val)
struct mii_bus *bus = priv->bus;
int ret;
- ret = bus->write(bus, priv->mdio_addr, REALTEK_MDIO_CTRL0_REG,
REALTEK_MDIO_ADDR_OP);
+ ret = mdiobus_write(bus, priv->mdio_addr, REALTEK_MDIO_CTRL0_REG,
+ REALTEK_MDIO_ADDR_OP);
if (ret)
- goto out_unlock;
+ return ret;
- ret = bus->write(bus, priv->mdio_addr, REALTEK_MDIO_ADDRESS_REG, reg);
+ ret = mdiobus_write(bus, priv->mdio_addr, REALTEK_MDIO_ADDRESS_REG,
+ reg);
if (ret)
- goto out_unlock;
+ return ret;
- ret = bus->write(bus, priv->mdio_addr, REALTEK_MDIO_CTRL1_REG,
REALTEK_MDIO_READ_OP);
+ ret = mdiobus_write(bus, priv->mdio_addr, REALTEK_MDIO_CTRL1_REG,
+ REALTEK_MDIO_READ_OP);
if (ret)
- goto out_unlock;
+ return ret;
- ret = bus->read(bus, priv->mdio_addr, REALTEK_MDIO_DATA_READ_REG);
- if (ret >= 0) {
- *val = ret;
- ret = 0;
- }
+ ret = mdiobus_read(bus, priv->mdio_addr, REALTEK_MDIO_DATA_READ_REG);
+ if (ret < 0)
+ return ret;
-out_unlock:
- return
Re: [PATCH v2] nvmem: regmap: Fix nvmem size
On 20.12.23 13:38, Robin van der Gracht wrote:
> Hi Ahmad,
>
> On 2023-12-20 10:00, Ahmad Fatoum wrote:
>> Hello Robin,
>>
>> Thanks for the fix.
>>
>> On 20.12.23 09:29, Robin van der Gracht wrote:
>>> - if (roffset + rbytes > stride * regmap_get_max_register(map))
>>> + if (roffset + rbytes > regmap_size_bytes(map) * stride)
>>
>> Shouldn't stride on the right hand side be dropped?
>
> roffset = register index * stride.
>
> I.e. 380 for register with index 95.
>
> For stm32mp1x bsec:
> map->format.val_bytes = 4
> map->reg_stride = 4
>
> regmap_size_bytes() = map->format.val_bytes * (95 + 1) / map->reg_stride = 96
Ouch. What have I been thinking when I named regmap_size_bytes() this way...
Change looks ok then, but we should really rename regmap_size_bytes()...
>
> So the result with the stride on the right size is correct.
>
> I moved stride from left to right to be consistent with the size calculation
> in nvmem_regmap_register_with_pp()
>
> Kind regards,
> Robin
>
>
>>
>> Cheers,
>> Ahmad
>>
>>> return -EINVAL;
>>>
>>> for (i = roffset; i < roffset + rbytes; i += stride) {
>>> @@ -78,7 +78,7 @@ nvmem_regmap_register_with_pp(struct regmap *map, const
>>> char *name,
>>> config.priv = map;
>>> config.stride = 1;
>>> config.word_size = 1;
>>> - config.size = regmap_get_max_register(map) *
>>> regmap_get_reg_stride(map);
>>> + config.size = regmap_size_bytes(map) * regmap_get_reg_stride(map);
>>> config.cell_post_process = cell_post_process;
>>> config.reg_write = nvmem_regmap_write;
>>> config.reg_read = nvmem_regmap_read;
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
