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;
Date:   Wed, 22 Jun 2022 17:26:54 +0200
From:   Johan Hovold <johan@kernel.org>
To:     Konrad Dybcio <konrad.dybcio@somainline.org>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Andy Gross <agross@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Manivannan Sadhasivam <mani@kernel.org>,
        Jassi Brar <jassisinghbrar@gmail.com>,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 4/6] arm64: dts: qcom: sc8280xp: Add reference device
Message-ID: <YrM0vhXpY3uWgprs@hovoldconsulting.com>
References: <20220622041224.627803-1-bjorn.andersson@linaro.org>
 <20220622041224.627803-5-bjorn.andersson@linaro.org>
 <099cc82f-d52f-315f-189d-bcc40c1afd49@somainline.org>
 <YrMccQXwsz/zC/gl@hovoldconsulting.com>
 <9d0c1897-195f-0548-ea5d-ffc35768f518@somainline.org>
 <b2c9faf0-95a8-772f-c211-f1599b35f8f8@linaro.org>
 <51965fa3-d146-70f1-2ad8-db6197989348@somainline.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <51965fa3-d146-70f1-2ad8-db6197989348@somainline.org>
Precedence: bulk

On Wed, Jun 22, 2022 at 05:10:50PM +0200, Konrad Dybcio wrote:
> 
> 
> On 22.06.2022 16:48, Krzysztof Kozlowski wrote:
> > On 22/06/2022 16:36, Konrad Dybcio wrote:
> >>
> >>
> >> On 22.06.2022 15:43, Johan Hovold wrote:
> >>> On Wed, Jun 22, 2022 at 02:33:02PM +0200, Konrad Dybcio wrote:
> >>>> On 22.06.2022 06:12, Bjorn Andersson wrote:
> >>>
> >>>>> +&qup2_i2c5 {
> >>>>> +	clock-frequency = <400000>;
> >>>>> +
> >>>>> +	pinctrl-names = "default";
> >>>>> +	pinctrl-0 = <&qup2_i2c5_default>, <&kybd_default>, <&tpad_default>;
> >>>>> +
> >>>>> +	status = "okay";
> >>>>> +
> >>>> I think all device DTs generally have 'status = "okay"' at the beginning. Should we change that?
> >>>>
> >>>
> >>> No, quite the opposite, status go at the end.
> >> Then all other device DTs should be updated, as in dts/qcom/
> >> everybody keeps it first in non-SoC/PMIC files.
> > 
> > The word "should" is a bit too much here, but I agree, we can update all
> > of them to match one, chosen approach.
> > 
> > However the location for "status" property is more important for the
> > definition of nodes in DTSI, because it's the least important piece
> > there and also kind of expected - here go properties + I disable it. For
> > me this is more important.

Right, and this is the argument for keeping status last, something which
is well defined.

If you look at some of the qcom dtsi it's hard to determine whether a
node is disabled or not because the status property does not actually go
"first" but is rather typically mixed up somewhere in the middle (or
upper part) of nodes.

> > For node redefinition in DTS, I see benefits in two approaches:
> > 1. Let me first enable the node and then configure it.
> > 2. Let me configure the node and enable it.

So for consistency, just put status last everywhere (dtsi and dts) and
be done with it.

> I looked around non-qcom device trees and it looks like the common
> consensus is 2. Although I personally visually prefer 1. and it's
> been used in all qcom arm64 DTs to date, I don't think there are any
> blockers for us to switch to 1. going forward to keep it consistent.

You mean inconsistent with the majority of dts? ;)

> That's if we want to clean up the existing ones, as changing the rules
> and not applying that to the older files will make for a huge mess as
> time goes on and will unnecessarily prolong the review process (as
> existing DTs are commonly a source of reference and people make
> certain choices based on those).

That's a fair point. Consistency is good, and dt snipped tends to be
copied, but it's not the end of the world to not update old dts either.

> I don't think the DTS specification or the Linux docs explicitly which
> one to choose though.

No, but a praxis has been developed over time (e.g. compatible first,
reg second, status last).

Johan