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@xxxxxxxxxxxx> >> --- >> .../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@xxxxxxxxxxxx> >> + >> +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
