From: Peng Fan <[email protected]>
i.MX95 System counter module has similar design as i.MX93, so add
fallback compatible with nxp,sysctr-timer
Signed-off-by: Peng Fan <[email protected]>
---
.../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
From: Peng Fan <[email protected]>
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 <[email protected]>
---
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
On Mon, Jan 22, 2024 at 05:22:24PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> 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 <[email protected]>
Acked-by: Conor Dooley <[email protected]>
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
>
Hi Peng,
On 24-01-22, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>
>
>
On Mon, Jan 22, 2024 at 05:22:24PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> i.MX95 System counter module has similar design as i.MX93, so add
> fallback compatible with nxp,sysctr-timer
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> .../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