Re: [net-next,v2,2/8] net: macb: rename macb_default_usrio to at91_default_usrio as not all platforms have mii mode control in usrio
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-02-28 23:26:04
Also in:
linux-riscv, lkml, netdev
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- net: macb: rename macb_default_usrio to at91_default_usrio as not all platforms have mii mode control in usrio This patch renames macb_default_usrio to at91_default_usrio to better reflect that the usrio configuration is platform-specific, and introduces a new capability flag (MACB_CAPS_USRIO_HAS_MII) to gate MII mode configuration code for platforms that actually support it.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h --- a/drivers/net/ethernet/cadence/macb.h +++ b/drivers/net/ethernet/cadence/macb.h@@ -779,6 +779,7 @@ #define MACB_CAPS_DMA_64B BIT(21) #define MACB_CAPS_DMA_PTP BIT(22) #define MACB_CAPS_RSC BIT(23) #define MACB_CAPS_NO_LSO BIT(24) +#define MACB_CAPS_USRIO_HAS_MII BIT(25) /* LSO settings */ #define MACB_LSO_UFO_ENABLE 0x01diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c@@ -4614,13 +4614,15 @@ if (!(bp->caps & MACB_CAPS_USRIO_DISABLED)) { val = 0; - if (phy_interface_mode_is_rgmii(bp->phy_interface)) - val = bp->usrio->rgmii; - else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII && - (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII)) - val = bp->usrio->rmii; - else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII)) - val = bp->usrio->mii; + if (bp->caps & MACB_CAPS_USRIO_HAS_MII) { + if (phy_interface_mode_is_rgmii(bp->phy_interface)) + val = bp->usrio->rgmii; + else if (bp->phy_interface == PHY_INTERFACE_MODE_RMII && + (bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII)) + val = bp->usrio->rmii; + else if (!(bp->caps & MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII)) + val = bp->usrio->mii; + } if (bp->caps & MACB_CAPS_USRIO_HAS_CLKEN) val |= bp->usrio->refclk;
Can bp->usrio be NULL here? The new MACB_CAPS_USRIO_HAS_MII capability
gates the MII-related dereferences of bp->usrio, but bp->usrio is also
dereferenced when MACB_CAPS_USRIO_HAS_CLKEN is set.
With the change to default_gem_config later in this patch:
static const struct macb_config default_gem_config = {
...
.usrio = NULL,
...
};
A device that matches the generic 'cdns,macb' compatible string (without
specific .data) will get bp->usrio = NULL during probe. If the device
tree then has 'cdns,refclk-ext', macb_configure_caps() adds
MACB_CAPS_USRIO_HAS_CLKEN without checking if bp->usrio is valid:
macb_configure_caps() {
...
if (of_property_read_bool(np, "cdns,refclk-ext"))
bp->caps |= MACB_CAPS_USRIO_HAS_CLKEN;
...
}
Then in macb_init(), the code would dereference bp->usrio->refclk with
bp->usrio being NULL, causing a crash.
The MACB_CAPS_USRIO_HAS_MII gate only protects the MII-related
dereferences above, not the CLKEN dereference.
quoted hunk ↗ jump to hunk
@@ -4638,13 +4640,6 @@ return 0; } -static const struct macb_usrio_config macb_default_usrio = { - .mii = MACB_BIT(MII), - .rmii = MACB_BIT(RMII), - .rgmii = GEM_BIT(RGMII), - .refclk = MACB_BIT(CLKEN), -}; -
[ ... ]
quoted hunk ↗ jump to hunk
@@ -5219,6 +5214,13 @@ static int eyeq5_init(struct platform_device *pdev) return ret; } +static const struct macb_usrio_config at91_default_usrio = { + .mii = MACB_BIT(MII), + .rmii = MACB_BIT(RMII), + .rgmii = GEM_BIT(RGMII), + .refclk = MACB_BIT(CLKEN), +}; +
[ ... ]
quoted hunk ↗ jump to hunk
@@ -5432,7 +5448,7 @@ MODULE_DEVICE_TABLE(of, macb_dt_ids); .dma_burst_length = 16, .clk_init = macb_clk_init, .init = macb_init, - .usrio = &macb_default_usrio, + .usrio = NULL, .jumbo_max_len = 10240, };
-- pw-bot: cr