[Letux-kernel] [PATCH 00/20] A bunch of JZ4730 fixups for letux-kernel

[Paul Boddie]

On Tuesday, 29 December 2020 17:23:31 CET H. Nikolaus Schaller wrote:
> Well, bad behaviour to answer only to one's own mails...

Well, I do it quite a lot. :-)

> > Hm. How and with which parameters is the ingenic-timer setup
> > calling sched_clock_register()?
> 
> Finally found it:
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/clocksource/ingenic-timer.c#L346
> 
> 	rate = clk_get_rate(tcu->cs_clk);
> 	sched_clock_register(ingenic_tcu_timer_read, 16, rate);
> 
> So bits = 16 and rate = tcu->cs_clk.
> 
> Do we define the cs clock wrongly in DTS? With wrong clock rate?
> Can I read that through sysfs?

As far as I know, the following properties of the tcu node are applicable:

    clocks = <&cgu JZ4730_CLK_RTC>,
             <&cgu JZ4730_CLK_EXT>,
             <&cgu JZ4730_CLK_PCLK>,
             <&cgu JZ4730_CLK_TCU>;
    clock-names = "rtc", "ext", "pclk", "tcu";

This is where I was uncertain about the previous TCU driver code (drivers/clk/
ingenic/tcu.c), since there are four clocks listed here, all but the last one 
being used to configure the timer, but that code employed seven parent clocks 
(breaking out the divided PCLK into four distinct clocks) for timer 
configuration. However, these clocks are not acquired to configure the timer 
(or not primarily), which is what I was worried about.

Reviewing the code again, I see something that I probably missed when 
modifying it: the ingenic_tcu_timer_parents which map the register-appropriate 
values for the input clocks employs the JZ4740 values, not ones suitable for 
the JZ4730 as was done before. This is definitely going to cause the wrong 
clock to be selected, I think.

So, I recommend reverting the "Simplified JZ4730 TCU driver modifications." 
patch sent earlier. I don't think there was anything particularly wrong with 
what it was doing.

Sorry for the inconvenience!

Paul