Thread (29 messages) flat view 29 messages, 4 authors, 2020-08-25

Re: [PATCH 03/16] arm64: dts: imx8mm-beacon-som.dtsi: Align regulator names with schema

From: "krzk@kernel.org" <krzk@kernel.org>
Date: 2020-08-25 07:50:49
Also in: linux-arm-kernel, lkml

On Tue, Aug 25, 2020 at 09:45:00AM +0200, krzk@kernel.org wrote:
On Tue, Aug 25, 2020 at 09:25:37AM +0200, krzk@kernel.org wrote:
quoted
On Tue, Aug 25, 2020 at 06:51:33AM +0000, Vaittinen, Matti wrote:
quoted
Hello Krzysztof,

Just some questions - please ignore if I misunderstood the impact of
the change.

On Mon, 2020-08-24 at 21:06 +0200, Krzysztof Kozlowski wrote:
quoted
Device tree schema expects regulator names to be lowercase.  This
fixes
dtbs_check warnings like:

    arch/arm64/boot/dts/freescale/imx8mn-ddr4-evk.dt.yaml: pmic@4b:
regulators:LDO1:regulator-name:0: 'LDO1' does not match '^ldo[1-6]$'

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 .../boot/dts/freescale/imx8mn-ddr4-evk.dts    | 22 +++++++++------
----
 1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mn-ddr4-evk.dts
b/arch/arm64/boot/dts/freescale/imx8mn-ddr4-evk.dts
index a1e5483dbbbe..299caed5d46e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn-ddr4-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mn-ddr4-evk.dts
@@ -60,7 +60,7 @@
 
 		regulators {
 			buck1_reg: BUCK1 {
-				regulator-name = "BUCK1";
+				regulator-name = "buck1";
I am not against this change but I would expect seeing some other
patches too? I guess this will change the regulator name in regulator
core, right? So maybe I am mistaken but it looks to me this change is
visible in suppliers, sysfs and debugfs too? Thus changing this sounds
a bit like asking for a nose bleed :) Am I right that the impact of
this change has been thoroughly tested? Are there any other patches
(that I have not seen) related to this change?
Oh, crap, the names of regulators in the driver are lowercase, but they
use of_match_ptr for upper case. Seriously, why making a binding which
is contradictory to the driver implementation on the first day?

The driver goes with binding, right? One expects uppercase, other
lowercase...

And tell me, what is now the ABI? The binding or the incorrect
implementation?
Wait, my mistake. I got confused by my own change. The node name stays
the same, so of_match will be correct.

The driver internally already uses lowercase names.

Everything looks good. I will just double check whether the constraints
did not change on the board after boot.
Constraints look good.
quoted
quoted
quoted
 				regulator-min-microvolt = <700000>;
 				regulator-max-microvolt = <1300000>;
 				regulator-boot-on;
@@ -69,7 +69,7 @@
 			};
 
 			buck2_reg: BUCK2 {
-				regulator-name = "BUCK2";
+				regulator-name = "buck2";
 				regulator-min-microvolt = <700000>;
 				regulator-max-microvolt = <1300000>;
 				regulator-boot-on;
@@ -79,14 +79,14 @@
 
 			buck3_reg: BUCK3 {
 				// BUCK5 in datasheet
-				regulator-name = "BUCK3";
+				regulator-name = "buck3";
 				regulator-min-microvolt = <700000>;
 				regulator-max-microvolt = <1350000>;
 			};
 
 			buck4_reg: BUCK4 {
 				// BUCK6 in datasheet
-				regulator-name = "BUCK4";
+				regulator-name = "buck4";
 				regulator-min-microvolt = <3000000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-boot-on;
@@ -95,7 +95,7 @@
 
 			buck5_reg: BUCK5 {
 				// BUCK7 in datasheet
-				regulator-name = "BUCK5";
+				regulator-name = "buck5";
What I see in bd718x7-regulator.c for LDO6 desc is:

                        /* LDO6 is supplied by buck5 */
                        .supply_name = "buck5",

So, is this change going to change the supply-chain for the board? Is
this intended? (Or am I mistaken on what is the impact of regulator-
name property?)
Good point, let me check the supplies.
This patch actually fixes the supplies which before were not working
because of case mismatch.

Before:

 regulator                      use open bypass  opmode voltage current     min     max
---------------------------------------------------------------------------------------
 regulator-dummy                  4    5      0 unknown     0mV     0mA     0mV     0mV
    LDO6                          1    0      0 unknown  1200mV     0mA   900mV  1800mV
 BUCK1                            1    0      0 unknown   850mV     0mA   700mV  1300mV
 BUCK2                            2    1      0 unknown  1000mV     0mA   700mV  1300mV
    cpu0-cpu                      1                                 0mA  1000mV  1000mV
 BUCK3                            1    0      0 unknown   975mV     0mA   700mV  1350mV
 BUCK4                            1    0      0 unknown  3300mV     0mA  3000mV  3300mV
 BUCK5                            1    0      0 unknown  1800mV     0mA  1605mV  1995mV
 BUCK6                            1    0      0 unknown  1200mV     0mA   800mV  1400mV
 LDO1                             1    0      0 unknown  1800mV     0mA  1600mV  1900mV
 LDO2                             1    0      0 unknown   800mV     0mA   800mV   900mV
 LDO3                             1    0      0 unknown  1800mV     0mA  1800mV  3300mV
 LDO4                             1    0      0 unknown   900mV     0mA   900mV  1800mV
 ldo5                             1    4      0 unknown  1800mV     0mA  1800mV  1800mV



After:
 regulator                      use open bypass  opmode voltage current     min     max
---------------------------------------------------------------------------------------
buck1                            1    0      0 unknown   850mV     0mA   700mV  1300mV
 buck2                            2    1      0 unknown   850mV     0mA   700mV  1300mV
    cpu0-cpu                      1                                 0mA   850mV   850mV
 buck3                            1    0      0 unknown   975mV     0mA   700mV  1350mV
 buck4                            1    0      0 unknown  3300mV     0mA  3000mV  3300mV
 buck5                            2    1      0 unknown  1800mV     0mA  1605mV  1995mV
    ldo6                          1    0      0 unknown  1200mV     0mA   900mV  1800mV
 buck6                            1    0      0 unknown  1200mV     0mA   800mV  1400mV
 ldo1                             1    0      0 unknown  1800mV     0mA  1600mV  1900mV
 ldo2                             1    0      0 unknown   800mV     0mA   800mV   900mV
 ldo3                             1    0      0 unknown  1800mV     0mA  1800mV  3300mV
 ldo4                             1    0      0 unknown   900mV     0mA   900mV  1800mV
 ldo5                             0    0      0 unknown  3300mV     0mA     0mV     0mV

Best regards,
Krzysztof
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help