* [PATCH 0/2] dt-bindings: interrupt-controller: Convert Aspeed (C)VIC to DT schema
@ 2024-08-02 5:36 Andrew Jeffery
2024-08-02 5:36 ` [PATCH 1/2] dt-bindings: interrupt-controller: aspeed,ast2400-vic: Convert " Andrew Jeffery
2024-08-02 5:36 ` [PATCH 2/2] dt-bindings: misc: aspeed,ast2400-cvic: " Andrew Jeffery
0 siblings, 2 replies; 10+ messages in thread
From: Andrew Jeffery @ 2024-08-02 5:36 UTC (permalink / raw)
To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Joel Stanley, Andrew Jeffery
Cc: linux-kernel, devicetree, linux-arm-kernel, linux-aspeed
Hello,
This short series converts the Aspeed VIC and CVIC bindings over to DT
schema. The CVIC has historically been documented under "misc" as it's
the interrupt controller for the Coldfire co-processor embedded in the
SoCs, and not for the main ARM core. Regardless, I've updated both in
this series.
I tried to document the historical oddities and conversion quirks in the
commit messages where appropriate.
Please review!
Andrew
---
Andrew Jeffery (2):
dt-bindings: interrupt-controller: aspeed,ast2400-vic: Convert to DT schema
dt-bindings: misc: aspeed,ast2400-cvic: Convert to DT schema
.../interrupt-controller/aspeed,ast2400-vic.txt | 23 --------
.../interrupt-controller/aspeed,ast2400-vic.yaml | 63 ++++++++++++++++++++++
.../bindings/misc/aspeed,ast2400-cvic.yaml | 60 +++++++++++++++++++++
.../devicetree/bindings/misc/aspeed,cvic.txt | 35 ------------
4 files changed, 123 insertions(+), 58 deletions(-)
---
base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b
change-id: 20240802-dt-warnings-irq-aspeed-dt-schema-5f5efd5d431a
Best regards,
--
Andrew Jeffery <andrew@codeconstruct.com.au>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] dt-bindings: interrupt-controller: aspeed,ast2400-vic: Convert to DT schema 2024-08-02 5:36 [PATCH 0/2] dt-bindings: interrupt-controller: Convert Aspeed (C)VIC to DT schema Andrew Jeffery @ 2024-08-02 5:36 ` Andrew Jeffery 2024-08-06 6:07 ` Krzysztof Kozlowski 2024-08-02 5:36 ` [PATCH 2/2] dt-bindings: misc: aspeed,ast2400-cvic: " Andrew Jeffery 1 sibling, 1 reply; 10+ messages in thread From: Andrew Jeffery @ 2024-08-02 5:36 UTC (permalink / raw) To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery Cc: linux-kernel, devicetree, linux-arm-kernel, linux-aspeed Squash warnings such as: arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-galaxy100.dtb: /ahb/interrupt-controller@1e6c0080: failed to match any schema with compatible: ['aspeed,ast2400-vic'] The YAML DT schema defines an optional property, valid-sources, which was not previously described in the prose binding. It is added to document existing practice in the Aspeed devicetrees. Unfortunately the property seems to predate the requirement that vendor-specific properties be prefixed. Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au> --- .../interrupt-controller/aspeed,ast2400-vic.txt | 23 -------- .../interrupt-controller/aspeed,ast2400-vic.yaml | 63 ++++++++++++++++++++++ 2 files changed, 63 insertions(+), 23 deletions(-) diff --git a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-vic.txt b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-vic.txt deleted file mode 100644 index e3fea0758d25..000000000000 --- a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-vic.txt +++ /dev/null @@ -1,23 +0,0 @@ -Aspeed Vectored Interrupt Controller - -These bindings are for the Aspeed interrupt controller. The AST2400 and -AST2500 SoC families include a legacy register layout before a re-designed -layout, but the bindings do not prescribe the use of one or the other. - -Required properties: - -- compatible : "aspeed,ast2400-vic" - "aspeed,ast2500-vic" - -- interrupt-controller : Identifies the node as an interrupt controller -- #interrupt-cells : Specifies the number of cells needed to encode an - interrupt source. The value shall be 1. - -Example: - - vic: interrupt-controller@1e6c0080 { - compatible = "aspeed,ast2400-vic"; - interrupt-controller; - #interrupt-cells = <1>; - reg = <0x1e6c0080 0x80>; - }; diff --git a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-vic.yaml b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-vic.yaml new file mode 100644 index 000000000000..1ecbc55571e2 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-vic.yaml @@ -0,0 +1,63 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/aspeed,ast2400-vic.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Aspeed Vectored Interrupt Controller + +maintainers: + - Andrew Jeffery <andrew@codeconstruct.com.au> + +description: + The AST2400 and AST2500 SoC families include a legacy register layout before + a redesigned layout, but the bindings do not prescribe the use of one or the + other. + +properties: + compatible: + enum: + - aspeed,ast2400-vic + - aspeed,ast2500-vic + + interrupt-controller: true + + "#interrupt-cells": + const: 1 + description: + Specifies the number of cells needed to encode an interrupt source. It + must be 1 as the VIC has no configuration options for interrupt sources. + The single cell defines the interrupt number. + + valid-sources: + $ref: /schemas/types.yaml#/definitions/uint32-array + description: + One cell, bitmap of support sources for the implementation. + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + +required: + - compatible + - reg + - interrupt-controller + - "#interrupt-cells" + +allOf: + - $ref: /schemas/interrupt-controller.yaml + +additionalProperties: false + +examples: + - | + interrupt-controller@1e6c0080 { + compatible = "aspeed,ast2400-vic"; + reg = <0x1e6c0080 0x80>; + interrupt-controller; + #interrupt-cells = <1>; + }; + +... -- 2.39.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] dt-bindings: interrupt-controller: aspeed,ast2400-vic: Convert to DT schema 2024-08-02 5:36 ` [PATCH 1/2] dt-bindings: interrupt-controller: aspeed,ast2400-vic: Convert " Andrew Jeffery @ 2024-08-06 6:07 ` Krzysztof Kozlowski 2024-08-08 2:02 ` Andrew Jeffery 0 siblings, 1 reply; 10+ messages in thread From: Krzysztof Kozlowski @ 2024-08-06 6:07 UTC (permalink / raw) To: Andrew Jeffery, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley Cc: linux-kernel, devicetree, linux-arm-kernel, linux-aspeed On 02/08/2024 07:36, Andrew Jeffery wrote: > Squash warnings such as: > > arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-galaxy100.dtb: /ahb/interrupt-controller@1e6c0080: failed to match any schema with compatible: ['aspeed,ast2400-vic'] > > The YAML DT schema defines an optional property, valid-sources, which > was not previously described in the prose binding. It is added to > document existing practice in the Aspeed devicetrees. Unfortunately > the property seems to predate the requirement that vendor-specific > properties be prefixed. > > Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au> > + > +description: > + The AST2400 and AST2500 SoC families include a legacy register layout before > + a redesigned layout, but the bindings do not prescribe the use of one or the > + other. > + > +properties: > + compatible: > + enum: > + - aspeed,ast2400-vic > + - aspeed,ast2500-vic > + > + interrupt-controller: true > + > + "#interrupt-cells": > + const: 1 > + description: > + Specifies the number of cells needed to encode an interrupt source. It > + must be 1 as the VIC has no configuration options for interrupt sources. > + The single cell defines the interrupt number. > + > + valid-sources: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + description: > + One cell, bitmap of support sources for the implementation. maxItems: 2 What does "one cell" mean? uint32? DTS has two items. > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 Is this correct? DTS does not have parent interrupt controller for this device. > + > +required: > + - compatible > + - reg > + - interrupt-controller > + - "#interrupt-cells" > + > +allOf: > + - $ref: /schemas/interrupt-controller.yaml > + > +additionalProperties: false > + > +examples: > + - | > + interrupt-controller@1e6c0080 { > + compatible = "aspeed,ast2400-vic"; > + reg = <0x1e6c0080 0x80>; > + interrupt-controller; > + #interrupt-cells = <1>; Make the example complete - add valid-sources interupts. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] dt-bindings: interrupt-controller: aspeed,ast2400-vic: Convert to DT schema 2024-08-06 6:07 ` Krzysztof Kozlowski @ 2024-08-08 2:02 ` Andrew Jeffery 0 siblings, 0 replies; 10+ messages in thread From: Andrew Jeffery @ 2024-08-08 2:02 UTC (permalink / raw) To: Krzysztof Kozlowski, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley Cc: linux-kernel, devicetree, linux-arm-kernel, linux-aspeed On Tue, 2024-08-06 at 08:07 +0200, Krzysztof Kozlowski wrote: > On 02/08/2024 07:36, Andrew Jeffery wrote: > > Squash warnings such as: > > > > arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-galaxy100.dtb: /ahb/interrupt-controller@1e6c0080: failed to match any schema with compatible: ['aspeed,ast2400-vic'] > > > > The YAML DT schema defines an optional property, valid-sources, which > > was not previously described in the prose binding. It is added to > > document existing practice in the Aspeed devicetrees. Unfortunately > > the property seems to predate the requirement that vendor-specific > > properties be prefixed. > > > > Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au> > > > > + > > +description: > > + The AST2400 and AST2500 SoC families include a legacy register layout before > > + a redesigned layout, but the bindings do not prescribe the use of one or the > > + other. > > + > > +properties: > > + compatible: > > + enum: > > + - aspeed,ast2400-vic > > + - aspeed,ast2500-vic > > + > > + interrupt-controller: true > > + > > + "#interrupt-cells": > > + const: 1 > > + description: > > + Specifies the number of cells needed to encode an interrupt source. It > > + must be 1 as the VIC has no configuration options for interrupt sources. > > + The single cell defines the interrupt number. > > + > > + valid-sources: > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > + description: > > + One cell, bitmap of support sources for the implementation. > > maxItems: 2 Ack. > What does "one cell" mean? uint32? DTS has two items. Hah, I think that was a process error :) Two is correct here. I'll rework the description. > > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > Is this correct? DTS does not have parent interrupt controller for this > device. I'll removed it, it's not necessary. > > > + > > +required: > > + - compatible > > + - reg > > + - interrupt-controller > > + - "#interrupt-cells" > > + > > +allOf: > > + - $ref: /schemas/interrupt-controller.yaml > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + interrupt-controller@1e6c0080 { > > + compatible = "aspeed,ast2400-vic"; > > + reg = <0x1e6c0080 0x80>; > > + interrupt-controller; > > + #interrupt-cells = <1>; > > Make the example complete - add valid-sources interupts. > Ack. Thanks for the review. Andrew ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] dt-bindings: misc: aspeed,ast2400-cvic: Convert to DT schema 2024-08-02 5:36 [PATCH 0/2] dt-bindings: interrupt-controller: Convert Aspeed (C)VIC to DT schema Andrew Jeffery 2024-08-02 5:36 ` [PATCH 1/2] dt-bindings: interrupt-controller: aspeed,ast2400-vic: Convert " Andrew Jeffery @ 2024-08-02 5:36 ` Andrew Jeffery 2024-08-06 6:12 ` Krzysztof Kozlowski 2024-08-06 17:29 ` Rob Herring 1 sibling, 2 replies; 10+ messages in thread From: Andrew Jeffery @ 2024-08-02 5:36 UTC (permalink / raw) To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery Cc: linux-kernel, devicetree, linux-arm-kernel, linux-aspeed Address warnings such as: arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-galaxy100.dtb: interrupt-controller@1e6c0080: 'valid-sources' does not match any of the regexes: 'pinctrl-[0-9]+' and arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-galaxy100.dtb: /ahb/copro-interrupt-controller@1e6c2000: failed to match any schema with compatible: ['aspeed,ast2400-cvic', 'aspeed-cvic'] Note that the conversion to DT schema causes some further warnings to be emitted, because the Aspeed devicetrees are not in great shape. These new warnings are resolved in a separate series: https://lore.kernel.org/lkml/20240802-dt-warnings-bmc-dts-cleanups-v1-0-1cb1378e5fcd@codeconstruct.com.au/ Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au> --- .../bindings/misc/aspeed,ast2400-cvic.yaml | 60 ++++++++++++++++++++++ .../devicetree/bindings/misc/aspeed,cvic.txt | 35 ------------- 2 files changed, 60 insertions(+), 35 deletions(-) diff --git a/Documentation/devicetree/bindings/misc/aspeed,ast2400-cvic.yaml b/Documentation/devicetree/bindings/misc/aspeed,ast2400-cvic.yaml new file mode 100644 index 000000000000..3c85b4924c05 --- /dev/null +++ b/Documentation/devicetree/bindings/misc/aspeed,ast2400-cvic.yaml @@ -0,0 +1,60 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/misc/aspeed,ast2400-cvic.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Aspeed Coprocessor Vectored Interrupt Controller + +maintainers: + - Andrew Jeffery <andrew@codeconstruct.com.au> + +description: + The Aspeed AST2400 and AST2500 SoCs have a controller that provides interrupts + to the ColdFire coprocessor. It's not a normal interrupt controller and it + would be rather inconvenient to create an interrupt tree for it, as it + somewhat shares some of the same sources as the main ARM interrupt controller + but with different numbers. + + The AST2500 also supports a software generated interrupt. + +properties: + compatible: + items: + - enum: + - aspeed,ast2400-cvic + - aspeed,ast2500-cvic + - const: aspeed,cvic + + reg: + maxItems: 1 + + valid-sources: + $ref: /schemas/types.yaml#/definitions/uint32-array + description: + One cell, bitmap of support sources for the implementation. + + copro-sw-interrupts: + $ref: /schemas/types.yaml#/definitions/uint32-array + description: + A list of interrupt numbers that can be used as software interrupts from + the ARM to the coprocessor. + +required: + - compatible + - reg + - valid-sources + +allOf: + - $ref: /schemas/interrupt-controller.yaml# + +additionalProperties: false + +examples: + - | + interrupt-controller@1e6c2000 { + compatible = "aspeed,ast2500-cvic", "aspeed,cvic"; + reg = <0x1e6c2000 0x80>; + valid-sources = <0xffffffff>; + copro-sw-interrupts = <1>; + }; diff --git a/Documentation/devicetree/bindings/misc/aspeed,cvic.txt b/Documentation/devicetree/bindings/misc/aspeed,cvic.txt deleted file mode 100644 index d62c783d1d5e..000000000000 --- a/Documentation/devicetree/bindings/misc/aspeed,cvic.txt +++ /dev/null @@ -1,35 +0,0 @@ -* ASPEED AST2400 and AST2500 coprocessor interrupt controller - -This file describes the bindings for the interrupt controller present -in the AST2400 and AST2500 BMC SoCs which provides interrupt to the -ColdFire coprocessor. - -It is not a normal interrupt controller and it would be rather -inconvenient to create an interrupt tree for it as it somewhat shares -some of the same sources as the main ARM interrupt controller but with -different numbers. - -The AST2500 supports a SW generated interrupt - -Required properties: -- reg: address and length of the register for the device. -- compatible: "aspeed,cvic" and one of: - "aspeed,ast2400-cvic" - or - "aspeed,ast2500-cvic" - -- valid-sources: One cell, bitmap of supported sources for the implementation - -Optional properties; -- copro-sw-interrupts: List of interrupt numbers that can be used as - SW interrupts from the ARM to the coprocessor. - (AST2500 only) - -Example: - - cvic: copro-interrupt-controller@1e6c2000 { - compatible = "aspeed,ast2500-cvic"; - valid-sources = <0xffffffff>; - copro-sw-interrupts = <1>; - reg = <0x1e6c2000 0x80>; - }; -- 2.39.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] dt-bindings: misc: aspeed,ast2400-cvic: Convert to DT schema 2024-08-02 5:36 ` [PATCH 2/2] dt-bindings: misc: aspeed,ast2400-cvic: " Andrew Jeffery @ 2024-08-06 6:12 ` Krzysztof Kozlowski 2024-08-08 2:06 ` Andrew Jeffery 2024-08-06 17:29 ` Rob Herring 1 sibling, 1 reply; 10+ messages in thread From: Krzysztof Kozlowski @ 2024-08-06 6:12 UTC (permalink / raw) To: Andrew Jeffery, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley Cc: linux-kernel, devicetree, linux-arm-kernel, linux-aspeed On 02/08/2024 07:36, Andrew Jeffery wrote: > Address warnings such as: > > +description: > + The Aspeed AST2400 and AST2500 SoCs have a controller that provides interrupts > + to the ColdFire coprocessor. It's not a normal interrupt controller and it > + would be rather inconvenient to create an interrupt tree for it, as it > + somewhat shares some of the same sources as the main ARM interrupt controller > + but with different numbers. > + > + The AST2500 also supports a software generated interrupt. > + > +properties: > + compatible: > + items: > + - enum: > + - aspeed,ast2400-cvic > + - aspeed,ast2500-cvic > + - const: aspeed,cvic > + > + reg: > + maxItems: 1 > + > + valid-sources: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + description: > + One cell, bitmap of support sources for the implementation. maxItems: 1 (and drop "One cell" - no need to repeat constraints in free form text) BTW, for both bindings, I do not see any user in the kernel. Why is this property needed in the DTS? > + > + copro-sw-interrupts: > + $ref: /schemas/types.yaml#/definitions/uint32-array uint32? I do not see anywhere usage as an array. The in-kernel driver explicitly reads just uint32. Anyway, if this is supposed to stay as array, then min/maxItems. > + description: > + A list of interrupt numbers that can be used as software interrupts from > + the ARM to the coprocessor. > + > +required: > + - compatible > + - reg > + - valid-sources> + > +allOf: > + - $ref: /schemas/interrupt-controller.yaml# Best regards, Krzysztof ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] dt-bindings: misc: aspeed,ast2400-cvic: Convert to DT schema 2024-08-06 6:12 ` Krzysztof Kozlowski @ 2024-08-08 2:06 ` Andrew Jeffery 2024-08-08 3:52 ` Andrew Jeffery 0 siblings, 1 reply; 10+ messages in thread From: Andrew Jeffery @ 2024-08-08 2:06 UTC (permalink / raw) To: Krzysztof Kozlowski, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley Cc: linux-kernel, devicetree, linux-arm-kernel, linux-aspeed On Tue, 2024-08-06 at 08:12 +0200, Krzysztof Kozlowski wrote: > On 02/08/2024 07:36, Andrew Jeffery wrote: > > Address warnings such as: > > > > > > +description: > > + The Aspeed AST2400 and AST2500 SoCs have a controller that provides interrupts > > + to the ColdFire coprocessor. It's not a normal interrupt controller and it > > + would be rather inconvenient to create an interrupt tree for it, as it > > + somewhat shares some of the same sources as the main ARM interrupt controller > > + but with different numbers. > > + > > + The AST2500 also supports a software generated interrupt. > > + > > +properties: > > + compatible: > > + items: > > + - enum: > > + - aspeed,ast2400-cvic > > + - aspeed,ast2500-cvic > > + - const: aspeed,cvic > > + > > + reg: > > + maxItems: 1 > > + > > + valid-sources: > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > + description: > > + One cell, bitmap of support sources for the implementation. > > maxItems: 1 > (and drop "One cell" - no need to repeat constraints in free form text) Ack to both. > > BTW, for both bindings, I do not see any user in the kernel. Why is this > property needed in the DTS? Good question. This is a hangover from when benh was involved in the Aspeed kernel port. Given it's specified in the prose binding and the devicetrees contain the property I'll leaving it in for now, but I think it's something we could consider removing down the track. > > > + > > + copro-sw-interrupts: > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > uint32? I do not see anywhere usage as an array. The in-kernel driver > explicitly reads just uint32. You're right, and in the context of the hardware an array doesn't make sense here. I'll switch it to a uint32. Thanks for the review. Andrew ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] dt-bindings: misc: aspeed,ast2400-cvic: Convert to DT schema 2024-08-08 2:06 ` Andrew Jeffery @ 2024-08-08 3:52 ` Andrew Jeffery 0 siblings, 0 replies; 10+ messages in thread From: Andrew Jeffery @ 2024-08-08 3:52 UTC (permalink / raw) To: Krzysztof Kozlowski, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley Cc: linux-kernel, devicetree, linux-arm-kernel, linux-aspeed On Thu, 2024-08-08 at 11:36 +0930, Andrew Jeffery wrote: > On Tue, 2024-08-06 at 08:12 +0200, Krzysztof Kozlowski wrote: > > On 02/08/2024 07:36, Andrew Jeffery wrote: > > > Address warnings such as: > > > > > > > > > > +description: > > > + The Aspeed AST2400 and AST2500 SoCs have a controller that provides interrupts > > > + to the ColdFire coprocessor. It's not a normal interrupt controller and it > > > + would be rather inconvenient to create an interrupt tree for it, as it > > > + somewhat shares some of the same sources as the main ARM interrupt controller > > > + but with different numbers. > > > + > > > + The AST2500 also supports a software generated interrupt. > > > + > > > +properties: > > > + compatible: > > > + items: > > > + - enum: > > > + - aspeed,ast2400-cvic > > > + - aspeed,ast2500-cvic > > > + - const: aspeed,cvic > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + valid-sources: > > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > > + description: > > > + One cell, bitmap of support sources for the implementation. > > > > maxItems: 1 > > (and drop "One cell" - no need to repeat constraints in free form text) > > Ack to both. > > > > > BTW, for both bindings, I do not see any user in the kernel. Why is this > > property needed in the DTS? > > Good question. This is a hangover from when benh was involved in the > Aspeed kernel port. > > Given it's specified in the prose binding and the devicetrees contain > the property I'll leaving it in for now, but I think it's something we > could consider removing down the track. > > > > > > + > > > + copro-sw-interrupts: > > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > > > uint32? I do not see anywhere usage as an array. The in-kernel driver > > explicitly reads just uint32. > > You're right, and in the context of the hardware an array doesn't make > sense here. I'll switch it to a uint32. > Actually, on further inspection, the description says the property should contain a list of interrupt _numbers_ (the hardware exposes 32 software drive-able interrupt bits). Given aspeed-g5.dtsi only lists the single value '1' the intent feels somewhat murky. When I wrote the reply above I had in my head that it was a bitmask like valid-sources but the description suggests that's not true. I'm not sure the described behaviour was chosen to be different to valid-sources, however, for the co-processor, index 1 is an interrupt from the main ARM core. Perhaps it felt less tedious just to write the index and not the mask. I'm going to backtrack on my reply above leave this as uint32-array with `maxItems: 32` to meet the intent of the description. If there are problems down the track we can consider the driver to have a bug with respect to the binding (I don't think there's much risk there though). Andrew ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] dt-bindings: misc: aspeed,ast2400-cvic: Convert to DT schema 2024-08-02 5:36 ` [PATCH 2/2] dt-bindings: misc: aspeed,ast2400-cvic: " Andrew Jeffery 2024-08-06 6:12 ` Krzysztof Kozlowski @ 2024-08-06 17:29 ` Rob Herring 2024-08-08 2:07 ` Andrew Jeffery 1 sibling, 1 reply; 10+ messages in thread From: Rob Herring @ 2024-08-06 17:29 UTC (permalink / raw) To: Andrew Jeffery Cc: Thomas Gleixner, Krzysztof Kozlowski, Conor Dooley, Joel Stanley, linux-kernel, devicetree, linux-arm-kernel, linux-aspeed On Fri, Aug 02, 2024 at 03:06:31PM +0930, Andrew Jeffery wrote: > Address warnings such as: > > arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-galaxy100.dtb: interrupt-controller@1e6c0080: 'valid-sources' does not match any of the regexes: 'pinctrl-[0-9]+' > > and > > arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-galaxy100.dtb: /ahb/copro-interrupt-controller@1e6c2000: failed to match any schema with compatible: ['aspeed,ast2400-cvic', 'aspeed-cvic'] > > Note that the conversion to DT schema causes some further warnings to > be emitted, because the Aspeed devicetrees are not in great shape. These > new warnings are resolved in a separate series: > > https://lore.kernel.org/lkml/20240802-dt-warnings-bmc-dts-cleanups-v1-0-1cb1378e5fcd@codeconstruct.com.au/ > > Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au> > --- > .../bindings/misc/aspeed,ast2400-cvic.yaml | 60 ++++++++++++++++++++++ > .../devicetree/bindings/misc/aspeed,cvic.txt | 35 ------------- > 2 files changed, 60 insertions(+), 35 deletions(-) > > diff --git a/Documentation/devicetree/bindings/misc/aspeed,ast2400-cvic.yaml b/Documentation/devicetree/bindings/misc/aspeed,ast2400-cvic.yaml > new file mode 100644 > index 000000000000..3c85b4924c05 > --- /dev/null > +++ b/Documentation/devicetree/bindings/misc/aspeed,ast2400-cvic.yaml > @@ -0,0 +1,60 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/misc/aspeed,ast2400-cvic.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Aspeed Coprocessor Vectored Interrupt Controller > + > +maintainers: > + - Andrew Jeffery <andrew@codeconstruct.com.au> > + > +description: > + The Aspeed AST2400 and AST2500 SoCs have a controller that provides interrupts > + to the ColdFire coprocessor. It's not a normal interrupt controller and it > + would be rather inconvenient to create an interrupt tree for it, as it > + somewhat shares some of the same sources as the main ARM interrupt controller > + but with different numbers. > + > + The AST2500 also supports a software generated interrupt. > + > +properties: > + compatible: > + items: > + - enum: > + - aspeed,ast2400-cvic > + - aspeed,ast2500-cvic > + - const: aspeed,cvic > + > + reg: > + maxItems: 1 > + > + valid-sources: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + description: > + One cell, bitmap of support sources for the implementation. > + > + copro-sw-interrupts: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + description: > + A list of interrupt numbers that can be used as software interrupts from > + the ARM to the coprocessor. > + > +required: > + - compatible > + - reg > + - valid-sources > + > +allOf: > + - $ref: /schemas/interrupt-controller.yaml# Doesn't really look like this schema applies to this binding. Drop the ref. > + > +additionalProperties: false > + > +examples: > + - | > + interrupt-controller@1e6c2000 { > + compatible = "aspeed,ast2500-cvic", "aspeed,cvic"; > + reg = <0x1e6c2000 0x80>; > + valid-sources = <0xffffffff>; > + copro-sw-interrupts = <1>; > + }; ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] dt-bindings: misc: aspeed,ast2400-cvic: Convert to DT schema 2024-08-06 17:29 ` Rob Herring @ 2024-08-08 2:07 ` Andrew Jeffery 0 siblings, 0 replies; 10+ messages in thread From: Andrew Jeffery @ 2024-08-08 2:07 UTC (permalink / raw) To: Rob Herring Cc: Thomas Gleixner, Krzysztof Kozlowski, Conor Dooley, Joel Stanley, linux-kernel, devicetree, linux-arm-kernel, linux-aspeed On Tue, 2024-08-06 at 11:29 -0600, Rob Herring wrote: > On Fri, Aug 02, 2024 at 03:06:31PM +0930, Andrew Jeffery wrote: > > Address warnings such as: > > > > arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-galaxy100.dtb: interrupt-controller@1e6c0080: 'valid-sources' does not match any of the regexes: 'pinctrl-[0-9]+' > > > > and > > > > arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-galaxy100.dtb: /ahb/copro-interrupt-controller@1e6c2000: failed to match any schema with compatible: ['aspeed,ast2400-cvic', 'aspeed-cvic'] > > > > Note that the conversion to DT schema causes some further warnings to > > be emitted, because the Aspeed devicetrees are not in great shape. These > > new warnings are resolved in a separate series: > > > > https://lore.kernel.org/lkml/20240802-dt-warnings-bmc-dts-cleanups-v1-0-1cb1378e5fcd@codeconstruct.com.au/ > > > > Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au> > > --- > > .../bindings/misc/aspeed,ast2400-cvic.yaml | 60 ++++++++++++++++++++++ > > .../devicetree/bindings/misc/aspeed,cvic.txt | 35 ------------- > > 2 files changed, 60 insertions(+), 35 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/misc/aspeed,ast2400-cvic.yaml b/Documentation/devicetree/bindings/misc/aspeed,ast2400-cvic.yaml > > new file mode 100644 > > index 000000000000..3c85b4924c05 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/misc/aspeed,ast2400-cvic.yaml > > @@ -0,0 +1,60 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/misc/aspeed,ast2400-cvic.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Aspeed Coprocessor Vectored Interrupt Controller > > + > > +maintainers: > > + - Andrew Jeffery <andrew@codeconstruct.com.au> > > + > > +description: > > + The Aspeed AST2400 and AST2500 SoCs have a controller that provides interrupts > > + to the ColdFire coprocessor. It's not a normal interrupt controller and it > > + would be rather inconvenient to create an interrupt tree for it, as it > > + somewhat shares some of the same sources as the main ARM interrupt controller > > + but with different numbers. > > + > > + The AST2500 also supports a software generated interrupt. > > + > > +properties: > > + compatible: > > + items: > > + - enum: > > + - aspeed,ast2400-cvic > > + - aspeed,ast2500-cvic > > + - const: aspeed,cvic > > + > > + reg: > > + maxItems: 1 > > + > > + valid-sources: > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > + description: > > + One cell, bitmap of support sources for the implementation. > > + > > + copro-sw-interrupts: > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > + description: > > + A list of interrupt numbers that can be used as software interrupts from > > + the ARM to the coprocessor. > > + > > +required: > > + - compatible > > + - reg > > + - valid-sources > > + > > +allOf: > > + - $ref: /schemas/interrupt-controller.yaml# > > Doesn't really look like this schema applies to this binding. Drop the > ref. > Ack. Thanks for the review. Andrew > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-08-08 3:53 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-02 5:36 [PATCH 0/2] dt-bindings: interrupt-controller: Convert Aspeed (C)VIC to DT schema Andrew Jeffery 2024-08-02 5:36 ` [PATCH 1/2] dt-bindings: interrupt-controller: aspeed,ast2400-vic: Convert " Andrew Jeffery 2024-08-06 6:07 ` Krzysztof Kozlowski 2024-08-08 2:02 ` Andrew Jeffery 2024-08-02 5:36 ` [PATCH 2/2] dt-bindings: misc: aspeed,ast2400-cvic: " Andrew Jeffery 2024-08-06 6:12 ` Krzysztof Kozlowski 2024-08-08 2:06 ` Andrew Jeffery 2024-08-08 3:52 ` Andrew Jeffery 2024-08-06 17:29 ` Rob Herring 2024-08-08 2:07 ` Andrew Jeffery
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).