Messages in this thread | | | Date | Tue, 29 Nov 2022 10:43:48 -0800 (PST) | Subject | Re: [PATCH v4 3/3] clocksource: timer-riscv: Set CLOCK_EVT_FEAT_C3STOP based on DT | From | Palmer Dabbelt <> |
| |
On Tue, 29 Nov 2022 09:22:22 PST (-0800), apatel@ventanamicro.com wrote: > On Tue, Nov 29, 2022 at 10:47 PM Conor Dooley <conor@kernel.org> wrote: >> >> On Tue, Nov 29, 2022 at 10:41:09PM +0530, Anup Patel wrote: >> > On Tue, Nov 29, 2022 at 8:06 PM Conor Dooley <conor.dooley@microchip.com> wrote: >> > > >> > > On Tue, Nov 29, 2022 at 07:33:13PM +0530, Anup Patel wrote: >> > > > We should set CLOCK_EVT_FEAT_C3STOP for a clock_event_device only >> > > > when riscv,timer-cant-wake-up DT property is present in the RISC-V >> > > > timer DT node. >> > > > >> > > > This way CLOCK_EVT_FEAT_C3STOP feature is set for clock_event_device >> > > > based on RISC-V platform capabilities rather than having it set for >> > > > all RISC-V platforms. >> > > > >> > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> >> > > >> > > I thought I had left an R-b on this one? >> > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> >> > > >> > > Also, I think that we need to backport *something* that disables C3STOP >> > > which is why I had suggested keeping the revert in place. >> > > Patch 1 of this series only solves the timer issues but does not restore >> > > sleep states to their prior behaviour, right? >> > > Either this patch or the revert needs to go to stable IMO. >> > >> > Since it works for you with the C3STOP set and broadcast timer enabled, >> > we can directly go with this patch. I am fine including the revert as well. >> >> I don't mind which gets backported. To me, this one is preferable as it >> is more "complete" but it is a bit on the new feature side of things, >> no? >> >> Whoever applies it can decide, and I'll backport the revert if they >> decide that this patch is not stable material :)
IIRC the clock folks took the original C3 patch, so that's probably the best way to take these as well?
>> >> Thanks again for helping sort this mess out, I see it helped with your >> IPI series too! > > Yes, I was surprised to see that it helped the IPI series as well. > Thanks for your patch. > > Regards, > Anup > >> >> Conor. >> >> > > > --- >> > > > drivers/clocksource/timer-riscv.c | 12 +++++++++++- >> > > > 1 file changed, 11 insertions(+), 1 deletion(-) >> > > > >> > > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c >> > > > index 969a552da8d2..0c8bdd168a45 100644 >> > > > --- a/drivers/clocksource/timer-riscv.c >> > > > +++ b/drivers/clocksource/timer-riscv.c >> > > > @@ -28,6 +28,7 @@ >> > > > #include <asm/timex.h> >> > > > >> > > > static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available); >> > > > +static bool riscv_timer_cant_wake_cpu; >> > > > >> > > > static int riscv_clock_next_event(unsigned long delta, >> > > > struct clock_event_device *ce) >> > > > @@ -51,7 +52,7 @@ static int riscv_clock_next_event(unsigned long delta, >> > > > static unsigned int riscv_clock_event_irq; >> > > > static DEFINE_PER_CPU(struct clock_event_device, riscv_clock_event) = { >> > > > .name = "riscv_timer_clockevent", >> > > > - .features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP, >> > > > + .features = CLOCK_EVT_FEAT_ONESHOT, >> > > > .rating = 100, >> > > > .set_next_event = riscv_clock_next_event, >> > > > }; >> > > > @@ -85,6 +86,8 @@ static int riscv_timer_starting_cpu(unsigned int cpu) >> > > > >> > > > ce->cpumask = cpumask_of(cpu); >> > > > ce->irq = riscv_clock_event_irq; >> > > > + if (riscv_timer_cant_wake_cpu) >> > > > + ce->features |= CLOCK_EVT_FEAT_C3STOP; >> > > > clockevents_config_and_register(ce, riscv_timebase, 100, 0x7fffffff); >> > > > >> > > > enable_percpu_irq(riscv_clock_event_irq, >> > > > @@ -139,6 +142,13 @@ static int __init riscv_timer_init_dt(struct device_node *n) >> > > > if (cpuid != smp_processor_id()) >> > > > return 0; >> > > > >> > > > + child = of_find_compatible_node(NULL, NULL, "riscv,timer"); >> > > > + if (child) { >> > > > + riscv_timer_cant_wake_cpu = of_property_read_bool(child, >> > > > + "riscv,timer-cant-wake-cpu"); >> > > > + of_node_put(child); >> > > > + } >> > > > + >> > > > domain = NULL; >> > > > child = of_get_compatible_child(n, "riscv,cpu-intc"); >> > > > if (!child) { >> > > > -- >> > > > 2.34.1 >> > > >
| |