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

[Paul Boddie]

On Saturday, 26 December 2020 16:34:32 CET H. Nikolaus Schaller wrote:
> 
> I had needed some guesses what to correctly do in interrupt mode (this is
> missing in the documentation) but my driver seems to work well now in all
> "standard" cases (send bytes, read bytes and detect NACK/non-present device
> - at least on write).

A lot of the effort involved is experimentation. I spent ages with the JZ4780 
and discovering that the peripheral won't fill buffers unless an extra read 
request or a stop condition is issued, as far as I remember, which rather 
works against exactly the kind of buffer management you might expect, since 
the idea is to wait for the buffer-full condition, empty the buffer, and then 
request more data, not risk a buffer overflow by deliberately asking for too 
much data just to make the interrupt occur.

I think the state machine is a good idea. This means that the control 
conditions can be handled when interrupts occur rather than having sleep-wait 
loops. The states can have their own enum type, too, I suppose.

> What I can't test are 10-bit address and write+read commands (e.g. write 5
> bytes and then switch to read - this is something the xfer can do by
> defining protocol modifying flags). And I am not sure if NACK detection on
> read or abort by client is handled correctly. This might need a hardware
> setup with connecting some programmable i2c client (i.e. a microcontroller)
> to the debug connector and injecting specific protocol responses. Nothing
> what I want to do is is worth the effort.
> 
> But we need neither for the mipsbook.

Indeed.

> Yes, it is attached. IMHO quite streamlined now. I think we can simplify
> jz4730_i2c_xfer a little more by replacing all goto out; by a return and
> removing the ret variable.

Yes.

> I still have to test if I can now remove the irq disable/enable hacks, but
> I did no longer get messages about spurious interrupts. So it likely works
> now.

I did wonder whether the transmission of the last byte might need to be 
accompanied by the stop condition, just as the non-acknowledgement condition 
accompanies the reading of the last byte of an incoming transfer. In other 
words, upon writing to the data register and setting DRF, it might also be 
appropriate to set STO. Without doing this, the bus might still be considered 
busy and be waiting for a final transfer.

Anyway, this is some great work and a considerable improvement over what was 
there before.

Paul