linux-kernel - Re: [PATCH v2 3/6] clk: hi6220: Document devicetree bindings for hi6220 clock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 14 Apr 2015 20:37:11 +0800
From:	Bintian <bintian.wang@...wei.com>
To:	Arnd Bergmann <arnd@...db.de>,
	<linux-arm-kernel@...ts.infradead.org>
CC:	<mark.rutland@....com>, <dan.zhao@...ilicon.com>,
	<btw@...l.itp.ac.cn>, <catalin.marinas@....com>,
	<wangbinghui@...ilicon.com>, <will.deacon@....com>,
	<huxinwei@...wei.com>, <haojian.zhuang@...aro.org>,
	<yanhaifeng@...il.com>, <rob.herring@...aro.org>,
	<mturquette@...aro.org>, <victor.lixin@...ilicon.com>,
	<xuwei5@...ilicon.com>, <jh80.chung@...sung.com>,
	<sledge.yanwei@...wei.com>, <kong.kongxinwei@...ilicon.com>,
	<heyunlei@...wei.com>, <puck.chen@...ilicon.com>,
	<zhangfei.gao@...aro.org>, <z.liuxinliang@...wei.com>,
	<devicetree@...r.kernel.org>, <khilman@...aro.org>,
	<pawel.moll@....com>, <ijc+devicetree@...lion.org.uk>,
	<tyler.baker@...aro.org>, <olof@...om.net>, <robh+dt@...nel.org>,
	<linux@....linux.org.uk>, <zhenwei.wang@...ilicon.com>,
	<w.f@...wei.com>, <guodong.xu@...aro.org>,
	<tomeu.vizoso@...labora.com>, <sboyd@...eaurora.org>,
	<linux-kernel@...r.kernel.org>, <galak@...eaurora.org>,
	<xuejiancheng@...wei.com>, <jorge.ramirez-ortiz@...aro.org>,
	<xuyiping@...ilicon.com>, <liguozhu@...ilicon.com>
Subject: Re: [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
>
> .
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/