On Tue, Mar 30, 2021 at 02:52:29PM +0530, mkrishn@xxxxxxxxxxxxxx wrote: > On 2021-03-29 08:49, Stephen Boyd wrote: > > Quoting mkrishn@xxxxxxxxxxxxxx (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@xxxxxxxxxxxxxx> > > > >> + > > > >> +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@xxxxxxxxxxxxxx/) Maybe I had a wrong assumption that you needed the child nodes too? > 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 This works if you want to use some, but not all properties in a referenced schema. If all apply or listing them all here is too much duplication (such as child nodes, but that's a judgement call), then use 'unevaluatedProperties'. unevaluatedProperties is also currently a nop because the underlying tools don't yet support it. So it won't catch any errors and those errors will all have to be fixed when the tools add support. Rob
