Received-SPF: pass (google.com: domain of linux-kernel+bounces-103757-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 14 Mar 2024 13:07:40 -0700
From: Elliot Berman <quic_eberman@quicinc.com>
To: Caleb Connolly <caleb.connolly@linaro.org>
CC: Amrit Anand <quic_amrianan@quicinc.com>, <robh@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>,
        <agross@kernel.org>, <andersson@kernel.org>,
        <konrad.dybcio@linaro.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <kernel@quicinc.com>, <peter.griffin@linaro.org>,
        <linux-riscv@lists.infradead.org>, <chrome-platform@lists.linux.dev>,
        <linux-mediatek@lists.infradead.org>, Simon Glass <sjg@chromium.org>
Subject: Re: Re: [PATCH v2 2/2] dt-bindings: qcom: Update DT bindings for
 multiple DT
Message-ID: <20240314113908471-0700.eberman@hu-eberman-lv.qualcomm.com>
Mail-Followup-To: Caleb Connolly <caleb.connolly@linaro.org>, 
	Amrit Anand <quic_amrianan@quicinc.com>, robh@kernel.org, krzysztof.kozlowski+dt@linaro.org, 
	conor+dt@kernel.org, agross@kernel.org, andersson@kernel.org, 
	konrad.dybcio@linaro.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, 
	linux-arm-msm@vger.kernel.org, kernel@quicinc.com, peter.griffin@linaro.org, 
	linux-riscv@lists.infradead.org, chrome-platform@lists.linux.dev, 
	linux-mediatek@lists.infradead.org, Simon Glass <sjg@chromium.org>
References: <1710418312-6559-1-git-send-email-quic_amrianan@quicinc.com>
 <1710418312-6559-3-git-send-email-quic_amrianan@quicinc.com>
 <f6f317d9-830d-4c38-998f-b229b3d9f95a@linaro.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <f6f317d9-830d-4c38-998f-b229b3d9f95a@linaro.org>

On Thu, Mar 14, 2024 at 02:20:38PM +0000, Caleb Connolly wrote:
> Hi Amrit,
> 
> On 14/03/2024 12:11, Amrit Anand wrote:
..
> >  
> > +examples:
> > +  - |
> > +    #include <dt-bindings/arm/qcom,ids.h>
> > +    / {
> > +         model = "Qualcomm Technologies, Inc. sc7280 IDP SKU1 platform";
> > +         compatible = "qcom,sc7280-idp", "google,senor", "qcom,sc7280";
> > +
> > +         #board-id-cells = <2>;
> > +         board-id = <QCOM_SOC_ID(SC7280) QCOM_SOC_REVISION(1)>,
> > +                    <QCOM_SOC_ID(SC7280) QCOM_SOC_REVISION(2)>,
> > +                    <QCOM_BOARD_ID(IDP, 1, 0) QCOM_BOARD_SUBTYPE(UFS, ANY, 1)>;
> > +         board-id-types = "qcom,soc-id",
> > +                          "qcom,soc-id",
> > +                          "qcom,board-id";
> Forgive me if this is a particularly cynical view, but this seems
> incredibly blatant, the "qcom,board-id" property is deprecated for
> various good reasons, just using a key/value map where "qcom,board-id"
> is a key doesn't change that. There are two main issues I have with the
> proposal here:
> 
> 1. This breaks backwards compatibility, millions of production devices
> with bootloaders that will never receive another update might be
> compatible with the downstream "qcom,board-id" property, but they won't
> work with this.
> 2. A top level board-id property that isn't namespaced implies that it
> isn't vendor specific, but the proposed implementation doesn't even
> pretend to be vendor agnostic.
> 

I agree with the concerns you listed.

One point I wanted to bring is that if you provide a boot image that has
only one DTB, all production Qualcomm bootloaders I'm aware of will use
that DTB so long as "qcom,board-id" isn't a mismatch. I believe this is
what most everyone is doing if using the DTBs from kernel.org. We'd like
to use an open standard for DTB selection and that would very likely be
incompatible with existing bootloaders that don't have whatever that
standard is.

> U-Boot also has some ideas around this issue, there you can pass in
> multiple DTBs and provide some board specific "best match" function.
> I think there's definitely some value in exposing this information, but
> there's no good reason to define the same data as `qcom,board-id` while
> breaking production bootloaders.

One concern we have with U-Boot's approach is that it's based on
metadata external to the DTB and, in our experience, makes it hard to
track which board goes to which DTB. This approach also isn't
necessarily portable to other image types/boot flows.

Thanks,
Elliot