Re: [PATCH v4 3/4] arm64: dts: allwinner: h616: Add BigTreeTech CB1 SoM & boards support

From: Jernej Škrabec
Date: Mon Aug 14 2023 - 06:23:15 EST

Dne nedelja, 13. avgust 2023 ob 18:20:00 CEST je Martin Botka napisal(a):
> On Sun, Aug 13 2023 at 05:52:10 PM +02:00:00, Jernej Škrabec
>
> <jernej.skrabec@xxxxxxxxx> wrote:
> > Hi Martin,
> >
> > since this will be obviously delayed to 6.7 cycle due to mfd patch
> > not being
> > merged in time, I have few nits below.
> >
> > Dne ponedeljek, 07. avgust 2023 ob 16:53:23 CEST je Martin Botka
> >
> > napisal(a):
> >> From: Martin Botka <martin.botka@xxxxxxxxxxxxxx>
> >>
> >> CB1 is Compute Module style board that plugs into Rpi board style
> >>
> >> adapter or
> >>
> >> Manta 3D printer boards (M4P/M8P).
> >>
> >> The SoM features:
> >> - H616 SoC
> >> - 1GiB of RAM
> >> - AXP313A PMIC
> >> - RTL8189FTV WiFi
> >>
> >> Boards feature:
> >> - 4x USB via USB2 hub (usb1 on SoM).
> >> - SDcard slot for loading images.
> >> - Ethernet port wired to the internal PHY. (100M)
> >> - 2x HDMI 2.0. (Only 1 usable on CB1)
> >> - Power and Status LEDs. (Only Status LED usable on CB1)
> >> - 40 pin GPIO header
> >>
> >> Currently working:
> >> - Booting
> >> - USB
> >> - UART
> >> - MMC
> >> - Status LED
> >> - WiFi (RTL8189FS via out of tree driver)
> >>
> >> I didnt want to duplicate things so the manta DTS can also be used
> >>
> >> on BTT
> >>
> >> pi4b adapter. CB1 SoM has its own DTSI file in case other boards
> >>
> >> shows up
> >>
> >> that accept this SoM.
> >>
> >> Signed-off-by: Martin Botka <martin.botka@xxxxxxxxxxxxxx>
> >> Reviewed-by: Andre Przywara <andre.przywara@xxxxxxx>
> >> ---
> >>
> >> Changes in V2:
> >> - Fixed whitespace errors
> >> - Move UART into carrier boards and BTT Pi
> >> - Remove usb1-vbus regulator
> >> - Fix ranges and naming of AXP313A rails
> >> - Add comment specifying why broken-cd in mmc0 is needed
> >> - Rename sdio_wifi to wifi
> >> - Specify in commit description that USB-OTG doesnt work
> >>
> >> Changes in V3:
> >> - Add missed semicolons
> >> - Move model string from dtsi to board dts
> >> - Add cb1 compatible
> >> - Remove extra empty line
> >>
> >> Changed in V4:
> >> - Extend the range of vcc-dram to 1.5V (1.35V max caused issues
> >>
> >> with
> >>
> >> booting up
> >>
> >> arch/arm64/boot/dts/allwinner/Makefile | 1 +
> >> .../sun50i-h616-bigtreetech-cb1-manta.dts | 35 +++++
> >> .../sun50i-h616-bigtreetech-cb1.dtsi | 140
> >>
> >> ++++++++++++++++++
> >>
> >> 3 files changed, 176 insertions(+)
> >> create mode 100644
> >>
> >> arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1-manta.dts
> >>
> >> create
> >>
> >> mode 100644
> >>
> >> arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi
> >>
> >> diff --git a/arch/arm64/boot/dts/allwinner/Makefile
> >> b/arch/arm64/boot/dts/allwinner/Makefile index
> >>
> >> 6a96494a2e0a..7b386428510b
> >>
> >> 100644
> >> --- a/arch/arm64/boot/dts/allwinner/Makefile
> >> +++ b/arch/arm64/boot/dts/allwinner/Makefile
> >> @@ -38,5 +38,6 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64.dtb
> >>
> >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64-model-b.dtb
> >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-tanix-tx6.dtb
> >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-tanix-tx6-mini.dtb
> >>
> >> +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-bigtreetech-cb1-manta.dtb
> >>
> >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-orangepi-zero2.dtb
> >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-x96-mate.dtb
> >>
> >> diff --git
> >>
> >> a/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1-manta.dts
> >>
> >> b/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1-manta.dts
> >> new
> >>
> >> file mode 100644
> >> index 000000000000..dbce61b355d6
> >> --- /dev/null
> >> +++
> >>
> >> b/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1-manta.dts
> >>
> >> @@ -0,0 +1,35 @@
> >> +// SPDX-License-Identifier: (GPL-2.0+ or MIT)
> >> +/*
> >> + * Copyright (C) 2023 Martin Botka <martin.botka@xxxxxxxxxxxxxx>.
> >> + */
> >> +
> >> +/dts-v1/;
> >> +
> >> +#include "sun50i-h616-bigtreetech-cb1.dtsi"
> >> +
> >> +/ {
> >> + model = "BigTreeTech CB1";
> >> + compatible = "bigtreetech,cb1-manta", "bigtreetech,cb1",
> >> "allwinner,sun50i-h616"; +
> >> + aliases {
> >> + serial0 = &uart0;
> >> + };
> >> +
> >> + chosen {
> >> + stdout-path = "serial0:115200n8";
> >> + };
> >> +};
> >> +
> >> +&ehci1 {
> >> + status = "okay";
> >> +};
> >> +
> >> +&ohci1 {
> >> + status = "okay";
> >> +};
> >> +
> >> +&uart0 {
> >> + pinctrl-names = "default";
> >> + pinctrl-0 = <&uart0_ph_pins>;
> >> + status = "okay";
> >> +};
> >> diff --git
> >>
> >> a/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi
> >>
> >> b/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi
> >>
> >> new file
> >>
> >> mode 100644
> >> index 000000000000..5e756f217813
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi
> >> @@ -0,0 +1,140 @@
> >> +// SPDX-License-Identifier: (GPL-2.0+ or MIT)
> >> +/*
> >> + * Copyright (C) 2023 Martin Botka <martin.botka@xxxxxxxxxxxxxx>.
> >> + */
> >> +
> >> +/dts-v1/;
> >> +
> >> +#include "sun50i-h616.dtsi"
> >> +
> >> +#include <dt-bindings/gpio/gpio.h>
> >> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> >> +#include <dt-bindings/leds/common.h>
> >> +
> >> +/ {
> >> + aliases {
> >> + ethernet0 = &rtl8189ftv;
> >> + };
> >> +
> >> + leds {
> >> + compatible = "gpio-leds";
> >> +
> >> + led-0 {
> >> + function = LED_FUNCTION_STATUS;
> >> + color = <LED_COLOR_ID_GREEN>;
> >> + gpios = <&pio 7 5 GPIO_ACTIVE_HIGH>; /* PH5
> >
> > */
> >
> >> + };
> >> + };
> >> +
> >> + reg_vcc5v: regulator-vcc5v {
> >> + /* board wide 5V supply from carrier boards */
> >> + compatible = "regulator-fixed";
> >> + regulator-name = "vcc-5v";
> >> + regulator-min-microvolt = <5000000>;
> >> + regulator-max-microvolt = <5000000>;
> >> + regulator-always-on;
> >> + };
> >> +
> >> + reg_vcc33_wifi: vcc33-wifi {
> >> + /* Always on 3.3V regulator for WiFi */
> >
> > Please drop the comment. It's pretty obvious from properties.
> >
> >> + compatible = "regulator-fixed";
> >> + regulator-name = "vcc33-wifi";
> >> + regulator-min-microvolt = <3300000>;
> >> + regulator-max-microvolt = <3300000>;
> >> + regulator-always-on;
> >> + vin-supply = <&reg_vcc5v>;
> >> + };
> >> +
> >> + reg_vcc_wifi_io: vcc-wifi-io {
> >> + /* Always on 1.8V/300mA regulator for WiFi */
> >
> > Ditto.
> >
> > Once fixed, you can add:
> > Reviewed-by: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
> >
> > Best regards,
> > Jernej
>
> Got it for both comments. Will send V5 tomorrow.

You should not send new versions before all discussions are finished, in this
case for adxl345. If in doubt, ask, but certainly wait on response.

Best regards,
Jernej

>
> Cheers,
> Martin
>
> >> + compatible = "regulator-fixed";
> >> + regulator-name = "vcc-wifi-io";
> >> + regulator-min-microvolt = <1800000>;
> >> + regulator-max-microvolt = <1800000>;
> >> + regulator-always-on;
> >> + vin-supply = <&reg_vcc33_wifi>;
> >> + };
> >> +
> >> + wifi_pwrseq: wifi-pwrseq {
> >> + compatible = "mmc-pwrseq-simple";
> >> + clocks = <&rtc 1>;
> >> + clock-names = "ext_clock";
> >> + reset-gpios = <&pio 6 18 GPIO_ACTIVE_LOW>; /* PG18 */
> >> + post-power-on-delay-ms = <200>;
> >> + };
> >> +};
> >> +
> >> +&mmc0 {
> >> + vmmc-supply = <&reg_dldo1>;
> >> + /* Card detection pin is not connected */
> >> + broken-cd;
> >> + bus-width = <4>;
> >> + status = "okay";
> >> +};
> >> +
> >> +&mmc1 {
> >> + vmmc-supply = <&reg_vcc33_wifi>;
> >> + vqmmc-supply = <&reg_vcc_wifi_io>;
> >> + mmc-pwrseq = <&wifi_pwrseq>;
> >> + bus-width = <4>;
> >> + non-removable;
> >> + mmc-ddr-1_8v;
> >> + status = "okay";
> >> +
> >> + rtl8189ftv: wifi@1 {
> >> + reg = <1>;
> >> + };
> >> +};
> >> +
> >> +&r_i2c {
> >> + status = "okay";
> >> +
> >> + axp313a: pmic@36 {
> >> + compatible = "x-powers,axp313a";
> >> + reg = <0x36>;
> >> + interrupt-controller;
> >> + #interrupt-cells = <1>;
> >> +
> >> + regulators{
> >> + reg_dcdc1: dcdc1 {
> >> + regulator-name = "vdd-gpu-sys";
> >> + regulator-min-microvolt =
> >
> > <810000>;
> >
> >> + regulator-max-microvolt =
> >
> > <990000>;
> >
> >> + regulator-always-on;
> >> + };
> >> +
> >> + reg_dcdc2: dcdc2 {
> >> + regulator-name = "vdd-cpu";
> >> + regulator-min-microvolt =
> >
> > <810000>;
> >
> >> + regulator-max-microvolt =
> >
> > <1100000>;
> >
> >> + regulator-ramp-delay = <200>;
> >> + regulator-always-on;
> >> + };
> >> +
> >> + reg_dcdc3: dcdc3 {
> >> + regulator-name = "vcc-dram";
> >> + regulator-min-microvolt =
> >
> > <1350000>;
> >
> >> + regulator-max-microvolt =
> >
> > <1500000>;
> >
> >> + regulator-always-on;
> >> + };
> >> +
> >> + reg_aldo1: aldo1 {
> >> + regulator-name = "vcc-1v8-pll";
> >> + regulator-min-microvolt =
> >
> > <1800000>;
> >
> >> + regulator-max-microvolt =
> >
> > <1800000>;
> >
> >> + regulator-always-on;
> >> + };
> >> +
> >> + reg_dldo1: dldo1 {
> >> + regulator-name = "vcc-3v3-io";
> >> + regulator-min-microvolt =
> >
> > <3300000>;
> >
> >> + regulator-max-microvolt =
> >
> > <3300000>;
> >
> >> + regulator-always-on;
> >> + };
> >> + };
> >> + };
> >> +};
> >> +
> >> +&usbphy {
> >> + status = "okay";
> >> +};