Skip to contentSkip to conversation

fsi: Add IBM I2C Responder virtual FSI master

PATCH Superseded Eddie James 3 participants 7 messages Jan 26, 2023 – Jan 27, 2023

Subsystems (3)

v3 of 8 Superseded Latest is v8 →

lore
Share & tools
lore

Eddie James

author maintainer
#0 lore raw

v3 cover fsi: Add IBM I2C Responder virtual FSI master

The I2C Responder (I2CR) is an I2C device that translates I2C commands to CFAM or SCOM operations, effectively implementing an FSI master and bus.

Changes since v2:
- Fix the bindings again, sorry for the spam

Changes since v1:
- Fix the binding document
- Change the binding name
- Clean up the size argument checking
- Reduce __force by using packed struct for the command

Eddie James (2):
dt-bindings: fsi: Document the IBM I2C Responder virtual FSI master
fsi: Add IBM I2C Responder virtual FSI master

 .../bindings/fsi/ibm,i2cr-fsi-master.yaml     |  41 ++++
 drivers/fsi/Kconfig                           |   9 +
 drivers/fsi/Makefile                          |   1 +
 drivers/fsi/fsi-master-i2cr.c                 | 225 ++++++++++++++++++
 include/trace/events/fsi_master_i2cr.h        |  96 ++++++++
 5 files changed, 372 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fsi/ibm,i2cr-fsi-master.yaml
 create mode 100644 drivers/fsi/fsi-master-i2cr.c
 create mode 100644 include/trace/events/fsi_master_i2cr.h
Signature · 1 line
2.31.1
author maintainer
#1 lore raw

PATCH v3 1/2 dt-bindings: fsi: Document the IBM I2C Responder virtual FSI master

The I2C Responder translates I2C commands to CFAM or SCOM operations, effectively implementing an FSI master.

---
 .../bindings/fsi/ibm,i2cr-fsi-master.yaml     | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fsi/ibm,i2cr-fsi-master.yaml
Documentation/devicetree/bindings/fsi/ibm,i2cr-fsi-master.yaml new file +41 -0
new file mode 100644
index 000000000000..442cecdc57cb
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/fsi/ibm,i2cr-fsi-master.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: IBM I2C Responder virtual FSI master
+
+maintainers:
+  - Eddie James <[email protected]>
+
+description: |
+  The I2C Responder (I2CR) is a an I2C device that's connected to an FSI CFAM
+  (see fsi.txt). The I2CR translates I2C bus operations to FSI CFAM reads and
+  writes or SCOM operations, thereby acting as an FSI master.
+
+properties:
+  compatible:
+    enum:
+      - ibm,i2cr-fsi-master
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      i2cr@20 {
+        compatible = "ibm,i2cr-fsi-master";
+        reg = <0x20>;
+      };
+    };
Signature · 1 line
2.31.1
Reviewed-by on v3 1/2
#3 lore raw

Re: [PATCH v3 1/2] dt-bindings: fsi: Document the IBM I2C Responder virtual FSI master

In reply to Eddie James
Quoted 9 lines · Eddie James

The I2C Responder translates I2C commands to CFAM or SCOM operations, effectively implementing an FSI master.

---
 .../bindings/fsi/ibm,i2cr-fsi-master.yaml     | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fsi/ibm,i2cr-fsi-master.yaml

Best regards,
Krzysztof

author maintainer
#4 lore raw

PATCH v3 2/2 fsi: Add IBM I2C Responder virtual FSI master

In reply to Eddie James

The I2C Responder (I2CR) is an I2C device that translates I2C commands to CFAM or SCOM operations, effectively implementing an FSI master and bus.

---
 drivers/fsi/Kconfig                    |   9 +
 drivers/fsi/Makefile                   |   1 +
 drivers/fsi/fsi-master-i2cr.c          | 225 +++++++++++++++++++++++++
 include/trace/events/fsi_master_i2cr.h |  96 +++++++++++
 4 files changed, 331 insertions(+)
 create mode 100644 drivers/fsi/fsi-master-i2cr.c
 create mode 100644 include/trace/events/fsi_master_i2cr.h
drivers/fsi/Kconfig +9 -0
index e6668a869913..999be82720c5 100644
@@ -62,6 +62,15 @@ config FSI_MASTER_ASPEED
 
 	 Enable it for your BMC kernel in an OpenPower or IBM Power system.
 
+config FSI_MASTER_I2CR
+	tristate "IBM I2C Responder virtual FSI master"
+	depends on I2C
+	help
+	  This option enables a virtual FSI master in order to access a CFAM
+	  behind an IBM I2C Responder (I2CR) chip. The I2CR is an I2C device
+	  that translates I2C commands to CFAM or SCOM operations, effectively
+	  implementing an FSI master and bus.
+
 config FSI_SCOM
 	tristate "SCOM FSI client device driver"
 	help
drivers/fsi/Makefile +1 -0
index da218a1ad8e1..34dbaa1c452e 100644
@@ -4,6 +4,7 @@ obj-$(CONFIG_FSI) += fsi-core.o
 obj-$(CONFIG_FSI_MASTER_HUB) += fsi-master-hub.o
 obj-$(CONFIG_FSI_MASTER_ASPEED) += fsi-master-aspeed.o
 obj-$(CONFIG_FSI_MASTER_GPIO) += fsi-master-gpio.o
+obj-$(CONFIG_FSI_MASTER_I2CR) += fsi-master-i2cr.o
 obj-$(CONFIG_FSI_MASTER_AST_CF) += fsi-master-ast-cf.o
 obj-$(CONFIG_FSI_SCOM) += fsi-scom.o
 obj-$(CONFIG_FSI_SBEFIFO) += fsi-sbefifo.o
drivers/fsi/fsi-master-i2cr.c new file +225 -0

225 lines not on this page · raw message

include/trace/events/fsi_master_i2cr.h new file +96 -0

96 lines not on this page · raw message

Signature · 1 line
2.31.1
#5 lore raw

Hi Eddie,

Quoted 208 lines · Eddie James

The I2C Responder (I2CR) is an I2C device that translates I2C commands to CFAM or SCOM operations, effectively implementing an FSI master and bus.

---
 drivers/fsi/Kconfig                    |   9 +
 drivers/fsi/Makefile                   |   1 +
 drivers/fsi/fsi-master-i2cr.c          | 225 +++++++++++++++++++++++++
 include/trace/events/fsi_master_i2cr.h |  96 +++++++++++
 4 files changed, 331 insertions(+)
 create mode 100644 drivers/fsi/fsi-master-i2cr.c
 create mode 100644 include/trace/events/fsi_master_i2cr.h
drivers/fsi/Kconfig +9 -0
index e6668a869913..999be82720c5 100644
@@ -62,6 +62,15 @@ config FSI_MASTER_ASPEED
 
 	 Enable it for your BMC kernel in an OpenPower or IBM Power system.
 
+config FSI_MASTER_I2CR
+	tristate "IBM I2C Responder virtual FSI master"
+	depends on I2C
+	help
+	  This option enables a virtual FSI master in order to access a CFAM
+	  behind an IBM I2C Responder (I2CR) chip. The I2CR is an I2C device
+	  that translates I2C commands to CFAM or SCOM operations, effectively
+	  implementing an FSI master and bus.
+
 config FSI_SCOM
 	tristate "SCOM FSI client device driver"
 	help
drivers/fsi/Makefile +1 -0
index da218a1ad8e1..34dbaa1c452e 100644
@@ -4,6 +4,7 @@ obj-$(CONFIG_FSI) += fsi-core.o
 obj-$(CONFIG_FSI_MASTER_HUB) += fsi-master-hub.o
 obj-$(CONFIG_FSI_MASTER_ASPEED) += fsi-master-aspeed.o
 obj-$(CONFIG_FSI_MASTER_GPIO) += fsi-master-gpio.o
+obj-$(CONFIG_FSI_MASTER_I2CR) += fsi-master-i2cr.o
 obj-$(CONFIG_FSI_MASTER_AST_CF) += fsi-master-ast-cf.o
 obj-$(CONFIG_FSI_SCOM) += fsi-scom.o
 obj-$(CONFIG_FSI_SBEFIFO) += fsi-sbefifo.o
drivers/fsi/fsi-master-i2cr.c new file +54 -0
new file mode 100644
index 000000000000..0eadc9b26063
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) IBM Corporation 2023 */
+
+#include <linux/device.h>
+#include <linux/fsi.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+
+#include "fsi-master.h"
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/fsi_master_i2cr.h>
+
+#define I2CR_ADDRESS_CFAM(a)	((a) >> 2)
+#define I2CR_STATUS		0x30001
+#define  I2CR_STATUS_ERR	 BIT_ULL(61)
+#define I2CR_ERROR		0x30002
+
+struct fsi_master_i2cr {
+	struct fsi_master master;
+	struct mutex lock;	/* protect HW access */
+	struct i2c_client *client;
+};
+
+static bool i2cr_check_parity(u32 v, bool parity)
+{
+	u32 i;
+
+	for (i = 0; i < 32; ++i) {
+		if (v & (1 << i))
+			parity = !parity;
+	}
+
+	return parity;
+}
+
+static __be32 i2cr_get_command(u32 address, bool parity)
+{
+	__be32 command;
+
+	address <<= 1;
+
+	if (i2cr_check_parity(address, parity))
+		address |= 1;
+
+	command = cpu_to_be32(address);
+	trace_i2cr_command((__force uint32_t)command);
+
+	return command;
+}
+
+static int i2cr_transfer(struct i2c_client *client, u32 address, 

__be64 *data)

drivers/fsi/fsi-master-i2cr.c new file +46 -0
+{
+	struct i2c_msg msgs[2];
+	__be32 command;
+	int ret;
+
+	command = i2cr_get_command(address, true);
+	msgs[0].addr = client->addr;
+	msgs[0].flags = 0;
+	msgs[0].len = sizeof(command);
+	msgs[0].buf = (__u8 *)&command;
+	msgs[1].addr = client->addr;
+	msgs[1].flags = I2C_M_RD;
+	msgs[1].len = sizeof(*data);
+	msgs[1].buf = (__u8 *)data;
+
+	ret = i2c_transfer(client->adapter, msgs, 2);
+	if (ret == 2)
+		return 0;
+
+	trace_i2cr_i2c_error(ret);
+
+	if (ret < 0)
+		return ret;
+
+	return -EIO;
+}
+
+static int i2cr_check_status(struct i2c_client *client)
+{
+	__be64 status_be = 0;
+	u64 status;
+	int ret;
+
+	ret = i2cr_transfer(client, I2CR_STATUS, &status_be);
+	if (ret)
+		return ret;
+
+	status = be64_to_cpu(status_be);
+	if (status & I2CR_STATUS_ERR) {
+		__be64 error_be = 0;
+		u64 error;
+
+		i2cr_transfer(client, I2CR_ERROR, &error_be);
+		error = be64_to_cpu(error_be);
+		trace_i2cr_status_error(status, error);
+		dev_err(&client->dev, "status:%016llx error:%016llx\n", status, 

error);

drivers/fsi/fsi-master-i2cr.c new file +8 -0
+		return -EREMOTEIO;
+	}
+
+	trace_i2cr_status(status);
+	return 0;
+}
+
+static int i2cr_read(struct fsi_master *master, int link, uint8_t id, 

uint32_t addr, void *val,

drivers/fsi/fsi-master-i2cr.c new file +3 -0
+		     size_t size)
+{
+	struct fsi_master_i2cr *i2cr = container_of(master, struct 

fsi_master_i2cr, master);

drivers/fsi/fsi-master-i2cr.c new file +4 -0
+	__be64 data = 0;
+	int ret;
+
+	if (link || id || (addr & 0xffff0000) || !(size == 1 || size == 2 || 

size == 4))

drivers/fsi/fsi-master-i2cr.c new file +21 -0
+		return -EINVAL;
+
+	mutex_lock(&i2cr->lock);
+
+	ret = i2cr_transfer(i2cr->client, I2CR_ADDRESS_CFAM(addr), &data);
+	if (ret)
+		goto unlock;
+
+	ret = i2cr_check_status(i2cr->client);
+	if (ret)
+		goto unlock;
+
+	trace_i2cr_read(addr, size, (__force uint32_t)data);
+	memcpy(val, &data, size);
+
+unlock:
+	mutex_unlock(&i2cr->lock);
+	return ret;
+}
+
+static int i2cr_write(struct fsi_master *master, int link, uint8_t id, 

uint32_t addr,

drivers/fsi/fsi-master-i2cr.c new file +3 -0
+		      const void *val, size_t size)
+{
+	struct fsi_master_i2cr *i2cr = container_of(master, struct 

fsi_master_i2cr, master);

drivers/fsi/fsi-master-i2cr.c new file +8 -0
+	struct {
+		__be32 command;
+		u32 val;
+		u32 rsvd;
+	} __packed data = { 0, 0, 0 };
+	int ret;
+
+	if (link || id || (addr & 0xffff0000) || !(size == 1 || size == 2 || 

size == 4))

drivers/fsi/fsi-master-i2cr.c line 150

Quoted 3 lines
drivers/fsi/fsi-master-i2cr.c new file +3 -0
+		return -EINVAL;
+
+	memcpy(&data.val, val, size);

Still nervous about endian mixups here given the buffers in val tend to be pointers to big-endian values but data.val is native-endian (likely little-endian). It probably doesn't matter functionally given we pass the pointer to the packed struct through i2c_master_send(), but do the values come out right in the trace data?

What do you think about adding some commentary outlining the value representations to help with confidence here?

Quoted 7 lines
drivers/fsi/fsi-master-i2cr.c new file +6 -0
+	data.command = i2cr_get_command(I2CR_ADDRESS_CFAM(addr),
+					i2cr_check_parity(data.val, true));
+
+	mutex_lock(&i2cr->lock);
+
+	ret = i2c_master_send(i2cr->client, (const char *)&data, 

sizeof(data));

drivers/fsi/fsi-master-i2cr.c line 166

Quoted 10 lines
drivers/fsi/fsi-master-i2cr.c new file +10 -0
+	if (ret == sizeof(data)) {
+		ret = i2cr_check_status(i2cr->client);
+		if (!ret)
+			trace_i2cr_write(addr, size, data.val);
+	} else {
+		trace_i2cr_i2c_error(ret);
+
+		if (ret >= 0)
+			ret = -EIO;
+	}

The i2cr_transfer() call in i2cr_check_status() can error but that won't be traced. Is that intentional? What about this instead?

ret = i2c_master_send(...)

if (ret == sizeof(data)) {
    ret = i2cr_check_status(i2cr->client);
} else {
    ret = -EIO;
}
if (ret) {
    trace_i2cr_i2c_error(ret);
} else {
    trace_i2cr_write(addr, size, data.val);
}

Cheers,

Andrew

author maintainer
#6 lore raw
Quoted 220 lines · Andrew Jeffery

Hi Eddie,

Quoted 208 lines · Eddie James

The I2C Responder (I2CR) is an I2C device that translates I2C commands to CFAM or SCOM operations, effectively implementing an FSI master and bus.

---
  drivers/fsi/Kconfig                    |   9 +
  drivers/fsi/Makefile                   |   1 +
  drivers/fsi/fsi-master-i2cr.c          | 225 +++++++++++++++++++++++++
  include/trace/events/fsi_master_i2cr.h |  96 +++++++++++
  4 files changed, 331 insertions(+)

create mode 100644 drivers/fsi/fsi-master-i2cr.c create mode 100644 include/trace/events/fsi_master_i2cr.h

drivers/fsi/Kconfig +9 -0
index e6668a869913..999be82720c5 100644
@@ -62,6 +62,15 @@ config FSI_MASTER_ASPEED
 
  	 Enable it for your BMC kernel in an OpenPower or IBM Power system.
 
+config FSI_MASTER_I2CR
+	tristate "IBM I2C Responder virtual FSI master"
+	depends on I2C
+	help
+	  This option enables a virtual FSI master in order to access a CFAM
+	  behind an IBM I2C Responder (I2CR) chip. The I2CR is an I2C device
+	  that translates I2C commands to CFAM or SCOM operations, effectively
+	  implementing an FSI master and bus.
+
  config FSI_SCOM
  	tristate "SCOM FSI client device driver"
  	help
drivers/fsi/Makefile +1 -0
index da218a1ad8e1..34dbaa1c452e 100644
@@ -4,6 +4,7 @@ obj-$(CONFIG_FSI) += fsi-core.o
  obj-$(CONFIG_FSI_MASTER_HUB) += fsi-master-hub.o
  obj-$(CONFIG_FSI_MASTER_ASPEED) += fsi-master-aspeed.o
  obj-$(CONFIG_FSI_MASTER_GPIO) += fsi-master-gpio.o
+obj-$(CONFIG_FSI_MASTER_I2CR) += fsi-master-i2cr.o
  obj-$(CONFIG_FSI_MASTER_AST_CF) += fsi-master-ast-cf.o
  obj-$(CONFIG_FSI_SCOM) += fsi-scom.o
  obj-$(CONFIG_FSI_SBEFIFO) += fsi-sbefifo.o
drivers/fsi/fsi-master-i2cr.c new file +54 -0
new file mode 100644
index 000000000000..0eadc9b26063
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) IBM Corporation 2023 */
+
+#include <linux/device.h>
+#include <linux/fsi.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+
+#include "fsi-master.h"
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/fsi_master_i2cr.h>
+
+#define I2CR_ADDRESS_CFAM(a)	((a) >> 2)
+#define I2CR_STATUS		0x30001
+#define  I2CR_STATUS_ERR	 BIT_ULL(61)
+#define I2CR_ERROR		0x30002
+
+struct fsi_master_i2cr {
+	struct fsi_master master;
+	struct mutex lock;	/* protect HW access */
+	struct i2c_client *client;
+};
+
+static bool i2cr_check_parity(u32 v, bool parity)
+{
+	u32 i;
+
+	for (i = 0; i < 32; ++i) {
+		if (v & (1 << i))
+			parity = !parity;
+	}
+
+	return parity;
+}
+
+static __be32 i2cr_get_command(u32 address, bool parity)
+{
+	__be32 command;
+
+	address <<= 1;
+
+	if (i2cr_check_parity(address, parity))
+		address |= 1;
+
+	command = cpu_to_be32(address);
+	trace_i2cr_command((__force uint32_t)command);
+
+	return command;
+}
+
+static int i2cr_transfer(struct i2c_client *client, u32 address,

__be64 *data)

drivers/fsi/fsi-master-i2cr.c new file +46 -0
+{
+	struct i2c_msg msgs[2];
+	__be32 command;
+	int ret;
+
+	command = i2cr_get_command(address, true);
+	msgs[0].addr = client->addr;
+	msgs[0].flags = 0;
+	msgs[0].len = sizeof(command);
+	msgs[0].buf = (__u8 *)&command;
+	msgs[1].addr = client->addr;
+	msgs[1].flags = I2C_M_RD;
+	msgs[1].len = sizeof(*data);
+	msgs[1].buf = (__u8 *)data;
+
+	ret = i2c_transfer(client->adapter, msgs, 2);
+	if (ret == 2)
+		return 0;
+
+	trace_i2cr_i2c_error(ret);
+
+	if (ret < 0)
+		return ret;
+
+	return -EIO;
+}
+
+static int i2cr_check_status(struct i2c_client *client)
+{
+	__be64 status_be = 0;
+	u64 status;
+	int ret;
+
+	ret = i2cr_transfer(client, I2CR_STATUS, &status_be);
+	if (ret)
+		return ret;
+
+	status = be64_to_cpu(status_be);
+	if (status & I2CR_STATUS_ERR) {
+		__be64 error_be = 0;
+		u64 error;
+
+		i2cr_transfer(client, I2CR_ERROR, &error_be);
+		error = be64_to_cpu(error_be);
+		trace_i2cr_status_error(status, error);
+		dev_err(&client->dev, "status:%016llx error:%016llx\n", status,

error);

drivers/fsi/fsi-master-i2cr.c new file +8 -0
+		return -EREMOTEIO;
+	}
+
+	trace_i2cr_status(status);
+	return 0;
+}
+
+static int i2cr_read(struct fsi_master *master, int link, uint8_t id,

uint32_t addr, void *val,

drivers/fsi/fsi-master-i2cr.c new file +3 -0
+		     size_t size)
+{
+	struct fsi_master_i2cr *i2cr = container_of(master, struct

fsi_master_i2cr, master);

drivers/fsi/fsi-master-i2cr.c new file +4 -0
+	__be64 data = 0;
+	int ret;
+
+	if (link || id || (addr & 0xffff0000) || !(size == 1 || size == 2 ||

size == 4))

drivers/fsi/fsi-master-i2cr.c new file +21 -0
+		return -EINVAL;
+
+	mutex_lock(&i2cr->lock);
+
+	ret = i2cr_transfer(i2cr->client, I2CR_ADDRESS_CFAM(addr), &data);
+	if (ret)
+		goto unlock;
+
+	ret = i2cr_check_status(i2cr->client);
+	if (ret)
+		goto unlock;
+
+	trace_i2cr_read(addr, size, (__force uint32_t)data);
+	memcpy(val, &data, size);
+
+unlock:
+	mutex_unlock(&i2cr->lock);
+	return ret;
+}
+
+static int i2cr_write(struct fsi_master *master, int link, uint8_t id,

uint32_t addr,

drivers/fsi/fsi-master-i2cr.c new file +3 -0
+		      const void *val, size_t size)
+{
+	struct fsi_master_i2cr *i2cr = container_of(master, struct

fsi_master_i2cr, master);

drivers/fsi/fsi-master-i2cr.c new file +8 -0
+	struct {
+		__be32 command;
+		u32 val;
+		u32 rsvd;
+	} __packed data = { 0, 0, 0 };
+	int ret;
+
+	if (link || id || (addr & 0xffff0000) || !(size == 1 || size == 2 ||

size == 4))

drivers/fsi/fsi-master-i2cr.c line 150

Quoted 3 lines
drivers/fsi/fsi-master-i2cr.c new file +3 -0
+		return -EINVAL;
+
+	memcpy(&data.val, val, size);

Still nervous about endian mixups here given the buffers in val tend to be pointers to big-endian values but data.val is native-endian (likely little-endian). It probably doesn't matter functionally given we pass the pointer to the packed struct through i2c_master_send(), but do the values come out right in the trace data?

Quoted 2 lines

What do you think about adding some commentary outlining the value representations to help with confidence here?

Yea, this is still not the cleanest. I'll either add comments or rework it a little bit. I have reworked the trace functions to get rid of any casting too.

Quoted 12 lines
Quoted 12 lines
drivers/fsi/fsi-master-i2cr.c new file +6 -0
+	data.command = i2cr_get_command(I2CR_ADDRESS_CFAM(addr),
+					i2cr_check_parity(data.val, true));
+
+	mutex_lock(&i2cr->lock);
+
+	ret = i2c_master_send(i2cr->client, (const char *)&data,

sizeof(data));

drivers/fsi/fsi-master-i2cr.c new file +4 -0
+	if (ret == sizeof(data)) {
+		ret = i2cr_check_status(i2cr->client);
+		if (!ret)
+			trace_i2cr_write(addr, size, data.val);
Quoted 8 lines

drivers/fsi/fsi-master-i2cr.c line 166

Quoted 6 lines
drivers/fsi/fsi-master-i2cr.c new file +6 -0
+	} else {
+		trace_i2cr_i2c_error(ret);
+
+		if (ret >= 0)
+			ret = -EIO;
+	}

The i2cr_transfer() call in i2cr_check_status() can error but that won't be traced. Is that intentional? What about this instead?

i2cr_transfer itself traces on i2c error, so we should be covered? In your snippet below, you'll get an "i2cr_i2c_error" trace even for a failed status check, which isn't my intention for that function.

Thanks for the review!

Eddie

Quoted 17 lines

ret = i2c_master_send(...)

if (ret == sizeof(data)) {
     ret = i2cr_check_status(i2cr->client);
} else {
     ret = -EIO;
}
if (ret) {
     trace_i2cr_i2c_error(ret);
} else {
     trace_i2cr_write(addr, size, data.val);
}

Cheers,

Andrew