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

From: Rob Herring
Date: Tue Jul 12 2022 - 15:43:07 EST


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@xxxxxxxxxxxx>
> > > ---
> > > .../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