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

[Konrad Dybcio <konrad.dybcio@linaro.org>]

On 12.07.2024 6:04 PM, Nikita Travkin wrote:
> From: Adam Słaboń <asaillen@protonmail.com>
> 
> 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@protonmail.com>
> [Nikita: Minor cleanup, commit message]
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
>  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 {

No way somebody called the gpu opp table "opp table"..

> +	/delete-node/ opp-550000000;

Looking at downstream, seems like there isn't a speedbin fuse for
this :(

[...]

> +
> +&blsp_i2c2 {
> +	status = "okay";
> +
> +	accelerometer@68 {
> +		compatible = "invensense,icm20608";
> +		reg = <0x68>;
> +
> +		pinctrl-0 = <&accelerometer_default>;
> +		pinctrl-names = "default";

interesting choice to stick pintrl before interrupts

> +
> +		interrupts-extended = <&tlmm 115 IRQ_TYPE_EDGE_FALLING>;
> +
> +		vdd-supply = <&pm8916_l17>;
> +		vddio-supply = <&pm8916_l6>;
> +
> +		mount-matrix = "-1", "0", "0",
> +				"0", "1", "0",
> +				"0", "0", "1";
> +	};
> +};

[...]

> +&pm8916_mpps {
> +	pwm_out: mpp4-state {
> +		pins = "mpp4";
> +		function = "digital";
> +		power-source = <PM8916_MPP_VPH>;
> +		output-low;
> +		qcom,dtest = <1>;

I think you meant qcom,dtest-output

looks good otherwise

Konrad