[PATCH 0/3] i2c: testunit: let it trigger SMBusAlerts

[PATCH 0/3] i2c: testunit: let it trigger SMBusAlerts

[Wolfram Sang]

Finally, after all the prerequisites, here is now the series to let the
testunit trigger SMBusAlerts. First two patches are preparations, third
one is the real thing.

The patches are based on i2c/for-next and have been tested on a Renesas
Lager board (R-Car H2) and a Renesas Falcon board (R-Car V3U).

Next and final step is to update/clarify DT bindings so GPIOs are
recognized as custom SMBALERT# pins. Code is all there already.

Looking forward to comments!


Wolfram Sang (3):
  i2c: testunit: describe fwnode based instantiation
  i2c: testunit: move code to avoid a forward declaration
  i2c: testunit: add SMBusAlert trigger

 Documentation/i2c/slave-testunit-backend.rst |  60 +++++++++
 drivers/i2c/i2c-slave-testunit.c             | 123 ++++++++++++++-----
 2 files changed, 151 insertions(+), 32 deletions(-)

-- 
2.43.0

[PATCH 1/3] i2c: testunit: describe fwnode based instantiation

[Wolfram Sang]

The testunit can also be instantiated via firmware nodes. Give a
devicetree node as an example.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 Documentation/i2c/slave-testunit-backend.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/i2c/slave-testunit-backend.rst b/Documentation/i2c/slave-testunit-backend.rst
index d3ab5944877d..3743188ecfc7 100644
--- a/Documentation/i2c/slave-testunit-backend.rst
+++ b/Documentation/i2c/slave-testunit-backend.rst
@@ -20,6 +20,18 @@ Instantiating the device is regular. Example for bus 0, address 0x30::
 
   # echo "slave-testunit 0x1030" > /sys/bus/i2c/devices/i2c-0/new_device
 
+Or using firmware nodes. Here is a devicetree example (note this is only a
+debug device, so there are no official DT bindings)::
+
+  &i2c0	{
+        ...
+
+	testunit@30 {
+		compatible = "slave-testunit";
+		reg = <(0x30 | I2C_OWN_SLAVE_ADDRESS)>;
+	};
+  };
+
 After that, you will have the device listening. Reading will return a single
 byte. Its value is 0 if the testunit is idle, otherwise the command number of
 the currently running command.
-- 
2.43.0

[PATCH 2/3] i2c: testunit: move code to avoid a forward declaration

[Wolfram Sang]

To avoid forward declarations in upcoming code, move the workqueue
handler as-is downwards. This will ease review of the new features.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/i2c-slave-testunit.c | 84 ++++++++++++++++----------------
 1 file changed, 42 insertions(+), 42 deletions(-)

diff --git a/drivers/i2c/i2c-slave-testunit.c b/drivers/i2c/i2c-slave-testunit.c
index 04060bc8a9d0..df54185e5e9a 100644
--- a/drivers/i2c/i2c-slave-testunit.c
+++ b/drivers/i2c/i2c-slave-testunit.c
@@ -48,48 +48,6 @@ struct testunit_data {
 
 static char tu_version_info[] = "v" UTS_RELEASE "\n\0";
 
-static void i2c_slave_testunit_work(struct work_struct *work)
-{
-	struct testunit_data *tu = container_of(work, struct testunit_data, worker.work);
-	struct i2c_msg msg;
-	u8 msgbuf[256];
-	int ret = 0;
-
-	msg.addr = I2C_CLIENT_END;
-	msg.buf = msgbuf;
-
-	switch (tu->regs[TU_REG_CMD]) {
-	case TU_CMD_READ_BYTES:
-		msg.addr = tu->regs[TU_REG_DATAL];
-		msg.flags = I2C_M_RD;
-		msg.len = tu->regs[TU_REG_DATAH];
-		break;
-
-	case TU_CMD_SMBUS_HOST_NOTIFY:
-		msg.addr = 0x08;
-		msg.flags = 0;
-		msg.len = 3;
-		msgbuf[0] = tu->client->addr;
-		msgbuf[1] = tu->regs[TU_REG_DATAL];
-		msgbuf[2] = tu->regs[TU_REG_DATAH];
-		break;
-
-	default:
-		break;
-	}
-
-	if (msg.addr != I2C_CLIENT_END) {
-		ret = i2c_transfer(tu->client->adapter, &msg, 1);
-		/* convert '0 msgs transferred' to errno */
-		ret = (ret == 0) ? -EIO : ret;
-	}
-
-	if (ret < 0)
-		dev_err(&tu->client->dev, "CMD%02X failed (%d)\n", tu->regs[TU_REG_CMD], ret);
-
-	clear_bit(TU_FLAG_IN_PROCESS, &tu->flags);
-}
-
 static int i2c_slave_testunit_slave_cb(struct i2c_client *client,
 				     enum i2c_slave_event event, u8 *val)
 {
@@ -166,6 +124,48 @@ static int i2c_slave_testunit_slave_cb(struct i2c_client *client,
 	return ret;
 }
 
+static void i2c_slave_testunit_work(struct work_struct *work)
+{
+	struct testunit_data *tu = container_of(work, struct testunit_data, worker.work);
+	struct i2c_msg msg;
+	u8 msgbuf[256];
+	int ret = 0;
+
+	msg.addr = I2C_CLIENT_END;
+	msg.buf = msgbuf;
+
+	switch (tu->regs[TU_REG_CMD]) {
+	case TU_CMD_READ_BYTES:
+		msg.addr = tu->regs[TU_REG_DATAL];
+		msg.flags = I2C_M_RD;
+		msg.len = tu->regs[TU_REG_DATAH];
+		break;
+
+	case TU_CMD_SMBUS_HOST_NOTIFY:
+		msg.addr = 0x08;
+		msg.flags = 0;
+		msg.len = 3;
+		msgbuf[0] = tu->client->addr;
+		msgbuf[1] = tu->regs[TU_REG_DATAL];
+		msgbuf[2] = tu->regs[TU_REG_DATAH];
+		break;
+
+	default:
+		break;
+	}
+
+	if (msg.addr != I2C_CLIENT_END) {
+		ret = i2c_transfer(tu->client->adapter, &msg, 1);
+		/* convert '0 msgs transferred' to errno */
+		ret = (ret == 0) ? -EIO : ret;
+	}
+
+	if (ret < 0)
+		dev_err(&tu->client->dev, "CMD%02X failed (%d)\n", tu->regs[TU_REG_CMD], ret);
+
+	clear_bit(TU_FLAG_IN_PROCESS, &tu->flags);
+}
+
 static int i2c_slave_testunit_probe(struct i2c_client *client)
 {
 	struct testunit_data *tu;
-- 
2.43.0

[PATCH 3/3] i2c: testunit: add SMBusAlert trigger

[Wolfram Sang]

To test SMBusAlert handlers, let the testunit be able to trigger
SMBusAlert interrupts. This new command needs a GPIO connected to the
SMBAlert# line.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 Documentation/i2c/slave-testunit-backend.rst | 48 ++++++++++++++++
 drivers/i2c/i2c-slave-testunit.c             | 59 ++++++++++++++++++++
 2 files changed, 107 insertions(+)

diff --git a/Documentation/i2c/slave-testunit-backend.rst b/Documentation/i2c/slave-testunit-backend.rst
index 3743188ecfc7..d752f433be07 100644
--- a/Documentation/i2c/slave-testunit-backend.rst
+++ b/Documentation/i2c/slave-testunit-backend.rst
@@ -185,3 +185,51 @@ default response::
 
   # i2cset -y 0 0x30 4 0 0 i; i2cget -y 0 0x30
   0x00
+
+0x05 SMBUS_ALERT_REQUEST
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. list-table::
+  :header-rows: 1
+
+  * - CMD
+    - DATAL
+    - DATAH
+    - DELAY
+
+  * - 0x05
+    - response value (7 MSBs interpreted as I2C address)
+    - currently unused
+    - n * 10ms
+
+This test raises an interrupt via the SMBAlert pin which the host controller
+must handle. The pin must be connected to the testunit as a GPIO. GPIO access
+is not allowed to sleep. Currently, this can only be described using firmware
+nodes. So, for devicetree, you would add something like this to the testunit
+node::
+
+  gpios = <&gpio1 24 GPIO_ACTIVE_LOW>;
+
+The following command will trigger the alert with a response of 0xc9 after 1
+second of delay::
+
+  # i2cset -y 0 0x30 5 0xc9 0x00 100 i
+
+If the host controller supports SMBusAlert, this message with debug level
+should appear::
+
+  smbus_alert 0-000c: SMBALERT# from dev 0x64, flag 1
+
+This message may appear more than once because the testunit is software not
+hardware and, thus, may not be able to react to the response of the host fast
+enough. The interrupt count should increase only by one, though::
+
+  # cat /proc/interrupts | grep smbus_alert
+   93:          1  gpio-rcar  26 Edge      smbus_alert
+
+If the host does not respond to the alert within 1 second, the test will be
+aborted and the testunit will report an error.
+
+For this test, the testunit will shortly drop its assigned address and listen
+on the SMBus Alert Response Address (0x0c). It will reassign its original
+address afterwards.
diff --git a/drivers/i2c/i2c-slave-testunit.c b/drivers/i2c/i2c-slave-testunit.c
index df54185e5e9a..9fe3150378e8 100644
--- a/drivers/i2c/i2c-slave-testunit.c
+++ b/drivers/i2c/i2c-slave-testunit.c
@@ -8,6 +8,8 @@
 
 #include <generated/utsrelease.h>
 #include <linux/bitops.h>
+#include <linux/completion.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
 #include <linux/module.h>
@@ -22,6 +24,7 @@ enum testunit_cmds {
 	TU_CMD_SMBUS_HOST_NOTIFY,
 	TU_CMD_SMBUS_BLOCK_PROC_CALL,
 	TU_CMD_GET_VERSION_WITH_REP_START,
+	TU_CMD_SMBUS_ALERT_REQUEST,
 	TU_NUM_CMDS
 };
 
@@ -44,10 +47,37 @@ struct testunit_data {
 	u8 read_idx;
 	struct i2c_client *client;
 	struct delayed_work worker;
+	struct gpio_desc *gpio;
+	struct completion alert_done;
 };
 
 static char tu_version_info[] = "v" UTS_RELEASE "\n\0";
 
+static int i2c_slave_testunit_smbalert_cb(struct i2c_client *client,
+					  enum i2c_slave_event event, u8 *val)
+{
+	struct testunit_data *tu = i2c_get_clientdata(client);
+
+	switch (event) {
+	case I2C_SLAVE_READ_PROCESSED:
+		gpiod_set_value(tu->gpio, 0);
+		fallthrough;
+	case I2C_SLAVE_READ_REQUESTED:
+		*val = tu->regs[TU_REG_DATAL];
+		break;
+
+	case I2C_SLAVE_STOP:
+		complete(&tu->alert_done);
+		break;
+
+	case I2C_SLAVE_WRITE_REQUESTED:
+	case I2C_SLAVE_WRITE_RECEIVED:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
 static int i2c_slave_testunit_slave_cb(struct i2c_client *client,
 				     enum i2c_slave_event event, u8 *val)
 {
@@ -127,8 +157,10 @@ static int i2c_slave_testunit_slave_cb(struct i2c_client *client,
 static void i2c_slave_testunit_work(struct work_struct *work)
 {
 	struct testunit_data *tu = container_of(work, struct testunit_data, worker.work);
+	unsigned long time_left;
 	struct i2c_msg msg;
 	u8 msgbuf[256];
+	u16 orig_addr;
 	int ret = 0;
 
 	msg.addr = I2C_CLIENT_END;
@@ -150,6 +182,26 @@ static void i2c_slave_testunit_work(struct work_struct *work)
 		msgbuf[2] = tu->regs[TU_REG_DATAH];
 		break;
 
+	case TU_CMD_SMBUS_ALERT_REQUEST:
+		i2c_slave_unregister(tu->client);
+		orig_addr = tu->client->addr;
+		tu->client->addr = 0x0c;
+		ret = i2c_slave_register(tu->client, i2c_slave_testunit_smbalert_cb);
+		if (ret)
+			goto out_smbalert;
+
+		reinit_completion(&tu->alert_done);
+		gpiod_set_value(tu->gpio, 1);
+		time_left = wait_for_completion_timeout(&tu->alert_done, HZ);
+		if (!time_left)
+			ret = -ETIMEDOUT;
+
+		i2c_slave_unregister(tu->client);
+out_smbalert:
+		tu->client->addr = orig_addr;
+		i2c_slave_register(tu->client, i2c_slave_testunit_slave_cb);
+		break;
+
 	default:
 		break;
 	}
@@ -176,8 +228,15 @@ static int i2c_slave_testunit_probe(struct i2c_client *client)
 
 	tu->client = client;
 	i2c_set_clientdata(client, tu);
+	init_completion(&tu->alert_done);
 	INIT_DELAYED_WORK(&tu->worker, i2c_slave_testunit_work);
 
+	tu->gpio = devm_gpiod_get_index_optional(&client->dev, NULL, 0, GPIOD_OUT_LOW);
+	if (gpiod_cansleep(tu->gpio)) {
+		dev_err(&client->dev, "GPIO access which may sleep is not allowed\n");
+		return -EDEADLK;
+	}
+
 	if (sizeof(tu_version_info) > TU_VERSION_MAX_LENGTH)
 		tu_version_info[TU_VERSION_MAX_LENGTH - 1] = 0;
 
-- 
2.43.0

Re: [PATCH 1/3] i2c: testunit: describe fwnode based instantiation

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 269 bytes --]

On Wed, Aug 14, 2024 at 08:22:07PM +0200, Wolfram Sang wrote:
> The testunit can also be instantiated via firmware nodes. Give a
> devicetree node as an example.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3] i2c: testunit: move code to avoid a forward declaration

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 308 bytes --]

On Wed, Aug 14, 2024 at 08:22:08PM +0200, Wolfram Sang wrote:
> To avoid forward declarations in upcoming code, move the workqueue
> handler as-is downwards. This will ease review of the new features.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/3] i2c: testunit: add SMBusAlert trigger

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 327 bytes --]

On Wed, Aug 14, 2024 at 08:22:09PM +0200, Wolfram Sang wrote:
> To test SMBusAlert handlers, let the testunit be able to trigger
> SMBusAlert interrupts. This new command needs a GPIO connected to the
> SMBAlert# line.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]