From: Robin Murphy <robin.murphy@arm.com>
To: "Heiko Stübner" <heiko@sntech.de>,
"Thierry Reding" <thierry.reding@gmail.com>,
"Simon Glass" <sjg@chromium.org>, "Tom Rini" <trini@konsulko.com>
Cc: devicetree@vger.kernel.org, linux-rockchip@lists.infradead.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arm64: rockchip: Remove unknown regulator-init-microvolt property
Date: Mon, 14 Nov 2022 10:58:51 +0000 [thread overview]
Message-ID: <ab2c77b8-e009-49b2-e38d-c12237ceb8e2@arm.com> (raw)
In-Reply-To: <6087432.MhkbZ0Pkbq@diego>
On 2022-11-13 14:44, Heiko Stübner wrote:
> Am Freitag, 4. November 2022, 15:41:09 CET schrieb Robin Murphy:
>> On 2022-11-04 13:20, Thierry Reding wrote:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> The regulator-init-microvolt is not defined anywhere and not used by any
>>> driver, so remove it from existing device trees.
>>
>> <vague memory triggers...>
>>
>> There *are* drivers that use it, just not in Linux[1][2][3]. Having a
>> single canonical bindings repo can't come soon enough :(
>>
>> Robin.
>>
>> [1]
>> https://source.denx.de/u-boot/u-boot/-/blob/master/doc/device-tree-bindings/regulator/regulator.txt#L40
>> [2]
>> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/power/regulator/pwm_regulator.c#L108
>> [3]
>> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/power/regulator/regulator-uclass.c#L455
>
> U-Boot always tries to merge Linux devicetrees with minimal changes,
> but I guess nobody ever tried to "upstream" that property?
That's what I had in mind - when somebody next syncs these DTs to U-Boot
and the properties disappear, that seems like unnecessary risk of
breakage. Given that we know this property *does* have a use, surely the
obvious correct course of action is to fix the binding?
Given that on the other hand we're merging entire DTs into the Linux
tree purely to fix schema warnings in U-Boot, I think we could do with
being a bit more logical and consistent here.
Thanks,
Robin.
> In any case, in a lot of rockchip boards in the u-boot source,
> I do see the regulator-init-microvolt in the *-u-boot.dtsi override file,
> so it already seems to be agreed that this stuff is bootloader-specific.
> [rk3399-pinebook-pro-u-boot.dtsi, and a number more]
>
> So I guess, if nobody complains to loudly, I guess I'll merge this patch.
>
>
> Heiko
>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>> arch/arm64/boot/dts/rockchip/rk3308-roc-cc.dts | 1 -
>>> arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts | 1 -
>>> arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 1 -
>>> arch/arm64/boot/dts/rockchip/rk3566-anbernic-rgxx3.dtsi | 3 ---
>>> arch/arm64/boot/dts/rockchip/rk3566-pinenote.dtsi | 2 --
>>> arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts | 2 --
>>> arch/arm64/boot/dts/rockchip/rk3566-quartz64-b.dts | 2 --
>>> arch/arm64/boot/dts/rockchip/rk3566-roc-pc.dts | 2 --
>>> arch/arm64/boot/dts/rockchip/rk3566-soquartz.dtsi | 3 ---
>>> arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts | 3 ---
>>> arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts | 3 ---
>>> arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts | 3 ---
>>> arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 3 ---
>>> 13 files changed, 29 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3308-roc-cc.dts b/arch/arm64/boot/dts/rockchip/rk3308-roc-cc.dts
>>> index 7ea48167747c..9232357f4fec 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3308-roc-cc.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3308-roc-cc.dts
>>> @@ -106,7 +106,6 @@ vdd_core: vdd-core {
>>> regulator-name = "vdd_core";
>>> regulator-min-microvolt = <827000>;
>>> regulator-max-microvolt = <1340000>;
>>> - regulator-init-microvolt = <1015000>;
>>> regulator-settling-time-up-us = <250>;
>>> regulator-always-on;
>>> regulator-boot-on;
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts b/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts
>>> index a71f249ed384..e9810d2f0407 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3308-rock-pi-s.dts
>>> @@ -105,7 +105,6 @@ vdd_core: vdd-core {
>>> regulator-name = "vdd_core";
>>> regulator-min-microvolt = <827000>;
>>> regulator-max-microvolt = <1340000>;
>>> - regulator-init-microvolt = <1015000>;
>>> regulator-settling-time-up-us = <250>;
>>> regulator-always-on;
>>> regulator-boot-on;
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
>>> index b6e082f1f6d9..7c5f441a2219 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
>>> @@ -375,7 +375,6 @@ regulator-state-mem {
>>> vcc_sdio: LDO_REG4 {
>>> regulator-always-on;
>>> regulator-boot-on;
>>> - regulator-init-microvolt = <3000000>;
>>> regulator-min-microvolt = <1800000>;
>>> regulator-max-microvolt = <3300000>;
>>> regulator-name = "vcc_sdio";
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rgxx3.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rgxx3.dtsi
>>> index 41262a69d33e..a71973b16075 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rgxx3.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rgxx3.dtsi
>>> @@ -356,7 +356,6 @@ vdd_logic: DCDC_REG1 {
>>> regulator-boot-on;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-ramp-delay = <6001>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-name = "vdd_logic";
>>> @@ -371,7 +370,6 @@ vdd_gpu: DCDC_REG2 {
>>> regulator-boot-on;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-ramp-delay = <6001>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-name = "vdd_gpu";
>>> @@ -533,7 +531,6 @@ vdd_cpu: regulator@40 {
>>> regulator-boot-on;
>>> regulator-min-microvolt = <712500>;
>>> regulator-max-microvolt = <1390000>;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-name = "vdd_cpu";
>>> regulator-ramp-delay = <2300>;
>>> vin-supply = <&vcc_sys>;
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-pinenote.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-pinenote.dtsi
>>> index 8d61f824c12d..d899087bf0b5 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3566-pinenote.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-pinenote.dtsi
>>> @@ -264,7 +264,6 @@ vdd_logic: DCDC_REG1 {
>>> regulator-always-on;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-ramp-delay = <6001>;
>>> regulator-initial-mode = <0x2>;
>>>
>>> @@ -278,7 +277,6 @@ vdd_gpu_npu: DCDC_REG2 {
>>> regulator-name = "vdd_gpu_npu";
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-ramp-delay = <6001>;
>>> regulator-initial-mode = <0x2>;
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
>>> index 25a8c781f4e7..854d02b46e6f 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
>>> @@ -366,7 +366,6 @@ vdd_logic: DCDC_REG1 {
>>> regulator-boot-on;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-ramp-delay = <6001>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-name = "vdd_logic";
>>> @@ -381,7 +380,6 @@ vdd_gpu: DCDC_REG2 {
>>> regulator-boot-on;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-ramp-delay = <6001>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-name = "vdd_gpu";
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-b.dts b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-b.dts
>>> index 77b179cd20e7..fc38b30d3722 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-b.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-b.dts
>>> @@ -277,7 +277,6 @@ vdd_log: DCDC_REG1 {
>>> regulator-boot-on;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-ramp-delay = <6001>;
>>>
>>> regulator-state-mem {
>>> @@ -292,7 +291,6 @@ vdd_gpu: DCDC_REG2 {
>>> regulator-boot-on;
>>> regulator-min-microvolt = <900000>;
>>> regulator-max-microvolt = <1350000>;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-ramp-delay = <6001>;
>>>
>>> regulator-state-mem {
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-roc-pc.dts b/arch/arm64/boot/dts/rockchip/rk3566-roc-pc.dts
>>> index 61c7a3ad7387..45807d7e22eb 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3566-roc-pc.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-roc-pc.dts
>>> @@ -250,7 +250,6 @@ vdd_log: DCDC_REG1 {
>>> regulator-boot-on;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-ramp-delay = <6001>;
>>>
>>> regulator-state-mem {
>>> @@ -263,7 +262,6 @@ vdd_gpu: DCDC_REG2 {
>>> regulator-name = "vdd_gpu";
>>> regulator-min-microvolt = <900000>;
>>> regulator-max-microvolt = <1350000>;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-ramp-delay = <6001>;
>>>
>>> regulator-state-mem {
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3566-soquartz.dtsi b/arch/arm64/boot/dts/rockchip/rk3566-soquartz.dtsi
>>> index 5bcd4be32964..e23e2293d10a 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3566-soquartz.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3566-soquartz.dtsi
>>> @@ -192,7 +192,6 @@ vdd_logic: DCDC_REG1 {
>>> regulator-boot-on;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-ramp-delay = <6001>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-state-mem {
>>> @@ -207,7 +206,6 @@ vdd_gpu: DCDC_REG2 {
>>> regulator-boot-on;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-ramp-delay = <6001>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-state-mem {
>>> @@ -230,7 +228,6 @@ vdd_npu: DCDC_REG4 {
>>> regulator-boot-on;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-name = "vdd_npu";
>>> regulator-state-mem {
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
>>> index 26d7fda275ed..a70b89e39dd6 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-bpi-r2-pro.dts
>>> @@ -301,7 +301,6 @@ vdd_logic: DCDC_REG1 {
>>> regulator-name = "vdd_logic";
>>> regulator-always-on;
>>> regulator-boot-on;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> @@ -315,7 +314,6 @@ regulator-state-mem {
>>> vdd_gpu: DCDC_REG2 {
>>> regulator-name = "vdd_gpu";
>>> regulator-always-on;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> @@ -339,7 +337,6 @@ regulator-state-mem {
>>>
>>> vdd_npu: DCDC_REG4 {
>>> regulator-name = "vdd_npu";
>>> - regulator-init-microvolt = <900000>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
>>> index 674792567fa6..19f8fc369b13 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
>>> @@ -293,7 +293,6 @@ vdd_logic: DCDC_REG1 {
>>> regulator-name = "vdd_logic";
>>> regulator-always-on;
>>> regulator-boot-on;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> @@ -307,7 +306,6 @@ regulator-state-mem {
>>> vdd_gpu: DCDC_REG2 {
>>> regulator-name = "vdd_gpu";
>>> regulator-always-on;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> @@ -331,7 +329,6 @@ regulator-state-mem {
>>>
>>> vdd_npu: DCDC_REG4 {
>>> regulator-name = "vdd_npu";
>>> - regulator-init-microvolt = <900000>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
>>> index 59ecf868dbd0..a337f547caf5 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
>>> @@ -291,7 +291,6 @@ vdd_logic: DCDC_REG1 {
>>> regulator-name = "vdd_logic";
>>> regulator-always-on;
>>> regulator-boot-on;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> @@ -305,7 +304,6 @@ regulator-state-mem {
>>> vdd_gpu: DCDC_REG2 {
>>> regulator-name = "vdd_gpu";
>>> regulator-always-on;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> @@ -329,7 +327,6 @@ regulator-state-mem {
>>>
>>> vdd_npu: DCDC_REG4 {
>>> regulator-name = "vdd_npu";
>>> - regulator-init-microvolt = <900000>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
>>> index ea74ba32fbbd..482c892567de 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
>>> @@ -340,7 +340,6 @@ vdd_logic: DCDC_REG1 {
>>> regulator-name = "vdd_logic";
>>> regulator-always-on;
>>> regulator-boot-on;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> @@ -354,7 +353,6 @@ regulator-state-mem {
>>> vdd_gpu: DCDC_REG2 {
>>> regulator-name = "vdd_gpu";
>>> regulator-always-on;
>>> - regulator-init-microvolt = <900000>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>> @@ -378,7 +376,6 @@ regulator-state-mem {
>>>
>>> vdd_npu: DCDC_REG4 {
>>> regulator-name = "vdd_npu";
>>> - regulator-init-microvolt = <900000>;
>>> regulator-initial-mode = <0x2>;
>>> regulator-min-microvolt = <500000>;
>>> regulator-max-microvolt = <1350000>;
>>
>
>
>
>
prev parent reply other threads:[~2022-11-14 10:59 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-04 13:20 [PATCH] arm64: rockchip: Remove unknown regulator-init-microvolt property Thierry Reding
2022-11-04 14:19 ` Da Xue
2022-11-04 14:41 ` Robin Murphy
2022-11-13 14:44 ` Heiko Stübner
2022-11-14 10:58 ` Robin Murphy [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ab2c77b8-e009-49b2-e38d-c12237ceb8e2@arm.com \
--to=robin.murphy@arm.com \
--cc=devicetree@vger.kernel.org \
--cc=heiko@sntech.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=sjg@chromium.org \
--cc=thierry.reding@gmail.com \
--cc=trini@konsulko.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).