From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sylwester Nawrocki Subject: Re: [PATCH v3 12/13] ARM: dts: Add S5K5BA sensor regulator definitions for Trats board Date: Mon, 08 Jul 2013 16:12:56 +0200 Message-ID: <51DAC8E8.2070405@samsung.com> References: <1372692155-17653-1-git-send-email-s.nawrocki@samsung.com> <1372692155-17653-13-git-send-email-s.nawrocki@samsung.com> <11736061.3CXfgF4Q9h@flatron> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: Received: from mailout4.w1.samsung.com ([210.118.77.14]:56880 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751442Ab3GHOM7 (ORCPT ); Mon, 8 Jul 2013 10:12:59 -0400 Received: from eucpsbgm1.samsung.com (unknown [203.254.199.244]) by mailout4.w1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0MPM00328FFZW470@mailout4.w1.samsung.com> for linux-samsung-soc@vger.kernel.org; Mon, 08 Jul 2013 15:12:57 +0100 (BST) In-reply-to: <11736061.3CXfgF4Q9h@flatron> Sender: linux-samsung-soc-owner@vger.kernel.org List-Id: linux-samsung-soc@vger.kernel.org To: Tomasz Figa Cc: kgene.kim@samsung.com, t.figa@samsung.com, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, myungjoo.ham@samsung.com, m.szyprowski@samsung.com, phil.carmody@partner.samsung.com, j.anaszewski@samsung.com, kyungmin.park@samsung.com, devicetree-discuss@lists.ozlabs.org, Andrzej Hajda Hi, On 07/06/2013 01:26 AM, Tomasz Figa wrote: > On Monday 01 of July 2013 17:22:34 Sylwester Nawrocki wrote: >> From: Andrzej Hajda >> >> Add MAX8998 LDO12 and fixed voltage regulator nodes. While at it, >> all fixed voltage regulator nodes are grouped in a 'regulators' node. >> >> Signed-off-by: Andrzej Hajda >> Signed-off-by: Sylwester Nawrocki >> Signed-off-by: Kyungmin Park >> --- >> arch/arm/boot/dts/exynos4210-trats.dts | 80 >> +++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 16 >> deletions(-) >> >> diff --git a/arch/arm/boot/dts/exynos4210-trats.dts >> b/arch/arm/boot/dts/exynos4210-trats.dts index 6b1568e..f62e299 100644 >> --- a/arch/arm/boot/dts/exynos4210-trats.dts >> +++ b/arch/arm/boot/dts/exynos4210-trats.dts >> @@ -30,13 +30,64 @@ >> bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5 > rootwait >> earlyprintk panic=5"; }; >> >> - vemmc_reg: voltage-regulator@0 { >> - compatible = "regulator-fixed"; >> - regulator-name = "VMEM_VDD_2.8V"; >> - regulator-min-microvolt = <2800000>; >> - regulator-max-microvolt = <2800000>; >> - gpio = <&gpk0 2 0>; >> - enable-active-high; >> + regulators { >> + compatible = "simple-bus"; >> + #address-cells = <1>; >> + #size-cells = <0>; > > I don't think any addressing is needed for these regulators, so I'd > suggest removing those #properties and replacing @N with -N suffix. Originally there were also 'reg' properties in the individual regulator nodes, but these were unused and I've removed them before posting. Just missed to get rid of #size/address-cells as well. Please note you similarly use such properties in patch [1]. I suppose it is correct to have something like: regulators { compatible = "simple-bus"; regulator-0 { ... }; regulator-1 { ... }; ... }; rather than: regulators { compatible = "simple-bus"; #address-cells = <1>; #size-cells = <0>; regulator@0 { reg = <...>; ... }; regulator@1 { reg = <...>; ... }; }; Both patterns seem to be used in existing *.dts files. I'm going to use the first option in the next iteration, unless someone suggest otherwise. [1] http://www.spinics.net/lists/arm-kernel/msg253184.html > Otherwise looks good. > > Best regards, > Tomasz > >> + >> + vemmc_reg: regulator@0 { >> + compatible = "regulator-fixed"; >> + regulator-name = "VMEM_VDD_2.8V"; >> + regulator-min-microvolt = <2800000>; >> + regulator-max-microvolt = <2800000>; >> + gpio = <&gpk0 2 0>; >> + enable-active-high; >> + }; >> + >> + tsp_reg: regulator@1 { >> + compatible = "regulator-fixed"; >> + regulator-name = "TSP_FIXED_VOLTAGES"; >> + regulator-min-microvolt = <2800000>; >> + regulator-max-microvolt = <2800000>; >> + gpio = <&gpl0 3 0>; >> + enable-active-high; >> + }; >> + >> + cam_af_28v_reg: regulator@2 { >> + compatible = "regulator-fixed"; >> + regulator-name = "8M_AF_2.8V_EN"; >> + regulator-min-microvolt = <2800000>; >> + regulator-max-microvolt = <2800000>; >> + gpio = <&gpk1 1 0>; >> + enable-active-high; >> + }; >> + >> + cam_io_en_reg: regulator@3 { >> + compatible = "regulator-fixed"; >> + regulator-name = "CAM_IO_EN"; >> + regulator-min-microvolt = <2800000>; >> + regulator-max-microvolt = <2800000>; >> + gpio = <&gpe2 1 0>; >> + enable-active-high; >> + }; >> + >> + cam_io_12v_reg: regulator@4 { >> + compatible = "regulator-fixed"; >> + regulator-name = "8M_1.2V_EN"; >> + regulator-min-microvolt = <1200000>; >> + regulator-max-microvolt = <1200000>; >> + gpio = <&gpe2 5 0>; >> + enable-active-high; >> + }; >> + >> + vt_core_15v_reg: regulator@5 { >> + compatible = "regulator-fixed"; >> + regulator-name = "VT_CORE_1.5V"; >> + regulator-min-microvolt = <1500000>; >> + regulator-max-microvolt = <1500000>; >> + gpio = <&gpe2 2 0>; >> + enable-active-high; >> + }; >> }; Thanks, Sylwester From mboxrd@z Thu Jan 1 00:00:00 1970 From: s.nawrocki@samsung.com (Sylwester Nawrocki) Date: Mon, 08 Jul 2013 16:12:56 +0200 Subject: [PATCH v3 12/13] ARM: dts: Add S5K5BA sensor regulator definitions for Trats board In-Reply-To: <11736061.3CXfgF4Q9h@flatron> References: <1372692155-17653-1-git-send-email-s.nawrocki@samsung.com> <1372692155-17653-13-git-send-email-s.nawrocki@samsung.com> <11736061.3CXfgF4Q9h@flatron> Message-ID: <51DAC8E8.2070405@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, On 07/06/2013 01:26 AM, Tomasz Figa wrote: > On Monday 01 of July 2013 17:22:34 Sylwester Nawrocki wrote: >> From: Andrzej Hajda >> >> Add MAX8998 LDO12 and fixed voltage regulator nodes. While at it, >> all fixed voltage regulator nodes are grouped in a 'regulators' node. >> >> Signed-off-by: Andrzej Hajda >> Signed-off-by: Sylwester Nawrocki >> Signed-off-by: Kyungmin Park >> --- >> arch/arm/boot/dts/exynos4210-trats.dts | 80 >> +++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 16 >> deletions(-) >> >> diff --git a/arch/arm/boot/dts/exynos4210-trats.dts >> b/arch/arm/boot/dts/exynos4210-trats.dts index 6b1568e..f62e299 100644 >> --- a/arch/arm/boot/dts/exynos4210-trats.dts >> +++ b/arch/arm/boot/dts/exynos4210-trats.dts >> @@ -30,13 +30,64 @@ >> bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5 > rootwait >> earlyprintk panic=5"; }; >> >> - vemmc_reg: voltage-regulator at 0 { >> - compatible = "regulator-fixed"; >> - regulator-name = "VMEM_VDD_2.8V"; >> - regulator-min-microvolt = <2800000>; >> - regulator-max-microvolt = <2800000>; >> - gpio = <&gpk0 2 0>; >> - enable-active-high; >> + regulators { >> + compatible = "simple-bus"; >> + #address-cells = <1>; >> + #size-cells = <0>; > > I don't think any addressing is needed for these regulators, so I'd > suggest removing those #properties and replacing @N with -N suffix. Originally there were also 'reg' properties in the individual regulator nodes, but these were unused and I've removed them before posting. Just missed to get rid of #size/address-cells as well. Please note you similarly use such properties in patch [1]. I suppose it is correct to have something like: regulators { compatible = "simple-bus"; regulator-0 { ... }; regulator-1 { ... }; ... }; rather than: regulators { compatible = "simple-bus"; #address-cells = <1>; #size-cells = <0>; regulator at 0 { reg = <...>; ... }; regulator at 1 { reg = <...>; ... }; }; Both patterns seem to be used in existing *.dts files. I'm going to use the first option in the next iteration, unless someone suggest otherwise. [1] http://www.spinics.net/lists/arm-kernel/msg253184.html > Otherwise looks good. > > Best regards, > Tomasz > >> + >> + vemmc_reg: regulator at 0 { >> + compatible = "regulator-fixed"; >> + regulator-name = "VMEM_VDD_2.8V"; >> + regulator-min-microvolt = <2800000>; >> + regulator-max-microvolt = <2800000>; >> + gpio = <&gpk0 2 0>; >> + enable-active-high; >> + }; >> + >> + tsp_reg: regulator at 1 { >> + compatible = "regulator-fixed"; >> + regulator-name = "TSP_FIXED_VOLTAGES"; >> + regulator-min-microvolt = <2800000>; >> + regulator-max-microvolt = <2800000>; >> + gpio = <&gpl0 3 0>; >> + enable-active-high; >> + }; >> + >> + cam_af_28v_reg: regulator at 2 { >> + compatible = "regulator-fixed"; >> + regulator-name = "8M_AF_2.8V_EN"; >> + regulator-min-microvolt = <2800000>; >> + regulator-max-microvolt = <2800000>; >> + gpio = <&gpk1 1 0>; >> + enable-active-high; >> + }; >> + >> + cam_io_en_reg: regulator at 3 { >> + compatible = "regulator-fixed"; >> + regulator-name = "CAM_IO_EN"; >> + regulator-min-microvolt = <2800000>; >> + regulator-max-microvolt = <2800000>; >> + gpio = <&gpe2 1 0>; >> + enable-active-high; >> + }; >> + >> + cam_io_12v_reg: regulator at 4 { >> + compatible = "regulator-fixed"; >> + regulator-name = "8M_1.2V_EN"; >> + regulator-min-microvolt = <1200000>; >> + regulator-max-microvolt = <1200000>; >> + gpio = <&gpe2 5 0>; >> + enable-active-high; >> + }; >> + >> + vt_core_15v_reg: regulator at 5 { >> + compatible = "regulator-fixed"; >> + regulator-name = "VT_CORE_1.5V"; >> + regulator-min-microvolt = <1500000>; >> + regulator-max-microvolt = <1500000>; >> + gpio = <&gpe2 2 0>; >> + enable-active-high; >> + }; >> }; Thanks, Sylwester