i2c: complete and stabilize support for target i2c devices

[pabigot]

The current i2c_slave driver API is marked experimental and is generally undocumented. It is also unsuitable for DMA-based solutions like Nordic SPIS.

Review the existing API and the requirements of other hardware and provide an API that can be fully documented, tested, and implemented on any platform that supports this functionality.

[nashif]

@teburd can you please take a look at this and see what needs to be done and if this still applies?

[zephyrbot]

Hi @teburd,

This issue, marked as an Enhancement, was opened a while ago and did not get any traction. Please confirm the issue is correctly assigned and re-assign it otherwise.

Please take a moment to review if the issue is still relevant to the project. If it is, please provide feedback and direction on how to move forward. If it is not, has already been addressed, is a duplicate, or is no longer relevant, please close it with a short comment explaining the reason.

@pabigot you are also encouraged to help moving this issue forward by providing additional information and confirming this request/issue is still relevant to you.

Thanks!

[teburd]

Renamed slave to target, conforming with the updated terminology. The target APIs are a bit limiting today as they..

1. expect single byte transfers and callbacks between
2. require callbacks (which are not useful from usermode threads at all).

That said the API is documented, there are users in the tree, and its been around awhile. So I think while the current API may not necessarily be "complete" or ideal, it is well past the experimental stage.

[dpkristensen]

Hello, I am in the process of implementing an I2C target driver for the device to be used in a "command+response" protocol. I started on an implementation of the nordic,nrf-twis I2C driver, and there are a few other problems you should be aware of:

• The nrf-pinctrl driver does not recognize an NRF_TWIS function in the GPIO. If the node is changed to TWIS, the TWIM logic will be disabled and therefore pinctrl_apply_state() will return -ENOSUP. So a TWIS function will need to be added for this.
• The nrfx_twis_init() function requires all addresses to be given at once. A DT_FOREACH_CHILD macro could be used to get the list of child addresses, but this is complicated by the lack of DT_CHILD_NUM macro for the case where the bus is defined with no children. So I think the addresses may need to be empty at first and then use nrfx_twis_reconfigure() at registration time. BUT, this will fail if the peripheral is in the middle of a transaction, so it needs some synchronization.

A lot of generic I2C APIs I've seen in the past rely on the pattern of just being given a buffer of data, and it is agnostic as to whether it is a single byte or using DMA. The abstraction has some dependencies on the hardware; for example:

• ST generally likes the architecture of having the address byte separately from the data buffer, because that's how they design the I2C peripheral in terms of hardware interface.
• Other vendors generally like the whole buffer with the address byte at the beginning because they do not treat the address+RW bit special.

So (primarily for 7-bit addresses), the best way to abstract it for those cases has been to just take the whole buffer with the address byte first and then the implementation can separate it. A byte-wise driver could supply just one byte each time, or another could supply the whole buffer if it used DMA. This avoids having to create a separate buffer; but it also puts some extra work on the app layer to form the transaction appropriately without having to re-allocate a whole other buffer for the transaction.

IMO, the best way to do this without breaking the existing API would be to create a separate "IXC" API that can handle I2C and I3C in a platform-agnostic way.

EDIT: Also as far as the callbacks, I think you should require the I2C target API to be implemented in the kernel, and then it can use whatever mechanism it needs to interface with userspace (in Linux, this might be akin to Unix DGRAM sockets or via sysfs nodes). Since I2C target is read on demand from the remote system, you will generally have to have some buffer readily available for the kernel to read; so either the kernel has access to these resources already or the driver will have to prepare a buffer ahead of time for it to be ready at the time of request.

[dpkristensen]

@teburd I am attaching my implementation of the TWIS driver for reference if you want, as well as a driver I wrote to use it. It is kind of a hack as it only supports "buffer mode" instead of the normal byte-wise API, but it does provide a path for using TWIS on the Nordic platform.

i2c-nordic-twis.zip

To instantiate, it requires a custom driver provided by the build; the DTS changes for nRF5340 processors look like this:

&i2c1 {
    // Two-Wire Interface Slave with EasyDMA
    // SDA: P1.02
    // SCL: P1.03
    compatible = "nordic,nrf-twis";

    my_foo_target: foo-driver@4c {
        compatible = "my-company,i2c-target-command";
        status = "okay";

        reg = <0x4c>;
    };
};

To initialize the handler layer, it is necessary to hook up the driver:

static i2c_target_command_context_t const cmd_ctx = {
    .dev = DEVICE_DT_GET(DT_NODELABEL(my_foo_target)),
    .command_handler = _command_handler,
    .stop_handler = _stop_handler,
};

static uint8_t rsp_buffer[30]; // Max response size
static i2c_target_command_response_context_t rsp_ctx = {
    .rsp_buf = rsp_buffer;
};

i2c_target_command_init(&cmd_ctx, &rsp_ctx);

The command handler should set the data and size in rsp_ctx for the next read transaction from the host.

[dpkristensen]

Since the protocol and function of the target is device-specific, there's not really any generic functionality to add; it is up to the implementer to make the device do what it needs to.

You perform an I2C write transaction to send a command with no data, and an I2C write+read transaction to write a command that expects some data in return (it has to be available immediately because it is in IRQ context, so it may require a signaling mechanism to say when data is ready).