Hi Jonas,
On Mon, 7 Apr 2025 at 09:36, Jonas Karlman <jonas@...> 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@...> 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
