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

[Frank Li <Frank.li@nxp.com>]

On Mon, Apr 13, 2026 at 11:59:59AM +0800, Stanley Chu wrote:
> On Mon, Apr 13, 2026 at 11:00 AM Frank Li <Frank.li@nxp.com> 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@nxp.com> wrote:
> > > >
> > > > On Mon, Apr 13, 2026 at 08:50:39AM +0800, Stanley Chu wrote:
> > > > > From: Stanley Chu <yschu@nuvoton.com>
> > > > >
> > > > > 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.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> 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@nuvoton.com>
> > > > > ---
> > > > >  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
> > > > >

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c