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

From: Neil Armstrong
Date: Tue Oct 03 2023 - 03:38:59 EST


On 03/10/2023 09:21, Jerome Brunet wrote:

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

On 3 Oct 2023, at 1:15 am, Da Xue <da@xxxxxxxxxxxxxxxx> wrote:

On Tue, Oct 3, 2023 at 3:13 AM Jerome Brunet <jbrunet@xxxxxxxxxxxx> wrote:


On Mon 02 Oct 2023 at 18:45, Neil Armstrong <neil.armstrong@xxxxxxxxxx> 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@xxxxxxxxxxxx>
Signed-off-by: Da Xue <da.xue@xxxxxxxxxxxx>
Signed-off-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx>
---
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@xxxxxxxxxxxx>
+ */
+
+/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>