Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20211020095926.735938-1-flokli@flokli.de>
In-Reply-To: <20211020095926.735938-1-flokli@flokli.de>
From:   Dennis Gilmore <dgilmore@redhat.com>
Date:   Thu, 28 Oct 2021 19:08:32 -0500
Message-ID: <CAJesf3Opx3KBPgBdQbKYTdBkoWhuuvphKm6BoVoaqrs_JBz13g@mail.gmail.com>
Subject: Re: [PATCH 1/2] arm64: dts: rockchip: Enable HDD power on helios64
To:     Florian Klink <flokli@flokli.de>
Cc:     Heiko Stuebner <heiko@sntech.de>, Aditya Prayoga <aditya@kobol.io>,
        Rob Herring <robh+dt@kernel.org>,
        =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= <uwe@kleine-koenig.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        "moderated list:ARM/Rockchip SoC support" 
        <linux-arm-kernel@lists.infradead.org>,
        linux-rockchip@lists.infradead.org,
        open list <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Tested-by: Dennis Gilmore <dgilmore@redhat.com>

On Thu, Oct 28, 2021 at 11:37 AM Florian Klink <flokli@flokli.de> wrote:
>
> This adds the hdd_{a,b}_power blocks present in the armbian helios64
> dts. [1]
>
> Without those powered up, no HDDs will appear (except one connected via
> the m.2 slot).
>
> From https://wiki.kobol.io/helios64/sata/#hdd-power:
>
> > The power delivery of the HDDs is divided into two group:
> >
> >     HDD Rail A (Max. 3x Drives)
> >     HDD Rail B (Max. 2x Drives)
> >
> > Helios64 implements a power staggering approach where HDD Rail A will be
> > powered up first, then few seconds later HDD Rail B will be powered up.
> > This power control scenario is performed to reduce the inrush current
> > during disk spin-up.
>
> In practice, this power staggering approach will be included in the
> bootloader (not in the kernel), as we might want to boot from a SATA
> drive.
>
> From my experiments, if the bootloader doesn't implement the power
> staggering, only one HDD will get recognized (probably cause the others
> didn't boot due to few power).
>
> Still, it makes sense to expose this block in the device-tree, so the
> kernel can ensure both rails are on (and this can be shared with
> u-boot).
>
> [1] https://github.com/armbian/build/blob/744ea89a589d62cb6f409baab60fc6664520bc39/patch/kernel/archive/rockchip64-5.14/add-board-helios64.patch
>
> Signed-off-by: Florian Klink <flokli@flokli.de>
> ---
>  .../dts/rockchip/rk3399-kobol-helios64.dts    | 32 +++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts b/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> index 738cfd21df3e..93745dcc2af6 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-kobol-helios64.dts
> @@ -82,6 +82,30 @@ led-1 {
>                 };
>         };
>
> +       hdd_a_power: hdd-a-power {
> +               compatible = "regulator-fixed";
> +               enable-active-high;
> +               gpio = <&gpio1 RK_PA0 GPIO_ACTIVE_HIGH>;
> +               pinctrl-0 = <&hdd_a_power_en>;
> +               pinctrl-names = "default";
> +               regulator-always-on;
> +               regulator-boot-on;
> +               regulator-name = "hdd_a_power";
> +               startup-delay-us = <2000000>;
> +       };
> +
> +       hdd_b_power: hdd-b-power {
> +               compatible = "regulator-fixed";
> +               enable-active-high;
> +               gpio = <&gpio1 RK_PA1 GPIO_ACTIVE_HIGH>;
> +               pinctrl-0 = <&hdd_b_power_en>;
> +               pinctrl-names = "default";
> +               regulator-always-on;
> +               regulator-boot-on;
> +               regulator-name = "hdd_b_power";
> +               startup-delay-us = <2000000>;
> +       };
> +
>         pcie_power: pcie-power {
>                 compatible = "regulator-fixed";
>                 enable-active-high;
> @@ -422,6 +446,14 @@ pmic_int_l: pmic-int-l {
>         };
>
>         power {
> +               hdd_a_power_en: hdd-a-power-en {
> +                       rockchip,pins = <1 RK_PA0 RK_FUNC_GPIO &pcfg_pull_none>;
> +               };
> +
> +               hdd_b_power_en: hdd-b-power-en {
> +                       rockchip,pins = <1 RK_PA1 RK_FUNC_GPIO &pcfg_pull_none>;
> +               };
> +
>                 vcc5v0_usb_en: vcc5v0-usb-en {
>                         rockchip,pins = <1 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
>                 };
> --
> 2.33.0
>


-- 
Dennis Gilmore
Multiple Architecture Portfolio Enablement
T: +1-312-660-3523