Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932076AbbGCKrD (ORCPT ); Fri, 3 Jul 2015 06:47:03 -0400 Received: from mail-yk0-f173.google.com ([209.85.160.173]:34223 "EHLO mail-yk0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754898AbbGCKqx (ORCPT ); Fri, 3 Jul 2015 06:46:53 -0400 MIME-Version: 1.0 In-Reply-To: <5595CBDC.7090101@codeaurora.org> References: <1435633127-31952-1-git-send-email-jamesjj.liao@mediatek.com> <1435633127-31952-3-git-send-email-jamesjj.liao@mediatek.com> <1435805534.3526.23.camel@mtksdaap41> <5595CBDC.7090101@codeaurora.org> From: Daniel Kurtz Date: Fri, 3 Jul 2015 18:46:33 +0800 X-Google-Sender-Auth: bYGS_mOqOVLcY7X-ZEmkxacPUoQ Message-ID: Subject: Re: [PATCH v2 2/4] dt-bindings: ARM: Mediatek: Document devicetree bindings for clock controllers To: Stephen Boyd Cc: James Liao , "open list:OPEN FIRMWARE AND..." , Mike Turquette , srv_heupstream , "linux-kernel@vger.kernel.org" , Ricky Liang , Rob Herring , linux-mediatek@lists.infradead.org, Sascha Hauer , Matthias Brugger , "linux-arm-kernel@lists.infradead.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3836 Lines: 101 On Fri, Jul 3, 2015 at 7:40 AM, Stephen Boyd wrote: > On 07/01/2015 09:26 PM, Daniel Kurtz wrote: >> On Thu, Jul 2, 2015 at 10:52 AM, James Liao 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@15000000 { >>>> 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@15000000 { >>> 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@14000000 { >> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/