[PATCH] ARM64: dts: meson-gxl: add support for the Xiaomi Mi Box 3

Christian Hewitt christianshewitt at gmail.com
Mon Apr 3 01:45:36 PDT 2023


> On 3 Apr 2023, at 11:47 am, Karl Chan <exkcmailist at yahoo.com> wrote:

Subject line doesn’t follow the format of previous submissions, e.g.

arm64: dts: meson-gxl: add support for Xiaomi Mi-Box 3
^ lowercase

You have also missed a bunch of required recipients. You should be
using ./scipts/get_maintainer.pl <patch> to find all the people to
add. I personally use a git alias to simplify the process of sending
patches to all the required people/lists.

> The Xiaomi Mi Box 3 is a TV box based on the Amlogic S905X chipset.
> There are two variants:
> - 2 GiB/16GIB
> - 1 GiB/8GIB
> 
> Both variants come with:
> - 802.11a/b/g/n/ac wifi (BCM4345)
> - HDMI , AV (CVBS) and spdif optical output
> - 1x USB (utilizing both USB ports provided by the SoC)
> 
> The board seems to be very similar to the P212 reference
> boards, which is why it includes meson-gxl-s905x-p212.dtsi:

^ Patch description should describe the contents of the patch or
the change being made so the “seems like” (opinion) working is
not appropriate. Keep it brief/factual about the patch:

The Xiaomi Mi Box 3 is an Android STB based on the P212 reference
design with the following specification:

1GB/2GB DDR3 RAM
8GB/16GB eMMC
<more specs>

> Signed-off-by: Karl Chan <exxxxkc at getgoogleoff.me>
> Tested-by: Karl Chan <exxxxkc at getgoogleoff.me>

^ Tested-by isn’t required if you’re the patch author/submitter.

> ---

NB: If you want to say “this is all a bit similar to an existing
board” .. add comments below the — here and it’s visible do the
maintainers but will not be included in the merge. If you are
submitting multiple patches add a —-cover-letter when generating
the patches and make comments in the cover letter for the series.

> arch/arm64/boot/dts/amlogic/Makefile          |   1 +
> .../amlogic/meson-gxl-s905x-xiaomi-once.dts   | 142 ++++++++++++++++++
> 2 files changed, 143 insertions(+)
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxl-s905x-xiaomi-once.dts
> 
> diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile
> index e213aeebb..904bb1e19 100644
> --- a/arch/arm64/boot/dts/amlogic/Makefile
> +++ b/arch/arm64/boot/dts/amlogic/Makefile
> @@ -45,6 +45,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-libretech-cc-v2.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-libretech-cc.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-nexbox-a95x.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-p212.dtb
> +dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-xiaomi-once.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-gxm-gt1-ultimate.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-gxm-khadas-vim2.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-gxm-mecool-kiii-pro.dtb
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-xiaomi-once.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-xiaomi-once.dts
> new file mode 100644
> index 000000000..6169c0dc0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-xiaomi-once.dts

Perhaps “meson-gxl-s905x-xiaomi-mibox3.dts” to better reflect
the box name?

> @@ -0,0 +1,142 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2023 exkc <exxxxkc at getgoogleoff.me>

^ Real name not nickname.

> + * Based on meson-gxl-s905x-p212.dts

^ If it’s mostly copy/paste from an existing dts then it should
reflect the original copyright too. Your copyright only applies
to the new/unique things you added.

> + *
> + */
> +
> +/dts-v1/;
> +
> +#include "meson-gxl-s905x-p212.dtsi"
> +#include <dt-bindings/sound/meson-aiu.h>
> +
> +/ {
> +    compatible = "xiaomi,once", "amlogic,s905x", "amlogic,meson-gxl";

^ The compatible binding must be submitted as a separate patch (as
it changes a file in another part of the kernel tree). I’d also
suggest “xiaomi,mibox3” or "xiaomi,mi-box3” (once?)

Also, from this point the dts is formatted with space indentation
instead of tabs. You should run ./checkpatch.pl <patch> to look for
formatting mistakes before (re)submitting.

> +    model = "Xiaomi Mi Box 3";
> +
> +    dio2133: analog-amplifier {
> +        compatible = "simple-audio-amplifier";
> +        sound-name-prefix = "AU2";
> +        VCC-supply = <&hdmi_5v>;
> +        enable-gpios = <&gpio GPIOH_5 GPIO_ACTIVE_HIGH>;
> +    };
> +
> +    cvbs-connector {
> +        compatible = "composite-video-connector";
> +
> +        port {
> +            cvbs_connector_in: endpoint {
> +                remote-endpoint = <&cvbs_vdac_out>;
> +            };
> +        };
> +    };
> +
> +    hdmi-connector {
> +        compatible = "hdmi-connector";
> +        type = "a";
> +
> +        port {
> +            hdmi_connector_in: endpoint {
> +                remote-endpoint = <&hdmi_tx_tmds_out>;
> +            };
> +        };
> +    };
> +
> +    sound {
> +        compatible = "amlogic,gx-sound-card";
> +        model = "XIAOMI-ONCE";

Perhaps XIAOMI-MI-BOX3 or some (short) variant?

> +        audio-aux-devs = <&dio2133>;
> +        audio-widgets = "Line", "Lineout";
> +        audio-routing = "Lineout", "AU2 OUTL",
> +                "Lineout", "AU2 OUTR";
> +        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>;
> +};
> +
> +&cvbs_vdac_port {
> +    cvbs_vdac_out: endpoint {
> +        remote-endpoint = <&cvbs_connector_in>;
> +    };
> +};
> +
> +&hdmi_tx {
> +    status = "okay";
> +    pinctrl-0 = <&hdmi_hpd_pins>, <&hdmi_i2c_pins>;
> +    pinctrl-names = "default";
> +    hdmi-supply = <&hdmi_5v>;
> +};
> +
> +&hdmi_tx_tmds_port {
> +    hdmi_tx_tmds_out: endpoint {
> +        remote-endpoint = <&hdmi_connector_in>;
> +    };
> +};
> +
> +&ethmac {
> +    status = "disabled";
> +};
> +
> +&usb {
> +    status = "okay";
> +    dr_mode = "host";
> +};
> +
> +/* This UART is brought out to the uarl pad on the pcb*/
> +&uart_AO {
> +    status = "okay";
> +};
> -- 
> 2.39.2

I’ve now seen that you submitted a v2 patch, along with a second
board patch (marked as v2 but there is no v1). The same comments
about formatting etc. apply to both boards/patches.

Please make changes and push the patches to a public git repo so
we can pre-review and correct the basic formatting mistakes before
sending the next patch series to the list. Feel free to ping me
via IRC if anything isn’t clear, I’ll be online later in the day.

Christian

> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic




More information about the linux-amlogic mailing list