Re: [PATCH 2/2] arm64: dts: allwinner: h616: Add Sipeed Longan SoM 3H and Pi 3H board support

Jernej Škrabec
2024-02-05T18:29:12Z

Dne sobota, 03. februar 2024 ob 13:36:42 CET je Jisheng Zhang napisal(a):
> On Tue, Jan 09, 2024 at 09:37:03PM +0100, Jernej Škrabec wrote:
> > Hi Jisheng,
>
> Hi Jernej,
>
> >
> > Andre gave you thorough review already, so just one additional comment
> > below.
> >
> > Dne četrtek, 28. december 2023 ob 15:56:47 CET je Jisheng Zhang napisal(a):
> > > The Sipeed Longan SoM 3H is a system on module based on the Allwinner
> > > H618 SoC. The SoM features:
> > >
> > > - Four ARM Cortex-A53 cores, Mali-G31 MP2 GPU
> > > - 2/4 GiB LPDDR4 DRAM SoMs
> > > - AXP313a PMIC
> > > - eMMC
> > >
> > > The Sipeed Longan PI 3H is a development board based on the above SoM.
> > > The board features:
> > > - Longan SoM 3H
> > > - Raspberry-Pi-1 compatible GPIO header
> > > - 2 USB 2.0 host port
> > > - 1 USB 2.0 type C port (power supply + OTG)
> > > - MicroSD slot
> > > - 1Gbps Ethernet port (via RTL8211 PHY)
> > > - HDMI port
> > > - WiFi/BT chip
> > >
> > > Add the devicetree file describing the currently supported features,
> > > namely PMIC, LEDs, UART, SD card, eMMC, USB and Ethernet.
> > >
> > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > > ---
> > > arch/arm64/boot/dts/allwinner/Makefile | 1 +
> > > .../sun50i-h618-longan-module-3h.dtsi | 82 +++++++++++
> > > .../dts/allwinner/sun50i-h618-longanpi-3h.dts | 133 ++++++++++++++++++
> > > 3 files changed, 216 insertions(+)
> > > create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h618-longan-module-3h.dtsi
> > > create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h618-longanpi-3h.dts
> > >
> > > diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
> > > index 3aca6787a167..00db504a9b8c 100644
> > > --- a/arch/arm64/boot/dts/allwinner/Makefile
> > > +++ b/arch/arm64/boot/dts/allwinner/Makefile
> > > @@ -42,4 +42,5 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-bigtreetech-cb1-manta.dtb
> > > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-bigtreetech-pi.dtb
> > > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-orangepi-zero2.dtb
> > > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-x96-mate.dtb
> > > +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-longanpi-3h.dtb
> > > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-orangepi-zero3.dtb
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-longan-module-3h.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h618-longan-module-3h.dtsi
> > > new file mode 100644
> > > index 000000000000..88a7d287b73c
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-longan-module-3h.dtsi
> > > @@ -0,0 +1,82 @@
> > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > > +/*
> > > + * Copyright (C) Jisheng Zhang <jszhang@kernel.org>
> > > + */
> > > +
> > > +/dts-v1/;
> > > +
> > > +#include "sun50i-h616.dtsi"
> > > +
> > > +/ {
> > > + model = "Sipeed Longan Module 3H";
> > > + compatible = "sipeed,longan-module-3h", "allwinner,sun50i-h618";
> > > +};
> > > +
> > > +&mmc2 {
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&mmc2_pins>;
> > > + vmmc-supply = <&reg_dldo1>;
> > > + vqmmc-supply = <&reg_aldo1>;
> > > + bus-width = <8>;
> > > + non-removable;
> > > + cap-mmc-hw-reset;
> > > + mmc-ddr-1_8v;
> >
> > I think above is superfluous if hs200 is also defined.
>
> I read the mmc code again, IMHO, mmc-ddr-1_8v and mmc-hs200-1_8v
> are for different caps, the former is for MMC_CAP_1_8V_DDR and
> the later is for MMC_CAP2_HS200_1_8V_SDR, I must admit that
> mmc-hs200-1_8v does imply mmc-ddr-1_8v in normal host controllers
> but they are different, and I found other dts files also contain
> both, so I kept this as is in v2.

Yeah, this was oversight. Most DTs have only second one defined.

Best regards,
Jernej

>
> Thanks
>
> >
> > > + mmc-hs200-1_8v;
> > > + status = "okay";
> > > +};
> > > +
>
>