[PATCH v3 02/13] driver core: Enable suppliers to implement fine grained sync_state support

[Saravana Kannan]

On Wed, May 13, 2026 at 2:05 AM Ulf Hansson <ulf.hansson at linaro.org> wrote:
>
> On Wed, 13 May 2026 at 07:01, Saravana Kannan <saravanak at kernel.org> wrote:
> >
> > On Mon, May 11, 2026 at 2:43 AM Ulf Hansson <ulf.hansson at linaro.org> wrote:
> > >
> > > On Mon, 11 May 2026 at 07:09, Saravana Kannan <saravanak at kernel.org> wrote:
> > > >
> > > > On Fri, May 8, 2026 at 5:39 AM Ulf Hansson <ulf.hansson at linaro.org> wrote:
> > > > >
> > > > > The common sync_state support isn't fine grained enough for some types of
> > > > > suppliers, like power domains for example. Especially when a supplier
> > > > > provides multiple independent power domains, each with their own set of
> > > > > consumers. In these cases we need to wait for all consumers for all the
> > > > > provided power domains before invoking the supplier's ->sync_state().
> > > > >
> > > > > To allow a more fine grained sync_state support to be implemented on per
> > > > > supplier's driver basis, let's add a new optional callback. As soon as
> > > > > there is an update worth to consider in regards to managing sync_state for
> > > > > a supplier device, __device_links_queue_sync_state() queues the device in a
> > > > > list, allowing the new callback to be invoked when flushing the list in
> > > > > device_links_flush_sync_list().
> > > > >
> > > > > Signed-off-by: Ulf Hansson <ulf.hansson at linaro.org>
> > > > > ---
> > > > >
> > > > > Changes in v3:
> > > > >         - Re-worked the approach to use a list to queue/flush devices for
> > > > >         ->queue_sync_state(). This should make sure the device lock is being
> > > > >         held when it's needed, as pointed out by Danilo.
> > > > >
> > > >
> > > > Hi Ulf,
> > > >
> > > > Thanks for working on this!
> > > >
> > > > And please bear with my slow replies.
> > >
> > > I will try, but taking more than 2 months to reply isn't sustainable,
> > > I think. Let's hope you can get some more bandwidth for reviews when
> > > moving forward.
> > >
> > > >
> > > > > ---
> > > > >  drivers/base/base.h           | 18 ++++++++
> > > > >  drivers/base/core.c           | 77 ++++++++++++++++++++++++++---------
> > > > >  drivers/base/driver.c         |  7 ++++
> > > > >  include/linux/device.h        |  2 +
> > > > >  include/linux/device/driver.h |  7 ++++
> > > > >  5 files changed, 91 insertions(+), 20 deletions(-)
> > > > >
> > > > > diff --git a/drivers/base/base.h b/drivers/base/base.h
> > > > > index 30b416588617..c8be24af92c3 100644
> > > > > --- a/drivers/base/base.h
> > > > > +++ b/drivers/base/base.h
> > > > > @@ -196,6 +196,24 @@ static inline void dev_sync_state(struct device *dev)
> > > > >                 dev->driver->sync_state(dev);
> > > > >  }
> > > > >
> > > > > +static inline bool dev_has_queue_sync_state(struct device *dev)
> > > >
> > > > Let's please pick a better name. This is too similar to the actual
> > > > queue function you call __device_links_queue_sync_state() and is very
> > > > confusing. Maybe something that has a meaning along the lines of
> > > > "another consumer probed". So, maybe:
> > > > * consumer_probed()
> > > > * change_of_active_consumers()
> > >
> > > The whole point of naming it "queue_sync_state" was exactly to refer
> > > to __device_links_queue_sync_state(). The point is, the callback can't
> > > be invoked unless __device_links_queue_sync_state() has been called
> > > for the device first.
> > >
> > > Not sure why you think that is confusing? To me, that is rather the
> > > opposite. :-)
> >
> >
> > sync_state() is a callback telling the driver that it's okay to sync
> > the state of the hardware with the software.
> >
> > queue_sync_state() means nothing to the driver. It's just leaking an
> > internal implementation detail (name of the internal function) to the
> > driver that's not really relevant. What you are really telling the
> > driver is that some additional consumers have probed.
> >
> > > Before deciding on another name, note also that
> > > __device_links_queue_sync_state() is called when resuming sync_state
> > > from device_links_supplier_sync_state_resume() and from
> > > device_links_driver_bound(). I am not sure "consumer_probed" a good
> > > name that covers both of these cases; what do you think?
> >
> > Yes, I'm well aware of that :) The contract of sync_state() is that
> > it'll come after a point where it's safe to sync the state. Not "as
> > soon as it's safe" -- meaning no timing guarantee. Really, this is
> > just to make sure sync_state() doesn't come before late initcalls
> > (because a lot of frameworks make assumptions around it) are done and
> > before all the top level devices are added.
> >
> > So, even today, sync_state() can come a bit late. In general, the
> > driver framework doesn't guarantee immediate action. There is no
> > guarantee that a drive probe will be called as soon as it's registered
> > (if there is a device) and vice versa. The same will be true for a
> > "consumer_probed()" callback too.
> >
> > Also, your implementation is literally trying to check which consumers
> > have probed and which ones haven't. So something like
> > "consumer_probed()" or "consumer_change()" should work well and is
> > actually meaningful for a driver developer.
>
> I have no strong opinion on the name, so if you prefer
> "consumer_probed|changed", let me just pick one of them then.

Thanks for taking my input.

I'm thinking "consumer_state_changed()" would be good because:
1. Not using "probed" because we might need to send this in the future
when a consumer gets deleted.
2. Adding "state" because "consumer_changed()" could be mistaken to
mean the actual consumer(s) changed.

With that said, I don't have a strong preference between
consumer_probed(), consumer_changed() or consumer_state_changed().

Thanks,
Saravana