From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753111AbcIBL3f (ORCPT ); Fri, 2 Sep 2016 07:29:35 -0400 Received: from mailout3.samsung.com ([203.254.224.33]:37337 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751762AbcIBL3b (ORCPT ); Fri, 2 Sep 2016 07:29:31 -0400 MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 X-AuditID: cbfee68d-f79286d000007a9a-e9-57c962988e97 Content-transfer-encoding: 8BIT Message-id: <57C96297.1080704@samsung.com> Date: Fri, 02 Sep 2016 20:29:27 +0900 From: Chanwoo Choi Organization: Samsung Electronics User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Javier Martinez Canillas , k.kozlowski@samsung.com, kgene@kernel.org, robh+dt@kernel.org, mark.rutland@arm.com, catalin.marinas@arm.com, will.deacon@arm.com, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org Cc: krzk@kernel.org, jh80.chung@samsung.com, sw0312.kim@samsung.com, jy0922.shim@samsung.com, inki.dae@samsung.com, jonghwa3.lee@samsung.com, beomho.seo@samsung.com, jaewon02.kim@samsung.com, human.hwang@samsung.com, ideal.song@samsung.com, ingi2.kim@samsung.com, m.szyprowski@samsung.com, a.hajda@samsung.com, s.nawrocki@samsung.com, chanwoo@kernel.org Subject: Re: [PATCH v2 6/7] arm64: dts: exynos: Add dts file for Exynos5433-based TM2 board References: <1472046551-703-1-git-send-email-cw00.choi@samsung.com> <1472046551-703-7-git-send-email-cw00.choi@samsung.com> <0a6dffe0-8508-724e-c831-d14c32f8cd64@osg.samsung.com> In-reply-to: <0a6dffe0-8508-724e-c831-d14c32f8cd64@osg.samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA01Sa0iTYRjl/W77XFlv8/Y66OIgo0BLp/Z2QYqCBkFE/TACcdM+5krn2qej QaDQVcvUGbZGllpNm2Y1taaQ6Vw3y8wsW5YWzGpDK6sVdpN2IfLf4ZzDeZ4DhyVFFbSYVanz Oa1akSNhhFRTeNLeOGPm/bQVblsUHm5+ROMHX64L8CfzcYArnE8pfM7h4y4aT9C4489rgG0v vAw2vC6nsK3IQeOJD00Edv48TOPivjESu0cHKDzuTsBlrnES9/dfFWCra4jGgx1nGGzs7yTw ZceIAF98PkDgQzcdAtwzcYTGxkoPgz2f71HrxLKms01AZrUUM7KWC4Wy1rIRgexEqwXIvloX bGV2Ctfu4nJUOk67PFUuzB4fqiE17TH7zJNVZBE4F10CQlgEk9BkbTMdxJHo8egVpgQIWRFs AKjz/FvBP9P7j98EQcEEUF99F+MXQuE8NFU5SpUAliXhQuR4ssdPk3AJMpytI4P+NwBVubqJ oH8ZqmuvpfyYgotRa/FwADM+/pbbGcicC2PQsykX8GdGwB3o2D29Pycc9hDIe+dLIJSEHgI1 jvYK/KYwmI5+TxPBYyUEOnW0PFAnBG5AfXdvBAQEvSzqPFBMBy9D9L3SHvgawfnI2kUGW0aj 7gYnVQ6iTDO6mf53M83oVgNIC4jgNFkaPlOpTYjnFbl8gVoZn5WXawW+pTyYfldqAy+71tgB ZIFkdqhcfj9NRCt0vD7XDpJ9T1SQ4oisPN+41PkZCdKURJyclCxNXLkqRRIVGiP+sV0ElYp8 bg/HaThthrYgh+PtgGBDxEVAt7l/eKxG8ftVS/KcCfu1zwMj26RFHeuZ+v2bzljGGm/bmEvK 9LboPtWv5av1sUmrhtpmq+vM1dqTP2aJeN3ghUXqsl53rL7tJm9Y2py2W2U7+FN6+hST4imk LHHdD13V02vkR6p7TN6UrbXNkVP1hrHSsFkb/2xJy3dKbqeaeyUUn61IWEZqecVf7vXIaSQD AAA= X-Brightmail-Tracker: H4sIAAAAAAAAA2VSe0hTYRzlu2+t1XVlfa1QuyBFMHU+v6DCMGxRgVRmBKU3vam4XW1XzVWQ CGk+yFeYiJjZk+UjVMosK22VJTrtoctyRtNU1EozkMhqU6yo76/D+c45v/ODH4PLLaSCiRUT BZ3IazjKkWj7YXVSFh96GuY1M74I9VZ3kKht8iaNPl3JASjf/JJA54027nLxGRI1zvQD1PB6 ikIF/XkEakg1kmhsvBJD5m/pJMpsH8DRsKWLQKPDKpRrHcWRyXSDRrXWbhK9aCylULHpHoaq jH00utzThaFTTUYaPRzLIFFx4QiFRiZaiUCFurKsEqhrDZmUuu7SSXV9bh+tPlNvAOovtS4h 1P5UsCFG4KMEnZsgRsZHxYrRG7ntu8ODwv38vVRK1XoUwLmJvFbYyG3ZEaIMjtXYNuXcknlN ko0K4SWJ89z0f0LonmAlmjfuU+7cs+u3x9vrnxdRCWJGu8vxhNurU658LsJTwfkVWcCBgawv HPr4lZ7Dy2CnpYbKAo6MnC0BsP3qA8r+IWOd4HShhcgCDIOzrtD4PM5O4+waWFBWgc/p3wFY ZG3G5vTrYMXtC4QdE6w7rM/sncWUjb8/bJ7NXMyuhq+mrcCe6czug9mtenvOUvYhBqceT86G 4uwIBq9bntF20RL2APz+A5sbloXBc6fzSHuQAxsE25/cwvKAU8lfXUv+dC35q2s5wA0ACgmR CdKhaK1KFI56SLxWShKjPSLjtbVg9rY+KBpAc+PWFsAygFsoYw4+DZOTfLKk17YAyODcUpkf b6NkUbz+mKCLD9claQSpBfjZls3HFc6R8bZLFRPDVT6+/t7rvQNUyNsngFsus/6s3itno/lE IU4QEgTdvA9jHBSpIMc8sY1c5dLbme0UuPJaOm+q8zxrKTT7zmSWn3Dsr5nMeDxdvdbPo+rC kc1y07DyEV0xcZOgIzLy+2cmqy5O6DoG37fmJh9vMpTvHExrVnz3xLp9jPohXd1bR/eyqZ7S BXdDsTcphqi0w6IZ+A/eCXbhkvMupY09qB7Q6F0tIkdIMbxqHa6T+F9Y24lAcQMAAA== 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 Hi Javier, On 2016년 08월 27일 03:30, Javier Martinez Canillas wrote: > Hello Chanwoo, > > The patch looks mostly good to me, I've just some comments: > > [snip] > >> + >> +&decon { >> + status = "okay"; >> + iommu-reserved-mapping = <0x20000000 0x20000000 0xc0000000>; >> + > > This property never made to mainline due not having an agreement on > how this should be fixed properly IIUC [0]. So you should remove it. OK. I'll remove it. > > [snip] > >> + >> + s2mps13-pmic@66 { >> + compatible = "samsung,s2mps13-pmic"; >> + interrupt-parent = <&gpa0>; >> + interrupts = <7 IRQ_TYPE_NONE>; >> + reg = <0x66>; >> + samsung,s2mps11-wrstbi-ground; >> + >> + s2mps13_osc: clocks { >> + compatible = "samsung,s2mps13-clk"; >> + #clock-cells = <1>; >> + clock-output-names = "s2mps13_ap", "s2mps13_cp", >> + "s2mps13_bt"; >> + }; >> + > > I see that most of the following regulators are marked as always-on > but I wonder if this is really needed. For example some of them are > looked up by consumer devices. > > [snip] > >> + }; >> + >> + ldo3_reg: LDO3 { >> + regulator-name = "VDD1_E_1.8V_AP"; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <1800000>; >> + regulator-always-on; >> + }; > > This is used by both the ADC and the TMU so I guess it should be safe > to not mark it as always-on (unless is used by other critical IP block > not described in the DT). This regulator should be always ON state. This regulator provides the voltage to ALIVE domain of Exynos5433. > > [snip] > >> + >> + ldo6_reg: LDO6 { >> + regulator-name = "VDD10_MIPI2L_1.0V_AP"; >> + regulator-min-microvolt = <1000000>; >> + regulator-max-microvolt = <1000000>; >> + regulator-always-on; > > Same question, this is used by both the dsi and usbdrd30 nodes so maybe > it shouldn't be marked as always-on as well. OK. I'll remove it. > >> + regulator-state-mem { >> + regulator-off-in-suspend; >> + }; >> + }; >> + >> + ldo7_reg: LDO7 { >> + regulator-name = "VDD18_MIPI2L_1.8V_AP"; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <1800000>; >> + regulator-always-on; > > This is used by the dsi node as well. OK. I'll remove it. > > [snip] > >> + >> + ldo10_reg: LDO10 { >> + regulator-name = "VDD33_USB30_3.0V_AP"; >> + regulator-min-microvolt = <3000000>; >> + regulator-max-microvolt = <3000000>; >> + regulator-always-on; > > Use by the usbdrd30 node. OK. I'll remove it. > > [snip] > >> + >> + ldo18_reg: LDO18 { >> + regulator-name = "V_CODEC_1.8V_AP"; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <1800000>; >> + regulator-always-on; > > Use by the wm5110-codec node. OK. I'll remove it. > > [snip] > >> + >> + buck2_reg: BUCK2 { >> + regulator-name = "VDD_EGL_1.0V_AP"; > > I wonder if this shouldn't be "VDD_ATL_1.0V_AP" or something since > the big cluster isn't called Eagle like in arm32 Exynos but Atlas? I used the regulator's name according to TM2's schematic. As I knew, Eagle means the big cores. > >> + regulator-min-microvolt = <900000>; >> + regulator-max-microvolt = <1300000>; >> + regulator-always-on; >> + regulator-state-mem { >> + regulator-off-in-suspend; >> + }; >> + }; >> + >> + buck3_reg: BUCK3 { >> + regulator-name = "VDD_KFC_1.0V_AP"; > > Same, maybe using "VDD_APL_1.0V_AP" since the big cluster is Apollo? ditto. The KFC (King Fisher) means the little cores. > >> + regulator-min-microvolt = <800000>; >> + regulator-max-microvolt = <1200000>; >> + regulator-always-on; >> + regulator-state-mem { >> + regulator-off-in-suspend; >> + }; >> + }; >> + > > Used by the big and LITTLE clusters respectively, although for these two > I'm not that sure if it would be safe to remove the always-on property. > > Reviewed-by: Javier Martinez Canillas Thanks for your review. > > [0]: http://www.spinics.net/lists/arm-kernel/msg419747.html > > Best regards, > -- Best Regards, Chanwoo Choi