Krzysztof Kozlowski писал(а) 13.07.2024 15:02:
> 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.
>
I think Adam wanted to keep the common device dtsi based on msm8939.dtsi to
simplify things a bit. I was also a bit unsure if I should change how it's
done but decided to keep it as it was. I will rework the v2 so:
- msm8929.dtsi includes msm8939.dtsi
- devices .dts include needed soc.dtsi, then include the common.dtsi
- common.dtsi doesn't include any soc.dtsi
Thanks for the review!
Nikita
>
> Best regards,
> Krzysztof
