lkml.org 
[lkml]   [2022]   [Aug]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2 2/3] arm64: dts: qcom: sdm845-xiaomi-beryllium-common: move common nodes to a common dtsi
From
On 01/08/2022 20:53, Marijn Suijten wrote:
> On 2022-08-01 16:55:11, Joel Selvaraj wrote:
>> Since there are two variants of Xiaomi Poco F1, move the common nodes from
>> Tianma variant into a new common dtsi. The EBBG variant will also inherit
>> the new common dtsi.
>>
>> Signed-off-by: Joel Selvaraj <joel.selvaraj@outlook.com>
>> ---
>
> Any summary what changed since v1?

+1 (although I already reviewed it)

>
>> .../qcom/sdm845-xiaomi-beryllium-common.dtsi | 595 ++++++++++++++++++
>> .../qcom/sdm845-xiaomi-beryllium-tianma.dts | 590 +----------------
>> 2 files changed, 598 insertions(+), 587 deletions(-)
>> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-common.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-common.dtsi
>> new file mode 100644
>> index 000000000000..83edcb1171f5
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-common.dtsi
>
> I haven't re-read what was discussed in v1, but doing it this way causes
> git to _not_ record this as a rename but instead state that everything
> has been removed from sdm845-xiaomi-beryllium-tianma.dts, and a new file
> sdm845-xiaomi-beryllium-common.dtsi was introduced with inconveniently
> almost identical contents (see the unnecessary size of the patch that
> follows).

The patch should be formatted a bit different. I agree that if combined
with first patch and proper settings (-M10% -C10%, optionally also
experiment with -B although here it looks not needed).

I reviewed the diff side-by-sie and there were differences (labels)
tricky to spot. If you generate the patch correctly, not much of review
is needed...

>
> Instead, I'd keep the original patch with a rename from
> sdm845-xiaomi-beryllium.dts to sdm845-xiaomi-beryllium-common.dtsi, and
> _also_ update the existing:
>
> dtb-$(CONFIG_ARCH_QCOM) += sdm845-xiaomi-beryllium.dtb
>
> in Makefile to match this rename so that it keeps compiling, even if
> that means we treat a .dtsi as a .dts which may (likely) not be treated
> correctly by existing build rules.
>
> If it doesn't - and this approach is probably frowned upon anyway - it
> is perhaps easiest to generalize sdm845-xiaomi-beryllium.dtb (as
> suggested above) _and_ introduce sdm845-xiaomi-beryllium-tianma.dtb
> _and_ update Makefile in a _single_ patch, such that everyting keeps
> compiling and stays consistent wrt how git treats renames. Later
> patches update the compatible and add the ebbg variant.
>
> - Marijn


Best regards,
Krzysztof

\
 
 \ /
  Last update: 2022-08-02 09:47    [W:0.054 / U:3.908 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site