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

[Stanley Chu]

On Mon, Apr 13, 2026 at 11:00 AM Frank Li <Frank.li@xxxxxxx> wrote:
>
> On Mon, Apr 13, 2026 at 09:40:02AM +0800, Stanley Chu wrote:
> > 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.
>
> Because reading register and IBI request arriving are totally async
>
> readl(master->regs + SVC_I3C_MSTATUS),  may return two results
> 1. before IBI request
> 2. after IBI request
>
> How do you make sure second readl(master->regs + SVC_I3C_MSTATUS) always
> get the result of after IBI request.
>
> Frank

Hi Frank,
The ordering between the second readl(MSTATUS) and the real IBI arrival is
not critical, as long as the second read is performed after clearing SLVSTART.

There are two cases:
1. IBI arrives before the second read
After SLVSTART is cleared, HW has already updated the SLVREQ state, so the
second MSTATUS read can observe the pending IBI and handle it in the same
IRQ.

2. IBI arrives after the second read
Even though the second MSTATUS read does not reflect the pending IBI, the
IBI will assert SLVSTART again. A new interrupt will be generated
after the current
handler returns, and the pending IBI is handled in the next IRQ.

Thanks.

>
> > - 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
> > > >