diff mbox series

[net-next] net: phy: marvell-88q2xxx: Init PHY private structure for mv88q211x

Message ID 20250214174650.2056949-1-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
State Under Review
Delegated to: Geert Uytterhoeven
Headers show
Series [net-next] net: phy: marvell-88q2xxx: Init PHY private structure for mv88q211x | expand

Commit Message

Niklas Söderlund Feb. 14, 2025, 5:46 p.m. UTC 
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@ragnatech.se>
---
 drivers/net/phy/marvell-88q2xxx.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Comments

Andrew Lunn Feb. 14, 2025, 5:58 p.m. UTC | #1 
On Fri, Feb 14, 2025 at 06:46:50PM +0100, Niklas Söderlund wrote:
> 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.
> 
> 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@ragnatech.se>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Dimitri Fedrau Feb. 14, 2025, 7:38 p.m. UTC | #2 
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@ragnatech.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
Andrew Lunn Feb. 14, 2025, 8 p.m. UTC | #3 
> 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.

It would be good to get that tested. The less special cases we have in
a driver the better.

	Andrew
Geert Uytterhoeven Feb. 18, 2025, 9:25 a.m. UTC | #4 
Hi Niklas,

On Fri, 14 Feb 2025 at 18:50, Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> 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@ragnatech.se>

Thanks, this fixes the crash during boot on Gray Hawk Single
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert
patchwork-bot+netdevbpf@kernel.org Feb. 18, 2025, 2:40 p.m. UTC | #5 
Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri, 14 Feb 2025 18:46:50 +0100 you wrote:
> 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)
> 
> [...]

Here is the summary with links:
  - [net-next] net: phy: marvell-88q2xxx: Init PHY private structure for mv88q211x
    https://git.kernel.org/netdev/net-next/c/4991b88c2514

You are awesome, thank you!
diff mbox series

Patch

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,