linux-kernel - Re: [PATCH v2 2/3] arm64: dts: qcom: sdm845-xiaomi-beryllium-common: move common nodes to a common dtsi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 2 Aug 2022 09:46:33 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Marijn Suijten <marijn.suijten@...ainline.org>,
        Joel Selvaraj <joel.selvaraj@...look.com>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        ~postmarketos/upstreaming@...ts.sr.ht, phone-devel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] arm64: dts: qcom: sdm845-xiaomi-beryllium-common:
 move common nodes to a common dtsi

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@...look.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