[PATCH 2/3] i3c: mipi-i3c-hci: Add PIO queue management support for HCI v1.2

[廖建銘]

On Fri, Jul 10, 2026 at 02:57:00AM +0000, Frank Li wrote:
>
> On Thu, Jul 09, 2026 at 03:17:43PM +0800, Jian-Ming Liao wrote:
> > - Support explicit enablement and starting of PIO queues as
> >   required by HCI v1.2.
> > - Handle alternate PIO queue sizes via ALT_QUEUE_SIZE register.
> > - Implement explicit PIO queue stopping/disabling and restart
> >   logic after errors.
>
> Remove -
>
> Support explicit enablement and starting of PIO queues as required by
> HCI v1.2.
>
> Handle alternate PIO queue sizes via ALT_QUEUE_SIZE register.
>
> Implement explicit PIO queue stopping/disabling and restart logic after
> errors.
>
> if you prefer -, need
>
> Some sentense ...
>   - abc...
>

Hi Frank,

Thank you for the feedback.

I will remove the '-' prefix in the commit message.

> >  #define IBI_STATUS_SIZE                      GENMASK(15, 8)
> >  #define CR_QUEUE_SIZE                        GENMASK(7, 0)
> >
> > +#define PIO_ALT_QUEUE_SIZE      0x1C
> > +#define EXT_IBI_QUEUE_EN        BIT(28)
> > +#define ALT_RESP_QUEUE_EN       BIT(24)
> > +#define ALT_RESP_QUEUE_SIZE     GENMASK(7, 0)
> > +
>
> Number start pos align to prevous GENMASK(7, 0) of CR_QUEUE_SIZE
>

I will fix it.

> >       __hci_pio_init(hci, &size_val);
> >
> > -     dev_dbg(&hci->master.dev, "CMD/RESP FIFO = %ld entries\n",
> > -             FIELD_GET(CR_QUEUE_SIZE, size_val));
> > -     dev_dbg(&hci->master.dev, "IBI FIFO = %ld bytes\n",
> > -             4 * FIELD_GET(IBI_STATUS_SIZE, size_val));
> > -     dev_dbg(&hci->master.dev, "RX data FIFO = %d bytes\n",
> > -             4 * (2 << FIELD_GET(RX_DATA_BUFFER_SIZE, size_val)));
> > -     dev_dbg(&hci->master.dev, "TX data FIFO = %d bytes\n",
> > -             4 * (2 << FIELD_GET(TX_DATA_BUFFER_SIZE, size_val)));
> > -
>
> why need move this part?

I understand that unnecessary code movement makes the diff harder to review.
My intention for moving the dev_dbg lines was to place them after
calculating the adjusted resp_sz and ibi_val(which are dynamic in
v1.2) @ func "__hci_pio_init".
However, I agree with you, so I will revert the movement and calculate
the adjusted sizes(resp_sz, ibi_val) in place to keep the diff clean.


> > @@ -246,6 +280,12 @@ static void hci_pio_cleanup(struct i3c_hci *hci)
> >               BUG_ON(pio->curr_rx);
> >               BUG_ON(pio->curr_tx);
> >               BUG_ON(pio->curr_resp);
> > +             /* MIPI I3C HCI v1.2 requires explicitly stopping and disabling PIO queues */
> > +             if (hci->version_major == 1 && hci->version_minor >= 2)
> > +                     pio_reg_write(CONTROL, 0x0);
> > +
> > +             kfree(pio);
>
> why need kfree() here? if fix existing problem, use seperate patch.
>

I have verified that pio is managed by devm_kzalloc. I will remove the
manual kfree() to prevent potential double-free issues.

> > @@ -766,6 +806,18 @@ static void hci_pio_err(struct i3c_hci *hci, struct hci_pio_data *pio,
> >               hci_pio_dequeue_xfer_common(hci, pio, pio->curr_tx, 1);
> >       /* then reset the hardware */
> >       mipi_i3c_hci_pio_reset(hci);
> > +
> > +     /* MIPI I3C HCI v1.2 requires explicitly restarting PIO queues after error/abort */
> > +     if (hci->version_major == 1 && hci->version_minor >= 2) {
>
> Can you use helper macro this check, many place use similar check
>
> Frank
>

I agree that a helper macro for checking the HCI version is better for
maintainability.
I will define a helper like hci_version_at_least(hci, maj, min) and
use it to replace the hardcoded checks throughout the driver.

Best regards,
Jian-Ming Liao