| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20200825075038.GA10148@kozik-lap>
Date: Tue, 25 Aug 2020 09:50:38 +0200
From: "krzk@...nel.org" <krzk@...nel.org>
To: "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>,
"robh+dt@...nel.org" <robh+dt@...nel.org>
Cc: "miquel.raynal@...tlin.com" <miquel.raynal@...tlin.com>,
"linux-imx@....com" <linux-imx@....com>,
"han.xu@....com" <han.xu@....com>,
"Anson.Huang@....com" <Anson.Huang@....com>,
"yibin.gong@....com" <yibin.gong@....com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"shawnguo@...nel.org" <shawnguo@...nel.org>,
"aford173@...il.com" <aford173@...il.com>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"kernel@...gutronix.de" <kernel@...gutronix.de>,
"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
"richard@....at" <richard@....at>,
"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"daniel.baluta@....com" <daniel.baluta@....com>,
"lee.jones@...aro.org" <lee.jones@...aro.org>,
"festevam@...il.com" <festevam@...il.com>,
"vigneshr@...com" <vigneshr@...com>,
"jun.li@....com" <jun.li@....com>
Subject: Re: [PATCH 03/16] arm64: dts: imx8mm-beacon-som.dtsi: Align
regulator names with schema
On Tue, Aug 25, 2020 at 09:45:00AM +0200, krzk@...nel.org wrote:
> On Tue, Aug 25, 2020 at 09:25:37AM +0200, krzk@...nel.org wrote:
> > On Tue, Aug 25, 2020 at 06:51:33AM +0000, Vaittinen, Matti wrote:
> > > 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:
> > > > 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@...nel.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.
>
> >
> > >
> > > > 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
Powered by blists - more mailing lists