From: Adrian Ng Ho Yin <[email protected]>
Validate GET CCC payload length after a successful transfer. Treat a
short read as I3C_ERROR_M0 and return -EIO. GETMRL accepts exactly 2 or
3 bytes per the I3C spec defined formats. GETMXDS may return 2 bytes
(format 1) or 5 bytes (format 2) per I3C spec.
Retry GET CCCs once on retriable errors: I3C_ERROR_M0 (frame error) and
I3C_ERROR_M2 (address-header NACK, e.g. IBI or Controller Role Request
arbitration per I3C spec section 5.1.2.2.3). SET CCCs are not retried
to avoid repeating side-effecting commands. Restore dests[].payload.len
to the originally requested length before each attempt and again before
returning an error, so callers that adjust the length on failure (e.g.
i3c_master_getmxds_locked()) do not underflow a shortened value.
Use a stack buffer for the common single-destination GET case and only
kmalloc when ndests > 1.
Signed-off-by: Adrian Ng Ho Yin <[email protected]>
Signed-off-by: Tze Yee Ng <[email protected]>
---
drivers/i3c/master.c | 111 ++++++++++++++++++++++++++++-
drivers/i3c/master/dw-i3c-master.c | 2 +-
2 files changed, 111 insertions(+), 2 deletions(-)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 5cd4e5da2233..c94d37cd8b3f 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -26,6 +26,12 @@ static DEFINE_MUTEX(i3c_core_lock);
static int __i3c_first_dynamic_bus_num;
static BLOCKING_NOTIFIER_HEAD(i3c_bus_notifier);
+#define I3C_CCC_GETMRL_LEN_SHORT 2
+#define I3C_CCC_GETMRL_LEN_FULL 3
+#define I3C_CCC_GETMXDS_LEN_SHORT 2
+#define I3C_CCC_GETMXDS_LEN_FULL 5
+#define I3C_CCC_MAX_RETRIES 2
+
/**
* i3c_bus_maintenance_lock - Lock the bus for a maintenance operation
* @bus: I3C bus to take the lock on
@@ -925,6 +931,61 @@ static void i3c_ccc_cmd_init(struct i3c_ccc_cmd *cmd, bool rnw, u8 id,
cmd->err = I3C_ERROR_UNKNOWN;
}
+static bool i3c_ccc_get_payload_ok(u8 id, u16 req_len, u16 actual_len)
+{
+ if (actual_len > req_len)
+ return false;
+
+ if (!req_len)
+ return actual_len == 0;
+
+ if (id == I3C_CCC_GETMRL)
+ return actual_len == I3C_CCC_GETMRL_LEN_SHORT ||
+ actual_len == I3C_CCC_GETMRL_LEN_FULL;
+
+ if (id == I3C_CCC_GETMXDS)
+ return actual_len == I3C_CCC_GETMXDS_LEN_SHORT ||
+ actual_len == I3C_CCC_GETMXDS_LEN_FULL;
+
+ return actual_len == req_len;
+}
+
+static int i3c_ccc_validate_payload_len(struct i3c_ccc_cmd *cmd,
+ const u16 *req_lens)
+{
+ unsigned int i;
+
+ if (!cmd->rnw)
+ return 0;
+
+ for (i = 0; i < cmd->ndests; i++) {
+ u16 actual = cmd->dests[i].payload.len;
+ u16 req = req_lens[i];
+
+ if (!i3c_ccc_get_payload_ok(cmd->id, req, actual)) {
+ cmd->err = I3C_ERROR_M0;
+ return -EIO;
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * M0: transient frame errors.
+ * M2: address-header NACK (I3C spec section 5.1.2.2.3), e.g. when a target
+ * simultaneously asserts an IBI or Controller Role Request and neither
+ * side ACKs. Software should re-issue the transfer; the controller wins
+ * arbitration after Repeated START.
+ *
+ * Retries apply to GET CCCs only; SET CCCs are not retried to avoid
+ * repeating side-effecting commands.
+ */
+static bool i3c_ccc_err_retriable(enum i3c_error_code err)
+{
+ return err == I3C_ERROR_M0 || err == I3C_ERROR_M2;
+}
+
/**
* i3c_master_send_ccc_cmd_locked() - send a CCC (Common Command Codes)
* @master: master used to send frames on the bus
@@ -936,9 +997,17 @@ static void i3c_ccc_cmd_init(struct i3c_ccc_cmd *cmd, bool rnw, u8 id,
static int i3c_master_send_ccc_cmd_locked(struct i3c_master_controller *master,
struct i3c_ccc_cmd *cmd)
{
+ u16 req_len;
+ u16 *req_lens = NULL;
+ u16 *req_lens_alloc = NULL;
+ unsigned int i;
+ int ret, retries;
+
if (!cmd || !master)
return -EINVAL;
+ retries = cmd->rnw ? I3C_CCC_MAX_RETRIES : 1;
+
if (WARN_ON(master->init_done &&
!rwsem_is_locked(&master->bus.lock)))
return -EINVAL;
@@ -953,7 +1022,47 @@ static int i3c_master_send_ccc_cmd_locked(struct i3c_master_controller *master,
!master->ops->supports_ccc_cmd(master, cmd))
return -EOPNOTSUPP;
- return master->ops->send_ccc_cmd(master, cmd);
+ if (cmd->rnw && cmd->dests && cmd->ndests) {
+ if (cmd->ndests == 1) {
+ req_len = cmd->dests[0].payload.len;
+ req_lens = &req_len;
+ } else {
+ req_lens_alloc = kmalloc_array(cmd->ndests,
+ sizeof(*req_lens_alloc),
+ GFP_KERNEL);
+ if (!req_lens_alloc)
+ return -ENOMEM;
+
+ req_lens = req_lens_alloc;
+ for (i = 0; i < cmd->ndests; i++)
+ req_lens[i] = cmd->dests[i].payload.len;
+ }
+ }
+
+ do {
+ cmd->err = I3C_ERROR_UNKNOWN;
+ if (req_lens) {
+ for (i = 0; i < cmd->ndests; i++)
+ cmd->dests[i].payload.len = req_lens[i];
+ }
+ ret = master->ops->send_ccc_cmd(master, cmd);
+ if (!ret && req_lens)
+ ret = i3c_ccc_validate_payload_len(cmd, req_lens);
+ } while (--retries && ret && i3c_ccc_err_retriable(cmd->err));
+
+ if (ret && req_lens) {
+ /*
+ * Drivers may update payload.len to the actual RX count;
+ * restore the requested length so callers can safely adjust
+ * it on error (e.g. i3c_master_getmxds_locked()).
+ */
+ for (i = 0; i < cmd->ndests; i++)
+ cmd->dests[i].payload.len = req_lens[i];
+ }
+
+ kfree(req_lens_alloc);
+
+ return ret;
}
static struct i2c_dev_desc *
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 45bde92d0342..fbd1c899e728 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -493,7 +493,6 @@ static void dw_i3c_master_end_xfer_locked(struct dw_i3c_master *master, u32 isr)
break;
case RESPONSE_ERROR_PARITY:
case RESPONSE_ERROR_IBA_NACK:
- case RESPONSE_ERROR_ADDRESS_NACK:
case RESPONSE_ERROR_TRANSF_ABORT:
case RESPONSE_ERROR_CRC:
case RESPONSE_ERROR_FRAME:
@@ -503,6 +502,7 @@ static void dw_i3c_master_end_xfer_locked(struct dw_i3c_master *master, u32 isr)
ret = -ENOSPC;
break;
case RESPONSE_ERROR_I2C_W_NACK_ERR:
+ case RESPONSE_ERROR_ADDRESS_NACK:
default:
ret = -EINVAL;
break;
--
2.43.7
--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.