RE: [PATCH 2/3] PCI: imx6: Use the external clock as REF_CLK when needed for i.MX8MQ

> -----Original Message-----
> From: Rudi Heitbaum <[email protected]>
> Sent: Sunday, August 2, 2026 7:32 PM
> To: Hongxing Zhu <[email protected]>; Lucas Stach
> <[email protected]>; Frank Li <[email protected]>; Sascha Hauer
> <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>; Krzysztof Wilczyński
> <[email protected]>; Manivannan Sadhasivam <[email protected]>; Rob
> Herring <[email protected]>; Bjorn Helgaas <[email protected]>;
> Krzysztof Kozlowski <[email protected]>; Conor Dooley
> <[email protected]>; Fabio Estevam <[email protected]>; linux-
> [email protected]; [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]
> Subject: [PATCH 2/3] PCI: imx6: Use the external clock as REF_CLK when
> needed for i.MX8MQ
> 
> i.MX8MQ has two PCIe REF_CLK sources: an off-chip oscillator and the
> internal PLL. The driver unconditionally selected the oscillator, which is what
> the
> 
> 	/* TODO: This code assumes external oscillator is being used */
> 
> comment refers to. Boards wiring an oscillator to the pad describe it as the
> "pcie_bus" clock, but nothing in the driver looks that clock up by name, so it
> does not select anything.
> 
> Reuse the generic enable_ext_refclk flag, set when an "extref" clock is
> supplied, to select between the two, as is already done for i.MX95 in commit
> d8574ce57d76 ("PCI: imx6: Add external reference clock input mode
> support").
> 
> Signed-off-by: Rudi Heitbaum <[email protected]>
Sorry for the late reply.

The code changes look good to me.

However, I'm concerned about backward compatibility. This patch changes the
default REF_CLK source from external oscillator to internal PLL when no
"extref" clock is specified, which could break existing i.MX8MQ boards.

If the dt-binding maintainers approve this ABI change.
Acked-by: Richard Zhu <[email protected]>

Best Regards
Richard Zhu
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> b/drivers/pci/controller/dwc/pci-imx6.c
> index dcdeba1856f7..5a350b458782 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -445,11 +445,16 @@ static int pcie_phy_write(struct imx_pcie *imx_pcie,
> int addr, u16 data)
> 
>  static int imx8mq_pcie_init_phy(struct imx_pcie *imx_pcie)  {
> -	/* TODO: This code assumes external oscillator is being used */
> +	bool ext = imx_pcie->enable_ext_refclk;
> +
> +	/*
> +	 * Select the off-chip oscillator as REF_CLK when an "extref" clock is
> +	 * supplied, otherwise fall back to the internal PLL.
> +	 */
>  	regmap_update_bits(imx_pcie->iomuxc_gpr,
>  			   imx_pcie_grp_offset(imx_pcie),
>  			   IMX8MQ_GPR_PCIE_REF_USE_PAD,
> -			   IMX8MQ_GPR_PCIE_REF_USE_PAD);
> +			   ext ? IMX8MQ_GPR_PCIE_REF_USE_PAD : 0);
>  	/*
>  	 * Per the datasheet, the PCIE_VPH is suggested to be 1.8V.  If the
>  	 * PCIE_VPH is supplied by 3.3V, the VREG_BYPASS should be cleared
> --
> 2.53.0