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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 25 Aug 2020 09:45:00 +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: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.

> 
> > 
> > >  				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.

> 
> The names will take regulator names from the driver. The problem is with
> matching the of_node.
> 
> 
> Dear Rob,
> 
> Maybe you have an idea how to fix this driver-binding ABI
> incompatibility? Or better just leave it?

Not valid anymore, I just got confused...

Best regards,
Krzysztof

Powered by blists - more mailing lists