Re: [PATCH v3] PCI: imx6: Don't remove MSI capability for i.MX7D/i.MX8M

[Manivannan Sadhasivam]

On Fri, Apr 03, 2026 at 04:56:56PM -0500, Bjorn Helgaas wrote:
> On Tue, Mar 31, 2026 at 04:52:52PM +0800, Richard Zhu wrote:
> > The MSI trigger mechanism for endpoint devices connected to i.MX7D,
> > i.MX8MM, and i.MX8MQ PCIe root complex ports depends on the MSI
> > capability register settings in the root complex. Removing the MSI
> > capability breaks MSI functionality for these endpoints.
> > 
> > Add keep_rp_msi_en flag to indicate platforms (i.MX7D, i.MX8MM, i.MX8MQ)
> > that should preserve the MSI capability during initialization.
> 
> I guess Mani added this to the commit log:
> 
>   Note that by keeping Root Port MSI capability, Root Port MSIs such as AER,
>   PME and others won't be received by default. So users need to use
>   workarounds such as passing 'pcie_pme=nomsi' cmdline param.
> 

Yes, I did.

> Why can't we fix this automatically?  I hate it when users are
> expected to use command line parameters to work around issues.
> 
> Obviously we know at probe-time, before any PCI devices are
> enumerated, so it seems like we should be able to do the equivalent of
> "pcie_pme=nomsi" even without the parameter.
> 

That's why we came up with the idea of disabling the MSI/MSI-X capabilities of
the Root Port so that the code falls back to INTx. But the issue is that,
disabling that capability or MSI Enable bit (mayebe?) prevents endpoint MSIs
from being received. We don't know exactly what's going on inside the hardware
though. But is broken at its best. Fortunately, this behavior only applies to a
subset of the NXP SoCs.

- Mani

> > Cc: stable@xxxxxxxxxxxxxxx
> > Fixes: f5cd8a929c825 ("PCI: dwc: Remove MSI/MSIX capability for Root Port if iMSI-RX is used as MSI controller")
> > Suggested-by: Manivannan Sadhasivam <mani@xxxxxxxxxx>
> > Signed-off-by: Richard Zhu <hongxing.zhu@xxxxxxx>
> > ---
> > v3 changes:
> > Use a flag 'dw_pcie_rp::keep_rp_msi_en' to identify SoCs that require MSI
> > capability preservation, and skip the capability removal in
> > pcie-designware-host.c accordingly.
> > 
> > v2 changes:
> > CC stable tree.
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c             | 7 +++++++
> >  drivers/pci/controller/dwc/pcie-designware-host.c | 2 +-
> >  drivers/pci/controller/dwc/pcie-designware.h      | 1 +
> >  3 files changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index 20dafd2710a3..fde173770933 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -117,6 +117,8 @@ enum imx_pcie_variants {
> >  #define IMX_PCIE_FLAG_HAS_LUT			BIT(10)
> >  #define IMX_PCIE_FLAG_8GT_ECN_ERR051586		BIT(11)
> >  #define IMX_PCIE_FLAG_SKIP_L23_READY		BIT(12)
> > +/* Preserve MSI capability for platforms that require it */
> > +#define IMX_PCIE_FLAG_KEEP_MSI_CAP		BIT(13)
> >  
> >  #define imx_check_flag(pci, val)	(pci->drvdata->flags & val)
> >  
> > @@ -1820,6 +1822,8 @@ static int imx_pcie_probe(struct platform_device *pdev)
> >  	} else {
> >  		if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_SKIP_L23_READY))
> >  			pci->pp.skip_l23_ready = true;
> > +		if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_KEEP_MSI_CAP))
> > +			pci->pp.keep_rp_msi_en = true;
> >  		pci->pp.use_atu_msg = true;
> >  		ret = dw_pcie_host_init(&pci->pp);
> >  		if (ret < 0)
> > @@ -1897,6 +1901,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
> >  	[IMX7D] = {
> >  		.variant = IMX7D,
> >  		.flags = IMX_PCIE_FLAG_SUPPORTS_SUSPEND |
> > +			 IMX_PCIE_FLAG_KEEP_MSI_CAP |
> >  			 IMX_PCIE_FLAG_HAS_APP_RESET |
> >  			 IMX_PCIE_FLAG_SKIP_L23_READY |
> >  			 IMX_PCIE_FLAG_HAS_PHY_RESET,
> > @@ -1909,6 +1914,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
> >  	[IMX8MQ] = {
> >  		.variant = IMX8MQ,
> >  		.flags = IMX_PCIE_FLAG_HAS_APP_RESET |
> > +			 IMX_PCIE_FLAG_KEEP_MSI_CAP |
> >  			 IMX_PCIE_FLAG_HAS_PHY_RESET |
> >  			 IMX_PCIE_FLAG_SUPPORTS_SUSPEND,
> >  		.gpr = "fsl,imx8mq-iomuxc-gpr",
> > @@ -1923,6 +1929,7 @@ static const struct imx_pcie_drvdata drvdata[] = {
> >  	[IMX8MM] = {
> >  		.variant = IMX8MM,
> >  		.flags = IMX_PCIE_FLAG_SUPPORTS_SUSPEND |
> > +			 IMX_PCIE_FLAG_KEEP_MSI_CAP |
> >  			 IMX_PCIE_FLAG_HAS_PHYDRV |
> >  			 IMX_PCIE_FLAG_HAS_APP_RESET,
> >  		.gpr = "fsl,imx8mm-iomuxc-gpr",
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index a74339982c24..7b5558561e15 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -1171,7 +1171,7 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
> >  	 * the MSI and MSI-X capabilities of the Root Port to allow the drivers
> >  	 * to fall back to INTx instead.
> >  	 */
> > -	if (pp->use_imsi_rx) {
> > +	if (pp->use_imsi_rx && !pp->keep_rp_msi_en) {
> >  		dw_pcie_remove_capability(pci, PCI_CAP_ID_MSI);
> >  		dw_pcie_remove_capability(pci, PCI_CAP_ID_MSIX);
> >  	}
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index ae6389dd9caa..b12c5334552c 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -421,6 +421,7 @@ struct dw_pcie_host_ops {
> >  
> >  struct dw_pcie_rp {
> >  	bool			use_imsi_rx:1;
> > +	bool			keep_rp_msi_en:1;
> >  	bool			cfg0_io_shared:1;
> >  	u64			cfg0_base;
> >  	void __iomem		*va_cfg0_base;
> > -- 
> > 2.37.1
> > 

-- 
மணிவண்ணன் சதாசிவம்