.../devicetree/bindings/net/cdns,macb.yaml | 54 ++++- drivers/net/ethernet/cadence/macb.h | 6 + drivers/net/ethernet/cadence/macb_main.c | 197 ++++++++++++------ 3 files changed, 186 insertions(+), 71 deletions(-)
From: Conor Dooley <conor.dooley@microchip.com>
Hey folks,
At the very least, it'd be good of the soc vendor folks could check
their platforms and see if their usrio stuff actually lines up with what
the driver currently calls "macb_default_usrio". Ours didn't and it was
a nasty surprise.
Theo, you added eyeq5 recently. Does it genuinely have the same usrio
bits as the at91 devices?
Ryan and I figured out that the sama7g5 stuff is not actually using the
same usrio bits as earlier devices, so there's now more patches in this
series to split them apart. I've not tested the split or the new
property due to lack of hardware, but Ryan has.
Cheers,
Conor.
v3:
- reorder patches
- fix smatch issue reported by Simon
- add patches reworking usrio handling of clken/refclk (and remove the
issue the llm reported in the process)
- add a new devicetree property for refclk selection, replacing the
existing one.
- drop the dts patch
CC: Valentina.FernandezAlanis@microchip.com
CC: Andrew Lunn <andrew+netdev@lunn.ch>
CC: David S. Miller <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jakub Kicinski <kuba@kernel.org>
CC: Paolo Abeni <pabeni@redhat.com>
CC: Rob Herring <robh@kernel.org>
CC: Krzysztof Kozlowski <krzk+dt@kernel.org>
CC: Conor Dooley <conor+dt@kernel.org>
CC: Daire McNamara <daire.mcnamara@microchip.com>
CC: Paul Walmsley <pjw@kernel.org>
CC: Palmer Dabbelt <palmer@dabbelt.com>
CC: Albert Ou <aou@eecs.berkeley.edu>
CC: Alexandre Ghiti <alex@ghiti.fr>
CC: Nicolas Ferre <nicolas.ferre@microchip.com>
CC: Claudiu Beznea <claudiu.beznea@tuxon.dev>
CC: Richard Cochran <richardcochran@gmail.com>
CC: Samuel Holland <samuel.holland@sifive.com>
CC: netdev@vger.kernel.org
CC: devicetree@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: linux-riscv@lists.infradead.org
CC: Dave Stevenson <dave.stevenson@raspberrypi.com>
CC: Sean Anderson <sean.anderson@linux.dev>
CC: Vineeth Karumanchi <vineeth.karumanchi@amd.com>
CC: Abin Joseph <abin.joseph@amd.com>
CC: Théo Lebrun <theo.lebrun@bootlin.com>
CC: Ryan.Wanner@microchip.com
Conor Dooley (10):
net: macb: rename macb_default_usrio to at91_default_usrio as not all
platforms have mii mode control in usrio
net: macb: split USRIO_HAS_CLKEN capability in two
dt-bindings: net: cdns,macb: replace cdns,refclk-ext with
cdns,refclk-source
net: macb: rework usrio refclk selection code
net: macb: np4 doesn't need a usrio pointer
net: macb: add mpfs specific usrio configuration
net: macb: warn on pclk use as a tsu_clk fallback
net: macb: clean up tsu clk rate acquisition
dt-bindings: net: macb: add property indicating timer adjust mode
net: macb: timer adjust mode is not supported
.../devicetree/bindings/net/cdns,macb.yaml | 54 ++++-
drivers/net/ethernet/cadence/macb.h | 6 +
drivers/net/ethernet/cadence/macb_main.c | 197 ++++++++++++------
3 files changed, 186 insertions(+), 71 deletions(-)
--
2.51.0
Hello Conor, On Tue Mar 10, 2026 at 6:17 PM CET, Conor Dooley wrote: > At the very least, it'd be good of the soc vendor folks could check > their platforms and see if their usrio stuff actually lines up with what > the driver currently calls "macb_default_usrio". Ours didn't and it was > a nasty surprise. > > Theo, you added eyeq5 recently. Does it genuinely have the same usrio > bits as the at91 devices? Sorry I missed your direct mention. After checking (because I completely ignored this part of the code before), the User I/O feature is disabled on EyeQ5 & EyeQ6H. It can be seen from DCFG1 BIT(9). It was invisible because the USRIO register turns read-only when User I/O is disabled. 1. So I thought about adding runtime detection. 2. But then having eyeq5_config->usrio made no sense so I dropped it. 3. And then I thought that a config having usrio being NULL should imply MACB_CAPS_USRIO_DISABLED to ensure we don't NULL dereference the bp->usrio pointer. #1 is useless for EyeQ combined with #2 and #3, but it should be useful for the many compatibles that inherited the wrong default value of at91_default_usrio. I am sending those three patches as a reply, feel free to pick them up if you consider them useful. They apply on top of your series and have been tested on EyeQ5. Thanks Conor, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Thu, Mar 12, 2026 at 11:09:24AM +0100, Théo Lebrun wrote: > Hello Conor, > > On Tue Mar 10, 2026 at 6:17 PM CET, Conor Dooley wrote: > > At the very least, it'd be good of the soc vendor folks could check > > their platforms and see if their usrio stuff actually lines up with what > > the driver currently calls "macb_default_usrio". Ours didn't and it was > > a nasty surprise. > > > > Theo, you added eyeq5 recently. Does it genuinely have the same usrio > > bits as the at91 devices? > > Sorry I missed your direct mention. After checking (because I completely > ignored this part of the code before), the User I/O feature is disabled > on EyeQ5 & EyeQ6H. It can be seen from DCFG1 BIT(9). It was invisible > because the USRIO register turns read-only when User I/O is disabled. Ye, my assumption was that on your platform and sifive it's not enabled but didn't have the behaviour of np4 where accesses don't complete. > > 1. So I thought about adding runtime detection. > > 2. But then having eyeq5_config->usrio made no sense so I dropped it. > > 3. And then I thought that a config having usrio being NULL should > imply MACB_CAPS_USRIO_DISABLED to ensure we don't NULL dereference > the bp->usrio pointer. > > #1 is useless for EyeQ combined with #2 and #3, but it should be useful > for the many compatibles that inherited the wrong default value of > at91_default_usrio. Yeah, that's a good shout. I never checked to see if this had a control. > I am sending those three patches as a reply, feel free to pick them up > if you consider them useful. They apply on top of your series and have > been tested on EyeQ5. Ye, I think they're a good idea. Thanks for sending em :)
DCFG1 (design config 1 register) carries a bit indicating whether User
I/O feature has been enabled or not. The MACB/GEM driver has a cap flag
indicating that HW has the feature disabled (default is enabled). Add
the missing connection between DCFG1 bit and MACB_CAPS_USRIO_DISABLED.
Indirect impact: avoid useless writel() on USERIO register; this is not
an important fix because USERIO is anyway read-only when feature is
disabled.
If for some reason a compatible sets USRIO_DISABLED but DCFG1 indicates
it is enabled, we still keep the disabled capability flag. This ensures
we don't break "cdns,np4-macb" that sets the flag from compatible match
data.
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
drivers/net/ethernet/cadence/macb.h | 2 ++
drivers/net/ethernet/cadence/macb_main.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 8e5305f9a754..16527dbab875 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -524,6 +524,8 @@
#define GEM_IRQCOR_SIZE 1
#define GEM_DBWDEF_OFFSET 25
#define GEM_DBWDEF_SIZE 3
+#define GEM_USERIO_OFFSET 9
+#define GEM_USERIO_SIZE 1
#define GEM_NO_PCS_OFFSET 0
#define GEM_NO_PCS_SIZE 1
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 6ac68731784f..ecab7fdc962d 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4481,6 +4481,8 @@ static void macb_configure_caps(struct macb *bp,
bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
if (GEM_BFEXT(NO_PCS, dcfg) == 0)
bp->caps |= MACB_CAPS_PCS;
+ if (!(dcfg & GEM_BIT(USERIO)))
+ bp->caps |= MACB_CAPS_USRIO_DISABLED;
dcfg = gem_readl(bp, DCFG12);
if (GEM_BFEXT(HIGH_SPEED, dcfg) == 1)
bp->caps |= MACB_CAPS_HIGH_SPEED;
--
2.53.0
bp->usrio is copied directly from dt_conf->usrio in macb_probe().
If dt_conf->usrio is NULL, we do not want to land in USRIO write
codepaths which dereference bp->usrio. Inherit automatically
MACB_CAPS_USRIO_DISABLED to avoid those.
This means a macb_config that wants to disable usrio can simply drop
its .usrio field, rather than add the disabled capability explicitly.
Nit: drop the dt_conf NULL check because the pointer is always valid.
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
drivers/net/ethernet/cadence/macb_main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index ecab7fdc962d..972499aa4191 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4470,8 +4470,10 @@ static void macb_configure_caps(struct macb *bp,
{
u32 dcfg;
- if (dt_conf)
- bp->caps = dt_conf->caps;
+ bp->caps = dt_conf->caps;
+
+ if (!dt_conf->usrio)
+ bp->caps |= MACB_CAPS_USRIO_DISABLED;
if (hw_is_gem(bp->regs, bp->native_io)) {
bp->caps |= MACB_CAPS_MACB_IS_GEM;
--
2.53.0
USRIO is disabled on this platform, drop its inherited usrio config.
We will end up with MACB_CAPS_USRIO_DISABLED on this platform:
- We have no config->usrio so macb_configure_caps() deduces that the
feature is disabled.
- Anecdotally, we would also land in the runtime detection codepath
that reads DCFG1.
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
drivers/net/ethernet/cadence/macb_main.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 972499aa4191..0e2200cf4206 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -5592,7 +5592,6 @@ static const struct macb_config eyeq5_config = {
.clk_init = macb_clk_init,
.init = eyeq5_init,
.jumbo_max_len = 10240,
- .usrio = &at91_default_usrio,
};
static const struct macb_config raspberrypi_rp1_config = {
--
2.53.0
© 2016 - 2026 Red Hat, Inc.