Re: [PATCH 3/3] arm64: dts: qcom: msm8939-wingtech-wt82918: Add Lenovo Vibe K5 devices

[Krzysztof Kozlowski]

On 12/07/2024 18:04, Nikita Travkin wrote:
> From: Adam Słaboń <asaillen@xxxxxxxxxxxxxx>
> 
> This commit introduces multiple hardware variants of Lenovo Vibe K5.
> 
> - A6020a40 (msm8929-wingtech-wt82918hd)
> - A6020a46/A6020l36 (msm8939-wingtech-wt82918)
> - A6020a40 S616 H39 (msm8939-wingtech-wt82918hd)
> 
> These devices are added with support for many features, notably:
> 
> - Basic features like USB, mmc/sd storage, wifi, buttons, leds;
> - Accelerometer;
> - Touchscreen;
> - Sound and modem.
> 
> Note that "HD" variant of K5 is based on msm8929 which is a lower bin
> of msm8939 SoC. A simple dtsi is added for this soc along with the new
> devices.
> 
> Unfortunately, despite the heavy similarities, the combination of minor
> differences between variants make them incompatible between each other.
> 
> Signed-off-by: Adam Słaboń <asaillen@xxxxxxxxxxxxxx>
> [Nikita: Minor cleanup, commit message]
> Signed-off-by: Nikita Travkin <nikita@xxxxxxx>
> ---
>  arch/arm64/boot/dts/qcom/Makefile                  |   3 +
>  .../boot/dts/qcom/msm8929-wingtech-wt82918hd.dts   |  17 ++
>  arch/arm64/boot/dts/qcom/msm8929.dtsi              |   5 +
>  .../boot/dts/qcom/msm8939-wingtech-wt82918.dts     |  16 ++
>  .../boot/dts/qcom/msm8939-wingtech-wt82918.dtsi    | 254 +++++++++++++++++++++
>  .../boot/dts/qcom/msm8939-wingtech-wt82918hd.dts   |  16 ++
>  6 files changed, 311 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index fd4c7c41ddc4..48ec781fa1d8 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -58,10 +58,13 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-wingtech-wt86518.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-wingtech-wt86528.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-wingtech-wt88047.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-yiming-uz801v3.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8929-wingtech-wt82918hd.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8939-huawei-kiwi.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8939-longcheer-l9100.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8939-samsung-a7.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8939-sony-xperia-kanuti-tulip.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8939-wingtech-wt82918.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8939-wingtech-wt82918hd.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8953-motorola-potter.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8953-xiaomi-daisy.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8953-xiaomi-mido.dtb
> diff --git a/arch/arm64/boot/dts/qcom/msm8929-wingtech-wt82918hd.dts b/arch/arm64/boot/dts/qcom/msm8929-wingtech-wt82918hd.dts
> new file mode 100644
> index 000000000000..f9a358e852f8
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8929-wingtech-wt82918hd.dts
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/dts-v1/;
> +
> +#include "msm8939-wingtech-wt82918.dtsi"
> +#include "msm8929.dtsi"
> +
> +/ {
> +	model = "Lenovo Vibe K5 (HD) (Wingtech WT82918)";
> +	compatible = "wingtech,wt82918hd", "qcom,msm8929";
> +	chassis-type = "handset";
> +};
> +
> +&touchscreen {
> +	touchscreen-size-x = <720>;
> +	touchscreen-size-y = <1280>;
> +};
> diff --git a/arch/arm64/boot/dts/qcom/msm8929.dtsi b/arch/arm64/boot/dts/qcom/msm8929.dtsi
> new file mode 100644
> index 000000000000..c3d1d1ace2f6
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8929.dtsi
> @@ -0,0 +1,5 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +&opp_table {
> +	/delete-node/ opp-550000000;
> +};

That's a very odd SoC DTSI.

SoCs DTSIs are not meant to be included as complementary, but rather as
full DTSI.

IOW, this is very confusing code and will confuse everyone reading it.


Best regards,
Krzysztof