[PATCH] dt-bindings: mfd: aspeed: Drop 'oneOf' for pinctrl node

[PATCH] dt-bindings: mfd: aspeed: Drop 'oneOf' for pinctrl node

[Rob Herring]

The use of 'oneOf' to include 1 of 3 possible child node schemas results
in error messages containing the actual error message(s) for the correct
SoC buried in the tons of error messages from the 2 schemas that don't
apply. It also causes the pinctrl schema to be applied twice as it will
be applied when the compatible matches.

All that's really needed in the parent schema is to ensure one of the
possible compatible strings is present in the pinctrl node so that its
schema will be applied separately.

Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
 .../bindings/mfd/aspeed,ast2x00-scu.yaml         | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
index 1689b986f441..86ee69c0f45b 100644
--- a/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
+++ b/Documentation/devicetree/bindings/mfd/aspeed,ast2x00-scu.yaml
@@ -47,10 +47,18 @@ patternProperties:
     type: object
 
   '^pinctrl(@[0-9a-f]+)?$':
-    oneOf:
-      - $ref: /schemas/pinctrl/aspeed,ast2400-pinctrl.yaml
-      - $ref: /schemas/pinctrl/aspeed,ast2500-pinctrl.yaml
-      - $ref: /schemas/pinctrl/aspeed,ast2600-pinctrl.yaml
+    type: object
+    additionalProperties: true
+    properties:
+      compatible:
+        contains:
+          enum:
+            - aspeed,ast2400-pinctrl
+            - aspeed,ast2500-pinctrl
+            - aspeed,ast2600-pinctrl
+
+    required:
+      - compatible
 
   '^interrupt-controller@[0-9a-f]+$':
     description: See Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2xxx-scu-ic.txt
-- 
2.43.0

[PATCH] dt-bindings: mfd: aspeed: Drop 'oneOf' for pinctrl node

[Andrew Jeffery]

On Tue, 2024-04-30 at 12:25 -0500, Rob Herring (Arm) wrote:
> The use of 'oneOf' to include 1 of 3 possible child node schemas results
> in error messages containing the actual error message(s) for the correct
> SoC buried in the tons of error messages from the 2 schemas that don't
> apply. It also causes the pinctrl schema to be applied twice as it will
> be applied when the compatible matches.
> 
> All that's really needed in the parent schema is to ensure one of the
> possible compatible strings is present in the pinctrl node so that its
> schema will be applied separately.

Thanks, I think it improves the readability of intent in the binding as
well.

To understand the impact better I grabbed the patch and diffed the
output of `make CHECK_DTBS=y aspeed/aspeed-ast2600-evb.dtb` before and
after applying it, but there was no significant difference in output.
Should that not demonstrate the errors being cleaned up? If not, what
should?

Andrew

[PATCH] dt-bindings: mfd: aspeed: Drop 'oneOf' for pinctrl node

[Rob Herring]

On Tue, Apr 30, 2024 at 7:40?PM Andrew Jeffery
<andrew@codeconstruct.com.au> wrote:
>
> On Tue, 2024-04-30 at 12:25 -0500, Rob Herring (Arm) wrote:
> > The use of 'oneOf' to include 1 of 3 possible child node schemas results
> > in error messages containing the actual error message(s) for the correct
> > SoC buried in the tons of error messages from the 2 schemas that don't
> > apply. It also causes the pinctrl schema to be applied twice as it will
> > be applied when the compatible matches.
> >
> > All that's really needed in the parent schema is to ensure one of the
> > possible compatible strings is present in the pinctrl node so that its
> > schema will be applied separately.
>
> Thanks, I think it improves the readability of intent in the binding as
> well.
>
> To understand the impact better I grabbed the patch and diffed the
> output of `make CHECK_DTBS=y aspeed/aspeed-ast2600-evb.dtb` before and
> after applying it, but there was no significant difference in output.
> Should that not demonstrate the errors being cleaned up? If not, what
> should?

Try it on one of the new boards posted in the last 1-2 days. It showed
up on my testing dtbs_check on patches. I didn't send a report because
there was so much noise in it.

Rob

[PATCH] dt-bindings: mfd: aspeed: Drop 'oneOf' for pinctrl node

[Andrew Jeffery]

On Wed, 2024-05-01 at 07:39 -0500, Rob Herring wrote:
> On Tue, Apr 30, 2024 at 7:40?PM Andrew Jeffery
> <andrew@codeconstruct.com.au> wrote:
> > 
> > On Tue, 2024-04-30 at 12:25 -0500, Rob Herring (Arm) wrote:
> > > The use of 'oneOf' to include 1 of 3 possible child node schemas results
> > > in error messages containing the actual error message(s) for the correct
> > > SoC buried in the tons of error messages from the 2 schemas that don't
> > > apply. It also causes the pinctrl schema to be applied twice as it will
> > > be applied when the compatible matches.
> > > 
> > > All that's really needed in the parent schema is to ensure one of the
> > > possible compatible strings is present in the pinctrl node so that its
> > > schema will be applied separately.
> > 
> > Thanks, I think it improves the readability of intent in the binding as
> > well.
> > 
> > To understand the impact better I grabbed the patch and diffed the
> > output of `make CHECK_DTBS=y aspeed/aspeed-ast2600-evb.dtb` before and
> > after applying it, but there was no significant difference in output.
> > Should that not demonstrate the errors being cleaned up? If not, what
> > should?
> 
> Try it on one of the new boards posted in the last 1-2 days. It showed
> up on my testing dtbs_check on patches. I didn't send a report because
> there was so much noise in it.

I tried with aspeed/aspeed-bmc-ibm-blueridge.dtb and yeah, it does
clean up a lot of barf. Nice. Thanks.

Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>

[1]: https://lore.kernel.org/lkml/20240429210131.373487-14-eajames at linux.ibm.com/

(subset) [PATCH] dt-bindings: mfd: aspeed: Drop 'oneOf' for pinctrl node

[Lee Jones]

On Tue, 30 Apr 2024 12:25:19 -0500, Rob Herring (Arm) wrote:
> The use of 'oneOf' to include 1 of 3 possible child node schemas results
> in error messages containing the actual error message(s) for the correct
> SoC buried in the tons of error messages from the 2 schemas that don't
> apply. It also causes the pinctrl schema to be applied twice as it will
> be applied when the compatible matches.
> 
> All that's really needed in the parent schema is to ensure one of the
> possible compatible strings is present in the pinctrl node so that its
> schema will be applied separately.
> 
> [...]

Applied, thanks!

[1/1] dt-bindings: mfd: aspeed: Drop 'oneOf' for pinctrl node
      commit: 8f553c27697bc5e11074e37dfc0def6e39e489b2

--
Lee Jones [???]