Re: [PATCH 1/3] dt-bindings: clock: s32g: add uSDHC clock IDs

On 1/22/24 12:09, Ghennadi Procopciuc wrote:

On 1/21/24 15:32, Conor Dooley wrote:

quoted

On Fri, Jan 19, 2024 at 11:25:57PM +0200, Ghennadi Procopciuc wrote:

quoted

On 1/19/24 18:14, Conor Dooley wrote:

quoted

On Fri, Jan 19, 2024 at 04:11:37PM +0000, Conor Dooley wrote:

quoted

On Fri, Jan 19, 2024 at 03:02:28PM +0200, Ghennadi Procopciuc (OSS) wrote:

quoted

From: Ghennadi Procopciuc <redacted>

Add the SCMI clock IDs for the uSDHC controller present on
S32G SoCs.

Signed-off-by: Ciprian Costea <redacted>
Signed-off-by: Ghennadi Procopciuc <redacted>
---
 include/dt-bindings/clock/s32g-scmi-clock.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 include/dt-bindings/clock/s32g-scmi-clock.h

diff --git a/include/dt-bindings/clock/s32g-scmi-clock.h b/include/dt-bindings/clock/s32g-scmi-clock.h
new file mode 100644
index 000000000000..739f98a924c3
--- /dev/null
+++ b/include/dt-bindings/clock/s32g-scmi-clock.h

@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */
+/*
+ * Copyright 2020-2024 NXP
+ */
+#ifndef _DT_BINDINGS_SCMI_CLK_S32G_H
+#define _DT_BINDINGS_SCMI_CLK_S32G_H
+
+/* uSDHC */
+#define S32G_SCMI_CLK_USDHC_AHB		31
+#define S32G_SCMI_CLK_USDHC_MODULE	32
+#define S32G_SCMI_CLK_USDHC_CORE	33
+#define S32G_SCMI_CLK_USDHC_MOD32K	34

Why do these numbers not start at 0?

Ah, because these are the SCMI IDs directly. If these are numbers that
are in the TRM, just use the numbers directly - there's no need to
create bindings for that.

Hi Conor,

I appreciate you taking the time to review the proposed changes. I
wanted to clarify that the IDs mentioned in the header are SCMI IDs
exported by the TF-A and are utilized by the second patch of this
series. These IDs are for the uSDHC controller to control its clocks. As
other SoCs use this model, I have included all the necessary IDs in a
dedicated header file:
- rk3588s     (arch/arm64/boot/dts/rockchip/rk3588s.dtsi:97 [0])
- stm32mp157c (arch/arm/boot/dts/st/stm32mp157c-ed1-scmi.dts:73 [1])
- stm32mp131  (arch/arm/boot/dts/st/stm32mp131.dtsi:1372 [2])

Should I remove the header and use raw numbers in the uSDHC node?

IMO, yes. There's no abstraction/binding being created here if they're
the SCMI IDs.

I included the suggestion in the second version of this patchset.

quoted

Thanks,
conor.

Thank you for letting me know. I will make sure to include this change
in the second version of the patchset.

quoted

quoted

For
example:

quoted

+		usdhc0: mmc@402f0000 {
+			compatible = "nxp,s32g2-usdhc";
+			reg = <0x402f0000 0x1000>;
+			interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clks 32>,
+				 <&clks 31>,
+				 <&clks 33>;
+			clock-names = "ipg", "ahb", "per";
+			bus-width = <8>;
+			status = "disabled";
+		};

[0]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588s.dtsi#n97
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/st/stm32mp157c-ed1-scmi.dts#n73
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/st/stm32mp131.dtsi#n1372

-- 
Regards,
Ghennadi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel