LinuxLists.cc - [PATCH 0/4] Arm MHUv2 Mailbox Controller Driver

[permalink] [raw]

Subject: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

From: Morten Borup Petersen <[email protected]>

This patch adds device tree binding for Message Handling Unit
controller version 2 from Arm.

Signed-off-by: Morten Borup Petersen <[email protected]>
Signed-off-by: Tushar Khandelwal <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
.../devicetree/bindings/mailbox/arm,mhuv2.txt | 108 ++++++++++++++++++
1 file changed, 108 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt

diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
new file mode 100644
index 000000000000..3a05593414bc
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
@@ -0,0 +1,108 @@
+Arm MHUv2 Mailbox Driver
+========================
+
+The Arm Message-Handling-Unit (MHU) Version 2 is a mailbox controller that has
+between 1 and 124 channel windows to provide unidirectional communication with
+remote processor(s).
+
+Given the unidirectional nature of the device, an MHUv2 mailbox may only be
+written to or read from. If a pair of MHU devices is implemented between two
+processing elements to provide bidirectional communication, these must be
+specified as two separate mailboxes.
+
+A device tree node for an Arm MHUv2 device must specify either a receiver frame
+or a sender frame, indicating which end of the unidirectional MHU device which
+the device node entry describes.
+
+An MHU device must be specified with a transport protocol. The transport
+protocol of an MHU device determines the method of data transmission as well as
+the number of provided mailboxes.
+Following are the possible transport protocol types:
+- Single-word: An MHU device implements as many mailboxes as it
+ provides channel windows. Data is transmitted through
+ the MHU registers.
+- Multi-word: An MHU device implements a single mailbox. All channel windows
+ will be used during transmission. Data is transmitted through
+ the MHU registers.
+- Doorbell: An MHU device implements as many mailboxes as there are flag
+ bits available in its channel windows. Optionally, data may
+ be transmitted through a shared memory region, wherein the MHU
+ is used strictly as an interrupt generation mechanism.
+
+Mailbox Device Node:
+====================
+
+Required properties:
+--------------------
+- compatible: Shall be "arm,mhuv2" & "arm,primecell"
+- reg: Contains the mailbox register address range (base
+ address and length)
+- #mbox-cells Shall be 1 - the index of the channel needed.
+- mhu-frame Frame type of the device.
+ Shall be either "sender" or "receiver"
+- mhu-protocol Transport protocol of the device. Shall be one of the
+ following: "single-word", "multi-word", "doorbell"
+
+Required properties (receiver frame):
+-------------------------------------
+- interrupts: Contains the interrupt information corresponding to the
+ combined interrupt of the receiver frame
+
+Example:
+--------
+
+ mbox_mw_tx: mhu@10000000 {
+ compatible = "arm,mhuv2","arm,primecell";
+ reg = <0x10000000 0x1000>;
+ clocks = <&refclk100mhz>;
+ clock-names = "apb_pclk";
+ #mbox-cells = <1>;
+ mhu-protocol = "multi-word";
+ mhu-frame = "sender";
+ };
+
+ mbox_sw_tx: mhu@10000000 {
+ compatible = "arm,mhuv2","arm,primecell";
+ reg = <0x11000000 0x1000>;
+ clocks = <&refclk100mhz>;
+ clock-names = "apb_pclk";
+ #mbox-cells = <1>;
+ mhu-protocol = "single-word";
+ mhu-frame = "sender";
+ };
+
+ mbox_db_rx: mhu@10000000 {
+ compatible = "arm,mhuv2","arm,primecell";
+ reg = <0x12000000 0x1000>;
+ clocks = <&refclk100mhz>;
+ clock-names = "apb_pclk";
+ #mbox-cells = <1>;
+ interrupts = <0 45 4>;
+ interrupt-names = "mhu_rx";
+ mhu-protocol = "doorbell";
+ mhu-frame = "receiver";
+ };
+
+ mhu_client: scb@2e000000 {
+ compatible = "fujitsu,mb86s70-scb-1.0";
+ reg = <0 0x2e000000 0x4000>;
+ mboxes =
+ // For multi-word frames, client may only instantiate a single
+ // mailbox for a mailbox controller
+ <&mbox_mw_tx 0>,
+
+ // For single-word frames, client may instantiate as many
+ // mailboxes as there are channel windows in the MHU
+ <&mbox_sw_tx 0>,
+ <&mbox_sw_tx 1>,
+ <&mbox_sw_tx 2>,
+ <&mbox_sw_tx 3>,
+
+ // For doorbell frames, client may instantiate as many mailboxes
+ // as there are bits available in the combined number of channel
+ // windows ((channel windows * 32) mailboxes)
+ <mbox_db_rx 0>,
+ <mbox_db_rx 1>,
+ ...
+ <mbox_db_rx 17>;
+ };
\ No newline at end of file
--
2.17.1

2019-07-17 19:27:46

[permalink] [raw]

Subject: [PATCH 2/4] mailbox: arm_mhuv2: add arm mhuv2 driver

From: Morten Borup Petersen <[email protected]>

This commit adds a driver for the ARM MHUv2 (Message Handling Unit).
The driver registers itself as a mailbox controller within the common
mailbox framework.

This commit implements the single-word transport protocol;
In single-word mode, the mailbox controller will provide a mailbox for each
channel window available in the MHU device.
Transmitting and receiving data through the mailbox framework in
single-word mode is done through a struct arm_mbox_msg.

Signed-off-by: Morten Borup Petersen <[email protected]>
Signed-off-by: Tushar Khandelwal <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/mailbox/Kconfig | 7 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/arm_mhu_v2.c | 735 +++++++++++++++++++++++
include/linux/mailbox/arm-mbox-message.h | 37 ++
4 files changed, 781 insertions(+)
create mode 100644 drivers/mailbox/arm_mhu_v2.c
create mode 100644 include/linux/mailbox/arm-mbox-message.h

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index e63d29a95e76..6808bba5bf9b 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -15,6 +15,13 @@ config ARM_MHU
The controller has 3 mailbox channels, the last of which can be
used in Secure mode only.

+config ARM_MHU_V2
+ tristate "ARM MHUv2 Mailbox"
+ depends on ARM_AMBA
+ help
+ Say Y here if you want to build the ARM MHUv2 controller driver,
+ which provides unidirectional mailboxes between processing elements.
+
config PLATFORM_MHU
tristate "Platform MHU Mailbox"
depends on OF
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index f15788ccc572..6edef52d74bb 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -7,6 +7,8 @@ obj-$(CONFIG_MAILBOX_TEST) += mailbox-test.o

obj-$(CONFIG_ARM_MHU) += arm_mhu.o

+obj-$(CONFIG_ARM_MHU_V2) += arm_mhu_v2.o
+
obj-$(CONFIG_PLATFORM_MHU) += platform_mhu.o

obj-$(CONFIG_PL320_MBOX) += pl320-ipc.o
diff --git a/drivers/mailbox/arm_mhu_v2.c b/drivers/mailbox/arm_mhu_v2.c
new file mode 100644
index 000000000000..a0af683b83a2
--- /dev/null
+++ b/drivers/mailbox/arm_mhu_v2.c
@@ -0,0 +1,735 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Arm Message Handling Unit Version 2 (MHUv2) driver
+ *
+ * An MHU device may function in one of three transport protocol modes
+ * (single-word, multi-word and doorbell).
+ * This transport protocol should be specified in the device tree entry for the
+ * device. The transport protocol determines how the underlying hardware
+ * resources of the device are utilized when transmitting data.
+ *
+ * The arm MHUv2 driver registers as a mailbox controller with the common
+ * mailbox framework. Each mailbox channel represents a separate virtual
+ * communication channel through the MHU device.
+ * The number of registered mailbox channels is dependent on both the
+ * underlying hardware - mainly the number of channel windows within each MHU
+ * frame, as well as the selected transport protocol.
+ *
+ * Copyright (C) 2019 Arm Ltd.
+ */
+
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/amba/bus.h>
+#include <linux/mailbox_controller.h>
+#include <linux/mailbox/arm-mbox-message.h>
+#include <linux/of_address.h>
+#include <linux/interrupt.h>
+
+/* Maximum number of channel windows */
+#define MHUV2_CHANNEL_MAX 124
+/* Number of combined interrupt status registers */
+#define MHUV2_CMB_INT_ST_REG_CNT 4
+#define MHUV2_CH_UNKNOWN -1
+
+/* Channel window status register type */
+typedef u32 mhuv2_stat_reg_t;
+#define MHUV2_STAT_BYTES sizeof(mhuv2_stat_reg_t)
+#define MHUV2_STAT_BITS (MHUV2_STAT_BYTES * __CHAR_BIT__)
+
+#define LSB_MASK(n) ((1 << (n)) - 1)
+
+#ifndef PAD
+#define _PADLINE(line) pad##line
+#define _XSTR(line) _PADLINE(line)
+#define PAD _XSTR(__LINE__)
+#endif
+
+/* ====== Arm MHUv2 register defines ====== */
+
+/* Register Message Handling Unit Configuration fields */
+struct MHU_CFG_t {
+ u32 NUM_CH : 7;
+ u32 PAD : 25;
+} __packed;
+
+/* Register Implementer Identification fields */
+struct IIDR_t {
+ u32 IMPLEMENTER : 12;
+ u32 REVISION : 4;
+ u32 VARIANT : 4;
+ u32 PRODUCT_ID : 12;
+} __packed;
+
+/* Register Architecture Identification Register fields */
+struct AIDR_t {
+ u32 ARCH_MINOR_REV : 4;
+ u32 ARCH_MAJOR_REV : 4;
+ u32 PAD : 24;
+} __packed;
+
+/* register Interrupt Status fields */
+struct INT_ST_t {
+ u32 NR2R : 1;
+ u32 R2NR : 1;
+ u32 PAD : 30;
+} __packed;
+
+/* Register Interrupt Clear fields */
+struct INT_CLR_t {
+ u32 NR2R : 1;
+ u32 R2NR : 1;
+ u32 PAD : 30;
+} __packed;
+
+/* Register Interrupt Enable fields */
+struct INT_EN_t {
+ u32 R2NR : 1;
+ u32 NR2R : 1;
+ u32 CHCOMB : 1;
+ u32 PAD : 29;
+} __packed;
+
+/* Sender Channel Window fields */
+struct mhu2_send_channel_reg {
+ mhuv2_stat_reg_t STAT;
+ u8 PAD[0xC - 0x4];
+ mhuv2_stat_reg_t STAT_SET;
+ u8 PAD[0x20 - 0x10];
+} __packed;
+
+/* Sender frame register fields */
+struct mhu2_send_frame_reg {
+ struct mhu2_send_channel_reg channel[MHUV2_CHANNEL_MAX];
+ struct MHU_CFG_t MHU_CFG;
+ u32 RESP_CFG;
+ u32 ACCESS_REQUEST;
+ u32 ACCESS_READY;
+ struct INT_ST_t INT_ST;
+ struct INT_CLR_t INT_CLR;
+ struct INT_EN_t INT_EN;
+ u32 RESERVED0;
+ u32 CHCOMB_INT_ST[MHUV2_CMB_INT_ST_REG_CNT];
+ u8 PAD[0xFC8 - 0xFB0];
+ struct IIDR_t IIDR;
+ struct AIDR_t AIDR;
+} __packed;
+
+/* Receiver Channel Window fields */
+struct mhu2_recv_channel_reg {
+ mhuv2_stat_reg_t STAT;
+ mhuv2_stat_reg_t STAT_PEND;
+ mhuv2_stat_reg_t STAT_CLEAR;
+ u8 RESERVED0[0x10 - 0x0C];
+ mhuv2_stat_reg_t MASK;
+ mhuv2_stat_reg_t MASK_SET;
+ mhuv2_stat_reg_t MASK_CLEAR;
+ u8 PAD[0x20 - 0x1C];
+} __packed;
+
+/* Receiver frame register fields */
+struct mhu2_recv_frame_reg {
+ struct mhu2_recv_channel_reg channel[MHUV2_CHANNEL_MAX];
+ struct MHU_CFG_t MHU_CFG;
+ u8 RESERVED0[0xF90 - 0xF84];
+ struct INT_ST_t INT_ST;
+ struct INT_CLR_t INT_CLR;
+ struct INT_EN_t INT_EN;
+ u32 PAD;
+ mhuv2_stat_reg_t CHCOMB_INT_ST[MHUV2_CMB_INT_ST_REG_CNT];
+ u8 RESERVED2[0xFC8 - 0xFB0];
+ struct IIDR_t IIDR;
+ struct AIDR_t AIDR;
+} __packed;
+
+/* ====== Arm MHUv2 device tree property identifiers ====== */
+
+static const char *const mhuv2_protocol_dt_identifiers[] = { "single-word",
+ "multi-word",
+ "doorbell" };
+
+static const char *const mhuv2_frame_dt_identifiers[] = { "receiver",
+ "sender" };
+
+/* ====== Arm MHUv2 data structures ====== */
+
+enum mhuv2_transport_protocol { SINGLE_WORD, MULTI_WORD, DOORBELL };
+
+enum mhuv2_frame { RECEIVER_FRAME, SENDER_FRAME };
+
+/**
+ * Arm MHUv2 operations
+ *
+ * Each transport protocol must provide an implementation of the operations
+ * presented in this structure.
+ * Most operations present a struct mbox_chan* as argument. This channel will
+ * correspond to one of the virtual channels within the MHU device. What
+ * constitutes a channel within the MHU device is dependent on the transport
+ * protocol.
+ */
+struct arm_mhuv2;
+struct mhuv2_ops {
+ int (*read_data)(struct arm_mhuv2 *mhuv2, struct mbox_chan *chan,
+ struct arm_mbox_msg *msg);
+ int (*clear_data)(struct arm_mhuv2 *mhuv2, struct mbox_chan *chan,
+ struct arm_mbox_msg *msg);
+ int (*send_data)(struct arm_mhuv2 *mhuv2, struct mbox_chan *chan,
+ const struct arm_mbox_msg *msg);
+ int (*setup)(struct arm_mhuv2 *mhuv2);
+ int (*last_tx_done)(struct arm_mhuv2 *mhuv2, struct mbox_chan *chan);
+ struct mbox_chan *(*get_active_mbox_chan)(struct arm_mhuv2 *mhuv2);
+};
+
+/**
+ * Arm MHUv2 mailbox channel information
+ *
+ * A channel contains a notion of its index within the array of mailbox channels
+ * which a mailbox controller allocates.
+ */
+struct arm_mhuv2_mbox_chan_priv {
+ u32 ch_idx;
+};
+
+#define mhuv2_chan_idx(_chanptr) \
+ (((struct arm_mhuv2_mbox_chan_priv *)(_chanptr)->con_priv)->ch_idx)
+
+/**
+ * Arm MHUv2 mailbox controller data
+ *
+ * @reg: Base address of the register mapping region
+ * @protocol: Transport protocol, derived from device tree
+ * @frame: Frame type, derived from device tree
+ * @irq: Interrupt, only valid for receiver frames
+ * @mbox: Mailbox controller belonging to the MHU frame
+ * @ops: Pointer to transport-protocol specific operations
+ * @dev: Device to which it is attached
+ */
+struct arm_mhuv2 {
+ union {
+ struct mhu2_send_frame_reg __iomem *send;
+ struct mhu2_recv_frame_reg __iomem *recv;
+ } reg;
+ enum mhuv2_transport_protocol protocol;
+ enum mhuv2_frame frame;
+ unsigned int irq;
+ struct mbox_controller mbox;
+ struct mhuv2_ops *ops;
+ struct device *dev;
+};
+
+#define mhu2_from_mbox_ctrl(_mbox) container_of(_mbox, struct arm_mhuv2, mbox)
+#define mhu2_from_mbox_chan(_chan) mhu2_from_mbox_ctrl(_chan->mbox)
+
+/* Macro for reading a bitfield within a physically mapped packed struct */
+#define readl_relaxed_bitfield(_regptr, _field) \
+ ({ \
+ mhuv2_stat_reg_t _regval; \
+ BUILD_BUG_ON(sizeof(*(_regptr)) > sizeof(typeof(_regval))); \
+ _regval = readl_relaxed((_regptr)); \
+ (*(typeof((_regptr)))(&_regval))._field; \
+ })
+
+/* Macro for writing a bitfield within a physically mapped packed struct */
+#define writel_relaxed_bitfield(_value, _regptr, _field) \
+ ({ \
+ mhuv2_stat_reg_t _regval; \
+ BUILD_BUG_ON(sizeof(*_regptr) > sizeof(typeof(_regval))); \
+ _regval = readl_relaxed(_regptr); \
+ (*(typeof(_regptr))(&_regval))._field = _value; \
+ writel_relaxed(_regval, _regptr); \
+ })
+
+static inline int __find_set_bit(uint32_t val)
+{
+ const uint32_t trailing_zeros = __builtin_ctz(val);
+ return trailing_zeros == 32 ? -1 : trailing_zeros;
+}
+
+/**
+ * Get index of a set bit within the combined interrupt status registers
+ *
+ * The function will calculate the index being the offset from the LSB of the
+ * first combined interrupt status register.
+ *
+ */
+static inline int mhuv2_combint_idx(struct arm_mhuv2 *mhuv2)
+{
+ int ch_idx = 0;
+ int set_bit_index, reg_idx;
+
+ for (reg_idx = 0; reg_idx < MHUV2_CMB_INT_ST_REG_CNT; reg_idx++) {
+ mhuv2_stat_reg_t stat_reg;
+
+ stat_reg =
+ readl_relaxed(&mhuv2->reg.recv->CHCOMB_INT_ST[reg_idx]);
+ set_bit_index = __find_set_bit(stat_reg);
+ if (set_bit_index == -1) {
+ ch_idx += MHUV2_STAT_BITS;
+ } else {
+ ch_idx += set_bit_index;
+ break;
+ }
+ }
+ return (ch_idx >= (MHUV2_CMB_INT_ST_REG_CNT * MHUV2_STAT_BITS) ?
+ MHUV2_CH_UNKNOWN :
+ ch_idx);
+}
+
+/* ================ Single word transport protocol operations =============== */
+static inline int mhuv20_get_non_zero_ch_idx(struct arm_mhuv2 *mhuv2)
+{
+ /* Locate a channel window with a non-zero STAT register */
+ int ch_idx;
+ int ch = MHUV2_CH_UNKNOWN;
+
+ for (ch_idx = 0;
+ ch_idx < readl_relaxed_bitfield(&mhuv2->reg.recv->MHU_CFG, NUM_CH);
+ ch_idx++) {
+ if (readl_relaxed(&mhuv2->reg.recv->channel[ch].STAT)) {
+ ch = ch_idx;
+ break;
+ }
+ }
+ return ch;
+}
+
+static inline int mhuv2_get_non_zero_ch_idx(struct arm_mhuv2 *mhuv2)
+{
+ /* Identify index of channel window containing non-zero data */
+ switch (readl_relaxed_bitfield(&mhuv2->reg.recv->AIDR,
+ ARCH_MINOR_REV)) {
+ case 1:
+ return mhuv2_combint_idx(mhuv2);
+ default:
+ return mhuv20_get_non_zero_ch_idx(mhuv2);
+ }
+}
+
+static inline int mhuv2_read_data_single_word(struct arm_mhuv2 *mhuv2,
+ struct mbox_chan *chan,
+ struct arm_mbox_msg *msg)
+{
+ const u32 ch_idx = mhuv2_chan_idx(chan);
+
+ msg->data = kzalloc(MHUV2_STAT_BYTES, GFP_KERNEL);
+ if (!msg->data)
+ return -ENOMEM;
+
+ *(mhuv2_stat_reg_t *)msg->data =
+ readl_relaxed(&mhuv2->reg.recv->channel[ch_idx].STAT);
+ msg->len = MHUV2_STAT_BYTES;
+ return 0;
+}
+
+static inline int mhuv2_clear_data_single_word(struct arm_mhuv2 *mhuv2,
+ struct mbox_chan *chan,
+ struct arm_mbox_msg *msg)
+{
+ const u32 ch_idx = mhuv2_chan_idx(chan);
+
+ writel_relaxed(readl_relaxed(&mhuv2->reg.recv->channel[ch_idx].STAT),
+ &mhuv2->reg.recv->channel[ch_idx].STAT_CLEAR);
+ return 0;
+}
+
+static inline int mhuv2_send_data_single_word(struct arm_mhuv2 *mhuv2,
+ struct mbox_chan *chan,
+ const struct arm_mbox_msg *msg)
+{
+ const u32 ch_idx = mhuv2_chan_idx(chan);
+ int bytes_left = msg->len;
+ char *data = msg->data;
+
+ if (ch_idx >= readl_relaxed_bitfield(&mhuv2->reg.recv->MHU_CFG, NUM_CH))
+ return -ENODEV;
+
+ while (bytes_left > 0) {
+ mhuv2_stat_reg_t word = *(mhuv2_stat_reg_t *)(data);
+
+ if (bytes_left < MHUV2_STAT_BYTES)
+ word &= LSB_MASK(bytes_left * __CHAR_BIT__);
+
+ if (!word) {
+ dev_err(mhuv2->dev,
+ "Data transmitted in single-word mode must be non-zero\n");
+ return -EINVAL;
+ }
+ writel_relaxed(word,
+ &mhuv2->reg.send->channel[ch_idx].STAT_SET);
+ while (readl_relaxed(&mhuv2->reg.send->channel[ch_idx].STAT))
+ continue;
+ bytes_left -= MHUV2_STAT_BYTES;
+ data += MHUV2_STAT_BYTES;
+ }
+
+ return 0;
+}
+
+static inline int mhuv2_last_tx_done_single_word(struct arm_mhuv2 *mhuv2,
+ struct mbox_chan *chan)
+{
+ const u32 ch_idx = mhuv2_chan_idx(chan);
+
+ return readl_relaxed(&mhuv2->reg.send->channel[ch_idx].STAT) == 0;
+}
+
+static inline int mhuv2_setup_single_word(struct arm_mhuv2 *mhuv2)
+{
+ int i;
+ const u32 channel_windows =
+ readl_relaxed_bitfield(mhuv2->frame == RECEIVER_FRAME ?
+ &mhuv2->reg.recv->MHU_CFG :
+ &mhuv2->reg.send->MHU_CFG,
+ NUM_CH);
+
+ mhuv2->mbox.num_chans = channel_windows;
+ mhuv2->mbox.chans =
+ devm_kzalloc(mhuv2->dev,
+ mhuv2->mbox.num_chans * sizeof(struct mbox_chan),
+ GFP_KERNEL);
+
+ for (i = 0; i < mhuv2->mbox.num_chans; i++) {
+ mhuv2->mbox.chans[i].con_priv =
+ devm_kzalloc(mhuv2->dev,
+ sizeof(struct arm_mhuv2_mbox_chan_priv),
+ GFP_KERNEL);
+ mhuv2_chan_idx(&mhuv2->mbox.chans[i]) = i;
+ }
+
+ if (mhuv2->frame == RECEIVER_FRAME) {
+ /* Ensure all status registers are unmasked */
+ for (i = 0; i < channel_windows; i++) {
+ writel_relaxed(0x0,
+ &mhuv2->reg.recv->channel[i].MASK_SET);
+ }
+ }
+
+ return 0;
+}
+
+static inline struct mbox_chan *
+ mhuv2_get_active_mbox_chan_single_word(struct arm_mhuv2 *mhuv2)
+{
+ const u32 ch_idx = mhuv2_get_non_zero_ch_idx(mhuv2);
+
+ if (ch_idx >= mhuv2->mbox.num_chans) {
+ dev_err(mhuv2->dev,
+ "Invalid active channel in single word mode\n");
+ return ERR_PTR(-EINVAL);
+ }
+ return &mhuv2->mbox.chans[ch_idx];
+}
+
+static const struct mhuv2_ops mhuv2_single_word_ops = {
+ .read_data = mhuv2_read_data_single_word,
+ .clear_data = mhuv2_clear_data_single_word,
+ .send_data = mhuv2_send_data_single_word,
+ .setup = mhuv2_setup_single_word,
+ .last_tx_done = mhuv2_last_tx_done_single_word,
+ .get_active_mbox_chan = mhuv2_get_active_mbox_chan_single_word,
+};
+/* ========================================================================== */
+
+/*
+ * MHUv2 receiver interrupt service routine
+ *
+ * This routine will be called whenever a reception interrupt is raised on the
+ * MHU device. Given that an MHU device may manage multiple mailboxes, it is
+ * up to the protocol-specific operations to determine:
+ * - What is the active mailbox channel
+ * - Read the data within the MHU corresponding to the channel
+ * - Clear the data within the MHU corresponding to the channel
+ *
+ * These operations must also ensure to not overwrite any data which may belong
+ * to a different mailbox channel. For instance, if data is received in two
+ * channel windows in single-word mode, the ISR will read and clear the data
+ * from one of these channel windows within a pass. This will result in a status
+ * register being non-zero upon returning from this routine, which in turn
+ * will keep the interrupt asserted for a second round.
+ */
+static irqreturn_t mhuv2_rx_interrupt(int irq, void *data)
+{
+ struct arm_mbox_msg msg;
+ int status;
+ struct arm_mhuv2 *mhuv2 = data;
+ struct mbox_chan *chan = mhuv2->ops->get_active_mbox_chan(mhuv2);
+
+ msg.data = NULL;
+ msg.len = 0;
+
+ status = mhuv2->ops->read_data(mhuv2, chan, &msg);
+ if (status != 0)
+ goto rx_exit;
+
+ if (!chan->cl) {
+ dev_warn(
+ mhuv2->dev,
+ "Warning: Received data on channel not currently attached to a mailbox client\n");
+ } else {
+ mbox_chan_received_data(chan, (void *)&msg);
+ }
+
+ status = mhuv2->ops->clear_data(mhuv2, chan, &msg);
+
+rx_exit:
+ kfree(msg.data);
+
+ return status == 0 ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static bool mhuv2_last_tx_done(struct mbox_chan *chan)
+{
+ struct arm_mhuv2 *mhuv2 = mhu2_from_mbox_chan(chan);
+
+ return mhuv2->ops->last_tx_done(mhuv2, chan);
+}
+
+static int mhuv2_send_data(struct mbox_chan *chan, void *data)
+{
+ struct arm_mhuv2 *mhuv2 = mhu2_from_mbox_chan(chan);
+ struct arm_mbox_msg *msg = data;
+ int ret;
+
+ if (!mhuv2->ops->last_tx_done(mhuv2, chan))
+ return -EBUSY;
+
+ ret = mhuv2->ops->send_data(mhuv2, chan, msg);
+ return ret;
+}
+
+static int mhuv2_startup(struct mbox_chan *chan)
+{
+ struct arm_mhuv2 *mhuv2 = mhu2_from_mbox_ctrl(chan->mbox);
+
+ writel_relaxed(0x1, &mhuv2->reg.send->ACCESS_REQUEST);
+ while (!readl_relaxed(&mhuv2->reg.send->ACCESS_READY))
+ continue;
+
+ return 0;
+}
+
+static void mhuv2_shutdown(struct mbox_chan *chan)
+{
+ struct arm_mhuv2 *mhuv2 = mhu2_from_mbox_ctrl(chan->mbox);
+
+ writel_relaxed(0x0, &mhuv2->reg.send->ACCESS_REQUEST);
+}
+
+static int mhuv2_recv_startup(struct mbox_chan *chan)
+{
+ return 0;
+}
+
+static void mhuv2_recv_shutdown(struct mbox_chan *chan)
+{
+}
+
+static int mhuv2_recv_send_data(struct mbox_chan *chan, void *data)
+{
+ dev_err(chan->mbox->dev,
+ "Trying to transmit on a receiver MHU frame\n");
+ return -EIO;
+}
+
+static bool mhuv2_recv_last_tx_done(struct mbox_chan *chan)
+{
+ dev_err(chan->mbox->dev, "Trying to Tx poll on a receiver MHU frame\n");
+ return true;
+}
+
+static const struct mbox_chan_ops mhuv2_receiver_ops = {
+ .send_data = mhuv2_recv_send_data,
+ .startup = mhuv2_recv_startup,
+ .shutdown = mhuv2_recv_shutdown,
+ .last_tx_done = mhuv2_recv_last_tx_done,
+};
+
+static const struct mbox_chan_ops mhuv2_sender_ops = {
+ .send_data = mhuv2_send_data,
+ .startup = mhuv2_startup,
+ .shutdown = mhuv2_shutdown,
+ .last_tx_done = mhuv2_last_tx_done,
+};
+
+static struct mbox_chan *mhuv2_mbox_of_xlate(struct mbox_controller *ctrl,
+ const struct of_phandle_args *pa)
+{
+ struct mbox_chan *chan;
+
+ if (pa->args_count != 1)
+ return ERR_PTR(-EINVAL);
+
+ if (pa->args[0] >= ctrl->num_chans)
+ return ERR_PTR(-ENOENT);
+
+ chan = &ctrl->chans[pa->args[0]];
+ return chan;
+}
+
+static int mhuv2_probe(struct amba_device *adev, const struct amba_id *id)
+{
+ int err;
+ struct device *dev = &adev->dev;
+ const struct device_node *np = dev->of_node;
+ struct arm_mhuv2 *mhuv2;
+ const char *str;
+
+ /* Allocate memory for device */
+ mhuv2 = devm_kzalloc(dev, sizeof(*mhuv2), GFP_KERNEL);
+ if (!mhuv2)
+ return -ENOMEM;
+
+ mhuv2->dev = dev;
+
+ /* Retrieve MHU frame specifier from device tree node */
+ err = of_property_read_string(np, "mhu-frame", &str);
+ if (err) {
+ dev_err(dev, "Probe failed: MHU frame not specified.");
+ return -ENODEV;
+ } else if (strcmp(str, mhuv2_frame_dt_identifiers[SENDER_FRAME]) == 0) {
+ mhuv2->frame = SENDER_FRAME;
+ } else if (strcmp(str, mhuv2_frame_dt_identifiers[RECEIVER_FRAME]) ==
+ 0) {
+ mhuv2->frame = RECEIVER_FRAME;
+ } else {
+ dev_err(dev,
+ "Probe failed; '%s' is not a valid MHU frame specifier\n",
+ str);
+ return -ENODEV;
+ }
+
+ /* Retrieve transport protocol specifier from device tree node */
+ err = of_property_read_string(np, "mhu-protocol", &str);
+ if (err) {
+ dev_err(dev,
+ "Probe failed: no transport protocol specified\n");
+ return -ENODEV;
+ } else if (strcmp(str, mhuv2_protocol_dt_identifiers[SINGLE_WORD]) ==
+ 0) {
+ mhuv2->protocol = SINGLE_WORD;
+ } else if (strcmp(str, mhuv2_protocol_dt_identifiers[MULTI_WORD]) ==
+ 0) {
+ mhuv2->protocol = MULTI_WORD;
+ } else if (strcmp(str, mhuv2_protocol_dt_identifiers[DOORBELL]) == 0) {
+ mhuv2->protocol = DOORBELL;
+ } else {
+ dev_err(dev,
+ "Probe failed: '%s' is not a valid transport protocol specifier\n",
+ str);
+ return -ENODEV;
+ }
+
+ /* Get MHU type specific properties from device tree */
+ if (mhuv2->frame == RECEIVER_FRAME) {
+ mhuv2->reg.recv = (struct mhu2_recv_frame_reg *)of_iomap(
+ (struct device_node *)np, 0);
+ if (!mhuv2->reg.recv)
+ goto io_fail;
+ mhuv2->irq = adev->irq[0];
+ } else {
+ mhuv2->reg.send = (struct mhu2_send_frame_reg *)of_iomap(
+ (struct device_node *)np, 0);
+ if (!mhuv2->reg.send)
+ goto io_fail;
+ }
+
+ /* Assign transport protocol-specific operations */
+ switch (mhuv2->protocol) {
+ case SINGLE_WORD:
+ mhuv2->ops = &mhuv2_single_word_ops;
+ break;
+ default:
+ return -ENODEV;
+ }
+
+ /* Mailbox controller setup */
+ mhuv2->mbox.dev = dev;
+ mhuv2->mbox.txdone_irq = false;
+ mhuv2->mbox.txdone_poll = true;
+ mhuv2->mbox.txpoll_period = 1;
+ mhuv2->mbox.of_xlate = mhuv2_mbox_of_xlate;
+ mhuv2->mbox.ops = mhuv2->frame == SENDER_FRAME ? &mhuv2_sender_ops :
+ &mhuv2_receiver_ops;
+ /*
+ * Transport protocol specific setup
+ * Setup function _must_ allocate mailbox channels according to the
+ * number of channels provided in the given transport protocol mode.
+ */
+ err = mhuv2->ops->setup(mhuv2);
+ if (err)
+ return err;
+
+ /* Request an interrupt if this is a receiver frame */
+ if (mhuv2->frame == RECEIVER_FRAME) {
+ err = request_irq(mhuv2->irq, mhuv2_rx_interrupt, IRQF_SHARED,
+ "mhuv2_link", mhuv2);
+ if (err) {
+ dev_err(dev, "unable to acquire IRQ %d\n", mhuv2->irq);
+ return err;
+ }
+ /*
+ * For minor version 1 and forward, the combined interrupt of
+ * the receiver frame must be explicitly enabled during startup.
+ */
+ if (readl_relaxed_bitfield(&mhuv2->reg.recv->AIDR,
+ ARCH_MINOR_REV) > 0) {
+ writel_relaxed_bitfield(1, &mhuv2->reg.recv->INT_EN,
+ CHCOMB);
+ }
+ }
+
+ amba_set_drvdata(adev, mhuv2);
+
+ err = mbox_controller_register(&mhuv2->mbox);
+ if (err) {
+ dev_err(dev, "failed to register ARM MHUv2 driver %d\n", err);
+ iounmap(mhuv2->frame == RECEIVER_FRAME ? mhuv2->reg.recv :
+ mhuv2->reg.send);
+ return err;
+ }
+
+ dev_info(dev, "ARM MHUv2 %s frame (%s) Mailbox driver registered\n",
+ mhuv2_frame_dt_identifiers[mhuv2->frame],
+ mhuv2_protocol_dt_identifiers[mhuv2->protocol]);
+
+ return 0;
+
+io_fail:
+ dev_err(dev, "Probe failed: failed to map '%s' frame\n",
+ mhuv2_frame_dt_identifiers[mhuv2->frame]);
+ iounmap(mhuv2->frame == RECEIVER_FRAME ? mhuv2->reg.recv :
+ mhuv2->reg.send);
+ return -ENOMEM;
+}
+
+static int mhuv2_remove(struct amba_device *adev)
+{
+ return 0;
+}
+
+static struct amba_id mhuv2_ids[] = {
+ {
+ .id = 0x4b0d1,
+ .mask = 0xfffff,
+ },
+ {
+ .id = 0xbb0d1,
+ .mask = 0xfffff,
+ },
+ { 0, 0 },
+};
+MODULE_DEVICE_TABLE(amba, mhuv2_ids);
+
+static struct amba_driver arm_mhuv2_driver = {
+ .drv = {
+ .name = "mhuv2",
+ },
+ .id_table = mhuv2_ids,
+ .probe = mhuv2_probe,
+ .remove = mhuv2_remove,
+};
+module_amba_driver(arm_mhuv2_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("ARM MHUv2 Driver");
+MODULE_AUTHOR("Morten Borup Petersen <[email protected]>");
diff --git a/include/linux/mailbox/arm-mbox-message.h b/include/linux/mailbox/arm-mbox-message.h
new file mode 100644
index 000000000000..112b4f927c1a
--- /dev/null
+++ b/include/linux/mailbox/arm-mbox-message.h
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Arm Mailbox Message
+ *
+ * The Arm mailbox message structure is used to pass data- and length
+ * information between a mailbox client and mailbox controller, through the
+ * provided void* of the common mailbox frameworks send- and receive APIs.
+ *
+ * This will be utilized when a mailbox controller is able to transmit
+ * more than a single word within a transmission, allowing the controller
+ * to fully utilize its available resources.
+ * No message protocol is enforced through this structure - a utilizing mailbox
+ * client driver shall implement its own message protocol, which may then be
+ * transmitted through an arm_mbox_msg.
+ *
+ * Given a message protocol of size A and an arm_mbox_msg containing data of
+ * length B, a mailbox channel may callback with B < A. In this case, the
+ * message protocol driver must implement a state machine which allows for
+ * multiple callbacks that provides parts of a full message of size A. This
+ * state machine must account for, that the length of the arm_mbox_msg received
+ * may vary between callbacks based on the underlying hardware as well as the
+ * transmitted data.
+ *
+ * Copyright (C) 2019 Arm Ltd.
+ */
+
+#ifndef _LINUX_ARM_MBOX_MESSAGE_H_
+#define _LINUX_ARM_MBOX_MESSAGE_H_
+
+#include <linux/types.h>
+
+struct arm_mbox_msg {
+ void *data;
+ size_t len;
+};
+
+#endif /* _LINUX_ARM_MBOX_MESSAGE_H_ */
--
2.17.1

2019-07-17 19:27:51

[permalink] [raw]

Subject: [PATCH 3/4] mailbox: arm_mhuv2: add doorbell transport protocol operations

From: Morten Borup Petersen <[email protected]>

In doorbell mode, the mailbox controller will provide a mailbox for each
flag bit available in the combined number of channel windows available
within the MHU device.

When in doorbell mode, the MHU should be used solely as an interrupt
generating mechanism. If data is to be transmitted, this must be done
out-band, ie. through shared memory, by a driving utilizing the mailbox
for interrupt generation.

Signed-off-by: Morten Borup Petersen <[email protected]>
Signed-off-by: Tushar Khandelwal <[email protected]>
Cc:Jassi Brar <[email protected]
Cc: [email protected]
---
drivers/mailbox/arm_mhu_v2.c | 108 +++++++++++++++++++++++++++++++++++
1 file changed, 108 insertions(+)

diff --git a/drivers/mailbox/arm_mhu_v2.c b/drivers/mailbox/arm_mhu_v2.c
index a0af683b83a2..0e3fa5917925 100644
--- a/drivers/mailbox/arm_mhu_v2.c
+++ b/drivers/mailbox/arm_mhu_v2.c
@@ -430,6 +430,111 @@ static const struct mhuv2_ops mhuv2_single_word_ops = {
};
/* ========================================================================== */

+/* =================== Doorbell transport protocol operations =============== */
+
+static inline int mhuv2_read_data_doorbell(struct arm_mhuv2 *mhuv2,
+ struct mbox_chan *chan,
+ struct arm_mbox_msg *msg)
+{
+ return 0;
+}
+
+static inline int mhuv2_clear_data_doorbell(struct arm_mhuv2 *mhuv2,
+ struct mbox_chan *chan,
+ struct arm_mbox_msg *msg)
+{
+ const u32 ch_mbox_idx = mhuv2_chan_idx(chan);
+ const u32 ch_window_idx = ch_mbox_idx / MHUV2_STAT_BITS;
+ const u32 ch_window_reg_idx = ch_mbox_idx % MHUV2_STAT_BITS;
+
+ writel_relaxed(BIT(ch_window_reg_idx),
+ &mhuv2->reg.recv->channel[ch_window_idx].STAT_CLEAR);
+ return 0;
+}
+
+static inline int mhuv2_send_data_doorbell(struct arm_mhuv2 *mhuv2,
+ struct mbox_chan *chan,
+ const struct arm_mbox_msg *msg)
+{
+ const u32 ch_mbox_idx = mhuv2_chan_idx(chan);
+ const u32 ch_window_idx = ch_mbox_idx / MHUV2_STAT_BITS;
+ const u32 ch_window_reg_idx = ch_mbox_idx % MHUV2_STAT_BITS;
+
+ writel_relaxed(
+ readl_relaxed(&mhuv2->reg.send->channel[ch_window_idx].STAT) |
+ BIT(ch_window_reg_idx),
+ &mhuv2->reg.send->channel[ch_window_idx].STAT_SET);
+ return 0;
+}
+
+static inline int mhuv2_last_tx_done_doorbell(struct arm_mhuv2 *mhuv2,
+ struct mbox_chan *chan)
+{
+ const u32 ch_mbox_idx = mhuv2_chan_idx(chan);
+ const u32 ch_window_idx = ch_mbox_idx / MHUV2_STAT_BITS;
+ const u32 ch_window_reg_idx = ch_mbox_idx % MHUV2_STAT_BITS;
+
+ return (readl_relaxed(&mhuv2->reg.send->channel[ch_window_idx].STAT) &
+ BIT(ch_window_reg_idx)) == 0;
+}
+
+static inline int mhuv2_setup_doorbell(struct arm_mhuv2 *mhuv2)
+{
+ int i;
+ const u32 channel_windows =
+ readl_relaxed_bitfield(mhuv2->frame == RECEIVER_FRAME ?
+ &mhuv2->reg.recv->MHU_CFG :
+ &mhuv2->reg.send->MHU_CFG,
+ NUM_CH);
+
+ mhuv2->mbox.num_chans = MHUV2_STAT_BITS * channel_windows;
+ mhuv2->mbox.chans =
+ devm_kzalloc(mhuv2->dev,
+ mhuv2->mbox.num_chans * sizeof(struct mbox_chan),
+ GFP_KERNEL);
+
+ for (i = 0; i < mhuv2->mbox.num_chans; i++) {
+ mhuv2->mbox.chans[i].con_priv =
+ devm_kzalloc(mhuv2->dev,
+ sizeof(struct arm_mhuv2_mbox_chan_priv),
+ GFP_KERNEL);
+ mhuv2_chan_idx(&mhuv2->mbox.chans[i]) = i;
+ }
+
+ if (mhuv2->frame == RECEIVER_FRAME) {
+ /* Ensure all status registers are unmasked */
+ for (i = 0; i < channel_windows; i++) {
+ writel_relaxed(0x0,
+ &mhuv2->reg.recv->channel[i].MASK_SET);
+ }
+ }
+
+ return 0;
+}
+
+static inline struct mbox_chan *
+ mhuv2_get_active_mbox_chan_doorbell(struct arm_mhuv2 *mhuv2)
+{
+ const u32 ch_window_idx = mhuv2_combint_idx(mhuv2);
+ const u32 ch_window_reg_idx = __find_set_bit(
+ readl_relaxed(&mhuv2->reg.recv->channel[ch_window_idx].STAT));
+ if (ch_window_reg_idx == -1)
+ return ERR_PTR(-EIO);
+
+ return &mhuv2->mbox.chans[ch_window_reg_idx +
+ ch_window_idx * MHUV2_STAT_BITS];
+}
+
+static const struct mhuv2_ops mhuv2_doorbell_ops = {
+ .read_data = mhuv2_read_data_doorbell,
+ .clear_data = mhuv2_clear_data_doorbell,
+ .send_data = mhuv2_send_data_doorbell,
+ .setup = mhuv2_setup_doorbell,
+ .last_tx_done = mhuv2_last_tx_done_doorbell,
+ .get_active_mbox_chan = mhuv2_get_active_mbox_chan_doorbell,
+};
+/* ========================================================================== */
+
/*
* MHUv2 receiver interrupt service routine
*
@@ -638,6 +743,9 @@ static int mhuv2_probe(struct amba_device *adev, const struct amba_id *id)
case SINGLE_WORD:
mhuv2->ops = &mhuv2_single_word_ops;
break;
+ case DOORBELL:
+ mhuv2->ops = &mhuv2_doorbell_ops;
+ break;
default:
return -ENODEV;
}
--
2.17.1

2019-07-17 19:28:01

[permalink] [raw]

Subject: [PATCH 4/4] mailbox: arm_mhuv2: add multi word transport protocol operations

From: Morten Borup Petersen <[email protected]>

When in multi-word mode, the mailbox controller will provide a single
mailbox. It is required that the MHU device has at least 2 channel windows
available for the MHU to function in multi-word mode.

Transmitting and receiving data through the mailbox framework in
multi-word mode is done through a struct arm_mbox_msg.

Signed-off-by: Morten Borup Petersen <[email protected]>
Signed-off-by: Tushar Khandelwal <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/mailbox/arm_mhu_v2.c | 225 +++++++++++++++++++++++++++++++++++
1 file changed, 225 insertions(+)

diff --git a/drivers/mailbox/arm_mhu_v2.c b/drivers/mailbox/arm_mhu_v2.c
index 0e3fa5917925..324b19bdb28a 100644
--- a/drivers/mailbox/arm_mhu_v2.c
+++ b/drivers/mailbox/arm_mhu_v2.c
@@ -430,6 +430,228 @@ static const struct mhuv2_ops mhuv2_single_word_ops = {
};
/* ========================================================================== */

+/* ================ Multi word transport protocol operations ================ */
+static inline int mhuv2_read_data_multi_word(struct arm_mhuv2 *mhuv2,
+ struct mbox_chan *chan,
+ struct arm_mbox_msg *msg)
+{
+ int ch;
+ const int channels =
+ readl_relaxed_bitfield(&mhuv2->reg.recv->MHU_CFG, NUM_CH);
+
+ msg->data = kzalloc(MHUV2_STAT_BYTES * channels, GFP_KERNEL);
+
+ for (ch = 0; ch < channels; ch++) {
+ /*
+ * Messages are expected to be received in order of most
+ * significant word to least significant word.
+ * (see mhuv2_send_data_multi_word)
+ */
+ const mhuv2_stat_reg_t word =
+ readl_relaxed(&mhuv2->reg.recv->channel[ch].STAT);
+ ((mhuv2_stat_reg_t *)msg->data)[channels - 1 - ch] = word;
+ }
+
+ msg->len = channels * MHUV2_STAT_BYTES;
+ return 0;
+}
+
+static inline int mhuv2_clear_data_multi_word(struct arm_mhuv2 *mhuv2,
+ struct mbox_chan *chan,
+ struct arm_mbox_msg *msg)
+{
+ int ch;
+ const int channels =
+ readl_relaxed_bitfield(&mhuv2->reg.recv->MHU_CFG, NUM_CH);
+
+ for (ch = 0; ch < channels; ch++) {
+ /*
+ * Last channel window must be cleared as the final operation.
+ * Upon clearing the last channel window register, which is
+ * unmasked in multi-word mode, the interrupt is deasserted.
+ */
+ writel_relaxed(
+ readl_relaxed(&mhuv2->reg.recv->channel[ch].STAT),
+ &mhuv2->reg.recv->channel[ch].STAT_CLEAR);
+ }
+ return 0;
+}
+
+static inline int __mhuv2_mw_bytes_to_send(const int bytes_in_round,
+ const int bytes_left)
+{
+ /*
+ * Bytes to send on the current channel will always be MHUV2_STAT_BYTES
+ * unless in the last round and
+ * msg->len % MHUV2_STAT_BYTES != 0
+ */
+ if (bytes_in_round % MHUV2_STAT_BYTES != 0) {
+ const int bts = bytes_left % MHUV2_STAT_BYTES;
+ return bts == 0 ? MHUV2_STAT_BYTES : bts;
+ } else {
+ return MHUV2_STAT_BYTES;
+ }
+}
+
+static inline int mhuv2_send_data_multi_word(struct arm_mhuv2 *mhuv2,
+ struct mbox_chan *chan,
+ const struct arm_mbox_msg *msg)
+{
+ /*
+ * Message will be transmitted from most significant to least
+ * significant word. This is to allow for messages shorter than
+ * $channels to still trigger the receiver interrupt which gets
+ * activated when the last STAT register is written. As an example, a
+ * 6-word message is to be written on a 4-channel MHU connection:
+ * Registers marked with '*' are masked, and will not generate an
+ * interrupt on the receiver side once written.
+ *
+ * uint32_t *data = [0x00000001],[0x00000002],[0x00000003],[0x00000004],
+ * [0x00000005], [0x00000006]
+ *
+ * ROUND 1:
+ * STAT reg To write Write sequence
+ * [ STAT 3 ] <- [0x00000001] 4 <- triggers interrupt on receiver
+ * *[ STAT 2 ] <- [0x00000002] 3
+ * *[ STAT 1 ] <- [0x00000003] 2
+ * *[ STAT 0 ] <- [0x00000004] 1
+ *
+ * data += 4 // Increment data pointer by number of STAT regs
+ *
+ * ROUND 2:
+ * STAT reg To write Write sequence
+ * [ STAT 3 ] <- [0x00000005] 2 <- triggers interrupt on receiver
+ * *[ STAT 2 ] <- [0x00000006] 1
+ * *[ STAT 1 ] <- [0x00000000]
+ * *[ STAT 0 ] <- [0x00000000]
+ */
+ int bytes_left, bytes_to_send, i, ch_idx;
+ const int ch_windows =
+ readl_relaxed_bitfield(&mhuv2->reg.recv->MHU_CFG, NUM_CH);
+ const size_t round_capacity = ch_windows * MHUV2_STAT_BYTES;
+
+ bytes_left = msg->len;
+ mhuv2_stat_reg_t *data = msg->data;
+
+ while (bytes_left > 0) {
+ /* Note: Each entry of this loop indicates a new ROUND */
+ if (*(u32 *)data == 0) {
+ dev_err(mhuv2->dev,
+ "values in *data aligned on NUM_STAT boundaries must not be zero to ensure that receiver interrupt is triggered\n",
+ ch_windows);
+ return -EINVAL;
+ }
+
+ const int bytes_in_round = bytes_left > round_capacity ?
+ round_capacity :
+ bytes_left;
+
+ for (i = (ch_windows - 1); i >= 0; i--) {
+ ch_idx = ch_windows - 1 - i;
+ /*
+ * Check whether data should be transmitted in register
+ * of index 'ch'.
+ */
+ if (bytes_in_round > (i * MHUV2_STAT_BYTES)) {
+ mhuv2_stat_reg_t word = data[i];
+
+ bytes_to_send = __mhuv2_mw_bytes_to_send(
+ bytes_in_round, bytes_left);
+
+ if (bytes_to_send != MHUV2_STAT_BYTES) {
+ word &= LSB_MASK(bytes_to_send *
+ __CHAR_BIT__);
+ }
+ while (readl_relaxed(
+ &mhuv2->reg.send->channel[ch_idx]
+ .STAT) != 0)
+ continue;
+
+ writel_relaxed(
+ word,
+ &mhuv2->reg.send->channel[ch_idx].STAT_SET);
+ bytes_left -= bytes_to_send;
+ }
+ }
+
+ data += ch_windows;
+
+ for (ch_idx = 0; ch_idx < ch_windows; ch_idx++) {
+ while (readl_relaxed(
+ &mhuv2->reg.send->channel[ch_idx].STAT) != 0)
+ continue;
+ }
+ }
+ return 0;
+}
+
+
+static inline int mhuv2_last_tx_done_multi_word(struct arm_mhuv2 *mhuv2,
+ struct mbox_chan *chan)
+{
+ int ch_idx;
+ bool tx_done = true;
+
+ for (ch_idx = 0;
+ ch_idx < readl_relaxed_bitfield(&mhuv2->reg.send->MHU_CFG, NUM_CH);
+ ch_idx++) {
+ tx_done &= readl_relaxed(
+ &mhuv2->reg.send->channel[ch_idx].STAT) == 0;
+ }
+ return tx_done;
+}
+
+static inline int mhuv2_setup_multi_word(struct arm_mhuv2 *mhuv2)
+{
+ int ret, i;
+
+ const u32 channel_windows =
+ readl_relaxed_bitfield(mhuv2->frame == RECEIVER_FRAME ?
+ &mhuv2->reg.recv->MHU_CFG :
+ &mhuv2->reg.send->MHU_CFG,
+ NUM_CH);
+ if (channel_windows < 2) {
+ dev_err(mhuv2->dev,
+ "Error: at least 2 MHU channel windows are required for using the multi-word transfer protocol");
+ return -ENODEV;
+ }
+
+ if (mhuv2->frame == RECEIVER_FRAME) {
+ /*
+ * The multi-word transport protocol mandates that all but
+ * the last status register must be masked.
+ */
+ for (i = 0; i < (channel_windows - 1); i++) {
+ writel_relaxed(-1,
+ &mhuv2->reg.recv->channel[i].MASK_SET);
+ }
+ }
+
+ mhuv2->mbox.num_chans = 1;
+ mhuv2->mbox.chans =
+ devm_kzalloc(mhuv2->dev,
+ mhuv2->mbox.num_chans * sizeof(struct mbox_chan),
+ GFP_KERNEL);
+
+ return 0;
+}
+
+static inline struct mbox_chan *
+ mhuv2_get_active_mbox_chan_multi_word(struct arm_mhuv2 *mhuv2)
+{
+ return &mhuv2->mbox.chans[0];
+}
+
+static const struct mhuv2_ops mhuv2_multi_word_ops = {
+ .read_data = mhuv2_read_data_multi_word,
+ .clear_data = mhuv2_clear_data_multi_word,
+ .send_data = mhuv2_send_data_multi_word,
+ .setup = mhuv2_setup_multi_word,
+ .last_tx_done = mhuv2_last_tx_done_multi_word,
+ .get_active_mbox_chan = mhuv2_get_active_mbox_chan_multi_word,
+};
+/* ========================================================================== */
+
/* =================== Doorbell transport protocol operations =============== */

static inline int mhuv2_read_data_doorbell(struct arm_mhuv2 *mhuv2,
@@ -740,6 +962,9 @@ static int mhuv2_probe(struct amba_device *adev, const struct amba_id *id)

/* Assign transport protocol-specific operations */
switch (mhuv2->protocol) {
+ case MULTI_WORD:
+ mhuv2->ops = &mhuv2_multi_word_ops;
+ break;
case SINGLE_WORD:
mhuv2->ops = &mhuv2_single_word_ops;
break;
--
2.17.1

2019-07-21 22:46:07

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On Wed, Jul 17, 2019 at 2:26 PM Tushar Khandelwal
<[email protected]> wrote:

> diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
> new file mode 100644
> index 000000000000..3a05593414bc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
> @@ -0,0 +1,108 @@
> +Arm MHUv2 Mailbox Driver
> +========================
> +
> +The Arm Message-Handling-Unit (MHU) Version 2 is a mailbox controller that has
> +between 1 and 124 channel windows to provide unidirectional communication with
> +remote processor(s).
> +
> +Given the unidirectional nature of the device, an MHUv2 mailbox may only be
> +written to or read from. If a pair of MHU devices is implemented between two
> +processing elements to provide bidirectional communication, these must be
> +specified as two separate mailboxes.
> +
> +A device tree node for an Arm MHUv2 device must specify either a receiver frame
> +or a sender frame, indicating which end of the unidirectional MHU device which
> +the device node entry describes.
> +
> +An MHU device must be specified with a transport protocol. The transport
> +protocol of an MHU device determines the method of data transmission as well as
> +the number of provided mailboxes.
> +Following are the possible transport protocol types:
> +- Single-word: An MHU device implements as many mailboxes as it
> + provides channel windows. Data is transmitted through
> + the MHU registers.
> +- Multi-word: An MHU device implements a single mailbox. All channel windows
> + will be used during transmission. Data is transmitted through
> + the MHU registers.
> +- Doorbell: An MHU device implements as many mailboxes as there are flag
> + bits available in its channel windows. Optionally, data may
> + be transmitted through a shared memory region, wherein the MHU
> + is used strictly as an interrupt generation mechanism.
> +
> +Mailbox Device Node:
> +====================
> +
> +Required properties:
> +--------------------
> +- compatible: Shall be "arm,mhuv2" & "arm,primecell"
> +- reg: Contains the mailbox register address range (base
> + address and length)
> +- #mbox-cells Shall be 1 - the index of the channel needed.
> +- mhu-frame Frame type of the device.
> + Shall be either "sender" or "receiver"
> +- mhu-protocol Transport protocol of the device. Shall be one of the
> + following: "single-word", "multi-word", "doorbell"
> +
> +Required properties (receiver frame):
> +-------------------------------------
> +- interrupts: Contains the interrupt information corresponding to the
> + combined interrupt of the receiver frame
> +
> +Example:
> +--------
> +
> + mbox_mw_tx: mhu@10000000 {
> + compatible = "arm,mhuv2","arm,primecell";
> + reg = <0x10000000 0x1000>;
> + clocks = <&refclk100mhz>;
> + clock-names = "apb_pclk";
> + #mbox-cells = <1>;
> + mhu-protocol = "multi-word";
> + mhu-frame = "sender";
> + };
> +
> + mbox_sw_tx: mhu@10000000 {
> + compatible = "arm,mhuv2","arm,primecell";
> + reg = <0x11000000 0x1000>;
> + clocks = <&refclk100mhz>;
> + clock-names = "apb_pclk";
> + #mbox-cells = <1>;
> + mhu-protocol = "single-word";
> + mhu-frame = "sender";
> + };
> +
> + mbox_db_rx: mhu@10000000 {
> + compatible = "arm,mhuv2","arm,primecell";
> + reg = <0x12000000 0x1000>;
> + clocks = <&refclk100mhz>;
> + clock-names = "apb_pclk";
> + #mbox-cells = <1>;
> + interrupts = <0 45 4>;
> + interrupt-names = "mhu_rx";
> + mhu-protocol = "doorbell";
> + mhu-frame = "receiver";
> + };
> +
> + mhu_client: scb@2e000000 {
> + compatible = "fujitsu,mb86s70-scb-1.0";
> + reg = <0 0x2e000000 0x4000>;
> + mboxes =
> + // For multi-word frames, client may only instantiate a single
> + // mailbox for a mailbox controller
> + <&mbox_mw_tx 0>,
> +
> + // For single-word frames, client may instantiate as many
> + // mailboxes as there are channel windows in the MHU
> + <&mbox_sw_tx 0>,
> + <&mbox_sw_tx 1>,
> + <&mbox_sw_tx 2>,
> + <&mbox_sw_tx 3>,
> +
> + // For doorbell frames, client may instantiate as many mailboxes
> + // as there are bits available in the combined number of channel
> + // windows ((channel windows * 32) mailboxes)
> + <mbox_db_rx 0>,
> + <mbox_db_rx 1>,
> + ...
> + <mbox_db_rx 17>;
> + };

If the mhuv2 instance implements, say, 3 channel windows between
sender (linux) and receiver (firmware), and Linux runs two protocols
each requiring 1 and 2-word sized messages respectively. The hardware
supports that by assigning windows [0] and [1,2] to each protocol.
However, I don't think the driver can support that. Or does it?

Also I see problem with implementing "protocol modes" in the
controller driver - 'mhu-protocol' property should go away. And
'mhu-frame' is unncessary - presence of interrupt property could imply
'receiver', otherwise 'sender'.

Cheers!

2019-07-25 12:05:08

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On Sun, Jul 21, 2019 at 4:58 PM Jassi Brar <[email protected]> wrote:
>
> On Wed, Jul 17, 2019 at 2:26 PM Tushar Khandelwal
> <[email protected]> wrote:
>
> > diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
> > new file mode 100644
> > index 000000000000..3a05593414bc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
> > @@ -0,0 +1,108 @@
> > +Arm MHUv2 Mailbox Driver
> > +========================
> > +
> > +The Arm Message-Handling-Unit (MHU) Version 2 is a mailbox controller that has
> > +between 1 and 124 channel windows to provide unidirectional communication with
> > +remote processor(s).
> > +
> > +Given the unidirectional nature of the device, an MHUv2 mailbox may only be
> > +written to or read from. If a pair of MHU devices is implemented between two
> > +processing elements to provide bidirectional communication, these must be
> > +specified as two separate mailboxes.
> > +
> > +A device tree node for an Arm MHUv2 device must specify either a receiver frame
> > +or a sender frame, indicating which end of the unidirectional MHU device which
> > +the device node entry describes.
> > +
> > +An MHU device must be specified with a transport protocol. The transport
> > +protocol of an MHU device determines the method of data transmission as well as
> > +the number of provided mailboxes.
> > +Following are the possible transport protocol types:
> > +- Single-word: An MHU device implements as many mailboxes as it
> > + provides channel windows. Data is transmitted through
> > + the MHU registers.
> > +- Multi-word: An MHU device implements a single mailbox. All channel windows
> > + will be used during transmission. Data is transmitted through
> > + the MHU registers.
> > +- Doorbell: An MHU device implements as many mailboxes as there are flag
> > + bits available in its channel windows. Optionally, data may
> > + be transmitted through a shared memory region, wherein the MHU
> > + is used strictly as an interrupt generation mechanism.
> > +
> > +Mailbox Device Node:
> > +====================
> > +
> > +Required properties:
> > +--------------------
> > +- compatible: Shall be "arm,mhuv2" & "arm,primecell"
> > +- reg: Contains the mailbox register address range (base
> > + address and length)
> > +- #mbox-cells Shall be 1 - the index of the channel needed.
> > +- mhu-frame Frame type of the device.
> > + Shall be either "sender" or "receiver"
> > +- mhu-protocol Transport protocol of the device. Shall be one of the
> > + following: "single-word", "multi-word", "doorbell"
> > +
> > +Required properties (receiver frame):
> > +-------------------------------------
> > +- interrupts: Contains the interrupt information corresponding to the
> > + combined interrupt of the receiver frame
> > +
> > +Example:
> > +--------
> > +
> > + mbox_mw_tx: mhu@10000000 {
> > + compatible = "arm,mhuv2","arm,primecell";
> > + reg = <0x10000000 0x1000>;
> > + clocks = <&refclk100mhz>;
> > + clock-names = "apb_pclk";
> > + #mbox-cells = <1>;
> > + mhu-protocol = "multi-word";
> > + mhu-frame = "sender";
> > + };
> > +
> > + mbox_sw_tx: mhu@10000000 {
> > + compatible = "arm,mhuv2","arm,primecell";
> > + reg = <0x11000000 0x1000>;
> > + clocks = <&refclk100mhz>;
> > + clock-names = "apb_pclk";
> > + #mbox-cells = <1>;
> > + mhu-protocol = "single-word";
> > + mhu-frame = "sender";
> > + };
> > +
> > + mbox_db_rx: mhu@10000000 {
> > + compatible = "arm,mhuv2","arm,primecell";
> > + reg = <0x12000000 0x1000>;
> > + clocks = <&refclk100mhz>;
> > + clock-names = "apb_pclk";
> > + #mbox-cells = <1>;
> > + interrupts = <0 45 4>;
> > + interrupt-names = "mhu_rx";
> > + mhu-protocol = "doorbell";
> > + mhu-frame = "receiver";
> > + };
> > +
> > + mhu_client: scb@2e000000 {
> > + compatible = "fujitsu,mb86s70-scb-1.0";
> > + reg = <0 0x2e000000 0x4000>;
> > + mboxes =
> > + // For multi-word frames, client may only instantiate a single
> > + // mailbox for a mailbox controller
> > + <&mbox_mw_tx 0>,
> > +
> > + // For single-word frames, client may instantiate as many
> > + // mailboxes as there are channel windows in the MHU
> > + <&mbox_sw_tx 0>,
> > + <&mbox_sw_tx 1>,
> > + <&mbox_sw_tx 2>,
> > + <&mbox_sw_tx 3>,
> > +
> > + // For doorbell frames, client may instantiate as many mailboxes
> > + // as there are bits available in the combined number of channel
> > + // windows ((channel windows * 32) mailboxes)
> > + <mbox_db_rx 0>,
> > + <mbox_db_rx 1>,
> > + ...
> > + <mbox_db_rx 17>;
> > + };
>
> If the mhuv2 instance implements, say, 3 channel windows between
> sender (linux) and receiver (firmware), and Linux runs two protocols
> each requiring 1 and 2-word sized messages respectively. The hardware
> supports that by assigning windows [0] and [1,2] to each protocol.
> However, I don't think the driver can support that. Or does it?
>
Thinking about it, IMO, the mbox-cell should carry a 128 (4x32) bit
mask specifying the set of windows (corresponding to the bits set in
the mask) associated with the channel.
And the controller driver should see any channel as associated with
variable number of windows 'N', where N is [0,124]

mhu_client1: proto1@2e000000 {
.....
mboxes = <&mbox 0x0 0x0 0x0 0x1>
}

mhu_client2: proto2@2f000000 {
.....
mboxes = <&mbox 0x0 0x0 0x0 0x6>
}

Cheers!

2019-07-28 21:28:33

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On 7/21/19 11:58 PM, Jassi Brar wrote:
> On Wed, Jul 17, 2019 at 2:26 PM Tushar Khandelwal
> <[email protected]> wrote:
>
>> diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
>> new file mode 100644
>> index 000000000000..3a05593414bc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
>> @@ -0,0 +1,108 @@
>> +Arm MHUv2 Mailbox Driver
>> +========================
>> +
>> +The Arm Message-Handling-Unit (MHU) Version 2 is a mailbox controller that has
>> +between 1 and 124 channel windows to provide unidirectional communication with
>> +remote processor(s).
>> +
>> +Given the unidirectional nature of the device, an MHUv2 mailbox may only be
>> +written to or read from. If a pair of MHU devices is implemented between two
>> +processing elements to provide bidirectional communication, these must be
>> +specified as two separate mailboxes.
>> +
>> +A device tree node for an Arm MHUv2 device must specify either a receiver frame
>> +or a sender frame, indicating which end of the unidirectional MHU device which
>> +the device node entry describes.
>> +
>> +An MHU device must be specified with a transport protocol. The transport
>> +protocol of an MHU device determines the method of data transmission as well as
>> +the number of provided mailboxes.
>> +Following are the possible transport protocol types:
>> +- Single-word: An MHU device implements as many mailboxes as it
>> + provides channel windows. Data is transmitted through
>> + the MHU registers.
>> +- Multi-word: An MHU device implements a single mailbox. All channel windows
>> + will be used during transmission. Data is transmitted through
>> + the MHU registers.
>> +- Doorbell: An MHU device implements as many mailboxes as there are flag
>> + bits available in its channel windows. Optionally, data may
>> + be transmitted through a shared memory region, wherein the MHU
>> + is used strictly as an interrupt generation mechanism.
>> +
>> +Mailbox Device Node:
>> +====================
>> +
>> +Required properties:
>> +--------------------
>> +- compatible: Shall be "arm,mhuv2" & "arm,primecell"
>> +- reg: Contains the mailbox register address range (base
>> + address and length)
>> +- #mbox-cells Shall be 1 - the index of the channel needed.
>> +- mhu-frame Frame type of the device.
>> + Shall be either "sender" or "receiver"
>> +- mhu-protocol Transport protocol of the device. Shall be one of the
>> + following: "single-word", "multi-word", "doorbell"
>> +
>> +Required properties (receiver frame):
>> +-------------------------------------
>> +- interrupts: Contains the interrupt information corresponding to the
>> + combined interrupt of the receiver frame
>> +
>> +Example:
>> +--------
>> +
>> + mbox_mw_tx: mhu@10000000 {
>> + compatible = "arm,mhuv2","arm,primecell";
>> + reg = <0x10000000 0x1000>;
>> + clocks = <&refclk100mhz>;
>> + clock-names = "apb_pclk";
>> + #mbox-cells = <1>;
>> + mhu-protocol = "multi-word";
>> + mhu-frame = "sender";
>> + };
>> +
>> + mbox_sw_tx: mhu@10000000 {
>> + compatible = "arm,mhuv2","arm,primecell";
>> + reg = <0x11000000 0x1000>;
>> + clocks = <&refclk100mhz>;
>> + clock-names = "apb_pclk";
>> + #mbox-cells = <1>;
>> + mhu-protocol = "single-word";
>> + mhu-frame = "sender";
>> + };
>> +
>> + mbox_db_rx: mhu@10000000 {
>> + compatible = "arm,mhuv2","arm,primecell";
>> + reg = <0x12000000 0x1000>;
>> + clocks = <&refclk100mhz>;
>> + clock-names = "apb_pclk";
>> + #mbox-cells = <1>;
>> + interrupts = <0 45 4>;
>> + interrupt-names = "mhu_rx";
>> + mhu-protocol = "doorbell";
>> + mhu-frame = "receiver";
>> + };
>> +
>> + mhu_client: scb@2e000000 {
>> + compatible = "fujitsu,mb86s70-scb-1.0";
>> + reg = <0 0x2e000000 0x4000>;
>> + mboxes =
>> + // For multi-word frames, client may only instantiate a single
>> + // mailbox for a mailbox controller
>> + <&mbox_mw_tx 0>,
>> +
>> + // For single-word frames, client may instantiate as many
>> + // mailboxes as there are channel windows in the MHU
>> + <&mbox_sw_tx 0>,
>> + <&mbox_sw_tx 1>,
>> + <&mbox_sw_tx 2>,
>> + <&mbox_sw_tx 3>,
>> +
>> + // For doorbell frames, client may instantiate as many mailboxes
>> + // as there are bits available in the combined number of channel
>> + // windows ((channel windows * 32) mailboxes)
>> + <mbox_db_rx 0>,
>> + <mbox_db_rx 1>,
>> + ...
>> + <mbox_db_rx 17>;
>> + };
>
> If the mhuv2 instance implements, say, 3 channel windows between
> sender (linux) and receiver (firmware), and Linux runs two protocols
> each requiring 1 and 2-word sized messages respectively. The hardware
> supports that by assigning windows [0] and [1,2] to each protocol.
> However, I don't think the driver can support that. Or does it?
>

Correct, this version of the driver does not support mixing-and-matching
protocols for an MHU device.
Given the current use-cases for the driver, we do not currently see a
need for this functionality. However, as you mention, the hardware does
not restrict this and it would be possible to add in a future version.

> Also I see problem with implementing "protocol modes" in the
> controller driver - 'mhu-protocol' property should go away. And
> 'mhu-frame' is unncessary - presence of interrupt property could imply
> 'receiver', otherwise 'sender'.
>
> Cheers!
>

I agree that the 'mhu-frame' property can be removed and the frame type
can be deduced from whether an interrupt property is present.
In regards to 'mhu-protocol', i still see value in having it as a device
tree property. As mentioned above, mixing protocols within an MHU is not
currently supported. We decided to specify the protocol for an MHU in
the device tree, given that the protocol influences how many mboxes it
is allowed for a mailbox client to register with a controller,

Thanks,
Morten

2019-07-28 21:29:36

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On 7/25/19 7:49 AM, Jassi Brar wrote:
> On Sun, Jul 21, 2019 at 4:58 PM Jassi Brar <[email protected]> wrote:
>>
>> On Wed, Jul 17, 2019 at 2:26 PM Tushar Khandelwal
>> <[email protected]> wrote:
>>
>>> diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
>>> new file mode 100644
>>> index 000000000000..3a05593414bc
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
>>> @@ -0,0 +1,108 @@
>>> +Arm MHUv2 Mailbox Driver
>>> +========================
>>> +
>>> +The Arm Message-Handling-Unit (MHU) Version 2 is a mailbox controller that has
>>> +between 1 and 124 channel windows to provide unidirectional communication with
>>> +remote processor(s).
>>> +
>>> +Given the unidirectional nature of the device, an MHUv2 mailbox may only be
>>> +written to or read from. If a pair of MHU devices is implemented between two
>>> +processing elements to provide bidirectional communication, these must be
>>> +specified as two separate mailboxes.
>>> +
>>> +A device tree node for an Arm MHUv2 device must specify either a receiver frame
>>> +or a sender frame, indicating which end of the unidirectional MHU device which
>>> +the device node entry describes.
>>> +
>>> +An MHU device must be specified with a transport protocol. The transport
>>> +protocol of an MHU device determines the method of data transmission as well as
>>> +the number of provided mailboxes.
>>> +Following are the possible transport protocol types:
>>> +- Single-word: An MHU device implements as many mailboxes as it
>>> + provides channel windows. Data is transmitted through
>>> + the MHU registers.
>>> +- Multi-word: An MHU device implements a single mailbox. All channel windows
>>> + will be used during transmission. Data is transmitted through
>>> + the MHU registers.
>>> +- Doorbell: An MHU device implements as many mailboxes as there are flag
>>> + bits available in its channel windows. Optionally, data may
>>> + be transmitted through a shared memory region, wherein the MHU
>>> + is used strictly as an interrupt generation mechanism.
>>> +
>>> +Mailbox Device Node:
>>> +====================
>>> +
>>> +Required properties:
>>> +--------------------
>>> +- compatible: Shall be "arm,mhuv2" & "arm,primecell"
>>> +- reg: Contains the mailbox register address range (base
>>> + address and length)
>>> +- #mbox-cells Shall be 1 - the index of the channel needed.
>>> +- mhu-frame Frame type of the device.
>>> + Shall be either "sender" or "receiver"
>>> +- mhu-protocol Transport protocol of the device. Shall be one of the
>>> + following: "single-word", "multi-word", "doorbell"
>>> +
>>> +Required properties (receiver frame):
>>> +-------------------------------------
>>> +- interrupts: Contains the interrupt information corresponding to the
>>> + combined interrupt of the receiver frame
>>> +
>>> +Example:
>>> +--------
>>> +
>>> + mbox_mw_tx: mhu@10000000 {
>>> + compatible = "arm,mhuv2","arm,primecell";
>>> + reg = <0x10000000 0x1000>;
>>> + clocks = <&refclk100mhz>;
>>> + clock-names = "apb_pclk";
>>> + #mbox-cells = <1>;
>>> + mhu-protocol = "multi-word";
>>> + mhu-frame = "sender";
>>> + };
>>> +
>>> + mbox_sw_tx: mhu@10000000 {
>>> + compatible = "arm,mhuv2","arm,primecell";
>>> + reg = <0x11000000 0x1000>;
>>> + clocks = <&refclk100mhz>;
>>> + clock-names = "apb_pclk";
>>> + #mbox-cells = <1>;
>>> + mhu-protocol = "single-word";
>>> + mhu-frame = "sender";
>>> + };
>>> +
>>> + mbox_db_rx: mhu@10000000 {
>>> + compatible = "arm,mhuv2","arm,primecell";
>>> + reg = <0x12000000 0x1000>;
>>> + clocks = <&refclk100mhz>;
>>> + clock-names = "apb_pclk";
>>> + #mbox-cells = <1>;
>>> + interrupts = <0 45 4>;
>>> + interrupt-names = "mhu_rx";
>>> + mhu-protocol = "doorbell";
>>> + mhu-frame = "receiver";
>>> + };
>>> +
>>> + mhu_client: scb@2e000000 {
>>> + compatible = "fujitsu,mb86s70-scb-1.0";
>>> + reg = <0 0x2e000000 0x4000>;
>>> + mboxes =
>>> + // For multi-word frames, client may only instantiate a single
>>> + // mailbox for a mailbox controller
>>> + <&mbox_mw_tx 0>,
>>> +
>>> + // For single-word frames, client may instantiate as many
>>> + // mailboxes as there are channel windows in the MHU
>>> + <&mbox_sw_tx 0>,
>>> + <&mbox_sw_tx 1>,
>>> + <&mbox_sw_tx 2>,
>>> + <&mbox_sw_tx 3>,
>>> +
>>> + // For doorbell frames, client may instantiate as many mailboxes
>>> + // as there are bits available in the combined number of channel
>>> + // windows ((channel windows * 32) mailboxes)
>>> + <mbox_db_rx 0>,
>>> + <mbox_db_rx 1>,
>>> + ...
>>> + <mbox_db_rx 17>;
>>> + };
>>
>> If the mhuv2 instance implements, say, 3 channel windows between
>> sender (linux) and receiver (firmware), and Linux runs two protocols
>> each requiring 1 and 2-word sized messages respectively. The hardware
>> supports that by assigning windows [0] and [1,2] to each protocol.
>> However, I don't think the driver can support that. Or does it?
>>
> Thinking about it, IMO, the mbox-cell should carry a 128 (4x32) bit
> mask specifying the set of windows (corresponding to the bits set in
> the mask) associated with the channel.
> And the controller driver should see any channel as associated with
> variable number of windows 'N', where N is [0,124]
>
> mhu_client1: proto1@2e000000 {
> .....
> mboxes = <&mbox 0x0 0x0 0x0 0x1>
> }
>
> mhu_client2: proto2@2f000000 {
> .....
> mboxes = <&mbox 0x0 0x0 0x0 0x6>
> }
>
> Cheers!
>

As mentioned in the response to your initial comment, the driver does
not currently support mixing protocols.
If mixing protocols is to be supported in the future, then this seems
like a suitable way of specifying which channels are associated with
which mailboxes (especially for mixing single- and multi-word modes).
However, there still is an issue in that both single-word and doorbell
requires only 1 channel window - and as such, the transport protocol
cannot be deduced from merely the number of masked channel windows.
Furthermore, for doorbell, a mbox may be registered for _each_ available
bit within a channel window (further complicating things if we were to
include mixing protocols in this initial driver version), making
assigning channel windows to mailboxes semantically different from when
assigning to single- or multi-word.

Thanks,
Morten

2019-07-31 08:29:13

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On Sun, Jul 28, 2019 at 4:28 PM Morten Borup Petersen <[email protected]> wrote:
>
>
>
> On 7/25/19 7:49 AM, Jassi Brar wrote:
> > On Sun, Jul 21, 2019 at 4:58 PM Jassi Brar <[email protected]> wrote:
> >>
> >> On Wed, Jul 17, 2019 at 2:26 PM Tushar Khandelwal
> >> <[email protected]> wrote:
> >>
> >>> diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
> >>> new file mode 100644
> >>> index 000000000000..3a05593414bc
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
> >>> @@ -0,0 +1,108 @@
> >>> +Arm MHUv2 Mailbox Driver
> >>> +========================
> >>> +
> >>> +The Arm Message-Handling-Unit (MHU) Version 2 is a mailbox controller that has
> >>> +between 1 and 124 channel windows to provide unidirectional communication with
> >>> +remote processor(s).
> >>> +
> >>> +Given the unidirectional nature of the device, an MHUv2 mailbox may only be
> >>> +written to or read from. If a pair of MHU devices is implemented between two
> >>> +processing elements to provide bidirectional communication, these must be
> >>> +specified as two separate mailboxes.
> >>> +
> >>> +A device tree node for an Arm MHUv2 device must specify either a receiver frame
> >>> +or a sender frame, indicating which end of the unidirectional MHU device which
> >>> +the device node entry describes.
> >>> +
> >>> +An MHU device must be specified with a transport protocol. The transport
> >>> +protocol of an MHU device determines the method of data transmission as well as
> >>> +the number of provided mailboxes.
> >>> +Following are the possible transport protocol types:
> >>> +- Single-word: An MHU device implements as many mailboxes as it
> >>> + provides channel windows. Data is transmitted through
> >>> + the MHU registers.
> >>> +- Multi-word: An MHU device implements a single mailbox. All channel windows
> >>> + will be used during transmission. Data is transmitted through
> >>> + the MHU registers.
> >>> +- Doorbell: An MHU device implements as many mailboxes as there are flag
> >>> + bits available in its channel windows. Optionally, data may
> >>> + be transmitted through a shared memory region, wherein the MHU
> >>> + is used strictly as an interrupt generation mechanism.
> >>> +
> >>> +Mailbox Device Node:
> >>> +====================
> >>> +
> >>> +Required properties:
> >>> +--------------------
> >>> +- compatible: Shall be "arm,mhuv2" & "arm,primecell"
> >>> +- reg: Contains the mailbox register address range (base
> >>> + address and length)
> >>> +- #mbox-cells Shall be 1 - the index of the channel needed.
> >>> +- mhu-frame Frame type of the device.
> >>> + Shall be either "sender" or "receiver"
> >>> +- mhu-protocol Transport protocol of the device. Shall be one of the
> >>> + following: "single-word", "multi-word", "doorbell"
> >>> +
> >>> +Required properties (receiver frame):
> >>> +-------------------------------------
> >>> +- interrupts: Contains the interrupt information corresponding to the
> >>> + combined interrupt of the receiver frame
> >>> +
> >>> +Example:
> >>> +--------
> >>> +
> >>> + mbox_mw_tx: mhu@10000000 {
> >>> + compatible = "arm,mhuv2","arm,primecell";
> >>> + reg = <0x10000000 0x1000>;
> >>> + clocks = <&refclk100mhz>;
> >>> + clock-names = "apb_pclk";
> >>> + #mbox-cells = <1>;
> >>> + mhu-protocol = "multi-word";
> >>> + mhu-frame = "sender";
> >>> + };
> >>> +
> >>> + mbox_sw_tx: mhu@10000000 {
> >>> + compatible = "arm,mhuv2","arm,primecell";
> >>> + reg = <0x11000000 0x1000>;
> >>> + clocks = <&refclk100mhz>;
> >>> + clock-names = "apb_pclk";
> >>> + #mbox-cells = <1>;
> >>> + mhu-protocol = "single-word";
> >>> + mhu-frame = "sender";
> >>> + };
> >>> +
> >>> + mbox_db_rx: mhu@10000000 {
> >>> + compatible = "arm,mhuv2","arm,primecell";
> >>> + reg = <0x12000000 0x1000>;
> >>> + clocks = <&refclk100mhz>;
> >>> + clock-names = "apb_pclk";
> >>> + #mbox-cells = <1>;
> >>> + interrupts = <0 45 4>;
> >>> + interrupt-names = "mhu_rx";
> >>> + mhu-protocol = "doorbell";
> >>> + mhu-frame = "receiver";
> >>> + };
> >>> +
> >>> + mhu_client: scb@2e000000 {
> >>> + compatible = "fujitsu,mb86s70-scb-1.0";
> >>> + reg = <0 0x2e000000 0x4000>;
> >>> + mboxes =
> >>> + // For multi-word frames, client may only instantiate a single
> >>> + // mailbox for a mailbox controller
> >>> + <&mbox_mw_tx 0>,
> >>> +
> >>> + // For single-word frames, client may instantiate as many
> >>> + // mailboxes as there are channel windows in the MHU
> >>> + <&mbox_sw_tx 0>,
> >>> + <&mbox_sw_tx 1>,
> >>> + <&mbox_sw_tx 2>,
> >>> + <&mbox_sw_tx 3>,
> >>> +
> >>> + // For doorbell frames, client may instantiate as many mailboxes
> >>> + // as there are bits available in the combined number of channel
> >>> + // windows ((channel windows * 32) mailboxes)
> >>> + <mbox_db_rx 0>,
> >>> + <mbox_db_rx 1>,
> >>> + ...
> >>> + <mbox_db_rx 17>;
> >>> + };
> >>
> >> If the mhuv2 instance implements, say, 3 channel windows between
> >> sender (linux) and receiver (firmware), and Linux runs two protocols
> >> each requiring 1 and 2-word sized messages respectively. The hardware
> >> supports that by assigning windows [0] and [1,2] to each protocol.
> >> However, I don't think the driver can support that. Or does it?
> >>
> > Thinking about it, IMO, the mbox-cell should carry a 128 (4x32) bit
> > mask specifying the set of windows (corresponding to the bits set in
> > the mask) associated with the channel.
> > And the controller driver should see any channel as associated with
> > variable number of windows 'N', where N is [0,124]
> >
> > mhu_client1: proto1@2e000000 {
> > .....
> > mboxes = <&mbox 0x0 0x0 0x0 0x1>
> > }
> >
> > mhu_client2: proto2@2f000000 {
> > .....
> > mboxes = <&mbox 0x0 0x0 0x0 0x6>
> > }
> >
> > Cheers!
> >
>
> As mentioned in the response to your initial comment, the driver does
> not currently support mixing protocols.
>
Thanks for acknowledging that limitation. But lets also address it.

> If mixing protocols is to be supported in the future, then this seems
> like a suitable way of specifying which channels are associated with
> which mailboxes (especially for mixing single- and multi-word modes).
>
We can not change DT bindings again when we feel like updating the driver.
The bindings should precisely and completely define the h/w, not what
mode we currently implement.
It is not for pure idealism, it actually makes the code simpler and futureproof.

> However, there still is an issue in that both single-word and doorbell
> requires only 1 channel window - and as such, the transport protocol
> cannot be deduced from merely the number of masked channel windows.
>
I don't see why the driver should worry -- the channel carries 32-bit
message or some random value just to trigger an interrupt is purely
upto the client driver.

> Furthermore, for doorbell, a mbox may be registered for _each_ available
> bit within a channel window (further complicating things if we were to
> include mixing protocols in this initial driver version), making
> assigning channel windows to mailboxes semantically different from when
> assigning to single- or multi-word.
>
Not sure about that, that would be implementing virtual channels. Each
window carries one signal, and that is the minimum bandwidth assigned
to a channel.

Thanks

2019-08-02 13:43:52

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On 7/31/19 9:31 AM, Jassi Brar wrote:
> On Sun, Jul 28, 2019 at 4:28 PM Morten Borup Petersen <[email protected]> wrote:
>>
>>
>>
>> On 7/25/19 7:49 AM, Jassi Brar wrote:
>>> On Sun, Jul 21, 2019 at 4:58 PM Jassi Brar <[email protected]> wrote:
>>>>
>>>> On Wed, Jul 17, 2019 at 2:26 PM Tushar Khandelwal
>>>> <[email protected]> wrote:
>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
>>>>> new file mode 100644
>>>>> index 000000000000..3a05593414bc
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
>>>>> @@ -0,0 +1,108 @@
>>>>> +Arm MHUv2 Mailbox Driver
>>>>> +========================
>>>>> +
>>>>> +The Arm Message-Handling-Unit (MHU) Version 2 is a mailbox controller that has
>>>>> +between 1 and 124 channel windows to provide unidirectional communication with
>>>>> +remote processor(s).
>>>>> +
>>>>> +Given the unidirectional nature of the device, an MHUv2 mailbox may only be
>>>>> +written to or read from. If a pair of MHU devices is implemented between two
>>>>> +processing elements to provide bidirectional communication, these must be
>>>>> +specified as two separate mailboxes.
>>>>> +
>>>>> +A device tree node for an Arm MHUv2 device must specify either a receiver frame
>>>>> +or a sender frame, indicating which end of the unidirectional MHU device which
>>>>> +the device node entry describes.
>>>>> +
>>>>> +An MHU device must be specified with a transport protocol. The transport
>>>>> +protocol of an MHU device determines the method of data transmission as well as
>>>>> +the number of provided mailboxes.
>>>>> +Following are the possible transport protocol types:
>>>>> +- Single-word: An MHU device implements as many mailboxes as it
>>>>> + provides channel windows. Data is transmitted through
>>>>> + the MHU registers.
>>>>> +- Multi-word: An MHU device implements a single mailbox. All channel windows
>>>>> + will be used during transmission. Data is transmitted through
>>>>> + the MHU registers.
>>>>> +- Doorbell: An MHU device implements as many mailboxes as there are flag
>>>>> + bits available in its channel windows. Optionally, data may
>>>>> + be transmitted through a shared memory region, wherein the MHU
>>>>> + is used strictly as an interrupt generation mechanism.
>>>>> +
>>>>> +Mailbox Device Node:
>>>>> +====================
>>>>> +
>>>>> +Required properties:
>>>>> +--------------------
>>>>> +- compatible: Shall be "arm,mhuv2" & "arm,primecell"
>>>>> +- reg: Contains the mailbox register address range (base
>>>>> + address and length)
>>>>> +- #mbox-cells Shall be 1 - the index of the channel needed.
>>>>> +- mhu-frame Frame type of the device.
>>>>> + Shall be either "sender" or "receiver"
>>>>> +- mhu-protocol Transport protocol of the device. Shall be one of the
>>>>> + following: "single-word", "multi-word", "doorbell"
>>>>> +
>>>>> +Required properties (receiver frame):
>>>>> +-------------------------------------
>>>>> +- interrupts: Contains the interrupt information corresponding to the
>>>>> + combined interrupt of the receiver frame
>>>>> +
>>>>> +Example:
>>>>> +--------
>>>>> +
>>>>> + mbox_mw_tx: mhu@10000000 {
>>>>> + compatible = "arm,mhuv2","arm,primecell";
>>>>> + reg = <0x10000000 0x1000>;
>>>>> + clocks = <&refclk100mhz>;
>>>>> + clock-names = "apb_pclk";
>>>>> + #mbox-cells = <1>;
>>>>> + mhu-protocol = "multi-word";
>>>>> + mhu-frame = "sender";
>>>>> + };
>>>>> +
>>>>> + mbox_sw_tx: mhu@10000000 {
>>>>> + compatible = "arm,mhuv2","arm,primecell";
>>>>> + reg = <0x11000000 0x1000>;
>>>>> + clocks = <&refclk100mhz>;
>>>>> + clock-names = "apb_pclk";
>>>>> + #mbox-cells = <1>;
>>>>> + mhu-protocol = "single-word";
>>>>> + mhu-frame = "sender";
>>>>> + };
>>>>> +
>>>>> + mbox_db_rx: mhu@10000000 {
>>>>> + compatible = "arm,mhuv2","arm,primecell";
>>>>> + reg = <0x12000000 0x1000>;
>>>>> + clocks = <&refclk100mhz>;
>>>>> + clock-names = "apb_pclk";
>>>>> + #mbox-cells = <1>;
>>>>> + interrupts = <0 45 4>;
>>>>> + interrupt-names = "mhu_rx";
>>>>> + mhu-protocol = "doorbell";
>>>>> + mhu-frame = "receiver";
>>>>> + };
>>>>> +
>>>>> + mhu_client: scb@2e000000 {
>>>>> + compatible = "fujitsu,mb86s70-scb-1.0";
>>>>> + reg = <0 0x2e000000 0x4000>;
>>>>> + mboxes =
>>>>> + // For multi-word frames, client may only instantiate a single
>>>>> + // mailbox for a mailbox controller
>>>>> + <&mbox_mw_tx 0>,
>>>>> +
>>>>> + // For single-word frames, client may instantiate as many
>>>>> + // mailboxes as there are channel windows in the MHU
>>>>> + <&mbox_sw_tx 0>,
>>>>> + <&mbox_sw_tx 1>,
>>>>> + <&mbox_sw_tx 2>,
>>>>> + <&mbox_sw_tx 3>,
>>>>> +
>>>>> + // For doorbell frames, client may instantiate as many mailboxes
>>>>> + // as there are bits available in the combined number of channel
>>>>> + // windows ((channel windows * 32) mailboxes)
>>>>> + <mbox_db_rx 0>,
>>>>> + <mbox_db_rx 1>,
>>>>> + ...
>>>>> + <mbox_db_rx 17>;
>>>>> + };
>>>>
>>>> If the mhuv2 instance implements, say, 3 channel windows between
>>>> sender (linux) and receiver (firmware), and Linux runs two protocols
>>>> each requiring 1 and 2-word sized messages respectively. The hardware
>>>> supports that by assigning windows [0] and [1,2] to each protocol.
>>>> However, I don't think the driver can support that. Or does it?
>>>>
>>> Thinking about it, IMO, the mbox-cell should carry a 128 (4x32) bit
>>> mask specifying the set of windows (corresponding to the bits set in
>>> the mask) associated with the channel.
>>> And the controller driver should see any channel as associated with
>>> variable number of windows 'N', where N is [0,124]
>>>
>>> mhu_client1: proto1@2e000000 {
>>> .....
>>> mboxes = <&mbox 0x0 0x0 0x0 0x1>
>>> }
>>>
>>> mhu_client2: proto2@2f000000 {
>>> .....
>>> mboxes = <&mbox 0x0 0x0 0x0 0x6>
>>> }
>>>
>>> Cheers!
>>>
>>
>> As mentioned in the response to your initial comment, the driver does
>> not currently support mixing protocols.
>>
> Thanks for acknowledging that limitation. But lets also address it.
>

We are hesitant to dedicate time to developing mixing protocols given
that we don't have any current usecase nor any current platform which
would support this.

>> If mixing protocols is to be supported in the future, then this seems
>> like a suitable way of specifying which channels are associated with
>> which mailboxes (especially for mixing single- and multi-word modes).
>>
> We can not change DT bindings again when we feel like updating the driver.
> The bindings should precisely and completely define the h/w, not what
> mode we currently implement.
> It is not for pure idealism, it actually makes the code simpler and futureproof.
>
>> However, there still is an issue in that both single-word and doorbell
>> requires only 1 channel window - and as such, the transport protocol
>> cannot be deduced from merely the number of masked channel windows.
>>
> I don't see why the driver should worry -- the channel carries 32-bit
> message or some random value just to trigger an interrupt is purely
> upto the client driver.
>

With the current design, it is not up to the client driver to
distinguish which bit was set within a channel window when an interrupt
was raised in doorbell mode. Currently, in doorbell mode, each bit
within a channel isspecified to be a distinct mailbox. Having this,
different mailbox clients may register mailboxes for different bits
within a single MHU device.
with the current design, when an interrupt is raised and an MHU is in
doorbell mode, the MHU driver will be responsible for deducing which
flag bit was set and from this set bit decide which mailbox to trigger a
callback on.
This is why we need to be able to specify the bit number when in
doorbell mode, in the device tree.

>> Furthermore, for doorbell, a mbox may be registered for _each_ available
>> bit within a channel window (further complicating things if we were to
>> include mixing protocols in this initial driver version), making
>> assigning channel windows to mailboxes semantically different from when
>> assigning to single- or multi-word.
>>
> Not sure about that, that would be implementing virtual channels. Each
> window carries one signal, and that is the minimum bandwidth assigned
> to a channel.
>
> Thanks
>
If implementing transport protocol mixing would be a requirement for the
acceptance of the driver, then we agree on that the format which you've
suggested would be a clean solution. However, given that we would like
to keep the ability to specify doorbell mailboxes in the device tree, we
suggest a format such as the following:

mhu_client1: proto2@2f000000 {
.....
/* Requesting to use channel window 0 of &mbox, registering
a mailbox in singe-word mode. */
mboxes = <&mbox 0x0 0x0 0x0 0x1>
}

mhu_client2: proto1@2e000000 {
.....
/* Requesting to use channel window 1 of &mbox, registering
mailboxes in doorbell mode, using bits 3 and 5. The MHU
driver is able to discern between putting channel window 1
into doorbell mode over single word mode, given the
presence of the extra argument. */
mboxes = <&mbox 0x0 0x0 0x0 0x2 3>,
<&mbox 0x0 0x0 0x0 0x2 5>
}

This would remove the ambiguity around deducing a mailbox to be in
single-word or doorbell mode.
Deciding to put channel window(s) into multi-word mode would be, as you
proposed, to mask more than one channel for a mailbox, ie:

mhu_client3: proto2@2f000000 {
.....
/* Requesting to use channel window 2-3 of &mbox, registering
a mailbox in multi-word mode. */
mboxes = <&mbox 0x0 0x0 0x0 0xC>
}

Thanks,

Morten

2019-08-02 21:19:34

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On Sun, Jul 21, 2019 at 04:58:04PM -0500, Jassi Brar wrote:
> On Wed, Jul 17, 2019 at 2:26 PM Tushar Khandelwal
> <[email protected]> wrote:
>
> > diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
> > new file mode 100644
> > index 000000000000..3a05593414bc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
> > @@ -0,0 +1,108 @@
> > +Arm MHUv2 Mailbox Driver
> > +========================
> > +
> > +The Arm Message-Handling-Unit (MHU) Version 2 is a mailbox controller that has
> > +between 1 and 124 channel windows to provide unidirectional communication with
> > +remote processor(s).
> > +
> > +Given the unidirectional nature of the device, an MHUv2 mailbox may only be
> > +written to or read from. If a pair of MHU devices is implemented between two
> > +processing elements to provide bidirectional communication, these must be
> > +specified as two separate mailboxes.
> > +
> > +A device tree node for an Arm MHUv2 device must specify either a receiver frame
> > +or a sender frame, indicating which end of the unidirectional MHU device which
> > +the device node entry describes.
> > +
> > +An MHU device must be specified with a transport protocol. The transport
> > +protocol of an MHU device determines the method of data transmission as well as
> > +the number of provided mailboxes.
> > +Following are the possible transport protocol types:
> > +- Single-word: An MHU device implements as many mailboxes as it
> > + provides channel windows. Data is transmitted through
> > + the MHU registers.
> > +- Multi-word: An MHU device implements a single mailbox. All channel windows
> > + will be used during transmission. Data is transmitted through
> > + the MHU registers.
> > +- Doorbell: An MHU device implements as many mailboxes as there are flag
> > + bits available in its channel windows. Optionally, data may
> > + be transmitted through a shared memory region, wherein the MHU
> > + is used strictly as an interrupt generation mechanism.
> > +
> > +Mailbox Device Node:
> > +====================
> > +
> > +Required properties:
> > +--------------------
> > +- compatible: Shall be "arm,mhuv2" & "arm,primecell"
> > +- reg: Contains the mailbox register address range (base
> > + address and length)
> > +- #mbox-cells Shall be 1 - the index of the channel needed.
> > +- mhu-frame Frame type of the device.
> > + Shall be either "sender" or "receiver"
> > +- mhu-protocol Transport protocol of the device. Shall be one of the
> > + following: "single-word", "multi-word", "doorbell"
> > +
> > +Required properties (receiver frame):
> > +-------------------------------------
> > +- interrupts: Contains the interrupt information corresponding to the
> > + combined interrupt of the receiver frame
> > +
> > +Example:
> > +--------
> > +
> > + mbox_mw_tx: mhu@10000000 {
> > + compatible = "arm,mhuv2","arm,primecell";
> > + reg = <0x10000000 0x1000>;
> > + clocks = <&refclk100mhz>;
> > + clock-names = "apb_pclk";
> > + #mbox-cells = <1>;
> > + mhu-protocol = "multi-word";
> > + mhu-frame = "sender";
> > + };
> > +
> > + mbox_sw_tx: mhu@10000000 {
> > + compatible = "arm,mhuv2","arm,primecell";
> > + reg = <0x11000000 0x1000>;
> > + clocks = <&refclk100mhz>;
> > + clock-names = "apb_pclk";
> > + #mbox-cells = <1>;
> > + mhu-protocol = "single-word";
> > + mhu-frame = "sender";
> > + };
> > +
> > + mbox_db_rx: mhu@10000000 {
> > + compatible = "arm,mhuv2","arm,primecell";
> > + reg = <0x12000000 0x1000>;
> > + clocks = <&refclk100mhz>;
> > + clock-names = "apb_pclk";
> > + #mbox-cells = <1>;
> > + interrupts = <0 45 4>;
> > + interrupt-names = "mhu_rx";
> > + mhu-protocol = "doorbell";
> > + mhu-frame = "receiver";
> > + };
> > +
> > + mhu_client: scb@2e000000 {
> > + compatible = "fujitsu,mb86s70-scb-1.0";
> > + reg = <0 0x2e000000 0x4000>;
> > + mboxes =
> > + // For multi-word frames, client may only instantiate a single
> > + // mailbox for a mailbox controller
> > + <&mbox_mw_tx 0>,
> > +
> > + // For single-word frames, client may instantiate as many
> > + // mailboxes as there are channel windows in the MHU
> > + <&mbox_sw_tx 0>,
> > + <&mbox_sw_tx 1>,
> > + <&mbox_sw_tx 2>,
> > + <&mbox_sw_tx 3>,
> > +
> > + // For doorbell frames, client may instantiate as many mailboxes
> > + // as there are bits available in the combined number of channel
> > + // windows ((channel windows * 32) mailboxes)
> > + <mbox_db_rx 0>,
> > + <mbox_db_rx 1>,
> > + ...
> > + <mbox_db_rx 17>;
> > + };
>
> If the mhuv2 instance implements, say, 3 channel windows between
> sender (linux) and receiver (firmware), and Linux runs two protocols
> each requiring 1 and 2-word sized messages respectively. The hardware
> supports that by assigning windows [0] and [1,2] to each protocol.
> However, I don't think the driver can support that. Or does it?
>

FWIW, the IP is designed to cover wide range of usecase from IoT to servers
with variable window length. I don't see the need to complicate the driver
supporting mix-n-match at the cost of latency. Each platform choose one
transport protocol for all it's use.

--
Regards,
Sudeep

2019-08-02 21:41:54

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On Thu, Jul 25, 2019 at 12:49:58AM -0500, Jassi Brar wrote:
> On Sun, Jul 21, 2019 at 4:58 PM Jassi Brar <[email protected]> wrote:
> >

[...]

> > If the mhuv2 instance implements, say, 3 channel windows between
> > sender (linux) and receiver (firmware), and Linux runs two protocols
> > each requiring 1 and 2-word sized messages respectively. The hardware
> > supports that by assigning windows [0] and [1,2] to each protocol.
> > However, I don't think the driver can support that. Or does it?
> >
> Thinking about it, IMO, the mbox-cell should carry a 128 (4x32) bit
> mask specifying the set of windows (corresponding to the bits set in
> the mask) associated with the channel.
> And the controller driver should see any channel as associated with
> variable number of windows 'N', where N is [0,124]
>
> mhu_client1: proto1@2e000000 {
> .....
> mboxes = <&mbox 0x0 0x0 0x0 0x1>
> }
>
> mhu_client2: proto2@2f000000 {
> .....
> mboxes = <&mbox 0x0 0x0 0x0 0x6>
> }
>

This still doesn't address the overhead I mentioned in my arm_mhu_v1
series.

As per you suggestion, we will have one channel with all possible
bit mask value to specify the window. Let's imagine that 2 protocols
share the same channel, then the requests are serialised.
E.g. if bits 0 and 1 are allocated for say protocol#1 and bits 2 and 3
for protocol#2.

Further protocol#1 has higher latency requirements like sched-governor
DVFS and there are 3-4 pending requests on protocol#2, then the incoming
requests for protocol#1 is blocked.

This is definitely overhead and I have seen lots of issue around this
and hence I was requesting that we need to create individual channels
for each of these. Having abstraction on top to multiplex or arbitrate
won't help.

--
Regards,
Sudeep

2019-08-07 11:13:08

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On 02/08/2019, 11:54, "Sudeep Holla" <[email protected]> wrote:

On Thu, Jul 25, 2019 at 12:49:58AM -0500, Jassi Brar wrote:
> On Sun, Jul 21, 2019 at 4:58 PM Jassi Brar <[email protected]> wrote:
> >

[...]

> > If the mhuv2 instance implements, say, 3 channel windows between
> > sender (linux) and receiver (firmware), and Linux runs two protocols
> > each requiring 1 and 2-word sized messages respectively. The hardware
> > supports that by assigning windows [0] and [1,2] to each protocol.
> > However, I don't think the driver can support that. Or does it?
> >
> Thinking about it, IMO, the mbox-cell should carry a 128 (4x32) bit
> mask specifying the set of windows (corresponding to the bits set in
> the mask) associated with the channel.
> And the controller driver should see any channel as associated with
> variable number of windows 'N', where N is [0,124]
>
> mhu_client1: proto1@2e000000 {
> .....
> mboxes = <&mbox 0x0 0x0 0x0 0x1>
> }
>
> mhu_client2: proto2@2f000000 {
> .....
> mboxes = <&mbox 0x0 0x0 0x0 0x6>
> }
>

This still doesn't address the overhead I mentioned in my arm_mhu_v1
series.

As per you suggestion, we will have one channel with all possible
bit mask value to specify the window. Let's imagine that 2 protocols
share the same channel, then the requests are serialised.
E.g. if bits 0 and 1 are allocated for say protocol#1 and bits 2 and 3
for protocol#2.

At a given time only one protocol can be used by a client. No mix-match
of protocols are handled by the driver currently. Also its not possible to address all
possible scenarios offered by the IP. That's why the current driver design is
based on the implementation in the existing platforms.

Further protocol#1 has higher latency requirements like sched-governor
DVFS and there are 3-4 pending requests on protocol#2, then the incoming
requests for protocol#1 is blocked.

This is definitely overhead and I have seen lots of issue around this
and hence I was requesting that we need to create individual channels
for each of these. Having abstraction on top to multiplex or arbitrate
won't help.

Also the (mbox-cells) approach will not allow us to differentiate between
single-word and doorbell which is required to make the controller driver
aware of the data expected whether it's a pointer to a location or in
register itself.

--Tushar
--
Regards,
Sudeep

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

2019-08-07 11:19:16

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On 02/08/2019, 11:59, "Sudeep Holla" <[email protected]> wrote:

On Sun, Jul 21, 2019 at 04:58:04PM -0500, Jassi Brar wrote:
> On Wed, Jul 17, 2019 at 2:26 PM Tushar Khandelwal
> <[email protected]> wrote:
>
> > diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
> > new file mode 100644
> > index 000000000000..3a05593414bc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
> > @@ -0,0 +1,108 @@
> > +Arm MHUv2 Mailbox Driver
> > +========================
> > +
> > +The Arm Message-Handling-Unit (MHU) Version 2 is a mailbox controller that has
> > +between 1 and 124 channel windows to provide unidirectional communication with
> > +remote processor(s).
> > +
> > +Given the unidirectional nature of the device, an MHUv2 mailbox may only be
> > +written to or read from. If a pair of MHU devices is implemented between two
> > +processing elements to provide bidirectional communication, these must be
> > +specified as two separate mailboxes.
> > +
> > +A device tree node for an Arm MHUv2 device must specify either a receiver frame
> > +or a sender frame, indicating which end of the unidirectional MHU device which
> > +the device node entry describes.
> > +
> > +An MHU device must be specified with a transport protocol. The transport
> > +protocol of an MHU device determines the method of data transmission as well as
> > +the number of provided mailboxes.
> > +Following are the possible transport protocol types:
> > +- Single-word: An MHU device implements as many mailboxes as it
> > + provides channel windows. Data is transmitted through
> > + the MHU registers.
> > +- Multi-word: An MHU device implements a single mailbox. All channel windows
> > + will be used during transmission. Data is transmitted through
> > + the MHU registers.
> > +- Doorbell: An MHU device implements as many mailboxes as there are flag
> > + bits available in its channel windows. Optionally, data may
> > + be transmitted through a shared memory region, wherein the MHU
> > + is used strictly as an interrupt generation mechanism.
> > +
> > +Mailbox Device Node:
> > +====================
> > +
> > +Required properties:
> > +--------------------
> > +- compatible: Shall be "arm,mhuv2" & "arm,primecell"
> > +- reg: Contains the mailbox register address range (base
> > + address and length)
> > +- #mbox-cells Shall be 1 - the index of the channel needed.
> > +- mhu-frame Frame type of the device.
> > + Shall be either "sender" or "receiver"
> > +- mhu-protocol Transport protocol of the device. Shall be one of the
> > + following: "single-word", "multi-word", "doorbell"
> > +
> > +Required properties (receiver frame):
> > +-------------------------------------
> > +- interrupts: Contains the interrupt information corresponding to the
> > + combined interrupt of the receiver frame
> > +
> > +Example:
> > +--------
> > +
> > + mbox_mw_tx: mhu@10000000 {
> > + compatible = "arm,mhuv2","arm,primecell";
> > + reg = <0x10000000 0x1000>;
> > + clocks = <&refclk100mhz>;
> > + clock-names = "apb_pclk";
> > + #mbox-cells = <1>;
> > + mhu-protocol = "multi-word";
> > + mhu-frame = "sender";
> > + };
> > +
> > + mbox_sw_tx: mhu@10000000 {
> > + compatible = "arm,mhuv2","arm,primecell";
> > + reg = <0x11000000 0x1000>;
> > + clocks = <&refclk100mhz>;
> > + clock-names = "apb_pclk";
> > + #mbox-cells = <1>;
> > + mhu-protocol = "single-word";
> > + mhu-frame = "sender";
> > + };
> > +
> > + mbox_db_rx: mhu@10000000 {
> > + compatible = "arm,mhuv2","arm,primecell";
> > + reg = <0x12000000 0x1000>;
> > + clocks = <&refclk100mhz>;
> > + clock-names = "apb_pclk";
> > + #mbox-cells = <1>;
> > + interrupts = <0 45 4>;
> > + interrupt-names = "mhu_rx";
> > + mhu-protocol = "doorbell";
> > + mhu-frame = "receiver";
> > + };
> > +
> > + mhu_client: scb@2e000000 {
> > + compatible = "fujitsu,mb86s70-scb-1.0";
> > + reg = <0 0x2e000000 0x4000>;
> > + mboxes =
> > + // For multi-word frames, client may only instantiate a single
> > + // mailbox for a mailbox controller
> > + <&mbox_mw_tx 0>,
> > +
> > + // For single-word frames, client may instantiate as many
> > + // mailboxes as there are channel windows in the MHU
> > + <&mbox_sw_tx 0>,
> > + <&mbox_sw_tx 1>,
> > + <&mbox_sw_tx 2>,
> > + <&mbox_sw_tx 3>,
> > +
> > + // For doorbell frames, client may instantiate as many mailboxes
> > + // as there are bits available in the combined number of channel
> > + // windows ((channel windows * 32) mailboxes)
> > + <mbox_db_rx 0>,
> > + <mbox_db_rx 1>,
> > + ...
> > + <mbox_db_rx 17>;
> > + };
>
> If the mhuv2 instance implements, say, 3 channel windows between
> sender (linux) and receiver (firmware), and Linux runs two protocols
> each requiring 1 and 2-word sized messages respectively. The hardware
> supports that by assigning windows [0] and [1,2] to each protocol.
> However, I don't think the driver can support that. Or does it?
>

FWIW, the IP is designed to cover wide range of usecase from IoT to servers
with variable window length. I don't see the need to complicate the driver
supporting mix-n-match at the cost of latency. Each platform choose one
transport protocol for all it's use.

The driver design is to address the most probable scenarios and not all.
Single-word : Client gets one 32-bit window
Doorbell : Client gets 32 data pointers (arm_message)
Multi-word: Client gets all channels available in the platform.

--Tushar
--
Regards,
Sudeep

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

2019-08-08 10:34:03

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On 8/7/19 1:17 PM, Tushar Khandelwal wrote:
>
>
> On 02/08/2019, 11:59, "Sudeep Holla" <[email protected]> wrote:
>
> On Sun, Jul 21, 2019 at 04:58:04PM -0500, Jassi Brar wrote:
> > On Wed, Jul 17, 2019 at 2:26 PM Tushar Khandelwal
> > <[email protected]> wrote:
> >
> > > diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
> > > new file mode 100644
> > > index 000000000000..3a05593414bc
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
> > > @@ -0,0 +1,108 @@
> > > +Arm MHUv2 Mailbox Driver
> > > +========================
> > > +
> > > +The Arm Message-Handling-Unit (MHU) Version 2 is a mailbox controller that has
> > > +between 1 and 124 channel windows to provide unidirectional communication with
> > > +remote processor(s).
> > > +
> > > +Given the unidirectional nature of the device, an MHUv2 mailbox may only be
> > > +written to or read from. If a pair of MHU devices is implemented between two
> > > +processing elements to provide bidirectional communication, these must be
> > > +specified as two separate mailboxes.
> > > +
> > > +A device tree node for an Arm MHUv2 device must specify either a receiver frame
> > > +or a sender frame, indicating which end of the unidirectional MHU device which
> > > +the device node entry describes.
> > > +
> > > +An MHU device must be specified with a transport protocol. The transport
> > > +protocol of an MHU device determines the method of data transmission as well as
> > > +the number of provided mailboxes.
> > > +Following are the possible transport protocol types:
> > > +- Single-word: An MHU device implements as many mailboxes as it
> > > + provides channel windows. Data is transmitted through
> > > + the MHU registers.
> > > +- Multi-word: An MHU device implements a single mailbox. All channel windows
> > > + will be used during transmission. Data is transmitted through
> > > + the MHU registers.
> > > +- Doorbell: An MHU device implements as many mailboxes as there are flag
> > > + bits available in its channel windows. Optionally, data may
> > > + be transmitted through a shared memory region, wherein the MHU
> > > + is used strictly as an interrupt generation mechanism.
> > > +
> > > +Mailbox Device Node:
> > > +====================
> > > +
> > > +Required properties:
> > > +--------------------
> > > +- compatible: Shall be "arm,mhuv2" & "arm,primecell"
> > > +- reg: Contains the mailbox register address range (base
> > > + address and length)
> > > +- #mbox-cells Shall be 1 - the index of the channel needed.
> > > +- mhu-frame Frame type of the device.
> > > + Shall be either "sender" or "receiver"
> > > +- mhu-protocol Transport protocol of the device. Shall be one of the
> > > + following: "single-word", "multi-word", "doorbell"
> > > +
> > > +Required properties (receiver frame):
> > > +-------------------------------------
> > > +- interrupts: Contains the interrupt information corresponding to the
> > > + combined interrupt of the receiver frame
> > > +
> > > +Example:
> > > +--------
> > > +
> > > + mbox_mw_tx: mhu@10000000 {
> > > + compatible = "arm,mhuv2","arm,primecell";
> > > + reg = <0x10000000 0x1000>;
> > > + clocks = <&refclk100mhz>;
> > > + clock-names = "apb_pclk";
> > > + #mbox-cells = <1>;
> > > + mhu-protocol = "multi-word";
> > > + mhu-frame = "sender";
> > > + };
> > > +
> > > + mbox_sw_tx: mhu@10000000 {
> > > + compatible = "arm,mhuv2","arm,primecell";
> > > + reg = <0x11000000 0x1000>;
> > > + clocks = <&refclk100mhz>;
> > > + clock-names = "apb_pclk";
> > > + #mbox-cells = <1>;
> > > + mhu-protocol = "single-word";
> > > + mhu-frame = "sender";
> > > + };
> > > +
> > > + mbox_db_rx: mhu@10000000 {
> > > + compatible = "arm,mhuv2","arm,primecell";
> > > + reg = <0x12000000 0x1000>;
> > > + clocks = <&refclk100mhz>;
> > > + clock-names = "apb_pclk";
> > > + #mbox-cells = <1>;
> > > + interrupts = <0 45 4>;
> > > + interrupt-names = "mhu_rx";
> > > + mhu-protocol = "doorbell";
> > > + mhu-frame = "receiver";
> > > + };
> > > +
> > > + mhu_client: scb@2e000000 {
> > > + compatible = "fujitsu,mb86s70-scb-1.0";
> > > + reg = <0 0x2e000000 0x4000>;
> > > + mboxes =
> > > + // For multi-word frames, client may only instantiate a single
> > > + // mailbox for a mailbox controller
> > > + <&mbox_mw_tx 0>,
> > > +
> > > + // For single-word frames, client may instantiate as many
> > > + // mailboxes as there are channel windows in the MHU
> > > + <&mbox_sw_tx 0>,
> > > + <&mbox_sw_tx 1>,
> > > + <&mbox_sw_tx 2>,
> > > + <&mbox_sw_tx 3>,
> > > +
> > > + // For doorbell frames, client may instantiate as many mailboxes
> > > + // as there are bits available in the combined number of channel
> > > + // windows ((channel windows * 32) mailboxes)
> > > + <mbox_db_rx 0>,
> > > + <mbox_db_rx 1>,
> > > + ...
> > > + <mbox_db_rx 17>;
> > > + };
> >
> > If the mhuv2 instance implements, say, 3 channel windows between
> > sender (linux) and receiver (firmware), and Linux runs two protocols
> > each requiring 1 and 2-word sized messages respectively. The hardware
> > supports that by assigning windows [0] and [1,2] to each protocol.
> > However, I don't think the driver can support that. Or does it?
> >
>
> FWIW, the IP is designed to cover wide range of usecase from IoT to servers
> with variable window length. I don't see the need to complicate the driver
> supporting mix-n-match at the cost of latency. Each platform choose one
> transport protocol for all it's use.
>
> The driver design is to address the most probable scenarios and not all.
> Single-word : Client gets one 32-bit window
> Doorbell : Client gets 32 data pointers (arm_message)
> Multi-word: Client gets all channels available in the platform.
>
> --Tushar
>

To elaborate for better understanding;
arm_message is used when a mailbox client is to transmit data through an
MHUv2 mailbox (ie. in-band transmission), wherein the MHUv2 is in a
single-word or multi-word protocol mode.
arm_message allows for a mailbox client to transmit data with arbitrary
length, without having to know the technicalities of underlying
transport protocol, number of MHU channels etc. As such, the mailbox
client driver code may remain unchanged, if the underlying transport
protocol is changed from ie. single-word to multi-word, or if more
channel-windows are added to an MHUv2 device in multi-word mode.
Transmission of data as well as maximum utilization of the available
channel windows (in multi-word mode) is solely job for the MHUv2 driver.

Given the differences between using an MHUv2 mailbox in single- and
multi-word mode compared to doorbell mode, it is however expected that
mailbox client driver code must be changed if switching from single- or
multi-word to doorbell.
As per the MHUv2 spec, when in doorbell mode, it is expected that the
receiver and transmitter has decided beforehand both where data is
placed in shared memory, as well as the format of this data. Here we do
not require that arm_message is used, given that all data is transmitted
out-of-band, and the MHUv2 is only used as an interrupt generating
mechanism.

To give an example of how arm_message may be used;
Say, a client-driver protocol has been written as follows:

> struct {
> int cmd;
> int[4] args;
> } foo_t;

Now, a client driver may transmit this through a mailbox which has an
MHUv2 as the controller - we do not need to know the underlying
transport protocol, just that it has to be either single- or multi word.

> // Create user-specific protocol struct
> foo_t foo;
>
> /* Modify foo with the data to be transmitted... */
>
> /* Wrap message into arm_message format, so the MHUv2 driver knows how
> much data to send*/
>
> arm_message msg;
> msg.data = &foo;
> msg.len = sizeof(foo);
>
> /* Transmit the arm_message */
> mbox_send_message(mbox, &msg);
>

On the receiver side, there should be some accompanying logic in the
client driver which is aware of foo_t and from this may reconstruct the
received message. Reconstruction of a received message is a task for the
mailbox client driver.

- Morten

2019-08-13 16:50:50

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On Fri, Aug 2, 2019 at 5:41 AM Morten Borup Petersen <[email protected]> wrote:
>
>
>
> On 7/31/19 9:31 AM, Jassi Brar wrote:
> > On Sun, Jul 28, 2019 at 4:28 PM Morten Borup Petersen <[email protected]> wrote:
> >>
> >>
> >>
> >> On 7/25/19 7:49 AM, Jassi Brar wrote:
> >>> On Sun, Jul 21, 2019 at 4:58 PM Jassi Brar <[email protected]> wrote:
> >>>>
> >>>> On Wed, Jul 17, 2019 at 2:26 PM Tushar Khandelwal
> >>>> <[email protected]> wrote:
> >>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
> >>>>> new file mode 100644
> >>>>> index 000000000000..3a05593414bc
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
> >>>>> @@ -0,0 +1,108 @@
> >>>>> +Arm MHUv2 Mailbox Driver
> >>>>> +========================
> >>>>> +
> >>>>> +The Arm Message-Handling-Unit (MHU) Version 2 is a mailbox controller that has
> >>>>> +between 1 and 124 channel windows to provide unidirectional communication with
> >>>>> +remote processor(s).
> >>>>> +
> >>>>> +Given the unidirectional nature of the device, an MHUv2 mailbox may only be
> >>>>> +written to or read from. If a pair of MHU devices is implemented between two
> >>>>> +processing elements to provide bidirectional communication, these must be
> >>>>> +specified as two separate mailboxes.
> >>>>> +
> >>>>> +A device tree node for an Arm MHUv2 device must specify either a receiver frame
> >>>>> +or a sender frame, indicating which end of the unidirectional MHU device which
> >>>>> +the device node entry describes.
> >>>>> +
> >>>>> +An MHU device must be specified with a transport protocol. The transport
> >>>>> +protocol of an MHU device determines the method of data transmission as well as
> >>>>> +the number of provided mailboxes.
> >>>>> +Following are the possible transport protocol types:
> >>>>> +- Single-word: An MHU device implements as many mailboxes as it
> >>>>> + provides channel windows. Data is transmitted through
> >>>>> + the MHU registers.
> >>>>> +- Multi-word: An MHU device implements a single mailbox. All channel windows
> >>>>> + will be used during transmission. Data is transmitted through
> >>>>> + the MHU registers.
> >>>>> +- Doorbell: An MHU device implements as many mailboxes as there are flag
> >>>>> + bits available in its channel windows. Optionally, data may
> >>>>> + be transmitted through a shared memory region, wherein the MHU
> >>>>> + is used strictly as an interrupt generation mechanism.
> >>>>> +
> >>>>> +Mailbox Device Node:
> >>>>> +====================
> >>>>> +
> >>>>> +Required properties:
> >>>>> +--------------------
> >>>>> +- compatible: Shall be "arm,mhuv2" & "arm,primecell"
> >>>>> +- reg: Contains the mailbox register address range (base
> >>>>> + address and length)
> >>>>> +- #mbox-cells Shall be 1 - the index of the channel needed.
> >>>>> +- mhu-frame Frame type of the device.
> >>>>> + Shall be either "sender" or "receiver"
> >>>>> +- mhu-protocol Transport protocol of the device. Shall be one of the
> >>>>> + following: "single-word", "multi-word", "doorbell"
> >>>>> +
> >>>>> +Required properties (receiver frame):
> >>>>> +-------------------------------------
> >>>>> +- interrupts: Contains the interrupt information corresponding to the
> >>>>> + combined interrupt of the receiver frame
> >>>>> +
> >>>>> +Example:
> >>>>> +--------
> >>>>> +
> >>>>> + mbox_mw_tx: mhu@10000000 {
> >>>>> + compatible = "arm,mhuv2","arm,primecell";
> >>>>> + reg = <0x10000000 0x1000>;
> >>>>> + clocks = <&refclk100mhz>;
> >>>>> + clock-names = "apb_pclk";
> >>>>> + #mbox-cells = <1>;
> >>>>> + mhu-protocol = "multi-word";
> >>>>> + mhu-frame = "sender";
> >>>>> + };
> >>>>> +
> >>>>> + mbox_sw_tx: mhu@10000000 {
> >>>>> + compatible = "arm,mhuv2","arm,primecell";
> >>>>> + reg = <0x11000000 0x1000>;
> >>>>> + clocks = <&refclk100mhz>;
> >>>>> + clock-names = "apb_pclk";
> >>>>> + #mbox-cells = <1>;
> >>>>> + mhu-protocol = "single-word";
> >>>>> + mhu-frame = "sender";
> >>>>> + };
> >>>>> +
> >>>>> + mbox_db_rx: mhu@10000000 {
> >>>>> + compatible = "arm,mhuv2","arm,primecell";
> >>>>> + reg = <0x12000000 0x1000>;
> >>>>> + clocks = <&refclk100mhz>;
> >>>>> + clock-names = "apb_pclk";
> >>>>> + #mbox-cells = <1>;
> >>>>> + interrupts = <0 45 4>;
> >>>>> + interrupt-names = "mhu_rx";
> >>>>> + mhu-protocol = "doorbell";
> >>>>> + mhu-frame = "receiver";
> >>>>> + };
> >>>>> +
> >>>>> + mhu_client: scb@2e000000 {
> >>>>> + compatible = "fujitsu,mb86s70-scb-1.0";
> >>>>> + reg = <0 0x2e000000 0x4000>;
> >>>>> + mboxes =
> >>>>> + // For multi-word frames, client may only instantiate a single
> >>>>> + // mailbox for a mailbox controller
> >>>>> + <&mbox_mw_tx 0>,
> >>>>> +
> >>>>> + // For single-word frames, client may instantiate as many
> >>>>> + // mailboxes as there are channel windows in the MHU
> >>>>> + <&mbox_sw_tx 0>,
> >>>>> + <&mbox_sw_tx 1>,
> >>>>> + <&mbox_sw_tx 2>,
> >>>>> + <&mbox_sw_tx 3>,
> >>>>> +
> >>>>> + // For doorbell frames, client may instantiate as many mailboxes
> >>>>> + // as there are bits available in the combined number of channel
> >>>>> + // windows ((channel windows * 32) mailboxes)
> >>>>> + <mbox_db_rx 0>,
> >>>>> + <mbox_db_rx 1>,
> >>>>> + ...
> >>>>> + <mbox_db_rx 17>;
> >>>>> + };
> >>>>
> >>>> If the mhuv2 instance implements, say, 3 channel windows between
> >>>> sender (linux) and receiver (firmware), and Linux runs two protocols
> >>>> each requiring 1 and 2-word sized messages respectively. The hardware
> >>>> supports that by assigning windows [0] and [1,2] to each protocol.
> >>>> However, I don't think the driver can support that. Or does it?
> >>>>
> >>> Thinking about it, IMO, the mbox-cell should carry a 128 (4x32) bit
> >>> mask specifying the set of windows (corresponding to the bits set in
> >>> the mask) associated with the channel.
> >>> And the controller driver should see any channel as associated with
> >>> variable number of windows 'N', where N is [0,124]
> >>>
> >>> mhu_client1: proto1@2e000000 {
> >>> .....
> >>> mboxes = <&mbox 0x0 0x0 0x0 0x1>
> >>> }
> >>>
> >>> mhu_client2: proto2@2f000000 {
> >>> .....
> >>> mboxes = <&mbox 0x0 0x0 0x0 0x6>
> >>> }
> >>>
> >>> Cheers!
> >>>
> >>
> >> As mentioned in the response to your initial comment, the driver does
> >> not currently support mixing protocols.
> >>
> > Thanks for acknowledging that limitation. But lets also address it.
> >
>
> We are hesitant to dedicate time to developing mixing protocols given
> that we don't have any current usecase nor any current platform which
> would support this.
>
Can you please share the client code against which you tested this driver?
From my past experience, I realise it is much more efficient to tidyup
the code myself, than endlessly trying to explain the benefits.

Thanks

2019-08-14 10:06:39

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On Tue, Aug 13, 2019 at 11:36:56AM -0500, Jassi Brar wrote:
[...]

> > >>
> > >> As mentioned in the response to your initial comment, the driver does
> > >> not currently support mixing protocols.
> > >>
> > > Thanks for acknowledging that limitation. But lets also address it.
> > >
> >
> > We are hesitant to dedicate time to developing mixing protocols given
> > that we don't have any current usecase nor any current platform which
> > would support this.
> >
> Can you please share the client code against which you tested this driver?
> From my past experience, I realise it is much more efficient to tidyup
> the code myself, than endlessly trying to explain the benefits.
>

Thanks for the patience and offer. Can we try the same with MHUv1 and SCMI
upstream driver.

The firmware just uses High Priority physical channel bit 0 and 2 as Tx
and bit 1 and 3 as Rx. Bit 2 and 3 are for perf which shouldn't get blocked
by bit 0 and 1. I mean I can have 10 requests covering clocks/sensors and
others on bit 0 and 1, but the bits 2 and 3 are dedicated for DVFS and
shouldn't be blocked because of other non DVFS requests.

The DT looks something like this(modified MHU binding for 2 cells)

mailbox: mhu@2b1f0000 {
compatible = "arm,primecell";
reg = <0x0 0x2b1f0000 0x0 0x1000>;
interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
interrupt-names = "mhu_lpri_rx",
"mhu_hpri_rx";
#mbox-cells = <2>;
mbox-name = "ARM-MHU";
clocks = <&soc_refclk100mhz>;
clock-names = "apb_pclk";
};
firmware {
scmi {
compatible = "arm,scmi";
mbox-names = "tx", "rx";
mboxes = <&mailbox 0 0 &mailbox 0 1>;
#address-cells = <1>;
#size-cells = <0>;

scmi_devpd: protocol@11 {
reg = <0x11>;
#power-domain-cells = <1>;
};

scmi_dvfs: protocol@13 {
reg = <0x13>;
#clock-cells = <1>;
mbox-names = "tx", "rx";
mboxes = <&mailbox 0 2 &mailbox 0 3>;
};

scmi_clk: protocol@14 {
reg = <0x14>;
#clock-cells = <1>;
};

scmi_sensors0: protocol@15 {
reg = <0x15>;
#thermal-sensor-cells = <1>;
};
};
};

--
Regards,
Sudeep

2019-08-14 14:55:01

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On Wed, Aug 14, 2019 at 5:05 AM Sudeep Holla <[email protected]> wrote:
>
> On Tue, Aug 13, 2019 at 11:36:56AM -0500, Jassi Brar wrote:
> [...]
>
> > > >>
> > > >> As mentioned in the response to your initial comment, the driver does
> > > >> not currently support mixing protocols.
> > > >>
> > > > Thanks for acknowledging that limitation. But lets also address it.
> > > >
> > >
> > > We are hesitant to dedicate time to developing mixing protocols given
> > > that we don't have any current usecase nor any current platform which
> > > would support this.
> > >
> > Can you please share the client code against which you tested this driver?
> > From my past experience, I realise it is much more efficient to tidyup
> > the code myself, than endlessly trying to explain the benefits.
> >
>
> Thanks for the patience and offer.
>
Ok, but the offer is to Morten for MHUv2 driver.

> Can we try the same with MHUv1 and SCMI
> upstream driver.
>
MHUv1 driver is fine as it is.
I did try my best to keep you from messing the SCMI driver, without success
https://lkml.org/lkml/2017/8/7/924

2019-08-14 16:52:38

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On Wed, Aug 14, 2019 at 09:52:25AM -0500, Jassi Brar wrote:
> On Wed, Aug 14, 2019 at 5:05 AM Sudeep Holla <[email protected]> wrote:
> >
> > On Tue, Aug 13, 2019 at 11:36:56AM -0500, Jassi Brar wrote:
> > [...]
> >
> > > > >>
> > > > >> As mentioned in the response to your initial comment, the driver does
> > > > >> not currently support mixing protocols.
> > > > >>
> > > > > Thanks for acknowledging that limitation. But lets also address it.
> > > > >
> > > >
> > > > We are hesitant to dedicate time to developing mixing protocols given
> > > > that we don't have any current usecase nor any current platform which
> > > > would support this.
> > > >
> > > Can you please share the client code against which you tested this driver?
> > > From my past experience, I realise it is much more efficient to tidyup
> > > the code myself, than endlessly trying to explain the benefits.
> > >
> >
> > Thanks for the patience and offer.
> >
> Ok, but the offer is to Morten for MHUv2 driver.
>
> > Can we try the same with MHUv1 and SCMI
> > upstream driver.
> >
> MHUv1 driver is fine as it is.
> I did try my best to keep you from messing the SCMI driver, without success
> https://lkml.org/lkml/2017/8/7/924

I disagree, you haven't told me how to address the usecase which I mentioned
with the abstraction/multiplexer on top of MHU as you have been suggesting.

I am sure MHUv2 will have the same usecase.

--
Regards,
Sudeep

2019-08-14 22:36:42

[permalink] [raw]

Subject: Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation

On 13/08/2019 17:36, Jassi Brar wrote:
> On Fri, Aug 2, 2019 at 5:41 AM Morten Borup Petersen <[email protected]> wrote:
>>
>>
>>
>> On 7/31/19 9:31 AM, Jassi Brar wrote:
>>> On Sun, Jul 28, 2019 at 4:28 PM Morten Borup Petersen <[email protected]> wrote:
>>>>
>>>>
>>>>
>>>> On 7/25/19 7:49 AM, Jassi Brar wrote:
>>>>> On Sun, Jul 21, 2019 at 4:58 PM Jassi Brar <[email protected]> wrote:
>>>>>>
>>>>>> On Wed, Jul 17, 2019 at 2:26 PM Tushar Khandelwal
>>>>>> <[email protected]> wrote:
>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..3a05593414bc
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
>>>>>>> @@ -0,0 +1,108 @@
>>>>>>> +Arm MHUv2 Mailbox Driver
>>>>>>> +========================
>>>>>>> +
>>>>>>> +The Arm Message-Handling-Unit (MHU) Version 2 is a mailbox controller that has
>>>>>>> +between 1 and 124 channel windows to provide unidirectional communication with
>>>>>>> +remote processor(s).
>>>>>>> +
>>>>>>> +Given the unidirectional nature of the device, an MHUv2 mailbox may only be
>>>>>>> +written to or read from. If a pair of MHU devices is implemented between two
>>>>>>> +processing elements to provide bidirectional communication, these must be
>>>>>>> +specified as two separate mailboxes.
>>>>>>> +
>>>>>>> +A device tree node for an Arm MHUv2 device must specify either a receiver frame
>>>>>>> +or a sender frame, indicating which end of the unidirectional MHU device which
>>>>>>> +the device node entry describes.
>>>>>>> +
>>>>>>> +An MHU device must be specified with a transport protocol. The transport
>>>>>>> +protocol of an MHU device determines the method of data transmission as well as
>>>>>>> +the number of provided mailboxes.
>>>>>>> +Following are the possible transport protocol types:
>>>>>>> +- Single-word: An MHU device implements as many mailboxes as it
>>>>>>> + provides channel windows. Data is transmitted through
>>>>>>> + the MHU registers.
>>>>>>> +- Multi-word: An MHU device implements a single mailbox. All channel windows
>>>>>>> + will be used during transmission. Data is transmitted through
>>>>>>> + the MHU registers.
>>>>>>> +- Doorbell: An MHU device implements as many mailboxes as there are flag
>>>>>>> + bits available in its channel windows. Optionally, data may
>>>>>>> + be transmitted through a shared memory region, wherein the MHU
>>>>>>> + is used strictly as an interrupt generation mechanism.
>>>>>>> +
>>>>>>> +Mailbox Device Node:
>>>>>>> +====================
>>>>>>> +
>>>>>>> +Required properties:
>>>>>>> +--------------------
>>>>>>> +- compatible: Shall be "arm,mhuv2" & "arm,primecell"
>>>>>>> +- reg: Contains the mailbox register address range (base
>>>>>>> + address and length)
>>>>>>> +- #mbox-cells Shall be 1 - the index of the channel needed.
>>>>>>> +- mhu-frame Frame type of the device.
>>>>>>> + Shall be either "sender" or "receiver"
>>>>>>> +- mhu-protocol Transport protocol of the device. Shall be one of the
>>>>>>> + following: "single-word", "multi-word", "doorbell"
>>>>>>> +
>>>>>>> +Required properties (receiver frame):
>>>>>>> +-------------------------------------
>>>>>>> +- interrupts: Contains the interrupt information corresponding to the
>>>>>>> + combined interrupt of the receiver frame
>>>>>>> +
>>>>>>> +Example:
>>>>>>> +--------
>>>>>>> +
>>>>>>> + mbox_mw_tx: mhu@10000000 {
>>>>>>> + compatible = "arm,mhuv2","arm,primecell";
>>>>>>> + reg = <0x10000000 0x1000>;
>>>>>>> + clocks = <&refclk100mhz>;
>>>>>>> + clock-names = "apb_pclk";
>>>>>>> + #mbox-cells = <1>;
>>>>>>> + mhu-protocol = "multi-word";
>>>>>>> + mhu-frame = "sender";
>>>>>>> + };
>>>>>>> +
>>>>>>> + mbox_sw_tx: mhu@10000000 {
>>>>>>> + compatible = "arm,mhuv2","arm,primecell";
>>>>>>> + reg = <0x11000000 0x1000>;
>>>>>>> + clocks = <&refclk100mhz>;
>>>>>>> + clock-names = "apb_pclk";
>>>>>>> + #mbox-cells = <1>;
>>>>>>> + mhu-protocol = "single-word";
>>>>>>> + mhu-frame = "sender";
>>>>>>> + };
>>>>>>> +
>>>>>>> + mbox_db_rx: mhu@10000000 {
>>>>>>> + compatible = "arm,mhuv2","arm,primecell";
>>>>>>> + reg = <0x12000000 0x1000>;
>>>>>>> + clocks = <&refclk100mhz>;
>>>>>>> + clock-names = "apb_pclk";
>>>>>>> + #mbox-cells = <1>;
>>>>>>> + interrupts = <0 45 4>;
>>>>>>> + interrupt-names = "mhu_rx";
>>>>>>> + mhu-protocol = "doorbell";
>>>>>>> + mhu-frame = "receiver";
>>>>>>> + };
>>>>>>> +
>>>>>>> + mhu_client: scb@2e000000 {
>>>>>>> + compatible = "fujitsu,mb86s70-scb-1.0";
>>>>>>> + reg = <0 0x2e000000 0x4000>;
>>>>>>> + mboxes =
>>>>>>> + // For multi-word frames, client may only instantiate a single
>>>>>>> + // mailbox for a mailbox controller
>>>>>>> + <&mbox_mw_tx 0>,
>>>>>>> +
>>>>>>> + // For single-word frames, client may instantiate as many
>>>>>>> + // mailboxes as there are channel windows in the MHU
>>>>>>> + <&mbox_sw_tx 0>,
>>>>>>> + <&mbox_sw_tx 1>,
>>>>>>> + <&mbox_sw_tx 2>,
>>>>>>> + <&mbox_sw_tx 3>,
>>>>>>> +
>>>>>>> + // For doorbell frames, client may instantiate as many mailboxes
>>>>>>> + // as there are bits available in the combined number of channel
>>>>>>> + // windows ((channel windows * 32) mailboxes)
>>>>>>> + <mbox_db_rx 0>,
>>>>>>> + <mbox_db_rx 1>,
>>>>>>> + ...
>>>>>>> + <mbox_db_rx 17>;
>>>>>>> + };
>>>>>>
>>>>>> If the mhuv2 instance implements, say, 3 channel windows between
>>>>>> sender (linux) and receiver (firmware), and Linux runs two protocols
>>>>>> each requiring 1 and 2-word sized messages respectively. The hardware
>>>>>> supports that by assigning windows [0] and [1,2] to each protocol.
>>>>>> However, I don't think the driver can support that. Or does it?
>>>>>>
>>>>> Thinking about it, IMO, the mbox-cell should carry a 128 (4x32) bit
>>>>> mask specifying the set of windows (corresponding to the bits set in
>>>>> the mask) associated with the channel.
>>>>> And the controller driver should see any channel as associated with
>>>>> variable number of windows 'N', where N is [0,124]
>>>>>
>>>>> mhu_client1: proto1@2e000000 {
>>>>> .....
>>>>> mboxes = <&mbox 0x0 0x0 0x0 0x1>
>>>>> }
>>>>>
>>>>> mhu_client2: proto2@2f000000 {
>>>>> .....
>>>>> mboxes = <&mbox 0x0 0x0 0x0 0x6>
>>>>> }
>>>>>
>>>>> Cheers!
>>>>>
>>>>
>>>> As mentioned in the response to your initial comment, the driver does
>>>> not currently support mixing protocols.
>>>>
>>> Thanks for acknowledging that limitation. But lets also address it.
>>>
>>
>> We are hesitant to dedicate time to developing mixing protocols given
>> that we don't have any current usecase nor any current platform which
>> would support this.
>>
> Can you please share the client code against which you tested this driver?
> From my past experience, I realise it is much more efficient to tidyup
> the code myself, than endlessly trying to explain the benefits.
>

Yes, I will share that soon.

> Thanks
>

2019-08-14 22:38:49