[PATCH 2/5] clk: bcm2835: enable management of PCM clock

Martin Sperl
2016-01-10T10:55:48Z

> On 10.01.2016, at 10:30, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Arnd,
>
> On Sat, Jan 9, 2016 at 9:56 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Saturday 09 January 2016 09:25:54 kernel at martin.sperl.org wrote:
>>> --- a/include/dt-bindings/clock/bcm2835.h
>>> +++ b/include/dt-bindings/clock/bcm2835.h
>>> @@ -44,5 +44,6 @@
>>> #define BCM2835_CLOCK_EMMC 28
>>> #define BCM2835_CLOCK_PERI_IMAGE 29
>>> #define BCM2835_CLOCK_PWM 30
>>> +#define BCM2835_CLOCK_PCM 31
>>>
>>> -#define BCM2835_CLOCK_COUNT 31
>>> +#define BCM2835_CLOCK_COUNT 32
>>
>> The last line contains an incompatible change, please don't do that.
>> If you have to add another clock, do that after the BCM2835_CLOCK_COUNT
>> definition to avoid changing dts files that use that number.
>
> While I agree this changes dts files (in an unexpected way?), not updating
> BCM2835_CLOCK_COUNT makes it definition useless. Which teaches that
> having such definitions in DT headers is not a good idea in the first place...
>
> Hence it can better be replaced (it seems to be unused in dts files, but you
> can keep the definition to be 100% sure) by an ARRAY_SIZE() in the C driver.
> This requires changing the driver to e.g. initialize clks[] in
> bcm2835_clk_probe() based on a table instead of explicit code.

That is a more general issue with the clock driver (and there
have been already some discussions around this when implementing
the PWM clock.

So if someone with a better idea how to keep those dt-binding includes
synchronized with the definitions used by the code step forward and
propose a better solution how to get that implemented?

I guess there will be a few more occurrences of clocks that are
currently not defined, which will need to get introduced in the future
PWM and PCM were just the last in this series.

Thanks,
Martin