On Mon, 13 Sep 2021 22:43:02 -0700 Dipen Patel <dipenp@xxxxxxxxxx> 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
