Re: [PATCH v2] PCI: tegra: move to use request_irq by IRQF_NO_AUTOEN flag

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Jun 23, 2021 at 10:45:19AM +0100, Lorenzo Pieralisi wrote:
> [+Marc, Thomas]
> 
> thread: https://lore.kernel.org/linux-pci/1621213953-54030-1-git-send-email-tiantao6@hisilicon.com
> 
> On Mon, May 31, 2021 at 03:32:33PM +0530, Vidya Sagar wrote:
> > I want to re-examine this patch.
> > I don't see any references in the kernel where IRQ_NOAUTOEN is passed
> > directly in request_irq APIs.
> 
> AFAICS there are many references, not sure what you actually checked.
> 
> > It is always set explicitly through irq_set_status_flags() *before*
> > calling request_irq APIs.  I don't see any comment in the header file
> > either that says something like it should always be set before
> > requesting the irq.  Lorenzo/Bjorn, could you please throw some light
> > on what is correct thing to do here?
> 
> To be honest I don't know. Certainly Marc and Thomas know if they have
> a minute to chime in.

I don't know either.  If this *is* the right thing to do it needs a
much better commit log because this is not merely a simplification
that can be verified with a couple minutes of code reading.

This is the only use of IRQ_NOAUTOEN in drivers/pci/, which itself
raises the question of why we need it.

Also, please update the subject line to s/tegra/tegra194/ and start
with a capital letter to match convention.

> > On 5/17/2021 3:47 PM, Vidya Sagar wrote:
> > > Thanks for the patch.
> > > 
> > > Reviewed-by: Vidya Sagar <vidyas@nvidia.com>
> > > 
> > > - Vidya Sagar
> > > 
> > > On 5/17/2021 6:42 AM, Tian Tao wrote:
> > > > External email: Use caution opening links or attachments
> > > > 
> > > > 
> > > > request_irq() after setting IRQ_NOAUTOEN as below
> > > > irq_set_status_flags(irq, IRQ_NOAUTOEN);
> > > > request_irq(dev, irq...);
> > > > can be replaced by request_irq() with IRQF_NO_AUTOEN flag.
> > > > 
> > > > this change is just to simplify the code, no actual functional changes.
> > > > 
> > > > Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
> > > > ---
> > > > 
> > > > v2: update the commit message.
> > > > ---
> > > >   drivers/pci/controller/dwc/pcie-tegra194.c | 5 ++---
> > > >   1 file changed, 2 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c
> > > > b/drivers/pci/controller/dwc/pcie-tegra194.c
> > > > index bafd2c6..7349926 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> > > > @@ -2021,14 +2021,13 @@ static int tegra_pcie_config_ep(struct
> > > > tegra_pcie_dw *pcie,
> > > >                  return -ENOMEM;
> > > >          }
> > > > 
> > > > -       irq_set_status_flags(pcie->pex_rst_irq, IRQ_NOAUTOEN);
> > > > -
> > > >          pcie->ep_state = EP_STATE_DISABLED;
> > > > 
> > > >          ret = devm_request_threaded_irq(dev, pcie->pex_rst_irq, NULL,
> > > >                                          tegra_pcie_ep_pex_rst_irq,
> > > >                                          IRQF_TRIGGER_RISING |
> > > > -                                       IRQF_TRIGGER_FALLING |
> > > > IRQF_ONESHOT,
> > > > +                                       IRQF_TRIGGER_FALLING |
> > > > +                                       IRQF_ONESHOT | IRQF_NO_AUTOEN
> > > >                                          name, (void *)pcie);
> > > >          if (ret < 0) {
> > > >                  dev_err(dev, "Failed to request IRQ for PERST:
> > > > %d\n", ret);
> > > > -- 
> > > > 2.7.4
> > > >