Re: [PATCH v14 3/4] dt-bindings: msm: dsi: add yaml schemas for DSI PHY bindings

[mkrishn@codeaurora.org]

On 2021-03-29 08:49, Stephen Boyd wrote:
> Quoting mkrishn@codeaurora.org (2021-03-26 03:36:30)
>> On 2021-03-26 04:28, Stephen Boyd wrote:
>> > Quoting Krishna Manikandan (2021-03-25 05:01:00)
>> >> diff --git
>> >> a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
>> >> b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
>> >> new file mode 100644
>> >> index 0000000..4a26bef
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
>> >> @@ -0,0 +1,68 @@
>> >> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
>> >> +%YAML 1.2
>> >> +---
>> >> +$id: http://devicetree.org/schemas/display/msm/dsi-phy-10nm.yaml#
>> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> >> +
>> >> +title: Qualcomm Display DSI 10nm PHY
>> >> +
>> >> +maintainers:
>> >> +  - Krishna Manikandan <mkrishn@codeaurora.org>
>> >> +
>> >> +allOf:
>> >> +  - $ref: dsi-phy-common.yaml#
>> >> +
>> >> +properties:
>> >> +  compatible:
>> >> +    oneOf:
> [..]
>> >> and
>> >> +      connected to VDDA_MIPI_DSI_0_PLL_0P9 pin for sdm845 target
>> >> +
>> >> +required:
>> >> +  - compatible
>> >> +  - reg
>> >> +  - reg-names
>> >> +  - vdds-supply
>> >> +
>> >> +unevaluatedProperties: false
>> >
>> > additionalProperties: false instead? This comment applies to the other
>> > bindings in this patch.
>> 
>> Hi Stephen,
>> We are referencing dsi-phy-common.yaml in this file. Since the
>> properties of dsi-phy-common.yaml are applicable to this file also, I
>> added unevaluatedProperties: false. If we add additionalProperties:
>> false instead, then the properties of dsi-phy-common.yaml will not be
>> applicable here and this will throw an error if we add the properties
>> from dsi-phy-common.yaml in the example.
>> 
> 
> Does that matter? I was wondering about that and so I peeked at the
> qcom pinctrl binding and it seems to follow a similar design but 
> doesn't
> have unevaluatedProperties: false. Instead it has additionalProperies:
> false. See qcom,sc8180x-pinctrl.yaml for an example. So did you try it
> or does something say you can't do this?

Hi Stephen,
I had tried the same thing in one of my initial patches and I got a 
comment from Rob that we have to use unevaluatedProperties when we are 
referring another 
file(https://patchwork.kernel.org/project/linux-arm-msm/patch/1589868421-30062-1-git-send-email-mkrishn@codeaurora.org/)
In latest dt-schema tool, we will get error if we try to change it to 
additionalProperties: false.
For example, in this patch "#clock-cells' and '#phy-cells' are mentioned 
in dsi-phy-common.yaml and I am referring this file in 
dsi-phy-10nm.yaml. If I add
additionalProperties: false instead of unevaluatedProperties: false, I 
will get the error mentioned below.

I checked qcom,sc8180x-pinctrl.yaml that you had mentioned in the 
comment and this file is compiling without any issues even though it is 
using additionalProperties: false. But I see that the properties 
mentioned in the reference file (in this case, qcom,tlmm-common.yaml) 
are again declared in the main file qcom,sc8180x-pinctrl.yaml even 
though these are mentioned as required properties in the common yaml 
file. If I remove these properties from qcom,sc8180x-pinctrl.yaml, I can 
see the same error that I am getting for my file also if 
additionalProperties are used. If I follow the same approach , ie define 
the properties again in dsi-phy-10nm.yaml and add additionalProperties: 
false, I dont see any errors during check (working change mentioned 
below). Should I make this change for all the files?

Error logs:
mkrishn@mkrishn-linux:/local/mnt/workspace/linux-next-latest/linux-next$ 
  make dt_binding_check 
DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
   CHKDT   
Documentation/devicetree/bindings/processed-schema-examples.json
   SCHEMA  
Documentation/devicetree/bindings/processed-schema-examples.json
   DTEX    
Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.example.dts
   DTC     
Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.example.dt.yaml
   CHECK   
Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.example.dt.yaml
/local/mnt/workspace/linux-next-latest/linux-next/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.example.dt.yaml: 
dsi-phy@ae94400: '#clock-cells', '#phy-cells', 'clock-names', 'clocks' 
do not match any of the regexes: 'pinctrl-[0-9]+'
         From schema: 
/local/mnt/workspace/linux-next-latest/linux-next/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml

Working Change:
--- a/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml
@@ -30,6 +30,11 @@ properties:
        - const: dsi_phy_lane
        - const: dsi_pll

+  '#clock-cells': true
+  '#phy-cells': true
+  clocks: true
+  clock-names: true
+
    vdds-supply:
      description: |
        Connected to DSI0_MIPI_DSI_PLL_VDDA0P9 pin for sc7180 target and
@@ -41,7 +46,7 @@ required:
    - reg-names
    - vdds-supply

-unevaluatedProperties: false
+additionalProperties: false

Thanks,
Krishna