diff mbox series

[v6,10/14] arm64: dts: freescale: imx8qxp: Remove unnecessary clock related entries

Message ID 20220629164414.301813-11-viorel.suman@oss.nxp.com
State Superseded
Headers show
Series dt-bindings: arm: freescale: Switch fsl,scu from txt to yaml | expand

Commit Message

Viorel Suman (OSS) June 29, 2022, 4:44 p.m. UTC 
From: Viorel Suman <viorel.suman@nxp.com>

"clocks" and "clock-names" are not used the driver, so
remove them in order to match the yaml definition.

Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 2 --
 1 file changed, 2 deletions(-)

Comments

Viorel Suman June 30, 2022, 8:36 a.m. UTC | #1 
On 22-06-29 20:04:43, Krzysztof Kozlowski wrote:
> On 29/06/2022 18:44, Viorel Suman (OSS) wrote:
> > From: Viorel Suman <viorel.suman@nxp.com>
> > 
> > "clocks" and "clock-names" are not used the driver, so
> > remove them in order to match the yaml definition.
> 
> So this explains the unexpected change in the bindings... but actually
> it does not explain whether it is correct or not. Just because driver
> does not use it, is not a proof that clocks are not there. In different
> OS/implementation this DTS might break stuff, so basically it is ABI
> break. DTS should describe the hardware fully, so if the clocks are
> there, should be in DTS regardless of the driver.

Hi Krzysztof,

Both XTAL clocks - 24MHz and 32kHz - are still defined in DTSI files, see for instance in
arch/arm64/boot/dts/freescale/imx8qxp.dtsi :
---------------
        xtal32k: clock-xtal32k {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <32768>;
                clock-output-names = "xtal_32KHz";
        };

        xtal24m: clock-xtal24m {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <24000000>;
                clock-output-names = "xtal_24MHz";
        };
---------------
Both can be seen in /sys/kernel/debug/clk/clk_summary once boot is complete, both can be referenced
in any DTS node, so there is no ABI break.

"DTS should describe the hardware fully" - this is true in case the OS is supposed to controll the
hardware fully. i.MX8 System Controller Unit concept implies resources being allocated and managed
by SCU, there is no direct OS access to some hardware. SCU actually defines the hardware environment
the OS is being able to see and run within. SCU is able to define several such isolated hardware
environments, each having its own OS running. So, in this particular case - i.MX8 SCU concept -
DTS should describe the hardware from the perspective of the hardware environment exposed by SCU to
OS.

Best regards,
Viorel
Krzysztof Kozlowski July 4, 2022, 8:06 a.m. UTC | #2 
On 30/06/2022 21:48, Viorel Suman wrote:
> 
> The question context looks a bit shifted. The "clocks" and "clock-names"
> attributes are removed from a clock provider device.
> 
> The OS clock provider in this case is a client which uses some protocol
> to communicate with SCU via a messaging unit. There is no
> access to xtal clocks via the existing OS<->SCU communication protocol.

SCU does not need to access them via communication protocol. It's enough
that they are clock inputs, physical clocks being fed to your hardware
which you describe in the DTS.


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 483996a1f2d5..878c2aa663f1 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -218,8 +218,6 @@  pd: imx8qx-pd {
 		clk: clock-controller {
 			compatible = "fsl,imx8qxp-clk";
 			#clock-cells = <2>;
-			clocks = <&xtal32k &xtal24m>;
-			clock-names = "xtal_32KHz", "xtal_24Mhz";
 		};
 
 		iomuxc: pinctrl {