Re: [PATCH v3 3/3] arm64: dts: qcom: Add support for SONY Xperia X Performance / XZ / XZs (msm8996, Tone platform)

[Stephan Gerhold <stephan@gerhold.net>]

On Tue, Jun 08, 2021 at 05:27:36PM +0200, Konrad Dybcio wrote:
> From: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> 
> Add support for following boards:
> 
> - Xperia X Performance (dora)
> - Xperia XZ (kagura)
> - Xperia XZs (keyaki)
> 
> They are all based on the SONY Tone platform and feature largely similar hardware
> with the most obvious differences being lack of USB-C and ToF sensor on Dora and
> different camera sensor on Keyaki.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> ---
> Changes since v2:
> - Remove unneeded labels from the pinned(pun intended)-by-default pins
> 
>  arch/arm64/boot/dts/qcom/Makefile             |   6 +
>  .../msm8996-pmi8996-sony-xperia-tone-dora.dts |  11 +
>  ...sm8996-pmi8996-sony-xperia-tone-kagura.dts |  11 +
>  ...sm8996-pmi8996-sony-xperia-tone-keyaki.dts |  11 +
>  .../qcom/msm8996-sony-xperia-tone-dora.dts    |  27 +
>  .../qcom/msm8996-sony-xperia-tone-kagura.dts  |  15 +
>  .../qcom/msm8996-sony-xperia-tone-keyaki.dts  |  26 +
>  .../dts/qcom/msm8996-sony-xperia-tone.dtsi    | 959 ++++++++++++++++++
>  arch/arm64/boot/dts/qcom/msm8996.dtsi         |  12 +-
>  9 files changed, 1072 insertions(+), 6 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8996-pmi8996-sony-xperia-tone-dora.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8996-pmi8996-sony-xperia-tone-kagura.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8996-pmi8996-sony-xperia-tone-keyaki.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-dora.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-kagura.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-keyaki.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone.dtsi
> 
[...]
> diff --git a/arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone.dtsi b/arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone.dtsi
> new file mode 100644
> index 000000000000..1f71b3b2dc57
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone.dtsi
> @@ -0,0 +1,959 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2021, AngeloGioacchino Del Regno
> + *                     <angelogioacchino.delregno@somainline.org>
> + * Copyright (c) 2021, Konrad Dybcio <konrad.dybcio@somainline.org>
> + */
> +
> +#include "msm8996.dtsi"
> +#include "pm8994.dtsi"
> +#include "pmi8994.dtsi"
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
> +#include <dt-bindings/pinctrl/qcom,pmic-mpp.h>
> +
> +/delete-node/ &hdmi;
> +/delete-node/ &hdmi_phy;
> +/delete-node/ &mdp5_intf3_out;

Is it not enough to set those to status = "disabled"? Kind of strange
that you have to delete those entirely. I guess ideally "hdmi" should
even be disabled by default in the SoC device tree.

> +/delete-node/ &slpi_region;
> +/delete-node/ &venus_region;
> +/delete-node/ &zap_shader_region;
> +
> +/ {
> +	qcom,msm-id = <246 0x30001>; /* MSM8996 V3.1 (Final) */
> +	qcom,pmic-id = <0x20009 0x2000a 0 0>; /* PM8994 + PMI8994 */
> +	qcom,board-id = <8 0>;
> +
> +	chosen {
> +		/*
> +		 * Due to an unknown-for-a-few-years regression,
> +		 * SDHCI only works on MSM8996 in PIO (lame) mode.
> +		 */
> +		bootargs = "sdhci.debug_quirks=0x40 sdhci.debug_quirks2=0x4 maxcpus=2";
> +	};
> +
> +	reserved-memory {
> +		ramoops@a7f00000 {
> +			compatible = "ramoops";
> +			reg = <0 0xa7f00000 0 0x100000>;
> +			record-size = <0x20000>;
> +			console-size = <0x40000>;
> +			ftrace-size = <0x20000>;
> +			pmsg-size = <0x20000>;
> +			ecc-size = <16>;
> +		};
> +
> +		cont_splash_mem: memory@83401000 {
> +			reg = <0 0x83401000 0 0x23ff000>;
> +			no-map;
> +		};
> +
> +		zap_shader_region: gpu@90400000 {
> +			compatible = "shared-dma-pool";
> +			reg = <0x0 0x90400000 0x0 0x2000>;
> +			no-map;
> +		};
> +
> +		slpi_region: memory@90500000 {
> +			reg = <0 0x90500000 0 0xa00000>;
> +			no-map;
> +		};
> +
> +		venus_region: memory@90f00000 {
> +			reg = <0 0x90f00000 0 0x500000>;
> +			no-map;
> +		};
> +	};
> +
> +	panel_tvdd: tvdd-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "panel_tvdd";
> +		gpio = <&tlmm 50 GPIO_ACTIVE_HIGH>;

regulator-fixed is active-low without "enable-active-high;"
If that's what you want it's probably more clear to write
GPIO_ACTIVE_LOW. Otherwise, perhaps you forgot that property? :)

Stephan