Re: [PATCH] pinctrl: qcom: Enable device links to consumers

[Stephen Boyd <swboyd@xxxxxxxxxxxx>]

Quoting Linus Walleij (2019-05-23 12:26:20)
> On Thu, May 23, 2019 at 7:52 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> 
> > Also, what is the usecase for device links in pinctrl?
> 
> Most prominent is a device (such as a GPIO block or I2C
> block or something) that need to suspend before the pin
> controller itself suspend()s.

And this i2c block or GPIO block is using the pins from this pin
controller? Wouldn't the i2c or GPIO block have probe deferred if this
pin controller wasn't providing the pins yet?

> 
> > Doesn't the
> > driver core reorder the suspend list when probing devices so that
> > devices that probe defer get moved later in list and thus suspended
> > first?
> 
> AFAIK it does not, the device links however can do this so that
> the probe order does not have to use deferral at all.

I'm not too worried about using device links for probe ordering. I'm
saying that this probe defer logic in drivers/base/dd.c already takes
care of reordering the list so suspend/resume will be correct.

                /*
                 * Force the device to the end of the dpm_list since
                 * the PM code assumes that the order we add things to
                 * the list is a good order for suspend but deferred
                 * probe makes that very unsafe.
                 */
                device_pm_move_to_tail(dev);

Basically all providers will come before consumers and then suspend will
run through the list in reverse and suspend consumers first before
providers.

> But the
> links can also be added later at runtime (like when pin control
> states are requested by drivers) and that is what this patch
> does.

Ok, great! So if some consumer is only informing the provider of the
driver dependency after probe succeeds then this will help by fixing the
list order. This is what I'm looking for. It's unfortunate that devices
aren't getting all resources in probe so this could be avoided.

> 
> By way of the chicken-and-egg problem we cannot really use
> these device links much for probe ordering, but they can
> readily be used to control suspend/resume sequencing
> like this.
> 
> >I can understand that runtime suspend may be important because
> > order of suspend isn't fixed, but system suspend should be unaffected,
> > right?
> 
> AFAIK both runtime PM and system suspend use the device
> links, this was implemented especially for system suspend/resume
> and tested with the STMFX driver on STM32.

Yes that's my understanding too.

> 
> > >         pctrl->desc.npins = pctrl->soc->npins;
> > > +       pctrl->desc.link_consumers = true;
> >
> > Why is it an opt-in flag instead of a mandated feature for all pinctrl
> > providers?
> 
> I am afraid of breaking stuff. (OK maybe I am chicken...)
> 
> We slammed in device links in the DRM core and it exploded in
> our face. Because of fear of causing a similar debacle and
> having to back out all drivers that definately need this,
> I am making it opt-in for the moment.
> 
> Once we have a feeling that this is not breaking (on e.g.
> qcom) we might just make it default to link all devices
> getting pinctrl states to their pin controllers.
> 

Alright. Thanks!