Re: [PATCH v2 5/5] clk: implement sync_state support

[Brian Masney]

Hi Ulf,

On Wed, Jun 17, 2026 at 05:29:51PM +0200, Ulf Hansson wrote:
> On Wed, Jun 17, 2026 at 5:02 PM Brian Masney <bmasney@xxxxxxxxxx> wrote:
> > On Wed, Jun 17, 2026 at 04:24:05PM +0200, Ulf Hansson wrote:
> > > On Tue, Jun 16, 2026 at 11:09 PM Brian Masney <bmasney@xxxxxxxxxx> wrote:
> > > > @@ -4345,8 +4382,17 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
> > > >         core->dev = dev;
> > > >         clk_pm_runtime_init(core);
> > > >         core->of_node = np;
> > > > -       if (dev && dev->driver)
> > > > +       if (dev && dev->driver) {
> > > >                 core->owner = dev->driver->owner;
> > > > +
> > > > +               /*
> > > > +                * If a clk provider sets their own sync_state, then it needs to
> > > > +                * also call clk_sync_state(). dev_set_drv_sync_state() won't
> > > > +                * overwrite the sync_state callback, and this call will fail
> > > > +                * with -EBUSY.
> > > > +                */
> > > > +               dev_set_drv_sync_state(dev, clk_sync_state);
> > >
> > > We have cases where a device node represents a provider for multiple
> > > types of resources, like clocks, power-domains (genpds), resets, etc,
> > > as in the qcom case, for example.
> > >
> > > For power-domain provider drivers (genpd) we also try to assign the
> > > ->sync_state() callback, see of_genpd_add_provider_simple() and
> > > of_genpd_add_provider_simple(). This means the above doesn't play well
> > > with how genpd behaves, so we need to figure out a way to manage these
> > > cases.
> > >
> > > In this regard, we also have of_genpd_sync_state(), which allows a
> > > genpd provider driver to explicitly call genpd's sync state function,
> > > if/when needed.
> > >
> > > Unfortunately I am not able to suggest a detailed solution for how to
> > > move this forward at this point, as it requires some more thinking and
> > > I am heading for some vacation very soon.
> >
> > One approach I initially considered was to make it so that we can have a
> > list of sync_state callbacks that can be added to. I already did some
> > work on this, but I didn't think it was worth it for just the QC clk
> > drivers in isolation, but it would address the concern here.
> 
> Right, maybe that would work.
> 
> One more thing to somewhat consider, is the problem I have been trying
> to address for power-domains providers [1]. At least for genpd
> providers, we need a more fine grained sync state solution, which also
> must be able to co-exists with other subsystems sync state support. I
> am not sure if something like this is needed for clocks too?
> 
> [1]
> https://lore.kernel.org/all/20260508123910.114273-1-ulf.hansson@xxxxxxxxxx/

I am not aware of any clks setup that way but that's not to say they don't
exist. Stephen, and/or some of the folks from QC would know better.

Brian