Message-ID: <6a4356e2-e201-4b87-bac0-056b29a07fc8@linaro.org>
Date: Thu, 21 Dec 2023 09:57:08 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: "Aiqun Yu (Maria)" <quic_aiquny@...cinc.com>,
 Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Tengfei Fan <quic_tengfan@...cinc.com>, andersson@...nel.org,
 konrad.dybcio@...aro.org, robh+dt@...nel.org,
 krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
 linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, kernel@...cinc.com
Subject: Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove
 address/size-cells from mdss_dsi1

On 21/12/2023 09:49, Aiqun Yu (Maria) wrote:
> For example:
> 
> --- a/Documentation/devicetree/bindings/display/msm/qcom,sm8550-mdss.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/qcom,sm8550-mdss.yaml
> 
> @@ -55,14 +50,7 @@ patternProperties:
>             - const: qcom,sm8350-dp
> 
>     "^dsi@[0-9a-f]+$":
> -    type: object
> -    additionalProperties: true
> -
> -    properties:
> -      compatible:
> -        items:
> -          - const: qcom,sm8550-dsi-ctrl
> -          - const: qcom,mdss-dsi-ctrl
> +    $ref: ../dsi-controller-main.yaml#
> 
> With above unified reference change, it will be easier for other 
> developers to reference bindings files next time.
> Also dsi@[0-9a-f] node in mdss node will be correctly fully described.

No, this does not make sense and allows anything as dsi. It is opposite
of what we want in bindings, so NAK.

>>
>>> In my opinion if the example have "#address-cells" "#size-cells", then
>>> it's better to also include "panel@0" with "reg = <0>" to not confuse.
>>
>> It is already there, see dsi-controller.yaml.
>>
>>>>> 3. Too many bindings files for driver "qcom,mdss-dsi-ctrl", shall we align them into 1 binding files.
>>>>
>>>> There is just one.
>>> Currently I mentioned bindings files was searched the compatible
>>> "qcom,mdss-dsi-ctrl", and find binding docs like "qcom,sm8550-mdss.yaml"
>>> "qcom,sm8450-mdss.yaml" etc.
>>> There is duplicate information on "qcom,sm8550-mdss.yaml" etc, while
>>> "qcom,mdss-common.yaml" is not common enough for my understanding.
>>
>> If you had compared the qcom,SOC-mdss.yaml, you would have seen that
>> they provide tight binding between compatible strings used for all the
>> subblocks. The `mdss-common.yaml` describes MDSS common properties. It
>> describes everything except the platform specifics. It can not be made
>> more common. And there is no duplication.
>>
>> If you think you can improve the bindings, please send the patches.
> I am thinking of a unified qcom,mdss.yaml instead of "qcom,*each 
> SOC*-mdss.yaml". I will try to have a patch.

I asked first of you to read previous discussions. If you still insist
on sending patch for this, it means you did not read them.

How you wrote this idea, sounds like exactly opposite of what we were
doing and what I was recommending few times on the list, so it is very
likely I will NAK it.

>> They must pass the `make dt_binding_check` check.
> Thx for the remind.

And follow bindings guidelines.

>>
>>>>> 4. enhance the dtc warning check if we still want to have "#address-cells" "#size-cells" even if there is no "panel@0" attached.
>>>>
>>>> In my opinion this is a way to go, if any. Did you include devicetree@ ML and the corresponding maintainers into the discussion?
>>> Already included devicetree@ ML at the very beginning.
>>
>> Good, thanks for the confirmation.
>>
>>> If the required properties part in each yaml is marked good enough, I
>>> think it can be an input for avoid unnecessary dtc warnings.
>>
>> Patches are welcome.
> Improving developer efficiency with unnecessary warnings is one of my 
> interest as well.
> First of all, I'd better to make sure "Required properties" attribute in 
> current bindings are good enough. Let me try to get back on this in a 

I don't understand why do you keep mentioning the "required properties".
They have nothing to do with any of this here.



Best regards,
Krzysztof

Powered by blists - more mailing lists