[Letux-kernel] LX16/LX20 cgu issue

[H. Nikolaus Schaller]

> Am 09.06.2025 um 17:03 schrieb H. Nikolaus Schaller <hns at goldelico.com>:
> 
> A test approach would be to restore some (or all) of the old code with
> private variables and printk the diffs between old and new on the x2000...

Here is some test with printk.

There are indeed differences for od_enc and od between old and new code:

[    0.000000] ingenic_pll_recalc_rate: idx=2 name=apll
[    0.000000] ingenic_pll_recalc_rate: old_enc 00000001
[    0.000000] ingenic_pll_recalc_rate: old_od 1
[    0.000000] ingenic_pll_recalc_rate_od: new od_enc 00000000
[    0.000000] ingenic_pll_recalc_rate_od: new od 65
[    0.000000] ingenic_pll_recalc_rate_od: new od_enc 00000000
[    0.000000] ingenic_pll_recalc_rate_od: new od 1
[    0.000000] ingenic_pll_recalc_rate: od 65 od1 1 n 65 total

So I checked why old_enc and new od_enc are different...

1) upstream code
https://elixir.bootlin.com/linux/v6.15.1/source/drivers/clk/ingenic/cgu.c#L93

ctl = readl(cgu->base + pll_info->reg);

m = (ctl >> pll_info->m_shift) & GENMASK(pll_info->m_bits - 1, 0);
m += pll_info->m_offset;
n = (ctl >> pll_info->n_shift) & GENMASK(pll_info->n_bits - 1, 0);
n += pll_info->n_offset;

if (pll_info->od_bits > 0) {
	od_enc = ctl >> pll_info->od_shift;
	od_enc &= GENMASK(pll_info->od_bits - 1, 0);
}

if (pll_info->bypass_bit >= 0) {
	ctl = readl(cgu->base + pll_info->bypass_reg);

	bypass = !!(ctl & BIT(pll_info->bypass_bit));

	if (bypass)
		return parent_rate;
}

for (od = 0; od < pll_info->od_max; od++)
	if (pll_info->od_encoding[od] == od_enc)
		break;

2) lx16 code
https://git.goldelico.com/?p=letux-kernel.git;a=blob;f=drivers/clk/ingenic/cgu.c;h=db184e3b5c5e0c827c07aa91eb047d39b70af275;hb=refs/heads/letux-current#l129

ctl = readl(cgu->base + pll_info->reg);

m = (ctl >> pll_info->m_shift) & GENMASK(pll_info->m_bits - 1, 0);
m += pll_info->m_offset;
n = (ctl >> pll_info->n_shift) & GENMASK(pll_info->n_bits - 1, 0);
n += pll_info->n_offset;

if (pll_info->bypass_bit >= 0) {
	ctl = readl(cgu->base + pll_info->bypass_reg);

	bypass = !!(ctl & BIT(pll_info->bypass_bit));

	if (bypass)
		return parent_rate;
}

od = ingenic_pll_recalc_rate_od(ctl, pll_info->od_shift, pll_info->od_bits,
pll_info->od_max, pll_info->od_encoding);


Obviously in the upstream code, the ctl value has already been processed when
the pll_info->bypass_bit >= 0 code is executed where ctl is simply reused
instead of letting the compiler find that the same variable can be used...

In the new code ctl is overwritten in the pll_info->bypass_bit >= 0 case
and only then passed to ingenic_pll_recalc_rate_od(ctl, ...) with a wrong
value.

The fix is simple: use a dedicated temporary variable for processing of
the pll_info->bypass_bit >= 0 case.

That makes it work. At least for LX20 (not yet checked if LX16/CI20 are now
broken).

I'll integrate and test all this for 6.16-rc1 (and 6.15.y).

Now, itappears as if we really have a MIPS generic kernel. Well, only that
x1600 is still broken with CONFIG_SMP.

BR,
Nikolaus