Linux-RISC-V Archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] riscv: improve nommu and timer-clint
@ 2024-04-10 14:23 Jisheng Zhang
  2024-04-10 14:23 ` [PATCH v3 1/2] riscv: nommu: remove PAGE_OFFSET hardcoding Jisheng Zhang
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Jisheng Zhang @ 2024-04-10 14:23 UTC (permalink / raw
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano,
	Thomas Gleixner, Samuel Holland, Conor Dooley
  Cc: linux-riscv, linux-kernel

As is known, the sophgo CV1800B contains so called little core, which
is C906 w/o MMU, so I want to run nommu linux on it. This series is
the result of the bring up. After this series, w/ proper dts, we can
run nommu linux on milkv duo's little core.

First of all, patch1 removes the PAGE_OFFSET hardcoding by introducing
DRAM_BASE Kconfig option.

Secondly, to use the T-HEAD C9xx clint in RISCV-M NOMMU env, we need
to take care two points:

1.The mtimecmp in T-Head C9xx clint only supports 32bit read/write,
implement such support.

2. As pointed out by commit ca7810aecdba ("lib: utils/timer: mtimer:
add a quirk for lacking mtime register") of opensbi:

"T-Head developers surely have a different understanding of time CSR and
CLINT's mtime register with SiFive ones, that they did not implement
the mtime register at all -- as shown in openC906 source code, their
time CSR value is just exposed at the top of their processor IP block
and expects an external continous counter, which makes it not
overrideable, and thus mtime register is not implemented, even not for
reading. However, if CLINTEE is not enabled in T-Head's MXSTATUS
extended CSR, these systems still rely on the mtimecmp registers to
generate timer interrupts. This makes it necessary to implement T-Head
C9xx CLINT support in OpenSBI MTIMER driver, which skips implementing
reading mtime register and falls back to default code that reads time
CSR."

So, we need to fall back to read time CSR instead of mtime register.
Add riscv_csr_time_available static key for this purpose.

The second patch adds T-Head C9xxx clint support to timer-clint driver
by taking care of above two points.

Since v2:
  - drop CONFIG_CLINT_USE_CSR_INSTEADOF_MTIME, instead we use runtime
    code patch to dynamically enable CSR TIME code path or MTIME code
    path.

Since v1:
  - fix c900_clint_timer_init_dt() defined but not used build warning
  - add option CONFIG_CLINT_USE_CSR_INSTEADOF_MTIME instead of removing
    mtime usage for all platforms, since not all platforms implement the
    time CSR in HW in M mode.
  - rebase on the timer-clint improvement series
https://lore.kernel.org/linux-riscv/20240406111757.1597-1-jszhang@kernel.org/T/#t


Jisheng Zhang (2):
  riscv: nommu: remove PAGE_OFFSET hardcoding
  clocksource/drivers/timer-clint: Add T-Head C9xx clint

 arch/riscv/Kconfig                |  8 +++++-
 arch/riscv/include/asm/clint.h    |  2 ++
 arch/riscv/include/asm/timex.h    | 18 +++++++++---
 drivers/clocksource/timer-clint.c | 48 +++++++++++++++++++++++++++----
 4 files changed, 66 insertions(+), 10 deletions(-)

-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/2] riscv: nommu: remove PAGE_OFFSET hardcoding
  2024-04-10 14:23 [PATCH v3 0/2] riscv: improve nommu and timer-clint Jisheng Zhang
@ 2024-04-10 14:23 ` Jisheng Zhang
  2024-04-10 15:30   ` Clément Léger
  2024-04-10 14:23 ` [PATCH v3 2/2] clocksource/drivers/timer-clint: Add T-Head C9xx clint Jisheng Zhang
  2024-04-20 11:04 ` [PATCH v3 0/2] riscv: improve nommu and timer-clint Jisheng Zhang
  2 siblings, 1 reply; 8+ messages in thread
From: Jisheng Zhang @ 2024-04-10 14:23 UTC (permalink / raw
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano,
	Thomas Gleixner, Samuel Holland, Conor Dooley
  Cc: linux-riscv, linux-kernel

Currently, PAGE_OFFSET is hardcoded as 0x8000_0000, it works fine since
there's only one nommu platform in the mainline. However, there are
many cases where the (S)DRAM base address isn't 0x8000_0000, so remove
the hardcoding value, and introduce DRAM_BASE which will be set by
users during configuring. DRAM_BASE is 0x8000_0000 by default.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 arch/riscv/Kconfig | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 7895c77545f1..b4af1df86352 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -247,10 +247,16 @@ config MMU
 	  Select if you want MMU-based virtualised addressing space
 	  support by paged memory management. If unsure, say 'Y'.
 
+if !MMU
+config DRAM_BASE
+	hex '(S)DRAM Base Address'
+	default 0x80000000
+endif
+
 config PAGE_OFFSET
 	hex
 	default 0xC0000000 if 32BIT && MMU
-	default 0x80000000 if !MMU
+	default DRAM_BASE if !MMU
 	default 0xff60000000000000 if 64BIT
 
 config KASAN_SHADOW_OFFSET
-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/2] clocksource/drivers/timer-clint: Add T-Head C9xx clint
  2024-04-10 14:23 [PATCH v3 0/2] riscv: improve nommu and timer-clint Jisheng Zhang
  2024-04-10 14:23 ` [PATCH v3 1/2] riscv: nommu: remove PAGE_OFFSET hardcoding Jisheng Zhang
@ 2024-04-10 14:23 ` Jisheng Zhang
  2024-04-24 11:53   ` Conor Dooley
  2024-04-20 11:04 ` [PATCH v3 0/2] riscv: improve nommu and timer-clint Jisheng Zhang
  2 siblings, 1 reply; 8+ messages in thread
From: Jisheng Zhang @ 2024-04-10 14:23 UTC (permalink / raw
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano,
	Thomas Gleixner, Samuel Holland, Conor Dooley
  Cc: linux-riscv, linux-kernel

To use the T-HEAD C9xx clint in RISCV-M NOMMU env, we need to take
care two points:

1.The mtimecmp in T-Head C9xx clint only supports 32bit read/write,
implement such support.

2. As pointed out by commit ca7810aecdba ("lib: utils/timer: mtimer:
add a quirk for lacking mtime register") of opensbi:

"T-Head developers surely have a different understanding of time CSR and
CLINT's mtime register with SiFive ones, that they did not implement
the mtime register at all -- as shown in openC906 source code, their
time CSR value is just exposed at the top of their processor IP block
and expects an external continous counter, which makes it not
overrideable, and thus mtime register is not implemented, even not for
reading. However, if CLINTEE is not enabled in T-Head's MXSTATUS
extended CSR, these systems still rely on the mtimecmp registers to
generate timer interrupts. This makes it necessary to implement T-Head
C9xx CLINT support in OpenSBI MTIMER driver, which skips implementing
reading mtime register and falls back to default code that reads time
CSR."

So, we need to fall back to read time CSR instead of mtime register.
Add riscv_csr_time_available static key for this purpose.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 arch/riscv/include/asm/clint.h    |  2 ++
 arch/riscv/include/asm/timex.h    | 18 +++++++++---
 drivers/clocksource/timer-clint.c | 48 +++++++++++++++++++++++++++----
 3 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/include/asm/clint.h b/arch/riscv/include/asm/clint.h
index 0789fd37b40a..c6057a182c5d 100644
--- a/arch/riscv/include/asm/clint.h
+++ b/arch/riscv/include/asm/clint.h
@@ -10,6 +10,7 @@
 #include <asm/mmio.h>
 
 #ifdef CONFIG_RISCV_M_MODE
+#include <linux/jump_label.h>
 /*
  * This lives in the CLINT driver, but is accessed directly by timex.h to avoid
  * any overhead when accessing the MMIO timer.
@@ -21,6 +22,7 @@
  * like "riscv_mtime", to signify that these non-ISA assumptions must hold.
  */
 extern u64 __iomem *clint_time_val;
+DECLARE_STATIC_KEY_FALSE(riscv_csr_time_available);
 #endif
 
 #endif
diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
index a06697846e69..007a15482d75 100644
--- a/arch/riscv/include/asm/timex.h
+++ b/arch/riscv/include/asm/timex.h
@@ -17,18 +17,27 @@ typedef unsigned long cycles_t;
 #ifdef CONFIG_64BIT
 static inline cycles_t get_cycles(void)
 {
-	return readq_relaxed(clint_time_val);
+	if (static_branch_likely(&riscv_csr_time_available))
+		return csr_read(CSR_TIME);
+	else
+		return readq_relaxed(clint_time_val);
 }
 #else /* !CONFIG_64BIT */
 static inline u32 get_cycles(void)
 {
-	return readl_relaxed(((u32 *)clint_time_val));
+	if (static_branch_likely(&riscv_csr_time_available))
+		return csr_read(CSR_TIME);
+	else
+		return readl_relaxed(((u32 *)clint_time_val));
 }
 #define get_cycles get_cycles
 
 static inline u32 get_cycles_hi(void)
 {
-	return readl_relaxed(((u32 *)clint_time_val) + 1);
+	if (static_branch_likely(&riscv_csr_time_available))
+		return csr_read(CSR_TIMEH);
+	else
+		return readl_relaxed(((u32 *)clint_time_val) + 1);
 }
 #define get_cycles_hi get_cycles_hi
 #endif /* CONFIG_64BIT */
@@ -40,7 +49,8 @@ static inline u32 get_cycles_hi(void)
  */
 static inline unsigned long random_get_entropy(void)
 {
-	if (unlikely(clint_time_val == NULL))
+	if (!static_branch_likely(&riscv_csr_time_available) &&
+	   (unlikely(clint_time_val == NULL)))
 		return random_get_entropy_fallback();
 	return get_cycles();
 }
diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c
index f468fa8bf5f0..acfcd78cc576 100644
--- a/drivers/clocksource/timer-clint.c
+++ b/drivers/clocksource/timer-clint.c
@@ -39,10 +39,13 @@ static u64 __iomem *clint_timer_cmp;
 static u64 __iomem *clint_timer_val;
 static unsigned long clint_timer_freq;
 static unsigned int clint_timer_irq;
+static bool is_c900_clint;
 
 #ifdef CONFIG_RISCV_M_MODE
 u64 __iomem *clint_time_val;
 EXPORT_SYMBOL(clint_time_val);
+DEFINE_STATIC_KEY_FALSE(riscv_csr_time_available);
+EXPORT_SYMBOL(riscv_csr_time_available);
 #endif
 
 #ifdef CONFIG_SMP
@@ -79,17 +82,27 @@ static void clint_ipi_interrupt(struct irq_desc *desc)
 #ifdef CONFIG_64BIT
 static u64 notrace clint_get_cycles64(void)
 {
-	return clint_get_cycles();
+	if (static_branch_likely(&riscv_csr_time_available))
+		return csr_read(CSR_TIME);
+	else
+		return clint_get_cycles();
 }
 #else /* CONFIG_64BIT */
 static u64 notrace clint_get_cycles64(void)
 {
 	u32 hi, lo;
 
-	do {
-		hi = clint_get_cycles_hi();
-		lo = clint_get_cycles();
-	} while (hi != clint_get_cycles_hi());
+	if (static_branch_likely(&riscv_csr_time_available)) {
+		do {
+			hi = clint_get_cycles_hi();
+			lo = clint_get_cycles();
+		} while (hi != clint_get_cycles_hi());
+	} else {
+		do {
+			hi = csr_read(CSR_TIMEH);
+			lo = csr_read(CSR_TIME);
+		} while (hi != csr_read(CSR_TIMEH));
+	}
 
 	return ((u64)hi << 32) | lo;
 }
@@ -128,6 +141,19 @@ static int clint_clock_shutdown(struct clock_event_device *evt)
 	return 0;
 }
 
+static int c900_clint_clock_next_event(unsigned long delta,
+				       struct clock_event_device *ce)
+{
+	void __iomem *r = clint_timer_cmp +
+			  cpuid_to_hartid_map(smp_processor_id());
+	u64 val = clint_get_cycles64() + delta;
+
+	csr_set(CSR_IE, IE_TIE);
+	writel_relaxed(val, r);
+	writel_relaxed(val >> 32, r + 4);
+	return 0;
+}
+
 static DEFINE_PER_CPU(struct clock_event_device, clint_clock_event) = {
 	.name				= "clint_clockevent",
 	.features			= CLOCK_EVT_FEAT_ONESHOT,
@@ -141,6 +167,9 @@ static int clint_timer_starting_cpu(unsigned int cpu)
 {
 	struct clock_event_device *ce = per_cpu_ptr(&clint_clock_event, cpu);
 
+	if (is_c900_clint)
+		ce->set_next_event = c900_clint_clock_next_event;
+
 	ce->cpumask = cpumask_of(cpu);
 	clockevents_config_and_register(ce, clint_timer_freq, 100, ULONG_MAX);
 
@@ -284,5 +313,14 @@ static int __init clint_timer_init_dt(struct device_node *np)
 	return rc;
 }
 
+static int __init c900_clint_timer_init_dt(struct device_node *np)
+{
+	is_c900_clint = true;
+	static_branch_enable(&riscv_csr_time_available);
+
+	return clint_timer_init_dt(np);
+}
+
 TIMER_OF_DECLARE(clint_timer, "riscv,clint0", clint_timer_init_dt);
 TIMER_OF_DECLARE(clint_timer1, "sifive,clint0", clint_timer_init_dt);
+TIMER_OF_DECLARE(clint_timer2, "thead,c900-clint", c900_clint_timer_init_dt);
-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] riscv: nommu: remove PAGE_OFFSET hardcoding
  2024-04-10 14:23 ` [PATCH v3 1/2] riscv: nommu: remove PAGE_OFFSET hardcoding Jisheng Zhang
@ 2024-04-10 15:30   ` Clément Léger
  2024-04-11  9:12     ` Clément Léger
  0 siblings, 1 reply; 8+ messages in thread
From: Clément Léger @ 2024-04-10 15:30 UTC (permalink / raw
  To: Jisheng Zhang, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Daniel Lezcano, Thomas Gleixner, Samuel Holland, Conor Dooley
  Cc: linux-riscv, linux-kernel



On 10/04/2024 16:23, Jisheng Zhang wrote:
> Currently, PAGE_OFFSET is hardcoded as 0x8000_0000, it works fine since
> there's only one nommu platform in the mainline. However, there are
> many cases where the (S)DRAM base address isn't 0x8000_0000, so remove
> the hardcoding value, and introduce DRAM_BASE which will be set by

Hi Jisheng,

Typo: s/harcoding/hardcoded

> users during configuring. DRAM_BASE is 0x8000_0000 by default.
> 
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>  arch/riscv/Kconfig | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 7895c77545f1..b4af1df86352 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -247,10 +247,16 @@ config MMU
>  	  Select if you want MMU-based virtualised addressing space
>  	  support by paged memory management. If unsure, say 'Y'.
>  
> +if !MMU
> +config DRAM_BASE
> +	hex '(S)DRAM Base Address'
> +	default 0x80000000
> +endif

I'm not sure but it feels odd to have this at top level config menu.
Maybe it would make more sense for this to be located under the
"Platform Type" section ?

Thanks,

Clément

> +
>  config PAGE_OFFSET
>  	hex
>  	default 0xC0000000 if 32BIT && MMU
> -	default 0x80000000 if !MMU
> +	default DRAM_BASE if !MMU
>  	default 0xff60000000000000 if 64BIT
>  
>  config KASAN_SHADOW_OFFSET

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] riscv: nommu: remove PAGE_OFFSET hardcoding
  2024-04-10 15:30   ` Clément Léger
@ 2024-04-11  9:12     ` Clément Léger
  0 siblings, 0 replies; 8+ messages in thread
From: Clément Léger @ 2024-04-11  9:12 UTC (permalink / raw
  To: Jisheng Zhang, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Daniel Lezcano, Thomas Gleixner, Samuel Holland, Conor Dooley
  Cc: linux-riscv, linux-kernel



On 10/04/2024 17:30, Clément Léger wrote:
> 
> 
> On 10/04/2024 16:23, Jisheng Zhang wrote:
>> Currently, PAGE_OFFSET is hardcoded as 0x8000_0000, it works fine since
>> there's only one nommu platform in the mainline. However, there are
>> many cases where the (S)DRAM base address isn't 0x8000_0000, so remove
>> the hardcoding value, and introduce DRAM_BASE which will be set by
> 
> Hi Jisheng,
> 
> Typo: s/harcoding/hardcoded
> 
>> users during configuring. DRAM_BASE is 0x8000_0000 by default.
>>
>> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
>> ---
>>  arch/riscv/Kconfig | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 7895c77545f1..b4af1df86352 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -247,10 +247,16 @@ config MMU
>>  	  Select if you want MMU-based virtualised addressing space
>>  	  support by paged memory management. If unsure, say 'Y'.
>>  
>> +if !MMU
>> +config DRAM_BASE
>> +	hex '(S)DRAM Base Address'
>> +	default 0x80000000
>> +endif
> 
> I'm not sure but it feels odd to have this at top level config menu.
> Maybe it would make more sense for this to be located under the
> "Platform Type" section ?
> 
> Thanks,
> 
> Clément
> 
>> +
>>  config PAGE_OFFSET
>>  	hex
>>  	default 0xC0000000 if 32BIT && MMU
>> -	default 0x80000000 if !MMU
>> +	default DRAM_BASE if !MMU
>>  	default 0xff60000000000000 if 64BIT

By the way, you should probably rebase that on top of Samuel's work [1]
in order to support !MMU is S-mode.

Thanks,

Clément

Link:
https://lore.kernel.org/lkml/20240227003630.3634533-5-samuel.holland@sifive.com/
[1]

>>  
>>  config KASAN_SHADOW_OFFSET

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/2] riscv: improve nommu and timer-clint
  2024-04-10 14:23 [PATCH v3 0/2] riscv: improve nommu and timer-clint Jisheng Zhang
  2024-04-10 14:23 ` [PATCH v3 1/2] riscv: nommu: remove PAGE_OFFSET hardcoding Jisheng Zhang
  2024-04-10 14:23 ` [PATCH v3 2/2] clocksource/drivers/timer-clint: Add T-Head C9xx clint Jisheng Zhang
@ 2024-04-20 11:04 ` Jisheng Zhang
  2 siblings, 0 replies; 8+ messages in thread
From: Jisheng Zhang @ 2024-04-20 11:04 UTC (permalink / raw
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano,
	Thomas Gleixner, Samuel Holland, Conor Dooley
  Cc: linux-riscv, linux-kernel, cleger

On Wed, Apr 10, 2024 at 10:23:45PM +0800, Jisheng Zhang wrote:
> As is known, the sophgo CV1800B contains so called little core, which
> is C906 w/o MMU, so I want to run nommu linux on it. This series is
> the result of the bring up. After this series, w/ proper dts, we can
> run nommu linux on milkv duo's little core.
> 
> First of all, patch1 removes the PAGE_OFFSET hardcoding by introducing
> DRAM_BASE Kconfig option.
> 
> Secondly, to use the T-HEAD C9xx clint in RISCV-M NOMMU env, we need
> to take care two points:
> 
> 1.The mtimecmp in T-Head C9xx clint only supports 32bit read/write,
> implement such support.
> 
> 2. As pointed out by commit ca7810aecdba ("lib: utils/timer: mtimer:
> add a quirk for lacking mtime register") of opensbi:
> 
> "T-Head developers surely have a different understanding of time CSR and
> CLINT's mtime register with SiFive ones, that they did not implement
> the mtime register at all -- as shown in openC906 source code, their
> time CSR value is just exposed at the top of their processor IP block
> and expects an external continous counter, which makes it not
> overrideable, and thus mtime register is not implemented, even not for
> reading. However, if CLINTEE is not enabled in T-Head's MXSTATUS
> extended CSR, these systems still rely on the mtimecmp registers to
> generate timer interrupts. This makes it necessary to implement T-Head
> C9xx CLINT support in OpenSBI MTIMER driver, which skips implementing
> reading mtime register and falls back to default code that reads time
> CSR."
> 
> So, we need to fall back to read time CSR instead of mtime register.
> Add riscv_csr_time_available static key for this purpose.
> 
> The second patch adds T-Head C9xxx clint support to timer-clint driver
> by taking care of above two points.
> 
> Since v2:
>   - drop CONFIG_CLINT_USE_CSR_INSTEADOF_MTIME, instead we use runtime
>     code patch to dynamically enable CSR TIME code path or MTIME code
>     path.
> 
> Since v1:
>   - fix c900_clint_timer_init_dt() defined but not used build warning
>   - add option CONFIG_CLINT_USE_CSR_INSTEADOF_MTIME instead of removing
>     mtime usage for all platforms, since not all platforms implement the
>     time CSR in HW in M mode.
>   - rebase on the timer-clint improvement series
> https://lore.kernel.org/linux-riscv/20240406111757.1597-1-jszhang@kernel.org/T/#t

Hi Palmer, Daniel,

This series itself relies on the timer-clint improvement series
https://lore.kernel.org/linux-riscv/20240406111757.1597-1-jszhang@kernel.org/T/#t

But as suggested by Clément, it's better to rebase the patch1 on
the Samuel's work which has been merged into riscv for-next,
So I have two dependencies now, how do I handle this series then?

Could I leave patch1 alone and move patch2 into the timer-clint
improvement series? Any suggestion?

Thanks in advance
> 
> 
> Jisheng Zhang (2):
>   riscv: nommu: remove PAGE_OFFSET hardcoding
>   clocksource/drivers/timer-clint: Add T-Head C9xx clint
> 
>  arch/riscv/Kconfig                |  8 +++++-
>  arch/riscv/include/asm/clint.h    |  2 ++
>  arch/riscv/include/asm/timex.h    | 18 +++++++++---
>  drivers/clocksource/timer-clint.c | 48 +++++++++++++++++++++++++++----
>  4 files changed, 66 insertions(+), 10 deletions(-)
> 
> -- 
> 2.43.0
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 2/2] clocksource/drivers/timer-clint: Add T-Head C9xx clint
  2024-04-10 14:23 ` [PATCH v3 2/2] clocksource/drivers/timer-clint: Add T-Head C9xx clint Jisheng Zhang
@ 2024-04-24 11:53   ` Conor Dooley
  2024-04-24 12:24     ` Emil Renner Berthing
  0 siblings, 1 reply; 8+ messages in thread
From: Conor Dooley @ 2024-04-24 11:53 UTC (permalink / raw
  To: Jisheng Zhang
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano,
	Thomas Gleixner, Samuel Holland, linux-riscv, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 3595 bytes --]

On Wed, Apr 10, 2024 at 10:23:47PM +0800, Jisheng Zhang wrote:
> To use the T-HEAD C9xx clint in RISCV-M NOMMU env, we need to take
> care two points:
> 
> 1.The mtimecmp in T-Head C9xx clint only supports 32bit read/write,
> implement such support.
> 
> 2. As pointed out by commit ca7810aecdba ("lib: utils/timer: mtimer:
> add a quirk for lacking mtime register") of opensbi:
> 
> "T-Head developers surely have a different understanding of time CSR and
> CLINT's mtime register with SiFive ones, that they did not implement
> the mtime register at all -- as shown in openC906 source code, their
> time CSR value is just exposed at the top of their processor IP block
> and expects an external continous counter, which makes it not
> overrideable, and thus mtime register is not implemented, even not for
> reading. However, if CLINTEE is not enabled in T-Head's MXSTATUS
> extended CSR, these systems still rely on the mtimecmp registers to
> generate timer interrupts. This makes it necessary to implement T-Head
> C9xx CLINT support in OpenSBI MTIMER driver, which skips implementing
> reading mtime register and falls back to default code that reads time
> CSR."
> 
> So, we need to fall back to read time CSR instead of mtime register.
> Add riscv_csr_time_available static key for this purpose.
> 
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>  arch/riscv/include/asm/clint.h    |  2 ++
>  arch/riscv/include/asm/timex.h    | 18 +++++++++---
>  drivers/clocksource/timer-clint.c | 48 +++++++++++++++++++++++++++----
>  3 files changed, 59 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/clint.h b/arch/riscv/include/asm/clint.h
> index 0789fd37b40a..c6057a182c5d 100644
> --- a/arch/riscv/include/asm/clint.h
> +++ b/arch/riscv/include/asm/clint.h
> @@ -10,6 +10,7 @@
>  #include <asm/mmio.h>
>  
>  #ifdef CONFIG_RISCV_M_MODE
> +#include <linux/jump_label.h>
>  /*
>   * This lives in the CLINT driver, but is accessed directly by timex.h to avoid
>   * any overhead when accessing the MMIO timer.
> @@ -21,6 +22,7 @@
>   * like "riscv_mtime", to signify that these non-ISA assumptions must hold.
>   */
>  extern u64 __iomem *clint_time_val;
> +DECLARE_STATIC_KEY_FALSE(riscv_csr_time_available);
>  #endif
>  
>  #endif
> diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
> index a06697846e69..007a15482d75 100644
> --- a/arch/riscv/include/asm/timex.h
> +++ b/arch/riscv/include/asm/timex.h
> @@ -17,18 +17,27 @@ typedef unsigned long cycles_t;
>  #ifdef CONFIG_64BIT
>  static inline cycles_t get_cycles(void)
>  {
> -	return readq_relaxed(clint_time_val);
> +	if (static_branch_likely(&riscv_csr_time_available))
> +		return csr_read(CSR_TIME);
> +	else
> +		return readq_relaxed(clint_time_val);
>  }
>  #else /* !CONFIG_64BIT */
>  static inline u32 get_cycles(void)
>  {
> -	return readl_relaxed(((u32 *)clint_time_val));
> +	if (static_branch_likely(&riscv_csr_time_available))
> +		return csr_read(CSR_TIME);
> +	else
> +		return readl_relaxed(((u32 *)clint_time_val));
>  }
>  #define get_cycles get_cycles
>  
>  static inline u32 get_cycles_hi(void)
>  {
> -	return readl_relaxed(((u32 *)clint_time_val) + 1);
> +	if (static_branch_likely(&riscv_csr_time_available))
> +		return csr_read(CSR_TIMEH);
> +	else
> +		return readl_relaxed(((u32 *)clint_time_val) + 1);
>  }

None of the else branches here need to actually be an else, since the
other branch returns. Otherwise, looks aight to me, thanks for the
update.


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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 2/2] clocksource/drivers/timer-clint: Add T-Head C9xx clint
  2024-04-24 11:53   ` Conor Dooley
@ 2024-04-24 12:24     ` Emil Renner Berthing
  0 siblings, 0 replies; 8+ messages in thread
From: Emil Renner Berthing @ 2024-04-24 12:24 UTC (permalink / raw
  To: Conor Dooley, Jisheng Zhang
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Daniel Lezcano,
	Thomas Gleixner, Samuel Holland, linux-riscv, linux-kernel

Conor Dooley wrote:
> On Wed, Apr 10, 2024 at 10:23:47PM +0800, Jisheng Zhang wrote:
> > To use the T-HEAD C9xx clint in RISCV-M NOMMU env, we need to take
> > care two points:
> >
> > 1.The mtimecmp in T-Head C9xx clint only supports 32bit read/write,
> > implement such support.
> >
> > 2. As pointed out by commit ca7810aecdba ("lib: utils/timer: mtimer:
> > add a quirk for lacking mtime register") of opensbi:
> >
> > "T-Head developers surely have a different understanding of time CSR and
> > CLINT's mtime register with SiFive ones, that they did not implement
> > the mtime register at all -- as shown in openC906 source code, their
> > time CSR value is just exposed at the top of their processor IP block
> > and expects an external continous counter, which makes it not
> > overrideable, and thus mtime register is not implemented, even not for
> > reading. However, if CLINTEE is not enabled in T-Head's MXSTATUS
> > extended CSR, these systems still rely on the mtimecmp registers to
> > generate timer interrupts. This makes it necessary to implement T-Head
> > C9xx CLINT support in OpenSBI MTIMER driver, which skips implementing
> > reading mtime register and falls back to default code that reads time
> > CSR."
> >
> > So, we need to fall back to read time CSR instead of mtime register.
> > Add riscv_csr_time_available static key for this purpose.
> >
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> >  arch/riscv/include/asm/clint.h    |  2 ++
> >  arch/riscv/include/asm/timex.h    | 18 +++++++++---
> >  drivers/clocksource/timer-clint.c | 48 +++++++++++++++++++++++++++----
> >  3 files changed, 59 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/clint.h b/arch/riscv/include/asm/clint.h
> > index 0789fd37b40a..c6057a182c5d 100644
> > --- a/arch/riscv/include/asm/clint.h
> > +++ b/arch/riscv/include/asm/clint.h
> > @@ -10,6 +10,7 @@
> >  #include <asm/mmio.h>
> >
> >  #ifdef CONFIG_RISCV_M_MODE
> > +#include <linux/jump_label.h>
> >  /*
> >   * This lives in the CLINT driver, but is accessed directly by timex.h to avoid
> >   * any overhead when accessing the MMIO timer.
> > @@ -21,6 +22,7 @@
> >   * like "riscv_mtime", to signify that these non-ISA assumptions must hold.
> >   */
> >  extern u64 __iomem *clint_time_val;
> > +DECLARE_STATIC_KEY_FALSE(riscv_csr_time_available);
> >  #endif
> >
> >  #endif
> > diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
> > index a06697846e69..007a15482d75 100644
> > --- a/arch/riscv/include/asm/timex.h
> > +++ b/arch/riscv/include/asm/timex.h
> > @@ -17,18 +17,27 @@ typedef unsigned long cycles_t;
> >  #ifdef CONFIG_64BIT
> >  static inline cycles_t get_cycles(void)
> >  {
> > -	return readq_relaxed(clint_time_val);
> > +	if (static_branch_likely(&riscv_csr_time_available))
> > +		return csr_read(CSR_TIME);
> > +	else
> > +		return readq_relaxed(clint_time_val);
> >  }
> >  #else /* !CONFIG_64BIT */
> >  static inline u32 get_cycles(void)
> >  {
> > -	return readl_relaxed(((u32 *)clint_time_val));
> > +	if (static_branch_likely(&riscv_csr_time_available))
> > +		return csr_read(CSR_TIME);
> > +	else
> > +		return readl_relaxed(((u32 *)clint_time_val));
> >  }
> >  #define get_cycles get_cycles
> >
> >  static inline u32 get_cycles_hi(void)
> >  {
> > -	return readl_relaxed(((u32 *)clint_time_val) + 1);
> > +	if (static_branch_likely(&riscv_csr_time_available))
> > +		return csr_read(CSR_TIMEH);
> > +	else
> > +		return readl_relaxed(((u32 *)clint_time_val) + 1);
> >  }
>
> None of the else branches here need to actually be an else, since the
> other branch returns. Otherwise, looks aight to me, thanks for the
> update.

Also the static key is initialized to false and the commit message says only
the C9xx cores need to use the CSR_TIME, so shouldn't it be

  if (static_branch_unlikely(&riscv_csr_time_available))

instead, or am I reading this wrong?

/Emil

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-04-24 12:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-10 14:23 [PATCH v3 0/2] riscv: improve nommu and timer-clint Jisheng Zhang
2024-04-10 14:23 ` [PATCH v3 1/2] riscv: nommu: remove PAGE_OFFSET hardcoding Jisheng Zhang
2024-04-10 15:30   ` Clément Léger
2024-04-11  9:12     ` Clément Léger
2024-04-10 14:23 ` [PATCH v3 2/2] clocksource/drivers/timer-clint: Add T-Head C9xx clint Jisheng Zhang
2024-04-24 11:53   ` Conor Dooley
2024-04-24 12:24     ` Emil Renner Berthing
2024-04-20 11:04 ` [PATCH v3 0/2] riscv: improve nommu and timer-clint Jisheng Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).