| Message ID | 20201028204646.356535-2-robh@kernel.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: dwc: Another round of clean-ups | expand |
Rob Herring <robh@kernel.org> writes: > No other host driver sets the PCI_MSI_FLAGS_ENABLE bit, so it must not > be necessary. If it is, a comment is needed. Yeah, but git blame directly points to: 75cb8d20c112 ("PCI: imx: Enable MSI from downstream components") Which has a pretty long explanation. The relevant bit probably being: ... on i.MX6, the MSI Enable bit controls delivery of MSI interrupts from components below the Root Port. So it seems a little rash to just remove the code. cheers > Cc: Richard Zhu <hongxing.zhu@nxp.com> > Cc: Lucas Stach <l.stach@pengutronix.de> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > Cc: Pengutronix Kernel Team <kernel@pengutronix.de> > Cc: Fabio Estevam <festevam@gmail.com> > Cc: NXP Linux Team <linux-imx@nxp.com> > Signed-off-by: Rob Herring <robh@kernel.org> > --- > drivers/pci/controller/dwc/pci-imx6.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index 5cf1ef12fb9b..7dd137d62dca 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -1002,7 +1002,6 @@ static int imx6_pcie_probe(struct platform_device *pdev) > struct resource *dbi_base; > struct device_node *node = dev->of_node; > int ret; > - u16 val; > > imx6_pcie = devm_kzalloc(dev, sizeof(*imx6_pcie), GFP_KERNEL); > if (!imx6_pcie) > @@ -1167,13 +1166,6 @@ static int imx6_pcie_probe(struct platform_device *pdev) > if (ret < 0) > return ret; > > - if (pci_msi_enabled()) { > - u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI); > - val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS); > - val |= PCI_MSI_FLAGS_ENABLE; > - dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val); > - } > - > return 0; > } > > -- > 2.25.1
On Wed, Oct 28, 2020 at 7:21 PM Michael Ellerman <mpe@ellerman.id.au> wrote: > > Rob Herring <robh@kernel.org> writes: > > No other host driver sets the PCI_MSI_FLAGS_ENABLE bit, so it must not > > be necessary. If it is, a comment is needed. > > Yeah, but git blame directly points to: > > 75cb8d20c112 ("PCI: imx: Enable MSI from downstream components") I think I did read this at some point and then forgot about it when I made the change later... > Which has a pretty long explanation. The relevant bit probably being: > > ... on i.MX6, the MSI Enable bit controls delivery of MSI interrupts > from components below the Root Port. The thing is that all seems not i.MX6 specific but DWC specific given MSI handling is contained within the DWC block. So I don't see how this could be an integration difference. So maybe everyone else is still just setting CONFIG_PCIEPORTBUS typically and haven't noticed? Is it correct for the host driver to set MSI enable? Rob
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index 5cf1ef12fb9b..7dd137d62dca 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -1002,7 +1002,6 @@ static int imx6_pcie_probe(struct platform_device *pdev) struct resource *dbi_base; struct device_node *node = dev->of_node; int ret; - u16 val; imx6_pcie = devm_kzalloc(dev, sizeof(*imx6_pcie), GFP_KERNEL); if (!imx6_pcie) @@ -1167,13 +1166,6 @@ static int imx6_pcie_probe(struct platform_device *pdev) if (ret < 0) return ret; - if (pci_msi_enabled()) { - u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI); - val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS); - val |= PCI_MSI_FLAGS_ENABLE; - dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val); - } - return 0; }
No other host driver sets the PCI_MSI_FLAGS_ENABLE bit, so it must not be necessary. If it is, a comment is needed. Cc: Richard Zhu <hongxing.zhu@nxp.com> Cc: Lucas Stach <l.stach@pengutronix.de> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Shawn Guo <shawnguo@kernel.org> Cc: Sascha Hauer <s.hauer@pengutronix.de> Cc: Pengutronix Kernel Team <kernel@pengutronix.de> Cc: Fabio Estevam <festevam@gmail.com> Cc: NXP Linux Team <linux-imx@nxp.com> Signed-off-by: Rob Herring <robh@kernel.org> --- drivers/pci/controller/dwc/pci-imx6.c | 8 -------- 1 file changed, 8 deletions(-)