[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250214193813.GA244828@debian>
Date: Fri, 14 Feb 2025 20:38:13 +0100
From: Dimitri Fedrau <dima.fedrau@...il.com>
To: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
Russell King <linux@...linux.org.uk>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Stefan Eichenberger <eichest@...il.com>, netdev@...r.kernel.org,
linux-renesas-soc@...r.kernel.org
Subject: Re: [net-next] net: phy: marvell-88q2xxx: Init PHY private structure
for mv88q211x
Hi Niklas,
Am Fri, Feb 14, 2025 at 06:46:50PM +0100 schrieb Niklas Söderlund:
> When adding LED support for mv88q222x devices the PHY private data
> structure was added to the mv88q211x code path, the data structure is
> however only allocated during mv88q222x probe. This results in a nullptr
> deference for mv88q2110 devices.
>
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000001
> Mem abort info:
> ESR = 0x0000000096000004
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x04: level 0 translation fault
> Data abort info:
> ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [0000000000000001] user address but active_mm is swapper
> Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
> CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.14.0-rc1-arm64-renesas-00342-ga3783dbf2574 #7
> Hardware name: Renesas White Hawk Single board based on r8a779g2 (DT)
> pstate: 20400005 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : mv88q2xxx_config_init+0x28/0x84
> lr : mv88q2110_config_init+0x98/0xb0
> sp : ffff8000823eb9d0
> x29: ffff8000823eb9d0 x28: ffff000440942000 x27: ffff80008144e400
> x26: 0000000000001002 x25: 0000000000000000 x24: 0000000000000000
> x23: 0000000000000009 x22: ffff8000810534f0 x21: ffff800081053550
> x20: 0000000000000000 x19: ffff0004437d6800 x18: 0000000000000018
> x17: 00000000000961c8 x16: ffff0006bef75ec0 x15: 0000000000000001
> x14: 0000000000000001 x13: ffff000440218080 x12: 071c71c71c71c71c
> x11: ffff000440218080 x10: 0000000000001420 x9 : ffff8000823eb770
> x8 : ffff8000823eb650 x7 : ffff8000823eb750 x6 : ffff8000823eb710
> x5 : 0000000000000000 x4 : 0000000000000800 x3 : 0000000000000001
> x2 : 0000000000000000 x1 : 00000000ffffffff x0 : ffff0004437d6800
> Call trace:
> mv88q2xxx_config_init+0x28/0x84 (P)
> mv88q2110_config_init+0x98/0xb0
> phy_init_hw+0x64/0x9c
> phy_attach_direct+0x118/0x320
> phy_connect_direct+0x24/0x80
> of_phy_connect+0x5c/0xa0
> rtsn_open+0x5bc/0x78c
> __dev_open+0xf8/0x1fc
> __dev_change_flags+0x198/0x220
> dev_change_flags+0x20/0x64
> ip_auto_config+0x270/0xefc
> do_one_initcall+0xe4/0x22c
> kernel_init_freeable+0x2a8/0x308
> kernel_init+0x20/0x130
> ret_from_fork+0x10/0x20
> Code: b907e404 f9432814 3100083f 540000e3 (39400680)
> ---[ end trace 0000000000000000 ]---
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> SMP: stopping secondary CPUs
> Kernel Offset: disabled
> CPU features: 0x000,00000070,00801250,8200700b
> Memory Limit: none
> ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
>
> Fix this by using a generic probe function for both mv88q211x and
> mv88q222x devices that allocates the PHY private data structure, while
> only the mv88q222x probes for LED support.
>
> Fixes: a3783dbf2574 ("net: phy: marvell-88q2xxx: Add support for PHY LEDs on 88q2xxx")
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
> ---
> drivers/net/phy/marvell-88q2xxx.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
> index bad5e7b2357d..4fca9ffc90fe 100644
> --- a/drivers/net/phy/marvell-88q2xxx.c
> +++ b/drivers/net/phy/marvell-88q2xxx.c
> @@ -825,13 +825,24 @@ static int mv88q2xxx_leds_probe(struct phy_device *phydev)
> static int mv88q2xxx_probe(struct phy_device *phydev)
> {
> struct mv88q2xxx_priv *priv;
> - int ret;
>
> priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
> if (!priv)
> return -ENOMEM;
>
> phydev->priv = priv;
> +
> + return 0;
> +}
> +
> +static int mv88q222x_probe(struct phy_device *phydev)
> +{
> + int ret;
> +
> + ret = mv88q2xxx_probe(phydev);
> + if (ret)
> + return ret;
> +
> ret = mv88q2xxx_leds_probe(phydev);
> if (ret)
> return ret;
> @@ -1101,6 +1112,7 @@ static struct phy_driver mv88q2xxx_driver[] = {
> .phy_id = MARVELL_PHY_ID_88Q2110,
> .phy_id_mask = MARVELL_PHY_ID_MASK,
> .name = "mv88q2110",
> + .probe = mv88q2xxx_probe,
> .get_features = mv88q2xxx_get_features,
> .config_aneg = mv88q2xxx_config_aneg,
> .config_init = mv88q2110_config_init,
> @@ -1115,7 +1127,7 @@ static struct phy_driver mv88q2xxx_driver[] = {
> .phy_id_mask = MARVELL_PHY_ID_MASK,
> .name = "mv88q2220",
> .flags = PHY_POLL_CABLE_TEST,
> - .probe = mv88q2xxx_probe,
> + .probe = mv88q222x_probe,
> .get_features = mv88q2xxx_get_features,
> .config_aneg = mv88q2xxx_config_aneg,
> .aneg_done = genphy_c45_aneg_done,
> --
> 2.48.1
>
thanks for your patch. You could also just switch to mv88q2xxx_probe.
Then you would get HWMON and LED support while fixing this. 88Q2110
supports both.
Best regards,
Dimitri Fedrau
Powered by blists - more mailing lists