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

[H. Nikolaus Schaller]

Hi Paul,

> Am 28.12.2020 um 18:23 schrieb Paul Boddie <paul at boddie.org.uk>:
> 
> On Sunday, 27 December 2020 23:25:30 CET H. Nikolaus Schaller wrote:
>> 
>> So we have to spend a little more time in reverse engineering the behaviour
>> of the SR flags and their relation to interrupts.
>> 
>> Hm. Is it possible to read the interrupt request register without triggering
>> an interrupt? Then I can easily try to deduce from examples which bits are
>> or-ed together to form the I2C-IRQ... Should be ICSR at 0x10001000, right?
>> And there BIT(1). Even if interrupts are disabled by the kernel. I just
>> have to enable IEN in the I2CCR.
> 
> Sorry to be slow in getting back to you!
> 
> Yes, it should be possible to examine ICSR (at 0x10001000), testing the I2C 
> bit (bit 1), to see whether a condition applies to the peripheral.
> 
> Although some manuals for CPUs can be a bit vague about whether a condition 
> will occur if interrupts are disabled, I seem to remember that the JZ-series 
> will allow interrupt conditions to occur regardless of whether interrupt 
> requests (IRQs) may occur.
> 
> It seems that the pending register (ICPR, at 0x10001010) would be used to test 
> only for conditions that are not masked, with the mask registers effectively 
> silencing conditions present in ICSR so that they would not appear in ICPR.
> 
> At the top level, of course, is the CPU's own interrupt register and handling, 
> but we shouldn't have to worry too much about that.

Yes, that worked. What I have found is that interrupts are only generated
if I2CSR.BUSY is set. This is activated by sending a STA condition and ends
with sending the STO.

The other status bits have no direct influence. Or I can't see it.
To further analyse it would probably be necessary to slow down the i2c bus
speed to the minimum. So that sending a STA bit or data byte takes longer
than the printk. But that is far beyond my time budget. 

Anyways it might be possible to implement I2C_M_NOSTART and I2C_M_STOP except
for the first and the last message in an xfer(). If this is tried, the
interrupts will probably lock up...

It also appears as if the 1.5.3 Read Operation flowchart is a little
incomplete that one should send an STO at the end...

And what is also not clear if the interrupt/state machine can stall if
I2C_M_REV_DIR_ADDR is set and both, the chip and the jz4730 are sending or
receiving.

To make most of these things work I have rearranged the code so that all
activities are done in the irq code so that messages can be spliced together
with or without STA in between and with or without sending the address.
And it becomes possible to switch between write and read on the fly.
See:

https://www.kernel.org/doc/Documentation/i2c/i2c-protocol

It requires a single wait_for_completion_timeout() for the whole set of
xfer messages.

This approach also seems to relieve from the spurious interrupts problem.

What I have to fight against is that the irq code now directly operates
on the messages array and that becomes invalid after a timeout because
it is memory not owned by the irq handler or the driver. Could even be
on stack of the caller of xfer().

And NACK events are also tricky to handle.

Otherwise the reworked code starts to work. I was already able to read
the hwclock (once :).

I still have the printk sched_clock timer issue. Which also may have
an influence since I don't know if it is used by wait_for_completion_timeout()
and triggers "early" timeout. Then I would try to debug this in the i2c
driver...

Here is what a printk on sched_clock and jiffies shows:

{
int i;
for(i=0; i<20; i++)
	printk("%s: %llu %lu\n", __func__, sched_clock(), jiffies);
}

[    2.115138] adapter_test: 2114010415 4294937381
[    2.258854] adapter_test: 2257829860 4294937381
[    2.402586] adapter_test: 2401545138 4294937381
[    2.546336] adapter_test: 2545260415 4294937381
[    2.690052] adapter_test: 2689010415 4294937381
[    1.695989] adapter_test: 2832725693 4294937381
[    1.839722] adapter_test: 1838663193 4294937381
[    1.989531] adapter_test: 1988020832 4294937382
[    2.133784] adapter_test: 2132708332 4294937382
[    2.277482] adapter_test: 2276458332 4294937382
[    2.421215] adapter_test: 2420156249 4294937382
[    2.564930] adapter_test: 2563871526 4294937382
[    2.708628] adapter_test: 2707586804 4294937382
[    1.714565] adapter_test: 1713524304 4294937382
[    1.858298] adapter_test: 1857239582 4294937382
[    2.008020] adapter_test: 2006527776 4294937383
[    2.152239] adapter_test: 2151197915 4294937383
[    2.295954] adapter_test: 2294913193 4294937383
[    2.439687] adapter_test: 2438628471 4294937383
[    2.583420] adapter_test: 2582361110 4294937383

As you can see the jiffies are incrementing nicely
while the sched_clock() returns randomized nonsense
around the real value.

And there is one case where it jumped between preparing
the printk and doing the printk:

[    1.695989] adapter_test: 2832725693 4294937381

Otherwise, there is ca. 1ms time difference.

To me the pattern looks like integer overflow during
multiplication / division or expansion from 32 bit
to 64 bit time value.

Question: how does sched_clock() work on the jz4730?

Obviously it does not use the simple jiffies based clock:

https://elixir.bootlin.com/linux/latest/source/kernel/sched/clock.c#L64

It likely does use

https://elixir.bootlin.com/linux/latest/source/kernel/time/sched_clock.c#L82

But I could not identify a call to sched_clock_register()
or how the setup works. And which registers sched_clock() reads.

So we probably should focus on that first before I
continue with the i2c driver.

BR,
Nikolaus