| Message ID | 20240430172520.535179-1-robh@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | dt-bindings: mfd: aspeed: Drop 'oneOf' for pinctrl node | expand |
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
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
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@linux.ibm.com/
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 [李琼斯]
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
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(-)