Re: [PATCH] PCI: mediatek: Change MSI interrupt processing sequence

[qizhong.cheng <qizhong.cheng@mediatek.com>]

On Mon, 2022-01-24 at 14:55 +0800, Chen-Yu Tsai wrote:
> On Mon, Jan 24, 2022 at 2:27 PM qizhong.cheng
> <qizhong.cheng@mediatek.com> wrote:
> > 
> > Hi chenYu,
> > 
> > On Mon, 2022-01-24 at 11:12 +0800, Chen-Yu Tsai wrote:
> > > Hi,
> > > 
> > > On Sun, Jan 23, 2022 at 11:34 AM qizhong cheng
> > > <qizhong.cheng@mediatek.com> wrote:
> > > > 
> > > > As an edge-triggered interrupts, its interrupt status should be
> > > > cleared
> > > > before dispatch to the handler of device.
> > > 
> > > I'm curious, is this just a code correction or are there real
> > > world
> > > cases where something fails?
> > 
> > Yes, we found a failure when used iperf tool for wifi and network
> > cards
> > performance testing. The function of "while" has just been
> > executed,
> > and the EP sent an MSI before executing "Clear MSI interrupt
> > status".
> > After executing "Clear MSI interrupt status", this edge-triggered
> > interrupt status is cleared, but EP is still waiting for interrupt
> > handler.
> 
> Can you also include this in the commit log?  It would be nice to
> record
> the exact scenario that this fix targets.

Thanks for your suggestion. I will add commit log in the next version
for others review.

> 
> ChenYu
> 
> > > 
> > > Also, please add a Fixes tag and maybe Cc stable so this gets
> > > backported
> > > automatically.
> > 
> > Thanks for your review, I will fix it in the next version.
> > 
> > > 
> > > ChenYu
> > > 
> > > > Signed-off-by: qizhong cheng <qizhong.cheng@mediatek.com>
> > > > ---
> > > >  drivers/pci/controller/pcie-mediatek.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/pci/controller/pcie-mediatek.c
> > > > b/drivers/pci/controller/pcie-mediatek.c
> > > > index 2f3f974977a3..705ea33758b1 100644
> > > > --- a/drivers/pci/controller/pcie-mediatek.c
> > > > +++ b/drivers/pci/controller/pcie-mediatek.c
> > > > @@ -624,12 +624,12 @@ static void mtk_pcie_intr_handler(struct
> > > > irq_desc *desc)
> > > >                 if (status & MSI_STATUS){
> > > >                         unsigned long imsi_status;
> > > > 
> > > > +                       /* Clear MSI interrupt status */
> > > > +                       writel(MSI_STATUS, port->base +
> > > > PCIE_INT_STATUS);
> > > >                         while ((imsi_status = readl(port->base
> > > > +
> > > > PCIE_IMSI_STATUS))) {
> > > >                                 for_each_set_bit(bit,
> > > > &imsi_status,
> > > > MTK_MSI_IRQS_NUM)
> > > >                                         generic_handle_domain_i
> > > > rq(p
> > > > ort->inner_domain, bit);
> > > >                         }
> > > > -                       /* Clear MSI interrupt status */
> > > > -                       writel(MSI_STATUS, port->base +
> > > > PCIE_INT_STATUS);
> > > >                 }
> > > >         }
> > > > 
> > > > --
> > > > 2.25.1
> > > > 
> > > > 
> > > > _______________________________________________
> > > > Linux-mediatek mailing list
> > > > Linux-mediatek@lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/linux-mediatek
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek