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 o=
lder device trees for i.MX8MQ boards that rely on an external oscillator.
- [High] PCIe endpoint nodes on boards with external oscillators are intent=
ionally omitted from the 'extref' clock update, which will misconfigure the=
 PHY to use the internal PLL and break the link when endpoint mode is enabl=
ed.
--

--- Patch [3]: [PATCH 3/3] arm64: dts: imx8mq: Declare the PCIe extref cloc=
k ---
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
>    =20
> 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.
>    =20
> 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 dri=
ver
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 clear=
ed.

> 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 t=
he
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]>

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/cover.1785669457.gi=
[email protected]?part=3D3