All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frank Li <Frank.li@oss.nxp.com>
To: tze.yee.ng@altera.com
Cc: "Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	"Frank Li" <Frank.Li@nxp.com>,
	linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org,
	"Jorge Marques" <jorge.marques@analog.com>,
	"Przemysław Gaj" <pgaj@cadence.com>,
	"Wolfram Sang" <wsa+renesas@sang-engineering.com>,
	"Tommaso Merciai" <tommaso.merciai.xr@bp.renesas.com>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Adrian Ng Ho Yin" <adrian.ho.yin.ng@altera.com>,
	"Felix Gu" <ustc.gu@gmail.com>,
	"Manikanta Guntupalli" <manikanta.guntupalli@amd.com>,
	"Sakari Ailus" <sakari.ailus@linux.intel.com>,
	"Adrian Hunter" <adrian.hunter@intel.com>,
	"Jarkko Nikula" <jarkko.nikula@linux.intel.com>,
	imx@lists.linux.dev
Subject: Re: [PATCH v6 5/5] i3c: master: Add optional_bytes for variable-length GET CCC validation
Date: Wed, 8 Jul 2026 10:01:05 -0500	[thread overview]
Message-ID: <ak5mMbPlO8dCxH8R@SMW015318> (raw)
In-Reply-To: <2e07dc944eab1c4358be1da87fa5000e711ac8bd.1783493868.git.tze.yee.ng@altera.com>

On Wed, Jul 08, 2026 at 12:17:41AM -0700, tze.yee.ng@altera.com wrote:
> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
>
> Add optional_bytes to struct i3c_ccc_cmd_payload so callers describe
> variable-length GET CCC responses. GETMRL and GETMXDS set optional_bytes
> at the call site. Extend i3c_ccc_validate_payload_len() to honour it.
>
> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---
> Changes in v5:
> - Split from v4 patch 3/3: optional_bytes and variable-length GET rules only.
> - Simplify validation: drop redundant exact-length check when
>   optional_bytes == 0 (per review).
> - Clarify GETMXDS fallback: with optional_bytes = 3, a 2-byte success on
>   the first attempt no longer needs the fallback; it remains for when the
>   5-byte request fails rather than short-reading.
> ---
>  drivers/i3c/master.c    | 29 +++++++++++++++++++++++------
>  include/linux/i3c/ccc.h |  2 ++
>  2 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 3b6d3ecd9d12..12ca7c929ee5 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -902,6 +902,7 @@ static void *i3c_ccc_cmd_dest_init(struct i3c_ccc_cmd_dest *dest, u8 addr,
>  	dest->addr = addr;
>  	dest->payload.len = payloadlen;
>  	dest->payload.actual_len = 0;
> +	dest->payload.optional_bytes = 0;
>  	if (payloadlen)
>  		dest->payload.data = kzalloc(payloadlen, GFP_KERNEL);
>  	else
> @@ -944,11 +945,19 @@ static int i3c_ccc_validate_payload_len(struct i3c_ccc_cmd *cmd)
>
>  	for (i = 0; i < cmd->ndests; i++) {
>  		struct i3c_ccc_cmd_payload *p = &cmd->dests[i].payload;
> +		u16 min_len;
> +
> +		if (p->optional_bytes > p->len)
> +			return -EINVAL;
>
>  		if (p->actual_len > p->len)
>  			return -EIO;
>
> -		if (p->len && p->actual_len != p->len)
> +		if (!p->len)
> +			continue;
> +
> +		min_len = p->len - p->optional_bytes;
> +		if (p->actual_len < min_len)
>  			return -EIO;
>  	}
>
> @@ -1342,10 +1351,14 @@ static int i3c_master_getmrl_locked(struct i3c_master_controller *master,
>  		return -ENOMEM;
>
>  	/*
> -	 * When the device does not have IBI payload GETMRL only returns 2
> -	 * bytes of data.
> +	 * GETMRL returns 2 bytes (max read length) when the device does not
> +	 * advertise IBI payload, or 2 or 3 bytes when it does (the optional
> +	 * third byte is max IBI length). Use optional_bytes to allow either
> +	 * length when IBI payload is supported.
>  	 */
> -	if (!(info->bcr & I3C_BCR_IBI_PAYLOAD))
> +	if (info->bcr & I3C_BCR_IBI_PAYLOAD)
> +		dest.payload.optional_bytes = 1;
> +	else
>  		dest.payload.len -= 1;
>
>  	i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMRL, &dest, 1);
> @@ -1414,14 +1427,18 @@ static int i3c_master_getmxds_locked(struct i3c_master_controller *master,
>  	if (!getmaxds)
>  		return -ENOMEM;
>
> +	dest.payload.optional_bytes = 3;
> +
>  	i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);
>  	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>  	if (ret) {
>  		/*
> -		 * Retry when the device does not support max read turnaround
> -		 * while expecting shorter length from this CCC command.
> +		 * optional_bytes = 3 accepts a 2-byte response on the first
> +		 * attempt, so this fallback runs only when the 5-byte request
> +		 * fails rather than returning a short read.
>  		 */
>  		dest.payload.len -= 3;
> +		dest.payload.optional_bytes = 0;
>  		i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);

where use retry != 1 case ? if all is 1, patch 4 is not necessary.

Frank

>  		ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>  		if (ret)
> diff --git a/include/linux/i3c/ccc.h b/include/linux/i3c/ccc.h
> index 2506d83b8255..7ad677baf761 100644
> --- a/include/linux/i3c/ccc.h
> +++ b/include/linux/i3c/ccc.h
> @@ -347,11 +347,13 @@ struct i3c_ccc_getxtime {
>   *
>   * @len: requested payload length
>   * @actual_len: number of bytes received on a GET CCC (filled by the driver)
> + * @optional_bytes: GET CCCs may return up to this many fewer bytes than @len
>   * @data: payload data. This buffer must be DMA-able
>   */
>  struct i3c_ccc_cmd_payload {
>  	u16 len;
>  	u16 actual_len;
> +	u16 optional_bytes;
>  	void *data;
>  };
>
> --
> 2.43.7
>
>

WARNING: multiple messages have this Message-ID (diff)
From: Frank Li <Frank.li@oss.nxp.com>
To: tze.yee.ng@altera.com
Cc: "Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	"Frank Li" <Frank.Li@nxp.com>,
	linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org,
	"Jorge Marques" <jorge.marques@analog.com>,
	"Przemysław Gaj" <pgaj@cadence.com>,
	"Wolfram Sang" <wsa+renesas@sang-engineering.com>,
	"Tommaso Merciai" <tommaso.merciai.xr@bp.renesas.com>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Adrian Ng Ho Yin" <adrian.ho.yin.ng@altera.com>,
	"Felix Gu" <ustc.gu@gmail.com>,
	"Manikanta Guntupalli" <manikanta.guntupalli@amd.com>,
	"Sakari Ailus" <sakari.ailus@linux.intel.com>,
	"Adrian Hunter" <adrian.hunter@intel.com>,
	"Jarkko Nikula" <jarkko.nikula@linux.intel.com>,
	imx@lists.linux.dev
Subject: Re: [PATCH v6 5/5] i3c: master: Add optional_bytes for variable-length GET CCC validation
Date: Wed, 8 Jul 2026 10:01:05 -0500	[thread overview]
Message-ID: <ak5mMbPlO8dCxH8R@SMW015318> (raw)
In-Reply-To: <2e07dc944eab1c4358be1da87fa5000e711ac8bd.1783493868.git.tze.yee.ng@altera.com>

On Wed, Jul 08, 2026 at 12:17:41AM -0700, tze.yee.ng@altera.com wrote:
> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
>
> Add optional_bytes to struct i3c_ccc_cmd_payload so callers describe
> variable-length GET CCC responses. GETMRL and GETMXDS set optional_bytes
> at the call site. Extend i3c_ccc_validate_payload_len() to honour it.
>
> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---
> Changes in v5:
> - Split from v4 patch 3/3: optional_bytes and variable-length GET rules only.
> - Simplify validation: drop redundant exact-length check when
>   optional_bytes == 0 (per review).
> - Clarify GETMXDS fallback: with optional_bytes = 3, a 2-byte success on
>   the first attempt no longer needs the fallback; it remains for when the
>   5-byte request fails rather than short-reading.
> ---
>  drivers/i3c/master.c    | 29 +++++++++++++++++++++++------
>  include/linux/i3c/ccc.h |  2 ++
>  2 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 3b6d3ecd9d12..12ca7c929ee5 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -902,6 +902,7 @@ static void *i3c_ccc_cmd_dest_init(struct i3c_ccc_cmd_dest *dest, u8 addr,
>  	dest->addr = addr;
>  	dest->payload.len = payloadlen;
>  	dest->payload.actual_len = 0;
> +	dest->payload.optional_bytes = 0;
>  	if (payloadlen)
>  		dest->payload.data = kzalloc(payloadlen, GFP_KERNEL);
>  	else
> @@ -944,11 +945,19 @@ static int i3c_ccc_validate_payload_len(struct i3c_ccc_cmd *cmd)
>
>  	for (i = 0; i < cmd->ndests; i++) {
>  		struct i3c_ccc_cmd_payload *p = &cmd->dests[i].payload;
> +		u16 min_len;
> +
> +		if (p->optional_bytes > p->len)
> +			return -EINVAL;
>
>  		if (p->actual_len > p->len)
>  			return -EIO;
>
> -		if (p->len && p->actual_len != p->len)
> +		if (!p->len)
> +			continue;
> +
> +		min_len = p->len - p->optional_bytes;
> +		if (p->actual_len < min_len)
>  			return -EIO;
>  	}
>
> @@ -1342,10 +1351,14 @@ static int i3c_master_getmrl_locked(struct i3c_master_controller *master,
>  		return -ENOMEM;
>
>  	/*
> -	 * When the device does not have IBI payload GETMRL only returns 2
> -	 * bytes of data.
> +	 * GETMRL returns 2 bytes (max read length) when the device does not
> +	 * advertise IBI payload, or 2 or 3 bytes when it does (the optional
> +	 * third byte is max IBI length). Use optional_bytes to allow either
> +	 * length when IBI payload is supported.
>  	 */
> -	if (!(info->bcr & I3C_BCR_IBI_PAYLOAD))
> +	if (info->bcr & I3C_BCR_IBI_PAYLOAD)
> +		dest.payload.optional_bytes = 1;
> +	else
>  		dest.payload.len -= 1;
>
>  	i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMRL, &dest, 1);
> @@ -1414,14 +1427,18 @@ static int i3c_master_getmxds_locked(struct i3c_master_controller *master,
>  	if (!getmaxds)
>  		return -ENOMEM;
>
> +	dest.payload.optional_bytes = 3;
> +
>  	i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);
>  	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>  	if (ret) {
>  		/*
> -		 * Retry when the device does not support max read turnaround
> -		 * while expecting shorter length from this CCC command.
> +		 * optional_bytes = 3 accepts a 2-byte response on the first
> +		 * attempt, so this fallback runs only when the 5-byte request
> +		 * fails rather than returning a short read.
>  		 */
>  		dest.payload.len -= 3;
> +		dest.payload.optional_bytes = 0;
>  		i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);

where use retry != 1 case ? if all is 1, patch 4 is not necessary.

Frank

>  		ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>  		if (ret)
> diff --git a/include/linux/i3c/ccc.h b/include/linux/i3c/ccc.h
> index 2506d83b8255..7ad677baf761 100644
> --- a/include/linux/i3c/ccc.h
> +++ b/include/linux/i3c/ccc.h
> @@ -347,11 +347,13 @@ struct i3c_ccc_getxtime {
>   *
>   * @len: requested payload length
>   * @actual_len: number of bytes received on a GET CCC (filled by the driver)
> + * @optional_bytes: GET CCCs may return up to this many fewer bytes than @len
>   * @data: payload data. This buffer must be DMA-able
>   */
>  struct i3c_ccc_cmd_payload {
>  	u16 len;
>  	u16 actual_len;
> +	u16 optional_bytes;
>  	void *data;
>  };
>
> --
> 2.43.7
>
>

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

  reply	other threads:[~2026-07-08 15:01 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-08  7:17 [PATCH v6 0/5] i3c: Improve CCC reliability with actual_len, validation, and Direct GET retry tze.yee.ng
2026-07-08  7:17 ` tze.yee.ng
2026-07-08  7:17 ` [PATCH v6 1/5] i3c: ccc: Add actual_len to struct i3c_ccc_cmd_payload tze.yee.ng
2026-07-08  7:17   ` tze.yee.ng
2026-07-08 13:11   ` Frank Li
2026-07-08 13:11     ` Frank Li
2026-07-08 14:52   ` Alexandre Mergnat
2026-07-08 14:52     ` Alexandre Mergnat
2026-07-08  7:17 ` [PATCH v6 2/5] i3c: master: Report actual GET CCC payload length on success tze.yee.ng
2026-07-08  7:17   ` tze.yee.ng
2026-07-08  9:44   ` sashiko-bot
2026-07-08  9:44     ` sashiko-bot
2026-07-08 13:16   ` Frank Li
2026-07-08 13:16     ` Frank Li
2026-07-08 14:52   ` Alexandre Mergnat
2026-07-08 14:52     ` Alexandre Mergnat
2026-07-08  7:17 ` [PATCH v6 3/5] i3c: master: dw: Map CCC hardware errors to I3C M0/M2 tze.yee.ng
2026-07-08  7:17   ` tze.yee.ng
2026-07-08 13:20   ` Frank Li
2026-07-08 13:20     ` Frank Li
2026-07-08  7:17 ` [PATCH v6 4/5] i3c: master: Validate GET CCC payload length and retry Direct GET once tze.yee.ng
2026-07-08  7:17   ` tze.yee.ng
2026-07-08 10:01   ` sashiko-bot
2026-07-08 10:01     ` sashiko-bot
2026-07-08 14:53   ` Frank Li
2026-07-08 14:53     ` Frank Li
2026-07-08 15:19   ` Alexandre Mergnat
2026-07-08 15:19     ` Alexandre Mergnat
2026-07-08  7:17 ` [PATCH v6 5/5] i3c: master: Add optional_bytes for variable-length GET CCC validation tze.yee.ng
2026-07-08  7:17   ` tze.yee.ng
2026-07-08 15:01   ` Frank Li [this message]
2026-07-08 15:01     ` Frank Li
2026-07-09  2:56     ` NG, TZE YEE
2026-07-09  2:56       ` NG, TZE YEE
2026-07-09  3:37       ` Frank Li
2026-07-09  3:37         ` Frank Li
2026-07-09 11:32   ` Alexandre Mergnat
2026-07-09 11:32     ` Alexandre Mergnat
2026-07-13 13:58 ` [PATCH v6 0/5] i3c: Improve CCC reliability with actual_len, validation, and Direct GET retry claudiu beznea
2026-07-13 13:58   ` claudiu beznea
2026-07-13 17:27 ` Tommaso Merciai
2026-07-13 17:27   ` Tommaso Merciai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ak5mMbPlO8dCxH8R@SMW015318 \
    --to=frank.li@oss.nxp.com \
    --cc=Frank.Li@nxp.com \
    --cc=adrian.ho.yin.ng@altera.com \
    --cc=adrian.hunter@intel.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=imx@lists.linux.dev \
    --cc=jarkko.nikula@linux.intel.com \
    --cc=jorge.marques@analog.com \
    --cc=linux-i3c@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manikanta.guntupalli@amd.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=pgaj@cadence.com \
    --cc=sakari.ailus@linux.intel.com \
    --cc=tommaso.merciai.xr@bp.renesas.com \
    --cc=tze.yee.ng@altera.com \
    --cc=ustc.gu@gmail.com \
    --cc=wsa+renesas@sang-engineering.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.