Message-ID: <20231002173019.GA2037244-robh@kernel.org>
Date: Mon, 2 Oct 2023 12:30:19 -0500
From: Rob Herring <robh@...nel.org>
To: Gatien Chevallier <gatien.chevallier@...s.st.com>
Cc: Oleksii_Moisieiev@...m.com, gregkh@...uxfoundation.org,
	herbert@...dor.apana.org.au, davem@...emloft.net,
	krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
	alexandre.torgue@...s.st.com, vkoul@...nel.org, jic23@...nel.org,
	olivier.moysan@...s.st.com, arnaud.pouliquen@...s.st.com,
	mchehab@...nel.org, fabrice.gasnier@...s.st.com,
	andi.shyti@...nel.org, ulf.hansson@...aro.org, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, hugues.fruchet@...s.st.com,
	lee@...nel.org, will@...nel.org, catalin.marinas@....com,
	arnd@...nel.org, richardcochran@...il.com,
	Frank Rowand <frowand.list@...il.com>, peng.fan@....nxp.com,
	linux-crypto@...r.kernel.org, devicetree@...r.kernel.org,
	linux-stm32@...md-mailman.stormreply.com,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	dmaengine@...r.kernel.org, linux-i2c@...r.kernel.org,
	linux-iio@...r.kernel.org, alsa-devel@...a-project.org,
	linux-media@...r.kernel.org, linux-mmc@...r.kernel.org,
	netdev@...r.kernel.org, linux-p@....codeaurora.org,
	hy@...ts.infradead.org, linux-serial@...r.kernel.org,
	linux-spi@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCH v5 01/11] dt-bindings: document generic access controller

On Fri, Sep 29, 2023 at 04:28:42PM +0200, Gatien Chevallier wrote:
> From: Oleksii Moisieiev <Oleksii_Moisieiev@...m.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@...m.com>
> Signed-off-by: Gatien Chevallier <gatien.chevallier@...s.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@...m.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@...00 {
> +        reg = <0x50000 0x10>;
> +        access-control-provider;
> +        #access-controller-cells = <2>;
> +    };
> +
> +    bus_controller: bus@...00 {
> +        reg = <0x60000 0x10000>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        ranges;
> +        access-control-provider;
> +        #access-controller-cells = <3>;
> +
> +        uart4: serial@...00 {
> +            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

Powered by blists - more mailing lists