lkml.org 
[lkml]   [2015]   [Apr]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 3/6] clk: hi6220: Document devicetree bindings for hi6220 clock
Hello Arnd,

On 2015/4/14 18:19, Arnd Bergmann wrote:
> On Tuesday 14 April 2015 11:35:26 Bintian wrote:
>> Hello Arnd,
>>
>> On 2015/4/13 23:32, Arnd Bergmann wrote:
>>> On Monday 13 April 2015 17:17:37 Bintian Wang wrote:
>>>> +- compatible: the compatible should be one of the following strings to
>>>> + indicate the clock controller functionality.
>>>> +
>>>> + - "hisilicon,aoctrl"
>>>> + - "hisilicon,sysctrl"
>>>> + - "hisilicon,mediactrl"
>>>> + - "hisilicon,pmctrl"
>>>> +
>>>>
>>>
>>> These ones already have bindings, you can't reuse the strings.
>>> Please work with someone in hisilicon to set up a registry of
>>> device names so you can avoid conflicts in the future.
>> All the clock registers are under above four system controllers,
>> discussed with Mark and Haojian two months ago, I think using above
>> same four binding strings is enough for clk module.
>> On second thoughts, there really some problems for future hisilicon
>> code upstream, how about change back to the first version of this
>> patch set, just like following:
>> + sys_ctrl: sys_ctrl {
>> + compatible = "hisilicon,sysctrl", "syscon";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + reg = <0x0 0xf7030000 0x0 0x2000>;
>> + ranges = <0 0x0 0xf7030000 0x2000>;
>> +
>> + clock_sys: clock1@0 {
>> + compatible = "hisilicon,hi6220-clock-sys";
>> + reg = <0 0x1000>;
>> + #clock-cells = <1>;
>> + };
>> + };
>
> Sub-nodes are fine, but you also can't have a device node that
> is just 'compatible = "hisilicon,sysctrl", "syscon";' when
> hisilicon,sysctrl can refer to multiple mutually incompatible
> controllers.
>
> The minimum fix would be to mandate the string to be
> compatible = "hisilicon,hi6220-sysctrl", "hisilicon,sysctrl", "syscon";
> for this model, and use respective compatible strings for the other
> chips.
It's really a smart fix!

For the four system controllers, how about change the following strings:
+ - "hisilicon,aoctrl"
+ - "hisilicon,sysctrl"
+ - "hisilicon,mediactrl"
+ - "hisilicon,pmctrl"

to
+ - "hisilicon,hi6220-aoctrl"
+ - "hisilicon,hi6220-sysctrl"
+ - "hisilicon,hi6220-mediactrl"
+ - "hisilicon,hi6220-pmctrl"
?

and I also use "hisilicon,hi6220-xxxx" for hi6220 clk driver directly ?

>
> For the documentation, I think it would make sense to move that
> description to "hisilicon,sysctrl" and list all the variants of that
> component and the respective compatible strings there, rather than
> have one document per chip and list multiple components in it.
Sure,

Thanks,

Bintian
>
> Arnd
>
> .
>



\
 
 \ /
  Last update: 2015-04-14 15:21    [W:0.099 / U:0.608 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site