Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Aiqun Yu (Maria)]

On 12/21/2023 4:57 PM, Krzysztof Kozlowski wrote:
> 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.
Do you have the previous discussion title/link that you are refereed 
here pls?
> 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

--
Thx and BRs,
Aiqun(Maria) Yu