diff mbox series

[v5,2/8] dt-bindings: mailbox: Add a sun6i message box binding

Message ID 20191215042455.51001-3-samuel@sholland.org (mailing list archive)
State Not Applicable, archived
Headers show
Series None | expand

Commit Message

Samuel Holland Dec. 15, 2019, 4:24 a.m. UTC
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@sholland.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

Comments

Maxime Ripard Dec. 16, 2019, 2:04 p.m. UTC | #1
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@sholland.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@sholland.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@1c17000 {
> +            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
Samuel Holland Dec. 16, 2019, 7:45 p.m. UTC | #2
Hi,

On 12/16/19 8:04 AM, Maxime Ripard wrote:
> 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@sholland.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@sholland.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 asked me earlier to only include compatibles that had been tested, so I did.
This hasn't been tested on the A31, so there's no A31-only compatible.

> 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

Will do for v6.

>> +
>> +  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.

Will do for v6.

>> +  '#mbox-cells':
>> +    const: 1
> 
> However, you should document what the argument is about?

Ok. "Number of cells used to encode a mailbox specifier" should work.

>> +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@1c17000 {
>> +            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
> 

Thanks,
Samuel
Maxime Ripard Dec. 26, 2019, 10:14 a.m. UTC | #3
Hi,

On Mon, Dec 16, 2019 at 01:45:08PM -0600, Samuel Holland wrote:
> On 12/16/19 8:04 AM, Maxime Ripard wrote:
> > 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@sholland.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@sholland.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 asked me earlier to only include compatibles that had been tested, so I did.
> This hasn't been tested on the A31, so there's no A31-only compatible.

The binding is the description, and that description already matches
the A31 compatible, and it's completely abstract from whether we have
software to support it.

We have bindings that have no drivers in the tree for example, but are
just there to make the representation complete.

In this case, we shouldn't enable it in the A31 DTSI, but we should
document the binding properly.

> >> +  '#mbox-cells':
> >> +    const: 1
> >
> > However, you should document what the argument is about?
>
> Ok. "Number of cells used to encode a mailbox specifier" should work.

It's not really what I meant, what I meant is what is is that
specifier you're talking about here. The customers' mailbox properties
will have a phandle and a number: what is this number representing?

Maxime
diff mbox series

Patch

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@sholland.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
+
+  reg:
+    items:
+      - description: MMIO register range
+
+  clocks:
+    maxItems: 1
+    description: bus clock
+
+  resets:
+    maxItems: 1
+    description: bus reset
+
+  interrupts:
+    maxItems: 1
+    description: controller interrupt
+
+  '#mbox-cells':
+    const: 1
+
+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@1c17000 {
+            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>;
+    };
+
+...