From: Jisheng Zhang <jszhang@kernel.org> To: Bo Gan <ganboing@gmail.com> Cc: Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>, Daniel Lezcano <daniel.lezcano@linaro.org>, Thomas Gleixner <tglx@linutronix.de>, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 5/5] clocksource/drivers/timer-clint: Add T-Head C9xx clint support Date: Tue, 26 Mar 2024 09:25:05 +0800 [thread overview] Message-ID: <ZgIj8QQ7zO_eSZcA@xhacker> (raw) In-Reply-To: <72eac56e-61d4-e42f-cfbd-8bcc35ed7bb6@gmail.com> On Mon, Mar 25, 2024 at 03:22:11PM -0700, Bo Gan wrote: > On 3/25/24 9:40 AM, Jisheng Zhang wrote: > > The mtimecmp in T-Head C9xx clint only supports 32bit read/write, > > implement such support. > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > --- > > drivers/clocksource/timer-clint.c | 24 ++++++++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c > > index 4537c77e623c..71188732e8a3 100644 > > --- a/drivers/clocksource/timer-clint.c > > +++ b/drivers/clocksource/timer-clint.c > > @@ -34,6 +34,7 @@ static unsigned int clint_ipi_irq; > > static u64 __iomem *clint_timer_cmp; > > static unsigned long clint_timer_freq; > > static unsigned int clint_timer_irq; > > +static bool is_c900_clint; > > #ifdef CONFIG_SMP > > static void clint_send_ipi(unsigned int cpu) > > @@ -88,6 +89,19 @@ static int clint_clock_next_event(unsigned long delta, > > 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); > Perhaps you should do a writel_relaxed(-1, r) here. just like openSBI, because the update > to mtimecmp is now split into 2 parts. > https://github.com/riscv-software-src/opensbi/blob/v1.4/lib/utils/timer/aclint_mtimer.c#L54 Thanks, I also noticed the mtimecmp update on 32bit platforms doesn't strictly follow the riscv spec: # New comparand is in a1:a0. li t0, -1 la t1, mtimecmp sw t0, 0(t1) # No smaller than old value. sw a1, 4(t1) # No smaller than new value. sw a0, 0(t1) # New value. So A new bug fix patch will be added in v2. > > > + 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, > > @@ -99,6 +113,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); > > @@ -233,5 +250,12 @@ 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; > > + 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", clint_timer_init_dt); > > > Better use a more generic term to describe the fact that mtimecmp doesn't support > 64-bit mmio, just like what openSBI is currently doing, instead of making it c900 specific: This has been mentioned in commit msg, but adding a code comment seems fine. > > https://github.com/riscv-software-src/opensbi/blob/v1.4/lib/utils/timer/fdt_timer_mtimer.c#L152 > > Then your `is_c900_clint` becomes something like `timecmp_64bit_mmio`. > > Bo
WARNING: multiple messages have this Message-ID (diff)
From: Jisheng Zhang <jszhang@kernel.org> To: Bo Gan <ganboing@gmail.com> Cc: Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>, Daniel Lezcano <daniel.lezcano@linaro.org>, Thomas Gleixner <tglx@linutronix.de>, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 5/5] clocksource/drivers/timer-clint: Add T-Head C9xx clint support Date: Tue, 26 Mar 2024 09:25:05 +0800 [thread overview] Message-ID: <ZgIj8QQ7zO_eSZcA@xhacker> (raw) In-Reply-To: <72eac56e-61d4-e42f-cfbd-8bcc35ed7bb6@gmail.com> On Mon, Mar 25, 2024 at 03:22:11PM -0700, Bo Gan wrote: > On 3/25/24 9:40 AM, Jisheng Zhang wrote: > > The mtimecmp in T-Head C9xx clint only supports 32bit read/write, > > implement such support. > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > --- > > drivers/clocksource/timer-clint.c | 24 ++++++++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c > > index 4537c77e623c..71188732e8a3 100644 > > --- a/drivers/clocksource/timer-clint.c > > +++ b/drivers/clocksource/timer-clint.c > > @@ -34,6 +34,7 @@ static unsigned int clint_ipi_irq; > > static u64 __iomem *clint_timer_cmp; > > static unsigned long clint_timer_freq; > > static unsigned int clint_timer_irq; > > +static bool is_c900_clint; > > #ifdef CONFIG_SMP > > static void clint_send_ipi(unsigned int cpu) > > @@ -88,6 +89,19 @@ static int clint_clock_next_event(unsigned long delta, > > 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); > Perhaps you should do a writel_relaxed(-1, r) here. just like openSBI, because the update > to mtimecmp is now split into 2 parts. > https://github.com/riscv-software-src/opensbi/blob/v1.4/lib/utils/timer/aclint_mtimer.c#L54 Thanks, I also noticed the mtimecmp update on 32bit platforms doesn't strictly follow the riscv spec: # New comparand is in a1:a0. li t0, -1 la t1, mtimecmp sw t0, 0(t1) # No smaller than old value. sw a1, 4(t1) # No smaller than new value. sw a0, 0(t1) # New value. So A new bug fix patch will be added in v2. > > > + 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, > > @@ -99,6 +113,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); > > @@ -233,5 +250,12 @@ 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; > > + 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", clint_timer_init_dt); > > > Better use a more generic term to describe the fact that mtimecmp doesn't support > 64-bit mmio, just like what openSBI is currently doing, instead of making it c900 specific: This has been mentioned in commit msg, but adding a code comment seems fine. > > https://github.com/riscv-software-src/opensbi/blob/v1.4/lib/utils/timer/fdt_timer_mtimer.c#L152 > > Then your `is_c900_clint` becomes something like `timecmp_64bit_mmio`. > > Bo _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2024-03-26 1:38 UTC|newest] Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-03-25 16:40 [PATCH 0/5] riscv: improve nommu and timer-clint Jisheng Zhang 2024-03-25 16:40 ` Jisheng Zhang 2024-03-25 16:40 ` [PATCH 1/5] riscv: nommu: remove PAGE_OFFSET hardcoding Jisheng Zhang 2024-03-25 16:40 ` Jisheng Zhang 2024-03-25 22:46 ` Bo Gan 2024-03-25 22:46 ` Bo Gan 2024-03-26 1:28 ` Jisheng Zhang 2024-03-26 1:28 ` Jisheng Zhang 2024-03-26 2:32 ` Samuel Holland 2024-03-26 2:32 ` Samuel Holland 2024-03-25 16:40 ` [PATCH 2/5] riscv: nommu: use CSR_TIME* for get_cycles* implementation Jisheng Zhang 2024-03-25 16:40 ` Jisheng Zhang 2024-03-26 2:39 ` Samuel Holland 2024-03-26 2:39 ` Samuel Holland 2024-03-26 16:29 ` Jisheng Zhang 2024-03-26 16:29 ` Jisheng Zhang 2024-03-27 7:58 ` [External] " yunhui cui 2024-03-27 7:58 ` yunhui cui 2024-03-28 10:11 ` Jisheng Zhang 2024-03-28 10:11 ` Jisheng Zhang 2024-03-25 16:40 ` [PATCH 3/5] clocksource/drivers/timer-clint: Remove clint_time_val Jisheng Zhang 2024-03-25 16:40 ` Jisheng Zhang 2024-03-25 16:40 ` [PATCH 4/5] clocksource/drivers/timer-clint: Use get_cycles() Jisheng Zhang 2024-03-25 16:40 ` Jisheng Zhang 2024-03-25 16:40 ` [PATCH 5/5] clocksource/drivers/timer-clint: Add T-Head C9xx clint support Jisheng Zhang 2024-03-25 16:40 ` Jisheng Zhang 2024-03-25 16:50 ` Jisheng Zhang 2024-03-25 16:50 ` Jisheng Zhang 2024-03-25 22:22 ` Bo Gan 2024-03-25 22:22 ` Bo Gan 2024-03-26 1:25 ` Jisheng Zhang [this message] 2024-03-26 1:25 ` Jisheng Zhang 2024-03-26 1:31 ` Jisheng Zhang 2024-03-26 1:31 ` Jisheng Zhang 2024-03-26 16:33 ` Jisheng Zhang 2024-03-26 16:33 ` Jisheng Zhang 2024-03-27 22:53 ` kernel test robot 2024-03-27 22:53 ` kernel test robot
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=ZgIj8QQ7zO_eSZcA@xhacker \ --to=jszhang@kernel.org \ --cc=aou@eecs.berkeley.edu \ --cc=daniel.lezcano@linaro.org \ --cc=ganboing@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=palmer@dabbelt.com \ --cc=paul.walmsley@sifive.com \ --cc=tglx@linutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.