[2/2] arm64: dts: amlogic: add libretech cottonwood support

  Add support for the Libretech cottonwood board family.
These 2 boards are based on the same PCB, with an RPi B form factor.

The "Alta" board uses an a311d while the "Solitude" variant uses an s905d3.

Co-developed-by: Da Xue <da.xue@libretech.co>
Signed-off-by: Da Xue <da.xue@libretech.co>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/Makefile          |   2 +
 .../amlogic/meson-g12b-a311d-libretech-cc.dts | 133 ++++
 .../amlogic/meson-libretech-cottonwood.dtsi   | 610 ++++++++++++++++++
 .../amlogic/meson-sm1-s905d3-libretech-cc.dts |  89 +++
 4 files changed, 834 insertions(+)
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
  

Hi,

On 02/10/2023 16:10, Jerome Brunet wrote:
> Add support for the Libretech cottonwood board family.
> These 2 boards are based on the same PCB, with an RPi B form factor.
> 
> The "Alta" board uses an a311d while the "Solitude" variant uses an s905d3.
> 
> Co-developed-by: Da Xue <da.xue@libretech.co>
> Signed-off-by: Da Xue <da.xue@libretech.co>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>   arch/arm64/boot/dts/amlogic/Makefile          |   2 +
>   .../amlogic/meson-g12b-a311d-libretech-cc.dts | 133 ++++
>   .../amlogic/meson-libretech-cottonwood.dtsi   | 610 ++++++++++++++++++
>   .../amlogic/meson-sm1-s905d3-libretech-cc.dts |  89 +++
>   4 files changed, 834 insertions(+)
>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
> 
> diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile
> index 4ce401d17b63..cc8b34bd583d 100644
> --- a/arch/arm64/boot/dts/amlogic/Makefile
> +++ b/arch/arm64/boot/dts/amlogic/Makefile
> @@ -18,6 +18,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-g12b-bananapi-cm4-cm4io.dtb
>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gsking-x.dtb
>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking-pro.dtb
>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking.dtb
> +dtb-$(CONFIG_ARCH_MESON) += meson-g12b-a311d-libretech-cc.dtb
>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-go-ultra.dtb
>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2-plus.dtb
>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2.dtb
> @@ -73,6 +74,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-sm1-bananapi-m2-pro.dtb
>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-bananapi-m5.dtb
>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-h96-max.dtb
>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-khadas-vim3l.dtb
> +dtb-$(CONFIG_ARCH_MESON) += meson-sm1-s905d3-libretech-cc.dtb
>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-c4.dtb
>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-hc4.dtb
>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
> new file mode 100644
> index 000000000000..fc890e235dbd
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2023 BayLibre, SAS.
> + * Author: Jerome Brunet <jbrunet@baylibre.com>
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/clock/g12a-clkc.h>
> +#include "meson-g12b-a311d.dtsi"
> +#include "meson-libretech-cottonwood.dtsi"
> +
> +/ {
> +	compatible = "libretech,aml-a311d-cc", "amlogic,a311d", "amlogic,g12b";
> +	model = "Libre Computer AML-A311D-CC Alta";
> +
> +	vddcpu_a: regulator-vddcpu-a {
> +		compatible = "pwm-regulator";
> +		regulator-name = "VDDCPU_A";
> +		regulator-min-microvolt = <730000>;
> +		regulator-max-microvolt = <1011000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +		pwm-supply = <&dc_in>;
> +		pwms = <&pwm_ab 0 1250 0>;
> +		pwm-dutycycle-range = <100 0>;
> +	};
> +
> +	sound {
> +		model = "Alta";

I think those sound model properties should be coherent with the
other Libre Computer boards:
arch/arm64/boot/dts/amlogic/meson-gx-libretech-pc.dtsi:         model = "LIBRETECH-PC";
arch/arm64/boot/dts/amlogic/meson-gxl-s805x-libretech-ac.dts:           model = "LIBRETECH-AC";
arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc-v2.dts:                model = "LIBRETECH-CC-V2";
arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts:           model = "LIBRETECH-CC";

It's ok to change the scheme since it's tried to keep the name under the 15 characters limit,
will the next board keep this naming ?


> +		audio-routing = "TDMOUT_A IN 0", "FRDDR_A OUT 0",
> +				"TDMOUT_A IN 1", "FRDDR_B OUT 0",
> +				"TDMOUT_A IN 2", "FRDDR_C OUT 0",
> +				"TDM_A Playback", "TDMOUT_A OUT",
> +				"TDMOUT_B IN 0", "FRDDR_A OUT 1",
> +				"TDMOUT_B IN 1", "FRDDR_B OUT 1",
> +				"TDMOUT_B IN 2", "FRDDR_C OUT 1",
> +				"TDM_B Playback", "TDMOUT_B OUT",
> +				"TDMOUT_C IN 0", "FRDDR_A OUT 2",
> +				"TDMOUT_C IN 1", "FRDDR_B OUT 2",
> +				"TDMOUT_C IN 2", "FRDDR_C OUT 2",
> +				"TDM_C Playback", "TDMOUT_C OUT",
> +				"TDMIN_A IN 0", "TDM_A Capture",
> +				"TDMIN_B IN 0", "TDM_A Capture",
> +				"TDMIN_C IN 0", "TDM_A Capture",
> +				"TDMIN_A IN 3", "TDM_A Loopback",
> +				"TDMIN_B IN 3", "TDM_A Loopback",
> +				"TDMIN_C IN 3", "TDM_A Loopback",
> +				"TDMIN_A IN 1", "TDM_B Capture",
> +				"TDMIN_B IN 1", "TDM_B Capture",
> +				"TDMIN_C IN 1", "TDM_B Capture",
> +				"TDMIN_A IN 4", "TDM_B Loopback",
> +				"TDMIN_B IN 4", "TDM_B Loopback",
> +				"TDMIN_C IN 4", "TDM_B Loopback",
> +				"TDMIN_A IN 2", "TDM_C Capture",
> +				"TDMIN_B IN 2", "TDM_C Capture",
> +				"TDMIN_C IN 2", "TDM_C Capture",
> +				"TDMIN_A IN 5", "TDM_C Loopback",
> +				"TDMIN_B IN 5", "TDM_C Loopback",
> +				"TDMIN_C IN 5", "TDM_C Loopback",
> +				"TODDR_A IN 0", "TDMIN_A OUT",
> +				"TODDR_B IN 0", "TDMIN_A OUT",
> +				"TODDR_C IN 0", "TDMIN_A OUT",
> +				"TODDR_A IN 1", "TDMIN_B OUT",
> +				"TODDR_B IN 1", "TDMIN_B OUT",
> +				"TODDR_C IN 1", "TDMIN_B OUT",
> +				"TODDR_A IN 2", "TDMIN_C OUT",
> +				"TODDR_B IN 2", "TDMIN_C OUT",
> +				"TODDR_C IN 2", "TDMIN_C OUT",
> +				"Lineout", "ACODEC LOLP",
> +				"Lineout", "ACODEC LORP";
> +	};
> +};
> +
> +&cpu_opp_table_0 {
> +		opp-1800000000 {
> +			opp-hz = /bits/ 64 <1800000000>;
> +			opp-microvolt = <1001000>;
> +		};
> +
> +		opp-2016000000 {
> +			opp-hz = /bits/ 64 <2016000000>;
> +			opp-microvolt = <1011000>;
> +		};
> +};
> +
> +&cpu0 {
> +	cpu-supply = <&vddcpu_b>;
> +	operating-points-v2 = <&cpu_opp_table_0>;
> +	clocks = <&clkc CLKID_CPU_CLK>;
> +	clock-latency = <50000>;
> +};
> +
> +&cpu1 {
> +	cpu-supply = <&vddcpu_b>;
> +	operating-points-v2 = <&cpu_opp_table_0>;
> +	clocks = <&clkc CLKID_CPU_CLK>;
> +	clock-latency = <50000>;
> +};
> +
> +&cpu100 {
> +	cpu-supply = <&vddcpu_a>;
> +	operating-points-v2 = <&cpub_opp_table_1>;
> +	clocks = <&clkc CLKID_CPUB_CLK>;
> +	clock-latency = <50000>;
> +};
> +
> +&cpu101 {
> +	cpu-supply = <&vddcpu_a>;
> +	operating-points-v2 = <&cpub_opp_table_1>;
> +	clocks = <&clkc CLKID_CPUB_CLK>;
> +	clock-latency = <50000>;
> +};
> +
> +&cpu102 {
> +	cpu-supply = <&vddcpu_a>;
> +	operating-points-v2 = <&cpub_opp_table_1>;
> +	clocks = <&clkc CLKID_CPUB_CLK>;
> +	clock-latency = <50000>;
> +};
> +
> +&cpu103 {
> +	cpu-supply = <&vddcpu_a>;
> +	operating-points-v2 = <&cpub_opp_table_1>;
> +	clocks = <&clkc CLKID_CPUB_CLK>;
> +	clock-latency = <50000>;
> +};
> +
> +&pwm_ab {
> +	pinctrl-0 = <&pwm_a_e_pins>, <&pwm_b_x7_pins>;
> +	clocks = <&xtal>, <&xtal>;
> +	clock-names = "clkin0", "clkin1";
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi b/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
> new file mode 100644
> index 000000000000..a7fc8963ff54
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
> @@ -0,0 +1,610 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2023 BayLibre, SAS.
> + * Author: Jerome Brunet <jbrunet@baylibre.com>
> + */
> +
> +#include <dt-bindings/clock/g12a-clkc.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/gpio/meson-g12a-gpio.h>
> +#include <dt-bindings/sound/meson-g12a-toacodec.h>
> +#include <dt-bindings/sound/meson-g12a-tohdmitx.h>
> +
> +/ {
> +	aliases {
> +		serial0 = &uart_AO;
> +		ethernet0 = &ethmac;
> +		spi0 = &spifc;
> +	};
> +
> +	memory@0 {
> +		device_type = "memory";
> +		reg = <0x0 0x0 0x0 0x80000000>;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	dioo2133: audio-amplifier-0 {
> +		compatible = "simple-audio-amplifier";
> +		enable-gpios = <&gpio GPIOX_0 GPIO_ACTIVE_HIGH>;
> +		VCC-supply = <&vcc_5v>;
> +		sound-name-prefix = "10U2";
> +	};
> +
> +	/* TOFIX: handle CVBS_DET on SARADC channel 0 */
> +	cvbs-connector {
> +		compatible = "composite-video-connector";
> +
> +		port {
> +			cvbs_connector_in: endpoint {
> +				remote-endpoint = <&cvbs_vdac_out>;
> +			};
> +		};
> +	};
> +
> +	emmc_pwrseq: emmc-pwrseq {
> +		compatible = "mmc-pwrseq-emmc";
> +		reset-gpios = <&gpio BOOT_12 GPIO_ACTIVE_LOW>;
> +	};
> +
> +	hdmi-connector {
> +		compatible = "hdmi-connector";
> +		type = "a";
> +
> +		port {
> +			hdmi_connector_in: endpoint {
> +				remote-endpoint = <&hdmi_tx_tmds_out>;
> +			};
> +		};
> +	};
> +
> +	leds-pwm {
> +		compatible = "pwm-leds";
> +
> +		led-green {
> +			color = <LED_COLOR_ID_GREEN>;
> +			function = LED_FUNCTION_STATUS;
> +			linux,default-trigger = "default-on";
> +			panic-indicator;
> +			max-brightness = <255>;
> +			pwms = <&pwm_cd 1 1250 0>;
> +			active-low;
> +		};
> +
> +		led-blue {
> +			color = <LED_COLOR_ID_BLUE>;
> +			function = LED_FUNCTION_ACTIVITY;
> +			linux,default-trigger = "activity";
> +			max-brightness = <255>;
> +			pwms = <&pwm_ab 1 1250 0>;
> +			active-low;
> +		};
> +	};
> +
> +	leds-gpio {
> +		compatible = "gpio-leds";
> +
> +		led-orange {
> +			color = <LED_COLOR_ID_AMBER>;
> +			function = LED_FUNCTION_STANDBY;
> +			gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
> +		};
> +	};
> +
> +	dc_in: regulator-dc-in {
> +		compatible = "regulator-fixed";
> +		regulator-name = "5V_IN";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +	};
> +
> +	flash_1v8: regulator-flash-1v8 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "FLASH_1V8";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		regulator-always-on;
> +		vin-supply = <&vcc_3v3>;
> +	};
> +
> +	vcc_card: regulator-vcc-card {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VCC_CARD";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		vin-supply = <&vddao_3v3>;
> +		gpio = <&gpio GPIOX_2 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +		enable-active-high;
> +		gpio-open-drain;
> +	};
> +
> +	vcc_3v3: regulator-vcc-3v3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VCC_3V3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-always-on;
> +		vin-supply = <&vddao_3v3>;
> +
> +		/* FIXME: controlled by TEST_N */
> +	};
> +
> +	vcc_5v: regulator-vcc-5v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VCC_5V";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +		vin-supply = <&dc_in>;
> +		gpio = <&gpio GPIOH_8 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +		enable-active-high;
> +		gpio-open-drain;
> +	};
> +
> +	vddao_3v3: regulator-vddao_3v3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VDDAO_3V3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-always-on;
> +		vin-supply = <&dc_in>;
> +	};
> +
> +	vddcpu_b: regulator-vddcpu-b {
> +		compatible = "pwm-regulator";
> +		regulator-name = "VDDCPU_B";
> +		regulator-min-microvolt = <730000>;
> +		regulator-max-microvolt = <1011000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +		pwm-supply = <&dc_in>;
> +		pwms = <&pwm_AO_cd 1 1250 0>;
> +		pwm-dutycycle-range = <100 0>;
> +	};
> +
> +	vddio_ao18: regulator-vddio_ao18 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "VDDIO_AO18";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		regulator-always-on;
> +		vin-supply = <&vddao_3v3>;
> +	};
> +
> +	vddio_c: regulator-vddio_c {
> +		compatible = "regulator-gpio";
> +		regulator-name = "VDDIO_C";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-settling-time-up-us = <200>;
> +		regulator-settling-time-down-us = <50000>;
> +		vin-supply = <&vddao_3v3>;
> +		gpios = <&gpio GPIOX_4 GPIO_ACTIVE_HIGH>;
> +		states = <3300000 0>,
> +			 <1800000 1>;
> +	};
> +
> +	sound {
> +		compatible = "amlogic,axg-sound-card";
> +		audio-widgets = "Line", "Lineout";
> +		audio-aux-devs = <&tdmout_a>, <&tdmout_b>, <&tdmout_c>,
> +				 <&tdmin_a>, <&tdmin_b>, <&tdmin_c>,
> +				 <&dioo2133>;
> +
> +		assigned-clocks = <&clkc CLKID_MPLL2>,
> +				  <&clkc CLKID_MPLL0>,
> +				  <&clkc CLKID_MPLL1>;
> +		assigned-clock-parents = <0>, <0>, <0>;
> +		assigned-clock-rates = <294912000>,
> +				       <270950400>,
> +				       <393216000>;
> +
> +		dai-link-0 {
> +			sound-dai = <&frddr_a>;
> +		};
> +
> +		dai-link-1 {
> +			sound-dai = <&frddr_b>;
> +		};
> +
> +		dai-link-2 {
> +			sound-dai = <&frddr_c>;
> +		};
> +
> +		dai-link-3 {
> +			sound-dai = <&toddr_a>;
> +		};
> +
> +		dai-link-4 {
> +			sound-dai = <&toddr_b>;
> +		};
> +
> +		dai-link-5 {
> +			sound-dai = <&toddr_c>;
> +		};
> +
> +		/*
> +		 * Audio setup: The 40 pins header provides access to 2 TDMs,
> +		 * SPDIF In/Out and PDM inputs.
> +		 * - TDM A: 2 lanes
> +		 *    D0:    40/X9
> +		 *    D1:    38/X8
> +		 *    BCLK:  12/X11
> +		 *    FS:    35/X10
> +		 * - TDM B: 4 lanes
> +		 *    D0:    37/A3
> +		 *    D1:    16/A4
> +		 *    D2:    18/A5 or 7/AO6
> +		 *    D3:    22/A6 or 21/H5
> +		 *    BCLK:  29/A1 or 8/AO8
> +		 *    FS:    31/A2 or 11/AO7
> +		 * - 2 Master Clocks:
> +		 *    MCLK0: 15/A0 or 10/AO9
> +		 *    MCLK1: 33/X15
> +		 * - SPDIF:
> +		 *    OUT:   32/A11
> +		 *    IN:    21/H5
> +		 * - PDM Input:
> +		 *    DO:    13/A8
> +		 *    D1:    26/A9
> +		 *    D2:    22/A6
> +		 *    D3:    18/A5
> +		 *    DCLK:  36/A7
> +		 *
> +		 * TDM C is not usable on the 40 pins connector so it is
> +		 * setup for the HDMI 4 lanes i2s.
> +		 *
> +		 * No pinctrl is enabled by default to preserve the
> +		 * genericity of the 40 pins header. Many configurations are
> +		 * possible based on the desired use case. Please adjust TDM
> +		 * masks, clock setups and pinctrl accordingly.
> +		 */
> +
> +		dai-link-6 {
> +			sound-dai = <&tdmif_a>;
> +			dai-format = "dsp_a";
> +			dai-tdm-slot-tx-mask-0 = <1 1>;
> +			mclk-fs = <256>;
> +
> +			codec-0 {
> +				sound-dai = <&tohdmitx TOHDMITX_I2S_IN_A>;
> +			};
> +
> +			codec-1 {
> +				sound-dai = <&toacodec TOACODEC_IN_A>;
> +			};
> +		};
> +
> +		dai-link-7 {
> +			sound-dai = <&tdmif_b>;
> +			dai-format = "i2s";
> +			dai-tdm-slot-tx-mask-0 = <1 1>;
> +			dai-tdm-slot-rx-mask-1 = <1 1>;
> +			mclk-fs = <256>;
> +
> +			codec-0 {
> +				sound-dai = <&tohdmitx TOHDMITX_I2S_IN_B>;
> +			};
> +
> +			codec-1 {
> +				sound-dai = <&toacodec TOACODEC_IN_B>;
> +			};
> +		};
> +
> +		dai-link-8 {
> +			sound-dai = <&tdmif_c>;
> +			dai-format = "i2s";
> +			dai-tdm-slot-tx-mask-0 = <1 1>;
> +			dai-tdm-slot-tx-mask-1 = <1 1>;
> +			dai-tdm-slot-tx-mask-2 = <1 1>;
> +			dai-tdm-slot-tx-mask-3 = <1 1>;
> +			mclk-fs = <256>;
> +
> +			codec-0 {
> +				sound-dai = <&tohdmitx TOHDMITX_I2S_IN_C>;
> +			};
> +
> +			codec-1 {
> +				sound-dai = <&toacodec TOACODEC_IN_C>;
> +			};
> +		};
> +
> +		dai-link-9 {
> +			sound-dai = <&tohdmitx TOHDMITX_I2S_OUT>;
> +
> +			codec {
> +				sound-dai = <&hdmi_tx>;
> +			};
> +		};
> +
> +		dai-link-10 {
> +			sound-dai = <&toacodec TOACODEC_OUT>;
> +
> +			codec {
> +				sound-dai = <&acodec>;
> +			};
> +		};
> +	};
> +};
> +
> +&acodec {
> +	status = "okay";
> +	AVDD-supply = <&vddio_ao18>;
> +};
> +
> +&arb {
> +	status = "okay";
> +};
> +
> +&cecb_AO {
> +	status = "okay";
> +	pinctrl-0 = <&cec_ao_b_h_pins>;
> +	pinctrl-names = "default";
> +	hdmi-phandle = <&hdmi_tx>;
> +};
> +
> +&clkc_audio {
> +	status = "okay";
> +};
> +
> +&cvbs_vdac_port {
> +	cvbs_vdac_out: endpoint {
> +		remote-endpoint = <&cvbs_connector_in>;
> +	};
> +};
> +
> +&ethmac {
> +	pinctrl-0 = <&eth_pins>, <&eth_rgmii_pins>, <&eth_phy_irq_pins>;
> +	pinctrl-names = "default";
> +	status = "okay";
> +	phy-mode = "rgmii";
> +	phy-handle = <&external_phy>;
> +	amlogic,tx-delay-ns = <2>;
> +};
> +
> +&ext_mdio {
> +	external_phy: ethernet-phy@0 {
> +		/* Realtek RTL8211F (0x001cc916) */
> +		reg = <0>;
> +		max-speed = <1000>;
> +
> +		reset-assert-us = <100000>;
> +		reset-deassert-us = <100000>;
> +		reset-gpios = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
> +
> +		interrupt-parent = <&gpio_intc>;
> +		/* MAC_INTR on GPIOZ_14 */
> +		interrupts = <26 IRQ_TYPE_LEVEL_LOW>;
> +	};
> +};
> +
> +&frddr_a {
> +	status = "okay";
> +};
> +
> +&frddr_b {
> +	status = "okay";
> +};
> +
> +&frddr_c {
> +	status = "okay";
> +};
> +
> +&hdmi_tx {
> +	status = "okay";
> +	pinctrl-0 = <&hdmitx_hpd_pins>, <&hdmitx_ddc_pins>;
> +	pinctrl-names = "default";
> +	hdmi-supply = <&vcc_5v>;
> +};
> +
> +&hdmi_tx_tmds_port {
> +	hdmi_tx_tmds_out: endpoint {
> +		remote-endpoint = <&hdmi_connector_in>;
> +	};
> +};
> +
> +&ir {
> +	status = "okay";
> +	pinctrl-0 = <&remote_input_ao_pins>;
> +	pinctrl-names = "default";
> +};
> +
> +&npu {
> +	status = "okay";
> +};

Are you sure you want this enabled ?
AFAIK if etnaviv is enabled, it will be used first as a render node by mesa and fail.

> +
> +&periphs_pinctrl {
> +	spi_cs_disable_pins: spi-cs-disable {
> +		mux {
> +			groups = "BOOT_14";
> +			function = "gpio_periphs";
> +			bias-disable;
> +			output-high;
> +		};
> +	};
> +
> +	eth_phy_irq_pins: eth-phy-irq {
> +		mux {
> +			groups = "GPIOZ_14";
> +			function = "gpio_periphs";
> +			bias-pull-up;
> +			output-disable;
> +		};
> +	};
> +};
> +
> +&pwm_AO_cd {
> +	status = "okay";
> +	pinctrl-0 = <&pwm_ao_d_e_pins>;
> +	pinctrl-names = "default";
> +	clocks = <&xtal>;
> +	clock-names = "clkin1";
> +};
> +
> +&pwm_ab {
> +	status = "okay";
> +	pinctrl-0 = <&pwm_b_x7_pins>;
> +	pinctrl-names = "default";
> +	clocks = <&xtal>;
> +	clock-names = "clkin1";
> +};
> +
> +&pwm_cd {
> +	status = "okay";
> +	pinctrl-0 = <&pwm_d_x3_pins>;
> +	pinctrl-names = "default";
> +	clocks = <&xtal>;
> +	clock-names = "clkin1";
> +};
> +
> +&saradc {
> +	status = "okay";
> +	vref-supply = <&vddio_ao18>;
> +};
> +
> +/* SD card */
> +&sd_emmc_b {
> +	status = "okay";
> +	pinctrl-0 = <&sdcard_c_pins>;
> +	pinctrl-1 = <&sdcard_clk_gate_c_pins>;
> +	pinctrl-names = "default", "clk-gate";
> +
> +	bus-width = <4>;
> +	cap-sd-highspeed;
> +	sd-uhs-sdr12;
> +	sd-uhs-sdr25;
> +	sd-uhs-sdr50;
> +	sd-uhs-sdr104;
> +	max-frequency = <200000000>;
> +	disable-wp;
> +
> +	cd-gpios = <&gpio GPIOC_6 GPIO_ACTIVE_LOW>;
> +	vmmc-supply = <&vcc_card>;
> +	vqmmc-supply = <&vddio_c>;
> +};
> +
> +/*
> + * EMMC_D4, EMMC_D5, EMMC_D6 and EMMC_D7 pins are shared between SPI NOR CS
> + * and eMMC Data 4 to 7 pins.
> + * Replace emmc_data_8b_pins to emmc_data_4b_pins from sd_emmc_c pinctrl-0,
> + * and change bus-width to 4 then spifc can be enabled.
> + */
> +&sd_emmc_c {
> +	status = "okay";
> +	pinctrl-0 = <&emmc_ctrl_pins>, <&emmc_data_8b_pins>, <&emmc_ds_pins>,
> +		    <&spi_cs_disable_pins>;
> +	pinctrl-1 = <&emmc_clk_gate_pins>;
> +	pinctrl-names = "default", "clk-gate";
> +
> +	bus-width = <8>;
> +	cap-mmc-highspeed;
> +	mmc-hs200-1_8v;
> +	max-frequency = <200000000>;
> +	disable-wp;
> +
> +	mmc-pwrseq = <&emmc_pwrseq>;
> +	vmmc-supply = <&vcc_3v3>;
> +	vqmmc-supply = <&flash_1v8>;
> +};
> +
> +&spifc {
> +	status = "disabled";
> +	pinctrl-0 = <&nor_pins>;
> +	pinctrl-names = "default";
> +	cs-gpios = <&gpio BOOT_14 GPIO_ACTIVE_LOW>;
> +
> +	w25lq128d: flash@0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		spi-max-frequency = <80000000>;
> +	};
> +};
> +
> +&tdmif_a {
> +	status = "okay";
> +};
> +
> +&tdmif_b {
> +	status = "okay";
> +};
> +
> +&tdmif_c {
> +	status = "okay";
> +};
> +
> +&tdmin_a {
> +	status = "okay";
> +};
> +
> +&tdmin_b {
> +	status = "okay";
> +};
> +
> +&tdmin_c {
> +	status = "okay";
> +};
> +
> +&tdmout_a {
> +	status = "okay";
> +};
> +
> +&tdmout_b {
> +	status = "okay";
> +};
> +
> +&tdmout_c {
> +	status = "okay";
> +};
> +
> +&toacodec {
> +	status = "okay";
> +};
> +
> +&toddr_a {
> +	status = "okay";
> +};
> +
> +&toddr_b {
> +	status = "okay";
> +};
> +
> +&toddr_c {
> +	status = "okay";
> +};
> +
> +&tohdmitx {
> +	status = "okay";
> +};
> +
> +&uart_AO {
> +	status = "okay";
> +	pinctrl-0 = <&uart_ao_a_pins>;
> +	pinctrl-names = "default";
> +};
> +
> +&usb2_phy1 {
> +	phy-supply = <&dc_in>;
> +};
> +
> +&usb3_pcie_phy {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	phy-supply = <&vcc_5v>;
> +
> +	hub: hub@1 {
> +		compatible = "usb5e3,626";
> +		reg = <1>;
> +		reset-gpios = <&gpio GPIOC_7 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
> +	};

Not sure the PHY is the right place to put the USB HUB,
and it's probable the HUB is connected to both the USB2 and USB3 lines
so you should have both USB IDs in DT like it'd done for the Odroid-C4:

/ {
...
          /* USB hub supports both USB 2.0 and USB 3.0 root hub */
          usb-hub {
                  dr_mode = "host";
                  #address-cells = <1>;
                  #size-cells = <0>;

                  /* 2.0 hub on port 1 */
                  hub_2_0: hub@1 {
                          compatible = "usb2109,2817";
                          reg = <1>;
                          peer-hub = <&hub_3_0>;
                          reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
                          vdd-supply = <&vcc_5v>;
                  };

                  /* 3.1 hub on port 4 */
                  hub_3_0: hub@2 {
                          compatible = "usb2109,817";
                          reg = <2>;
                          peer-hub = <&hub_2_0>;
                          reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
                          vdd-supply = <&vcc_5v>;
                  };
          };
...
};

if it only has a single USB ID, then it should go under the dwc3 node.

> +};
> +
> +&usb {
> +	status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
> new file mode 100644
> index 000000000000..077e7506ce4f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2023 BayLibre, SAS.
> + * Author: Jerome Brunet <jbrunet@baylibre.com>
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/clock/g12a-clkc.h>
> +#include "meson-sm1.dtsi"
> +#include "meson-libretech-cottonwood.dtsi"
> +
> +/ {
> +	compatible = "libretech,aml-s905d3-cc", "amlogic,sm1";
> +	model = "Libre Computer AML-S905D3-CC Solitude";
> +
> +	sound {
> +		model = "Solitude";
> +		audio-routing = "TDMOUT_A IN 0", "FRDDR_A OUT 0",
> +				"TDMOUT_A IN 1", "FRDDR_B OUT 0",
> +				"TDMOUT_A IN 2", "FRDDR_C OUT 0",
> +				"TDM_A Playback", "TDMOUT_A OUT",
> +				"TDMOUT_B IN 0", "FRDDR_A OUT 1",
> +				"TDMOUT_B IN 1", "FRDDR_B OUT 1",
> +				"TDMOUT_B IN 2", "FRDDR_C OUT 1",
> +				"TDM_B Playback", "TDMOUT_B OUT",
> +				"TDMOUT_C IN 0", "FRDDR_A OUT 2",
> +				"TDMOUT_C IN 1", "FRDDR_B OUT 2",
> +				"TDMOUT_C IN 2", "FRDDR_C OUT 2",
> +				"TDM_C Playback", "TDMOUT_C OUT",
> +				"TDMIN_A IN 0", "TDM_A Capture",
> +				"TDMIN_B IN 0", "TDM_A Capture",
> +				"TDMIN_C IN 0", "TDM_A Capture",
> +				"TDMIN_A IN 13", "TDM_A Loopback",
> +				"TDMIN_B IN 13", "TDM_A Loopback",
> +				"TDMIN_C IN 13", "TDM_A Loopback",
> +				"TDMIN_A IN 1", "TDM_B Capture",
> +				"TDMIN_B IN 1", "TDM_B Capture",
> +				"TDMIN_C IN 1", "TDM_B Capture",
> +				"TDMIN_A IN 14", "TDM_B Loopback",
> +				"TDMIN_B IN 14", "TDM_B Loopback",
> +				"TDMIN_C IN 14", "TDM_B Loopback",
> +				"TDMIN_A IN 2", "TDM_C Capture",
> +				"TDMIN_B IN 2", "TDM_C Capture",
> +				"TDMIN_C IN 2", "TDM_C Capture",
> +				"TDMIN_A IN 15", "TDM_C Loopback",
> +				"TDMIN_B IN 15", "TDM_C Loopback",
> +				"TDMIN_C IN 15", "TDM_C Loopback",
> +				"TODDR_A IN 0", "TDMIN_A OUT",
> +				"TODDR_B IN 0", "TDMIN_A OUT",
> +				"TODDR_C IN 0", "TDMIN_A OUT",
> +				"TODDR_A IN 1", "TDMIN_B OUT",
> +				"TODDR_B IN 1", "TDMIN_B OUT",
> +				"TODDR_C IN 1", "TDMIN_B OUT",
> +				"TODDR_A IN 2", "TDMIN_C OUT",
> +				"TODDR_B IN 2", "TDMIN_C OUT",
> +				"TODDR_C IN 2", "TDMIN_C OUT",
> +				"Lineout", "ACODEC LOLP",
> +				"Lineout", "ACODEC LORP";
> +	};
> +};
> +
> +&cpu0 {
> +	cpu-supply = <&vddcpu_b>;
> +	operating-points-v2 = <&cpu_opp_table>;
> +	clocks = <&clkc CLKID_CPU_CLK>;
> +	clock-latency = <50000>;
> +};
> +
> +&cpu1 {
> +	cpu-supply = <&vddcpu_b>;
> +	operating-points-v2 = <&cpu_opp_table>;
> +	clocks = <&clkc CLKID_CPU1_CLK>;
> +	clock-latency = <50000>;
> +};
> +
> +&cpu2 {
> +	cpu-supply = <&vddcpu_b>;
> +	operating-points-v2 = <&cpu_opp_table>;
> +	clocks = <&clkc CLKID_CPU2_CLK>;
> +	clock-latency = <50000>;
> +};
> +
> +&cpu3 {
> +	cpu-supply = <&vddcpu_b>;
> +	operating-points-v2 = <&cpu_opp_table>;
> +	clocks = <&clkc CLKID_CPU3_CLK>;
> +	clock-latency = <50000>;
> +};

Apart that, it looks fine,


Thanks,
Neil
  

On Mon 02 Oct 2023 at 18:45, Neil Armstrong <neil.armstrong@linaro.org> wrote:

> Hi,
>
> On 02/10/2023 16:10, Jerome Brunet wrote:
>> Add support for the Libretech cottonwood board family.
>> These 2 boards are based on the same PCB, with an RPi B form factor.
>> The "Alta" board uses an a311d while the "Solitude" variant uses an
>> s905d3.
>> Co-developed-by: Da Xue <da.xue@libretech.co>
>> Signed-off-by: Da Xue <da.xue@libretech.co>
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>>   arch/arm64/boot/dts/amlogic/Makefile          |   2 +
>>   .../amlogic/meson-g12b-a311d-libretech-cc.dts | 133 ++++
>>   .../amlogic/meson-libretech-cottonwood.dtsi   | 610 ++++++++++++++++++
>>   .../amlogic/meson-sm1-s905d3-libretech-cc.dts |  89 +++
>>   4 files changed, 834 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
>>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
>>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
>> diff --git a/arch/arm64/boot/dts/amlogic/Makefile
>> b/arch/arm64/boot/dts/amlogic/Makefile
>> index 4ce401d17b63..cc8b34bd583d 100644
>> --- a/arch/arm64/boot/dts/amlogic/Makefile
>> +++ b/arch/arm64/boot/dts/amlogic/Makefile
>> @@ -18,6 +18,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-g12b-bananapi-cm4-cm4io.dtb
>>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gsking-x.dtb
>>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking-pro.dtb
>>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking.dtb
>> +dtb-$(CONFIG_ARCH_MESON) += meson-g12b-a311d-libretech-cc.dtb
>>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-go-ultra.dtb
>>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2-plus.dtb
>>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2.dtb
>> @@ -73,6 +74,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-sm1-bananapi-m2-pro.dtb
>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-bananapi-m5.dtb
>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-h96-max.dtb
>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-khadas-vim3l.dtb
>> +dtb-$(CONFIG_ARCH_MESON) += meson-sm1-s905d3-libretech-cc.dtb
>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-c4.dtb
>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-hc4.dtb
>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
>> new file mode 100644
>> index 000000000000..fc890e235dbd
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
>> @@ -0,0 +1,133 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright (c) 2023 BayLibre, SAS.
>> + * Author: Jerome Brunet <jbrunet@baylibre.com>
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include <dt-bindings/clock/g12a-clkc.h>
>> +#include "meson-g12b-a311d.dtsi"
>> +#include "meson-libretech-cottonwood.dtsi"
>> +
>> +/ {
>> +	compatible = "libretech,aml-a311d-cc", "amlogic,a311d", "amlogic,g12b";
>> +	model = "Libre Computer AML-A311D-CC Alta";
>> +
>> +	vddcpu_a: regulator-vddcpu-a {
>> +		compatible = "pwm-regulator";
>> +		regulator-name = "VDDCPU_A";
>> +		regulator-min-microvolt = <730000>;
>> +		regulator-max-microvolt = <1011000>;
>> +		regulator-boot-on;
>> +		regulator-always-on;
>> +		pwm-supply = <&dc_in>;
>> +		pwms = <&pwm_ab 0 1250 0>;
>> +		pwm-dutycycle-range = <100 0>;
>> +	};
>> +
>> +	sound {
>> +		model = "Alta";
>
> I think those sound model properties should be coherent with the
> other Libre Computer boards:
> arch/arm64/boot/dts/amlogic/meson-gx-libretech-pc.dtsi:         model = "LIBRETECH-PC";
> arch/arm64/boot/dts/amlogic/meson-gxl-s805x-libretech-ac.dts:           model = "LIBRETECH-AC";
> arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc-v2.dts:                model = "LIBRETECH-CC-V2";
> arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts:           model = "LIBRETECH-CC";

"LIBRETECH-CC-" leave very little room to play with
That's not really something that could have been anticipated 5+ years ago

>
> It's ok to change the scheme since it's tried to keep the name under the 15 characters limit,
> will the next board keep this naming ?

I don't know what the next board will be so I can hardly make any prediction
I'm open to suggestion if you prefer something else

>
>
>> +		audio-routing = "TDMOUT_A IN 0", "FRDDR_A OUT 0",
>> +				"TDMOUT_A IN 1", "FRDDR_B OUT 0",
>> +				"TDMOUT_A IN 2", "FRDDR_C OUT 0",
>> +				"TDM_A Playback", "TDMOUT_A OUT",
>> +				"TDMOUT_B IN 0", "FRDDR_A OUT 1",
>> +				"TDMOUT_B IN 1", "FRDDR_B OUT 1",
>> +				"TDMOUT_B IN 2", "FRDDR_C OUT 1",
>> +				"TDM_B Playback", "TDMOUT_B OUT",
>> +				"TDMOUT_C IN 0", "FRDDR_A OUT 2",
>> +				"TDMOUT_C IN 1", "FRDDR_B OUT 2",
>> +				"TDMOUT_C IN 2", "FRDDR_C OUT 2",
>> +				"TDM_C Playback", "TDMOUT_C OUT",
>> +				"TDMIN_A IN 0", "TDM_A Capture",
>> +				"TDMIN_B IN 0", "TDM_A Capture",
>> +				"TDMIN_C IN 0", "TDM_A Capture",
>> +				"TDMIN_A IN 3", "TDM_A Loopback",
>> +				"TDMIN_B IN 3", "TDM_A Loopback",
>> +				"TDMIN_C IN 3", "TDM_A Loopback",
>> +				"TDMIN_A IN 1", "TDM_B Capture",
>> +				"TDMIN_B IN 1", "TDM_B Capture",
>> +				"TDMIN_C IN 1", "TDM_B Capture",
>> +				"TDMIN_A IN 4", "TDM_B Loopback",
>> +				"TDMIN_B IN 4", "TDM_B Loopback",
>> +				"TDMIN_C IN 4", "TDM_B Loopback",
>> +				"TDMIN_A IN 2", "TDM_C Capture",
>> +				"TDMIN_B IN 2", "TDM_C Capture",
>> +				"TDMIN_C IN 2", "TDM_C Capture",
>> +				"TDMIN_A IN 5", "TDM_C Loopback",
>> +				"TDMIN_B IN 5", "TDM_C Loopback",
>> +				"TDMIN_C IN 5", "TDM_C Loopback",
>> +				"TODDR_A IN 0", "TDMIN_A OUT",
>> +				"TODDR_B IN 0", "TDMIN_A OUT",
>> +				"TODDR_C IN 0", "TDMIN_A OUT",
>> +				"TODDR_A IN 1", "TDMIN_B OUT",
>> +				"TODDR_B IN 1", "TDMIN_B OUT",
>> +				"TODDR_C IN 1", "TDMIN_B OUT",
>> +				"TODDR_A IN 2", "TDMIN_C OUT",
>> +				"TODDR_B IN 2", "TDMIN_C OUT",
>> +				"TODDR_C IN 2", "TDMIN_C OUT",
>> +				"Lineout", "ACODEC LOLP",
>> +				"Lineout", "ACODEC LORP";
>> +	};
>> +};
>> +
>> +&cpu_opp_table_0 {
>> +		opp-1800000000 {
>> +			opp-hz = /bits/ 64 <1800000000>;
>> +			opp-microvolt = <1001000>;
>> +		};
>> +
>> +		opp-2016000000 {
>> +			opp-hz = /bits/ 64 <2016000000>;
>> +			opp-microvolt = <1011000>;
>> +		};
>> +};
>> +
>> +&cpu0 {
>> +	cpu-supply = <&vddcpu_b>;
>> +	operating-points-v2 = <&cpu_opp_table_0>;
>> +	clocks = <&clkc CLKID_CPU_CLK>;
>> +	clock-latency = <50000>;
>> +};
>> +
>> +&cpu1 {
>> +	cpu-supply = <&vddcpu_b>;
>> +	operating-points-v2 = <&cpu_opp_table_0>;
>> +	clocks = <&clkc CLKID_CPU_CLK>;
>> +	clock-latency = <50000>;
>> +};
>> +
>> +&cpu100 {
>> +	cpu-supply = <&vddcpu_a>;
>> +	operating-points-v2 = <&cpub_opp_table_1>;
>> +	clocks = <&clkc CLKID_CPUB_CLK>;
>> +	clock-latency = <50000>;
>> +};
>> +
>> +&cpu101 {
>> +	cpu-supply = <&vddcpu_a>;
>> +	operating-points-v2 = <&cpub_opp_table_1>;
>> +	clocks = <&clkc CLKID_CPUB_CLK>;
>> +	clock-latency = <50000>;
>> +};
>> +
>> +&cpu102 {
>> +	cpu-supply = <&vddcpu_a>;
>> +	operating-points-v2 = <&cpub_opp_table_1>;
>> +	clocks = <&clkc CLKID_CPUB_CLK>;
>> +	clock-latency = <50000>;
>> +};
>> +
>> +&cpu103 {
>> +	cpu-supply = <&vddcpu_a>;
>> +	operating-points-v2 = <&cpub_opp_table_1>;
>> +	clocks = <&clkc CLKID_CPUB_CLK>;
>> +	clock-latency = <50000>;
>> +};
>> +
>> +&pwm_ab {
>> +	pinctrl-0 = <&pwm_a_e_pins>, <&pwm_b_x7_pins>;
>> +	clocks = <&xtal>, <&xtal>;
>> +	clock-names = "clkin0", "clkin1";
>> +};
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi b/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
>> new file mode 100644
>> index 000000000000..a7fc8963ff54
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
>> @@ -0,0 +1,610 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright (c) 2023 BayLibre, SAS.
>> + * Author: Jerome Brunet <jbrunet@baylibre.com>
>> + */
>> +
>> +#include <dt-bindings/clock/g12a-clkc.h>
>> +#include <dt-bindings/input/input.h>
>> +#include <dt-bindings/leds/common.h>
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/gpio/meson-g12a-gpio.h>
>> +#include <dt-bindings/sound/meson-g12a-toacodec.h>
>> +#include <dt-bindings/sound/meson-g12a-tohdmitx.h>
>> +
>> +/ {
>> +	aliases {
>> +		serial0 = &uart_AO;
>> +		ethernet0 = &ethmac;
>> +		spi0 = &spifc;
>> +	};
>> +
>> +	memory@0 {
>> +		device_type = "memory";
>> +		reg = <0x0 0x0 0x0 0x80000000>;
>> +	};
>> +
>> +	chosen {
>> +		stdout-path = "serial0:115200n8";
>> +	};
>> +
>> +	dioo2133: audio-amplifier-0 {
>> +		compatible = "simple-audio-amplifier";
>> +		enable-gpios = <&gpio GPIOX_0 GPIO_ACTIVE_HIGH>;
>> +		VCC-supply = <&vcc_5v>;
>> +		sound-name-prefix = "10U2";
>> +	};
>> +
>> +	/* TOFIX: handle CVBS_DET on SARADC channel 0 */
>> +	cvbs-connector {
>> +		compatible = "composite-video-connector";
>> +
>> +		port {
>> +			cvbs_connector_in: endpoint {
>> +				remote-endpoint = <&cvbs_vdac_out>;
>> +			};
>> +		};
>> +	};
>> +
>> +	emmc_pwrseq: emmc-pwrseq {
>> +		compatible = "mmc-pwrseq-emmc";
>> +		reset-gpios = <&gpio BOOT_12 GPIO_ACTIVE_LOW>;
>> +	};
>> +
>> +	hdmi-connector {
>> +		compatible = "hdmi-connector";
>> +		type = "a";
>> +
>> +		port {
>> +			hdmi_connector_in: endpoint {
>> +				remote-endpoint = <&hdmi_tx_tmds_out>;
>> +			};
>> +		};
>> +	};
>> +
>> +	leds-pwm {
>> +		compatible = "pwm-leds";
>> +
>> +		led-green {
>> +			color = <LED_COLOR_ID_GREEN>;
>> +			function = LED_FUNCTION_STATUS;
>> +			linux,default-trigger = "default-on";
>> +			panic-indicator;
>> +			max-brightness = <255>;
>> +			pwms = <&pwm_cd 1 1250 0>;
>> +			active-low;
>> +		};
>> +
>> +		led-blue {
>> +			color = <LED_COLOR_ID_BLUE>;
>> +			function = LED_FUNCTION_ACTIVITY;
>> +			linux,default-trigger = "activity";
>> +			max-brightness = <255>;
>> +			pwms = <&pwm_ab 1 1250 0>;
>> +			active-low;
>> +		};
>> +	};
>> +
>> +	leds-gpio {
>> +		compatible = "gpio-leds";
>> +
>> +		led-orange {
>> +			color = <LED_COLOR_ID_AMBER>;
>> +			function = LED_FUNCTION_STANDBY;
>> +			gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
>> +		};
>> +	};
>> +
>> +	dc_in: regulator-dc-in {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "5V_IN";
>> +		regulator-min-microvolt = <5000000>;
>> +		regulator-max-microvolt = <5000000>;
>> +		regulator-always-on;
>> +	};
>> +
>> +	flash_1v8: regulator-flash-1v8 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "FLASH_1V8";
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +		regulator-always-on;
>> +		vin-supply = <&vcc_3v3>;
>> +	};
>> +
>> +	vcc_card: regulator-vcc-card {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "VCC_CARD";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		vin-supply = <&vddao_3v3>;
>> +		gpio = <&gpio GPIOX_2 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> +		enable-active-high;
>> +		gpio-open-drain;
>> +	};
>> +
>> +	vcc_3v3: regulator-vcc-3v3 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "VCC_3V3";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		regulator-always-on;
>> +		vin-supply = <&vddao_3v3>;
>> +
>> +		/* FIXME: controlled by TEST_N */
>> +	};
>> +
>> +	vcc_5v: regulator-vcc-5v {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "VCC_5V";
>> +		regulator-min-microvolt = <5000000>;
>> +		regulator-max-microvolt = <5000000>;
>> +		regulator-always-on;
>> +		vin-supply = <&dc_in>;
>> +		gpio = <&gpio GPIOH_8 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> +		enable-active-high;
>> +		gpio-open-drain;
>> +	};
>> +
>> +	vddao_3v3: regulator-vddao_3v3 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "VDDAO_3V3";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		regulator-always-on;
>> +		vin-supply = <&dc_in>;
>> +	};
>> +
>> +	vddcpu_b: regulator-vddcpu-b {
>> +		compatible = "pwm-regulator";
>> +		regulator-name = "VDDCPU_B";
>> +		regulator-min-microvolt = <730000>;
>> +		regulator-max-microvolt = <1011000>;
>> +		regulator-boot-on;
>> +		regulator-always-on;
>> +		pwm-supply = <&dc_in>;
>> +		pwms = <&pwm_AO_cd 1 1250 0>;
>> +		pwm-dutycycle-range = <100 0>;
>> +	};
>> +
>> +	vddio_ao18: regulator-vddio_ao18 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "VDDIO_AO18";
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +		regulator-always-on;
>> +		vin-supply = <&vddao_3v3>;
>> +	};
>> +
>> +	vddio_c: regulator-vddio_c {
>> +		compatible = "regulator-gpio";
>> +		regulator-name = "VDDIO_C";
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		regulator-settling-time-up-us = <200>;
>> +		regulator-settling-time-down-us = <50000>;
>> +		vin-supply = <&vddao_3v3>;
>> +		gpios = <&gpio GPIOX_4 GPIO_ACTIVE_HIGH>;
>> +		states = <3300000 0>,
>> +			 <1800000 1>;
>> +	};
>> +
>> +	sound {
>> +		compatible = "amlogic,axg-sound-card";
>> +		audio-widgets = "Line", "Lineout";
>> +		audio-aux-devs = <&tdmout_a>, <&tdmout_b>, <&tdmout_c>,
>> +				 <&tdmin_a>, <&tdmin_b>, <&tdmin_c>,
>> +				 <&dioo2133>;
>> +
>> +		assigned-clocks = <&clkc CLKID_MPLL2>,
>> +				  <&clkc CLKID_MPLL0>,
>> +				  <&clkc CLKID_MPLL1>;
>> +		assigned-clock-parents = <0>, <0>, <0>;
>> +		assigned-clock-rates = <294912000>,
>> +				       <270950400>,
>> +				       <393216000>;
>> +
>> +		dai-link-0 {
>> +			sound-dai = <&frddr_a>;
>> +		};
>> +
>> +		dai-link-1 {
>> +			sound-dai = <&frddr_b>;
>> +		};
>> +
>> +		dai-link-2 {
>> +			sound-dai = <&frddr_c>;
>> +		};
>> +
>> +		dai-link-3 {
>> +			sound-dai = <&toddr_a>;
>> +		};
>> +
>> +		dai-link-4 {
>> +			sound-dai = <&toddr_b>;
>> +		};
>> +
>> +		dai-link-5 {
>> +			sound-dai = <&toddr_c>;
>> +		};
>> +
>> +		/*
>> +		 * Audio setup: The 40 pins header provides access to 2 TDMs,
>> +		 * SPDIF In/Out and PDM inputs.
>> +		 * - TDM A: 2 lanes
>> +		 *    D0:    40/X9
>> +		 *    D1:    38/X8
>> +		 *    BCLK:  12/X11
>> +		 *    FS:    35/X10
>> +		 * - TDM B: 4 lanes
>> +		 *    D0:    37/A3
>> +		 *    D1:    16/A4
>> +		 *    D2:    18/A5 or 7/AO6
>> +		 *    D3:    22/A6 or 21/H5
>> +		 *    BCLK:  29/A1 or 8/AO8
>> +		 *    FS:    31/A2 or 11/AO7
>> +		 * - 2 Master Clocks:
>> +		 *    MCLK0: 15/A0 or 10/AO9
>> +		 *    MCLK1: 33/X15
>> +		 * - SPDIF:
>> +		 *    OUT:   32/A11
>> +		 *    IN:    21/H5
>> +		 * - PDM Input:
>> +		 *    DO:    13/A8
>> +		 *    D1:    26/A9
>> +		 *    D2:    22/A6
>> +		 *    D3:    18/A5
>> +		 *    DCLK:  36/A7
>> +		 *
>> +		 * TDM C is not usable on the 40 pins connector so it is
>> +		 * setup for the HDMI 4 lanes i2s.
>> +		 *
>> +		 * No pinctrl is enabled by default to preserve the
>> +		 * genericity of the 40 pins header. Many configurations are
>> +		 * possible based on the desired use case. Please adjust TDM
>> +		 * masks, clock setups and pinctrl accordingly.
>> +		 */
>> +
>> +		dai-link-6 {
>> +			sound-dai = <&tdmif_a>;
>> +			dai-format = "dsp_a";
>> +			dai-tdm-slot-tx-mask-0 = <1 1>;
>> +			mclk-fs = <256>;
>> +
>> +			codec-0 {
>> +				sound-dai = <&tohdmitx TOHDMITX_I2S_IN_A>;
>> +			};
>> +
>> +			codec-1 {
>> +				sound-dai = <&toacodec TOACODEC_IN_A>;
>> +			};
>> +		};
>> +
>> +		dai-link-7 {
>> +			sound-dai = <&tdmif_b>;
>> +			dai-format = "i2s";
>> +			dai-tdm-slot-tx-mask-0 = <1 1>;
>> +			dai-tdm-slot-rx-mask-1 = <1 1>;
>> +			mclk-fs = <256>;
>> +
>> +			codec-0 {
>> +				sound-dai = <&tohdmitx TOHDMITX_I2S_IN_B>;
>> +			};
>> +
>> +			codec-1 {
>> +				sound-dai = <&toacodec TOACODEC_IN_B>;
>> +			};
>> +		};
>> +
>> +		dai-link-8 {
>> +			sound-dai = <&tdmif_c>;
>> +			dai-format = "i2s";
>> +			dai-tdm-slot-tx-mask-0 = <1 1>;
>> +			dai-tdm-slot-tx-mask-1 = <1 1>;
>> +			dai-tdm-slot-tx-mask-2 = <1 1>;
>> +			dai-tdm-slot-tx-mask-3 = <1 1>;
>> +			mclk-fs = <256>;
>> +
>> +			codec-0 {
>> +				sound-dai = <&tohdmitx TOHDMITX_I2S_IN_C>;
>> +			};
>> +
>> +			codec-1 {
>> +				sound-dai = <&toacodec TOACODEC_IN_C>;
>> +			};
>> +		};
>> +
>> +		dai-link-9 {
>> +			sound-dai = <&tohdmitx TOHDMITX_I2S_OUT>;
>> +
>> +			codec {
>> +				sound-dai = <&hdmi_tx>;
>> +			};
>> +		};
>> +
>> +		dai-link-10 {
>> +			sound-dai = <&toacodec TOACODEC_OUT>;
>> +
>> +			codec {
>> +				sound-dai = <&acodec>;
>> +			};
>> +		};
>> +	};
>> +};
>> +
>> +&acodec {
>> +	status = "okay";
>> +	AVDD-supply = <&vddio_ao18>;
>> +};
>> +
>> +&arb {
>> +	status = "okay";
>> +};
>> +
>> +&cecb_AO {
>> +	status = "okay";
>> +	pinctrl-0 = <&cec_ao_b_h_pins>;
>> +	pinctrl-names = "default";
>> +	hdmi-phandle = <&hdmi_tx>;
>> +};
>> +
>> +&clkc_audio {
>> +	status = "okay";
>> +};
>> +
>> +&cvbs_vdac_port {
>> +	cvbs_vdac_out: endpoint {
>> +		remote-endpoint = <&cvbs_connector_in>;
>> +	};
>> +};
>> +
>> +&ethmac {
>> +	pinctrl-0 = <&eth_pins>, <&eth_rgmii_pins>, <&eth_phy_irq_pins>;
>> +	pinctrl-names = "default";
>> +	status = "okay";
>> +	phy-mode = "rgmii";
>> +	phy-handle = <&external_phy>;
>> +	amlogic,tx-delay-ns = <2>;
>> +};
>> +
>> +&ext_mdio {
>> +	external_phy: ethernet-phy@0 {
>> +		/* Realtek RTL8211F (0x001cc916) */
>> +		reg = <0>;
>> +		max-speed = <1000>;
>> +
>> +		reset-assert-us = <100000>;
>> +		reset-deassert-us = <100000>;
>> +		reset-gpios = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
>> +
>> +		interrupt-parent = <&gpio_intc>;
>> +		/* MAC_INTR on GPIOZ_14 */
>> +		interrupts = <26 IRQ_TYPE_LEVEL_LOW>;
>> +	};
>> +};
>> +
>> +&frddr_a {
>> +	status = "okay";
>> +};
>> +
>> +&frddr_b {
>> +	status = "okay";
>> +};
>> +
>> +&frddr_c {
>> +	status = "okay";
>> +};
>> +
>> +&hdmi_tx {
>> +	status = "okay";
>> +	pinctrl-0 = <&hdmitx_hpd_pins>, <&hdmitx_ddc_pins>;
>> +	pinctrl-names = "default";
>> +	hdmi-supply = <&vcc_5v>;
>> +};
>> +
>> +&hdmi_tx_tmds_port {
>> +	hdmi_tx_tmds_out: endpoint {
>> +		remote-endpoint = <&hdmi_connector_in>;
>> +	};
>> +};
>> +
>> +&ir {
>> +	status = "okay";
>> +	pinctrl-0 = <&remote_input_ao_pins>;
>> +	pinctrl-names = "default";
>> +};
>> +
>> +&npu {
>> +	status = "okay";
>> +};
>
> Are you sure you want this enabled ?
> AFAIK if etnaviv is enabled, it will be used first as a render node by mesa and fail.
>

AFAIK, it is enabled in the pre-flashed bootloader that is running
debian, fedora and opensuse ... but etnaviv is blacklisted there

It should indeed be removed.


>> +
>> +&periphs_pinctrl {
>> +	spi_cs_disable_pins: spi-cs-disable {
>> +		mux {
>> +			groups = "BOOT_14";
>> +			function = "gpio_periphs";
>> +			bias-disable;
>> +			output-high;
>> +		};
>> +	};
>> +
>> +	eth_phy_irq_pins: eth-phy-irq {
>> +		mux {
>> +			groups = "GPIOZ_14";
>> +			function = "gpio_periphs";
>> +			bias-pull-up;
>> +			output-disable;
>> +		};
>> +	};
>> +};
>> +
>> +&pwm_AO_cd {
>> +	status = "okay";
>> +	pinctrl-0 = <&pwm_ao_d_e_pins>;
>> +	pinctrl-names = "default";
>> +	clocks = <&xtal>;
>> +	clock-names = "clkin1";
>> +};
>> +
>> +&pwm_ab {
>> +	status = "okay";
>> +	pinctrl-0 = <&pwm_b_x7_pins>;
>> +	pinctrl-names = "default";
>> +	clocks = <&xtal>;
>> +	clock-names = "clkin1";
>> +};
>> +
>> +&pwm_cd {
>> +	status = "okay";
>> +	pinctrl-0 = <&pwm_d_x3_pins>;
>> +	pinctrl-names = "default";
>> +	clocks = <&xtal>;
>> +	clock-names = "clkin1";
>> +};
>> +
>> +&saradc {
>> +	status = "okay";
>> +	vref-supply = <&vddio_ao18>;
>> +};
>> +
>> +/* SD card */
>> +&sd_emmc_b {
>> +	status = "okay";
>> +	pinctrl-0 = <&sdcard_c_pins>;
>> +	pinctrl-1 = <&sdcard_clk_gate_c_pins>;
>> +	pinctrl-names = "default", "clk-gate";
>> +
>> +	bus-width = <4>;
>> +	cap-sd-highspeed;
>> +	sd-uhs-sdr12;
>> +	sd-uhs-sdr25;
>> +	sd-uhs-sdr50;
>> +	sd-uhs-sdr104;
>> +	max-frequency = <200000000>;
>> +	disable-wp;
>> +
>> +	cd-gpios = <&gpio GPIOC_6 GPIO_ACTIVE_LOW>;
>> +	vmmc-supply = <&vcc_card>;
>> +	vqmmc-supply = <&vddio_c>;
>> +};
>> +
>> +/*
>> + * EMMC_D4, EMMC_D5, EMMC_D6 and EMMC_D7 pins are shared between SPI NOR CS
>> + * and eMMC Data 4 to 7 pins.
>> + * Replace emmc_data_8b_pins to emmc_data_4b_pins from sd_emmc_c pinctrl-0,
>> + * and change bus-width to 4 then spifc can be enabled.
>> + */
>> +&sd_emmc_c {
>> +	status = "okay";
>> +	pinctrl-0 = <&emmc_ctrl_pins>, <&emmc_data_8b_pins>, <&emmc_ds_pins>,
>> +		    <&spi_cs_disable_pins>;
>> +	pinctrl-1 = <&emmc_clk_gate_pins>;
>> +	pinctrl-names = "default", "clk-gate";
>> +
>> +	bus-width = <8>;
>> +	cap-mmc-highspeed;
>> +	mmc-hs200-1_8v;
>> +	max-frequency = <200000000>;
>> +	disable-wp;
>> +
>> +	mmc-pwrseq = <&emmc_pwrseq>;
>> +	vmmc-supply = <&vcc_3v3>;
>> +	vqmmc-supply = <&flash_1v8>;
>> +};
>> +
>> +&spifc {
>> +	status = "disabled";
>> +	pinctrl-0 = <&nor_pins>;
>> +	pinctrl-names = "default";
>> +	cs-gpios = <&gpio BOOT_14 GPIO_ACTIVE_LOW>;
>> +
>> +	w25lq128d: flash@0 {
>> +		compatible = "jedec,spi-nor";
>> +		reg = <0>;
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		spi-max-frequency = <80000000>;
>> +	};
>> +};
>> +
>> +&tdmif_a {
>> +	status = "okay";
>> +};
>> +
>> +&tdmif_b {
>> +	status = "okay";
>> +};
>> +
>> +&tdmif_c {
>> +	status = "okay";
>> +};
>> +
>> +&tdmin_a {
>> +	status = "okay";
>> +};
>> +
>> +&tdmin_b {
>> +	status = "okay";
>> +};
>> +
>> +&tdmin_c {
>> +	status = "okay";
>> +};
>> +
>> +&tdmout_a {
>> +	status = "okay";
>> +};
>> +
>> +&tdmout_b {
>> +	status = "okay";
>> +};
>> +
>> +&tdmout_c {
>> +	status = "okay";
>> +};
>> +
>> +&toacodec {
>> +	status = "okay";
>> +};
>> +
>> +&toddr_a {
>> +	status = "okay";
>> +};
>> +
>> +&toddr_b {
>> +	status = "okay";
>> +};
>> +
>> +&toddr_c {
>> +	status = "okay";
>> +};
>> +
>> +&tohdmitx {
>> +	status = "okay";
>> +};
>> +
>> +&uart_AO {
>> +	status = "okay";
>> +	pinctrl-0 = <&uart_ao_a_pins>;
>> +	pinctrl-names = "default";
>> +};
>> +
>> +&usb2_phy1 {
>> +	phy-supply = <&dc_in>;
>> +};
>> +
>> +&usb3_pcie_phy {
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	phy-supply = <&vcc_5v>;
>> +
>> +	hub: hub@1 {
>> +		compatible = "usb5e3,626";
>> +		reg = <1>;
>> +		reset-gpios = <&gpio GPIOC_7 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
>> +	};
>
> Not sure the PHY is the right place to put the USB HUB,
> and it's probable the HUB is connected to both the USB2 and USB3 lines

It is connected to the USB3.0 only

> so you should have both USB IDs in DT like it'd done for the Odroid-C4:
>
> / {
> ...
>          /* USB hub supports both USB 2.0 and USB 3.0 root hub */
>          usb-hub {
>                  dr_mode = "host";
>                  #address-cells = <1>;
>                  #size-cells = <0>;
>
>                  /* 2.0 hub on port 1 */
>                  hub_2_0: hub@1 {
>                          compatible = "usb2109,2817";
>                          reg = <1>;
>                          peer-hub = <&hub_3_0>;
>                          reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>                          vdd-supply = <&vcc_5v>;
>                  };
>
>                  /* 3.1 hub on port 4 */
>                  hub_3_0: hub@2 {
>                          compatible = "usb2109,817";
>                          reg = <2>;
>                          peer-hub = <&hub_2_0>;
>                          reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>                          vdd-supply = <&vcc_5v>;
>                  };
>          };
> ...
> };
>
> if it only has a single USB ID, then it should go under the dwc3 node.

The usb controller is connected to the PHY and what's coming out of the PHY
goes to the hub. It seems logical to hub the hub under it.

Why bypass the PHY ?

>
>> +};
>> +
>> +&usb {
>> +	status = "okay";
>> +};
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
>> new file mode 100644
>> index 000000000000..077e7506ce4f
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
>> @@ -0,0 +1,89 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright (c) 2023 BayLibre, SAS.
>> + * Author: Jerome Brunet <jbrunet@baylibre.com>
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include <dt-bindings/clock/g12a-clkc.h>
>> +#include "meson-sm1.dtsi"
>> +#include "meson-libretech-cottonwood.dtsi"
>> +
>> +/ {
>> +	compatible = "libretech,aml-s905d3-cc", "amlogic,sm1";
>> +	model = "Libre Computer AML-S905D3-CC Solitude";
>> +
>> +	sound {
>> +		model = "Solitude";
>> +		audio-routing = "TDMOUT_A IN 0", "FRDDR_A OUT 0",
>> +				"TDMOUT_A IN 1", "FRDDR_B OUT 0",
>> +				"TDMOUT_A IN 2", "FRDDR_C OUT 0",
>> +				"TDM_A Playback", "TDMOUT_A OUT",
>> +				"TDMOUT_B IN 0", "FRDDR_A OUT 1",
>> +				"TDMOUT_B IN 1", "FRDDR_B OUT 1",
>> +				"TDMOUT_B IN 2", "FRDDR_C OUT 1",
>> +				"TDM_B Playback", "TDMOUT_B OUT",
>> +				"TDMOUT_C IN 0", "FRDDR_A OUT 2",
>> +				"TDMOUT_C IN 1", "FRDDR_B OUT 2",
>> +				"TDMOUT_C IN 2", "FRDDR_C OUT 2",
>> +				"TDM_C Playback", "TDMOUT_C OUT",
>> +				"TDMIN_A IN 0", "TDM_A Capture",
>> +				"TDMIN_B IN 0", "TDM_A Capture",
>> +				"TDMIN_C IN 0", "TDM_A Capture",
>> +				"TDMIN_A IN 13", "TDM_A Loopback",
>> +				"TDMIN_B IN 13", "TDM_A Loopback",
>> +				"TDMIN_C IN 13", "TDM_A Loopback",
>> +				"TDMIN_A IN 1", "TDM_B Capture",
>> +				"TDMIN_B IN 1", "TDM_B Capture",
>> +				"TDMIN_C IN 1", "TDM_B Capture",
>> +				"TDMIN_A IN 14", "TDM_B Loopback",
>> +				"TDMIN_B IN 14", "TDM_B Loopback",
>> +				"TDMIN_C IN 14", "TDM_B Loopback",
>> +				"TDMIN_A IN 2", "TDM_C Capture",
>> +				"TDMIN_B IN 2", "TDM_C Capture",
>> +				"TDMIN_C IN 2", "TDM_C Capture",
>> +				"TDMIN_A IN 15", "TDM_C Loopback",
>> +				"TDMIN_B IN 15", "TDM_C Loopback",
>> +				"TDMIN_C IN 15", "TDM_C Loopback",
>> +				"TODDR_A IN 0", "TDMIN_A OUT",
>> +				"TODDR_B IN 0", "TDMIN_A OUT",
>> +				"TODDR_C IN 0", "TDMIN_A OUT",
>> +				"TODDR_A IN 1", "TDMIN_B OUT",
>> +				"TODDR_B IN 1", "TDMIN_B OUT",
>> +				"TODDR_C IN 1", "TDMIN_B OUT",
>> +				"TODDR_A IN 2", "TDMIN_C OUT",
>> +				"TODDR_B IN 2", "TDMIN_C OUT",
>> +				"TODDR_C IN 2", "TDMIN_C OUT",
>> +				"Lineout", "ACODEC LOLP",
>> +				"Lineout", "ACODEC LORP";
>> +	};
>> +};
>> +
>> +&cpu0 {
>> +	cpu-supply = <&vddcpu_b>;
>> +	operating-points-v2 = <&cpu_opp_table>;
>> +	clocks = <&clkc CLKID_CPU_CLK>;
>> +	clock-latency = <50000>;
>> +};
>> +
>> +&cpu1 {
>> +	cpu-supply = <&vddcpu_b>;
>> +	operating-points-v2 = <&cpu_opp_table>;
>> +	clocks = <&clkc CLKID_CPU1_CLK>;
>> +	clock-latency = <50000>;
>> +};
>> +
>> +&cpu2 {
>> +	cpu-supply = <&vddcpu_b>;
>> +	operating-points-v2 = <&cpu_opp_table>;
>> +	clocks = <&clkc CLKID_CPU2_CLK>;
>> +	clock-latency = <50000>;
>> +};
>> +
>> +&cpu3 {
>> +	cpu-supply = <&vddcpu_b>;
>> +	operating-points-v2 = <&cpu_opp_table>;
>> +	clocks = <&clkc CLKID_CPU3_CLK>;
>> +	clock-latency = <50000>;
>> +};
>
> Apart that, it looks fine,
>
>
> Thanks,
> Neil
  

On Tue, Oct 3, 2023 at 3:13 AM Jerome Brunet <jbrunet@baylibre.com> wrote:
>
>
> On Mon 02 Oct 2023 at 18:45, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
> > Hi,
> >
> > On 02/10/2023 16:10, Jerome Brunet wrote:
> >> Add support for the Libretech cottonwood board family.
> >> These 2 boards are based on the same PCB, with an RPi B form factor.
> >> The "Alta" board uses an a311d while the "Solitude" variant uses an
> >> s905d3.
> >> Co-developed-by: Da Xue <da.xue@libretech.co>
> >> Signed-off-by: Da Xue <da.xue@libretech.co>
> >> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> >> ---
> >>   arch/arm64/boot/dts/amlogic/Makefile          |   2 +
> >>   .../amlogic/meson-g12b-a311d-libretech-cc.dts | 133 ++++
> >>   .../amlogic/meson-libretech-cottonwood.dtsi   | 610 ++++++++++++++++++
> >>   .../amlogic/meson-sm1-s905d3-libretech-cc.dts |  89 +++
> >>   4 files changed, 834 insertions(+)
> >>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
> >>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
> >>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
> >> diff --git a/arch/arm64/boot/dts/amlogic/Makefile
> >> b/arch/arm64/boot/dts/amlogic/Makefile
> >> index 4ce401d17b63..cc8b34bd583d 100644
> >> --- a/arch/arm64/boot/dts/amlogic/Makefile
> >> +++ b/arch/arm64/boot/dts/amlogic/Makefile
> >> @@ -18,6 +18,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-g12b-bananapi-cm4-cm4io.dtb
> >>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gsking-x.dtb
> >>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking-pro.dtb
> >>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking.dtb
> >> +dtb-$(CONFIG_ARCH_MESON) += meson-g12b-a311d-libretech-cc.dtb
> >>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-go-ultra.dtb
> >>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2-plus.dtb
> >>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2.dtb
> >> @@ -73,6 +74,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-sm1-bananapi-m2-pro.dtb
> >>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-bananapi-m5.dtb
> >>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-h96-max.dtb
> >>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-khadas-vim3l.dtb
> >> +dtb-$(CONFIG_ARCH_MESON) += meson-sm1-s905d3-libretech-cc.dtb
> >>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-c4.dtb
> >>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-hc4.dtb
> >>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb
> >> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
> >> new file mode 100644
> >> index 000000000000..fc890e235dbd
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
> >> @@ -0,0 +1,133 @@
> >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >> +/*
> >> + * Copyright (c) 2023 BayLibre, SAS.
> >> + * Author: Jerome Brunet <jbrunet@baylibre.com>
> >> + */
> >> +
> >> +/dts-v1/;
> >> +
> >> +#include <dt-bindings/clock/g12a-clkc.h>
> >> +#include "meson-g12b-a311d.dtsi"
> >> +#include "meson-libretech-cottonwood.dtsi"
> >> +
> >> +/ {
> >> +    compatible = "libretech,aml-a311d-cc", "amlogic,a311d", "amlogic,g12b";
> >> +    model = "Libre Computer AML-A311D-CC Alta";
> >> +
> >> +    vddcpu_a: regulator-vddcpu-a {
> >> +            compatible = "pwm-regulator";
> >> +            regulator-name = "VDDCPU_A";
> >> +            regulator-min-microvolt = <730000>;
> >> +            regulator-max-microvolt = <1011000>;
> >> +            regulator-boot-on;
> >> +            regulator-always-on;
> >> +            pwm-supply = <&dc_in>;
> >> +            pwms = <&pwm_ab 0 1250 0>;
> >> +            pwm-dutycycle-range = <100 0>;
> >> +    };
> >> +
> >> +    sound {
> >> +            model = "Alta";
> >
> > I think those sound model properties should be coherent with the
> > other Libre Computer boards:
> > arch/arm64/boot/dts/amlogic/meson-gx-libretech-pc.dtsi:         model = "LIBRETECH-PC";
> > arch/arm64/boot/dts/amlogic/meson-gxl-s805x-libretech-ac.dts:           model = "LIBRETECH-AC";
> > arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc-v2.dts:                model = "LIBRETECH-CC-V2";
> > arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts:           model = "LIBRETECH-CC";
>
> "LIBRETECH-CC-" leave very little room to play with
> That's not really something that could have been anticipated 5+ years ago
>

I think the formal model name is best, maybe with LC prefix.
eg. LC-AML-A311D-CC and LC-AML-S905D3-CC

https://hub.libre.computer/t/libre-computer-board-naming-and-conventions/100

> >
> > It's ok to change the scheme since it's tried to keep the name under the 15 characters limit,
> > will the next board keep this naming ?
>
> I don't know what the next board will be so I can hardly make any prediction
> I'm open to suggestion if you prefer something else
>
> >
> >
> >> +            audio-routing = "TDMOUT_A IN 0", "FRDDR_A OUT 0",
> >> +                            "TDMOUT_A IN 1", "FRDDR_B OUT 0",
> >> +                            "TDMOUT_A IN 2", "FRDDR_C OUT 0",
> >> +                            "TDM_A Playback", "TDMOUT_A OUT",
> >> +                            "TDMOUT_B IN 0", "FRDDR_A OUT 1",
> >> +                            "TDMOUT_B IN 1", "FRDDR_B OUT 1",
> >> +                            "TDMOUT_B IN 2", "FRDDR_C OUT 1",
> >> +                            "TDM_B Playback", "TDMOUT_B OUT",
> >> +                            "TDMOUT_C IN 0", "FRDDR_A OUT 2",
> >> +                            "TDMOUT_C IN 1", "FRDDR_B OUT 2",
> >> +                            "TDMOUT_C IN 2", "FRDDR_C OUT 2",
> >> +                            "TDM_C Playback", "TDMOUT_C OUT",
> >> +                            "TDMIN_A IN 0", "TDM_A Capture",
> >> +                            "TDMIN_B IN 0", "TDM_A Capture",
> >> +                            "TDMIN_C IN 0", "TDM_A Capture",
> >> +                            "TDMIN_A IN 3", "TDM_A Loopback",
> >> +                            "TDMIN_B IN 3", "TDM_A Loopback",
> >> +                            "TDMIN_C IN 3", "TDM_A Loopback",
> >> +                            "TDMIN_A IN 1", "TDM_B Capture",
> >> +                            "TDMIN_B IN 1", "TDM_B Capture",
> >> +                            "TDMIN_C IN 1", "TDM_B Capture",
> >> +                            "TDMIN_A IN 4", "TDM_B Loopback",
> >> +                            "TDMIN_B IN 4", "TDM_B Loopback",
> >> +                            "TDMIN_C IN 4", "TDM_B Loopback",
> >> +                            "TDMIN_A IN 2", "TDM_C Capture",
> >> +                            "TDMIN_B IN 2", "TDM_C Capture",
> >> +                            "TDMIN_C IN 2", "TDM_C Capture",
> >> +                            "TDMIN_A IN 5", "TDM_C Loopback",
> >> +                            "TDMIN_B IN 5", "TDM_C Loopback",
> >> +                            "TDMIN_C IN 5", "TDM_C Loopback",
> >> +                            "TODDR_A IN 0", "TDMIN_A OUT",
> >> +                            "TODDR_B IN 0", "TDMIN_A OUT",
> >> +                            "TODDR_C IN 0", "TDMIN_A OUT",
> >> +                            "TODDR_A IN 1", "TDMIN_B OUT",
> >> +                            "TODDR_B IN 1", "TDMIN_B OUT",
> >> +                            "TODDR_C IN 1", "TDMIN_B OUT",
> >> +                            "TODDR_A IN 2", "TDMIN_C OUT",
> >> +                            "TODDR_B IN 2", "TDMIN_C OUT",
> >> +                            "TODDR_C IN 2", "TDMIN_C OUT",
> >> +                            "Lineout", "ACODEC LOLP",
> >> +                            "Lineout", "ACODEC LORP";
> >> +    };
> >> +};
> >> +
> >> +&cpu_opp_table_0 {
> >> +            opp-1800000000 {
> >> +                    opp-hz = /bits/ 64 <1800000000>;
> >> +                    opp-microvolt = <1001000>;
> >> +            };

This seems to match
https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/amlogic/meson-g12b-a311d.dtsi#L44

> >> +
> >> +            opp-2016000000 {
> >> +                    opp-hz = /bits/ 64 <2016000000>;
> >> +                    opp-microvolt = <1011000>;
> >> +            };

We run manufacturing verification at this speed but I don't think we
should include this in upstream.

> >> +};
> >> +
> >> +&cpu0 {
> >> +    cpu-supply = <&vddcpu_b>;
> >> +    operating-points-v2 = <&cpu_opp_table_0>;
> >> +    clocks = <&clkc CLKID_CPU_CLK>;
> >> +    clock-latency = <50000>;
> >> +};
> >> +
> >> +&cpu1 {
> >> +    cpu-supply = <&vddcpu_b>;
> >> +    operating-points-v2 = <&cpu_opp_table_0>;
> >> +    clocks = <&clkc CLKID_CPU_CLK>;
> >> +    clock-latency = <50000>;
> >> +};
> >> +
> >> +&cpu100 {
> >> +    cpu-supply = <&vddcpu_a>;
> >> +    operating-points-v2 = <&cpub_opp_table_1>;
> >> +    clocks = <&clkc CLKID_CPUB_CLK>;
> >> +    clock-latency = <50000>;
> >> +};
> >> +
> >> +&cpu101 {
> >> +    cpu-supply = <&vddcpu_a>;
> >> +    operating-points-v2 = <&cpub_opp_table_1>;
> >> +    clocks = <&clkc CLKID_CPUB_CLK>;
> >> +    clock-latency = <50000>;
> >> +};
> >> +
> >> +&cpu102 {
> >> +    cpu-supply = <&vddcpu_a>;
> >> +    operating-points-v2 = <&cpub_opp_table_1>;
> >> +    clocks = <&clkc CLKID_CPUB_CLK>;
> >> +    clock-latency = <50000>;
> >> +};
> >> +
> >> +&cpu103 {
> >> +    cpu-supply = <&vddcpu_a>;
> >> +    operating-points-v2 = <&cpub_opp_table_1>;
> >> +    clocks = <&clkc CLKID_CPUB_CLK>;
> >> +    clock-latency = <50000>;
> >> +};
> >> +
> >> +&pwm_ab {
> >> +    pinctrl-0 = <&pwm_a_e_pins>, <&pwm_b_x7_pins>;
> >> +    clocks = <&xtal>, <&xtal>;
> >> +    clock-names = "clkin0", "clkin1";
> >> +};
> >> diff --git a/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi b/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
> >> new file mode 100644
> >> index 000000000000..a7fc8963ff54
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
> >> @@ -0,0 +1,610 @@
> >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >> +/*
> >> + * Copyright (c) 2023 BayLibre, SAS.
> >> + * Author: Jerome Brunet <jbrunet@baylibre.com>
> >> + */
> >> +
> >> +#include <dt-bindings/clock/g12a-clkc.h>
> >> +#include <dt-bindings/input/input.h>
> >> +#include <dt-bindings/leds/common.h>
> >> +#include <dt-bindings/gpio/gpio.h>
> >> +#include <dt-bindings/gpio/meson-g12a-gpio.h>
> >> +#include <dt-bindings/sound/meson-g12a-toacodec.h>
> >> +#include <dt-bindings/sound/meson-g12a-tohdmitx.h>
> >> +
> >> +/ {
> >> +    aliases {
> >> +            serial0 = &uart_AO;
> >> +            ethernet0 = &ethmac;
> >> +            spi0 = &spifc;
> >> +    };
> >> +
> >> +    memory@0 {
> >> +            device_type = "memory";
> >> +            reg = <0x0 0x0 0x0 0x80000000>;
> >> +    };
> >> +
> >> +    chosen {
> >> +            stdout-path = "serial0:115200n8";
> >> +    };
> >> +
> >> +    dioo2133: audio-amplifier-0 {
> >> +            compatible = "simple-audio-amplifier";
> >> +            enable-gpios = <&gpio GPIOX_0 GPIO_ACTIVE_HIGH>;
> >> +            VCC-supply = <&vcc_5v>;
> >> +            sound-name-prefix = "10U2";
> >> +    };
> >> +
> >> +    /* TOFIX: handle CVBS_DET on SARADC channel 0 */
> >> +    cvbs-connector {
> >> +            compatible = "composite-video-connector";
> >> +
> >> +            port {
> >> +                    cvbs_connector_in: endpoint {
> >> +                            remote-endpoint = <&cvbs_vdac_out>;
> >> +                    };
> >> +            };
> >> +    };
> >> +
> >> +    emmc_pwrseq: emmc-pwrseq {
> >> +            compatible = "mmc-pwrseq-emmc";
> >> +            reset-gpios = <&gpio BOOT_12 GPIO_ACTIVE_LOW>;
> >> +    };
> >> +
> >> +    hdmi-connector {
> >> +            compatible = "hdmi-connector";
> >> +            type = "a";
> >> +
> >> +            port {
> >> +                    hdmi_connector_in: endpoint {
> >> +                            remote-endpoint = <&hdmi_tx_tmds_out>;
> >> +                    };
> >> +            };
> >> +    };
> >> +
> >> +    leds-pwm {
> >> +            compatible = "pwm-leds";
> >> +
> >> +            led-green {
> >> +                    color = <LED_COLOR_ID_GREEN>;
> >> +                    function = LED_FUNCTION_STATUS;
> >> +                    linux,default-trigger = "default-on";
> >> +                    panic-indicator;
> >> +                    max-brightness = <255>;
> >> +                    pwms = <&pwm_cd 1 1250 0>;
> >> +                    active-low;
> >> +            };
> >> +
> >> +            led-blue {
> >> +                    color = <LED_COLOR_ID_BLUE>;
> >> +                    function = LED_FUNCTION_ACTIVITY;
> >> +                    linux,default-trigger = "activity";
> >> +                    max-brightness = <255>;
> >> +                    pwms = <&pwm_ab 1 1250 0>;
> >> +                    active-low;
> >> +            };
> >> +    };
> >> +
> >> +    leds-gpio {
> >> +            compatible = "gpio-leds";
> >> +
> >> +            led-orange {
> >> +                    color = <LED_COLOR_ID_AMBER>;

Should this be LED_COLOR_ID_ORANGE?

> >> +                    function = LED_FUNCTION_STANDBY;
> >> +                    gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
> >> +            };
> >> +    };
> >> +
> >> +    dc_in: regulator-dc-in {
> >> +            compatible = "regulator-fixed";
> >> +            regulator-name = "5V_IN";
> >> +            regulator-min-microvolt = <5000000>;
> >> +            regulator-max-microvolt = <5000000>;
> >> +            regulator-always-on;
> >> +    };
> >> +
> >> +    flash_1v8: regulator-flash-1v8 {
> >> +            compatible = "regulator-fixed";
> >> +            regulator-name = "FLASH_1V8";
> >> +            regulator-min-microvolt = <1800000>;
> >> +            regulator-max-microvolt = <1800000>;
> >> +            regulator-always-on;
> >> +            vin-supply = <&vcc_3v3>;
> >> +    };
> >> +
> >> +    vcc_card: regulator-vcc-card {
> >> +            compatible = "regulator-fixed";
> >> +            regulator-name = "VCC_CARD";
> >> +            regulator-min-microvolt = <3300000>;
> >> +            regulator-max-microvolt = <3300000>;
> >> +            vin-supply = <&vddao_3v3>;
> >> +            gpio = <&gpio GPIOX_2 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> >> +            enable-active-high;
> >> +            gpio-open-drain;
> >> +    };
> >> +
> >> +    vcc_3v3: regulator-vcc-3v3 {
> >> +            compatible = "regulator-fixed";
> >> +            regulator-name = "VCC_3V3";
> >> +            regulator-min-microvolt = <3300000>;
> >> +            regulator-max-microvolt = <3300000>;
> >> +            regulator-always-on;
> >> +            vin-supply = <&vddao_3v3>;
> >> +
> >> +            /* FIXME: controlled by TEST_N */
> >> +    };
> >> +
> >> +    vcc_5v: regulator-vcc-5v {
> >> +            compatible = "regulator-fixed";
> >> +            regulator-name = "VCC_5V";
> >> +            regulator-min-microvolt = <5000000>;
> >> +            regulator-max-microvolt = <5000000>;
> >> +            regulator-always-on;
> >> +            vin-supply = <&dc_in>;
> >> +            gpio = <&gpio GPIOH_8 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> >> +            enable-active-high;
> >> +            gpio-open-drain;
> >> +    };
> >> +
> >> +    vddao_3v3: regulator-vddao_3v3 {
> >> +            compatible = "regulator-fixed";
> >> +            regulator-name = "VDDAO_3V3";
> >> +            regulator-min-microvolt = <3300000>;
> >> +            regulator-max-microvolt = <3300000>;
> >> +            regulator-always-on;
> >> +            vin-supply = <&dc_in>;
> >> +    };
> >> +
> >> +    vddcpu_b: regulator-vddcpu-b {
> >> +            compatible = "pwm-regulator";
> >> +            regulator-name = "VDDCPU_B";
> >> +            regulator-min-microvolt = <730000>;
> >> +            regulator-max-microvolt = <1011000>;
> >> +            regulator-boot-on;
> >> +            regulator-always-on;
> >> +            pwm-supply = <&dc_in>;
> >> +            pwms = <&pwm_AO_cd 1 1250 0>;
> >> +            pwm-dutycycle-range = <100 0>;
> >> +    };
> >> +
> >> +    vddio_ao18: regulator-vddio_ao18 {
> >> +            compatible = "regulator-fixed";
> >> +            regulator-name = "VDDIO_AO18";
> >> +            regulator-min-microvolt = <1800000>;
> >> +            regulator-max-microvolt = <1800000>;
> >> +            regulator-always-on;
> >> +            vin-supply = <&vddao_3v3>;
> >> +    };
> >> +
> >> +    vddio_c: regulator-vddio_c {
> >> +            compatible = "regulator-gpio";
> >> +            regulator-name = "VDDIO_C";
> >> +            regulator-min-microvolt = <1800000>;
> >> +            regulator-max-microvolt = <3300000>;
> >> +            regulator-settling-time-up-us = <200>;
> >> +            regulator-settling-time-down-us = <50000>;
> >> +            vin-supply = <&vddao_3v3>;
> >> +            gpios = <&gpio GPIOX_4 GPIO_ACTIVE_HIGH>;
> >> +            states = <3300000 0>,
> >> +                     <1800000 1>;
> >> +    };
> >> +
> >> +    sound {
> >> +            compatible = "amlogic,axg-sound-card";
> >> +            audio-widgets = "Line", "Lineout";
> >> +            audio-aux-devs = <&tdmout_a>, <&tdmout_b>, <&tdmout_c>,
> >> +                             <&tdmin_a>, <&tdmin_b>, <&tdmin_c>,
> >> +                             <&dioo2133>;
> >> +
> >> +            assigned-clocks = <&clkc CLKID_MPLL2>,
> >> +                              <&clkc CLKID_MPLL0>,
> >> +                              <&clkc CLKID_MPLL1>;
> >> +            assigned-clock-parents = <0>, <0>, <0>;
> >> +            assigned-clock-rates = <294912000>,
> >> +                                   <270950400>,
> >> +                                   <393216000>;
> >> +
> >> +            dai-link-0 {
> >> +                    sound-dai = <&frddr_a>;
> >> +            };
> >> +
> >> +            dai-link-1 {
> >> +                    sound-dai = <&frddr_b>;
> >> +            };
> >> +
> >> +            dai-link-2 {
> >> +                    sound-dai = <&frddr_c>;
> >> +            };
> >> +
> >> +            dai-link-3 {
> >> +                    sound-dai = <&toddr_a>;
> >> +            };
> >> +
> >> +            dai-link-4 {
> >> +                    sound-dai = <&toddr_b>;
> >> +            };
> >> +
> >> +            dai-link-5 {
> >> +                    sound-dai = <&toddr_c>;
> >> +            };
> >> +
> >> +            /*
> >> +             * Audio setup: The 40 pins header provides access to 2 TDMs,
> >> +             * SPDIF In/Out and PDM inputs.
> >> +             * - TDM A: 2 lanes
> >> +             *    D0:    40/X9
> >> +             *    D1:    38/X8
> >> +             *    BCLK:  12/X11
> >> +             *    FS:    35/X10
> >> +             * - TDM B: 4 lanes
> >> +             *    D0:    37/A3
> >> +             *    D1:    16/A4
> >> +             *    D2:    18/A5 or 7/AO6
> >> +             *    D3:    22/A6 or 21/H5
> >> +             *    BCLK:  29/A1 or 8/AO8
> >> +             *    FS:    31/A2 or 11/AO7
> >> +             * - 2 Master Clocks:
> >> +             *    MCLK0: 15/A0 or 10/AO9
> >> +             *    MCLK1: 33/X15
> >> +             * - SPDIF:
> >> +             *    OUT:   32/A11
> >> +             *    IN:    21/H5
> >> +             * - PDM Input:
> >> +             *    DO:    13/A8
> >> +             *    D1:    26/A9
> >> +             *    D2:    22/A6
> >> +             *    D3:    18/A5
> >> +             *    DCLK:  36/A7
> >> +             *
> >> +             * TDM C is not usable on the 40 pins connector so it is
> >> +             * setup for the HDMI 4 lanes i2s.
> >> +             *
> >> +             * No pinctrl is enabled by default to preserve the
> >> +             * genericity of the 40 pins header. Many configurations are
> >> +             * possible based on the desired use case. Please adjust TDM
> >> +             * masks, clock setups and pinctrl accordingly.
> >> +             */
> >> +
> >> +            dai-link-6 {
> >> +                    sound-dai = <&tdmif_a>;
> >> +                    dai-format = "dsp_a";
> >> +                    dai-tdm-slot-tx-mask-0 = <1 1>;
> >> +                    mclk-fs = <256>;
> >> +
> >> +                    codec-0 {
> >> +                            sound-dai = <&tohdmitx TOHDMITX_I2S_IN_A>;
> >> +                    };
> >> +
> >> +                    codec-1 {
> >> +                            sound-dai = <&toacodec TOACODEC_IN_A>;
> >> +                    };
> >> +            };
> >> +
> >> +            dai-link-7 {
> >> +                    sound-dai = <&tdmif_b>;
> >> +                    dai-format = "i2s";
> >> +                    dai-tdm-slot-tx-mask-0 = <1 1>;
> >> +                    dai-tdm-slot-rx-mask-1 = <1 1>;
> >> +                    mclk-fs = <256>;
> >> +
> >> +                    codec-0 {
> >> +                            sound-dai = <&tohdmitx TOHDMITX_I2S_IN_B>;
> >> +                    };
> >> +
> >> +                    codec-1 {
> >> +                            sound-dai = <&toacodec TOACODEC_IN_B>;
> >> +                    };
> >> +            };
> >> +
> >> +            dai-link-8 {
> >> +                    sound-dai = <&tdmif_c>;
> >> +                    dai-format = "i2s";
> >> +                    dai-tdm-slot-tx-mask-0 = <1 1>;
> >> +                    dai-tdm-slot-tx-mask-1 = <1 1>;
> >> +                    dai-tdm-slot-tx-mask-2 = <1 1>;
> >> +                    dai-tdm-slot-tx-mask-3 = <1 1>;
> >> +                    mclk-fs = <256>;
> >> +
> >> +                    codec-0 {
> >> +                            sound-dai = <&tohdmitx TOHDMITX_I2S_IN_C>;
> >> +                    };
> >> +
> >> +                    codec-1 {
> >> +                            sound-dai = <&toacodec TOACODEC_IN_C>;
> >> +                    };
> >> +            };
> >> +
> >> +            dai-link-9 {
> >> +                    sound-dai = <&tohdmitx TOHDMITX_I2S_OUT>;
> >> +
> >> +                    codec {
> >> +                            sound-dai = <&hdmi_tx>;
> >> +                    };
> >> +            };
> >> +
> >> +            dai-link-10 {
> >> +                    sound-dai = <&toacodec TOACODEC_OUT>;
> >> +
> >> +                    codec {
> >> +                            sound-dai = <&acodec>;
> >> +                    };
> >> +            };
> >> +    };
> >> +};
> >> +
> >> +&acodec {
> >> +    status = "okay";
> >> +    AVDD-supply = <&vddio_ao18>;
> >> +};
> >> +
> >> +&arb {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&cecb_AO {
> >> +    status = "okay";
> >> +    pinctrl-0 = <&cec_ao_b_h_pins>;
> >> +    pinctrl-names = "default";
> >> +    hdmi-phandle = <&hdmi_tx>;
> >> +};
> >> +
> >> +&clkc_audio {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&cvbs_vdac_port {
> >> +    cvbs_vdac_out: endpoint {
> >> +            remote-endpoint = <&cvbs_connector_in>;
> >> +    };
> >> +};
> >> +
> >> +&ethmac {
> >> +    pinctrl-0 = <&eth_pins>, <&eth_rgmii_pins>, <&eth_phy_irq_pins>;
> >> +    pinctrl-names = "default";
> >> +    status = "okay";
> >> +    phy-mode = "rgmii";
> >> +    phy-handle = <&external_phy>;
> >> +    amlogic,tx-delay-ns = <2>;
> >> +};
> >> +
> >> +&ext_mdio {
> >> +    external_phy: ethernet-phy@0 {
> >> +            /* Realtek RTL8211F (0x001cc916) */
> >> +            reg = <0>;
> >> +            max-speed = <1000>;
> >> +
> >> +            reset-assert-us = <100000>;
> >> +            reset-deassert-us = <100000>;
> >> +            reset-gpios = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
> >> +
> >> +            interrupt-parent = <&gpio_intc>;
> >> +            /* MAC_INTR on GPIOZ_14 */
> >> +            interrupts = <26 IRQ_TYPE_LEVEL_LOW>;
> >> +    };
> >> +};
> >> +
> >> +&frddr_a {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&frddr_b {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&frddr_c {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&hdmi_tx {
> >> +    status = "okay";
> >> +    pinctrl-0 = <&hdmitx_hpd_pins>, <&hdmitx_ddc_pins>;
> >> +    pinctrl-names = "default";
> >> +    hdmi-supply = <&vcc_5v>;
> >> +};
> >> +
> >> +&hdmi_tx_tmds_port {
> >> +    hdmi_tx_tmds_out: endpoint {
> >> +            remote-endpoint = <&hdmi_connector_in>;
> >> +    };
> >> +};
> >> +
> >> +&ir {
> >> +    status = "okay";
> >> +    pinctrl-0 = <&remote_input_ao_pins>;
> >> +    pinctrl-names = "default";
> >> +};
> >> +
> >> +&npu {
> >> +    status = "okay";
> >> +};
> >
> > Are you sure you want this enabled ?
> > AFAIK if etnaviv is enabled, it will be used first as a render node by mesa and fail.
> >
>
> AFAIK, it is enabled in the pre-flashed bootloader that is running
> debian, fedora and opensuse ... but etnaviv is blacklisted there
>
> It should indeed be removed.
>
>

Yes, let's remove this until there are future developments in this
area. We will add it to our tooling to enable separately.

> >> +
> >> +&periphs_pinctrl {
> >> +    spi_cs_disable_pins: spi-cs-disable {
> >> +            mux {
> >> +                    groups = "BOOT_14";
> >> +                    function = "gpio_periphs";
> >> +                    bias-disable;
> >> +                    output-high;
> >> +            };
> >> +    };
> >> +
> >> +    eth_phy_irq_pins: eth-phy-irq {
> >> +            mux {
> >> +                    groups = "GPIOZ_14";
> >> +                    function = "gpio_periphs";
> >> +                    bias-pull-up;
> >> +                    output-disable;
> >> +            };
> >> +    };
> >> +};
> >> +
> >> +&pwm_AO_cd {
> >> +    status = "okay";
> >> +    pinctrl-0 = <&pwm_ao_d_e_pins>;
> >> +    pinctrl-names = "default";
> >> +    clocks = <&xtal>;
> >> +    clock-names = "clkin1";
> >> +};
> >> +
> >> +&pwm_ab {
> >> +    status = "okay";
> >> +    pinctrl-0 = <&pwm_b_x7_pins>;
> >> +    pinctrl-names = "default";
> >> +    clocks = <&xtal>;
> >> +    clock-names = "clkin1";
> >> +};
> >> +
> >> +&pwm_cd {
> >> +    status = "okay";
> >> +    pinctrl-0 = <&pwm_d_x3_pins>;
> >> +    pinctrl-names = "default";
> >> +    clocks = <&xtal>;
> >> +    clock-names = "clkin1";
> >> +};
> >> +
> >> +&saradc {
> >> +    status = "okay";
> >> +    vref-supply = <&vddio_ao18>;
> >> +};
> >> +
> >> +/* SD card */
> >> +&sd_emmc_b {
> >> +    status = "okay";
> >> +    pinctrl-0 = <&sdcard_c_pins>;
> >> +    pinctrl-1 = <&sdcard_clk_gate_c_pins>;
> >> +    pinctrl-names = "default", "clk-gate";
> >> +
> >> +    bus-width = <4>;
> >> +    cap-sd-highspeed;
> >> +    sd-uhs-sdr12;
> >> +    sd-uhs-sdr25;
> >> +    sd-uhs-sdr50;
> >> +    sd-uhs-sdr104;
> >> +    max-frequency = <200000000>;
> >> +    disable-wp;
> >> +
> >> +    cd-gpios = <&gpio GPIOC_6 GPIO_ACTIVE_LOW>;
> >> +    vmmc-supply = <&vcc_card>;
> >> +    vqmmc-supply = <&vddio_c>;
> >> +};
> >> +
> >> +/*
> >> + * EMMC_D4, EMMC_D5, EMMC_D6 and EMMC_D7 pins are shared between SPI NOR CS
> >> + * and eMMC Data 4 to 7 pins.
> >> + * Replace emmc_data_8b_pins to emmc_data_4b_pins from sd_emmc_c pinctrl-0,
> >> + * and change bus-width to 4 then spifc can be enabled.
> >> + */
> >> +&sd_emmc_c {
> >> +    status = "okay";
> >> +    pinctrl-0 = <&emmc_ctrl_pins>, <&emmc_data_8b_pins>, <&emmc_ds_pins>,
> >> +                <&spi_cs_disable_pins>;
> >> +    pinctrl-1 = <&emmc_clk_gate_pins>;
> >> +    pinctrl-names = "default", "clk-gate";
> >> +
> >> +    bus-width = <8>;
> >> +    cap-mmc-highspeed;
> >> +    mmc-hs200-1_8v;
> >> +    max-frequency = <200000000>;
> >> +    disable-wp;
> >> +
> >> +    mmc-pwrseq = <&emmc_pwrseq>;
> >> +    vmmc-supply = <&vcc_3v3>;
> >> +    vqmmc-supply = <&flash_1v8>;
> >> +};
> >> +
> >> +&spifc {
> >> +    status = "disabled";
> >> +    pinctrl-0 = <&nor_pins>;
> >> +    pinctrl-names = "default";
> >> +    cs-gpios = <&gpio BOOT_14 GPIO_ACTIVE_LOW>;
> >> +
> >> +    w25lq128d: flash@0 {
> >> +            compatible = "jedec,spi-nor";
> >> +            reg = <0>;
> >> +            #address-cells = <1>;
> >> +            #size-cells = <1>;
> >> +            spi-max-frequency = <80000000>;
> >> +    };
> >> +};
> >> +
> >> +&tdmif_a {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&tdmif_b {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&tdmif_c {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&tdmin_a {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&tdmin_b {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&tdmin_c {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&tdmout_a {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&tdmout_b {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&tdmout_c {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&toacodec {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&toddr_a {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&toddr_b {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&toddr_c {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&tohdmitx {
> >> +    status = "okay";
> >> +};
> >> +
> >> +&uart_AO {
> >> +    status = "okay";
> >> +    pinctrl-0 = <&uart_ao_a_pins>;
> >> +    pinctrl-names = "default";
> >> +};
> >> +
> >> +&usb2_phy1 {
> >> +    phy-supply = <&dc_in>;
> >> +};
> >> +
> >> +&usb3_pcie_phy {
> >> +    #address-cells = <1>;
> >> +    #size-cells = <0>;
> >> +    phy-supply = <&vcc_5v>;
> >> +
> >> +    hub: hub@1 {
> >> +            compatible = "usb5e3,626";
> >> +            reg = <1>;
> >> +            reset-gpios = <&gpio GPIOC_7 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
> >> +    };
> >
> > Not sure the PHY is the right place to put the USB HUB,
> > and it's probable the HUB is connected to both the USB2 and USB3 lines
>
> It is connected to the USB3.0 only

https://drive.google.com/file/d/17WNK9m8VZ9CfS7GMiTLdq1zHsmHR4OcV/view

USBHOST_A_DM and USBHOST_A_DP are connected to the USB 3 hub DM0 and DP0.
Not sure if this counts?

>
> > so you should have both USB IDs in DT like it'd done for the Odroid-C4:
> >
> > / {
> > ...
> >          /* USB hub supports both USB 2.0 and USB 3.0 root hub */
> >          usb-hub {
> >                  dr_mode = "host";
> >                  #address-cells = <1>;
> >                  #size-cells = <0>;
> >
> >                  /* 2.0 hub on port 1 */
> >                  hub_2_0: hub@1 {
> >                          compatible = "usb2109,2817";
> >                          reg = <1>;
> >                          peer-hub = <&hub_3_0>;
> >                          reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
> >                          vdd-supply = <&vcc_5v>;
> >                  };
> >
> >                  /* 3.1 hub on port 4 */
> >                  hub_3_0: hub@2 {
> >                          compatible = "usb2109,817";
> >                          reg = <2>;
> >                          peer-hub = <&hub_2_0>;
> >                          reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
> >                          vdd-supply = <&vcc_5v>;
> >                  };
> >          };
> > ...
> > };
> >
> > if it only has a single USB ID, then it should go under the dwc3 node.
>
> The usb controller is connected to the PHY and what's coming out of the PHY
> goes to the hub. It seems logical to hub the hub under it.
>
> Why bypass the PHY ?
>
> >
> >> +};
> >> +
> >> +&usb {
> >> +    status = "okay";
> >> +};
> >> diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
> >> new file mode 100644
> >> index 000000000000..077e7506ce4f
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
> >> @@ -0,0 +1,89 @@
> >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >> +/*
> >> + * Copyright (c) 2023 BayLibre, SAS.
> >> + * Author: Jerome Brunet <jbrunet@baylibre.com>
> >> + */
> >> +
> >> +/dts-v1/;
> >> +
> >> +#include <dt-bindings/clock/g12a-clkc.h>
> >> +#include "meson-sm1.dtsi"
> >> +#include "meson-libretech-cottonwood.dtsi"
> >> +
> >> +/ {
> >> +    compatible = "libretech,aml-s905d3-cc", "amlogic,sm1";
> >> +    model = "Libre Computer AML-S905D3-CC Solitude";
> >> +
> >> +    sound {
> >> +            model = "Solitude";
> >> +            audio-routing = "TDMOUT_A IN 0", "FRDDR_A OUT 0",
> >> +                            "TDMOUT_A IN 1", "FRDDR_B OUT 0",
> >> +                            "TDMOUT_A IN 2", "FRDDR_C OUT 0",
> >> +                            "TDM_A Playback", "TDMOUT_A OUT",
> >> +                            "TDMOUT_B IN 0", "FRDDR_A OUT 1",
> >> +                            "TDMOUT_B IN 1", "FRDDR_B OUT 1",
> >> +                            "TDMOUT_B IN 2", "FRDDR_C OUT 1",
> >> +                            "TDM_B Playback", "TDMOUT_B OUT",
> >> +                            "TDMOUT_C IN 0", "FRDDR_A OUT 2",
> >> +                            "TDMOUT_C IN 1", "FRDDR_B OUT 2",
> >> +                            "TDMOUT_C IN 2", "FRDDR_C OUT 2",
> >> +                            "TDM_C Playback", "TDMOUT_C OUT",
> >> +                            "TDMIN_A IN 0", "TDM_A Capture",
> >> +                            "TDMIN_B IN 0", "TDM_A Capture",
> >> +                            "TDMIN_C IN 0", "TDM_A Capture",
> >> +                            "TDMIN_A IN 13", "TDM_A Loopback",
> >> +                            "TDMIN_B IN 13", "TDM_A Loopback",
> >> +                            "TDMIN_C IN 13", "TDM_A Loopback",
> >> +                            "TDMIN_A IN 1", "TDM_B Capture",
> >> +                            "TDMIN_B IN 1", "TDM_B Capture",
> >> +                            "TDMIN_C IN 1", "TDM_B Capture",
> >> +                            "TDMIN_A IN 14", "TDM_B Loopback",
> >> +                            "TDMIN_B IN 14", "TDM_B Loopback",
> >> +                            "TDMIN_C IN 14", "TDM_B Loopback",
> >> +                            "TDMIN_A IN 2", "TDM_C Capture",
> >> +                            "TDMIN_B IN 2", "TDM_C Capture",
> >> +                            "TDMIN_C IN 2", "TDM_C Capture",
> >> +                            "TDMIN_A IN 15", "TDM_C Loopback",
> >> +                            "TDMIN_B IN 15", "TDM_C Loopback",
> >> +                            "TDMIN_C IN 15", "TDM_C Loopback",
> >> +                            "TODDR_A IN 0", "TDMIN_A OUT",
> >> +                            "TODDR_B IN 0", "TDMIN_A OUT",
> >> +                            "TODDR_C IN 0", "TDMIN_A OUT",
> >> +                            "TODDR_A IN 1", "TDMIN_B OUT",
> >> +                            "TODDR_B IN 1", "TDMIN_B OUT",
> >> +                            "TODDR_C IN 1", "TDMIN_B OUT",
> >> +                            "TODDR_A IN 2", "TDMIN_C OUT",
> >> +                            "TODDR_B IN 2", "TDMIN_C OUT",
> >> +                            "TODDR_C IN 2", "TDMIN_C OUT",
> >> +                            "Lineout", "ACODEC LOLP",
> >> +                            "Lineout", "ACODEC LORP";
> >> +    };
> >> +};
> >> +
> >> +&cpu0 {
> >> +    cpu-supply = <&vddcpu_b>;
> >> +    operating-points-v2 = <&cpu_opp_table>;
> >> +    clocks = <&clkc CLKID_CPU_CLK>;
> >> +    clock-latency = <50000>;
> >> +};
> >> +
> >> +&cpu1 {
> >> +    cpu-supply = <&vddcpu_b>;
> >> +    operating-points-v2 = <&cpu_opp_table>;
> >> +    clocks = <&clkc CLKID_CPU1_CLK>;
> >> +    clock-latency = <50000>;
> >> +};
> >> +
> >> +&cpu2 {
> >> +    cpu-supply = <&vddcpu_b>;
> >> +    operating-points-v2 = <&cpu_opp_table>;
> >> +    clocks = <&clkc CLKID_CPU2_CLK>;
> >> +    clock-latency = <50000>;
> >> +};
> >> +
> >> +&cpu3 {
> >> +    cpu-supply = <&vddcpu_b>;
> >> +    operating-points-v2 = <&cpu_opp_table>;
> >> +    clocks = <&clkc CLKID_CPU3_CLK>;
> >> +    clock-latency = <50000>;
> >> +};
> >
> > Apart that, it looks fine,
> >
> >
> > Thanks,
> > Neil
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
  

> On 3 Oct 2023, at 1:15 am, Da Xue <da@lessconfused.com> wrote:
> 
> On Tue, Oct 3, 2023 at 3:13 AM Jerome Brunet <jbrunet@baylibre.com> wrote:
>> 
>> 
>> On Mon 02 Oct 2023 at 18:45, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>> 
>>> Hi,
>>> 
>>> On 02/10/2023 16:10, Jerome Brunet wrote:
>>>> Add support for the Libretech cottonwood board family.
>>>> These 2 boards are based on the same PCB, with an RPi B form factor.
>>>> The "Alta" board uses an a311d while the "Solitude" variant uses an
>>>> s905d3.
>>>> Co-developed-by: Da Xue <da.xue@libretech.co>
>>>> Signed-off-by: Da Xue <da.xue@libretech.co>
>>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>>> ---
>>>>  arch/arm64/boot/dts/amlogic/Makefile          |   2 +
>>>>  .../amlogic/meson-g12b-a311d-libretech-cc.dts | 133 ++++
>>>>  .../amlogic/meson-libretech-cottonwood.dtsi   | 610 ++++++++++++++++++
>>>>  .../amlogic/meson-sm1-s905d3-libretech-cc.dts |  89 +++
>>>>  4 files changed, 834 insertions(+)
>>>>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
>>>>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
>>>>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
>>>> diff --git a/arch/arm64/boot/dts/amlogic/Makefile
>>>> b/arch/arm64/boot/dts/amlogic/Makefile
>>>> index 4ce401d17b63..cc8b34bd583d 100644
>>>> --- a/arch/arm64/boot/dts/amlogic/Makefile
>>>> +++ b/arch/arm64/boot/dts/amlogic/Makefile
>>>> @@ -18,6 +18,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-g12b-bananapi-cm4-cm4io.dtb
>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gsking-x.dtb
>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking-pro.dtb
>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking.dtb
>>>> +dtb-$(CONFIG_ARCH_MESON) += meson-g12b-a311d-libretech-cc.dtb
>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-go-ultra.dtb
>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2-plus.dtb
>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2.dtb
>>>> @@ -73,6 +74,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-sm1-bananapi-m2-pro.dtb
>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-sm1-bananapi-m5.dtb
>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-sm1-h96-max.dtb
>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-sm1-khadas-vim3l.dtb
>>>> +dtb-$(CONFIG_ARCH_MESON) += meson-sm1-s905d3-libretech-cc.dtb
>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-c4.dtb
>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-hc4.dtb
>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb
>>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
>>>> new file mode 100644
>>>> index 000000000000..fc890e235dbd
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
>>>> @@ -0,0 +1,133 @@
>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>> +/*
>>>> + * Copyright (c) 2023 BayLibre, SAS.
>>>> + * Author: Jerome Brunet <jbrunet@baylibre.com>
>>>> + */
>>>> +
>>>> +/dts-v1/;
>>>> +
>>>> +#include <dt-bindings/clock/g12a-clkc.h>
>>>> +#include "meson-g12b-a311d.dtsi"
>>>> +#include "meson-libretech-cottonwood.dtsi"
>>>> +
>>>> +/ {
>>>> +    compatible = "libretech,aml-a311d-cc", "amlogic,a311d", "amlogic,g12b";
>>>> +    model = "Libre Computer AML-A311D-CC Alta";
>>>> +
>>>> +    vddcpu_a: regulator-vddcpu-a {
>>>> +            compatible = "pwm-regulator";
>>>> +            regulator-name = "VDDCPU_A";
>>>> +            regulator-min-microvolt = <730000>;
>>>> +            regulator-max-microvolt = <1011000>;
>>>> +            regulator-boot-on;
>>>> +            regulator-always-on;
>>>> +            pwm-supply = <&dc_in>;
>>>> +            pwms = <&pwm_ab 0 1250 0>;
>>>> +            pwm-dutycycle-range = <100 0>;
>>>> +    };
>>>> +
>>>> +    sound {
>>>> +            model = "Alta";
>>> 
>>> I think those sound model properties should be coherent with the
>>> other Libre Computer boards:
>>> arch/arm64/boot/dts/amlogic/meson-gx-libretech-pc.dtsi:         model = "LIBRETECH-PC";
>>> arch/arm64/boot/dts/amlogic/meson-gxl-s805x-libretech-ac.dts:           model = "LIBRETECH-AC";
>>> arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc-v2.dts:                model = "LIBRETECH-CC-V2";
>>> arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts:           model = "LIBRETECH-CC";
>> 
>> "LIBRETECH-CC-" leave very little room to play with
>> That's not really something that could have been anticipated 5+ years ago
>> 
> 
> I think the formal model name is best, maybe with LC prefix.
> eg. LC-AML-A311D-CC and LC-AML-S905D3-CC

The first is valid. The second will be truncated to LC-AML-S905D3-C by the
alsa 15-character naming limit (mentioned below).

So name/rename them to:

LC-XXXXXXXXXXXX <= MAX SIZE (15 Chars)
LC-LEPOTATO
LC-LEPOTATO-V2
LC-LAFRITE
LC-TARTIFLETTE
LC-ALTA
LC-SOLITUDE

Personally I think the plain codenames (no "LC-“) work best as they are all
distinctive. Whenever I see lists of the official board names they look/read
the same at first glance and then I have to spot-the-difference to tell them
apart.

At the moment AFAIK these names are just cosmetic as there’s no Amlogic alsa
ucm confs using board model (only downstream confs based on driver name). So
IMHO rework the names now before the confs go upstream.

CH.

> https://hub.libre.computer/t/libre-computer-board-naming-and-conventions/100
> 
>>> 
>>> It's ok to change the scheme since it's tried to keep the name under the 15 characters limit,
>>> will the next board keep this naming ?
>> 
>> I don't know what the next board will be so I can hardly make any prediction
>> I'm open to suggestion if you prefer something else
>> 
>>> 
>>> 
>>>> +            audio-routing = "TDMOUT_A IN 0", "FRDDR_A OUT 0",
>>>> +                            "TDMOUT_A IN 1", "FRDDR_B OUT 0",
>>>> +                            "TDMOUT_A IN 2", "FRDDR_C OUT 0",
>>>> +                            "TDM_A Playback", "TDMOUT_A OUT",
>>>> +                            "TDMOUT_B IN 0", "FRDDR_A OUT 1",
>>>> +                            "TDMOUT_B IN 1", "FRDDR_B OUT 1",
>>>> +                            "TDMOUT_B IN 2", "FRDDR_C OUT 1",
>>>> +                            "TDM_B Playback", "TDMOUT_B OUT",
>>>> +                            "TDMOUT_C IN 0", "FRDDR_A OUT 2",
>>>> +                            "TDMOUT_C IN 1", "FRDDR_B OUT 2",
>>>> +                            "TDMOUT_C IN 2", "FRDDR_C OUT 2",
>>>> +                            "TDM_C Playback", "TDMOUT_C OUT",
>>>> +                            "TDMIN_A IN 0", "TDM_A Capture",
>>>> +                            "TDMIN_B IN 0", "TDM_A Capture",
>>>> +                            "TDMIN_C IN 0", "TDM_A Capture",
>>>> +                            "TDMIN_A IN 3", "TDM_A Loopback",
>>>> +                            "TDMIN_B IN 3", "TDM_A Loopback",
>>>> +                            "TDMIN_C IN 3", "TDM_A Loopback",
>>>> +                            "TDMIN_A IN 1", "TDM_B Capture",
>>>> +                            "TDMIN_B IN 1", "TDM_B Capture",
>>>> +                            "TDMIN_C IN 1", "TDM_B Capture",
>>>> +                            "TDMIN_A IN 4", "TDM_B Loopback",
>>>> +                            "TDMIN_B IN 4", "TDM_B Loopback",
>>>> +                            "TDMIN_C IN 4", "TDM_B Loopback",
>>>> +                            "TDMIN_A IN 2", "TDM_C Capture",
>>>> +                            "TDMIN_B IN 2", "TDM_C Capture",
>>>> +                            "TDMIN_C IN 2", "TDM_C Capture",
>>>> +                            "TDMIN_A IN 5", "TDM_C Loopback",
>>>> +                            "TDMIN_B IN 5", "TDM_C Loopback",
>>>> +                            "TDMIN_C IN 5", "TDM_C Loopback",
>>>> +                            "TODDR_A IN 0", "TDMIN_A OUT",
>>>> +                            "TODDR_B IN 0", "TDMIN_A OUT",
>>>> +                            "TODDR_C IN 0", "TDMIN_A OUT",
>>>> +                            "TODDR_A IN 1", "TDMIN_B OUT",
>>>> +                            "TODDR_B IN 1", "TDMIN_B OUT",
>>>> +                            "TODDR_C IN 1", "TDMIN_B OUT",
>>>> +                            "TODDR_A IN 2", "TDMIN_C OUT",
>>>> +                            "TODDR_B IN 2", "TDMIN_C OUT",
>>>> +                            "TODDR_C IN 2", "TDMIN_C OUT",
>>>> +                            "Lineout", "ACODEC LOLP",
>>>> +                            "Lineout", "ACODEC LORP";
>>>> +    };
>>>> +};
>>>> +
>>>> +&cpu_opp_table_0 {
>>>> +            opp-1800000000 {
>>>> +                    opp-hz = /bits/ 64 <1800000000>;
>>>> +                    opp-microvolt = <1001000>;
>>>> +            };
> 
> This seems to match
> https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/amlogic/meson-g12b-a311d.dtsi#L44
> 
>>>> +
>>>> +            opp-2016000000 {
>>>> +                    opp-hz = /bits/ 64 <2016000000>;
>>>> +                    opp-microvolt = <1011000>;
>>>> +            };
> 
> We run manufacturing verification at this speed but I don't think we
> should include this in upstream.
> 
>>>> +};
>>>> +
>>>> +&cpu0 {
>>>> +    cpu-supply = <&vddcpu_b>;
>>>> +    operating-points-v2 = <&cpu_opp_table_0>;
>>>> +    clocks = <&clkc CLKID_CPU_CLK>;
>>>> +    clock-latency = <50000>;
>>>> +};
>>>> +
>>>> +&cpu1 {
>>>> +    cpu-supply = <&vddcpu_b>;
>>>> +    operating-points-v2 = <&cpu_opp_table_0>;
>>>> +    clocks = <&clkc CLKID_CPU_CLK>;
>>>> +    clock-latency = <50000>;
>>>> +};
>>>> +
>>>> +&cpu100 {
>>>> +    cpu-supply = <&vddcpu_a>;
>>>> +    operating-points-v2 = <&cpub_opp_table_1>;
>>>> +    clocks = <&clkc CLKID_CPUB_CLK>;
>>>> +    clock-latency = <50000>;
>>>> +};
>>>> +
>>>> +&cpu101 {
>>>> +    cpu-supply = <&vddcpu_a>;
>>>> +    operating-points-v2 = <&cpub_opp_table_1>;
>>>> +    clocks = <&clkc CLKID_CPUB_CLK>;
>>>> +    clock-latency = <50000>;
>>>> +};
>>>> +
>>>> +&cpu102 {
>>>> +    cpu-supply = <&vddcpu_a>;
>>>> +    operating-points-v2 = <&cpub_opp_table_1>;
>>>> +    clocks = <&clkc CLKID_CPUB_CLK>;
>>>> +    clock-latency = <50000>;
>>>> +};
>>>> +
>>>> +&cpu103 {
>>>> +    cpu-supply = <&vddcpu_a>;
>>>> +    operating-points-v2 = <&cpub_opp_table_1>;
>>>> +    clocks = <&clkc CLKID_CPUB_CLK>;
>>>> +    clock-latency = <50000>;
>>>> +};
>>>> +
>>>> +&pwm_ab {
>>>> +    pinctrl-0 = <&pwm_a_e_pins>, <&pwm_b_x7_pins>;
>>>> +    clocks = <&xtal>, <&xtal>;
>>>> +    clock-names = "clkin0", "clkin1";
>>>> +};
>>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi b/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
>>>> new file mode 100644
>>>> index 000000000000..a7fc8963ff54
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
>>>> @@ -0,0 +1,610 @@
>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>> +/*
>>>> + * Copyright (c) 2023 BayLibre, SAS.
>>>> + * Author: Jerome Brunet <jbrunet@baylibre.com>
>>>> + */
>>>> +
>>>> +#include <dt-bindings/clock/g12a-clkc.h>
>>>> +#include <dt-bindings/input/input.h>
>>>> +#include <dt-bindings/leds/common.h>
>>>> +#include <dt-bindings/gpio/gpio.h>
>>>> +#include <dt-bindings/gpio/meson-g12a-gpio.h>
>>>> +#include <dt-bindings/sound/meson-g12a-toacodec.h>
>>>> +#include <dt-bindings/sound/meson-g12a-tohdmitx.h>
>>>> +
>>>> +/ {
>>>> +    aliases {
>>>> +            serial0 = &uart_AO;
>>>> +            ethernet0 = &ethmac;
>>>> +            spi0 = &spifc;
>>>> +    };
>>>> +
>>>> +    memory@0 {
>>>> +            device_type = "memory";
>>>> +            reg = <0x0 0x0 0x0 0x80000000>;
>>>> +    };
>>>> +
>>>> +    chosen {
>>>> +            stdout-path = "serial0:115200n8";
>>>> +    };
>>>> +
>>>> +    dioo2133: audio-amplifier-0 {
>>>> +            compatible = "simple-audio-amplifier";
>>>> +            enable-gpios = <&gpio GPIOX_0 GPIO_ACTIVE_HIGH>;
>>>> +            VCC-supply = <&vcc_5v>;
>>>> +            sound-name-prefix = "10U2";
>>>> +    };
>>>> +
>>>> +    /* TOFIX: handle CVBS_DET on SARADC channel 0 */
>>>> +    cvbs-connector {
>>>> +            compatible = "composite-video-connector";
>>>> +
>>>> +            port {
>>>> +                    cvbs_connector_in: endpoint {
>>>> +                            remote-endpoint = <&cvbs_vdac_out>;
>>>> +                    };
>>>> +            };
>>>> +    };
>>>> +
>>>> +    emmc_pwrseq: emmc-pwrseq {
>>>> +            compatible = "mmc-pwrseq-emmc";
>>>> +            reset-gpios = <&gpio BOOT_12 GPIO_ACTIVE_LOW>;
>>>> +    };
>>>> +
>>>> +    hdmi-connector {
>>>> +            compatible = "hdmi-connector";
>>>> +            type = "a";
>>>> +
>>>> +            port {
>>>> +                    hdmi_connector_in: endpoint {
>>>> +                            remote-endpoint = <&hdmi_tx_tmds_out>;
>>>> +                    };
>>>> +            };
>>>> +    };
>>>> +
>>>> +    leds-pwm {
>>>> +            compatible = "pwm-leds";
>>>> +
>>>> +            led-green {
>>>> +                    color = <LED_COLOR_ID_GREEN>;
>>>> +                    function = LED_FUNCTION_STATUS;
>>>> +                    linux,default-trigger = "default-on";
>>>> +                    panic-indicator;
>>>> +                    max-brightness = <255>;
>>>> +                    pwms = <&pwm_cd 1 1250 0>;
>>>> +                    active-low;
>>>> +            };
>>>> +
>>>> +            led-blue {
>>>> +                    color = <LED_COLOR_ID_BLUE>;
>>>> +                    function = LED_FUNCTION_ACTIVITY;
>>>> +                    linux,default-trigger = "activity";
>>>> +                    max-brightness = <255>;
>>>> +                    pwms = <&pwm_ab 1 1250 0>;
>>>> +                    active-low;
>>>> +            };
>>>> +    };
>>>> +
>>>> +    leds-gpio {
>>>> +            compatible = "gpio-leds";
>>>> +
>>>> +            led-orange {
>>>> +                    color = <LED_COLOR_ID_AMBER>;
> 
> Should this be LED_COLOR_ID_ORANGE?
> 
>>>> +                    function = LED_FUNCTION_STANDBY;
>>>> +                    gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
>>>> +            };
>>>> +    };
>>>> +
>>>> +    dc_in: regulator-dc-in {
>>>> +            compatible = "regulator-fixed";
>>>> +            regulator-name = "5V_IN";
>>>> +            regulator-min-microvolt = <5000000>;
>>>> +            regulator-max-microvolt = <5000000>;
>>>> +            regulator-always-on;
>>>> +    };
>>>> +
>>>> +    flash_1v8: regulator-flash-1v8 {
>>>> +            compatible = "regulator-fixed";
>>>> +            regulator-name = "FLASH_1V8";
>>>> +            regulator-min-microvolt = <1800000>;
>>>> +            regulator-max-microvolt = <1800000>;
>>>> +            regulator-always-on;
>>>> +            vin-supply = <&vcc_3v3>;
>>>> +    };
>>>> +
>>>> +    vcc_card: regulator-vcc-card {
>>>> +            compatible = "regulator-fixed";
>>>> +            regulator-name = "VCC_CARD";
>>>> +            regulator-min-microvolt = <3300000>;
>>>> +            regulator-max-microvolt = <3300000>;
>>>> +            vin-supply = <&vddao_3v3>;
>>>> +            gpio = <&gpio GPIOX_2 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>>>> +            enable-active-high;
>>>> +            gpio-open-drain;
>>>> +    };
>>>> +
>>>> +    vcc_3v3: regulator-vcc-3v3 {
>>>> +            compatible = "regulator-fixed";
>>>> +            regulator-name = "VCC_3V3";
>>>> +            regulator-min-microvolt = <3300000>;
>>>> +            regulator-max-microvolt = <3300000>;
>>>> +            regulator-always-on;
>>>> +            vin-supply = <&vddao_3v3>;
>>>> +
>>>> +            /* FIXME: controlled by TEST_N */
>>>> +    };
>>>> +
>>>> +    vcc_5v: regulator-vcc-5v {
>>>> +            compatible = "regulator-fixed";
>>>> +            regulator-name = "VCC_5V";
>>>> +            regulator-min-microvolt = <5000000>;
>>>> +            regulator-max-microvolt = <5000000>;
>>>> +            regulator-always-on;
>>>> +            vin-supply = <&dc_in>;
>>>> +            gpio = <&gpio GPIOH_8 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>>>> +            enable-active-high;
>>>> +            gpio-open-drain;
>>>> +    };
>>>> +
>>>> +    vddao_3v3: regulator-vddao_3v3 {
>>>> +            compatible = "regulator-fixed";
>>>> +            regulator-name = "VDDAO_3V3";
>>>> +            regulator-min-microvolt = <3300000>;
>>>> +            regulator-max-microvolt = <3300000>;
>>>> +            regulator-always-on;
>>>> +            vin-supply = <&dc_in>;
>>>> +    };
>>>> +
>>>> +    vddcpu_b: regulator-vddcpu-b {
>>>> +            compatible = "pwm-regulator";
>>>> +            regulator-name = "VDDCPU_B";
>>>> +            regulator-min-microvolt = <730000>;
>>>> +            regulator-max-microvolt = <1011000>;
>>>> +            regulator-boot-on;
>>>> +            regulator-always-on;
>>>> +            pwm-supply = <&dc_in>;
>>>> +            pwms = <&pwm_AO_cd 1 1250 0>;
>>>> +            pwm-dutycycle-range = <100 0>;
>>>> +    };
>>>> +
>>>> +    vddio_ao18: regulator-vddio_ao18 {
>>>> +            compatible = "regulator-fixed";
>>>> +            regulator-name = "VDDIO_AO18";
>>>> +            regulator-min-microvolt = <1800000>;
>>>> +            regulator-max-microvolt = <1800000>;
>>>> +            regulator-always-on;
>>>> +            vin-supply = <&vddao_3v3>;
>>>> +    };
>>>> +
>>>> +    vddio_c: regulator-vddio_c {
>>>> +            compatible = "regulator-gpio";
>>>> +            regulator-name = "VDDIO_C";
>>>> +            regulator-min-microvolt = <1800000>;
>>>> +            regulator-max-microvolt = <3300000>;
>>>> +            regulator-settling-time-up-us = <200>;
>>>> +            regulator-settling-time-down-us = <50000>;
>>>> +            vin-supply = <&vddao_3v3>;
>>>> +            gpios = <&gpio GPIOX_4 GPIO_ACTIVE_HIGH>;
>>>> +            states = <3300000 0>,
>>>> +                     <1800000 1>;
>>>> +    };
>>>> +
>>>> +    sound {
>>>> +            compatible = "amlogic,axg-sound-card";
>>>> +            audio-widgets = "Line", "Lineout";
>>>> +            audio-aux-devs = <&tdmout_a>, <&tdmout_b>, <&tdmout_c>,
>>>> +                             <&tdmin_a>, <&tdmin_b>, <&tdmin_c>,
>>>> +                             <&dioo2133>;
>>>> +
>>>> +            assigned-clocks = <&clkc CLKID_MPLL2>,
>>>> +                              <&clkc CLKID_MPLL0>,
>>>> +                              <&clkc CLKID_MPLL1>;
>>>> +            assigned-clock-parents = <0>, <0>, <0>;
>>>> +            assigned-clock-rates = <294912000>,
>>>> +                                   <270950400>,
>>>> +                                   <393216000>;
>>>> +
>>>> +            dai-link-0 {
>>>> +                    sound-dai = <&frddr_a>;
>>>> +            };
>>>> +
>>>> +            dai-link-1 {
>>>> +                    sound-dai = <&frddr_b>;
>>>> +            };
>>>> +
>>>> +            dai-link-2 {
>>>> +                    sound-dai = <&frddr_c>;
>>>> +            };
>>>> +
>>>> +            dai-link-3 {
>>>> +                    sound-dai = <&toddr_a>;
>>>> +            };
>>>> +
>>>> +            dai-link-4 {
>>>> +                    sound-dai = <&toddr_b>;
>>>> +            };
>>>> +
>>>> +            dai-link-5 {
>>>> +                    sound-dai = <&toddr_c>;
>>>> +            };
>>>> +
>>>> +            /*
>>>> +             * Audio setup: The 40 pins header provides access to 2 TDMs,
>>>> +             * SPDIF In/Out and PDM inputs.
>>>> +             * - TDM A: 2 lanes
>>>> +             *    D0:    40/X9
>>>> +             *    D1:    38/X8
>>>> +             *    BCLK:  12/X11
>>>> +             *    FS:    35/X10
>>>> +             * - TDM B: 4 lanes
>>>> +             *    D0:    37/A3
>>>> +             *    D1:    16/A4
>>>> +             *    D2:    18/A5 or 7/AO6
>>>> +             *    D3:    22/A6 or 21/H5
>>>> +             *    BCLK:  29/A1 or 8/AO8
>>>> +             *    FS:    31/A2 or 11/AO7
>>>> +             * - 2 Master Clocks:
>>>> +             *    MCLK0: 15/A0 or 10/AO9
>>>> +             *    MCLK1: 33/X15
>>>> +             * - SPDIF:
>>>> +             *    OUT:   32/A11
>>>> +             *    IN:    21/H5
>>>> +             * - PDM Input:
>>>> +             *    DO:    13/A8
>>>> +             *    D1:    26/A9
>>>> +             *    D2:    22/A6
>>>> +             *    D3:    18/A5
>>>> +             *    DCLK:  36/A7
>>>> +             *
>>>> +             * TDM C is not usable on the 40 pins connector so it is
>>>> +             * setup for the HDMI 4 lanes i2s.
>>>> +             *
>>>> +             * No pinctrl is enabled by default to preserve the
>>>> +             * genericity of the 40 pins header. Many configurations are
>>>> +             * possible based on the desired use case. Please adjust TDM
>>>> +             * masks, clock setups and pinctrl accordingly.
>>>> +             */
>>>> +
>>>> +            dai-link-6 {
>>>> +                    sound-dai = <&tdmif_a>;
>>>> +                    dai-format = "dsp_a";
>>>> +                    dai-tdm-slot-tx-mask-0 = <1 1>;
>>>> +                    mclk-fs = <256>;
>>>> +
>>>> +                    codec-0 {
>>>> +                            sound-dai = <&tohdmitx TOHDMITX_I2S_IN_A>;
>>>> +                    };
>>>> +
>>>> +                    codec-1 {
>>>> +                            sound-dai = <&toacodec TOACODEC_IN_A>;
>>>> +                    };
>>>> +            };
>>>> +
>>>> +            dai-link-7 {
>>>> +                    sound-dai = <&tdmif_b>;
>>>> +                    dai-format = "i2s";
>>>> +                    dai-tdm-slot-tx-mask-0 = <1 1>;
>>>> +                    dai-tdm-slot-rx-mask-1 = <1 1>;
>>>> +                    mclk-fs = <256>;
>>>> +
>>>> +                    codec-0 {
>>>> +                            sound-dai = <&tohdmitx TOHDMITX_I2S_IN_B>;
>>>> +                    };
>>>> +
>>>> +                    codec-1 {
>>>> +                            sound-dai = <&toacodec TOACODEC_IN_B>;
>>>> +                    };
>>>> +            };
>>>> +
>>>> +            dai-link-8 {
>>>> +                    sound-dai = <&tdmif_c>;
>>>> +                    dai-format = "i2s";
>>>> +                    dai-tdm-slot-tx-mask-0 = <1 1>;
>>>> +                    dai-tdm-slot-tx-mask-1 = <1 1>;
>>>> +                    dai-tdm-slot-tx-mask-2 = <1 1>;
>>>> +                    dai-tdm-slot-tx-mask-3 = <1 1>;
>>>> +                    mclk-fs = <256>;
>>>> +
>>>> +                    codec-0 {
>>>> +                            sound-dai = <&tohdmitx TOHDMITX_I2S_IN_C>;
>>>> +                    };
>>>> +
>>>> +                    codec-1 {
>>>> +                            sound-dai = <&toacodec TOACODEC_IN_C>;
>>>> +                    };
>>>> +            };
>>>> +
>>>> +            dai-link-9 {
>>>> +                    sound-dai = <&tohdmitx TOHDMITX_I2S_OUT>;
>>>> +
>>>> +                    codec {
>>>> +                            sound-dai = <&hdmi_tx>;
>>>> +                    };
>>>> +            };
>>>> +
>>>> +            dai-link-10 {
>>>> +                    sound-dai = <&toacodec TOACODEC_OUT>;
>>>> +
>>>> +                    codec {
>>>> +                            sound-dai = <&acodec>;
>>>> +                    };
>>>> +            };
>>>> +    };
>>>> +};
>>>> +
>>>> +&acodec {
>>>> +    status = "okay";
>>>> +    AVDD-supply = <&vddio_ao18>;
>>>> +};
>>>> +
>>>> +&arb {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&cecb_AO {
>>>> +    status = "okay";
>>>> +    pinctrl-0 = <&cec_ao_b_h_pins>;
>>>> +    pinctrl-names = "default";
>>>> +    hdmi-phandle = <&hdmi_tx>;
>>>> +};
>>>> +
>>>> +&clkc_audio {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&cvbs_vdac_port {
>>>> +    cvbs_vdac_out: endpoint {
>>>> +            remote-endpoint = <&cvbs_connector_in>;
>>>> +    };
>>>> +};
>>>> +
>>>> +&ethmac {
>>>> +    pinctrl-0 = <&eth_pins>, <&eth_rgmii_pins>, <&eth_phy_irq_pins>;
>>>> +    pinctrl-names = "default";
>>>> +    status = "okay";
>>>> +    phy-mode = "rgmii";
>>>> +    phy-handle = <&external_phy>;
>>>> +    amlogic,tx-delay-ns = <2>;
>>>> +};
>>>> +
>>>> +&ext_mdio {
>>>> +    external_phy: ethernet-phy@0 {
>>>> +            /* Realtek RTL8211F (0x001cc916) */
>>>> +            reg = <0>;
>>>> +            max-speed = <1000>;
>>>> +
>>>> +            reset-assert-us = <100000>;
>>>> +            reset-deassert-us = <100000>;
>>>> +            reset-gpios = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
>>>> +
>>>> +            interrupt-parent = <&gpio_intc>;
>>>> +            /* MAC_INTR on GPIOZ_14 */
>>>> +            interrupts = <26 IRQ_TYPE_LEVEL_LOW>;
>>>> +    };
>>>> +};
>>>> +
>>>> +&frddr_a {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&frddr_b {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&frddr_c {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&hdmi_tx {
>>>> +    status = "okay";
>>>> +    pinctrl-0 = <&hdmitx_hpd_pins>, <&hdmitx_ddc_pins>;
>>>> +    pinctrl-names = "default";
>>>> +    hdmi-supply = <&vcc_5v>;
>>>> +};
>>>> +
>>>> +&hdmi_tx_tmds_port {
>>>> +    hdmi_tx_tmds_out: endpoint {
>>>> +            remote-endpoint = <&hdmi_connector_in>;
>>>> +    };
>>>> +};
>>>> +
>>>> +&ir {
>>>> +    status = "okay";
>>>> +    pinctrl-0 = <&remote_input_ao_pins>;
>>>> +    pinctrl-names = "default";
>>>> +};
>>>> +
>>>> +&npu {
>>>> +    status = "okay";
>>>> +};
>>> 
>>> Are you sure you want this enabled ?
>>> AFAIK if etnaviv is enabled, it will be used first as a render node by mesa and fail.
>>> 
>> 
>> AFAIK, it is enabled in the pre-flashed bootloader that is running
>> debian, fedora and opensuse ... but etnaviv is blacklisted there
>> 
>> It should indeed be removed.
>> 
>> 
> 
> Yes, let's remove this until there are future developments in this
> area. We will add it to our tooling to enable separately.
> 
>>>> +
>>>> +&periphs_pinctrl {
>>>> +    spi_cs_disable_pins: spi-cs-disable {
>>>> +            mux {
>>>> +                    groups = "BOOT_14";
>>>> +                    function = "gpio_periphs";
>>>> +                    bias-disable;
>>>> +                    output-high;
>>>> +            };
>>>> +    };
>>>> +
>>>> +    eth_phy_irq_pins: eth-phy-irq {
>>>> +            mux {
>>>> +                    groups = "GPIOZ_14";
>>>> +                    function = "gpio_periphs";
>>>> +                    bias-pull-up;
>>>> +                    output-disable;
>>>> +            };
>>>> +    };
>>>> +};
>>>> +
>>>> +&pwm_AO_cd {
>>>> +    status = "okay";
>>>> +    pinctrl-0 = <&pwm_ao_d_e_pins>;
>>>> +    pinctrl-names = "default";
>>>> +    clocks = <&xtal>;
>>>> +    clock-names = "clkin1";
>>>> +};
>>>> +
>>>> +&pwm_ab {
>>>> +    status = "okay";
>>>> +    pinctrl-0 = <&pwm_b_x7_pins>;
>>>> +    pinctrl-names = "default";
>>>> +    clocks = <&xtal>;
>>>> +    clock-names = "clkin1";
>>>> +};
>>>> +
>>>> +&pwm_cd {
>>>> +    status = "okay";
>>>> +    pinctrl-0 = <&pwm_d_x3_pins>;
>>>> +    pinctrl-names = "default";
>>>> +    clocks = <&xtal>;
>>>> +    clock-names = "clkin1";
>>>> +};
>>>> +
>>>> +&saradc {
>>>> +    status = "okay";
>>>> +    vref-supply = <&vddio_ao18>;
>>>> +};
>>>> +
>>>> +/* SD card */
>>>> +&sd_emmc_b {
>>>> +    status = "okay";
>>>> +    pinctrl-0 = <&sdcard_c_pins>;
>>>> +    pinctrl-1 = <&sdcard_clk_gate_c_pins>;
>>>> +    pinctrl-names = "default", "clk-gate";
>>>> +
>>>> +    bus-width = <4>;
>>>> +    cap-sd-highspeed;
>>>> +    sd-uhs-sdr12;
>>>> +    sd-uhs-sdr25;
>>>> +    sd-uhs-sdr50;
>>>> +    sd-uhs-sdr104;
>>>> +    max-frequency = <200000000>;
>>>> +    disable-wp;
>>>> +
>>>> +    cd-gpios = <&gpio GPIOC_6 GPIO_ACTIVE_LOW>;
>>>> +    vmmc-supply = <&vcc_card>;
>>>> +    vqmmc-supply = <&vddio_c>;
>>>> +};
>>>> +
>>>> +/*
>>>> + * EMMC_D4, EMMC_D5, EMMC_D6 and EMMC_D7 pins are shared between SPI NOR CS
>>>> + * and eMMC Data 4 to 7 pins.
>>>> + * Replace emmc_data_8b_pins to emmc_data_4b_pins from sd_emmc_c pinctrl-0,
>>>> + * and change bus-width to 4 then spifc can be enabled.
>>>> + */
>>>> +&sd_emmc_c {
>>>> +    status = "okay";
>>>> +    pinctrl-0 = <&emmc_ctrl_pins>, <&emmc_data_8b_pins>, <&emmc_ds_pins>,
>>>> +                <&spi_cs_disable_pins>;
>>>> +    pinctrl-1 = <&emmc_clk_gate_pins>;
>>>> +    pinctrl-names = "default", "clk-gate";
>>>> +
>>>> +    bus-width = <8>;
>>>> +    cap-mmc-highspeed;
>>>> +    mmc-hs200-1_8v;
>>>> +    max-frequency = <200000000>;
>>>> +    disable-wp;
>>>> +
>>>> +    mmc-pwrseq = <&emmc_pwrseq>;
>>>> +    vmmc-supply = <&vcc_3v3>;
>>>> +    vqmmc-supply = <&flash_1v8>;
>>>> +};
>>>> +
>>>> +&spifc {
>>>> +    status = "disabled";
>>>> +    pinctrl-0 = <&nor_pins>;
>>>> +    pinctrl-names = "default";
>>>> +    cs-gpios = <&gpio BOOT_14 GPIO_ACTIVE_LOW>;
>>>> +
>>>> +    w25lq128d: flash@0 {
>>>> +            compatible = "jedec,spi-nor";
>>>> +            reg = <0>;
>>>> +            #address-cells = <1>;
>>>> +            #size-cells = <1>;
>>>> +            spi-max-frequency = <80000000>;
>>>> +    };
>>>> +};
>>>> +
>>>> +&tdmif_a {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&tdmif_b {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&tdmif_c {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&tdmin_a {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&tdmin_b {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&tdmin_c {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&tdmout_a {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&tdmout_b {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&tdmout_c {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&toacodec {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&toddr_a {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&toddr_b {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&toddr_c {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&tohdmitx {
>>>> +    status = "okay";
>>>> +};
>>>> +
>>>> +&uart_AO {
>>>> +    status = "okay";
>>>> +    pinctrl-0 = <&uart_ao_a_pins>;
>>>> +    pinctrl-names = "default";
>>>> +};
>>>> +
>>>> +&usb2_phy1 {
>>>> +    phy-supply = <&dc_in>;
>>>> +};
>>>> +
>>>> +&usb3_pcie_phy {
>>>> +    #address-cells = <1>;
>>>> +    #size-cells = <0>;
>>>> +    phy-supply = <&vcc_5v>;
>>>> +
>>>> +    hub: hub@1 {
>>>> +            compatible = "usb5e3,626";
>>>> +            reg = <1>;
>>>> +            reset-gpios = <&gpio GPIOC_7 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
>>>> +    };
>>> 
>>> Not sure the PHY is the right place to put the USB HUB,
>>> and it's probable the HUB is connected to both the USB2 and USB3 lines
>> 
>> It is connected to the USB3.0 only
> 
> https://drive.google.com/file/d/17WNK9m8VZ9CfS7GMiTLdq1zHsmHR4OcV/view
> 
> USBHOST_A_DM and USBHOST_A_DP are connected to the USB 3 hub DM0 and DP0.
> Not sure if this counts?
> 
>> 
>>> so you should have both USB IDs in DT like it'd done for the Odroid-C4:
>>> 
>>> / {
>>> ...
>>>         /* USB hub supports both USB 2.0 and USB 3.0 root hub */
>>>         usb-hub {
>>>                 dr_mode = "host";
>>>                 #address-cells = <1>;
>>>                 #size-cells = <0>;
>>> 
>>>                 /* 2.0 hub on port 1 */
>>>                 hub_2_0: hub@1 {
>>>                         compatible = "usb2109,2817";
>>>                         reg = <1>;
>>>                         peer-hub = <&hub_3_0>;
>>>                         reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>                         vdd-supply = <&vcc_5v>;
>>>                 };
>>> 
>>>                 /* 3.1 hub on port 4 */
>>>                 hub_3_0: hub@2 {
>>>                         compatible = "usb2109,817";
>>>                         reg = <2>;
>>>                         peer-hub = <&hub_2_0>;
>>>                         reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>                         vdd-supply = <&vcc_5v>;
>>>                 };
>>>         };
>>> ...
>>> };
>>> 
>>> if it only has a single USB ID, then it should go under the dwc3 node.
>> 
>> The usb controller is connected to the PHY and what's coming out of the PHY
>> goes to the hub. It seems logical to hub the hub under it.
>> 
>> Why bypass the PHY ?
>> 
>>> 
>>>> +};
>>>> +
>>>> +&usb {
>>>> +    status = "okay";
>>>> +};
>>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
>>>> new file mode 100644
>>>> index 000000000000..077e7506ce4f
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
>>>> @@ -0,0 +1,89 @@
>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>> +/*
>>>> + * Copyright (c) 2023 BayLibre, SAS.
>>>> + * Author: Jerome Brunet <jbrunet@baylibre.com>
>>>> + */
>>>> +
>>>> +/dts-v1/;
>>>> +
>>>> +#include <dt-bindings/clock/g12a-clkc.h>
>>>> +#include "meson-sm1.dtsi"
>>>> +#include "meson-libretech-cottonwood.dtsi"
>>>> +
>>>> +/ {
>>>> +    compatible = "libretech,aml-s905d3-cc", "amlogic,sm1";
>>>> +    model = "Libre Computer AML-S905D3-CC Solitude";
>>>> +
>>>> +    sound {
>>>> +            model = "Solitude";
>>>> +            audio-routing = "TDMOUT_A IN 0", "FRDDR_A OUT 0",
>>>> +                            "TDMOUT_A IN 1", "FRDDR_B OUT 0",
>>>> +                            "TDMOUT_A IN 2", "FRDDR_C OUT 0",
>>>> +                            "TDM_A Playback", "TDMOUT_A OUT",
>>>> +                            "TDMOUT_B IN 0", "FRDDR_A OUT 1",
>>>> +                            "TDMOUT_B IN 1", "FRDDR_B OUT 1",
>>>> +                            "TDMOUT_B IN 2", "FRDDR_C OUT 1",
>>>> +                            "TDM_B Playback", "TDMOUT_B OUT",
>>>> +                            "TDMOUT_C IN 0", "FRDDR_A OUT 2",
>>>> +                            "TDMOUT_C IN 1", "FRDDR_B OUT 2",
>>>> +                            "TDMOUT_C IN 2", "FRDDR_C OUT 2",
>>>> +                            "TDM_C Playback", "TDMOUT_C OUT",
>>>> +                            "TDMIN_A IN 0", "TDM_A Capture",
>>>> +                            "TDMIN_B IN 0", "TDM_A Capture",
>>>> +                            "TDMIN_C IN 0", "TDM_A Capture",
>>>> +                            "TDMIN_A IN 13", "TDM_A Loopback",
>>>> +                            "TDMIN_B IN 13", "TDM_A Loopback",
>>>> +                            "TDMIN_C IN 13", "TDM_A Loopback",
>>>> +                            "TDMIN_A IN 1", "TDM_B Capture",
>>>> +                            "TDMIN_B IN 1", "TDM_B Capture",
>>>> +                            "TDMIN_C IN 1", "TDM_B Capture",
>>>> +                            "TDMIN_A IN 14", "TDM_B Loopback",
>>>> +                            "TDMIN_B IN 14", "TDM_B Loopback",
>>>> +                            "TDMIN_C IN 14", "TDM_B Loopback",
>>>> +                            "TDMIN_A IN 2", "TDM_C Capture",
>>>> +                            "TDMIN_B IN 2", "TDM_C Capture",
>>>> +                            "TDMIN_C IN 2", "TDM_C Capture",
>>>> +                            "TDMIN_A IN 15", "TDM_C Loopback",
>>>> +                            "TDMIN_B IN 15", "TDM_C Loopback",
>>>> +                            "TDMIN_C IN 15", "TDM_C Loopback",
>>>> +                            "TODDR_A IN 0", "TDMIN_A OUT",
>>>> +                            "TODDR_B IN 0", "TDMIN_A OUT",
>>>> +                            "TODDR_C IN 0", "TDMIN_A OUT",
>>>> +                            "TODDR_A IN 1", "TDMIN_B OUT",
>>>> +                            "TODDR_B IN 1", "TDMIN_B OUT",
>>>> +                            "TODDR_C IN 1", "TDMIN_B OUT",
>>>> +                            "TODDR_A IN 2", "TDMIN_C OUT",
>>>> +                            "TODDR_B IN 2", "TDMIN_C OUT",
>>>> +                            "TODDR_C IN 2", "TDMIN_C OUT",
>>>> +                            "Lineout", "ACODEC LOLP",
>>>> +                            "Lineout", "ACODEC LORP";
>>>> +    };
>>>> +};
>>>> +
>>>> +&cpu0 {
>>>> +    cpu-supply = <&vddcpu_b>;
>>>> +    operating-points-v2 = <&cpu_opp_table>;
>>>> +    clocks = <&clkc CLKID_CPU_CLK>;
>>>> +    clock-latency = <50000>;
>>>> +};
>>>> +
>>>> +&cpu1 {
>>>> +    cpu-supply = <&vddcpu_b>;
>>>> +    operating-points-v2 = <&cpu_opp_table>;
>>>> +    clocks = <&clkc CLKID_CPU1_CLK>;
>>>> +    clock-latency = <50000>;
>>>> +};
>>>> +
>>>> +&cpu2 {
>>>> +    cpu-supply = <&vddcpu_b>;
>>>> +    operating-points-v2 = <&cpu_opp_table>;
>>>> +    clocks = <&clkc CLKID_CPU2_CLK>;
>>>> +    clock-latency = <50000>;
>>>> +};
>>>> +
>>>> +&cpu3 {
>>>> +    cpu-supply = <&vddcpu_b>;
>>>> +    operating-points-v2 = <&cpu_opp_table>;
>>>> +    clocks = <&clkc CLKID_CPU3_CLK>;
>>>> +    clock-latency = <50000>;
>>>> +};
>>> 
>>> Apart that, it looks fine,
>>> 
>>> 
>>> Thanks,
>>> Neil
>> 
>> 
>> _______________________________________________
>> linux-amlogic mailing list
>> linux-amlogic@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
  

On Tue 03 Oct 2023 at 05:23, Christian Hewitt <christianshewitt@gmail.com> wrote:

>> On 3 Oct 2023, at 1:15 am, Da Xue <da@lessconfused.com> wrote:
>> 
>> On Tue, Oct 3, 2023 at 3:13 AM Jerome Brunet <jbrunet@baylibre.com> wrote:
>>> 
>>> 
>>> On Mon 02 Oct 2023 at 18:45, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>>> 
>>>> Hi,
>>>> 
>>>> On 02/10/2023 16:10, Jerome Brunet wrote:
>>>>> Add support for the Libretech cottonwood board family.
>>>>> These 2 boards are based on the same PCB, with an RPi B form factor.
>>>>> The "Alta" board uses an a311d while the "Solitude" variant uses an
>>>>> s905d3.
>>>>> Co-developed-by: Da Xue <da.xue@libretech.co>
>>>>> Signed-off-by: Da Xue <da.xue@libretech.co>
>>>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>>>> ---
>>>>>  arch/arm64/boot/dts/amlogic/Makefile          |   2 +
>>>>>  .../amlogic/meson-g12b-a311d-libretech-cc.dts | 133 ++++
>>>>>  .../amlogic/meson-libretech-cottonwood.dtsi   | 610 ++++++++++++++++++
>>>>>  .../amlogic/meson-sm1-s905d3-libretech-cc.dts |  89 +++
>>>>>  4 files changed, 834 insertions(+)
>>>>>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
>>>>>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
>>>>>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
>>>>> diff --git a/arch/arm64/boot/dts/amlogic/Makefile
>>>>> b/arch/arm64/boot/dts/amlogic/Makefile
>>>>> index 4ce401d17b63..cc8b34bd583d 100644
>>>>> --- a/arch/arm64/boot/dts/amlogic/Makefile
>>>>> +++ b/arch/arm64/boot/dts/amlogic/Makefile
>>>>> @@ -18,6 +18,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-g12b-bananapi-cm4-cm4io.dtb
>>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gsking-x.dtb
>>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking-pro.dtb
>>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking.dtb
>>>>> +dtb-$(CONFIG_ARCH_MESON) += meson-g12b-a311d-libretech-cc.dtb
>>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-go-ultra.dtb
>>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2-plus.dtb
>>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2.dtb
>>>>> @@ -73,6 +74,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-sm1-bananapi-m2-pro.dtb
>>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-sm1-bananapi-m5.dtb
>>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-sm1-h96-max.dtb
>>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-sm1-khadas-vim3l.dtb
>>>>> +dtb-$(CONFIG_ARCH_MESON) += meson-sm1-s905d3-libretech-cc.dtb
>>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-c4.dtb
>>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-hc4.dtb
>>>>>  dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb
>>>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
>>>>> new file mode 100644
>>>>> index 000000000000..fc890e235dbd
>>>>> --- /dev/null
>>>>> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
>>>>> @@ -0,0 +1,133 @@
>>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>>> +/*
>>>>> + * Copyright (c) 2023 BayLibre, SAS.
>>>>> + * Author: Jerome Brunet <jbrunet@baylibre.com>
>>>>> + */
>>>>> +
>>>>> +/dts-v1/;
>>>>> +
>>>>> +#include <dt-bindings/clock/g12a-clkc.h>
>>>>> +#include "meson-g12b-a311d.dtsi"
>>>>> +#include "meson-libretech-cottonwood.dtsi"
>>>>> +
>>>>> +/ {
>>>>> +    compatible = "libretech,aml-a311d-cc", "amlogic,a311d", "amlogic,g12b";
>>>>> +    model = "Libre Computer AML-A311D-CC Alta";
>>>>> +
>>>>> +    vddcpu_a: regulator-vddcpu-a {
>>>>> +            compatible = "pwm-regulator";
>>>>> +            regulator-name = "VDDCPU_A";
>>>>> +            regulator-min-microvolt = <730000>;
>>>>> +            regulator-max-microvolt = <1011000>;
>>>>> +            regulator-boot-on;
>>>>> +            regulator-always-on;
>>>>> +            pwm-supply = <&dc_in>;
>>>>> +            pwms = <&pwm_ab 0 1250 0>;
>>>>> +            pwm-dutycycle-range = <100 0>;
>>>>> +    };
>>>>> +
>>>>> +    sound {
>>>>> +            model = "Alta";
>>>> 
>>>> I think those sound model properties should be coherent with the
>>>> other Libre Computer boards:
>>>> arch/arm64/boot/dts/amlogic/meson-gx-libretech-pc.dtsi:         model = "LIBRETECH-PC";
>>>> arch/arm64/boot/dts/amlogic/meson-gxl-s805x-libretech-ac.dts:           model = "LIBRETECH-AC";
>>>> arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc-v2.dts:                model = "LIBRETECH-CC-V2";
>>>> arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts:           model = "LIBRETECH-CC";
>>> 
>>> "LIBRETECH-CC-" leave very little room to play with
>>> That's not really something that could have been anticipated 5+ years ago
>>> 
>> 
>> I think the formal model name is best, maybe with LC prefix.
>> eg. LC-AML-A311D-CC and LC-AML-S905D3-CC
>
> The first is valid. The second will be truncated to LC-AML-S905D3-C by the
> alsa 15-character naming limit (mentioned below).
>
> So name/rename them to:
>
> LC-XXXXXXXXXXXX <= MAX SIZE (15 Chars)
> LC-LEPOTATO
> LC-LEPOTATO-V2
> LC-LAFRITE
> LC-TARTIFLETTE
> LC-ALTA
> LC-SOLITUDE
>
> Personally I think the plain codenames (no "LC-“) work best as they are all
> distinctive. Whenever I see lists of the official board names they look/read
> the same at first glance and then I have to spot-the-difference to tell them
> apart.
>
> At the moment AFAIK these names are just cosmetic as there’s no Amlogic alsa
> ucm confs using board model (only downstream confs based on driver name). So
> IMHO rework the names now before the confs go upstream.
>

No they are not cosmetic. It can be used to match the card.
Changing old names may break userspace.

> CH.
>
>> https://hub.libre.computer/t/libre-computer-board-naming-and-conventions/100
>> 
>>>> 
>>>> It's ok to change the scheme since it's tried to keep the name under the 15 characters limit,
>>>> will the next board keep this naming ?
>>> 
>>> I don't know what the next board will be so I can hardly make any prediction
>>> I'm open to suggestion if you prefer something else
>>> 
>>>> 
>>>> 
>>>>> +            audio-routing = "TDMOUT_A IN 0", "FRDDR_A OUT 0",
>>>>> +                            "TDMOUT_A IN 1", "FRDDR_B OUT 0",
>>>>> +                            "TDMOUT_A IN 2", "FRDDR_C OUT 0",
>>>>> +                            "TDM_A Playback", "TDMOUT_A OUT",
>>>>> +                            "TDMOUT_B IN 0", "FRDDR_A OUT 1",
>>>>> +                            "TDMOUT_B IN 1", "FRDDR_B OUT 1",
>>>>> +                            "TDMOUT_B IN 2", "FRDDR_C OUT 1",
>>>>> +                            "TDM_B Playback", "TDMOUT_B OUT",
>>>>> +                            "TDMOUT_C IN 0", "FRDDR_A OUT 2",
>>>>> +                            "TDMOUT_C IN 1", "FRDDR_B OUT 2",
>>>>> +                            "TDMOUT_C IN 2", "FRDDR_C OUT 2",
>>>>> +                            "TDM_C Playback", "TDMOUT_C OUT",
>>>>> +                            "TDMIN_A IN 0", "TDM_A Capture",
>>>>> +                            "TDMIN_B IN 0", "TDM_A Capture",
>>>>> +                            "TDMIN_C IN 0", "TDM_A Capture",
>>>>> +                            "TDMIN_A IN 3", "TDM_A Loopback",
>>>>> +                            "TDMIN_B IN 3", "TDM_A Loopback",
>>>>> +                            "TDMIN_C IN 3", "TDM_A Loopback",
>>>>> +                            "TDMIN_A IN 1", "TDM_B Capture",
>>>>> +                            "TDMIN_B IN 1", "TDM_B Capture",
>>>>> +                            "TDMIN_C IN 1", "TDM_B Capture",
>>>>> +                            "TDMIN_A IN 4", "TDM_B Loopback",
>>>>> +                            "TDMIN_B IN 4", "TDM_B Loopback",
>>>>> +                            "TDMIN_C IN 4", "TDM_B Loopback",
>>>>> +                            "TDMIN_A IN 2", "TDM_C Capture",
>>>>> +                            "TDMIN_B IN 2", "TDM_C Capture",
>>>>> +                            "TDMIN_C IN 2", "TDM_C Capture",
>>>>> +                            "TDMIN_A IN 5", "TDM_C Loopback",
>>>>> +                            "TDMIN_B IN 5", "TDM_C Loopback",
>>>>> +                            "TDMIN_C IN 5", "TDM_C Loopback",
>>>>> +                            "TODDR_A IN 0", "TDMIN_A OUT",
>>>>> +                            "TODDR_B IN 0", "TDMIN_A OUT",
>>>>> +                            "TODDR_C IN 0", "TDMIN_A OUT",
>>>>> +                            "TODDR_A IN 1", "TDMIN_B OUT",
>>>>> +                            "TODDR_B IN 1", "TDMIN_B OUT",
>>>>> +                            "TODDR_C IN 1", "TDMIN_B OUT",
>>>>> +                            "TODDR_A IN 2", "TDMIN_C OUT",
>>>>> +                            "TODDR_B IN 2", "TDMIN_C OUT",
>>>>> +                            "TODDR_C IN 2", "TDMIN_C OUT",
>>>>> +                            "Lineout", "ACODEC LOLP",
>>>>> +                            "Lineout", "ACODEC LORP";
>>>>> +    };
>>>>> +};
>>>>> +
>>>>> +&cpu_opp_table_0 {
>>>>> +            opp-1800000000 {
>>>>> +                    opp-hz = /bits/ 64 <1800000000>;
>>>>> +                    opp-microvolt = <1001000>;
>>>>> +            };
>> 
>> This seems to match
>> https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/amlogic/meson-g12b-a311d.dtsi#L44
>> 
>>>>> +
>>>>> +            opp-2016000000 {
>>>>> +                    opp-hz = /bits/ 64 <2016000000>;
>>>>> +                    opp-microvolt = <1011000>;
>>>>> +            };
>> 
>> We run manufacturing verification at this speed but I don't think we
>> should include this in upstream.
>> 
>>>>> +};
>>>>> +
>>>>> +&cpu0 {
>>>>> +    cpu-supply = <&vddcpu_b>;
>>>>> +    operating-points-v2 = <&cpu_opp_table_0>;
>>>>> +    clocks = <&clkc CLKID_CPU_CLK>;
>>>>> +    clock-latency = <50000>;
>>>>> +};
>>>>> +
>>>>> +&cpu1 {
>>>>> +    cpu-supply = <&vddcpu_b>;
>>>>> +    operating-points-v2 = <&cpu_opp_table_0>;
>>>>> +    clocks = <&clkc CLKID_CPU_CLK>;
>>>>> +    clock-latency = <50000>;
>>>>> +};
>>>>> +
>>>>> +&cpu100 {
>>>>> +    cpu-supply = <&vddcpu_a>;
>>>>> +    operating-points-v2 = <&cpub_opp_table_1>;
>>>>> +    clocks = <&clkc CLKID_CPUB_CLK>;
>>>>> +    clock-latency = <50000>;
>>>>> +};
>>>>> +
>>>>> +&cpu101 {
>>>>> +    cpu-supply = <&vddcpu_a>;
>>>>> +    operating-points-v2 = <&cpub_opp_table_1>;
>>>>> +    clocks = <&clkc CLKID_CPUB_CLK>;
>>>>> +    clock-latency = <50000>;
>>>>> +};
>>>>> +
>>>>> +&cpu102 {
>>>>> +    cpu-supply = <&vddcpu_a>;
>>>>> +    operating-points-v2 = <&cpub_opp_table_1>;
>>>>> +    clocks = <&clkc CLKID_CPUB_CLK>;
>>>>> +    clock-latency = <50000>;
>>>>> +};
>>>>> +
>>>>> +&cpu103 {
>>>>> +    cpu-supply = <&vddcpu_a>;
>>>>> +    operating-points-v2 = <&cpub_opp_table_1>;
>>>>> +    clocks = <&clkc CLKID_CPUB_CLK>;
>>>>> +    clock-latency = <50000>;
>>>>> +};
>>>>> +
>>>>> +&pwm_ab {
>>>>> +    pinctrl-0 = <&pwm_a_e_pins>, <&pwm_b_x7_pins>;
>>>>> +    clocks = <&xtal>, <&xtal>;
>>>>> +    clock-names = "clkin0", "clkin1";
>>>>> +};
>>>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi b/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
>>>>> new file mode 100644
>>>>> index 000000000000..a7fc8963ff54
>>>>> --- /dev/null
>>>>> +++ b/arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
>>>>> @@ -0,0 +1,610 @@
>>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>>> +/*
>>>>> + * Copyright (c) 2023 BayLibre, SAS.
>>>>> + * Author: Jerome Brunet <jbrunet@baylibre.com>
>>>>> + */
>>>>> +
>>>>> +#include <dt-bindings/clock/g12a-clkc.h>
>>>>> +#include <dt-bindings/input/input.h>
>>>>> +#include <dt-bindings/leds/common.h>
>>>>> +#include <dt-bindings/gpio/gpio.h>
>>>>> +#include <dt-bindings/gpio/meson-g12a-gpio.h>
>>>>> +#include <dt-bindings/sound/meson-g12a-toacodec.h>
>>>>> +#include <dt-bindings/sound/meson-g12a-tohdmitx.h>
>>>>> +
>>>>> +/ {
>>>>> +    aliases {
>>>>> +            serial0 = &uart_AO;
>>>>> +            ethernet0 = &ethmac;
>>>>> +            spi0 = &spifc;
>>>>> +    };
>>>>> +
>>>>> +    memory@0 {
>>>>> +            device_type = "memory";
>>>>> +            reg = <0x0 0x0 0x0 0x80000000>;
>>>>> +    };
>>>>> +
>>>>> +    chosen {
>>>>> +            stdout-path = "serial0:115200n8";
>>>>> +    };
>>>>> +
>>>>> +    dioo2133: audio-amplifier-0 {
>>>>> +            compatible = "simple-audio-amplifier";
>>>>> +            enable-gpios = <&gpio GPIOX_0 GPIO_ACTIVE_HIGH>;
>>>>> +            VCC-supply = <&vcc_5v>;
>>>>> +            sound-name-prefix = "10U2";
>>>>> +    };
>>>>> +
>>>>> +    /* TOFIX: handle CVBS_DET on SARADC channel 0 */
>>>>> +    cvbs-connector {
>>>>> +            compatible = "composite-video-connector";
>>>>> +
>>>>> +            port {
>>>>> +                    cvbs_connector_in: endpoint {
>>>>> +                            remote-endpoint = <&cvbs_vdac_out>;
>>>>> +                    };
>>>>> +            };
>>>>> +    };
>>>>> +
>>>>> +    emmc_pwrseq: emmc-pwrseq {
>>>>> +            compatible = "mmc-pwrseq-emmc";
>>>>> +            reset-gpios = <&gpio BOOT_12 GPIO_ACTIVE_LOW>;
>>>>> +    };
>>>>> +
>>>>> +    hdmi-connector {
>>>>> +            compatible = "hdmi-connector";
>>>>> +            type = "a";
>>>>> +
>>>>> +            port {
>>>>> +                    hdmi_connector_in: endpoint {
>>>>> +                            remote-endpoint = <&hdmi_tx_tmds_out>;
>>>>> +                    };
>>>>> +            };
>>>>> +    };
>>>>> +
>>>>> +    leds-pwm {
>>>>> +            compatible = "pwm-leds";
>>>>> +
>>>>> +            led-green {
>>>>> +                    color = <LED_COLOR_ID_GREEN>;
>>>>> +                    function = LED_FUNCTION_STATUS;
>>>>> +                    linux,default-trigger = "default-on";
>>>>> +                    panic-indicator;
>>>>> +                    max-brightness = <255>;
>>>>> +                    pwms = <&pwm_cd 1 1250 0>;
>>>>> +                    active-low;
>>>>> +            };
>>>>> +
>>>>> +            led-blue {
>>>>> +                    color = <LED_COLOR_ID_BLUE>;
>>>>> +                    function = LED_FUNCTION_ACTIVITY;
>>>>> +                    linux,default-trigger = "activity";
>>>>> +                    max-brightness = <255>;
>>>>> +                    pwms = <&pwm_ab 1 1250 0>;
>>>>> +                    active-low;
>>>>> +            };
>>>>> +    };
>>>>> +
>>>>> +    leds-gpio {
>>>>> +            compatible = "gpio-leds";
>>>>> +
>>>>> +            led-orange {
>>>>> +                    color = <LED_COLOR_ID_AMBER>;
>> 
>> Should this be LED_COLOR_ID_ORANGE?
>> 
>>>>> +                    function = LED_FUNCTION_STANDBY;
>>>>> +                    gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
>>>>> +            };
>>>>> +    };
>>>>> +
>>>>> +    dc_in: regulator-dc-in {
>>>>> +            compatible = "regulator-fixed";
>>>>> +            regulator-name = "5V_IN";
>>>>> +            regulator-min-microvolt = <5000000>;
>>>>> +            regulator-max-microvolt = <5000000>;
>>>>> +            regulator-always-on;
>>>>> +    };
>>>>> +
>>>>> +    flash_1v8: regulator-flash-1v8 {
>>>>> +            compatible = "regulator-fixed";
>>>>> +            regulator-name = "FLASH_1V8";
>>>>> +            regulator-min-microvolt = <1800000>;
>>>>> +            regulator-max-microvolt = <1800000>;
>>>>> +            regulator-always-on;
>>>>> +            vin-supply = <&vcc_3v3>;
>>>>> +    };
>>>>> +
>>>>> +    vcc_card: regulator-vcc-card {
>>>>> +            compatible = "regulator-fixed";
>>>>> +            regulator-name = "VCC_CARD";
>>>>> +            regulator-min-microvolt = <3300000>;
>>>>> +            regulator-max-microvolt = <3300000>;
>>>>> +            vin-supply = <&vddao_3v3>;
>>>>> +            gpio = <&gpio GPIOX_2 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>>>>> +            enable-active-high;
>>>>> +            gpio-open-drain;
>>>>> +    };
>>>>> +
>>>>> +    vcc_3v3: regulator-vcc-3v3 {
>>>>> +            compatible = "regulator-fixed";
>>>>> +            regulator-name = "VCC_3V3";
>>>>> +            regulator-min-microvolt = <3300000>;
>>>>> +            regulator-max-microvolt = <3300000>;
>>>>> +            regulator-always-on;
>>>>> +            vin-supply = <&vddao_3v3>;
>>>>> +
>>>>> +            /* FIXME: controlled by TEST_N */
>>>>> +    };
>>>>> +
>>>>> +    vcc_5v: regulator-vcc-5v {
>>>>> +            compatible = "regulator-fixed";
>>>>> +            regulator-name = "VCC_5V";
>>>>> +            regulator-min-microvolt = <5000000>;
>>>>> +            regulator-max-microvolt = <5000000>;
>>>>> +            regulator-always-on;
>>>>> +            vin-supply = <&dc_in>;
>>>>> +            gpio = <&gpio GPIOH_8 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>>>>> +            enable-active-high;
>>>>> +            gpio-open-drain;
>>>>> +    };
>>>>> +
>>>>> +    vddao_3v3: regulator-vddao_3v3 {
>>>>> +            compatible = "regulator-fixed";
>>>>> +            regulator-name = "VDDAO_3V3";
>>>>> +            regulator-min-microvolt = <3300000>;
>>>>> +            regulator-max-microvolt = <3300000>;
>>>>> +            regulator-always-on;
>>>>> +            vin-supply = <&dc_in>;
>>>>> +    };
>>>>> +
>>>>> +    vddcpu_b: regulator-vddcpu-b {
>>>>> +            compatible = "pwm-regulator";
>>>>> +            regulator-name = "VDDCPU_B";
>>>>> +            regulator-min-microvolt = <730000>;
>>>>> +            regulator-max-microvolt = <1011000>;
>>>>> +            regulator-boot-on;
>>>>> +            regulator-always-on;
>>>>> +            pwm-supply = <&dc_in>;
>>>>> +            pwms = <&pwm_AO_cd 1 1250 0>;
>>>>> +            pwm-dutycycle-range = <100 0>;
>>>>> +    };
>>>>> +
>>>>> +    vddio_ao18: regulator-vddio_ao18 {
>>>>> +            compatible = "regulator-fixed";
>>>>> +            regulator-name = "VDDIO_AO18";
>>>>> +            regulator-min-microvolt = <1800000>;
>>>>> +            regulator-max-microvolt = <1800000>;
>>>>> +            regulator-always-on;
>>>>> +            vin-supply = <&vddao_3v3>;
>>>>> +    };
>>>>> +
>>>>> +    vddio_c: regulator-vddio_c {
>>>>> +            compatible = "regulator-gpio";
>>>>> +            regulator-name = "VDDIO_C";
>>>>> +            regulator-min-microvolt = <1800000>;
>>>>> +            regulator-max-microvolt = <3300000>;
>>>>> +            regulator-settling-time-up-us = <200>;
>>>>> +            regulator-settling-time-down-us = <50000>;
>>>>> +            vin-supply = <&vddao_3v3>;
>>>>> +            gpios = <&gpio GPIOX_4 GPIO_ACTIVE_HIGH>;
>>>>> +            states = <3300000 0>,
>>>>> +                     <1800000 1>;
>>>>> +    };
>>>>> +
>>>>> +    sound {
>>>>> +            compatible = "amlogic,axg-sound-card";
>>>>> +            audio-widgets = "Line", "Lineout";
>>>>> +            audio-aux-devs = <&tdmout_a>, <&tdmout_b>, <&tdmout_c>,
>>>>> +                             <&tdmin_a>, <&tdmin_b>, <&tdmin_c>,
>>>>> +                             <&dioo2133>;
>>>>> +
>>>>> +            assigned-clocks = <&clkc CLKID_MPLL2>,
>>>>> +                              <&clkc CLKID_MPLL0>,
>>>>> +                              <&clkc CLKID_MPLL1>;
>>>>> +            assigned-clock-parents = <0>, <0>, <0>;
>>>>> +            assigned-clock-rates = <294912000>,
>>>>> +                                   <270950400>,
>>>>> +                                   <393216000>;
>>>>> +
>>>>> +            dai-link-0 {
>>>>> +                    sound-dai = <&frddr_a>;
>>>>> +            };
>>>>> +
>>>>> +            dai-link-1 {
>>>>> +                    sound-dai = <&frddr_b>;
>>>>> +            };
>>>>> +
>>>>> +            dai-link-2 {
>>>>> +                    sound-dai = <&frddr_c>;
>>>>> +            };
>>>>> +
>>>>> +            dai-link-3 {
>>>>> +                    sound-dai = <&toddr_a>;
>>>>> +            };
>>>>> +
>>>>> +            dai-link-4 {
>>>>> +                    sound-dai = <&toddr_b>;
>>>>> +            };
>>>>> +
>>>>> +            dai-link-5 {
>>>>> +                    sound-dai = <&toddr_c>;
>>>>> +            };
>>>>> +
>>>>> +            /*
>>>>> +             * Audio setup: The 40 pins header provides access to 2 TDMs,
>>>>> +             * SPDIF In/Out and PDM inputs.
>>>>> +             * - TDM A: 2 lanes
>>>>> +             *    D0:    40/X9
>>>>> +             *    D1:    38/X8
>>>>> +             *    BCLK:  12/X11
>>>>> +             *    FS:    35/X10
>>>>> +             * - TDM B: 4 lanes
>>>>> +             *    D0:    37/A3
>>>>> +             *    D1:    16/A4
>>>>> +             *    D2:    18/A5 or 7/AO6
>>>>> +             *    D3:    22/A6 or 21/H5
>>>>> +             *    BCLK:  29/A1 or 8/AO8
>>>>> +             *    FS:    31/A2 or 11/AO7
>>>>> +             * - 2 Master Clocks:
>>>>> +             *    MCLK0: 15/A0 or 10/AO9
>>>>> +             *    MCLK1: 33/X15
>>>>> +             * - SPDIF:
>>>>> +             *    OUT:   32/A11
>>>>> +             *    IN:    21/H5
>>>>> +             * - PDM Input:
>>>>> +             *    DO:    13/A8
>>>>> +             *    D1:    26/A9
>>>>> +             *    D2:    22/A6
>>>>> +             *    D3:    18/A5
>>>>> +             *    DCLK:  36/A7
>>>>> +             *
>>>>> +             * TDM C is not usable on the 40 pins connector so it is
>>>>> +             * setup for the HDMI 4 lanes i2s.
>>>>> +             *
>>>>> +             * No pinctrl is enabled by default to preserve the
>>>>> +             * genericity of the 40 pins header. Many configurations are
>>>>> +             * possible based on the desired use case. Please adjust TDM
>>>>> +             * masks, clock setups and pinctrl accordingly.
>>>>> +             */
>>>>> +
>>>>> +            dai-link-6 {
>>>>> +                    sound-dai = <&tdmif_a>;
>>>>> +                    dai-format = "dsp_a";
>>>>> +                    dai-tdm-slot-tx-mask-0 = <1 1>;
>>>>> +                    mclk-fs = <256>;
>>>>> +
>>>>> +                    codec-0 {
>>>>> +                            sound-dai = <&tohdmitx TOHDMITX_I2S_IN_A>;
>>>>> +                    };
>>>>> +
>>>>> +                    codec-1 {
>>>>> +                            sound-dai = <&toacodec TOACODEC_IN_A>;
>>>>> +                    };
>>>>> +            };
>>>>> +
>>>>> +            dai-link-7 {
>>>>> +                    sound-dai = <&tdmif_b>;
>>>>> +                    dai-format = "i2s";
>>>>> +                    dai-tdm-slot-tx-mask-0 = <1 1>;
>>>>> +                    dai-tdm-slot-rx-mask-1 = <1 1>;
>>>>> +                    mclk-fs = <256>;
>>>>> +
>>>>> +                    codec-0 {
>>>>> +                            sound-dai = <&tohdmitx TOHDMITX_I2S_IN_B>;
>>>>> +                    };
>>>>> +
>>>>> +                    codec-1 {
>>>>> +                            sound-dai = <&toacodec TOACODEC_IN_B>;
>>>>> +                    };
>>>>> +            };
>>>>> +
>>>>> +            dai-link-8 {
>>>>> +                    sound-dai = <&tdmif_c>;
>>>>> +                    dai-format = "i2s";
>>>>> +                    dai-tdm-slot-tx-mask-0 = <1 1>;
>>>>> +                    dai-tdm-slot-tx-mask-1 = <1 1>;
>>>>> +                    dai-tdm-slot-tx-mask-2 = <1 1>;
>>>>> +                    dai-tdm-slot-tx-mask-3 = <1 1>;
>>>>> +                    mclk-fs = <256>;
>>>>> +
>>>>> +                    codec-0 {
>>>>> +                            sound-dai = <&tohdmitx TOHDMITX_I2S_IN_C>;
>>>>> +                    };
>>>>> +
>>>>> +                    codec-1 {
>>>>> +                            sound-dai = <&toacodec TOACODEC_IN_C>;
>>>>> +                    };
>>>>> +            };
>>>>> +
>>>>> +            dai-link-9 {
>>>>> +                    sound-dai = <&tohdmitx TOHDMITX_I2S_OUT>;
>>>>> +
>>>>> +                    codec {
>>>>> +                            sound-dai = <&hdmi_tx>;
>>>>> +                    };
>>>>> +            };
>>>>> +
>>>>> +            dai-link-10 {
>>>>> +                    sound-dai = <&toacodec TOACODEC_OUT>;
>>>>> +
>>>>> +                    codec {
>>>>> +                            sound-dai = <&acodec>;
>>>>> +                    };
>>>>> +            };
>>>>> +    };
>>>>> +};
>>>>> +
>>>>> +&acodec {
>>>>> +    status = "okay";
>>>>> +    AVDD-supply = <&vddio_ao18>;
>>>>> +};
>>>>> +
>>>>> +&arb {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&cecb_AO {
>>>>> +    status = "okay";
>>>>> +    pinctrl-0 = <&cec_ao_b_h_pins>;
>>>>> +    pinctrl-names = "default";
>>>>> +    hdmi-phandle = <&hdmi_tx>;
>>>>> +};
>>>>> +
>>>>> +&clkc_audio {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&cvbs_vdac_port {
>>>>> +    cvbs_vdac_out: endpoint {
>>>>> +            remote-endpoint = <&cvbs_connector_in>;
>>>>> +    };
>>>>> +};
>>>>> +
>>>>> +&ethmac {
>>>>> +    pinctrl-0 = <&eth_pins>, <&eth_rgmii_pins>, <&eth_phy_irq_pins>;
>>>>> +    pinctrl-names = "default";
>>>>> +    status = "okay";
>>>>> +    phy-mode = "rgmii";
>>>>> +    phy-handle = <&external_phy>;
>>>>> +    amlogic,tx-delay-ns = <2>;
>>>>> +};
>>>>> +
>>>>> +&ext_mdio {
>>>>> +    external_phy: ethernet-phy@0 {
>>>>> +            /* Realtek RTL8211F (0x001cc916) */
>>>>> +            reg = <0>;
>>>>> +            max-speed = <1000>;
>>>>> +
>>>>> +            reset-assert-us = <100000>;
>>>>> +            reset-deassert-us = <100000>;
>>>>> +            reset-gpios = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
>>>>> +
>>>>> +            interrupt-parent = <&gpio_intc>;
>>>>> +            /* MAC_INTR on GPIOZ_14 */
>>>>> +            interrupts = <26 IRQ_TYPE_LEVEL_LOW>;
>>>>> +    };
>>>>> +};
>>>>> +
>>>>> +&frddr_a {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&frddr_b {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&frddr_c {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&hdmi_tx {
>>>>> +    status = "okay";
>>>>> +    pinctrl-0 = <&hdmitx_hpd_pins>, <&hdmitx_ddc_pins>;
>>>>> +    pinctrl-names = "default";
>>>>> +    hdmi-supply = <&vcc_5v>;
>>>>> +};
>>>>> +
>>>>> +&hdmi_tx_tmds_port {
>>>>> +    hdmi_tx_tmds_out: endpoint {
>>>>> +            remote-endpoint = <&hdmi_connector_in>;
>>>>> +    };
>>>>> +};
>>>>> +
>>>>> +&ir {
>>>>> +    status = "okay";
>>>>> +    pinctrl-0 = <&remote_input_ao_pins>;
>>>>> +    pinctrl-names = "default";
>>>>> +};
>>>>> +
>>>>> +&npu {
>>>>> +    status = "okay";
>>>>> +};
>>>> 
>>>> Are you sure you want this enabled ?
>>>> AFAIK if etnaviv is enabled, it will be used first as a render node by mesa and fail.
>>>> 
>>> 
>>> AFAIK, it is enabled in the pre-flashed bootloader that is running
>>> debian, fedora and opensuse ... but etnaviv is blacklisted there
>>> 
>>> It should indeed be removed.
>>> 
>>> 
>> 
>> Yes, let's remove this until there are future developments in this
>> area. We will add it to our tooling to enable separately.
>> 
>>>>> +
>>>>> +&periphs_pinctrl {
>>>>> +    spi_cs_disable_pins: spi-cs-disable {
>>>>> +            mux {
>>>>> +                    groups = "BOOT_14";
>>>>> +                    function = "gpio_periphs";
>>>>> +                    bias-disable;
>>>>> +                    output-high;
>>>>> +            };
>>>>> +    };
>>>>> +
>>>>> +    eth_phy_irq_pins: eth-phy-irq {
>>>>> +            mux {
>>>>> +                    groups = "GPIOZ_14";
>>>>> +                    function = "gpio_periphs";
>>>>> +                    bias-pull-up;
>>>>> +                    output-disable;
>>>>> +            };
>>>>> +    };
>>>>> +};
>>>>> +
>>>>> +&pwm_AO_cd {
>>>>> +    status = "okay";
>>>>> +    pinctrl-0 = <&pwm_ao_d_e_pins>;
>>>>> +    pinctrl-names = "default";
>>>>> +    clocks = <&xtal>;
>>>>> +    clock-names = "clkin1";
>>>>> +};
>>>>> +
>>>>> +&pwm_ab {
>>>>> +    status = "okay";
>>>>> +    pinctrl-0 = <&pwm_b_x7_pins>;
>>>>> +    pinctrl-names = "default";
>>>>> +    clocks = <&xtal>;
>>>>> +    clock-names = "clkin1";
>>>>> +};
>>>>> +
>>>>> +&pwm_cd {
>>>>> +    status = "okay";
>>>>> +    pinctrl-0 = <&pwm_d_x3_pins>;
>>>>> +    pinctrl-names = "default";
>>>>> +    clocks = <&xtal>;
>>>>> +    clock-names = "clkin1";
>>>>> +};
>>>>> +
>>>>> +&saradc {
>>>>> +    status = "okay";
>>>>> +    vref-supply = <&vddio_ao18>;
>>>>> +};
>>>>> +
>>>>> +/* SD card */
>>>>> +&sd_emmc_b {
>>>>> +    status = "okay";
>>>>> +    pinctrl-0 = <&sdcard_c_pins>;
>>>>> +    pinctrl-1 = <&sdcard_clk_gate_c_pins>;
>>>>> +    pinctrl-names = "default", "clk-gate";
>>>>> +
>>>>> +    bus-width = <4>;
>>>>> +    cap-sd-highspeed;
>>>>> +    sd-uhs-sdr12;
>>>>> +    sd-uhs-sdr25;
>>>>> +    sd-uhs-sdr50;
>>>>> +    sd-uhs-sdr104;
>>>>> +    max-frequency = <200000000>;
>>>>> +    disable-wp;
>>>>> +
>>>>> +    cd-gpios = <&gpio GPIOC_6 GPIO_ACTIVE_LOW>;
>>>>> +    vmmc-supply = <&vcc_card>;
>>>>> +    vqmmc-supply = <&vddio_c>;
>>>>> +};
>>>>> +
>>>>> +/*
>>>>> + * EMMC_D4, EMMC_D5, EMMC_D6 and EMMC_D7 pins are shared between SPI NOR CS
>>>>> + * and eMMC Data 4 to 7 pins.
>>>>> + * Replace emmc_data_8b_pins to emmc_data_4b_pins from sd_emmc_c pinctrl-0,
>>>>> + * and change bus-width to 4 then spifc can be enabled.
>>>>> + */
>>>>> +&sd_emmc_c {
>>>>> +    status = "okay";
>>>>> +    pinctrl-0 = <&emmc_ctrl_pins>, <&emmc_data_8b_pins>, <&emmc_ds_pins>,
>>>>> +                <&spi_cs_disable_pins>;
>>>>> +    pinctrl-1 = <&emmc_clk_gate_pins>;
>>>>> +    pinctrl-names = "default", "clk-gate";
>>>>> +
>>>>> +    bus-width = <8>;
>>>>> +    cap-mmc-highspeed;
>>>>> +    mmc-hs200-1_8v;
>>>>> +    max-frequency = <200000000>;
>>>>> +    disable-wp;
>>>>> +
>>>>> +    mmc-pwrseq = <&emmc_pwrseq>;
>>>>> +    vmmc-supply = <&vcc_3v3>;
>>>>> +    vqmmc-supply = <&flash_1v8>;
>>>>> +};
>>>>> +
>>>>> +&spifc {
>>>>> +    status = "disabled";
>>>>> +    pinctrl-0 = <&nor_pins>;
>>>>> +    pinctrl-names = "default";
>>>>> +    cs-gpios = <&gpio BOOT_14 GPIO_ACTIVE_LOW>;
>>>>> +
>>>>> +    w25lq128d: flash@0 {
>>>>> +            compatible = "jedec,spi-nor";
>>>>> +            reg = <0>;
>>>>> +            #address-cells = <1>;
>>>>> +            #size-cells = <1>;
>>>>> +            spi-max-frequency = <80000000>;
>>>>> +    };
>>>>> +};
>>>>> +
>>>>> +&tdmif_a {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&tdmif_b {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&tdmif_c {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&tdmin_a {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&tdmin_b {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&tdmin_c {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&tdmout_a {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&tdmout_b {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&tdmout_c {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&toacodec {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&toddr_a {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&toddr_b {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&toddr_c {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&tohdmitx {
>>>>> +    status = "okay";
>>>>> +};
>>>>> +
>>>>> +&uart_AO {
>>>>> +    status = "okay";
>>>>> +    pinctrl-0 = <&uart_ao_a_pins>;
>>>>> +    pinctrl-names = "default";
>>>>> +};
>>>>> +
>>>>> +&usb2_phy1 {
>>>>> +    phy-supply = <&dc_in>;
>>>>> +};
>>>>> +
>>>>> +&usb3_pcie_phy {
>>>>> +    #address-cells = <1>;
>>>>> +    #size-cells = <0>;
>>>>> +    phy-supply = <&vcc_5v>;
>>>>> +
>>>>> +    hub: hub@1 {
>>>>> +            compatible = "usb5e3,626";
>>>>> +            reg = <1>;
>>>>> +            reset-gpios = <&gpio GPIOC_7 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
>>>>> +    };
>>>> 
>>>> Not sure the PHY is the right place to put the USB HUB,
>>>> and it's probable the HUB is connected to both the USB2 and USB3 lines
>>> 
>>> It is connected to the USB3.0 only
>> 
>> https://drive.google.com/file/d/17WNK9m8VZ9CfS7GMiTLdq1zHsmHR4OcV/view
>> 
>> USBHOST_A_DM and USBHOST_A_DP are connected to the USB 3 hub DM0 and DP0.
>> Not sure if this counts?
>> 
>>> 
>>>> so you should have both USB IDs in DT like it'd done for the Odroid-C4:
>>>> 
>>>> / {
>>>> ...
>>>>         /* USB hub supports both USB 2.0 and USB 3.0 root hub */
>>>>         usb-hub {
>>>>                 dr_mode = "host";
>>>>                 #address-cells = <1>;
>>>>                 #size-cells = <0>;
>>>> 
>>>>                 /* 2.0 hub on port 1 */
>>>>                 hub_2_0: hub@1 {
>>>>                         compatible = "usb2109,2817";
>>>>                         reg = <1>;
>>>>                         peer-hub = <&hub_3_0>;
>>>>                         reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>>                         vdd-supply = <&vcc_5v>;
>>>>                 };
>>>> 
>>>>                 /* 3.1 hub on port 4 */
>>>>                 hub_3_0: hub@2 {
>>>>                         compatible = "usb2109,817";
>>>>                         reg = <2>;
>>>>                         peer-hub = <&hub_2_0>;
>>>>                         reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>>                         vdd-supply = <&vcc_5v>;
>>>>                 };
>>>>         };
>>>> ...
>>>> };
>>>> 
>>>> if it only has a single USB ID, then it should go under the dwc3 node.
>>> 
>>> The usb controller is connected to the PHY and what's coming out of the PHY
>>> goes to the hub. It seems logical to hub the hub under it.
>>> 
>>> Why bypass the PHY ?
>>> 
>>>> 
>>>>> +};
>>>>> +
>>>>> +&usb {
>>>>> +    status = "okay";
>>>>> +};
>>>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
>>>>> new file mode 100644
>>>>> index 000000000000..077e7506ce4f
>>>>> --- /dev/null
>>>>> +++ b/arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
>>>>> @@ -0,0 +1,89 @@
>>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>>> +/*
>>>>> + * Copyright (c) 2023 BayLibre, SAS.
>>>>> + * Author: Jerome Brunet <jbrunet@baylibre.com>
>>>>> + */
>>>>> +
>>>>> +/dts-v1/;
>>>>> +
>>>>> +#include <dt-bindings/clock/g12a-clkc.h>
>>>>> +#include "meson-sm1.dtsi"
>>>>> +#include "meson-libretech-cottonwood.dtsi"
>>>>> +
>>>>> +/ {
>>>>> +    compatible = "libretech,aml-s905d3-cc", "amlogic,sm1";
>>>>> +    model = "Libre Computer AML-S905D3-CC Solitude";
>>>>> +
>>>>> +    sound {
>>>>> +            model = "Solitude";
>>>>> +            audio-routing = "TDMOUT_A IN 0", "FRDDR_A OUT 0",
>>>>> +                            "TDMOUT_A IN 1", "FRDDR_B OUT 0",
>>>>> +                            "TDMOUT_A IN 2", "FRDDR_C OUT 0",
>>>>> +                            "TDM_A Playback", "TDMOUT_A OUT",
>>>>> +                            "TDMOUT_B IN 0", "FRDDR_A OUT 1",
>>>>> +                            "TDMOUT_B IN 1", "FRDDR_B OUT 1",
>>>>> +                            "TDMOUT_B IN 2", "FRDDR_C OUT 1",
>>>>> +                            "TDM_B Playback", "TDMOUT_B OUT",
>>>>> +                            "TDMOUT_C IN 0", "FRDDR_A OUT 2",
>>>>> +                            "TDMOUT_C IN 1", "FRDDR_B OUT 2",
>>>>> +                            "TDMOUT_C IN 2", "FRDDR_C OUT 2",
>>>>> +                            "TDM_C Playback", "TDMOUT_C OUT",
>>>>> +                            "TDMIN_A IN 0", "TDM_A Capture",
>>>>> +                            "TDMIN_B IN 0", "TDM_A Capture",
>>>>> +                            "TDMIN_C IN 0", "TDM_A Capture",
>>>>> +                            "TDMIN_A IN 13", "TDM_A Loopback",
>>>>> +                            "TDMIN_B IN 13", "TDM_A Loopback",
>>>>> +                            "TDMIN_C IN 13", "TDM_A Loopback",
>>>>> +                            "TDMIN_A IN 1", "TDM_B Capture",
>>>>> +                            "TDMIN_B IN 1", "TDM_B Capture",
>>>>> +                            "TDMIN_C IN 1", "TDM_B Capture",
>>>>> +                            "TDMIN_A IN 14", "TDM_B Loopback",
>>>>> +                            "TDMIN_B IN 14", "TDM_B Loopback",
>>>>> +                            "TDMIN_C IN 14", "TDM_B Loopback",
>>>>> +                            "TDMIN_A IN 2", "TDM_C Capture",
>>>>> +                            "TDMIN_B IN 2", "TDM_C Capture",
>>>>> +                            "TDMIN_C IN 2", "TDM_C Capture",
>>>>> +                            "TDMIN_A IN 15", "TDM_C Loopback",
>>>>> +                            "TDMIN_B IN 15", "TDM_C Loopback",
>>>>> +                            "TDMIN_C IN 15", "TDM_C Loopback",
>>>>> +                            "TODDR_A IN 0", "TDMIN_A OUT",
>>>>> +                            "TODDR_B IN 0", "TDMIN_A OUT",
>>>>> +                            "TODDR_C IN 0", "TDMIN_A OUT",
>>>>> +                            "TODDR_A IN 1", "TDMIN_B OUT",
>>>>> +                            "TODDR_B IN 1", "TDMIN_B OUT",
>>>>> +                            "TODDR_C IN 1", "TDMIN_B OUT",
>>>>> +                            "TODDR_A IN 2", "TDMIN_C OUT",
>>>>> +                            "TODDR_B IN 2", "TDMIN_C OUT",
>>>>> +                            "TODDR_C IN 2", "TDMIN_C OUT",
>>>>> +                            "Lineout", "ACODEC LOLP",
>>>>> +                            "Lineout", "ACODEC LORP";
>>>>> +    };
>>>>> +};
>>>>> +
>>>>> +&cpu0 {
>>>>> +    cpu-supply = <&vddcpu_b>;
>>>>> +    operating-points-v2 = <&cpu_opp_table>;
>>>>> +    clocks = <&clkc CLKID_CPU_CLK>;
>>>>> +    clock-latency = <50000>;
>>>>> +};
>>>>> +
>>>>> +&cpu1 {
>>>>> +    cpu-supply = <&vddcpu_b>;
>>>>> +    operating-points-v2 = <&cpu_opp_table>;
>>>>> +    clocks = <&clkc CLKID_CPU1_CLK>;
>>>>> +    clock-latency = <50000>;
>>>>> +};
>>>>> +
>>>>> +&cpu2 {
>>>>> +    cpu-supply = <&vddcpu_b>;
>>>>> +    operating-points-v2 = <&cpu_opp_table>;
>>>>> +    clocks = <&clkc CLKID_CPU2_CLK>;
>>>>> +    clock-latency = <50000>;
>>>>> +};
>>>>> +
>>>>> +&cpu3 {
>>>>> +    cpu-supply = <&vddcpu_b>;
>>>>> +    operating-points-v2 = <&cpu_opp_table>;
>>>>> +    clocks = <&clkc CLKID_CPU3_CLK>;
>>>>> +    clock-latency = <50000>;
>>>>> +};
>>>> 
>>>> Apart that, it looks fine,
>>>> 
>>>> 
>>>> Thanks,
>>>> Neil
>>> 
>>> 
>>> _______________________________________________
>>> linux-amlogic mailing list
>>> linux-amlogic@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
>> 
>> _______________________________________________
>> linux-amlogic mailing list
>> linux-amlogic@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
  

On 02/10/2023 20:57, Jerome Brunet wrote:
> 
> On Mon 02 Oct 2023 at 18:45, Neil Armstrong <neil.armstrong@linaro.org> wrote:
> 

<snip>

>>> +&usb3_pcie_phy {
>>> +	#address-cells = <1>;
>>> +	#size-cells = <0>;
>>> +	phy-supply = <&vcc_5v>;
>>> +
>>> +	hub: hub@1 {
>>> +		compatible = "usb5e3,626";
>>> +		reg = <1>;
>>> +		reset-gpios = <&gpio GPIOC_7 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
>>> +	};
>>
>> Not sure the PHY is the right place to put the USB HUB,
>> and it's probable the HUB is connected to both the USB2 and USB3 lines
> 
> It is connected to the USB3.0 only
> 
>> so you should have both USB IDs in DT like it'd done for the Odroid-C4:
>>
>> / {
>> ...
>>           /* USB hub supports both USB 2.0 and USB 3.0 root hub */
>>           usb-hub {
>>                   dr_mode = "host";
>>                   #address-cells = <1>;
>>                   #size-cells = <0>;
>>
>>                   /* 2.0 hub on port 1 */
>>                   hub_2_0: hub@1 {
>>                           compatible = "usb2109,2817";
>>                           reg = <1>;
>>                           peer-hub = <&hub_3_0>;
>>                           reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>                           vdd-supply = <&vcc_5v>;
>>                   };
>>
>>                   /* 3.1 hub on port 4 */
>>                   hub_3_0: hub@2 {
>>                           compatible = "usb2109,817";
>>                           reg = <2>;
>>                           peer-hub = <&hub_2_0>;
>>                           reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>                           vdd-supply = <&vcc_5v>;
>>                   };
>>           };
>> ...
>> };
>>
>> if it only has a single USB ID, then it should go under the dwc3 node.
> 
> The usb controller is connected to the PHY and what's coming out of the PHY
> goes to the hub. It seems logical to hub the hub under it.
> 
> Why bypass the PHY ?

The USB bindings the USB devices nodes should be under the controller's node,
not the PHY, see:

Documentation/devicetree/bindings/usb/usb-hcd.yaml
...
patternProperties:
   "^.*@[0-9a-f]{1,2}$":
     description: The hard wired USB devices
     type: object
     $ref: /schemas/usb/usb-device.yaml
...
and the example.

Subnodes aren't allowed in the PHY node.

Neil

> 
>>
>>> +};
>>> +
>>> +&usb {
>>> +	status = "okay";
>>> +};

<snip>
  

On 03/10/2023 09:21, Jerome Brunet wrote:
> 
> On Tue 03 Oct 2023 at 05:23, Christian Hewitt <christianshewitt@gmail.com> wrote:
> 
>>> On 3 Oct 2023, at 1:15 am, Da Xue <da@lessconfused.com> wrote:
>>>
>>> On Tue, Oct 3, 2023 at 3:13 AM Jerome Brunet <jbrunet@baylibre.com> wrote:
>>>>
>>>>
>>>> On Mon 02 Oct 2023 at 18:45, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> On 02/10/2023 16:10, Jerome Brunet wrote:
>>>>>> Add support for the Libretech cottonwood board family.
>>>>>> These 2 boards are based on the same PCB, with an RPi B form factor.
>>>>>> The "Alta" board uses an a311d while the "Solitude" variant uses an
>>>>>> s905d3.
>>>>>> Co-developed-by: Da Xue <da.xue@libretech.co>
>>>>>> Signed-off-by: Da Xue <da.xue@libretech.co>
>>>>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>>>>> ---
>>>>>>   arch/arm64/boot/dts/amlogic/Makefile          |   2 +
>>>>>>   .../amlogic/meson-g12b-a311d-libretech-cc.dts | 133 ++++
>>>>>>   .../amlogic/meson-libretech-cottonwood.dtsi   | 610 ++++++++++++++++++
>>>>>>   .../amlogic/meson-sm1-s905d3-libretech-cc.dts |  89 +++
>>>>>>   4 files changed, 834 insertions(+)
>>>>>>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
>>>>>>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
>>>>>>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
>>>>>> diff --git a/arch/arm64/boot/dts/amlogic/Makefile
>>>>>> b/arch/arm64/boot/dts/amlogic/Makefile
>>>>>> index 4ce401d17b63..cc8b34bd583d 100644
>>>>>> --- a/arch/arm64/boot/dts/amlogic/Makefile
>>>>>> +++ b/arch/arm64/boot/dts/amlogic/Makefile
>>>>>> @@ -18,6 +18,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-g12b-bananapi-cm4-cm4io.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gsking-x.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking-pro.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking.dtb
>>>>>> +dtb-$(CONFIG_ARCH_MESON) += meson-g12b-a311d-libretech-cc.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-go-ultra.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2-plus.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2.dtb
>>>>>> @@ -73,6 +74,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-sm1-bananapi-m2-pro.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-bananapi-m5.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-h96-max.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-khadas-vim3l.dtb
>>>>>> +dtb-$(CONFIG_ARCH_MESON) += meson-sm1-s905d3-libretech-cc.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-c4.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-hc4.dtb
>>>>>>   dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb
>>>>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
>>>>>> new file mode 100644
>>>>>> index 000000000000..fc890e235dbd
>>>>>> --- /dev/null
>>>>>> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
>>>>>> @@ -0,0 +1,133 @@
>>>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>>>> +/*
>>>>>> + * Copyright (c) 2023 BayLibre, SAS.
>>>>>> + * Author: Jerome Brunet <jbrunet@baylibre.com>
>>>>>> + */
>>>>>> +
>>>>>> +/dts-v1/;
>>>>>> +
>>>>>> +#include <dt-bindings/clock/g12a-clkc.h>
>>>>>> +#include "meson-g12b-a311d.dtsi"
>>>>>> +#include "meson-libretech-cottonwood.dtsi"
>>>>>> +
>>>>>> +/ {
>>>>>> +    compatible = "libretech,aml-a311d-cc", "amlogic,a311d", "amlogic,g12b";
>>>>>> +    model = "Libre Computer AML-A311D-CC Alta";
>>>>>> +
>>>>>> +    vddcpu_a: regulator-vddcpu-a {
>>>>>> +            compatible = "pwm-regulator";
>>>>>> +            regulator-name = "VDDCPU_A";
>>>>>> +            regulator-min-microvolt = <730000>;
>>>>>> +            regulator-max-microvolt = <1011000>;
>>>>>> +            regulator-boot-on;
>>>>>> +            regulator-always-on;
>>>>>> +            pwm-supply = <&dc_in>;
>>>>>> +            pwms = <&pwm_ab 0 1250 0>;
>>>>>> +            pwm-dutycycle-range = <100 0>;
>>>>>> +    };
>>>>>> +
>>>>>> +    sound {
>>>>>> +            model = "Alta";
>>>>>
>>>>> I think those sound model properties should be coherent with the
>>>>> other Libre Computer boards:
>>>>> arch/arm64/boot/dts/amlogic/meson-gx-libretech-pc.dtsi:         model = "LIBRETECH-PC";
>>>>> arch/arm64/boot/dts/amlogic/meson-gxl-s805x-libretech-ac.dts:           model = "LIBRETECH-AC";
>>>>> arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc-v2.dts:                model = "LIBRETECH-CC-V2";
>>>>> arch/arm64/boot/dts/amlogic/meson-gxl-s905x-libretech-cc.dts:           model = "LIBRETECH-CC";
>>>>
>>>> "LIBRETECH-CC-" leave very little room to play with
>>>> That's not really something that could have been anticipated 5+ years ago
>>>>
>>>
>>> I think the formal model name is best, maybe with LC prefix.
>>> eg. LC-AML-A311D-CC and LC-AML-S905D3-CC
>>
>> The first is valid. The second will be truncated to LC-AML-S905D3-C by the
>> alsa 15-character naming limit (mentioned below).
>>
>> So name/rename them to:
>>
>> LC-XXXXXXXXXXXX <= MAX SIZE (15 Chars)
>> LC-LEPOTATO
>> LC-LEPOTATO-V2
>> LC-LAFRITE
>> LC-TARTIFLETTE
>> LC-ALTA
>> LC-SOLITUDE
>>
>> Personally I think the plain codenames (no "LC-“) work best as they are all
>> distinctive. Whenever I see lists of the official board names they look/read
>> the same at first glance and then I have to spot-the-difference to tell them
>> apart.
>>
>> At the moment AFAIK these names are just cosmetic as there’s no Amlogic alsa
>> ucm confs using board model (only downstream confs based on driver name). So
>> IMHO rework the names now before the confs go upstream.
>>
> 
> No they are not cosmetic. It can be used to match the card.
> Changing old names may break userspace.

We changed them already because there was a clash from the 15 characters max width,
but now it's different.

So I think it's time to start a new clean scheme for the future LC boards, so:
LC-ALTA
LC-SOLITUDE

seems ok for me.

Using LC-AML-A311D-CC and LC-AML-S905D3-CC goes beyond the 15 chars limit.

Neil

> 
>> CH.
>>
>>> https://hub.libre.computer/t/libre-computer-board-naming-and-conventions/100
>>>
>>>>>
>>>>> It's ok to change the scheme since it's tried to keep the name under the 15 characters limit,
>>>>> will the next board keep this naming ?
>>>>
>>>> I don't know what the next board will be so I can hardly make any prediction
>>>> I'm open to suggestion if you prefer something else
>>>>
>>>>>
>>>>>
>>>>>> +            audio-routing = "TDMOUT_A IN 0", "FRDDR_A OUT 0",
>>>>>> +                            "TDMOUT_A IN 1", "FRDDR_B OUT 0",
>>>>>> +                            "TDMOUT_A IN 2", "FRDDR_C OUT 0",
>>>>>> +                            "TDM_A Playback", "TDMOUT_A OUT",
>>>>>> +                            "TDMOUT_B IN 0", "FRDDR_A OUT 1",
>>>>>> +                            "TDMOUT_B IN 1", "FRDDR_B OUT 1",
>>>>>> +                            "TDMOUT_B IN 2", "FRDDR_C OUT 1",
>>>>>> +                            "TDM_B Playback", "TDMOUT_B OUT",
>>>>>> +                            "TDMOUT_C IN 0", "FRDDR_A OUT 2",
>>>>>> +                            "TDMOUT_C IN 1", "FRDDR_B OUT 2",
>>>>>> +                            "TDMOUT_C IN 2", "FRDDR_C OUT 2",
>>>>>> +                            "TDM_C Playback", "TDMOUT_C OUT",
>>>>>> +                            "TDMIN_A IN 0", "TDM_A Capture",
>>>>>> +                            "TDMIN_B IN 0", "TDM_A Capture",
>>>>>> +                            "TDMIN_C IN 0", "TDM_A Capture",
>>>>>> +                            "TDMIN_A IN 3", "TDM_A Loopback",
>>>>>> +                            "TDMIN_B IN 3", "TDM_A Loopback",
>>>>>> +                            "TDMIN_C IN 3", "TDM_A Loopback",
>>>>>> +                            "TDMIN_A IN 1", "TDM_B Capture",
>>>>>> +                            "TDMIN_B IN 1", "TDM_B Capture",
>>>>>> +                            "TDMIN_C IN 1", "TDM_B Capture",
>>>>>> +                            "TDMIN_A IN 4", "TDM_B Loopback",
>>>>>> +                            "TDMIN_B IN 4", "TDM_B Loopback",
>>>>>> +                            "TDMIN_C IN 4", "TDM_B Loopback",
>>>>>> +                            "TDMIN_A IN 2", "TDM_C Capture",
>>>>>> +                            "TDMIN_B IN 2", "TDM_C Capture",
>>>>>> +                            "TDMIN_C IN 2", "TDM_C Capture",
>>>>>> +                            "TDMIN_A IN 5", "TDM_C Loopback",
>>>>>> +                            "TDMIN_B IN 5", "TDM_C Loopback",
>>>>>> +                            "TDMIN_C IN 5", "TDM_C Loopback",
>>>>>> +                            "TODDR_A IN 0", "TDMIN_A OUT",
>>>>>> +                            "TODDR_B IN 0", "TDMIN_A OUT",
>>>>>> +                            "TODDR_C IN 0", "TDMIN_A OUT",
>>>>>> +                            "TODDR_A IN 1", "TDMIN_B OUT",
>>>>>> +                            "TODDR_B IN 1", "TDMIN_B OUT",
>>>>>> +                            "TODDR_C IN 1", "TDMIN_B OUT",
>>>>>> +                            "TODDR_A IN 2", "TDMIN_C OUT",
>>>>>> +                            "TODDR_B IN 2", "TDMIN_C OUT",
>>>>>> +                            "TODDR_C IN 2", "TDMIN_C OUT",
>>>>>> +                            "Lineout", "ACODEC LOLP",
>>>>>> +                            "Lineout", "ACODEC LORP";
>>>>>> +    };
>>>>>> +};
>>>>>> +

<snip>
  

On 02/10/2023 16:10, Jerome Brunet wrote:
> Add support for the Libretech cottonwood board family.
> These 2 boards are based on the same PCB, with an RPi B form factor.
> 
> The "Alta" board uses an a311d while the "Solitude" variant uses an s905d3.
> 
> Co-developed-by: Da Xue <da.xue@libretech.co>
> Signed-off-by: Da Xue <da.xue@libretech.co>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>   arch/arm64/boot/dts/amlogic/Makefile          |   2 +
>   .../amlogic/meson-g12b-a311d-libretech-cc.dts | 133 ++++
>   .../amlogic/meson-libretech-cottonwood.dtsi   | 610 ++++++++++++++++++
>   .../amlogic/meson-sm1-s905d3-libretech-cc.dts |  89 +++
>   4 files changed, 834 insertions(+)
>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
> 

<snip>

> +
> +	leds-pwm {
> +		compatible = "pwm-leds";
> +
> +		led-green {
> +			color = <LED_COLOR_ID_GREEN>;
> +			function = LED_FUNCTION_STATUS;
> +			linux,default-trigger = "default-on";
> +			panic-indicator;
> +			max-brightness = <255>;
> +			pwms = <&pwm_cd 1 1250 0>;
> +			active-low;
> +		};
> +
> +		led-blue {
> +			color = <LED_COLOR_ID_BLUE>;
> +			function = LED_FUNCTION_ACTIVITY;
> +			linux,default-trigger = "activity";

"activity" isn't documented, perhaps heartbeat instead ?

> +			max-brightness = <255>;
> +			pwms = <&pwm_ab 1 1250 0>;
> +			active-low;
> +		};

leds subnodes should be named as led(-[0-9a-f]+)

see Documentation/devicetree/bindings/leds/leds-pwm.yaml

> +	};
> +
> +	leds-gpio {
> +		compatible = "gpio-leds";
> +
> +		led-orange {
> +			color = <LED_COLOR_ID_AMBER>;
> +			function = LED_FUNCTION_STANDBY;
> +			gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
> +		};

Ditto, but you can simply use "led" since it's the only one.

See Documentation/devicetree/bindings/leds/leds-gpio.yaml

Neil


<snip>
  

On Wed 04 Oct 2023 at 11:20, Neil Armstrong <neil.armstrong@linaro.org> wrote:

> On 02/10/2023 16:10, Jerome Brunet wrote:
>> Add support for the Libretech cottonwood board family.
>> These 2 boards are based on the same PCB, with an RPi B form factor.
>> The "Alta" board uses an a311d while the "Solitude" variant uses an
>> s905d3.
>> Co-developed-by: Da Xue <da.xue@libretech.co>
>> Signed-off-by: Da Xue <da.xue@libretech.co>
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>>   arch/arm64/boot/dts/amlogic/Makefile          |   2 +
>>   .../amlogic/meson-g12b-a311d-libretech-cc.dts | 133 ++++
>>   .../amlogic/meson-libretech-cottonwood.dtsi   | 610 ++++++++++++++++++
>>   .../amlogic/meson-sm1-s905d3-libretech-cc.dts |  89 +++
>>   4 files changed, 834 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
>>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
>>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
>> 
>
> <snip>
>
>> +
>> +	leds-pwm {
>> +		compatible = "pwm-leds";
>> +
>> +		led-green {
>> +			color = <LED_COLOR_ID_GREEN>;
>> +			function = LED_FUNCTION_STATUS;
>> +			linux,default-trigger = "default-on";
>> +			panic-indicator;
>> +			max-brightness = <255>;
>> +			pwms = <&pwm_cd 1 1250 0>;
>> +			active-low;
>> +		};
>> +
>> +		led-blue {
>> +			color = <LED_COLOR_ID_BLUE>;
>> +			function = LED_FUNCTION_ACTIVITY;
>> +			linux,default-trigger = "activity";
>
> "activity" isn't documented, perhaps heartbeat instead ?
>

The trigger does exist though. The other way is to extend the DT doc.
I don't really care one way or the other

I'll defer to Da on this one

>> +			max-brightness = <255>;
>> +			pwms = <&pwm_ab 1 1250 0>;
>> +			active-low;
>> +		};
>
> leds subnodes should be named as led(-[0-9a-f]+)
>
> see Documentation/devicetree/bindings/leds/leds-pwm.yaml

That I do care. The schematics refer to the leds by name. There is no
number assigned, much less hex. Making one up makes no sense.

User should be able to quickly (and easily) link  what they see in the
schematics with DT.

So I'd prefer to submit a change for the regex rather than changing this

>
>> +	};
>> +
>> +	leds-gpio {
>> +		compatible = "gpio-leds";
>> +
>> +		led-orange {
>> +			color = <LED_COLOR_ID_AMBER>;
>> +			function = LED_FUNCTION_STANDBY;
>> +			gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
>> +		};
>
> Ditto, but you can simply use "led" since it's the only one.
>
> See Documentation/devicetree/bindings/leds/leds-gpio.yaml
>
> Neil
>
>
> <snip>
  

On Tue 03 Oct 2023 at 05:15, Da Xue <da@lessconfused.com> wrote:

>> >> +
>> >> +    leds-gpio {
>> >> +            compatible = "gpio-leds";
>> >> +
>> >> +            led-orange {
>> >> +                    color = <LED_COLOR_ID_AMBER>;
>
> Should this be LED_COLOR_ID_ORANGE?

It should (and initally was)
It was coming funny '(null)-standby' in sysfs and I did really checked
why

I can change it back, no problem

>
>> >> +                    function = LED_FUNCTION_STANDBY;
>> >> +                    gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
>> >> +            };
>> >> +    };
>> >> +
  

On Wed, Oct 4, 2023 at 5:59 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>
>
> On Wed 04 Oct 2023 at 11:20, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
> > On 02/10/2023 16:10, Jerome Brunet wrote:
> >> Add support for the Libretech cottonwood board family.
> >> These 2 boards are based on the same PCB, with an RPi B form factor.
> >> The "Alta" board uses an a311d while the "Solitude" variant uses an
> >> s905d3.
> >> Co-developed-by: Da Xue <da.xue@libretech.co>
> >> Signed-off-by: Da Xue <da.xue@libretech.co>
> >> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> >> ---
> >>   arch/arm64/boot/dts/amlogic/Makefile          |   2 +
> >>   .../amlogic/meson-g12b-a311d-libretech-cc.dts | 133 ++++
> >>   .../amlogic/meson-libretech-cottonwood.dtsi   | 610 ++++++++++++++++++
> >>   .../amlogic/meson-sm1-s905d3-libretech-cc.dts |  89 +++
> >>   4 files changed, 834 insertions(+)
> >>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts
> >>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi
> >>   create mode 100644 arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts
> >>
> >
> > <snip>
> >
> >> +
> >> +    leds-pwm {
> >> +            compatible = "pwm-leds";
> >> +
> >> +            led-green {
> >> +                    color = <LED_COLOR_ID_GREEN>;
> >> +                    function = LED_FUNCTION_STATUS;
> >> +                    linux,default-trigger = "default-on";
> >> +                    panic-indicator;
> >> +                    max-brightness = <255>;
> >> +                    pwms = <&pwm_cd 1 1250 0>;
> >> +                    active-low;
> >> +            };
> >> +
> >> +            led-blue {
> >> +                    color = <LED_COLOR_ID_BLUE>;
> >> +                    function = LED_FUNCTION_ACTIVITY;
> >> +                    linux,default-trigger = "activity";
> >
> > "activity" isn't documented, perhaps heartbeat instead ?
> >
>
> The trigger does exist though. The other way is to extend the DT doc.
> I don't really care one way or the other

We can and should use heartbeat to both be consistent and also show
Linux has started for developers.

>
> I'll defer to Da on this one
>
> >> +                    max-brightness = <255>;
> >> +                    pwms = <&pwm_ab 1 1250 0>;
> >> +                    active-low;
> >> +            };
> >
> > leds subnodes should be named as led(-[0-9a-f]+)
> >
> > see Documentation/devicetree/bindings/leds/leds-pwm.yaml
>
> That I do care. The schematics refer to the leds by name. There is no
> number assigned, much less hex. Making one up makes no sense.
>
> User should be able to quickly (and easily) link  what they see in the
> schematics with DT.
>
> So I'd prefer to submit a change for the regex rather than changing this

I don't get why the LEDs should be hex either. It's contradictory to
https://www.kernel.org/doc/html/latest/leds/leds-class.html

I did a simple grep and didn't find any references to
LED_COLOR_ID_ORANGE. We can debug this later.

>
> >
> >> +    };
> >> +
> >> +    leds-gpio {
> >> +            compatible = "gpio-leds";
> >> +
> >> +            led-orange {
> >> +                    color = <LED_COLOR_ID_AMBER>;
> >> +                    function = LED_FUNCTION_STANDBY;
> >> +                    gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
> >> +            };
> >
> > Ditto, but you can simply use "led" since it's the only one.
> >
> > See Documentation/devicetree/bindings/leds/leds-gpio.yaml
> >
> > Neil
> >
> >
> > <snip>
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
  

On Tue 03 Oct 2023 at 09:35, Neil Armstrong <neil.armstrong@linaro.org> wrote:

> On 02/10/2023 20:57, Jerome Brunet wrote:
>> On Mon 02 Oct 2023 at 18:45, Neil Armstrong <neil.armstrong@linaro.org>
>> wrote:
>> 
>
> <snip>
>
>>>> +&usb3_pcie_phy {
>>>> +	#address-cells = <1>;
>>>> +	#size-cells = <0>;
>>>> +	phy-supply = <&vcc_5v>;
>>>> +
>>>> +	hub: hub@1 {
>>>> +		compatible = "usb5e3,626";
>>>> +		reg = <1>;
>>>> +		reset-gpios = <&gpio GPIOC_7 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
>>>> +	};
>>>
>>> Not sure the PHY is the right place to put the USB HUB,
>>> and it's probable the HUB is connected to both the USB2 and USB3 lines
>> It is connected to the USB3.0 only
>> 
>>> so you should have both USB IDs in DT like it'd done for the Odroid-C4:
>>>
>>> / {
>>> ...
>>>           /* USB hub supports both USB 2.0 and USB 3.0 root hub */
>>>           usb-hub {
>>>                   dr_mode = "host";
>>>                   #address-cells = <1>;
>>>                   #size-cells = <0>;
>>>
>>>                   /* 2.0 hub on port 1 */
>>>                   hub_2_0: hub@1 {
>>>                           compatible = "usb2109,2817";
>>>                           reg = <1>;
>>>                           peer-hub = <&hub_3_0>;
>>>                           reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>                           vdd-supply = <&vcc_5v>;
>>>                   };
>>>
>>>                   /* 3.1 hub on port 4 */
>>>                   hub_3_0: hub@2 {
>>>                           compatible = "usb2109,817";
>>>                           reg = <2>;
>>>                           peer-hub = <&hub_2_0>;
>>>                           reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>                           vdd-supply = <&vcc_5v>;
>>>                   };
>>>           };
>>> ...
>>> };
>>>
>>> if it only has a single USB ID, then it should go under the dwc3 node.
>> The usb controller is connected to the PHY and what's coming out of the
>> PHY
>> goes to the hub. It seems logical to hub the hub under it.
>> Why bypass the PHY ?
>
> The USB bindings the USB devices nodes should be under the controller's node,
> not the PHY, see:
>
> Documentation/devicetree/bindings/usb/usb-hcd.yaml
> ...
> patternProperties:
>   "^.*@[0-9a-f]{1,2}$":
>     description: The hard wired USB devices
>     type: object
>     $ref: /schemas/usb/usb-device.yaml
> ...
> and the example.
>
> Subnodes aren't allowed in the PHY node.

Ok, that is what schema says.
HW wise there is possible problem though.

The phy node has the power supply to the bus.
In that case it is a controllable one.

If fixed USB devices go under the controller instead of the PHY, isn't
it possible that the kernel may attempt to probe them before the bus is
powered ? For this particular board, it would make the reset we are
trying to apply useless.

>
> Neil
>
>> 
>>>
>>>> +};
>>>> +
>>>> +&usb {
>>>> +	status = "okay";
>>>> +};
>
> <snip>
  

On 05/10/2023 11:42, Jerome Brunet wrote:
> 
> On Tue 03 Oct 2023 at 09:35, Neil Armstrong <neil.armstrong@linaro.org> wrote:
> 
>> On 02/10/2023 20:57, Jerome Brunet wrote:
>>> On Mon 02 Oct 2023 at 18:45, Neil Armstrong <neil.armstrong@linaro.org>
>>> wrote:
>>>
>>
>> <snip>
>>
>>>>> +&usb3_pcie_phy {
>>>>> +	#address-cells = <1>;
>>>>> +	#size-cells = <0>;
>>>>> +	phy-supply = <&vcc_5v>;
>>>>> +
>>>>> +	hub: hub@1 {
>>>>> +		compatible = "usb5e3,626";
>>>>> +		reg = <1>;
>>>>> +		reset-gpios = <&gpio GPIOC_7 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
>>>>> +	};
>>>>
>>>> Not sure the PHY is the right place to put the USB HUB,
>>>> and it's probable the HUB is connected to both the USB2 and USB3 lines
>>> It is connected to the USB3.0 only
>>>
>>>> so you should have both USB IDs in DT like it'd done for the Odroid-C4:
>>>>
>>>> / {
>>>> ...
>>>>            /* USB hub supports both USB 2.0 and USB 3.0 root hub */
>>>>            usb-hub {
>>>>                    dr_mode = "host";
>>>>                    #address-cells = <1>;
>>>>                    #size-cells = <0>;
>>>>
>>>>                    /* 2.0 hub on port 1 */
>>>>                    hub_2_0: hub@1 {
>>>>                            compatible = "usb2109,2817";
>>>>                            reg = <1>;
>>>>                            peer-hub = <&hub_3_0>;
>>>>                            reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>>                            vdd-supply = <&vcc_5v>;
>>>>                    };
>>>>
>>>>                    /* 3.1 hub on port 4 */
>>>>                    hub_3_0: hub@2 {
>>>>                            compatible = "usb2109,817";
>>>>                            reg = <2>;
>>>>                            peer-hub = <&hub_2_0>;
>>>>                            reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>>                            vdd-supply = <&vcc_5v>;
>>>>                    };
>>>>            };
>>>> ...
>>>> };
>>>>
>>>> if it only has a single USB ID, then it should go under the dwc3 node.
>>> The usb controller is connected to the PHY and what's coming out of the
>>> PHY
>>> goes to the hub. It seems logical to hub the hub under it.
>>> Why bypass the PHY ?
>>
>> The USB bindings the USB devices nodes should be under the controller's node,
>> not the PHY, see:
>>
>> Documentation/devicetree/bindings/usb/usb-hcd.yaml
>> ...
>> patternProperties:
>>    "^.*@[0-9a-f]{1,2}$":
>>      description: The hard wired USB devices
>>      type: object
>>      $ref: /schemas/usb/usb-device.yaml
>> ...
>> and the example.
>>
>> Subnodes aren't allowed in the PHY node.
> 
> Ok, that is what schema says.
> HW wise there is possible problem though.
> 
> The phy node has the power supply to the bus.
> In that case it is a controllable one.
> 
> If fixed USB devices go under the controller instead of the PHY, isn't
> it possible that the kernel may attempt to probe them before the bus is
> powered ? For this particular board, it would make the reset we are
> trying to apply useless.

The usb core has a special handling for those usb hubs doing the power
up at the right time during the USB setup, including the PHY powering up.
So the power sequence should be fine.

This has been done on Odroid-C2 and Odroid-N2 already.

Neil

> 
>>
>> Neil
>>
>>>
>>>>
>>>>> +};
>>>>> +
>>>>> +&usb {
>>>>> +	status = "okay";
>>>>> +};
>>
>> <snip>
>
  

On Thu 05 Oct 2023 at 12:04, Neil Armstrong <neil.armstrong@linaro.org> wrote:

> On 05/10/2023 11:42, Jerome Brunet wrote:
>> On Tue 03 Oct 2023 at 09:35, Neil Armstrong <neil.armstrong@linaro.org>
>> wrote:
>> 
>>> On 02/10/2023 20:57, Jerome Brunet wrote:
>>>> On Mon 02 Oct 2023 at 18:45, Neil Armstrong <neil.armstrong@linaro.org>
>>>> wrote:
>>>>
>>>
>>> <snip>
>>>
>>>>>> +&usb3_pcie_phy {
>>>>>> +	#address-cells = <1>;
>>>>>> +	#size-cells = <0>;
>>>>>> +	phy-supply = <&vcc_5v>;
>>>>>> +
>>>>>> +	hub: hub@1 {
>>>>>> +		compatible = "usb5e3,626";
>>>>>> +		reg = <1>;
>>>>>> +		reset-gpios = <&gpio GPIOC_7 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
>>>>>> +	};
>>>>>
>>>>> Not sure the PHY is the right place to put the USB HUB,
>>>>> and it's probable the HUB is connected to both the USB2 and USB3 lines
>>>> It is connected to the USB3.0 only
>>>>
>>>>> so you should have both USB IDs in DT like it'd done for the Odroid-C4:
>>>>>
>>>>> / {
>>>>> ...
>>>>>            /* USB hub supports both USB 2.0 and USB 3.0 root hub */
>>>>>            usb-hub {
>>>>>                    dr_mode = "host";
>>>>>                    #address-cells = <1>;
>>>>>                    #size-cells = <0>;
>>>>>
>>>>>                    /* 2.0 hub on port 1 */
>>>>>                    hub_2_0: hub@1 {
>>>>>                            compatible = "usb2109,2817";
>>>>>                            reg = <1>;
>>>>>                            peer-hub = <&hub_3_0>;
>>>>>                            reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>>>                            vdd-supply = <&vcc_5v>;
>>>>>                    };
>>>>>
>>>>>                    /* 3.1 hub on port 4 */
>>>>>                    hub_3_0: hub@2 {
>>>>>                            compatible = "usb2109,817";
>>>>>                            reg = <2>;
>>>>>                            peer-hub = <&hub_2_0>;
>>>>>                            reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>>>                            vdd-supply = <&vcc_5v>;
>>>>>                    };
>>>>>            };
>>>>> ...
>>>>> };
>>>>>
>>>>> if it only has a single USB ID, then it should go under the dwc3 node.
>>>> The usb controller is connected to the PHY and what's coming out of the
>>>> PHY
>>>> goes to the hub. It seems logical to hub the hub under it.
>>>> Why bypass the PHY ?
>>>
>>> The USB bindings the USB devices nodes should be under the controller's node,
>>> not the PHY, see:
>>>
>>> Documentation/devicetree/bindings/usb/usb-hcd.yaml
>>> ...
>>> patternProperties:
>>>    "^.*@[0-9a-f]{1,2}$":
>>>      description: The hard wired USB devices
>>>      type: object
>>>      $ref: /schemas/usb/usb-device.yaml
>>> ...
>>> and the example.
>>>
>>> Subnodes aren't allowed in the PHY node.
>> Ok, that is what schema says.
>> HW wise there is possible problem though.
>> The phy node has the power supply to the bus.
>> In that case it is a controllable one.
>> If fixed USB devices go under the controller instead of the PHY, isn't
>> it possible that the kernel may attempt to probe them before the bus is
>> powered ? For this particular board, it would make the reset we are
>> trying to apply useless.
>
> The usb core has a special handling for those usb hubs doing the power
> up at the right time during the USB setup, including the PHY powering up.
> So the power sequence should be fine.
>
> This has been done on Odroid-C2 and Odroid-N2 already.

Tried it. Unfortunately something is off with the hub under the dwc3 node
I often get this error (like once in 3 boots):

[    0.419301] usbcore: registered new interface driver usbfs
[    0.424434] usbcore: registered new interface driver hub
[    0.429696] usbcore: registered new device driver usb
[    0.921460] usbcore: registered new interface driver usb-storage
[    0.968157] usbcore: registered new interface driver usbhid
[    0.972114] usbhid: USB HID core driver
[    1.132529] dwc3-meson-g12a ffe09000.usb: USB2 ports: 2
[    1.134897] dwc3-meson-g12a ffe09000.usb: USB3 ports: 1
[    1.144451] dwc2 ff400000.usb: supply vusb_d not found, using dummy regulator
[    1.147231] dwc2 ff400000.usb: supply vusb_a not found, using dummy regulator
[    1.154464] dwc2 ff400000.usb: EPs: 7, dedicated fifos, 712 entries in SPRAM
[    1.219515] usb usb2: We don't know the algorithms for LPM for this host, disabling LPM.
[    1.469260] usb 1-1: new high-speed USB device number 2 using xhci-hcd
[    1.745395] usb 2-1: new SuperSpeed USB device number 2 using xhci-hcd
[    9.794777] usbcore: registered new device driver onboard-usb-hub
[   10.255484] onboard-usb-hub 1-1: Failed to suspend device, error -32
[   10.261699] onboard-usb-hub 1-1: can't set config #1, error -71
[   10.287500] onboard-usb-hub 1-1: Failed to suspend device, error -32
[   10.287844] onboard-usb-hub 1-1: USB disconnect, device number 2
[   10.573277] usb 1-1: new high-speed USB device number 3 using xhci-hcd
[   10.921468] usb 2-1: reset SuperSpeed USB device number 2 using xhci-hcd
[   11.193453] usb 2-1: reset SuperSpeed USB device number 2 using xhci-hcd

While it works reliably when the onboard-usb-hub is under the phy node.

I added the 5v supply as vdd under the hub for good measure.

>
> Neil
>
>> 
>>>
>>> Neil
>>>
>>>>
>>>>>
>>>>>> +};
>>>>>> +
>>>>>> +&usb {
>>>>>> +	status = "okay";
>>>>>> +};
>>>
>>> <snip>
>>
  

On 06/10/2023 10:21, Jerome Brunet wrote:
> 
> On Thu 05 Oct 2023 at 12:04, Neil Armstrong <neil.armstrong@linaro.org> wrote:
> 
>> On 05/10/2023 11:42, Jerome Brunet wrote:
>>> On Tue 03 Oct 2023 at 09:35, Neil Armstrong <neil.armstrong@linaro.org>
>>> wrote:
>>>
>>>> On 02/10/2023 20:57, Jerome Brunet wrote:
>>>>> On Mon 02 Oct 2023 at 18:45, Neil Armstrong <neil.armstrong@linaro.org>
>>>>> wrote:
>>>>>
>>>>
>>>> <snip>
>>>>
>>>>>>> +&usb3_pcie_phy {
>>>>>>> +	#address-cells = <1>;
>>>>>>> +	#size-cells = <0>;
>>>>>>> +	phy-supply = <&vcc_5v>;
>>>>>>> +
>>>>>>> +	hub: hub@1 {
>>>>>>> +		compatible = "usb5e3,626";
>>>>>>> +		reg = <1>;
>>>>>>> +		reset-gpios = <&gpio GPIOC_7 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
>>>>>>> +	};
>>>>>>
>>>>>> Not sure the PHY is the right place to put the USB HUB,
>>>>>> and it's probable the HUB is connected to both the USB2 and USB3 lines
>>>>> It is connected to the USB3.0 only
>>>>>
>>>>>> so you should have both USB IDs in DT like it'd done for the Odroid-C4:
>>>>>>
>>>>>> / {
>>>>>> ...
>>>>>>             /* USB hub supports both USB 2.0 and USB 3.0 root hub */
>>>>>>             usb-hub {
>>>>>>                     dr_mode = "host";
>>>>>>                     #address-cells = <1>;
>>>>>>                     #size-cells = <0>;
>>>>>>
>>>>>>                     /* 2.0 hub on port 1 */
>>>>>>                     hub_2_0: hub@1 {
>>>>>>                             compatible = "usb2109,2817";
>>>>>>                             reg = <1>;
>>>>>>                             peer-hub = <&hub_3_0>;
>>>>>>                             reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>>>>                             vdd-supply = <&vcc_5v>;
>>>>>>                     };
>>>>>>
>>>>>>                     /* 3.1 hub on port 4 */
>>>>>>                     hub_3_0: hub@2 {
>>>>>>                             compatible = "usb2109,817";
>>>>>>                             reg = <2>;
>>>>>>                             peer-hub = <&hub_2_0>;
>>>>>>                             reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>>>>                             vdd-supply = <&vcc_5v>;
>>>>>>                     };
>>>>>>             };
>>>>>> ...
>>>>>> };
>>>>>>
>>>>>> if it only has a single USB ID, then it should go under the dwc3 node.
>>>>> The usb controller is connected to the PHY and what's coming out of the
>>>>> PHY
>>>>> goes to the hub. It seems logical to hub the hub under it.
>>>>> Why bypass the PHY ?
>>>>
>>>> The USB bindings the USB devices nodes should be under the controller's node,
>>>> not the PHY, see:
>>>>
>>>> Documentation/devicetree/bindings/usb/usb-hcd.yaml
>>>> ...
>>>> patternProperties:
>>>>     "^.*@[0-9a-f]{1,2}$":
>>>>       description: The hard wired USB devices
>>>>       type: object
>>>>       $ref: /schemas/usb/usb-device.yaml
>>>> ...
>>>> and the example.
>>>>
>>>> Subnodes aren't allowed in the PHY node.
>>> Ok, that is what schema says.
>>> HW wise there is possible problem though.
>>> The phy node has the power supply to the bus.
>>> In that case it is a controllable one.
>>> If fixed USB devices go under the controller instead of the PHY, isn't
>>> it possible that the kernel may attempt to probe them before the bus is
>>> powered ? For this particular board, it would make the reset we are
>>> trying to apply useless.
>>
>> The usb core has a special handling for those usb hubs doing the power
>> up at the right time during the USB setup, including the PHY powering up.
>> So the power sequence should be fine.
>>
>> This has been done on Odroid-C2 and Odroid-N2 already.
> 
> Tried it. Unfortunately something is off with the hub under the dwc3 node
> I often get this error (like once in 3 boots):
> 
> [    0.419301] usbcore: registered new interface driver usbfs
> [    0.424434] usbcore: registered new interface driver hub
> [    0.429696] usbcore: registered new device driver usb
> [    0.921460] usbcore: registered new interface driver usb-storage
> [    0.968157] usbcore: registered new interface driver usbhid
> [    0.972114] usbhid: USB HID core driver
> [    1.132529] dwc3-meson-g12a ffe09000.usb: USB2 ports: 2
> [    1.134897] dwc3-meson-g12a ffe09000.usb: USB3 ports: 1
> [    1.144451] dwc2 ff400000.usb: supply vusb_d not found, using dummy regulator
> [    1.147231] dwc2 ff400000.usb: supply vusb_a not found, using dummy regulator
> [    1.154464] dwc2 ff400000.usb: EPs: 7, dedicated fifos, 712 entries in SPRAM
> [    1.219515] usb usb2: We don't know the algorithms for LPM for this host, disabling LPM.
> [    1.469260] usb 1-1: new high-speed USB device number 2 using xhci-hcd
> [    1.745395] usb 2-1: new SuperSpeed USB device number 2 using xhci-hcd
> [    9.794777] usbcore: registered new device driver onboard-usb-hub
> [   10.255484] onboard-usb-hub 1-1: Failed to suspend device, error -32
> [   10.261699] onboard-usb-hub 1-1: can't set config #1, error -71
> [   10.287500] onboard-usb-hub 1-1: Failed to suspend device, error -32
> [   10.287844] onboard-usb-hub 1-1: USB disconnect, device number 2
> [   10.573277] usb 1-1: new high-speed USB device number 3 using xhci-hcd
> [   10.921468] usb 2-1: reset SuperSpeed USB device number 2 using xhci-hcd
> [   11.193453] usb 2-1: reset SuperSpeed USB device number 2 using xhci-hcd
> 
> While it works reliably when the onboard-usb-hub is under the phy node.
> 
> I added the 5v supply as vdd under the hub for good measure.

The .reset_us you used from genesys_gl852g is probably too low, you may need to use a bigger one then.

Neil

> 
>>
>> Neil
>>
>>>
>>>>
>>>> Neil
>>>>
>>>>>
>>>>>>
>>>>>>> +};
>>>>>>> +
>>>>>>> +&usb {
>>>>>>> +	status = "okay";
>>>>>>> +};
>>>>
>>>> <snip>
>>>
>
  

On Fri 06 Oct 2023 at 10:32, Neil Armstrong <neil.armstrong@linaro.org> wrote:

> On 06/10/2023 10:21, Jerome Brunet wrote:
>> On Thu 05 Oct 2023 at 12:04, Neil Armstrong <neil.armstrong@linaro.org>
>> wrote:
>> 
>>> On 05/10/2023 11:42, Jerome Brunet wrote:
>>>> On Tue 03 Oct 2023 at 09:35, Neil Armstrong <neil.armstrong@linaro.org>
>>>> wrote:
>>>>
>>>>> On 02/10/2023 20:57, Jerome Brunet wrote:
>>>>>> On Mon 02 Oct 2023 at 18:45, Neil Armstrong <neil.armstrong@linaro.org>
>>>>>> wrote:
>>>>>>
>>>>>
>>>>> <snip>
>>>>>
>>>>>>>> +&usb3_pcie_phy {
>>>>>>>> +	#address-cells = <1>;
>>>>>>>> +	#size-cells = <0>;
>>>>>>>> +	phy-supply = <&vcc_5v>;
>>>>>>>> +
>>>>>>>> +	hub: hub@1 {
>>>>>>>> +		compatible = "usb5e3,626";
>>>>>>>> +		reg = <1>;
>>>>>>>> +		reset-gpios = <&gpio GPIOC_7 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
>>>>>>>> +	};
>>>>>>>
>>>>>>> Not sure the PHY is the right place to put the USB HUB,
>>>>>>> and it's probable the HUB is connected to both the USB2 and USB3 lines
>>>>>> It is connected to the USB3.0 only
>>>>>>
>>>>>>> so you should have both USB IDs in DT like it'd done for the Odroid-C4:
>>>>>>>
>>>>>>> / {
>>>>>>> ...
>>>>>>>             /* USB hub supports both USB 2.0 and USB 3.0 root hub */
>>>>>>>             usb-hub {
>>>>>>>                     dr_mode = "host";
>>>>>>>                     #address-cells = <1>;
>>>>>>>                     #size-cells = <0>;
>>>>>>>
>>>>>>>                     /* 2.0 hub on port 1 */
>>>>>>>                     hub_2_0: hub@1 {
>>>>>>>                             compatible = "usb2109,2817";
>>>>>>>                             reg = <1>;
>>>>>>>                             peer-hub = <&hub_3_0>;
>>>>>>>                             reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>>>>>                             vdd-supply = <&vcc_5v>;
>>>>>>>                     };
>>>>>>>
>>>>>>>                     /* 3.1 hub on port 4 */
>>>>>>>                     hub_3_0: hub@2 {
>>>>>>>                             compatible = "usb2109,817";
>>>>>>>                             reg = <2>;
>>>>>>>                             peer-hub = <&hub_2_0>;
>>>>>>>                             reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
>>>>>>>                             vdd-supply = <&vcc_5v>;
>>>>>>>                     };
>>>>>>>             };
>>>>>>> ...
>>>>>>> };
>>>>>>>
>>>>>>> if it only has a single USB ID, then it should go under the dwc3 node.
>>>>>> The usb controller is connected to the PHY and what's coming out of the
>>>>>> PHY
>>>>>> goes to the hub. It seems logical to hub the hub under it.
>>>>>> Why bypass the PHY ?
>>>>>
>>>>> The USB bindings the USB devices nodes should be under the controller's node,
>>>>> not the PHY, see:
>>>>>
>>>>> Documentation/devicetree/bindings/usb/usb-hcd.yaml
>>>>> ...
>>>>> patternProperties:
>>>>>     "^.*@[0-9a-f]{1,2}$":
>>>>>       description: The hard wired USB devices
>>>>>       type: object
>>>>>       $ref: /schemas/usb/usb-device.yaml
>>>>> ...
>>>>> and the example.
>>>>>
>>>>> Subnodes aren't allowed in the PHY node.
>>>> Ok, that is what schema says.
>>>> HW wise there is possible problem though.
>>>> The phy node has the power supply to the bus.
>>>> In that case it is a controllable one.
>>>> If fixed USB devices go under the controller instead of the PHY, isn't
>>>> it possible that the kernel may attempt to probe them before the bus is
>>>> powered ? For this particular board, it would make the reset we are
>>>> trying to apply useless.
>>>
>>> The usb core has a special handling for those usb hubs doing the power
>>> up at the right time during the USB setup, including the PHY powering up.
>>> So the power sequence should be fine.
>>>
>>> This has been done on Odroid-C2 and Odroid-N2 already.
>> Tried it. Unfortunately something is off with the hub under the dwc3 node
>> I often get this error (like once in 3 boots):
>> [    0.419301] usbcore: registered new interface driver usbfs
>> [    0.424434] usbcore: registered new interface driver hub
>> [    0.429696] usbcore: registered new device driver usb
>> [    0.921460] usbcore: registered new interface driver usb-storage
>> [    0.968157] usbcore: registered new interface driver usbhid
>> [    0.972114] usbhid: USB HID core driver
>> [    1.132529] dwc3-meson-g12a ffe09000.usb: USB2 ports: 2
>> [    1.134897] dwc3-meson-g12a ffe09000.usb: USB3 ports: 1
>> [    1.144451] dwc2 ff400000.usb: supply vusb_d not found, using dummy regulator
>> [    1.147231] dwc2 ff400000.usb: supply vusb_a not found, using dummy regulator
>> [    1.154464] dwc2 ff400000.usb: EPs: 7, dedicated fifos, 712 entries in SPRAM
>> [    1.219515] usb usb2: We don't know the algorithms for LPM for this host, disabling LPM.
>> [    1.469260] usb 1-1: new high-speed USB device number 2 using xhci-hcd
>> [    1.745395] usb 2-1: new SuperSpeed USB device number 2 using xhci-hcd
>> [    9.794777] usbcore: registered new device driver onboard-usb-hub
>> [   10.255484] onboard-usb-hub 1-1: Failed to suspend device, error -32
>> [   10.261699] onboard-usb-hub 1-1: can't set config #1, error -71
>> [   10.287500] onboard-usb-hub 1-1: Failed to suspend device, error -32
>> [   10.287844] onboard-usb-hub 1-1: USB disconnect, device number 2
>> [   10.573277] usb 1-1: new high-speed USB device number 3 using xhci-hcd
>> [   10.921468] usb 2-1: reset SuperSpeed USB device number 2 using xhci-hcd
>> [   11.193453] usb 2-1: reset SuperSpeed USB device number 2 using xhci-hcd
>> While it works reliably when the onboard-usb-hub is under the phy node.
>> I added the 5v supply as vdd under the hub for good measure.
>
> The .reset_us you used from genesys_gl852g is probably too low, you may need to use a bigger one then.
>

Indeed. This seems to do the trick.
I'll change this

Thx

> Neil
>
>> 
>>>
>>> Neil
>>>
>>>>
>>>>>
>>>>> Neil
>>>>>
>>>>>>
>>>>>>>
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +&usb {
>>>>>>>> +	status = "okay";
>>>>>>>> +};
>>>>>
>>>>> <snip>
>>>>
>>