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

[Manivannan Sadhasivam]

On Thu, Jul 30, 2026 at 07:07:58AM +0000, Hongxing Zhu (OSS) wrote:
> > -----Original Message-----
> > From: Manivannan Sadhasivam <mani@xxxxxxxxxx>
> > Sent: Wednesday, July 29, 2026 9:58 PM
> > To: Hongxing Zhu (OSS) <hongxing.zhu@xxxxxxxxxxx>
> > Cc: Frank Li <frank.li@xxxxxxx>; l.stach@xxxxxxxxxxxxxx; lpieralisi@xxxxxxxxxx;
> > kwilczynski@xxxxxxxxxx; robh@xxxxxxxxxx; bhelgaas@xxxxxxxxxx;
> > s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; linux-
> > pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx;
> > linux-kernel@xxxxxxxxxxxxxxx; Hongxing Zhu <hongxing.zhu@xxxxxxx>
> > Subject: Re: [PATCH v3] PCI: imx6: Update MPLLB bandwidth to improve i.MX95
> > Gen3 PCIe stability
> > 
> > On Tue, Jul 14, 2026 at 10:41:07AM +0800, hongxing.zhu@xxxxxxxxxxx wrote:
> > > From: Richard Zhu <hongxing.zhu@xxxxxxx>
> > >
> > > Bandwidth marginality was observed during i.MX95 Gen3 PCIe tests with
> > > the default MPLLB_BANDWIDTH value. This margin degradation worsens
> > > across voltage and temperature (VT) variations and different test
> > > matrices, potentially causing link stability issues.
> > >
> > > Testing with MPLLB_BANDWIDTH 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@xxxxxxx>
> > > ---
> > > Changes in v3:
> > > Regarding Frank's comments, refine naming consistency and clarity.
> > >
> > > 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 | 26 ++++++++++++++++++++++++++
> > >  1 file changed, 26 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > index 53f3da6ab30d5..fadf14de10f7c 100644
> > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > @@ -80,6 +80,17 @@
> > >  #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
> > > +/* BIT(10): Override enable for mpllb_bandwidth[15:0] */
> > > +#define IMX95_PCIE_PHY_MPLLB_OVRD_BW_EN		0x400
> > 
> > Use BIT(10) please.
> Okay, will change to BIT(10).
> 
> > 
> > > +/* 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
> > 
> > Is it possible to define individual bits?
> > 
> This is a 16-bit bandwidth value (140 decimal = 0x8c) recommended by the IC
> designers. It appears to be a complete value rather than a bit field, but
> please let me know if you'd prefer a different representation.
> 

Fine then.

> > > +
> > >  #define IMX95_PCIE_RST_CTRL			0x3010
> > >  #define IMX95_PCIE_COLD_RST			BIT(0)
> > >
> > > @@ -269,6 +280,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);
> > 
> > Why is this delay at the start of this function?
> A proper delay is required between the assertion of IMX95_PCIE_PHY_CR_PARA_SEL
> (in imx95_pcie_init_phy) and each subsequent PHY register write operation.
> Without this delay, the values cannot be successfully written to the PHY
> registers. Placing the delay at the start of the helper function ensures it's
> applied before every PHY write.
> 

Then the delay should be moved to imx95_pcie_init_phy() with a comment.

- Mani

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