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

[Paul Boddie]

On Tuesday, 29 December 2020 11:38:25 CET H. Nikolaus Schaller wrote:
> 
> So we do an ingenic_tcu_timer_read() which makes the hickups...
> 
> This is supposed to return an u64 from a regmap:
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/clocksource/ingenic-timer.c#L52
> 
> Our code expands this to
> 
> static u64 notrace ingenic_tcu_timer_read(void)
> {
> 	struct ingenic_tcu *tcu = ingenic_tcu;
> 	unsigned int reload;
> 	unsigned int count;
> 
> 	if (tcu->soc_info->jz4740_regs) {
> 		regmap_read(tcu->map, TCU_REG_TCNTc(tcu->cs_channel), &count);
> 	} else {
> 		regmap_read(tcu->map, TCU_JZ4730_REG_TCNTc(tcu->cs_channel), &count);
> 		regmap_read(tcu->map, TCU_JZ4730_REG_TRDRc(tcu->cs_channel), &reload);
> 		count = (u16)reload - (u16)count;
> 	}
> 
> 	return count;
> }
> 
> Hm. 16 bit. If I get it right we access the OTCNT0 and OTDR0 at 0x10002014
> and 0x10002010. These are 32 bit registers. So why do we 16 bit truncation?
> Strangely, experiment shows that removal of the (u16) doesn't make a
> difference.

I don't remember why the 16-bit truncation is done. From the log, this might 
have been added by Lubomir (unless it was my code that somehow got propagated 
and the history has been lost), but I agree that we probably shouldn't be 
truncating the values since it would not produce the desired results in 
general.

> So here is the devmem2 of the related registers:
> 
> root at letux:~# devmem2 0x10002000
> /dev/mem opened.
> Memory mapped at address 0x77eb6000.
> Value at address 0x10002000 (0x77eb6000): 0xFFFFFF03
> root at letux:~# devmem2 0x10002010
> /dev/mem opened.
> Memory mapped at address 0x77eb4000.
> Value at address 0x10002010 (0x77eb4010): 0x23F
> root at letux:~# devmem2 0x10002014
> /dev/mem opened.
> Memory mapped at address 0x77ed8000.
> Value at address 0x10002014 (0x77ed8014): 0x9D7B
> root at letux:~# devmem2 0x10002018
> /dev/mem opened.
> Memory mapped at address 0x77ece000.
> Value at address 0x10002018 (0x77ece018): 0xFFFF0022
> root at letux:~# devmem2 0x1000201c
> /dev/mem opened.
> Memory mapped at address 0x77ef3000.
> Value at address 0x1000201C (0x77ef301c): 0x9D7B

What we see here is two timers enabled (OST1 and OST0), a reload value of 
0x23F, a counter value of 0x9D7B, and a status with IRQ enabled and an input 
clock of 2 (PCLK/64).

> root at letux:~# devmem2 0x10002010
> /dev/mem opened.
> Memory mapped at address 0x77ec5000.
> Value at address 0x10002010 (0x77ec5010): 0x23F
> root at letux:~# devmem2 0x10002010
> /dev/mem opened.
> Memory mapped at address 0x77ea4000.
> Value at address 0x10002010 (0x77ea4010): 0x23F

No changes from the above here.

> root at letux:~# devmem2 0x10002030
> /dev/mem opened.
> Memory mapped at address 0x77e09000.
> Value at address 0x10002030 (0x77e09030): 0xFFFF
> root at letux:~# devmem2 0x10002034
> /dev/mem opened.
> Memory mapped at address 0x77e19000.
> Value at address 0x10002034 (0x77e19034): 0xABA3
> root at letux:~# devmem2 0x10002038
> /dev/mem opened.
> Memory mapped at address 0x77e76000.
> Value at address 0x10002038 (0x77e76038): 0xFFFF0042
> root at letux:~# devmem2 0x1000203c
> /dev/mem opened.
> Memory mapped at address 0x77e5c000.
> Value at address 0x1000203C (0x77e5c03c): 0xD1C6
> root at letux:~# devmem2 0x10002030
> /dev/mem opened.
> Memory mapped at address 0x77e88000.
> Value at address 0x10002030 (0x77e88030): 0xFFFF
> root at letux:~# devmem2 0x10002034
> /dev/mem opened.
> Memory mapped at address 0x77e09000.
> Value at address 0x10002034 (0x77e09034): 0x9E61
> root at letux:~#
> 
> Looks as if we are using OS Timer 1? Right?

Here, the reload value is 0xFFFF and the counter is updating. The status 
indicates an IRQ condition/flag (0x40) but not IRQ enable (0x20). Meanwhile, 
the input clock is also 2 (PCLK/64).

> Now I also understand why it does not make a difference to remove the (u16)
> because the timer is running on 16 bit precision anyways and initialized
> with 2^16-1.

This does seem to be the case, although since the timer counts down from the 
reload value to zero and then underflows, being reset to the reload value, 
which is not necessarily how timers work on all devices - I'm pretty sure that 
they don't do this on PIC32 microcontrollers, for instance - I am not 
convinced that the ingenic_tcu_cevt_set_next function is correct in this 
regard:

    regmap_write(tcu->map, TCU_JZ4730_REG_TRDRc(timer->channel), next);
    regmap_write(tcu->map, TCU_JZ4730_REG_TCNTc(timer->channel), 0xffff);

Although it probably doesn't matter very much, I don't think it is correct to 
set the counter to something potentially greater than the reload value. 
Indeed, all other places set it to zero in the driver.

> OTCSR1 is 0x0042 which means PCLK/64. What frequency is PCLK?
> If it were let's say 100MHz, we still get a 16 bit overflow only
> after 40 msec.

Reading CFCR at 0x10000000, bits 11..8, will give the clock divider for PCLK, 
assuming that the PLL is enabled (see PLCR1 at 0x10000010, bit 8).

>From an earlier register dump, CFCR contained 0x0d522220, suggesting a divider 
field of 2 and thus a division of 3 (because it uses a table where position 2 
yields 3). PLCR1 contained 0x5a000520 whose bit 8 is set.

The PLL fields are as follows:

* Multiplier (31..23) is 180, yielding a multiplication of 182
* Divider (22..18) is 0, yielding a division of 2
* Divider (17..16) is 0, yielding a division of 1

The first field is written up as a divider in the manual, but the vendor code 
(and the documentation I have been writing) indicates that it is a multiplier, 
as one would expect from a PLL. The addition of 2 is suggested by the vendor 
code, and the PLL frequency description in the manual (1.2.5.2) confirms this 
and the presence of a multiplier.

I would therefore expect a PLL frequency of...

  (EXTAL * 182 / 2) / 1
= (3686400 * 182 / 2) / 1
= 335462400

PCLK should be that divided by 3, which is 111820800.

PCLK/64 would then be 1747200.

Given a reload value of 0xFFFF, presumably giving a period of 0x10000 cycles, 
the counter would underflow 26 times a second, by my estimation.

Paul