RE: [PATCH v2] PCI: imx6: Don't remove MSI capability For i.MX7D/i.MX8M

[Hongxing Zhu]

> -----Original Message-----
> From: Frank Li <frank.li@xxxxxxx>
> Sent: 2026年3月19日 22:17
> To: Hongxing Zhu <hongxing.zhu@xxxxxxx>
> Cc: l.stach@xxxxxxxxxxxxxx; lpieralisi@xxxxxxxxxx; kwilczynski@xxxxxxxxxx;
> mani@xxxxxxxxxx; robh@xxxxxxxxxx; bhelgaas@xxxxxxxxxx;
> s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx;
> linux-pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> imx@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v2] PCI: imx6: Don't remove MSI capability For
> i.MX7D/i.MX8M
> 
> On Thu, Mar 19, 2026 at 05:18:23PM +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.
> >
> > Preserve the MSI capability for i.MX7D/i.MX8M PCIe root complex to
> > maintain MSI functionality.
> >
> > Cc: stable@xxxxxxxxxxxxxxx
> > Fixes: f5cd8a929c825 ("PCI: dwc: Remove MSI/MSIX capability for Root
> > Port if iMSI-RX is used as MSI controller")
> 
> I think it'd better add another varible to check in f5cd8a929c825 if
> (pp->has_msi_ctrl && !pp->xxx_broken) or direct use IP version, which
> already auto detected.
> 
> Previous patch have not consider this old version controller.
Hi Frank:
From what I've observed, this behavior seems tied to the specific controller
design. For example, neither the i.MX6Q nor the i.MX6SX exhibit this issue.

The intention of commit f5cd8a929c825 is to remove the MSI capability from the
Root Complex (RC). From the author's perspective, this change should not
affect the  Endpoint's (EP) MSI functionality.

I'm not sure do this check (pp->has_msi_ctrl && !pp->msi_broken) is proper or not.
Best Regards
Richard Zhu
> 
> Frank
> 
> > Signed-off-by: Richard Zhu <hongxing.zhu@xxxxxxx>
> > ---
> > v2 changes:
> > CC stable tree.
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 20dafd2710a3..0b0d6a210406 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -41,6 +41,7 @@
> >  #define IMX8MQ_GPR_PCIE_CLK_REQ_OVERRIDE	BIT(11)
> >  #define IMX8MQ_GPR_PCIE_VREG_BYPASS		BIT(12)
> >  #define IMX8MQ_GPR12_PCIE2_CTRL_DEVICE_TYPE	GENMASK(11, 8)
> > +#define IMX8MM_PCIE_MSI_CAP_OFFSET		0x50
> >
> >  #define IMX95_PCIE_PHY_GEN_CTRL			0x0
> >  #define IMX95_PCIE_REF_USE_PAD			BIT(17)
> > @@ -117,6 +118,7 @@ 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)
> > +#define IMX_PCIE_FLAG_KEEP_MSI_CAP		BIT(13)
> >
> >  #define imx_check_flag(pci, val)	(pci->drvdata->flags & val)
> >
> > @@ -976,10 +978,17 @@ static int imx_pcie_start_link(struct dw_pcie
> > *pci)  {
> >  	struct imx_pcie *imx_pcie = to_imx_pcie(pci);
> >  	struct device *dev = pci->dev;
> > -	u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> > +	u8 offset;
> >  	u32 tmp;
> >  	int ret;
> >
> > +	if (imx_pcie->drvdata->flags & IMX_PCIE_FLAG_KEEP_MSI_CAP) {
> > +		offset = dw_pcie_find_capability(pci, PCI_CAP_ID_PM);
> > +		dw_pcie_dbi_ro_wr_en(pci);
> > +		dw_pcie_writeb_dbi(pci, offset + 1,
> IMX8MM_PCIE_MSI_CAP_OFFSET);
> > +		dw_pcie_dbi_ro_wr_dis(pci);
> > +	}
> > +
> >  	if (!(imx_pcie->drvdata->flags &
> >  	    IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND)) {
> >  		imx_pcie_ltssm_enable(dev);
> > @@ -991,6 +1000,7 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
> >  	 * started in Gen2 mode, there is a possibility the devices on the
> >  	 * bus will not be detected at all.  This happens with PCIe switches.
> >  	 */
> > +	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> >  	dw_pcie_dbi_ro_wr_en(pci);
> >  	tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
> >  	tmp &= ~PCI_EXP_LNKCAP_SLS;
> > @@ -1897,6 +1907,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 +1920,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 +1935,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",
> > --
> > 2.37.1
> >