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

[Simon Glass]

Hi Jonas,

On Mon, 7 Apr 2025 at 09:36, Jonas Karlman <jonas at kwiboo.se> wrote:
>
> Hi Simon,
>
> On 2025-04-06 23:10, Simon Glass wrote:
> > Hi Jonas,
> >
> > On Sun, 6 Apr 2025 at 21:02, Jonas Karlman <jonas at kwiboo.se> wrote:
> >>
> >> 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.
> >>
> >
> > That's interesting. I wrote that code about 9 years ago so perhaps can
> > be forgiven for forgetting.
> >
> > Very few drivers set the DM_FLAG_ACTIVE_DMA flag, certainly not the
> > designware ones. So how would they be removed?
>
> Ahh, I see, remove() is only called for devices with ACTIVE_DMA or
> OS_PREPARE. So I guess that was probably the issue all along?

I suppose so, yes.

>
> There is some confusion that dm_remove_devices_active() does not remove
> device_active()=true devices and instead only remove ACTIVE_DMA and/or
> OS_PREPARE flagged devices.
>
> >
> >> 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.
> >
> > Hmm yes I saw that but then apparently forgot to include it.
> >
> > I can perhaps redo this patch to add the DMA flag.
>
> Maybe eth_post_probe() or similar should add the OS_PREPARE or
> ACTIVE_DMA flag for all eth-uclass, to ensure remove() and thus stop()
> gets called when dm_remove_devices_active() is called.

But not all network drivers use DMA. It should really be set by the driver.

>
> There is probably no need to add all these remove() ops, as they would
> still not have been called by dm_remove_devices_active().

Well, since we can rely on the eth uclass, that is correct, but I
still think it is better to add it, as an example to other drivers.

Regards,
Simon