Re: [PATCH v2 02/14] media: qcom: camss: Add PM clock support and integrate with runtime PM

[Loic Poulain]

On Mon, Apr 27, 2026 at 4:04 PM Konrad Dybcio
<konrad.dybcio@xxxxxxxxxxxxxxxx> wrote:
>
> On 4/27/26 2:43 PM, Loic Poulain wrote:
> > Add optional PM clock support to the CAMSS driver using the PM clock
> > framework. This allows CAMSS clocks to be registered once and
> > automatically managed during runtime suspend and resume.
> >
> > This is especially useful for global CAMSS clocks that are shared across
> > multiple CAMSS subnodes. Now that CAMSS is modeled as a simple-bus,
> > these clocks are automatically enabled whenever a child node becomes
> > active.
> >
> > This avoids the need for each subdevice to reference and manage the
> > shared clocks individually. A typical example is the set of clocks in
> > the top_group, which may be used by CSID, PHY, CCI, OPE, and other
> > CAMSS blocks.
> >
> > Introduce a small PM clock descriptor table in the CAMSS resources
> > structure to describe clocks and their optional rates. Initialize
> > these clocks at probe time and delegate clock ownership to the PM
> > core.
> >
> > Hook PM clock handling into the runtime PM callbacks to ensure clocks
> > are properly suspended and resumed alongside power domains and ICC
> > paths.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxxxxxxxx>
> > ---
>
> [...]
>
> > +     for (i = 0; i < CAMSS_RES_MAX && camss->res->pm_clks[i].name; i++) {
> > +             const struct camss_pm_clk *entry = &camss->res->pm_clks[i];
> > +             struct clk *clk;
> > +
> > +             clk = clk_get(dev, entry->name);
> > +             if (IS_ERR(clk)) {
> > +                     dev_warn(dev, "failed to get pm_clk %s: %pe\n",
> > +                              entry->name, clk);
> > +                     continue;
> > +             }
> > +
> > +             if (entry->rate) {
> > +                     ret = clk_set_rate(clk, entry->rate);
> > +                     if (ret)
> > +                             dev_warn(dev, "failed to set rate for pm_clk %s: %d\n",
> > +                                      entry->name, ret);
> > +             }
>
> So this makes a couple fragile assumptions:
>
> * there's only one "on/operational" rate
> * no OPP votes
>
> I would imagine that in the camss-is-the-bus model, the top-level
> device would house an OPP table.. but we have two somewhat independent
> clocks that may possibly have separate RPMH requirements for their M/N
> number of rates, which could result in M*N-long opp table

I think we can remove the clock configuration here and handle it
properly in a dedicated place once we have a strong plan.
For now, I guess we can simply rely on assigned-clock-rates in the DTS
to set a sensible default (nominal) frequency.

Regards,
Loic