[PATCH 1/2] dt-bindings: timer: nxp,sysctr-timer: support i.MX95

[PATCH 1/2] dt-bindings: timer: nxp,sysctr-timer: support i.MX95

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

i.MX95 System counter module has similar design as i.MX93, so add
fallback compatible with nxp,sysctr-timer

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../devicetree/bindings/timer/nxp,sysctr-timer.yaml         | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.yaml b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.yaml
index 2b9653dafab8..4f0b660d5ce3 100644
--- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.yaml
+++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.yaml
@@ -18,7 +18,11 @@ description: |
 
 properties:
   compatible:
-    const: nxp,sysctr-timer
+    oneOf:
+      - const: nxp,sysctr-timer
+      - items:
+          - const: nxp,imx95-sysctr-timer
+          - const: nxp,sysctr-timer
 
   reg:
     maxItems: 1
-- 
2.37.1

[PATCH 2/2] clocksource: timer-imx-sysctr: support i.MX95

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

To i.MX95 System counter module, we use Read register space to get
the counter, not the Control register space to get the counter, because
System Manager firmware not allow Linux to read Control register space.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clocksource/timer-imx-sysctr.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/timer-imx-sysctr.c b/drivers/clocksource/timer-imx-sysctr.c
index 5a7a951c4efc..3d3bc16388ed 100644
--- a/drivers/clocksource/timer-imx-sysctr.c
+++ b/drivers/clocksource/timer-imx-sysctr.c
@@ -8,9 +8,12 @@
 #include "timer-of.h"
 
 #define CMP_OFFSET	0x10000
+#define RD_OFFSET	0x20000
 
 #define CNTCV_LO	0x8
 #define CNTCV_HI	0xc
+#define CNTCV_LO_IMX95	(RD_OFFSET + 0x8)
+#define CNTCV_HI_IMX95	(RD_OFFSET + 0xc)
 #define CMPCV_LO	(CMP_OFFSET + 0x20)
 #define CMPCV_HI	(CMP_OFFSET + 0x24)
 #define CMPCR		(CMP_OFFSET + 0x2c)
@@ -22,6 +25,8 @@
 
 static void __iomem *sys_ctr_base __ro_after_init;
 static u32 cmpcr __ro_after_init;
+static u32 cntcv_hi = CNTCV_HI;
+static u32 cntcv_lo = CNTCV_LO;
 
 static void sysctr_timer_enable(bool enable)
 {
@@ -43,9 +48,9 @@ static inline u64 sysctr_read_counter(void)
 	u32 cnt_hi, tmp_hi, cnt_lo;
 
 	do {
-		cnt_hi = readl_relaxed(sys_ctr_base + CNTCV_HI);
-		cnt_lo = readl_relaxed(sys_ctr_base + CNTCV_LO);
-		tmp_hi = readl_relaxed(sys_ctr_base + CNTCV_HI);
+		cnt_hi = readl_relaxed(sys_ctr_base + cntcv_hi);
+		cnt_lo = readl_relaxed(sys_ctr_base + cntcv_lo);
+		tmp_hi = readl_relaxed(sys_ctr_base + cntcv_hi);
 	} while (tmp_hi != cnt_hi);
 
 	return  ((u64) cnt_hi << 32) | cnt_lo;
@@ -139,6 +144,11 @@ static int __init sysctr_timer_init(struct device_node *np)
 		to_sysctr.of_clk.rate /= SYS_CTR_CLK_DIV;
 	}
 
+	if (of_device_is_compatible(np, "nxp,imx95-sysctr-timer")) {
+		cntcv_hi = CNTCV_HI_IMX95;
+		cntcv_lo = CNTCV_LO_IMX95;
+	}
+
 	sys_ctr_base = timer_of_base(&to_sysctr);
 	cmpcr = readl(sys_ctr_base + CMPCR);
 	cmpcr &= ~SYS_CTR_EN;
-- 
2.37.1

Re: [PATCH 1/2] dt-bindings: timer: nxp,sysctr-timer: support i.MX95

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1255 bytes --]

On Mon, Jan 22, 2024 at 05:22:24PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> i.MX95 System counter module has similar design as i.MX93, so add
> fallback compatible with nxp,sysctr-timer

It would be good, in my opinion, to add some device specific compatibles
and deprecate using "sysctr-timer" in isolation.

> Signed-off-by: Peng Fan <peng.fan@nxp.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

> ---
>  .../devicetree/bindings/timer/nxp,sysctr-timer.yaml         | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.yaml b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.yaml
> index 2b9653dafab8..4f0b660d5ce3 100644
> --- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.yaml
> +++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.yaml
> @@ -18,7 +18,11 @@ description: |
>  
>  properties:
>    compatible:
> -    const: nxp,sysctr-timer
> +    oneOf:
> +      - const: nxp,sysctr-timer
> +      - items:
> +          - const: nxp,imx95-sysctr-timer
> +          - const: nxp,sysctr-timer
>  
>    reg:
>      maxItems: 1
> -- 
> 2.37.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/2] clocksource: timer-imx-sysctr: support i.MX95

[Marco Felsch]

Hi Peng,

On 24-01-22, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> To i.MX95 System counter module, we use Read register space to get
> the counter, not the Control register space to get the counter, because
> System Manager firmware not allow Linux to read Control register space.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/clocksource/timer-imx-sysctr.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clocksource/timer-imx-sysctr.c b/drivers/clocksource/timer-imx-sysctr.c
> index 5a7a951c4efc..3d3bc16388ed 100644
> --- a/drivers/clocksource/timer-imx-sysctr.c
> +++ b/drivers/clocksource/timer-imx-sysctr.c
> @@ -8,9 +8,12 @@
>  #include "timer-of.h"
>  
>  #define CMP_OFFSET	0x10000
> +#define RD_OFFSET	0x20000
>  
>  #define CNTCV_LO	0x8
>  #define CNTCV_HI	0xc
> +#define CNTCV_LO_IMX95	(RD_OFFSET + 0x8)
> +#define CNTCV_HI_IMX95	(RD_OFFSET + 0xc)
>  #define CMPCV_LO	(CMP_OFFSET + 0x20)
>  #define CMPCV_HI	(CMP_OFFSET + 0x24)
>  #define CMPCR		(CMP_OFFSET + 0x2c)
> @@ -22,6 +25,8 @@
>  
>  static void __iomem *sys_ctr_base __ro_after_init;
>  static u32 cmpcr __ro_after_init;
> +static u32 cntcv_hi = CNTCV_HI;
> +static u32 cntcv_lo = CNTCV_LO;
>  
>  static void sysctr_timer_enable(bool enable)
>  {
> @@ -43,9 +48,9 @@ static inline u64 sysctr_read_counter(void)
>  	u32 cnt_hi, tmp_hi, cnt_lo;
>  
>  	do {
> -		cnt_hi = readl_relaxed(sys_ctr_base + CNTCV_HI);
> -		cnt_lo = readl_relaxed(sys_ctr_base + CNTCV_LO);
> -		tmp_hi = readl_relaxed(sys_ctr_base + CNTCV_HI);
> +		cnt_hi = readl_relaxed(sys_ctr_base + cntcv_hi);
> +		cnt_lo = readl_relaxed(sys_ctr_base + cntcv_lo);
> +		tmp_hi = readl_relaxed(sys_ctr_base + cntcv_hi);
>  	} while (tmp_hi != cnt_hi);
>  
>  	return  ((u64) cnt_hi << 32) | cnt_lo;
> @@ -139,6 +144,11 @@ static int __init sysctr_timer_init(struct device_node *np)
>  		to_sysctr.of_clk.rate /= SYS_CTR_CLK_DIV;
>  	}
>  
> +	if (of_device_is_compatible(np, "nxp,imx95-sysctr-timer")) {
> +		cntcv_hi = CNTCV_HI_IMX95;
> +		cntcv_lo = CNTCV_LO_IMX95;
> +	}

I'm not a fan of this, since TIMER_OF_DECLARE() can do the compat check.
So instead of using fallback bindings just use the correct binding
within the dts file. Also I'm not a fan of this clobal variable setting
albeit there is just one instance _yet_ we really should instead rework
this driver a bit and instead provide a i.MX95 specific
sysctr_read_counter() function. This is clearly a bit more work but IMO
a cleaner approach. Also afterwards once you add i.MX9x which is again a
bit different would gain from this work.

Regards,
  Marco

> +
>  	sys_ctr_base = timer_of_base(&to_sysctr);
>  	cmpcr = readl(sys_ctr_base + CMPCR);
>  	cmpcr &= ~SYS_CTR_EN;
> -- 
> 2.37.1
> 
> 
> 

Re: [PATCH 1/2] dt-bindings: timer: nxp,sysctr-timer: support i.MX95

[Rob Herring]

On Mon, Jan 22, 2024 at 05:22:24PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> i.MX95 System counter module has similar design as i.MX93, so add
> fallback compatible with nxp,sysctr-timer
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  .../devicetree/bindings/timer/nxp,sysctr-timer.yaml         | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.yaml b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.yaml
> index 2b9653dafab8..4f0b660d5ce3 100644
> --- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.yaml
> +++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.yaml
> @@ -18,7 +18,11 @@ description: |
>  
>  properties:
>    compatible:
> -    const: nxp,sysctr-timer
> +    oneOf:
> +      - const: nxp,sysctr-timer
> +      - items:
> +          - const: nxp,imx95-sysctr-timer
> +          - const: nxp,sysctr-timer

Based on the driver changes, imx95 is not compatible with the existing 
hardware. Different register layout equals not compatible.

Rob