Re: [PATCH v2 1/4] designware: Use the remove() method with related drivers

On 07/04/2025 03:35, Simon Glass wrote:
> Several drivers make use of the designware Ethernet driver but do not
> implement the remove() method. Add this to keep things tidy.
> 
> Note that the Ethernet uclass calls each driver's stop() method on
> removal, so there is no need to do that here.
> 
> Signed-off-by: Simon Glass <[email protected]>
> Reported-by: Christian Kohlschütter <[email protected]>
> ---
> 
> Changes in v2:
> - Use the designware_eth_remove() method directly
> - Add gmac_rockchip too
> - Reword the commit message to mention the uclass' actions
> 
>   drivers/net/designware.c    |  2 +-
>   drivers/net/designware.h    | 12 ++++++++++++
>   drivers/net/dwmac_meson8b.c |  1 +
>   drivers/net/dwmac_s700.c    |  1 +
>   drivers/net/dwmac_socfpga.c |  1 +
>   drivers/net/gmac_rockchip.c |  1 +
>   6 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
> index 07b0f49ef58..5385849ea6b 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..bf94e50842d 100644
> --- a/drivers/net/dwmac_meson8b.c
> +++ b/drivers/net/dwmac_meson8b.c
> @@ -158,6 +158,7 @@ U_BOOT_DRIVER(dwmac_meson8b) = {
>   	.of_match	= dwmac_meson8b_ids,
>   	.of_to_plat = dwmac_meson8b_of_to_plat,
>   	.probe		= dwmac_meson8b_probe,
> +	.remove		= designware_eth_remove,
>   	.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..e78193f3376 100644
> --- a/drivers/net/dwmac_s700.c
> +++ b/drivers/net/dwmac_s700.c
> @@ -60,6 +60,7 @@ U_BOOT_DRIVER(dwmac_s700) = {
>   	.of_match = dwmac_s700_ids,
>   	.of_to_plat = dwmac_s700_of_to_plat,
>   	.probe  = dwmac_s700_probe,
> +	.remove	= designware_eth_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..dda33f90d65 100644
> --- a/drivers/net/dwmac_socfpga.c
> +++ b/drivers/net/dwmac_socfpga.c
> @@ -141,6 +141,7 @@ U_BOOT_DRIVER(dwmac_socfpga) = {
>   	.of_match	= dwmac_socfpga_ids,
>   	.of_to_plat = dwmac_socfpga_of_to_plat,
>   	.probe		= dwmac_socfpga_probe,
> +	.remove		= designware_eth_remove,
>   	.ops		= &designware_eth_ops,
>   	.priv_auto	= sizeof(struct dw_eth_dev),
>   	.plat_auto	= sizeof(struct dwmac_socfpga_plat),
> diff --git a/drivers/net/gmac_rockchip.c b/drivers/net/gmac_rockchip.c
> index 8cfeeffe95b..5c542d62bd8 100644
> --- a/drivers/net/gmac_rockchip.c
> +++ b/drivers/net/gmac_rockchip.c
> @@ -756,6 +756,7 @@ U_BOOT_DRIVER(eth_gmac_rockchip) = {
>   	.of_match = rockchip_gmac_ids,
>   	.of_to_plat = gmac_rockchip_of_to_plat,
>   	.probe	= gmac_rockchip_probe,
> +	.remove	= designware_eth_remove,
>   	.ops	= &gmac_rockchip_eth_ops,
>   	.priv_auto	= sizeof(struct dw_eth_dev),
>   	.plat_auto	= sizeof(struct gmac_rockchip_plat),

Reviewed-by: Neil Armstrong <[email protected]>