Received-SPF: pass (google.com: domain of linux-kernel+bounces-51087-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Sat, 3 Feb 2024 20:36:42 +0800
From: Jisheng Zhang <jszhang@kernel.org>
To: Jernej =?utf-8?Q?=C5=A0krabec?= <jernej.skrabec@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
	Samuel Holland <samuel@sholland.org>,
	Andre Przywara <andre.przywara@arm.com>,
	Icenowy Zheng <uwu@icenowy.me>, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] arm64: dts: allwinner: h616: Add Sipeed Longan SoM
 3H and Pi 3H board support
Message-ID: <Zb4zWsfzgJkZa2KQ@xhacker>
References: <20231228145647.1470-1-jszhang@kernel.org>
 <20231228145647.1470-3-jszhang@kernel.org>
 <2589997.Lt9SDvczpP@jernej-laptop>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <2589997.Lt9SDvczpP@jernej-laptop>

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.

Thanks

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