Re: [PATCH v2] i3c: mipi-i3c-hci: fix IBI payload length calculation for final status

[Frank Li]

On Thu, Apr 09, 2026 at 01:31:16AM +0000, 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@xxxxxxxxxxxxxx>
> > > ---
> > > Changes in v2:
> > > - EDITME: describe what is new in this series revision.
> > > - EDITME: use bulletpoints and terse descriptions.
> > > - Link to v1: https://lore.kernel.org/r/20260331-i3c-hci-dma-v1-1-e5cd295c44ff@xxxxxxxxxxxxxx
> > > ---
> > > Changes in v2:
> > > - Fix IBI processing logic to properly handle IBIs without data payload.
> > > - Link to v1: https://lore.kernel.org/r/20260331-i3c-hci-dma-v1-1-e5cd295c44ff@xxxxxxxxxxxxxx
> > > ---
> > >  drivers/i3c/master/mipi-i3c-hci/dma.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> > > index b903a2da1fd1..c7272155c486 100644
> > > --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> > > +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> > > @@ -721,7 +721,10 @@ 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 += FIELD_GET(IBI_DATA_LENGTH, ibi_status);
> > > +                     if (chunks) {
> > > +                             ibi_size += (chunks - 1) * rh->ibi_chunk_sz;
> > > +                             ibi_size += FIELD_GET(IBI_DATA_LENGTH, ibi_status);
>
> > this should be added to ibi_size reguardless chucks.
> >
> > if (chunks)
> >         ibi_size += (chunks - 1) * rh->ibi_chunk_sz;
> >
> > ibi_size += FIELD_GET(IBI_DATA_LENGTH, ibi_status);
>
> According to the HCI spec, CHUNKS represents the number of valid data
> chunks in the IBI Data Ring, while DATA_LENGTH (in DMA mode) represents
> the number of valid bytes in the last data chunk.
>
> This implies that DATA_LENGTH is only meaningful when at least one
> chunk exists. If CHUNKS == 0, there is no "last data chunk", and thus
> DATA_LENGTH is expected to be 0.
>
> So a combination of CHUNKS == 0 with a non-zero DATA_LENGTH would be
> an invalid descriptor encoding.

If it is true, your code should be fine.

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>