Re: [PATCH v2] PCI: imx6: Update MPLLB bandwidth to improve i.MX95 Gen3 PCIe stability

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Jul 09, 2026 at 07:55:10AM +0000, Hongxing Zhu (OSS) wrote:
> > -----Original Message-----
> > From: Frank Li (OSS) <frank.li@oss.nxp.com>
> > Sent: Wednesday, July 8, 2026 11:19 PM
> > To: Hongxing Zhu (OSS) <hongxing.zhu@oss.nxp.com>
> > Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> > kwilczynski@kernel.org; mani@kernel.org; robh@kernel.org;
> > bhelgaas@google.com; s.hauer@pengutronix.de; kernel@pengutronix.de;
> > festevam@gmail.com; linux-pci@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; imx@lists.linux.dev; linux-kernel@vger.kernel.org;
> > Hongxing Zhu <hongxing.zhu@nxp.com>
> > Subject: Re: [PATCH v2] PCI: imx6: Update MPLLB bandwidth to improve i.MX95
> > Gen3 PCIe stability
> >
> > On Wed, Jul 08, 2026 at 11:59:28AM +0800, hongxing.zhu@oss.nxp.com wrote:
> > > From: Richard Zhu <hongxing.zhu@nxp.com>
> > >
> > > Bandwidth marginality was observed during i.MX95 Gen3 PCIe tests with
> > > the default MPLLB_BINDWIDTH value. This margin degradation worsens
> > > across voltage and temperature (VT) variations and different test
> > > matrices, potentially causing link stability issues.
> > >
> > > Testing with MPLLB_BINDWIDTH value of 140 (0x8c) shows significant
> > > improvement in bandwidth margins across all VT conditions and test
> > > scenarios.
> > >
> > > Implement PHY register write helper function and configure:
> > > - MPLLB_BW_OVRD_IN = 140 (0x8c) for improved bandwidth margin
> > > - MPLLB_BW_OVRD_EN to enable the override
> > >
> > > This ensures robust PCIe Gen3 performance across all operating
> > > conditions.
> > >
> > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > ---
> > > Changes in v2:
> > > Update the register name and bit definitions.
> > > Don't move IMX95_PCIE_PHY_CR_PARA_SEL settings.
> > > ---
> > >  drivers/pci/controller/dwc/pci-imx6.c | 24 ++++++++++++++++++++++++
> > >  1 file changed, 24 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > index 53f3da6ab30d5..5f75aa09e0377 100644
> > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > @@ -80,6 +80,15 @@
> > >  #define IMX95_SID_MASK				GENMASK(5, 0)
> > >  #define IMX95_MAX_LUT				32
> > >
> > > +#define IMX95_PCIE_PHY_REG_ADDR			0x3008
> > > +#define IMX95_PCIE_PHY_REG_EN			BIT(31)
> > > +#define IMX95_PCIE_PHY_REG_ADDR_MASK		GENMASK(15, 0)
> > > +#define IMX95_PCIE_PHY_REG_DATA			0x300c
> > > +#define IMX95_PCIE_PHY_MPLLB_OVRD_IN		0x2004
> > > +#define IMX95_PCIE_PHY_MPLLB_OVRD_BW_EN		0x400
> >
> > > +#define IMX95_PCIE_PHY_MPLLB_OVRD_BW_IN		0x2005
> >
> > This one IMX95_PCIE_PHY_MPLLB_BW_IN to keep naming consistent
> >
> > write IMX95_PCIE_PHY_MPLLB_OVRD_BW_EN ->
> > IMX95_PCIE_PHY_MPLLB_OVRD_IN write IMX95_PCIE_PHY_MPLLB_BW ->
> > IMX95_PCIE_PHY_MPLLB_BW_IN
> >
> > > +#define IMX95_PCIE_PHY_MPLLB_BW			0x8c
> >
> > 0x8c only one special value.
> >
> > IMX95_PCIE_PHY_MPLLB_BW_XXX put special name for 0x8c, if no means, you
> > can put 0x8c direct at
> >
> > imx95_pcie_phy_write(imx_pcie, IMX95_PCIE_PHY_MPLLB_OVRD_BW_IN,
> > 0x8c)
> >
> > We need knows what's macro means, like it is offset of regiser or it is field in
> > register easily.
> How about add some comments as below?
> +/* BIT(10): Override enable for mpllb_bandwidth[15:0] */
>  #define IMX95_PCIE_PHY_MPLLB_OVRD_BW_EN                0x400
> -#define IMX95_PCIE_PHY_MPLLB_OVRD_BW_IN                0x2005
> -#define IMX95_PCIE_PHY_MPLLB_BW                        0x8c
> +/* Register offset: Override value for mpllb_bandwidth[15:0] */
> +#define IMX95_PCIE_PHY_MPLLB_BW_IN             0x2005
> +#define IMX95_PCIE_PHY_MPLLB_BW_VAL            0x8c

Okay

Frank

>
>  #define IMX95_PCIE_RST_CTRL                    0x3010
>  #define IMX95_PCIE_COLD_RST                    BIT(0)
> @@ -309,8 +311,8 @@ static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
>                         IMX95_PCIE_PHY_CR_PARA_SEL,
>                         IMX95_PCIE_PHY_CR_PARA_SEL);
>
> -       imx95_pcie_phy_write(imx_pcie, IMX95_PCIE_PHY_MPLLB_OVRD_BW_IN,
> -                            IMX95_PCIE_PHY_MPLLB_BW);
> +       imx95_pcie_phy_write(imx_pcie, IMX95_PCIE_PHY_MPLLB_BW_IN,
> +                            IMX95_PCIE_PHY_MPLLB_BW_VAL);
>
> Best Regards
> Richard Zhu
> >
> > Frank
> >
> > > +
> > >  #define IMX95_PCIE_RST_CTRL			0x3010
> > >  #define IMX95_PCIE_COLD_RST			BIT(0)
> > >
> > > @@ -269,6 +278,16 @@ static int imx95_pcie_select_ref_clk_src(struct
> > imx_pcie *imx_pcie)
> > >  	return 0;
> > >  }
> > >
> > > +static void imx95_pcie_phy_write(struct imx_pcie *imx_pcie, int addr,
> > > +u16 data) {
> > > +	udelay(200);
> > > +	regmap_update_bits(imx_pcie->iomuxc_gpr,
> > IMX95_PCIE_PHY_REG_ADDR,
> > > +			   IMX95_PCIE_PHY_REG_EN,
> > IMX95_PCIE_PHY_REG_EN);
> > > +	regmap_update_bits(imx_pcie->iomuxc_gpr,
> > IMX95_PCIE_PHY_REG_ADDR,
> > > +			   IMX95_PCIE_PHY_REG_ADDR_MASK, addr);
> > > +	regmap_write(imx_pcie->iomuxc_gpr, IMX95_PCIE_PHY_REG_DATA,
> > data); }
> > > +
> > >  static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)  {
> > >  	/*
> > > @@ -289,6 +308,11 @@ static int imx95_pcie_init_phy(struct imx_pcie
> > *imx_pcie)
> > >  			IMX95_PCIE_PHY_CR_PARA_SEL,
> > >  			IMX95_PCIE_PHY_CR_PARA_SEL);
> > >
> > > +	imx95_pcie_phy_write(imx_pcie,
> > IMX95_PCIE_PHY_MPLLB_OVRD_BW_IN,
> > > +			     IMX95_PCIE_PHY_MPLLB_BW);
> > > +	imx95_pcie_phy_write(imx_pcie, IMX95_PCIE_PHY_MPLLB_OVRD_IN,
> > > +			     IMX95_PCIE_PHY_MPLLB_OVRD_BW_EN);
> > > +
> > >  	return 0;
> > >  }
> > >
> > > --
> > > 2.34.1
> > >
> > >