Date:   Thu, 9 Mar 2023 13:19:13 +0100
From:   Francesco Dolcini <francesco@dolcini.it>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Shawn Guo <shawnguo@kernel.org>
Cc:     Philippe Schenker <dev@pschenker.ch>, devicetree@vger.kernel.org,
        Sascha Hauer <s.hauer@pengutronix.de>,
        NXP Linux Team <linux-imx@nxp.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Pengutronix Kernel Team <kernel@pengutronix.de>,
        Frank Rowand <frowand.list@gmail.com>,
        linux-arm-kernel@lists.infradead.org,
        Fabio Estevam <festevam@gmail.com>,
        Philippe Schenker <philippe.schenker@toradex.com>,
        linux-kernel@vger.kernel.org,
        Marcel Ziswiler <marcel.ziswiler@toradex.com>
Subject: Re: [PATCH v1 03/25] arm64: dts: colibri-imx8x: Sort properties
Message-ID: <ZAnOwaXpcqI30jFi@francesco-nb.int.toradex.com>
References: <20230308125300.58244-1-dev@pschenker.ch>
 <20230308125300.58244-4-dev@pschenker.ch>
 <9d213504-d457-21a6-d467-41d8783d53d3@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <9d213504-d457-21a6-d467-41d8783d53d3@linaro.org>
Precedence: bulk

Hello Krzysztof, first thanks for your review.

Let's try to get some clarity on this with the help of Shawn.

On Wed, Mar 08, 2023 at 01:57:38PM +0100, Krzysztof Kozlowski wrote:
> On 08/03/2023 13:52, Philippe Schenker wrote:
> > From: Philippe Schenker <philippe.schenker@toradex.com>
> > 
> > Sort properties according to the following order and inside these
> > alphabetically.
> > 
> > 1. compatible
> > 2. reg
> > 3. standard properties
> > 4. specific properties
> > 5. status
> 
> Is this approved coding style for IMX DTS?

I 100% understand your concerns here.

With that said let me try to briefly explain the reasoning here, in
various threads we were asked in the past to move node around based on
some not 100% defined rules [0][1].

On Sun, 2023-01-29 at 11:19 +0800, Shawn Guo wrote:
>> +&usbotg1 {
>> +	adp-disable;
>> +	ci-disable-lpm;
>> +	hnp-disable;
>> +	over-current-active-low;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_usbotg1>;
>
>We generally want to put such generic properties before device specific
>ones.

In addition to that we find convenient to have properties sorted
alphabetically when no other rule is available, it just prevents any
kind of discussion, minimize merge conflicts and make comparing files
easier.

I also agree that the difference between "generic"/"specific" is fuzzy
at best.

With all that said ...

Shawn: What should we do? We can of course avoid any kind of re-ordering
from now on.

I am fine to be very pragmatic here, no-reordering on existing DTS
files, newly added DTS files we discuss whatever is the reasoning of the
reviewer/maintainer on a case-by-case basis.

Francesco

[0] https://lore.kernel.org/all/895e7df5-65e5-7b26-81d6-864e68957ab6@linaro.org/
[1] https://lore.kernel.org/all/20230129031932.GO20713@T480/