| Message ID | 20240731-pci-qcom-hotplug-v3-6-a1426afdee3b@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: qcom: Enumerate endpoints based on Link up event in 'global_irq' interrupt | expand |
On Thu, Aug 01, 2024 at 12:23:08PM -0500, Bjorn Helgaas wrote: > On Wed, Jul 31, 2024 at 04:20:09PM +0530, Manivannan Sadhasivam via B4 Relay wrote: > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > > > Currently, the IRQ device name for both of these IRQs doesn't have Qcom > > specific prefix and PCIe domain number. This causes 2 issues: > > > > 1. Pollutes the global IRQ namespace since 'global' is a common name. > > 2. When more than one EP controller instance is present in the SoC, naming > > conflict will occur. > > > > Hence, add 'qcom_pcie_ep_' prefix and PCIe domain number suffix to the IRQ > > names to uniquely identify the IRQs and also to fix the above mentioned > > issues. > > > > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > drivers/pci/controller/dwc/pcie-qcom-ep.c | 16 ++++++++++++++-- > > 1 file changed, 14 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c > > index 0bb0a056dd8f..d0a27fa6fdc8 100644 > > --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c > > +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c > > @@ -711,8 +711,15 @@ static irqreturn_t qcom_pcie_ep_perst_irq_thread(int irq, void *data) > > static int qcom_pcie_ep_enable_irq_resources(struct platform_device *pdev, > > struct qcom_pcie_ep *pcie_ep) > > { > > + struct device *dev = pcie_ep->pci.dev; > > + char *name; > > int ret; > > > > + name = devm_kasprintf(dev, GFP_KERNEL, "qcom_pcie_ep_global_irq%d", > > + pcie_ep->pci.ep.epc->domain_nr); > > + if (!name) > > + return -ENOMEM; > > I assume this is what shows up in /proc/interrupts? Yes. > I always wonder > why it doesn't include dev_name(). A few drivers do that, but > apparently it's not universally desirable. It's sort of annoying > that, e.g., we get a bunch of "aerdrv" interrupts with no clue which > device they relate to. > dev_name() can be big at times. I wouldn't recommend to include it unless there are no other ways to differentiate between IRQs. Luckily PCIe has the domain number that we can use to differentiate these IRQs. - Mani > > pcie_ep->global_irq = platform_get_irq_byname(pdev, "global"); > > if (pcie_ep->global_irq < 0) > > return pcie_ep->global_irq; > > @@ -720,18 +727,23 @@ static int qcom_pcie_ep_enable_irq_resources(struct platform_device *pdev, > > ret = devm_request_threaded_irq(&pdev->dev, pcie_ep->global_irq, NULL, > > qcom_pcie_ep_global_irq_thread, > > IRQF_ONESHOT, > > - "global_irq", pcie_ep); > > + name, pcie_ep); > > if (ret) { > > dev_err(&pdev->dev, "Failed to request Global IRQ\n"); > > return ret; > > } > > > > + name = devm_kasprintf(dev, GFP_KERNEL, "qcom_pcie_ep_perst_irq%d", > > + pcie_ep->pci.ep.epc->domain_nr); > > + if (!name) > > + return -ENOMEM; > > + > > pcie_ep->perst_irq = gpiod_to_irq(pcie_ep->reset); > > irq_set_status_flags(pcie_ep->perst_irq, IRQ_NOAUTOEN); > > ret = devm_request_threaded_irq(&pdev->dev, pcie_ep->perst_irq, NULL, > > qcom_pcie_ep_perst_irq_thread, > > IRQF_TRIGGER_HIGH | IRQF_ONESHOT, > > - "perst_irq", pcie_ep); > > + name, pcie_ep); > > if (ret) { > > dev_err(&pdev->dev, "Failed to request PERST IRQ\n"); > > disable_irq(pcie_ep->global_irq); > > > > -- > > 2.25.1 > > > >
diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c index 0bb0a056dd8f..d0a27fa6fdc8 100644 --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c @@ -711,8 +711,15 @@ static irqreturn_t qcom_pcie_ep_perst_irq_thread(int irq, void *data) static int qcom_pcie_ep_enable_irq_resources(struct platform_device *pdev, struct qcom_pcie_ep *pcie_ep) { + struct device *dev = pcie_ep->pci.dev; + char *name; int ret; + name = devm_kasprintf(dev, GFP_KERNEL, "qcom_pcie_ep_global_irq%d", + pcie_ep->pci.ep.epc->domain_nr); + if (!name) + return -ENOMEM; + pcie_ep->global_irq = platform_get_irq_byname(pdev, "global"); if (pcie_ep->global_irq < 0) return pcie_ep->global_irq; @@ -720,18 +727,23 @@ static int qcom_pcie_ep_enable_irq_resources(struct platform_device *pdev, ret = devm_request_threaded_irq(&pdev->dev, pcie_ep->global_irq, NULL, qcom_pcie_ep_global_irq_thread, IRQF_ONESHOT, - "global_irq", pcie_ep); + name, pcie_ep); if (ret) { dev_err(&pdev->dev, "Failed to request Global IRQ\n"); return ret; } + name = devm_kasprintf(dev, GFP_KERNEL, "qcom_pcie_ep_perst_irq%d", + pcie_ep->pci.ep.epc->domain_nr); + if (!name) + return -ENOMEM; + pcie_ep->perst_irq = gpiod_to_irq(pcie_ep->reset); irq_set_status_flags(pcie_ep->perst_irq, IRQ_NOAUTOEN); ret = devm_request_threaded_irq(&pdev->dev, pcie_ep->perst_irq, NULL, qcom_pcie_ep_perst_irq_thread, IRQF_TRIGGER_HIGH | IRQF_ONESHOT, - "perst_irq", pcie_ep); + name, pcie_ep); if (ret) { dev_err(&pdev->dev, "Failed to request PERST IRQ\n"); disable_irq(pcie_ep->global_irq);