Messages in this thread | | | From | Anup Patel <> | Date | Wed, 23 Nov 2022 12:11:47 +0530 | Subject | Re: [PATCH] cpuidle: riscv-sbi: Stop using non-retentive suspend |
| |
On Wed, Nov 23, 2022 at 11:57 AM Samuel Holland <samuel@sholland.org> wrote: > > On 11/23/22 00:10, Anup Patel wrote: > > On Wed, Nov 23, 2022 at 11:08 AM Samuel Holland <samuel@sholland.org> wrote: > >> > >> Hi Anup, > >> > >> On 11/22/22 23:35, Anup Patel wrote: > >>> On Wed, Nov 23, 2022 at 10:41 AM Samuel Holland <samuel@sholland.org> wrote: > >>>> On 11/22/22 09:28, Palmer Dabbelt wrote: > >>>>> I also think we should stop entering non-retentive suspend until we can > >>>>> sort out how reliably wake up from it, as the SBI makes that a > >>>>> platform-specific detail. If the answer there is "non-retentive suspend > >>>>> is fine on the D1 as long as we don't use the SBI timers" then that > >>>>> seems fine, we just need some way to describe that in Linux -- that > >>>>> doesn't fix other platforms and other interrupts, but at least it's a > >>>>> start. > >>>> > >>>> We need some way to describe the situation from the SBI implementation > >>>> to Linux. > >>>> > >>>> Non-retentive suspend is fine on the D1 as long as either one of these > >>>> conditions is met: > >>>> 1) we don't use the SBI timers, or > >>>> 2) the SBI timer implementation does not use the CLINT > >>>> > >>>> And it is up to the SBI implementation which timer hardware it uses, so > >>>> the SBI implementation needs to patch this information in to the DT at > >>>> runtime. > >>> > >>> Rather than SBI implementation patching information in DT, it is much > >>> simpler to add a quirk in RISC-V timer driver for D1 platform (i.e. based > >>> on D1 compatible string in root node). > >> > >> It would be simpler, but it would be wrong, as I just explained. > >> > >> Only the SBI implementation knows if the SBI timer extension can wake > >> any given CPU from any given non-retentive suspend state. > > > > The SBI implementation would derive this information from platform > > compatible string which is already available to the Linux kernel so > > why does SBI implementation have to patch the DTB and put the > > same information in a different way ? > > It is not the same information. The SBI implementation also chooses, at > runtime, which timer hardware (CLINT, platform-specific MMIO timer, > etc.) is used to implement the SBI timer extension. The value of the > sbi-timer-can-wake-cpu property depends on this choice. > > Using D1 as an example, there are two MMIO timer peripherals ("sun4i" > TIMER and "sun5i" HSTIMER) where the sbi-timer-can-wake-cpu property > should be set. But the property should not be set if the CLINT is used > by SBI. > > It would be perfectly reasonable for the SBI implementation to claim one > of the wakeup-capable MMIO timers for itself, mark it as "reserved" in > the DT passed to Linux, and thus force Linux to use the SBI timer or a > native CLINT driver (C906 CLINT has S-mode extensions). Then the SBI > timer _would_ be capable of waking the CPU from non-retentive suspend.
Fair enough but the DT property should not be SBI specific because same situation can happen with Sstc as well where a particular non-retentive state does not preserve the state of stimecmp CSRs in the HARTs.
Better to keep the DT property name as "riscv,timer-can-wake-cpu".
Regards, Anup
| |