Re: Component API not right for DSA?

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Wed, Sep 18, 2024 at 02:47:30PM +0100, Russell King (Oracle) wrote:
> On Wed, Sep 18, 2024 at 02:10:08PM +0300, Vladimir Oltean wrote:
> > This is all great, but then I realized that, for addressing issue #2,
> > it is no better than what we currently have. Namely, by default the tree
> > looks like this:
> > 
> ...
> > 
> > but after this operation:
> > 
> > $ echo d0032004.mdio-mii:11 > /sys/bus/mdio_bus/devices/d0032004.mdio-mii\:11/driver/unbind
> > $ cat /sys/kernel/debug/device_component/dsa_tree.0.auto
> > aggregate_device name                                  status
> > -------------------------------------------------------------
> > dsa_tree.0.auto                                     not bound
> > 
> > device name                                            status
> > -------------------------------------------------------------
> > (unknown)                                      not registered
> > d0032004.mdio-mii:10                                not bound
> > d0032004.mdio-mii:12                                not bound
> > 
> > the tree (component master) is unbound, its unbind() method calls
> > component_unbind_all(), and this also unbinds the other switches.
> 
> Correct. As author of the component helper... The component helper was
> designed for an overall device that is made up of multiple component
> devices that are themselves drivers, and _all_ need to be present in
> order for the overall device to be functional. It is not intended to
> address cases where an overall device has optional components.
> 
> The helper was originally written to address that problem for the
> Freescale i.MX IPU, which had been sitting in staging for considerable
> time, and was blocked from being moved out because of issues with this
> that weren't solvable at the time (we didn't have device links back
> then, which probably could've been used instead.)

Thanks for confirming. Although I could use the component helper in DSA
as it is now, it would ossify a limitation which I would like to remove.
The logic I would need is "aggregate device is bound as long as one
component is added, and gets notified of the addition/removal of all
other components". I guess it would be better suited to open-code this
logic in DSA.

As for the actual device_component debugfs folder that the component API
creates by default, I like the introspection into the switch tree state
that it offers when applied to DSA (it is useful especially if some
switches will go missing). Maybe I will keep the idea of having a
platform_device per dsa_switch_tree, and also create a debugfs with
similar information for it. If I also open-code this, I could also look
into adding a column with the last error code returned by each switch's
setup() function (I've lost count of how many times I had to debug
cross-chip probe issues). I will have to see.