Re: [PATCH 3/3] arm64: dts: imx8mq: Declare the PCIe extref clock

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The requirement for a new 'extref' clock name breaks PCIe on older device trees for i.MX8MQ boards that rely on an external oscillator.
- [High] PCIe endpoint nodes on boards with external oscillators are intentionally omitted from the 'extref' clock update, which will misconfigure the PHY to use the internal PLL and break the link when endpoint mode is enabled.
--

--- Patch [3]: [PATCH 3/3] arm64: dts: imx8mq: Declare the PCIe extref clock ---
commit ad57d8a5f5ec29362d83c5dccc786006a9f4c2cb
Author: Rudi Heitbaum <[email protected]>

arm64: dts: imx8mq: Declare the PCIe extref clock

This commit updates i.MX8MQ board device trees to explicitly define the
"extref" clock for PCIe nodes, keeping "pcie_bus" entries intact. The
change aligns the device trees with a driver update that relies on the
"extref" clock name to configure the external oscillator.

> arm64: dts: imx8mq: Declare the PCIe extref clock
>     
> Boards feeding PCIe REF_CLK from an off-chip oscillator described it as
> the "pcie_bus" clock, which the driver never looks up by name and which
> therefore selects nothing. The oscillator was used only because the
> driver forced IMX8MQ_GPR_PCIE_REF_USE_PAD unconditionally.
>     
> Now that the selection follows the "extref" clock, name the oscillator
> explicitly on every board that depends on it so the behaviour is
> unchanged. The "pcie_bus" entries are left alone.

[Severity: Critical]
Does this break backward compatibility for older device trees?

If a user upgrades to a kernel with the new driver behavior but boots with an
older DTB that lacks the newly introduced "extref" clock name, will the driver
in drivers/pci/controller/dwc/pci-imx6.c:imx8mq_pcie_init_phy() clear the
IMX8MQ_GPR_PCIE_REF_USE_PAD bit because "extref" is absent?

Device trees represent a stable ABI, and older deployed DTBs might lose PCIe
functionality if they rely on the external oscillator and this bit is cleared.

> Endpoint nodes are not touched: they are all disabled and use a separate
> binding.

[Severity: High]
Since endpoint mode initialization also goes through the same PHY setup
routine, will omitting the "extref" clock from endpoint nodes misconfigure
the PHY for endpoint operations?

If a user enables the pcie0_ep or pcie1_ep node on an affected board like the
EVK, the driver will not find the "extref" clock and imx8mq_pcie_init_phy()
will incorrectly clear IMX8MQ_GPR_PCIE_REF_USE_PAD.

Does this mean the PHY will be misconfigured to use the internal PLL instead
of the required external oscillator, breaking the link when endpoint mode is
enabled?

> Signed-off-by: Rudi Heitbaum <[email protected]>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3