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

[Paul Boddie]

On Monday, 28 December 2020 19:03:51 CET H. Nikolaus Schaller wrote:
> 
> 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 :).

We had a discussion off list about this, and I decided to modify the amended 
Linux driver to take into account various conditions that I think need to be 
observed to reliably perform I2C operations. Obviously, these modifications 
are not tested in the Linux kernel because my testing environment does not 
permit any observations (no UART, LCD not yet working), but my L4Re 
experiments with a working LCD and with "console" output in a window have 
permitted some observations to be made.

One thing I looked at yesterday is that the busy state of the bus has to be 
cleared before starting a new transaction. Waiting for an interrupt and 
testing the busy flag seems to do the trick, with a suitable timeout of up to 
about 10ms involved if interrupts do not occur. (Given that my L4Re program 
will mask interrupts, this might not be as complicated in an environment where 
interrupts are always being delivered.)

I had a look at the state machine in the amended Linux driver. What I wanted 
to do was to reproduce some of the tests performed in my experiments that 
guard read and write operations. For example, I found that reading needs to be 
guarded by tests for NACK (which should stop the operation) and for DRF 
(indicating valid data for reading). Similarly, writing needs to be guarded by 
tests for NACK and for !DRF (indicating that data can be written).

In my L4Re code, I loop waiting for interrupts to test these conditions, but I 
think that in the Linux driver, we might just exit the interrupt handler 
without advancing in the state machine where such conditions are not 
satisfied. Where NACK occurs, an error condition would be signalled instead.

I have attached the driver with my own modifications for comparison with 
previous versions. I cannot promise that it will work, but you should be able 
to see what I have been trying to do. For reference, the L4Re code can be 
found here:

https://hg.boddie.org.uk/Landfall/file/d58099373644/pkg/devices/lib/i2c/src/
jz4730.cc#l138

The set_address function contains the testing for the busy flag. Meanwhile, 
the read and write methods contain the other testing that happens:

https://hg.boddie.org.uk/Landfall/file/d58099373644/pkg/devices/lib/i2c/src/
jz4730.cc#l174

I hope this is informative, at least.

Paul
-------------- next part --------------
A non-text attachment was scrubbed...
Name: i2c-jz4730.c
Type: text/x-csrc
Size: 10641 bytes
Desc: not available
URL: <http://lists.goldelico.com/pipermail/letux-kernel/attachments/20210112/95856ff0/attachment-0001.bin>