public inbox for linux-i3c@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] i3c: mipi-i3c-hci: fix IBI payload length calculation for final status
@ 2026-03-31 10:27 Billy Tsai
  2026-03-31 15:06 ` Frank Li
  2026-04-01 13:33 ` Adrian Hunter
  0 siblings, 2 replies; 6+ messages in thread
From: Billy Tsai @ 2026-03-31 10:27 UTC (permalink / raw)
  To: Alexandre Belloni, Frank Li, Nicolas Pitre, Boris Brezillon
  Cc: linux-i3c, linux-kernel, Billy Tsai

In DMA mode, the IBI status descriptor encodes the payload using
CHUNKS (number of chunks) and DATA_LENGTH (valid bytes in the last
chunk). All preceding chunks are implicitly full-sized.

The current code accumulates full chunk sizes for non-final status
descriptors, but for the final status descriptor it only adds
DATA_LENGTH. This ignores the contribution of the preceding full
chunks described by the same final status entry.

As a result, the computed IBI payload length is truncated whenever
the final status spans multiple chunks. For example, with a chunk
size of 4 bytes, CHUNKS=2 and DATA_LENGTH=1 should result in a total
payload size of 5 bytes, but the current code reports only 1 byte.

Fix the calculation by adding the size of (CHUNKS - 1) full chunks
plus DATA_LENGTH for the last chunk.

Fixes: 9ad9a52cce28 ("i3c/master: introduce the mipi-i3c-hci driver")
Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/i3c/master/mipi-i3c-hci/dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
index b903a2da1fd1..f4c76f168276 100644
--- a/drivers/i3c/master/mipi-i3c-hci/dma.c
+++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
@@ -721,6 +721,7 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
 		if (!(ibi_status & IBI_LAST_STATUS)) {
 			ibi_size += chunks * rh->ibi_chunk_sz;
 		} else {
+			ibi_size += (chunks - 1) * rh->ibi_chunk_sz;
 			ibi_size += FIELD_GET(IBI_DATA_LENGTH, ibi_status);
 			last_ptr = ptr;
 			break;

---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20260331-i3c-hci-dma-3aae908b4e8e

Best regards,
-- 
Billy Tsai <billy_tsai@aspeedtech.com>


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

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

* Re: [PATCH] i3c: mipi-i3c-hci: fix IBI payload length calculation for final status
  2026-03-31 10:27 [PATCH] i3c: mipi-i3c-hci: fix IBI payload length calculation for final status Billy Tsai
@ 2026-03-31 15:06 ` Frank Li
  2026-04-01 13:33 ` Adrian Hunter
  1 sibling, 0 replies; 6+ messages in thread
From: Frank Li @ 2026-03-31 15:06 UTC (permalink / raw)
  To: Billy Tsai
  Cc: Alexandre Belloni, Nicolas Pitre, Boris Brezillon, linux-i3c,
	linux-kernel

On Tue, Mar 31, 2026 at 06:27:30PM +0800, Billy Tsai wrote:
> In DMA mode, the IBI status descriptor encodes the payload using
> CHUNKS (number of chunks) and DATA_LENGTH (valid bytes in the last
> chunk). All preceding chunks are implicitly full-sized.
>
> The current code accumulates full chunk sizes for non-final status
> descriptors, but for the final status descriptor it only adds
> DATA_LENGTH. This ignores the contribution of the preceding full
> chunks described by the same final status entry.
>
> As a result, the computed IBI payload length is truncated whenever
> the final status spans multiple chunks. For example, with a chunk
> size of 4 bytes, CHUNKS=2 and DATA_LENGTH=1 should result in a total
> payload size of 5 bytes, but the current code reports only 1 byte.
>
> Fix the calculation by adding the size of (CHUNKS - 1) full chunks
> plus DATA_LENGTH for the last chunk.
>
> Fixes: 9ad9a52cce28 ("i3c/master: introduce the mipi-i3c-hci driver")
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>
>  drivers/i3c/master/mipi-i3c-hci/dma.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index b903a2da1fd1..f4c76f168276 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -721,6 +721,7 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
>  		if (!(ibi_status & IBI_LAST_STATUS)) {
>  			ibi_size += chunks * rh->ibi_chunk_sz;
>  		} else {
> +			ibi_size += (chunks - 1) * rh->ibi_chunk_sz;
>  			ibi_size += FIELD_GET(IBI_DATA_LENGTH, ibi_status);
>  			last_ptr = ptr;
>  			break;
>
> ---
> base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
> change-id: 20260331-i3c-hci-dma-3aae908b4e8e
>
> Best regards,
> --
> Billy Tsai <billy_tsai@aspeedtech.com>
>

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

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

* Re: [PATCH] i3c: mipi-i3c-hci: fix IBI payload length calculation for final status
  2026-03-31 10:27 [PATCH] i3c: mipi-i3c-hci: fix IBI payload length calculation for final status Billy Tsai
  2026-03-31 15:06 ` Frank Li
@ 2026-04-01 13:33 ` Adrian Hunter
  2026-04-02  8:07   ` Billy Tsai
  1 sibling, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2026-04-01 13:33 UTC (permalink / raw)
  To: Billy Tsai, Alexandre Belloni, Frank Li, Nicolas Pitre,
	Boris Brezillon
  Cc: linux-i3c, linux-kernel

On 31/03/2026 13:27, Billy Tsai wrote:
> In DMA mode, the IBI status descriptor encodes the payload using
> CHUNKS (number of chunks) and DATA_LENGTH (valid bytes in the last
> chunk). All preceding chunks are implicitly full-sized.
> 
> The current code accumulates full chunk sizes for non-final status
> descriptors, but for the final status descriptor it only adds
> DATA_LENGTH. This ignores the contribution of the preceding full
> chunks described by the same final status entry.
> 
> As a result, the computed IBI payload length is truncated whenever
> the final status spans multiple chunks. For example, with a chunk
> size of 4 bytes, CHUNKS=2 and DATA_LENGTH=1 should result in a total
> payload size of 5 bytes, but the current code reports only 1 byte.
> 
> Fix the calculation by adding the size of (CHUNKS - 1) full chunks
> plus DATA_LENGTH for the last chunk.
> 
> Fixes: 9ad9a52cce28 ("i3c/master: introduce the mipi-i3c-hci driver")
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> ---
>  drivers/i3c/master/mipi-i3c-hci/dma.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index b903a2da1fd1..f4c76f168276 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -721,6 +721,7 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
>  		if (!(ibi_status & IBI_LAST_STATUS)) {
>  			ibi_size += chunks * rh->ibi_chunk_sz;
>  		} else {
> +			ibi_size += (chunks - 1) * rh->ibi_chunk_sz;

That assumes chunks is not 0.  It would be better to
defend against that possibility i.e.

			if (chunks) {
				ibi_size += (chunks - 1) * rh->ibi_chunk_sz;
				ibi_size += FIELD_GET(IBI_DATA_LENGTH, ibi_status);
			}

>  			ibi_size += FIELD_GET(IBI_DATA_LENGTH, ibi_status);
>  			last_ptr = ptr;
>  			break;
> 
> ---
> base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
> change-id: 20260331-i3c-hci-dma-3aae908b4e8e
> 
> Best regards,


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

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

* Re: [PATCH] i3c: mipi-i3c-hci: fix IBI payload length calculation for final status
  2026-04-01 13:33 ` Adrian Hunter
@ 2026-04-02  8:07   ` Billy Tsai
  2026-04-02 12:45     ` Adrian Hunter
  0 siblings, 1 reply; 6+ messages in thread
From: Billy Tsai @ 2026-04-02  8:07 UTC (permalink / raw)
  To: Adrian Hunter, Alexandre Belloni, Frank Li, Nicolas Pitre,
	Boris Brezillon
  Cc: linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org

On 31/03/2026 13:27, Billy Tsai wrote:
> > In DMA mode, the IBI status descriptor encodes the payload using
> > CHUNKS (number of chunks) and DATA_LENGTH (valid bytes in the last
> > chunk). All preceding chunks are implicitly full-sized.
> >
> > The current code accumulates full chunk sizes for non-final status
> > descriptors, but for the final status descriptor it only adds
> > DATA_LENGTH. This ignores the contribution of the preceding full
> > chunks described by the same final status entry.
> >
> > As a result, the computed IBI payload length is truncated whenever
> > the final status spans multiple chunks. For example, with a chunk
> > size of 4 bytes, CHUNKS=2 and DATA_LENGTH=1 should result in a total
> > payload size of 5 bytes, but the current code reports only 1 byte.
> >
> > Fix the calculation by adding the size of (CHUNKS - 1) full chunks
> > plus DATA_LENGTH for the last chunk.
> >
> > Fixes: 9ad9a52cce28 ("i3c/master: introduce the mipi-i3c-hci driver")
> > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> > ---
> >  drivers/i3c/master/mipi-i3c-hci/dma.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> > index b903a2da1fd1..f4c76f168276 100644
> > --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> > +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> > @@ -721,6 +721,7 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
> >               if (!(ibi_status & IBI_LAST_STATUS)) {
> >                       ibi_size += chunks * rh->ibi_chunk_sz;
> >               } else {
> > +                     ibi_size += (chunks - 1) * rh->ibi_chunk_sz;

> That assumes chunks is not 0.  It would be better to
> defend against that possibility i.e.
> 
>                         if (chunks) {
>                                 ibi_size += (chunks - 1) * rh->ibi_chunk_sz;
>                                 ibi_size += FIELD_GET(IBI_DATA_LENGTH, ibi_status);
>                         }
> 

As expected, the value should never be 0, as this is guaranteed by the hardware.

If we add a check for 0, it may be appropriate to include a WARN_ON message to
indicate unexpected hardware behavior.

Thanks

Billy Tsai

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

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

* Re: [PATCH] i3c: mipi-i3c-hci: fix IBI payload length calculation for final status
  2026-04-02  8:07   ` Billy Tsai
@ 2026-04-02 12:45     ` Adrian Hunter
  2026-04-02 15:16       ` Billy Tsai
  0 siblings, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2026-04-02 12:45 UTC (permalink / raw)
  To: Billy Tsai, Alexandre Belloni, Frank Li, Nicolas Pitre,
	Boris Brezillon
  Cc: linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org

On 02/04/2026 11:07, Billy Tsai wrote:
> On 31/03/2026 13:27, Billy Tsai wrote:
>>> In DMA mode, the IBI status descriptor encodes the payload using
>>> CHUNKS (number of chunks) and DATA_LENGTH (valid bytes in the last
>>> chunk). All preceding chunks are implicitly full-sized.
>>>
>>> The current code accumulates full chunk sizes for non-final status
>>> descriptors, but for the final status descriptor it only adds
>>> DATA_LENGTH. This ignores the contribution of the preceding full
>>> chunks described by the same final status entry.
>>>
>>> As a result, the computed IBI payload length is truncated whenever
>>> the final status spans multiple chunks. For example, with a chunk
>>> size of 4 bytes, CHUNKS=2 and DATA_LENGTH=1 should result in a total
>>> payload size of 5 bytes, but the current code reports only 1 byte.
>>>
>>> Fix the calculation by adding the size of (CHUNKS - 1) full chunks
>>> plus DATA_LENGTH for the last chunk.
>>>
>>> Fixes: 9ad9a52cce28 ("i3c/master: introduce the mipi-i3c-hci driver")
>>> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
>>> ---
>>>  drivers/i3c/master/mipi-i3c-hci/dma.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
>>> index b903a2da1fd1..f4c76f168276 100644
>>> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
>>> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
>>> @@ -721,6 +721,7 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
>>>               if (!(ibi_status & IBI_LAST_STATUS)) {
>>>                       ibi_size += chunks * rh->ibi_chunk_sz;
>>>               } else {
>>> +                     ibi_size += (chunks - 1) * rh->ibi_chunk_sz;
> 
>> That assumes chunks is not 0.  It would be better to
>> defend against that possibility i.e.
>>
>>                         if (chunks) {
>>                                 ibi_size += (chunks - 1) * rh->ibi_chunk_sz;
>>                                 ibi_size += FIELD_GET(IBI_DATA_LENGTH, ibi_status);
>>                         }
>>
> 
> As expected, the value should never be 0, as this is guaranteed by the hardware.

Not sure the spec. actually says that anywhere.

> 
> If we add a check for 0, it may be appropriate to include a WARN_ON message to
> indicate unexpected hardware behavior.

Please no.  If the target driver does not get the amount of data
it is expecting, then it can complain.


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

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

* Re: [PATCH] i3c: mipi-i3c-hci: fix IBI payload length calculation for final status
  2026-04-02 12:45     ` Adrian Hunter
@ 2026-04-02 15:16       ` Billy Tsai
  0 siblings, 0 replies; 6+ messages in thread
From: Billy Tsai @ 2026-04-02 15:16 UTC (permalink / raw)
  To: Adrian Hunter, Alexandre Belloni, Frank Li, Nicolas Pitre,
	Boris Brezillon
  Cc: linux-i3c@lists.infradead.org, linux-kernel@vger.kernel.org


>>>> In DMA mode, the IBI status descriptor encodes the payload using
>>>> CHUNKS (number of chunks) and DATA_LENGTH (valid bytes in the last
>>>> chunk). All preceding chunks are implicitly full-sized.
>>>>
>>>> The current code accumulates full chunk sizes for non-final status
>>>> descriptors, but for the final status descriptor it only adds
>>>> DATA_LENGTH. This ignores the contribution of the preceding full
>>>> chunks described by the same final status entry.
>>>>
>>>> As a result, the computed IBI payload length is truncated whenever
>>>> the final status spans multiple chunks. For example, with a chunk
>>>> size of 4 bytes, CHUNKS=2 and DATA_LENGTH=1 should result in a total
>>>> payload size of 5 bytes, but the current code reports only 1 byte.
>>>>
>>>> Fix the calculation by adding the size of (CHUNKS - 1) full chunks
>>>> plus DATA_LENGTH for the last chunk.
>>>>
>>>> Fixes: 9ad9a52cce28 ("i3c/master: introduce the mipi-i3c-hci driver")
>>>> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
>>>> ---
>>>>  drivers/i3c/master/mipi-i3c-hci/dma.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
>>>> index b903a2da1fd1..f4c76f168276 100644
>>>> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
>>>> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
>>>> @@ -721,6 +721,7 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
>>>>               if (!(ibi_status & IBI_LAST_STATUS)) {
>>>>                       ibi_size += chunks * rh->ibi_chunk_sz;
>>>>               } else {
>>>> +                     ibi_size += (chunks - 1) * rh->ibi_chunk_sz;
>>
>>> That assumes chunks is not 0.  It would be better to
>>> defend against that possibility i.e.
>>>
>>>                         if (chunks) {
>>>                                 ibi_size += (chunks - 1) * rh->ibi_chunk_sz;
>>>                                 ibi_size += FIELD_GET(IBI_DATA_LENGTH, ibi_status);
>>>                         }
>>>
>>
>> As expected, the value should never be 0, as this is guaranteed by the hardware.

> Not sure the spec. actually says that anywhere.

Apologies—this assumption is too strong. I overlooked the IBI condition when no data
is present. It may be implicitly understood that the CHUNKS field should be set to 0.

> >
> > If we add a check for 0, it may be appropriate to include a WARN_ON message to
> > indicate unexpected hardware behavior.

> Please no.  If the target driver does not get the amount of data
> it is expecting, then it can complain.

Got it. I will update the code to handle the case of 0 chunks without adding a WARN_ON message.

Best regards,
Billy Tsai
-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

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

end of thread, other threads:[~2026-04-02 15:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-31 10:27 [PATCH] i3c: mipi-i3c-hci: fix IBI payload length calculation for final status Billy Tsai
2026-03-31 15:06 ` Frank Li
2026-04-01 13:33 ` Adrian Hunter
2026-04-02  8:07   ` Billy Tsai
2026-04-02 12:45     ` Adrian Hunter
2026-04-02 15:16       ` Billy Tsai

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