public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove
@ 2026-04-03  9:05 Jian Zhang
  2026-04-03  9:05 ` [PATCH v2 2/5] ipmi: ssif_bmc: fix missing check for copy_to_user() partial failure Jian Zhang
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Jian Zhang @ 2026-04-03  9:05 UTC (permalink / raw)
  To: Corey Minyard, Quan Nguyen, openipmi-developer, linux-kernel

The response timer can stay armed across device teardown. If it fires after
remove, the callback dereferences the SSIF context and the i2c client after
teardown has started.

Cancel the timer in remove so the callback cannot run after the device is
unregistered.

Signed-off-by: Jian Zhang <zhangjian.3032@bytedance.com>
---
v2: use timer_delete_sync() to cancel the timer

 drivers/char/ipmi/ssif_bmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c
index 7a52e3ea49ed..dc1d5bb4a460 100644
--- a/drivers/char/ipmi/ssif_bmc.c
+++ b/drivers/char/ipmi/ssif_bmc.c
@@ -843,6 +843,7 @@ static void ssif_bmc_remove(struct i2c_client *client)
 {
 	struct ssif_bmc_ctx *ssif_bmc = i2c_get_clientdata(client);
 
+	timer_delete_sync(&ssif_bmc->response_timer);
 	i2c_slave_unregister(client);
 	misc_deregister(&ssif_bmc->miscdev);
 }
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/5] ipmi: ssif_bmc: fix missing check for copy_to_user() partial failure
  2026-04-03  9:05 [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Jian Zhang
@ 2026-04-03  9:05 ` Jian Zhang
  2026-04-03  9:06 ` [PATCH v2 3/5] ipmi: ssif_bmc: fix message desynchronization after truncated response Jian Zhang
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Jian Zhang @ 2026-04-03  9:05 UTC (permalink / raw)
  To: Corey Minyard, Quan Nguyen, openipmi-developer, linux-kernel

copy_to_user() returns the number of bytes that could not be copied,
with a non-zero value indicating a partial or complete failure. The
current code only checks for negative return values and treats all
non-negative results as success.

Treating any positive return value from copy_to_user() as
an error and returning -EFAULT.

Fixes: dd2bc5cc9e25 ("ipmi: ssif_bmc: Add SSIF BMC driver")
Signed-off-by: Jian Zhang <zhangjian.3032@bytedance.com>
---
 drivers/char/ipmi/ssif_bmc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c
index dc1d5bb4a460..fbc7d2cfd535 100644
--- a/drivers/char/ipmi/ssif_bmc.c
+++ b/drivers/char/ipmi/ssif_bmc.c
@@ -163,6 +163,8 @@ static ssize_t ssif_bmc_read(struct file *file, char __user *buf, size_t count,
 		spin_unlock_irqrestore(&ssif_bmc->lock, flags);
 
 		ret = copy_to_user(buf, &msg, count);
+		if (ret > 0)
+			ret = -EFAULT;
 	}
 
 	return (ret < 0) ? ret : count;
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/5] ipmi: ssif_bmc: fix message desynchronization after truncated response
  2026-04-03  9:05 [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Jian Zhang
  2026-04-03  9:05 ` [PATCH v2 2/5] ipmi: ssif_bmc: fix missing check for copy_to_user() partial failure Jian Zhang
@ 2026-04-03  9:06 ` Jian Zhang
  2026-04-03  9:06 ` [PATCH v2 4/5] ipmi: ssif_bmc: change log level to dbg in irq callback Jian Zhang
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Jian Zhang @ 2026-04-03  9:06 UTC (permalink / raw)
  To: Corey Minyard, Quan Nguyen, openipmi-developer, linux-kernel

A truncated response, caused by host power-off, or other conditions,
can lead to message desynchronization.

Raw trace data (STOP loss scenario, add state transition comment):

1. T-1: Read response phase (SSIF_RES_SENDING)
8271.955342  WR_RCV [03]                          <- Read polling cmd
8271.955348  RD_REQ [04]  <== SSIF_RES_SENDING    <- start sending response
8271.955436  RD_PRO [b4]
8271.955527  RD_PRO [00]
8271.955618  RD_PRO [c1]
8271.955707  RD_PRO [00]
8271.955814  RD_PRO [ad]  <== SSIF_RES_SENDING     <- last byte
	<- !! STOP lost (truncated response)

2. T: New Write request arrives, BMC still in SSIF_RES_SENDING
8271.967973  WR_REQ []    <== SSIF_RES_SENDING >> SSIF_ABORTING  <- log: unexpected WR_REQ in RES_SENDING
8271.968447  WR_RCV [02]  <== SSIF_ABORTING  <- do nothing
8271.968452  WR_RCV [02]  <== SSIF_ABORTING  <- do nothing
8271.968454  WR_RCV [18]  <== SSIF_ABORTING  <- do nothing
8271.968456  WR_RCV [01]  <== SSIF_ABORTING  <- do nothing
8271.968458  WR_RCV [66]  <== SSIF_ABORTING  <- do nothing
8271.978714  STOP []      <== SSIF_ABORTING >> SSIF_READY  <- log: unexpected SLAVE STOP in state=SSIF_ABORTING

3. T+1: Next Read polling, treated as a fresh transaction
8271.979125  WR_REQ []    <== SSIF_READY >> SSIF_START
8271.979326  WR_RCV [03]  <== SSIF_START >> SSIF_SMBUS_CMD        <- smbus_cmd=0x03
8271.979331  RD_REQ [04]  <== SSIF_RES_SENDING      <- sending response
8271.979427  RD_PRO [b4]                            <- !! this is T's stale response -> desynchronization

When in SSIF_ABORTING state, a newly arrived command should still be
handled to avoid dropping the request or causing message
desynchronization.

Fixes: dd2bc5cc9e25 ("ipmi: ssif_bmc: Add SSIF BMC driver")
Signed-off-by: Jian Zhang <zhangjian.3032@bytedance.com>
---
 drivers/char/ipmi/ssif_bmc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c
index fbc7d2cfd535..a1e1a7f1e104 100644
--- a/drivers/char/ipmi/ssif_bmc.c
+++ b/drivers/char/ipmi/ssif_bmc.c
@@ -458,6 +458,15 @@ static bool supported_write_cmd(u8 cmd)
 	return false;
 }
 
+static bool supported_write_start_cmd(u8 cmd)
+{
+	if (cmd == SSIF_IPMI_SINGLEPART_WRITE ||
+	    cmd == SSIF_IPMI_MULTIPART_WRITE_START)
+		return true;
+
+	return false;
+}
+
 /* Process the IPMI response that will be read by master */
 static void handle_read_processed(struct ssif_bmc_ctx *ssif_bmc, u8 *val)
 {
@@ -709,6 +718,11 @@ static void on_write_received_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val)
 			ssif_bmc->state = SSIF_ABORTING;
 		else
 			ssif_bmc->state = SSIF_REQ_RECVING;
+	} else if (ssif_bmc->state == SSIF_ABORTING) {
+		if (supported_write_start_cmd(*val)) {
+			ssif_bmc->state = SSIF_SMBUS_CMD;
+			ssif_bmc->aborting = false;
+		}
 	}
 
 	/* This is response sending state */
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/5] ipmi: ssif_bmc: change log level to dbg in irq callback
  2026-04-03  9:05 [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Jian Zhang
  2026-04-03  9:05 ` [PATCH v2 2/5] ipmi: ssif_bmc: fix missing check for copy_to_user() partial failure Jian Zhang
  2026-04-03  9:06 ` [PATCH v2 3/5] ipmi: ssif_bmc: fix message desynchronization after truncated response Jian Zhang
@ 2026-04-03  9:06 ` Jian Zhang
  2026-04-03  9:06 ` [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine Jian Zhang
  2026-04-03 13:12 ` [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Corey Minyard
  4 siblings, 0 replies; 9+ messages in thread
From: Jian Zhang @ 2026-04-03  9:06 UTC (permalink / raw)
  To: Corey Minyard, Quan Nguyen, openipmi-developer, linux-kernel

Long-running tests indicate that this logging can occasionally disrupt
timing and lead to request/response corruption.

Irq handler need to be executed as fast as possible,
most I2C slave IRQ implementations are byte-level, logging here
can significantly affect transfer behavior and timing. It is recommended
to use dev_dbg() for these messages.

Fixes: dd2bc5cc9e25 ("ipmi: ssif_bmc: Add SSIF BMC driver")
Signed-off-by: Jian Zhang <zhangjian.3032@bytedance.com>
---
 drivers/char/ipmi/ssif_bmc.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c
index a1e1a7f1e104..646a1e9ffbb7 100644
--- a/drivers/char/ipmi/ssif_bmc.c
+++ b/drivers/char/ipmi/ssif_bmc.c
@@ -569,7 +569,7 @@ static void process_request_part(struct ssif_bmc_ctx *ssif_bmc)
 		len = ssif_bmc->request.len + part->length;
 		/* Do the bound check here, not allow the request len exceed 254 bytes */
 		if (len > IPMI_SSIF_PAYLOAD_MAX) {
-			dev_warn(&ssif_bmc->client->dev,
+			dev_dbg(&ssif_bmc->client->dev,
 				 "Warn: Request exceeded 254 bytes, aborting");
 			/* Request too long, aborting */
 			ssif_bmc->aborting =  true;
@@ -615,7 +615,7 @@ static void on_read_requested_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val)
 	    ssif_bmc->state == SSIF_START ||
 	    ssif_bmc->state == SSIF_REQ_RECVING ||
 	    ssif_bmc->state == SSIF_RES_SENDING) {
-		dev_warn(&ssif_bmc->client->dev,
+		dev_dbg(&ssif_bmc->client->dev,
 			 "Warn: %s unexpected READ REQUESTED in state=%s\n",
 			 __func__, state_to_string(ssif_bmc->state));
 		ssif_bmc->state = SSIF_ABORTING;
@@ -624,7 +624,7 @@ static void on_read_requested_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val)
 
 	} else if (ssif_bmc->state == SSIF_SMBUS_CMD) {
 		if (!supported_read_cmd(ssif_bmc->part_buf.smbus_cmd)) {
-			dev_warn(&ssif_bmc->client->dev, "Warn: Unknown SMBus read command=0x%x",
+			dev_dbg(&ssif_bmc->client->dev, "Warn: Unknown SMBus read command=0x%x",
 				 ssif_bmc->part_buf.smbus_cmd);
 			ssif_bmc->aborting = true;
 		}
@@ -659,7 +659,7 @@ static void on_read_processed_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val)
 	    ssif_bmc->state == SSIF_START ||
 	    ssif_bmc->state == SSIF_REQ_RECVING ||
 	    ssif_bmc->state == SSIF_SMBUS_CMD) {
-		dev_warn(&ssif_bmc->client->dev,
+		dev_dbg(&ssif_bmc->client->dev,
 			 "Warn: %s unexpected READ PROCESSED in state=%s\n",
 			 __func__, state_to_string(ssif_bmc->state));
 		ssif_bmc->state = SSIF_ABORTING;
@@ -684,7 +684,7 @@ static void on_write_requested_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val)
 	} else if (ssif_bmc->state == SSIF_START ||
 		   ssif_bmc->state == SSIF_REQ_RECVING ||
 		   ssif_bmc->state == SSIF_RES_SENDING) {
-		dev_warn(&ssif_bmc->client->dev,
+		dev_dbg(&ssif_bmc->client->dev,
 			 "Warn: %s unexpected WRITE REQUEST in state=%s\n",
 			 __func__, state_to_string(ssif_bmc->state));
 		ssif_bmc->state = SSIF_ABORTING;
@@ -699,7 +699,7 @@ static void on_write_received_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val)
 {
 	if (ssif_bmc->state == SSIF_READY ||
 	    ssif_bmc->state == SSIF_RES_SENDING) {
-		dev_warn(&ssif_bmc->client->dev,
+		dev_dbg(&ssif_bmc->client->dev,
 			 "Warn: %s unexpected WRITE RECEIVED in state=%s\n",
 			 __func__, state_to_string(ssif_bmc->state));
 		ssif_bmc->state = SSIF_ABORTING;
@@ -709,7 +709,7 @@ static void on_write_received_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val)
 
 	} else if (ssif_bmc->state == SSIF_SMBUS_CMD) {
 		if (!supported_write_cmd(ssif_bmc->part_buf.smbus_cmd)) {
-			dev_warn(&ssif_bmc->client->dev, "Warn: Unknown SMBus write command=0x%x",
+			dev_dbg(&ssif_bmc->client->dev, "Warn: Unknown SMBus write command=0x%x",
 				 ssif_bmc->part_buf.smbus_cmd);
 			ssif_bmc->aborting = true;
 		}
@@ -738,7 +738,7 @@ static void on_stop_event(struct ssif_bmc_ctx *ssif_bmc, u8 *val)
 	    ssif_bmc->state == SSIF_START ||
 	    ssif_bmc->state == SSIF_SMBUS_CMD ||
 	    ssif_bmc->state == SSIF_ABORTING) {
-		dev_warn(&ssif_bmc->client->dev,
+		dev_dbg(&ssif_bmc->client->dev,
 			 "Warn: %s unexpected SLAVE STOP in state=%s\n",
 			 __func__, state_to_string(ssif_bmc->state));
 		ssif_bmc->state = SSIF_READY;
@@ -805,7 +805,7 @@ static int ssif_bmc_cb(struct i2c_client *client, enum i2c_slave_event event, u8
 		break;
 
 	default:
-		dev_warn(&ssif_bmc->client->dev, "Warn: Unknown i2c slave event\n");
+		dev_dbg(&ssif_bmc->client->dev, "Warn: Unknown i2c slave event\n");
 		break;
 	}
 
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine
  2026-04-03  9:05 [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Jian Zhang
                   ` (2 preceding siblings ...)
  2026-04-03  9:06 ` [PATCH v2 4/5] ipmi: ssif_bmc: change log level to dbg in irq callback Jian Zhang
@ 2026-04-03  9:06 ` Jian Zhang
  2026-04-03 13:14   ` Corey Minyard
  2026-04-03 13:12 ` [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Corey Minyard
  4 siblings, 1 reply; 9+ messages in thread
From: Jian Zhang @ 2026-04-03  9:06 UTC (permalink / raw)
  To: Corey Minyard, Quan Nguyen, openipmi-developer, linux-kernel

Add some unit test for state machine when in SSIF_ABORTING state.

Fixes: dd2bc5cc9e25 ("ipmi: ssif_bmc: Add SSIF BMC driver")
Signed-off-by: Jian Zhang <zhangjian.3032@bytedance.com>
---
v2: remove undefined symbol response_in_send

 drivers/char/ipmi/Kconfig    |  10 +
 drivers/char/ipmi/ssif_bmc.c | 401 +++++++++++++++++++++++++++++++++++
 2 files changed, 411 insertions(+)

diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
index 92bed266d07c..709820f80266 100644
--- a/drivers/char/ipmi/Kconfig
+++ b/drivers/char/ipmi/Kconfig
@@ -187,6 +187,16 @@ config SSIF_IPMI_BMC
 	  The driver implements the BMC side of the SMBus system
 	  interface (SSIF).
 
+config SSIF_IPMI_BMC_KUNIT_TEST
+	bool "KUnit tests for SSIF IPMI BMC driver" if !KUNIT_ALL_TESTS
+	depends on KUNIT=y
+	depends on SSIF_IPMI_BMC=y
+	default KUNIT_ALL_TESTS
+	help
+	  This option builds unit tests that exercise the SSIF BMC state
+	  machine, including request handling, response transmission,
+	  and error paths such as aborted or truncated transfers.
+
 config IPMB_DEVICE_INTERFACE
 	tristate 'IPMB Interface handler'
 	depends on I2C
diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c
index 646a1e9ffbb7..726626126cea 100644
--- a/drivers/char/ipmi/ssif_bmc.c
+++ b/drivers/char/ipmi/ssif_bmc.c
@@ -18,6 +18,9 @@
 #include <linux/timer.h>
 #include <linux/jiffies.h>
 #include <linux/ipmi_ssif_bmc.h>
+#if IS_ENABLED(CONFIG_SSIF_IPMI_BMC_KUNIT_TEST)
+#include <kunit/test.h>
+#endif
 
 #define DEVICE_NAME                             "ipmi-ssif-host"
 
@@ -886,6 +889,404 @@ static struct i2c_driver ssif_bmc_driver = {
 	.id_table       = ssif_bmc_id,
 };
 
+#if IS_ENABLED(CONFIG_SSIF_IPMI_BMC_KUNIT_TEST)
+struct ssif_bmc_test_ctx {
+	struct ssif_bmc_ctx ssif_bmc;
+	struct i2c_client client;
+	struct i2c_adapter adapter;
+	struct i2c_algorithm algo;
+};
+
+static int ssif_bmc_test_init(struct kunit *test)
+{
+	struct ssif_bmc_test_ctx *test_ctx;
+
+	test_ctx = kunit_kzalloc(test, sizeof(*test_ctx), GFP_KERNEL);
+	if (!test_ctx)
+		return -ENOMEM;
+
+	test_ctx->adapter.algo = &test_ctx->algo;
+	test_ctx->client.addr = 0x20;
+	test_ctx->client.adapter = &test_ctx->adapter;
+
+	spin_lock_init(&test_ctx->ssif_bmc.lock);
+	init_waitqueue_head(&test_ctx->ssif_bmc.wait_queue);
+	test_ctx->ssif_bmc.client = &test_ctx->client;
+	i2c_set_clientdata(&test_ctx->client, &test_ctx->ssif_bmc);
+
+	test->priv = test_ctx;
+
+	return 0;
+}
+
+static void ssif_bmc_test_exit(struct kunit *test)
+{
+	struct ssif_bmc_test_ctx *test_ctx = test->priv;
+
+	if (test_ctx->ssif_bmc.response_timer_inited)
+		timer_delete_sync(&test_ctx->ssif_bmc.response_timer);
+}
+
+static int ssif_bmc_test_run_event_val(struct ssif_bmc_test_ctx *test_ctx,
+					       enum i2c_slave_event event,
+					       u8 *value)
+{
+	return ssif_bmc_cb(&test_ctx->client, event, value);
+}
+
+static int ssif_bmc_test_run_event(struct ssif_bmc_test_ctx *test_ctx,
+					   enum i2c_slave_event event, u8 value)
+{
+	return ssif_bmc_test_run_event_val(test_ctx, event, &value);
+}
+
+static void ssif_bmc_test_singlepart_req(struct kunit *test)
+{
+	struct ssif_bmc_test_ctx *test_ctx = test->priv;
+	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
+
+	ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
+				GET_8BIT_ADDR(test_ctx->client.addr));
+	ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
+				SSIF_IPMI_SINGLEPART_WRITE);
+	ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 2);
+
+	ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xaa);
+
+	ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x55);
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
+
+	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
+	KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
+	KUNIT_EXPECT_TRUE(test, ssif_bmc->busy);
+	KUNIT_EXPECT_FALSE(test, ssif_bmc->aborting);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 2);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0xaa);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[1], 0x55);
+	KUNIT_EXPECT_TRUE(test, ssif_bmc->response_timer_inited);
+}
+
+static void ssif_bmc_test_restart_write_without_stop(struct kunit *test)
+{
+	struct ssif_bmc_test_ctx *test_ctx = test->priv;
+	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
+
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
+						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
+						SSIF_IPMI_SINGLEPART_WRITE), 0);
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 2), 0);
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xde), 0);
+
+	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING);
+
+	/* Write transaction, without stop, and new request coming */
+	ssif_bmc_test_singlepart_req(test);
+}
+
+
+static void ssif_bmc_test_restart_after_invalid_command(struct kunit *test)
+{
+	struct ssif_bmc_test_ctx *test_ctx = test->priv;
+	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
+
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
+						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xff), 0);
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
+
+	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_ABORTING);
+	KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting);
+
+	/* After An Invalid Command, expect could handle new request */
+	ssif_bmc_test_singlepart_req(test);
+}
+
+static void ssif_bmc_test_restart_after_invalid_length(struct kunit *test)
+{
+	struct ssif_bmc_test_ctx *test_ctx = test->priv;
+	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
+	int i;
+
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
+						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
+						SSIF_IPMI_SINGLEPART_WRITE), 0);
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
+						MAX_PAYLOAD_PER_TRANSACTION + 1), 0);
+
+	for (i = 0; i < MAX_PAYLOAD_PER_TRANSACTION + 1; i++)
+		KUNIT_ASSERT_EQ(test,
+				ssif_bmc_test_run_event(test_ctx,
+							I2C_SLAVE_WRITE_RECEIVED, i), 0);
+
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
+	KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting);
+	KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING);
+
+	ssif_bmc_test_singlepart_req(test);
+}
+
+static void ssif_bmc_test_singlepart_read_response_completion(struct kunit *test)
+{
+	struct ssif_bmc_test_ctx *test_ctx = test->priv;
+	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
+	u8 value;
+
+	ssif_bmc->state = SSIF_SMBUS_CMD;
+	ssif_bmc->part_buf.smbus_cmd = SSIF_IPMI_SINGLEPART_READ;
+	ssif_bmc->response.len = 2;
+	ssif_bmc->response.payload[0] = 0x11;
+	ssif_bmc->response.payload[1] = 0x22;
+	ssif_bmc->response_in_progress = true;
+	ssif_bmc->is_singlepart_read = true;
+	ssif_bmc->pec_support = true;
+
+	value = 0;
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
+						    &value), 0);
+	KUNIT_EXPECT_EQ(test, value, 2);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_RES_SENDING);
+
+	value = 0;
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
+						    &value), 0);
+	KUNIT_EXPECT_EQ(test, value, 0x11);
+
+	value = 0;
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
+						    &value), 0);
+	KUNIT_EXPECT_EQ(test, value, 0x22);
+
+	value = 0;
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
+						    &value), 0);
+	KUNIT_EXPECT_EQ(test, value, ssif_bmc->part_buf.pec);
+
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
+	KUNIT_EXPECT_FALSE(test, ssif_bmc->response_in_progress);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->response.len, 0);
+}
+
+static void ssif_bmc_test_stop_during_start_discards_partial_request(struct kunit *test)
+{
+	struct ssif_bmc_test_ctx *test_ctx = test->priv;
+	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
+
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
+						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_START);
+
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
+	KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->msg_idx, 0);
+
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
+						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
+						SSIF_IPMI_SINGLEPART_WRITE), 0);
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x77), 0);
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
+
+	KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 1);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0x77);
+}
+
+static void ssif_bmc_test_read_interrupts_partial_write(struct kunit *test)
+{
+	struct ssif_bmc_test_ctx *test_ctx = test->priv;
+	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
+	u8 value = 0xff;
+
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
+						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
+						SSIF_IPMI_SINGLEPART_WRITE), 0);
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 2), 0);
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xab), 0);
+
+	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING);
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
+						    &value), 0);
+	KUNIT_EXPECT_EQ(test, value, 0);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_ABORTING);
+
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
+	KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 0);
+
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
+						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
+						SSIF_IPMI_SINGLEPART_WRITE), 0);
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xcd), 0);
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
+
+	KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 1);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0xcd);
+}
+
+static void ssif_bmc_test_write_interrupts_response_send(struct kunit *test)
+{
+	struct ssif_bmc_test_ctx *test_ctx = test->priv;
+	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
+	u8 value = 0;
+
+	ssif_bmc->state = SSIF_SMBUS_CMD;
+	ssif_bmc->part_buf.smbus_cmd = SSIF_IPMI_SINGLEPART_READ;
+	ssif_bmc->response.len = 1;
+	ssif_bmc->response.payload[0] = 0x66;
+	ssif_bmc->response_in_progress = true;
+	ssif_bmc->is_singlepart_read = true;
+
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
+						    &value), 0);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_RES_SENDING);
+
+	/* READ_REQUESTED transaction */
+	ssif_bmc_test_singlepart_req(test);
+}
+
+static void ssif_bmc_test_write_interrupts_response_sending(struct kunit *test)
+{
+	struct ssif_bmc_test_ctx *test_ctx = test->priv;
+	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
+	u8 value = 0;
+
+	ssif_bmc->state = SSIF_SMBUS_CMD;
+	ssif_bmc->part_buf.smbus_cmd = SSIF_IPMI_SINGLEPART_READ;
+	ssif_bmc->response.len = 1;
+	ssif_bmc->response.payload[0] = 0x66;
+	ssif_bmc->response_in_progress = true;
+	ssif_bmc->is_singlepart_read = true;
+
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
+						    &value), 0);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_RES_SENDING);
+
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
+						    &value), 0);
+	KUNIT_EXPECT_EQ(test, value, 0x66);
+
+	/* READ_REQUESTED transaction */
+	ssif_bmc_test_singlepart_req(test);
+}
+
+static void ssif_bmc_test_timeout_interrupt_allows_retry(struct kunit *test)
+{
+	struct ssif_bmc_test_ctx *test_ctx = test->priv;
+	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
+
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
+						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
+						SSIF_IPMI_SINGLEPART_WRITE), 0);
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x21), 0);
+	KUNIT_ASSERT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
+
+	KUNIT_ASSERT_TRUE(test, timer_pending(&ssif_bmc->response_timer));
+	timer_delete_sync(&ssif_bmc->response_timer);
+	response_timeout(&ssif_bmc->response_timer);
+
+	KUNIT_EXPECT_FALSE(test, ssif_bmc->busy);
+	KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting);
+	KUNIT_EXPECT_FALSE(test, ssif_bmc->response_timer_inited);
+
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
+						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
+						SSIF_IPMI_SINGLEPART_WRITE), 0);
+	KUNIT_EXPECT_FALSE(test, ssif_bmc->aborting);
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x22), 0);
+	KUNIT_EXPECT_EQ(test,
+			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
+
+	KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 1);
+	KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0x22);
+}
+
+static struct kunit_case ssif_bmc_test_cases[] = {
+	KUNIT_CASE(ssif_bmc_test_singlepart_req),
+	KUNIT_CASE(ssif_bmc_test_restart_write_without_stop),
+	KUNIT_CASE(ssif_bmc_test_restart_after_invalid_command),
+	KUNIT_CASE(ssif_bmc_test_restart_after_invalid_length),
+	KUNIT_CASE(ssif_bmc_test_singlepart_read_response_completion),
+	KUNIT_CASE(ssif_bmc_test_stop_during_start_discards_partial_request),
+	KUNIT_CASE(ssif_bmc_test_read_interrupts_partial_write),
+	KUNIT_CASE(ssif_bmc_test_write_interrupts_response_send),
+	KUNIT_CASE(ssif_bmc_test_write_interrupts_response_sending),
+	KUNIT_CASE(ssif_bmc_test_timeout_interrupt_allows_retry),
+	{}
+};
+
+static struct kunit_suite ssif_bmc_test_suite = {
+	.name = "ssif_bmc_test",
+	.init = ssif_bmc_test_init,
+	.exit = ssif_bmc_test_exit,
+	.test_cases = ssif_bmc_test_cases,
+};
+
+kunit_test_suite(ssif_bmc_test_suite);
+#endif
+
 module_i2c_driver(ssif_bmc_driver);
 
 MODULE_AUTHOR("Quan Nguyen <quan@os.amperecomputing.com>");
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove
  2026-04-03  9:05 [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Jian Zhang
                   ` (3 preceding siblings ...)
  2026-04-03  9:06 ` [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine Jian Zhang
@ 2026-04-03 13:12 ` Corey Minyard
  4 siblings, 0 replies; 9+ messages in thread
From: Corey Minyard @ 2026-04-03 13:12 UTC (permalink / raw)
  To: Jian Zhang; +Cc: Quan Nguyen, openipmi-developer, linux-kernel

On Fri, Apr 03, 2026 at 05:05:58PM +0800, Jian Zhang wrote:
> The response timer can stay armed across device teardown. If it fires after
> remove, the callback dereferences the SSIF context and the i2c client after
> teardown has started.
> 
> Cancel the timer in remove so the callback cannot run after the device is
> unregistered.

Thanks for the updates on this.  I have the new version in my tree.  I
have one question on the kunit patch, I'll ask it there.

-corey

> 
> Signed-off-by: Jian Zhang <zhangjian.3032@bytedance.com>
> ---
> v2: use timer_delete_sync() to cancel the timer
> 
>  drivers/char/ipmi/ssif_bmc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c
> index 7a52e3ea49ed..dc1d5bb4a460 100644
> --- a/drivers/char/ipmi/ssif_bmc.c
> +++ b/drivers/char/ipmi/ssif_bmc.c
> @@ -843,6 +843,7 @@ static void ssif_bmc_remove(struct i2c_client *client)
>  {
>  	struct ssif_bmc_ctx *ssif_bmc = i2c_get_clientdata(client);
>  
> +	timer_delete_sync(&ssif_bmc->response_timer);
>  	i2c_slave_unregister(client);
>  	misc_deregister(&ssif_bmc->miscdev);
>  }
> -- 
> 2.20.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine
  2026-04-03  9:06 ` [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine Jian Zhang
@ 2026-04-03 13:14   ` Corey Minyard
  2026-04-03 14:12     ` Jian Zhang
  0 siblings, 1 reply; 9+ messages in thread
From: Corey Minyard @ 2026-04-03 13:14 UTC (permalink / raw)
  To: Jian Zhang; +Cc: Quan Nguyen, openipmi-developer, linux-kernel

On Fri, Apr 03, 2026 at 05:06:02PM +0800, Jian Zhang wrote:
> Add some unit test for state machine when in SSIF_ABORTING state.
> 
> Fixes: dd2bc5cc9e25 ("ipmi: ssif_bmc: Add SSIF BMC driver")
> Signed-off-by: Jian Zhang <zhangjian.3032@bytedance.com>
> ---
> v2: remove undefined symbol response_in_send
> 
>  drivers/char/ipmi/Kconfig    |  10 +
>  drivers/char/ipmi/ssif_bmc.c | 401 +++++++++++++++++++++++++++++++++++
>  2 files changed, 411 insertions(+)
> 
> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> index 92bed266d07c..709820f80266 100644
> --- a/drivers/char/ipmi/Kconfig
> +++ b/drivers/char/ipmi/Kconfig
> @@ -187,6 +187,16 @@ config SSIF_IPMI_BMC
>  	  The driver implements the BMC side of the SMBus system
>  	  interface (SSIF).
>  
> +config SSIF_IPMI_BMC_KUNIT_TEST
> +	bool "KUnit tests for SSIF IPMI BMC driver" if !KUNIT_ALL_TESTS
> +	depends on KUNIT=y
> +	depends on SSIF_IPMI_BMC=y

Any reason for the "=y" in the above two?

It's best to remove them if they are not needed so it can work in a
module.  I can make that change, there's no need for a new series for
that.

Oh, and I forgot to say in the reply on the first change, thanks Quan
for reviewing these.

-corey

> +	default KUNIT_ALL_TESTS
> +	help
> +	  This option builds unit tests that exercise the SSIF BMC state
> +	  machine, including request handling, response transmission,
> +	  and error paths such as aborted or truncated transfers.
> +
>  config IPMB_DEVICE_INTERFACE
>  	tristate 'IPMB Interface handler'
>  	depends on I2C
> diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c
> index 646a1e9ffbb7..726626126cea 100644
> --- a/drivers/char/ipmi/ssif_bmc.c
> +++ b/drivers/char/ipmi/ssif_bmc.c
> @@ -18,6 +18,9 @@
>  #include <linux/timer.h>
>  #include <linux/jiffies.h>
>  #include <linux/ipmi_ssif_bmc.h>
> +#if IS_ENABLED(CONFIG_SSIF_IPMI_BMC_KUNIT_TEST)
> +#include <kunit/test.h>
> +#endif
>  
>  #define DEVICE_NAME                             "ipmi-ssif-host"
>  
> @@ -886,6 +889,404 @@ static struct i2c_driver ssif_bmc_driver = {
>  	.id_table       = ssif_bmc_id,
>  };
>  
> +#if IS_ENABLED(CONFIG_SSIF_IPMI_BMC_KUNIT_TEST)
> +struct ssif_bmc_test_ctx {
> +	struct ssif_bmc_ctx ssif_bmc;
> +	struct i2c_client client;
> +	struct i2c_adapter adapter;
> +	struct i2c_algorithm algo;
> +};
> +
> +static int ssif_bmc_test_init(struct kunit *test)
> +{
> +	struct ssif_bmc_test_ctx *test_ctx;
> +
> +	test_ctx = kunit_kzalloc(test, sizeof(*test_ctx), GFP_KERNEL);
> +	if (!test_ctx)
> +		return -ENOMEM;
> +
> +	test_ctx->adapter.algo = &test_ctx->algo;
> +	test_ctx->client.addr = 0x20;
> +	test_ctx->client.adapter = &test_ctx->adapter;
> +
> +	spin_lock_init(&test_ctx->ssif_bmc.lock);
> +	init_waitqueue_head(&test_ctx->ssif_bmc.wait_queue);
> +	test_ctx->ssif_bmc.client = &test_ctx->client;
> +	i2c_set_clientdata(&test_ctx->client, &test_ctx->ssif_bmc);
> +
> +	test->priv = test_ctx;
> +
> +	return 0;
> +}
> +
> +static void ssif_bmc_test_exit(struct kunit *test)
> +{
> +	struct ssif_bmc_test_ctx *test_ctx = test->priv;
> +
> +	if (test_ctx->ssif_bmc.response_timer_inited)
> +		timer_delete_sync(&test_ctx->ssif_bmc.response_timer);
> +}
> +
> +static int ssif_bmc_test_run_event_val(struct ssif_bmc_test_ctx *test_ctx,
> +					       enum i2c_slave_event event,
> +					       u8 *value)
> +{
> +	return ssif_bmc_cb(&test_ctx->client, event, value);
> +}
> +
> +static int ssif_bmc_test_run_event(struct ssif_bmc_test_ctx *test_ctx,
> +					   enum i2c_slave_event event, u8 value)
> +{
> +	return ssif_bmc_test_run_event_val(test_ctx, event, &value);
> +}
> +
> +static void ssif_bmc_test_singlepart_req(struct kunit *test)
> +{
> +	struct ssif_bmc_test_ctx *test_ctx = test->priv;
> +	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> +
> +	ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> +				GET_8BIT_ADDR(test_ctx->client.addr));
> +	ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> +				SSIF_IPMI_SINGLEPART_WRITE);
> +	ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 2);
> +
> +	ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xaa);
> +
> +	ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x55);
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
> +
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
> +	KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
> +	KUNIT_EXPECT_TRUE(test, ssif_bmc->busy);
> +	KUNIT_EXPECT_FALSE(test, ssif_bmc->aborting);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 2);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0xaa);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[1], 0x55);
> +	KUNIT_EXPECT_TRUE(test, ssif_bmc->response_timer_inited);
> +}
> +
> +static void ssif_bmc_test_restart_write_without_stop(struct kunit *test)
> +{
> +	struct ssif_bmc_test_ctx *test_ctx = test->priv;
> +	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> +
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> +						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> +						SSIF_IPMI_SINGLEPART_WRITE), 0);
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 2), 0);
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xde), 0);
> +
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING);
> +
> +	/* Write transaction, without stop, and new request coming */
> +	ssif_bmc_test_singlepart_req(test);
> +}
> +
> +
> +static void ssif_bmc_test_restart_after_invalid_command(struct kunit *test)
> +{
> +	struct ssif_bmc_test_ctx *test_ctx = test->priv;
> +	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> +
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> +						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xff), 0);
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
> +
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_ABORTING);
> +	KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting);
> +
> +	/* After An Invalid Command, expect could handle new request */
> +	ssif_bmc_test_singlepart_req(test);
> +}
> +
> +static void ssif_bmc_test_restart_after_invalid_length(struct kunit *test)
> +{
> +	struct ssif_bmc_test_ctx *test_ctx = test->priv;
> +	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> +	int i;
> +
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> +						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> +						SSIF_IPMI_SINGLEPART_WRITE), 0);
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> +						MAX_PAYLOAD_PER_TRANSACTION + 1), 0);
> +
> +	for (i = 0; i < MAX_PAYLOAD_PER_TRANSACTION + 1; i++)
> +		KUNIT_ASSERT_EQ(test,
> +				ssif_bmc_test_run_event(test_ctx,
> +							I2C_SLAVE_WRITE_RECEIVED, i), 0);
> +
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
> +	KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting);
> +	KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING);
> +
> +	ssif_bmc_test_singlepart_req(test);
> +}
> +
> +static void ssif_bmc_test_singlepart_read_response_completion(struct kunit *test)
> +{
> +	struct ssif_bmc_test_ctx *test_ctx = test->priv;
> +	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> +	u8 value;
> +
> +	ssif_bmc->state = SSIF_SMBUS_CMD;
> +	ssif_bmc->part_buf.smbus_cmd = SSIF_IPMI_SINGLEPART_READ;
> +	ssif_bmc->response.len = 2;
> +	ssif_bmc->response.payload[0] = 0x11;
> +	ssif_bmc->response.payload[1] = 0x22;
> +	ssif_bmc->response_in_progress = true;
> +	ssif_bmc->is_singlepart_read = true;
> +	ssif_bmc->pec_support = true;
> +
> +	value = 0;
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
> +						    &value), 0);
> +	KUNIT_EXPECT_EQ(test, value, 2);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_RES_SENDING);
> +
> +	value = 0;
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
> +						    &value), 0);
> +	KUNIT_EXPECT_EQ(test, value, 0x11);
> +
> +	value = 0;
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
> +						    &value), 0);
> +	KUNIT_EXPECT_EQ(test, value, 0x22);
> +
> +	value = 0;
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
> +						    &value), 0);
> +	KUNIT_EXPECT_EQ(test, value, ssif_bmc->part_buf.pec);
> +
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
> +	KUNIT_EXPECT_FALSE(test, ssif_bmc->response_in_progress);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->response.len, 0);
> +}
> +
> +static void ssif_bmc_test_stop_during_start_discards_partial_request(struct kunit *test)
> +{
> +	struct ssif_bmc_test_ctx *test_ctx = test->priv;
> +	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> +
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> +						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_START);
> +
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
> +	KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->msg_idx, 0);
> +
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> +						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> +						SSIF_IPMI_SINGLEPART_WRITE), 0);
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x77), 0);
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
> +
> +	KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 1);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0x77);
> +}
> +
> +static void ssif_bmc_test_read_interrupts_partial_write(struct kunit *test)
> +{
> +	struct ssif_bmc_test_ctx *test_ctx = test->priv;
> +	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> +	u8 value = 0xff;
> +
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> +						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> +						SSIF_IPMI_SINGLEPART_WRITE), 0);
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 2), 0);
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xab), 0);
> +
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING);
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
> +						    &value), 0);
> +	KUNIT_EXPECT_EQ(test, value, 0);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_ABORTING);
> +
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
> +	KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 0);
> +
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> +						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> +						SSIF_IPMI_SINGLEPART_WRITE), 0);
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xcd), 0);
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
> +
> +	KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 1);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0xcd);
> +}
> +
> +static void ssif_bmc_test_write_interrupts_response_send(struct kunit *test)
> +{
> +	struct ssif_bmc_test_ctx *test_ctx = test->priv;
> +	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> +	u8 value = 0;
> +
> +	ssif_bmc->state = SSIF_SMBUS_CMD;
> +	ssif_bmc->part_buf.smbus_cmd = SSIF_IPMI_SINGLEPART_READ;
> +	ssif_bmc->response.len = 1;
> +	ssif_bmc->response.payload[0] = 0x66;
> +	ssif_bmc->response_in_progress = true;
> +	ssif_bmc->is_singlepart_read = true;
> +
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
> +						    &value), 0);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_RES_SENDING);
> +
> +	/* READ_REQUESTED transaction */
> +	ssif_bmc_test_singlepart_req(test);
> +}
> +
> +static void ssif_bmc_test_write_interrupts_response_sending(struct kunit *test)
> +{
> +	struct ssif_bmc_test_ctx *test_ctx = test->priv;
> +	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> +	u8 value = 0;
> +
> +	ssif_bmc->state = SSIF_SMBUS_CMD;
> +	ssif_bmc->part_buf.smbus_cmd = SSIF_IPMI_SINGLEPART_READ;
> +	ssif_bmc->response.len = 1;
> +	ssif_bmc->response.payload[0] = 0x66;
> +	ssif_bmc->response_in_progress = true;
> +	ssif_bmc->is_singlepart_read = true;
> +
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
> +						    &value), 0);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_RES_SENDING);
> +
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
> +						    &value), 0);
> +	KUNIT_EXPECT_EQ(test, value, 0x66);
> +
> +	/* READ_REQUESTED transaction */
> +	ssif_bmc_test_singlepart_req(test);
> +}
> +
> +static void ssif_bmc_test_timeout_interrupt_allows_retry(struct kunit *test)
> +{
> +	struct ssif_bmc_test_ctx *test_ctx = test->priv;
> +	struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> +
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> +						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> +						SSIF_IPMI_SINGLEPART_WRITE), 0);
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x21), 0);
> +	KUNIT_ASSERT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
> +
> +	KUNIT_ASSERT_TRUE(test, timer_pending(&ssif_bmc->response_timer));
> +	timer_delete_sync(&ssif_bmc->response_timer);
> +	response_timeout(&ssif_bmc->response_timer);
> +
> +	KUNIT_EXPECT_FALSE(test, ssif_bmc->busy);
> +	KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting);
> +	KUNIT_EXPECT_FALSE(test, ssif_bmc->response_timer_inited);
> +
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> +						GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> +						SSIF_IPMI_SINGLEPART_WRITE), 0);
> +	KUNIT_EXPECT_FALSE(test, ssif_bmc->aborting);
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x22), 0);
> +	KUNIT_EXPECT_EQ(test,
> +			ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
> +
> +	KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 1);
> +	KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0x22);
> +}
> +
> +static struct kunit_case ssif_bmc_test_cases[] = {
> +	KUNIT_CASE(ssif_bmc_test_singlepart_req),
> +	KUNIT_CASE(ssif_bmc_test_restart_write_without_stop),
> +	KUNIT_CASE(ssif_bmc_test_restart_after_invalid_command),
> +	KUNIT_CASE(ssif_bmc_test_restart_after_invalid_length),
> +	KUNIT_CASE(ssif_bmc_test_singlepart_read_response_completion),
> +	KUNIT_CASE(ssif_bmc_test_stop_during_start_discards_partial_request),
> +	KUNIT_CASE(ssif_bmc_test_read_interrupts_partial_write),
> +	KUNIT_CASE(ssif_bmc_test_write_interrupts_response_send),
> +	KUNIT_CASE(ssif_bmc_test_write_interrupts_response_sending),
> +	KUNIT_CASE(ssif_bmc_test_timeout_interrupt_allows_retry),
> +	{}
> +};
> +
> +static struct kunit_suite ssif_bmc_test_suite = {
> +	.name = "ssif_bmc_test",
> +	.init = ssif_bmc_test_init,
> +	.exit = ssif_bmc_test_exit,
> +	.test_cases = ssif_bmc_test_cases,
> +};
> +
> +kunit_test_suite(ssif_bmc_test_suite);
> +#endif
> +
>  module_i2c_driver(ssif_bmc_driver);
>  
>  MODULE_AUTHOR("Quan Nguyen <quan@os.amperecomputing.com>");
> -- 
> 2.20.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine
  2026-04-03 13:14   ` Corey Minyard
@ 2026-04-03 14:12     ` Jian Zhang
  2026-04-03 14:31       ` Corey Minyard
  0 siblings, 1 reply; 9+ messages in thread
From: Jian Zhang @ 2026-04-03 14:12 UTC (permalink / raw)
  To: Corey Minyard; +Cc: Quan Nguyen, openipmi-developer, linux-kernel

> 2026年4月3日 21:14,Corey Minyard <corey@minyard.net> 写道:
> 
> On Fri, Apr 03, 2026 at 05:06:02PM +0800, Jian Zhang wrote:
>> Add some unit test for state machine when in SSIF_ABORTING state.
>> 
>> Fixes: dd2bc5cc9e25 ("ipmi: ssif_bmc: Add SSIF BMC driver")
>> Signed-off-by: Jian Zhang <zhangjian.3032@bytedance.com>
>> ---
>> v2: remove undefined symbol response_in_send
>> 
>> drivers/char/ipmi/Kconfig    |  10 +
>> drivers/char/ipmi/ssif_bmc.c | 401 +++++++++++++++++++++++++++++++++++
>> 2 files changed, 411 insertions(+)
>> 
>> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
>> index 92bed266d07c..709820f80266 100644
>> --- a/drivers/char/ipmi/Kconfig
>> +++ b/drivers/char/ipmi/Kconfig
>> @@ -187,6 +187,16 @@ config SSIF_IPMI_BMC
>>   The driver implements the BMC side of the SMBus system
>>   interface (SSIF).
>> 
>> +config SSIF_IPMI_BMC_KUNIT_TEST
>> + bool "KUnit tests for SSIF IPMI BMC driver" if !KUNIT_ALL_TESTS
>> + depends on KUNIT=y
>> + depends on SSIF_IPMI_BMC=y
> 
> Any reason for the "=y" in the above two?
> 
> It's best to remove them if they are not needed so it can work in a
> module.  I can make that change, there's no need for a new series for
> that.

Nothing special and agree, Thanks.
There’s one more thing that needs adjustment as well. 
Sorry about that—I picked it from dev-6.1 without testing (at the time there was a build error, so I didn’t run the unit tests). 
The test “ssif_bmc_test_restart_after_invalid_length” also needs to be removed. I do have a local patch regarding the length limitation,
 but it still needs thorough testing. Sorry again, I’m not very familiar with it yet, I’ll pay more attention in the future!

- Jian
> 
> Oh, and I forgot to say in the reply on the first change, thanks Quan
> for reviewing these.
> 
> -corey
> 
>> + default KUNIT_ALL_TESTS
>> + help
>> +   This option builds unit tests that exercise the SSIF BMC state
>> +   machine, including request handling, response transmission,
>> +   and error paths such as aborted or truncated transfers.
>> +
>> config IPMB_DEVICE_INTERFACE
>> tristate 'IPMB Interface handler'
>> depends on I2C
>> diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c
>> index 646a1e9ffbb7..726626126cea 100644
>> --- a/drivers/char/ipmi/ssif_bmc.c
>> +++ b/drivers/char/ipmi/ssif_bmc.c
>> @@ -18,6 +18,9 @@
>> #include <linux/timer.h>
>> #include <linux/jiffies.h>
>> #include <linux/ipmi_ssif_bmc.h>
>> +#if IS_ENABLED(CONFIG_SSIF_IPMI_BMC_KUNIT_TEST)
>> +#include <kunit/test.h>
>> +#endif
>> 
>> #define DEVICE_NAME                             "ipmi-ssif-host"
>> 
>> @@ -886,6 +889,404 @@ static struct i2c_driver ssif_bmc_driver = {
>> .id_table       = ssif_bmc_id,
>> };
>> 
>> +#if IS_ENABLED(CONFIG_SSIF_IPMI_BMC_KUNIT_TEST)
>> +struct ssif_bmc_test_ctx {
>> + struct ssif_bmc_ctx ssif_bmc;
>> + struct i2c_client client;
>> + struct i2c_adapter adapter;
>> + struct i2c_algorithm algo;
>> +};
>> +
>> +static int ssif_bmc_test_init(struct kunit *test)
>> +{
>> + struct ssif_bmc_test_ctx *test_ctx;
>> +
>> + test_ctx = kunit_kzalloc(test, sizeof(*test_ctx), GFP_KERNEL);
>> + if (!test_ctx)
>> + return -ENOMEM;
>> +
>> + test_ctx->adapter.algo = &test_ctx->algo;
>> + test_ctx->client.addr = 0x20;
>> + test_ctx->client.adapter = &test_ctx->adapter;
>> +
>> + spin_lock_init(&test_ctx->ssif_bmc.lock);
>> + init_waitqueue_head(&test_ctx->ssif_bmc.wait_queue);
>> + test_ctx->ssif_bmc.client = &test_ctx->client;
>> + i2c_set_clientdata(&test_ctx->client, &test_ctx->ssif_bmc);
>> +
>> + test->priv = test_ctx;
>> +
>> + return 0;
>> +}
>> +
>> +static void ssif_bmc_test_exit(struct kunit *test)
>> +{
>> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
>> +
>> + if (test_ctx->ssif_bmc.response_timer_inited)
>> + timer_delete_sync(&test_ctx->ssif_bmc.response_timer);
>> +}
>> +
>> +static int ssif_bmc_test_run_event_val(struct ssif_bmc_test_ctx *test_ctx,
>> +        enum i2c_slave_event event,
>> +        u8 *value)
>> +{
>> + return ssif_bmc_cb(&test_ctx->client, event, value);
>> +}
>> +
>> +static int ssif_bmc_test_run_event(struct ssif_bmc_test_ctx *test_ctx,
>> +    enum i2c_slave_event event, u8 value)
>> +{
>> + return ssif_bmc_test_run_event_val(test_ctx, event, &value);
>> +}
>> +
>> +static void ssif_bmc_test_singlepart_req(struct kunit *test)
>> +{
>> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
>> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
>> +
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
>> + GET_8BIT_ADDR(test_ctx->client.addr));
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
>> + SSIF_IPMI_SINGLEPART_WRITE);
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 2);
>> +
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xaa);
>> +
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x55);
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
>> +
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
>> + KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
>> + KUNIT_EXPECT_TRUE(test, ssif_bmc->busy);
>> + KUNIT_EXPECT_FALSE(test, ssif_bmc->aborting);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 2);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0xaa);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[1], 0x55);
>> + KUNIT_EXPECT_TRUE(test, ssif_bmc->response_timer_inited);
>> +}
>> +
>> +static void ssif_bmc_test_restart_write_without_stop(struct kunit *test)
>> +{
>> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
>> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
>> +
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
>> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
>> + SSIF_IPMI_SINGLEPART_WRITE), 0);
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 2), 0);
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xde), 0);
>> +
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING);
>> +
>> + /* Write transaction, without stop, and new request coming */
>> + ssif_bmc_test_singlepart_req(test);
>> +}
>> +
>> +
>> +static void ssif_bmc_test_restart_after_invalid_command(struct kunit *test)
>> +{
>> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
>> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
>> +
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
>> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xff), 0);
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
>> +
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_ABORTING);
>> + KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting);
>> +
>> + /* After An Invalid Command, expect could handle new request */
>> + ssif_bmc_test_singlepart_req(test);
>> +}
>> +
>> +static void ssif_bmc_test_restart_after_invalid_length(struct kunit *test)
>> +{
>> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
>> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
>> + int i;
>> +
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
>> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
>> + SSIF_IPMI_SINGLEPART_WRITE), 0);
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
>> + MAX_PAYLOAD_PER_TRANSACTION + 1), 0);
>> +
>> + for (i = 0; i < MAX_PAYLOAD_PER_TRANSACTION + 1; i++)
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx,
>> + I2C_SLAVE_WRITE_RECEIVED, i), 0);
>> +
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
>> + KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting);
>> + KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING);
>> +
>> + ssif_bmc_test_singlepart_req(test);
>> +}
>> +
>> +static void ssif_bmc_test_singlepart_read_response_completion(struct kunit *test)
>> +{
>> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
>> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
>> + u8 value;
>> +
>> + ssif_bmc->state = SSIF_SMBUS_CMD;
>> + ssif_bmc->part_buf.smbus_cmd = SSIF_IPMI_SINGLEPART_READ;
>> + ssif_bmc->response.len = 2;
>> + ssif_bmc->response.payload[0] = 0x11;
>> + ssif_bmc->response.payload[1] = 0x22;
>> + ssif_bmc->response_in_progress = true;
>> + ssif_bmc->is_singlepart_read = true;
>> + ssif_bmc->pec_support = true;
>> +
>> + value = 0;
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
>> +     &value), 0);
>> + KUNIT_EXPECT_EQ(test, value, 2);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_RES_SENDING);
>> +
>> + value = 0;
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
>> +     &value), 0);
>> + KUNIT_EXPECT_EQ(test, value, 0x11);
>> +
>> + value = 0;
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
>> +     &value), 0);
>> + KUNIT_EXPECT_EQ(test, value, 0x22);
>> +
>> + value = 0;
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
>> +     &value), 0);
>> + KUNIT_EXPECT_EQ(test, value, ssif_bmc->part_buf.pec);
>> +
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
>> + KUNIT_EXPECT_FALSE(test, ssif_bmc->response_in_progress);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->response.len, 0);
>> +}
>> +
>> +static void ssif_bmc_test_stop_during_start_discards_partial_request(struct kunit *test)
>> +{
>> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
>> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
>> +
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
>> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_START);
>> +
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
>> + KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->msg_idx, 0);
>> +
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
>> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
>> + SSIF_IPMI_SINGLEPART_WRITE), 0);
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x77), 0);
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
>> +
>> + KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 1);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0x77);
>> +}
>> +
>> +static void ssif_bmc_test_read_interrupts_partial_write(struct kunit *test)
>> +{
>> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
>> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
>> + u8 value = 0xff;
>> +
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
>> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
>> + SSIF_IPMI_SINGLEPART_WRITE), 0);
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 2), 0);
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xab), 0);
>> +
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING);
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
>> +     &value), 0);
>> + KUNIT_EXPECT_EQ(test, value, 0);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_ABORTING);
>> +
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
>> + KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 0);
>> +
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
>> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
>> + SSIF_IPMI_SINGLEPART_WRITE), 0);
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xcd), 0);
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
>> +
>> + KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 1);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0xcd);
>> +}
>> +
>> +static void ssif_bmc_test_write_interrupts_response_send(struct kunit *test)
>> +{
>> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
>> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
>> + u8 value = 0;
>> +
>> + ssif_bmc->state = SSIF_SMBUS_CMD;
>> + ssif_bmc->part_buf.smbus_cmd = SSIF_IPMI_SINGLEPART_READ;
>> + ssif_bmc->response.len = 1;
>> + ssif_bmc->response.payload[0] = 0x66;
>> + ssif_bmc->response_in_progress = true;
>> + ssif_bmc->is_singlepart_read = true;
>> +
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
>> +     &value), 0);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_RES_SENDING);
>> +
>> + /* READ_REQUESTED transaction */
>> + ssif_bmc_test_singlepart_req(test);
>> +}
>> +
>> +static void ssif_bmc_test_write_interrupts_response_sending(struct kunit *test)
>> +{
>> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
>> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
>> + u8 value = 0;
>> +
>> + ssif_bmc->state = SSIF_SMBUS_CMD;
>> + ssif_bmc->part_buf.smbus_cmd = SSIF_IPMI_SINGLEPART_READ;
>> + ssif_bmc->response.len = 1;
>> + ssif_bmc->response.payload[0] = 0x66;
>> + ssif_bmc->response_in_progress = true;
>> + ssif_bmc->is_singlepart_read = true;
>> +
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
>> +     &value), 0);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_RES_SENDING);
>> +
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
>> +     &value), 0);
>> + KUNIT_EXPECT_EQ(test, value, 0x66);
>> +
>> + /* READ_REQUESTED transaction */
>> + ssif_bmc_test_singlepart_req(test);
>> +}
>> +
>> +static void ssif_bmc_test_timeout_interrupt_allows_retry(struct kunit *test)
>> +{
>> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
>> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
>> +
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
>> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
>> + SSIF_IPMI_SINGLEPART_WRITE), 0);
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x21), 0);
>> + KUNIT_ASSERT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
>> +
>> + KUNIT_ASSERT_TRUE(test, timer_pending(&ssif_bmc->response_timer));
>> + timer_delete_sync(&ssif_bmc->response_timer);
>> + response_timeout(&ssif_bmc->response_timer);
>> +
>> + KUNIT_EXPECT_FALSE(test, ssif_bmc->busy);
>> + KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting);
>> + KUNIT_EXPECT_FALSE(test, ssif_bmc->response_timer_inited);
>> +
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
>> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
>> + SSIF_IPMI_SINGLEPART_WRITE), 0);
>> + KUNIT_EXPECT_FALSE(test, ssif_bmc->aborting);
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x22), 0);
>> + KUNIT_EXPECT_EQ(test,
>> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
>> +
>> + KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 1);
>> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0x22);
>> +}
>> +
>> +static struct kunit_case ssif_bmc_test_cases[] = {
>> + KUNIT_CASE(ssif_bmc_test_singlepart_req),
>> + KUNIT_CASE(ssif_bmc_test_restart_write_without_stop),
>> + KUNIT_CASE(ssif_bmc_test_restart_after_invalid_command),
>> + KUNIT_CASE(ssif_bmc_test_restart_after_invalid_length),
>> + KUNIT_CASE(ssif_bmc_test_singlepart_read_response_completion),
>> + KUNIT_CASE(ssif_bmc_test_stop_during_start_discards_partial_request),
>> + KUNIT_CASE(ssif_bmc_test_read_interrupts_partial_write),
>> + KUNIT_CASE(ssif_bmc_test_write_interrupts_response_send),
>> + KUNIT_CASE(ssif_bmc_test_write_interrupts_response_sending),
>> + KUNIT_CASE(ssif_bmc_test_timeout_interrupt_allows_retry),
>> + {}
>> +};
>> +
>> +static struct kunit_suite ssif_bmc_test_suite = {
>> + .name = "ssif_bmc_test",
>> + .init = ssif_bmc_test_init,
>> + .exit = ssif_bmc_test_exit,
>> + .test_cases = ssif_bmc_test_cases,
>> +};
>> +
>> +kunit_test_suite(ssif_bmc_test_suite);
>> +#endif
>> +
>> module_i2c_driver(ssif_bmc_driver);
>> 
>> MODULE_AUTHOR("Quan Nguyen <quan@os.amperecomputing.com>");
>> -- 
>> 2.20.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine
  2026-04-03 14:12     ` Jian Zhang
@ 2026-04-03 14:31       ` Corey Minyard
  0 siblings, 0 replies; 9+ messages in thread
From: Corey Minyard @ 2026-04-03 14:31 UTC (permalink / raw)
  To: Jian Zhang; +Cc: Quan Nguyen, openipmi-developer, linux-kernel

On Fri, Apr 03, 2026 at 10:12:45PM +0800, Jian Zhang wrote:
> > 2026年4月3日 21:14,Corey Minyard <corey@minyard.net> 写道:
> > 
> > On Fri, Apr 03, 2026 at 05:06:02PM +0800, Jian Zhang wrote:
> >> Add some unit test for state machine when in SSIF_ABORTING state.
> >> 
> >> Fixes: dd2bc5cc9e25 ("ipmi: ssif_bmc: Add SSIF BMC driver")
> >> Signed-off-by: Jian Zhang <zhangjian.3032@bytedance.com>
> >> ---
> >> v2: remove undefined symbol response_in_send
> >> 
> >> drivers/char/ipmi/Kconfig    |  10 +
> >> drivers/char/ipmi/ssif_bmc.c | 401 +++++++++++++++++++++++++++++++++++
> >> 2 files changed, 411 insertions(+)
> >> 
> >> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> >> index 92bed266d07c..709820f80266 100644
> >> --- a/drivers/char/ipmi/Kconfig
> >> +++ b/drivers/char/ipmi/Kconfig
> >> @@ -187,6 +187,16 @@ config SSIF_IPMI_BMC
> >>   The driver implements the BMC side of the SMBus system
> >>   interface (SSIF).
> >> 
> >> +config SSIF_IPMI_BMC_KUNIT_TEST
> >> + bool "KUnit tests for SSIF IPMI BMC driver" if !KUNIT_ALL_TESTS
> >> + depends on KUNIT=y
> >> + depends on SSIF_IPMI_BMC=y
> > 
> > Any reason for the "=y" in the above two?
> > 
> > It's best to remove them if they are not needed so it can work in a
> > module.  I can make that change, there's no need for a new series for
> > that.
> 
> Nothing special and agree, Thanks.
> There’s one more thing that needs adjustment as well. 
> Sorry about that—I picked it from dev-6.1 without testing (at the time there was a build error, so I didn’t run the unit tests). 
> The test “ssif_bmc_test_restart_after_invalid_length” also needs to be removed. I do have a local patch regarding the length limitation,
>  but it still needs thorough testing. Sorry again, I’m not very familiar with it yet, I’ll pay more attention in the future!

In that case, can you re-send just patch 5?

Thanks,

-corey

> 
> - Jian
> > 
> > Oh, and I forgot to say in the reply on the first change, thanks Quan
> > for reviewing these.
> > 
> > -corey
> > 
> >> + default KUNIT_ALL_TESTS
> >> + help
> >> +   This option builds unit tests that exercise the SSIF BMC state
> >> +   machine, including request handling, response transmission,
> >> +   and error paths such as aborted or truncated transfers.
> >> +
> >> config IPMB_DEVICE_INTERFACE
> >> tristate 'IPMB Interface handler'
> >> depends on I2C
> >> diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c
> >> index 646a1e9ffbb7..726626126cea 100644
> >> --- a/drivers/char/ipmi/ssif_bmc.c
> >> +++ b/drivers/char/ipmi/ssif_bmc.c
> >> @@ -18,6 +18,9 @@
> >> #include <linux/timer.h>
> >> #include <linux/jiffies.h>
> >> #include <linux/ipmi_ssif_bmc.h>
> >> +#if IS_ENABLED(CONFIG_SSIF_IPMI_BMC_KUNIT_TEST)
> >> +#include <kunit/test.h>
> >> +#endif
> >> 
> >> #define DEVICE_NAME                             "ipmi-ssif-host"
> >> 
> >> @@ -886,6 +889,404 @@ static struct i2c_driver ssif_bmc_driver = {
> >> .id_table       = ssif_bmc_id,
> >> };
> >> 
> >> +#if IS_ENABLED(CONFIG_SSIF_IPMI_BMC_KUNIT_TEST)
> >> +struct ssif_bmc_test_ctx {
> >> + struct ssif_bmc_ctx ssif_bmc;
> >> + struct i2c_client client;
> >> + struct i2c_adapter adapter;
> >> + struct i2c_algorithm algo;
> >> +};
> >> +
> >> +static int ssif_bmc_test_init(struct kunit *test)
> >> +{
> >> + struct ssif_bmc_test_ctx *test_ctx;
> >> +
> >> + test_ctx = kunit_kzalloc(test, sizeof(*test_ctx), GFP_KERNEL);
> >> + if (!test_ctx)
> >> + return -ENOMEM;
> >> +
> >> + test_ctx->adapter.algo = &test_ctx->algo;
> >> + test_ctx->client.addr = 0x20;
> >> + test_ctx->client.adapter = &test_ctx->adapter;
> >> +
> >> + spin_lock_init(&test_ctx->ssif_bmc.lock);
> >> + init_waitqueue_head(&test_ctx->ssif_bmc.wait_queue);
> >> + test_ctx->ssif_bmc.client = &test_ctx->client;
> >> + i2c_set_clientdata(&test_ctx->client, &test_ctx->ssif_bmc);
> >> +
> >> + test->priv = test_ctx;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static void ssif_bmc_test_exit(struct kunit *test)
> >> +{
> >> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
> >> +
> >> + if (test_ctx->ssif_bmc.response_timer_inited)
> >> + timer_delete_sync(&test_ctx->ssif_bmc.response_timer);
> >> +}
> >> +
> >> +static int ssif_bmc_test_run_event_val(struct ssif_bmc_test_ctx *test_ctx,
> >> +        enum i2c_slave_event event,
> >> +        u8 *value)
> >> +{
> >> + return ssif_bmc_cb(&test_ctx->client, event, value);
> >> +}
> >> +
> >> +static int ssif_bmc_test_run_event(struct ssif_bmc_test_ctx *test_ctx,
> >> +    enum i2c_slave_event event, u8 value)
> >> +{
> >> + return ssif_bmc_test_run_event_val(test_ctx, event, &value);
> >> +}
> >> +
> >> +static void ssif_bmc_test_singlepart_req(struct kunit *test)
> >> +{
> >> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
> >> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> >> +
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> >> + GET_8BIT_ADDR(test_ctx->client.addr));
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> >> + SSIF_IPMI_SINGLEPART_WRITE);
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 2);
> >> +
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xaa);
> >> +
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x55);
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
> >> +
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
> >> + KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
> >> + KUNIT_EXPECT_TRUE(test, ssif_bmc->busy);
> >> + KUNIT_EXPECT_FALSE(test, ssif_bmc->aborting);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 2);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0xaa);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[1], 0x55);
> >> + KUNIT_EXPECT_TRUE(test, ssif_bmc->response_timer_inited);
> >> +}
> >> +
> >> +static void ssif_bmc_test_restart_write_without_stop(struct kunit *test)
> >> +{
> >> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
> >> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> >> +
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> >> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> >> + SSIF_IPMI_SINGLEPART_WRITE), 0);
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 2), 0);
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xde), 0);
> >> +
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING);
> >> +
> >> + /* Write transaction, without stop, and new request coming */
> >> + ssif_bmc_test_singlepart_req(test);
> >> +}
> >> +
> >> +
> >> +static void ssif_bmc_test_restart_after_invalid_command(struct kunit *test)
> >> +{
> >> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
> >> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> >> +
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> >> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xff), 0);
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
> >> +
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_ABORTING);
> >> + KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting);
> >> +
> >> + /* After An Invalid Command, expect could handle new request */
> >> + ssif_bmc_test_singlepart_req(test);
> >> +}
> >> +
> >> +static void ssif_bmc_test_restart_after_invalid_length(struct kunit *test)
> >> +{
> >> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
> >> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> >> + int i;
> >> +
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> >> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> >> + SSIF_IPMI_SINGLEPART_WRITE), 0);
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> >> + MAX_PAYLOAD_PER_TRANSACTION + 1), 0);
> >> +
> >> + for (i = 0; i < MAX_PAYLOAD_PER_TRANSACTION + 1; i++)
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx,
> >> + I2C_SLAVE_WRITE_RECEIVED, i), 0);
> >> +
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
> >> + KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting);
> >> + KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING);
> >> +
> >> + ssif_bmc_test_singlepart_req(test);
> >> +}
> >> +
> >> +static void ssif_bmc_test_singlepart_read_response_completion(struct kunit *test)
> >> +{
> >> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
> >> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> >> + u8 value;
> >> +
> >> + ssif_bmc->state = SSIF_SMBUS_CMD;
> >> + ssif_bmc->part_buf.smbus_cmd = SSIF_IPMI_SINGLEPART_READ;
> >> + ssif_bmc->response.len = 2;
> >> + ssif_bmc->response.payload[0] = 0x11;
> >> + ssif_bmc->response.payload[1] = 0x22;
> >> + ssif_bmc->response_in_progress = true;
> >> + ssif_bmc->is_singlepart_read = true;
> >> + ssif_bmc->pec_support = true;
> >> +
> >> + value = 0;
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
> >> +     &value), 0);
> >> + KUNIT_EXPECT_EQ(test, value, 2);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_RES_SENDING);
> >> +
> >> + value = 0;
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
> >> +     &value), 0);
> >> + KUNIT_EXPECT_EQ(test, value, 0x11);
> >> +
> >> + value = 0;
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
> >> +     &value), 0);
> >> + KUNIT_EXPECT_EQ(test, value, 0x22);
> >> +
> >> + value = 0;
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
> >> +     &value), 0);
> >> + KUNIT_EXPECT_EQ(test, value, ssif_bmc->part_buf.pec);
> >> +
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
> >> + KUNIT_EXPECT_FALSE(test, ssif_bmc->response_in_progress);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->response.len, 0);
> >> +}
> >> +
> >> +static void ssif_bmc_test_stop_during_start_discards_partial_request(struct kunit *test)
> >> +{
> >> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
> >> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> >> +
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> >> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_START);
> >> +
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
> >> + KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->msg_idx, 0);
> >> +
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> >> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> >> + SSIF_IPMI_SINGLEPART_WRITE), 0);
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x77), 0);
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
> >> +
> >> + KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 1);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0x77);
> >> +}
> >> +
> >> +static void ssif_bmc_test_read_interrupts_partial_write(struct kunit *test)
> >> +{
> >> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
> >> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> >> + u8 value = 0xff;
> >> +
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> >> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> >> + SSIF_IPMI_SINGLEPART_WRITE), 0);
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 2), 0);
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xab), 0);
> >> +
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_REQ_RECVING);
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
> >> +     &value), 0);
> >> + KUNIT_EXPECT_EQ(test, value, 0);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_ABORTING);
> >> +
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), 0);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_READY);
> >> + KUNIT_EXPECT_FALSE(test, ssif_bmc->request_available);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 0);
> >> +
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> >> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> >> + SSIF_IPMI_SINGLEPART_WRITE), 0);
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0xcd), 0);
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
> >> +
> >> + KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 1);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0xcd);
> >> +}
> >> +
> >> +static void ssif_bmc_test_write_interrupts_response_send(struct kunit *test)
> >> +{
> >> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
> >> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> >> + u8 value = 0;
> >> +
> >> + ssif_bmc->state = SSIF_SMBUS_CMD;
> >> + ssif_bmc->part_buf.smbus_cmd = SSIF_IPMI_SINGLEPART_READ;
> >> + ssif_bmc->response.len = 1;
> >> + ssif_bmc->response.payload[0] = 0x66;
> >> + ssif_bmc->response_in_progress = true;
> >> + ssif_bmc->is_singlepart_read = true;
> >> +
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
> >> +     &value), 0);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_RES_SENDING);
> >> +
> >> + /* READ_REQUESTED transaction */
> >> + ssif_bmc_test_singlepart_req(test);
> >> +}
> >> +
> >> +static void ssif_bmc_test_write_interrupts_response_sending(struct kunit *test)
> >> +{
> >> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
> >> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> >> + u8 value = 0;
> >> +
> >> + ssif_bmc->state = SSIF_SMBUS_CMD;
> >> + ssif_bmc->part_buf.smbus_cmd = SSIF_IPMI_SINGLEPART_READ;
> >> + ssif_bmc->response.len = 1;
> >> + ssif_bmc->response.payload[0] = 0x66;
> >> + ssif_bmc->response_in_progress = true;
> >> + ssif_bmc->is_singlepart_read = true;
> >> +
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_REQUESTED,
> >> +     &value), 0);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->state, SSIF_RES_SENDING);
> >> +
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event_val(test_ctx, I2C_SLAVE_READ_PROCESSED,
> >> +     &value), 0);
> >> + KUNIT_EXPECT_EQ(test, value, 0x66);
> >> +
> >> + /* READ_REQUESTED transaction */
> >> + ssif_bmc_test_singlepart_req(test);
> >> +}
> >> +
> >> +static void ssif_bmc_test_timeout_interrupt_allows_retry(struct kunit *test)
> >> +{
> >> + struct ssif_bmc_test_ctx *test_ctx = test->priv;
> >> + struct ssif_bmc_ctx *ssif_bmc = &test_ctx->ssif_bmc;
> >> +
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> >> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> >> + SSIF_IPMI_SINGLEPART_WRITE), 0);
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x21), 0);
> >> + KUNIT_ASSERT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
> >> +
> >> + KUNIT_ASSERT_TRUE(test, timer_pending(&ssif_bmc->response_timer));
> >> + timer_delete_sync(&ssif_bmc->response_timer);
> >> + response_timeout(&ssif_bmc->response_timer);
> >> +
> >> + KUNIT_EXPECT_FALSE(test, ssif_bmc->busy);
> >> + KUNIT_EXPECT_TRUE(test, ssif_bmc->aborting);
> >> + KUNIT_EXPECT_FALSE(test, ssif_bmc->response_timer_inited);
> >> +
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_REQUESTED,
> >> + GET_8BIT_ADDR(test_ctx->client.addr)), 0);
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED,
> >> + SSIF_IPMI_SINGLEPART_WRITE), 0);
> >> + KUNIT_EXPECT_FALSE(test, ssif_bmc->aborting);
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 1), 0);
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_WRITE_RECEIVED, 0x22), 0);
> >> + KUNIT_EXPECT_EQ(test,
> >> + ssif_bmc_test_run_event(test_ctx, I2C_SLAVE_STOP, 0), -EBUSY);
> >> +
> >> + KUNIT_EXPECT_TRUE(test, ssif_bmc->request_available);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.len, 1);
> >> + KUNIT_EXPECT_EQ(test, ssif_bmc->request.payload[0], 0x22);
> >> +}
> >> +
> >> +static struct kunit_case ssif_bmc_test_cases[] = {
> >> + KUNIT_CASE(ssif_bmc_test_singlepart_req),
> >> + KUNIT_CASE(ssif_bmc_test_restart_write_without_stop),
> >> + KUNIT_CASE(ssif_bmc_test_restart_after_invalid_command),
> >> + KUNIT_CASE(ssif_bmc_test_restart_after_invalid_length),
> >> + KUNIT_CASE(ssif_bmc_test_singlepart_read_response_completion),
> >> + KUNIT_CASE(ssif_bmc_test_stop_during_start_discards_partial_request),
> >> + KUNIT_CASE(ssif_bmc_test_read_interrupts_partial_write),
> >> + KUNIT_CASE(ssif_bmc_test_write_interrupts_response_send),
> >> + KUNIT_CASE(ssif_bmc_test_write_interrupts_response_sending),
> >> + KUNIT_CASE(ssif_bmc_test_timeout_interrupt_allows_retry),
> >> + {}
> >> +};
> >> +
> >> +static struct kunit_suite ssif_bmc_test_suite = {
> >> + .name = "ssif_bmc_test",
> >> + .init = ssif_bmc_test_init,
> >> + .exit = ssif_bmc_test_exit,
> >> + .test_cases = ssif_bmc_test_cases,
> >> +};
> >> +
> >> +kunit_test_suite(ssif_bmc_test_suite);
> >> +#endif
> >> +
> >> module_i2c_driver(ssif_bmc_driver);
> >> 
> >> MODULE_AUTHOR("Quan Nguyen <quan@os.amperecomputing.com>");
> >> -- 
> >> 2.20.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-04-03 14:31 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-03  9:05 [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Jian Zhang
2026-04-03  9:05 ` [PATCH v2 2/5] ipmi: ssif_bmc: fix missing check for copy_to_user() partial failure Jian Zhang
2026-04-03  9:06 ` [PATCH v2 3/5] ipmi: ssif_bmc: fix message desynchronization after truncated response Jian Zhang
2026-04-03  9:06 ` [PATCH v2 4/5] ipmi: ssif_bmc: change log level to dbg in irq callback Jian Zhang
2026-04-03  9:06 ` [PATCH v2 5/5] ipmi: ssif_bmc: add unit test for state machine Jian Zhang
2026-04-03 13:14   ` Corey Minyard
2026-04-03 14:12     ` Jian Zhang
2026-04-03 14:31       ` Corey Minyard
2026-04-03 13:12 ` [PATCH v2 1/5] ipmi: ssif_bmc: cancel response timer on remove Corey Minyard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox