Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <62d6931c.1c69fb81.8b9a9.8594@mx.google.com>
Date:   Tue, 19 Jul 2022 11:12:16 +0200
From:   Christian Marangi <ansuelsmth@gmail.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc:     Konrad Dybcio <konrad.dybcio@somainline.org>,
        Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] ARM: dts: qcom: ipq8064: reorganize node order and
 sort them
References: <20220718153815.29414-1-ansuelsmth@gmail.com>
 <7f2a4f21-5e07-9320-8f7b-573ccc562f43@linaro.org>
 <CAA8EJppCxrcQOtCDZvUX-CThGV7aZXYv__gz3KRBf28TCRTBEg@mail.gmail.com>
 <78230095-6b45-4536-f41d-12bb23308d34@linaro.org>
 <f625ccde-8ecd-c06d-e8b2-ecb51c9ac9b8@somainline.org>
 <62d686c0.1c69fb81.4a957.bf03@mx.google.com>
 <26b43f6d-2b35-aab7-f906-31458c1b824b@linaro.org>
 <62d68f9e.1c69fb81.985fb.30aa@mx.google.com>
 <a4c805c3-8d78-98bd-40be-204fe98e6153@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a4c805c3-8d78-98bd-40be-204fe98e6153@linaro.org>
Precedence: bulk

On Tue, Jul 19, 2022 at 01:14:07PM +0200, Krzysztof Kozlowski wrote:
> On 19/07/2022 10:57, Christian Marangi wrote:
> > On Tue, Jul 19, 2022 at 12:56:18PM +0200, Krzysztof Kozlowski wrote:
> >> On 19/07/2022 10:19, Christian Marangi wrote:
> >>> On Tue, Jul 19, 2022 at 12:22:24PM +0200, Konrad Dybcio wrote:
> >>>>
> >>>>
> >>>> On 19.07.2022 12:16, Krzysztof Kozlowski wrote:
> >>>>> On 19/07/2022 11:59, Dmitry Baryshkov wrote:
> >>>>>> On Tue, 19 Jul 2022 at 12:56, Krzysztof Kozlowski
> >>>>>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>>>>>
> >>>>>>> On 18/07/2022 17:38, Christian Marangi wrote:
> >>>>>>>> Reorganize node order and sort them by address.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> >>>>>>>> ---
> >>>>>>>>
> >>>>>>>> This was picked from for-next qcom branch [1]. Reorganize dtsi as requested.
> >>>>>>>>
> >>>>>>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/?h=for-next
> >>>>>>>
> >>>>>>> If this is picked by qcom branch, no need to resend it.
> >>>>>>>
> >>>>>>> I don't see value in such reshuffle. Reviewing is not possible and you
> >>>>>>> did not mention tests (results should be equal).
> >>>>>>
> >>>>>> The value is usual for all the cleanups: make it follow the
> >>>>>> established practice.
> >>>>>
> >>>>> Are you sure this is established practice?
> >>>> Yes.
> >>>>
> >>>>  New DTSI files (see SC8280XP,
> >>>>> sm8450 although sc7280 looked ordered) do not always follow it, so why
> >>>>> imposing it for existing code?
> >>>> Perhaps it slipped through review.. Partially my bad.
> >>>>
> >>>>
> >>>> Such reshuffle can cause conflicts thus
> >>>>> stops parallel development. Review is close to impossible...
> >>>> Almost any addition or removal also causes conflicts, because git is
> >>>> not as smart as we would like it to be. If the commit is structured
> >>>> properly (i.e. it *only* changes the order and nothing else),
> >>>> decompiling the dtbs before and after applying it and using a tool
> >>>> like meld that can find similar chunks of text at different offsets
> >>>> review is definitely possible, though not very pleasant (you can't
> >>>> just diff them, as order is preserved & phandles change due to that)
> >>>> as you have to look at it manually and can't tell much by just taking
> >>>> a look at the email.
> >>>>
> >>>
> >>> Can you give me an example of such tool? So I can put these data in the
> >>> commit description. I have to rebase this anyway as more changes got
> >>> merged so it might be a good idea to add more info about how this won't
> >>> make actualy changes.
> >>>
> >>
> >> scripts/dtc/dtx_diff
> >> fdtdump + diff
> >>
> >> There should be an empty diff from at least one of methods above. If you
> >> have a difference, I am not sure how can we verify this.
> >>
> >>
> >> Best regards,
> >> Krzysztof
> > 
> > This [1] is the output of dtx_diff.
> > The file diff is just phanle change that are related to the node order.
> > 
> > Seems a nice way to verify if the shuffle didn't drop any node.
> 
> Therefore run the second method - fdtdump.
>

The output I have with fdtdump keeps the original node order so it doesn't
produce correct correct diff as it will find difference with moved
nodes. (unless i'm calling fdtdump or diff with the wrong args)

> Best regards,
> Krzysztof

-- 
	Ansuel