diff mbox series

net: ti: am65-cpsw-nuss: Don't crash in am65_cpsw_phy_init()

Message ID 20240327170941.436610-1-alexander.sverdlin@siemens.com
State Superseded
Delegated to: Ramon Fried
Headers show
Series net: ti: am65-cpsw-nuss: Don't crash in am65_cpsw_phy_init() | expand

Commit Message

Sverdlin, Alexander March 27, 2024, 5:09 p.m. UTC 
From: Alexander Sverdlin <alexander.sverdlin@siemens.com>

am65_cpsw_ofdata_parse_phy() tries to handle the case when PHY is not
specified in DT gracefully:

am65_cpsw_nuss_port ethernet@8000000port@1: can't parse phy-handle port 1 (-2)

am65_cpsw_mdio_init() is turn is prepared for this, checks
if priv->has_phy == 0 and bails out (leaving cpsw_common->bus == NULL).

am65_cpsw_phy_init() however is not prepared for this and calls
phy_connect(cpsw_common->bus, ...) unconditionally, which leads to:

"Synchronous Abort" handler, esr 0x8600000d, far 0x0
elr: ffffffff808ab000 lr : 000000008083bde4 (reloc)

where lr points to the instruction right after bus->read() in get_phy_id().

Fixes: 9d0dca1199d1 ("net: ethernet: ti: Introduce am654 gigabit eth switch subsystem driver")
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
---
 drivers/net/ti/am65-cpsw-nuss.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Siddharth Vadapalli March 28, 2024, 5:54 a.m. UTC | #1 
On Wed, Mar 27, 2024 at 06:09:40PM +0100, A. Sverdlin wrote:
> From: Alexander Sverdlin <alexander.sverdlin@siemens.com>

In the $subject, "Don't crash.." seems to indicate the consequence of
not handling the absence of PHYs gracefully within am65_cpsw_phy_init().
So the subject should probably be reworded to emphasize this instead.
Something like:

net: ti: am65-cpsw-nuss: Update am65_cpsw_phy_init() to handle PHY absence

The rest of the patch looks good to me. With the subject reworded,

Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com>

Regards,
Siddharth.

> 
> am65_cpsw_ofdata_parse_phy() tries to handle the case when PHY is not
> specified in DT gracefully:
> 
> am65_cpsw_nuss_port ethernet@8000000port@1: can't parse phy-handle port 1 (-2)
> 
> am65_cpsw_mdio_init() is turn is prepared for this, checks
> if priv->has_phy == 0 and bails out (leaving cpsw_common->bus == NULL).
> 
> am65_cpsw_phy_init() however is not prepared for this and calls
> phy_connect(cpsw_common->bus, ...) unconditionally, which leads to:
> 
> "Synchronous Abort" handler, esr 0x8600000d, far 0x0
> elr: ffffffff808ab000 lr : 000000008083bde4 (reloc)
> 
> where lr points to the instruction right after bus->read() in get_phy_id().
> 
> Fixes: 9d0dca1199d1 ("net: ethernet: ti: Introduce am654 gigabit eth switch subsystem driver")
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> ---
>  drivers/net/ti/am65-cpsw-nuss.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c
> index 6da018c0f9d5..d1e452b6b43c 100644
> --- a/drivers/net/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ti/am65-cpsw-nuss.c
> @@ -722,6 +722,9 @@ static int am65_cpsw_phy_init(struct udevice *dev)
>  	u32 supported = PHY_GBIT_FEATURES;
>  	int ret;
>  
> +	if (!priv->has_phy || !cpsw_common->bus)
> +		return 0;
> +
>  	phydev = phy_connect(cpsw_common->bus,
>  			     priv->phy_addr,
>  			     priv->dev,
> -- 
> 2.44.0
>
diff mbox series

Patch

diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c
index 6da018c0f9d5..d1e452b6b43c 100644
--- a/drivers/net/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ti/am65-cpsw-nuss.c
@@ -722,6 +722,9 @@  static int am65_cpsw_phy_init(struct udevice *dev)
 	u32 supported = PHY_GBIT_FEATURES;
 	int ret;
 
+	if (!priv->has_phy || !cpsw_common->bus)
+		return 0;
+
 	phydev = phy_connect(cpsw_common->bus,
 			     priv->phy_addr,
 			     priv->dev,