[PATCH v3 02/13] driver core: Enable suppliers to implement fine grained sync_state support
Ulf Hansson
ulf.hansson at linaro.org
Wed May 13 02:04:25 PDT 2026
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.
[...]
Kind regards
Uffe
More information about the linux-arm-kernel
mailing list