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

[Maxime Ripard]

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@xxxxxxxxxxxx>
> > > ---
> > >  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

Attachment: signature.asc
Description: PGP signature