Re: [PATCH v1 03/16] dt-bindings: power: mediatek: Refine multiple level power domain nodes

[Rob Herring <robh@kernel.org>]

On Wed, Jul 06, 2022 at 02:19:08PM +0800, Tinghan Shen wrote:
> On Tue, 2022-07-05 at 14:57 -0600, Rob Herring wrote:
> > On Mon, Jul 04, 2022 at 06:00:15PM +0800, Tinghan Shen wrote:
> > > Extract duplicated properties and support more levels of power
> > > domain nodes.
> > > 
> > > This change fix following error when do dtbs_check,
> > >     arch/arm64/boot/dts/mediatek/mt8195-evb.dtb: power-controller: power-domain@15:
> > > power-domain@16:power-domain@18: 'power-domain@19', 'power-domain@20', 'power-domain@21' do not
> > > match any of the regexes: 'pinctrl-[0-9]+'
> > > 	 From schema: Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > > 
> > > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > > ---
> > >  .../power/mediatek,power-controller.yaml      | 132 ++----------------
> > >  1 file changed, 12 insertions(+), 120 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > > b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > > index 135c6f722091..09a537a802b8 100644
> > > --- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > > +++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > > @@ -39,8 +39,17 @@ properties:
> > >    '#size-cells':
> > >      const: 0
> > >  
> > > +required:
> > > +  - compatible
> > > +
> > > +additionalProperties: false
> > > +
> > >  patternProperties:
> > >    "^power-domain@[0-9a-f]+$":
> > > +    $ref: "#/$defs/power-domain-node"
> > > +
> > > +$defs:
> > > +  power-domain-node:
> > >      type: object
> > >      description: |
> > >        Represents the power domains within the power controller node as documented
> > > @@ -98,127 +107,10 @@ patternProperties:
> > >          $ref: /schemas/types.yaml#/definitions/phandle
> > >          description: phandle to the device containing the SMI register range.
> > >  
> > > -    patternProperties:
> > > -      "^power-domain@[0-9a-f]+$":
> > > -        type: object
> > > -        description: |
> > > -          Represents a power domain child within a power domain parent node.
> > > -
> > > -        properties:
> > > -
> > > -          '#power-domain-cells':
> > > -            description:
> > > -              Must be 0 for nodes representing a single PM domain and 1 for nodes
> > > -              providing multiple PM domains.
> > > -
> > > -          '#address-cells':
> > > -            const: 1
> > > -
> > > -          '#size-cells':
> > > -            const: 0
> > > -
> > > -          reg:
> > > -            maxItems: 1
> > > -
> > > -          clocks:
> > > -            description: |
> > > -              A number of phandles to clocks that need to be enabled during domain
> > > -              power-up sequencing.
> > > -
> > > -          clock-names:
> > > -            description: |
> > > -              List of names of clocks, in order to match the power-up sequencing
> > > -              for each power domain we need to group the clocks by name. BASIC
> > > -              clocks need to be enabled before enabling the corresponding power
> > > -              domain, and should not have a '-' in their name (i.e mm, mfg, venc).
> > > -              SUSBYS clocks need to be enabled before releasing the bus protection,
> > > -              and should contain a '-' in their name (i.e mm-0, isp-0, cam-0).
> > > -
> > > -              In order to follow properly the power-up sequencing, the clocks must
> > > -              be specified by order, adding first the BASIC clocks followed by the
> > > -              SUSBSYS clocks.
> > > -
> > > -          domain-supply:
> > > -            description: domain regulator supply.
> > > -
> > > -          mediatek,infracfg:
> > > -            $ref: /schemas/types.yaml#/definitions/phandle
> > > -            description: phandle to the device containing the INFRACFG register range.
> > > -
> > > -          mediatek,smi:
> > > -            $ref: /schemas/types.yaml#/definitions/phandle
> > > -            description: phandle to the device containing the SMI register range.
> > > -
> > > -        patternProperties:
> > > -          "^power-domain@[0-9a-f]+$":
> > > -            type: object
> > > -            description: |
> > > -              Represents a power domain child within a power domain parent node.
> > > -
> > > -            properties:
> > > +      required:
> > > +        - reg
> > >  
> > > -              '#power-domain-cells':
> > > -                description:
> > > -                  Must be 0 for nodes representing a single PM domain and 1 for nodes
> > > -                  providing multiple PM domains.
> > > -
> > > -              '#address-cells':
> > > -                const: 1
> > > -
> > > -              '#size-cells':
> > > -                const: 0
> > > -
> > > -              reg:
> > > -                maxItems: 1
> > > -
> > > -              clocks:
> > > -                description: |
> > > -                  A number of phandles to clocks that need to be enabled during domain
> > > -                  power-up sequencing.
> > > -
> > > -              clock-names:
> > > -                description: |
> > > -                  List of names of clocks, in order to match the power-up sequencing
> > > -                  for each power domain we need to group the clocks by name. BASIC
> > > -                  clocks need to be enabled before enabling the corresponding power
> > > -                  domain, and should not have a '-' in their name (i.e mm, mfg, venc).
> > > -                  SUSBYS clocks need to be enabled before releasing the bus protection,
> > > -                  and should contain a '-' in their name (i.e mm-0, isp-0, cam-0).
> > > -
> > > -                  In order to follow properly the power-up sequencing, the clocks must
> > > -                  be specified by order, adding first the BASIC clocks followed by the
> > > -                  SUSBSYS clocks.
> > > -
> > > -              domain-supply:
> > > -                description: domain regulator supply.
> > > -
> > > -              mediatek,infracfg:
> > > -                $ref: /schemas/types.yaml#/definitions/phandle
> > > -                description: phandle to the device containing the INFRACFG register range.
> > > -
> > > -              mediatek,smi:
> > > -                $ref: /schemas/types.yaml#/definitions/phandle
> > > -                description: phandle to the device containing the SMI register range.
> > > -
> > > -            required:
> > > -              - reg
> > > -
> > > -            additionalProperties: false
> > > -
> > > -        required:
> > > -          - reg
> > > -
> > > -        additionalProperties: false
> > > -
> > > -    required:
> > > -      - reg
> > > -
> > > -    additionalProperties: false
> > > -
> > > -required:
> > > -  - compatible
> > > -
> > > -additionalProperties: false
> > > +      additionalProperties: false
> > You now aren't checking more than 1 level because you have defined 
> > 'additionalProperties' to be a DT property. Check the indentation.
> > 
> > You need this in $defs/power-domain-node to recurse:
> > 
> >     additionalProperties:
> >       $ref: #/$defs/power-domain-node
> Hi Rob,
> 
> I get the following error after adding the 'additionalProperties' to $defs/power-domain-node.
> The same error occurs when I run dt_binding_check on power/renesas,sysc-rmobile.yaml, which has the
> similar property.
> 
> $ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/power/mediatek,power-
> controller.yaml O=out
> make[1]: Entering directory '/test/upstream-cros/src/third_party/kernel/v5.10/out'
>   LINT    Documentation/devicetree/bindings
>   CHKDT   Documentation/devicetree/bindings/processed-schema.json
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> /test/upstream-
> cros/src/third_party/kernel/v5.10/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
> : ignoring, error in schema: patternProperties: ^thermistor@: properties: adi,excitation-current-
> nanoamp
> /test/upstream-
> cros/src/third_party/kernel/v5.10/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-
> sor.yaml: ignoring, error in schema: allOf: 1: if: not: properties
> /test/upstream-
> cros/src/third_party/kernel/v5.10/Documentation/devicetree/bindings/mfd/mps,mp2629.yaml: ignoring,
> error parsing file
>   DTEX    Documentation/devicetree/bindings/power/mediatek,power-controller.example.dts
>   DTC     Documentation/devicetree/bindings/power/mediatek,power-controller.example.dtb
>   CHECK   Documentation/devicetree/bindings/power/mediatek,power-controller.example.dtb
> Traceback (most recent call last):
>   File "/test/.venv/py3.9/bin/dt-validate", line 173, in <module>
>     testtree = dtschema.load(filename, line_number=args.line_number)
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 913, in load
>     return [ dtschema.dtb.fdt_unflatten(f.read()) ]
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/dtb.py", line 463, in fdt_unflatten
>     p = dtschema.get_prop_types()
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 892, in get_prop_types
>     props = dtschema.extract_types(schema_cache)
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 883, in extract_types
>     _extract_subschema_types(props, sch, sch)
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 874, in
> _extract_subschema_types
> 
> [...snip...]
> 
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 803, in
> _extract_prop_type
>     _extract_prop_type(props, schema, propname, subschema)
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 794, in
> _extract_prop_type
>     _extract_subschema_types(props, schema, subschema)
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 874, in
> _extract_subschema_types
>     _extract_prop_type(props, schema, p, v)
>   File "/test/.venv/py3.9/lib/python3.9/site-packages/dtschema/lib.py", line 790, in
> _extract_prop_type
>     if not isinstance(subschema, dict):
> RecursionError: maximum recursion depth exceeded while calling a Python object
> make[1]: Leaving directory '/test/upstream-cros/src/third_party/kernel/v5.10/out'

Okay, I think you need something more like this that doesn't recurse 
infinitely:

patternProperties:
  "^power-domain@[0-9a-f]+$":
    $ref: #/$defs/power-domain-node

    unevaluatedProperties:
      $ref: #/$defs/power-domain-node

If you need a 3rd level of nodes:
      unevaluatedProperties:
        $ref: #/$defs/power-domain-node


Rob