linux-kernel - Re: [PATCH v5 2/8] dt-bindings: mailbox: Add a sun6i message box binding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191216140422.on4bredklgdxywbw@gilmour.lan>
Date:   Mon, 16 Dec 2019 15:04:22 +0100
From:   Maxime Ripard <mripard@...nel.org>
To:     Samuel Holland <samuel@...lland.org>
Cc:     Chen-Yu Tsai <wens@...e.org>,
        Jassi Brar <jassisinghbrar@...il.com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Sudeep Holla <sudeep.holla@....com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Ondrej Jirman <megous@...ous.com>,
        Vasily Khoruzhick <anarsoul@...il.com>,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-sunxi@...glegroups.com
Subject: Re: [PATCH v5 2/8] dt-bindings: mailbox: Add a sun6i message box
 binding

Hi,

On Sat, Dec 14, 2019 at 10:24:49PM -0600, Samuel Holland wrote:
> This mailbox hardware is present in Allwinner sun6i, sun8i, sun9i, and
> sun50i SoCs. Add a device tree binding for it. As it has only been
> tested on the A83T, A64, H3/H5, and H6 SoCs, only those compatibles are
> included.
>
> Signed-off-by: Samuel Holland <samuel@...lland.org>
> ---
>  .../mailbox/allwinner,sun6i-a31-msgbox.yaml   | 78 +++++++++++++++++++
>  1 file changed, 78 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sun6i-a31-msgbox.yaml
>
> diff --git a/Documentation/devicetree/bindings/mailbox/allwinner,sun6i-a31-msgbox.yaml b/Documentation/devicetree/bindings/mailbox/allwinner,sun6i-a31-msgbox.yaml
> new file mode 100644
> index 000000000000..dd746e07acfd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/allwinner,sun6i-a31-msgbox.yaml
> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mailbox/allwinner,sun6i-a31-msgbox.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner sunxi Message Box
> +
> +maintainers:
> +  - Samuel Holland <samuel@...lland.org>
> +
> +description: |
> +  The hardware message box on sun6i, sun8i, sun9i, and sun50i SoCs is a
> +  two-user mailbox controller containing 8 unidirectional FIFOs. An interrupt
> +  is raised for received messages, but software must poll to know when a
> +  transmitted message has been acknowledged by the remote user. Each FIFO can
> +  hold four 32-bit messages; when a FIFO is full, clients must wait before
> +  attempting more transmissions.
> +
> +  Refer to ./mailbox.txt for generic information about mailbox device-tree
> +  bindings.
> +
> +properties:
> +  compatible:
> +     items:
> +      - enum:
> +          - allwinner,sun8i-a83t-msgbox
> +          - allwinner,sun8i-h3-msgbox
> +          - allwinner,sun50i-a64-msgbox
> +          - allwinner,sun50i-h6-msgbox
> +      - const: allwinner,sun6i-a31-msgbox

This will fail for the A31, since it won't have two compatibles but
just one.

You can have something like this if you want to do it with an enum:

compatible:
  oneOf:
    - const: allwinner,sun6i-a31-msgbox
    - items:
      - enum:
        - allwinner,sun8i-a83t-msgbox
        - allwinner,sun8i-h3-msgbox
        - allwinner,sun50i-a64-msgbox
        - allwinner,sun50i-h6-msgbox
      - const: allwinner,sun6i-a31-msgbox

> +  reg:
> +    items:
> +      - description: MMIO register range

There's no need for an obvious description like this.
Just set it to maxItems: 1

> +
> +  clocks:
> +    maxItems: 1
> +    description: bus clock
> +
> +  resets:
> +    maxItems: 1
> +    description: bus reset
> +
> +  interrupts:
> +    maxItems: 1
> +    description: controller interrupt

Ditto, you can drop the description here.

> +  '#mbox-cells':
> +    const: 1

However, you should document what the argument is about?

> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - resets
> +  - interrupts
> +  - '#mbox-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/sun8i-h3-ccu.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/reset/sun8i-h3-ccu.h>
> +
> +    msgbox: mailbox@...7000 {
> +            compatible = "allwinner,sun8i-h3-msgbox",
> +                         "allwinner,sun6i-a31-msgbox";
> +            reg = <0x01c17000 0x1000>;
> +            clocks = <&ccu CLK_BUS_MSGBOX>;
> +            resets = <&ccu RST_BUS_MSGBOX>;
> +            interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
> +            #mbox-cells = <1>;
> +    };

Look good otherwise, thanks!
Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)