driver: i2c_mcux: unable to perform more than one write transfer like i2c_burst_write

[jfischer-no]

The i2c shim driver is unable to perform more than one write transfer like i2c_burst_write. The second I2C_MasterTransferNonBlocking does not start and returns an error. I suspect the combination of kI2C_TransferNoStopFlag and I2C_MasterTransferNonBlocking are not possible when writing.

[MaureenHelm]

I can get the mcux shim driver to return successfully with the following change:

diff --git a/include/i2c.h b/include/i2c.h
index c1543d2..b94788b 100644
--- a/include/i2c.h
+++ b/include/i2c.h
@@ -456,7 +456,7 @@ static inline int i2c_burst_write(struct device *dev, u16_t dev_addr,
 
        msg[1].buf = buf;
        msg[1].len = num_bytes;
-       msg[1].flags = I2C_MSG_WRITE | I2C_MSG_STOP;
+       msg[1].flags = I2C_MSG_RESTART | I2C_MSG_WRITE | I2C_MSG_STOP;
 
        return i2c_transfer(dev, msg, 2, dev_addr);
 }

And I can see a valid i2c transaction on a scope, but there is another problem due to the fact that we break up the burst write into two messages. This doesn't map well to the underlying mcux driver interface I2C_MasterTransferNonBlocking(), and we end up transmitting the slave address twice. This is a valid i2c transaction (write; repeated start; write), but I don't think slaves handle it like you'd expect (at least the fxos8700 accelerometer doesn't, because the write doesn't take effect). A change to struct i2c_driver_api might be needed to allow a driver to support the i2c_burst_write functionality directly.

[nashif]

@MaureenHelm will this be fixed for 1.13?

[MaureenHelm]

No, I think we should wait on this and handle as part of #4040.

[carlescufi]

@MaureenHelm any update on this?

[MaureenHelm]

@MaureenHelm any update on this?

No, not yet.

[jfischer-no]

I had to test the SSD1306 driver and solved it that way:

diff --git a/drivers/i2c/i2c_mcux.c b/drivers/i2c/i2c_mcux.c                                    [16/3771]
index cf26448dd2..e244816fd1 100644
--- a/drivers/i2c/i2c_mcux.c
+++ b/drivers/i2c/i2c_mcux.c
@@ -100,10 +100,8 @@ static int i2c_mcux_transfer(struct device *dev, struct i2c_msg *msgs,             
        i2c_master_transfer_t transfer;
        status_t status;                                                                                
                                                                                                        
-       /* Iterate over all the messages */
-       for (int i = 0; i < num_msgs; i++) {                                                            
-
-               /* Initialize the transfer descriptor */                                                
+       /* Initialize the transfer descriptor */
+       if (num_msgs == 1) {
                transfer.flags = i2c_mcux_convert_flags(msgs->flags);                                   
                transfer.slaveAddress = addr;
                transfer.direction = (msgs->flags & I2C_MSG_READ)                                       
@@ -112,30 +110,40 @@ static int i2c_mcux_transfer(struct device *dev, struct i2c_msg *msgs,            
                transfer.subaddressSize = 0;
                transfer.data = msgs->buf;
                transfer.dataSize = msgs->len;
+       } else if (num_msgs == 2) {
+               msgs[1].flags |= msgs[0].flags;
+               transfer.flags = i2c_mcux_convert_flags(msgs[1].flags);
+               transfer.slaveAddress = addr;
+               transfer.direction = (msgs[1].flags & I2C_MSG_READ)
+                       ? kI2C_Read : kI2C_Write;
+               transfer.subaddress = *msgs[0].buf;
+               transfer.subaddressSize = 1;
+               transfer.data = msgs[1].buf;
+               transfer.dataSize = msgs[1].len;
+               msgs += 2;
+       } else {
+               return -EIO;
+       }
+
+       /* Start the transfer */
+       status = I2C_MasterTransferNonBlocking(base,
+                       &data->handle, &transfer);
+
+       /* Return an error if the transfer didn't start successfully
+        * e.g., if the bus was busy
+        */
+       if (status != kStatus_Success) {
+               return -EIO;
+       }
+
+       /* Wait for the transfer to complete */
+       k_sem_take(&data->device_sync_sem, K_FOREVER);

-               /* Start the transfer */
-               status = I2C_MasterTransferNonBlocking(base,
-                               &data->handle, &transfer);
-
-               /* Return an error if the transfer didn't start successfully
-                * e.g., if the bus was busy
-                */
-               if (status != kStatus_Success) {
-                       return -EIO;
-               }
-
-               /* Wait for the transfer to complete */
-               k_sem_take(&data->device_sync_sem, K_FOREVER);
-
-               /* Return an error if the transfer didn't complete
-                * successfully. e.g., nak, timeout, lost arbitration
-                */
-               if (data->callback_status != kStatus_Success) {
-                       return -EIO;
-               }
-
-               /* Move to the next message */
-               msgs++;
+       /* Return an error if the transfer didn't complete
+        * successfully. e.g., nak, timeout, lost arbitration
+        */
+       if (data->callback_status != kStatus_Success) {
+               return -EIO;
        }

        return 0;

[galak]

Same issue as in #11612

[erwango]

@MaureenHelm , to answer your question, stm32 i2c driver supports i2c_burst_write. It first checks content and flags of 2 msg[] then build one single data message and set flags accordingly. This was not an obvious thing to do since indeed the API is not quite clear indeed (discussed here: https://lists.zephyrproject.org/g/devel/message/310).

[jfischer-no]

fixed in #14015