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

[Frank Li]

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

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