Re: [PATCH v2 1/2] devicetree: add binding for generic mmio clocksource

["Måns Rullgård" <mans@mansr.com>]

Stephen Boyd <sboyd@codeaurora.org> writes:

> On 10/09, Rob Herring wrote:
>> +Stephen who has worked on this code.
>> 
>> On Fri, Oct 9, 2015 at 11:19 AM, Måns Rullgård <mans@mansr.com> wrote:
>> > Måns Rullgård <mans@mansr.com> writes:
>> >
>> >> Rob Herring <robherring2@gmail.com> writes:
>> >>
>> >>> On Wed, Oct 7, 2015 at 11:47 AM, Måns Rullgård <mans@mansr.com> wrote:
>> >>>> What would be a proper way to select a sched_clock source?  I realise
>> >>>> it's a Linux-specific thing and DT is supposed to be generic, but the
>> >>>> information must be provided somehow.
>> >>>
>> >>> The kernel already has some logic to do this. Most number of bits
>> >>> followed by highest frequency will be the winning sched_clock. You
>> >>> might also want to look at things like always on or not.
>> >>
>> >> The problem is that sched_clock_register() doesn't take a pointer to be
>> >> passed back to the read_sched_clock callback like most interfaces of
>> >> this type do.  This means the callback must use global variables set up
>> >> before the register call, but at that time there's no way of knowing
>> >> which one will be used.  If there were a way of getting a pointer to the
>> >> callback, it would be a simple matter of registering all instances and
>> >> letting the kernel choose which to use.
>> >
>> > Anyone got a comment on this?  Do I have to send a patch adding this
>> > before anyone will tell me why it's a bad idea?  (That method almost
>> > always works.)
>> 
>> Adding a ptr to the callback seems fine to me.
>> 
>
> Does that mean a flag day? Urgh. Pain. I'm not opposed to adding
> a pointer, in fact it might be better for performance so that we
> don't take a cache miss in read() functions that need to load
> some pointer. We were talking about that problem a few months
> ago, but nothing came of it.

Flag day in what sense?  There aren't all that many users of the
interface (56, to be precise), and sched_clock_register() isn't
exported.  Verifying the change will be a minor pain, but I don't see
why it should have any major consequences.  Obviously I'd just set the
pointer to null for existing users and leave it for the respective
maintainers to make proper use of it where sensible.

-- 
Måns Rullgård
mans@mansr.com