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

[H. Nikolaus Schaller]

Hi Paul,

> Am 23.12.2020 um 23:14 schrieb Paul Boddie <paul at boddie.org.uk>:
> 
> On Wednesday, 23 December 2020 18:29:46 CET H. Nikolaus Schaller wrote:
>> 
>> So the (priority) list boils down to:
>> - µs timer
>> - boot speed
>> - i2c
> 
> Another patch provided here changes the clock referenced in the device tree. 
> Since I changed the I2C clock source in a previous patch, it makes sense to 
> actually use the resulting clock in the device tree.

Thanks! Applied well, but it was not sufficient.

> 
> The driver is an adaptation of the JZ4740 driver in terms of boilerplate, but 
> it incorporates the mechanisms of the legacy drivers. So, unless I carried 
> things forward incorrectly, the techniques should be familiar.

I have tried to understand and compare with the old driver

	https://git.goldelico.com/?p=letux-400.git;a=blob;f=drivers/i2c/busses/i2c-jz47xx.c;h=5afb3bdc7a0dad9c307a25572dc55c669a44e156;hb=874c29e4cc7da18308a3166c1e9542ac725a09ce

After a while and by studying the flow-charts in the new old programming manual, I realized:

* there was a typo so that any xfer did set the STO bit and not STA
* this made the i2c master engine hang and not process data and always timeout
* but then setting STA triggers an irq which ends up in accessing uninitialized i2c->msg and kernel panic
* interrupts are always enabled at the beginning of every xfer, even for xfer_write...
* the irq handler did check for !I2C_M_RD so that received data was not collected
* cleanup did disable the i2c controller after the first transfer and nobody did
  enable it again

With some hacks I could get it working to some extent. Which means that clocks
are now set up (almost) correctly.

But, I think we should rewrite the driver a little so that sending address and
data is also done interrupt driven. This means that we have to store the protocol
state (idle, send address, write data, receive data, done) somewhere and make the irq
handler walk through protocol states. With this setup, the xfer function can always
enable interrupts (or keep them enabled) and just triggers the whole process
by setting the STA bit (well, I think we can't implement I2C_M_NOSTART at all
and the driver does/should not report capability I2C_FUNC_PROTOCOL_MANGLING).

We do not need to separate into xfer_read() and xfer_write() then. And jz4730_i2c_recv()
and jz4730_i2c_send() may boil down to directly read/write the I2CDR without waiting
logic because an interrupt occurs only if there is data available or can be
sent.

Read/write is not very different any more and we can get rid of the timeout
loops. Just a single timed completion handler remains.

In summary we should only have initialization, xfer and an irq handler.

I have developed and tested such code which almost working. There is only one
issue that after sending a packet there is another interrupt pending and the
status register reports "busy". So for the moment I simply stop interrupts.

And this seems to have the effect that reading a device works only every
second attempt. This breaks finding devices during probe and I only see
the pmu (which is working quite well!). But not the rtc chip.
But it responds by i2cget. And can detect/report unused i2c addresses by
i2cdetect (by missing ACK bit).

I have attached the driver code (not a patch because the diffs are not very
helpful).

> 
>> - lcd and backlight pwm
> 
> A brief review of the PWM documentation gave nothing else away here.
> 
> Paul<0001-Fixed-the-I2C-device-s-clock-reference.patch>

Initially the display backlight was always on but did not change by setting the brightness.
Now, I have fixed something in the DT for the LCD (there was a bad base address for the lcd
controller and pinmux setup) and now the screen now completely turns black as soon as
DRM is initialized. Still no image. We may have to enable the LCD through some gpio
if I read the schematics correctly. And the missing backlight may be because the LCD
controller isn't initialized and no /dev/fb0 is created.

What I also have fixed are some small issues with the compatible strings in
drivers and device tree.

Anyways, debugging on a shell command line is much better than before. So if possible
you should dig out your '400 and give it a try to boot :)

BR,
Nikolaus

-------------- next part --------------
A non-text attachment was scrubbed...
Name: i2c-jz4730.c
Type: application/octet-stream
Size: 11478 bytes
Desc: not available
URL: <http://lists.goldelico.com/pipermail/letux-kernel/attachments/20201226/5c2c6609/attachment.obj>