On 2021-03-31 01:12, Rob Herring wrote:
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
Thanks Rob for the clarification. I will make the changes accordingly.
Thanks,
Krishna
