linux-kernel - Re: [PATCH] riscv: dts: sifive unmatched: Add gpio poweroff

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-next>] [day] [month] [year] [list]

Date:   Fri, 31 Dec 2021 17:16:00 +0000
From:   Jessica Clarke <jrtc27@...c27.com>
To:     Dimitri John Ledkov <dimitri.ledkov@...onical.com>
Cc:     Ron Economos <w6rz@...cast.net>, Rob Herring <robh+dt@...nel.org>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Vincent Pelletier <plr.vincent@...il.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        Qiu Wenbo <qiuwenbo@...inos.com.cn>,
        Yash Shah <yash.shah@...ive.com>, devicetree@...r.kernel.org,
        linux-riscv <linux-riscv@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org List" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] riscv: dts: sifive unmatched: Add gpio poweroff

On 31 Dec 2021, at 16:49, Dimitri John Ledkov <dimitri.ledkov@...onical.com> wrote:
> 
> However, in some configurations uboot loads kernel provided dtb, then this pin needs to be defines for SBI to provide poweroff via this pin.
> 
> Kernel, uboot, opensbi dtbs for unmatched must be consistent with each other and all should define poweroff pin.

U-Boot SPL loads its embedded DTB for OpenSBI’s use. U-Boot “proper”
loads the kernel’s DTB for the kernel’s use. The DTB loaded for the
kernel is never fed back somehow. Just as we don’t provide DDR timing
information in the kernel DTB, only U-Boot’s, there should be no need
to provide information about this GPIO to the kernel. Either the kernel
will prioritise SBI power-off, which renders the DTB node a complete
waste of space, and possibly confusing to exist, or the kernel will
prioritise GPIO power-off, which should be discouraged as you’re
supposed to use standardised firmware interfaces for these kinds of
platform-specific things.

Jess

> On Fri, 31 Dec 2021, 06:58 James Clarke, <jrtc27@...c27.com> wrote:
> On 31 Dec 2021, at 06:11, Ron Economos <w6rz@...cast.net> wrote:
> > 
> > This patch is required for the following commit to work.
> > 
> > commit f2928e224d85 ("riscv: set default pm_power_off to NULL")
> > 
> > Signed-off-by: Ron Economos <w6rz@...cast.net>
> > ---
> > arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 5 +++++
> > 1 file changed, 5 insertions(+)
> > 
> > diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> > index 6bfa1f24d3de..c4ed9efdff03 100644
> > --- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> > +++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> > @@ -39,6 +39,11 @@ rtcclk: rtcclk {
> >        clock-frequency = <RTCCLK_FREQ>;
> >        clock-output-names = "rtcclk";
> >    };
> > +
> > +    gpio-poweroff {
> > +        compatible = "gpio-poweroff";
> > +        gpios = <&gpio 2 GPIO_ACTIVE_LOW>;
> > +    };
> 
> Why? It’s abstracted by firmware, which works.
> 
> Jess
>