Re: [PATCH] component: Move host device to end of device lists on binding

[Saravana Kannan]

On Mon, May 10, 2021 at 4:59 AM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
>
> On Sat, May 8, 2021 at 9:41 AM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> >
> > The device lists are poorly ordered when the component device code is
> > used. This is because component_master_add_with_match() returns 0
> > regardless of component devices calling component_add() first. It can
> > really only fail if an allocation fails, in which case everything is
> > going bad and we're out of memory. The host device (called master_dev in
> > the code), can succeed at probe and be put on the device lists before
> > any of the component devices are probed and put on the lists.
> >
> > Within the component device framework this usually isn't that bad
> > because the real driver work is done at bind time via
> > component{,master}_ops::bind(). It becomes a problem when the driver
> > core, or host driver, wants to operate on the component device outside
> > of the bind/unbind functions, e.g. via 'remove' or 'shutdown'. The
> > driver core doesn't understand the relationship between the host device
> > and the component devices and could possibly try to operate on component
> > devices when they're already removed from the system or shut down.
> >
> > Normally, device links or probe defer would reorder the lists and put
> > devices that depend on other devices in the lists at the correct
> > location, but with component devices this doesn't happen because this
> > information isn't expressed anywhere. Drivers simply succeed at
> > registering their component or host with the component framework and
> > wait for their bind() callback to be called once the other components
> > are ready. We could make various device links between 'master_dev' and
> > 'component->dev' but it's not necessary. Let's simply move the hosting
> > device to the end of the device lists when the component device fully
> > binds. This way we know that all components are present and have probed
> > properly and now the host device has really probed so it's safe to
> > assume the host driver ops can operate on any component device.
>
> Moving a device to the end of dpm_list is generally risky in cases
> when some dependency information may be missing.
>
> For example, if there is a device depending on the hosting one, but
> that dependency is not represented by a device link or a direct
> ancestor-descendant relationship (or generally a path in the device
> dependency graph leading from one of them to the other), then moving
> it to the end of dpm_list would cause system-wide suspend to fail (the
> hosting device would be suspended before the one depending on it).
>
> That may not be a concern here, but at least it would be good to
> document why it is not a concern.

I've been reading up on the component code and at least one of the msm
drivers that use it. I've also read part of the other thread that's
going on.

If device links work, why not use them? Also, are you trying this with
fw_devlink=on?

Looks like what you are missing (I can't tell without looking at the
DT/your specific driver) is a device link from the DSI bridge to the
I2C bridge/controller? If we add that, will things work properly? If
yes, why not add that?

Also, can you please add me to all the threads on this topic (if you
reply to them) so it's easier for me to reply?

Thanks,
Saravana