* Re: [PATCH v3] PCI: imx6: Update MPLLB bandwidth to improve i.MX95 Gen3 PCIe stability
2026-07-14 2:41 [PATCH v3] PCI: imx6: Update MPLLB bandwidth to improve i.MX95 Gen3 PCIe stability hongxing.zhu
@ 2026-07-14 2:53 ` sashiko-bot
2026-07-14 14:41 ` Frank Li
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-07-14 2:53 UTC (permalink / raw)
To: hongxing.zhu; +Cc: linux-pci, imx, Frank.Li
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Non-atomic configuration of the PHY address and enable bit in `imx95_pcie_phy_write` creates a potentially unsafe intermediate hardware state.
- [Medium] The unconditional 200µs delay is misplaced at the beginning of the PHY write function, leading to a missing post-write settling delay.
- [Medium] Raw write to `IMX95_PCIE_PHY_MPLLB_OVRD_IN` potentially clobbers other override bits.
--
--- Patch [1]: [PATCH v3] PCI: imx6: Update MPLLB bandwidth to improve i.MX95 Gen3 PCIe stability ---
commit 62a7810800383a4677a34a256947f86390e8e2a1
Author: Richard Zhu <hongxing.zhu@nxp.com>
PCI: imx6: Update MPLLB bandwidth to improve i.MX95 Gen3 PCIe stability
This commit updates the MPLLB bandwidth margin settings for the i.MX95 Gen3
PCIe driver to improve link stability across voltage and temperature
variations. It implements a PHY register write helper function to configure
the override registers.
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 98e1db751132a..55f8aa47cccfd 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
[ ... ]
> @@ -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);
[Severity: Medium]
Is the udelay(200) placed correctly here before the register updates?
If this delay is intended to allow the hardware to process the PHY register
write, placing it unconditionally before the writes means the final write in a
sequence will not have a settling delay before initialization proceeds. Would
it make more sense to place this delay after the regmap_write()?
> + 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);
[Severity: Medium]
Does setting the enable bit in a separate transaction before updating the
address expose a stale address to the PHY interface?
If setting the IMX95_PCIE_PHY_REG_EN bit with a stale address initiates an
access, this sequence might cause a spurious hardware transaction. Could both
the address mask and the enable bit be updated simultaneously in a single
atomic read-modify-write operation?
> + 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 +310,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_BW_IN,
> + IMX95_PCIE_PHY_MPLLB_BW_VAL);
> + imx95_pcie_phy_write(imx_pcie, IMX95_PCIE_PHY_MPLLB_OVRD_IN,
> + IMX95_PCIE_PHY_MPLLB_OVRD_BW_EN);
[Severity: Medium]
Could this write to IMX95_PCIE_PHY_MPLLB_OVRD_IN unintentionally clobber
other bits in the register?
imx95_pcie_phy_write() calls regmap_write() for the data, which will clear
all unspecified bits in the target register to zero. If there are other
necessary override bits or default settings in this register, they would be
disabled by this raw write. Should this use a read-modify-write sequence
instead?
> +
> return 0;
> }
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714024107.4113926-1-hongxing.zhu@oss.nxp.com?part=1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3] PCI: imx6: Update MPLLB bandwidth to improve i.MX95 Gen3 PCIe stability
2026-07-14 2:41 [PATCH v3] PCI: imx6: Update MPLLB bandwidth to improve i.MX95 Gen3 PCIe stability hongxing.zhu
2026-07-14 2:53 ` sashiko-bot
@ 2026-07-14 14:41 ` Frank Li
2026-07-29 13:57 ` Manivannan Sadhasivam
2026-07-29 16:56 ` Bjorn Helgaas
3 siblings, 0 replies; 8+ messages in thread
From: Frank Li @ 2026-07-14 14:41 UTC (permalink / raw)
To: hongxing.zhu
Cc: frank.li, l.stach, lpieralisi, kwilczynski, mani, robh, bhelgaas,
s.hauer, kernel, festevam, linux-pci, linux-arm-kernel, imx,
linux-kernel, Richard Zhu
On Tue, Jul 14, 2026 at 10:41:07AM +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_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@nxp.com>
> ---
> 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
> +/* 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
> +
not sure why hex value have not align to previous line, maybe patch
show problem. Please make sure it is algin in code.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> #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);
> + 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 +310,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_BW_IN,
> + IMX95_PCIE_PHY_MPLLB_BW_VAL);
> + imx95_pcie_phy_write(imx_pcie, IMX95_PCIE_PHY_MPLLB_OVRD_IN,
> + IMX95_PCIE_PHY_MPLLB_OVRD_BW_EN);
> +
> return 0;
> }
>
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3] PCI: imx6: Update MPLLB bandwidth to improve i.MX95 Gen3 PCIe stability
2026-07-14 2:41 [PATCH v3] PCI: imx6: Update MPLLB bandwidth to improve i.MX95 Gen3 PCIe stability hongxing.zhu
2026-07-14 2:53 ` sashiko-bot
2026-07-14 14:41 ` Frank Li
@ 2026-07-29 13:57 ` Manivannan Sadhasivam
2026-07-30 7:07 ` Hongxing Zhu (OSS)
2026-07-29 16:56 ` Bjorn Helgaas
3 siblings, 1 reply; 8+ messages in thread
From: Manivannan Sadhasivam @ 2026-07-29 13:57 UTC (permalink / raw)
To: hongxing.zhu
Cc: frank.li, l.stach, lpieralisi, kwilczynski, robh, bhelgaas,
s.hauer, kernel, festevam, linux-pci, linux-arm-kernel, imx,
linux-kernel, Richard Zhu
On Tue, Jul 14, 2026 at 10:41:07AM +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_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@nxp.com>
> ---
> 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.
> +/* 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?
> +
> #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?
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 8+ messages in thread* RE: [PATCH v3] PCI: imx6: Update MPLLB bandwidth to improve i.MX95 Gen3 PCIe stability
2026-07-29 13:57 ` Manivannan Sadhasivam
@ 2026-07-30 7:07 ` Hongxing Zhu (OSS)
2026-07-30 7:16 ` Manivannan Sadhasivam
0 siblings, 1 reply; 8+ messages in thread
From: Hongxing Zhu (OSS) @ 2026-07-30 7:07 UTC (permalink / raw)
To: Manivannan Sadhasivam, Hongxing Zhu (OSS)
Cc: Frank Li, l.stach@pengutronix.de, lpieralisi@kernel.org,
kwilczynski@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
> -----Original Message-----
> From: Manivannan Sadhasivam <mani@kernel.org>
> Sent: Wednesday, July 29, 2026 9:58 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; 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 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@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_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@nxp.com>
> > ---
> > 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.
> > +
> > #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.
Best Regards
Richard Zhu>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3] PCI: imx6: Update MPLLB bandwidth to improve i.MX95 Gen3 PCIe stability
2026-07-30 7:07 ` Hongxing Zhu (OSS)
@ 2026-07-30 7:16 ` Manivannan Sadhasivam
0 siblings, 0 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2026-07-30 7:16 UTC (permalink / raw)
To: Hongxing Zhu (OSS)
Cc: Frank Li, l.stach@pengutronix.de, lpieralisi@kernel.org,
kwilczynski@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
On Thu, Jul 30, 2026 at 07:07:58AM +0000, Hongxing Zhu (OSS) wrote:
> > -----Original Message-----
> > From: Manivannan Sadhasivam <mani@kernel.org>
> > Sent: Wednesday, July 29, 2026 9:58 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; 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 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@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_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@nxp.com>
> > > ---
> > > 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
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] PCI: imx6: Update MPLLB bandwidth to improve i.MX95 Gen3 PCIe stability
2026-07-14 2:41 [PATCH v3] PCI: imx6: Update MPLLB bandwidth to improve i.MX95 Gen3 PCIe stability hongxing.zhu
` (2 preceding siblings ...)
2026-07-29 13:57 ` Manivannan Sadhasivam
@ 2026-07-29 16:56 ` Bjorn Helgaas
2026-07-30 7:08 ` Hongxing Zhu (OSS)
3 siblings, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2026-07-29 16:56 UTC (permalink / raw)
To: hongxing.zhu
Cc: frank.li, l.stach, lpieralisi, kwilczynski, mani, robh, bhelgaas,
s.hauer, kernel, festevam, linux-pci, linux-arm-kernel, imx,
linux-kernel, Richard Zhu
On Tue, Jul 14, 2026 at 10:41:07AM +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_BANDWIDTH value. This margin degradation worsens
> across voltage and temperature (VT) variations and different test
> matrices, potentially causing link stability issues.
What does "bandwidth marginality" mean? And what does it mean to
observe it? I guess it must mean some measurable thing is close to
being out of spec?
> 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
AFAICT we don't know what MPLLB_BW_OVRD_IN was before, so we don't
know whether this increases or decreases the value. And we don't
really even know what MPLLB_BW_OVRD_IN specifies.
> This ensures robust PCIe Gen3 performance across all operating
> conditions.
>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
> 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
> +/* 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
> +
> #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);
> + 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 +310,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_BW_IN,
> + IMX95_PCIE_PHY_MPLLB_BW_VAL);
> + imx95_pcie_phy_write(imx_pcie, IMX95_PCIE_PHY_MPLLB_OVRD_IN,
> + IMX95_PCIE_PHY_MPLLB_OVRD_BW_EN);
> +
> return 0;
> }
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread* RE: [PATCH v3] PCI: imx6: Update MPLLB bandwidth to improve i.MX95 Gen3 PCIe stability
2026-07-29 16:56 ` Bjorn Helgaas
@ 2026-07-30 7:08 ` Hongxing Zhu (OSS)
0 siblings, 0 replies; 8+ messages in thread
From: Hongxing Zhu (OSS) @ 2026-07-30 7:08 UTC (permalink / raw)
To: Bjorn Helgaas, Hongxing Zhu (OSS)
Cc: Frank Li, 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
> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Thursday, July 30, 2026 12:57 AM
> 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 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@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_BANDWIDTH value. This margin degradation worsens
> > across voltage and temperature (VT) variations and different test
> > matrices, potentially causing link stability issues.
>
> What does "bandwidth marginality" mean? And what does it mean to observe it?
> I guess it must mean some measurable thing is close to being out of spec?
>
Yes, your assumption is correct.
The MPLLB_BW_OVRD_IN register provides an override value for MPLLB_BW (MPLL
Bandwidth) parameter in the i.MX95 PCIe PHY.
The default MPLLB_BW value is 120. During Gen3 PCIe testing, this default
value showed insufficient bandwidth margin, with further degradation observed
across voltage and temperature (VT) variations. Based on IC designer
recommendations, updating this value from 120 to 140 (0x8c) significantly
improves bandwidth margins across all VT conditions and test scenarios,
ensuring stable PCIe link operation.
> > 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
>
> AFAICT we don't know what MPLLB_BW_OVRD_IN was before, so we don't know
> whether this increases or decreases the value. And we don't really even know
> what MPLLB_BW_OVRD_IN specifies.
>
This patch increases the MPLLB_BW value from its default of 120 to 140 as
recommended by the IC designers.
Best Regards
Richard Zhu
> > This ensures robust PCIe Gen3 performance across all operating
> > conditions.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> > 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
> > +/* 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
> > +
> > #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);
> > + 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 +310,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_BW_IN,
> > + IMX95_PCIE_PHY_MPLLB_BW_VAL);
> > + imx95_pcie_phy_write(imx_pcie, IMX95_PCIE_PHY_MPLLB_OVRD_IN,
> > + IMX95_PCIE_PHY_MPLLB_OVRD_BW_EN);
> > +
> > return 0;
> > }
> >
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 8+ messages in thread