linux-kernel - Re: [PATCH v2 2/4] dt-bindings: ARM: Mediatek: Document devicetree bindings for clock controllers

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]

Message-ID: <CAGS+omAo-d_bK=2HDu98xHvqhnVgJUBy4jbYVGKqsfWRX-EfDA@mail.gmail.com>
Date:	Fri, 3 Jul 2015 18:46:33 +0800
From:	Daniel Kurtz <djkurtz@...omium.org>
To:	Stephen Boyd <sboyd@...eaurora.org>
Cc:	James Liao <jamesjj.liao@...iatek.com>,
	"open list:OPEN FIRMWARE AND..." <devicetree@...r.kernel.org>,
	Mike Turquette <mturquette@...aro.org>,
	srv_heupstream <srv_heupstream@...iatek.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Ricky Liang <jcliang@...omium.org>,
	Rob Herring <robh+dt@...nel.org>,
	linux-mediatek@...ts.infradead.org,
	Sascha Hauer <kernel@...gutronix.de>,
	Matthias Brugger <matthias.bgg@...il.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 2/4] dt-bindings: ARM: Mediatek: Document devicetree
 bindings for clock controllers

On Fri, Jul 3, 2015 at 7:40 AM, Stephen Boyd <sboyd@...eaurora.org> wrote:
> On 07/01/2015 09:26 PM, Daniel Kurtz wrote:
>> On Thu, Jul 2, 2015 at 10:52 AM, James Liao <jamesjj.liao@...iatek.com> wrote:
>>> Hi Daniel,
>>>
>>>>> +Required Properties:
>>>>> +
>>>>> +- compatible: Should be:
>>>>> +       - "mediatek,mt8173-imgsys", "syscon"
>>>>> +- #clock-cells: Must be 1
>>>>> +
>>>>> +The imgsys controller uses the common clk binding from
>>>>> +Documentation/devicetree/bindings/clock/clock-bindings.txt
>>>>> +The available clocks are defined in dt-bindings/clock/mt*-clk.h.
>>>>> +
>>>>> +Example:
>>>>> +
>>>>> +imgsys: imgsys@...00000 {
>>>> Since these nodes will be supplying clocks to the rest of the system,
>>>> I think the "name" part of each of these should all be
>>>> "clock-controller", like topckgen and apmixedsys:
>>>>
>>>>   imgsys: clock-controller@...00000 {
>>> These subsystems (and topckgen also) also contains other functions such
>>> as reset controller, which may be implemented in clk/mediatek/ in the
>>> future. It is suitable to use "clock-controller" as their name?
>> Hmm,
>>
>> I don't know the "right way" to do this either.
>> Pardon me if you've already had these discussions.
>> I only recently started looking at these clock nodes in detail :-).
>>
>> I think what we really have in register space is a "syscon", as
>> described in [0]:
>> [0] Documentation/devicetree/bindings/mfd/syscon.txt
>>
>> So, we can define this block of registers as a syscon:
>>
>> mmsys_syscon: syscon@...00000 {
>>        compatible = "mediatek,mt8173-mmsys", "syscon";
>>        reg = <0 0x14000000 0 0x1000>;
>> };
>>
>>
>> Then for the clock controller functionality, we create a node with a
>> "clock-controller" name and a "-clock" compatible, like this:
>>
>> mmsys_clock: clock-controller {
>>        compatible = "mediatek,mt8173-mmsys-clock";
>>        #clock-cells = <1>;
>>        mediatek,syscon = <&mmsys_syscon>;
>> };
>>
>> You could then do:
>> CLK_OF_DECLARE(mtk_mmsys, "mediatek,mt8173-mmsys-clock", mtk_mmsys_init);
>>
>>
>> If you want to reuse the same register range for some other
>> functionality, we could then use a different node, with a different
>> compatible:
>>
>> mmsys: reset-controller {
>>        compatible = "mediatek,mt8173-mmsys-reset";
>>        mediatek,syscon = <&mmsys_syscon>;
>> };
>>
>> What do you think of this approach?
>
> DT nodes typically have a reg property. Not having a reg property is a
> good indicator of a problem with the binding. A syscon is used when you
> have a DT node with a reg property and some driver attached to it, but
> you need to poke some bits in another register region that isn't part of
> the reg property. Instead of having multiple nodes with two reg
> properties where the second one is the same, we use a phandle and a syscon.
>
> If clock-controller isn't acceptable maybe clock-reset-contoller would
> work? Or "power-controller"? We certainly shouldn't be making up
> multiple nodes for one hardware block. Of course, the subject of the
> patch is "bindings for clock controllers", so it may be that the
> registers are predominantly clock related and so the name is appropriate
> already.

Using "clock-controller" seems to fit best with the bindings
introduced by this patch.

However, if these bindings are for hardware blocks that contain a grab
bag of various functionality that will be added in later patches, then
I think "syscon" might be best.

-Dan

>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
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/