* Re: [PATCH v2 1/2] soc: aspeed: Add LPC mailbox support
@ 2022-03-18 7:05 ` Joel Stanley
0 siblings, 0 replies; 23+ messages in thread
From: Joel Stanley @ 2022-03-18 7:05 UTC (permalink / raw)
To: Chia-Wei Wang, Arnd Bergmann
Cc: Rob Herring, Andrew Jeffery, Cyril Bur, devicetree, Linux ARM,
linux-aspeed, Linux Kernel Mailing List, OpenBMC Maillist
On Tue, 17 Aug 2021 at 02:58, Chia-Wei Wang <chiawei_wang@aspeedtech.com> wrote:
>
> The LPC mailbox controller consists of multiple general
> registers for the communication between the Host and the BMC.
> The interrupts for data update signaling are also introduced.
The concern with this approach is the userspace API. We don't want
every driver to invent it's own way of communicating with usersapce.
I know when Cyril first submitted this driver it was suggested to use
the kernel mailbox subsystem. After investigation it was decided that
this hardware is not a fit; although it is a mailbox it is more like a
IPMI BT or KCS device, that needs an interface to push data to and
from userspace over the device.
Chai-Wei, can you link to some userspace programs that use the
existing ioctl interface?
Arnd, do you have any suggestions?
>
> Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/soc/aspeed/Kconfig | 10 +
> drivers/soc/aspeed/Makefile | 9 +-
> drivers/soc/aspeed/aspeed-lpc-mbox.c | 418 +++++++++++++++++++++++++++
> 3 files changed, 433 insertions(+), 4 deletions(-)
> create mode 100644 drivers/soc/aspeed/aspeed-lpc-mbox.c
>
> diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
> index 243ca196e6ad..5b31a6e2620c 100644
> --- a/drivers/soc/aspeed/Kconfig
> +++ b/drivers/soc/aspeed/Kconfig
> @@ -24,6 +24,16 @@ config ASPEED_LPC_SNOOP
> allows the BMC to listen on and save the data written by
> the host to an arbitrary LPC I/O port.
>
> +config ASPEED_LPC_MAILBOX
> + tristate "ASPEED LPC mailbox support"
> + select REGMAP
> + select MFD_SYSCON
> + default ARCH_ASPEED
> + help
> + Provides a driver to control the LPC mailbox which possesses
> + up to 32 data registers for the communication between the Host
> + and the BMC over LPC.
> +
> config ASPEED_P2A_CTRL
> tristate "ASPEED P2A (VGA MMIO to BMC) bridge control"
> select REGMAP
> diff --git a/drivers/soc/aspeed/Makefile b/drivers/soc/aspeed/Makefile
> index fcab7192e1a4..cde6b4514c97 100644
> --- a/drivers/soc/aspeed/Makefile
> +++ b/drivers/soc/aspeed/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> -obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> -obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
> -obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o
> +obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> +obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> +obj-$(CONFIG_ASPEED_LPC_MAILBOX) += aspeed-lpc-mbox.o
> +obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
> +obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o
> diff --git a/drivers/soc/aspeed/aspeed-lpc-mbox.c b/drivers/soc/aspeed/aspeed-lpc-mbox.c
> new file mode 100644
> index 000000000000..a09ca6a175f7
> --- /dev/null
> +++ b/drivers/soc/aspeed/aspeed-lpc-mbox.c
> @@ -0,0 +1,418 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2017 IBM Corporation
> + * Copyright 2021 Aspeed Technology Inc.
> + */
> +#include <linux/interrupt.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/poll.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#define DEVICE_NAME "aspeed-mbox"
> +
> +#define ASPEED_MBOX_DR(dr, n) (dr + (n * 4))
> +#define ASPEED_MBOX_STR(str, n) (str + (n / 8) * 4)
> +#define ASPEED_MBOX_BIE(bie, n) (bie + (n / 8) * 4)
> +#define ASPEED_MBOX_HIE(hie, n) (hie + (n / 8) * 4)
> +
> +#define ASPEED_MBOX_BCR_RECV BIT(7)
> +#define ASPEED_MBOX_BCR_MASK BIT(1)
> +#define ASPEED_MBOX_BCR_SEND BIT(0)
> +
> +/* ioctl code */
> +#define ASPEED_MBOX_IOCTL 0xA3
> +#define ASPEED_MBOX_IOCTL_GET_SIZE \
> + _IOR(ASPEED_MBOX_IOCTL, 0, struct aspeed_mbox_ioctl_data)
> +
> +struct aspeed_mbox_ioctl_data {
> + unsigned int data;
> +};
> +
> +struct aspeed_mbox_model {
> + unsigned int dr_num;
> +
> + /* offsets to the MBOX registers */
> + unsigned int dr;
> + unsigned int str;
> + unsigned int bcr;
> + unsigned int hcr;
> + unsigned int bie;
> + unsigned int hie;
> +};
> +
> +struct aspeed_mbox {
> + struct miscdevice miscdev;
> + struct regmap *map;
> + unsigned int base;
> + wait_queue_head_t queue;
> + struct mutex mutex;
> + const struct aspeed_mbox_model *model;
> +};
> +
> +static atomic_t aspeed_mbox_open_count = ATOMIC_INIT(0);
> +
> +static u8 aspeed_mbox_inb(struct aspeed_mbox *mbox, int reg)
> +{
> + /*
> + * The mbox registers are actually only one byte but are addressed
> + * four bytes apart. The other three bytes are marked 'reserved',
> + * they *should* be zero but lets not rely on it.
> + * I am going to rely on the fact we can casually read/write to them...
> + */
> + unsigned int val = 0xff; /* If regmap throws an error return 0xff */
> + int rc = regmap_read(mbox->map, mbox->base + reg, &val);
> +
> + if (rc)
> + dev_err(mbox->miscdev.parent, "regmap_read() failed with "
> + "%d (reg: 0x%08x)\n", rc, reg);
> +
> + return val & 0xff;
> +}
> +
> +static void aspeed_mbox_outb(struct aspeed_mbox *mbox, u8 data, int reg)
> +{
> + int rc = regmap_write(mbox->map, mbox->base + reg, data);
> +
> + if (rc)
> + dev_err(mbox->miscdev.parent, "regmap_write() failed with "
> + "%d (data: %u reg: 0x%08x)\n", rc, data, reg);
> +}
> +
> +static struct aspeed_mbox *file_mbox(struct file *file)
> +{
> + return container_of(file->private_data, struct aspeed_mbox, miscdev);
> +}
> +
> +static int aspeed_mbox_open(struct inode *inode, struct file *file)
> +{
> + struct aspeed_mbox *mbox = file_mbox(file);
> + const struct aspeed_mbox_model *model = mbox->model;
> +
> + if (atomic_inc_return(&aspeed_mbox_open_count) == 1) {
> + /*
> + * Clear the interrupt status bit if it was left on and unmask
> + * interrupts.
> + * ASPEED_MBOX_BCR_RECV bit is W1C, this also unmasks in 1 step
> + */
> + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV, model->bcr);
> + return 0;
> + }
> +
> + atomic_dec(&aspeed_mbox_open_count);
> + return -EBUSY;
> +}
> +
> +static ssize_t aspeed_mbox_read(struct file *file, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct aspeed_mbox *mbox = file_mbox(file);
> + const struct aspeed_mbox_model *model = mbox->model;
> + char __user *p = buf;
> + ssize_t ret;
> + int i;
> +
> + if (!access_ok(buf, count))
> + return -EFAULT;
> +
> + if (count + *ppos > model->dr_num)
> + return -EINVAL;
> +
> + if (file->f_flags & O_NONBLOCK) {
> + if (!(aspeed_mbox_inb(mbox, model->bcr) &
> + ASPEED_MBOX_BCR_RECV))
> + return -EAGAIN;
> + } else if (wait_event_interruptible(mbox->queue,
> + aspeed_mbox_inb(mbox, model->bcr) &
> + ASPEED_MBOX_BCR_RECV)) {
> + return -ERESTARTSYS;
> + }
> +
> + mutex_lock(&mbox->mutex);
> +
> + for (i = *ppos; count > 0 && i < model->dr_num; i++) {
> + uint8_t reg = aspeed_mbox_inb(mbox, ASPEED_MBOX_DR(model->dr, i));
> +
> + ret = __put_user(reg, p);
> + if (ret)
> + goto out_unlock;
> +
> + p++;
> + count--;
> + }
> +
> + /* ASPEED_MBOX_BCR_RECV bit is write to clear, this also unmasks in 1 step */
> + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV, model->bcr);
> + ret = p - buf;
> +
> +out_unlock:
> + mutex_unlock(&mbox->mutex);
> + return ret;
> +}
> +
> +static ssize_t aspeed_mbox_write(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct aspeed_mbox *mbox = file_mbox(file);
> + const struct aspeed_mbox_model *model = mbox->model;
> + const char __user *p = buf;
> + ssize_t ret;
> + char c;
> + int i;
> +
> + if (!access_ok(buf, count))
> + return -EFAULT;
> +
> + if (count + *ppos > model->dr_num)
> + return -EINVAL;
> +
> + mutex_lock(&mbox->mutex);
> +
> + for (i = *ppos; count > 0 && i < model->dr_num; i++) {
> + ret = __get_user(c, p);
> + if (ret)
> + goto out_unlock;
> +
> + aspeed_mbox_outb(mbox, c, ASPEED_MBOX_DR(model->dr, i));
> + p++;
> + count--;
> + }
> +
> + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_SEND, model->bcr);
> + ret = p - buf;
> +
> +out_unlock:
> + mutex_unlock(&mbox->mutex);
> + return ret;
> +}
> +
> +static __poll_t aspeed_mbox_poll(struct file *file, poll_table *wait)
> +{
> + struct aspeed_mbox *mbox = file_mbox(file);
> + const struct aspeed_mbox_model *model = mbox->model;
> + __poll_t mask = 0;
> +
> + poll_wait(file, &mbox->queue, wait);
> +
> + if (aspeed_mbox_inb(mbox, model->bcr) & ASPEED_MBOX_BCR_RECV)
> + mask |= POLLIN;
> +
> + return mask;
> +}
> +
> +static int aspeed_mbox_release(struct inode *inode, struct file *file)
> +{
> + atomic_dec(&aspeed_mbox_open_count);
> + return 0;
> +}
> +
> +static long aspeed_mbox_ioctl(struct file *file, unsigned int cmd,
> + unsigned long param)
> +{
> + long ret = 0;
> + struct aspeed_mbox *mbox = file_mbox(file);
> + const struct aspeed_mbox_model *model = mbox->model;
> + struct aspeed_mbox_ioctl_data data;
> +
> + switch (cmd) {
> + case ASPEED_MBOX_IOCTL_GET_SIZE:
> + data.data = model->dr_num;
> + if (copy_to_user((void __user *)param, &data, sizeof(data)))
> + ret = -EFAULT;
> + break;
> + default:
> + ret = -ENOTTY;
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static const struct file_operations aspeed_mbox_fops = {
> + .owner = THIS_MODULE,
> + .llseek = no_seek_end_llseek,
> + .read = aspeed_mbox_read,
> + .write = aspeed_mbox_write,
> + .open = aspeed_mbox_open,
> + .release = aspeed_mbox_release,
> + .poll = aspeed_mbox_poll,
> + .unlocked_ioctl = aspeed_mbox_ioctl,
> +};
> +
> +static irqreturn_t aspeed_mbox_irq(int irq, void *arg)
> +{
> + struct aspeed_mbox *mbox = arg;
> + const struct aspeed_mbox_model *model = mbox->model;
> +
> + if (!(aspeed_mbox_inb(mbox, model->bcr) & ASPEED_MBOX_BCR_RECV))
> + return IRQ_NONE;
> +
> + /*
> + * Leave the status bit set so that we know the data is for us,
> + * clear it once it has been read.
> + */
> +
> + /* Mask it off, we'll clear it when we the data gets read */
> + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_MASK, model->bcr);
> +
> + wake_up(&mbox->queue);
> + return IRQ_HANDLED;
> +}
> +
> +static int aspeed_mbox_config_irq(struct aspeed_mbox *mbox,
> + struct platform_device *pdev)
> +{
> + const struct aspeed_mbox_model *model = mbox->model;
> + struct device *dev = &pdev->dev;
> + int i, rc, irq;
> +
> + irq = irq_of_parse_and_map(dev->of_node, 0);
> + if (!irq)
> + return -ENODEV;
> +
> + rc = devm_request_irq(dev, irq, aspeed_mbox_irq,
> + IRQF_SHARED, DEVICE_NAME, mbox);
> + if (rc < 0) {
> + dev_err(dev, "Unable to request IRQ %d\n", irq);
> + return rc;
> + }
> +
> + /*
> + * Disable all register based interrupts.
> + */
> + for (i = 0; i < model->dr_num / 8; ++i)
> + aspeed_mbox_outb(mbox, 0x00, ASPEED_MBOX_BIE(model->bie, i));
> +
> + /* These registers are write one to clear. Clear them. */
> + for (i = 0; i < model->dr_num / 8; ++i)
> + aspeed_mbox_outb(mbox, 0xff, ASPEED_MBOX_STR(model->str, i));
> +
> + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV, model->bcr);
> + return 0;
> +}
> +
> +static int aspeed_mbox_probe(struct platform_device *pdev)
> +{
> + struct aspeed_mbox *mbox;
> + struct device *dev;
> + int rc;
> +
> + dev = &pdev->dev;
> +
> + mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> + if (!mbox)
> + return -ENOMEM;
> +
> + dev_set_drvdata(&pdev->dev, mbox);
> +
> + rc = of_property_read_u32(dev->of_node, "reg", &mbox->base);
> + if (rc) {
> + dev_err(dev, "Couldn't read reg device tree property\n");
> + return rc;
> + }
> +
> + mbox->model = of_device_get_match_data(dev);
> + if (IS_ERR(mbox->model)) {
> + dev_err(dev, "Couldn't get model data\n");
> + return -ENODEV;
> + }
> +
> + mbox->map = syscon_node_to_regmap(
> + pdev->dev.parent->of_node);
> + if (IS_ERR(mbox->map)) {
> + dev_err(dev, "Couldn't get regmap\n");
> + return -ENODEV;
> + }
> +
> + mutex_init(&mbox->mutex);
> + init_waitqueue_head(&mbox->queue);
> +
> + mbox->miscdev.minor = MISC_DYNAMIC_MINOR;
> + mbox->miscdev.name = DEVICE_NAME;
> + mbox->miscdev.fops = &aspeed_mbox_fops;
> + mbox->miscdev.parent = dev;
> + rc = misc_register(&mbox->miscdev);
> + if (rc) {
> + dev_err(dev, "Unable to register device\n");
> + return rc;
> + }
> +
> + rc = aspeed_mbox_config_irq(mbox, pdev);
> + if (rc) {
> + dev_err(dev, "Failed to configure IRQ\n");
> + misc_deregister(&mbox->miscdev);
> + return rc;
> + }
> +
> + return 0;
> +}
> +
> +static int aspeed_mbox_remove(struct platform_device *pdev)
> +{
> + struct aspeed_mbox *mbox = dev_get_drvdata(&pdev->dev);
> +
> + misc_deregister(&mbox->miscdev);
> +
> + return 0;
> +}
> +
> +static const struct aspeed_mbox_model ast2400_model = {
> + .dr_num = 16,
> + .dr = 0x0,
> + .str = 0x40,
> + .bcr = 0x48,
> + .hcr = 0x4c,
> + .bie = 0x50,
> + .hie = 0x58,
> +};
> +
> +static const struct aspeed_mbox_model ast2500_model = {
> + .dr_num = 16,
> + .dr = 0x0,
> + .str = 0x40,
> + .bcr = 0x48,
> + .hcr = 0x4c,
> + .bie = 0x50,
> + .hie = 0x58,
> +};
> +
> +static const struct aspeed_mbox_model ast2600_model = {
> + .dr_num = 32,
> + .dr = 0x0,
> + .str = 0x80,
> + .bcr = 0x90,
> + .hcr = 0x94,
> + .bie = 0xa0,
> + .hie = 0xb0,
> +};
> +
> +static const struct of_device_id aspeed_mbox_match[] = {
> + { .compatible = "aspeed,ast2400-mbox",
> + .data = &ast2400_model },
> + { .compatible = "aspeed,ast2500-mbox",
> + .data = &ast2500_model },
> + { .compatible = "aspeed,ast2600-mbox",
> + .data = &ast2600_model },
> + { },
> +};
> +
> +static struct platform_driver aspeed_mbox_driver = {
> + .driver = {
> + .name = DEVICE_NAME,
> + .of_match_table = aspeed_mbox_match,
> + },
> + .probe = aspeed_mbox_probe,
> + .remove = aspeed_mbox_remove,
> +};
> +
> +module_platform_driver(aspeed_mbox_driver);
> +MODULE_DEVICE_TABLE(of, aspeed_mbox_match);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Cyril Bur <cyrilbur@gmail.com>");
> +MODULE_AUTHOR("Chia-Wei Wang <chiawei_wang@aspeedtech.com");
> +MODULE_DESCRIPTION("Aspeed mailbox device driver");
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 1/2] soc: aspeed: Add LPC mailbox support
@ 2022-03-18 7:05 ` Joel Stanley
0 siblings, 0 replies; 23+ messages in thread
From: Joel Stanley @ 2022-03-18 7:05 UTC (permalink / raw)
To: Chia-Wei Wang, Arnd Bergmann
Cc: Rob Herring, Andrew Jeffery, Cyril Bur, devicetree, Linux ARM,
linux-aspeed, Linux Kernel Mailing List, OpenBMC Maillist
On Tue, 17 Aug 2021 at 02:58, Chia-Wei Wang <chiawei_wang@aspeedtech.com> wrote:
>
> The LPC mailbox controller consists of multiple general
> registers for the communication between the Host and the BMC.
> The interrupts for data update signaling are also introduced.
The concern with this approach is the userspace API. We don't want
every driver to invent it's own way of communicating with usersapce.
I know when Cyril first submitted this driver it was suggested to use
the kernel mailbox subsystem. After investigation it was decided that
this hardware is not a fit; although it is a mailbox it is more like a
IPMI BT or KCS device, that needs an interface to push data to and
from userspace over the device.
Chai-Wei, can you link to some userspace programs that use the
existing ioctl interface?
Arnd, do you have any suggestions?
>
> Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/soc/aspeed/Kconfig | 10 +
> drivers/soc/aspeed/Makefile | 9 +-
> drivers/soc/aspeed/aspeed-lpc-mbox.c | 418 +++++++++++++++++++++++++++
> 3 files changed, 433 insertions(+), 4 deletions(-)
> create mode 100644 drivers/soc/aspeed/aspeed-lpc-mbox.c
>
> diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
> index 243ca196e6ad..5b31a6e2620c 100644
> --- a/drivers/soc/aspeed/Kconfig
> +++ b/drivers/soc/aspeed/Kconfig
> @@ -24,6 +24,16 @@ config ASPEED_LPC_SNOOP
> allows the BMC to listen on and save the data written by
> the host to an arbitrary LPC I/O port.
>
> +config ASPEED_LPC_MAILBOX
> + tristate "ASPEED LPC mailbox support"
> + select REGMAP
> + select MFD_SYSCON
> + default ARCH_ASPEED
> + help
> + Provides a driver to control the LPC mailbox which possesses
> + up to 32 data registers for the communication between the Host
> + and the BMC over LPC.
> +
> config ASPEED_P2A_CTRL
> tristate "ASPEED P2A (VGA MMIO to BMC) bridge control"
> select REGMAP
> diff --git a/drivers/soc/aspeed/Makefile b/drivers/soc/aspeed/Makefile
> index fcab7192e1a4..cde6b4514c97 100644
> --- a/drivers/soc/aspeed/Makefile
> +++ b/drivers/soc/aspeed/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> -obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> -obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
> -obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o
> +obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> +obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> +obj-$(CONFIG_ASPEED_LPC_MAILBOX) += aspeed-lpc-mbox.o
> +obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
> +obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o
> diff --git a/drivers/soc/aspeed/aspeed-lpc-mbox.c b/drivers/soc/aspeed/aspeed-lpc-mbox.c
> new file mode 100644
> index 000000000000..a09ca6a175f7
> --- /dev/null
> +++ b/drivers/soc/aspeed/aspeed-lpc-mbox.c
> @@ -0,0 +1,418 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2017 IBM Corporation
> + * Copyright 2021 Aspeed Technology Inc.
> + */
> +#include <linux/interrupt.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/poll.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#define DEVICE_NAME "aspeed-mbox"
> +
> +#define ASPEED_MBOX_DR(dr, n) (dr + (n * 4))
> +#define ASPEED_MBOX_STR(str, n) (str + (n / 8) * 4)
> +#define ASPEED_MBOX_BIE(bie, n) (bie + (n / 8) * 4)
> +#define ASPEED_MBOX_HIE(hie, n) (hie + (n / 8) * 4)
> +
> +#define ASPEED_MBOX_BCR_RECV BIT(7)
> +#define ASPEED_MBOX_BCR_MASK BIT(1)
> +#define ASPEED_MBOX_BCR_SEND BIT(0)
> +
> +/* ioctl code */
> +#define ASPEED_MBOX_IOCTL 0xA3
> +#define ASPEED_MBOX_IOCTL_GET_SIZE \
> + _IOR(ASPEED_MBOX_IOCTL, 0, struct aspeed_mbox_ioctl_data)
> +
> +struct aspeed_mbox_ioctl_data {
> + unsigned int data;
> +};
> +
> +struct aspeed_mbox_model {
> + unsigned int dr_num;
> +
> + /* offsets to the MBOX registers */
> + unsigned int dr;
> + unsigned int str;
> + unsigned int bcr;
> + unsigned int hcr;
> + unsigned int bie;
> + unsigned int hie;
> +};
> +
> +struct aspeed_mbox {
> + struct miscdevice miscdev;
> + struct regmap *map;
> + unsigned int base;
> + wait_queue_head_t queue;
> + struct mutex mutex;
> + const struct aspeed_mbox_model *model;
> +};
> +
> +static atomic_t aspeed_mbox_open_count = ATOMIC_INIT(0);
> +
> +static u8 aspeed_mbox_inb(struct aspeed_mbox *mbox, int reg)
> +{
> + /*
> + * The mbox registers are actually only one byte but are addressed
> + * four bytes apart. The other three bytes are marked 'reserved',
> + * they *should* be zero but lets not rely on it.
> + * I am going to rely on the fact we can casually read/write to them...
> + */
> + unsigned int val = 0xff; /* If regmap throws an error return 0xff */
> + int rc = regmap_read(mbox->map, mbox->base + reg, &val);
> +
> + if (rc)
> + dev_err(mbox->miscdev.parent, "regmap_read() failed with "
> + "%d (reg: 0x%08x)\n", rc, reg);
> +
> + return val & 0xff;
> +}
> +
> +static void aspeed_mbox_outb(struct aspeed_mbox *mbox, u8 data, int reg)
> +{
> + int rc = regmap_write(mbox->map, mbox->base + reg, data);
> +
> + if (rc)
> + dev_err(mbox->miscdev.parent, "regmap_write() failed with "
> + "%d (data: %u reg: 0x%08x)\n", rc, data, reg);
> +}
> +
> +static struct aspeed_mbox *file_mbox(struct file *file)
> +{
> + return container_of(file->private_data, struct aspeed_mbox, miscdev);
> +}
> +
> +static int aspeed_mbox_open(struct inode *inode, struct file *file)
> +{
> + struct aspeed_mbox *mbox = file_mbox(file);
> + const struct aspeed_mbox_model *model = mbox->model;
> +
> + if (atomic_inc_return(&aspeed_mbox_open_count) == 1) {
> + /*
> + * Clear the interrupt status bit if it was left on and unmask
> + * interrupts.
> + * ASPEED_MBOX_BCR_RECV bit is W1C, this also unmasks in 1 step
> + */
> + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV, model->bcr);
> + return 0;
> + }
> +
> + atomic_dec(&aspeed_mbox_open_count);
> + return -EBUSY;
> +}
> +
> +static ssize_t aspeed_mbox_read(struct file *file, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct aspeed_mbox *mbox = file_mbox(file);
> + const struct aspeed_mbox_model *model = mbox->model;
> + char __user *p = buf;
> + ssize_t ret;
> + int i;
> +
> + if (!access_ok(buf, count))
> + return -EFAULT;
> +
> + if (count + *ppos > model->dr_num)
> + return -EINVAL;
> +
> + if (file->f_flags & O_NONBLOCK) {
> + if (!(aspeed_mbox_inb(mbox, model->bcr) &
> + ASPEED_MBOX_BCR_RECV))
> + return -EAGAIN;
> + } else if (wait_event_interruptible(mbox->queue,
> + aspeed_mbox_inb(mbox, model->bcr) &
> + ASPEED_MBOX_BCR_RECV)) {
> + return -ERESTARTSYS;
> + }
> +
> + mutex_lock(&mbox->mutex);
> +
> + for (i = *ppos; count > 0 && i < model->dr_num; i++) {
> + uint8_t reg = aspeed_mbox_inb(mbox, ASPEED_MBOX_DR(model->dr, i));
> +
> + ret = __put_user(reg, p);
> + if (ret)
> + goto out_unlock;
> +
> + p++;
> + count--;
> + }
> +
> + /* ASPEED_MBOX_BCR_RECV bit is write to clear, this also unmasks in 1 step */
> + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV, model->bcr);
> + ret = p - buf;
> +
> +out_unlock:
> + mutex_unlock(&mbox->mutex);
> + return ret;
> +}
> +
> +static ssize_t aspeed_mbox_write(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct aspeed_mbox *mbox = file_mbox(file);
> + const struct aspeed_mbox_model *model = mbox->model;
> + const char __user *p = buf;
> + ssize_t ret;
> + char c;
> + int i;
> +
> + if (!access_ok(buf, count))
> + return -EFAULT;
> +
> + if (count + *ppos > model->dr_num)
> + return -EINVAL;
> +
> + mutex_lock(&mbox->mutex);
> +
> + for (i = *ppos; count > 0 && i < model->dr_num; i++) {
> + ret = __get_user(c, p);
> + if (ret)
> + goto out_unlock;
> +
> + aspeed_mbox_outb(mbox, c, ASPEED_MBOX_DR(model->dr, i));
> + p++;
> + count--;
> + }
> +
> + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_SEND, model->bcr);
> + ret = p - buf;
> +
> +out_unlock:
> + mutex_unlock(&mbox->mutex);
> + return ret;
> +}
> +
> +static __poll_t aspeed_mbox_poll(struct file *file, poll_table *wait)
> +{
> + struct aspeed_mbox *mbox = file_mbox(file);
> + const struct aspeed_mbox_model *model = mbox->model;
> + __poll_t mask = 0;
> +
> + poll_wait(file, &mbox->queue, wait);
> +
> + if (aspeed_mbox_inb(mbox, model->bcr) & ASPEED_MBOX_BCR_RECV)
> + mask |= POLLIN;
> +
> + return mask;
> +}
> +
> +static int aspeed_mbox_release(struct inode *inode, struct file *file)
> +{
> + atomic_dec(&aspeed_mbox_open_count);
> + return 0;
> +}
> +
> +static long aspeed_mbox_ioctl(struct file *file, unsigned int cmd,
> + unsigned long param)
> +{
> + long ret = 0;
> + struct aspeed_mbox *mbox = file_mbox(file);
> + const struct aspeed_mbox_model *model = mbox->model;
> + struct aspeed_mbox_ioctl_data data;
> +
> + switch (cmd) {
> + case ASPEED_MBOX_IOCTL_GET_SIZE:
> + data.data = model->dr_num;
> + if (copy_to_user((void __user *)param, &data, sizeof(data)))
> + ret = -EFAULT;
> + break;
> + default:
> + ret = -ENOTTY;
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static const struct file_operations aspeed_mbox_fops = {
> + .owner = THIS_MODULE,
> + .llseek = no_seek_end_llseek,
> + .read = aspeed_mbox_read,
> + .write = aspeed_mbox_write,
> + .open = aspeed_mbox_open,
> + .release = aspeed_mbox_release,
> + .poll = aspeed_mbox_poll,
> + .unlocked_ioctl = aspeed_mbox_ioctl,
> +};
> +
> +static irqreturn_t aspeed_mbox_irq(int irq, void *arg)
> +{
> + struct aspeed_mbox *mbox = arg;
> + const struct aspeed_mbox_model *model = mbox->model;
> +
> + if (!(aspeed_mbox_inb(mbox, model->bcr) & ASPEED_MBOX_BCR_RECV))
> + return IRQ_NONE;
> +
> + /*
> + * Leave the status bit set so that we know the data is for us,
> + * clear it once it has been read.
> + */
> +
> + /* Mask it off, we'll clear it when we the data gets read */
> + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_MASK, model->bcr);
> +
> + wake_up(&mbox->queue);
> + return IRQ_HANDLED;
> +}
> +
> +static int aspeed_mbox_config_irq(struct aspeed_mbox *mbox,
> + struct platform_device *pdev)
> +{
> + const struct aspeed_mbox_model *model = mbox->model;
> + struct device *dev = &pdev->dev;
> + int i, rc, irq;
> +
> + irq = irq_of_parse_and_map(dev->of_node, 0);
> + if (!irq)
> + return -ENODEV;
> +
> + rc = devm_request_irq(dev, irq, aspeed_mbox_irq,
> + IRQF_SHARED, DEVICE_NAME, mbox);
> + if (rc < 0) {
> + dev_err(dev, "Unable to request IRQ %d\n", irq);
> + return rc;
> + }
> +
> + /*
> + * Disable all register based interrupts.
> + */
> + for (i = 0; i < model->dr_num / 8; ++i)
> + aspeed_mbox_outb(mbox, 0x00, ASPEED_MBOX_BIE(model->bie, i));
> +
> + /* These registers are write one to clear. Clear them. */
> + for (i = 0; i < model->dr_num / 8; ++i)
> + aspeed_mbox_outb(mbox, 0xff, ASPEED_MBOX_STR(model->str, i));
> +
> + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV, model->bcr);
> + return 0;
> +}
> +
> +static int aspeed_mbox_probe(struct platform_device *pdev)
> +{
> + struct aspeed_mbox *mbox;
> + struct device *dev;
> + int rc;
> +
> + dev = &pdev->dev;
> +
> + mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> + if (!mbox)
> + return -ENOMEM;
> +
> + dev_set_drvdata(&pdev->dev, mbox);
> +
> + rc = of_property_read_u32(dev->of_node, "reg", &mbox->base);
> + if (rc) {
> + dev_err(dev, "Couldn't read reg device tree property\n");
> + return rc;
> + }
> +
> + mbox->model = of_device_get_match_data(dev);
> + if (IS_ERR(mbox->model)) {
> + dev_err(dev, "Couldn't get model data\n");
> + return -ENODEV;
> + }
> +
> + mbox->map = syscon_node_to_regmap(
> + pdev->dev.parent->of_node);
> + if (IS_ERR(mbox->map)) {
> + dev_err(dev, "Couldn't get regmap\n");
> + return -ENODEV;
> + }
> +
> + mutex_init(&mbox->mutex);
> + init_waitqueue_head(&mbox->queue);
> +
> + mbox->miscdev.minor = MISC_DYNAMIC_MINOR;
> + mbox->miscdev.name = DEVICE_NAME;
> + mbox->miscdev.fops = &aspeed_mbox_fops;
> + mbox->miscdev.parent = dev;
> + rc = misc_register(&mbox->miscdev);
> + if (rc) {
> + dev_err(dev, "Unable to register device\n");
> + return rc;
> + }
> +
> + rc = aspeed_mbox_config_irq(mbox, pdev);
> + if (rc) {
> + dev_err(dev, "Failed to configure IRQ\n");
> + misc_deregister(&mbox->miscdev);
> + return rc;
> + }
> +
> + return 0;
> +}
> +
> +static int aspeed_mbox_remove(struct platform_device *pdev)
> +{
> + struct aspeed_mbox *mbox = dev_get_drvdata(&pdev->dev);
> +
> + misc_deregister(&mbox->miscdev);
> +
> + return 0;
> +}
> +
> +static const struct aspeed_mbox_model ast2400_model = {
> + .dr_num = 16,
> + .dr = 0x0,
> + .str = 0x40,
> + .bcr = 0x48,
> + .hcr = 0x4c,
> + .bie = 0x50,
> + .hie = 0x58,
> +};
> +
> +static const struct aspeed_mbox_model ast2500_model = {
> + .dr_num = 16,
> + .dr = 0x0,
> + .str = 0x40,
> + .bcr = 0x48,
> + .hcr = 0x4c,
> + .bie = 0x50,
> + .hie = 0x58,
> +};
> +
> +static const struct aspeed_mbox_model ast2600_model = {
> + .dr_num = 32,
> + .dr = 0x0,
> + .str = 0x80,
> + .bcr = 0x90,
> + .hcr = 0x94,
> + .bie = 0xa0,
> + .hie = 0xb0,
> +};
> +
> +static const struct of_device_id aspeed_mbox_match[] = {
> + { .compatible = "aspeed,ast2400-mbox",
> + .data = &ast2400_model },
> + { .compatible = "aspeed,ast2500-mbox",
> + .data = &ast2500_model },
> + { .compatible = "aspeed,ast2600-mbox",
> + .data = &ast2600_model },
> + { },
> +};
> +
> +static struct platform_driver aspeed_mbox_driver = {
> + .driver = {
> + .name = DEVICE_NAME,
> + .of_match_table = aspeed_mbox_match,
> + },
> + .probe = aspeed_mbox_probe,
> + .remove = aspeed_mbox_remove,
> +};
> +
> +module_platform_driver(aspeed_mbox_driver);
> +MODULE_DEVICE_TABLE(of, aspeed_mbox_match);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Cyril Bur <cyrilbur@gmail.com>");
> +MODULE_AUTHOR("Chia-Wei Wang <chiawei_wang@aspeedtech.com");
> +MODULE_DESCRIPTION("Aspeed mailbox device driver");
> --
> 2.17.1
>
_______________________________________________
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] 23+ messages in thread* Re: [PATCH v2 1/2] soc: aspeed: Add LPC mailbox support
@ 2022-03-18 7:05 ` Joel Stanley
0 siblings, 0 replies; 23+ messages in thread
From: Joel Stanley @ 2022-03-18 7:05 UTC (permalink / raw)
To: Chia-Wei Wang, Arnd Bergmann
Cc: devicetree, linux-aspeed, Andrew Jeffery, OpenBMC Maillist,
Linux Kernel Mailing List, Rob Herring, Cyril Bur, Linux ARM
On Tue, 17 Aug 2021 at 02:58, Chia-Wei Wang <chiawei_wang@aspeedtech.com> wrote:
>
> The LPC mailbox controller consists of multiple general
> registers for the communication between the Host and the BMC.
> The interrupts for data update signaling are also introduced.
The concern with this approach is the userspace API. We don't want
every driver to invent it's own way of communicating with usersapce.
I know when Cyril first submitted this driver it was suggested to use
the kernel mailbox subsystem. After investigation it was decided that
this hardware is not a fit; although it is a mailbox it is more like a
IPMI BT or KCS device, that needs an interface to push data to and
from userspace over the device.
Chai-Wei, can you link to some userspace programs that use the
existing ioctl interface?
Arnd, do you have any suggestions?
>
> Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/soc/aspeed/Kconfig | 10 +
> drivers/soc/aspeed/Makefile | 9 +-
> drivers/soc/aspeed/aspeed-lpc-mbox.c | 418 +++++++++++++++++++++++++++
> 3 files changed, 433 insertions(+), 4 deletions(-)
> create mode 100644 drivers/soc/aspeed/aspeed-lpc-mbox.c
>
> diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
> index 243ca196e6ad..5b31a6e2620c 100644
> --- a/drivers/soc/aspeed/Kconfig
> +++ b/drivers/soc/aspeed/Kconfig
> @@ -24,6 +24,16 @@ config ASPEED_LPC_SNOOP
> allows the BMC to listen on and save the data written by
> the host to an arbitrary LPC I/O port.
>
> +config ASPEED_LPC_MAILBOX
> + tristate "ASPEED LPC mailbox support"
> + select REGMAP
> + select MFD_SYSCON
> + default ARCH_ASPEED
> + help
> + Provides a driver to control the LPC mailbox which possesses
> + up to 32 data registers for the communication between the Host
> + and the BMC over LPC.
> +
> config ASPEED_P2A_CTRL
> tristate "ASPEED P2A (VGA MMIO to BMC) bridge control"
> select REGMAP
> diff --git a/drivers/soc/aspeed/Makefile b/drivers/soc/aspeed/Makefile
> index fcab7192e1a4..cde6b4514c97 100644
> --- a/drivers/soc/aspeed/Makefile
> +++ b/drivers/soc/aspeed/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> -obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> -obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
> -obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o
> +obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> +obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> +obj-$(CONFIG_ASPEED_LPC_MAILBOX) += aspeed-lpc-mbox.o
> +obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
> +obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o
> diff --git a/drivers/soc/aspeed/aspeed-lpc-mbox.c b/drivers/soc/aspeed/aspeed-lpc-mbox.c
> new file mode 100644
> index 000000000000..a09ca6a175f7
> --- /dev/null
> +++ b/drivers/soc/aspeed/aspeed-lpc-mbox.c
> @@ -0,0 +1,418 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2017 IBM Corporation
> + * Copyright 2021 Aspeed Technology Inc.
> + */
> +#include <linux/interrupt.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/poll.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#define DEVICE_NAME "aspeed-mbox"
> +
> +#define ASPEED_MBOX_DR(dr, n) (dr + (n * 4))
> +#define ASPEED_MBOX_STR(str, n) (str + (n / 8) * 4)
> +#define ASPEED_MBOX_BIE(bie, n) (bie + (n / 8) * 4)
> +#define ASPEED_MBOX_HIE(hie, n) (hie + (n / 8) * 4)
> +
> +#define ASPEED_MBOX_BCR_RECV BIT(7)
> +#define ASPEED_MBOX_BCR_MASK BIT(1)
> +#define ASPEED_MBOX_BCR_SEND BIT(0)
> +
> +/* ioctl code */
> +#define ASPEED_MBOX_IOCTL 0xA3
> +#define ASPEED_MBOX_IOCTL_GET_SIZE \
> + _IOR(ASPEED_MBOX_IOCTL, 0, struct aspeed_mbox_ioctl_data)
> +
> +struct aspeed_mbox_ioctl_data {
> + unsigned int data;
> +};
> +
> +struct aspeed_mbox_model {
> + unsigned int dr_num;
> +
> + /* offsets to the MBOX registers */
> + unsigned int dr;
> + unsigned int str;
> + unsigned int bcr;
> + unsigned int hcr;
> + unsigned int bie;
> + unsigned int hie;
> +};
> +
> +struct aspeed_mbox {
> + struct miscdevice miscdev;
> + struct regmap *map;
> + unsigned int base;
> + wait_queue_head_t queue;
> + struct mutex mutex;
> + const struct aspeed_mbox_model *model;
> +};
> +
> +static atomic_t aspeed_mbox_open_count = ATOMIC_INIT(0);
> +
> +static u8 aspeed_mbox_inb(struct aspeed_mbox *mbox, int reg)
> +{
> + /*
> + * The mbox registers are actually only one byte but are addressed
> + * four bytes apart. The other three bytes are marked 'reserved',
> + * they *should* be zero but lets not rely on it.
> + * I am going to rely on the fact we can casually read/write to them...
> + */
> + unsigned int val = 0xff; /* If regmap throws an error return 0xff */
> + int rc = regmap_read(mbox->map, mbox->base + reg, &val);
> +
> + if (rc)
> + dev_err(mbox->miscdev.parent, "regmap_read() failed with "
> + "%d (reg: 0x%08x)\n", rc, reg);
> +
> + return val & 0xff;
> +}
> +
> +static void aspeed_mbox_outb(struct aspeed_mbox *mbox, u8 data, int reg)
> +{
> + int rc = regmap_write(mbox->map, mbox->base + reg, data);
> +
> + if (rc)
> + dev_err(mbox->miscdev.parent, "regmap_write() failed with "
> + "%d (data: %u reg: 0x%08x)\n", rc, data, reg);
> +}
> +
> +static struct aspeed_mbox *file_mbox(struct file *file)
> +{
> + return container_of(file->private_data, struct aspeed_mbox, miscdev);
> +}
> +
> +static int aspeed_mbox_open(struct inode *inode, struct file *file)
> +{
> + struct aspeed_mbox *mbox = file_mbox(file);
> + const struct aspeed_mbox_model *model = mbox->model;
> +
> + if (atomic_inc_return(&aspeed_mbox_open_count) == 1) {
> + /*
> + * Clear the interrupt status bit if it was left on and unmask
> + * interrupts.
> + * ASPEED_MBOX_BCR_RECV bit is W1C, this also unmasks in 1 step
> + */
> + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV, model->bcr);
> + return 0;
> + }
> +
> + atomic_dec(&aspeed_mbox_open_count);
> + return -EBUSY;
> +}
> +
> +static ssize_t aspeed_mbox_read(struct file *file, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct aspeed_mbox *mbox = file_mbox(file);
> + const struct aspeed_mbox_model *model = mbox->model;
> + char __user *p = buf;
> + ssize_t ret;
> + int i;
> +
> + if (!access_ok(buf, count))
> + return -EFAULT;
> +
> + if (count + *ppos > model->dr_num)
> + return -EINVAL;
> +
> + if (file->f_flags & O_NONBLOCK) {
> + if (!(aspeed_mbox_inb(mbox, model->bcr) &
> + ASPEED_MBOX_BCR_RECV))
> + return -EAGAIN;
> + } else if (wait_event_interruptible(mbox->queue,
> + aspeed_mbox_inb(mbox, model->bcr) &
> + ASPEED_MBOX_BCR_RECV)) {
> + return -ERESTARTSYS;
> + }
> +
> + mutex_lock(&mbox->mutex);
> +
> + for (i = *ppos; count > 0 && i < model->dr_num; i++) {
> + uint8_t reg = aspeed_mbox_inb(mbox, ASPEED_MBOX_DR(model->dr, i));
> +
> + ret = __put_user(reg, p);
> + if (ret)
> + goto out_unlock;
> +
> + p++;
> + count--;
> + }
> +
> + /* ASPEED_MBOX_BCR_RECV bit is write to clear, this also unmasks in 1 step */
> + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV, model->bcr);
> + ret = p - buf;
> +
> +out_unlock:
> + mutex_unlock(&mbox->mutex);
> + return ret;
> +}
> +
> +static ssize_t aspeed_mbox_write(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct aspeed_mbox *mbox = file_mbox(file);
> + const struct aspeed_mbox_model *model = mbox->model;
> + const char __user *p = buf;
> + ssize_t ret;
> + char c;
> + int i;
> +
> + if (!access_ok(buf, count))
> + return -EFAULT;
> +
> + if (count + *ppos > model->dr_num)
> + return -EINVAL;
> +
> + mutex_lock(&mbox->mutex);
> +
> + for (i = *ppos; count > 0 && i < model->dr_num; i++) {
> + ret = __get_user(c, p);
> + if (ret)
> + goto out_unlock;
> +
> + aspeed_mbox_outb(mbox, c, ASPEED_MBOX_DR(model->dr, i));
> + p++;
> + count--;
> + }
> +
> + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_SEND, model->bcr);
> + ret = p - buf;
> +
> +out_unlock:
> + mutex_unlock(&mbox->mutex);
> + return ret;
> +}
> +
> +static __poll_t aspeed_mbox_poll(struct file *file, poll_table *wait)
> +{
> + struct aspeed_mbox *mbox = file_mbox(file);
> + const struct aspeed_mbox_model *model = mbox->model;
> + __poll_t mask = 0;
> +
> + poll_wait(file, &mbox->queue, wait);
> +
> + if (aspeed_mbox_inb(mbox, model->bcr) & ASPEED_MBOX_BCR_RECV)
> + mask |= POLLIN;
> +
> + return mask;
> +}
> +
> +static int aspeed_mbox_release(struct inode *inode, struct file *file)
> +{
> + atomic_dec(&aspeed_mbox_open_count);
> + return 0;
> +}
> +
> +static long aspeed_mbox_ioctl(struct file *file, unsigned int cmd,
> + unsigned long param)
> +{
> + long ret = 0;
> + struct aspeed_mbox *mbox = file_mbox(file);
> + const struct aspeed_mbox_model *model = mbox->model;
> + struct aspeed_mbox_ioctl_data data;
> +
> + switch (cmd) {
> + case ASPEED_MBOX_IOCTL_GET_SIZE:
> + data.data = model->dr_num;
> + if (copy_to_user((void __user *)param, &data, sizeof(data)))
> + ret = -EFAULT;
> + break;
> + default:
> + ret = -ENOTTY;
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static const struct file_operations aspeed_mbox_fops = {
> + .owner = THIS_MODULE,
> + .llseek = no_seek_end_llseek,
> + .read = aspeed_mbox_read,
> + .write = aspeed_mbox_write,
> + .open = aspeed_mbox_open,
> + .release = aspeed_mbox_release,
> + .poll = aspeed_mbox_poll,
> + .unlocked_ioctl = aspeed_mbox_ioctl,
> +};
> +
> +static irqreturn_t aspeed_mbox_irq(int irq, void *arg)
> +{
> + struct aspeed_mbox *mbox = arg;
> + const struct aspeed_mbox_model *model = mbox->model;
> +
> + if (!(aspeed_mbox_inb(mbox, model->bcr) & ASPEED_MBOX_BCR_RECV))
> + return IRQ_NONE;
> +
> + /*
> + * Leave the status bit set so that we know the data is for us,
> + * clear it once it has been read.
> + */
> +
> + /* Mask it off, we'll clear it when we the data gets read */
> + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_MASK, model->bcr);
> +
> + wake_up(&mbox->queue);
> + return IRQ_HANDLED;
> +}
> +
> +static int aspeed_mbox_config_irq(struct aspeed_mbox *mbox,
> + struct platform_device *pdev)
> +{
> + const struct aspeed_mbox_model *model = mbox->model;
> + struct device *dev = &pdev->dev;
> + int i, rc, irq;
> +
> + irq = irq_of_parse_and_map(dev->of_node, 0);
> + if (!irq)
> + return -ENODEV;
> +
> + rc = devm_request_irq(dev, irq, aspeed_mbox_irq,
> + IRQF_SHARED, DEVICE_NAME, mbox);
> + if (rc < 0) {
> + dev_err(dev, "Unable to request IRQ %d\n", irq);
> + return rc;
> + }
> +
> + /*
> + * Disable all register based interrupts.
> + */
> + for (i = 0; i < model->dr_num / 8; ++i)
> + aspeed_mbox_outb(mbox, 0x00, ASPEED_MBOX_BIE(model->bie, i));
> +
> + /* These registers are write one to clear. Clear them. */
> + for (i = 0; i < model->dr_num / 8; ++i)
> + aspeed_mbox_outb(mbox, 0xff, ASPEED_MBOX_STR(model->str, i));
> +
> + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV, model->bcr);
> + return 0;
> +}
> +
> +static int aspeed_mbox_probe(struct platform_device *pdev)
> +{
> + struct aspeed_mbox *mbox;
> + struct device *dev;
> + int rc;
> +
> + dev = &pdev->dev;
> +
> + mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> + if (!mbox)
> + return -ENOMEM;
> +
> + dev_set_drvdata(&pdev->dev, mbox);
> +
> + rc = of_property_read_u32(dev->of_node, "reg", &mbox->base);
> + if (rc) {
> + dev_err(dev, "Couldn't read reg device tree property\n");
> + return rc;
> + }
> +
> + mbox->model = of_device_get_match_data(dev);
> + if (IS_ERR(mbox->model)) {
> + dev_err(dev, "Couldn't get model data\n");
> + return -ENODEV;
> + }
> +
> + mbox->map = syscon_node_to_regmap(
> + pdev->dev.parent->of_node);
> + if (IS_ERR(mbox->map)) {
> + dev_err(dev, "Couldn't get regmap\n");
> + return -ENODEV;
> + }
> +
> + mutex_init(&mbox->mutex);
> + init_waitqueue_head(&mbox->queue);
> +
> + mbox->miscdev.minor = MISC_DYNAMIC_MINOR;
> + mbox->miscdev.name = DEVICE_NAME;
> + mbox->miscdev.fops = &aspeed_mbox_fops;
> + mbox->miscdev.parent = dev;
> + rc = misc_register(&mbox->miscdev);
> + if (rc) {
> + dev_err(dev, "Unable to register device\n");
> + return rc;
> + }
> +
> + rc = aspeed_mbox_config_irq(mbox, pdev);
> + if (rc) {
> + dev_err(dev, "Failed to configure IRQ\n");
> + misc_deregister(&mbox->miscdev);
> + return rc;
> + }
> +
> + return 0;
> +}
> +
> +static int aspeed_mbox_remove(struct platform_device *pdev)
> +{
> + struct aspeed_mbox *mbox = dev_get_drvdata(&pdev->dev);
> +
> + misc_deregister(&mbox->miscdev);
> +
> + return 0;
> +}
> +
> +static const struct aspeed_mbox_model ast2400_model = {
> + .dr_num = 16,
> + .dr = 0x0,
> + .str = 0x40,
> + .bcr = 0x48,
> + .hcr = 0x4c,
> + .bie = 0x50,
> + .hie = 0x58,
> +};
> +
> +static const struct aspeed_mbox_model ast2500_model = {
> + .dr_num = 16,
> + .dr = 0x0,
> + .str = 0x40,
> + .bcr = 0x48,
> + .hcr = 0x4c,
> + .bie = 0x50,
> + .hie = 0x58,
> +};
> +
> +static const struct aspeed_mbox_model ast2600_model = {
> + .dr_num = 32,
> + .dr = 0x0,
> + .str = 0x80,
> + .bcr = 0x90,
> + .hcr = 0x94,
> + .bie = 0xa0,
> + .hie = 0xb0,
> +};
> +
> +static const struct of_device_id aspeed_mbox_match[] = {
> + { .compatible = "aspeed,ast2400-mbox",
> + .data = &ast2400_model },
> + { .compatible = "aspeed,ast2500-mbox",
> + .data = &ast2500_model },
> + { .compatible = "aspeed,ast2600-mbox",
> + .data = &ast2600_model },
> + { },
> +};
> +
> +static struct platform_driver aspeed_mbox_driver = {
> + .driver = {
> + .name = DEVICE_NAME,
> + .of_match_table = aspeed_mbox_match,
> + },
> + .probe = aspeed_mbox_probe,
> + .remove = aspeed_mbox_remove,
> +};
> +
> +module_platform_driver(aspeed_mbox_driver);
> +MODULE_DEVICE_TABLE(of, aspeed_mbox_match);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Cyril Bur <cyrilbur@gmail.com>");
> +MODULE_AUTHOR("Chia-Wei Wang <chiawei_wang@aspeedtech.com");
> +MODULE_DESCRIPTION("Aspeed mailbox device driver");
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH v2 1/2] soc: aspeed: Add LPC mailbox support
2022-03-18 7:05 ` Joel Stanley
(?)
(?)
@ 2022-03-18 7:59 ` ChiaWei Wang
-1 siblings, 0 replies; 23+ messages in thread
From: ChiaWei Wang @ 2022-03-18 7:59 UTC (permalink / raw)
To: linux-aspeed
Hi Joel,
> From: Joel Stanley <joel@jms.id.au>
> Sent: Friday, March 18, 2022 3:06 PM
>
> On Tue, 17 Aug 2021 at 02:58, Chia-Wei Wang
> <chiawei_wang@aspeedtech.com> wrote:
> >
> > The LPC mailbox controller consists of multiple general registers for
> > the communication between the Host and the BMC.
> > The interrupts for data update signaling are also introduced.
>
> The concern with this approach is the userspace API. We don't want every
> driver to invent it's own way of communicating with usersapce.
>
> I know when Cyril first submitted this driver it was suggested to use the kernel
> mailbox subsystem. After investigation it was decided that this hardware is not
> a fit; although it is a mailbox it is more like a IPMI BT or KCS device, that needs
> an interface to push data to and from userspace over the device.
>
Thanks for the explanation.
The current implementation exports the mailbox register access in the file (device node) read/write way.
The read/write behavior is slightly different than a usual text file though...
The IOCTL is used only to retrieve the number of mailbox registers available.
We can consider exporting this through sysfs as a driver property, perhaps?
Is the IOCTL part the major concern? Or the file-based read/write also needs to be re-considered?
> Chai-Wei, can you link to some userspace programs that use the existing ioctl
> interface?
The kcs_bt_test APP in Aspeed GitHub has the example to access LPC mailbox with read()/write() toward its device node.
However, as kcs_bt_test is for internal test only, it is now removed in the latest release.
You can still find the source in the early release tags (e.g. v00.01.00).
https://github.com/AspeedTech-BMC/aspeed_app/blob/v00.01.00/kcs_bt_test/kcs_bt_test.c#L157
Thanks,
Chiawei
>
> Arnd, do you have any suggestions?
>
> >
> > Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com>
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > drivers/soc/aspeed/Kconfig | 10 +
> > drivers/soc/aspeed/Makefile | 9 +-
> > drivers/soc/aspeed/aspeed-lpc-mbox.c | 418
> > +++++++++++++++++++++++++++
> > 3 files changed, 433 insertions(+), 4 deletions(-) create mode
> > 100644 drivers/soc/aspeed/aspeed-lpc-mbox.c
> >
> > diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
> > index 243ca196e6ad..5b31a6e2620c 100644
> > --- a/drivers/soc/aspeed/Kconfig
> > +++ b/drivers/soc/aspeed/Kconfig
> > @@ -24,6 +24,16 @@ config ASPEED_LPC_SNOOP
> > allows the BMC to listen on and save the data written by
> > the host to an arbitrary LPC I/O port.
> >
> > +config ASPEED_LPC_MAILBOX
> > + tristate "ASPEED LPC mailbox support"
> > + select REGMAP
> > + select MFD_SYSCON
> > + default ARCH_ASPEED
> > + help
> > + Provides a driver to control the LPC mailbox which possesses
> > + up to 32 data registers for the communication between the Host
> > + and the BMC over LPC.
> > +
> > config ASPEED_P2A_CTRL
> > tristate "ASPEED P2A (VGA MMIO to BMC) bridge control"
> > select REGMAP
> > diff --git a/drivers/soc/aspeed/Makefile b/drivers/soc/aspeed/Makefile
> > index fcab7192e1a4..cde6b4514c97 100644
> > --- a/drivers/soc/aspeed/Makefile
> > +++ b/drivers/soc/aspeed/Makefile
> > @@ -1,5 +1,6 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > -obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> > -obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> > -obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
> > -obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o
> > +obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> > +obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> > +obj-$(CONFIG_ASPEED_LPC_MAILBOX) += aspeed-lpc-mbox.o
> > +obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
> > +obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o
> > diff --git a/drivers/soc/aspeed/aspeed-lpc-mbox.c
> > b/drivers/soc/aspeed/aspeed-lpc-mbox.c
> > new file mode 100644
> > index 000000000000..a09ca6a175f7
> > --- /dev/null
> > +++ b/drivers/soc/aspeed/aspeed-lpc-mbox.c
> > @@ -0,0 +1,418 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright 2017 IBM Corporation
> > + * Copyright 2021 Aspeed Technology Inc.
> > + */
> > +#include <linux/interrupt.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/miscdevice.h>
> > +#include <linux/module.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/poll.h>
> > +#include <linux/regmap.h>
> > +#include <linux/slab.h>
> > +
> > +#define DEVICE_NAME "aspeed-mbox"
> > +
> > +#define ASPEED_MBOX_DR(dr, n) (dr + (n * 4))
> > +#define ASPEED_MBOX_STR(str, n) (str + (n / 8) * 4)
> > +#define ASPEED_MBOX_BIE(bie, n) (bie + (n / 8) * 4)
> > +#define ASPEED_MBOX_HIE(hie, n) (hie + (n / 8) * 4)
> > +
> > +#define ASPEED_MBOX_BCR_RECV BIT(7)
> > +#define ASPEED_MBOX_BCR_MASK BIT(1)
> > +#define ASPEED_MBOX_BCR_SEND BIT(0)
> > +
> > +/* ioctl code */
> > +#define ASPEED_MBOX_IOCTL 0xA3
> > +#define ASPEED_MBOX_IOCTL_GET_SIZE \
> > + _IOR(ASPEED_MBOX_IOCTL, 0, struct aspeed_mbox_ioctl_data)
> > +
> > +struct aspeed_mbox_ioctl_data {
> > + unsigned int data;
> > +};
> > +
> > +struct aspeed_mbox_model {
> > + unsigned int dr_num;
> > +
> > + /* offsets to the MBOX registers */
> > + unsigned int dr;
> > + unsigned int str;
> > + unsigned int bcr;
> > + unsigned int hcr;
> > + unsigned int bie;
> > + unsigned int hie;
> > +};
> > +
> > +struct aspeed_mbox {
> > + struct miscdevice miscdev;
> > + struct regmap *map;
> > + unsigned int base;
> > + wait_queue_head_t queue;
> > + struct mutex mutex;
> > + const struct aspeed_mbox_model *model; };
> > +
> > +static atomic_t aspeed_mbox_open_count = ATOMIC_INIT(0);
> > +
> > +static u8 aspeed_mbox_inb(struct aspeed_mbox *mbox, int reg) {
> > + /*
> > + * The mbox registers are actually only one byte but are addressed
> > + * four bytes apart. The other three bytes are marked 'reserved',
> > + * they *should* be zero but lets not rely on it.
> > + * I am going to rely on the fact we can casually read/write to
> them...
> > + */
> > + unsigned int val = 0xff; /* If regmap throws an error return 0xff */
> > + int rc = regmap_read(mbox->map, mbox->base + reg, &val);
> > +
> > + if (rc)
> > + dev_err(mbox->miscdev.parent, "regmap_read() failed
> with "
> > + "%d (reg: 0x%08x)\n", rc, reg);
> > +
> > + return val & 0xff;
> > +}
> > +
> > +static void aspeed_mbox_outb(struct aspeed_mbox *mbox, u8 data, int
> > +reg) {
> > + int rc = regmap_write(mbox->map, mbox->base + reg, data);
> > +
> > + if (rc)
> > + dev_err(mbox->miscdev.parent, "regmap_write() failed
> with "
> > + "%d (data: %u reg: 0x%08x)\n", rc, data, reg);
> > +}
> > +
> > +static struct aspeed_mbox *file_mbox(struct file *file) {
> > + return container_of(file->private_data, struct aspeed_mbox,
> > +miscdev); }
> > +
> > +static int aspeed_mbox_open(struct inode *inode, struct file *file) {
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > +
> > + if (atomic_inc_return(&aspeed_mbox_open_count) == 1) {
> > + /*
> > + * Clear the interrupt status bit if it was left on and
> unmask
> > + * interrupts.
> > + * ASPEED_MBOX_BCR_RECV bit is W1C, this also
> unmasks in 1 step
> > + */
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV,
> model->bcr);
> > + return 0;
> > + }
> > +
> > + atomic_dec(&aspeed_mbox_open_count);
> > + return -EBUSY;
> > +}
> > +
> > +static ssize_t aspeed_mbox_read(struct file *file, char __user *buf,
> > + size_t count, loff_t *ppos) {
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + char __user *p = buf;
> > + ssize_t ret;
> > + int i;
> > +
> > + if (!access_ok(buf, count))
> > + return -EFAULT;
> > +
> > + if (count + *ppos > model->dr_num)
> > + return -EINVAL;
> > +
> > + if (file->f_flags & O_NONBLOCK) {
> > + if (!(aspeed_mbox_inb(mbox, model->bcr) &
> > + ASPEED_MBOX_BCR_RECV))
> > + return -EAGAIN;
> > + } else if (wait_event_interruptible(mbox->queue,
> > + aspeed_mbox_inb(mbox, model->bcr)
> &
> > + ASPEED_MBOX_BCR_RECV)) {
> > + return -ERESTARTSYS;
> > + }
> > +
> > + mutex_lock(&mbox->mutex);
> > +
> > + for (i = *ppos; count > 0 && i < model->dr_num; i++) {
> > + uint8_t reg = aspeed_mbox_inb(mbox,
> > + ASPEED_MBOX_DR(model->dr, i));
> > +
> > + ret = __put_user(reg, p);
> > + if (ret)
> > + goto out_unlock;
> > +
> > + p++;
> > + count--;
> > + }
> > +
> > + /* ASPEED_MBOX_BCR_RECV bit is write to clear, this also
> unmasks in 1 step */
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV,
> model->bcr);
> > + ret = p - buf;
> > +
> > +out_unlock:
> > + mutex_unlock(&mbox->mutex);
> > + return ret;
> > +}
> > +
> > +static ssize_t aspeed_mbox_write(struct file *file, const char __user *buf,
> > + size_t count, loff_t *ppos) {
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + const char __user *p = buf;
> > + ssize_t ret;
> > + char c;
> > + int i;
> > +
> > + if (!access_ok(buf, count))
> > + return -EFAULT;
> > +
> > + if (count + *ppos > model->dr_num)
> > + return -EINVAL;
> > +
> > + mutex_lock(&mbox->mutex);
> > +
> > + for (i = *ppos; count > 0 && i < model->dr_num; i++) {
> > + ret = __get_user(c, p);
> > + if (ret)
> > + goto out_unlock;
> > +
> > + aspeed_mbox_outb(mbox, c,
> ASPEED_MBOX_DR(model->dr, i));
> > + p++;
> > + count--;
> > + }
> > +
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_SEND,
> model->bcr);
> > + ret = p - buf;
> > +
> > +out_unlock:
> > + mutex_unlock(&mbox->mutex);
> > + return ret;
> > +}
> > +
> > +static __poll_t aspeed_mbox_poll(struct file *file, poll_table *wait)
> > +{
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + __poll_t mask = 0;
> > +
> > + poll_wait(file, &mbox->queue, wait);
> > +
> > + if (aspeed_mbox_inb(mbox, model->bcr) &
> ASPEED_MBOX_BCR_RECV)
> > + mask |= POLLIN;
> > +
> > + return mask;
> > +}
> > +
> > +static int aspeed_mbox_release(struct inode *inode, struct file
> > +*file) {
> > + atomic_dec(&aspeed_mbox_open_count);
> > + return 0;
> > +}
> > +
> > +static long aspeed_mbox_ioctl(struct file *file, unsigned int cmd,
> > + unsigned long param) {
> > + long ret = 0;
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + struct aspeed_mbox_ioctl_data data;
> > +
> > + switch (cmd) {
> > + case ASPEED_MBOX_IOCTL_GET_SIZE:
> > + data.data = model->dr_num;
> > + if (copy_to_user((void __user *)param, &data,
> sizeof(data)))
> > + ret = -EFAULT;
> > + break;
> > + default:
> > + ret = -ENOTTY;
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static const struct file_operations aspeed_mbox_fops = {
> > + .owner = THIS_MODULE,
> > + .llseek = no_seek_end_llseek,
> > + .read = aspeed_mbox_read,
> > + .write = aspeed_mbox_write,
> > + .open = aspeed_mbox_open,
> > + .release = aspeed_mbox_release,
> > + .poll = aspeed_mbox_poll,
> > + .unlocked_ioctl = aspeed_mbox_ioctl, };
> > +
> > +static irqreturn_t aspeed_mbox_irq(int irq, void *arg) {
> > + struct aspeed_mbox *mbox = arg;
> > + const struct aspeed_mbox_model *model = mbox->model;
> > +
> > + if (!(aspeed_mbox_inb(mbox, model->bcr) &
> ASPEED_MBOX_BCR_RECV))
> > + return IRQ_NONE;
> > +
> > + /*
> > + * Leave the status bit set so that we know the data is for us,
> > + * clear it once it has been read.
> > + */
> > +
> > + /* Mask it off, we'll clear it when we the data gets read */
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_MASK,
> model->bcr);
> > +
> > + wake_up(&mbox->queue);
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int aspeed_mbox_config_irq(struct aspeed_mbox *mbox,
> > + struct platform_device *pdev) {
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + struct device *dev = &pdev->dev;
> > + int i, rc, irq;
> > +
> > + irq = irq_of_parse_and_map(dev->of_node, 0);
> > + if (!irq)
> > + return -ENODEV;
> > +
> > + rc = devm_request_irq(dev, irq, aspeed_mbox_irq,
> > + IRQF_SHARED, DEVICE_NAME, mbox);
> > + if (rc < 0) {
> > + dev_err(dev, "Unable to request IRQ %d\n", irq);
> > + return rc;
> > + }
> > +
> > + /*
> > + * Disable all register based interrupts.
> > + */
> > + for (i = 0; i < model->dr_num / 8; ++i)
> > + aspeed_mbox_outb(mbox, 0x00,
> > + ASPEED_MBOX_BIE(model->bie, i));
> > +
> > + /* These registers are write one to clear. Clear them. */
> > + for (i = 0; i < model->dr_num / 8; ++i)
> > + aspeed_mbox_outb(mbox, 0xff,
> > + ASPEED_MBOX_STR(model->str, i));
> > +
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV,
> model->bcr);
> > + return 0;
> > +}
> > +
> > +static int aspeed_mbox_probe(struct platform_device *pdev) {
> > + struct aspeed_mbox *mbox;
> > + struct device *dev;
> > + int rc;
> > +
> > + dev = &pdev->dev;
> > +
> > + mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> > + if (!mbox)
> > + return -ENOMEM;
> > +
> > + dev_set_drvdata(&pdev->dev, mbox);
> > +
> > + rc = of_property_read_u32(dev->of_node, "reg", &mbox->base);
> > + if (rc) {
> > + dev_err(dev, "Couldn't read reg device tree property\n");
> > + return rc;
> > + }
> > +
> > + mbox->model = of_device_get_match_data(dev);
> > + if (IS_ERR(mbox->model)) {
> > + dev_err(dev, "Couldn't get model data\n");
> > + return -ENODEV;
> > + }
> > +
> > + mbox->map = syscon_node_to_regmap(
> > + pdev->dev.parent->of_node);
> > + if (IS_ERR(mbox->map)) {
> > + dev_err(dev, "Couldn't get regmap\n");
> > + return -ENODEV;
> > + }
> > +
> > + mutex_init(&mbox->mutex);
> > + init_waitqueue_head(&mbox->queue);
> > +
> > + mbox->miscdev.minor = MISC_DYNAMIC_MINOR;
> > + mbox->miscdev.name = DEVICE_NAME;
> > + mbox->miscdev.fops = &aspeed_mbox_fops;
> > + mbox->miscdev.parent = dev;
> > + rc = misc_register(&mbox->miscdev);
> > + if (rc) {
> > + dev_err(dev, "Unable to register device\n");
> > + return rc;
> > + }
> > +
> > + rc = aspeed_mbox_config_irq(mbox, pdev);
> > + if (rc) {
> > + dev_err(dev, "Failed to configure IRQ\n");
> > + misc_deregister(&mbox->miscdev);
> > + return rc;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int aspeed_mbox_remove(struct platform_device *pdev) {
> > + struct aspeed_mbox *mbox = dev_get_drvdata(&pdev->dev);
> > +
> > + misc_deregister(&mbox->miscdev);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct aspeed_mbox_model ast2400_model = {
> > + .dr_num = 16,
> > + .dr = 0x0,
> > + .str = 0x40,
> > + .bcr = 0x48,
> > + .hcr = 0x4c,
> > + .bie = 0x50,
> > + .hie = 0x58,
> > +};
> > +
> > +static const struct aspeed_mbox_model ast2500_model = {
> > + .dr_num = 16,
> > + .dr = 0x0,
> > + .str = 0x40,
> > + .bcr = 0x48,
> > + .hcr = 0x4c,
> > + .bie = 0x50,
> > + .hie = 0x58,
> > +};
> > +
> > +static const struct aspeed_mbox_model ast2600_model = {
> > + .dr_num = 32,
> > + .dr = 0x0,
> > + .str = 0x80,
> > + .bcr = 0x90,
> > + .hcr = 0x94,
> > + .bie = 0xa0,
> > + .hie = 0xb0,
> > +};
> > +
> > +static const struct of_device_id aspeed_mbox_match[] = {
> > + { .compatible = "aspeed,ast2400-mbox",
> > + .data = &ast2400_model },
> > + { .compatible = "aspeed,ast2500-mbox",
> > + .data = &ast2500_model },
> > + { .compatible = "aspeed,ast2600-mbox",
> > + .data = &ast2600_model },
> > + { },
> > +};
> > +
> > +static struct platform_driver aspeed_mbox_driver = {
> > + .driver = {
> > + .name = DEVICE_NAME,
> > + .of_match_table = aspeed_mbox_match,
> > + },
> > + .probe = aspeed_mbox_probe,
> > + .remove = aspeed_mbox_remove,
> > +};
> > +
> > +module_platform_driver(aspeed_mbox_driver);
> > +MODULE_DEVICE_TABLE(of, aspeed_mbox_match);
> MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Cyril Bur <cyrilbur@gmail.com>");
> > +MODULE_AUTHOR("Chia-Wei Wang <chiawei_wang@aspeedtech.com");
> > +MODULE_DESCRIPTION("Aspeed mailbox device driver");
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 23+ messages in thread* RE: [PATCH v2 1/2] soc: aspeed: Add LPC mailbox support
@ 2022-03-18 7:59 ` ChiaWei Wang
0 siblings, 0 replies; 23+ messages in thread
From: ChiaWei Wang @ 2022-03-18 7:59 UTC (permalink / raw)
To: Joel Stanley, Arnd Bergmann
Cc: Rob Herring, Andrew Jeffery, Cyril Bur, devicetree, Linux ARM,
linux-aspeed, Linux Kernel Mailing List, OpenBMC Maillist
Hi Joel,
> From: Joel Stanley <joel@jms.id.au>
> Sent: Friday, March 18, 2022 3:06 PM
>
> On Tue, 17 Aug 2021 at 02:58, Chia-Wei Wang
> <chiawei_wang@aspeedtech.com> wrote:
> >
> > The LPC mailbox controller consists of multiple general registers for
> > the communication between the Host and the BMC.
> > The interrupts for data update signaling are also introduced.
>
> The concern with this approach is the userspace API. We don't want every
> driver to invent it's own way of communicating with usersapce.
>
> I know when Cyril first submitted this driver it was suggested to use the kernel
> mailbox subsystem. After investigation it was decided that this hardware is not
> a fit; although it is a mailbox it is more like a IPMI BT or KCS device, that needs
> an interface to push data to and from userspace over the device.
>
Thanks for the explanation.
The current implementation exports the mailbox register access in the file (device node) read/write way.
The read/write behavior is slightly different than a usual text file though...
The IOCTL is used only to retrieve the number of mailbox registers available.
We can consider exporting this through sysfs as a driver property, perhaps?
Is the IOCTL part the major concern? Or the file-based read/write also needs to be re-considered?
> Chai-Wei, can you link to some userspace programs that use the existing ioctl
> interface?
The kcs_bt_test APP in Aspeed GitHub has the example to access LPC mailbox with read()/write() toward its device node.
However, as kcs_bt_test is for internal test only, it is now removed in the latest release.
You can still find the source in the early release tags (e.g. v00.01.00).
https://github.com/AspeedTech-BMC/aspeed_app/blob/v00.01.00/kcs_bt_test/kcs_bt_test.c#L157
Thanks,
Chiawei
>
> Arnd, do you have any suggestions?
>
> >
> > Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com>
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > drivers/soc/aspeed/Kconfig | 10 +
> > drivers/soc/aspeed/Makefile | 9 +-
> > drivers/soc/aspeed/aspeed-lpc-mbox.c | 418
> > +++++++++++++++++++++++++++
> > 3 files changed, 433 insertions(+), 4 deletions(-) create mode
> > 100644 drivers/soc/aspeed/aspeed-lpc-mbox.c
> >
> > diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
> > index 243ca196e6ad..5b31a6e2620c 100644
> > --- a/drivers/soc/aspeed/Kconfig
> > +++ b/drivers/soc/aspeed/Kconfig
> > @@ -24,6 +24,16 @@ config ASPEED_LPC_SNOOP
> > allows the BMC to listen on and save the data written by
> > the host to an arbitrary LPC I/O port.
> >
> > +config ASPEED_LPC_MAILBOX
> > + tristate "ASPEED LPC mailbox support"
> > + select REGMAP
> > + select MFD_SYSCON
> > + default ARCH_ASPEED
> > + help
> > + Provides a driver to control the LPC mailbox which possesses
> > + up to 32 data registers for the communication between the Host
> > + and the BMC over LPC.
> > +
> > config ASPEED_P2A_CTRL
> > tristate "ASPEED P2A (VGA MMIO to BMC) bridge control"
> > select REGMAP
> > diff --git a/drivers/soc/aspeed/Makefile b/drivers/soc/aspeed/Makefile
> > index fcab7192e1a4..cde6b4514c97 100644
> > --- a/drivers/soc/aspeed/Makefile
> > +++ b/drivers/soc/aspeed/Makefile
> > @@ -1,5 +1,6 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > -obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> > -obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> > -obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
> > -obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o
> > +obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> > +obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> > +obj-$(CONFIG_ASPEED_LPC_MAILBOX) += aspeed-lpc-mbox.o
> > +obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
> > +obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o
> > diff --git a/drivers/soc/aspeed/aspeed-lpc-mbox.c
> > b/drivers/soc/aspeed/aspeed-lpc-mbox.c
> > new file mode 100644
> > index 000000000000..a09ca6a175f7
> > --- /dev/null
> > +++ b/drivers/soc/aspeed/aspeed-lpc-mbox.c
> > @@ -0,0 +1,418 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright 2017 IBM Corporation
> > + * Copyright 2021 Aspeed Technology Inc.
> > + */
> > +#include <linux/interrupt.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/miscdevice.h>
> > +#include <linux/module.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/poll.h>
> > +#include <linux/regmap.h>
> > +#include <linux/slab.h>
> > +
> > +#define DEVICE_NAME "aspeed-mbox"
> > +
> > +#define ASPEED_MBOX_DR(dr, n) (dr + (n * 4))
> > +#define ASPEED_MBOX_STR(str, n) (str + (n / 8) * 4)
> > +#define ASPEED_MBOX_BIE(bie, n) (bie + (n / 8) * 4)
> > +#define ASPEED_MBOX_HIE(hie, n) (hie + (n / 8) * 4)
> > +
> > +#define ASPEED_MBOX_BCR_RECV BIT(7)
> > +#define ASPEED_MBOX_BCR_MASK BIT(1)
> > +#define ASPEED_MBOX_BCR_SEND BIT(0)
> > +
> > +/* ioctl code */
> > +#define ASPEED_MBOX_IOCTL 0xA3
> > +#define ASPEED_MBOX_IOCTL_GET_SIZE \
> > + _IOR(ASPEED_MBOX_IOCTL, 0, struct aspeed_mbox_ioctl_data)
> > +
> > +struct aspeed_mbox_ioctl_data {
> > + unsigned int data;
> > +};
> > +
> > +struct aspeed_mbox_model {
> > + unsigned int dr_num;
> > +
> > + /* offsets to the MBOX registers */
> > + unsigned int dr;
> > + unsigned int str;
> > + unsigned int bcr;
> > + unsigned int hcr;
> > + unsigned int bie;
> > + unsigned int hie;
> > +};
> > +
> > +struct aspeed_mbox {
> > + struct miscdevice miscdev;
> > + struct regmap *map;
> > + unsigned int base;
> > + wait_queue_head_t queue;
> > + struct mutex mutex;
> > + const struct aspeed_mbox_model *model; };
> > +
> > +static atomic_t aspeed_mbox_open_count = ATOMIC_INIT(0);
> > +
> > +static u8 aspeed_mbox_inb(struct aspeed_mbox *mbox, int reg) {
> > + /*
> > + * The mbox registers are actually only one byte but are addressed
> > + * four bytes apart. The other three bytes are marked 'reserved',
> > + * they *should* be zero but lets not rely on it.
> > + * I am going to rely on the fact we can casually read/write to
> them...
> > + */
> > + unsigned int val = 0xff; /* If regmap throws an error return 0xff */
> > + int rc = regmap_read(mbox->map, mbox->base + reg, &val);
> > +
> > + if (rc)
> > + dev_err(mbox->miscdev.parent, "regmap_read() failed
> with "
> > + "%d (reg: 0x%08x)\n", rc, reg);
> > +
> > + return val & 0xff;
> > +}
> > +
> > +static void aspeed_mbox_outb(struct aspeed_mbox *mbox, u8 data, int
> > +reg) {
> > + int rc = regmap_write(mbox->map, mbox->base + reg, data);
> > +
> > + if (rc)
> > + dev_err(mbox->miscdev.parent, "regmap_write() failed
> with "
> > + "%d (data: %u reg: 0x%08x)\n", rc, data, reg);
> > +}
> > +
> > +static struct aspeed_mbox *file_mbox(struct file *file) {
> > + return container_of(file->private_data, struct aspeed_mbox,
> > +miscdev); }
> > +
> > +static int aspeed_mbox_open(struct inode *inode, struct file *file) {
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > +
> > + if (atomic_inc_return(&aspeed_mbox_open_count) == 1) {
> > + /*
> > + * Clear the interrupt status bit if it was left on and
> unmask
> > + * interrupts.
> > + * ASPEED_MBOX_BCR_RECV bit is W1C, this also
> unmasks in 1 step
> > + */
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV,
> model->bcr);
> > + return 0;
> > + }
> > +
> > + atomic_dec(&aspeed_mbox_open_count);
> > + return -EBUSY;
> > +}
> > +
> > +static ssize_t aspeed_mbox_read(struct file *file, char __user *buf,
> > + size_t count, loff_t *ppos) {
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + char __user *p = buf;
> > + ssize_t ret;
> > + int i;
> > +
> > + if (!access_ok(buf, count))
> > + return -EFAULT;
> > +
> > + if (count + *ppos > model->dr_num)
> > + return -EINVAL;
> > +
> > + if (file->f_flags & O_NONBLOCK) {
> > + if (!(aspeed_mbox_inb(mbox, model->bcr) &
> > + ASPEED_MBOX_BCR_RECV))
> > + return -EAGAIN;
> > + } else if (wait_event_interruptible(mbox->queue,
> > + aspeed_mbox_inb(mbox, model->bcr)
> &
> > + ASPEED_MBOX_BCR_RECV)) {
> > + return -ERESTARTSYS;
> > + }
> > +
> > + mutex_lock(&mbox->mutex);
> > +
> > + for (i = *ppos; count > 0 && i < model->dr_num; i++) {
> > + uint8_t reg = aspeed_mbox_inb(mbox,
> > + ASPEED_MBOX_DR(model->dr, i));
> > +
> > + ret = __put_user(reg, p);
> > + if (ret)
> > + goto out_unlock;
> > +
> > + p++;
> > + count--;
> > + }
> > +
> > + /* ASPEED_MBOX_BCR_RECV bit is write to clear, this also
> unmasks in 1 step */
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV,
> model->bcr);
> > + ret = p - buf;
> > +
> > +out_unlock:
> > + mutex_unlock(&mbox->mutex);
> > + return ret;
> > +}
> > +
> > +static ssize_t aspeed_mbox_write(struct file *file, const char __user *buf,
> > + size_t count, loff_t *ppos) {
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + const char __user *p = buf;
> > + ssize_t ret;
> > + char c;
> > + int i;
> > +
> > + if (!access_ok(buf, count))
> > + return -EFAULT;
> > +
> > + if (count + *ppos > model->dr_num)
> > + return -EINVAL;
> > +
> > + mutex_lock(&mbox->mutex);
> > +
> > + for (i = *ppos; count > 0 && i < model->dr_num; i++) {
> > + ret = __get_user(c, p);
> > + if (ret)
> > + goto out_unlock;
> > +
> > + aspeed_mbox_outb(mbox, c,
> ASPEED_MBOX_DR(model->dr, i));
> > + p++;
> > + count--;
> > + }
> > +
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_SEND,
> model->bcr);
> > + ret = p - buf;
> > +
> > +out_unlock:
> > + mutex_unlock(&mbox->mutex);
> > + return ret;
> > +}
> > +
> > +static __poll_t aspeed_mbox_poll(struct file *file, poll_table *wait)
> > +{
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + __poll_t mask = 0;
> > +
> > + poll_wait(file, &mbox->queue, wait);
> > +
> > + if (aspeed_mbox_inb(mbox, model->bcr) &
> ASPEED_MBOX_BCR_RECV)
> > + mask |= POLLIN;
> > +
> > + return mask;
> > +}
> > +
> > +static int aspeed_mbox_release(struct inode *inode, struct file
> > +*file) {
> > + atomic_dec(&aspeed_mbox_open_count);
> > + return 0;
> > +}
> > +
> > +static long aspeed_mbox_ioctl(struct file *file, unsigned int cmd,
> > + unsigned long param) {
> > + long ret = 0;
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + struct aspeed_mbox_ioctl_data data;
> > +
> > + switch (cmd) {
> > + case ASPEED_MBOX_IOCTL_GET_SIZE:
> > + data.data = model->dr_num;
> > + if (copy_to_user((void __user *)param, &data,
> sizeof(data)))
> > + ret = -EFAULT;
> > + break;
> > + default:
> > + ret = -ENOTTY;
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static const struct file_operations aspeed_mbox_fops = {
> > + .owner = THIS_MODULE,
> > + .llseek = no_seek_end_llseek,
> > + .read = aspeed_mbox_read,
> > + .write = aspeed_mbox_write,
> > + .open = aspeed_mbox_open,
> > + .release = aspeed_mbox_release,
> > + .poll = aspeed_mbox_poll,
> > + .unlocked_ioctl = aspeed_mbox_ioctl, };
> > +
> > +static irqreturn_t aspeed_mbox_irq(int irq, void *arg) {
> > + struct aspeed_mbox *mbox = arg;
> > + const struct aspeed_mbox_model *model = mbox->model;
> > +
> > + if (!(aspeed_mbox_inb(mbox, model->bcr) &
> ASPEED_MBOX_BCR_RECV))
> > + return IRQ_NONE;
> > +
> > + /*
> > + * Leave the status bit set so that we know the data is for us,
> > + * clear it once it has been read.
> > + */
> > +
> > + /* Mask it off, we'll clear it when we the data gets read */
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_MASK,
> model->bcr);
> > +
> > + wake_up(&mbox->queue);
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int aspeed_mbox_config_irq(struct aspeed_mbox *mbox,
> > + struct platform_device *pdev) {
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + struct device *dev = &pdev->dev;
> > + int i, rc, irq;
> > +
> > + irq = irq_of_parse_and_map(dev->of_node, 0);
> > + if (!irq)
> > + return -ENODEV;
> > +
> > + rc = devm_request_irq(dev, irq, aspeed_mbox_irq,
> > + IRQF_SHARED, DEVICE_NAME, mbox);
> > + if (rc < 0) {
> > + dev_err(dev, "Unable to request IRQ %d\n", irq);
> > + return rc;
> > + }
> > +
> > + /*
> > + * Disable all register based interrupts.
> > + */
> > + for (i = 0; i < model->dr_num / 8; ++i)
> > + aspeed_mbox_outb(mbox, 0x00,
> > + ASPEED_MBOX_BIE(model->bie, i));
> > +
> > + /* These registers are write one to clear. Clear them. */
> > + for (i = 0; i < model->dr_num / 8; ++i)
> > + aspeed_mbox_outb(mbox, 0xff,
> > + ASPEED_MBOX_STR(model->str, i));
> > +
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV,
> model->bcr);
> > + return 0;
> > +}
> > +
> > +static int aspeed_mbox_probe(struct platform_device *pdev) {
> > + struct aspeed_mbox *mbox;
> > + struct device *dev;
> > + int rc;
> > +
> > + dev = &pdev->dev;
> > +
> > + mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> > + if (!mbox)
> > + return -ENOMEM;
> > +
> > + dev_set_drvdata(&pdev->dev, mbox);
> > +
> > + rc = of_property_read_u32(dev->of_node, "reg", &mbox->base);
> > + if (rc) {
> > + dev_err(dev, "Couldn't read reg device tree property\n");
> > + return rc;
> > + }
> > +
> > + mbox->model = of_device_get_match_data(dev);
> > + if (IS_ERR(mbox->model)) {
> > + dev_err(dev, "Couldn't get model data\n");
> > + return -ENODEV;
> > + }
> > +
> > + mbox->map = syscon_node_to_regmap(
> > + pdev->dev.parent->of_node);
> > + if (IS_ERR(mbox->map)) {
> > + dev_err(dev, "Couldn't get regmap\n");
> > + return -ENODEV;
> > + }
> > +
> > + mutex_init(&mbox->mutex);
> > + init_waitqueue_head(&mbox->queue);
> > +
> > + mbox->miscdev.minor = MISC_DYNAMIC_MINOR;
> > + mbox->miscdev.name = DEVICE_NAME;
> > + mbox->miscdev.fops = &aspeed_mbox_fops;
> > + mbox->miscdev.parent = dev;
> > + rc = misc_register(&mbox->miscdev);
> > + if (rc) {
> > + dev_err(dev, "Unable to register device\n");
> > + return rc;
> > + }
> > +
> > + rc = aspeed_mbox_config_irq(mbox, pdev);
> > + if (rc) {
> > + dev_err(dev, "Failed to configure IRQ\n");
> > + misc_deregister(&mbox->miscdev);
> > + return rc;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int aspeed_mbox_remove(struct platform_device *pdev) {
> > + struct aspeed_mbox *mbox = dev_get_drvdata(&pdev->dev);
> > +
> > + misc_deregister(&mbox->miscdev);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct aspeed_mbox_model ast2400_model = {
> > + .dr_num = 16,
> > + .dr = 0x0,
> > + .str = 0x40,
> > + .bcr = 0x48,
> > + .hcr = 0x4c,
> > + .bie = 0x50,
> > + .hie = 0x58,
> > +};
> > +
> > +static const struct aspeed_mbox_model ast2500_model = {
> > + .dr_num = 16,
> > + .dr = 0x0,
> > + .str = 0x40,
> > + .bcr = 0x48,
> > + .hcr = 0x4c,
> > + .bie = 0x50,
> > + .hie = 0x58,
> > +};
> > +
> > +static const struct aspeed_mbox_model ast2600_model = {
> > + .dr_num = 32,
> > + .dr = 0x0,
> > + .str = 0x80,
> > + .bcr = 0x90,
> > + .hcr = 0x94,
> > + .bie = 0xa0,
> > + .hie = 0xb0,
> > +};
> > +
> > +static const struct of_device_id aspeed_mbox_match[] = {
> > + { .compatible = "aspeed,ast2400-mbox",
> > + .data = &ast2400_model },
> > + { .compatible = "aspeed,ast2500-mbox",
> > + .data = &ast2500_model },
> > + { .compatible = "aspeed,ast2600-mbox",
> > + .data = &ast2600_model },
> > + { },
> > +};
> > +
> > +static struct platform_driver aspeed_mbox_driver = {
> > + .driver = {
> > + .name = DEVICE_NAME,
> > + .of_match_table = aspeed_mbox_match,
> > + },
> > + .probe = aspeed_mbox_probe,
> > + .remove = aspeed_mbox_remove,
> > +};
> > +
> > +module_platform_driver(aspeed_mbox_driver);
> > +MODULE_DEVICE_TABLE(of, aspeed_mbox_match);
> MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Cyril Bur <cyrilbur@gmail.com>");
> > +MODULE_AUTHOR("Chia-Wei Wang <chiawei_wang@aspeedtech.com");
> > +MODULE_DESCRIPTION("Aspeed mailbox device driver");
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 23+ messages in thread* RE: [PATCH v2 1/2] soc: aspeed: Add LPC mailbox support
@ 2022-03-18 7:59 ` ChiaWei Wang
0 siblings, 0 replies; 23+ messages in thread
From: ChiaWei Wang @ 2022-03-18 7:59 UTC (permalink / raw)
To: Joel Stanley, Arnd Bergmann
Cc: Rob Herring, Andrew Jeffery, Cyril Bur, devicetree, Linux ARM,
linux-aspeed, Linux Kernel Mailing List, OpenBMC Maillist
Hi Joel,
> From: Joel Stanley <joel@jms.id.au>
> Sent: Friday, March 18, 2022 3:06 PM
>
> On Tue, 17 Aug 2021 at 02:58, Chia-Wei Wang
> <chiawei_wang@aspeedtech.com> wrote:
> >
> > The LPC mailbox controller consists of multiple general registers for
> > the communication between the Host and the BMC.
> > The interrupts for data update signaling are also introduced.
>
> The concern with this approach is the userspace API. We don't want every
> driver to invent it's own way of communicating with usersapce.
>
> I know when Cyril first submitted this driver it was suggested to use the kernel
> mailbox subsystem. After investigation it was decided that this hardware is not
> a fit; although it is a mailbox it is more like a IPMI BT or KCS device, that needs
> an interface to push data to and from userspace over the device.
>
Thanks for the explanation.
The current implementation exports the mailbox register access in the file (device node) read/write way.
The read/write behavior is slightly different than a usual text file though...
The IOCTL is used only to retrieve the number of mailbox registers available.
We can consider exporting this through sysfs as a driver property, perhaps?
Is the IOCTL part the major concern? Or the file-based read/write also needs to be re-considered?
> Chai-Wei, can you link to some userspace programs that use the existing ioctl
> interface?
The kcs_bt_test APP in Aspeed GitHub has the example to access LPC mailbox with read()/write() toward its device node.
However, as kcs_bt_test is for internal test only, it is now removed in the latest release.
You can still find the source in the early release tags (e.g. v00.01.00).
https://github.com/AspeedTech-BMC/aspeed_app/blob/v00.01.00/kcs_bt_test/kcs_bt_test.c#L157
Thanks,
Chiawei
>
> Arnd, do you have any suggestions?
>
> >
> > Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com>
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > drivers/soc/aspeed/Kconfig | 10 +
> > drivers/soc/aspeed/Makefile | 9 +-
> > drivers/soc/aspeed/aspeed-lpc-mbox.c | 418
> > +++++++++++++++++++++++++++
> > 3 files changed, 433 insertions(+), 4 deletions(-) create mode
> > 100644 drivers/soc/aspeed/aspeed-lpc-mbox.c
> >
> > diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
> > index 243ca196e6ad..5b31a6e2620c 100644
> > --- a/drivers/soc/aspeed/Kconfig
> > +++ b/drivers/soc/aspeed/Kconfig
> > @@ -24,6 +24,16 @@ config ASPEED_LPC_SNOOP
> > allows the BMC to listen on and save the data written by
> > the host to an arbitrary LPC I/O port.
> >
> > +config ASPEED_LPC_MAILBOX
> > + tristate "ASPEED LPC mailbox support"
> > + select REGMAP
> > + select MFD_SYSCON
> > + default ARCH_ASPEED
> > + help
> > + Provides a driver to control the LPC mailbox which possesses
> > + up to 32 data registers for the communication between the Host
> > + and the BMC over LPC.
> > +
> > config ASPEED_P2A_CTRL
> > tristate "ASPEED P2A (VGA MMIO to BMC) bridge control"
> > select REGMAP
> > diff --git a/drivers/soc/aspeed/Makefile b/drivers/soc/aspeed/Makefile
> > index fcab7192e1a4..cde6b4514c97 100644
> > --- a/drivers/soc/aspeed/Makefile
> > +++ b/drivers/soc/aspeed/Makefile
> > @@ -1,5 +1,6 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > -obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> > -obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> > -obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
> > -obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o
> > +obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> > +obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> > +obj-$(CONFIG_ASPEED_LPC_MAILBOX) += aspeed-lpc-mbox.o
> > +obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
> > +obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o
> > diff --git a/drivers/soc/aspeed/aspeed-lpc-mbox.c
> > b/drivers/soc/aspeed/aspeed-lpc-mbox.c
> > new file mode 100644
> > index 000000000000..a09ca6a175f7
> > --- /dev/null
> > +++ b/drivers/soc/aspeed/aspeed-lpc-mbox.c
> > @@ -0,0 +1,418 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright 2017 IBM Corporation
> > + * Copyright 2021 Aspeed Technology Inc.
> > + */
> > +#include <linux/interrupt.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/miscdevice.h>
> > +#include <linux/module.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/poll.h>
> > +#include <linux/regmap.h>
> > +#include <linux/slab.h>
> > +
> > +#define DEVICE_NAME "aspeed-mbox"
> > +
> > +#define ASPEED_MBOX_DR(dr, n) (dr + (n * 4))
> > +#define ASPEED_MBOX_STR(str, n) (str + (n / 8) * 4)
> > +#define ASPEED_MBOX_BIE(bie, n) (bie + (n / 8) * 4)
> > +#define ASPEED_MBOX_HIE(hie, n) (hie + (n / 8) * 4)
> > +
> > +#define ASPEED_MBOX_BCR_RECV BIT(7)
> > +#define ASPEED_MBOX_BCR_MASK BIT(1)
> > +#define ASPEED_MBOX_BCR_SEND BIT(0)
> > +
> > +/* ioctl code */
> > +#define ASPEED_MBOX_IOCTL 0xA3
> > +#define ASPEED_MBOX_IOCTL_GET_SIZE \
> > + _IOR(ASPEED_MBOX_IOCTL, 0, struct aspeed_mbox_ioctl_data)
> > +
> > +struct aspeed_mbox_ioctl_data {
> > + unsigned int data;
> > +};
> > +
> > +struct aspeed_mbox_model {
> > + unsigned int dr_num;
> > +
> > + /* offsets to the MBOX registers */
> > + unsigned int dr;
> > + unsigned int str;
> > + unsigned int bcr;
> > + unsigned int hcr;
> > + unsigned int bie;
> > + unsigned int hie;
> > +};
> > +
> > +struct aspeed_mbox {
> > + struct miscdevice miscdev;
> > + struct regmap *map;
> > + unsigned int base;
> > + wait_queue_head_t queue;
> > + struct mutex mutex;
> > + const struct aspeed_mbox_model *model; };
> > +
> > +static atomic_t aspeed_mbox_open_count = ATOMIC_INIT(0);
> > +
> > +static u8 aspeed_mbox_inb(struct aspeed_mbox *mbox, int reg) {
> > + /*
> > + * The mbox registers are actually only one byte but are addressed
> > + * four bytes apart. The other three bytes are marked 'reserved',
> > + * they *should* be zero but lets not rely on it.
> > + * I am going to rely on the fact we can casually read/write to
> them...
> > + */
> > + unsigned int val = 0xff; /* If regmap throws an error return 0xff */
> > + int rc = regmap_read(mbox->map, mbox->base + reg, &val);
> > +
> > + if (rc)
> > + dev_err(mbox->miscdev.parent, "regmap_read() failed
> with "
> > + "%d (reg: 0x%08x)\n", rc, reg);
> > +
> > + return val & 0xff;
> > +}
> > +
> > +static void aspeed_mbox_outb(struct aspeed_mbox *mbox, u8 data, int
> > +reg) {
> > + int rc = regmap_write(mbox->map, mbox->base + reg, data);
> > +
> > + if (rc)
> > + dev_err(mbox->miscdev.parent, "regmap_write() failed
> with "
> > + "%d (data: %u reg: 0x%08x)\n", rc, data, reg);
> > +}
> > +
> > +static struct aspeed_mbox *file_mbox(struct file *file) {
> > + return container_of(file->private_data, struct aspeed_mbox,
> > +miscdev); }
> > +
> > +static int aspeed_mbox_open(struct inode *inode, struct file *file) {
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > +
> > + if (atomic_inc_return(&aspeed_mbox_open_count) == 1) {
> > + /*
> > + * Clear the interrupt status bit if it was left on and
> unmask
> > + * interrupts.
> > + * ASPEED_MBOX_BCR_RECV bit is W1C, this also
> unmasks in 1 step
> > + */
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV,
> model->bcr);
> > + return 0;
> > + }
> > +
> > + atomic_dec(&aspeed_mbox_open_count);
> > + return -EBUSY;
> > +}
> > +
> > +static ssize_t aspeed_mbox_read(struct file *file, char __user *buf,
> > + size_t count, loff_t *ppos) {
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + char __user *p = buf;
> > + ssize_t ret;
> > + int i;
> > +
> > + if (!access_ok(buf, count))
> > + return -EFAULT;
> > +
> > + if (count + *ppos > model->dr_num)
> > + return -EINVAL;
> > +
> > + if (file->f_flags & O_NONBLOCK) {
> > + if (!(aspeed_mbox_inb(mbox, model->bcr) &
> > + ASPEED_MBOX_BCR_RECV))
> > + return -EAGAIN;
> > + } else if (wait_event_interruptible(mbox->queue,
> > + aspeed_mbox_inb(mbox, model->bcr)
> &
> > + ASPEED_MBOX_BCR_RECV)) {
> > + return -ERESTARTSYS;
> > + }
> > +
> > + mutex_lock(&mbox->mutex);
> > +
> > + for (i = *ppos; count > 0 && i < model->dr_num; i++) {
> > + uint8_t reg = aspeed_mbox_inb(mbox,
> > + ASPEED_MBOX_DR(model->dr, i));
> > +
> > + ret = __put_user(reg, p);
> > + if (ret)
> > + goto out_unlock;
> > +
> > + p++;
> > + count--;
> > + }
> > +
> > + /* ASPEED_MBOX_BCR_RECV bit is write to clear, this also
> unmasks in 1 step */
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV,
> model->bcr);
> > + ret = p - buf;
> > +
> > +out_unlock:
> > + mutex_unlock(&mbox->mutex);
> > + return ret;
> > +}
> > +
> > +static ssize_t aspeed_mbox_write(struct file *file, const char __user *buf,
> > + size_t count, loff_t *ppos) {
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + const char __user *p = buf;
> > + ssize_t ret;
> > + char c;
> > + int i;
> > +
> > + if (!access_ok(buf, count))
> > + return -EFAULT;
> > +
> > + if (count + *ppos > model->dr_num)
> > + return -EINVAL;
> > +
> > + mutex_lock(&mbox->mutex);
> > +
> > + for (i = *ppos; count > 0 && i < model->dr_num; i++) {
> > + ret = __get_user(c, p);
> > + if (ret)
> > + goto out_unlock;
> > +
> > + aspeed_mbox_outb(mbox, c,
> ASPEED_MBOX_DR(model->dr, i));
> > + p++;
> > + count--;
> > + }
> > +
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_SEND,
> model->bcr);
> > + ret = p - buf;
> > +
> > +out_unlock:
> > + mutex_unlock(&mbox->mutex);
> > + return ret;
> > +}
> > +
> > +static __poll_t aspeed_mbox_poll(struct file *file, poll_table *wait)
> > +{
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + __poll_t mask = 0;
> > +
> > + poll_wait(file, &mbox->queue, wait);
> > +
> > + if (aspeed_mbox_inb(mbox, model->bcr) &
> ASPEED_MBOX_BCR_RECV)
> > + mask |= POLLIN;
> > +
> > + return mask;
> > +}
> > +
> > +static int aspeed_mbox_release(struct inode *inode, struct file
> > +*file) {
> > + atomic_dec(&aspeed_mbox_open_count);
> > + return 0;
> > +}
> > +
> > +static long aspeed_mbox_ioctl(struct file *file, unsigned int cmd,
> > + unsigned long param) {
> > + long ret = 0;
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + struct aspeed_mbox_ioctl_data data;
> > +
> > + switch (cmd) {
> > + case ASPEED_MBOX_IOCTL_GET_SIZE:
> > + data.data = model->dr_num;
> > + if (copy_to_user((void __user *)param, &data,
> sizeof(data)))
> > + ret = -EFAULT;
> > + break;
> > + default:
> > + ret = -ENOTTY;
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static const struct file_operations aspeed_mbox_fops = {
> > + .owner = THIS_MODULE,
> > + .llseek = no_seek_end_llseek,
> > + .read = aspeed_mbox_read,
> > + .write = aspeed_mbox_write,
> > + .open = aspeed_mbox_open,
> > + .release = aspeed_mbox_release,
> > + .poll = aspeed_mbox_poll,
> > + .unlocked_ioctl = aspeed_mbox_ioctl, };
> > +
> > +static irqreturn_t aspeed_mbox_irq(int irq, void *arg) {
> > + struct aspeed_mbox *mbox = arg;
> > + const struct aspeed_mbox_model *model = mbox->model;
> > +
> > + if (!(aspeed_mbox_inb(mbox, model->bcr) &
> ASPEED_MBOX_BCR_RECV))
> > + return IRQ_NONE;
> > +
> > + /*
> > + * Leave the status bit set so that we know the data is for us,
> > + * clear it once it has been read.
> > + */
> > +
> > + /* Mask it off, we'll clear it when we the data gets read */
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_MASK,
> model->bcr);
> > +
> > + wake_up(&mbox->queue);
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int aspeed_mbox_config_irq(struct aspeed_mbox *mbox,
> > + struct platform_device *pdev) {
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + struct device *dev = &pdev->dev;
> > + int i, rc, irq;
> > +
> > + irq = irq_of_parse_and_map(dev->of_node, 0);
> > + if (!irq)
> > + return -ENODEV;
> > +
> > + rc = devm_request_irq(dev, irq, aspeed_mbox_irq,
> > + IRQF_SHARED, DEVICE_NAME, mbox);
> > + if (rc < 0) {
> > + dev_err(dev, "Unable to request IRQ %d\n", irq);
> > + return rc;
> > + }
> > +
> > + /*
> > + * Disable all register based interrupts.
> > + */
> > + for (i = 0; i < model->dr_num / 8; ++i)
> > + aspeed_mbox_outb(mbox, 0x00,
> > + ASPEED_MBOX_BIE(model->bie, i));
> > +
> > + /* These registers are write one to clear. Clear them. */
> > + for (i = 0; i < model->dr_num / 8; ++i)
> > + aspeed_mbox_outb(mbox, 0xff,
> > + ASPEED_MBOX_STR(model->str, i));
> > +
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV,
> model->bcr);
> > + return 0;
> > +}
> > +
> > +static int aspeed_mbox_probe(struct platform_device *pdev) {
> > + struct aspeed_mbox *mbox;
> > + struct device *dev;
> > + int rc;
> > +
> > + dev = &pdev->dev;
> > +
> > + mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> > + if (!mbox)
> > + return -ENOMEM;
> > +
> > + dev_set_drvdata(&pdev->dev, mbox);
> > +
> > + rc = of_property_read_u32(dev->of_node, "reg", &mbox->base);
> > + if (rc) {
> > + dev_err(dev, "Couldn't read reg device tree property\n");
> > + return rc;
> > + }
> > +
> > + mbox->model = of_device_get_match_data(dev);
> > + if (IS_ERR(mbox->model)) {
> > + dev_err(dev, "Couldn't get model data\n");
> > + return -ENODEV;
> > + }
> > +
> > + mbox->map = syscon_node_to_regmap(
> > + pdev->dev.parent->of_node);
> > + if (IS_ERR(mbox->map)) {
> > + dev_err(dev, "Couldn't get regmap\n");
> > + return -ENODEV;
> > + }
> > +
> > + mutex_init(&mbox->mutex);
> > + init_waitqueue_head(&mbox->queue);
> > +
> > + mbox->miscdev.minor = MISC_DYNAMIC_MINOR;
> > + mbox->miscdev.name = DEVICE_NAME;
> > + mbox->miscdev.fops = &aspeed_mbox_fops;
> > + mbox->miscdev.parent = dev;
> > + rc = misc_register(&mbox->miscdev);
> > + if (rc) {
> > + dev_err(dev, "Unable to register device\n");
> > + return rc;
> > + }
> > +
> > + rc = aspeed_mbox_config_irq(mbox, pdev);
> > + if (rc) {
> > + dev_err(dev, "Failed to configure IRQ\n");
> > + misc_deregister(&mbox->miscdev);
> > + return rc;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int aspeed_mbox_remove(struct platform_device *pdev) {
> > + struct aspeed_mbox *mbox = dev_get_drvdata(&pdev->dev);
> > +
> > + misc_deregister(&mbox->miscdev);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct aspeed_mbox_model ast2400_model = {
> > + .dr_num = 16,
> > + .dr = 0x0,
> > + .str = 0x40,
> > + .bcr = 0x48,
> > + .hcr = 0x4c,
> > + .bie = 0x50,
> > + .hie = 0x58,
> > +};
> > +
> > +static const struct aspeed_mbox_model ast2500_model = {
> > + .dr_num = 16,
> > + .dr = 0x0,
> > + .str = 0x40,
> > + .bcr = 0x48,
> > + .hcr = 0x4c,
> > + .bie = 0x50,
> > + .hie = 0x58,
> > +};
> > +
> > +static const struct aspeed_mbox_model ast2600_model = {
> > + .dr_num = 32,
> > + .dr = 0x0,
> > + .str = 0x80,
> > + .bcr = 0x90,
> > + .hcr = 0x94,
> > + .bie = 0xa0,
> > + .hie = 0xb0,
> > +};
> > +
> > +static const struct of_device_id aspeed_mbox_match[] = {
> > + { .compatible = "aspeed,ast2400-mbox",
> > + .data = &ast2400_model },
> > + { .compatible = "aspeed,ast2500-mbox",
> > + .data = &ast2500_model },
> > + { .compatible = "aspeed,ast2600-mbox",
> > + .data = &ast2600_model },
> > + { },
> > +};
> > +
> > +static struct platform_driver aspeed_mbox_driver = {
> > + .driver = {
> > + .name = DEVICE_NAME,
> > + .of_match_table = aspeed_mbox_match,
> > + },
> > + .probe = aspeed_mbox_probe,
> > + .remove = aspeed_mbox_remove,
> > +};
> > +
> > +module_platform_driver(aspeed_mbox_driver);
> > +MODULE_DEVICE_TABLE(of, aspeed_mbox_match);
> MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Cyril Bur <cyrilbur@gmail.com>");
> > +MODULE_AUTHOR("Chia-Wei Wang <chiawei_wang@aspeedtech.com");
> > +MODULE_DESCRIPTION("Aspeed mailbox device driver");
> > --
> > 2.17.1
> >
_______________________________________________
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] 23+ messages in thread* RE: [PATCH v2 1/2] soc: aspeed: Add LPC mailbox support
@ 2022-03-18 7:59 ` ChiaWei Wang
0 siblings, 0 replies; 23+ messages in thread
From: ChiaWei Wang @ 2022-03-18 7:59 UTC (permalink / raw)
To: Joel Stanley, Arnd Bergmann
Cc: devicetree, linux-aspeed, Andrew Jeffery, OpenBMC Maillist,
Linux Kernel Mailing List, Rob Herring, Cyril Bur, Linux ARM
Hi Joel,
> From: Joel Stanley <joel@jms.id.au>
> Sent: Friday, March 18, 2022 3:06 PM
>
> On Tue, 17 Aug 2021 at 02:58, Chia-Wei Wang
> <chiawei_wang@aspeedtech.com> wrote:
> >
> > The LPC mailbox controller consists of multiple general registers for
> > the communication between the Host and the BMC.
> > The interrupts for data update signaling are also introduced.
>
> The concern with this approach is the userspace API. We don't want every
> driver to invent it's own way of communicating with usersapce.
>
> I know when Cyril first submitted this driver it was suggested to use the kernel
> mailbox subsystem. After investigation it was decided that this hardware is not
> a fit; although it is a mailbox it is more like a IPMI BT or KCS device, that needs
> an interface to push data to and from userspace over the device.
>
Thanks for the explanation.
The current implementation exports the mailbox register access in the file (device node) read/write way.
The read/write behavior is slightly different than a usual text file though...
The IOCTL is used only to retrieve the number of mailbox registers available.
We can consider exporting this through sysfs as a driver property, perhaps?
Is the IOCTL part the major concern? Or the file-based read/write also needs to be re-considered?
> Chai-Wei, can you link to some userspace programs that use the existing ioctl
> interface?
The kcs_bt_test APP in Aspeed GitHub has the example to access LPC mailbox with read()/write() toward its device node.
However, as kcs_bt_test is for internal test only, it is now removed in the latest release.
You can still find the source in the early release tags (e.g. v00.01.00).
https://github.com/AspeedTech-BMC/aspeed_app/blob/v00.01.00/kcs_bt_test/kcs_bt_test.c#L157
Thanks,
Chiawei
>
> Arnd, do you have any suggestions?
>
> >
> > Signed-off-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com>
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > drivers/soc/aspeed/Kconfig | 10 +
> > drivers/soc/aspeed/Makefile | 9 +-
> > drivers/soc/aspeed/aspeed-lpc-mbox.c | 418
> > +++++++++++++++++++++++++++
> > 3 files changed, 433 insertions(+), 4 deletions(-) create mode
> > 100644 drivers/soc/aspeed/aspeed-lpc-mbox.c
> >
> > diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
> > index 243ca196e6ad..5b31a6e2620c 100644
> > --- a/drivers/soc/aspeed/Kconfig
> > +++ b/drivers/soc/aspeed/Kconfig
> > @@ -24,6 +24,16 @@ config ASPEED_LPC_SNOOP
> > allows the BMC to listen on and save the data written by
> > the host to an arbitrary LPC I/O port.
> >
> > +config ASPEED_LPC_MAILBOX
> > + tristate "ASPEED LPC mailbox support"
> > + select REGMAP
> > + select MFD_SYSCON
> > + default ARCH_ASPEED
> > + help
> > + Provides a driver to control the LPC mailbox which possesses
> > + up to 32 data registers for the communication between the Host
> > + and the BMC over LPC.
> > +
> > config ASPEED_P2A_CTRL
> > tristate "ASPEED P2A (VGA MMIO to BMC) bridge control"
> > select REGMAP
> > diff --git a/drivers/soc/aspeed/Makefile b/drivers/soc/aspeed/Makefile
> > index fcab7192e1a4..cde6b4514c97 100644
> > --- a/drivers/soc/aspeed/Makefile
> > +++ b/drivers/soc/aspeed/Makefile
> > @@ -1,5 +1,6 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > -obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> > -obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> > -obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
> > -obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o
> > +obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> > +obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> > +obj-$(CONFIG_ASPEED_LPC_MAILBOX) += aspeed-lpc-mbox.o
> > +obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
> > +obj-$(CONFIG_ASPEED_SOCINFO) += aspeed-socinfo.o
> > diff --git a/drivers/soc/aspeed/aspeed-lpc-mbox.c
> > b/drivers/soc/aspeed/aspeed-lpc-mbox.c
> > new file mode 100644
> > index 000000000000..a09ca6a175f7
> > --- /dev/null
> > +++ b/drivers/soc/aspeed/aspeed-lpc-mbox.c
> > @@ -0,0 +1,418 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright 2017 IBM Corporation
> > + * Copyright 2021 Aspeed Technology Inc.
> > + */
> > +#include <linux/interrupt.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/miscdevice.h>
> > +#include <linux/module.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/poll.h>
> > +#include <linux/regmap.h>
> > +#include <linux/slab.h>
> > +
> > +#define DEVICE_NAME "aspeed-mbox"
> > +
> > +#define ASPEED_MBOX_DR(dr, n) (dr + (n * 4))
> > +#define ASPEED_MBOX_STR(str, n) (str + (n / 8) * 4)
> > +#define ASPEED_MBOX_BIE(bie, n) (bie + (n / 8) * 4)
> > +#define ASPEED_MBOX_HIE(hie, n) (hie + (n / 8) * 4)
> > +
> > +#define ASPEED_MBOX_BCR_RECV BIT(7)
> > +#define ASPEED_MBOX_BCR_MASK BIT(1)
> > +#define ASPEED_MBOX_BCR_SEND BIT(0)
> > +
> > +/* ioctl code */
> > +#define ASPEED_MBOX_IOCTL 0xA3
> > +#define ASPEED_MBOX_IOCTL_GET_SIZE \
> > + _IOR(ASPEED_MBOX_IOCTL, 0, struct aspeed_mbox_ioctl_data)
> > +
> > +struct aspeed_mbox_ioctl_data {
> > + unsigned int data;
> > +};
> > +
> > +struct aspeed_mbox_model {
> > + unsigned int dr_num;
> > +
> > + /* offsets to the MBOX registers */
> > + unsigned int dr;
> > + unsigned int str;
> > + unsigned int bcr;
> > + unsigned int hcr;
> > + unsigned int bie;
> > + unsigned int hie;
> > +};
> > +
> > +struct aspeed_mbox {
> > + struct miscdevice miscdev;
> > + struct regmap *map;
> > + unsigned int base;
> > + wait_queue_head_t queue;
> > + struct mutex mutex;
> > + const struct aspeed_mbox_model *model; };
> > +
> > +static atomic_t aspeed_mbox_open_count = ATOMIC_INIT(0);
> > +
> > +static u8 aspeed_mbox_inb(struct aspeed_mbox *mbox, int reg) {
> > + /*
> > + * The mbox registers are actually only one byte but are addressed
> > + * four bytes apart. The other three bytes are marked 'reserved',
> > + * they *should* be zero but lets not rely on it.
> > + * I am going to rely on the fact we can casually read/write to
> them...
> > + */
> > + unsigned int val = 0xff; /* If regmap throws an error return 0xff */
> > + int rc = regmap_read(mbox->map, mbox->base + reg, &val);
> > +
> > + if (rc)
> > + dev_err(mbox->miscdev.parent, "regmap_read() failed
> with "
> > + "%d (reg: 0x%08x)\n", rc, reg);
> > +
> > + return val & 0xff;
> > +}
> > +
> > +static void aspeed_mbox_outb(struct aspeed_mbox *mbox, u8 data, int
> > +reg) {
> > + int rc = regmap_write(mbox->map, mbox->base + reg, data);
> > +
> > + if (rc)
> > + dev_err(mbox->miscdev.parent, "regmap_write() failed
> with "
> > + "%d (data: %u reg: 0x%08x)\n", rc, data, reg);
> > +}
> > +
> > +static struct aspeed_mbox *file_mbox(struct file *file) {
> > + return container_of(file->private_data, struct aspeed_mbox,
> > +miscdev); }
> > +
> > +static int aspeed_mbox_open(struct inode *inode, struct file *file) {
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > +
> > + if (atomic_inc_return(&aspeed_mbox_open_count) == 1) {
> > + /*
> > + * Clear the interrupt status bit if it was left on and
> unmask
> > + * interrupts.
> > + * ASPEED_MBOX_BCR_RECV bit is W1C, this also
> unmasks in 1 step
> > + */
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV,
> model->bcr);
> > + return 0;
> > + }
> > +
> > + atomic_dec(&aspeed_mbox_open_count);
> > + return -EBUSY;
> > +}
> > +
> > +static ssize_t aspeed_mbox_read(struct file *file, char __user *buf,
> > + size_t count, loff_t *ppos) {
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + char __user *p = buf;
> > + ssize_t ret;
> > + int i;
> > +
> > + if (!access_ok(buf, count))
> > + return -EFAULT;
> > +
> > + if (count + *ppos > model->dr_num)
> > + return -EINVAL;
> > +
> > + if (file->f_flags & O_NONBLOCK) {
> > + if (!(aspeed_mbox_inb(mbox, model->bcr) &
> > + ASPEED_MBOX_BCR_RECV))
> > + return -EAGAIN;
> > + } else if (wait_event_interruptible(mbox->queue,
> > + aspeed_mbox_inb(mbox, model->bcr)
> &
> > + ASPEED_MBOX_BCR_RECV)) {
> > + return -ERESTARTSYS;
> > + }
> > +
> > + mutex_lock(&mbox->mutex);
> > +
> > + for (i = *ppos; count > 0 && i < model->dr_num; i++) {
> > + uint8_t reg = aspeed_mbox_inb(mbox,
> > + ASPEED_MBOX_DR(model->dr, i));
> > +
> > + ret = __put_user(reg, p);
> > + if (ret)
> > + goto out_unlock;
> > +
> > + p++;
> > + count--;
> > + }
> > +
> > + /* ASPEED_MBOX_BCR_RECV bit is write to clear, this also
> unmasks in 1 step */
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV,
> model->bcr);
> > + ret = p - buf;
> > +
> > +out_unlock:
> > + mutex_unlock(&mbox->mutex);
> > + return ret;
> > +}
> > +
> > +static ssize_t aspeed_mbox_write(struct file *file, const char __user *buf,
> > + size_t count, loff_t *ppos) {
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + const char __user *p = buf;
> > + ssize_t ret;
> > + char c;
> > + int i;
> > +
> > + if (!access_ok(buf, count))
> > + return -EFAULT;
> > +
> > + if (count + *ppos > model->dr_num)
> > + return -EINVAL;
> > +
> > + mutex_lock(&mbox->mutex);
> > +
> > + for (i = *ppos; count > 0 && i < model->dr_num; i++) {
> > + ret = __get_user(c, p);
> > + if (ret)
> > + goto out_unlock;
> > +
> > + aspeed_mbox_outb(mbox, c,
> ASPEED_MBOX_DR(model->dr, i));
> > + p++;
> > + count--;
> > + }
> > +
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_SEND,
> model->bcr);
> > + ret = p - buf;
> > +
> > +out_unlock:
> > + mutex_unlock(&mbox->mutex);
> > + return ret;
> > +}
> > +
> > +static __poll_t aspeed_mbox_poll(struct file *file, poll_table *wait)
> > +{
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + __poll_t mask = 0;
> > +
> > + poll_wait(file, &mbox->queue, wait);
> > +
> > + if (aspeed_mbox_inb(mbox, model->bcr) &
> ASPEED_MBOX_BCR_RECV)
> > + mask |= POLLIN;
> > +
> > + return mask;
> > +}
> > +
> > +static int aspeed_mbox_release(struct inode *inode, struct file
> > +*file) {
> > + atomic_dec(&aspeed_mbox_open_count);
> > + return 0;
> > +}
> > +
> > +static long aspeed_mbox_ioctl(struct file *file, unsigned int cmd,
> > + unsigned long param) {
> > + long ret = 0;
> > + struct aspeed_mbox *mbox = file_mbox(file);
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + struct aspeed_mbox_ioctl_data data;
> > +
> > + switch (cmd) {
> > + case ASPEED_MBOX_IOCTL_GET_SIZE:
> > + data.data = model->dr_num;
> > + if (copy_to_user((void __user *)param, &data,
> sizeof(data)))
> > + ret = -EFAULT;
> > + break;
> > + default:
> > + ret = -ENOTTY;
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static const struct file_operations aspeed_mbox_fops = {
> > + .owner = THIS_MODULE,
> > + .llseek = no_seek_end_llseek,
> > + .read = aspeed_mbox_read,
> > + .write = aspeed_mbox_write,
> > + .open = aspeed_mbox_open,
> > + .release = aspeed_mbox_release,
> > + .poll = aspeed_mbox_poll,
> > + .unlocked_ioctl = aspeed_mbox_ioctl, };
> > +
> > +static irqreturn_t aspeed_mbox_irq(int irq, void *arg) {
> > + struct aspeed_mbox *mbox = arg;
> > + const struct aspeed_mbox_model *model = mbox->model;
> > +
> > + if (!(aspeed_mbox_inb(mbox, model->bcr) &
> ASPEED_MBOX_BCR_RECV))
> > + return IRQ_NONE;
> > +
> > + /*
> > + * Leave the status bit set so that we know the data is for us,
> > + * clear it once it has been read.
> > + */
> > +
> > + /* Mask it off, we'll clear it when we the data gets read */
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_MASK,
> model->bcr);
> > +
> > + wake_up(&mbox->queue);
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int aspeed_mbox_config_irq(struct aspeed_mbox *mbox,
> > + struct platform_device *pdev) {
> > + const struct aspeed_mbox_model *model = mbox->model;
> > + struct device *dev = &pdev->dev;
> > + int i, rc, irq;
> > +
> > + irq = irq_of_parse_and_map(dev->of_node, 0);
> > + if (!irq)
> > + return -ENODEV;
> > +
> > + rc = devm_request_irq(dev, irq, aspeed_mbox_irq,
> > + IRQF_SHARED, DEVICE_NAME, mbox);
> > + if (rc < 0) {
> > + dev_err(dev, "Unable to request IRQ %d\n", irq);
> > + return rc;
> > + }
> > +
> > + /*
> > + * Disable all register based interrupts.
> > + */
> > + for (i = 0; i < model->dr_num / 8; ++i)
> > + aspeed_mbox_outb(mbox, 0x00,
> > + ASPEED_MBOX_BIE(model->bie, i));
> > +
> > + /* These registers are write one to clear. Clear them. */
> > + for (i = 0; i < model->dr_num / 8; ++i)
> > + aspeed_mbox_outb(mbox, 0xff,
> > + ASPEED_MBOX_STR(model->str, i));
> > +
> > + aspeed_mbox_outb(mbox, ASPEED_MBOX_BCR_RECV,
> model->bcr);
> > + return 0;
> > +}
> > +
> > +static int aspeed_mbox_probe(struct platform_device *pdev) {
> > + struct aspeed_mbox *mbox;
> > + struct device *dev;
> > + int rc;
> > +
> > + dev = &pdev->dev;
> > +
> > + mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> > + if (!mbox)
> > + return -ENOMEM;
> > +
> > + dev_set_drvdata(&pdev->dev, mbox);
> > +
> > + rc = of_property_read_u32(dev->of_node, "reg", &mbox->base);
> > + if (rc) {
> > + dev_err(dev, "Couldn't read reg device tree property\n");
> > + return rc;
> > + }
> > +
> > + mbox->model = of_device_get_match_data(dev);
> > + if (IS_ERR(mbox->model)) {
> > + dev_err(dev, "Couldn't get model data\n");
> > + return -ENODEV;
> > + }
> > +
> > + mbox->map = syscon_node_to_regmap(
> > + pdev->dev.parent->of_node);
> > + if (IS_ERR(mbox->map)) {
> > + dev_err(dev, "Couldn't get regmap\n");
> > + return -ENODEV;
> > + }
> > +
> > + mutex_init(&mbox->mutex);
> > + init_waitqueue_head(&mbox->queue);
> > +
> > + mbox->miscdev.minor = MISC_DYNAMIC_MINOR;
> > + mbox->miscdev.name = DEVICE_NAME;
> > + mbox->miscdev.fops = &aspeed_mbox_fops;
> > + mbox->miscdev.parent = dev;
> > + rc = misc_register(&mbox->miscdev);
> > + if (rc) {
> > + dev_err(dev, "Unable to register device\n");
> > + return rc;
> > + }
> > +
> > + rc = aspeed_mbox_config_irq(mbox, pdev);
> > + if (rc) {
> > + dev_err(dev, "Failed to configure IRQ\n");
> > + misc_deregister(&mbox->miscdev);
> > + return rc;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int aspeed_mbox_remove(struct platform_device *pdev) {
> > + struct aspeed_mbox *mbox = dev_get_drvdata(&pdev->dev);
> > +
> > + misc_deregister(&mbox->miscdev);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct aspeed_mbox_model ast2400_model = {
> > + .dr_num = 16,
> > + .dr = 0x0,
> > + .str = 0x40,
> > + .bcr = 0x48,
> > + .hcr = 0x4c,
> > + .bie = 0x50,
> > + .hie = 0x58,
> > +};
> > +
> > +static const struct aspeed_mbox_model ast2500_model = {
> > + .dr_num = 16,
> > + .dr = 0x0,
> > + .str = 0x40,
> > + .bcr = 0x48,
> > + .hcr = 0x4c,
> > + .bie = 0x50,
> > + .hie = 0x58,
> > +};
> > +
> > +static const struct aspeed_mbox_model ast2600_model = {
> > + .dr_num = 32,
> > + .dr = 0x0,
> > + .str = 0x80,
> > + .bcr = 0x90,
> > + .hcr = 0x94,
> > + .bie = 0xa0,
> > + .hie = 0xb0,
> > +};
> > +
> > +static const struct of_device_id aspeed_mbox_match[] = {
> > + { .compatible = "aspeed,ast2400-mbox",
> > + .data = &ast2400_model },
> > + { .compatible = "aspeed,ast2500-mbox",
> > + .data = &ast2500_model },
> > + { .compatible = "aspeed,ast2600-mbox",
> > + .data = &ast2600_model },
> > + { },
> > +};
> > +
> > +static struct platform_driver aspeed_mbox_driver = {
> > + .driver = {
> > + .name = DEVICE_NAME,
> > + .of_match_table = aspeed_mbox_match,
> > + },
> > + .probe = aspeed_mbox_probe,
> > + .remove = aspeed_mbox_remove,
> > +};
> > +
> > +module_platform_driver(aspeed_mbox_driver);
> > +MODULE_DEVICE_TABLE(of, aspeed_mbox_match);
> MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Cyril Bur <cyrilbur@gmail.com>");
> > +MODULE_AUTHOR("Chia-Wei Wang <chiawei_wang@aspeedtech.com");
> > +MODULE_DESCRIPTION("Aspeed mailbox device driver");
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 23+ messages in thread