[RFC linux] drivers: fsi: Add SBE client device driver
Joel Stanley
joel at jms.id.au
Fri Dec 2 16:21:23 AEDT 2016
Hi Eddie,
On Fri, Dec 2, 2016 at 8:52 AM, <eajames.ibm at gmail.com> wrote:
> From: "Edward A. James" <eajames at us.ibm.com>
>
> This is a driver stub for the SBE FSI client driver. Just looking for
> feedback for driver structuring. SBE doesn't use traditional reads/writes
> but instead submits requests and receives replies from the SBE fifo (as
> far as I understand it - need to check with Karlo).
I assume this builds on top of Chris' latest OpenFSI series?
Does this only work for the P9 SBE, or is it relevant for P8?
>
> Signed-off-by: Edward A. James <eajames at us.ibm.com>
> ---
> drivers/fsi/Kconfig | 6 ++
> drivers/fsi/Makefile | 1 +
> drivers/fsi/fsi-sbe.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 168 insertions(+)
> create mode 100644 drivers/fsi/fsi-sbe.c
>
> diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
> index 5917006a1f..e314f82 100644
> --- a/drivers/fsi/Kconfig
> +++ b/drivers/fsi/Kconfig
> @@ -36,6 +36,12 @@ config FSI_MBX
> ---help---
> This option enables an FSI based Mailbox device driver.
>
> +config FSI_SBE
> + tristate "SBE FSI client device driver"
> + depends on FSI
> + ---help---
> + This option enables an FSI based SBE device driver.
> +
> endif
>
> endmenu
> diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
> index 36bcbd8..174c524 100644
> --- a/drivers/fsi/Makefile
> +++ b/drivers/fsi/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_FSI_MASTER_FAKE) += fsi-master-fake.o
> obj-$(CONFIG_FSI_MASTER_GPIO) += fsi-master-gpio.o
> obj-$(CONFIG_FSI_SCOM) += fsi-scom.o
> obj-$(CONFIG_FSI_MBX) += fsi-mbx.o
> +obj-$(CONFIG_FSI_SBE) += fsi-sbe.o
> diff --git a/drivers/fsi/fsi-sbe.c b/drivers/fsi/fsi-sbe.c
> new file mode 100644
> index 0000000..e1c7914
> --- /dev/null
> +++ b/drivers/fsi/fsi-sbe.c
> @@ -0,0 +1,161 @@
> +/*
> + * SBE FSI Client device driver
> + *
> + * Copyright (C) IBM Corporation 2016
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERGCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
Take a look at our other headers of the text we chose to use.
> + */
> +
> +#include <linux/fsi.h>
> +#include <linux/module.h>
> +#include <linux/cdev.h>
> +#include <linux/delay.h>
> +#include <linux/fs.h>
> +#include <linux/uaccess.h>
> +#include <linux/slab.h>
> +#include <linux/miscdevice.h>
> +#include <linux/list.h>
> +
> +#define FSI_ENGID_P9SBE 0x22
> +
> +#define SBE_IOCTL_SUBMIT 0x01
> +#define SBE_IOCTL_REQUEST_RESET 0x02
> +#define SBE_IOCTL_RESET 0x03
This is not where we declare ioctl information. Take a look at the
impi-bt patch for an example.
http://git.kernel.org/torvalds/c/54f9c4d0778b3f9ab791b1b7eb1a5d2809f02f50
> +
> +struct sbe_device {
> + struct list_head link;
> + struct fsi_device *fsi_dev;
> + struct miscdevice mdev;
> + char name[32];
> +};
> +
> +#define to_sbe_dev(x) container_of((x), struct sbe_device, mdev)
> +
> +static struct list_head sbe_devices;
> +static atomic_t sbe_idx = ATOMIC_INIT(0);
> +
> +static ssize_t sbe_read(struct file *filep, char __user *buf, size_t len,
> + loff_t *offset)
> +{
> + return len;
> +}
This is incorrect; the read call will return the users *buf
uninitialised (or with whatever was already in it).
Please omit the callback if it's not doing anything.
> +
> +static ssize_t sbe_write(struct file *filep, const char __user *buf,
> + size_t len, loff_t *offset)
> +{
> + return len;
> +}
Same here.
> +
> +static int sbe_submit(struct sbe_device *sbe, unsigned long data)
> +{
> + int rc = 0;
> +
> + return rc;
> +}
> +
> +static int sbe_reset(struct sbe_device *sbe)
> +{
> + int rc = 0;
> +
> + return rc;
> +}
> +
> +
> +static int sbe_request_reset(struct sbe_device *sbe)
> +{
> + return 0;
> +}
I assume this is the information you're waiting on?
> +
> +static long sbe_ioctl(struct file *filep, uint32_t cmd, unsigned long data)
> +{
> + struct miscdevice *mdev = (struct miscdevice *)filep->private_data;
> + struct sbe_device *sbe = to_sbe_dev(mdev);
> + struct device *dev = &sbe->fsi_dev->dev;
> + ssize_t rc;
> +
> + switch (cmd) {
> + case SBE_IOCTL_SUBMIT:
> + rc = sbe_submit(sbe, data);
Can we not read back from the SBE?
I suggest you use the normal write api for doing writes into the SBE.
The ioctls are useful for resetting.
> + break;
> + case SBE_IOCTL_REQUEST_RESET:
> + rc = sbe_request_reset(sbe);
> + break;
> + case SBE_IOCTL_RESET:
> + rc = sbe_reset(sbe);
> + break;
> + default:
> + dev_info(dev, "Ivalid ioctl command:%d\n", cmd);
> + return -EINVAL;
> + }
> +
> + return rc;
> +};
> +
> +static const struct file_operations sbe_fops = {
> + .owner = THIS_MODULE,
> + .llseek = no_seek_end_llseek,
> + .read = sbe_read,
> + .write = sbe_write,
> + .compat_ioctl = sbe_ioctl
> +};
> +
> +static int sbe_probe(struct device *dev)
> +{
> + struct fsi_device *fsi_dev = to_fsi_dev(dev);
> + struct sbe_device *sbe;
> +
> + sbe = devm_kzalloc(dev, sizeof(struct sbe_device), GFP_KERNEL);
> + if (!sbe)
> + return -ENOMEM;
> +
> + snprintf(sbe->name, 32, "sbe%d", atomic_inc_return(&sbe_idx));
> + sbe->fsi_dev = fsi_dev;
> +
> + sbe->mdev.minor = MISC_DYNAMIC_MINOR;
> + sbe->mdev.fops = &sbe_fops;
> + sbe->mdev.name = sbe->name;
> + sbe->mdev.parent = dev;
> +
> + list_add(&sbe->link, &sbe_devices);
> +
> + return misc_register(&sbe->mdev);
> +}
> +
> +static struct fsi_device_id sbe_ids[] = {
> + {
> + .engine_type = FSI_ENGID_P9SBE,
> + .version = FSI_VERSION_ANY,
> + },
> + { 0 }
> +};
> +
> +static struct fsi_driver sbe_drv = {
> + .id_table = sbe_ids,
> + .drv = {
> + .name = "sbe",
> + .bus = &fsi_bus_type,
> + .probe = sbe_probe,
> + }
> +};
> +
> +static int sbe_init(void)
> +{
> + INIT_LIST_HEAD(&sbe_devices);
> + return fsi_driver_register(&sbe_drv);
> +}
> +
> +static void sbe_exit(void)
> +{
> + fsi_driver_unregister(&sbe_drv);
> +}
> +
> +module_init(sbe_init);
> +module_exit(sbe_exit);
> +MODULE_LICENSE("GPL");
> --
> 1.9.1
>
More information about the openbmc
mailing list