* Re: [PATCH] ARM64: dts: meson-gxl: add support for the Xiaomi Mi Box 3
2023-04-03 7:47 ` [PATCH] ARM64: dts: meson-gxl: add support for the Xiaomi Mi Box 3 Karl Chan
@ 2023-04-03 8:45 ` Christian Hewitt
2023-05-31 8:54 ` Neil Armstrong
1 sibling, 0 replies; 3+ messages in thread
From: Christian Hewitt @ 2023-04-03 8:45 UTC (permalink / raw)
To: Karl Chan; +Cc: linux-amlogic
> On 3 Apr 2023, at 11:47 am, Karl Chan <exkcmailist@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@getgoogleoff.me>
> Tested-by: Karl Chan <exxxxkc@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@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>;
> + };
> +};
> +
> +ðmac {
> + 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@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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] ARM64: dts: meson-gxl: add support for the Xiaomi Mi Box 3
2023-04-03 7:47 ` [PATCH] ARM64: dts: meson-gxl: add support for the Xiaomi Mi Box 3 Karl Chan
2023-04-03 8:45 ` Christian Hewitt
@ 2023-05-31 8:54 ` Neil Armstrong
1 sibling, 0 replies; 3+ messages in thread
From: Neil Armstrong @ 2023-05-31 8:54 UTC (permalink / raw)
To: Karl Chan, linux-amlogic
Hi,
On 03/04/2023 09:47, Karl Chan wrote:
> 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:
Drop the last ":"
> Signed-off-by: Karl Chan <exxxxkc@getgoogleoff.me>
> Tested-by: Karl Chan <exxxxkc@getgoogleoff.me>
You can drop this one, Signed-off-by means you tested it
> ---
> 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
This is an invalid patch format
> 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
> @@ -0,0 +1,142 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2023 exkc <exxxxkc@getgoogleoff.me>
> + * Based on meson-gxl-s905x-p212.dts
> + *
> + */
> +
> +/dts-v1/;
> +
> +#include "meson-gxl-s905x-p212.dtsi"
> +#include <dt-bindings/sound/meson-aiu.h>
> +
> +/ {
> + compatible = "xiaomi,once", "amlogic,s905x", "amlogic,meson-gxl";
> + 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";
> + 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>;
> + };
> +};
> +
> +ðmac {
> + status = "disabled";
> +};
> +
> +&usb {
> + status = "okay";
> + dr_mode = "host";
> +};
> +
> +/* This UART is brought out to the uarl pad on the pcb*/
> +&uart_AO {
> + status = "okay";
> +};
Seems you re-edited the P212 DT, but you replaces all tabs by spaces, please use tabs only
As-is it doesn't apply cleanly and doesn't build:
# b4 shazam a36dd7f2-52fc-4623-e503-a2aeeb04f6ac@yahoo.com
Grabbing thread from lore.kernel.org/all/a36dd7f2-52fc-4623-e503-a2aeeb04f6ac%40yahoo.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Added from v2: 1 patches
Analyzing 3 messages in the thread
Will use the latest revision: v2
You can pick other revisions using the -vN flag
Checking attestation on all messages, may take a moment...
---
✗ [PATCH v2] ARM64: dts: meson-gxl: add support for the Xiaomi Mi Box 3
+ Link: https://lore.kernel.org/r/8035a445-5a8c-9901-d69f-63d7daa7e2f5@yahoo.com
+ Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
✗ BADSIG: DKIM/yahoo.com
✓ Signed: DKIM/lists.infradead.org (From: exkcmailist@yahoo.com)
---
Total patches: 1
---
Applying: ARM64: dts: meson-gxl: add support for the Xiaomi Mi Box 3
Patch failed at 0001 ARM64: dts: meson-gxl: add support for the Xiaomi Mi Box 3
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
error: git diff header lacks filename information when removing 1 leading pathname component (line 13)
hint: Use 'git am --show-current-patch=diff' to see the failed patch
# git am --show-current-patch=diff | patch -p1
patching file arch/arm64/boot/dts/amlogic/Makefile
patch: **** malformed patch at line 18: meson-gxl-s905x-libretech-cc-v2.dtb
If I fix this:
patching file arch/arm64/boot/dts/amlogic/Makefile
Hunk #1 FAILED at 45.
1 out of 1 hunk FAILED -- saving rejects to file arch/arm64/boot/dts/amlogic/Makefile.rej
If I fix this:
arch/arm64/boot/dts/amlogic/meson-gxl-s905x-xiaomi-once.dts:14.1-2 syntax error
FATAL ERROR: Unable to parse input tree
Neil
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 3+ messages in thread