* [PATCH 2/2] clocksource: timer-imx-sysctr: support i.MX95
2024-01-22 9:22 [PATCH 1/2] dt-bindings: timer: nxp,sysctr-timer: support i.MX95 Peng Fan (OSS)
@ 2024-01-22 9:22 ` Peng Fan (OSS)
2024-01-23 9:52 ` Marco Felsch
2024-01-22 18:08 ` [PATCH 1/2] dt-bindings: timer: nxp,sysctr-timer: " Conor Dooley
2024-01-23 20:50 ` Rob Herring
2 siblings, 1 reply; 5+ messages in thread
From: Peng Fan (OSS) @ 2024-01-22 9:22 UTC (permalink / raw)
To: daniel.lezcano, tglx, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, kernel, linux-imx
Cc: linux-kernel, devicetree, linux-arm-kernel, Peng Fan
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] clocksource: timer-imx-sysctr: support i.MX95
2024-01-22 9:22 ` [PATCH 2/2] clocksource: timer-imx-sysctr: " Peng Fan (OSS)
@ 2024-01-23 9:52 ` Marco Felsch
0 siblings, 0 replies; 5+ messages in thread
From: Marco Felsch @ 2024-01-23 9:52 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: daniel.lezcano, tglx, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, kernel, linux-imx, devicetree, Peng Fan,
linux-kernel, linux-arm-kernel
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
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] dt-bindings: timer: nxp,sysctr-timer: support i.MX95
2024-01-22 9:22 [PATCH 1/2] dt-bindings: timer: nxp,sysctr-timer: support i.MX95 Peng Fan (OSS)
2024-01-22 9:22 ` [PATCH 2/2] clocksource: timer-imx-sysctr: " Peng Fan (OSS)
@ 2024-01-22 18:08 ` Conor Dooley
2024-01-23 20:50 ` Rob Herring
2 siblings, 0 replies; 5+ messages in thread
From: Conor Dooley @ 2024-01-22 18:08 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: daniel.lezcano, tglx, robh+dt, krzysztof.kozlowski+dt, conor+dt,
shawnguo, s.hauer, kernel, linux-imx, linux-kernel, devicetree,
linux-arm-kernel, Peng Fan
[-- 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] dt-bindings: timer: nxp,sysctr-timer: support i.MX95
2024-01-22 9:22 [PATCH 1/2] dt-bindings: timer: nxp,sysctr-timer: support i.MX95 Peng Fan (OSS)
2024-01-22 9:22 ` [PATCH 2/2] clocksource: timer-imx-sysctr: " Peng Fan (OSS)
2024-01-22 18:08 ` [PATCH 1/2] dt-bindings: timer: nxp,sysctr-timer: " Conor Dooley
@ 2024-01-23 20:50 ` Rob Herring
2 siblings, 0 replies; 5+ messages in thread
From: Rob Herring @ 2024-01-23 20:50 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: daniel.lezcano, tglx, krzysztof.kozlowski+dt, conor+dt, shawnguo,
s.hauer, kernel, linux-imx, linux-kernel, devicetree,
linux-arm-kernel, Peng Fan
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
^ permalink raw reply [flat|nested] 5+ messages in thread