[PATCH 2/3] designware: Use the remove() method with related drivers

[Jonas Karlman]

Hi Simon,

On 2025-04-06 00:12, Simon Glass wrote:
> Several drivers make use of the designware Ethernet driver but do not
> implement the remove() method. Add this so that DMA is stopped when the
> OS is booted to avoid memory corruption, etc.

The designware Ethernet driver core should not need the remove() ops as
the eth_ops.stop() ops already should stop DMA. And the eth-uclass 
pre_remove() ops already call the eth_ops.stop() ops.

All these drivers seem to use designware_eth_ops and thus have correct
stop() ops configured and should really not need the remove() ops.

Side note, this also misses the gmac rockchip driver that uses its own
eth_ops instead of designware_eth_ops, however it also use same stop()
ops as all these drivers.

> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> Reported-by: Christian Kohlschütter <christian at kohlschutter.com>
> ---
> 
>  drivers/net/designware.c    |  2 +-
>  drivers/net/designware.h    | 12 ++++++++++++
>  drivers/net/dwmac_meson8b.c |  6 ++++++
>  drivers/net/dwmac_s700.c    |  6 ++++++
>  drivers/net/dwmac_socfpga.c |  6 ++++++
>  5 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
> index eebf14bd51a..d4d31925fca 100644
> --- a/drivers/net/designware.c
> +++ b/drivers/net/designware.c
> @@ -805,7 +805,7 @@ clk_err:
>  	return err;
>  }
>  
> -static int designware_eth_remove(struct udevice *dev)
> +int designware_eth_remove(struct udevice *dev)
>  {
>  	struct dw_eth_dev *priv = dev_get_priv(dev);
>  
> diff --git a/drivers/net/designware.h b/drivers/net/designware.h
> index e47101ccaf6..8c9d0190e03 100644
> --- a/drivers/net/designware.h
> +++ b/drivers/net/designware.h
> @@ -247,6 +247,18 @@ struct dw_eth_dev {
>  
>  int designware_eth_of_to_plat(struct udevice *dev);
>  int designware_eth_probe(struct udevice *dev);
> +
> +/**
> + * designware_eth_remove() - Remove the device
> + *
> + * Disables DMA and marks the device as remove. This must be called before
> + * booting an OS, to ensure that DMA is inactive.
> + *
> + * @dev: Device to remove
> + * Return 0 if OK, -ve on error
> + */
> +int designware_eth_remove(struct udevice *dev);
> +
>  extern const struct eth_ops designware_eth_ops;
>  
>  struct dw_eth_pdata {
> diff --git a/drivers/net/dwmac_meson8b.c b/drivers/net/dwmac_meson8b.c
> index fde4aabbace..b2152f8da9b 100644
> --- a/drivers/net/dwmac_meson8b.c
> +++ b/drivers/net/dwmac_meson8b.c
> @@ -145,6 +145,11 @@ static int dwmac_meson8b_probe(struct udevice *dev)
>  	return designware_eth_probe(dev);
>  }
>  
> +static int dwmac_meson8b_remove(struct udevice *dev)
> +{
> +	return designware_eth_remove(dev);
> +}
> +
>  static const struct udevice_id dwmac_meson8b_ids[] = {
>  	{ .compatible = "amlogic,meson-gxbb-dwmac", .data = (ulong)dwmac_setup_gx },
>  	{ .compatible = "amlogic,meson-g12a-dwmac", .data = (ulong)dwmac_setup_axg },
> @@ -158,6 +163,7 @@ U_BOOT_DRIVER(dwmac_meson8b) = {
>  	.of_match	= dwmac_meson8b_ids,
>  	.of_to_plat = dwmac_meson8b_of_to_plat,
>  	.probe		= dwmac_meson8b_probe,
> +	.remove		= dwmac_meson8b_remove,

There is no reason to add the dwmac_meson8b_remove function above, you
could just use designware_eth_remove directly here.

Same for remaining drivers below.

Regards,
Jonas

>  	.ops		= &designware_eth_ops,
>  	.priv_auto	= sizeof(struct dw_eth_dev),
>  	.plat_auto	= sizeof(struct dwmac_meson8b_plat),
> diff --git a/drivers/net/dwmac_s700.c b/drivers/net/dwmac_s700.c
> index 969d247b4f3..cfb37c3aa71 100644
> --- a/drivers/net/dwmac_s700.c
> +++ b/drivers/net/dwmac_s700.c
> @@ -44,6 +44,11 @@ static int dwmac_s700_probe(struct udevice *dev)
>  	return designware_eth_probe(dev);
>  }
>  
> +static int dwmac_s700_remove(struct udevice *dev)
> +{
> +	return designware_eth_remove(dev);
> +}
> +
>  static int dwmac_s700_of_to_plat(struct udevice *dev)
>  {
>  	return designware_eth_of_to_plat(dev);
> @@ -60,6 +65,7 @@ U_BOOT_DRIVER(dwmac_s700) = {
>  	.of_match = dwmac_s700_ids,
>  	.of_to_plat = dwmac_s700_of_to_plat,
>  	.probe  = dwmac_s700_probe,
> +	.remove	= dwmac_s700_remove,
>  	.ops    = &designware_eth_ops,
>  	.priv_auto	= sizeof(struct dw_eth_dev),
>  	.plat_auto	= sizeof(struct eth_pdata),
> diff --git a/drivers/net/dwmac_socfpga.c b/drivers/net/dwmac_socfpga.c
> index a9e2d8c0972..8e7a9975d28 100644
> --- a/drivers/net/dwmac_socfpga.c
> +++ b/drivers/net/dwmac_socfpga.c
> @@ -130,6 +130,11 @@ static int dwmac_socfpga_probe(struct udevice *dev)
>  	return designware_eth_probe(dev);
>  }
>  
> +static int dwmac_socfpga_remove(struct udevice *dev)
> +{
> +	return designware_eth_remove(dev);
> +}
> +
>  static const struct udevice_id dwmac_socfpga_ids[] = {
>  	{ .compatible = "altr,socfpga-stmmac" },
>  	{ }
> @@ -141,6 +146,7 @@ U_BOOT_DRIVER(dwmac_socfpga) = {
>  	.of_match	= dwmac_socfpga_ids,
>  	.of_to_plat = dwmac_socfpga_of_to_plat,
>  	.probe		= dwmac_socfpga_probe,
> +	.remove		= dwmac_socfpga_remove,
>  	.ops		= &designware_eth_ops,
>  	.priv_auto	= sizeof(struct dw_eth_dev),
>  	.plat_auto	= sizeof(struct dwmac_socfpga_plat),