Re: [PATCH 4/8] docs: counter: add unit timer sysfs attributes

[William Breathitt Gray <vilhelm.gray@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3785 bytes --]

On Mon, Nov 01, 2021 at 01:08:46PM +0900, William Breathitt Gray wrote:
> On Sat, Oct 30, 2021 at 11:40:27AM -0500, David Lechner wrote:
> > On 10/28/21 2:59 AM, William Breathitt Gray wrote:
> > > On Wed, Oct 27, 2021 at 10:30:36AM -0500, David Lechner wrote:
> > >> On 10/27/21 1:46 AM, William Breathitt Gray wrote:
> > >>> On Sat, Oct 16, 2021 at 08:33:39PM -0500, David Lechner wrote:
> > >>>> This documents new unit timer sysfs attributes for the counter
> > >>>> subsystem.
> > >>>>
> > >>>> Signed-off-by: David Lechner <david@lechnology.com>
> > >>>
> > >>> Hello David,
> > >>>
> > >>> The unit timer is effectively a Count in its own right, so instead of
> > >>> introducing new sysfs attributes you can just implement it as another
> > >>> Count in the driver. Count 0 is "QPOSCNT", so set the name of this new
> > >>> Count 1 as "Unit Timer" (or the datasheet naming if more apt) to
> > >>> differentiate the Counts. You can then provide the "unit_timer_enable",
> > >>> "unit_timer_period", and "unit_timer_time" functionalities as respective
> > >>> Count 1 extensions ("enable" and "period") and Count 1 "count".
> > > 
> > > Actually if the counter function here is COUNTER_FUNCTION_DECREASE, then
> > 
> > It is an increasing counter.
> > 
> > > instead of introducing a new "period" extension, define this as a
> > > "ceiling" extension; that's what ceiling represents in the Counter
> > > interface: "the upper limit for the respective counter", which is the
> > > period of a timer counting down to a timeout.
> > 
> > In one of the other patches, you made a comment about the semantics
> > of ceiling with relation to the overflow event. We can indeed treat
> > the timer as a counter and the period as the ceiling. However, the
> > unit timer event occurs when the count is equal to the period (ceiling)
> > whereas an overflow event occurs when the count exceeds the ceiling.
> > So what would this event be called in generic counter terms? "timeout"
> > doesn't seem right.
> 
> Okay, so COUNTER_EVENT_THRESHOLD would be the respective Counter event
> type for this behavior because the event triggers once a threshold is
> reached (ceiling in this case).
> 
> But implementing the unit timer as a counter might not be the best path
> forward as you've mentioned below.
> 
> > > 
> > > William Breathitt Gray
> > > 
> > >>>
> > >>> If you believe it appropriate, you can provide the raw timer ticks via
> > >>> the Count 1 "count" while a nanoseconds interface is provided via a
> > >>> Count 1 extension "timeout" (or something similar).
> > >>>
> > 
> > One area where this concept of treating a timer as a counter potentially
> > breaks down is the issue of CPU frequency scaling. By treating the unit
> > timer as a timer, then the kernel could take care of any changes in clock
> > rate internally by automatically adjusting the prescalar and period on
> > rate change events. But if we are just treating it as a counter, then we
> > should probably just have an attribute that provides the clock rate and
> > if we want to support CPU frequency scaling, add an event that indicates
> > that the clock rate changed.
> 
> You're right, treating the unit timer as a counter might not be the most
> appropriate interface. Because this is a timer afterall, perhaps
> exposing this via the hrtimer API is better. You then have an existing
> interface available designed for timer configuration, and you can
> leverage the struct hrtimer function callback to handle your timeout
> interrupts.
> 
> William Breathitt Gray

Sorry, I think I meant the clockevents framework, not hrtimers. I'm not
as familiar with timers but perhaps you know more than I do here.

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]