Re: [PATCH v4 3/5] clk: dt: binding for basic multiplexer clock

[Sylwester Nawrocki <sylvester.nawrocki@gmail.com>]

On 09/05/2013 10:30 PM, Stephen Warren wrote:
> On 09/05/2013 12:29 PM, Mike Turquette wrote:
>> >  On Wed, Sep 4, 2013 at 11:36 AM, Stephen Warren<swarren@wwwdotorg.org>  wrote:
>>> >>  On 09/03/2013 05:22 PM, Mike Turquette wrote:
>>>> >>>  Quoting Stephen Warren (2013-08-30 14:37:46)
>>>>> >>>>  On 08/30/2013 02:33 PM, Mike Turquette wrote:
>>> >>  ...
>>>>>> >>>>>  The clock_data_  seems to always have some churn to it. Moving it out to
>>>>>> >>>>>  DT reduces that churn from Linux. My concern above is not about kernel
>>>>>> >>>>>  data size.
>>>>> >>>>
>>>>> >>>>  That sounds like the opposite of what we should be doing.
>>>>> >>>>
>>>>> >>>>  It's fine for kernel code/data to change; that's a natural part of
>>>>> >>>>  development. Obviously, we should minimize churn, through thorough
>>>>> >>>>  review, domain knowledge, etc.
>>>> >>>
>>>> >>>  And with the "clock mapping" style bindings we'll end up changing both
>>>> >>>  the DT binding definition and the kernel. Not great.
>>> >>
>>> >>  What's a "clock mapping" style binding? I guess that means the style
>>> >>  where you have a single DT node that provides multiple clocks, rather
>>> >>  than one DT node per clock?
>>> >>
>>> >>  If the kernel driver changes its internal data, I don't see why that
>>> >>  would have any impact at all on the DT binding definition. We should be
>>> >>  able to use one DT binding definition with arbitrary drivers.
>> >
>> >  Yes, I'm referring to a single node providing multiple clocks. As an
>> >  example see the Exynos 5420 binding:
>> >  Documentation/devicetree/bindings/clock/exynos5420-clock.txt
>> >
>> >  The clock id's are stored as part of the binding definition resulting
>> >  in a mapping scheme that can be fragile.
>
> The mapping shouldn't be fragile if e.g.
> include/dt-bindings/clock/exynos5420.h were used to define the values.
> That way, both the Exynos clock driver and Exynos DT files could both
> include the header, and would always be in sync.

It was our intention to have things done this way since first time the idea
of the preprocessor support in dtc was proposed, and since very initial
versions of the exynos clocks driver. I took some time but there have been
recently already posted patches moving the values definition to common
headers [1].

[1] http://www.spinics.net/lists/arm-kernel/msg271807.html

--
Thanks,
Sylwester