Re: [RFC PATCH 2/8] dt-bindings: mailbox: Add bindings for RPMI shared memory transport

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4261 bytes --]

On Mon, Dec 16, 2024 at 02:18:11PM +0530, Anup Patel wrote:
> Add device tree bindings for the common RISC-V Platform Management
> Interface (RPMI) shared memory transport as a mailbox controller.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  .../mailbox/riscv,rpmi-shmem-mbox.yaml        | 135 ++++++++++++++++++
>  1 file changed, 135 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml b/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml
> new file mode 100644
> index 000000000000..8d713ba7ffc7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/riscv,rpmi-shmem-mbox.yaml
> @@ -0,0 +1,135 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mailbox/riscv,sbi-mpxy-mbox.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: RISC-V Platform Management Interface (RPMI) shared memory mailbox
> +
> +maintainers:
> +  - Anup Patel <anup@brainfault.org>
> +
> +description: |
> +  The RISC-V Platform Management Interface (RPMI) [1] defines a common shared
> +  memory based RPMI transport. This RPMI shared memory transport integrates as
> +  mailbox controller in the SBI implementation or supervisor software whereas
> +  each RPMI service group is mailbox client in the SBI implementation and
> +  supervisor software.
> +
> +  ===========================================
> +  References
> +  ===========================================
> +
> +  [1] RISC-V Platform Management Interface (RPMI)
> +      https://github.com/riscv-non-isa/riscv-rpmi/releases
> +
> +properties:
> +  compatible:
> +    const: riscv,rpmi-shmem-mbox
> +
> +  reg:
> +    oneOf:
> +      - items:
> +        - description: A2P request queue base address
> +        - description: P2A acknowledgment queue base address
> +        - description: P2A request queue base address
> +        - description: A2P acknowledgment queue base address
> +        - description: A2P doorbell address
> +      - items:
> +        - description: A2P request queue base address
> +        - description: P2A acknowledgment queue base address
> +        - description: A2P doorbell address
> +
> +  reg-names:
> +    oneOf:
> +      - items:
> +        - const: a2p-req
> +        - const: p2a-ack
> +        - const: p2a-req
> +        - const: a2p-ack
> +        - const: db-reg
> +      - items:
> +        - const: a2p-req
> +        - const: p2a-ack
> +        - const: db-reg
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 1
> +    description:
> +      The RPMI shared memory transport supports wired interrupt specified by
> +      this property as the P2A doorbell.
> +
> +  msi-parent:
> +    description:
> +      The RPMI shared memory transport supports MSI as P2A doorbell and this
> +      property specifies the target MSI controller.
> +
> +  riscv,slot-size:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 64
> +    description:
> +      Power-of-2 RPMI slot size of the RPMI shared memory transport.
> +
> +  riscv,db-mask:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Update only the register bits of doorbell defined by the mask (32 bit).

For an untested RFC, the bindings here look mostly fine. I'd suggest
renaming "db" to "doorbell" in properties etc, since we can easily
afford the extra characters. Please make sure to actually test the
bindings next time around, looks like all 3 bindings produced warnings.

General comments like not needing to provide minItems: 1 when maxItems
is also 1, and not needing a | unless you have formatting to preserve.

> +      If this property is not present then mask is assumed to be 0xffffffff.

Also, things like this should be handled with a `default: 0xffffffff`,
rather than describing it in text.

I'll give the !rfc bindings a proper review when they appear, but before
that I'll give the code here a go too - thanks for sending the patches.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]