RE: [PATCH v10 2/4] clk: cix: add sky1 audss clock controller

[Joakim Zhang]

Hi Brian,

> -----Original Message-----
> From: Brian Masney <bmasney@xxxxxxxxxx>
> Sent: Wednesday, July 22, 2026 8:23 PM
> To: Joakim Zhang <joakim.zhang@xxxxxxxxxxx>
> Cc: mturquette@xxxxxxxxxxxx; sboyd@xxxxxxxxxx; robh@xxxxxxxxxx;
> krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; p.zabel@xxxxxxxxxxxxxx; cix-kernel-
> upstream <cix-kernel-upstream@xxxxxxxxxxx>; linux-clk@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v10 2/4] clk: cix: add sky1 audss clock controller
> 
> EXTERNAL EMAIL
> 
> Hi Joakim,
> 
> On Wed, Jul 22, 2026 at 01:45:52AM +0000, Joakim  Zhang wrote:
> > > > +static void sky1_audss_clk_gate_disable(struct clk_hw *hw) {
> > > > +     sky1_audss_clk_gate_endisable(hw, 0);
> > >
> > > pm_runtime_put ?
> >
> > No. Runtime PM is handled by CCF: rpm_enabled is set when the provider has PM
> enabled before registration, and clk_prepare() (as well as set_rate/set_parent)
> already calls pm_runtime_resume_and_get() on the provider. That runs our
> runtime_resume before clk ops touch the CRU, so extra get/put in the clk_ops would
> be redundant. Doing get/put in .enable/.disable would also be wrong because those
> callbacks can run in atomic context and must not sleep.
> 
> OK yes I forgot about that. Yes, I see it now and agree it's not needed in the driver.
> 
> I think do the two other minor cleanups I mentioned and I think this driver is good,
> at least from my vantage point.

Thanks for the review and for confirming that part isn't needed in the driver.

I have already posted v11 with the other two minor cleanups you mentioned. Please take a look when you have a chance. If it looks good to you, a Reviewed-by tag would be much appreciated.

Thanks,
Joakim