Message ID | 6b32b34cf3940c9095e2fb232cf0b327eafdcf55.camel@kernel.crashing.org |
---|---|
State | Not Applicable, archived |
Headers | show |
Series | [linux,dev-4.13] fsi: occ: Rework and simplify driver | expand |
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@aj.id.au> > > Signed-off-by: Benjamin Herrenschmidt <benh@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> >
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>
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 Signed-off-by: Benjamin Herrenschmidt <benh@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