Message-Id: <D7H113ZIQYVN.1V7WM9W27T5PM@postmarketos.org>
Date: Sat, 01 Feb 2025 10:11:26 +0000
From: "Ferass El Hafidi" <funderscore@...tmarketos.org>
To: "Christian Hewitt" <christianshewitt@...il.com>
Cc: "Neil Armstrong" <neil.armstrong@...aro.org>, "Kevin Hilman"
 <khilman@...libre.com>, "Jerome Brunet" <jbrunet@...libre.com>, "Martin
 Blumenstingl" <martin.blumenstingl@...glemail.com>, "Rob Herring"
 <robh@...nel.org>, "Krzysztof Kozlowski" <krzk+dt@...nel.org>, "Conor
 Dooley" <conor+dt@...nel.org>, <linux-amlogic@...ts.infradead.org>,
 <linux-arm-kernel@...ts.infradead.org>, <devicetree@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <~postmarketos/upstreaming@...ts.sr.ht>,
 "Artur Weber" <aweber.kernel@...il.com>, "Karl Chan"
 <exxxxkc@...googleoff.me>
Subject: Re: [PATCH 2/2] arm64: dts: amlogic: add support for
 xiaomi-aquaman/Mi TV Stick

On Sat Feb 1, 2025 at 6:08 AM UTC, Christian Hewitt wrote:
<snip>
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s805y-xiaomi-aquaman.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s805y-xiaomi-aquaman.dts
> > new file mode 100644
> > index 000000000..6b72f054a
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s805y-xiaomi-aquaman.dts
> > @@ -0,0 +1,311 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (c) 2025 Ferass El Hafidi <funderscore@...tmarketos.org>
> > + * Based on meson-gxl-s805x-p241.dtb:
> > + *  - Copyright (c) 2018 BayLibre, SAS.
> > + *    Author: Neil Armstrong <narmstrong@...libre.com>
> > + *    Author: Jerome Brunet <jbrunet@...libre.com>
>
> If this was cloned from the p241 file I would retain the Baylibre
> Copyright and add myself as an additional author. You’ve extended
> their work not created a new one.
>

I did retain the BayLibre copyright and stated that it is based on the
p241 DT.

> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include <dt-bindings/input/input.h>
> > +#include <dt-bindings/leds/common.h>
> > +#include <dt-bindings/sound/meson-aiu.h>
> > +
> > +#include "meson-gxl-s805x.dtsi"
>
> Amlogic Y variants normally omit CVBS, Ethernet, SD, and sometimes IR,
> so you could clone the s805x.dtsi and remove nodes there, then include
> an s805y.dtsi file. Perhaps not worth the effort for a single device
> at this time, but could be done as future optimisation.
>

I could do it in v2.  Could be worth the effort long-term.

> > +
> > +/ {
> > + compatible = "xiaomi,aquaman", "amlogic,s805x", "amlogic,meson-gxl";
>
> After updating the bindings, use “amlogic,s805y” here.
>
> > + model = "Xiaomi Mi TV Stick (S805Y)";
>
> I’d use “Xiaomi Mi TV Stick (Aquaman)” so the connection between the
> binding name and model name is obvious.
>

I added "(S805Y)" to differenciate from the other, much newer stick also 
known as an "Xiaomi Mi TV Stick", which uses a completely different Amlogic 
SoC and has different specs, though from what I can tell most commonly call 
that completely different stick, a "Xiaomi Mi TV Stick 4K".

> > +
> > + aliases {
> > + serial0 = &uart_AO;
> > + serial1 = &uart_A;
> > + };
> > +
> > + au2: analog-amplifier {
> > + compatible = "simple-audio-amplifier";
> > + sound-name-prefix = "AU2";
> > + VCC-supply = <&vcc_5v>;
> > + enable-gpios = <&gpio GPIOH_5 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + chosen {
> > + stdout-path = "serial0:115200n8";
> > + };
> > +
> > + cvbs-connector {
> > + /* No CVBS connector */
> > + status = "disabled";
> > + };
> > +
> > + emmc_pwrseq: emmc-pwrseq {
> > + compatible = "mmc-pwrseq-emmc";
> > + reset-gpios = <&gpio BOOT_9 GPIO_ACTIVE_LOW>;
> > + };
> > +
> > + hdmi-connector {
> > + compatible = "hdmi-connector";
> > + type = "a";
> > +
> > + port {
> > + hdmi_connector_in: endpoint {
> > + remote-endpoint = <&hdmi_tx_tmds_out>;
> > + };
> > + };
> > + };
> > +
> > + leds {
> > + compatible = "gpio-leds";
> > +
> > + led-white {
> > + color = <LED_COLOR_ID_WHITE>;
> > + function = LED_FUNCTION_POWER;
> > + gpios = <&gpio GPIODV_24 GPIO_ACTIVE_HIGH>;
> > + default-state = "heartbeat";
> > + panic-indicator;
> > + };
> > + };
> > +
> > + memory@0 {
> > + device_type = "memory";
> > + reg = <0x0 0x0 0x0 0x40000000>;
> > + };
> > +
> > + vddio_boot: regulator-vddio-boot {
> > + compatible = "regulator-fixed";
> > + regulator-name = "VDDIO_BOOT";
> > + regulator-min-microvolt = <1800000>;
> > + regulator-max-microvolt = <1800000>;
> > + };
> > +
> > + vddao_3v3: regulator-vddao-3v3 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "VDDAO_3V3";
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + };
> > +
> > + vddio_ao18: regulator-vddio-ao18 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "VDDIO_AO18";
> > + regulator-min-microvolt = <1800000>;
> > + regulator-max-microvolt = <1800000>;
> > + };
> > +
> > + vcc_3v3: regulator-vcc-3v3 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "VCC_3V3";
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + };
> > +
> > + vcc_5v: regulator-vcc-5v {
> > + compatible = "regulator-fixed";
> > + regulator-name = "VCC_5V";
> > + regulator-min-microvolt = <5000000>;
> > + regulator-max-microvolt = <5000000>;
> > + };
> > +
> > +
> > + emmc_pwrseq: emmc-pwrseq {
> > + compatible = "mmc-pwrseq-emmc";
> > + reset-gpios = <&gpio BOOT_9 GPIO_ACTIVE_LOW>;
> > + };
> > +
> > + wifi32k: wifi32k {
> > + compatible = "pwm-clock";
> > + #clock-cells = <0>;
> > + clock-frequency = <32768>;
> > + pwms = <&pwm_ef 0 30518 0>; /* PWM_E at 32.768KHz */
> > + };
> > +
> > + sdio_pwrseq: sdio-pwrseq {
> > + compatible = "mmc-pwrseq-simple";
> > + reset-gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
> > + clocks = <&wifi32k>;
> > + clock-names = "ext_clock";
> > + };
> > +
> > + sound {
> > + compatible = "amlogic,gx-sound-card";
> > + model = "XIAOMI-AQUAMAN";
>
> Just “AQUAMAN” ?
>

I wanted to retain the vendor name, and since there is a 15 char limit,
just to be safe I used `MI` instead of `XIAOMI`. But I guess 
"XIAOMI-AQUAMAN" should be fine after all.

> > + audio-aux-devs = <&au2>;
> > + audio-widgets = "Line", "Lineout";
> > + audio-routing = "AU2 INL", "ACODEC LOLN",
> > + "AU2 INR", "ACODEC LORN",
> > + "Lineout", "AU2 OUTL",
> > + "Lineout", "AU2 OUTR";
> > + clocks = <&clkc CLKID_MPLL0>,
> > + <&clkc CLKID_MPLL1>,
> > + <&clkc CLKID_MPLL2>;
> > +
> > + assigned-clocks = <&clkc CLKID_MPLL0>,
> > +  <&clkc CLKID_MPLL1>,
> > +  <&clkc CLKID_MPLL2>;
> > + assigned-clock-parents = <0>, <0>, <0>;
> > + assigned-clock-rates = <294912000>,
> > +       <270950400>,
> > +       <393216000>;
> > +
> > + dai-link-0 {
> > + sound-dai = <&aiu AIU_CPU CPU_I2S_FIFO>;
> > + };
> > +
> > + dai-link-1 {
> > + sound-dai = <&aiu AIU_CPU CPU_I2S_ENCODER>;
> > + dai-format = "i2s";
> > + mclk-fs = <256>;
> > +
> > + codec-0 {
> > + sound-dai = <&aiu AIU_HDMI CTRL_I2S>;
> > + };
> > +
> > + codec-1 {
> > + sound-dai = <&aiu AIU_ACODEC CTRL_I2S>;
> > + };
> > + };
> > +
> > + dai-link-2 {
> > + sound-dai = <&aiu AIU_HDMI CTRL_OUT>;
> > +
> > + codec-0 {
> > + sound-dai = <&hdmi_tx>;
> > + };
> > + };
> > +
> > + dai-link-3 {
> > + sound-dai = <&aiu AIU_ACODEC CTRL_OUT>;
> > +
> > + codec-0 {
> > + sound-dai = <&acodec>;
> > + };
> > + };
> > + };
> > +};
> > +
> > +&acodec {
> > + AVDD-supply = <&vddio_ao18>;
> > + status = "okay";
> > +};
> > +
> > +&aiu {
> > + status = "okay";
> > +};
> > +
> > +&cec_AO {
> > + status = "okay";
> > + pinctrl-0 = <&ao_cec_pins>;
> > + pinctrl-names = "default";
> > + hdmi-phandle = <&hdmi_tx>;
> > +};
> > +
> > +&ethmac {
> > + /* No Ethernet connector */
> > + status = "disabled";
> > +};
> > +
> > +&internal_phy {
> > + pinctrl-0 = <&eth_link_led_pins>, <&eth_act_led_pins>;
> > + pinctrl-names = "default";
> > +};
> > +
> > +&ir {
> > + /* No IR */
> > + status = "disabled";
> > +};
> > +
> > +&hdmi_tx {
> > + status = "okay";
> > + pinctrl-0 = <&hdmi_hpd_pins>, <&hdmi_i2c_pins>;
> > + pinctrl-names = "default";
> > + hdmi-supply = <&vcc_5v>;
> > +};
> > +
> > +&hdmi_tx_tmds_port {
> > + hdmi_tx_tmds_out: endpoint {
> > + remote-endpoint = <&hdmi_connector_in>;
> > + };
> > +};
> > +
> > +&saradc {
> > + status = "okay";
> > + vref-supply = <&vddio_ao18>;
> > +};
> > +
> > +/* Wireless SDIO Module */
> > +&sd_emmc_a {
> > + status = "disabled";
> > + pinctrl-0 = <&sdio_pins>;
> > + pinctrl-1 = <&sdio_clk_gate_pins>;
> > + pinctrl-names = "default", "clk-gate";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + bus-width = <4>;
> > + cap-sd-highspeed;
> > + max-frequency = <50000000>;
> > +
> > + non-removable;
> > + disable-wp;
> > +
> > + /* WiFi firmware requires power to be kept while in suspend */
> > + keep-power-in-suspend;
> > +
> > + mmc-pwrseq = <&sdio_pwrseq>;
> > +
> > + vmmc-supply = <&vddao_3v3>;
> > + vqmmc-supply = <&vddio_boot>;
>
> If you add a generic SDIO node here board variants will have working
> WiFi regardless of the module used. The idea is used here:
>
> https://patchwork.kernel.org/project/linux-amlogic/patch/20241127043358.3799737-1-christianshewitt@gmail.com/
>

But not all modules are supported in mainline at the moment, that's what I 
meant by "out of scope for this patchset".

> > +};
> > +
> > +/* eMMC */
> > +&sd_emmc_c {
> > + status = "okay";
> > + pinctrl-0 = <&emmc_pins>, <&emmc_ds_pins>;
> > + pinctrl-1 = <&emmc_clk_gate_pins>;
> > + pinctrl-names = "default", "clk-gate";
> > +
> > + bus-width = <8>;
> > + cap-mmc-highspeed;
> > + max-frequency = <200000000>;
> > + non-removable;
> > + disable-wp;
> > + mmc-ddr-1_8v;
> > + mmc-hs200-1_8v;
> > +
> > + mmc-pwrseq = <&emmc_pwrseq>;
> > + vmmc-supply = <&vcc_3v3>;
> > + vqmmc-supply = <&vddio_boot>;
> > +};
> > +
> > +&pwm_ef {
> > + status = "okay";
> > + pinctrl-0 = <&pwm_e_pins>;
> > + pinctrl-names = "default";
> > + clocks = <&clkc CLKID_FCLK_DIV4>;
> > + clock-names = "clkin0";
> > +};
> > +
> > +/* This is connected to the Bluetooth module: */
> > +&uart_A {
> > + status = "okay";
> > + pinctrl-0 = <&uart_a_pins>, <&uart_a_cts_rts_pins>;
> > + pinctrl-names = "default";
> > + uart-has-rtscts;
> > +};
> > +
> > +&uart_AO {
> > + status = "okay";
> > + pinctrl-0 = <&uart_ao_a_pins>;
> > + pinctrl-names = "default";
> > +};
> > +
> > +&usb {
> > + status = "okay";
> > + dr_mode = "host";
> > +};
> > +
> > +&usb2_phy0 {
> > + phy-supply = <&vcc_5v>;
> > +};
> > -- 
> > 2.47.1
> > 
> > 
> > _______________________________________________
> > linux-amlogic mailing list
> > linux-amlogic@...ts.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-amlogic

Powered by blists - more mailing lists