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

[Dmitry Baryshkov]

On Sat, Jul 13, 2024 at 04:07:13PM GMT, Nikita Travkin wrote:
> Nikita Travkin писал(а) 13.07.2024 15:37:
> > 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.
> >>>

> >>> 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
> > 
> 
> (...) except gah this makes things quite a bit more complicated since the
> device makes use of the "generic design" msm8939-pm8916.dtsi and duplicating
> that would be quite silly IMO...
> 
> I wonder if we can clarify things without making everything too complicated 
> by calling that dtsi "msm8929-opp.dtsi" and keeping it as extension for now,
> then if we find that msm8929 has more differences - we can unfold and refactor
> everything.
> 
> What do you think?

What about adding msm8929-pm8916.dtsi, which includes just the right
things? This might result in duplication with the existing files, but in
the end msm8939-pm8916 and msm8919-pm8916 are also very similar.

-- 
With best wishes
Dmitry