| Message ID | 20190927084819.645-1-benjamin.gaignard@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: dts: stm32: Enable high resolution timer | expand |
On 2019-09-27 09:48, Benjamin Gaignard wrote: > Adding always-on makes arm arch_timer claim to be an high resolution > timer. > That is possible because power mode won't stop clocking the timer. The "always-on" is not about the clock. It is about the comparator. The clock itself is *guaranteed* to always tick. If it didn't, that'd be an integration bug, and a pretty bad one. What you're claiming here is that your CPU never enters a low-power mode? Ever? I find this very hard to believe. Furthermore, claiming that always-on is the way to force the arch-timer to be an hrtimer is factually wrong. This is what happens *if* this is the only timer in the system. The only case this is true is for virtual machines. Anything else has a global timer somewhere that will allow the arch timers to be used as an hrtimer. I'm pretty sure you too have a global timer somewhere in your system. Enable it, and enjoy hrtimers without having to lie about the properties of your system! ;-) M. > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com> > --- > arch/arm/boot/dts/stm32mp157c.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi > b/arch/arm/boot/dts/stm32mp157c.dtsi > index 9b11654a0a39..74f64745d60d 100644 > --- a/arch/arm/boot/dts/stm32mp157c.dtsi > +++ b/arch/arm/boot/dts/stm32mp157c.dtsi > @@ -50,6 +50,7 @@ > <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; > interrupt-parent = <&intc>; > + always-on; > }; > > clocks {
On 9/27/19 1:22 PM, Marc Zyngier wrote: > On 2019-09-27 09:48, Benjamin Gaignard wrote: >> Adding always-on makes arm arch_timer claim to be an high resolution >> timer. >> That is possible because power mode won't stop clocking the timer. > > The "always-on" is not about the clock. It is about the comparator. > The clock itself is *guaranteed* to always tick. If it didn't, that'd be > an integration bug, and a pretty bad one. > > What you're claiming here is that your CPU never enters a low-power mode? > Ever? I find this very hard to believe. > > Furthermore, claiming that always-on is the way to force the arch-timer > to be an hrtimer is factually wrong. This is what happens *if* this is > the only timer in the system. The only case this is true is for virtual > machines. Anything else has a global timer somewhere that will allow > the arch timers to be used as an hrtimer. > > I'm pretty sure you too have a global timer somewhere in your system. > Enable it, and enjoy hrtimers without having to lie about the properties > of your system! ;-) Hi Marc, This SoC doesn't have any other global timer. Use arch_time is the only we have to provide hrtimer on this system. Benjamin > > M. > >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com> >> --- >> arch/arm/boot/dts/stm32mp157c.dtsi | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi >> b/arch/arm/boot/dts/stm32mp157c.dtsi >> index 9b11654a0a39..74f64745d60d 100644 >> --- a/arch/arm/boot/dts/stm32mp157c.dtsi >> +++ b/arch/arm/boot/dts/stm32mp157c.dtsi >> @@ -50,6 +50,7 @@ >> <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | >> IRQ_TYPE_LEVEL_LOW)>, >> <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | >> IRQ_TYPE_LEVEL_LOW)>; >> interrupt-parent = <&intc>; >> + always-on; >> }; >> >> clocks { >
On 2019-09-27 13:36, Benjamin GAIGNARD wrote: > On 9/27/19 1:22 PM, Marc Zyngier wrote: >> On 2019-09-27 09:48, Benjamin Gaignard wrote: >>> Adding always-on makes arm arch_timer claim to be an high >>> resolution >>> timer. >>> That is possible because power mode won't stop clocking the timer. >> >> The "always-on" is not about the clock. It is about the comparator. >> The clock itself is *guaranteed* to always tick. If it didn't, >> that'd be >> an integration bug, and a pretty bad one. >> >> What you're claiming here is that your CPU never enters a low-power >> mode? >> Ever? I find this very hard to believe. >> >> Furthermore, claiming that always-on is the way to force the >> arch-timer >> to be an hrtimer is factually wrong. This is what happens *if* this >> is >> the only timer in the system. The only case this is true is for >> virtual >> machines. Anything else has a global timer somewhere that will allow >> the arch timers to be used as an hrtimer. >> >> I'm pretty sure you too have a global timer somewhere in your >> system. >> Enable it, and enjoy hrtimers without having to lie about the >> properties >> of your system! ;-) > > Hi Marc, > > This SoC doesn't have any other global timer. Use arch_time is the > only > we have to provide hrtimer on this system. And you don't have any form of power management either? What happens when your CPU goes into idle? If your system does any form of power management *and* doesn't have a separate timer, it is remarkably broken. M.
On 9/27/19 2:41 PM, Marc Zyngier wrote: > On 2019-09-27 13:36, Benjamin GAIGNARD wrote: >> On 9/27/19 1:22 PM, Marc Zyngier wrote: >>> On 2019-09-27 09:48, Benjamin Gaignard wrote: >>>> Adding always-on makes arm arch_timer claim to be an high resolution >>>> timer. >>>> That is possible because power mode won't stop clocking the timer. >>> >>> The "always-on" is not about the clock. It is about the comparator. >>> The clock itself is *guaranteed* to always tick. If it didn't, >>> that'd be >>> an integration bug, and a pretty bad one. >>> >>> What you're claiming here is that your CPU never enters a low-power >>> mode? >>> Ever? I find this very hard to believe. >>> >>> Furthermore, claiming that always-on is the way to force the arch-timer >>> to be an hrtimer is factually wrong. This is what happens *if* this is >>> the only timer in the system. The only case this is true is for virtual >>> machines. Anything else has a global timer somewhere that will allow >>> the arch timers to be used as an hrtimer. >>> >>> I'm pretty sure you too have a global timer somewhere in your system. >>> Enable it, and enjoy hrtimers without having to lie about the >>> properties >>> of your system! ;-) >> >> Hi Marc, >> >> This SoC doesn't have any other global timer. Use arch_time is the only >> we have to provide hrtimer on this system. > > And you don't have any form of power management either? What happens when > your CPU goes into idle? If your system does any form of power management > *and* doesn't have a separate timer, it is remarkably broken. Even in low-power modes this timer is always powered and clocked so it is working fine. > > M.
On 2019-09-27 13:44, Benjamin GAIGNARD wrote: > On 9/27/19 2:41 PM, Marc Zyngier wrote: >> On 2019-09-27 13:36, Benjamin GAIGNARD wrote: >>> On 9/27/19 1:22 PM, Marc Zyngier wrote: >>>> On 2019-09-27 09:48, Benjamin Gaignard wrote: >>>>> Adding always-on makes arm arch_timer claim to be an high >>>>> resolution >>>>> timer. >>>>> That is possible because power mode won't stop clocking the >>>>> timer. >>>> >>>> The "always-on" is not about the clock. It is about the >>>> comparator. >>>> The clock itself is *guaranteed* to always tick. If it didn't, >>>> that'd be >>>> an integration bug, and a pretty bad one. >>>> >>>> What you're claiming here is that your CPU never enters a >>>> low-power >>>> mode? >>>> Ever? I find this very hard to believe. >>>> >>>> Furthermore, claiming that always-on is the way to force the >>>> arch-timer >>>> to be an hrtimer is factually wrong. This is what happens *if* >>>> this is >>>> the only timer in the system. The only case this is true is for >>>> virtual >>>> machines. Anything else has a global timer somewhere that will >>>> allow >>>> the arch timers to be used as an hrtimer. >>>> >>>> I'm pretty sure you too have a global timer somewhere in your >>>> system. >>>> Enable it, and enjoy hrtimers without having to lie about the >>>> properties >>>> of your system! ;-) >>> >>> Hi Marc, >>> >>> This SoC doesn't have any other global timer. Use arch_time is the >>> only >>> we have to provide hrtimer on this system. >> >> And you don't have any form of power management either? What happens >> when >> your CPU goes into idle? If your system does any form of power >> management >> *and* doesn't have a separate timer, it is remarkably broken. > > Even in low-power modes this timer is always powered and clocked so > it > is working fine. You're missing the point again. It is not about the clock, but the comparator that is internal to the CPU, and not functional when the CPU is in its lowest power mode. See also the verbiage in [1] (44.3 STGEN functional description), which indicates that the clock source actually dies in low-power mode (going against the architecture which mandates it to be always-on). Also, coming back to your earlier assertion ("This SoC doesn't have any other global timer"): The documentation at [1] shows at least 17 timers that could be used and avoid this dirty hack. So for what it is worth, NAK to this patch. M. [1] https://www.st.com/resource/en/reference_manual/dm00366355.pdf
On Fri, Sep 27, 2019 at 12:44:55PM +0000, Benjamin GAIGNARD wrote: [...] > > Even in low-power modes this timer is always powered and clocked so it > is working fine. > Is that tested ? I see only cpu_{on,off} available on this platform with PSCI v0.1. Did you add cpu_suspend, idle-states and then gave it a spin ? Or do you have some other idle driver with which this is tested ? Also I don't understand how "always-on" is linked to hrtimer. Always on timers are just selected to be broadcast timer without sacrificing(simply keeping) a cpu to be always active for broadcast purposes. -- Regards, Sudeep
On 9/27/19 2:59 PM, Marc Zyngier wrote: > On 2019-09-27 13:44, Benjamin GAIGNARD wrote: >> On 9/27/19 2:41 PM, Marc Zyngier wrote: >>> On 2019-09-27 13:36, Benjamin GAIGNARD wrote: >>>> On 9/27/19 1:22 PM, Marc Zyngier wrote: >>>>> On 2019-09-27 09:48, Benjamin Gaignard wrote: >>>>>> Adding always-on makes arm arch_timer claim to be an high resolution >>>>>> timer. >>>>>> That is possible because power mode won't stop clocking the timer. >>>>> >>>>> The "always-on" is not about the clock. It is about the comparator. >>>>> The clock itself is *guaranteed* to always tick. If it didn't, >>>>> that'd be >>>>> an integration bug, and a pretty bad one. >>>>> >>>>> What you're claiming here is that your CPU never enters a low-power >>>>> mode? >>>>> Ever? I find this very hard to believe. >>>>> >>>>> Furthermore, claiming that always-on is the way to force the >>>>> arch-timer >>>>> to be an hrtimer is factually wrong. This is what happens *if* >>>>> this is >>>>> the only timer in the system. The only case this is true is for >>>>> virtual >>>>> machines. Anything else has a global timer somewhere that will allow >>>>> the arch timers to be used as an hrtimer. >>>>> >>>>> I'm pretty sure you too have a global timer somewhere in your system. >>>>> Enable it, and enjoy hrtimers without having to lie about the >>>>> properties >>>>> of your system! ;-) >>>> >>>> Hi Marc, >>>> >>>> This SoC doesn't have any other global timer. Use arch_time is the >>>> only >>>> we have to provide hrtimer on this system. >>> >>> And you don't have any form of power management either? What happens >>> when >>> your CPU goes into idle? If your system does any form of power >>> management >>> *and* doesn't have a separate timer, it is remarkably broken. >> >> Even in low-power modes this timer is always powered and clocked so it >> is working fine. > > You're missing the point again. It is not about the clock, but the > comparator > that is internal to the CPU, and not functional when the CPU is in its > lowest > power mode. See also the verbiage in [1] (44.3 STGEN functional > description), > which indicates that the clock source actually dies in low-power mode > (going > against the architecture which mandates it to be always-on). > > Also, coming back to your earlier assertion ("This SoC doesn't have > any other > global timer"): The documentation at [1] shows at least 17 timers that > could > be used and avoid this dirty hack. > > So for what it is worth, NAK to this patch. Hi Marc, I have listen your remarks and propose another way to solve this issue: https://lkml.org/lkml/2019/10/9/690 Thanks, Benjamin > > M. > > [1] https://www.st.com/resource/en/reference_manual/dm00366355.pdf
On 2019-10-14 10:31, Benjamin GAIGNARD wrote: > On 9/27/19 2:59 PM, Marc Zyngier wrote: >> On 2019-09-27 13:44, Benjamin GAIGNARD wrote: >>> On 9/27/19 2:41 PM, Marc Zyngier wrote: >>>> On 2019-09-27 13:36, Benjamin GAIGNARD wrote: >>>>> On 9/27/19 1:22 PM, Marc Zyngier wrote: >>>>>> On 2019-09-27 09:48, Benjamin Gaignard wrote: >>>>>>> Adding always-on makes arm arch_timer claim to be an high >>>>>>> resolution >>>>>>> timer. >>>>>>> That is possible because power mode won't stop clocking the >>>>>>> timer. >>>>>> >>>>>> The "always-on" is not about the clock. It is about the >>>>>> comparator. >>>>>> The clock itself is *guaranteed* to always tick. If it didn't, >>>>>> that'd be >>>>>> an integration bug, and a pretty bad one. >>>>>> >>>>>> What you're claiming here is that your CPU never enters a >>>>>> low-power >>>>>> mode? >>>>>> Ever? I find this very hard to believe. >>>>>> >>>>>> Furthermore, claiming that always-on is the way to force the >>>>>> arch-timer >>>>>> to be an hrtimer is factually wrong. This is what happens *if* >>>>>> this is >>>>>> the only timer in the system. The only case this is true is for >>>>>> virtual >>>>>> machines. Anything else has a global timer somewhere that will >>>>>> allow >>>>>> the arch timers to be used as an hrtimer. >>>>>> >>>>>> I'm pretty sure you too have a global timer somewhere in your >>>>>> system. >>>>>> Enable it, and enjoy hrtimers without having to lie about the >>>>>> properties >>>>>> of your system! ;-) >>>>> >>>>> Hi Marc, >>>>> >>>>> This SoC doesn't have any other global timer. Use arch_time is >>>>> the >>>>> only >>>>> we have to provide hrtimer on this system. >>>> >>>> And you don't have any form of power management either? What >>>> happens >>>> when >>>> your CPU goes into idle? If your system does any form of power >>>> management >>>> *and* doesn't have a separate timer, it is remarkably broken. >>> >>> Even in low-power modes this timer is always powered and clocked so >>> it >>> is working fine. >> >> You're missing the point again. It is not about the clock, but the >> comparator >> that is internal to the CPU, and not functional when the CPU is in >> its >> lowest >> power mode. See also the verbiage in [1] (44.3 STGEN functional >> description), >> which indicates that the clock source actually dies in low-power >> mode >> (going >> against the architecture which mandates it to be always-on). >> >> Also, coming back to your earlier assertion ("This SoC doesn't have >> any other >> global timer"): The documentation at [1] shows at least 17 timers >> that >> could >> be used and avoid this dirty hack. >> >> So for what it is worth, NAK to this patch. > > > Hi Marc, > > I have listen your remarks and propose another way to solve this > issue: > > https://lkml.org/lkml/2019/10/9/690 I don't think you have. You're just trying to move the same dirty hack to another place instead of properly describing your hardware, and Thomas has pointed you in the same direction. M.
diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi index 9b11654a0a39..74f64745d60d 100644 --- a/arch/arm/boot/dts/stm32mp157c.dtsi +++ b/arch/arm/boot/dts/stm32mp157c.dtsi @@ -50,6 +50,7 @@ <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; interrupt-parent = <&intc>; + always-on; }; clocks {
Adding always-on makes arm arch_timer claim to be an high resolution timer. That is possible because power mode won't stop clocking the timer. Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com> --- arch/arm/boot/dts/stm32mp157c.dtsi | 1 + 1 file changed, 1 insertion(+)