From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754065AbbKBNA7 (ORCPT ); Mon, 2 Nov 2015 08:00:59 -0500 Received: from mailout3.samsung.com ([203.254.224.33]:45497 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753131AbbKBNAx (ORCPT ); Mon, 2 Nov 2015 08:00:53 -0500 X-AuditID: cbfee68e-f791c6d000001498-14-56375e8251a1 Message-id: <56375EB1.2090504@samsung.com> Date: Mon, 02 Nov 2015 18:31:37 +0530 From: Alim Akhtar User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-version: 1.0 To: Krzysztof Kozlowski Cc: linux-samsung-soc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kgene@kernel.org Subject: Re: [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node References: <1446458641-4447-1-git-send-email-alim.akhtar@samsung.com> In-reply-to: Content-type: text/plain; charset=UTF-8; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprJIsWRmVeSWpSXmKPExsWyRsSkSrcpzjzM4HSzicX8I+dYLV6/MLTo f/ya2WLT42usFpd3zWGzmHF+H5MDm8emVZ1sHpuX1Hv0bVnF6PF5k1wASxSXTUpqTmZZapG+ XQJXxptTh5kLdi9grDh18x1bA+N3/y5GDg4JAROJm0/Tuxg5gUwxiQv31rN1MXJxCAmsYJTY dv47G0TCRGJ9yxFGiMQsRoldG1azQzgPGCWO7TrJAlLFK6AlsW7GJyYQm0VAVeLimfVg3WwC 2hJ3p29hAtkmKhAh8fiCEES5oMSPyffAWkUEDCUO7t7OBDKTWWA2o8SEHU1gc4QF/CXWPobZ 3AuUmDQVrINTIFjiwPbnjCA2s4CZxKOWdcwQtrzE5jVvmUEaJAQOsUt86l7HCnGRgMS3yYdY IH6Wldh0gBniNUmJgytusExgFJuF5KhZSMbOQjJ2ASPzKkbR1ILkguKk9CIjveLE3OLSvHS9 5PzcTYzAGDv971nfDsabB6wPMQpwMCrx8Ga4m4UJsSaWFVfmHmI0BbpiIrOUaHI+MJLzSuIN jc2MLExNTI2NzC3NlMR5E6R+BgsJpCeWpGanphakFsUXleakFh9iZOLglGpgjJAyWLUuuOUg n66T2PVwoe/996/MdBDqfmU8sc/ZtFKcmUOon0/dq4/7pVv+duvTFpIySyU9d+y+usjiyls/ ddlaz83VAizPZKNmFDjb6PVHpH8/rP995TSB/tuHirNjFCv3nq56tuP8u0PHptxv1Lrf2XaS qertFwE+y/dm107xGXcu37haiaU4I9FQi7moOBEAk/hgpqwCAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrOIsWRmVeSWpSXmKPExsVy+t9jQd2mOPMwg80r1S3mHznHavH6haFF /+PXzBabHl9jtbi8aw6bxYzz+5gc2Dw2repk89i8pN6jb8sqRo/Pm+QCWKIaGG0yUhNTUosU UvOS81My89JtlbyD453jTc0MDHUNLS3MlRTyEnNTbZVcfAJ03TJzgHYrKZQl5pQChQISi4uV 9O0wTQgNcdO1gGmM0PUNCYLrMTJAAwlrGDPenDrMXLB7AWPFqZvv2BoYv/t3MXJySAiYSKxv OcIIYYtJXLi3nq2LkYtDSGAWo8SuDavZIZwHjBLHdp1kAaniFdCSWDfjExOIzSKgKnHxDEgH JwebgLbE3elbgOIcHKICERKPLwhBlAtK/Jh8D6xVRMBQ4uDu7UwgM5kFZjNKTNjRBDZHWMBf Yu1jkCtAlvUCJSZNBevgFAiWOLD9Odh5zAJmEo9a1jFD2PISm9e8ZZ7ACHQnwpJZSMpmISlb wMi8ilEitSC5oDgpPdcwL7Vcrzgxt7g0L10vOT93EyM4kp9J7WA8uMv9EKMAB6MSD+8BT7Mw IdbEsuLK3EOMEhzMSiK8O+zNw4R4UxIrq1KL8uOLSnNSiw8xmgJDYSKzlGhyPjDJ5JXEGxqb mJsam1qaWJiYWSqJ8+p7GoUJCaQnlqRmp6YWpBbB9DFxcEo1MO53ff6a39JgZo7UkrZrPm9K HKY2SfG/zG1RPZS216uBqzXqyuJWB6dvTVZ7rEw5/BVn+jovtL+Ro3qzWiPVIaWN93bDTrVP u2dKLVoZ01325YbbvIMVrF/T/6z/z3Lq1xT2XTf/xKake7XxuyUpPlG9perJzVuambvYeYW/ 5cPTU3wrrBl3K7EUZyQaajEXFScCAEs3irL6AgAA DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/02/2015 05:55 PM, Krzysztof Kozlowski wrote: > 2015-11-02 19:04 GMT+09:00 Alim Akhtar : >> This patch add pmic (s2mps15) node of espresso board, > > This patch adds PMIC (S2MPS15)... > >> which includes addition of regulators and pmic-clk sub-nodes. >> >> Signed-off-by: Abhilash Kesavan >> Signed-off-by: Alim Akhtar >> --- >> This patch should go in after driver side changes [1] lands. >> [1]-> https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg47736.html > > Eh? Why? Usually there is not such requirement. > Got it, I just thought without driver changes these dts modification does not make sense. Thanks for clarification. >> >> arch/arm64/boot/dts/exynos/exynos7-espresso.dts | 349 +++++++++++++++++++++++ >> 1 file changed, 349 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts >> index 838a3626dac1..8ce04a0ec928 100644 >> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts >> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts >> @@ -53,6 +53,355 @@ >> status = "okay"; >> }; >> >> +&hsi2c_4 { >> + samsung,i2c-sda-delay = <100>; >> + samsung,i2c-max-bus-freq = <200000>; >> + status = "okay"; >> + >> + s2mps15_pmic@66 { >> + compatible = "samsung,s2mps15-pmic"; >> + reg = <0x66>; >> + interrupts = <2 0>; >> + interrupt-parent = <&gpa0>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pmic_irq>; >> + wakeup-source; >> + >> + s2mps15_osc: clocks { >> + compatible = "samsung,s2mps13-clk"; >> + #clock-cells = <1>; >> + clock-output-names = "s2mps13_ap", "s2mps13_cp", >> + "s2mps13_bt"; >> + }; > > Don't you want to use one of these clocks for s3c-rtc (&rtc node)? > yes, you are right, rtc on this board is currently broken, mainly because of the introduction of rtc_src clock in the s3c-rtc driver. That is on my do list next. will take a look. Are you suggesting to remove this -clk node now and add along with rtc changes? I feel this should go in along with this patch. >> + >> + regulators { >> + ldo1_reg: LDO1 { >> + regulator-name = "vdd_ldo1"; >> + regulator-min-microvolt = <500000>; >> + regulator-max-microvolt = <900000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo2_reg: LDO2 { >> + regulator-name = "vdd_ldo2"; >> + regulator-min-microvolt = <1620000>; >> + regulator-max-microvolt = <3300000>; >> + regulator-enable-ramp-delay = <125>; >> + regulator-always-on; >> + }; >> + >> + ldo3_reg: LDO3 { >> + regulator-name = "vdd_ldo3"; >> + regulator-min-microvolt = <1620000>; >> + regulator-max-microvolt = <1980000>; >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo4_reg: LDO4 { >> + regulator-name = "vdd_ldo4"; >> + regulator-min-microvolt = <800000>; >> + regulator-max-microvolt = <1110000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo5_reg: LDO5 { >> + regulator-name = "vdd_ldo5"; >> + regulator-min-microvolt = <1620000>; >> + regulator-max-microvolt = <1980000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo6_reg: LDO6 { >> + regulator-name = "vdd_ldo6"; >> + regulator-min-microvolt = <2250000>; >> + regulator-max-microvolt = <3300000>; >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo7_reg: LDO7 { >> + regulator-name = "vdd_ldo7"; >> + regulator-min-microvolt = <700000>; >> + regulator-max-microvolt = <1150000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo8_reg: LDO8 { >> + regulator-name = "vdd_ldo8"; >> + regulator-min-microvolt = <700000>; >> + regulator-max-microvolt = <1000000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo9_reg: LDO9 { >> + regulator-name = "vdd_ldo9"; >> + regulator-min-microvolt = <700000>; >> + regulator-max-microvolt = <1000000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo10_reg: LDO10 { >> + regulator-name = "vdd_ldo10"; >> + regulator-min-microvolt = <700000>; >> + regulator-max-microvolt = <1000000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo11_reg: LDO11 { >> + regulator-name = "vdd_ldo11"; >> + regulator-min-microvolt = <1000000>; >> + regulator-max-microvolt = <1300000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo12_reg: LDO12 { >> + regulator-name = "vdd_ldo12"; >> + regulator-min-microvolt = <1000000>; >> + regulator-max-microvolt = <1300000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo13_reg: LDO13 { >> + regulator-name = "vdd_ldo13"; >> + regulator-min-microvolt = <1000000>; >> + regulator-max-microvolt = <1300000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo14_reg: LDO14 { >> + regulator-name = "vdd_ldo14"; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <3375000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo15_reg: LDO15 { >> + regulator-name = "vdd_ldo15"; >> + regulator-min-microvolt = <1500000>; >> + regulator-max-microvolt = <2275000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo16_reg: LDO16 { >> + regulator-name = "vdd_ldo16"; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <1800000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo17_reg: LDO17 { >> + regulator-name = "vdd_ldo17"; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <3375000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo18_reg: LDO18 { >> + regulator-name = "vdd_ldo18"; >> + regulator-min-microvolt = <1500000>; >> + regulator-max-microvolt = <2275000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo19_reg: LDO19 { >> + regulator-name = "vdd_ldo19"; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <3375000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo20_reg: LDO20 { >> + regulator-name = "vdd_ldo20"; >> + regulator-min-microvolt = <1500000>; >> + regulator-max-microvolt = <2275000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo21_reg: LDO21 { >> + regulator-name = "vdd_ldo21"; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <3375000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo22_reg: LDO22 { >> + regulator-name = "vdd_ldo22"; >> + regulator-min-microvolt = <1200000>; >> + regulator-max-microvolt = <1200000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo23_reg: LDO23 { >> + regulator-name = "vdd_ldo23"; >> + regulator-min-microvolt = <1500000>; >> + regulator-max-microvolt = <2275000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo24_reg: LDO24 { >> + regulator-name = "vdd_ldo24"; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <3375000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo25_reg: LDO25 { >> + regulator-name = "vdd_ldo25"; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <3375000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo26_reg: LDO26 { >> + regulator-name = "vdd_ldo26"; >> + regulator-min-microvolt = <700000>; >> + regulator-max-microvolt = <1470000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + ldo27_reg: LDO27 { >> + regulator-name = "vdd_ldo27"; >> + regulator-min-microvolt = <1500000>; >> + regulator-max-microvolt = <2275000>; >> + regulator-always-on; >> + regulator-enable-ramp-delay = <125>; >> + }; >> + >> + buck1_reg: BUCK1 { >> + regulator-name = "vdd_mif"; >> + regulator-min-microvolt = <500000>; >> + regulator-max-microvolt = <1200000>; >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-ramp-delay = <25000>; >> + regulator-enable-ramp-delay = <250>; >> + }; >> + >> + buck2_reg: BUCK2 { >> + regulator-name = "vdd_atlas"; >> + regulator-min-microvolt = <1200000>; >> + regulator-max-microvolt = <1200000>; >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-ramp-delay = <12500>; >> + regulator-enable-ramp-delay = <250>; >> + }; >> + >> + buck3_reg: BUCK3 { >> + regulator-name = "vdd_apollo"; >> + regulator-min-microvolt = <1200000>; >> + regulator-max-microvolt = <1200000>; >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-ramp-delay = <12500>; >> + regulator-enable-ramp-delay = <250>; >> + }; >> + >> + buck4_reg: BUCK4 { >> + regulator-name = "vdd_int"; >> + regulator-min-microvolt = <500000>; >> + regulator-max-microvolt = <1200000>; >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-ramp-delay = <12500>; >> + regulator-enable-ramp-delay = <250>; >> + }; >> + >> + buck5_reg: BUCK5 { >> + regulator-name = "vdd_buck5"; >> + regulator-min-microvolt = <500000>; >> + regulator-max-microvolt = <1300000>; >> + regulator-always-on; >> + regulator-ramp-delay = <25000>; >> + regulator-enable-ramp-delay = <250>; >> + }; >> + >> + buck6_reg: BUCK6 { >> + regulator-name = "vdd_g3d"; >> + regulator-min-microvolt = <500000>; >> + regulator-max-microvolt = <1400000>; >> + regulator-always-on; >> + regulator-ramp-delay = <12500>; >> + regulator-enable-ramp-delay = <250>; >> + }; >> + >> + buck7_reg: BUCK7 { >> + regulator-name = "vdd_buck7"; >> + regulator-min-microvolt = <1000000>; >> + regulator-max-microvolt = <1500000>; >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-ramp-delay = <25000>; >> + regulator-enable-ramp-delay = <250>; >> + }; >> + >> + buck8_reg: BUCK8 { >> + regulator-name = "vdd_buck8"; >> + regulator-min-microvolt = <1000000>; >> + regulator-max-microvolt = <1500000>; >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-ramp-delay = <25000>; >> + regulator-enable-ramp-delay = <250>; >> + }; >> + >> + buck9_reg: BUCK9 { >> + regulator-name = "vdd_buck9"; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <2100000>; >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-ramp-delay = <25000>; >> + regulator-enable-ramp-delay = <250>; >> + }; >> + >> + buck10_reg: BUCK10 { >> + regulator-name = "vdd_buck10"; >> + regulator-min-microvolt = <1000000>; >> + regulator-max-microvolt = <3000000>; >> + regulator-boot-on; >> + regulator-always-on; >> + regulator-ramp-delay = <25000>; >> + regulator-enable-ramp-delay = <250>; >> + }; > > All of these ldo3 and bucks in vendor tree for Espresso board have > ramp delay of 12000. Also they don't have enable-ramp-delay set and > voltages sometimes differ. I don't have S2MPS15 datasheet so I don't > know what is the true value... I'll leave it up to you but it looks > suspicious. > These values generally comes from our board design team, so I cann't really comment on that, it may vary from board revision etc. I will check if we have any updated version of recommended value and update accordingly. >> + }; >> + }; >> +}; > > What will be the benefit of defining all of these regulators if they > are always on and without consumers? No one will disable them, no one > will change the voltage. Please provide some consumers. > As many drivers are not yet enabled in arm64 defconfig, that is one of the reason why we are not seeing many consumer for these nodes. This is the ground work being done for enabling those. If you insist will try to reduce what is being used now. Moreover this was used to verify functionality of pmic driver as well. > Best regards, > Krzysztof >