| Message ID | 20241115165717.15233-1-ilpo.jarvinen@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [1/1] PCI/bwctrl: Remove IRQF_ONESHOT and handle hardirqs instead | expand |
Hi, Am 15.11.24 um 17:57 schrieb Ilpo Järvinen: > bwctrl cannot use IRQF_ONESHOT because it shares interrupt with other > service drivers that are not using IRQF_ONESHOT nor compatible with it. > > Remove IRQF_ONESHOT from bwctrl and convert the irq thread to hardirq > handler. Rename the handler to pcie_bwnotif_irq() to indicate its new > purpose. > > The IRQ handler is simple enough to not require not require other > changes. > > Fixes: 058a4cb11620 ("PCI/bwctrl: Re-add BW notification portdrv as PCIe BW controller") > Reported-by: Stefan Wahren <wahrenst@gmx.net> > Link: https://lore.kernel.org/linux-pci/dcd660fd-a265-4f47-8696-776a85e097a0@gmx.net/ > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> this fixed the probing issue. Thanks Tested-by: Stefan Wahren <wahrenst@gmx.net> Is there anything more I can/should test? > --- > drivers/pci/pcie/bwctrl.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/pcie/bwctrl.c b/drivers/pci/pcie/bwctrl.c > index ff5d12e01f9c..a6c65bbe3735 100644 > --- a/drivers/pci/pcie/bwctrl.c > +++ b/drivers/pci/pcie/bwctrl.c > @@ -230,7 +230,7 @@ static void pcie_bwnotif_disable(struct pci_dev *port) > PCI_EXP_LNKCTL_LBMIE | PCI_EXP_LNKCTL_LABIE); > } > > -static irqreturn_t pcie_bwnotif_irq_thread(int irq, void *context) > +static irqreturn_t pcie_bwnotif_irq(int irq, void *context) > { > struct pcie_device *srv = context; > struct pcie_bwctrl_data *data = srv->port->link_bwctrl; > @@ -302,10 +302,8 @@ static int pcie_bwnotif_probe(struct pcie_device *srv) > if (ret) > return ret; > > - ret = devm_request_threaded_irq(&srv->device, srv->irq, NULL, > - pcie_bwnotif_irq_thread, > - IRQF_SHARED | IRQF_ONESHOT, > - "PCIe bwctrl", srv); > + ret = devm_request_irq(&srv->device, srv->irq, pcie_bwnotif_irq, > + IRQF_SHARED, "PCIe bwctrl", srv); > if (ret) > return ret; >
On Fri, Nov 15, 2024 at 06:57:17PM +0200, Ilpo Järvinen wrote: > bwctrl cannot use IRQF_ONESHOT because it shares interrupt with other > service drivers that are not using IRQF_ONESHOT nor compatible with it. > > Remove IRQF_ONESHOT from bwctrl and convert the irq thread to hardirq > handler. Rename the handler to pcie_bwnotif_irq() to indicate its new > purpose. > > The IRQ handler is simple enough to not require not require other > changes. > > Fixes: 058a4cb11620 ("PCI/bwctrl: Re-add BW notification portdrv as PCIe BW controller") > Reported-by: Stefan Wahren <wahrenst@gmx.net> > Link: https://lore.kernel.org/linux-pci/dcd660fd-a265-4f47-8696-776a85e097a0@gmx.net/ > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Squashed into 058a4cb11620, thanks! Also added your tested-by, Stefan, thanks very much for doing that! > --- > drivers/pci/pcie/bwctrl.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/pcie/bwctrl.c b/drivers/pci/pcie/bwctrl.c > index ff5d12e01f9c..a6c65bbe3735 100644 > --- a/drivers/pci/pcie/bwctrl.c > +++ b/drivers/pci/pcie/bwctrl.c > @@ -230,7 +230,7 @@ static void pcie_bwnotif_disable(struct pci_dev *port) > PCI_EXP_LNKCTL_LBMIE | PCI_EXP_LNKCTL_LABIE); > } > > -static irqreturn_t pcie_bwnotif_irq_thread(int irq, void *context) > +static irqreturn_t pcie_bwnotif_irq(int irq, void *context) > { > struct pcie_device *srv = context; > struct pcie_bwctrl_data *data = srv->port->link_bwctrl; > @@ -302,10 +302,8 @@ static int pcie_bwnotif_probe(struct pcie_device *srv) > if (ret) > return ret; > > - ret = devm_request_threaded_irq(&srv->device, srv->irq, NULL, > - pcie_bwnotif_irq_thread, > - IRQF_SHARED | IRQF_ONESHOT, > - "PCIe bwctrl", srv); > + ret = devm_request_irq(&srv->device, srv->irq, pcie_bwnotif_irq, > + IRQF_SHARED, "PCIe bwctrl", srv); > if (ret) > return ret; > > -- > 2.39.5 >
On Fri, 15 Nov 2024, Bjorn Helgaas wrote: > On Fri, Nov 15, 2024 at 06:57:17PM +0200, Ilpo Järvinen wrote: > > bwctrl cannot use IRQF_ONESHOT because it shares interrupt with other > > service drivers that are not using IRQF_ONESHOT nor compatible with it. > > > > Remove IRQF_ONESHOT from bwctrl and convert the irq thread to hardirq > > handler. Rename the handler to pcie_bwnotif_irq() to indicate its new > > purpose. > > > > The IRQ handler is simple enough to not require not require other > > changes. > > > > Fixes: 058a4cb11620 ("PCI/bwctrl: Re-add BW notification portdrv as PCIe BW controller") > > Reported-by: Stefan Wahren <wahrenst@gmx.net> > > Link: https://lore.kernel.org/linux-pci/dcd660fd-a265-4f47-8696-776a85e097a0@gmx.net/ > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > Squashed into 058a4cb11620, thanks! > > Also added your tested-by, Stefan, thanks very much for doing that! Hi Bjorn, You might want to also remove "3) ..." part from the commit message as it still refers to threaded IRQ and IRQF_ONESHOT so it won't confuse anybody when looking at this years from now :-).
On Sat, Nov 16, 2024 at 04:40:51PM +0200, Ilpo Järvinen wrote: > On Fri, 15 Nov 2024, Bjorn Helgaas wrote: > > > On Fri, Nov 15, 2024 at 06:57:17PM +0200, Ilpo Järvinen wrote: > > > bwctrl cannot use IRQF_ONESHOT because it shares interrupt with other > > > service drivers that are not using IRQF_ONESHOT nor compatible with it. > > > > > > Remove IRQF_ONESHOT from bwctrl and convert the irq thread to hardirq > > > handler. Rename the handler to pcie_bwnotif_irq() to indicate its new > > > purpose. > > > > > > The IRQ handler is simple enough to not require not require other > > > changes. > > > > > > Fixes: 058a4cb11620 ("PCI/bwctrl: Re-add BW notification portdrv as PCIe BW controller") > > > Reported-by: Stefan Wahren <wahrenst@gmx.net> > > > Link: https://lore.kernel.org/linux-pci/dcd660fd-a265-4f47-8696-776a85e097a0@gmx.net/ > > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > > > Squashed into 058a4cb11620, thanks! > > > > Also added your tested-by, Stefan, thanks very much for doing that! > > Hi Bjorn, > > You might want to also remove "3) ..." part from the commit message as it > still refers to threaded IRQ and IRQF_ONESHOT so it won't confuse anybody > when looking at this years from now :-). Removed, thanks for that!
diff --git a/drivers/pci/pcie/bwctrl.c b/drivers/pci/pcie/bwctrl.c index ff5d12e01f9c..a6c65bbe3735 100644 --- a/drivers/pci/pcie/bwctrl.c +++ b/drivers/pci/pcie/bwctrl.c @@ -230,7 +230,7 @@ static void pcie_bwnotif_disable(struct pci_dev *port) PCI_EXP_LNKCTL_LBMIE | PCI_EXP_LNKCTL_LABIE); } -static irqreturn_t pcie_bwnotif_irq_thread(int irq, void *context) +static irqreturn_t pcie_bwnotif_irq(int irq, void *context) { struct pcie_device *srv = context; struct pcie_bwctrl_data *data = srv->port->link_bwctrl; @@ -302,10 +302,8 @@ static int pcie_bwnotif_probe(struct pcie_device *srv) if (ret) return ret; - ret = devm_request_threaded_irq(&srv->device, srv->irq, NULL, - pcie_bwnotif_irq_thread, - IRQF_SHARED | IRQF_ONESHOT, - "PCIe bwctrl", srv); + ret = devm_request_irq(&srv->device, srv->irq, pcie_bwnotif_irq, + IRQF_SHARED, "PCIe bwctrl", srv); if (ret) return ret;
bwctrl cannot use IRQF_ONESHOT because it shares interrupt with other service drivers that are not using IRQF_ONESHOT nor compatible with it. Remove IRQF_ONESHOT from bwctrl and convert the irq thread to hardirq handler. Rename the handler to pcie_bwnotif_irq() to indicate its new purpose. The IRQ handler is simple enough to not require not require other changes. Fixes: 058a4cb11620 ("PCI/bwctrl: Re-add BW notification portdrv as PCIe BW controller") Reported-by: Stefan Wahren <wahrenst@gmx.net> Link: https://lore.kernel.org/linux-pci/dcd660fd-a265-4f47-8696-776a85e097a0@gmx.net/ Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> --- drivers/pci/pcie/bwctrl.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)