* [PATCH v2] PCI: tegra: move to use request_irq by IRQF_NO_AUTOEN flag
@ 2021-05-17 1:12 Tian Tao
2021-05-17 10:17 ` Vidya Sagar
0 siblings, 1 reply; 6+ messages in thread
From: Tian Tao @ 2021-05-17 1:12 UTC (permalink / raw)
To: p.zabel; +Cc: linux-pci, linux-tegra, Tian Tao
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: tegra: move to use request_irq by IRQF_NO_AUTOEN flag
2021-05-17 1:12 [PATCH v2] PCI: tegra: move to use request_irq by IRQF_NO_AUTOEN flag Tian Tao
@ 2021-05-17 10:17 ` Vidya Sagar
2021-05-31 10:02 ` Vidya Sagar
0 siblings, 1 reply; 6+ messages in thread
From: Vidya Sagar @ 2021-05-17 10:17 UTC (permalink / raw)
To: Tian Tao, p.zabel; +Cc: linux-pci, linux-tegra
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: tegra: move to use request_irq by IRQF_NO_AUTOEN flag
2021-05-17 10:17 ` Vidya Sagar
@ 2021-05-31 10:02 ` Vidya Sagar
2021-06-23 9:45 ` Lorenzo Pieralisi
2021-06-26 20:48 ` Michał Mirosław
0 siblings, 2 replies; 6+ messages in thread
From: Vidya Sagar @ 2021-05-31 10:02 UTC (permalink / raw)
To: Tian Tao, p.zabel, lorenzo.pieralisi, bhelgaas; +Cc: linux-pci, linux-tegra
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. 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?
Thanks,
Vidya Sagar
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
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: tegra: move to use request_irq by IRQF_NO_AUTOEN flag
2021-05-31 10:02 ` Vidya Sagar
@ 2021-06-23 9:45 ` Lorenzo Pieralisi
2021-06-23 12:27 ` Bjorn Helgaas
2021-06-26 20:48 ` Michał Mirosław
1 sibling, 1 reply; 6+ messages in thread
From: Lorenzo Pieralisi @ 2021-06-23 9:45 UTC (permalink / raw)
To: Vidya Sagar
Cc: Tian Tao, p.zabel, bhelgaas, linux-pci, linux-tegra, tglx, maz
[+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.
Lorenzo
> Thanks,
> Vidya Sagar
>
>
> 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
> > >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: tegra: move to use request_irq by IRQF_NO_AUTOEN flag
2021-06-23 9:45 ` Lorenzo Pieralisi
@ 2021-06-23 12:27 ` Bjorn Helgaas
0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2021-06-23 12:27 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Vidya Sagar, Tian Tao, p.zabel, bhelgaas, linux-pci, linux-tegra,
tglx, maz
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
> > > >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: tegra: move to use request_irq by IRQF_NO_AUTOEN flag
2021-05-31 10:02 ` Vidya Sagar
2021-06-23 9:45 ` Lorenzo Pieralisi
@ 2021-06-26 20:48 ` Michał Mirosław
1 sibling, 0 replies; 6+ messages in thread
From: Michał Mirosław @ 2021-06-26 20:48 UTC (permalink / raw)
To: Vidya Sagar
Cc: Tian Tao, p.zabel, lorenzo.pieralisi, bhelgaas, linux-pci, linux-tegra
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. It is always set explicitly through
> irq_set_status_flags() *before* calling request_irq APIs.
[...]
The request_irq() flag is called IRQF_NO_AUTOEN. As far as I can read
the cod, it does seem equivalent to changing irq_settings. I'm not sure
why we have two methods to do this in the kernel, though.
Hope that helps.
Best Regards
Michał Mirosław
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-06-26 20:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-17 1:12 [PATCH v2] PCI: tegra: move to use request_irq by IRQF_NO_AUTOEN flag Tian Tao
2021-05-17 10:17 ` Vidya Sagar
2021-05-31 10:02 ` Vidya Sagar
2021-06-23 9:45 ` Lorenzo Pieralisi
2021-06-23 12:27 ` Bjorn Helgaas
2021-06-26 20:48 ` Michał Mirosław
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.