> -----Original Message----- > From: Daniel Lezcano [mailto:daniel.lezc...@linaro.org] > Sent: Tuesday, May 21, 2019 8:20 PM > To: Jacky Bai <ping....@nxp.com>; t...@linutronix.de; robh...@kernel.org; > shawn...@kernel.org; mark.rutl...@arm.com; Aisheng Dong > <aisheng.d...@nxp.com> > Cc: linux-kernel@vger.kernel.org; devicet...@vger.kernel.org; dl-linux-imx > <linux-...@nxp.com> > Subject: Re: [PATCH v4 2/2] driver: clocksource: Add nxp system counter timer > driver support > > On 21/05/2019 14:01, Jacky Bai wrote: > > > >> -----Original Message----- > >> From: Daniel Lezcano [mailto:daniel.lezc...@linaro.org] > >> Sent: Tuesday, May 21, 2019 6:08 PM > >> To: Jacky Bai <ping....@nxp.com>; t...@linutronix.de; > >> robh...@kernel.org; shawn...@kernel.org; mark.rutl...@arm.com; > >> Aisheng Dong <aisheng.d...@nxp.com> > >> Cc: linux-kernel@vger.kernel.org; devicet...@vger.kernel.org; > >> dl-linux-imx <linux-...@nxp.com> > >> Subject: Re: [PATCH v4 2/2] driver: clocksource: Add nxp system > >> counter timer driver support > >> > >> On 21/05/2019 09:18, Jacky Bai wrote: > >>> From: Bai Ping <ping....@nxp.com> > >>> > >>> The system counter (sys_ctr) is a programmable system counter which > >>> provides a shared time base to the Cortex A15, A7, A53 etc cores. > >>> It is intended for use in applications where the counter is always > >>> powered on and supports multiple, unrelated clocks. The sys_ctr > >>> hardware > >>> supports: > >>> - 56-bit counter width (roll-over time greater than 40 years) > >> > >> The benefit of using more than 32bits on a 32bits system is not proven. > >> > > > > It is mainly used on 64bit ARMv8 system. > > Oh, ok. Fair enough. > > > > >> The function to read and build the 56bits value can have a very > >> significant impact on the performance of your platform. > >> > >> Using a 32bits counter can be enough if it does not wrap too fast. > >> > >> Can you consider a 32 bits counter ? > > > > this counter is ARMv8 arch timer's counter source. As it also has > > timer function, so I choose it to act as a broadcast timer for cpuidle. The > timer interrupt can only be triggered when 'compare[55:0] <= counter[55:0]'. > > So you mean that only use the lower 32bit to implement this timer? If so, I > can change to use only the lower 32bit. > > IMO it is preferable but you decide (probably compare with how long it takes > to wrap when 32bits). >
Normally, it is driven by fixed 8MHz clock. 32bit has risk, when the lower 32bit wrapped, then the 'counter value >= compare value' is true if only use 32bit for timer, then IRQ will Pending all the time. I will keep use the whole 56bit. > >