Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.120.0.1.13\))
Subject: Re: [PATCH] riscv: dts: sifive unmatched: Add gpio poweroff
From:   Jessica Clarke <jrtc27@jrtc27.com>
In-Reply-To: <CADWks+Yjh1B=Fu8NFS4KYrvNh_bU8P5pzb+_yNvseS+1+D5H7Q@mail.gmail.com>
Date:   Fri, 31 Dec 2021 17:16:00 +0000
Cc:     Ron Economos <w6rz@comcast.net>, Rob Herring <robh+dt@kernel.org>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Vincent Pelletier <plr.vincent@gmail.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
        Qiu Wenbo <qiuwenbo@kylinos.com.cn>,
        Yash Shah <yash.shah@sifive.com>, devicetree@vger.kernel.org,
        linux-riscv <linux-riscv@lists.infradead.org>,
        "linux-kernel@vger.kernel.org List" <linux-kernel@vger.kernel.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <ED7B7FCA-182C-45AC-90E7-17559762DE0B@jrtc27.com>
References: <20211231061110.89403-1-w6rz@comcast.net>
 <73F7FAE3-5113-48DD-B0F5-0EEAA0A8B0C1@jrtc27.com>
 <CADWks+Yjh1B=Fu8NFS4KYrvNh_bU8P5pzb+_yNvseS+1+D5H7Q@mail.gmail.com>
To:     Dimitri John Ledkov <dimitri.ledkov@canonical.com>
Precedence: bulk

On 31 Dec 2021, at 16:49, Dimitri John Ledkov =
<dimitri.ledkov@canonical.com> wrote:
>=20
> However, in some configurations uboot loads kernel provided dtb, then =
this pin needs to be defines for SBI to provide poweroff via this pin.
>=20
> 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=E2=80=99s use. U-Boot =
=E2=80=9Cproper=E2=80=9D
loads the kernel=E2=80=99s DTB for the kernel=E2=80=99s use. The DTB =
loaded for the
kernel is never fed back somehow. Just as we don=E2=80=99t provide DDR =
timing
information in the kernel DTB, only U-Boot=E2=80=99s, 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=E2=80=99re
supposed to use standardised firmware interfaces for these kinds of
platform-specific things.

Jess

> On Fri, 31 Dec 2021, 06:58 James Clarke, <jrtc27@jrtc27.com> wrote:
> On 31 Dec 2021, at 06:11, Ron Economos <w6rz@comcast.net> wrote:
> >=20
> > This patch is required for the following commit to work.
> >=20
> > commit f2928e224d85 ("riscv: set default pm_power_off to NULL")
> >=20
> > Signed-off-by: Ron Economos <w6rz@comcast.net>
> > ---
> > arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 5 +++++
> > 1 file changed, 5 insertions(+)
> >=20
> > 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 =3D <RTCCLK_FREQ>;
> >        clock-output-names =3D "rtcclk";
> >    };
> > +
> > +    gpio-poweroff {
> > +        compatible =3D "gpio-poweroff";
> > +        gpios =3D <&gpio 2 GPIO_ACTIVE_LOW>;
> > +    };
>=20
> Why? It=E2=80=99s abstracted by firmware, which works.
>=20
> Jess
>=20