Skip to contentSkip to conversation

lib: utils/mtimer: Add has_broken_mtime quirk

PATCH Superseded Anup Patel 3 participants 4 messages Jun 21, 2022 – Jun 22, 2022

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Anup Patel

author
#0 lore raw

PATCH 1/1 lib: utils/mtimer: Add has_broken_mtime quirk

The Allwinner D1 SoC has custom CLINT implementation where the MTIME register of the timer is not accessible as MMIO register instead the time CSR is to be used as MTIME.

We introduce "has_broken_mtime" quirk in MTIMER library and FDT based MTIMER driver to address the Allwiner D1 situation. And while we are here, let us extend the quirk mechanism in FDT based MTIMER driver for both CLINT and ACLINT.

---
 include/sbi_utils/timer/aclint_mtimer.h |  1 +
 lib/utils/timer/aclint_mtimer.c         | 30 +++++++++++++++++++------
 lib/utils/timer/fdt_timer_mtimer.c      | 29 ++++++++++++++++++------
 3 files changed, 46 insertions(+), 14 deletions(-)
include/sbi_utils/timer/aclint_mtimer.h +1 -0
index f02cc62..454a92f 100644
@@ -32,6 +32,7 @@ struct aclint_mtimer_data {
 	u32 first_hartid;
 	u32 hart_count;
 	bool has_64bit_mmio;
+	bool has_broken_mtime;
 	bool has_shared_mtime;
 	/* Private details (initialized and used by ACLINT MTIMER library) */
 	struct aclint_mtimer_data *time_delta_reference;
lib/utils/timer/aclint_mtimer.c +23 -7
index a957b1c..5db3120 100644
@@ -60,6 +60,12 @@ static u64 mtimer_value(void)
 	return mt->time_rd(time_val);
 }
 
+static u64 mtimer_dummy_value(void)
+{
+	/* Dummy function for MTIMER devices with broken MTIME */
+	return 0;
+}
+
 static void mtimer_event_stop(void)
 {
 	u32 target_hart = current_hartid();
@@ -95,7 +101,9 @@ void aclint_mtimer_sync(struct aclint_mtimer_data *mt)
 	struct aclint_mtimer_data *reference;
 
 	/* Sync-up non-shared MTIME if reference is available */
-	if (mt->has_shared_mtime || !mt->time_delta_reference)
+	if (mt->has_broken_mtime ||
+	    mt->has_shared_mtime ||
+	    !mt->time_delta_reference)
 		return;
 
 	reference = mt->time_delta_reference;
@@ -189,6 +197,10 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
 	if (reference && mt->mtime_freq != reference->mtime_freq)
 		return SBI_EINVAL;
 
+	/* Override timer_value() callback for broken MTIME */
+	if (mt->has_broken_mtime)
+		mtimer.timer_value = mtimer_dummy_value;
+
 	/* Initialize private data */
 	aclint_mtimer_set_reference(mt, reference);
 	mt->time_rd = mtimer_time_rd32;
@@ -207,21 +219,25 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
 		mtimer_hartid2data[mt->first_hartid + i] = mt;
 
 	/* Add MTIMER regions to the root domain */
-	if (mt->mtime_addr == (mt->mtimecmp_addr + mt->mtimecmp_size)) {
+	if (!mt->has_broken_mtime &&
+	    mt->mtime_addr == (mt->mtimecmp_addr + mt->mtimecmp_size)) {
 		rc = aclint_mtimer_add_regions(mt->mtimecmp_addr,
 					mt->mtime_size + mt->mtimecmp_size);
 		if (rc)
 			return rc;
-	} else if (mt->mtimecmp_addr == (mt->mtime_addr + mt->mtime_size)) {
+	} else if (!mt->has_broken_mtime &&
+		   mt->mtimecmp_addr == (mt->mtime_addr + mt->mtime_size)) {
 		rc = aclint_mtimer_add_regions(mt->mtime_addr,
 					mt->mtime_size + mt->mtimecmp_size);
 		if (rc)
 			return rc;
 	} else {
-		rc = aclint_mtimer_add_regions(mt->mtime_addr,
-						mt->mtime_size);
-		if (rc)
-			return rc;
+		if (!mt->has_broken_mtime) {
+			rc = aclint_mtimer_add_regions(mt->mtime_addr,
+							mt->mtime_size);
+			if (rc)
+				return rc;
+		}
 
 		rc = aclint_mtimer_add_regions(mt->mtimecmp_addr,
 						mt->mtimecmp_size);
lib/utils/timer/fdt_timer_mtimer.c +22 -7
index e140567..bc547ac 100644
@@ -16,8 +16,10 @@
 #define MTIMER_MAX_NR			16
 
 struct timer_mtimer_quirks {
+	bool		is_clint;
 	unsigned int	mtime_offset;
 	bool		has_64bit_mmio;
+	bool		has_broken_mtime;
 };
 
 static unsigned long mtimer_count = 0;
@@ -30,6 +32,11 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
 	int i, rc;
 	unsigned long addr[2], size[2];
 	struct aclint_mtimer_data *mt;
+	const struct timer_mtimer_quirks *quirks;
+
+	if (!match->data)
+		return SBI_ENOSYS;
+	quirks = match->data;
 
 	if (MTIMER_MAX_NR <= mtimer_count)
 		return SBI_ENOSPC;
@@ -40,16 +47,15 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
 				   &mt->first_hartid, &mt->hart_count);
 	if (rc)
 		return rc;
-	mt->has_64bit_mmio = true;
+	mt->has_64bit_mmio = quirks->has_64bit_mmio;
 	mt->has_shared_mtime = false;
+	mt->has_broken_mtime = quirks->has_broken_mtime;
 
 	rc = fdt_parse_timebase_frequency(fdt, &mt->mtime_freq);
 	if (rc)
 		return rc;
 
-	if (match->data) { /* SiFive CLINT */
-		const struct timer_mtimer_quirks *quirks = match->data;
-
+	if (quirks->is_clint) { /* SiFive CLINT */
 		/* Set CLINT addresses */
 		mt->mtimecmp_addr = addr[0] + ACLINT_DEFAULT_MTIMECMP_OFFSET;
 		mt->mtimecmp_size = ACLINT_DEFAULT_MTIMECMP_SIZE;
@@ -59,8 +65,6 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
 		mt->mtime_addr += quirks->mtime_offset;
 		mt->mtimecmp_addr += quirks->mtime_offset;
 		mt->mtime_size -= quirks->mtime_offset;
-		/* Apply additional CLINT quirks */
-		mt->has_64bit_mmio = quirks->has_64bit_mmio;
 	} else { /* RISC-V ACLINT MTIMER */
 		/* Set ACLINT MTIMER addresses */
 		mt->mtime_addr = addr[0];
@@ -110,20 +114,31 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
 }
 
 static const struct timer_mtimer_quirks d1_clint_quirks = {
+	.is_clint	= true,
 	.mtime_offset	= CLINT_MTIMER_OFFSET,
 	.has_64bit_mmio	= false,
+	.has_broken_mtime = true,
 };
 
 static const struct timer_mtimer_quirks sifive_clint_quirks = {
+	.is_clint	= true,
 	.mtime_offset	= CLINT_MTIMER_OFFSET,
 	.has_64bit_mmio	= true,
+	.has_broken_mtime = false,
+};
+
+static const struct timer_mtimer_quirks aclint_quirks = {
+	.is_clint	= false,
+	.mtime_offset	= 0,
+	.has_64bit_mmio	= true,
+	.has_broken_mtime = false,
 };
 
 static const struct fdt_match timer_mtimer_match[] = {
 	{ .compatible = "allwinner,sun20i-d1-clint", .data = &d1_clint_quirks },
 	{ .compatible = "riscv,clint0", .data = &sifive_clint_quirks },
 	{ .compatible = "sifive,clint0", .data = &sifive_clint_quirks },
-	{ .compatible = "riscv,aclint-mtimer" },
+	{ .compatible = "riscv,aclint-mtimer", .data = &aclint_quirks },
 	{ },
 };
 
Signature · 1 line
2.34.1
#1 lore raw

PATCH 1/1 lib: utils/mtimer: Add has_broken_mtime quirk

Quoted 34 lines · Anup Patel

The Allwinner D1 SoC has custom CLINT implementation where the MTIME register of the timer is not accessible as MMIO register instead the time CSR is to be used as MTIME.

We introduce "has_broken_mtime" quirk in MTIMER library and FDT based MTIMER driver to address the Allwiner D1 situation. And while we are here, let us extend the quirk mechanism in FDT based MTIMER driver for both CLINT and ACLINT.

---
 include/sbi_utils/timer/aclint_mtimer.h |  1 +
 lib/utils/timer/aclint_mtimer.c         | 30 +++++++++++++++++++------
 lib/utils/timer/fdt_timer_mtimer.c      | 29 ++++++++++++++++++------
 3 files changed, 46 insertions(+), 14 deletions(-)
include/sbi_utils/timer/aclint_mtimer.h +1 -0
index f02cc62..454a92f 100644
@@ -32,6 +32,7 @@ struct aclint_mtimer_data {
        u32 first_hartid;
        u32 hart_count;
        bool has_64bit_mmio;
+       bool has_broken_mtime;
        bool has_shared_mtime;
        /* Private details (initialized and used by ACLINT MTIMER library) */
        struct aclint_mtimer_data *time_delta_reference;
lib/utils/timer/aclint_mtimer.c +0 -0
index a957b1c..5db3120 100644

lib/utils/timer/aclint_mtimer.c static u64 mtimer_value(void) · line 65

Quoted 6 lines
lib/utils/timer/aclint_mtimer.c +3 -0
@@ -60,6 +60,12 @@ static u64 mtimer_value(void)
        return mt->time_rd(time_val);
 }
 
+static u64 mtimer_dummy_value(void)
+{
+       /* Dummy function for MTIMER devices with broken MTIME */

Could we get value from CSR_TIME? (define CSR_TIME 0xc01)

Quoted 150 lines
lib/utils/timer/aclint_mtimer.c +20 -7
+       return 0;
+}
+
 static void mtimer_event_stop(void)
 {
        u32 target_hart = current_hartid();
@@ -95,7 +101,9 @@ void aclint_mtimer_sync(struct aclint_mtimer_data *mt)
        struct aclint_mtimer_data *reference;
 
        /* Sync-up non-shared MTIME if reference is available */
-       if (mt->has_shared_mtime || !mt->time_delta_reference)
+       if (mt->has_broken_mtime ||
+           mt->has_shared_mtime ||
+           !mt->time_delta_reference)
                return;
 
        reference = mt->time_delta_reference;
@@ -189,6 +197,10 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
        if (reference && mt->mtime_freq != reference->mtime_freq)
                return SBI_EINVAL;
 
+       /* Override timer_value() callback for broken MTIME */
+       if (mt->has_broken_mtime)
+               mtimer.timer_value = mtimer_dummy_value;
+
        /* Initialize private data */
        aclint_mtimer_set_reference(mt, reference);
        mt->time_rd = mtimer_time_rd32;
@@ -207,21 +219,25 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
                mtimer_hartid2data[mt->first_hartid + i] = mt;
 
        /* Add MTIMER regions to the root domain */
-       if (mt->mtime_addr == (mt->mtimecmp_addr + mt->mtimecmp_size)) {
+       if (!mt->has_broken_mtime &&
+           mt->mtime_addr == (mt->mtimecmp_addr + mt->mtimecmp_size)) {
                rc = aclint_mtimer_add_regions(mt->mtimecmp_addr,
                                        mt->mtime_size + mt->mtimecmp_size);
                if (rc)
                        return rc;
-       } else if (mt->mtimecmp_addr == (mt->mtime_addr + mt->mtime_size)) {
+       } else if (!mt->has_broken_mtime &&
+                  mt->mtimecmp_addr == (mt->mtime_addr + mt->mtime_size)) {
                rc = aclint_mtimer_add_regions(mt->mtime_addr,
                                        mt->mtime_size + mt->mtimecmp_size);
                if (rc)
                        return rc;
        } else {
-               rc = aclint_mtimer_add_regions(mt->mtime_addr,
-                                               mt->mtime_size);
-               if (rc)
-                       return rc;
+               if (!mt->has_broken_mtime) {
+                       rc = aclint_mtimer_add_regions(mt->mtime_addr,
+                                                       mt->mtime_size);
+                       if (rc)
+                               return rc;
+               }
 
                rc = aclint_mtimer_add_regions(mt->mtimecmp_addr,
                                                mt->mtimecmp_size);
lib/utils/timer/fdt_timer_mtimer.c +22 -8
index e140567..bc547ac 100644
@@ -16,8 +16,10 @@
 #define MTIMER_MAX_NR                  16
 
 struct timer_mtimer_quirks {
+       bool            is_clint;
        unsigned int    mtime_offset;
        bool            has_64bit_mmio;
+       bool            has_broken_mtime;
 };
 
 static unsigned long mtimer_count = 0;
@@ -30,6 +32,11 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
        int i, rc;
        unsigned long addr[2], size[2];
        struct aclint_mtimer_data *mt;
+       const struct timer_mtimer_quirks *quirks;
+
+       if (!match->data)
+               return SBI_ENOSYS;
+       quirks = match->data;
 
        if (MTIMER_MAX_NR <= mtimer_count)
                return SBI_ENOSPC;
@@ -40,16 +47,15 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
                                   &mt->first_hartid, &mt->hart_count);
        if (rc)
                return rc;
-       mt->has_64bit_mmio = true;
+       mt->has_64bit_mmio = quirks->has_64bit_mmio;
        mt->has_shared_mtime = false;
+       mt->has_broken_mtime = quirks->has_broken_mtime;
 
        rc = fdt_parse_timebase_frequency(fdt, &mt->mtime_freq);
        if (rc)
                return rc;
 
-       if (match->data) { /* SiFive CLINT */
-               const struct timer_mtimer_quirks *quirks = match->data;
-
+       if (quirks->is_clint) { /* SiFive CLINT */
                /* Set CLINT addresses */
                mt->mtimecmp_addr = addr[0] + ACLINT_DEFAULT_MTIMECMP_OFFSET;
                mt->mtimecmp_size = ACLINT_DEFAULT_MTIMECMP_SIZE;
@@ -59,8 +65,6 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
                mt->mtime_addr += quirks->mtime_offset;
                mt->mtimecmp_addr += quirks->mtime_offset;
                mt->mtime_size -= quirks->mtime_offset;
-               /* Apply additional CLINT quirks */
-               mt->has_64bit_mmio = quirks->has_64bit_mmio;
        } else { /* RISC-V ACLINT MTIMER */
                /* Set ACLINT MTIMER addresses */
                mt->mtime_addr = addr[0];
@@ -110,20 +114,31 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
 }
 
 static const struct timer_mtimer_quirks d1_clint_quirks = {
+       .is_clint       = true,
        .mtime_offset   = CLINT_MTIMER_OFFSET,
        .has_64bit_mmio = false,
+       .has_broken_mtime = true,
 };
 
 static const struct timer_mtimer_quirks sifive_clint_quirks = {
+       .is_clint       = true,
        .mtime_offset   = CLINT_MTIMER_OFFSET,
        .has_64bit_mmio = true,
+       .has_broken_mtime = false,
+};
+
+static const struct timer_mtimer_quirks aclint_quirks = {
+       .is_clint       = false,
+       .mtime_offset   = 0,
+       .has_64bit_mmio = true,
+       .has_broken_mtime = false,
 };
 
 static const struct fdt_match timer_mtimer_match[] = {
        { .compatible = "allwinner,sun20i-d1-clint", .data = &d1_clint_quirks },
        { .compatible = "riscv,clint0", .data = &sifive_clint_quirks },
        { .compatible = "sifive,clint0", .data = &sifive_clint_quirks },
-       { .compatible = "riscv,aclint-mtimer" },
+       { .compatible = "riscv,aclint-mtimer", .data = &aclint_quirks },
        { },
 };
 
--
Quoted 2 lines

2.34.1

--
Best Regards
Guo Ren

ML: https://lore.kernel.org/linux-csky/

#2 lore raw

PATCH 1/1 lib: utils/mtimer: Add has_broken_mtime quirk

Quoted 39 lines · Guo Ren
Quoted 37 lines · Anup Patel

The Allwinner D1 SoC has custom CLINT implementation where the MTIME register of the timer is not accessible as MMIO register instead the time CSR is to be used as MTIME.

We introduce "has_broken_mtime" quirk in MTIMER library and FDT based MTIMER driver to address the Allwiner D1 situation. And while we are here, let us extend the quirk mechanism in FDT based MTIMER driver for both CLINT and ACLINT.

---
 include/sbi_utils/timer/aclint_mtimer.h |  1 +
 lib/utils/timer/aclint_mtimer.c         | 30 +++++++++++++++++++------
 lib/utils/timer/fdt_timer_mtimer.c      | 29 ++++++++++++++++++------
 3 files changed, 46 insertions(+), 14 deletions(-)
include/sbi_utils/timer/aclint_mtimer.h +1 -0
index f02cc62..454a92f 100644
@@ -32,6 +32,7 @@ struct aclint_mtimer_data {
        u32 first_hartid;
        u32 hart_count;
        bool has_64bit_mmio;
+       bool has_broken_mtime;
        bool has_shared_mtime;
        /* Private details (initialized and used by ACLINT MTIMER library) */
        struct aclint_mtimer_data *time_delta_reference;
lib/utils/timer/aclint_mtimer.c +0 -0
index a957b1c..5db3120 100644
@@ -60,6 +60,12 @@ static u64 mtimer_value(void)
        return mt->time_rd(time_val);
 }
 
Quoted 4 lines

lib/utils/timer/aclint_mtimer.c static u64 mtimer_value(void) · line 65

Quoted 3 lines
lib/utils/timer/aclint_mtimer.c +3 -0
+static u64 mtimer_dummy_value(void)
+{
+       /* Dummy function for MTIMER devices with broken MTIME */

Could we get value from CSR_TIME? (define CSR_TIME 0xc01)

The lib/sbi/sbi_timer.c will use CSR_TIME whenever it is available so we don't need to access it here.

Maybe we can drop mtimer_dummy_value() and set "mtimer.timer_value = NULL" when "has_broken_mtime = TRUE" ?

Can you try this patch on D1 ?

Regards,
Anup

Quoted 160 lines
Quoted 151 lines
lib/utils/timer/aclint_mtimer.c +20 -7
+       return 0;
+}
+
 static void mtimer_event_stop(void)
 {
        u32 target_hart = current_hartid();
@@ -95,7 +101,9 @@ void aclint_mtimer_sync(struct aclint_mtimer_data *mt)
        struct aclint_mtimer_data *reference;
 
        /* Sync-up non-shared MTIME if reference is available */
-       if (mt->has_shared_mtime || !mt->time_delta_reference)
+       if (mt->has_broken_mtime ||
+           mt->has_shared_mtime ||
+           !mt->time_delta_reference)
                return;
 
        reference = mt->time_delta_reference;
@@ -189,6 +197,10 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
        if (reference && mt->mtime_freq != reference->mtime_freq)
                return SBI_EINVAL;
 
+       /* Override timer_value() callback for broken MTIME */
+       if (mt->has_broken_mtime)
+               mtimer.timer_value = mtimer_dummy_value;
+
        /* Initialize private data */
        aclint_mtimer_set_reference(mt, reference);
        mt->time_rd = mtimer_time_rd32;
@@ -207,21 +219,25 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
                mtimer_hartid2data[mt->first_hartid + i] = mt;
 
        /* Add MTIMER regions to the root domain */
-       if (mt->mtime_addr == (mt->mtimecmp_addr + mt->mtimecmp_size)) {
+       if (!mt->has_broken_mtime &&
+           mt->mtime_addr == (mt->mtimecmp_addr + mt->mtimecmp_size)) {
                rc = aclint_mtimer_add_regions(mt->mtimecmp_addr,
                                        mt->mtime_size + mt->mtimecmp_size);
                if (rc)
                        return rc;
-       } else if (mt->mtimecmp_addr == (mt->mtime_addr + mt->mtime_size)) {
+       } else if (!mt->has_broken_mtime &&
+                  mt->mtimecmp_addr == (mt->mtime_addr + mt->mtime_size)) {
                rc = aclint_mtimer_add_regions(mt->mtime_addr,
                                        mt->mtime_size + mt->mtimecmp_size);
                if (rc)
                        return rc;
        } else {
-               rc = aclint_mtimer_add_regions(mt->mtime_addr,
-                                               mt->mtime_size);
-               if (rc)
-                       return rc;
+               if (!mt->has_broken_mtime) {
+                       rc = aclint_mtimer_add_regions(mt->mtime_addr,
+                                                       mt->mtime_size);
+                       if (rc)
+                               return rc;
+               }
 
                rc = aclint_mtimer_add_regions(mt->mtimecmp_addr,
                                                mt->mtimecmp_size);
lib/utils/timer/fdt_timer_mtimer.c +22 -8
index e140567..bc547ac 100644
@@ -16,8 +16,10 @@
 #define MTIMER_MAX_NR                  16
 
 struct timer_mtimer_quirks {
+       bool            is_clint;
        unsigned int    mtime_offset;
        bool            has_64bit_mmio;
+       bool            has_broken_mtime;
 };
 
 static unsigned long mtimer_count = 0;
@@ -30,6 +32,11 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
        int i, rc;
        unsigned long addr[2], size[2];
        struct aclint_mtimer_data *mt;
+       const struct timer_mtimer_quirks *quirks;
+
+       if (!match->data)
+               return SBI_ENOSYS;
+       quirks = match->data;
 
        if (MTIMER_MAX_NR <= mtimer_count)
                return SBI_ENOSPC;
@@ -40,16 +47,15 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
                                   &mt->first_hartid, &mt->hart_count);
        if (rc)
                return rc;
-       mt->has_64bit_mmio = true;
+       mt->has_64bit_mmio = quirks->has_64bit_mmio;
        mt->has_shared_mtime = false;
+       mt->has_broken_mtime = quirks->has_broken_mtime;
 
        rc = fdt_parse_timebase_frequency(fdt, &mt->mtime_freq);
        if (rc)
                return rc;
 
-       if (match->data) { /* SiFive CLINT */
-               const struct timer_mtimer_quirks *quirks = match->data;
-
+       if (quirks->is_clint) { /* SiFive CLINT */
                /* Set CLINT addresses */
                mt->mtimecmp_addr = addr[0] + ACLINT_DEFAULT_MTIMECMP_OFFSET;
                mt->mtimecmp_size = ACLINT_DEFAULT_MTIMECMP_SIZE;
@@ -59,8 +65,6 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
                mt->mtime_addr += quirks->mtime_offset;
                mt->mtimecmp_addr += quirks->mtime_offset;
                mt->mtime_size -= quirks->mtime_offset;
-               /* Apply additional CLINT quirks */
-               mt->has_64bit_mmio = quirks->has_64bit_mmio;
        } else { /* RISC-V ACLINT MTIMER */
                /* Set ACLINT MTIMER addresses */
                mt->mtime_addr = addr[0];
@@ -110,20 +114,31 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
 }
 
 static const struct timer_mtimer_quirks d1_clint_quirks = {
+       .is_clint       = true,
        .mtime_offset   = CLINT_MTIMER_OFFSET,
        .has_64bit_mmio = false,
+       .has_broken_mtime = true,
 };
 
 static const struct timer_mtimer_quirks sifive_clint_quirks = {
+       .is_clint       = true,
        .mtime_offset   = CLINT_MTIMER_OFFSET,
        .has_64bit_mmio = true,
+       .has_broken_mtime = false,
+};
+
+static const struct timer_mtimer_quirks aclint_quirks = {
+       .is_clint       = false,
+       .mtime_offset   = 0,
+       .has_64bit_mmio = true,
+       .has_broken_mtime = false,
 };
 
 static const struct fdt_match timer_mtimer_match[] = {
        { .compatible = "allwinner,sun20i-d1-clint", .data = &d1_clint_quirks },
        { .compatible = "riscv,clint0", .data = &sifive_clint_quirks },
        { .compatible = "sifive,clint0", .data = &sifive_clint_quirks },
-       { .compatible = "riscv,aclint-mtimer" },
+       { .compatible = "riscv,aclint-mtimer", .data = &aclint_quirks },
        { },
 };
 
--
Quoted 2 lines

2.34.1

--
Best Regards
Guo Ren

ML: https://lore.kernel.org/linux-csky/

#3 lore raw

PATCH 1/1 lib: utils/mtimer: Add has_broken_mtime quirk

Quoted 46 lines · Anup Patel
Quoted 39 lines · Guo Ren
Quoted 37 lines · Anup Patel

The Allwinner D1 SoC has custom CLINT implementation where the MTIME register of the timer is not accessible as MMIO register instead the time CSR is to be used as MTIME.

We introduce "has_broken_mtime" quirk in MTIMER library and FDT based MTIMER driver to address the Allwiner D1 situation. And while we are here, let us extend the quirk mechanism in FDT based MTIMER driver for both CLINT and ACLINT.

---
 include/sbi_utils/timer/aclint_mtimer.h |  1 +
 lib/utils/timer/aclint_mtimer.c         | 30 +++++++++++++++++++------
 lib/utils/timer/fdt_timer_mtimer.c      | 29 ++++++++++++++++++------
 3 files changed, 46 insertions(+), 14 deletions(-)
include/sbi_utils/timer/aclint_mtimer.h +1 -0
index f02cc62..454a92f 100644
@@ -32,6 +32,7 @@ struct aclint_mtimer_data {
        u32 first_hartid;
        u32 hart_count;
        bool has_64bit_mmio;
+       bool has_broken_mtime;
        bool has_shared_mtime;
        /* Private details (initialized and used by ACLINT MTIMER library) */
        struct aclint_mtimer_data *time_delta_reference;
lib/utils/timer/aclint_mtimer.c +0 -0
index a957b1c..5db3120 100644
@@ -60,6 +60,12 @@ static u64 mtimer_value(void)
        return mt->time_rd(time_val);
 }
 
Quoted 4 lines

lib/utils/timer/aclint_mtimer.c static u64 mtimer_value(void) · line 65

Quoted 3 lines
lib/utils/timer/aclint_mtimer.c +3 -0
+static u64 mtimer_dummy_value(void)
+{
+       /* Dummy function for MTIMER devices with broken MTIME */

Could we get value from CSR_TIME? (define CSR_TIME 0xc01)

Quoted 5 lines

The lib/sbi/sbi_timer.c will use CSR_TIME whenever it is available so we don't need to access it here.

Maybe we can drop mtimer_dummy_value() and set "mtimer.timer_value = NULL" when "has_broken_mtime = TRUE" ?

Agree, that make code clean.

Quoted 2 lines

Can you try this patch on D1 ?

I would, but I need to spend some time preparing the environment.

Quoted 164 lines

Regards,
Anup

Quoted 160 lines
Quoted 151 lines
lib/utils/timer/aclint_mtimer.c +20 -7
+       return 0;
+}
+
 static void mtimer_event_stop(void)
 {
        u32 target_hart = current_hartid();
@@ -95,7 +101,9 @@ void aclint_mtimer_sync(struct aclint_mtimer_data *mt)
        struct aclint_mtimer_data *reference;
 
        /* Sync-up non-shared MTIME if reference is available */
-       if (mt->has_shared_mtime || !mt->time_delta_reference)
+       if (mt->has_broken_mtime ||
+           mt->has_shared_mtime ||
+           !mt->time_delta_reference)
                return;
 
        reference = mt->time_delta_reference;
@@ -189,6 +197,10 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
        if (reference && mt->mtime_freq != reference->mtime_freq)
                return SBI_EINVAL;
 
+       /* Override timer_value() callback for broken MTIME */
+       if (mt->has_broken_mtime)
+               mtimer.timer_value = mtimer_dummy_value;
+
        /* Initialize private data */
        aclint_mtimer_set_reference(mt, reference);
        mt->time_rd = mtimer_time_rd32;
@@ -207,21 +219,25 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
                mtimer_hartid2data[mt->first_hartid + i] = mt;
 
        /* Add MTIMER regions to the root domain */
-       if (mt->mtime_addr == (mt->mtimecmp_addr + mt->mtimecmp_size)) {
+       if (!mt->has_broken_mtime &&
+           mt->mtime_addr == (mt->mtimecmp_addr + mt->mtimecmp_size)) {
                rc = aclint_mtimer_add_regions(mt->mtimecmp_addr,
                                        mt->mtime_size + mt->mtimecmp_size);
                if (rc)
                        return rc;
-       } else if (mt->mtimecmp_addr == (mt->mtime_addr + mt->mtime_size)) {
+       } else if (!mt->has_broken_mtime &&
+                  mt->mtimecmp_addr == (mt->mtime_addr + mt->mtime_size)) {
                rc = aclint_mtimer_add_regions(mt->mtime_addr,
                                        mt->mtime_size + mt->mtimecmp_size);
                if (rc)
                        return rc;
        } else {
-               rc = aclint_mtimer_add_regions(mt->mtime_addr,
-                                               mt->mtime_size);
-               if (rc)
-                       return rc;
+               if (!mt->has_broken_mtime) {
+                       rc = aclint_mtimer_add_regions(mt->mtime_addr,
+                                                       mt->mtime_size);
+                       if (rc)
+                               return rc;
+               }
 
                rc = aclint_mtimer_add_regions(mt->mtimecmp_addr,
                                                mt->mtimecmp_size);
lib/utils/timer/fdt_timer_mtimer.c +22 -8
index e140567..bc547ac 100644
@@ -16,8 +16,10 @@
 #define MTIMER_MAX_NR                  16
 
 struct timer_mtimer_quirks {
+       bool            is_clint;
        unsigned int    mtime_offset;
        bool            has_64bit_mmio;
+       bool            has_broken_mtime;
 };
 
 static unsigned long mtimer_count = 0;
@@ -30,6 +32,11 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
        int i, rc;
        unsigned long addr[2], size[2];
        struct aclint_mtimer_data *mt;
+       const struct timer_mtimer_quirks *quirks;
+
+       if (!match->data)
+               return SBI_ENOSYS;
+       quirks = match->data;
 
        if (MTIMER_MAX_NR <= mtimer_count)
                return SBI_ENOSPC;
@@ -40,16 +47,15 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
                                   &mt->first_hartid, &mt->hart_count);
        if (rc)
                return rc;
-       mt->has_64bit_mmio = true;
+       mt->has_64bit_mmio = quirks->has_64bit_mmio;
        mt->has_shared_mtime = false;
+       mt->has_broken_mtime = quirks->has_broken_mtime;
 
        rc = fdt_parse_timebase_frequency(fdt, &mt->mtime_freq);
        if (rc)
                return rc;
 
-       if (match->data) { /* SiFive CLINT */
-               const struct timer_mtimer_quirks *quirks = match->data;
-
+       if (quirks->is_clint) { /* SiFive CLINT */
                /* Set CLINT addresses */
                mt->mtimecmp_addr = addr[0] + ACLINT_DEFAULT_MTIMECMP_OFFSET;
                mt->mtimecmp_size = ACLINT_DEFAULT_MTIMECMP_SIZE;
@@ -59,8 +65,6 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
                mt->mtime_addr += quirks->mtime_offset;
                mt->mtimecmp_addr += quirks->mtime_offset;
                mt->mtime_size -= quirks->mtime_offset;
-               /* Apply additional CLINT quirks */
-               mt->has_64bit_mmio = quirks->has_64bit_mmio;
        } else { /* RISC-V ACLINT MTIMER */
                /* Set ACLINT MTIMER addresses */
                mt->mtime_addr = addr[0];
@@ -110,20 +114,31 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
 }
 
 static const struct timer_mtimer_quirks d1_clint_quirks = {
+       .is_clint       = true,
        .mtime_offset   = CLINT_MTIMER_OFFSET,
        .has_64bit_mmio = false,
+       .has_broken_mtime = true,
 };
 
 static const struct timer_mtimer_quirks sifive_clint_quirks = {
+       .is_clint       = true,
        .mtime_offset   = CLINT_MTIMER_OFFSET,
        .has_64bit_mmio = true,
+       .has_broken_mtime = false,
+};
+
+static const struct timer_mtimer_quirks aclint_quirks = {
+       .is_clint       = false,
+       .mtime_offset   = 0,
+       .has_64bit_mmio = true,
+       .has_broken_mtime = false,
 };
 
 static const struct fdt_match timer_mtimer_match[] = {
        { .compatible = "allwinner,sun20i-d1-clint", .data = &d1_clint_quirks },
        { .compatible = "riscv,clint0", .data = &sifive_clint_quirks },
        { .compatible = "sifive,clint0", .data = &sifive_clint_quirks },
-       { .compatible = "riscv,aclint-mtimer" },
+       { .compatible = "riscv,aclint-mtimer", .data = &aclint_quirks },
        { },
 };
 
--
Quoted 2 lines

2.34.1

--
Best Regards
Guo Ren

ML: https://lore.kernel.org/linux-csky/

Signature · 4 lines
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/