Electronic – Does Linux regmap handle repeated start/stop i2c transaction

i2clinux

I'm bringing up ADV7610 on Linux Kernel v4.14 on TI AM5728 platform. The Kernel module for this device is failing to successfully probe the device:

[ 4081.446593] adv7604 0-004c: GPIO lookup for consumer reset
[ 4081.446598] adv7604 0-004c: using device tree for GPIO lookup
[ 4081.466818] adv7611 0-004c: Error -121 reading IO Regmap

I've learned that the ADV7610 uses repeated start/stop transfers. This is the raw traffic during driver initialization by reading the 0xEA (hardware ID) which the value is 0x20:

    modprobe-2124  [001] ....  4055.146719: regmap_hw_read_done: 0-004c reg=ea count=1
    modprobe-2131  [001] ....  4081.466725: regmap_hw_read_start: 0-004c reg=ea count=1
    modprobe-2131  [001] ....  4081.466733: i2c_write: i2c-0 #0 a=04c f=0000 l=1 [ea]
    modprobe-2131  [001] ....  4081.466734: i2c_read: i2c-0 #1 a=04c f=0001 l=1
    modprobe-2131  [001] ....  4081.466811: i2c_result: i2c-0 n=0 ret=-121
    modprobe-2131  [001] ....  4081.466814: regmap_hw_read_done: 0-004c reg=ea count=1

You can see above that the driver is not receiving any data (n=0). This also explains why i2cdetect fails to detect the device (it simply writes slave address on the bus in read mode).

On the other hand, i2cget command works because it handles repeated start/stop:

    i2cget-2021  [000] ....   538.751863: i2c_write: i2c-0 #0 a=04c f=0000 l=1 [ea]    
    i2cget-2021  [000] ....   538.751867: i2c_read: i2c-0 #1 a=04c f=0001 l=1 
    i2cget-2021  [000] ....   538.752202: i2c_reply: i2c-0 #1 a=04c f=0001 l=1 [20]
    i2cget-2021  [000] ....   538.752204: i2c_result: i2c-0 n=2 ret=2

You can see above that the i2c_reply (repeated start) gets sent before reading the correct data back(i2c_result … n=2) in the next line.

I'm puzzled because I actually do not suspect there is a bug in either the Linux regmap/i2c subsystem or in the driver itself (adv7604.c). However, that doesn't leave much room for error. What might be going on here..?

Best Answer

There is no bug, nor anything specific regarding repeated start and support or non-support in the linux kernel. Indeed, from a software point of view initiating a repeated start i2c transaction (assuming we're in kernel space anyway) is as simple as passing more than one message struct in the form of an array to the i2c_transfer function, and it will combine them into a single repeated start sequence automatically.

It is also possible using ioctl in the user-space, which is how i2cget accomplishes it. Linux even has provisions to perform repeated start i2C transfers even when the i2c master doesn't directly support them via the i2c-gpio driver.

Beyond that, regmap is intended to NOT use repeated starts, and this is not incorrect behavior - NXP's i2c standard is quite clear that any slave that supports i2c is required to respond to both start/stop sequences (single byte or repeated start), and that this is dictated entirely by the i2c master(s). The slave needs to respond to whichever transaction the whims of the bus master decides to use for that particular i2c message. Regmap is choosing to use normal start stop transactions, and that is for regmap to decide. And even if regmap did support repeated start, there would be no way for a slave to indicate which it needed to use, and beyond that, the i2c standard forbids a slave deciding which to use in the first place.

But that's ok, because the solution is trivial: just don't use regmap in a linux i2c kernel driver if your chip requires repeated start only.

That brings us to the real problem here: the adv7604 driver isn't able to probe the ADV7610 because the adv7604 driver doesn't support the ADV7610. That is not the correct kernel module to use with the ADV7610 at all. Simply put, you're using the wrong driver. The adv7604 is not a generic adv76xx driver, it only supports the ADV7604, ADV7611 and ADV7612. This is why it has not been renamed adv76xx or even adv761x, as this would imply support for things it does not.

While it is very similar to the ADV7611, I think you've come upon the reason the adv7604 driver lacks support for the ADV7610 despite supporting the ADV7611: the ADV7610 does properly require i2c communication to use repeated start and regmap does not do this, thus adding support for the ADV7610 would require a substantial rewrite of the driver, which carries with it a lot of risk of breaking support for the already-supported devices, introducing new bugs, and requiring all devices be retested with the new driver.

I'm sure ADI originally envisioned that driver would support a wider selection of chips, but the repeated start requirement was not really intentional - it can be traced back Xilinx. Xilinx has some i2c IP blocks that require (and I mean actually require and won't work otherwise) repeated start transfers. And one of these IP blocks was ultimately used by ADI for the i2c communication in the ADV7610. And this is the situation we're kind of stuck with at this point.

The IMX6 branch of linux (for use with Freescale's IMX5/6 platform) has a v4l2 (video4linux) based driver which supports the ADV7610 called mxc_v4l2_capture. This likely has everything you'd need, but since it is an in-tree driver, it'll take some refactoring and reconfiguration if you want to build it in the mainline linux source tree. I haven't looked closely at the code and don't know if it depends on anything specific to the IMX6 kernel branch, or if it can be made to work more generally with v4l2 drivers or provide access that does not require the v412 API and support modules.

Your best bet might be to simply make your own driver based upon the adv7604 driver, but use something like i2c_transfer instead of regmap to achieve probing. And I'm sure you might even be able to get it added to the mainline linux kernel if it is done well, which would make at least a few people happy! Sadly, one way or another, there simply isn't support for that chip in the mainline linux kernel, so you're going to have to add support for it yourself. You can also talk to it from userspace using ioctl as mentioned earlier as well, so you have options.

Nothing is going to get around the fundamental problem though: it will take some work and no one has done that work for you already, so it is up to you. It looks like other people have at least had success but it is specific to IMX6 hardware:

https://community.nxp.com/thread/443555

https://wiki.voipac.com/xwiki/bin/view/imx6+tinyrex/linux

https://github.com/voipac/linux-fslc/blob/3.14-1.0.x-mx6-tinyrex/drivers/media/platform/mxc/capture/adv7610.c