* [PATCH 1/3] net: xilinx: fix load access fault
@ 2022-01-20 7:34 Andy Chiu
2022-01-20 7:34 ` [PATCH 2/3] net: xilinx: Move setup of 1G MAC to a function Andy Chiu
2022-01-20 7:34 ` [PATCH 3/3] net: xilinx: Force a probe failure if it cannot setup phy Andy Chiu
0 siblings, 2 replies; 4+ messages in thread
From: Andy Chiu @ 2022-01-20 7:34 UTC (permalink / raw)
To: monstr; +Cc: Greentime Hu, Andy Chiu, Joe Hershberger, Ramon Fried, u-boot
From: Greentime Hu <greentime.hu@sifive.com>
phy_connect() may fail by returning a NULL pointer. Thus,
axiemac_phy_init() should handle the case or we may get an access
fault when it tries to dereference it.
Signed-off-by: Greentime Hu <greentime.hu@sifive.com>
Reviewed-by: Andy Chiu <andy.chiu@sifive.com>
Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
drivers/net/xilinx_axi_emac.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/xilinx_axi_emac.c b/drivers/net/xilinx_axi_emac.c
index 2ec76d0f52..3117dae05e 100644
--- a/drivers/net/xilinx_axi_emac.c
+++ b/drivers/net/xilinx_axi_emac.c
@@ -312,6 +312,10 @@ static int axiemac_phy_init(struct udevice *dev)
/* Interface - look at tsec */
phydev = phy_connect(priv->bus, priv->phyaddr, dev, priv->interface);
+ if (!phydev) {
+ printf("phy_connect failed\n");
+ return -ENODEV;
+ }
phydev->supported &= supported;
phydev->advertising = phydev->supported;
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/3] net: xilinx: Move setup of 1G MAC to a function
2022-01-20 7:34 [PATCH 1/3] net: xilinx: fix load access fault Andy Chiu
@ 2022-01-20 7:34 ` Andy Chiu
2022-01-20 7:34 ` [PATCH 3/3] net: xilinx: Force a probe failure if it cannot setup phy Andy Chiu
1 sibling, 0 replies; 4+ messages in thread
From: Andy Chiu @ 2022-01-20 7:34 UTC (permalink / raw)
To: monstr; +Cc: Andy Chiu, Joe Hershberger, Ramon Fried, u-boot
Separate the flow out so that it would be easiler to implement error
handling logic.
Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
drivers/net/xilinx_axi_emac.c | 50 +++++++++++++++++++++--------------
1 file changed, 30 insertions(+), 20 deletions(-)
diff --git a/drivers/net/xilinx_axi_emac.c b/drivers/net/xilinx_axi_emac.c
index 3117dae05e..dbda6c70d8 100644
--- a/drivers/net/xilinx_axi_emac.c
+++ b/drivers/net/xilinx_axi_emac.c
@@ -763,38 +763,48 @@ static int axiemac_miiphy_write(struct mii_dev *bus, int addr, int devad,
return phywrite(bus->priv, addr, reg, value);
}
-static int axi_emac_probe(struct udevice *dev)
+static int axiemac_setup_emac(struct udevice *dev)
{
struct axidma_plat *plat = dev_get_plat(dev);
struct eth_pdata *pdata = &plat->eth_pdata;
struct axidma_priv *priv = dev_get_priv(dev);
int ret;
+ priv->eth_hasnobuf = plat->eth_hasnobuf;
+ priv->phyaddr = plat->phyaddr;
+ priv->phy_of_handle = plat->phy_of_handle;
+ priv->interface = pdata->phy_interface;
+
+ priv->bus = mdio_alloc();
+ priv->bus->read = axiemac_miiphy_read;
+ priv->bus->write = axiemac_miiphy_write;
+ priv->bus->priv = priv;
+
+ ret = mdio_register_seq(priv->bus, dev_seq(dev));
+ if (ret)
+ return ret;
+
+ axiemac_phy_init(dev);
+
+ return ret;
+}
+
+static int axi_emac_probe(struct udevice *dev)
+{
+ struct axidma_plat *plat = dev_get_plat(dev);
+ struct eth_pdata *pdata = &plat->eth_pdata;
+ struct axidma_priv *priv = dev_get_priv(dev);
+
priv->iobase = (struct axi_regs *)pdata->iobase;
priv->dmatx = plat->dmatx;
/* RX channel offset is 0x30 */
priv->dmarx = (struct axidma_reg *)((phys_addr_t)priv->dmatx + 0x30);
priv->mactype = plat->mactype;
- if (priv->mactype == EMAC_1G) {
- priv->eth_hasnobuf = plat->eth_hasnobuf;
- priv->phyaddr = plat->phyaddr;
- priv->phy_of_handle = plat->phy_of_handle;
- priv->interface = pdata->phy_interface;
-
- priv->bus = mdio_alloc();
- priv->bus->read = axiemac_miiphy_read;
- priv->bus->write = axiemac_miiphy_write;
- priv->bus->priv = priv;
-
- ret = mdio_register_seq(priv->bus, dev_seq(dev));
- if (ret)
- return ret;
-
- axiemac_phy_init(dev);
- }
-
- return 0;
+ if (priv->mactype == EMAC_1G)
+ return axiemac_setup_emac(dev);
+ else
+ return 0;
}
static int axi_emac_remove(struct udevice *dev)
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] net: xilinx: Force a probe failure if it cannot setup phy
2022-01-20 7:34 [PATCH 1/3] net: xilinx: fix load access fault Andy Chiu
2022-01-20 7:34 ` [PATCH 2/3] net: xilinx: Move setup of 1G MAC to a function Andy Chiu
@ 2022-01-20 7:34 ` Andy Chiu
2022-02-09 13:33 ` Andy Chiu
1 sibling, 1 reply; 4+ messages in thread
From: Andy Chiu @ 2022-01-20 7:34 UTC (permalink / raw)
To: monstr; +Cc: Andy Chiu, Greentime Hu, Joe Hershberger, Ramon Fried, u-boot
Or we may get load access faults afterward.
The `phydev` field on axi-ethernet’s private struct is not set on a
failed phy_connect():
axi_emac_probe()
=> axiemac_phy_init()
=> priv->phydev = phy_connect() <--- may fail
However, all of the following calls on `axi_emac_ops` assume a valid
`phydev` pointer. For example:
axiemac_start()
=> setup_phy()
=> phy_startup()
=> if (phydev->drv->startup) <--- deref of phydev
return phydev->drv->startup(phydev);
Thus, it would be better to fail at the driver probe and let u-boot
handle the rest (e.g. probe the driver again if needed), rather than
having access faults.
Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
Reviewed-by: Greentime Hu <greentime.hu@sifive.com>
---
drivers/net/xilinx_axi_emac.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/xilinx_axi_emac.c b/drivers/net/xilinx_axi_emac.c
index dbda6c70d8..08322aff88 100644
--- a/drivers/net/xilinx_axi_emac.c
+++ b/drivers/net/xilinx_axi_emac.c
@@ -782,10 +782,16 @@ static int axiemac_setup_emac(struct udevice *dev)
ret = mdio_register_seq(priv->bus, dev_seq(dev));
if (ret)
- return ret;
+ goto fail;
- axiemac_phy_init(dev);
+ ret = axiemac_phy_init(dev);
+ if (ret)
+ goto fail_free_mdio;
+fail_free_mdio:
+ mdio_unregister(priv->bus);
+ mdio_free(priv->bus);
+fail:
return ret;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 3/3] net: xilinx: Force a probe failure if it cannot setup phy
2022-01-20 7:34 ` [PATCH 3/3] net: xilinx: Force a probe failure if it cannot setup phy Andy Chiu
@ 2022-02-09 13:33 ` Andy Chiu
0 siblings, 0 replies; 4+ messages in thread
From: Andy Chiu @ 2022-02-09 13:33 UTC (permalink / raw)
To: Michal Simek; +Cc: Greentime Hu, Joe Hershberger, Ramon Fried, u-boot
Dear Maintainers,
I am sending this email to check if this patch was missed. I would be
really appreciated if I get any suggestion from you, thanks!
Best regards
Andy
On Thu, Jan 20, 2022 at 3:35 PM Andy Chiu <andy.chiu@sifive.com> wrote:
>
> Or we may get load access faults afterward.
>
> The `phydev` field on axi-ethernet’s private struct is not set on a
> failed phy_connect():
>
> axi_emac_probe()
> => axiemac_phy_init()
> => priv->phydev = phy_connect() <--- may fail
>
> However, all of the following calls on `axi_emac_ops` assume a valid
> `phydev` pointer. For example:
>
> axiemac_start()
> => setup_phy()
> => phy_startup()
> => if (phydev->drv->startup) <--- deref of phydev
> return phydev->drv->startup(phydev);
>
> Thus, it would be better to fail at the driver probe and let u-boot
> handle the rest (e.g. probe the driver again if needed), rather than
> having access faults.
>
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> Reviewed-by: Greentime Hu <greentime.hu@sifive.com>
> ---
>
> drivers/net/xilinx_axi_emac.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/xilinx_axi_emac.c b/drivers/net/xilinx_axi_emac.c
> index dbda6c70d8..08322aff88 100644
> --- a/drivers/net/xilinx_axi_emac.c
> +++ b/drivers/net/xilinx_axi_emac.c
> @@ -782,10 +782,16 @@ static int axiemac_setup_emac(struct udevice *dev)
>
> ret = mdio_register_seq(priv->bus, dev_seq(dev));
> if (ret)
> - return ret;
> + goto fail;
>
> - axiemac_phy_init(dev);
> + ret = axiemac_phy_init(dev);
> + if (ret)
> + goto fail_free_mdio;
>
> +fail_free_mdio:
> + mdio_unregister(priv->bus);
> + mdio_free(priv->bus);
> +fail:
> return ret;
> }
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-02-09 13:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-20 7:34 [PATCH 1/3] net: xilinx: fix load access fault Andy Chiu
2022-01-20 7:34 ` [PATCH 2/3] net: xilinx: Move setup of 1G MAC to a function Andy Chiu
2022-01-20 7:34 ` [PATCH 3/3] net: xilinx: Force a probe failure if it cannot setup phy Andy Chiu
2022-02-09 13:33 ` Andy Chiu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox