lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ