Re: [PATCH] clk: rockchip: rk3588: don't disable unused I2S MCLK output gates

On Thu, Aug 6, 2026 at 7:09 AM Brian Masney <[email protected]> wrote:
>
> Hi Chen-Yu,
>
> On Thu, Aug 06, 2026 at 01:00:02AM +0800, Chen-Yu Tsai wrote:
> > On Mon, Jun 29, 2026 at 3:13 AM Heiko Stuebner <[email protected]> wrote:
> > >
> > >
> > > On Wed, 24 Jun 2026 14:39:14 +0200, Daniele Briguglio wrote:
> > > > No in-tree board references these gates yet. Boards drive the codec
> > > > MCLK through the parent I2S*_8CH_MCLKOUT, and now that the gates are
> > > > managed clocks, clk_disable_unused() turns them off at boot. On a board
> > > > that relied on firmware leaving the output enabled, that cuts the MCLK
> > > > and analog audio stops working.
> > > >
> > > > Mark the four gates CLK_IGNORE_UNUSED so an unreferenced gate keeps the
> > > > state firmware left. A board that wants the kernel to own the gate can
> > > > reference I2S*_8CH_MCLKOUT_TO_IO from DT instead.
> > > >
> > > > [...]
> > >
> > > Applied, thanks!
> > >
> > > [1/1] clk: rockchip: rk3588: don't disable unused I2S MCLK output gates
> > >       commit: 946352b2f88fd2378f0341312e47dff1e8dc2fac
> >
> > In hindsight maybe it would have been a better idea to map the existing
> > clock ID I2S*_8CH_MCLKOUT to the new gates, and add (or not add) new
> > clocks for the internal MCLK gates.
> >
> > Then you wouldn't need to update the DTs, wouldn't need this workaround,
> > and wouldn't depend on the bootloader to set the registers correctly when
> > booting an old DT.
>
> Help me understand for the future: If the approach you describe would
> have been used, then the clocks in the kernel would have been mislabeled
> in the kernel driver compared to what's actually on the SoC, correct?
> That would have been more desirable in order to keep compatibility with
> the older DTs?

Which part are you referring to about mislabeled? The DT part would be
mislabeled, sure, but you can rename the macro to a more suitable name
without changing the ID, which is the binding part that cannot be changed.

In the driver, we would change I2S*_8CH_MCLKOUT to point to the gate,
while a new I2S*_8CH_MCLKOUT_INTERNAL or something, points to the original
mux.

Depending on the clk driver (library) design, it might not even need
a clk ID if it's only internal.

And IIRC clks don't even have to be registered in order. They can be
registered, be added as orphans, and when their parent appears, get
integrated into the clk tree.

> But the older DTs can still reference the mux, correct? From the
> kernel's perspective in this scenario, the important thing is for the
> mux to select the appropriate parent. The end gate will always be left
> on. From a power management perspective, the power will be cut further
> up the clock tree as needed.

Correct, old DTs still reference the mux, not the gate, which is a
problem if the gate wasn't enabled already.

Even with this "fix", the old DTs still rely on the bootloader having
enabled the gates beforehand. The flag added is "CLK_IGNORE_UNUSED",
not "CLK_TURN_IT_ON_BUT_LEAVE_IT_UNUSED". And BTW, the gate is missing
the CLK_SET_RATE_PARENT flag.


IMO this is better than having to carry workarounds for old DTs
indefinitely. Up to you and Heiko though.


ChenYu