[PATCH linux dev-4.10 2/3] drivers/fsi/occ: refactor to upstream list state
Eddie James
eajames at linux.vnet.ibm.com
Fri Sep 22 08:43:57 AEST 2017
From: "Edward A. James" <eajames at us.ibm.com>
Includes various fixes:
- Fix probe failure scenario
- General cleanup
- Reorder remove() operations for safety
- Add cancel boolean to prevent further xfrs when we're removing
Signed-off-by: Edward A. James <eajames at us.ibm.com>
---
drivers/fsi/occ.c | 227 ++++++++++++++++++++++++++++++++--------------------
include/linux/occ.h | 20 ++++-
2 files changed, 155 insertions(+), 92 deletions(-)
diff --git a/drivers/fsi/occ.c b/drivers/fsi/occ.c
index 621fbf0..4dd5048 100644
--- a/drivers/fsi/occ.c
+++ b/drivers/fsi/occ.c
@@ -10,16 +10,22 @@
#include <asm/unaligned.h>
#include <linux/device.h>
#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/fs.h>
#include <linux/fsi-sbefifo.h>
-#include <linux/init.h>
+#include <linux/idr.h>
#include <linux/kernel.h>
+#include <linux/list.h>
#include <linux/miscdevice.h>
#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/occ.h>
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
+#include <linux/sched.h>
#include <linux/slab.h>
+#include <linux/spinlock.h>
#include <linux/uaccess.h>
#include <linux/wait.h>
#include <linux/workqueue.h>
@@ -28,49 +34,45 @@
#define OCC_CMD_DATA_BYTES 4090
#define OCC_RESP_DATA_BYTES 4089
+#define OCC_TIMEOUT_MS 1000
+#define OCC_CMD_IN_PRG_WAIT_MS 50
+
struct occ {
struct device *sbefifo;
char name[32];
int idx;
struct miscdevice mdev;
struct list_head xfrs;
- spinlock_t list_lock;
- struct mutex occ_lock;
+ 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;
};
#define to_occ(x) container_of((x), struct occ, mdev)
-struct occ_command {
- u8 seq_no;
- u8 cmd_type;
- u16 data_length;
- u8 data[OCC_CMD_DATA_BYTES];
- u16 checksum;
-} __packed;
-
struct occ_response {
u8 seq_no;
u8 cmd_type;
u8 return_status;
- u16 data_length;
+ __be16 data_length;
u8 data[OCC_RESP_DATA_BYTES];
- u16 checksum;
+ __be16 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.
+ * 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.
+ * 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.
* XFR_WAITING is set from read() if the transfer isn't complete and
- * NONBLOCKING wasn't specified. Cleared in read() when transfer completes
- * or fails.
+ * O_NONBLOCK wasn't specified. Cleared in read() when transfer completes or
+ * fails.
*/
enum {
XFR_IN_PROGRESS,
@@ -92,9 +94,9 @@ struct occ_xfr {
* client flags
*
* CLIENT_NONBLOCKING is set during open() if the file was opened with the
- * O_NONBLOCKING flag.
+ * O_NONBLOCK flag.
* CLIENT_XFR_PENDING is set during write() and cleared when all data has been
- * read.
+ * read.
*/
enum {
CLIENT_NONBLOCKING,
@@ -104,7 +106,7 @@ enum {
struct occ_client {
struct occ *occ;
struct occ_xfr xfr;
- spinlock_t lock;
+ spinlock_t lock; /* lock access to the client state */
wait_queue_head_t wait;
size_t read_offset;
unsigned long flags;
@@ -116,12 +118,15 @@ struct occ_client {
static DEFINE_IDA(occ_ida);
-static void occ_enqueue_xfr(struct occ_xfr *xfr)
+static int occ_enqueue_xfr(struct occ_xfr *xfr)
{
int empty;
struct occ_client *client = to_client(xfr);
struct occ *occ = client->occ;
+ if (occ->cancel)
+ return -ECANCELED;
+
spin_lock_irq(&occ->list_lock);
empty = list_empty(&occ->xfrs);
@@ -131,14 +136,15 @@ static void occ_enqueue_xfr(struct occ_xfr *xfr)
if (empty)
queue_work(occ_wq, &occ->work);
+
+ return 0;
}
static struct occ_client *occ_open_common(struct occ *occ, unsigned long flags)
{
- struct occ_client *client;
+ struct occ_client *client = kzalloc(sizeof(*client), GFP_KERNEL);
- client = kzalloc(sizeof(*client), GFP_KERNEL);
- if (!client)
+ if (!client || occ->cancel)
return NULL;
client->occ = occ;
@@ -172,6 +178,7 @@ static ssize_t occ_read_common(struct occ_client *client, char __user *ubuf,
int rc;
size_t bytes;
struct occ_xfr *xfr = &client->xfr;
+ struct occ *occ = client->occ;
if (len > OCC_SRAM_BYTES)
return -EINVAL;
@@ -183,8 +190,9 @@ static ssize_t occ_read_common(struct occ_client *client, char __user *ubuf,
if (client->read_offset) {
rc = 0;
client->read_offset = 0;
- } else
+ } else {
rc = -ENOMSG;
+ }
goto done;
}
@@ -200,8 +208,11 @@ static ssize_t occ_read_common(struct occ_client *client, char __user *ubuf,
spin_unlock_irq(&client->lock);
rc = wait_event_interruptible(client->wait,
- test_bit(XFR_COMPLETE, &xfr->flags) ||
- test_bit(XFR_CANCELED, &xfr->flags));
+ test_bit(XFR_COMPLETE,
+ &xfr->flags) ||
+ test_bit(XFR_CANCELED,
+ &xfr->flags) ||
+ occ->cancel);
spin_lock_irq(&client->lock);
@@ -225,12 +236,14 @@ static ssize_t occ_read_common(struct occ_client *client, char __user *ubuf,
bytes = min(len, xfr->resp_data_length - client->read_offset);
if (ubuf) {
- if (copy_to_user(ubuf, &xfr->buf[client->read_offset], bytes)) {
+ if (copy_to_user(ubuf, &xfr->buf[client->read_offset],
+ bytes)) {
rc = -EFAULT;
goto done;
}
- } else
+ } else {
memcpy(kbuf, &xfr->buf[client->read_offset], bytes);
+ }
client->read_offset += bytes;
@@ -250,12 +263,6 @@ static ssize_t occ_read(struct file *file, char __user *buf, size_t len,
{
struct occ_client *client = file->private_data;
- /* check this ahead of time so we don't go changing the xfr state
- * needlessly
- */
- if (!access_ok(VERIFY_WRITE, buf, len))
- return -EFAULT;
-
return occ_read_common(client, buf, NULL, len);
}
@@ -272,28 +279,31 @@ static ssize_t occ_write_common(struct occ_client *client,
return -EINVAL;
spin_lock_irq(&client->lock);
- if (test_and_set_bit(CLIENT_XFR_PENDING, &client->flags)) {
+
+ if (test_bit(CLIENT_XFR_PENDING, &client->flags)) {
rc = -EBUSY;
goto done;
}
- /* clear out the transfer */
- memset(xfr, 0, sizeof(*xfr));
-
- xfr->buf[0] = 1;
+ memset(xfr, 0, sizeof(*xfr)); /* clear out the transfer */
+ xfr->buf[0] = 1; /* occ sequence number */
+ /* 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)) {
- kfree(xfr);
rc = -EFAULT;
goto done;
}
- } else
+ } else {
memcpy(&xfr->buf[1], kbuf, len);
+ }
data_length = (xfr->buf[2] << 8) + xfr->buf[3];
if (data_length > OCC_CMD_DATA_BYTES) {
- kfree(xfr);
rc = -EINVAL;
goto done;
}
@@ -306,9 +316,11 @@ static ssize_t occ_write_common(struct occ_client *client,
xfr->cmd_data_length = data_length + 6;
client->read_offset = 0;
+ rc = occ_enqueue_xfr(xfr);
+ if (rc)
+ goto done;
- occ_enqueue_xfr(xfr);
-
+ set_bit(CLIENT_XFR_PENDING, &client->flags);
rc = len;
done:
@@ -321,12 +333,6 @@ static ssize_t occ_write(struct file *file, const char __user *buf,
{
struct occ_client *client = file->private_data;
- /* check this ahead of time so we don't go changing the xfr state
- * needlessly
- */
- if (!access_ok(VERIFY_READ, buf, len))
- return -EFAULT;
-
return occ_write_common(client, buf, NULL, len);
}
@@ -366,7 +372,7 @@ static int occ_release_common(struct occ_client *client)
return 0;
}
- /* operation is in progress; let worker clean up*/
+ /* operation is in progress; let worker clean up */
spin_unlock_irq(&occ->list_lock);
spin_unlock_irq(&client->lock);
return 0;
@@ -403,7 +409,7 @@ static int occ_write_sbefifo(struct sbefifo_client *client, const char *buf,
total += rc;
} while (total < len);
- return (total == len) ? 0 : -EMSGSIZE;
+ return (total == len) ? 0 : -ENODATA;
}
static int occ_read_sbefifo(struct sbefifo_client *client, char *buf,
@@ -422,7 +428,7 @@ static int occ_read_sbefifo(struct sbefifo_client *client, char *buf,
total += rc;
} while (total < len);
- return (total == len) ? 0 : -EMSGSIZE;
+ return (total == len) ? 0 : -ENODATA;
}
static int occ_getsram(struct device *sbefifo, u32 address, u8 *data,
@@ -430,10 +436,13 @@ static int occ_getsram(struct device *sbefifo, u32 address, u8 *data,
{
int rc;
u8 *resp;
- u32 buf[5];
- u32 data_len = ((len + 7) / 8) * 8;
+ __be32 buf[5];
+ u32 data_len = ((len + 7) / 8) * 8; /* must be multiples of 8 B */
struct sbefifo_client *client;
+ /* Magic sequence to do SBE getsram command. SBE will fetch data from
+ * specified SRAM address.
+ */
buf[0] = cpu_to_be32(0x5);
buf[1] = cpu_to_be32(0xa403);
buf[2] = cpu_to_be32(1);
@@ -447,7 +456,7 @@ static int occ_getsram(struct device *sbefifo, u32 address, u8 *data,
rc = occ_write_sbefifo(client, (const char *)buf, sizeof(buf));
if (rc)
goto done;
-
+
resp = kzalloc(data_len, GFP_KERNEL);
if (!resp) {
rc = -ENOMEM;
@@ -467,7 +476,7 @@ static int occ_getsram(struct device *sbefifo, u32 address, u8 *data,
(be32_to_cpu(buf[1]) == 0xC0DEA403))
memcpy(data, resp, len);
else
- rc = -EFAULT;
+ rc = -EBADMSG;
free:
kfree(resp);
@@ -481,8 +490,8 @@ static int occ_putsram(struct device *sbefifo, u32 address, u8 *data,
ssize_t len)
{
int rc;
- u32 *buf;
- u32 data_len = ((len + 7) / 8) * 8;
+ __be32 *buf;
+ u32 data_len = ((len + 7) / 8) * 8; /* must be multiples of 8 B */
size_t cmd_len = data_len + 20;
struct sbefifo_client *client;
@@ -490,6 +499,9 @@ static int occ_putsram(struct device *sbefifo, u32 address, u8 *data,
if (!buf)
return -ENOMEM;
+ /* Magic sequence to do SBE putsram command. SBE will transfer
+ * data to specified SRAM address.
+ */
buf[0] = cpu_to_be32(0x5 + (data_len / 4));
buf[1] = cpu_to_be32(0xa404);
buf[2] = cpu_to_be32(1);
@@ -515,7 +527,7 @@ static int occ_putsram(struct device *sbefifo, u32 address, u8 *data,
/* check for good response */
if ((be32_to_cpu(buf[0]) != data_len) ||
(be32_to_cpu(buf[1]) != 0xC0DEA404))
- rc = -EFAULT;
+ rc = -EBADMSG;
done:
sbefifo_drv_release(client);
@@ -527,14 +539,17 @@ static int occ_putsram(struct device *sbefifo, u32 address, u8 *data,
static int occ_trigger_attn(struct device *sbefifo)
{
int rc;
- u32 buf[6];
+ __be32 buf[6];
struct sbefifo_client *client;
+ /* Magic sequence to do SBE putscom command. SBE will write 8 bytes to
+ * specified SCOM address.
+ */
buf[0] = cpu_to_be32(0x6);
buf[1] = cpu_to_be32(0xa202);
buf[2] = 0;
buf[3] = cpu_to_be32(0x6D035);
- buf[4] = cpu_to_be32(0x20010000);
+ buf[4] = cpu_to_be32(0x20010000); /* trigger occ attention */
buf[5] = 0;
client = sbefifo_drv_open(sbefifo, 0);
@@ -552,7 +567,7 @@ static int occ_trigger_attn(struct device *sbefifo)
/* check for good response */
if ((be32_to_cpu(buf[0]) != 0xC0DEA202) ||
(be32_to_cpu(buf[1]) & 0x0FFFFFFF))
- rc = -EFAULT;
+ rc = -EBADMSG;
done:
sbefifo_drv_release(client);
@@ -564,12 +579,19 @@ static void occ_worker(struct work_struct *work)
{
int rc = 0, empty, waiting, canceled;
u16 resp_data_length;
+ 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);
struct device *sbefifo = occ->sbefifo;
again:
+ if (occ->cancel)
+ return;
+
spin_lock_irq(&occ->list_lock);
xfr = list_first_entry_or_null(&occ->xfrs, struct occ_xfr, link);
@@ -578,11 +600,14 @@ static void occ_worker(struct work_struct *work)
return;
}
+ resp = (struct occ_response *)xfr->buf;
set_bit(XFR_IN_PROGRESS, &xfr->flags);
spin_unlock_irq(&occ->list_lock);
mutex_lock(&occ->occ_lock);
+ /* write occ command */
+ start = jiffies;
rc = occ_putsram(sbefifo, 0xFFFBE000, xfr->buf,
xfr->cmd_data_length);
if (rc)
@@ -592,13 +617,26 @@ static void occ_worker(struct work_struct *work)
if (rc)
goto done;
- rc = occ_getsram(sbefifo, 0xFFFBF000, xfr->buf, 8);
- if (rc)
- goto done;
+ /* read occ response */
+ do {
+ rc = occ_getsram(sbefifo, 0xFFFBF000, xfr->buf, 8);
+ if (rc)
+ goto done;
+
+ if (resp->return_status == OCC_RESP_CMD_IN_PRG) {
+ rc = -EALREADY;
+
+ if (time_after(jiffies, start + timeout))
+ break;
- resp_data_length = (xfr->buf[3] << 8) + xfr->buf[4];
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule_timeout(wait_time);
+ }
+ } while (rc);
+
+ resp_data_length = get_unaligned_be16(&resp->data_length);
if (resp_data_length > OCC_RESP_DATA_BYTES) {
- rc = -EDOM;
+ rc = -EMSGSIZE;
goto done;
}
@@ -704,9 +742,6 @@ static int occ_probe(struct platform_device *pdev)
mutex_init(&occ->occ_lock);
INIT_WORK(&occ->work, occ_worker);
- /* ensure NULL before we probe children, so they don't hang FSI */
- platform_set_drvdata(pdev, NULL);
-
if (dev->of_node) {
rc = of_property_read_u32(dev->of_node, "reg", ®);
if (!rc) {
@@ -716,23 +751,13 @@ static int occ_probe(struct platform_device *pdev)
if (occ->idx < 0)
occ->idx = ida_simple_get(&occ_ida, 1, INT_MAX,
GFP_KERNEL);
- } else
+ } else {
occ->idx = ida_simple_get(&occ_ida, 1, INT_MAX,
GFP_KERNEL);
-
- /* create platform devs for dts child nodes (hwmon, etc) */
- for_each_child_of_node(dev->of_node, np) {
- snprintf(child_name, sizeof(child_name), "occ%d-dev%d",
- occ->idx, child_idx++);
- child = of_platform_device_create(np, child_name, dev);
- if (!child)
- dev_warn(dev,
- "failed to create child node dev\n");
}
- } else
+ } else {
occ->idx = ida_simple_get(&occ_ida, 1, INT_MAX, GFP_KERNEL);
-
- platform_set_drvdata(pdev, occ);
+ }
snprintf(occ->name, sizeof(occ->name), "occ%d", occ->idx);
occ->mdev.fops = &occ_fops;
@@ -742,20 +767,42 @@ static int occ_probe(struct platform_device *pdev)
rc = misc_register(&occ->mdev);
if (rc) {
- dev_err(dev, "failed to register miscdevice\n");
+ dev_err(dev, "failed to register miscdevice: %d\n", rc);
+ ida_simple_remove(&occ_ida, occ->idx);
return rc;
}
+ /* create platform devs for dts child nodes (hwmon, etc) */
+ for_each_available_child_of_node(dev->of_node, np) {
+ snprintf(child_name, sizeof(child_name), "occ%d-dev%d",
+ occ->idx, child_idx++);
+ child = of_platform_device_create(np, child_name, dev);
+ if (!child)
+ dev_warn(dev, "failed to create child node dev\n");
+ }
+
+ platform_set_drvdata(pdev, occ);
+
return 0;
}
static int occ_remove(struct platform_device *pdev)
{
struct occ *occ = platform_get_drvdata(pdev);
+ struct occ_xfr *xfr;
+ struct occ_client *client;
+
+ occ->cancel = true;
+ list_for_each_entry(xfr, &occ->xfrs, link) {
+ client = to_client(xfr);
+ wake_up_interruptible(&client->wait);
+ }
- flush_work(&occ->work);
misc_deregister(&occ->mdev);
device_for_each_child(&pdev->dev, NULL, occ_unregister_child);
+
+ flush_work(&occ->work);
+
ida_simple_remove(&occ_ida, occ->idx);
return 0;
@@ -789,6 +836,8 @@ static void occ_exit(void)
destroy_workqueue(occ_wq);
platform_driver_unregister(&occ_driver);
+
+ ida_destroy(&occ_ida);
}
module_init(occ_init);
diff --git a/include/linux/occ.h b/include/linux/occ.h
index d78332c..0a4a54a 100644
--- a/include/linux/occ.h
+++ b/include/linux/occ.h
@@ -11,12 +11,26 @@
* GNU General Public License for more details.
*/
-#ifndef __OCC_H__
-#define __OCC_H__
+#ifndef LINUX_FSI_OCC_H
+#define LINUX_FSI_OCC_H
struct device;
struct occ_client;
+#define OCC_RESP_CMD_IN_PRG 0xFF
+#define OCC_RESP_SUCCESS 0
+#define OCC_RESP_CMD_INVAL 0x11
+#define OCC_RESP_CMD_LEN_INVAL 0x12
+#define OCC_RESP_DATA_INVAL 0x13
+#define OCC_RESP_CHKSUM_ERR 0x14
+#define OCC_RESP_INT_ERR 0x15
+#define OCC_RESP_BAD_STATE 0x16
+#define OCC_RESP_CRIT_EXCEPT 0xE0
+#define OCC_RESP_CRIT_INIT 0xE1
+#define OCC_RESP_CRIT_WATCHDOG 0xE2
+#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);
@@ -24,4 +38,4 @@ extern int occ_drv_write(struct occ_client *client, const char *buf,
size_t len);
extern void occ_drv_release(struct occ_client *client);
-#endif /* __OCC_H__ */
+#endif /* LINUX_FSI_OCC_H */
--
1.8.3.1
More information about the openbmc
mailing list