| Message ID | 1437990272-23111-6-git-send-email-lee.jones@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Mon, 10 Aug 2015, Jassi Brar wrote: > On Mon, Jul 27, 2015 at 3:14 PM, Lee Jones <lee.jones@linaro.org> wrote: > > This particular Client implementation uses shared memory in order > > to pass messages between Mailbox users; however, it can be easily > > hacked to support any type of Controller. > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > --- > > drivers/mailbox/Kconfig | 7 ++ > > drivers/mailbox/Makefile | 2 + > > drivers/mailbox/mailbox-test.c | 210 +++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 219 insertions(+) > > create mode 100644 drivers/mailbox/mailbox-test.c > > > > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig > > index 2cc4c39..7720bde 100644 > > --- a/drivers/mailbox/Kconfig > > +++ b/drivers/mailbox/Kconfig > > @@ -77,4 +77,11 @@ config STI_MBOX > > Mailbox implementation for STMicroelectonics family chips with > > hardware for interprocessor communication. > > > > +config MAILBOX_TEST > > + tristate "Mailbox Test Client" > > + depends on OF > > + help > > + Test client to help with testing new Controller driver > > + implementations. > > + > > endif > > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile > > index 7cb4766..92435ef 100644 > > --- a/drivers/mailbox/Makefile > > +++ b/drivers/mailbox/Makefile > > @@ -2,6 +2,8 @@ > > > > obj-$(CONFIG_MAILBOX) += mailbox.o > > > > +obj-$(CONFIG_MAILBOX_TEST) += mailbox-test.o > > + > > obj-$(CONFIG_ARM_MHU) += arm_mhu.o > > > > obj-$(CONFIG_PL320_MBOX) += pl320-ipc.o > > diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c > > new file mode 100644 > > index 0000000..10bfe3a > > --- /dev/null > > +++ b/drivers/mailbox/mailbox-test.c > > @@ -0,0 +1,210 @@ > > +/* > > + * Copyright (C) 2015 ST Microelectronics > > + * > > + * Author: Lee Jones <lee.jones@linaro.org> > > + * > > + * 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; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > + > > +#include <linux/debugfs.h> > > +#include <linux/err.h> > > +#include <linux/kernel.h> > > +#include <linux/mailbox_client.h> > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/platform_device.h> > > +#include <linux/slab.h> > > +#include <linux/uaccess.h> > > + > > +#define MBOX_MAX_MSG_LEN 128 > > + > > +static struct dentry *root_debugfs_dir; > > + > > +struct mbox_test_device { > > + struct device *dev; > > + struct mbox_chan *tx_channel; > > + struct mbox_chan *rx_channel; > > + void __iomem *mmio; > > + > > +}; > > + > > +static ssize_t mbox_test_write(struct file *filp, > > + const char __user *userbuf, > > + size_t count, loff_t *ppos) > > +{ > > + struct mbox_test_device *tdev = filp->private_data; > > + char *message; > > + int ret; > > + > > + if (count > MBOX_MAX_MSG_LEN) { > > + dev_err(tdev->dev, > > + "Message length %d greater than max allowed %d\n", > > + count, MBOX_MAX_MSG_LEN); > > + return -EINVAL; > > + } > > + > > + message = kzalloc(count, GFP_KERNEL); > > + if (!message) > > + return -ENOMEM; > > + > > + ret = copy_from_user(message, userbuf, count); > > + if (ret) > > + return -EFAULT; > > + > > + print_hex_dump(KERN_ERR, "Client: Sending: ", DUMP_PREFIX_ADDRESS, > > + 16, 1, message, 16, true); > > + > > + ret = mbox_send_message(tdev->tx_channel, message); > > + if (ret < 0) > > + dev_err(tdev->dev, "Failed to send message via mailbox\n"); > > + > > + kfree(message); > > + > > + return ret < 0 ? ret : count; > > +} > > + > > +static const struct file_operations mbox_test_ops = { > > + .write = mbox_test_write, > > + .open = simple_open, > > + .llseek = generic_file_llseek, > > +}; > > + > > +static int mbox_test_add_debugfs(struct platform_device *pdev, > > + struct mbox_test_device *tdev) > > +{ > > + if (!debugfs_initialized()) > > + return 0; > > + > > + root_debugfs_dir = debugfs_create_dir("mailbox", NULL); > > + if (!root_debugfs_dir) { > > + dev_err(&pdev->dev, "Failed to create Mailbox debugfs\n"); > > + return -EINVAL; > > + } > > + > > + debugfs_create_file("send-message", 0200, root_debugfs_dir, > > + tdev, &mbox_test_ops); > > + > > + return 0; > > +} > > + > > +static void mbox_test_receive_message(struct mbox_client *client, void *message) > > +{ > > + struct mbox_test_device *tdev = dev_get_drvdata(client->dev); > > + > > + if (!tdev->mmio) { > > + dev_info(tdev->dev, "Client: Recived something [read mmio]\n"); > > + return; > > + } > > + > > + print_hex_dump(KERN_ERR, "Client: from co-proc: ", DUMP_PREFIX_ADDRESS, > > + 16, 1, tdev->mmio, MBOX_MAX_MSG_LEN, true); > > +} > > + > > +static void mbox_test_prepare_message(struct mbox_client *client, void *message) > > +{ > > + struct mbox_test_device *tdev = dev_get_drvdata(client->dev); > > + > > + if (tdev->mmio) > > + memcpy(tdev->mmio, message, MBOX_MAX_MSG_LEN); > > > This is unlikely to work. Those platforms that need to send a 2 part > message, they do : > (a) Signal/Command/Target code via some controller register (via > mbox_send_message). > (b) Setup the payload in Shared-Memory (via tx_prepare). > > This test driver assumes both are the same. I think you have to > declare something like This driver assumes that the framework will call client->tx_prepare() first, which satisfies (b). It then assumes controller->send_data() will be invoked, which will send the platform specific signal/command/target code, which then satisfies (a). In what way does it assume they are the same? > struct mbox_test_message { /* same for TX and RX */ > unsigned sig_len; > void *signal; /* rx/tx via mailbox api */ > unsigned pl_len; > void *payload; /* rx/tx via shared-memory */ > }; How do you think this should be set and use these? > > + > > +static void mbox_test_message_sent(struct mbox_client *client, > > + void *message, int r) > > +{ > > + if (r) > > + dev_warn(client->dev, > > + "Client: Message could not be sent: %d\n", r); > > + else > > + dev_info(client->dev, > > + "Client: Message sent\n"); > > +} > > + > > +static struct mbox_chan * > > +mbox_test_request_channel(struct platform_device *pdev, const char *name) > > +{ > > + struct mbox_client *client; > > + > > + client = devm_kzalloc(&pdev->dev, sizeof(*client), GFP_KERNEL); > > + if (!client) > > + return ERR_PTR(-ENOMEM); > > + > > + client->dev = &pdev->dev; > > + client->rx_callback = mbox_test_receive_message; > > + client->tx_prepare = mbox_test_prepare_message; > > + client->tx_done = mbox_test_message_sent; > > + client->tx_block = true; > > + client->knows_txdone = false; > > + client->tx_tout = 500; > > + > > + return mbox_request_channel_byname(client, name); > > +} > > + > > +static int mbox_test_probe(struct platform_device *pdev) > > +{ > > + struct mbox_test_device *tdev; > > + struct resource *res; > > + int ret; > > + > > + tdev = devm_kzalloc(&pdev->dev, sizeof(*tdev), GFP_KERNEL); > > + if (!tdev) > > + return -ENOMEM; > > + > > + /* It's okay for MMIO to be NULL */ > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + tdev->mmio = devm_ioremap_resource(&pdev->dev, res); > > + if (IS_ERR(tdev->mmio)) > > + tdev->mmio = NULL; > > + > > + tdev->tx_channel = mbox_test_request_channel(pdev, "tx"); > > + if (IS_ERR(tdev->tx_channel)) > > + return PTR_ERR(tdev->tx_channel); > > + > > + tdev->rx_channel = mbox_test_request_channel(pdev, "rx"); > > + if (IS_ERR(tdev->rx_channel)) > > + return PTR_ERR(tdev->rx_channel); > > + > Should it really fail on TX or RX only clients? Good question. Probably not, but I guess we'd need a flag for that. > It takes write from userspace but shouldn't it also provide data > received to the userspace? Currently we only print the returning message. If you want me to put it in a userspace file too, that's not an issue.
On Thu, 13 Aug 2015, Jassi Brar wrote: > On Wed, Aug 12, 2015 at 3:53 PM, Lee Jones <lee.jones@linaro.org> wrote: > > On Mon, 10 Aug 2015, Jassi Brar wrote: > > > >> On Mon, Jul 27, 2015 at 3:14 PM, Lee Jones <lee.jones@linaro.org> wrote: > >> > This particular Client implementation uses shared memory in order > >> > to pass messages between Mailbox users; however, it can be easily > >> > hacked to support any type of Controller. > >> > > >> > Signed-off-by: Lee Jones <lee.jones@linaro.org> > >> > --- > >> > drivers/mailbox/Kconfig | 7 ++ > >> > drivers/mailbox/Makefile | 2 + > >> > drivers/mailbox/mailbox-test.c | 210 +++++++++++++++++++++++++++++++++++++++++ > >> > 3 files changed, 219 insertions(+) > >> > create mode 100644 drivers/mailbox/mailbox-test.c > >> > > >> > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig > >> > index 2cc4c39..7720bde 100644 > >> > --- a/drivers/mailbox/Kconfig > >> > +++ b/drivers/mailbox/Kconfig > >> > @@ -77,4 +77,11 @@ config STI_MBOX > >> > Mailbox implementation for STMicroelectonics family chips with > >> > hardware for interprocessor communication. > >> > > >> > +config MAILBOX_TEST > >> > + tristate "Mailbox Test Client" > >> > + depends on OF > >> > + help > >> > + Test client to help with testing new Controller driver > >> > + implementations. > >> > + > >> > endif > >> > diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile > >> > index 7cb4766..92435ef 100644 > >> > --- a/drivers/mailbox/Makefile > >> > +++ b/drivers/mailbox/Makefile > >> > @@ -2,6 +2,8 @@ > >> > > >> > obj-$(CONFIG_MAILBOX) += mailbox.o > >> > > >> > +obj-$(CONFIG_MAILBOX_TEST) += mailbox-test.o > >> > + > >> > obj-$(CONFIG_ARM_MHU) += arm_mhu.o > >> > > >> > obj-$(CONFIG_PL320_MBOX) += pl320-ipc.o > >> > diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c > >> > new file mode 100644 > >> > index 0000000..10bfe3a > >> > --- /dev/null > >> > +++ b/drivers/mailbox/mailbox-test.c > >> > @@ -0,0 +1,210 @@ > >> > +/* > >> > + * Copyright (C) 2015 ST Microelectronics > >> > + * > >> > + * Author: Lee Jones <lee.jones@linaro.org> > >> > + * > >> > + * 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; either version 2 of the License, or > >> > + * (at your option) any later version. > >> > + */ > >> > + > >> > +#include <linux/debugfs.h> > >> > +#include <linux/err.h> > >> > +#include <linux/kernel.h> > >> > +#include <linux/mailbox_client.h> > >> > +#include <linux/module.h> > >> > +#include <linux/of.h> > >> > +#include <linux/platform_device.h> > >> > +#include <linux/slab.h> > >> > +#include <linux/uaccess.h> > >> > + > >> > +#define MBOX_MAX_MSG_LEN 128 > >> > + > >> > +static struct dentry *root_debugfs_dir; > >> > + > >> > +struct mbox_test_device { > >> > + struct device *dev; > >> > + struct mbox_chan *tx_channel; > >> > + struct mbox_chan *rx_channel; > >> > + void __iomem *mmio; > >> > + > >> > +}; > >> > + > >> > +static ssize_t mbox_test_write(struct file *filp, > >> > + const char __user *userbuf, > >> > + size_t count, loff_t *ppos) > >> > +{ > >> > + struct mbox_test_device *tdev = filp->private_data; > >> > + char *message; > >> > + int ret; > >> > + > >> > + if (count > MBOX_MAX_MSG_LEN) { > >> > + dev_err(tdev->dev, > >> > + "Message length %d greater than max allowed %d\n", > >> > + count, MBOX_MAX_MSG_LEN); > >> > + return -EINVAL; > >> > + } > >> > + > >> > + message = kzalloc(count, GFP_KERNEL); > >> > + if (!message) > >> > + return -ENOMEM; > >> > + > >> > + ret = copy_from_user(message, userbuf, count); > >> > + if (ret) > >> > + return -EFAULT; > >> > + > >> > + print_hex_dump(KERN_ERR, "Client: Sending: ", DUMP_PREFIX_ADDRESS, > >> > + 16, 1, message, 16, true); > >> > + > >> > + ret = mbox_send_message(tdev->tx_channel, message); > >> > + if (ret < 0) > >> > + dev_err(tdev->dev, "Failed to send message via mailbox\n"); > >> > + > >> > + kfree(message); > >> > + > >> > + return ret < 0 ? ret : count; > >> > +} > >> > + > >> > +static const struct file_operations mbox_test_ops = { > >> > + .write = mbox_test_write, > >> > + .open = simple_open, > >> > + .llseek = generic_file_llseek, > >> > +}; > >> > + > >> > +static int mbox_test_add_debugfs(struct platform_device *pdev, > >> > + struct mbox_test_device *tdev) > >> > +{ > >> > + if (!debugfs_initialized()) > >> > + return 0; > >> > + > >> > + root_debugfs_dir = debugfs_create_dir("mailbox", NULL); > >> > + if (!root_debugfs_dir) { > >> > + dev_err(&pdev->dev, "Failed to create Mailbox debugfs\n"); > >> > + return -EINVAL; > >> > + } > >> > + > >> > + debugfs_create_file("send-message", 0200, root_debugfs_dir, > >> > + tdev, &mbox_test_ops); > >> > + > >> > + return 0; > >> > +} > >> > + > >> > +static void mbox_test_receive_message(struct mbox_client *client, void *message) > >> > +{ > >> > + struct mbox_test_device *tdev = dev_get_drvdata(client->dev); > >> > + > >> > + if (!tdev->mmio) { > >> > + dev_info(tdev->dev, "Client: Recived something [read mmio]\n"); > >> > + return; > >> > + } > >> > + > >> > + print_hex_dump(KERN_ERR, "Client: from co-proc: ", DUMP_PREFIX_ADDRESS, > >> > + 16, 1, tdev->mmio, MBOX_MAX_MSG_LEN, true); > >> > +} > >> > + > >> > +static void mbox_test_prepare_message(struct mbox_client *client, void *message) > >> > +{ > >> > + struct mbox_test_device *tdev = dev_get_drvdata(client->dev); > >> > + > >> > + if (tdev->mmio) > >> > + memcpy(tdev->mmio, message, MBOX_MAX_MSG_LEN); > >> > > >> This is unlikely to work. Those platforms that need to send a 2 part > >> message, they do : > >> (a) Signal/Command/Target code via some controller register (via > >> mbox_send_message). > >> (b) Setup the payload in Shared-Memory (via tx_prepare). > >> > >> This test driver assumes both are the same. I think you have to > >> declare something like > > > > This driver assumes that the framework will call client->tx_prepare() > > first, which satisfies (b). It then assumes controller->send_data() > > will be invoked, which will send the platform specific > > signal/command/target code, which then satisfies (a). > > > Yeah, but what would be that code? Who provides that? > > > In what way does it assume they are the same? > > > notice the 'message' pointer in > mbox_send_message(tdev->tx_channel, message); > and > memcpy(tdev->mmio, message, MBOX_MAX_MSG_LEN); > > >> struct mbox_test_message { /* same for TX and RX */ > >> unsigned sig_len; > >> void *signal; /* rx/tx via mailbox api */ > >> unsigned pl_len; > >> void *payload; /* rx/tx via shared-memory */ > >> }; > > > > How do you think this should be set and use these? > > > The userspace would want to specify the command code (32bits or not) > that would be passed via the fifo/register of the controller. So we > need signal[] > > The data to be passed via share-memory could be provided by userspace > via the payload[] array. Okay, so would the solution be two userspace files 'signal' and 'message', so in the case of a client specified signal we can write it into there first. echo 255 > signal echo test > message ... or in the case where no signal is required, or the controller driver taking care of it, we just don't write anything to signal? Just for clarification, in the case where a signal is required, do clients usually pass that through mbox_send_message() too? mbox_send_message(tdev->tx_channel, signal); mbox_send_message(tdev->tx_channel, message); ... or vice versa. > >> > +static void mbox_test_message_sent(struct mbox_client *client, > >> > + void *message, int r) > >> > +{ > >> > + if (r) > >> > + dev_warn(client->dev, > >> > + "Client: Message could not be sent: %d\n", r); > >> > + else > >> > + dev_info(client->dev, > >> > + "Client: Message sent\n"); > >> > +} > >> > + > >> > +static struct mbox_chan * > >> > +mbox_test_request_channel(struct platform_device *pdev, const char *name) > >> > +{ > >> > + struct mbox_client *client; > >> > + > >> > + client = devm_kzalloc(&pdev->dev, sizeof(*client), GFP_KERNEL); > >> > + if (!client) > >> > + return ERR_PTR(-ENOMEM); > >> > + > >> > + client->dev = &pdev->dev; > >> > + client->rx_callback = mbox_test_receive_message; > >> > + client->tx_prepare = mbox_test_prepare_message; > >> > + client->tx_done = mbox_test_message_sent; > >> > + client->tx_block = true; > >> > + client->knows_txdone = false; > >> > + client->tx_tout = 500; > >> > + > >> > + return mbox_request_channel_byname(client, name); > >> > +} > >> > + > >> > +static int mbox_test_probe(struct platform_device *pdev) > >> > +{ > >> > + struct mbox_test_device *tdev; > >> > + struct resource *res; > >> > + int ret; > >> > + > >> > + tdev = devm_kzalloc(&pdev->dev, sizeof(*tdev), GFP_KERNEL); > >> > + if (!tdev) > >> > + return -ENOMEM; > >> > + > >> > + /* It's okay for MMIO to be NULL */ > >> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > >> > + tdev->mmio = devm_ioremap_resource(&pdev->dev, res); > >> > + if (IS_ERR(tdev->mmio)) > >> > + tdev->mmio = NULL; > >> > + > >> > + tdev->tx_channel = mbox_test_request_channel(pdev, "tx"); > >> > + if (IS_ERR(tdev->tx_channel)) > >> > + return PTR_ERR(tdev->tx_channel); > >> > + > >> > + tdev->rx_channel = mbox_test_request_channel(pdev, "rx"); > >> > + if (IS_ERR(tdev->rx_channel)) > >> > + return PTR_ERR(tdev->rx_channel); > >> > + > >> Should it really fail on TX or RX only clients? > > > > Good question. Probably not, but I guess we'd need a flag for that. > > > Just assume 1 channel. You can't make it receive if the controller > can't do rx, so the userspace will simply block on read requests. And > you can't make the controller send, if it can't. So any write attempt > will simply return with an error. > And we are going to need very platform specific knowledge to execute > the tests. It's not like some loop enumerating all possible > combinations of parameters that the api must pass... so we are good. I'm guessing most tests will be some kind of send-reply test, so we'd need two channels for that. I can make it so the driver does not fail if 'tx' or 'rx' is missing, but will fail if both are. Bare in mind that this test driver isn't designed to cover all of the corner cases, but more designed as a helpful boiler plate where it will tick some use-case boxes, but is clear enough to be hacked around by other vendors who have different requirements. With regards to you comments on userspace; I suggest that whoever is testing the controller will know it and will not attempt to check for a reply from a controller (or co-proc) which is write-only. > >> It takes write from userspace but shouldn't it also provide data > >> received to the userspace? > > > > Currently we only print the returning message. If you want me to put > > it in a userspace file too, that's not an issue. > > > Ideally we should support 'read' just like write. Sure, I can do that.
On Thu, 13 Aug 2015, Jassi Brar wrote: > On Thu, Aug 13, 2015 at 2:49 PM, Lee Jones <lee.jones@linaro.org> wrote: > > On Thu, 13 Aug 2015, Jassi Brar wrote: > > >> >> > + > >> >> > +static void mbox_test_prepare_message(struct mbox_client *client, void *message) > >> >> > +{ > >> >> > + struct mbox_test_device *tdev = dev_get_drvdata(client->dev); > >> >> > + > >> >> > + if (tdev->mmio) > >> >> > + memcpy(tdev->mmio, message, MBOX_MAX_MSG_LEN); > >> >> > > >> >> This is unlikely to work. Those platforms that need to send a 2 part > >> >> message, they do : > >> >> (a) Signal/Command/Target code via some controller register (via > >> >> mbox_send_message). > >> >> (b) Setup the payload in Shared-Memory (via tx_prepare). > >> >> > >> >> This test driver assumes both are the same. I think you have to > >> >> declare something like > >> > > >> > This driver assumes that the framework will call client->tx_prepare() > >> > first, which satisfies (b). It then assumes controller->send_data() > >> > will be invoked, which will send the platform specific > >> > signal/command/target code, which then satisfies (a). > >> > > >> Yeah, but what would be that code? Who provides that? > >> > >> > In what way does it assume they are the same? > >> > > >> notice the 'message' pointer in > >> mbox_send_message(tdev->tx_channel, message); > >> and > >> memcpy(tdev->mmio, message, MBOX_MAX_MSG_LEN); > >> > >> >> struct mbox_test_message { /* same for TX and RX */ > >> >> unsigned sig_len; > >> >> void *signal; /* rx/tx via mailbox api */ > >> >> unsigned pl_len; > >> >> void *payload; /* rx/tx via shared-memory */ > >> >> }; > >> > > >> > How do you think this should be set and use these? > >> > > >> The userspace would want to specify the command code (32bits or not) > >> that would be passed via the fifo/register of the controller. So we > >> need signal[] > >> > >> The data to be passed via share-memory could be provided by userspace > >> via the payload[] array. > > > > Okay, so would the solution be two userspace files 'signal' and > > 'message', so in the case of a client specified signal we can write it > > into there first. > > > > echo 255 > signal > > echo test > message > > > > ... or in the case where no signal is required, or the controller > > driver taking care of it, we just don't write anything to signal? > > > file_operations.write() should parse signal and message, coming from > userspace, into a local structure before routing them via > mbox_send_message and tx_prepare respectively. Okay. So before I code this up we should agree on syntax. How would you like to represent the break between signal and message? Obviously ' ' would be a bad idea, as some clients may want to send messages with white space contained. How about '\t' or '\n'? > > Just for clarification, in the case where a signal is required, do > > clients usually pass that through mbox_send_message() too? > > > > mbox_send_message(tdev->tx_channel, signal); > > mbox_send_message(tdev->tx_channel, message); > > > No. Command/Signal code is passed via mbox_send_message(signal). The > payload is passed via Shared-Memory during tx_prepare(message) Okay, I see. Thanks for the clarification. > >> >> > +static void mbox_test_message_sent(struct mbox_client *client, > >> >> > + void *message, int r) > >> >> > +{ > >> >> > + if (r) > >> >> > + dev_warn(client->dev, > >> >> > + "Client: Message could not be sent: %d\n", r); > >> >> > + else > >> >> > + dev_info(client->dev, > >> >> > + "Client: Message sent\n"); > >> >> > +} > >> >> > + > >> >> > +static struct mbox_chan * > >> >> > +mbox_test_request_channel(struct platform_device *pdev, const char *name) > >> >> > +{ > >> >> > + struct mbox_client *client; > >> >> > + > >> >> > + client = devm_kzalloc(&pdev->dev, sizeof(*client), GFP_KERNEL); > >> >> > + if (!client) > >> >> > + return ERR_PTR(-ENOMEM); > >> >> > + > >> >> > + client->dev = &pdev->dev; > >> >> > + client->rx_callback = mbox_test_receive_message; > >> >> > + client->tx_prepare = mbox_test_prepare_message; > >> >> > + client->tx_done = mbox_test_message_sent; > >> >> > + client->tx_block = true; > >> >> > + client->knows_txdone = false; > >> >> > + client->tx_tout = 500; > >> >> > + > >> >> > + return mbox_request_channel_byname(client, name); > >> >> > +} > >> >> > + > >> >> > +static int mbox_test_probe(struct platform_device *pdev) > >> >> > +{ > >> >> > + struct mbox_test_device *tdev; > >> >> > + struct resource *res; > >> >> > + int ret; > >> >> > + > >> >> > + tdev = devm_kzalloc(&pdev->dev, sizeof(*tdev), GFP_KERNEL); > >> >> > + if (!tdev) > >> >> > + return -ENOMEM; > >> >> > + > >> >> > + /* It's okay for MMIO to be NULL */ > >> >> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > >> >> > + tdev->mmio = devm_ioremap_resource(&pdev->dev, res); > >> >> > + if (IS_ERR(tdev->mmio)) > >> >> > + tdev->mmio = NULL; > >> >> > + > >> >> > + tdev->tx_channel = mbox_test_request_channel(pdev, "tx"); > >> >> > + if (IS_ERR(tdev->tx_channel)) > >> >> > + return PTR_ERR(tdev->tx_channel); > >> >> > + > >> >> > + tdev->rx_channel = mbox_test_request_channel(pdev, "rx"); > >> >> > + if (IS_ERR(tdev->rx_channel)) > >> >> > + return PTR_ERR(tdev->rx_channel); > >> >> > + > >> >> Should it really fail on TX or RX only clients? > >> > > >> > Good question. Probably not, but I guess we'd need a flag for that. > >> > > >> Just assume 1 channel. You can't make it receive if the controller > >> can't do rx, so the userspace will simply block on read requests. And > >> you can't make the controller send, if it can't. So any write attempt > >> will simply return with an error. > >> And we are going to need very platform specific knowledge to execute > >> the tests. It's not like some loop enumerating all possible > >> combinations of parameters that the api must pass... so we are good. > > > > I'm guessing most tests will be some kind of send-reply test, so we'd > > need two channels for that. I can make it so the driver does not fail > > if 'tx' or 'rx' is missing, but will fail if both are. > > > I have seen more channels to be rx+tx, than rx/tx only. And for latter > case the user should run 2 test threads, one each for RX and TX. Okay, makes sense to me. > > Bare in mind that this test driver isn't designed to cover all of the > > corner cases, but more designed as a helpful boiler plate where it > > will tick some use-case boxes, but is clear enough to be hacked around > > by other vendors who have different requirements. > > > > With regards to you comments on userspace; I suggest that whoever is > > testing the controller will know it and will not attempt to check for > > a reply from a controller (or co-proc) which is write-only. > > > That's actually my point :) > When the userspace already has all that platform specific knowledge, > why check for 'valid' usage in controller driver? Simply reject any > request that the controller isn't capable of (which is practically > never supposed to happen). This conversation doesn't really belong in this thread; but seeing as you bought it up ... I absolutely and fundamentally disagree that the controller shouldn't do error checking during mbox_request_channel(). It is mbox_request_channel() that should fail on a known bad configuration, rather than pass back a known rotten apple and wait for that it to be eaten (appreciate I've gone a little overboard with the analogies) before flagging it up. And for what purpose? To make the controller driver a little simpler? I do hope that you do see sense on this one and allow me to keep the error checking in the driver! If you have a reply to this, please do so one the driver thread, so it's easier to follow. Secondly, a reply more akin to this thread. The guys using _this_ driver are more likely to be Controller authors. These are the chaps I was speaking about that are going to know the platform specifics of the controller. The error checking in mbox_request_channel() is designed for 'client' authors, who are more likely to be requesting invalid configurations.
On Thu, 13 Aug 2015, Jassi Brar wrote: > On Thu, Aug 13, 2015 at 3:53 PM, Lee Jones <lee.jones@linaro.org> wrote: > > On Thu, 13 Aug 2015, Jassi Brar wrote: > > > >> On Thu, Aug 13, 2015 at 2:49 PM, Lee Jones <lee.jones@linaro.org> wrote: > >> > On Thu, 13 Aug 2015, Jassi Brar wrote: > >> > >> >> >> > + > >> >> >> > +static void mbox_test_prepare_message(struct mbox_client *client, void *message) > >> >> >> > +{ > >> >> >> > + struct mbox_test_device *tdev = dev_get_drvdata(client->dev); > >> >> >> > + > >> >> >> > + if (tdev->mmio) > >> >> >> > + memcpy(tdev->mmio, message, MBOX_MAX_MSG_LEN); > >> >> >> > > >> >> >> This is unlikely to work. Those platforms that need to send a 2 part > >> >> >> message, they do : > >> >> >> (a) Signal/Command/Target code via some controller register (via > >> >> >> mbox_send_message). > >> >> >> (b) Setup the payload in Shared-Memory (via tx_prepare). > >> >> >> > >> >> >> This test driver assumes both are the same. I think you have to > >> >> >> declare something like > >> >> > > >> >> > This driver assumes that the framework will call client->tx_prepare() > >> >> > first, which satisfies (b). It then assumes controller->send_data() > >> >> > will be invoked, which will send the platform specific > >> >> > signal/command/target code, which then satisfies (a). > >> >> > > >> >> Yeah, but what would be that code? Who provides that? > >> >> > >> >> > In what way does it assume they are the same? > >> >> > > >> >> notice the 'message' pointer in > >> >> mbox_send_message(tdev->tx_channel, message); > >> >> and > >> >> memcpy(tdev->mmio, message, MBOX_MAX_MSG_LEN); > >> >> > >> >> >> struct mbox_test_message { /* same for TX and RX */ > >> >> >> unsigned sig_len; > >> >> >> void *signal; /* rx/tx via mailbox api */ > >> >> >> unsigned pl_len; > >> >> >> void *payload; /* rx/tx via shared-memory */ > >> >> >> }; > >> >> > > >> >> > How do you think this should be set and use these? > >> >> > > >> >> The userspace would want to specify the command code (32bits or not) > >> >> that would be passed via the fifo/register of the controller. So we > >> >> need signal[] > >> >> > >> >> The data to be passed via share-memory could be provided by userspace > >> >> via the payload[] array. > >> > > >> > Okay, so would the solution be two userspace files 'signal' and > >> > 'message', so in the case of a client specified signal we can write it > >> > into there first. > >> > > >> > echo 255 > signal > >> > echo test > message > >> > > >> > ... or in the case where no signal is required, or the controller > >> > driver taking care of it, we just don't write anything to signal? > >> > > >> file_operations.write() should parse signal and message, coming from > >> userspace, into a local structure before routing them via > >> mbox_send_message and tx_prepare respectively. > > > > Okay. So before I code this up we should agree on syntax. > > > > How would you like to represent the break between signal and message? > > Obviously ' ' would be a bad idea, as some clients may want to send > > messages with white space contained. How about '\t' or '\n'? > > > Yeah, I am not a fan of markers and flags either. > > Maybe we should share with userspace > struct mbox_test_message { /* same for TX and RX */ > unsigned sig_len; > void __user *signal; /* rx/tx via mailbox api */ > unsigned pl_len; > void __user *payload; /* rx/tx via shared-memory */ > }; > > First copy_from_user the structure of length sizof(struct > mbox_test_message) and then copy_from_user length sig_len and pl_len > from signal[] and payload[] respectively (usually ioctls would carry > such data). Simplicity and ease of use should be the goals here. Testers should not have to write applications to use this driver. Can we come up with a simple/effective method that uses SYSFS/DEBUGFS please? The easiest way I can think of which avoids markers/separators AND the requirement for users to have to write applications is to have two files, 'signal' and 'message' as mentioned before. Once both are populated I can get this driver to draft the message appropriately and fire it off.
On Thu, 13 Aug 2015, Jassi Brar wrote: > On Thu, Aug 13, 2015 at 4:30 PM, Lee Jones <lee.jones@linaro.org> wrote: > > On Thu, 13 Aug 2015, Jassi Brar wrote: > > > >> On Thu, Aug 13, 2015 at 3:53 PM, Lee Jones <lee.jones@linaro.org> wrote: > >> > On Thu, 13 Aug 2015, Jassi Brar wrote: > >> > > >> >> On Thu, Aug 13, 2015 at 2:49 PM, Lee Jones <lee.jones@linaro.org> wrote: > >> >> > On Thu, 13 Aug 2015, Jassi Brar wrote: > >> >> > >> >> >> >> > + > >> >> >> >> > +static void mbox_test_prepare_message(struct mbox_client *client, void *message) > >> >> >> >> > +{ > >> >> >> >> > + struct mbox_test_device *tdev = dev_get_drvdata(client->dev); > >> >> >> >> > + > >> >> >> >> > + if (tdev->mmio) > >> >> >> >> > + memcpy(tdev->mmio, message, MBOX_MAX_MSG_LEN); > >> >> >> >> > > >> >> >> >> This is unlikely to work. Those platforms that need to send a 2 part > >> >> >> >> message, they do : > >> >> >> >> (a) Signal/Command/Target code via some controller register (via > >> >> >> >> mbox_send_message). > >> >> >> >> (b) Setup the payload in Shared-Memory (via tx_prepare). > >> >> >> >> > >> >> >> >> This test driver assumes both are the same. I think you have to > >> >> >> >> declare something like > >> >> >> > > >> >> >> > This driver assumes that the framework will call client->tx_prepare() > >> >> >> > first, which satisfies (b). It then assumes controller->send_data() > >> >> >> > will be invoked, which will send the platform specific > >> >> >> > signal/command/target code, which then satisfies (a). > >> >> >> > > >> >> >> Yeah, but what would be that code? Who provides that? > >> >> >> > >> >> >> > In what way does it assume they are the same? > >> >> >> > > >> >> >> notice the 'message' pointer in > >> >> >> mbox_send_message(tdev->tx_channel, message); > >> >> >> and > >> >> >> memcpy(tdev->mmio, message, MBOX_MAX_MSG_LEN); > >> >> >> > >> >> >> >> struct mbox_test_message { /* same for TX and RX */ > >> >> >> >> unsigned sig_len; > >> >> >> >> void *signal; /* rx/tx via mailbox api */ > >> >> >> >> unsigned pl_len; > >> >> >> >> void *payload; /* rx/tx via shared-memory */ > >> >> >> >> }; > >> >> >> > > >> >> >> > How do you think this should be set and use these? > >> >> >> > > >> >> >> The userspace would want to specify the command code (32bits or not) > >> >> >> that would be passed via the fifo/register of the controller. So we > >> >> >> need signal[] > >> >> >> > >> >> >> The data to be passed via share-memory could be provided by userspace > >> >> >> via the payload[] array. > >> >> > > >> >> > Okay, so would the solution be two userspace files 'signal' and > >> >> > 'message', so in the case of a client specified signal we can write it > >> >> > into there first. > >> >> > > >> >> > echo 255 > signal > >> >> > echo test > message > >> >> > > >> >> > ... or in the case where no signal is required, or the controller > >> >> > driver taking care of it, we just don't write anything to signal? > >> >> > > >> >> file_operations.write() should parse signal and message, coming from > >> >> userspace, into a local structure before routing them via > >> >> mbox_send_message and tx_prepare respectively. > >> > > >> > Okay. So before I code this up we should agree on syntax. > >> > > >> > How would you like to represent the break between signal and message? > >> > Obviously ' ' would be a bad idea, as some clients may want to send > >> > messages with white space contained. How about '\t' or '\n'? > >> > > >> Yeah, I am not a fan of markers and flags either. > >> > >> Maybe we should share with userspace > >> struct mbox_test_message { /* same for TX and RX */ > >> unsigned sig_len; > >> void __user *signal; /* rx/tx via mailbox api */ > >> unsigned pl_len; > >> void __user *payload; /* rx/tx via shared-memory */ > >> }; > >> > >> First copy_from_user the structure of length sizof(struct > >> mbox_test_message) and then copy_from_user length sig_len and pl_len > >> from signal[] and payload[] respectively (usually ioctls would carry > >> such data). > > > > Simplicity and ease of use should be the goals here. Testers should > > not have to write applications to use this driver. Can we come up > > with a simple/effective method that uses SYSFS/DEBUGFS please? > > > > The easiest way I can think of which avoids markers/separators AND the > > requirement for users to have to write applications is to have two > > files, 'signal' and 'message' as mentioned before. Once both are > > populated I can get this driver to draft the message appropriately and > > fire it off. > > > And then write to more files for RX data? ... which should also be in > the form of 'signal' and 'message'. > > BTW like for spi there is a stock application in > Documentation/spi/spidev_test.c we could do the same? Coming from personal experience, testing drivers with (even stock) applications is much more painful that simply writing/reading (cat/echo) to a file in SYSFS/DEBUGFS. Particularly if people are using initramfs or thelike. If it is possible to use SYSFS/DEBUGFS, which it is in this case, then I believe that's the route we could go down. In answer to your question; we only need those two files. The reply can be placed back into 'message' and can be read from there. Simple to use, simple to code (on both sides), elegant, no overhead. Win, win, win, win.
On Thu, 13 Aug 2015, Jassi Brar wrote: > On Thu, Aug 13, 2015 at 5:10 PM, Lee Jones <lee.jones@linaro.org> wrote: > > On Thu, 13 Aug 2015, Jassi Brar wrote: > > > >> On Thu, Aug 13, 2015 at 4:30 PM, Lee Jones <lee.jones@linaro.org> wrote: > >> > On Thu, 13 Aug 2015, Jassi Brar wrote: > >> > > >> >> On Thu, Aug 13, 2015 at 3:53 PM, Lee Jones <lee.jones@linaro.org> wrote: > >> >> > On Thu, 13 Aug 2015, Jassi Brar wrote: > >> >> > > >> >> >> On Thu, Aug 13, 2015 at 2:49 PM, Lee Jones <lee.jones@linaro.org> wrote: > >> >> >> > On Thu, 13 Aug 2015, Jassi Brar wrote: > >> >> >> > >> >> >> >> >> > + > >> >> >> >> >> > +static void mbox_test_prepare_message(struct mbox_client *client, void *message) > >> >> >> >> >> > +{ > >> >> >> >> >> > + struct mbox_test_device *tdev = dev_get_drvdata(client->dev); > >> >> >> >> >> > + > >> >> >> >> >> > + if (tdev->mmio) > >> >> >> >> >> > + memcpy(tdev->mmio, message, MBOX_MAX_MSG_LEN); > >> >> >> >> >> > > >> >> >> >> >> This is unlikely to work. Those platforms that need to send a 2 part > >> >> >> >> >> message, they do : > >> >> >> >> >> (a) Signal/Command/Target code via some controller register (via > >> >> >> >> >> mbox_send_message). > >> >> >> >> >> (b) Setup the payload in Shared-Memory (via tx_prepare). > >> >> >> >> >> > >> >> >> >> >> This test driver assumes both are the same. I think you have to > >> >> >> >> >> declare something like > >> >> >> >> > > >> >> >> >> > This driver assumes that the framework will call client->tx_prepare() > >> >> >> >> > first, which satisfies (b). It then assumes controller->send_data() > >> >> >> >> > will be invoked, which will send the platform specific > >> >> >> >> > signal/command/target code, which then satisfies (a). > >> >> >> >> > > >> >> >> >> Yeah, but what would be that code? Who provides that? > >> >> >> >> > >> >> >> >> > In what way does it assume they are the same? > >> >> >> >> > > >> >> >> >> notice the 'message' pointer in > >> >> >> >> mbox_send_message(tdev->tx_channel, message); > >> >> >> >> and > >> >> >> >> memcpy(tdev->mmio, message, MBOX_MAX_MSG_LEN); > >> >> >> >> > >> >> >> >> >> struct mbox_test_message { /* same for TX and RX */ > >> >> >> >> >> unsigned sig_len; > >> >> >> >> >> void *signal; /* rx/tx via mailbox api */ > >> >> >> >> >> unsigned pl_len; > >> >> >> >> >> void *payload; /* rx/tx via shared-memory */ > >> >> >> >> >> }; > >> >> >> >> > > >> >> >> >> > How do you think this should be set and use these? > >> >> >> >> > > >> >> >> >> The userspace would want to specify the command code (32bits or not) > >> >> >> >> that would be passed via the fifo/register of the controller. So we > >> >> >> >> need signal[] > >> >> >> >> > >> >> >> >> The data to be passed via share-memory could be provided by userspace > >> >> >> >> via the payload[] array. > >> >> >> > > >> >> >> > Okay, so would the solution be two userspace files 'signal' and > >> >> >> > 'message', so in the case of a client specified signal we can write it > >> >> >> > into there first. > >> >> >> > > >> >> >> > echo 255 > signal > >> >> >> > echo test > message > >> >> >> > > >> >> >> > ... or in the case where no signal is required, or the controller > >> >> >> > driver taking care of it, we just don't write anything to signal? > >> >> >> > > >> >> >> file_operations.write() should parse signal and message, coming from > >> >> >> userspace, into a local structure before routing them via > >> >> >> mbox_send_message and tx_prepare respectively. > >> >> > > >> >> > Okay. So before I code this up we should agree on syntax. > >> >> > > >> >> > How would you like to represent the break between signal and message? > >> >> > Obviously ' ' would be a bad idea, as some clients may want to send > >> >> > messages with white space contained. How about '\t' or '\n'? > >> >> > > >> >> Yeah, I am not a fan of markers and flags either. > >> >> > >> >> Maybe we should share with userspace > >> >> struct mbox_test_message { /* same for TX and RX */ > >> >> unsigned sig_len; > >> >> void __user *signal; /* rx/tx via mailbox api */ > >> >> unsigned pl_len; > >> >> void __user *payload; /* rx/tx via shared-memory */ > >> >> }; > >> >> > >> >> First copy_from_user the structure of length sizof(struct > >> >> mbox_test_message) and then copy_from_user length sig_len and pl_len > >> >> from signal[] and payload[] respectively (usually ioctls would carry > >> >> such data). > >> > > >> > Simplicity and ease of use should be the goals here. Testers should > >> > not have to write applications to use this driver. Can we come up > >> > with a simple/effective method that uses SYSFS/DEBUGFS please? > >> > > >> > The easiest way I can think of which avoids markers/separators AND the > >> > requirement for users to have to write applications is to have two > >> > files, 'signal' and 'message' as mentioned before. Once both are > >> > populated I can get this driver to draft the message appropriately and > >> > fire it off. > >> > > >> And then write to more files for RX data? ... which should also be in > >> the form of 'signal' and 'message'. > >> > >> BTW like for spi there is a stock application in > >> Documentation/spi/spidev_test.c we could do the same? > > > > Coming from personal experience, testing drivers with (even stock) > > applications is much more painful that simply writing/reading > > (cat/echo) to a file in SYSFS/DEBUGFS. Particularly if people are > > using initramfs or thelike. If it is possible to use SYSFS/DEBUGFS, > > which it is in this case, then I believe that's the route we could go > > down. > > > Well, where could sysfs/debugfs not be used? :) Anyways I am ok if > prefer debugfs. You know what I mean; CPIO et. al. > > In answer to your question; we only need those two files. The reply > > can be placed back into 'message' and can be read from there. > > > Testing shouldn't be restricted to 'send command and receive reply'. > What if Linux only listens to broadcasts from the remote? Who knows > someone might want to (ab)use this test client to implement userspace > handler of remote commands? > So please see RX to be independent of TX. Sure. Now just agree with me that mbox_request_chan() should fail on request of a known bad configuration request and I can code all this up and re-submit. :D
On Thu, 13 Aug 2015, Jassi Brar wrote: > On Thu, Aug 13, 2015 at 6:37 PM, Lee Jones <lee.jones@linaro.org> wrote: > > > > Now just agree with me that mbox_request_chan() should fail on request > > of a known bad configuration request and I can code all this up and > > re-submit. :D > > > You make me look like a jerk :( My problem is not with validation as > such. I see problem in the way you implement that makes validation > necessary. I'll explain step-by-step in the driver post. That wasn't the intention, don't be so sensitive. ;)
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index 2cc4c39..7720bde 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -77,4 +77,11 @@ config STI_MBOX Mailbox implementation for STMicroelectonics family chips with hardware for interprocessor communication. +config MAILBOX_TEST + tristate "Mailbox Test Client" + depends on OF + help + Test client to help with testing new Controller driver + implementations. + endif diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index 7cb4766..92435ef 100644 --- a/drivers/mailbox/Makefile +++ b/drivers/mailbox/Makefile @@ -2,6 +2,8 @@ obj-$(CONFIG_MAILBOX) += mailbox.o +obj-$(CONFIG_MAILBOX_TEST) += mailbox-test.o + obj-$(CONFIG_ARM_MHU) += arm_mhu.o obj-$(CONFIG_PL320_MBOX) += pl320-ipc.o diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c new file mode 100644 index 0000000..10bfe3a --- /dev/null +++ b/drivers/mailbox/mailbox-test.c @@ -0,0 +1,210 @@ +/* + * Copyright (C) 2015 ST Microelectronics + * + * Author: Lee Jones <lee.jones@linaro.org> + * + * 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; either version 2 of the License, or + * (at your option) any later version. + */ + +#include <linux/debugfs.h> +#include <linux/err.h> +#include <linux/kernel.h> +#include <linux/mailbox_client.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/uaccess.h> + +#define MBOX_MAX_MSG_LEN 128 + +static struct dentry *root_debugfs_dir; + +struct mbox_test_device { + struct device *dev; + struct mbox_chan *tx_channel; + struct mbox_chan *rx_channel; + void __iomem *mmio; + +}; + +static ssize_t mbox_test_write(struct file *filp, + const char __user *userbuf, + size_t count, loff_t *ppos) +{ + struct mbox_test_device *tdev = filp->private_data; + char *message; + int ret; + + if (count > MBOX_MAX_MSG_LEN) { + dev_err(tdev->dev, + "Message length %d greater than max allowed %d\n", + count, MBOX_MAX_MSG_LEN); + return -EINVAL; + } + + message = kzalloc(count, GFP_KERNEL); + if (!message) + return -ENOMEM; + + ret = copy_from_user(message, userbuf, count); + if (ret) + return -EFAULT; + + print_hex_dump(KERN_ERR, "Client: Sending: ", DUMP_PREFIX_ADDRESS, + 16, 1, message, 16, true); + + ret = mbox_send_message(tdev->tx_channel, message); + if (ret < 0) + dev_err(tdev->dev, "Failed to send message via mailbox\n"); + + kfree(message); + + return ret < 0 ? ret : count; +} + +static const struct file_operations mbox_test_ops = { + .write = mbox_test_write, + .open = simple_open, + .llseek = generic_file_llseek, +}; + +static int mbox_test_add_debugfs(struct platform_device *pdev, + struct mbox_test_device *tdev) +{ + if (!debugfs_initialized()) + return 0; + + root_debugfs_dir = debugfs_create_dir("mailbox", NULL); + if (!root_debugfs_dir) { + dev_err(&pdev->dev, "Failed to create Mailbox debugfs\n"); + return -EINVAL; + } + + debugfs_create_file("send-message", 0200, root_debugfs_dir, + tdev, &mbox_test_ops); + + return 0; +} + +static void mbox_test_receive_message(struct mbox_client *client, void *message) +{ + struct mbox_test_device *tdev = dev_get_drvdata(client->dev); + + if (!tdev->mmio) { + dev_info(tdev->dev, "Client: Recived something [read mmio]\n"); + return; + } + + print_hex_dump(KERN_ERR, "Client: from co-proc: ", DUMP_PREFIX_ADDRESS, + 16, 1, tdev->mmio, MBOX_MAX_MSG_LEN, true); +} + +static void mbox_test_prepare_message(struct mbox_client *client, void *message) +{ + struct mbox_test_device *tdev = dev_get_drvdata(client->dev); + + if (tdev->mmio) + memcpy(tdev->mmio, message, MBOX_MAX_MSG_LEN); +} + +static void mbox_test_message_sent(struct mbox_client *client, + void *message, int r) +{ + if (r) + dev_warn(client->dev, + "Client: Message could not be sent: %d\n", r); + else + dev_info(client->dev, + "Client: Message sent\n"); +} + +static struct mbox_chan * +mbox_test_request_channel(struct platform_device *pdev, const char *name) +{ + struct mbox_client *client; + + client = devm_kzalloc(&pdev->dev, sizeof(*client), GFP_KERNEL); + if (!client) + return ERR_PTR(-ENOMEM); + + client->dev = &pdev->dev; + client->rx_callback = mbox_test_receive_message; + client->tx_prepare = mbox_test_prepare_message; + client->tx_done = mbox_test_message_sent; + client->tx_block = true; + client->knows_txdone = false; + client->tx_tout = 500; + + return mbox_request_channel_byname(client, name); +} + +static int mbox_test_probe(struct platform_device *pdev) +{ + struct mbox_test_device *tdev; + struct resource *res; + int ret; + + tdev = devm_kzalloc(&pdev->dev, sizeof(*tdev), GFP_KERNEL); + if (!tdev) + return -ENOMEM; + + /* It's okay for MMIO to be NULL */ + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + tdev->mmio = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(tdev->mmio)) + tdev->mmio = NULL; + + tdev->tx_channel = mbox_test_request_channel(pdev, "tx"); + if (IS_ERR(tdev->tx_channel)) + return PTR_ERR(tdev->tx_channel); + + tdev->rx_channel = mbox_test_request_channel(pdev, "rx"); + if (IS_ERR(tdev->rx_channel)) + return PTR_ERR(tdev->rx_channel); + + tdev->dev = &pdev->dev; + platform_set_drvdata(pdev, tdev); + + ret = mbox_test_add_debugfs(pdev, tdev); + if (ret) + return ret; + + dev_info(&pdev->dev, "Successfully registered\n"); + + return 0; +} + +static int mbox_test_remove(struct platform_device *pdev) +{ + struct mbox_test_device *tdev = platform_get_drvdata(pdev); + + debugfs_remove_recursive(root_debugfs_dir); + + mbox_free_channel(tdev->tx_channel); + mbox_free_channel(tdev->rx_channel); + + return 0; +} + +static const struct of_device_id mbox_test_match[] = { + { .compatible = "mailbox_test" }, + {}, +}; + +static struct platform_driver mbox_test_driver = { + .driver = { + .name = "mailbox_sti_test", + .of_match_table = mbox_test_match, + }, + .probe = mbox_test_probe, + .remove = mbox_test_remove, +}; +module_platform_driver(mbox_test_driver); + +MODULE_DESCRIPTION("Generic Mailbox Testing Facility"); +MODULE_AUTHOR("Lee Jones <lee.jones@linaro.org"); +MODULE_LICENSE("GPL v2");
This particular Client implementation uses shared memory in order to pass messages between Mailbox users; however, it can be easily hacked to support any type of Controller. Signed-off-by: Lee Jones <lee.jones@linaro.org> --- drivers/mailbox/Kconfig | 7 ++ drivers/mailbox/Makefile | 2 + drivers/mailbox/mailbox-test.c | 210 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 219 insertions(+) create mode 100644 drivers/mailbox/mailbox-test.c