* [PATCHv7 0/2] Adding driver for Xilinx LogiCORE IP mailbox.
@ 2015-07-15 1:00 ` Moritz Fischer
0 siblings, 0 replies; 21+ messages in thread
From: Moritz Fischer @ 2015-07-15 1:00 UTC (permalink / raw)
To: jassisinghbrar
Cc: linux-kernel, robh+dt, pawel.moll, mark.rutland, ijc+devicetree,
galak, michal.simek, soren.brinkmann, akpm, gregkh, mchehab, arnd,
joe, jingoohan1, devicetree, linux-arm-kernel, Moritz Fischer
Hi all,
Thanks for hanging in there, hopefully the last round of review ...
This patchset adds mailbox framework integration for the Xilinx LogiCORE IP
mailbox. The Xilinx LogiCORE IP mailbox is a fpga softcore that allows
interprocessor communication between AXI4 stream / memory mapped
processors.
Cheers,
Moritz
Changes from v6:
- As suggested by Jassi and Sören use #mbox-cells = <1>
Changes from v5:
- Fixed constness for ops struct
- Removed redundant checks
- Moved ops assignment
- Removed MODULE_ALIAS
Changes from v4:
- Have separate mbox_ops structs for polling / irq mode
- Moved clk handling to startup / shutdown
- Embedded struct mbox_chan in struct xilinx_mbox
- Fixed mbox-cells in devicetree documentation
- Misc stylistic issues
Changes from v3:
- Stylistic changes
- Changed reg size in dts to 0x100
Changes from v2:
- Fixed error condition for IRQ from >= 0 to > 0
- Fixed clock enable
- Cleaned up docs
Changes from v1:
- Added common clock framework support
- Deal with IRQs that happend before driver load,
since HW will not let us know about them when we enable IRQs
Changes from v0:
- Several stylistic issues
- Dropped superfluous intr_mode member
- Really masking the IRQs on mailbox_shutdown
- No longer using polling by accident in non-IRQ mode
- Swapped doc and driver commits
Moritz Fischer (2):
dts: Adding docs for Xilinx LogiCORE IP mailbox driver.
mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
.../devicetree/bindings/mailbox/xilinx-mailbox.txt | 44 +++
MAINTAINERS | 7 +
drivers/mailbox/Kconfig | 7 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/mailbox-xilinx.c | 367 +++++++++++++++++++++
5 files changed, 427 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
create mode 100644 drivers/mailbox/mailbox-xilinx.c
--
2.4.3
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCHv7 0/2] Adding driver for Xilinx LogiCORE IP mailbox.
@ 2015-07-15 1:00 ` Moritz Fischer
0 siblings, 0 replies; 21+ messages in thread
From: Moritz Fischer @ 2015-07-15 1:00 UTC (permalink / raw)
To: jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, michal.simek-gjFFaj9aHVfQT0dZR+AlfA,
soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
mchehab-JPH+aEBZ4P+UEJcrhfAQsw, arnd-r2nGTMty4D4,
joe-6d6DIl74uiNBDgjK7y7TUQ, jingoohan1-Re5JQEeQqe8AvxtiuMwx3w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Moritz Fischer
Hi all,
Thanks for hanging in there, hopefully the last round of review ...
This patchset adds mailbox framework integration for the Xilinx LogiCORE IP
mailbox. The Xilinx LogiCORE IP mailbox is a fpga softcore that allows
interprocessor communication between AXI4 stream / memory mapped
processors.
Cheers,
Moritz
Changes from v6:
- As suggested by Jassi and Sören use #mbox-cells = <1>
Changes from v5:
- Fixed constness for ops struct
- Removed redundant checks
- Moved ops assignment
- Removed MODULE_ALIAS
Changes from v4:
- Have separate mbox_ops structs for polling / irq mode
- Moved clk handling to startup / shutdown
- Embedded struct mbox_chan in struct xilinx_mbox
- Fixed mbox-cells in devicetree documentation
- Misc stylistic issues
Changes from v3:
- Stylistic changes
- Changed reg size in dts to 0x100
Changes from v2:
- Fixed error condition for IRQ from >= 0 to > 0
- Fixed clock enable
- Cleaned up docs
Changes from v1:
- Added common clock framework support
- Deal with IRQs that happend before driver load,
since HW will not let us know about them when we enable IRQs
Changes from v0:
- Several stylistic issues
- Dropped superfluous intr_mode member
- Really masking the IRQs on mailbox_shutdown
- No longer using polling by accident in non-IRQ mode
- Swapped doc and driver commits
Moritz Fischer (2):
dts: Adding docs for Xilinx LogiCORE IP mailbox driver.
mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
.../devicetree/bindings/mailbox/xilinx-mailbox.txt | 44 +++
MAINTAINERS | 7 +
drivers/mailbox/Kconfig | 7 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/mailbox-xilinx.c | 367 +++++++++++++++++++++
5 files changed, 427 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
create mode 100644 drivers/mailbox/mailbox-xilinx.c
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCHv7 0/2] Adding driver for Xilinx LogiCORE IP mailbox.
@ 2015-07-15 1:00 ` Moritz Fischer
0 siblings, 0 replies; 21+ messages in thread
From: Moritz Fischer @ 2015-07-15 1:00 UTC (permalink / raw)
To: linux-arm-kernel
Hi all,
Thanks for hanging in there, hopefully the last round of review ...
This patchset adds mailbox framework integration for the Xilinx LogiCORE IP
mailbox. The Xilinx LogiCORE IP mailbox is a fpga softcore that allows
interprocessor communication between AXI4 stream / memory mapped
processors.
Cheers,
Moritz
Changes from v6:
- As suggested by Jassi and S?ren use #mbox-cells = <1>
Changes from v5:
- Fixed constness for ops struct
- Removed redundant checks
- Moved ops assignment
- Removed MODULE_ALIAS
Changes from v4:
- Have separate mbox_ops structs for polling / irq mode
- Moved clk handling to startup / shutdown
- Embedded struct mbox_chan in struct xilinx_mbox
- Fixed mbox-cells in devicetree documentation
- Misc stylistic issues
Changes from v3:
- Stylistic changes
- Changed reg size in dts to 0x100
Changes from v2:
- Fixed error condition for IRQ from >= 0 to > 0
- Fixed clock enable
- Cleaned up docs
Changes from v1:
- Added common clock framework support
- Deal with IRQs that happend before driver load,
since HW will not let us know about them when we enable IRQs
Changes from v0:
- Several stylistic issues
- Dropped superfluous intr_mode member
- Really masking the IRQs on mailbox_shutdown
- No longer using polling by accident in non-IRQ mode
- Swapped doc and driver commits
Moritz Fischer (2):
dts: Adding docs for Xilinx LogiCORE IP mailbox driver.
mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
.../devicetree/bindings/mailbox/xilinx-mailbox.txt | 44 +++
MAINTAINERS | 7 +
drivers/mailbox/Kconfig | 7 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/mailbox-xilinx.c | 367 +++++++++++++++++++++
5 files changed, 427 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
create mode 100644 drivers/mailbox/mailbox-xilinx.c
--
2.4.3
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCHv7 1/2] dts: Adding docs for Xilinx LogiCORE IP mailbox driver.
2015-07-15 1:00 ` Moritz Fischer
@ 2015-07-15 1:00 ` Moritz Fischer
-1 siblings, 0 replies; 21+ messages in thread
From: Moritz Fischer @ 2015-07-15 1:00 UTC (permalink / raw)
To: jassisinghbrar
Cc: linux-kernel, robh+dt, pawel.moll, mark.rutland, ijc+devicetree,
galak, michal.simek, soren.brinkmann, akpm, gregkh, mchehab, arnd,
joe, jingoohan1, devicetree, linux-arm-kernel, Moritz Fischer
Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
Acked-by: Michal Simek <michal.simek@xilinx.com>
---
.../devicetree/bindings/mailbox/xilinx-mailbox.txt | 44 ++++++++++++++++++++++
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
diff --git a/Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt b/Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
new file mode 100644
index 0000000..f9ec46d
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
@@ -0,0 +1,44 @@
+Xilinx Mailbox Driver
+=====================
+
+Required properties:
+- compatible : "xlnx,mailbox-2.1".
+- reg : physical base address of the mailbox and length of
+ memory mapped region.
+- #mbox-cells : common mailbox binding property to identify the number
+ of cells required for the mailbox specifier, should be 1
+- clocks : phandle to clock provider
+- clock-names : must be 'mbox'
+
+Optional properties:
+- interrupt-parent : interrupt source phandle
+- interrupts : interrupt number, The interrupt specifier format
+ depends on the interrupt controller parent.
+
+Example:
+ mbox: mailbox@40400000 {
+ compatible = "xlnx,mailbox-2.1";
+ reg = <0x40400000 0x100>;
+ interrupt-parent = <&intc>;
+ interrupts = <5>;
+ #mbox-cells = <1>;
+ clocks = <&clkc 15>;
+ clock-names = "mbox";
+ };
+
+Mailbox client
+===============
+"mboxes" and the optional "mbox-names" (please see
+Documentation/devicetree/bindings/mailbox/mailbox.txt for details). Each value
+of the mboxes property should contain a phandle to the mailbox controller
+device node and second argument is the channel index. It must be 0 (hardware
+support only one channel). The equivalent "mbox-names" property value can be
+used to give a name to the communication channel to be used by the client user.
+
+Example:
+ mclient0: mclient0@400 {
+ compatible = "client-1.0";
+ reg = <0x400 0x10>;
+ mbox-names = "mbox";
+ mboxes = <&mbox 0>;
+ };
--
2.4.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCHv7 1/2] dts: Adding docs for Xilinx LogiCORE IP mailbox driver.
@ 2015-07-15 1:00 ` Moritz Fischer
0 siblings, 0 replies; 21+ messages in thread
From: Moritz Fischer @ 2015-07-15 1:00 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
Acked-by: Michal Simek <michal.simek@xilinx.com>
---
.../devicetree/bindings/mailbox/xilinx-mailbox.txt | 44 ++++++++++++++++++++++
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
diff --git a/Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt b/Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
new file mode 100644
index 0000000..f9ec46d
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
@@ -0,0 +1,44 @@
+Xilinx Mailbox Driver
+=====================
+
+Required properties:
+- compatible : "xlnx,mailbox-2.1".
+- reg : physical base address of the mailbox and length of
+ memory mapped region.
+- #mbox-cells : common mailbox binding property to identify the number
+ of cells required for the mailbox specifier, should be 1
+- clocks : phandle to clock provider
+- clock-names : must be 'mbox'
+
+Optional properties:
+- interrupt-parent : interrupt source phandle
+- interrupts : interrupt number, The interrupt specifier format
+ depends on the interrupt controller parent.
+
+Example:
+ mbox: mailbox at 40400000 {
+ compatible = "xlnx,mailbox-2.1";
+ reg = <0x40400000 0x100>;
+ interrupt-parent = <&intc>;
+ interrupts = <5>;
+ #mbox-cells = <1>;
+ clocks = <&clkc 15>;
+ clock-names = "mbox";
+ };
+
+Mailbox client
+===============
+"mboxes" and the optional "mbox-names" (please see
+Documentation/devicetree/bindings/mailbox/mailbox.txt for details). Each value
+of the mboxes property should contain a phandle to the mailbox controller
+device node and second argument is the channel index. It must be 0 (hardware
+support only one channel). The equivalent "mbox-names" property value can be
+used to give a name to the communication channel to be used by the client user.
+
+Example:
+ mclient0: mclient0 at 400 {
+ compatible = "client-1.0";
+ reg = <0x400 0x10>;
+ mbox-names = "mbox";
+ mboxes = <&mbox 0>;
+ };
--
2.4.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCHv7 2/2] mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
2015-07-15 1:00 ` Moritz Fischer
@ 2015-07-15 1:00 ` Moritz Fischer
-1 siblings, 0 replies; 21+ messages in thread
From: Moritz Fischer @ 2015-07-15 1:00 UTC (permalink / raw)
To: jassisinghbrar
Cc: linux-kernel, robh+dt, pawel.moll, mark.rutland, ijc+devicetree,
galak, michal.simek, soren.brinkmann, akpm, gregkh, mchehab, arnd,
joe, jingoohan1, devicetree, linux-arm-kernel, Moritz Fischer
The Xilinx LogiCORE IP mailbox is a FPGA core that allows for
interprocessor communication via AXI4 memory mapped / AXI4 stream
interfaces.
It is single channel per core and allows for transmit and receive.
Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
Acked-by: Michal Simek <michal.simek@xilinx.com>
---
MAINTAINERS | 7 +
drivers/mailbox/Kconfig | 7 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/mailbox-xilinx.c | 367 +++++++++++++++++++++++++++++++++++++++
4 files changed, 383 insertions(+)
create mode 100644 drivers/mailbox/mailbox-xilinx.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 2d3d55c..bfe2b78 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11320,6 +11320,13 @@ M: John Linn <John.Linn@xilinx.com>
S: Maintained
F: drivers/net/ethernet/xilinx/xilinx_axienet*
+XILINX MAILBOX DRIVER
+M: Moritz Fischer <moritz.fischer@ettus.com>
+L: linux-kernel@vger.kernel.org
+S: Maintained
+F: drivers/mailbox/mailbox-xilinx.c
+F: Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
+
XILINX UARTLITE SERIAL DRIVER
M: Peter Korsgaard <jacmet@sunsite.dk>
L: linux-serial@vger.kernel.org
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index e269f08..15b3ade 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -70,4 +70,11 @@ config BCM2835_MBOX
the services of the Videocore. Say Y here if you want to use the
BCM2835 Mailbox.
+config XILINX_MBOX
+ tristate "Xilinx Mailbox"
+ help
+ An implementation of the Xilinx Mailbox soft core. It is used
+ to send message between processors. Say Y here if you want to use the
+ Xilinx mailbox support.
+
endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 8e6d822..1966e26 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -13,3 +13,5 @@ obj-$(CONFIG_PCC) += pcc.o
obj-$(CONFIG_ALTERA_MBOX) += mailbox-altera.o
obj-$(CONFIG_BCM2835_MBOX) += bcm2835-mailbox.o
+
+obj-$(CONFIG_XILINX_MBOX) += mailbox-xilinx.o
diff --git a/drivers/mailbox/mailbox-xilinx.c b/drivers/mailbox/mailbox-xilinx.c
new file mode 100644
index 0000000..27011e6
--- /dev/null
+++ b/drivers/mailbox/mailbox-xilinx.c
@@ -0,0 +1,367 @@
+/*
+ * Copyright (c) 2015, National Instruments Corp. All rights reserved.
+ *
+ * Driver for the Xilinx LogiCORE mailbox IP block
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+/* register offsets */
+#define MAILBOX_REG_WRDATA 0x00
+#define MAILBOX_REG_RDDATA 0x08
+#define MAILBOX_REG_STATUS 0x10
+#define MAILBOX_REG_ERROR 0x14
+#define MAILBOX_REG_SIT 0x18
+#define MAILBOX_REG_RIT 0x1c
+#define MAILBOX_REG_IS 0x20
+#define MAILBOX_REG_IE 0x24
+#define MAILBOX_REG_IP 0x28
+
+/* status register */
+#define STS_RTA BIT(3)
+#define STS_STA BIT(2)
+#define STS_FULL BIT(1)
+#define STS_EMPTY BIT(0)
+
+/* error register */
+#define ERR_FULL BIT(1)
+#define ERR_EMPTY BIT(0)
+
+/* mailbox interrupt status register */
+#define INT_STATUS_ERR BIT(2)
+#define INT_STATUS_RTI BIT(1)
+#define INT_STATUS_STI BIT(0)
+
+/* mailbox interrupt enable register */
+#define INT_ENABLE_ERR BIT(2)
+#define INT_ENABLE_RTI BIT(1)
+#define INT_ENABLE_STI BIT(0)
+
+#define MBOX_POLLING_MS 5 /* polling interval 5ms */
+
+struct xilinx_mbox {
+ int irq;
+ void __iomem *mbox_base;
+ struct clk *clk;
+ struct device *dev;
+ struct mbox_controller controller;
+ struct mbox_chan chan;
+
+ /* if the controller supports only RX polling mode */
+ struct timer_list rxpoll_timer;
+};
+
+static struct xilinx_mbox *mbox_chan_to_xilinx_mbox(struct mbox_chan *chan)
+{
+ return container_of(chan, struct xilinx_mbox, chan);
+}
+
+static inline bool xilinx_mbox_full(struct xilinx_mbox *mbox)
+{
+ u32 status;
+
+ status = readl_relaxed(mbox->mbox_base + MAILBOX_REG_STATUS);
+
+ return status & STS_FULL;
+}
+
+static inline bool xilinx_mbox_pending(struct xilinx_mbox *mbox)
+{
+ u32 status;
+
+ status = readl_relaxed(mbox->mbox_base + MAILBOX_REG_STATUS);
+
+ return !(status & STS_EMPTY);
+}
+
+static void xilinx_mbox_intmask(struct xilinx_mbox *mbox, u32 mask, bool enable)
+{
+ u32 mask_reg;
+
+ mask_reg = readl_relaxed(mbox->mbox_base + MAILBOX_REG_IE);
+ if (enable)
+ mask_reg |= mask;
+ else
+ mask_reg &= ~mask;
+
+ writel_relaxed(mask_reg, mbox->mbox_base + MAILBOX_REG_IE);
+}
+
+static inline void xilinx_mbox_rx_intmask(struct xilinx_mbox *mbox, bool enable)
+{
+ xilinx_mbox_intmask(mbox, INT_ENABLE_RTI, enable);
+}
+
+static inline void xilinx_mbox_tx_intmask(struct xilinx_mbox *mbox, bool enable)
+{
+ xilinx_mbox_intmask(mbox, INT_ENABLE_STI, enable);
+}
+
+static void xilinx_mbox_rx_data(struct mbox_chan *chan)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+ u32 data;
+
+ if (xilinx_mbox_pending(mbox)) {
+ data = readl_relaxed(mbox->mbox_base + MAILBOX_REG_RDDATA);
+ mbox_chan_received_data(chan, (void *)data);
+ }
+}
+
+static void xilinx_mbox_poll_rx(unsigned long data)
+{
+ struct mbox_chan *chan = (struct mbox_chan *)data;
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ xilinx_mbox_rx_data(chan);
+
+ mod_timer(&mbox->rxpoll_timer,
+ jiffies + msecs_to_jiffies(MBOX_POLLING_MS));
+}
+
+static irqreturn_t xilinx_mbox_interrupt(int irq, void *p)
+{
+ u32 mask;
+ struct mbox_chan *chan = (struct mbox_chan *)p;
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ mask = readl_relaxed(mbox->mbox_base + MAILBOX_REG_IS);
+ if (mask & INT_STATUS_RTI)
+ xilinx_mbox_rx_data(chan);
+
+ /* mask irqs *before* notifying done, require tx_block=true */
+ if (mask & INT_STATUS_STI) {
+ xilinx_mbox_tx_intmask(mbox, false);
+ mbox_chan_txdone(chan, 0);
+ }
+
+ writel_relaxed(mask, mbox->mbox_base + MAILBOX_REG_IS);
+
+ return IRQ_HANDLED;
+}
+
+static int xilinx_mbox_irq_startup(struct mbox_chan *chan)
+{
+ int ret;
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ ret = request_irq(mbox->irq, xilinx_mbox_interrupt, 0,
+ dev_name(mbox->dev), chan);
+ if (unlikely(ret)) {
+ dev_err(mbox->dev,
+ "failed to register mailbox interrupt:%d\n",
+ ret);
+ return ret;
+ }
+
+ /* prep and enable the clock */
+ clk_prepare_enable(mbox->clk);
+
+ xilinx_mbox_rx_intmask(mbox, true);
+
+ /* if fifo was full already, we didn't get an interrupt */
+ while (xilinx_mbox_pending(mbox))
+ xilinx_mbox_rx_data(chan);
+
+ return 0;
+}
+
+static int xilinx_mbox_poll_startup(struct mbox_chan *chan)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ clk_prepare_enable(mbox->clk);
+
+ mod_timer(&mbox->rxpoll_timer,
+ jiffies + msecs_to_jiffies(MBOX_POLLING_MS));
+
+ return 0;
+}
+
+static int xilinx_mbox_irq_send_data(struct mbox_chan *chan, void *data)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+ u32 *udata = data;
+
+ if (xilinx_mbox_full(mbox))
+ return -EBUSY;
+
+ /* enable interrupt before send */
+ xilinx_mbox_tx_intmask(mbox, true);
+
+ writel_relaxed(*udata, mbox->mbox_base + MAILBOX_REG_WRDATA);
+
+ return 0;
+}
+
+static int xilinx_mbox_poll_send_data(struct mbox_chan *chan, void *data)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+ u32 *udata = data;
+
+ if (!mbox || !data)
+ return -EINVAL;
+
+ if (xilinx_mbox_full(mbox))
+ return -EBUSY;
+
+ writel_relaxed(*udata, mbox->mbox_base + MAILBOX_REG_WRDATA);
+
+ return 0;
+}
+
+static bool xilinx_mbox_last_tx_done(struct mbox_chan *chan)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ /* return false if mailbox is full */
+ return !xilinx_mbox_full(mbox);
+}
+
+static bool xilinx_mbox_peek_data(struct mbox_chan *chan)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ return xilinx_mbox_pending(mbox);
+}
+
+static void xilinx_mbox_irq_shutdown(struct mbox_chan *chan)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ /* mask all interrupts */
+ writel_relaxed(0, mbox->mbox_base + MAILBOX_REG_IE);
+
+ clk_disable_unprepare(mbox->clk);
+
+ free_irq(mbox->irq, chan);
+}
+
+static void xilinx_mbox_poll_shutdown(struct mbox_chan *chan)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ del_timer_sync(&mbox->rxpoll_timer);
+
+ clk_disable_unprepare(mbox->clk);
+}
+
+static const struct mbox_chan_ops xilinx_mbox_irq_ops = {
+ .send_data = xilinx_mbox_irq_send_data,
+ .startup = xilinx_mbox_irq_startup,
+ .shutdown = xilinx_mbox_irq_shutdown,
+ .last_tx_done = xilinx_mbox_last_tx_done,
+ .peek_data = xilinx_mbox_peek_data,
+};
+
+static const struct mbox_chan_ops xilinx_mbox_poll_ops = {
+ .send_data = xilinx_mbox_poll_send_data,
+ .startup = xilinx_mbox_poll_startup,
+ .shutdown = xilinx_mbox_poll_shutdown,
+ .last_tx_done = xilinx_mbox_last_tx_done,
+ .peek_data = xilinx_mbox_peek_data,
+};
+
+static int xilinx_mbox_probe(struct platform_device *pdev)
+{
+ struct xilinx_mbox *mbox;
+ struct resource *regs;
+ int ret;
+
+ mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
+ if (!mbox)
+ return -ENOMEM;
+
+ /* get clk and enable */
+ mbox->clk = devm_clk_get(&pdev->dev, "mbox");
+ if (IS_ERR(mbox->clk)) {
+ dev_err(&pdev->dev, "Couldn't get clk.\n");
+ return PTR_ERR(mbox->clk);
+ }
+
+ regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ mbox->mbox_base = devm_ioremap_resource(&pdev->dev, regs);
+ if (IS_ERR(mbox->mbox_base))
+ return PTR_ERR(mbox->mbox_base);
+
+ mbox->irq = platform_get_irq(pdev, 0);
+ /* if irq is present, we can use it, otherwise, poll */
+ if (mbox->irq > 0) {
+ mbox->controller.txdone_irq = true;
+ mbox->controller.ops = &xilinx_mbox_irq_ops;
+ } else {
+ dev_info(&pdev->dev, "IRQ not found, fallback to polling.\n");
+ mbox->controller.txdone_poll = true;
+ mbox->controller.txpoll_period = MBOX_POLLING_MS;
+ mbox->controller.ops = &xilinx_mbox_poll_ops;
+
+ setup_timer(&mbox->rxpoll_timer, xilinx_mbox_poll_rx,
+ (unsigned long)&mbox->chan);
+ }
+
+ mbox->dev = &pdev->dev;
+
+ /* hardware supports only one channel. */
+ mbox->controller.dev = mbox->dev;
+ mbox->controller.num_chans = 1;
+ mbox->controller.chans = &mbox->chan;
+
+ ret = mbox_controller_register(&mbox->controller);
+ if (ret) {
+ dev_err(&pdev->dev, "Register mailbox failed\n");
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, mbox);
+
+ return 0;
+}
+
+static int xilinx_mbox_remove(struct platform_device *pdev)
+{
+ struct xilinx_mbox *mbox = platform_get_drvdata(pdev);
+
+ mbox_controller_unregister(&mbox->controller);
+
+ return 0;
+}
+
+static const struct of_device_id xilinx_mbox_match[] = {
+ { .compatible = "xlnx,mailbox-2.1" },
+ { /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, xilinx_mbox_match);
+
+static struct platform_driver xilinx_mbox_driver = {
+ .probe = xilinx_mbox_probe,
+ .remove = xilinx_mbox_remove,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = xilinx_mbox_match,
+ },
+};
+
+module_platform_driver(xilinx_mbox_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Xilinx mailbox specific functions");
+MODULE_AUTHOR("Moritz Fischer <moritz.fischer@ettus.com>");
--
2.4.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCHv7 2/2] mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
@ 2015-07-15 1:00 ` Moritz Fischer
0 siblings, 0 replies; 21+ messages in thread
From: Moritz Fischer @ 2015-07-15 1:00 UTC (permalink / raw)
To: linux-arm-kernel
The Xilinx LogiCORE IP mailbox is a FPGA core that allows for
interprocessor communication via AXI4 memory mapped / AXI4 stream
interfaces.
It is single channel per core and allows for transmit and receive.
Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
Acked-by: Michal Simek <michal.simek@xilinx.com>
---
MAINTAINERS | 7 +
drivers/mailbox/Kconfig | 7 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/mailbox-xilinx.c | 367 +++++++++++++++++++++++++++++++++++++++
4 files changed, 383 insertions(+)
create mode 100644 drivers/mailbox/mailbox-xilinx.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 2d3d55c..bfe2b78 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11320,6 +11320,13 @@ M: John Linn <John.Linn@xilinx.com>
S: Maintained
F: drivers/net/ethernet/xilinx/xilinx_axienet*
+XILINX MAILBOX DRIVER
+M: Moritz Fischer <moritz.fischer@ettus.com>
+L: linux-kernel at vger.kernel.org
+S: Maintained
+F: drivers/mailbox/mailbox-xilinx.c
+F: Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
+
XILINX UARTLITE SERIAL DRIVER
M: Peter Korsgaard <jacmet@sunsite.dk>
L: linux-serial at vger.kernel.org
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index e269f08..15b3ade 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -70,4 +70,11 @@ config BCM2835_MBOX
the services of the Videocore. Say Y here if you want to use the
BCM2835 Mailbox.
+config XILINX_MBOX
+ tristate "Xilinx Mailbox"
+ help
+ An implementation of the Xilinx Mailbox soft core. It is used
+ to send message between processors. Say Y here if you want to use the
+ Xilinx mailbox support.
+
endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 8e6d822..1966e26 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -13,3 +13,5 @@ obj-$(CONFIG_PCC) += pcc.o
obj-$(CONFIG_ALTERA_MBOX) += mailbox-altera.o
obj-$(CONFIG_BCM2835_MBOX) += bcm2835-mailbox.o
+
+obj-$(CONFIG_XILINX_MBOX) += mailbox-xilinx.o
diff --git a/drivers/mailbox/mailbox-xilinx.c b/drivers/mailbox/mailbox-xilinx.c
new file mode 100644
index 0000000..27011e6
--- /dev/null
+++ b/drivers/mailbox/mailbox-xilinx.c
@@ -0,0 +1,367 @@
+/*
+ * Copyright (c) 2015, National Instruments Corp. All rights reserved.
+ *
+ * Driver for the Xilinx LogiCORE mailbox IP block
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+/* register offsets */
+#define MAILBOX_REG_WRDATA 0x00
+#define MAILBOX_REG_RDDATA 0x08
+#define MAILBOX_REG_STATUS 0x10
+#define MAILBOX_REG_ERROR 0x14
+#define MAILBOX_REG_SIT 0x18
+#define MAILBOX_REG_RIT 0x1c
+#define MAILBOX_REG_IS 0x20
+#define MAILBOX_REG_IE 0x24
+#define MAILBOX_REG_IP 0x28
+
+/* status register */
+#define STS_RTA BIT(3)
+#define STS_STA BIT(2)
+#define STS_FULL BIT(1)
+#define STS_EMPTY BIT(0)
+
+/* error register */
+#define ERR_FULL BIT(1)
+#define ERR_EMPTY BIT(0)
+
+/* mailbox interrupt status register */
+#define INT_STATUS_ERR BIT(2)
+#define INT_STATUS_RTI BIT(1)
+#define INT_STATUS_STI BIT(0)
+
+/* mailbox interrupt enable register */
+#define INT_ENABLE_ERR BIT(2)
+#define INT_ENABLE_RTI BIT(1)
+#define INT_ENABLE_STI BIT(0)
+
+#define MBOX_POLLING_MS 5 /* polling interval 5ms */
+
+struct xilinx_mbox {
+ int irq;
+ void __iomem *mbox_base;
+ struct clk *clk;
+ struct device *dev;
+ struct mbox_controller controller;
+ struct mbox_chan chan;
+
+ /* if the controller supports only RX polling mode */
+ struct timer_list rxpoll_timer;
+};
+
+static struct xilinx_mbox *mbox_chan_to_xilinx_mbox(struct mbox_chan *chan)
+{
+ return container_of(chan, struct xilinx_mbox, chan);
+}
+
+static inline bool xilinx_mbox_full(struct xilinx_mbox *mbox)
+{
+ u32 status;
+
+ status = readl_relaxed(mbox->mbox_base + MAILBOX_REG_STATUS);
+
+ return status & STS_FULL;
+}
+
+static inline bool xilinx_mbox_pending(struct xilinx_mbox *mbox)
+{
+ u32 status;
+
+ status = readl_relaxed(mbox->mbox_base + MAILBOX_REG_STATUS);
+
+ return !(status & STS_EMPTY);
+}
+
+static void xilinx_mbox_intmask(struct xilinx_mbox *mbox, u32 mask, bool enable)
+{
+ u32 mask_reg;
+
+ mask_reg = readl_relaxed(mbox->mbox_base + MAILBOX_REG_IE);
+ if (enable)
+ mask_reg |= mask;
+ else
+ mask_reg &= ~mask;
+
+ writel_relaxed(mask_reg, mbox->mbox_base + MAILBOX_REG_IE);
+}
+
+static inline void xilinx_mbox_rx_intmask(struct xilinx_mbox *mbox, bool enable)
+{
+ xilinx_mbox_intmask(mbox, INT_ENABLE_RTI, enable);
+}
+
+static inline void xilinx_mbox_tx_intmask(struct xilinx_mbox *mbox, bool enable)
+{
+ xilinx_mbox_intmask(mbox, INT_ENABLE_STI, enable);
+}
+
+static void xilinx_mbox_rx_data(struct mbox_chan *chan)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+ u32 data;
+
+ if (xilinx_mbox_pending(mbox)) {
+ data = readl_relaxed(mbox->mbox_base + MAILBOX_REG_RDDATA);
+ mbox_chan_received_data(chan, (void *)data);
+ }
+}
+
+static void xilinx_mbox_poll_rx(unsigned long data)
+{
+ struct mbox_chan *chan = (struct mbox_chan *)data;
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ xilinx_mbox_rx_data(chan);
+
+ mod_timer(&mbox->rxpoll_timer,
+ jiffies + msecs_to_jiffies(MBOX_POLLING_MS));
+}
+
+static irqreturn_t xilinx_mbox_interrupt(int irq, void *p)
+{
+ u32 mask;
+ struct mbox_chan *chan = (struct mbox_chan *)p;
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ mask = readl_relaxed(mbox->mbox_base + MAILBOX_REG_IS);
+ if (mask & INT_STATUS_RTI)
+ xilinx_mbox_rx_data(chan);
+
+ /* mask irqs *before* notifying done, require tx_block=true */
+ if (mask & INT_STATUS_STI) {
+ xilinx_mbox_tx_intmask(mbox, false);
+ mbox_chan_txdone(chan, 0);
+ }
+
+ writel_relaxed(mask, mbox->mbox_base + MAILBOX_REG_IS);
+
+ return IRQ_HANDLED;
+}
+
+static int xilinx_mbox_irq_startup(struct mbox_chan *chan)
+{
+ int ret;
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ ret = request_irq(mbox->irq, xilinx_mbox_interrupt, 0,
+ dev_name(mbox->dev), chan);
+ if (unlikely(ret)) {
+ dev_err(mbox->dev,
+ "failed to register mailbox interrupt:%d\n",
+ ret);
+ return ret;
+ }
+
+ /* prep and enable the clock */
+ clk_prepare_enable(mbox->clk);
+
+ xilinx_mbox_rx_intmask(mbox, true);
+
+ /* if fifo was full already, we didn't get an interrupt */
+ while (xilinx_mbox_pending(mbox))
+ xilinx_mbox_rx_data(chan);
+
+ return 0;
+}
+
+static int xilinx_mbox_poll_startup(struct mbox_chan *chan)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ clk_prepare_enable(mbox->clk);
+
+ mod_timer(&mbox->rxpoll_timer,
+ jiffies + msecs_to_jiffies(MBOX_POLLING_MS));
+
+ return 0;
+}
+
+static int xilinx_mbox_irq_send_data(struct mbox_chan *chan, void *data)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+ u32 *udata = data;
+
+ if (xilinx_mbox_full(mbox))
+ return -EBUSY;
+
+ /* enable interrupt before send */
+ xilinx_mbox_tx_intmask(mbox, true);
+
+ writel_relaxed(*udata, mbox->mbox_base + MAILBOX_REG_WRDATA);
+
+ return 0;
+}
+
+static int xilinx_mbox_poll_send_data(struct mbox_chan *chan, void *data)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+ u32 *udata = data;
+
+ if (!mbox || !data)
+ return -EINVAL;
+
+ if (xilinx_mbox_full(mbox))
+ return -EBUSY;
+
+ writel_relaxed(*udata, mbox->mbox_base + MAILBOX_REG_WRDATA);
+
+ return 0;
+}
+
+static bool xilinx_mbox_last_tx_done(struct mbox_chan *chan)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ /* return false if mailbox is full */
+ return !xilinx_mbox_full(mbox);
+}
+
+static bool xilinx_mbox_peek_data(struct mbox_chan *chan)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ return xilinx_mbox_pending(mbox);
+}
+
+static void xilinx_mbox_irq_shutdown(struct mbox_chan *chan)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ /* mask all interrupts */
+ writel_relaxed(0, mbox->mbox_base + MAILBOX_REG_IE);
+
+ clk_disable_unprepare(mbox->clk);
+
+ free_irq(mbox->irq, chan);
+}
+
+static void xilinx_mbox_poll_shutdown(struct mbox_chan *chan)
+{
+ struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
+
+ del_timer_sync(&mbox->rxpoll_timer);
+
+ clk_disable_unprepare(mbox->clk);
+}
+
+static const struct mbox_chan_ops xilinx_mbox_irq_ops = {
+ .send_data = xilinx_mbox_irq_send_data,
+ .startup = xilinx_mbox_irq_startup,
+ .shutdown = xilinx_mbox_irq_shutdown,
+ .last_tx_done = xilinx_mbox_last_tx_done,
+ .peek_data = xilinx_mbox_peek_data,
+};
+
+static const struct mbox_chan_ops xilinx_mbox_poll_ops = {
+ .send_data = xilinx_mbox_poll_send_data,
+ .startup = xilinx_mbox_poll_startup,
+ .shutdown = xilinx_mbox_poll_shutdown,
+ .last_tx_done = xilinx_mbox_last_tx_done,
+ .peek_data = xilinx_mbox_peek_data,
+};
+
+static int xilinx_mbox_probe(struct platform_device *pdev)
+{
+ struct xilinx_mbox *mbox;
+ struct resource *regs;
+ int ret;
+
+ mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
+ if (!mbox)
+ return -ENOMEM;
+
+ /* get clk and enable */
+ mbox->clk = devm_clk_get(&pdev->dev, "mbox");
+ if (IS_ERR(mbox->clk)) {
+ dev_err(&pdev->dev, "Couldn't get clk.\n");
+ return PTR_ERR(mbox->clk);
+ }
+
+ regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ mbox->mbox_base = devm_ioremap_resource(&pdev->dev, regs);
+ if (IS_ERR(mbox->mbox_base))
+ return PTR_ERR(mbox->mbox_base);
+
+ mbox->irq = platform_get_irq(pdev, 0);
+ /* if irq is present, we can use it, otherwise, poll */
+ if (mbox->irq > 0) {
+ mbox->controller.txdone_irq = true;
+ mbox->controller.ops = &xilinx_mbox_irq_ops;
+ } else {
+ dev_info(&pdev->dev, "IRQ not found, fallback to polling.\n");
+ mbox->controller.txdone_poll = true;
+ mbox->controller.txpoll_period = MBOX_POLLING_MS;
+ mbox->controller.ops = &xilinx_mbox_poll_ops;
+
+ setup_timer(&mbox->rxpoll_timer, xilinx_mbox_poll_rx,
+ (unsigned long)&mbox->chan);
+ }
+
+ mbox->dev = &pdev->dev;
+
+ /* hardware supports only one channel. */
+ mbox->controller.dev = mbox->dev;
+ mbox->controller.num_chans = 1;
+ mbox->controller.chans = &mbox->chan;
+
+ ret = mbox_controller_register(&mbox->controller);
+ if (ret) {
+ dev_err(&pdev->dev, "Register mailbox failed\n");
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, mbox);
+
+ return 0;
+}
+
+static int xilinx_mbox_remove(struct platform_device *pdev)
+{
+ struct xilinx_mbox *mbox = platform_get_drvdata(pdev);
+
+ mbox_controller_unregister(&mbox->controller);
+
+ return 0;
+}
+
+static const struct of_device_id xilinx_mbox_match[] = {
+ { .compatible = "xlnx,mailbox-2.1" },
+ { /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, xilinx_mbox_match);
+
+static struct platform_driver xilinx_mbox_driver = {
+ .probe = xilinx_mbox_probe,
+ .remove = xilinx_mbox_remove,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = xilinx_mbox_match,
+ },
+};
+
+module_platform_driver(xilinx_mbox_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Xilinx mailbox specific functions");
+MODULE_AUTHOR("Moritz Fischer <moritz.fischer@ettus.com>");
--
2.4.3
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCHv7 1/2] dts: Adding docs for Xilinx LogiCORE IP mailbox driver.
2015-07-15 1:00 ` Moritz Fischer
(?)
@ 2015-07-15 1:23 ` Sören Brinkmann
-1 siblings, 0 replies; 21+ messages in thread
From: Sören Brinkmann @ 2015-07-15 1:23 UTC (permalink / raw)
To: Moritz Fischer
Cc: jassisinghbrar, linux-kernel, robh+dt, pawel.moll, mark.rutland,
ijc+devicetree, galak, michal.simek, akpm, gregkh, mchehab, arnd,
joe, jingoohan1, devicetree, linux-arm-kernel
On Tue, 2015-07-14 at 06:00PM -0700, Moritz Fischer wrote:
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> Acked-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
Sören
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCHv7 1/2] dts: Adding docs for Xilinx LogiCORE IP mailbox driver.
@ 2015-07-15 1:23 ` Sören Brinkmann
0 siblings, 0 replies; 21+ messages in thread
From: Sören Brinkmann @ 2015-07-15 1:23 UTC (permalink / raw)
To: Moritz Fischer
Cc: mark.rutland, devicetree, arnd, pawel.moll, ijc+devicetree,
gregkh, jassisinghbrar, linux-kernel, michal.simek, jingoohan1,
robh+dt, linux-arm-kernel, galak, joe, akpm, mchehab
On Tue, 2015-07-14 at 06:00PM -0700, Moritz Fischer wrote:
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> Acked-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
Sören
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCHv7 1/2] dts: Adding docs for Xilinx LogiCORE IP mailbox driver.
@ 2015-07-15 1:23 ` Sören Brinkmann
0 siblings, 0 replies; 21+ messages in thread
From: Sören Brinkmann @ 2015-07-15 1:23 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 2015-07-14 at 06:00PM -0700, Moritz Fischer wrote:
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> Acked-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: S?ren Brinkmann <soren.brinkmann@xilinx.com>
S?ren
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCHv7 0/2] Adding driver for Xilinx LogiCORE IP mailbox.
@ 2015-07-28 22:44 ` Moritz Fischer
0 siblings, 0 replies; 21+ messages in thread
From: Moritz Fischer @ 2015-07-28 22:44 UTC (permalink / raw)
To: Jassi Brar
Cc: linux-kernel, robh+dt, pawel.moll, mark.rutland, ijc+devicetree,
Kumar Gala, Michal Simek, Sören Brinkmann, akpm, Greg KH,
mchehab, Arnd Bergmann, joe, Jingoo Han, devicetree,
linux-arm-kernel, Moritz Fischer
Hi Jassi,
just a ping to see if you're waiting on me to fix things or if this is
good to go from your point of view,
and you're just waiting some time to give people more time to review.
Let me know if you want me to resend with Soeren's Acked-By
Cheers,
Moritz
On Tue, Jul 14, 2015 at 6:00 PM, Moritz Fischer
<moritz.fischer@ettus.com> wrote:
> Hi all,
>
> Thanks for hanging in there, hopefully the last round of review ...
>
> This patchset adds mailbox framework integration for the Xilinx LogiCORE IP
> mailbox. The Xilinx LogiCORE IP mailbox is a fpga softcore that allows
> interprocessor communication between AXI4 stream / memory mapped
> processors.
>
> Cheers,
>
> Moritz
>
> Changes from v6:
> - As suggested by Jassi and Sören use #mbox-cells = <1>
>
> Changes from v5:
> - Fixed constness for ops struct
> - Removed redundant checks
> - Moved ops assignment
> - Removed MODULE_ALIAS
>
> Changes from v4:
> - Have separate mbox_ops structs for polling / irq mode
> - Moved clk handling to startup / shutdown
> - Embedded struct mbox_chan in struct xilinx_mbox
> - Fixed mbox-cells in devicetree documentation
> - Misc stylistic issues
>
> Changes from v3:
> - Stylistic changes
> - Changed reg size in dts to 0x100
>
> Changes from v2:
> - Fixed error condition for IRQ from >= 0 to > 0
> - Fixed clock enable
> - Cleaned up docs
>
> Changes from v1:
> - Added common clock framework support
> - Deal with IRQs that happend before driver load,
> since HW will not let us know about them when we enable IRQs
>
> Changes from v0:
> - Several stylistic issues
> - Dropped superfluous intr_mode member
> - Really masking the IRQs on mailbox_shutdown
> - No longer using polling by accident in non-IRQ mode
> - Swapped doc and driver commits
>
>
> Moritz Fischer (2):
> dts: Adding docs for Xilinx LogiCORE IP mailbox driver.
> mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
>
> .../devicetree/bindings/mailbox/xilinx-mailbox.txt | 44 +++
> MAINTAINERS | 7 +
> drivers/mailbox/Kconfig | 7 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/mailbox-xilinx.c | 367 +++++++++++++++++++++
> 5 files changed, 427 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
> create mode 100644 drivers/mailbox/mailbox-xilinx.c
>
> --
> 2.4.3
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCHv7 0/2] Adding driver for Xilinx LogiCORE IP mailbox.
@ 2015-07-28 22:44 ` Moritz Fischer
0 siblings, 0 replies; 21+ messages in thread
From: Moritz Fischer @ 2015-07-28 22:44 UTC (permalink / raw)
To: Jassi Brar
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
Kumar Gala, Michal Simek, Sören Brinkmann,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Greg KH,
mchehab-JPH+aEBZ4P+UEJcrhfAQsw, Arnd Bergmann,
joe-6d6DIl74uiNBDgjK7y7TUQ, Jingoo Han,
devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel,
Moritz Fischer
Hi Jassi,
just a ping to see if you're waiting on me to fix things or if this is
good to go from your point of view,
and you're just waiting some time to give people more time to review.
Let me know if you want me to resend with Soeren's Acked-By
Cheers,
Moritz
On Tue, Jul 14, 2015 at 6:00 PM, Moritz Fischer
<moritz.fischer-+aYTwkv1SeIAvxtiuMwx3w@public.gmane.org> wrote:
> Hi all,
>
> Thanks for hanging in there, hopefully the last round of review ...
>
> This patchset adds mailbox framework integration for the Xilinx LogiCORE IP
> mailbox. The Xilinx LogiCORE IP mailbox is a fpga softcore that allows
> interprocessor communication between AXI4 stream / memory mapped
> processors.
>
> Cheers,
>
> Moritz
>
> Changes from v6:
> - As suggested by Jassi and Sören use #mbox-cells = <1>
>
> Changes from v5:
> - Fixed constness for ops struct
> - Removed redundant checks
> - Moved ops assignment
> - Removed MODULE_ALIAS
>
> Changes from v4:
> - Have separate mbox_ops structs for polling / irq mode
> - Moved clk handling to startup / shutdown
> - Embedded struct mbox_chan in struct xilinx_mbox
> - Fixed mbox-cells in devicetree documentation
> - Misc stylistic issues
>
> Changes from v3:
> - Stylistic changes
> - Changed reg size in dts to 0x100
>
> Changes from v2:
> - Fixed error condition for IRQ from >= 0 to > 0
> - Fixed clock enable
> - Cleaned up docs
>
> Changes from v1:
> - Added common clock framework support
> - Deal with IRQs that happend before driver load,
> since HW will not let us know about them when we enable IRQs
>
> Changes from v0:
> - Several stylistic issues
> - Dropped superfluous intr_mode member
> - Really masking the IRQs on mailbox_shutdown
> - No longer using polling by accident in non-IRQ mode
> - Swapped doc and driver commits
>
>
> Moritz Fischer (2):
> dts: Adding docs for Xilinx LogiCORE IP mailbox driver.
> mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
>
> .../devicetree/bindings/mailbox/xilinx-mailbox.txt | 44 +++
> MAINTAINERS | 7 +
> drivers/mailbox/Kconfig | 7 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/mailbox-xilinx.c | 367 +++++++++++++++++++++
> 5 files changed, 427 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
> create mode 100644 drivers/mailbox/mailbox-xilinx.c
>
> --
> 2.4.3
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCHv7 0/2] Adding driver for Xilinx LogiCORE IP mailbox.
@ 2015-07-28 22:44 ` Moritz Fischer
0 siblings, 0 replies; 21+ messages in thread
From: Moritz Fischer @ 2015-07-28 22:44 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jassi,
just a ping to see if you're waiting on me to fix things or if this is
good to go from your point of view,
and you're just waiting some time to give people more time to review.
Let me know if you want me to resend with Soeren's Acked-By
Cheers,
Moritz
On Tue, Jul 14, 2015 at 6:00 PM, Moritz Fischer
<moritz.fischer@ettus.com> wrote:
> Hi all,
>
> Thanks for hanging in there, hopefully the last round of review ...
>
> This patchset adds mailbox framework integration for the Xilinx LogiCORE IP
> mailbox. The Xilinx LogiCORE IP mailbox is a fpga softcore that allows
> interprocessor communication between AXI4 stream / memory mapped
> processors.
>
> Cheers,
>
> Moritz
>
> Changes from v6:
> - As suggested by Jassi and S?ren use #mbox-cells = <1>
>
> Changes from v5:
> - Fixed constness for ops struct
> - Removed redundant checks
> - Moved ops assignment
> - Removed MODULE_ALIAS
>
> Changes from v4:
> - Have separate mbox_ops structs for polling / irq mode
> - Moved clk handling to startup / shutdown
> - Embedded struct mbox_chan in struct xilinx_mbox
> - Fixed mbox-cells in devicetree documentation
> - Misc stylistic issues
>
> Changes from v3:
> - Stylistic changes
> - Changed reg size in dts to 0x100
>
> Changes from v2:
> - Fixed error condition for IRQ from >= 0 to > 0
> - Fixed clock enable
> - Cleaned up docs
>
> Changes from v1:
> - Added common clock framework support
> - Deal with IRQs that happend before driver load,
> since HW will not let us know about them when we enable IRQs
>
> Changes from v0:
> - Several stylistic issues
> - Dropped superfluous intr_mode member
> - Really masking the IRQs on mailbox_shutdown
> - No longer using polling by accident in non-IRQ mode
> - Swapped doc and driver commits
>
>
> Moritz Fischer (2):
> dts: Adding docs for Xilinx LogiCORE IP mailbox driver.
> mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
>
> .../devicetree/bindings/mailbox/xilinx-mailbox.txt | 44 +++
> MAINTAINERS | 7 +
> drivers/mailbox/Kconfig | 7 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/mailbox-xilinx.c | 367 +++++++++++++++++++++
> 5 files changed, 427 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/xilinx-mailbox.txt
> create mode 100644 drivers/mailbox/mailbox-xilinx.c
>
> --
> 2.4.3
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCHv7 2/2] mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
@ 2015-08-10 8:05 ` Jassi Brar
0 siblings, 0 replies; 21+ messages in thread
From: Jassi Brar @ 2015-08-10 8:05 UTC (permalink / raw)
To: Moritz Fischer
Cc: Linux Kernel Mailing List, Rob Herring, Pawel Moll, Mark Rutland,
ijc+devicetree@hellion.org.uk, Kumar Gala, Michal Simek,
Sören Brinkmann, Andrew Morton, Greg Kroah-Hartman, mchehab,
Arnd Bergmann, joe, jingoohan1, Devicetree List,
linux-arm-kernel@lists.infradead.org
On Wed, Jul 15, 2015 at 6:30 AM, Moritz Fischer
<moritz.fischer@ettus.com> wrote:
> +
> +static void xilinx_mbox_rx_data(struct mbox_chan *chan)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> + u32 data;
> +
> + if (xilinx_mbox_pending(mbox)) {
> + data = readl_relaxed(mbox->mbox_base + MAILBOX_REG_RDDATA);
> + mbox_chan_received_data(chan, (void *)data);
>
If RDDATA is a FIFO, then above seems wrong - you are assuming
messages are always going to be exactly 32-bits for every protocol.
Ideally you should empty the fifo fully, at least when RX has an
interrupt.
> +
> +static int xilinx_mbox_irq_send_data(struct mbox_chan *chan, void *data)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> + u32 *udata = data;
> +
> + if (xilinx_mbox_full(mbox))
> + return -EBUSY;
>
This check is redundant. last_tx_done already makes sure this is always true.
> + /* enable interrupt before send */
> + xilinx_mbox_tx_intmask(mbox, true);
> +
> + writel_relaxed(*udata, mbox->mbox_base + MAILBOX_REG_WRDATA);
> +
>From status EMPTY and FULL, it seems WRDATA is a FIFO. So here also
you should expect messages to be <= fifo depth. And not assume exactly
32bit messages always.
> +
> +static bool xilinx_mbox_last_tx_done(struct mbox_chan *chan)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> +
> + /* return false if mailbox is full */
> + return !xilinx_mbox_full(mbox);
>
Instead of FULL, I think it should check for stricter EMPTY status ...
mbox api submits only 1 message at a time.
> +
> +static bool xilinx_mbox_peek_data(struct mbox_chan *chan)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> +
> + return xilinx_mbox_pending(mbox);
> +}
> +
> +static void xilinx_mbox_irq_shutdown(struct mbox_chan *chan)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> +
> + /* mask all interrupts */
> + writel_relaxed(0, mbox->mbox_base + MAILBOX_REG_IE);
> +
> + clk_disable_unprepare(mbox->clk);
> +
> + free_irq(mbox->irq, chan);
> +}
> +
> +static void xilinx_mbox_poll_shutdown(struct mbox_chan *chan)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> +
> + del_timer_sync(&mbox->rxpoll_timer);
> +
> + clk_disable_unprepare(mbox->clk);
> +}
> +
> +static const struct mbox_chan_ops xilinx_mbox_irq_ops = {
> + .send_data = xilinx_mbox_irq_send_data,
> + .startup = xilinx_mbox_irq_startup,
> + .shutdown = xilinx_mbox_irq_shutdown,
> + .last_tx_done = xilinx_mbox_last_tx_done,
> + .peek_data = xilinx_mbox_peek_data,
> +};
> +
> +static const struct mbox_chan_ops xilinx_mbox_poll_ops = {
> + .send_data = xilinx_mbox_poll_send_data,
> + .startup = xilinx_mbox_poll_startup,
> + .shutdown = xilinx_mbox_poll_shutdown,
> + .last_tx_done = xilinx_mbox_last_tx_done,
> + .peek_data = xilinx_mbox_peek_data,
> +};
> +
> +static int xilinx_mbox_probe(struct platform_device *pdev)
> +{
> + struct xilinx_mbox *mbox;
> + struct resource *regs;
> + int ret;
> +
> + mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
> + if (!mbox)
> + return -ENOMEM;
> +
> + /* get clk and enable */
> + mbox->clk = devm_clk_get(&pdev->dev, "mbox");
> + if (IS_ERR(mbox->clk)) {
> + dev_err(&pdev->dev, "Couldn't get clk.\n");
> + return PTR_ERR(mbox->clk);
> + }
> +
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> + mbox->mbox_base = devm_ioremap_resource(&pdev->dev, regs);
> + if (IS_ERR(mbox->mbox_base))
> + return PTR_ERR(mbox->mbox_base);
> +
> + mbox->irq = platform_get_irq(pdev, 0);
> + /* if irq is present, we can use it, otherwise, poll */
> + if (mbox->irq > 0) {
> + mbox->controller.txdone_irq = true;
> + mbox->controller.ops = &xilinx_mbox_irq_ops;
> + } else {
> + dev_info(&pdev->dev, "IRQ not found, fallback to polling.\n");
> + mbox->controller.txdone_poll = true;
> + mbox->controller.txpoll_period = MBOX_POLLING_MS;
> + mbox->controller.ops = &xilinx_mbox_poll_ops;
> +
> + setup_timer(&mbox->rxpoll_timer, xilinx_mbox_poll_rx,
> + (unsigned long)&mbox->chan);
>
I believe there is indeed some platform that doesn't have RX-Interrupt?
If no, please remove this.
If yes, you may want to get some hint from platform about the size of
messages and do mbox_chan_received_data() only upon reading those many
bytes.
Cheers.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCHv7 2/2] mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
@ 2015-08-10 8:05 ` Jassi Brar
0 siblings, 0 replies; 21+ messages in thread
From: Jassi Brar @ 2015-08-10 8:05 UTC (permalink / raw)
To: Moritz Fischer
Cc: Linux Kernel Mailing List, Rob Herring, Pawel Moll, Mark Rutland,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
Kumar Gala, Michal Simek, Sören Brinkmann, Andrew Morton,
Greg Kroah-Hartman, mchehab-JPH+aEBZ4P+UEJcrhfAQsw, Arnd Bergmann,
joe-6d6DIl74uiNBDgjK7y7TUQ, jingoohan1-Re5JQEeQqe8AvxtiuMwx3w,
Devicetree List,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
On Wed, Jul 15, 2015 at 6:30 AM, Moritz Fischer
<moritz.fischer-+aYTwkv1SeIAvxtiuMwx3w@public.gmane.org> wrote:
> +
> +static void xilinx_mbox_rx_data(struct mbox_chan *chan)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> + u32 data;
> +
> + if (xilinx_mbox_pending(mbox)) {
> + data = readl_relaxed(mbox->mbox_base + MAILBOX_REG_RDDATA);
> + mbox_chan_received_data(chan, (void *)data);
>
If RDDATA is a FIFO, then above seems wrong - you are assuming
messages are always going to be exactly 32-bits for every protocol.
Ideally you should empty the fifo fully, at least when RX has an
interrupt.
> +
> +static int xilinx_mbox_irq_send_data(struct mbox_chan *chan, void *data)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> + u32 *udata = data;
> +
> + if (xilinx_mbox_full(mbox))
> + return -EBUSY;
>
This check is redundant. last_tx_done already makes sure this is always true.
> + /* enable interrupt before send */
> + xilinx_mbox_tx_intmask(mbox, true);
> +
> + writel_relaxed(*udata, mbox->mbox_base + MAILBOX_REG_WRDATA);
> +
>From status EMPTY and FULL, it seems WRDATA is a FIFO. So here also
you should expect messages to be <= fifo depth. And not assume exactly
32bit messages always.
> +
> +static bool xilinx_mbox_last_tx_done(struct mbox_chan *chan)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> +
> + /* return false if mailbox is full */
> + return !xilinx_mbox_full(mbox);
>
Instead of FULL, I think it should check for stricter EMPTY status ...
mbox api submits only 1 message at a time.
> +
> +static bool xilinx_mbox_peek_data(struct mbox_chan *chan)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> +
> + return xilinx_mbox_pending(mbox);
> +}
> +
> +static void xilinx_mbox_irq_shutdown(struct mbox_chan *chan)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> +
> + /* mask all interrupts */
> + writel_relaxed(0, mbox->mbox_base + MAILBOX_REG_IE);
> +
> + clk_disable_unprepare(mbox->clk);
> +
> + free_irq(mbox->irq, chan);
> +}
> +
> +static void xilinx_mbox_poll_shutdown(struct mbox_chan *chan)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> +
> + del_timer_sync(&mbox->rxpoll_timer);
> +
> + clk_disable_unprepare(mbox->clk);
> +}
> +
> +static const struct mbox_chan_ops xilinx_mbox_irq_ops = {
> + .send_data = xilinx_mbox_irq_send_data,
> + .startup = xilinx_mbox_irq_startup,
> + .shutdown = xilinx_mbox_irq_shutdown,
> + .last_tx_done = xilinx_mbox_last_tx_done,
> + .peek_data = xilinx_mbox_peek_data,
> +};
> +
> +static const struct mbox_chan_ops xilinx_mbox_poll_ops = {
> + .send_data = xilinx_mbox_poll_send_data,
> + .startup = xilinx_mbox_poll_startup,
> + .shutdown = xilinx_mbox_poll_shutdown,
> + .last_tx_done = xilinx_mbox_last_tx_done,
> + .peek_data = xilinx_mbox_peek_data,
> +};
> +
> +static int xilinx_mbox_probe(struct platform_device *pdev)
> +{
> + struct xilinx_mbox *mbox;
> + struct resource *regs;
> + int ret;
> +
> + mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
> + if (!mbox)
> + return -ENOMEM;
> +
> + /* get clk and enable */
> + mbox->clk = devm_clk_get(&pdev->dev, "mbox");
> + if (IS_ERR(mbox->clk)) {
> + dev_err(&pdev->dev, "Couldn't get clk.\n");
> + return PTR_ERR(mbox->clk);
> + }
> +
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> + mbox->mbox_base = devm_ioremap_resource(&pdev->dev, regs);
> + if (IS_ERR(mbox->mbox_base))
> + return PTR_ERR(mbox->mbox_base);
> +
> + mbox->irq = platform_get_irq(pdev, 0);
> + /* if irq is present, we can use it, otherwise, poll */
> + if (mbox->irq > 0) {
> + mbox->controller.txdone_irq = true;
> + mbox->controller.ops = &xilinx_mbox_irq_ops;
> + } else {
> + dev_info(&pdev->dev, "IRQ not found, fallback to polling.\n");
> + mbox->controller.txdone_poll = true;
> + mbox->controller.txpoll_period = MBOX_POLLING_MS;
> + mbox->controller.ops = &xilinx_mbox_poll_ops;
> +
> + setup_timer(&mbox->rxpoll_timer, xilinx_mbox_poll_rx,
> + (unsigned long)&mbox->chan);
>
I believe there is indeed some platform that doesn't have RX-Interrupt?
If no, please remove this.
If yes, you may want to get some hint from platform about the size of
messages and do mbox_chan_received_data() only upon reading those many
bytes.
Cheers.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCHv7 2/2] mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
@ 2015-08-10 8:05 ` Jassi Brar
0 siblings, 0 replies; 21+ messages in thread
From: Jassi Brar @ 2015-08-10 8:05 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 15, 2015 at 6:30 AM, Moritz Fischer
<moritz.fischer@ettus.com> wrote:
> +
> +static void xilinx_mbox_rx_data(struct mbox_chan *chan)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> + u32 data;
> +
> + if (xilinx_mbox_pending(mbox)) {
> + data = readl_relaxed(mbox->mbox_base + MAILBOX_REG_RDDATA);
> + mbox_chan_received_data(chan, (void *)data);
>
If RDDATA is a FIFO, then above seems wrong - you are assuming
messages are always going to be exactly 32-bits for every protocol.
Ideally you should empty the fifo fully, at least when RX has an
interrupt.
> +
> +static int xilinx_mbox_irq_send_data(struct mbox_chan *chan, void *data)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> + u32 *udata = data;
> +
> + if (xilinx_mbox_full(mbox))
> + return -EBUSY;
>
This check is redundant. last_tx_done already makes sure this is always true.
> + /* enable interrupt before send */
> + xilinx_mbox_tx_intmask(mbox, true);
> +
> + writel_relaxed(*udata, mbox->mbox_base + MAILBOX_REG_WRDATA);
> +
>From status EMPTY and FULL, it seems WRDATA is a FIFO. So here also
you should expect messages to be <= fifo depth. And not assume exactly
32bit messages always.
> +
> +static bool xilinx_mbox_last_tx_done(struct mbox_chan *chan)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> +
> + /* return false if mailbox is full */
> + return !xilinx_mbox_full(mbox);
>
Instead of FULL, I think it should check for stricter EMPTY status ...
mbox api submits only 1 message at a time.
> +
> +static bool xilinx_mbox_peek_data(struct mbox_chan *chan)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> +
> + return xilinx_mbox_pending(mbox);
> +}
> +
> +static void xilinx_mbox_irq_shutdown(struct mbox_chan *chan)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> +
> + /* mask all interrupts */
> + writel_relaxed(0, mbox->mbox_base + MAILBOX_REG_IE);
> +
> + clk_disable_unprepare(mbox->clk);
> +
> + free_irq(mbox->irq, chan);
> +}
> +
> +static void xilinx_mbox_poll_shutdown(struct mbox_chan *chan)
> +{
> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
> +
> + del_timer_sync(&mbox->rxpoll_timer);
> +
> + clk_disable_unprepare(mbox->clk);
> +}
> +
> +static const struct mbox_chan_ops xilinx_mbox_irq_ops = {
> + .send_data = xilinx_mbox_irq_send_data,
> + .startup = xilinx_mbox_irq_startup,
> + .shutdown = xilinx_mbox_irq_shutdown,
> + .last_tx_done = xilinx_mbox_last_tx_done,
> + .peek_data = xilinx_mbox_peek_data,
> +};
> +
> +static const struct mbox_chan_ops xilinx_mbox_poll_ops = {
> + .send_data = xilinx_mbox_poll_send_data,
> + .startup = xilinx_mbox_poll_startup,
> + .shutdown = xilinx_mbox_poll_shutdown,
> + .last_tx_done = xilinx_mbox_last_tx_done,
> + .peek_data = xilinx_mbox_peek_data,
> +};
> +
> +static int xilinx_mbox_probe(struct platform_device *pdev)
> +{
> + struct xilinx_mbox *mbox;
> + struct resource *regs;
> + int ret;
> +
> + mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
> + if (!mbox)
> + return -ENOMEM;
> +
> + /* get clk and enable */
> + mbox->clk = devm_clk_get(&pdev->dev, "mbox");
> + if (IS_ERR(mbox->clk)) {
> + dev_err(&pdev->dev, "Couldn't get clk.\n");
> + return PTR_ERR(mbox->clk);
> + }
> +
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> + mbox->mbox_base = devm_ioremap_resource(&pdev->dev, regs);
> + if (IS_ERR(mbox->mbox_base))
> + return PTR_ERR(mbox->mbox_base);
> +
> + mbox->irq = platform_get_irq(pdev, 0);
> + /* if irq is present, we can use it, otherwise, poll */
> + if (mbox->irq > 0) {
> + mbox->controller.txdone_irq = true;
> + mbox->controller.ops = &xilinx_mbox_irq_ops;
> + } else {
> + dev_info(&pdev->dev, "IRQ not found, fallback to polling.\n");
> + mbox->controller.txdone_poll = true;
> + mbox->controller.txpoll_period = MBOX_POLLING_MS;
> + mbox->controller.ops = &xilinx_mbox_poll_ops;
> +
> + setup_timer(&mbox->rxpoll_timer, xilinx_mbox_poll_rx,
> + (unsigned long)&mbox->chan);
>
I believe there is indeed some platform that doesn't have RX-Interrupt?
If no, please remove this.
If yes, you may want to get some hint from platform about the size of
messages and do mbox_chan_received_data() only upon reading those many
bytes.
Cheers.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCHv7 2/2] mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
2015-08-10 8:05 ` Jassi Brar
@ 2015-12-02 17:26 ` Moritz Fischer
-1 siblings, 0 replies; 21+ messages in thread
From: Moritz Fischer @ 2015-12-02 17:26 UTC (permalink / raw)
To: Jassi Brar
Cc: Linux Kernel Mailing List, Rob Herring, Pawel Moll, Mark Rutland,
ijc+devicetree@hellion.org.uk, Kumar Gala, Michal Simek,
Sören Brinkmann, Andrew Morton, Greg Kroah-Hartman, mchehab,
Arnd Bergmann, joe, Jingoo Han, Devicetree List,
linux-arm-kernel@lists.infradead.org
Hi Jassi,
thanks for your feedback.
On Mon, Aug 10, 2015 at 1:05 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> On Wed, Jul 15, 2015 at 6:30 AM, Moritz Fischer
> <moritz.fischer@ettus.com> wrote:
>
>> +
>> +static void xilinx_mbox_rx_data(struct mbox_chan *chan)
>> +{
>> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
>> + u32 data;
>> +
>> + if (xilinx_mbox_pending(mbox)) {
>> + data = readl_relaxed(mbox->mbox_base + MAILBOX_REG_RDDATA);
>> + mbox_chan_received_data(chan, (void *)data);
>>
> If RDDATA is a FIFO, then above seems wrong - you are assuming
> messages are always going to be exactly 32-bits for every protocol.
> Ideally you should empty the fifo fully, at least when RX has an
> interrupt.
>From my understanding it's hard to tell how much actually is in the fifo,
you can tell if it's full for send direction, or empty for read direction.
Maybe the STI / RTI can be setup in a smart way to work with multiple message
sizes.
>
>> +
>> +static int xilinx_mbox_irq_send_data(struct mbox_chan *chan, void *data)
>> +{
>> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
>> + u32 *udata = data;
>> +
>> + if (xilinx_mbox_full(mbox))
>> + return -EBUSY;
>>
> This check is redundant. last_tx_done already makes sure this is always true.
Good to know. I'll fix it.
>
>> + /* enable interrupt before send */
>> + xilinx_mbox_tx_intmask(mbox, true);
>> +
>> + writel_relaxed(*udata, mbox->mbox_base + MAILBOX_REG_WRDATA);
>> +
> From status EMPTY and FULL, it seems WRDATA is a FIFO. So here also
> you should expect messages to be <= fifo depth. And not assume exactly
> 32bit messages always.
How do I determine the message size? Doesn't
drivers/mailbox/bcm2835-mailbox.c or
mailbox-altera.c make the same assumption?
>
>> +
>> +static bool xilinx_mbox_last_tx_done(struct mbox_chan *chan)
>> +{
>> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
>> +
>> + /* return false if mailbox is full */
>> + return !xilinx_mbox_full(mbox);
>>
> Instead of FULL, I think it should check for stricter EMPTY status ...
> mbox api submits only 1 message at a time.
The EMPTY status applies to the receive direction only, I could check
the STI status
bit though I suppose.
[...]
>> +
>> + mbox->irq = platform_get_irq(pdev, 0);
>> + /* if irq is present, we can use it, otherwise, poll */
>> + if (mbox->irq > 0) {
>> + mbox->controller.txdone_irq = true;
>> + mbox->controller.ops = &xilinx_mbox_irq_ops;
>> + } else {
>> + dev_info(&pdev->dev, "IRQ not found, fallback to polling.\n");
>> + mbox->controller.txdone_poll = true;
>> + mbox->controller.txpoll_period = MBOX_POLLING_MS;
>> + mbox->controller.ops = &xilinx_mbox_poll_ops;
>> +
>> + setup_timer(&mbox->rxpoll_timer, xilinx_mbox_poll_rx,
>> + (unsigned long)&mbox->chan);
>>
> I believe there is indeed some platform that doesn't have RX-Interrupt?
> If no, please remove this.
> If yes, you may want to get some hint from platform about the size of
> messages and do mbox_chan_received_data() only upon reading those many
> bytes.
I'd be fine to drop the polling use case for the moment, on my
platform I can wire up the IRQ.
We can always add it back in in a follow up use case.
Thanks,
Moritz
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCHv7 2/2] mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
@ 2015-12-02 17:26 ` Moritz Fischer
0 siblings, 0 replies; 21+ messages in thread
From: Moritz Fischer @ 2015-12-02 17:26 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jassi,
thanks for your feedback.
On Mon, Aug 10, 2015 at 1:05 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> On Wed, Jul 15, 2015 at 6:30 AM, Moritz Fischer
> <moritz.fischer@ettus.com> wrote:
>
>> +
>> +static void xilinx_mbox_rx_data(struct mbox_chan *chan)
>> +{
>> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
>> + u32 data;
>> +
>> + if (xilinx_mbox_pending(mbox)) {
>> + data = readl_relaxed(mbox->mbox_base + MAILBOX_REG_RDDATA);
>> + mbox_chan_received_data(chan, (void *)data);
>>
> If RDDATA is a FIFO, then above seems wrong - you are assuming
> messages are always going to be exactly 32-bits for every protocol.
> Ideally you should empty the fifo fully, at least when RX has an
> interrupt.
>From my understanding it's hard to tell how much actually is in the fifo,
you can tell if it's full for send direction, or empty for read direction.
Maybe the STI / RTI can be setup in a smart way to work with multiple message
sizes.
>
>> +
>> +static int xilinx_mbox_irq_send_data(struct mbox_chan *chan, void *data)
>> +{
>> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
>> + u32 *udata = data;
>> +
>> + if (xilinx_mbox_full(mbox))
>> + return -EBUSY;
>>
> This check is redundant. last_tx_done already makes sure this is always true.
Good to know. I'll fix it.
>
>> + /* enable interrupt before send */
>> + xilinx_mbox_tx_intmask(mbox, true);
>> +
>> + writel_relaxed(*udata, mbox->mbox_base + MAILBOX_REG_WRDATA);
>> +
> From status EMPTY and FULL, it seems WRDATA is a FIFO. So here also
> you should expect messages to be <= fifo depth. And not assume exactly
> 32bit messages always.
How do I determine the message size? Doesn't
drivers/mailbox/bcm2835-mailbox.c or
mailbox-altera.c make the same assumption?
>
>> +
>> +static bool xilinx_mbox_last_tx_done(struct mbox_chan *chan)
>> +{
>> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
>> +
>> + /* return false if mailbox is full */
>> + return !xilinx_mbox_full(mbox);
>>
> Instead of FULL, I think it should check for stricter EMPTY status ...
> mbox api submits only 1 message at a time.
The EMPTY status applies to the receive direction only, I could check
the STI status
bit though I suppose.
[...]
>> +
>> + mbox->irq = platform_get_irq(pdev, 0);
>> + /* if irq is present, we can use it, otherwise, poll */
>> + if (mbox->irq > 0) {
>> + mbox->controller.txdone_irq = true;
>> + mbox->controller.ops = &xilinx_mbox_irq_ops;
>> + } else {
>> + dev_info(&pdev->dev, "IRQ not found, fallback to polling.\n");
>> + mbox->controller.txdone_poll = true;
>> + mbox->controller.txpoll_period = MBOX_POLLING_MS;
>> + mbox->controller.ops = &xilinx_mbox_poll_ops;
>> +
>> + setup_timer(&mbox->rxpoll_timer, xilinx_mbox_poll_rx,
>> + (unsigned long)&mbox->chan);
>>
> I believe there is indeed some platform that doesn't have RX-Interrupt?
> If no, please remove this.
> If yes, you may want to get some hint from platform about the size of
> messages and do mbox_chan_received_data() only upon reading those many
> bytes.
I'd be fine to drop the polling use case for the moment, on my
platform I can wire up the IRQ.
We can always add it back in in a follow up use case.
Thanks,
Moritz
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCHv7 2/2] mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
@ 2015-12-03 5:05 ` Jassi Brar
0 siblings, 0 replies; 21+ messages in thread
From: Jassi Brar @ 2015-12-03 5:05 UTC (permalink / raw)
To: Moritz Fischer
Cc: Jassi Brar, Mark Rutland, Devicetree List, Arnd Bergmann,
Pawel Moll, ijc+devicetree@hellion.org.uk, Greg Kroah-Hartman,
mchehab, Linux Kernel Mailing List, Michal Simek, Jingoo Han,
Rob Herring, linux-arm-kernel@lists.infradead.org, Kumar Gala,
joe, Andrew Morton, Sören Brinkmann
On 2 December 2015 at 22:56, Moritz Fischer <moritz.fischer@ettus.com> wrote:
> Hi Jassi,
>
> thanks for your feedback.
>
> On Mon, Aug 10, 2015 at 1:05 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>> On Wed, Jul 15, 2015 at 6:30 AM, Moritz Fischer
>> <moritz.fischer@ettus.com> wrote:
>>
>>> +
>>> +static void xilinx_mbox_rx_data(struct mbox_chan *chan)
>>> +{
>>> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
>>> + u32 data;
>>> +
>>> + if (xilinx_mbox_pending(mbox)) {
>>> + data = readl_relaxed(mbox->mbox_base + MAILBOX_REG_RDDATA);
>>> + mbox_chan_received_data(chan, (void *)data);
>>>
>> If RDDATA is a FIFO, then above seems wrong - you are assuming
>> messages are always going to be exactly 32-bits for every protocol.
>> Ideally you should empty the fifo fully, at least when RX has an
>> interrupt.
>
> From my understanding it's hard to tell how much actually is in the fifo,
> you can tell if it's full for send direction, or empty for read direction.
>
Simply read the whole FIFO and leave it to the client driver to parse
that data according to the protocol it drives.
> Maybe the STI / RTI can be setup in a smart way to work with multiple message
> sizes.
>>
>>> +
>>> +static int xilinx_mbox_irq_send_data(struct mbox_chan *chan, void *data)
>>> +{
>>> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
>>> + u32 *udata = data;
>>> +
>>> + if (xilinx_mbox_full(mbox))
>>> + return -EBUSY;
>>>
>> This check is redundant. last_tx_done already makes sure this is always true.
>
> Good to know. I'll fix it.
>>
>>> + /* enable interrupt before send */
>>> + xilinx_mbox_tx_intmask(mbox, true);
>>> +
>>> + writel_relaxed(*udata, mbox->mbox_base + MAILBOX_REG_WRDATA);
>>> +
>> From status EMPTY and FULL, it seems WRDATA is a FIFO. So here also
>> you should expect messages to be <= fifo depth. And not assume exactly
>> 32bit messages always.
>
> How do I determine the message size?
>
Always expect any write request is exactly the size of FIFO depth.
> Doesn't
> drivers/mailbox/bcm2835-mailbox.c or
>
Well I did point it out
http://lists.infradead.org/pipermail/linux-rpi-kernel/2015-June/001902.html
... but developer assumes there will _never_ be need to pass a
message bigger than 32-bits. Sadly overlooking the possibility that
some protocol might have simple, say, 64-bits wide commands and
responses that could avoid using any Shared-Memory at all.
> mailbox-altera.c make the same assumption?
>
There are 2 registers, for CMD and PRT each, that make up 1 message.
It doesn't seem like a fifo.
>>
>>> +
>>> +static bool xilinx_mbox_last_tx_done(struct mbox_chan *chan)
>>> +{
>>> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
>>> +
>>> + /* return false if mailbox is full */
>>> + return !xilinx_mbox_full(mbox);
>>>
>> Instead of FULL, I think it should check for stricter EMPTY status ...
>> mbox api submits only 1 message at a time.
>
> The EMPTY status applies to the receive direction only, I could check
> the STI status
> bit though I suppose.
>
I don't know the h/w but you get my idea. So whatever is in the next revision.
> [...]
>>> +
>>> + mbox->irq = platform_get_irq(pdev, 0);
>>> + /* if irq is present, we can use it, otherwise, poll */
>>> + if (mbox->irq > 0) {
>>> + mbox->controller.txdone_irq = true;
>>> + mbox->controller.ops = &xilinx_mbox_irq_ops;
>>> + } else {
>>> + dev_info(&pdev->dev, "IRQ not found, fallback to polling.\n");
>>> + mbox->controller.txdone_poll = true;
>>> + mbox->controller.txpoll_period = MBOX_POLLING_MS;
>>> + mbox->controller.ops = &xilinx_mbox_poll_ops;
>>> +
>>> + setup_timer(&mbox->rxpoll_timer, xilinx_mbox_poll_rx,
>>> + (unsigned long)&mbox->chan);
>>>
>> I believe there is indeed some platform that doesn't have RX-Interrupt?
>> If no, please remove this.
>> If yes, you may want to get some hint from platform about the size of
>> messages and do mbox_chan_received_data() only upon reading those many
>> bytes.
>
> I'd be fine to drop the polling use case for the moment, on my
> platform I can wire up the IRQ.
> We can always add it back in in a follow up use case.
>
Thanks.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCHv7 2/2] mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
@ 2015-12-03 5:05 ` Jassi Brar
0 siblings, 0 replies; 21+ messages in thread
From: Jassi Brar @ 2015-12-03 5:05 UTC (permalink / raw)
To: Moritz Fischer
Cc: Jassi Brar, Mark Rutland, Devicetree List, Arnd Bergmann,
Pawel Moll,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
Greg Kroah-Hartman, mchehab-JPH+aEBZ4P+UEJcrhfAQsw,
Linux Kernel Mailing List, Michal Simek, Jingoo Han, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Kumar Gala, joe-6d6DIl74uiNBDgjK7y7TUQ, Andrew Morton,
Sören Brinkmann
On 2 December 2015 at 22:56, Moritz Fischer <moritz.fischer-+aYTwkv1SeIAvxtiuMwx3w@public.gmane.org> wrote:
> Hi Jassi,
>
> thanks for your feedback.
>
> On Mon, Aug 10, 2015 at 1:05 AM, Jassi Brar <jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Wed, Jul 15, 2015 at 6:30 AM, Moritz Fischer
>> <moritz.fischer-+aYTwkv1SeIAvxtiuMwx3w@public.gmane.org> wrote:
>>
>>> +
>>> +static void xilinx_mbox_rx_data(struct mbox_chan *chan)
>>> +{
>>> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
>>> + u32 data;
>>> +
>>> + if (xilinx_mbox_pending(mbox)) {
>>> + data = readl_relaxed(mbox->mbox_base + MAILBOX_REG_RDDATA);
>>> + mbox_chan_received_data(chan, (void *)data);
>>>
>> If RDDATA is a FIFO, then above seems wrong - you are assuming
>> messages are always going to be exactly 32-bits for every protocol.
>> Ideally you should empty the fifo fully, at least when RX has an
>> interrupt.
>
> From my understanding it's hard to tell how much actually is in the fifo,
> you can tell if it's full for send direction, or empty for read direction.
>
Simply read the whole FIFO and leave it to the client driver to parse
that data according to the protocol it drives.
> Maybe the STI / RTI can be setup in a smart way to work with multiple message
> sizes.
>>
>>> +
>>> +static int xilinx_mbox_irq_send_data(struct mbox_chan *chan, void *data)
>>> +{
>>> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
>>> + u32 *udata = data;
>>> +
>>> + if (xilinx_mbox_full(mbox))
>>> + return -EBUSY;
>>>
>> This check is redundant. last_tx_done already makes sure this is always true.
>
> Good to know. I'll fix it.
>>
>>> + /* enable interrupt before send */
>>> + xilinx_mbox_tx_intmask(mbox, true);
>>> +
>>> + writel_relaxed(*udata, mbox->mbox_base + MAILBOX_REG_WRDATA);
>>> +
>> From status EMPTY and FULL, it seems WRDATA is a FIFO. So here also
>> you should expect messages to be <= fifo depth. And not assume exactly
>> 32bit messages always.
>
> How do I determine the message size?
>
Always expect any write request is exactly the size of FIFO depth.
> Doesn't
> drivers/mailbox/bcm2835-mailbox.c or
>
Well I did point it out
http://lists.infradead.org/pipermail/linux-rpi-kernel/2015-June/001902.html
... but developer assumes there will _never_ be need to pass a
message bigger than 32-bits. Sadly overlooking the possibility that
some protocol might have simple, say, 64-bits wide commands and
responses that could avoid using any Shared-Memory at all.
> mailbox-altera.c make the same assumption?
>
There are 2 registers, for CMD and PRT each, that make up 1 message.
It doesn't seem like a fifo.
>>
>>> +
>>> +static bool xilinx_mbox_last_tx_done(struct mbox_chan *chan)
>>> +{
>>> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
>>> +
>>> + /* return false if mailbox is full */
>>> + return !xilinx_mbox_full(mbox);
>>>
>> Instead of FULL, I think it should check for stricter EMPTY status ...
>> mbox api submits only 1 message at a time.
>
> The EMPTY status applies to the receive direction only, I could check
> the STI status
> bit though I suppose.
>
I don't know the h/w but you get my idea. So whatever is in the next revision.
> [...]
>>> +
>>> + mbox->irq = platform_get_irq(pdev, 0);
>>> + /* if irq is present, we can use it, otherwise, poll */
>>> + if (mbox->irq > 0) {
>>> + mbox->controller.txdone_irq = true;
>>> + mbox->controller.ops = &xilinx_mbox_irq_ops;
>>> + } else {
>>> + dev_info(&pdev->dev, "IRQ not found, fallback to polling.\n");
>>> + mbox->controller.txdone_poll = true;
>>> + mbox->controller.txpoll_period = MBOX_POLLING_MS;
>>> + mbox->controller.ops = &xilinx_mbox_poll_ops;
>>> +
>>> + setup_timer(&mbox->rxpoll_timer, xilinx_mbox_poll_rx,
>>> + (unsigned long)&mbox->chan);
>>>
>> I believe there is indeed some platform that doesn't have RX-Interrupt?
>> If no, please remove this.
>> If yes, you may want to get some hint from platform about the size of
>> messages and do mbox_chan_received_data() only upon reading those many
>> bytes.
>
> I'd be fine to drop the polling use case for the moment, on my
> platform I can wire up the IRQ.
> We can always add it back in in a follow up use case.
>
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCHv7 2/2] mailbox: Adding driver for Xilinx LogiCORE IP mailbox.
@ 2015-12-03 5:05 ` Jassi Brar
0 siblings, 0 replies; 21+ messages in thread
From: Jassi Brar @ 2015-12-03 5:05 UTC (permalink / raw)
To: linux-arm-kernel
On 2 December 2015 at 22:56, Moritz Fischer <moritz.fischer@ettus.com> wrote:
> Hi Jassi,
>
> thanks for your feedback.
>
> On Mon, Aug 10, 2015 at 1:05 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>> On Wed, Jul 15, 2015 at 6:30 AM, Moritz Fischer
>> <moritz.fischer@ettus.com> wrote:
>>
>>> +
>>> +static void xilinx_mbox_rx_data(struct mbox_chan *chan)
>>> +{
>>> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
>>> + u32 data;
>>> +
>>> + if (xilinx_mbox_pending(mbox)) {
>>> + data = readl_relaxed(mbox->mbox_base + MAILBOX_REG_RDDATA);
>>> + mbox_chan_received_data(chan, (void *)data);
>>>
>> If RDDATA is a FIFO, then above seems wrong - you are assuming
>> messages are always going to be exactly 32-bits for every protocol.
>> Ideally you should empty the fifo fully, at least when RX has an
>> interrupt.
>
> From my understanding it's hard to tell how much actually is in the fifo,
> you can tell if it's full for send direction, or empty for read direction.
>
Simply read the whole FIFO and leave it to the client driver to parse
that data according to the protocol it drives.
> Maybe the STI / RTI can be setup in a smart way to work with multiple message
> sizes.
>>
>>> +
>>> +static int xilinx_mbox_irq_send_data(struct mbox_chan *chan, void *data)
>>> +{
>>> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
>>> + u32 *udata = data;
>>> +
>>> + if (xilinx_mbox_full(mbox))
>>> + return -EBUSY;
>>>
>> This check is redundant. last_tx_done already makes sure this is always true.
>
> Good to know. I'll fix it.
>>
>>> + /* enable interrupt before send */
>>> + xilinx_mbox_tx_intmask(mbox, true);
>>> +
>>> + writel_relaxed(*udata, mbox->mbox_base + MAILBOX_REG_WRDATA);
>>> +
>> From status EMPTY and FULL, it seems WRDATA is a FIFO. So here also
>> you should expect messages to be <= fifo depth. And not assume exactly
>> 32bit messages always.
>
> How do I determine the message size?
>
Always expect any write request is exactly the size of FIFO depth.
> Doesn't
> drivers/mailbox/bcm2835-mailbox.c or
>
Well I did point it out
http://lists.infradead.org/pipermail/linux-rpi-kernel/2015-June/001902.html
... but developer assumes there will _never_ be need to pass a
message bigger than 32-bits. Sadly overlooking the possibility that
some protocol might have simple, say, 64-bits wide commands and
responses that could avoid using any Shared-Memory at all.
> mailbox-altera.c make the same assumption?
>
There are 2 registers, for CMD and PRT each, that make up 1 message.
It doesn't seem like a fifo.
>>
>>> +
>>> +static bool xilinx_mbox_last_tx_done(struct mbox_chan *chan)
>>> +{
>>> + struct xilinx_mbox *mbox = mbox_chan_to_xilinx_mbox(chan);
>>> +
>>> + /* return false if mailbox is full */
>>> + return !xilinx_mbox_full(mbox);
>>>
>> Instead of FULL, I think it should check for stricter EMPTY status ...
>> mbox api submits only 1 message at a time.
>
> The EMPTY status applies to the receive direction only, I could check
> the STI status
> bit though I suppose.
>
I don't know the h/w but you get my idea. So whatever is in the next revision.
> [...]
>>> +
>>> + mbox->irq = platform_get_irq(pdev, 0);
>>> + /* if irq is present, we can use it, otherwise, poll */
>>> + if (mbox->irq > 0) {
>>> + mbox->controller.txdone_irq = true;
>>> + mbox->controller.ops = &xilinx_mbox_irq_ops;
>>> + } else {
>>> + dev_info(&pdev->dev, "IRQ not found, fallback to polling.\n");
>>> + mbox->controller.txdone_poll = true;
>>> + mbox->controller.txpoll_period = MBOX_POLLING_MS;
>>> + mbox->controller.ops = &xilinx_mbox_poll_ops;
>>> +
>>> + setup_timer(&mbox->rxpoll_timer, xilinx_mbox_poll_rx,
>>> + (unsigned long)&mbox->chan);
>>>
>> I believe there is indeed some platform that doesn't have RX-Interrupt?
>> If no, please remove this.
>> If yes, you may want to get some hint from platform about the size of
>> messages and do mbox_chan_received_data() only upon reading those many
>> bytes.
>
> I'd be fine to drop the polling use case for the moment, on my
> platform I can wire up the IRQ.
> We can always add it back in in a follow up use case.
>
Thanks.
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2015-12-03 5:05 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-15 1:00 [PATCHv7 0/2] Adding driver for Xilinx LogiCORE IP mailbox Moritz Fischer
2015-07-15 1:00 ` Moritz Fischer
2015-07-15 1:00 ` Moritz Fischer
2015-07-15 1:00 ` [PATCHv7 1/2] dts: Adding docs for Xilinx LogiCORE IP mailbox driver Moritz Fischer
2015-07-15 1:00 ` Moritz Fischer
2015-07-15 1:23 ` Sören Brinkmann
2015-07-15 1:23 ` Sören Brinkmann
2015-07-15 1:23 ` Sören Brinkmann
2015-07-15 1:00 ` [PATCHv7 2/2] mailbox: Adding driver for Xilinx LogiCORE IP mailbox Moritz Fischer
2015-07-15 1:00 ` Moritz Fischer
2015-08-10 8:05 ` Jassi Brar
2015-08-10 8:05 ` Jassi Brar
2015-08-10 8:05 ` Jassi Brar
2015-12-02 17:26 ` Moritz Fischer
2015-12-02 17:26 ` Moritz Fischer
2015-12-03 5:05 ` Jassi Brar
2015-12-03 5:05 ` Jassi Brar
2015-12-03 5:05 ` Jassi Brar
2015-07-28 22:44 ` [PATCHv7 0/2] " Moritz Fischer
2015-07-28 22:44 ` Moritz Fischer
2015-07-28 22:44 ` Moritz Fischer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.