[PATCH 4/4] net: dsa: remove master santiy check

[Michael Walle]

Am 2021-02-24 18:11, schrieb Vladimir Oltean:
> On Wed, Feb 24, 2021 at 05:40:42PM +0100, Michael Walle wrote:
>> Because we probe the master ourselves (and fail if there is no 
>> master),
>> it is not possible that we don't have a master device.
>> 
>> There is one catch though: device removal. We don't support that. It
>> wasn't supported neither before this patch. Because the master device
>> was only set in .pre_probe(), if a device was removed master_dev was a
>> dangling pointer and transmitting a frame cause a panic. I don't see a
>> good solution without having some sort of notify machanism when a
>> udevice is removed.
>> 
>> Signed-off-by: Michael Walle <michael at walle.cc>
>> ---
> 
> Ha, this is a tough one.
> So the DSA master is not a parent of the DSA switch, instead the SPI
> bus/whatever is. Additionally, the checks for NULL are pointless,
> because the master is not NULL, just deallocated.
> 
> In Linux we solved this by using device links:
> https://github.com/torvalds/linux/commit/07b90056cb15ff9877dca0d8f1b6583d1051f724
> 
> However it doesn't seem like we have such a generic dependency graph
> between udevices in U-Boot, do we? I could just find device_reparent,
> which is definitely not what we want.

I didn't find anything else either. Thus I just ignored it for the
moment.

> My best guess as to how to solve the DSA master removal scenario would
> be to:
> (a) scatter all DSA callbacks with device_probe(master), so even if it
>     was unbound, it will be rebound.
> (b) introduce DSA specific code in eth_pre_remove and a DSA-specific
>     pointer in eth_pdata to manually call device_remove for an attached
>     DSA switch, if that exists.
> 
> Other ideas are welcome, of course.

What is the reason to remove that device in the first place? Like is
this really a valid scenario? I really don't know when a device is
removed and if its remove, will it still be there or is it rather
a hot-plug type and rebinding it won't work anyways.

I also had (b) in mind, but again. Does it really apply? After all,
this is a bootloader ;)

-- 
-michael