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