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

[Joel Selvaraj]

Hi Krzysztof Kozlowski and Marijn Suijten

On 02/08/22 13:16, Krzysztof Kozlowski wrote:
> On 01/08/2022 20:53, Marijn Suijten wrote:
>> On 2022-08-01 16:55:11, Joel Selvaraj wrote:
>> Any summary what changed since v1?
> 
> +1 (although I already reviewed it)

Will add them in v3.

>> 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

I will work on v3 based on both of your comments.

Regards,
Joel Selvaraj