realtek-poe: Add support for PoE control over I2C

[ghost]

This adds PoE control over I2C instead of the serial console.

The protocol logic is different from the serial logic, since an I2C slave cannot initiate transfers. Instead, after each command (I2C write) sent, the master immediately fetches the response from the MCU via an I2C read via a single I2C r/w cycle. The I2C controller will if necessary wait for the slave to be ready to send its reply in the I2C read transfer.

There are no further libraries necessary to support I2C, as only iocl() calls are used on an i2c device in /dev/i2c-* We therefore always build with i2c support. For this to work, the kernel needs to be built with CONFIG_I2C_CHARDEV=y in order for the i2c character devices to be present in /dev.

I2C instead of UART is selected by simply putting e.g. a option i2c_bus '3' in the global section of the poe configuration file, where '3' gives the number of the I2C character device.

Optionally, an i2c_addr can be specified, which defaults to 0x20 as for the address in the only 2 known PoE devices with I2C.

Signed-off-by: Birger Koblitz git@birger-koblitz.de

[mrnuke]

Hi @bkobl

The first thing I see is the need to separate out the UART and I2C paths. Any part of the code that is not I2C or UART specific should not have to care if the MCU is on UART or I2C. if()/else() inside the queue handler goes the opposite direction.

Second thing I see is the I2C ioctl handles both the packet and and receive side, in a blocking manner. realtek-poe uses an asynchronous model where it fires off data and waits for things to happen. That way, we can respond to ubus requests while waiting for MCU packets. It's really cool. As a request happens, we process it, and send the response, all while the MCU communication is in progress.

With the way the I2C code is added, we're blocking program execution for the entire duration of the transfer. How long does an I2C round-trip take. How long do we block the ubus for the setup packets, and how long for each subsequent port poll? It seems that the current approach would make realtek-poe very unresponsive.

[] Is there a non-blocking I2C kernel call that we can use instead? [] How de we integrate blocking calls into the asynchronous design of realtek-poe?

[ghost]

The first thing I see is the need to separate out the UART and I2C paths. Any part of the code that is not I2C or UART specific should not have to care if the MCU is on UART or I2C. if()/else() inside the queue handler goes the opposite direction.

This is due to I2C being a synchronous protocol, whereas UART is asynchonous. There is necessarily going to be something in the queue handler that has to distinguish between the two. One could define 2 different modes: synchonous and asynchronous processing, and then do the if()s on that condition. The asynchronous code path would then do the UART transfers and any future

Second thing I see is the I2C ioctl handles both the packet and and receive side, in a blocking manner. realtek-poe uses an asynchronous model where it fires off data and waits for things to happen.

As already stated in the PR description, this is how it needs to be. I2C is a synchonous protocol (all transfers are initiated by master), while UART is asynchonous (slave can initiate transfer). The blocking is limited to < 60ms until even a started transfer times out. Note that UART is also blocking at that level of time, namely during the transfer itself, since the kernel is not preempting transfers. Also the I2C read is not really blocking until a reply is received, since the master initiates the transfer. A typical transfer takes 41 ms and if the slave does not reply within 10ms of the beginning of the transfer, the transfer times out, this causes all ioctl() to return within 10 to less than 60ms. On the time-scale of ubus, this is not really blocking anything.

I have also tested error conditions by blocking the I2C bus in hardware (just touch a strong pull-down to SDA) the code just happily ignores this, times out and retries or gets wrong checksums and continues (there is no error handling of wrong checksums in the code so far it seems):

realtek-poe: I2C TX -> 03 2c 01 01 ff ff ff ff ff ff ff 2a
realtek-poe: RX <- ff ff ff ff ff ff ff ff ff ff ff ff
realtek-poe: received reply with bad checksum
realtek-poe: I2C TX -> 00 2d 02 01 ff ff ff ff ff ff ff 29
realtek-poe: RX <- 00 2d 02 00 ff ff ff ff ff ff ff 28

[] Is there a non-blocking I2C kernel call that we can use instead? No, I have searched the net and kernel for this. I2C calls are guaranteed to return in such a short amount of time it would anyway not make sense. The requirement to answer quickly is part of the I2C protocol in order not to block the I2C bus for other devices. This is different for a serial bus with 2 equal participants. [] How de we integrate blocking calls into the asynchronous design of realtek-poe? I don't think there is a need to. Even when blocking the I2C bus completely, these calls do not block, but time out immediately.

[mrnuke]

I think you're confusing asynchronous link layer (not sharing the same clock) with asynchronous software design. I don't care what the underlying hardware does. I care that the application is responsive. The typical 41ms transfer turns to several seconds of deadlock whenever firing a burst of requests.

If you have the main uloop toggle a gpio every 100ms, what happens when I2C transactions start flowing?

[ghost]

The typical 41ms transfer turns to several seconds of deadlock whenever firing a burst of requests. No. This is not true. The entire transfer cycle takes maximum 60 milliseconds. In this sychronous design, there is only one request at a time. There are no longer bursts of requests, because they are not queued. And even then, if you would be counting the time for a complete "burst of requests", the longest I can see is 350ms at startup, and this can definitely be interrupted in the middle, i.e. the application stays responsive. The monitoring requests take about 200ms for the entire burst.

If 350ms are too long, the I2C speed can be raised. I went for something very conservative, but the OEM software does things 8 times faster, so an entire burst would be about 50ms.

I think you're confusing asynchronous link layer (not sharing the same clock) with asynchronous software design. I don't care what the underlying hardware does. I care that the application is responsive. The typical 41ms transfer turns to several seconds of deadlock whenever firing a burst of requests.

If you have the main uloop toggle a gpio every 100ms, what happens when I2C transactions start flowing? The uloop does not toggle GPIO, it toggles complete I2C transfers. The toggling of GPIOs happens in the GPIO-I2C driver at 100kHz.

[Hurricos]

Converting to a draft. I can't merge this until OpenWrt hardware has been ported for me to test this on.

[Hurricos]

Lots of thoughts:

• We heard from jow today that ustream is already very heavy-handed in its application to uart. uloop with a handmade uloop_fd to create callbacks would be preferable.
• I've added my reasons for not liking how this is implemented; particularly here
• @mrnuke has more than one reason that tx'ing i2c using uloop or uloop-derived mechanisms (e.g. ustream) to poll is not a straightforward choice
  • Using a C11 thread to handle i2c transactions and avoid waits is in the cards, but ...
• Most importantly, until OpenWrt hardware is available for us to test other options on, we can't merge this.

Give us a ping when one of these devices gets OpenWrt support merged.

[Neustradamus]

@Hurricos: Now the PR author account is a ghost, what is the situation now for this PR?

[mrnuke]

@Hurricos: Now the PR author account is a ghost, what is the situation now for this PR?

It's haunted.