Re: [PATCH] [v2]arm64: dts: qcom: Add sc7180-gelarshie

From: Matthias Kaehlcke
Date: Thu Apr 07 2022 - 12:44:02 EST

On Thu, Apr 07, 2022 at 03:54:26PM +0800, Mars Chen wrote:

> Subject: [PATCH] [v2]arm64: dts: qcom: Add sc7180-gelarshie

Krzysztof already pointed out that the subject is incorrect. Besides that
the version number also looks wrong. This is at least v3:

v3: this patch
v2 dupe (?): https://patchwork.kernel.org/project/linux-arm-msm/patch/20220406094156.3191-1-chenxiangrui@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
v2 dupe (?): https://patchwork.kernel.org/project/linux-arm-msm/patch/20220406074707.2393-1-chenxiangrui@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
v2: https://patchwork.kernel.org/project/linux-arm-msm/patch/20220406073756.2041-1-chenxiangrui@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
v1: https://patchwork.kernel.org/project/linux-arm-msm/patch/20220330090947.9100-1-chenxiangrui@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/

> Add device tree for Gelarshie, a trogdor variant
>
> Signed-off-by: Mars Chen <chenxiangrui@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
> ---
> arch/arm64/boot/dts/qcom/Makefile | 1 +
> .../dts/qcom/sc7180-trogdor-gelarshie-r0.dts | 15 +
> .../dts/qcom/sc7180-trogdor-gelarshie.dtsi | 280 ++++++++++++++++++
> 3 files changed, 296 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-gelarshie-r0.dts
> create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-gelarshie.dtsi
>
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index f9e6343acd03..cf8f88b065c3 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -57,6 +57,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-coachz-r1.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-coachz-r1-lte.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-coachz-r3.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-coachz-r3-lte.dtb
> +dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-gelarshie-r0.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-homestar-r2.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-homestar-r3.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-homestar-r4.dtb
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-gelarshie-r0.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-gelarshie-r0.dts
> new file mode 100644
> index 000000000000..027d6d563a5f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-gelarshie-r0.dts
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Gelarshie board device tree source
> + *
> + * Copyright 2022 Google LLC.
> + */
> +
> +/dts-v1/;
> +
> +#include "sc7180-trogdor-gelarshie.dtsi"
> +
> +/ {
> + model = "Google Gelarshie (rev0+)";
> + compatible = "google,gelarshie", "qcom,sc7180";
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-gelarshie.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-gelarshie.dtsi
> new file mode 100644
> index 000000000000..8758cafb2d89
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-gelarshie.dtsi
>
> ...
>
> +&sound {
> + compatible = "google,sc7180-gelarshie";

There is currently no device tree binding for this compatible string. Is
the gelarshie audio config different from that of coachz? If not the
compatible string "google,sc7180-coachz" should be used.

> + model = "sc7180-adau7002-max98357a";
> + audio-routing = "PDM_DAT", "DMIC";
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&dmic_clk_en>;
> +};
> +
> +&sound_multimedia0_codec {
> + sound-dai = <&adau7002>;
> +};
> +
> +/* PINCTRL - modifications to sc7180-trogdor.dtsi */
> +
> +&en_pp3300_dx_edp {
> + pinmux {
> + pins = "gpio67";
> + };
> +
> + pinconf {
> + pins = "gpio67";
> + };
> +};
> +
> +&ts_reset_l {
> + pinconf {
> + /*
> + * We want reset state by default and it will be up to the
> + * driver to disable this when it's ready.
> + */
> + output-low;
> + };
> +};
> +
> +/* PINCTRL - board-specific pinctrl */
> +
> +&tlmm {
> + gpio-line-names = "HUB_RST_L",

nit: to make this list more digestible you could add comments with
pin numbers for every 10th pin and an empty line to separate the
'pin groups' even more visually. See sc7280-herobrine-herobrine-r1.dts
for an example.

> + "AP_RAM_ID0",
> + "AP_SKU_ID2",
> + "AP_RAM_ID1",
> + "WF_CAM_EN2",
> + "AP_RAM_ID2",
> + "UF_CAM_EN",
> + "WF_CAM_EN",
> + "TS_RESET_L",
> + "TS_INT_L",
> + "",
> + "EDP_BRIJ_IRQ",
> + "AP_EDP_BKLTEN",
> + "UF_CAM_MCLK",
> + "WF_CAM_MCLK",
> + "EDP_BRIJ_I2C_SDA",
> + "EDP_BRIJ_I2C_SCL",
> + "UF_CAM_SDA",
> + "UF_CAM_SCL",
> + "WF_CAM_SDA",
> + "WF_CAM_SCL",
> + "",
> + "",
> + "AMP_EN",
> + "",
> + "",
> + "",
> + "",
> + "",
> + "WF_CAM_RST_L",
> + "UF_CAM_RST_L",
> + "AP_BRD_ID2",
> + "BRIJ_SUSPEND",
> + "AP_BRD_ID0",
> + "AP_H1_SPI_MISO",
> + "AP_H1_SPI_MOSI",
> + "AP_H1_SPI_CLK",
> + "AP_H1_SPI_CS_L",
> + "BT_UART_CTS",
> + "BT_UART_RTS",
> + "BT_UART_TXD",
> + "BT_UART_RXD",
> + "H1_AP_INT_ODL",
> + "",
> + "UART_AP_TX_DBG_RX",
> + "UART_DBG_TX_AP_RX",
> + "",
> + "",
> + "FORCED_USB_BOOT",
> + "AMP_BCLK",
> + "AMP_LRCLK",
> + "AMP_DIN",
> + "",
> + "HP_BCLK",
> + "HP_LRCLK",
> + "HP_DOUT",
> + "",
> + "",
> + "AP_SKU_ID0",
> + "AP_EC_SPI_MISO",
> + "AP_EC_SPI_MOSI",
> + "AP_EC_SPI_CLK",
> + "AP_EC_SPI_CS_L",
> + "AP_SPI_CLK",
> + "AP_SPI_MOSI",
> + "AP_SPI_MISO",
> + /*
> + * AP_FLASH_WP_L is crossystem ABI. Schematics
> + * call it BIOS_FLASH_WP_L.
> + */
> + "AP_FLASH_WP_L",
> + "EN_PP3300_DX_EDP",
> + "AP_SPI_CS0_L",
> + "",
> + "",
> + "",
> + "",
> + "WLAN_SW_CTRL",
> + "BOOT_CONFIG_0",
> + "REPORT_SWITCH",
> + "",
> + "",
> + "",
> + "",
> + "",
> + "",
> + "",
> + "DMIC_CLK_EN",
> + "HUB_EN",
> + "",
> + "",
> + "",
> + "",
> + "",
> + "AP_SKU_ID1",
> + "AP_RST_REQ",
> + "",
> + "AP_BRD_ID1",
> + "AP_EC_INT_L",
> + "BOOT_CONFIG_1",
> + "",
> + "",
> + "BOOT_CONFIG_4",
> + "BOOT_CONFIG_2",
> + "",
> + "",
> + "",
> + "",
> + "EDP_BRIJ_EN",
> + "",
> + "",
> + "BOOT_CONFIG_3",
> + "WCI2_LTE_COEX_TXD",
> + "WCI2_LTE_COEX_RXD",
> + "",
> + "",
> + "",
> + "",
> + "FORCED_USB_BOOT_POL",
> + "AP_TS_PEN_I2C_SDA",
> + "AP_TS_PEN_I2C_SCL",
> + "DP_HOT_PLUG_DET",
> + "EC_IN_RW_ODL";
> +
> + dmic_clk_en: dmic_clk_en {

node names should use dashes as separators, i.e.:
dmic_clk_en: dmic-clk-en {

> + pinmux {
> + pins = "gpio83";
> + function = "gpio";
> + };
> +
> + pinconf {
> + pins = "gpio83";
> + drive-strength = <8>;
> + bias-pull-up;
> + };
> + };
> +};