Re: [PATCH 2/5] arm: shmobile: r7s72100: add i2c clocks

[Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>]

Hello.

On 18-12-2013 18:44, Magnus Damm wrote:

>>>>>>>> @@ -173,6 +179,10 @@ static struct clk_lookup lookups[] = {
>>>>>>>>          CLKDEV_CON_ID("mtu2_fck", &mstp_clks[MSTP33]),

>>>>>>>>          /* ICK */
>>>>>>>> +       CLKDEV_DEV_ID("fcfee000.i2c", &mstp_clks[MSTP97]),
>>>>>>>> +       CLKDEV_DEV_ID("fcfee400.i2c", &mstp_clks[MSTP96]),
>>>>>>>> +       CLKDEV_DEV_ID("fcfee800.i2c", &mstp_clks[MSTP95]),
>>>>>>>> +       CLKDEV_DEV_ID("fcfeec00.i2c", &mstp_clks[MSTP94]),

>>>>>>>      These belong to some other place, the group marked by /* ICK */
>>>>>>> is only for CLKDEV_ICK_ID().

>>>>>> So, I'll create a /* DEV */ prefix?

>>>>>      I really don't know. Other places have /* MSTP */ comment in this
>>>>> case despite all clocks, CLKDEV_DEV_ID() and CLKDEV_ICK_ID() are
>>>>> really MSTP clocks. I considered the idea of separating
>>>>> CLKDEV_ICK_ID() under /* ICK */ comment silly from the very start
>>>>> but Simon didn't listen to me.

>>>> I am puzzled, too. ICK is a type of registration and not a clock domain.
>>>> Also, there is 'mtu2_fck' which is under ICK as well as MSTP? Looks
>>>> wrong. From what I understand now, removing the /* ICK */ comment would
>>>> be easiest and proper?

>>> I'm not sure that I really understand what all the fuss is about.

>>> As I understand things the convention that prevails for
>>> MSTP clocks under mach-shmobile is as follows:

>>> 1. Clocks not registered by CLKDEV_ICK_ID() are grouped together
>>>      under /* MSTP */ followed by:
>>> 2. Clocks registered using CLKDEV_ICK_ID() are grouped together
>>>      under /* ICK */

>>> I am unsure of the historical reason for this

>>     Recent patches by Morimoto-san.

>>> but it does seem to be consistent.

>>     No, it doesn't. These comments are *clearly* not consistent and should be
>> removed at least.

> Feel free to contribute patches!

    Of course, in my copious free time. I was against these ICKy comments (and 
the patches introducing them) in the first place but my opinion didn't count. 
I'm not sure it will count if I go and submit the patches (but the time will 
be lost).

WBR, Sergei