Re: [PATCH] clk: ingenic-tcu: Properly enable registers before accessing timers

[Paul Cercueil]

Le ven., juin 10 2022 at 16:43:27 +0100, Aidan MacDonald 
<aidanmacdonald.0x0@xxxxxxxxx> a écrit :
> Stephen Boyd <sboyd@xxxxxxxxxx> writes:
>
> >  Quoting Aidan MacDonald (2022-06-03 06:47:05)
> > >  Access to registers is guarded by 
> > > ingenic_tcu_{enable,disable}_regs()
> > >  so the stop bit can be cleared before accessing a timer channel, 
> > > but
> > >  those functions did not clear the stop bit on SoCs with a global 
> > > TCU
> > >  clock gate.
> > >
> > >  Testing on the X1000 has revealed that the stop bits must be 
> > > cleared
> > >  _and_ the global TCU clock must be ungated to access timer 
> > > registers.
> > >  Programming manuals for the X1000, JZ4740, and JZ4725B specify this
> > >  behavior. If the stop bit isn't cleared, then writes to registers 
> > > do
> > >  not take effect, which can leave clocks with no defined parent when
> > >  registered and leave clock tree state out of sync with the 
> > > hardware,
> > >  triggering bugs in downstream drivers relying on TCU clocks.
> > >
> > >  Fixing this is easy: have ingenic_tcu_{enable,disable}_regs() 
> > > always
> > >  clear the stop bit, regardless of the presence of a global TCU 
> > > gate.
> > >
> > >  Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@xxxxxxxxx>
> > >  ---
> >
> >  Any Fixes: tag?
>
> Probably 4f89e4b8f121 ("clk: ingenic: Add driver for the TCU clocks")
> but I don't have docs or hardware to confirm the bug affects the 
> jz4770,
> which is the only other SoC affected by the change.
>
> I think what caused my problem was my bootloader stopping all the 
> timer
> channels. The stop bits are supposed to be zeroed at reset, so I'd 
> guess
> the jz4770 relied on that and only worked by accident.

I'll test it on JZ4770 this weekend.

> I'll send a v2 along shortly. Is it worth CC'ing stable as well?

If the bug is in jz-5.18 or earlier, yes.

Cheers,
-Paul