Re: [RFC 02/11] drivers: Add HTE subsystem

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 13 Sep 2021 22:43:02 -0700
Dipen Patel <dipenp@nvidia.com> wrote:

> Hi Jonathan,
> 
> I got some time to implement RFC version 2 while doing so I have a follow up comment
> 
> inline regarding clock source comment of yours.
> 
> Best Regards,
> 
> Dipen Patel
> 
...

> >>>> +/**
> >>>> + * struct hte_clk_info - Clock source info that HTE provider uses.
> >>>> + * The provider uses hardware clock as a source to timestamp real time. This
> >>>> + * structure presents the clock information to consumers. 
> >>>> + *
> >>>> + * @hz: Clock rate in HZ, for example 1KHz clock = 1000.
> >>>> + * @type: Clock type. CLOCK_* types.    
> >>> So this is something we got a it wrong in IIO. It's much better to define
> >>> a subset of clocks that can be potentially used.  There are some that make
> >>> absolutely no sense and consumers really don't want to have to deal with them.    
> >> Is there anything I have to change here?  
> > Yes - specify which clocks would make sense.  You might not need to explicitly
> > allow only those, but that might also be worthwhile. Otherwise, the chances are
> > you'll end up with a bunch of special purpose code in consumers on the basis
> > they might get CLOCK_TAI or similar and have to deal with it.
> > As for exactly which clocks do make sense, that's one which may take some figuring
> > out. Probably REALTIME, MONOTONIC and BOOTTIME depending on whether you care
> > what happens when the time of the system gets adjusted, or whether it carries
> > on measuring time across suspend.   Very application dependent but there are some
> > you can definitely rule out. Don't repeat my mistake of leaving it vague
> > (which incidentally was a follow up to picking a silly clock to use for timestamps
> >  before we allowed it to be configured).  
> 
> I believe your comment is under assumption that providers have choice in selecting
> 
> clock source to timestamp in turns clients have it as well. For now, the provider
> 
> I have implemented has single clock source and hence I only implemented get_clock*
> 
> hook that provider implement and client can retrieve that information. I guess I can
> 
> always implement set_clock* hook as well for the future providers which support
> 
> multiple clock sources. Please let me if I missed your point.

I'll be honest I can't really remember :(  too many sleeps.

Sorry - if it is still relevant perhaps it'll come back to me on v2.

Thanks,

Jonathan