Re: [PATCH 1/3] clk: exynos5420: Add 5800 specific clocks

[Tomasz Figa <tomasz.figa@gmail.com>]

On 02.05.2014 21:35, Doug Anderson wrote:
> Arnd,
>
> On Fri, May 2, 2014 at 11:52 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Friday 02 May 2014 18:33:39 Arun Kumar K wrote:
>>> From: Alim Akhtar <alim.akhtar@samsung.com>
>>>
>>> Exynos5800 clock structure is mostly similar to 5420 with only
>>> a small delta changes. So the 5420 clock file is re-used for
>>> 5800 also. The common clocks for both are seggreagated and few
>>> clocks which are different for both are separately initialized.
>>>
>>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>>> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
>>
>> This isn't about your specific patch, but every time I see a new
>> exynos variant get supported, it comes with a clock driver patch
>> that is at least as big as all the other patches combined.
>>
>> New variants come out all the time now, and we are starting to
>> accumulate huge amounts of clock definitions both in the source
>> and the binary. I think we should try to come up with a better
>> way to represent the clocks. I don't think any other SoC
>> family is nearly as bad as Exynos, either because they have
>> much fewer models, or because they abstract their clocks more
>> and put all the tables into DT.
>>
>> I'm definitely not saying no to the exynos5800 addition for this,
>> but I'm starting to get a little annoyed, and I think it would be
>> good to come up with a new clock binding before we see 64-bit
>> Exynos variants.
>
> One thing to note: your suggestion will almost certainly not be
> conducive to get stable device trees.  IMHO there's pretty much a zero
> chance that you could properly describe all of the exynos clocks in
> the first, second, third, or twentieth attempt.  That means that if
> anyone ever took it in their head to actually ship a device tree that
> wasn't bundled with the kernel that it would probably be wrong.
>
> Declaring just "I have exynos5800 clocks" means that you're not
> relying on the device tree.
>
>
> The clocks are pretty table-based as-is, and I think that's about the
> best you're going to get.

+1 and similarly to pinctrl stuff. Both full-DT and table-based 
approaches were being discussed long time ago when moving Exynos to DT 
and generic frameworks and the conclusion was clearly in favor of the 
latter.

Moreover, I don't think we should really be concerned about this, 
because we already have far less changes (not counting device tree 
sources) needed to support a SoC than we had before, in board file 
times. Not even saying that new SoCs are not being added that often.

>
>
> NOTE: one could argue that possibly the 5420 and 5800 are different
> enough that they ought to have separate tables.  I don't feel like I'm
> in enough of an ownership position to make that tradeoff either way,
> though.

I don't have the datasheets, but looking at the changes needed, they 
don't seem to be more different than Exynos4210 and Exynos4x12. The 
approach taken looks fine for me.

Best regards,
Tomasz