Re: [PATCH v1 1/2] i3c: master: svc: Fix missed IBI after false SLVSTART on NPCM845

[Stanley Chu]

On Mon, Apr 13, 2026 at 9:16 AM Frank Li <Frank.li@xxxxxxx> wrote:
>
> On Mon, Apr 13, 2026 at 08:50:39AM +0800, Stanley Chu wrote:
> > From: Stanley Chu <yschu@xxxxxxxxxxx>
> >
> > The NPCM845 I3C controller may raise a false SLVSTART interrupt. The
> > handler first latches MSTATUS and then clears SLVSTART. If a real IBI
> > request arrives after the handler latches MSTATUS but before it clears
> > the SLVSTART interrupt status,
>
> How this happen? If irq handler running, hardware (real IBI) will trigger
> irq line before it.
>
> Do you enable other type irq?
>
> Frank

Hi Frank,

Only SLVSTART interrupt. Below is the sequence:
- HW raises a false SLVSTART interrupt.
- The IRQ handler latches the MSTATUS register.
- Before the handler clears the SLVSTART status, a real IBI request arrives.
- HW updates the SLVREQ state to indicate a pending IBI.
- The handler continues using the stale MSTATUS snapshot, does not see
  the new SLVREQ state, and returns early.
- The real IBI is missed and no further interrupt is generated.

Thanks.

>
> > HW sets the SLVREQ state. However, the
> > handler still relies on the stale MSTATUS snapshot, returns early, and
> > misses the real IBI. No further interrupt is generated for this pending
> > IBI.
> >
> > Re-read MSTATUS to obtain the latest state and avoid missing a real IBI
> > due to this race condition.
> >
> > Fixes: 4dd12e944f07 ("i3c: master: svc: Fix npcm845 invalid slvstart event")
> > Signed-off-by: Stanley Chu <yschu@xxxxxxxxxxx>
> > ---
> >  drivers/i3c/master/svc-i3c-master.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> > index b84b324e4111..7d88e8fe3742 100644
> > --- a/drivers/i3c/master/svc-i3c-master.c
> > +++ b/drivers/i3c/master/svc-i3c-master.c
> > @@ -672,10 +672,18 @@ static irqreturn_t svc_i3c_master_irq_handler(int irq, void *dev_id)
> >       /* Clear the interrupt status */
> >       writel(SVC_I3C_MINT_SLVSTART, master->regs + SVC_I3C_MSTATUS);
> >
> > -     /* Ignore the false event */
> > -     if (svc_has_quirk(master, SVC_I3C_QUIRK_FALSE_SLVSTART) &&
> > -         !SVC_I3C_MSTATUS_STATE_SLVREQ(active))
> > -             return IRQ_HANDLED;
> > +     if (svc_has_quirk(master, SVC_I3C_QUIRK_FALSE_SLVSTART)) {
> > +             /*
> > +              * Re-read MSTATUS to obtain the latest state and avoid
> > +              * missing an IBI that arrives after MSTATUS is latched
> > +              * but before SLVSTART is cleared.
> > +              */
> > +             active = readl(master->regs + SVC_I3C_MSTATUS);
> > +
> > +             /* Ignore the false event */
> > +             if (!SVC_I3C_MSTATUS_STATE_SLVREQ(active))
> > +                     return IRQ_HANDLED;
> > +     }
> >
> >       /*
> >        * The SDA line remains low until the request is processed.
> > --
> > 2.34.1
> >