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

[Samuel Holland]

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 <sam...@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 <sam...@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>;
+ };
+
+...
--
2.23.0

[Samuel Holland]

This series adds support for the "hardware message box" in sun8i, sun9i,
and sun50i SoCs, used for communication with the ARISC management
processor (the platform's equivalent of the ARM SCP). The end goal is to
use the arm_scpi driver as a client, communicating with firmware running
on the ARISC CPU.

I have tested this driver with various firmware programs on the A64, H5,
and H6 SoCs (including specifically this arm_scpi patch on A64 and H6),
and Ondrej Jirman has tested the driver on the A83T (using a similar
patch to arm_scpi).

The change to make the arm_scpi compatible with unidirectional mailbox
controllers is attached to the end of this patch series. While it would
be nice to get this merged too, I don't consider it a prerequisite to
getting the driver merged. And even without the driver, the clock change
(patch #1) can go in at any time.

Thanks,
Samuel

Changes from v4:
- Rebased on sunxi-next
- Dropped AR100 clock patch, as it was controversial and unnecessary
- Renamed sunxi-msgbox to sun6i-msgbox and sun6i-a31-msgbox
- Added comments about not asserting the reset line
- Dropped A80 DTS changes as they were untested
- Added Ondrej's Tested-by for A83T
- Dropped the demo; replaced with a real arm_scpi fix

Changes from v3:
- Rebased on sunxi-next
- Added Rob's Reviewed-by for patch 3
- Fixed a crash when receiving a message on a disabled channel
- Cleaned up some comments/formatting in the driver
- Fixed #mbox-cells in sunxi-h3-h5.dtsi (patch 7)
- Removed the irqchip example (no longer relevant to the fw design)
- Added a demo/example client that uses the driver and a toy firmware

Changes from v2:
- Merge patches 1-3
- Add a comment in the code explaining the CLK_IS_CRITICAL usage
- Add a patch to mark the AR100 clocks as critical
- Use YAML for the device tree binding
- Include a not-for-merge example usage of the mailbox

Changes from v1:
- Marked message box clocks as critical instead of hacks in the driver
- 8 unidirectional channels instead of 4 bidirectional pairs
- Use per-SoC compatible strings and an A31 fallback compatible
- Dropped the mailbox framework patch
- Include DT patches for SoCs that document the message box

Samuel Holland (8):
clk: sunxi-ng: Mark msgbox clocks as critical
dt-bindings: mailbox: Add a sun6i message box binding
mailbox: sun6i-msgbox: Add a new mailbox driver
ARM: dts: sunxi: a83t: Add msgbox node
ARM: dts: sunxi: h3/h5: Add msgbox node
arm64: dts: allwinner: a64: Add msgbox node
arm64: dts: allwinner: h6: Add msgbox node
firmware: arm_scpi: Support unidirectional mailbox channels

.../mailbox/allwinner,sun6i-a31-msgbox.yaml | 78 ++++
arch/arm/boot/dts/sun8i-a83t.dtsi | 10 +
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 10 +
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 10 +
arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 10 +
drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 3 +-
drivers/clk/sunxi-ng/ccu-sun50i-h6.c | 3 +-
drivers/clk/sunxi-ng/ccu-sun8i-a23.c | 3 +-
drivers/clk/sunxi-ng/ccu-sun8i-a33.c | 3 +-
drivers/clk/sunxi-ng/ccu-sun8i-a83t.c | 3 +-
drivers/clk/sunxi-ng/ccu-sun8i-h3.c | 3 +-
drivers/clk/sunxi-ng/ccu-sun9i-a80.c | 3 +-
drivers/firmware/arm_scpi.c | 58 ++-
drivers/mailbox/Kconfig | 9 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/sun6i-msgbox.c | 332 ++++++++++++++++++
16 files changed, 520 insertions(+), 19 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mailbox/allwinner,sun6i-a31-msgbox.yaml
create mode 100644 drivers/mailbox/sun6i-msgbox.c

--
2.23.0

[Samuel Holland]

The A83T SoC contains a message box that can be used to send messages
and interrupts back and forth between the ARM application CPUs and the
ARISC coprocessor. Add a device tree node for it.

Tested-by: Ondrej Jirman <meg...@megous.com>

Signed-off-by: Samuel Holland <sam...@sholland.org>
---

arch/arm/boot/dts/sun8i-a83t.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 53c38deb8a08..464b57d03dc0 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -592,6 +592,16 @@
clock-names = "bus", "mod";
};

+ msgbox: mailbox@1c17000 {
+ compatible = "allwinner,sun8i-a83t-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>;
+ };
+

usb_otg: usb@1c19000 {
compatible = "allwinner,sun8i-a83t-musb",
"allwinner,sun8i-a33-musb";
--
2.23.0

[Samuel Holland]

The H3 and H5 SoCs contain a message box that can be used to send

messages and interrupts back and forth between the ARM application CPUs
and the ARISC coprocessor. Add a device tree node for it.

Signed-off-by: Samuel Holland <sam...@sholland.org>
---

arch/arm/boot/dts/sunxi-h3-h5.dtsi | 10 ++++++++++

1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 0afea59486c2..6c5b283962a1 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -233,6 +233,16 @@
reg = <0x1c14000 0x400>;
};

+ 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>;
+ };
+
usb_otg: usb@1c19000 {

compatible = "allwinner,sun8i-h3-musb";
reg = <0x01c19000 0x400>;
--
2.23.0

[Samuel Holland]

The A64 SoC contains a message box that can be used to send messages and

interrupts back and forth between the ARM application CPUs and the ARISC
coprocessor. Add a device tree node for it.

Signed-off-by: Samuel Holland <sam...@sholland.org>
---

arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 10 ++++++++++

1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 27e48234f1c2..10edb4b59203 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -496,6 +496,16 @@
resets = <&ccu RST_BUS_CE>;
};

+ msgbox: mailbox@1c17000 {
+ compatible = "allwinner,sun50i-a64-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>;
+ };
+
usb_otg: usb@1c19000 {

compatible = "allwinner,sun8i-a33-musb";
reg = <0x01c19000 0x0400>;
--
2.23.0

[Samuel Holland]

Some mailbox controllers have only unidirectional channels, so we need a
pair of them for each SCPI channel. If a mbox-names property is present,
look for "rx" and "tx" mbox channels; otherwise, the existing behavior
is preserved, and a single mbox channel is used for each SCPI channel.

Note that since the mailbox framework only supports a single phandle
with each name (mbox_request_channel_byname always returns the first
one), this new mode only supports a single SCPI channel.

Signed-off-by: Samuel Holland <sam...@sholland.org>
---

drivers/firmware/arm_scpi.c | 58 +++++++++++++++++++++++++++++--------
1 file changed, 46 insertions(+), 12 deletions(-)

diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index a80c331c3a6e..36ff9dd8d0fa 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -231,7 +231,8 @@ struct scpi_xfer {

struct scpi_chan {
struct mbox_client cl;
- struct mbox_chan *chan;
+ struct mbox_chan *rx_chan;
+ struct mbox_chan *tx_chan;
void __iomem *tx_payload;
void __iomem *rx_payload;
struct list_head rx_pending;
@@ -505,7 +506,7 @@ static int scpi_send_message(u8 idx, void *tx_buf, unsigned int tx_len,
msg->rx_len = rx_len;
reinit_completion(&msg->done);

- ret = mbox_send_message(scpi_chan->chan, msg);
+ ret = mbox_send_message(scpi_chan->tx_chan, msg);
if (ret < 0 || !rx_buf)
goto out;

@@ -854,8 +855,13 @@ static void scpi_free_channels(void *data)
struct scpi_drvinfo *info = data;
int i;

- for (i = 0; i < info->num_chans; i++)
- mbox_free_channel(info->channels[i].chan);
+ for (i = 0; i < info->num_chans; i++) {
+ struct scpi_chan *pchan = &info->channels[i];
+
+ if (pchan->tx_chan != pchan->rx_chan)
+ mbox_free_channel(pchan->tx_chan);
+ mbox_free_channel(pchan->rx_chan);
+ }
}

static int scpi_remove(struct platform_device *pdev)
@@ -903,6 +909,7 @@ static int scpi_probe(struct platform_device *pdev)
struct resource res;
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
+ bool use_mbox_names = false;

scpi_info = devm_kzalloc(dev, sizeof(*scpi_info), GFP_KERNEL);
if (!scpi_info)
@@ -916,6 +923,14 @@ static int scpi_probe(struct platform_device *pdev)
dev_err(dev, "no mboxes property in '%pOF'\n", np);
return -ENODEV;
}
+ if (of_get_property(dev->of_node, "mbox-names", NULL)) {
+ use_mbox_names = true;
+ if (count != 2) {
+ dev_err(dev, "need exactly 2 mboxes with mbox-names\n");
+ return -ENODEV;
+ }
+ count /= 2;
+ }

scpi_info->channels = devm_kcalloc(dev, count, sizeof(struct scpi_chan),
GFP_KERNEL);
@@ -961,15 +976,34 @@ static int scpi_probe(struct platform_device *pdev)
mutex_init(&pchan->xfers_lock);

ret = scpi_alloc_xfer_list(dev, pchan);
- if (!ret) {
- pchan->chan = mbox_request_channel(cl, idx);
- if (!IS_ERR(pchan->chan))
- continue;
- ret = PTR_ERR(pchan->chan);
- if (ret != -EPROBE_DEFER)
- dev_err(dev, "failed to get channel%d err %d\n",
- idx, ret);
+ if (ret)
+ return ret;
+
+ if (use_mbox_names) {
+ pchan->rx_chan = mbox_request_channel_byname(cl, "rx");
+ if (IS_ERR(pchan->rx_chan)) {
+ ret = PTR_ERR(pchan->rx_chan);
+ goto fail;
+ }
+ pchan->tx_chan = mbox_request_channel_byname(cl, "tx");
+ if (IS_ERR(pchan->rx_chan)) {
+ ret = PTR_ERR(pchan->tx_chan);
+ goto fail;
+ }
+ } else {
+ pchan->rx_chan = mbox_request_channel(cl, idx);
+ if (IS_ERR(pchan->rx_chan)) {
+ ret = PTR_ERR(pchan->rx_chan);
+ goto fail;
+ }
+ pchan->tx_chan = pchan->rx_chan;
}
+ continue;
+
+fail:
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "failed to get channel%d err %d\n",
+ idx, ret);
return ret;
}

--
2.23.0

[Samuel Holland]

The H6 SoC contains a message box that can be used to send messages and

interrupts back and forth between the ARM application CPUs and the ARISC
coprocessor. Add a device tree node for it.

Signed-off-by: Samuel Holland <sam...@sholland.org>
---

arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 10 ++++++++++

1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 8e5999cd08bb..70ee56ba2091 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -240,6 +240,16 @@
#dma-cells = <1>;
};

+ msgbox: mailbox@3003000 {
+ compatible = "allwinner,sun50i-h6-msgbox",
+ "allwinner,sun6i-a31-msgbox";
+ reg = <0x03003000 0x1000>;

+ clocks = <&ccu CLK_BUS_MSGBOX>;
+ resets = <&ccu RST_BUS_MSGBOX>;

+ interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;

+ #mbox-cells = <1>;
+ };
+

sid: efuse@3006000 {
compatible = "allwinner,sun50i-h6-sid";
reg = <0x03006000 0x400>;
--
2.23.0

[Samuel Holland]

Hi,

On 12/16/19 8:04 AM, Maxime Ripard wrote:
> Hi,

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

> Look good otherwise, thanks!
> Maxime
>

Thanks,
Samuel