From: Konrad Dybcio <konrad.dybcio@linaro.org>
To: Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
Rob Clark <robdclark@gmail.com>,
Abhinav Kumar <quic_abhinavk@quicinc.com>,
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
Sean Paul <sean@poorly.run>, David Airlie <airlied@gmail.com>,
Daniel Vetter <daniel@ffwll.ch>, Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Krishna Manikandan <quic_mkrishn@quicinc.com>
Cc: Rob Herring <robh@kernel.org>,
linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
freedreno@lists.freedesktop.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated compatible
Date: Sat, 4 Mar 2023 18:35:09 +0100 [thread overview]
Message-ID: <30798bd2-5805-45e6-92d2-a9df6fb52600@linaro.org> (raw)
In-Reply-To: <c1a2ba5b-4cd9-362b-5a4e-e95a6bf27b3e@linaro.org>
On 4.03.2023 17:59, Bryan O'Donoghue wrote:
> On 04/03/2023 15:55, Konrad Dybcio wrote:
>> The point of the previous cleanup was to disallow "qcom,mdss-dsi-ctrl"
>> alone. This however didn't quite work out and the property became
>> undocumented instead of deprecated. Fix that.
>>
>> Fixes: 0c0f65c6dd44 ("dt-bindings: msm: dsi-controller-main: Add compatible strings for every current SoC")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>> Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>> index f195530ae964..d534451c8f7f 100644
>> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>> @@ -35,7 +35,7 @@ properties:
>> - items:
>> - enum:
>> - qcom,dsi-ctrl-6g-qcm2290
>> - - const: qcom,mdss-dsi-ctrl
>> + - qcom,mdss-dsi-ctrl # This should always come with an SoC-specific compatible
>> deprecated: true
>> reg:
>>
>
> This change would make compatible = "qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl"; break though
Intended, they were never supposed to go together, as at the time
before this patchset (and its stated dependency) the fallback
would not be sufficient, the driver wouldn't even probe.
>
> Take this example, I'm going to use 8916 because its easy.
>
> If we apply your change to dsi-controller-main.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> index e75a3efe4dace..e93c16431f0a1 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> @@ -34,7 +34,7 @@ properties:
> - items:
> - enum:
> - dsi-ctrl-6g-qcm2290
> - - const: qcom,mdss-dsi-ctrl
> + - qcom,mdss-dsi-ctrl
> deprecated: true
>
> reg:
>
> and then make 8916 == compatible = "qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl";
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 0733c2f4f3798..7332b5f66a09d 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -1094,7 +1094,7 @@ mdp5_intf1_out: endpoint {
> };
>
> dsi0: dsi@1a98000 {
> - compatible = "qcom,msm8916-dsi-ctrl",
> + compatible = "dsi-ctrl-6g-qcm2290",
> "qcom,mdss-dsi-ctrl";
> reg = <0x01a98000 0x25c>;
> reg-names = "dsi_ctrl";
>
> arch/arm64/boot/dts/qcom/apq8016-sbc.dtb: dsi@1a98000: compatible: 'oneOf' conditional failed, one must be fixed:
> ['dsi-ctrl-6g-qcm2290', 'qcom,mdss-dsi-ctrl'] is too long
>
>
> so compatible = "qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl"; is now invalid, not deprecated.
Intended
>
> This change also makes compatible = "qcom,dsi-ctrl-6g-qcm2290" or compatible = "qcom,mdss-dsi-ctrl" standalone valid compatible which is again not what we want.
-ish, it's marked as deprecated but it is valid.
>
> - enum:
> - qcom,dsi-ctrl-6g-qcm2290
> - qcom,mdss-dsi-ctrl
>
> means either "qcom,dsi-ctrl-6g-qcm2290" or "qcom,mdss-dsi-ctrl" are valid compat strings...
Correct
>
> As an example if you apply your change and then change the msm8916.dtsi to the below
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> index e75a3efe4dace..e93c16431f0a1 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> @@ -34,7 +34,7 @@ properties:
> - items:
> - enum:
> - dsi-ctrl-6g-qcm2290
> - - const: qcom,mdss-dsi-ctrl
> + - qcom,mdss-dsi-ctrl
> deprecated: true
>
> reg:
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 0733c2f4f3798..829fbe05b5713 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -1094,8 +1094,7 @@ mdp5_intf1_out: endpoint {
> };
>
> dsi0: dsi@1a98000 {
> - compatible = "qcom,msm8916-dsi-ctrl",
> - "qcom,mdss-dsi-ctrl";
> + compatible = "qcom,mdss-dsi-ctrl";
> reg = <0x01a98000 0x25c>;
> reg-names = "dsi_ctrl";
>
> Then test it with
>
> make O=$BUILDDIR DT_DOC_CHECKER=$DT_DOC_CHECKER DT_EXTRACT_EX=$DT_EXTRACT_EX DT_MK_SCHEMA=$DT_MK_SCHEMA DT_CHECKER=$DT_CHECKER CHECKER_FLAGS=-W=1 CHECK_DTBS=y qcom/apq8016-sbc.dtb
(sidenote: you can just do
make ARCH=.. OUT=.. CHECK_DTBS=y qcom/apq8016-sbc.dtb
the tools are picked up automatically by Kbuild)
>
> you'll see no error. However if you just do this
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 0733c2f4f3798..829fbe05b5713 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -1094,8 +1094,7 @@ mdp5_intf1_out: endpoint {
> };
>
> dsi0: dsi@1a98000 {
> - compatible = "qcom,msm8916-dsi-ctrl",
> - "qcom,mdss-dsi-ctrl";
> + compatible = "qcom,mdss-dsi-ctrl";
> reg = <0x01a98000 0x25c>;
> reg-names = "dsi_ctrl";
>
>
> and run the same test you get
Yes, correct. It's valid but it's deprecated, so the bindings are
sane. Keep in mind there's an ABI-like aspect to this.
Konrad
>
> apq8016-sbc.dtb: dsi@1a98000: compatible: 'oneOf' conditional failed, one must be fixed:
> ['qcom,mdss-dsi-ctrl'] is too short
> 'qcom,mdss-dsi-ctrl' is not one of ['qcom,apq8064-dsi-ctrl', 'qcom,msm8916-dsi-ctrl', 'qcom,msm8953-dsi-ctrl', 'qcom,msm8974-dsi-ctrl', 'qcom,msm8996-dsi-ctrl', 'qcom,msm8998-dsi-ctrl', 'qcom,qcm2290-dsi-ctrl', 'qcom,sc7180-dsi-ctrl', 'qcom,sc7280-dsi-ctrl', 'qcom,sdm660-dsi-ctrl', 'qcom,sdm845-dsi-ctrl', 'qcom,sm8150-dsi-ctrl', 'qcom,sm8250-dsi-ctrl', 'qcom,sm8350-dsi-ctrl', 'qcom,sm8450-dsi-ctrl', 'qcom,sm8550-dsi-ctrl']
>
> ---
> bod
next prev parent reply other threads:[~2023-03-04 17:35 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-04 15:55 [PATCH v3 0/2] Fix up Qualcomm DSI bindings Konrad Dybcio
2023-03-04 15:55 ` [PATCH v3 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated compatible Konrad Dybcio
2023-03-04 16:59 ` Bryan O'Donoghue
2023-03-04 17:35 ` Konrad Dybcio [this message]
2023-03-04 17:45 ` Bryan O'Donoghue
2023-03-04 17:53 ` Bryan O'Donoghue
2023-03-06 10:05 ` Konrad Dybcio
2023-03-06 8:54 ` Krzysztof Kozlowski
2023-03-04 15:55 ` [PATCH v3 2/2] dt-bindings: display: msm: sm6115-mdss: Fix DSI compatible Konrad Dybcio
2023-03-06 8:57 ` Krzysztof Kozlowski
2023-03-06 10:06 ` Konrad Dybcio
2023-03-07 9:16 ` Krzysztof Kozlowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=30798bd2-5805-45e6-92d2-a9df6fb52600@linaro.org \
--to=konrad.dybcio@linaro.org \
--cc=airlied@gmail.com \
--cc=bryan.odonoghue@linaro.org \
--cc=daniel@ffwll.ch \
--cc=devicetree@vger.kernel.org \
--cc=dmitry.baryshkov@linaro.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=quic_abhinavk@quicinc.com \
--cc=quic_mkrishn@quicinc.com \
--cc=robdclark@gmail.com \
--cc=robh+dt@kernel.org \
--cc=robh@kernel.org \
--cc=sean@poorly.run \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox