[PATCH linux dev-4.13] fsi: occ: Rework and simplify driver
Andrew Jeffery
andrew at aj.id.au
Tue Jun 19 14:32:40 AEST 2018
On Thu, 14 Jun 2018, at 13:10, Benjamin Herrenschmidt wrote:
> This gets rid of the struct xfer and workqueue, and provides
> a much simpler synchronous API for use in-kernel. The user
> API remains the same, though it loses the non-blocking
> facility which wasn't particularly useful.
>
> The tracepoints are gone as they were rather specific to the
> complex queuing mechanism that is now gone.179
179?
Anyway, I had a plan to do the same thing, so:
Acked-by: Andrew Jeffery <andrew at aj.id.au>
>
> Signed-off-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>
> ---
>
> This driver isn't upstream yet, so this is an update that
> could/should be "folded" into the final upstream submission.
>
> drivers/fsi/fsi-occ.c | 515 +++++++--------------------------
> drivers/hwmon/occ/common.c | 2 +
> drivers/hwmon/occ/p9_sbe.c | 56 +---
> include/linux/fsi-occ.h | 9 +-
> include/trace/events/fsi_occ.h | 86 ------
> 5 files changed, 119 insertions(+), 549 deletions(-)
> delete mode 100644 include/trace/events/fsi_occ.h
>
> diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c
> index 15f41f45fb31..e4ebda646453 100644
> --- a/drivers/fsi/fsi-occ.c
> +++ b/drivers/fsi/fsi-occ.c
> @@ -29,33 +29,30 @@
> #include <linux/wait.h>
> #include <linux/workqueue.h>
>
> -#define CREATE_TRACE_POINTS
> -#include <trace/events/fsi_occ.h>
> -
> #define OCC_SRAM_BYTES 4096
> #define OCC_CMD_DATA_BYTES 4090
> #define OCC_RESP_DATA_BYTES 4089
>
> +#define OCC_SRAM_CMD_ADDR 0xFFFBE000
> +#define OCC_SRAM_RSP_ADDR 0xFFFBF000
> +
> /*
> - * Assume we don't have FFDC, if we do we'll overflow and
> + * Assume we don't have much FFDC, if we do we'll overflow and
> * fail the command. This needs to be big enough for simple
> * commands as well.
> */
> -#define OCC_SBE_STATUS_WORDS 16
> +#define OCC_SBE_STATUS_WORDS 32
>
> #define OCC_TIMEOUT_MS 1000
> #define OCC_CMD_IN_PRG_WAIT_MS 50
>
> struct occ {
> + struct device *dev;
> struct device *sbefifo;
> char name[32];
> int idx;
> struct miscdevice mdev;
> - struct list_head xfrs;
> - spinlock_t list_lock; /* lock access to the xfrs list */
> - struct mutex occ_lock; /* lock access to the hardware */
> - struct work_struct work;
> - bool cancel;
> + struct mutex occ_lock;
> };
>
> #define to_occ(x) container_of((x), struct occ, mdev)
> @@ -68,247 +65,82 @@ struct occ_response {
> u8 data[OCC_RESP_DATA_BYTES + 2]; /* two bytes checksum */
> } __packed;
>
> -/*
> - * transfer flags are NOT mutually exclusive
> - *
> - * Initial flags are none; transfer is created and queued from write(). All
> - * flags are cleared when the transfer is completed by closing the file or
> - * reading all of the available response data.
> - * XFR_IN_PROGRESS is set when a transfer is started from occ_worker_putsram,
> - * and cleared if the transfer fails or occ_worker_getsram completes.
> - * XFR_COMPLETE is set when a transfer fails or finishes occ_worker_getsram.
> - * XFR_CANCELED is set when the transfer's client is released.
> - */
> -enum {
> - XFR_IN_PROGRESS,
> - XFR_COMPLETE,
> - XFR_CANCELED,
> -};
> -
> -struct occ_xfr {
> - struct list_head link;
> - int rc;
> - u8 buf[OCC_SRAM_BYTES];
> - size_t cmd_data_length;
> - size_t resp_data_length;
> - unsigned long flags;
> -};
> -
> -/*
> - * client flags
> - *
> - * CLIENT_NONBLOCKING is set during open() if the file was opened with the
> - * O_NONBLOCK flag.
> - * CLIENT_XFR_PENDING is set during write() and cleared when all data has been
> - * read.
> - */
> -enum {
> - CLIENT_NONBLOCKING,
> - CLIENT_XFR_PENDING,
> -};
> -
> struct occ_client {
> - struct kref kref;
> struct occ *occ;
> - struct occ_xfr xfr;
> - spinlock_t lock; /* lock access to the client state */
> - wait_queue_head_t wait;
> + struct mutex lock;
> + size_t data_size;
> size_t read_offset;
> - unsigned long flags;
> + u8 *buffer;
> };
>
> #define to_client(x) container_of((x), struct occ_client, xfr)
>
> -static struct workqueue_struct *occ_wq;
> -
> static DEFINE_IDA(occ_ida);
>
> -static int occ_enqueue_xfr(struct occ_xfr *xfr)
> -{
> - int empty;
> - unsigned long flags;
> - struct occ_client *client = to_client(xfr);
> - struct occ *occ = client->occ;
> -
> - if (occ->cancel)
> - return -ENODEV;
> -
> - spin_lock_irqsave(&occ->list_lock, flags);
> -
> - empty = list_empty(&occ->xfrs);
> - list_add_tail(&xfr->link, &occ->xfrs);
> -
> - spin_unlock_irqrestore(&occ->list_lock, flags);
> -
> - trace_occ_enq_xfer(client, xfr);
> -
> - if (empty)
> - queue_work(occ_wq, &occ->work);
> -
> - return 0;
> -}
> -
> -static void occ_get_client(struct occ_client *client)
> -{
> - kref_get(&client->kref);
> -}
> -
> -static void occ_client_release(struct kref *kref)
> -{
> - struct occ_client *client = container_of(kref, struct occ_client,
> - kref);
> -
> - kfree(client);
> -}
> -
> -static void occ_put_client(struct occ_client *client)
> -{
> - kref_put(&client->kref, occ_client_release);
> -}
> -
> -static struct occ_client *occ_open_common(struct occ *occ, unsigned long flags)
> -{
> - struct occ_client *client = kzalloc(sizeof(*client), GFP_KERNEL);
> -
> - if (!client)
> - return NULL;
> -
> - client->occ = occ;
> - kref_init(&client->kref);
> - spin_lock_init(&client->lock);
> - init_waitqueue_head(&client->wait);
> -
> - if (flags & O_NONBLOCK)
> - set_bit(CLIENT_NONBLOCKING, &client->flags);
> -
> - return client;
> -}
> -
> static int occ_open(struct inode *inode, struct file *file)
> {
> - struct occ_client *client;
> + struct occ_client *client = kzalloc(sizeof(*client), GFP_KERNEL);
> struct miscdevice *mdev = file->private_data;
> struct occ *occ = to_occ(mdev);
>
> - client = occ_open_common(occ, file->f_flags);
> if (!client)
> return -ENOMEM;
> -
> + client->buffer = (u8 *)__get_free_page(GFP_KERNEL);
> + if (!client->buffer) {
> + kfree(client);
> + return -ENOMEM;
> + }
> + client->occ = occ;
> + mutex_init(&client->lock);
> file->private_data = client;
>
> - return 0;
> -}
> + /* We allocate a 1-page buffer, make sure it all fits */
> + BUILD_BUG_ON((OCC_CMD_DATA_BYTES + 3) > PAGE_SIZE);
> + BUILD_BUG_ON((OCC_RESP_DATA_BYTES + 7) > PAGE_SIZE);
>
> -static inline bool occ_read_ready(struct occ_xfr *xfr, struct occ *occ)
> -{
> - return test_bit(XFR_COMPLETE, &xfr->flags) ||
> - test_bit(XFR_CANCELED, &xfr->flags) || occ->cancel;
> + return 0;
> }
>
> -static ssize_t occ_read_common(struct occ_client *client, char __user *ubuf,
> - char *kbuf, size_t len)
> +static ssize_t occ_read(struct file *file, char __user *buf, size_t len,
> + loff_t *offset)
> {
> - int rc;
> - unsigned long flags;
> - size_t bytes;
> - struct occ_xfr *xfr;
> - struct occ *occ;
> + struct occ_client *client = file->private_data;
> + ssize_t rc = 0;
>
> if (!client)
> return -ENODEV;
> -
> if (len > OCC_SRAM_BYTES)
> return -EINVAL;
>
> - occ_get_client(client);
> - xfr = &client->xfr;
> - occ = client->occ;
> -
> - spin_lock_irqsave(&client->lock, flags);
> -
> - if (!test_bit(CLIENT_XFR_PENDING, &client->flags)) {
> - /* we just finished reading all data, return 0 */
> - if (client->read_offset) {
> - rc = 0;
> - client->read_offset = 0;
> - } else {
> - rc = -ENOMSG;
> - }
> -
> - goto done;
> - }
> -
> - if (!test_bit(XFR_COMPLETE, &xfr->flags)) {
> - if (test_bit(CLIENT_NONBLOCKING, &client->flags)) {
> - rc = -EAGAIN;
> - goto done;
> - }
> -
> - spin_unlock_irqrestore(&client->lock, flags);
> -
> - rc = wait_event_interruptible(client->wait,
> - occ_read_ready(xfr, occ));
> -
> - spin_lock_irqsave(&client->lock, flags);
> -
> - if (!test_bit(XFR_COMPLETE, &xfr->flags)) {
> - if (occ->cancel || test_bit(XFR_CANCELED, &xfr->flags))
> - rc = -ENODEV;
> - else
> - rc = -EINTR;
> -
> - goto done;
> - }
> - }
> + mutex_lock(&client->lock);
>
> - if (xfr->rc) {
> - rc = xfr->rc;
> + /* This should not be possible ... */
> + if (WARN_ON_ONCE(client->read_offset > client->data_size)) {
> + rc = -EIO;
> goto done;
> }
>
> - bytes = min(len, xfr->resp_data_length - client->read_offset);
> - if (ubuf) {
> - if (copy_to_user(ubuf, &xfr->buf[client->read_offset],
> - bytes)) {
> - rc = -EFAULT;
> - goto done;
> - }
> - } else {
> - memcpy(kbuf, &xfr->buf[client->read_offset], bytes);
> - }
> -
> - client->read_offset += bytes;
> -
> - /* xfr done */
> - if (client->read_offset == xfr->resp_data_length)
> - clear_bit(CLIENT_XFR_PENDING, &client->flags);
> + /* Grab how much data we have to read */
> + rc = min(len, client->data_size - client->read_offset);
>
> - rc = bytes;
> + if (copy_to_user(buf, client->buffer + client->read_offset, rc))
> + rc = -EFAULT;
> + else
> + client->read_offset += rc;
> + done:
> + mutex_unlock(&client->lock);
>
> -done:
> - spin_unlock_irqrestore(&client->lock, flags);
> - trace_occ_read_complete(client, xfr);
> - occ_put_client(client);
> return rc;
> }
>
> -static ssize_t occ_read(struct file *file, char __user *buf, size_t len,
> - loff_t *offset)
> +static ssize_t occ_write(struct file *file, const char __user *buf,
> + size_t len, loff_t *offset)
> {
> struct occ_client *client = file->private_data;
> -
> - return occ_read_common(client, buf, NULL, len);
> -}
> -
> -static ssize_t occ_write_common(struct occ_client *client,
> - const char __user *ubuf, const char *kbuf,
> - size_t len)
> -{
> - int rc;
> - unsigned long flags;
> - unsigned int i;
> - u16 data_length, checksum = 0;
> - struct occ_xfr *xfr;
> + size_t rlen, data_length;
> + u16 checksum = 0;
> + ssize_t rc, i;
> + u8 *cmd;
>
> if (!client)
> return -ENODEV;
> @@ -316,115 +148,66 @@ static ssize_t occ_write_common(struct occ_client
> *client,
> if (len > (OCC_CMD_DATA_BYTES + 3) || len < 3)
> return -EINVAL;
>
> - occ_get_client(client);
> - xfr = &client->xfr;
> -
> - trace_occ_write_begin(client, xfr);
> - spin_lock_irqsave(&client->lock, flags);
> + mutex_lock(&client->lock);
>
> - if (test_bit(CLIENT_XFR_PENDING, &client->flags)) {
> - rc = -EBUSY;
> - goto done;
> - }
> + /* Construct the command */
> + cmd = client->buffer;
>
> - memset(xfr, 0, sizeof(*xfr)); /* clear out the transfer */
> - xfr->buf[0] = 1; /* occ sequence number */
> + /* Sequence number (we could increment it and compare with the response) */
> + cmd[0] = 1;
>
> /*
> - * Assume user data follows the occ command format.
> - * byte 0: command type
> + * Copy the user command (assume user data follows the occ command format)
> + * byte 0 : command type
> * bytes 1-2: data length (msb first)
> * bytes 3-n: data
> */
> - if (ubuf) {
> - if (copy_from_user(&xfr->buf[1], ubuf, len)) {
> - rc = -EFAULT;
> - goto done;
> - }
> - } else {
> - memcpy(&xfr->buf[1], kbuf, len);
> + if (copy_from_user(&cmd[1], buf, len)) {
> + rc = -EFAULT;
> + goto done;
> }
>
> - data_length = (xfr->buf[2] << 8) + xfr->buf[3];
> + /* Extract data length */
> + data_length = (cmd[2] << 8) + cmd[3];
> if (data_length > OCC_CMD_DATA_BYTES) {
> rc = -EINVAL;
> goto done;
> }
>
> + /* Calculate checksum */
> for (i = 0; i < data_length + 4; ++i)
> - checksum += xfr->buf[i];
> -
> - xfr->buf[data_length + 4] = checksum >> 8;
> - xfr->buf[data_length + 5] = checksum & 0xFF;
> + checksum += cmd[i];
> + cmd[data_length + 4] = checksum >> 8;
> + cmd[data_length + 5] = checksum & 0xFF;
>
> - xfr->cmd_data_length = data_length + 6;
> - client->read_offset = 0;
> -
> - rc = occ_enqueue_xfr(xfr);
> + /* Submit command */
> + rlen = PAGE_SIZE;
> + rc = fsi_occ_submit(client->occ->dev, cmd, data_length + 6, cmd, &rlen);
> if (rc)
> goto done;
>
> - set_bit(CLIENT_XFR_PENDING, &client->flags);
> + /* Set read tracking data */
> + client->data_size = rlen;
> + client->read_offset = 0;
> +
> + /* Done */
> rc = len;
> + done:
> + mutex_unlock(&client->lock);
>
> -done:
> - spin_unlock_irqrestore(&client->lock, flags);
> - occ_put_client(client);
> return rc;
> }
>
> -static ssize_t occ_write(struct file *file, const char __user *buf,
> - size_t len, loff_t *offset)
> +static int occ_release(struct inode *inode, struct file *file)
> {
> struct occ_client *client = file->private_data;
>
> - return occ_write_common(client, buf, NULL, len);
> -}
> -
> -static int occ_release_common(struct occ_client *client)
> -{
> - unsigned long flags;
> - struct occ *occ;
> - struct occ_xfr *xfr;
> -
> - if (!client)
> - return -ENODEV;
> -
> - xfr = &client->xfr;
> - occ = client->occ;
> -
> - spin_lock_irqsave(&client->lock, flags);
> -
> - set_bit(XFR_CANCELED, &xfr->flags);
> - if (!test_bit(CLIENT_XFR_PENDING, &client->flags))
> - goto done;
> -
> - spin_lock(&occ->list_lock);
> -
> - if (!test_bit(XFR_IN_PROGRESS, &xfr->flags)) {
> - /* already deleted from list if complete */
> - if (!test_bit(XFR_COMPLETE, &xfr->flags))
> - list_del(&xfr->link);
> - }
> -
> - spin_unlock(&occ->list_lock);
> -
> - wake_up_all(&client->wait);
> -
> -done:
> - spin_unlock_irqrestore(&client->lock, flags);
> + free_page((unsigned long)client->buffer);
> + kfree(client);
>
> - occ_put_client(client);
> return 0;
> }
>
> -static int occ_release(struct inode *inode, struct file *file)
> -{
> - struct occ_client *client = file->private_data;
> -
> - return occ_release_common(client);
> -}
> -
> static const struct file_operations occ_fops = {
> .owner = THIS_MODULE,
> .open = occ_open,
> @@ -435,10 +218,10 @@ static const struct file_operations occ_fops = {
>
> static int occ_verify_checksum(struct occ_response *resp, u16 data_length)
> {
> - u16 i;
> u16 checksum;
> /* Fetch the two bytes after the data for the checksum. */
> u16 checksum_resp = get_unaligned_be16(&resp->data[data_length]);
> + u16 i;
>
> checksum = resp->seq_no;
> checksum += resp->cmd_type;
> @@ -454,7 +237,7 @@ static int occ_verify_checksum(struct occ_response
> *resp, u16 data_length)
> return 0;
> }
>
> -static int occ_getsram(struct device *sbefifo, u32 address, u8 *data,
> +static int occ_getsram(struct device *sbefifo, u32 address, void *data,
> ssize_t len)
> {
> u32 data_len = ((len + 7) / 8) * 8; /* must be multiples of 8 B */
> @@ -504,7 +287,7 @@ static int occ_getsram(struct device *sbefifo, u32
> address, u8 *data,
> return rc;
> }
>
> -static int occ_putsram(struct device *sbefifo, u32 address, u8 *data,
> +static int occ_putsram(struct device *sbefifo, u32 address, const void *data,
> ssize_t len)
> {
> size_t cmd_len, buf_len, resp_len, resp_data_len;
> @@ -613,46 +396,28 @@ static int occ_trigger_attn(struct device *sbefifo)
> return rc;
> }
>
> -static void occ_worker(struct work_struct *work)
> +int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
> + void *response, size_t *resp_len)
> {
> - int rc = 0, empty;
> - u16 resp_data_length;
> - unsigned long flags;
> - unsigned long start;
> const unsigned long timeout = msecs_to_jiffies(OCC_TIMEOUT_MS);
> - const long int wait_time = msecs_to_jiffies(OCC_CMD_IN_PRG_WAIT_MS);
> - struct occ_xfr *xfr;
> - struct occ_response *resp;
> - struct occ_client *client;
> - struct occ *occ = container_of(work, struct occ, work);
> + const unsigned long wait_time = msecs_to_jiffies(OCC_CMD_IN_PRG_WAIT_MS);
> + struct occ *occ = dev_get_drvdata(dev);
> + struct occ_response *resp = response;
> struct device *sbefifo = occ->sbefifo;
> + u16 resp_data_length;
> + unsigned long start;
> + int rc;
>
> -again:
> - if (occ->cancel)
> - return;
> -
> - spin_lock_irqsave(&occ->list_lock, flags);
> -
> - xfr = list_first_entry_or_null(&occ->xfrs, struct occ_xfr, link);
> - if (!xfr) {
> - spin_unlock_irqrestore(&occ->list_lock, flags);
> - return;
> + if (!occ)
> + return -ENODEV;
> + if (*resp_len < 7) {
> + dev_dbg(dev, "Bad resplen %d\n", *resp_len);
> + return -EINVAL;
> }
>
> - client = to_client(xfr);
> - occ_get_client(client);
> - resp = (struct occ_response *)xfr->buf;
> - set_bit(XFR_IN_PROGRESS, &xfr->flags);
> -
> - spin_unlock_irqrestore(&occ->list_lock, flags);
> - trace_occ_worker_xfer_begin(client, xfr);
> mutex_lock(&occ->occ_lock);
>
> - start = jiffies;
> -
> - /* write occ command */
> - rc = occ_putsram(sbefifo, 0xFFFBE000, xfr->buf,
> - xfr->cmd_data_length);
> + rc = occ_putsram(sbefifo, OCC_SRAM_CMD_ADDR, request, req_len);
> if (rc)
> goto done;
>
> @@ -660,91 +425,54 @@ static void occ_worker(struct work_struct *work)
> if (rc)
> goto done;
>
> - /* read occ response */
> + /* Read occ response header */
> + start = jiffies;
> do {
> - rc = occ_getsram(sbefifo, 0xFFFBF000, xfr->buf, 8);
> + rc = occ_getsram(sbefifo, OCC_SRAM_RSP_ADDR, resp, 8);
> if (rc)
> goto done;
>
> if (resp->return_status == OCC_RESP_CMD_IN_PRG) {
> - rc = -EALREADY;
> + rc = -ETIMEDOUT;
>
> if (time_after(jiffies, start + timeout))
> break;
>
> - set_current_state(TASK_INTERRUPTIBLE);
> + set_current_state(TASK_UNINTERRUPTIBLE);
> schedule_timeout(wait_time);
> }
> } while (rc);
>
> + /* Extract size of response data */
> resp_data_length = get_unaligned_be16(&resp->data_length);
> - if (resp_data_length > OCC_RESP_DATA_BYTES) {
> +
> + /* Message size is data length + 5 bytes header + 2 bytes checksum */
> + if ((resp_data_length + 7) > *resp_len) {
> rc = -EMSGSIZE;
> goto done;
> }
>
> + dev_dbg(dev, "resp_status=%02x resp_data_len=%d\n",
> + resp->return_status, resp_data_length);
> +
> + /* Grab the rest */
> if (resp_data_length > 1) {
> /* already got 3 bytes resp, also need 2 bytes checksum */
> - rc = occ_getsram(sbefifo, 0xFFFBF008, &xfr->buf[8],
> - resp_data_length - 1);
> + rc = occ_getsram(sbefifo, OCC_SRAM_RSP_ADDR + 8,
> + &resp->data[3], resp_data_length - 1);
> if (rc)
> goto done;
> }
>
> - xfr->resp_data_length = resp_data_length + 7;
> + *resp_len = resp_data_length + 7;
>
> rc = occ_verify_checksum(resp, resp_data_length);
> -
> -done:
> + done:
> mutex_unlock(&occ->occ_lock);
>
> - xfr->rc = rc;
> - set_bit(XFR_COMPLETE, &xfr->flags);
> -
> - spin_lock_irqsave(&occ->list_lock, flags);
> -
> - clear_bit(XFR_IN_PROGRESS, &xfr->flags);
> - list_del(&xfr->link);
> - empty = list_empty(&occ->xfrs);
> -
> - spin_unlock_irqrestore(&occ->list_lock, flags);
> -
> - wake_up_interruptible(&client->wait);
> - trace_occ_worker_xfer_complete(client, xfr);
> - occ_put_client(client);
> -
> - if (!empty)
> - goto again;
> -}
> -
> -struct occ_client *occ_drv_open(struct device *dev, unsigned long flags)
> -{
> - struct occ *occ = dev_get_drvdata(dev);
> -
> - if (!occ)
> - return NULL;
> -
> - return occ_open_common(occ, flags);
> -}
> -EXPORT_SYMBOL_GPL(occ_drv_open);
> -
> -int occ_drv_read(struct occ_client *client, char *buf, size_t len)
> -{
> - return occ_read_common(client, NULL, buf, len);
> -}
> -EXPORT_SYMBOL_GPL(occ_drv_read);
> -
> -int occ_drv_write(struct occ_client *client, const char *buf, size_t len)
> -{
> - return occ_write_common(client, NULL, buf, len);
> -}
> -EXPORT_SYMBOL_GPL(occ_drv_write);
> -
> -void occ_drv_release(struct occ_client *client)
> -{
> - occ_release_common(client);
> + return rc;
> }
> -EXPORT_SYMBOL_GPL(occ_drv_release);
> +EXPORT_SYMBOL_GPL(fsi_occ_submit);
>
> static int occ_unregister_child(struct device *dev, void *data)
> {
> @@ -771,11 +499,9 @@ static int occ_probe(struct platform_device *pdev)
> if (!occ)
> return -ENOMEM;
>
> + occ->dev = dev;
> occ->sbefifo = dev->parent;
> - INIT_LIST_HEAD(&occ->xfrs);
> - spin_lock_init(&occ->list_lock);
> mutex_init(&occ->occ_lock);
> - INIT_WORK(&occ->work, occ_worker);
>
> if (dev->of_node) {
> rc = of_property_read_u32(dev->of_node, "reg", ®);
> @@ -819,24 +545,11 @@ static int occ_probe(struct platform_device *pdev)
>
> static int occ_remove(struct platform_device *pdev)
> {
> - unsigned long flags;
> struct occ *occ = platform_get_drvdata(pdev);
> - struct occ_xfr *xfr;
> - struct occ_client *client;
> -
> - occ->cancel = true;
> -
> - spin_lock_irqsave(&occ->list_lock, flags);
> - list_for_each_entry(xfr, &occ->xfrs, link) {
> - client = to_client(xfr);
> - wake_up_all(&client->wait);
> - }
> - spin_unlock_irqrestore(&occ->list_lock, flags);
>
> misc_deregister(&occ->mdev);
> - device_for_each_child(&pdev->dev, NULL, occ_unregister_child);
>
> - cancel_work_sync(&occ->work);
> + device_for_each_child(&pdev->dev, NULL, occ_unregister_child);
>
> ida_simple_remove(&occ_ida, occ->idx);
>
> @@ -859,17 +572,11 @@ static struct platform_driver occ_driver = {
>
> static int occ_init(void)
> {
> - occ_wq = create_singlethread_workqueue("occ");
> - if (!occ_wq)
> - return -ENOMEM;
> -
> return platform_driver_register(&occ_driver);
> }
>
> static void occ_exit(void)
> {
> - destroy_workqueue(occ_wq);
> -
> platform_driver_unregister(&occ_driver);
>
> ida_destroy(&occ_ida);
> diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
> index c1bccf53b425..d7e6a4de8d85 100644
> --- a/drivers/hwmon/occ/common.c
> +++ b/drivers/hwmon/occ/common.c
> @@ -7,6 +7,8 @@
> * (at your option) any later version.
> */
>
> +#define DEBUG
> +
> #include <linux/device.h>
> #include <linux/hwmon.h>
> #include <linux/hwmon-sysfs.h>
> diff --git a/drivers/hwmon/occ/p9_sbe.c b/drivers/hwmon/occ/p9_sbe.c
> index 9a1d3ae56d69..34fe4d3bc247 100644
> --- a/drivers/hwmon/occ/p9_sbe.c
> +++ b/drivers/hwmon/occ/p9_sbe.c
> @@ -12,69 +12,26 @@
> #include <linux/fsi-occ.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> -#include <linux/spinlock.h>
>
> #include "common.h"
>
> -/* Satisfy lockdep's need for static keys */
> -static struct lock_class_key p9_sbe_occ_client_lock_key;
> -
> struct p9_sbe_occ {
> struct occ occ;
> struct device *sbe;
> -
> - /*
> - * Pointer to occ device client. We store this so that we can cancel
> - * the client operations in remove() if necessary. We only need one
> - * pointer since we do one OCC operation (open, write, read, close) at
> - * a time (access to p9_sbe_occ_send_cmd is locked in the common code
> - * with occ.lock).
> - */
> - struct occ_client *client;
> -
> - /*
> - * This lock controls access to the client pointer and ensures atomic
> - * open, close and NULL assignment. This prevents simultaneous opening
> - * and closing of the client, or closing multiple times.
> - */
> - struct mutex client_lock;
> };
>
> #define to_p9_sbe_occ(x) container_of((x), struct p9_sbe_occ, occ)
>
> -static void p9_sbe_occ_close_client(struct p9_sbe_occ *ctx)
> -{
> - struct occ_client *tmp_client;
> -
> - mutex_lock(&ctx->client_lock);
> - tmp_client = ctx->client;
> - ctx->client = NULL;
> - occ_drv_release(tmp_client);
> - mutex_unlock(&ctx->client_lock);
> -}
> -
> static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd)
> {
> - int rc;
> struct occ_response *resp = &occ->resp;
> struct p9_sbe_occ *ctx = to_p9_sbe_occ(occ);
> + size_t resp_len = sizeof(*resp);
> + int rc;
>
> - mutex_lock(&ctx->client_lock);
> - if (ctx->sbe)
> - ctx->client = occ_drv_open(ctx->sbe, 0);
> - mutex_unlock(&ctx->client_lock);
> -
> - if (!ctx->client)
> - return -ENODEV;
> -
> - /* skip first byte (sequence number), OCC driver handles it */
> - rc = occ_drv_write(ctx->client, (const char *)&cmd[1], 7);
> - if (rc < 0)
> - goto err;
> -
> - rc = occ_drv_read(ctx->client, (char *)resp, sizeof(*resp));
> + rc = fsi_occ_submit(ctx->sbe, cmd, 8, resp, &resp_len);
> if (rc < 0)
> - goto err;
> + return rc;
>
> switch (resp->return_status) {
> case OCC_RESP_CMD_IN_PRG:
> @@ -102,8 +59,6 @@ static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd)
> rc = -EPROTO;
> }
>
> -err:
> - p9_sbe_occ_close_client(ctx);
> return rc;
> }
>
> @@ -117,8 +72,6 @@ static int p9_sbe_occ_probe(struct platform_device *pdev)
> if (!ctx)
> return -ENOMEM;
>
> - mutex_init(&ctx->client_lock);
> - lockdep_set_class(&ctx->client_lock, &p9_sbe_occ_client_lock_key);
> ctx->sbe = pdev->dev.parent;
> occ = &ctx->occ;
> occ->bus_dev = &pdev->dev;
> @@ -141,7 +94,6 @@ static int p9_sbe_occ_remove(struct platform_device *pdev)
> struct p9_sbe_occ *ctx = to_p9_sbe_occ(occ);
>
> ctx->sbe = NULL;
> - p9_sbe_occ_close_client(ctx);
>
> occ_shutdown(occ);
>
> diff --git a/include/linux/fsi-occ.h b/include/linux/fsi-occ.h
> index 0a4a54a08be3..4810368d4fb2 100644
> --- a/include/linux/fsi-occ.h
> +++ b/include/linux/fsi-occ.h
> @@ -15,7 +15,6 @@
> #define LINUX_FSI_OCC_H
>
> struct device;
> -struct occ_client;
>
> #define OCC_RESP_CMD_IN_PRG 0xFF
> #define OCC_RESP_SUCCESS 0
> @@ -31,11 +30,7 @@ struct occ_client;
> #define OCC_RESP_CRIT_OCB 0xE3
> #define OCC_RESP_CRIT_HW 0xE4
>
> -extern struct occ_client *occ_drv_open(struct device *dev,
> - unsigned long flags);
> -extern int occ_drv_read(struct occ_client *client, char *buf, size_t
> len);
> -extern int occ_drv_write(struct occ_client *client, const char *buf,
> - size_t len);
> -extern void occ_drv_release(struct occ_client *client);
> +extern int fsi_occ_submit(struct device *dev, const void *request,
> size_t req_len,
> + void *response, size_t *resp_len);
>
> #endif /* LINUX_FSI_OCC_H */
> diff --git a/include/trace/events/fsi_occ.h b/include/trace/events/fsi_occ.h
> deleted file mode 100644
> index 89562436de86..000000000000
> --- a/include/trace/events/fsi_occ.h
> +++ /dev/null
> @@ -1,86 +0,0 @@
> -#undef TRACE_SYSTEM
> -#define TRACE_SYSTEM fsi_occ
> -
> -#if !defined(_TRACE_TIMER_H) || defined(TRACE_HEADER_MULTI_READ)
> -#define _TRACE_OCC_H
> -
> -#include <linux/tracepoint.h>
> -#include <linux/fsi-occ.h>
> -
> -TRACE_EVENT(occ_enq_xfer,
> - TP_PROTO(const void *client, const void *xfer),
> - TP_ARGS(client, xfer),
> - TP_STRUCT__entry(
> - __field(const void *, client)
> - __field(const void *, xfer)
> - ),
> - TP_fast_assign(
> - __entry->client = client;
> - __entry->xfer = xfer;
> - ),
> - TP_printk("Client %p enqueued xfer %p", __entry->client, __entry->xfer)
> -);
> -
> -TRACE_EVENT(occ_read_complete,
> - TP_PROTO(const void *client, const void *xfer),
> - TP_ARGS(client, xfer),
> - TP_STRUCT__entry(
> - __field(const void *, client)
> - __field(const void *, xfer)
> - ),
> - TP_fast_assign(
> - __entry->client = client;
> - __entry->xfer = xfer;
> - ),
> - TP_printk("Client %p completed read for xfer %p",
> - __entry->client, __entry->xfer)
> -);
> -
> -TRACE_EVENT(occ_write_begin,
> - TP_PROTO(const void *client, const void *xfer),
> - TP_ARGS(client, xfer),
> - TP_STRUCT__entry(
> - __field(const void *, client)
> - __field(const void *, xfer)
> - ),
> - TP_fast_assign(
> - __entry->client = client;
> - __entry->xfer = xfer;
> - ),
> - TP_printk("Client %p began write for xfer %p",
> - __entry->client, __entry->xfer)
> -);
> -
> -TRACE_EVENT(occ_worker_xfer_begin,
> - TP_PROTO(const void *client, const void *xfer),
> - TP_ARGS(client, xfer),
> - TP_STRUCT__entry(
> - __field(const void *, client)
> - __field(const void *, xfer)
> - ),
> - TP_fast_assign(
> - __entry->client = client;
> - __entry->xfer = xfer;
> - ),
> - TP_printk("OCC worker began client %p xfer %p",
> - __entry->client, __entry->xfer)
> -);
> -
> -TRACE_EVENT(occ_worker_xfer_complete,
> - TP_PROTO(const void *client, const void *xfer),
> - TP_ARGS(client, xfer),
> - TP_STRUCT__entry(
> - __field(const void *, client)
> - __field(const void *, xfer)
> - ),
> - TP_fast_assign(
> - __entry->client = client;
> - __entry->xfer = xfer;
> - ),
> - TP_printk("OCC worker completed client %p xfer %p",
> - __entry->client, __entry->xfer)
> -);
> -
> -#endif
> -
> -#include <trace/define_trace.h>
>
More information about the openbmc
mailing list