Re: [PATCH 3/3] arm64: dts: allwinner: h618: add Transpeed 8K618-T TV box

From: Andre Przywara <andre.przywara@arm.com>
To: "Jernej Škrabec" <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>,
	Icenowy Zheng <uwu@icenowy.me>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-sunxi@lists.linux.dev
Subject: Re: [PATCH 3/3] arm64: dts: allwinner: h618: add Transpeed 8K618-T TV box
Date: Thu, 14 Dec 2023 01:23:57 +0000	[thread overview]
Message-ID: <20231214012357.37526feb@minigeek.lan> (raw)
In-Reply-To: <4533903.LvFx2qVVIh@archlinux>

On Wed, 13 Dec 2023 20:22:55 +0100
Jernej Škrabec <jernej.skrabec@gmail.com> wrote:

Hi Jernej,

thanks for having a look, much appreciated.

> On Tuesday, December 12, 2023 2:08:59 AM CET Andre Przywara wrote:
> > This is a Chinese TV box, probably very similar if not identical to
> > various other cheap TV boxes with the same specs:
> >       - Allwinner H618 SoC (4 * Arm Cortex-A53 cores, 1MB L2 cache)
> >       - 2 or 4GiB DDR3L DRAM
> >       - 32, 64, or 128 GiB eMMC flash
> >       - AXP313a PMIC
> >       - 100 Mbit/s Ethernet (using yet unsupported internal PHY)
> >       - HDMI port
> >       - 2 * USB 2.0 ports
> >       - microSD card slot
> >       - 3.5mm A/V port
> >       - 7-segment display
> >       - 5V barrel plug power supply
> > 
> > The PCB provides holes for soldering a UART header or cable, this is
> > connected to the debug UART0. UART1 is used for the Bluetooth chip,
> > although this isn't working yet.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  arch/arm64/boot/dts/allwinner/Makefile        |   1 +
> >  .../sun50i-h618-transpeed-8k618-t.dts         | 164 ++++++++++++++++++
> >  2 files changed, 165 insertions(+)
> >  create mode 100644
> > arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts
> > 
> > diff --git a/arch/arm64/boot/dts/allwinner/Makefile
> > b/arch/arm64/boot/dts/allwinner/Makefile index 3aca6787a1679..91d505b385de5
> > 100644
> > --- a/arch/arm64/boot/dts/allwinner/Makefile
> > +++ b/arch/arm64/boot/dts/allwinner/Makefile
> > @@ -43,3 +43,4 @@ 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-orangepi-zero3.dtb
> > +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h618-transpeed-8k618-t.dtb
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts
> > b/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts new file
> > mode 100644
> > index 0000000000000..fd7ea7bcde2c1
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h618-transpeed-8k618-t.dts
> > @@ -0,0 +1,164 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (C) 2023 Arm Ltd.
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "sun50i-h616.dtsi"
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +/ {
> > +	model = "Transpeed 8K618-T";
> > +	compatible = "transpeed,8k618-t", "allwinner,sun50i-h618";
> > +
> > +	aliases {
> > +		serial0 = &uart0;
> > +		serial1 = &uart1;  
> 
> Any particular reason for above alias?

Not really, and since UART1 is used for Bluetooth, its name should not
matter at all, the BT driver will just use its parent device.
Will remove it.

> 
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +	};
> > +
> > +	reg_vcc5v: vcc5v {
> > +		/* board wide 5V supply directly from the DC input */
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "vcc-5v";
> > +		regulator-min-microvolt = <5000000>;
> > +		regulator-max-microvolt = <5000000>;
> > +		regulator-always-on;
> > +	};
> > +
> > +	reg_vcc3v3: vcc3v3 {
> > +		/* discrete 3.3V regulator */
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "vcc-3v3";
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +		regulator-always-on;
> > +	};
> > +};
> > +
> > +&ehci0 {
> > +	status = "okay";
> > +};
> > +
> > +&ehci1 {
> > +	status = "okay";
> > +};
> > +
> > +&ir {
> > +	status = "okay";
> > +};
> > +
> > +&mmc0 {
> > +	vmmc-supply = <&reg_dldo1>;
> > +	cd-gpios = <&pio 8 16 GPIO_ACTIVE_LOW>;	/* PI16 */
> > +	bus-width = <4>;
> > +	status = "okay";
> > +};
> > +
> > +&mmc2 {
> > +	vmmc-supply = <&reg_dldo1>;
> > +	vqmmc-supply = <&reg_aldo1>;
> > +	bus-width = <8>;
> > +	non-removable;
> > +	cap-mmc-hw-reset;
> > +	mmc-ddr-1_8v;
> > +	mmc-hs200-1_8v;
> > +	status = "okay";
> > +};
> > +
> > +&ohci0 {
> > +	status = "okay";
> > +};
> > +
> > +&ohci1 {
> > +	status = "okay";
> > +};
> > +
> > +&r_i2c {
> > +	status = "okay";
> > +
> > +	axp313: pmic@36 {
> > +		compatible = "x-powers,axp313a";
> > +		reg = <0x36>;
> > +		#interrupt-cells = <1>;
> > +		interrupt-controller;
> > +
> > +		vin1-supply = <&reg_vcc5v>;
> > +		vin2-supply = <&reg_vcc5v>;
> > +		vin3-supply = <&reg_vcc5v>;
> > +
> > +		regulators {
> > +			/* Supplies VCC-PLL, so needs to be always   
> on. */
> > +			reg_aldo1: aldo1 {
> > +				regulator-always-on;
> > +				regulator-min-microvolt =   
> <1800000>;
> > +				regulator-max-microvolt =   
> <1800000>;
> > +				regulator-name = "vcc1v8";  
> 
> It would be great to expand all PMIC regulator names with (known) areas they 
> power, so something like "vcc1v8-pll-pc". With that, I think you can remove 
> comments.

Alright, will do. I will stick with those users that I am pretty sure
of, either because they are mentioned in the BSP DT, or because they
must be supplied by that rail: there seem to be only one 1.8V source.

Cheers,
Andre

> 
> Best regards,
> Jernej
> 
> > +			};
> > +
> > +			/* Supplies VCC-IO, so needs to be always on.   
> */
> > +			reg_dldo1: dldo1 {
> > +				regulator-always-on;
> > +				regulator-min-microvolt =   
> <3300000>;
> > +				regulator-max-microvolt =   
> <3300000>;
> > +				regulator-name = "vcc3v3";
> > +			};
> > +
> > +			reg_dcdc1: dcdc1 {
> > +				regulator-always-on;
> > +				regulator-min-microvolt =   
> <810000>;
> > +				regulator-max-microvolt =   
> <990000>;
> > +				regulator-name = "vdd-gpu-sys";
> > +			};
> > +
> > +			reg_dcdc2: dcdc2 {
> > +				regulator-always-on;
> > +				regulator-min-microvolt =   
> <810000>;
> > +				regulator-max-microvolt =   
> <1100000>;
> > +				regulator-name = "vdd-cpu";
> > +			};
> > +
> > +			reg_dcdc3: dcdc3 {
> > +				regulator-always-on;
> > +				regulator-min-microvolt =   
> <1360000>;
> > +				regulator-max-microvolt =   
> <1360000>;
> > +				regulator-name = "vdd-dram";
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&pio {
> > +	vcc-pc-supply = <&reg_aldo1>;
> > +	vcc-pg-supply = <&reg_dldo1>;
> > +	vcc-ph-supply = <&reg_dldo1>;
> > +	vcc-pi-supply = <&reg_dldo1>;
> > +};
> > +
> > +&uart0 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart0_ph_pins>;
> > +	status = "okay";
> > +};
> > +
> > +&uart1 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
> > +	uart-has-rtscts;
> > +	status = "okay";
> > +};
> > +
> > +&usbotg {
> > +	dr_mode = "host";	/* USB A type receptable */
> > +	status = "okay";
> > +};
> > +
> > +&usbphy {
> > +	status = "okay";
> > +};  
> 
> 
> 
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel