[PATCH 0/3] clk: at91: sckc: Add macros for the slow clock

[PATCH 0/3] clk: at91: sckc: Add macros for the slow clock

[Claudiu Beznea]

Hi,

Previously raw numbers were used for the clocks generated by the slow
clock controller. This generated confusion and wrong IDs were used on
device tree. To make it simpler and less error-prone series add IDs.

Thank you,
Claudiu Beznea

Claudiu Beznea (3):
  dt-bindings: clk: at91: Add clock IDs for the slow clock controller
  clk: at91: sckc: Use SCKC_{TD, MD}_SLCK IDs for clk32k clocks
  ARM: dts: microchip: Use SCKC_{TD, MD}_SLCK IDs for clk32k clocks

 arch/arm/boot/dts/microchip/sam9x60.dtsi | 18 +++++++++---------
 arch/arm/boot/dts/microchip/sama7g5.dtsi | 16 ++++++++--------
 drivers/clk/at91/sckc.c                  | 24 +++++++++++++-----------
 include/dt-bindings/clock/at91.h         |  4 ++++
 4 files changed, 34 insertions(+), 28 deletions(-)

-- 
2.39.2

[PATCH 1/3] dt-bindings: clk: at91: Add clock IDs for the slow clock controller

[Claudiu Beznea]

Add clock IDs for the slow clock controller. Previously, raw numbers
were used (0 or 1) for clocks generated by the slow clock controller. This
leads to confusion and wrong IDs were used on few device trees. To avoid
this add macros.

Signed-off-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>
---
 include/dt-bindings/clock/at91.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/dt-bindings/clock/at91.h b/include/dt-bindings/clock/at91.h
index 6ede88c3992d..99f4767ff6bb 100644
--- a/include/dt-bindings/clock/at91.h
+++ b/include/dt-bindings/clock/at91.h
@@ -55,4 +55,8 @@
 #define AT91_PMC_GCKRDY		24		/* Generated Clocks */
 #endif
 
+/* Slow clock. */
+#define SCKC_MD_SLCK		0
+#define SCKC_TD_SLCK		1
+
 #endif
-- 
2.39.2

[PATCH 2/3] clk: at91: sckc: Use SCKC_{TD, MD}_SLCK IDs for clk32k clocks

[Claudiu Beznea]

Use the newly introduced macros instead of raw numbers. With this the code
is a bit easier to understand.

Signed-off-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>
---
 drivers/clk/at91/sckc.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/at91/sckc.c b/drivers/clk/at91/sckc.c
index 7741d8f3dbee..021d1b412af4 100644
--- a/drivers/clk/at91/sckc.c
+++ b/drivers/clk/at91/sckc.c
@@ -12,6 +12,8 @@
 #include <linux/of_address.h>
 #include <linux/io.h>
 
+#include <dt-bindings/clock/at91.h>
+
 #define SLOW_CLOCK_FREQ		32768
 #define SLOWCK_SW_CYCLES	5
 #define SLOWCK_SW_TIME_USEC	((SLOWCK_SW_CYCLES * USEC_PER_SEC) / \
@@ -470,7 +472,7 @@ static void __init of_sam9x60_sckc_setup(struct device_node *np)
 {
 	void __iomem *regbase = of_iomap(np, 0);
 	struct clk_hw_onecell_data *clk_data;
-	struct clk_hw *slow_rc, *slow_osc;
+	struct clk_hw *slow_rc, *slow_osc, *hw;
 	const char *xtal_name;
 	const struct clk_hw *parent_hws[2];
 	static struct clk_parent_data parent_data = {
@@ -506,19 +508,19 @@ static void __init of_sam9x60_sckc_setup(struct device_node *np)
 
 	/* MD_SLCK and TD_SLCK. */
 	clk_data->num = 2;
-	clk_data->hws[0] = clk_hw_register_fixed_rate_parent_hw(NULL, "md_slck",
-								slow_rc,
-								0, 32768);
-	if (IS_ERR(clk_data->hws[0]))
+	hw = clk_hw_register_fixed_rate_parent_hw(NULL, "md_slck", slow_rc,
+						  0, 32768);
+	if (IS_ERR(hw))
 		goto clk_data_free;
+	clk_data->hws[SCKC_MD_SLCK] = hw;
 
 	parent_hws[0] = slow_rc;
 	parent_hws[1] = slow_osc;
-	clk_data->hws[1] = at91_clk_register_sam9x5_slow(regbase, "td_slck",
-							 parent_hws, 2,
-							 &at91sam9x60_bits);
-	if (IS_ERR(clk_data->hws[1]))
+	hw = at91_clk_register_sam9x5_slow(regbase, "td_slck", parent_hws,
+					   2, &at91sam9x60_bits);
+	if (IS_ERR(hw))
 		goto unregister_md_slck;
+	clk_data->hws[SCKC_TD_SLCK] = hw;
 
 	ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_data);
 	if (WARN_ON(ret))
@@ -527,9 +529,9 @@ static void __init of_sam9x60_sckc_setup(struct device_node *np)
 	return;
 
 unregister_td_slck:
-	at91_clk_unregister_sam9x5_slow(clk_data->hws[1]);
+	at91_clk_unregister_sam9x5_slow(clk_data->hws[SCKC_TD_SLCK]);
 unregister_md_slck:
-	clk_hw_unregister(clk_data->hws[0]);
+	clk_hw_unregister(clk_data->hws[SCKC_MD_SLCK]);
 clk_data_free:
 	kfree(clk_data);
 unregister_slow_osc:
-- 
2.39.2

[PATCH 3/3] ARM: dts: microchip: Use SCKC_{TD, MD}_SLCK IDs for clk32k clocks

[Claudiu Beznea]

Use the newly introduced macros instead of raw number. With this device
tree code is a bit easier to understand.

Signed-off-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>
---
 arch/arm/boot/dts/microchip/sam9x60.dtsi | 18 +++++++++---------
 arch/arm/boot/dts/microchip/sama7g5.dtsi | 16 ++++++++--------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi
index 04a6d716ecaf..eeda277e684f 100644
--- a/arch/arm/boot/dts/microchip/sam9x60.dtsi
+++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi
@@ -560,7 +560,7 @@ tcb0: timer@f8008000 {
 				#size-cells = <0>;
 				reg = <0xf8008000 0x100>;
 				interrupts = <17 IRQ_TYPE_LEVEL_HIGH 0>;
-				clocks = <&pmc PMC_TYPE_PERIPHERAL 17>, <&clk32k 0>;
+				clocks = <&pmc PMC_TYPE_PERIPHERAL 17>, <&clk32k SCKC_MD_SLCK>;
 				clock-names = "t0_clk", "slow_clk";
 			};
 
@@ -570,7 +570,7 @@ tcb1: timer@f800c000 {
 				#size-cells = <0>;
 				reg = <0xf800c000 0x100>;
 				interrupts = <45 IRQ_TYPE_LEVEL_HIGH 0>;
-				clocks = <&pmc PMC_TYPE_PERIPHERAL 45>, <&clk32k 0>;
+				clocks = <&pmc PMC_TYPE_PERIPHERAL 45>, <&clk32k SCKC_MD_SLCK>;
 				clock-names = "t0_clk", "slow_clk";
 			};
 
@@ -1038,7 +1038,7 @@ hlcdc: hlcdc@f8038000 {
 				compatible = "microchip,sam9x60-hlcdc";
 				reg = <0xf8038000 0x4000>;
 				interrupts = <25 IRQ_TYPE_LEVEL_HIGH 0>;
-				clocks = <&pmc PMC_TYPE_PERIPHERAL 25>, <&pmc PMC_TYPE_GCK 25>, <&clk32k 1>;
+				clocks = <&pmc PMC_TYPE_PERIPHERAL 25>, <&pmc PMC_TYPE_GCK 25>, <&clk32k SCKC_TD_SLCK>;
 				clock-names = "periph_clk","sys_clk", "slow_clk";
 				assigned-clocks = <&pmc PMC_TYPE_GCK 25>;
 				assigned-clock-parents = <&pmc PMC_TYPE_CORE PMC_MCK>;
@@ -1313,20 +1313,20 @@ pmc: clock-controller@fffffc00 {
 				reg = <0xfffffc00 0x200>;
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
 				#clock-cells = <2>;
-				clocks = <&clk32k 1>, <&clk32k 0>, <&main_xtal>;
+				clocks = <&clk32k SCKC_TD_SLCK>, <&clk32k SCKC_MD_SLCK>, <&main_xtal>;
 				clock-names = "td_slck", "md_slck", "main_xtal";
 			};
 
 			reset_controller: reset-controller@fffffe00 {
 				compatible = "microchip,sam9x60-rstc";
 				reg = <0xfffffe00 0x10>;
-				clocks = <&clk32k 0>;
+				clocks = <&clk32k SCKC_MD_SLCK>;
 			};
 
 			shutdown_controller: poweroff@fffffe10 {
 				compatible = "microchip,sam9x60-shdwc";
 				reg = <0xfffffe10 0x10>;
-				clocks = <&clk32k 0>;
+				clocks = <&clk32k SCKC_MD_SLCK>;
 				#address-cells = <1>;
 				#size-cells = <0>;
 				atmel,wakeup-rtc-timer;
@@ -1338,7 +1338,7 @@ rtt: rtc@fffffe20 {
 				compatible = "microchip,sam9x60-rtt", "atmel,at91sam9260-rtt";
 				reg = <0xfffffe20 0x20>;
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
-				clocks = <&clk32k 1>;
+				clocks = <&clk32k SCKC_TD_SLCK>;
 			};
 
 			pit: timer@fffffe40 {
@@ -1364,14 +1364,14 @@ rtc: rtc@fffffea8 {
 				compatible = "microchip,sam9x60-rtc", "atmel,at91sam9x5-rtc";
 				reg = <0xfffffea8 0x100>;
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
-				clocks = <&clk32k 1>;
+				clocks = <&clk32k SCKC_TD_SLCK>;
 			};
 
 			watchdog: watchdog@ffffff80 {
 				compatible = "microchip,sam9x60-wdt";
 				reg = <0xffffff80 0x24>;
 				interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
-				clocks = <&clk32k 0>;
+				clocks = <&clk32k SCKC_MD_SLCK>;
 				status = "disabled";
 			};
 		};
diff --git a/arch/arm/boot/dts/microchip/sama7g5.dtsi b/arch/arm/boot/dts/microchip/sama7g5.dtsi
index 17bcdcf0cf4a..2efca9838d69 100644
--- a/arch/arm/boot/dts/microchip/sama7g5.dtsi
+++ b/arch/arm/boot/dts/microchip/sama7g5.dtsi
@@ -246,7 +246,7 @@ pmc: clock-controller@e0018000 {
 			reg = <0xe0018000 0x200>;
 			interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
 			#clock-cells = <2>;
-			clocks = <&clk32k 1>, <&clk32k 0>, <&main_xtal>;
+			clocks = <&clk32k SCKC_TD_SLCK>, <&clk32k SCKC_MD_SLCK>, <&main_xtal>;
 			clock-names = "td_slck", "md_slck", "main_xtal";
 		};
 
@@ -254,13 +254,13 @@ reset_controller: reset-controller@e001d000 {
 			compatible = "microchip,sama7g5-rstc";
 			reg = <0xe001d000 0xc>, <0xe001d0e4 0x4>;
 			#reset-cells = <1>;
-			clocks = <&clk32k 0>;
+			clocks = <&clk32k SCKC_MD_SLCK>;
 		};
 
 		shdwc: poweroff@e001d010 {
 			compatible = "microchip,sama7g5-shdwc", "syscon";
 			reg = <0xe001d010 0x10>;
-			clocks = <&clk32k 0>;
+			clocks = <&clk32k SCKC_MD_SLCK>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			atmel,wakeup-rtc-timer;
@@ -272,7 +272,7 @@ rtt: rtc@e001d020 {
 			compatible = "microchip,sama7g5-rtt", "microchip,sam9x60-rtt", "atmel,at91sam9260-rtt";
 			reg = <0xe001d020 0x30>;
 			interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clk32k 1>;
+			clocks = <&clk32k SCKC_TD_SLCK>;
 		};
 
 		clk32k: clock-controller@e001d050 {
@@ -291,14 +291,14 @@ rtc: rtc@e001d0a8 {
 			compatible = "microchip,sama7g5-rtc", "microchip,sam9x60-rtc";
 			reg = <0xe001d0a8 0x30>;
 			interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clk32k 1>;
+			clocks = <&clk32k SCKC_TD_SLCK>;
 		};
 
 		ps_wdt: watchdog@e001d180 {
 			compatible = "microchip,sama7g5-wdt";
 			reg = <0xe001d180 0x24>;
 			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clk32k 0>;
+			clocks = <&clk32k SCKC_MD_SLCK>;
 		};
 
 		chipid@e0020000 {
@@ -312,7 +312,7 @@ tcb1: timer@e0800000 {
 			#size-cells = <0>;
 			reg = <0xe0800000 0x100>;
 			interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&pmc PMC_TYPE_PERIPHERAL 91>, <&pmc PMC_TYPE_PERIPHERAL 92>, <&pmc PMC_TYPE_PERIPHERAL 93>, <&clk32k 1>;
+			clocks = <&pmc PMC_TYPE_PERIPHERAL 91>, <&pmc PMC_TYPE_PERIPHERAL 92>, <&pmc PMC_TYPE_PERIPHERAL 93>, <&clk32k SCKC_TD_SLCK>;
 			clock-names = "t0_clk", "t1_clk", "t2_clk", "slow_clk";
 		};
 
@@ -906,7 +906,7 @@ tcb0: timer@e2814000 {
 			#size-cells = <0>;
 			reg = <0xe2814000 0x100>;
 			interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&pmc PMC_TYPE_PERIPHERAL 88>, <&pmc PMC_TYPE_PERIPHERAL 89>, <&pmc PMC_TYPE_PERIPHERAL 90>, <&clk32k 1>;
+			clocks = <&pmc PMC_TYPE_PERIPHERAL 88>, <&pmc PMC_TYPE_PERIPHERAL 89>, <&pmc PMC_TYPE_PERIPHERAL 90>, <&clk32k SCKC_TD_SLCK>;
 			clock-names = "t0_clk", "t1_clk", "t2_clk", "slow_clk";
 		};
 
-- 
2.39.2

Re: [PATCH 3/3] ARM: dts: microchip: Use SCKC_{TD, MD}_SLCK IDs for clk32k clocks

[claudiu beznea]

On 26.08.2024 20:31, Claudiu Beznea wrote:
> Use the newly introduced macros instead of raw number. With this device
> tree code is a bit easier to understand.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> ---
>  arch/arm/boot/dts/microchip/sam9x60.dtsi | 18 +++++++++---------
>  arch/arm/boot/dts/microchip/sama7g5.dtsi | 16 ++++++++--------
>  2 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi
> index 04a6d716ecaf..eeda277e684f 100644
> --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi
> +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi
> @@ -560,7 +560,7 @@ tcb0: timer@f8008000 {
>  				#size-cells = <0>;
>  				reg = <0xf8008000 0x100>;
>  				interrupts = <17 IRQ_TYPE_LEVEL_HIGH 0>;
> -				clocks = <&pmc PMC_TYPE_PERIPHERAL 17>, <&clk32k 0>;
> +				clocks = <&pmc PMC_TYPE_PERIPHERAL 17>, <&clk32k SCKC_MD_SLCK>;

Actually, looking again at it, I don't know if it worth as we use numbers
directly also for other PMC clock IDs.

Sorry for the noise,
Claudiu Beznea

Re: [PATCH 1/3] dt-bindings: clk: at91: Add clock IDs for the slow clock controller

[Krzysztof Kozlowski]

On Mon, Aug 26, 2024 at 08:31:14PM +0300, Claudiu Beznea wrote:
> Add clock IDs for the slow clock controller. Previously, raw numbers
> were used (0 or 1) for clocks generated by the slow clock controller. This
> leads to confusion and wrong IDs were used on few device trees. To avoid
> this add macros.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> ---
>  include/dt-bindings/clock/at91.h | 4 ++++

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 3/3] ARM: dts: microchip: Use SCKC_{TD, MD}_SLCK IDs for clk32k clocks

[Alexander Dahl]

Hello Claudiu,

Am Mon, Aug 26, 2024 at 08:42:10PM +0300 schrieb claudiu beznea:
> 
> 
> On 26.08.2024 20:31, Claudiu Beznea wrote:
> > Use the newly introduced macros instead of raw number. With this device
> > tree code is a bit easier to understand.
> > 
> > Signed-off-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> > ---
> >  arch/arm/boot/dts/microchip/sam9x60.dtsi | 18 +++++++++---------
> >  arch/arm/boot/dts/microchip/sama7g5.dtsi | 16 ++++++++--------
> >  2 files changed, 17 insertions(+), 17 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi
> > index 04a6d716ecaf..eeda277e684f 100644
> > --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi
> > +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi
> > @@ -560,7 +560,7 @@ tcb0: timer@f8008000 {
> >  				#size-cells = <0>;
> >  				reg = <0xf8008000 0x100>;
> >  				interrupts = <17 IRQ_TYPE_LEVEL_HIGH 0>;
> > -				clocks = <&pmc PMC_TYPE_PERIPHERAL 17>, <&clk32k 0>;
> > +				clocks = <&pmc PMC_TYPE_PERIPHERAL 17>, <&clk32k SCKC_MD_SLCK>;
> 
> Actually, looking again at it, I don't know if it worth as we use numbers
> directly also for other PMC clock IDs.

I think in this case it is worth it.  The macros you added are more
like the already existing PMC_MCK et al. macros for PMC_TYPE_CORE and
do essentially the same thing in driver code working as somewhat
arbitrary array index, without relation to SoC internals.

The PMC clock IDs on the other hand are for PMC_TYPE_PERIPHERAL and
are that long list in the Peripheral Identifiers table and correspond
to the SoC internal IDs, which are not used in the same way.

So from my point of view, the patch series is valuable and should be
further worked on.

Greets
Alex

> Sorry for the noise,
> Claudiu Beznea
> 
> 
> 

Re: [PATCH 2/3] clk: at91: sckc: Use SCKC_{TD, MD}_SLCK IDs for clk32k clocks

[Alexander Dahl]

Hello Claudiu,

Am Mon, Aug 26, 2024 at 08:31:15PM +0300 schrieb Claudiu Beznea:
> Use the newly introduced macros instead of raw numbers. With this the code
> is a bit easier to understand.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> ---
>  drivers/clk/at91/sckc.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/clk/at91/sckc.c b/drivers/clk/at91/sckc.c
> index 7741d8f3dbee..021d1b412af4 100644
> --- a/drivers/clk/at91/sckc.c
> +++ b/drivers/clk/at91/sckc.c
> @@ -12,6 +12,8 @@
>  #include <linux/of_address.h>
>  #include <linux/io.h>
>  
> +#include <dt-bindings/clock/at91.h>
> +
>  #define SLOW_CLOCK_FREQ		32768
>  #define SLOWCK_SW_CYCLES	5
>  #define SLOWCK_SW_TIME_USEC	((SLOWCK_SW_CYCLES * USEC_PER_SEC) / \
> @@ -470,7 +472,7 @@ static void __init of_sam9x60_sckc_setup(struct device_node *np)
>  {
>  	void __iomem *regbase = of_iomap(np, 0);
>  	struct clk_hw_onecell_data *clk_data;
> -	struct clk_hw *slow_rc, *slow_osc;
> +	struct clk_hw *slow_rc, *slow_osc, *hw;
>  	const char *xtal_name;
>  	const struct clk_hw *parent_hws[2];
>  	static struct clk_parent_data parent_data = {
> @@ -506,19 +508,19 @@ static void __init of_sam9x60_sckc_setup(struct device_node *np)
>  
>  	/* MD_SLCK and TD_SLCK. */
>  	clk_data->num = 2;
> -	clk_data->hws[0] = clk_hw_register_fixed_rate_parent_hw(NULL, "md_slck",
> -								slow_rc,
> -								0, 32768);
> -	if (IS_ERR(clk_data->hws[0]))
> +	hw = clk_hw_register_fixed_rate_parent_hw(NULL, "md_slck", slow_rc,
> +						  0, 32768);
> +	if (IS_ERR(hw))
>  		goto clk_data_free;
> +	clk_data->hws[SCKC_MD_SLCK] = hw;
>  
>  	parent_hws[0] = slow_rc;
>  	parent_hws[1] = slow_osc;
> -	clk_data->hws[1] = at91_clk_register_sam9x5_slow(regbase, "td_slck",
> -							 parent_hws, 2,
> -							 &at91sam9x60_bits);
> -	if (IS_ERR(clk_data->hws[1]))
> +	hw = at91_clk_register_sam9x5_slow(regbase, "td_slck", parent_hws,
> +					   2, &at91sam9x60_bits);
> +	if (IS_ERR(hw))
>  		goto unregister_md_slck;
> +	clk_data->hws[SCKC_TD_SLCK] = hw;
>  
>  	ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_data);
>  	if (WARN_ON(ret))
> @@ -527,9 +529,9 @@ static void __init of_sam9x60_sckc_setup(struct device_node *np)
>  	return;
>  
>  unregister_td_slck:
> -	at91_clk_unregister_sam9x5_slow(clk_data->hws[1]);
> +	at91_clk_unregister_sam9x5_slow(clk_data->hws[SCKC_TD_SLCK]);
>  unregister_md_slck:
> -	clk_hw_unregister(clk_data->hws[0]);
> +	clk_hw_unregister(clk_data->hws[SCKC_MD_SLCK]);
>  clk_data_free:
>  	kfree(clk_data);
>  unregister_slow_osc:

Reviewed-by: Alexander Dahl <ada@thorsis.com>

Thanks and Greets
Alex

Re: [PATCH 3/3] ARM: dts: microchip: Use SCKC_{TD, MD}_SLCK IDs for clk32k clocks

[Alexandre Belloni]

On 28/08/2024 09:07:05+0200, Alexander Dahl wrote:
> Hello Claudiu,
> 
> Am Mon, Aug 26, 2024 at 08:42:10PM +0300 schrieb claudiu beznea:
> > 
> > 
> > On 26.08.2024 20:31, Claudiu Beznea wrote:
> > > Use the newly introduced macros instead of raw number. With this device
> > > tree code is a bit easier to understand.
> > > 
> > > Signed-off-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> > > ---
> > >  arch/arm/boot/dts/microchip/sam9x60.dtsi | 18 +++++++++---------
> > >  arch/arm/boot/dts/microchip/sama7g5.dtsi | 16 ++++++++--------
> > >  2 files changed, 17 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/dts/microchip/sam9x60.dtsi
> > > index 04a6d716ecaf..eeda277e684f 100644
> > > --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi
> > > +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi
> > > @@ -560,7 +560,7 @@ tcb0: timer@f8008000 {
> > >  				#size-cells = <0>;
> > >  				reg = <0xf8008000 0x100>;
> > >  				interrupts = <17 IRQ_TYPE_LEVEL_HIGH 0>;
> > > -				clocks = <&pmc PMC_TYPE_PERIPHERAL 17>, <&clk32k 0>;
> > > +				clocks = <&pmc PMC_TYPE_PERIPHERAL 17>, <&clk32k SCKC_MD_SLCK>;
> > 
> > Actually, looking again at it, I don't know if it worth as we use numbers
> > directly also for other PMC clock IDs.
> 
> I think in this case it is worth it.  The macros you added are more
> like the already existing PMC_MCK et al. macros for PMC_TYPE_CORE and
> do essentially the same thing in driver code working as somewhat
> arbitrary array index, without relation to SoC internals.
> 
> The PMC clock IDs on the other hand are for PMC_TYPE_PERIPHERAL and
> are that long list in the Peripheral Identifiers table and correspond
> to the SoC internal IDs, which are not used in the same way.
> 
> So from my point of view, the patch series is valuable and should be
> further worked on.
> 

I agree with this.

> Greets
> Alex
> 
> > Sorry for the noise,
> > Claudiu Beznea
> > 
> > 
> > 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com