linux-kernel - Re: [PATCH v2] ARM: dts: sun8i-h2-plus-bananapi-m2-zero: add poweroff node to DT

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201128103958.q6glewhhch7vtczr@gilmour>
Date:   Sat, 28 Nov 2020 11:39:58 +0100
From:   Maxime Ripard <maxime@...no.tech>
To:     Michael Klein <michael@...sekall.de>
Cc:     Rob Herring <robh+dt@...nel.org>, Chen-Yu Tsai <wens@...e.org>,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] ARM: dts: sun8i-h2-plus-bananapi-m2-zero: add
 poweroff node to DT

On Tue, Nov 24, 2020 at 11:31:59PM +0100, Michael Klein wrote:
> On Tue, Nov 24, 2020 at 03:26:56PM +0100, Maxime Ripard wrote:
> > On Tue, Nov 24, 2020 at 02:36:33PM +0100, Michael Klein wrote:
> > > Add poweroff node to allow the board to power itself off after shutdown
> > > by disabling the SYSTEM and CPUX regulators (U5 resp. U6).  The RST
> > > button can be used to restart the board.
> > > 
> > > Signed-off-by: Michael Klein <michael@...sekall.de>
> > > ---
> > >  arch/arm/boot/dts/sun8i-h2-plus-bananapi-m2-zero.dts | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > 
> > You should have a summary of the changes between versions here
> > 
> > > diff --git a/arch/arm/boot/dts/sun8i-h2-plus-bananapi-m2-zero.dts b/arch/arm/boot/dts/sun8i-h2-plus-bananapi-m2-zero.dts
> > > index 4c6704e4c57e..ea2fa48a1647 100644
> > > --- a/arch/arm/boot/dts/sun8i-h2-plus-bananapi-m2-zero.dts
> > > +++ b/arch/arm/boot/dts/sun8i-h2-plus-bananapi-m2-zero.dts
> > > @@ -46,6 +46,11 @@ sw4 {
> > >  		};
> > >  	};
> > > 
> > > +	poweroff {
> > > +		compatible = "gpio-poweroff";
> > > +		gpios = <&r_pio 0 8 GPIO_ACTIVE_LOW>; /* PL8 */
> > > +	};
> > > +
> > 
> > Like I said in the previous version, this should really be modelled as a
> > regulator instead of just a GPIO
> 
> Please excuse my ignorance, do you mean something like this?
> 
>        reg_vdd_sys: vdd-sys {
>                compatible = "regulator-fixed";
>                regulator-name = "vdd-sys";
>                regulator-min-microvolt = <1200000>;
>                regulator-max-microvolt = <1200000>;
>                regulator-always-on;
>                regulator-boot-on;
>                enable-active-high;
>                gpio = <&r_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */
>                vin-supply = <&reg_vcc5v0>;
>         };
> 
> With this, the board still draws 60mA (cheap USB ampere meter) after
> shutdown, presumably because of "regulator-always-on".  Without this
> property the board powers off shortly after booting up.

Yes, because you're only describing the regulator itself here, but
you're not telling linux that it needs to shut it down to power-down the
board.

You'd need a driver similar to gpio-poweroff, using a regulator instead,
and calling regulator_force_disable to shut it down

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)