| Message ID | 20230929142852.578394-2-gatien.chevallier@foss.st.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Introduce STM32 Firewall framework | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
On Fri, 29 Sep 2023 16:28:42 +0200, Gatien Chevallier wrote: > From: Oleksii Moisieiev <Oleksii_Moisieiev@epam.com> > > Introducing of the generic access controller bindings for the > access controller provider and consumer devices. Those bindings are > intended to allow a better handling of accesses to resources in a > hardware architecture supporting several compartments. > > This patch is based on [1]. It is integrated in this patchset as it > provides a use-case for it. > > Diffs with [1]: > - Rename feature-domain* properties to access-control* to narrow > down the scope of the binding > - YAML errors and typos corrected. > - Example updated > - Some rephrasing in the binding description > > [1]: https://lore.kernel.org/lkml/0c0a82bb-18ae-d057-562b > > Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com> > Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com> > > --- > Changes in V5: > - Diffs with [1] > - Discarded the [IGNORE] tag as the patch is now part of the > patchset > > .../access-controllers/access-controller.yaml | 90 +++++++++++++++++++ > 1 file changed, 90 insertions(+) > create mode 100644 Documentation/devicetree/bindings/access-controllers/access-controller.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/access-controllers/access-controller.yaml: access-control-provider: missing type definition doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230929142852.578394-2-gatien.chevallier@foss.st.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 9/29/23 17:35, Rob Herring wrote: > > On Fri, 29 Sep 2023 16:28:42 +0200, Gatien Chevallier wrote: >> From: Oleksii Moisieiev <Oleksii_Moisieiev@epam.com> >> >> Introducing of the generic access controller bindings for the >> access controller provider and consumer devices. Those bindings are >> intended to allow a better handling of accesses to resources in a >> hardware architecture supporting several compartments. >> >> This patch is based on [1]. It is integrated in this patchset as it >> provides a use-case for it. >> >> Diffs with [1]: >> - Rename feature-domain* properties to access-control* to narrow >> down the scope of the binding >> - YAML errors and typos corrected. >> - Example updated >> - Some rephrasing in the binding description >> >> [1]: https://lore.kernel.org/lkml/0c0a82bb-18ae-d057-562b >> >> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com> >> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com> >> >> --- >> Changes in V5: >> - Diffs with [1] >> - Discarded the [IGNORE] tag as the patch is now part of the >> patchset >> >> .../access-controllers/access-controller.yaml | 90 +++++++++++++++++++ >> 1 file changed, 90 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/access-controllers/access-controller.yaml >> > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/access-controllers/access-controller.yaml: access-control-provider: missing type definition > > doc reference errors (make refcheckdocs): > > See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230929142852.578394-2-gatien.chevallier@foss.st.com > > The base for the series is generally the latest rc1. A different dependency > should be noted in *this* patch. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit after running the above command yourself. Note > that DT_SCHEMA_FILES can be set to your schema file to speed up checking > your schema. However, it must be unset to test all examples with your schema. > Hi Rob, Running: 1- make dt_binding_check | grep access-control 2- make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/access-controllers/access-controller.yaml from Krzysztof's slideset with pip3 show dtschema Name: dtschema Version: 2023.9 and pip3 show yamllint Name: yamllint Version: 1.32.0 I don't see any of the errors reported by the robot. I have to clone your repository to reproduce it. Should I resubmit with a clean dt-check using the latest dtschema? *********** However, I get: warning: ignoring duplicate '$id' value 'http://devicetree.org/schemas/reserved-memory/framebuffer.yaml# warning: ignoring duplicate '$id' value 'http://devicetree.org/schemas/reserved-memory/memory-region.yaml# warning: ignoring duplicate '$id' value 'http://devicetree.org/schemas/reserved-memory/shared-dma-pool.yaml# warning: ignoring duplicate '$id' value 'http://devicetree.org/schemas/reserved-memory/reserved-memory.yaml Above warnings disappears when switching to: pip3 show dtschema Name: dtschema Version: 2023.7 The above YAMLs seem to be duplicated in dtschema's latest version. I guess it's a synchro that needs to be done since: https://lore.kernel.org/all/20230830231758.2561402-2-sjg@chromium.org/ *********** Best regards, Gatien
On Fri, Sep 29, 2023 at 04:28:42PM +0200, Gatien Chevallier wrote: > From: Oleksii Moisieiev <Oleksii_Moisieiev@epam.com> > > Introducing of the generic access controller bindings for the > access controller provider and consumer devices. Those bindings are > intended to allow a better handling of accesses to resources in a > hardware architecture supporting several compartments. > > This patch is based on [1]. It is integrated in this patchset as it > provides a use-case for it. > > Diffs with [1]: > - Rename feature-domain* properties to access-control* to narrow > down the scope of the binding > - YAML errors and typos corrected. > - Example updated > - Some rephrasing in the binding description > > [1]: https://lore.kernel.org/lkml/0c0a82bb-18ae-d057-562b > > Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com> > Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com> > > --- > Changes in V5: > - Diffs with [1] > - Discarded the [IGNORE] tag as the patch is now part of the > patchset > > .../access-controllers/access-controller.yaml | 90 +++++++++++++++++++ > 1 file changed, 90 insertions(+) > create mode 100644 Documentation/devicetree/bindings/access-controllers/access-controller.yaml > > diff --git a/Documentation/devicetree/bindings/access-controllers/access-controller.yaml b/Documentation/devicetree/bindings/access-controllers/access-controller.yaml > new file mode 100644 > index 000000000000..9d305fccc333 > --- /dev/null > +++ b/Documentation/devicetree/bindings/access-controllers/access-controller.yaml > @@ -0,0 +1,90 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/access-controllers/access-controller.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Generic Domain Access Controller > + > +maintainers: > + - Oleksii Moisieiev <oleksii_moisieiev@epam.com> > + > +description: |+ > + Common access controllers properties > + > + Access controllers are in charge of stating which of the hardware blocks under > + their responsibility (their domain) can be accesssed by which compartment. A > + compartment can be a cluster of CPUs (or coprocessors), a range of addresses > + or a group of hardware blocks. An access controller's domain is the set of > + resources covered by the access controller. > + > + This device tree bindings can be used to bind devices to their access > + controller provided by access-controller property. In this case, the device is > + a consumer and the access controller is the provider. > + > + An access controller can be represented by any node in the device tree and > + can provide one or more configuration parameters, needed to control parameters > + of the consumer device. A consumer node can refer to the provider by phandle > + and a set of phandle arguments, specified by '#access-controller-cells' > + property in the access controller node. > + > + Access controllers are typically used to set/read the permissions of a > + hardware block and grant access to it. Any of which depends on the access > + controller. The capabilities of each access controller are defined by the > + binding of the access controller device. > + > + Each node can be a consumer for the several access controllers. > + > +# always select the core schema > +select: true > + > +properties: > + "#access-controller-cells": > + $ref: /schemas/types.yaml#/definitions/uint32 Drop. "#.*-cells" already defines the type. > + description: > + Number of cells in a access-controller specifier; > + Can be any value as specified by device tree binding documentation > + of a particular provider. > + > + access-control-provider: > + description: > + Indicates that the node is an access controller. Drop. The presence of "#access-controller-cells" is enough to do that. > + > + access-controller-names: > + $ref: /schemas/types.yaml#/definitions/string-array > + description: > + A list of access-controller names, sorted in the same order as > + access-controller entries. Consumer drivers will use > + access-controller-names to match with existing access-controller entries. > + > + access-controller: For consistency with other provider bindings: access-controllers > + $ref: /schemas/types.yaml#/definitions/phandle-array > + description: > + A list of access controller specifiers, as defined by the > + bindings of the access-controller provider. > + > +additionalProperties: true > + > +examples: > + - | > + uart_controller: access-controller@50000 { > + reg = <0x50000 0x10>; > + access-control-provider; > + #access-controller-cells = <2>; > + }; > + > + bus_controller: bus@60000 { > + reg = <0x60000 0x10000>; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + access-control-provider; > + #access-controller-cells = <3>; > + > + uart4: serial@60100 { > + reg = <0x60100 0x400>; > + access-controller = <&uart_controller 1 2>, > + <&bus_controller 1 3 5>; > + access-controller-names = "controller", "bus-controller"; Not great names. It should indicate what access is being controlled locally. Perhaps "reg" for register access, "dma" or "bus" for bus master access. (Not sure what your uart_controller is controlling access to.) Rob
Hi Rob, On 10/2/23 19:30, Rob Herring wrote: > On Fri, Sep 29, 2023 at 04:28:42PM +0200, Gatien Chevallier wrote: >> From: Oleksii Moisieiev <Oleksii_Moisieiev@epam.com> >> >> Introducing of the generic access controller bindings for the >> access controller provider and consumer devices. Those bindings are >> intended to allow a better handling of accesses to resources in a >> hardware architecture supporting several compartments. >> >> This patch is based on [1]. It is integrated in this patchset as it >> provides a use-case for it. >> >> Diffs with [1]: >> - Rename feature-domain* properties to access-control* to narrow >> down the scope of the binding >> - YAML errors and typos corrected. >> - Example updated >> - Some rephrasing in the binding description >> >> [1]: https://lore.kernel.org/lkml/0c0a82bb-18ae-d057-562b >> >> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com> >> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com> >> >> --- >> Changes in V5: >> - Diffs with [1] >> - Discarded the [IGNORE] tag as the patch is now part of the >> patchset >> >> .../access-controllers/access-controller.yaml | 90 +++++++++++++++++++ >> 1 file changed, 90 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/access-controllers/access-controller.yaml >> >> diff --git a/Documentation/devicetree/bindings/access-controllers/access-controller.yaml b/Documentation/devicetree/bindings/access-controllers/access-controller.yaml >> new file mode 100644 >> index 000000000000..9d305fccc333 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/access-controllers/access-controller.yaml >> @@ -0,0 +1,90 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/access-controllers/access-controller.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Generic Domain Access Controller >> + >> +maintainers: >> + - Oleksii Moisieiev <oleksii_moisieiev@epam.com> >> + >> +description: |+ >> + Common access controllers properties >> + >> + Access controllers are in charge of stating which of the hardware blocks under >> + their responsibility (their domain) can be accesssed by which compartment. A >> + compartment can be a cluster of CPUs (or coprocessors), a range of addresses >> + or a group of hardware blocks. An access controller's domain is the set of >> + resources covered by the access controller. >> + >> + This device tree bindings can be used to bind devices to their access >> + controller provided by access-controller property. In this case, the device is >> + a consumer and the access controller is the provider. >> + >> + An access controller can be represented by any node in the device tree and >> + can provide one or more configuration parameters, needed to control parameters >> + of the consumer device. A consumer node can refer to the provider by phandle >> + and a set of phandle arguments, specified by '#access-controller-cells' >> + property in the access controller node. >> + >> + Access controllers are typically used to set/read the permissions of a >> + hardware block and grant access to it. Any of which depends on the access >> + controller. The capabilities of each access controller are defined by the >> + binding of the access controller device. >> + >> + Each node can be a consumer for the several access controllers. >> + >> +# always select the core schema >> +select: true >> + >> +properties: >> + "#access-controller-cells": >> + $ref: /schemas/types.yaml#/definitions/uint32 > > Drop. "#.*-cells" already defines the type. > Ok, I will drop it for V6 >> + description: >> + Number of cells in a access-controller specifier; >> + Can be any value as specified by device tree binding documentation >> + of a particular provider. >> + >> + access-control-provider: >> + description: >> + Indicates that the node is an access controller. > > Drop. The presence of "#access-controller-cells" is enough to do that. > Ok, I wasn't sure. I'll will drop it for V6 >> + >> + access-controller-names: >> + $ref: /schemas/types.yaml#/definitions/string-array >> + description: >> + A list of access-controller names, sorted in the same order as >> + access-controller entries. Consumer drivers will use >> + access-controller-names to match with existing access-controller entries. >> + >> + access-controller: > > For consistency with other provider bindings: access-controllers > Ack >> + $ref: /schemas/types.yaml#/definitions/phandle-array >> + description: >> + A list of access controller specifiers, as defined by the >> + bindings of the access-controller provider. >> + >> +additionalProperties: true >> + >> +examples: >> + - | >> + uart_controller: access-controller@50000 { >> + reg = <0x50000 0x10>; >> + access-control-provider; >> + #access-controller-cells = <2>; >> + }; >> + >> + bus_controller: bus@60000 { >> + reg = <0x60000 0x10000>; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges; >> + access-control-provider; >> + #access-controller-cells = <3>; >> + >> + uart4: serial@60100 { >> + reg = <0x60100 0x400>; >> + access-controller = <&uart_controller 1 2>, >> + <&bus_controller 1 3 5>; >> + access-controller-names = "controller", "bus-controller"; > > Not great names. It should indicate what access is being controlled > locally. Perhaps "reg" for register access, "dma" or "bus" for bus > master access. (Not sure what your uart_controller is controlling access > to.) > > Rob Yes, I agree it's poor naming. I'll come up with something more adequate. Thank you for the input. Best regards, Gatien
diff --git a/Documentation/devicetree/bindings/access-controllers/access-controller.yaml b/Documentation/devicetree/bindings/access-controllers/access-controller.yaml new file mode 100644 index 000000000000..9d305fccc333 --- /dev/null +++ b/Documentation/devicetree/bindings/access-controllers/access-controller.yaml @@ -0,0 +1,90 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/access-controllers/access-controller.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Generic Domain Access Controller + +maintainers: + - Oleksii Moisieiev <oleksii_moisieiev@epam.com> + +description: |+ + Common access controllers properties + + Access controllers are in charge of stating which of the hardware blocks under + their responsibility (their domain) can be accesssed by which compartment. A + compartment can be a cluster of CPUs (or coprocessors), a range of addresses + or a group of hardware blocks. An access controller's domain is the set of + resources covered by the access controller. + + This device tree bindings can be used to bind devices to their access + controller provided by access-controller property. In this case, the device is + a consumer and the access controller is the provider. + + An access controller can be represented by any node in the device tree and + can provide one or more configuration parameters, needed to control parameters + of the consumer device. A consumer node can refer to the provider by phandle + and a set of phandle arguments, specified by '#access-controller-cells' + property in the access controller node. + + Access controllers are typically used to set/read the permissions of a + hardware block and grant access to it. Any of which depends on the access + controller. The capabilities of each access controller are defined by the + binding of the access controller device. + + Each node can be a consumer for the several access controllers. + +# always select the core schema +select: true + +properties: + "#access-controller-cells": + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Number of cells in a access-controller specifier; + Can be any value as specified by device tree binding documentation + of a particular provider. + + access-control-provider: + description: + Indicates that the node is an access controller. + + access-controller-names: + $ref: /schemas/types.yaml#/definitions/string-array + description: + A list of access-controller names, sorted in the same order as + access-controller entries. Consumer drivers will use + access-controller-names to match with existing access-controller entries. + + access-controller: + $ref: /schemas/types.yaml#/definitions/phandle-array + description: + A list of access controller specifiers, as defined by the + bindings of the access-controller provider. + +additionalProperties: true + +examples: + - | + uart_controller: access-controller@50000 { + reg = <0x50000 0x10>; + access-control-provider; + #access-controller-cells = <2>; + }; + + bus_controller: bus@60000 { + reg = <0x60000 0x10000>; + #address-cells = <1>; + #size-cells = <1>; + ranges; + access-control-provider; + #access-controller-cells = <3>; + + uart4: serial@60100 { + reg = <0x60100 0x400>; + access-controller = <&uart_controller 1 2>, + <&bus_controller 1 3 5>; + access-controller-names = "controller", "bus-controller"; + }; + };