[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Yk8Uws1/Uia1B4Ok@google.com>
Date: Thu, 7 Apr 2022 09:43:46 -0700
From: Matthias Kaehlcke <mka@...omium.org>
To: Mars Chen <chenxiangrui@...qin.corp-partner.google.com>
Cc: agross@...nel.org, Bjorn Andersson <bjorn.andersson@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [v2]arm64: dts: qcom: Add sc7180-gelarshie
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@huaqin.corp-partner.google.com/
v2 dupe (?): https://patchwork.kernel.org/project/linux-arm-msm/patch/20220406074707.2393-1-chenxiangrui@huaqin.corp-partner.google.com/
v2: https://patchwork.kernel.org/project/linux-arm-msm/patch/20220406073756.2041-1-chenxiangrui@huaqin.corp-partner.google.com/
v1: https://patchwork.kernel.org/project/linux-arm-msm/patch/20220330090947.9100-1-chenxiangrui@huaqin.corp-partner.google.com/
> Add device tree for Gelarshie, a trogdor variant
>
> Signed-off-by: Mars Chen <chenxiangrui@...qin.corp-partner.google.com>
> ---
> 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;
> + };
> + };
> +};
Powered by blists - more mailing lists