public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
To: Konrad Dybcio <konrad.dybcio@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 16:59:19 +0000	[thread overview]
Message-ID: <c1a2ba5b-4cd9-362b-5a4e-e95a6bf27b3e@linaro.org> (raw)
In-Reply-To: <20230304-topic-dsi_fixup-v3-1-b8565944d0e6@linaro.org>

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

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.

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.

- 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...

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

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

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

  reply	other threads:[~2023-03-04 16:59 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 [this message]
2023-03-04 17:35     ` Konrad Dybcio
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=c1a2ba5b-4cd9-362b-5a4e-e95a6bf27b3e@linaro.org \
    --to=bryan.odonoghue@linaro.org \
    --cc=airlied@gmail.com \
    --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=konrad.dybcio@linaro.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