From d0cff240b83babe72f5ba28044268b86aaa91d53 Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Tue, 24 Jul 2018 16:35:03 +1000 Subject: fsi: master-ast-cf: Fix build warnings on 64-bit platforms A couple of places forgot the 'z' qualifier for dev_dbg when printing a size_t Signed-off-by: Benjamin Herrenschmidt --- drivers/fsi/fsi-master-ast-cf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/fsi/fsi-master-ast-cf.c b/drivers/fsi/fsi-master-ast-cf.c index 57afaae0b691..c4a35579bfdc 100644 --- a/drivers/fsi/fsi-master-ast-cf.c +++ b/drivers/fsi/fsi-master-ast-cf.c @@ -606,7 +606,7 @@ static int fsi_master_acf_read(struct fsi_master *_master, int link, return -ENODEV; mutex_lock(&master->lock); - dev_dbg(master->dev, "read id %d addr %x size %ud\n", id, addr, size); + dev_dbg(master->dev, "read id %d addr %x size %zd\n", id, addr, size); build_ar_command(master, &cmd, id, addr, size, NULL); rc = fsi_master_acf_xfer(master, id, &cmd, size, val); last_address_update(master, id, rc == 0, addr); @@ -631,7 +631,7 @@ static int fsi_master_acf_write(struct fsi_master *_master, int link, mutex_lock(&master->lock); build_ar_command(master, &cmd, id, addr, size, val); - dev_dbg(master->dev, "write id %d addr %x size %ud raw_data: %08x\n", + dev_dbg(master->dev, "write id %d addr %x size %zd raw_data: %08x\n", id, addr, size, *(uint32_t *)val); rc = fsi_master_acf_xfer(master, id, &cmd, 0, NULL); last_address_update(master, id, rc == 0, addr); -- cgit v1.2.3-58-ga151 From 375cac70100b58ce8f4737ec530bfbdcb3086964 Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Wed, 25 Jul 2018 14:57:21 +1000 Subject: fsi: master-ast-cf: Mask unused bits in RTAG/RCRC Then reading the RTAG/RCRC "registers" from the coprocessor after a command is complete, mask out the top bits, only keep the relevant bits. Microcode v5 will leave garbage in those top bits as a result of a performance optimization. Signed-off-by: Benjamin Herrenschmidt --- --- drivers/fsi/fsi-master-ast-cf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/fsi/fsi-master-ast-cf.c b/drivers/fsi/fsi-master-ast-cf.c index c4a35579bfdc..b59abcf3ee32 100644 --- a/drivers/fsi/fsi-master-ast-cf.c +++ b/drivers/fsi/fsi-master-ast-cf.c @@ -377,8 +377,8 @@ static int send_request(struct fsi_master_acf *master, struct fsi_msg *cmd, static int read_copro_response(struct fsi_master_acf *master, uint8_t size, uint32_t *response, u8 *tag) { - uint8_t rtag = ioread8(master->sram + STAT_RTAG); - uint8_t rcrc = ioread8(master->sram + STAT_RCRC); + uint8_t rtag = ioread8(master->sram + STAT_RTAG) & 0xf; + uint8_t rcrc = ioread8(master->sram + STAT_RCRC) & 0xf; uint32_t rdata = 0; uint32_t crc; uint8_t ack; -- cgit v1.2.3-58-ga151 From 502defbb476bca222ddb5a78f2cc19c12c4e42f3 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Wed, 25 Jul 2018 08:38:32 -0500 Subject: fsi: master-ast-cf: Fix memory leak In case memory resources for *fw* were allocated, release them before return. Addresses-Coverity-ID: 1472044 ("Resource leak") Fixes: 6a794a27daca ("fsi: master-ast-cf: Add new FSI master using Aspeed ColdFire") Signed-off-by: Gustavo A. R. Silva Signed-off-by: Benjamin Herrenschmidt --- drivers/fsi/fsi-master-ast-cf.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/fsi/fsi-master-ast-cf.c b/drivers/fsi/fsi-master-ast-cf.c index b59abcf3ee32..6a36220e4b40 100644 --- a/drivers/fsi/fsi-master-ast-cf.c +++ b/drivers/fsi/fsi-master-ast-cf.c @@ -861,7 +861,8 @@ static int load_copro_firmware(struct fsi_master_acf *master) if (sig != wanted_sig) { dev_err(master->dev, "Failed to locate image sig %04x in FW blob\n", wanted_sig); - return -ENODEV; + rc = -ENODEV; + goto release_fw; } if (size > master->cf_mem_size) { dev_err(master->dev, "FW size (%zd) bigger than memory reserve (%zd)\n", @@ -870,8 +871,9 @@ static int load_copro_firmware(struct fsi_master_acf *master) } else { memcpy_toio(master->cf_mem, data, size); } - release_firmware(fw); +release_fw: + release_firmware(fw); return rc; } -- cgit v1.2.3-58-ga151 From 537052df223408b25bdd77dbb9bf40080f514d3c Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Thu, 26 Jul 2018 14:49:50 +1000 Subject: fsi: master-ast-cf: Rename dump_trace() to avoid name collision s390 defines a global dump_trace() symbol. Rename ours to dump_ucode_trace() to avoid a collision in build tests. Signed-off-by: Benjamin Herrenschmidt --- drivers/fsi/fsi-master-ast-cf.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/fsi/fsi-master-ast-cf.c b/drivers/fsi/fsi-master-ast-cf.c index 6a36220e4b40..04d10ea8d343 100644 --- a/drivers/fsi/fsi-master-ast-cf.c +++ b/drivers/fsi/fsi-master-ast-cf.c @@ -437,7 +437,7 @@ static int send_term(struct fsi_master_acf *master, uint8_t slave) return 0; } -static void dump_trace(struct fsi_master_acf *master) +static void dump_ucode_trace(struct fsi_master_acf *master) { char trbuf[52]; char *p; @@ -488,7 +488,7 @@ retry: } trace_fsi_master_acf_crc_rsp_error(master, crc_err_retries); if (master->trace_enabled) - dump_trace(master); + dump_ucode_trace(master); rc = clock_zeros(master, FSI_MASTER_EPOLL_CLOCKS); if (rc) { dev_warn(master->dev, @@ -525,7 +525,7 @@ retry: */ dev_dbg(master->dev, "Busy, retrying...\n"); if (master->trace_enabled) - dump_trace(master); + dump_ucode_trace(master); rc = clock_zeros(master, FSI_MASTER_DPOLL_CLOCKS); if (rc) { dev_warn(master->dev, @@ -550,13 +550,13 @@ retry: case FSI_RESP_ERRA: dev_dbg(master->dev, "ERRA received\n"); if (master->trace_enabled) - dump_trace(master); + dump_ucode_trace(master); rc = -EIO; break; case FSI_RESP_ERRC: dev_dbg(master->dev, "ERRC received\n"); if (master->trace_enabled) - dump_trace(master); + dump_ucode_trace(master); rc = -EAGAIN; break; } -- cgit v1.2.3-58-ga151 From 0ab5fe5374743d5a279b1ff6297ef2c54d06cd5f Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Wed, 20 Jun 2018 15:22:52 +1000 Subject: fsi: Add new central chardev support The various FSI devices (sbefifo, occ, scom, more to come) currently use misc devices. This is problematic as the minor device space for misc is limited and there can be a lot of them. Also it limits our ability to move them to a dedicated /dev/fsi directory or to be smart about device naming and numbering. It also means we have IDAs on every single of these drivers This creates a common fsi "device_type" for the optional /dev/fsi grouping and a dev_t allocator for all FSI devices. "Legacy" devices get to use a backward compatible numbering scheme (as long as chip id <16 and there's only one copy of a given unit type per chip). A single major number and a single IDA are shared for all FSI devices. This doesn't convert the FSI device drivers to use the new scheme yet, they will be converted individually. Signed-off-by: Benjamin Herrenschmidt --- drivers/fsi/Kconfig | 15 ++++++++ drivers/fsi/fsi-core.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++-- include/linux/fsi.h | 12 ++++++- 3 files changed, 119 insertions(+), 3 deletions(-) diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig index 8d82b1e60514..af3a20dd5aa4 100644 --- a/drivers/fsi/Kconfig +++ b/drivers/fsi/Kconfig @@ -12,6 +12,21 @@ menuconfig FSI if FSI +config FSI_NEW_DEV_NODE + bool "Create '/dev/fsi' directory for char devices" + default n + ---help--- + This option causes char devices created for FSI devices to be + located under a common /dev/fsi/ directory. Set to N unless your + userspace has been updated to handle the new location. + + Additionally, it also causes the char device names to be offset + by one so that chip 0 will have /dev/scom1 and chip1 /dev/scom2 + to match old userspace expectations. + + New userspace will use udev rules to generate predictable access + symlinks in /dev/fsi/by-path when this option is enabled. + config FSI_MASTER_GPIO tristate "GPIO-based FSI master" depends on GPIOLIB diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c index eab6c5c4990e..faa1760a5a40 100644 --- a/drivers/fsi/fsi-core.c +++ b/drivers/fsi/fsi-core.c @@ -92,6 +92,13 @@ struct fsi_slave { static const int slave_retries = 2; static int discard_errors; +static dev_t fsi_base_dev; +static DEFINE_IDA(fsi_minor_ida); +#define FSI_CHAR_MAX_DEVICES 0x1000 + +/* Legacy /dev numbering: 4 devices per chip, 16 chips */ +#define FSI_CHAR_LEGACY_TOP 64 + static int fsi_master_read(struct fsi_master *master, int link, uint8_t slave_id, uint32_t addr, void *val, size_t size); static int fsi_master_write(struct fsi_master *master, int link, @@ -627,6 +634,7 @@ static void fsi_slave_release(struct device *dev) { struct fsi_slave *slave = to_fsi_slave(dev); + fsi_free_minor(slave->dev.devt); of_node_put(dev->of_node); kfree(slave); } @@ -729,6 +737,75 @@ static ssize_t chip_id_show(struct device *dev, static DEVICE_ATTR_RO(chip_id); +static char *fsi_cdev_devnode(struct device *dev, umode_t *mode, + kuid_t *uid, kgid_t *gid) +{ +#ifdef CONFIG_FSI_NEW_DEV_NODE + return kasprintf(GFP_KERNEL, "fsi/%s", dev_name(dev)); +#else + return kasprintf(GFP_KERNEL, "%s", dev_name(dev)); +#endif +} + +const struct device_type fsi_cdev_type = { + .name = "fsi-cdev", + .devnode = fsi_cdev_devnode, +}; +EXPORT_SYMBOL_GPL(fsi_cdev_type); + +/* Backward compatible /dev/ numbering in "old style" mode */ +static int fsi_adjust_index(int index) +{ +#ifdef CONFIG_FSI_NEW_DEV_NODE + return index; +#else + return index + 1; +#endif +} + +static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type, + dev_t *out_dev, int *out_index) +{ + int cid = slave->chip_id; + int id; + + /* Check if we qualify for legacy numbering */ + if (cid >= 0 && cid < 16 && type < 4) { + /* Try reserving the legacy number */ + id = (cid << 4) | type; + id = ida_simple_get(&fsi_minor_ida, id, id + 1, GFP_KERNEL); + if (id >= 0) { + *out_index = fsi_adjust_index(cid); + *out_dev = fsi_base_dev + id; + return 0; + } + /* Other failure */ + if (id != -ENOSPC) + return id; + /* Fallback to non-legacy allocation */ + } + id = ida_simple_get(&fsi_minor_ida, FSI_CHAR_LEGACY_TOP, + FSI_CHAR_MAX_DEVICES, GFP_KERNEL); + if (id < 0) + return id; + *out_index = fsi_adjust_index(id); + *out_dev = fsi_base_dev + id; + return 0; +} + +int fsi_get_new_minor(struct fsi_device *fdev, enum fsi_dev_type type, + dev_t *out_dev, int *out_index) +{ + return __fsi_get_new_minor(fdev->slave, type, out_dev, out_index); +} +EXPORT_SYMBOL_GPL(fsi_get_new_minor); + +void fsi_free_minor(dev_t dev) +{ + ida_simple_remove(&fsi_minor_ida, MINOR(dev)); +} +EXPORT_SYMBOL_GPL(fsi_free_minor); + static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) { uint32_t chip_id; @@ -953,7 +1030,7 @@ static int fsi_slave_remove_device(struct device *dev, void *arg) static int fsi_master_remove_slave(struct device *dev, void *arg) { device_for_each_child(dev, NULL, fsi_slave_remove_device); - device_unregister(dev); + put_device(dev); return 0; } @@ -1091,13 +1168,27 @@ EXPORT_SYMBOL_GPL(fsi_bus_type); static int __init fsi_init(void) { - return bus_register(&fsi_bus_type); + int rc; + + rc = alloc_chrdev_region(&fsi_base_dev, 0, FSI_CHAR_MAX_DEVICES, "fsi"); + if (rc) + return rc; + rc = bus_register(&fsi_bus_type); + if (rc) + goto fail_bus; + return 0; + + fail_bus: + unregister_chrdev_region(fsi_base_dev, FSI_CHAR_MAX_DEVICES); + return rc; } postcore_initcall(fsi_init); static void fsi_exit(void) { bus_unregister(&fsi_bus_type); + unregister_chrdev_region(fsi_base_dev, FSI_CHAR_MAX_DEVICES); + ida_destroy(&fsi_minor_ida); } module_exit(fsi_exit); module_param(discard_errors, int, 0664); diff --git a/include/linux/fsi.h b/include/linux/fsi.h index 141fd38d061f..ec3be0d5b786 100644 --- a/include/linux/fsi.h +++ b/include/linux/fsi.h @@ -76,8 +76,18 @@ extern int fsi_slave_read(struct fsi_slave *slave, uint32_t addr, extern int fsi_slave_write(struct fsi_slave *slave, uint32_t addr, const void *val, size_t size); +extern struct bus_type fsi_bus_type; +extern const struct device_type fsi_cdev_type; +enum fsi_dev_type { + fsi_dev_cfam, + fsi_dev_sbefifo, + fsi_dev_scom, + fsi_dev_occ +}; -extern struct bus_type fsi_bus_type; +extern int fsi_get_new_minor(struct fsi_device *fdev, enum fsi_dev_type type, + dev_t *out_dev, int *out_index); +extern void fsi_free_minor(dev_t dev); #endif /* LINUX_FSI_H */ -- cgit v1.2.3-58-ga151 From 8b052dd64f998c7c8e0a0fd1d9feabc0e9bcfe42 Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Tue, 24 Jul 2018 14:39:55 +1000 Subject: fsi: sbefifo: Convert to use the new chardev This converts FSI sbefifo to use the new fsi-core controlled chardev allocator and use a real cdev instead of a miscdev. One side effect is to fix the object lifetime by removing the use of devm_kzalloc() for something that contains kobjects, and using proper reference counting. Signed-off-by: Benjamin Herrenschmidt --- drivers/fsi/fsi-sbefifo.c | 84 +++++++++++++++++++++++++++++++---------------- 1 file changed, 55 insertions(+), 29 deletions(-) diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c index 33a5d9a43a07..7550497da8ef 100644 --- a/drivers/fsi/fsi-sbefifo.c +++ b/drivers/fsi/fsi-sbefifo.c @@ -17,9 +17,8 @@ #include #include #include -#include #include -#include +#include #include #include #include @@ -118,11 +117,11 @@ struct sbefifo { uint32_t magic; #define SBEFIFO_MAGIC 0x53424546 /* "SBEF" */ struct fsi_device *fsi_dev; - struct miscdevice mdev; + struct device dev; + struct cdev cdev; struct mutex lock; - char name[32]; - int idx; bool broken; + bool dead; bool async_ffdc; }; @@ -133,9 +132,9 @@ struct sbefifo_user { size_t pending_len; }; -static DEFINE_IDA(sbefifo_ida); static DEFINE_MUTEX(sbefifo_ffdc_mutex); + static void __sbefifo_dump_ffdc(struct device *dev, const __be32 *ffdc, size_t ffdc_sz, bool internal) { @@ -667,6 +666,9 @@ static int __sbefifo_submit(struct sbefifo *sbefifo, struct device *dev = &sbefifo->fsi_dev->dev; int rc; + if (sbefifo->dead) + return -ENODEV; + if (cmd_len < 2 || be32_to_cpu(command[0]) != cmd_len) { dev_vdbg(dev, "Invalid command len %zd (header: %d)\n", cmd_len, be32_to_cpu(command[0])); @@ -751,8 +753,7 @@ EXPORT_SYMBOL_GPL(sbefifo_submit); */ static int sbefifo_user_open(struct inode *inode, struct file *file) { - struct sbefifo *sbefifo = container_of(file->private_data, - struct sbefifo, mdev); + struct sbefifo *sbefifo = container_of(inode->i_cdev, struct sbefifo, cdev); struct sbefifo_user *user; user = kzalloc(sizeof(struct sbefifo_user), GFP_KERNEL); @@ -889,6 +890,14 @@ static const struct file_operations sbefifo_fops = { .release = sbefifo_user_release, }; +static void sbefifo_free(struct device *dev) +{ + struct sbefifo *sbefifo = container_of(dev, struct sbefifo, dev); + + put_device(&sbefifo->fsi_dev->dev); + kfree(sbefifo); +} + /* * Probe/remove */ @@ -900,15 +909,23 @@ static int sbefifo_probe(struct device *dev) struct device_node *np; struct platform_device *child; char child_name[32]; - int rc, child_idx = 0; + int rc, didx, child_idx = 0; dev_dbg(dev, "Found sbefifo device\n"); - sbefifo = devm_kzalloc(dev, sizeof(*sbefifo), GFP_KERNEL); + sbefifo = kzalloc(sizeof(*sbefifo), GFP_KERNEL); if (!sbefifo) return -ENOMEM; + + /* Grab a reference to the device (parent of our cdev), we'll drop it later */ + if (!get_device(dev)) { + kfree(sbefifo); + return -ENODEV; + } + sbefifo->magic = SBEFIFO_MAGIC; sbefifo->fsi_dev = fsi_dev; + dev_set_drvdata(dev, sbefifo); mutex_init(&sbefifo->lock); /* @@ -919,28 +936,30 @@ static int sbefifo_probe(struct device *dev) if (rc && rc != -ESHUTDOWN) dev_err(dev, "Initial HW cleanup failed, will retry later\n"); - sbefifo->idx = ida_simple_get(&sbefifo_ida, 1, INT_MAX, GFP_KERNEL); - snprintf(sbefifo->name, sizeof(sbefifo->name), "sbefifo%d", - sbefifo->idx); + /* Create chardev for userspace access */ + sbefifo->dev.type = &fsi_cdev_type; + sbefifo->dev.parent = dev; + sbefifo->dev.release = sbefifo_free; + device_initialize(&sbefifo->dev); - dev_set_drvdata(dev, sbefifo); + /* Allocate a minor in the FSI space */ + rc = fsi_get_new_minor(fsi_dev, fsi_dev_sbefifo, &sbefifo->dev.devt, &didx); + if (rc) + goto err; - /* Create misc chardev for userspace access */ - sbefifo->mdev.minor = MISC_DYNAMIC_MINOR; - sbefifo->mdev.fops = &sbefifo_fops; - sbefifo->mdev.name = sbefifo->name; - sbefifo->mdev.parent = dev; - rc = misc_register(&sbefifo->mdev); + dev_set_name(&sbefifo->dev, "sbefifo%d", didx); + cdev_init(&sbefifo->cdev, &sbefifo_fops); + rc = cdev_device_add(&sbefifo->cdev, &sbefifo->dev); if (rc) { - dev_err(dev, "Failed to register miscdevice: %d\n", rc); - ida_simple_remove(&sbefifo_ida, sbefifo->idx); - return rc; + dev_err(dev, "Error %d creating char device %s\n", + rc, dev_name(&sbefifo->dev)); + goto err_free_minor; } /* Create platform devs for dts child nodes (occ, etc) */ for_each_available_child_of_node(dev->of_node, np) { snprintf(child_name, sizeof(child_name), "%s-dev%d", - sbefifo->name, child_idx++); + dev_name(&sbefifo->dev), child_idx++); child = of_platform_device_create(np, child_name, dev); if (!child) dev_warn(dev, "failed to create child %s dev\n", @@ -948,6 +967,11 @@ static int sbefifo_probe(struct device *dev) } return 0; + err_free_minor: + fsi_free_minor(sbefifo->dev.devt); + err: + put_device(&sbefifo->dev); + return rc; } static int sbefifo_unregister_child(struct device *dev, void *data) @@ -967,10 +991,14 @@ static int sbefifo_remove(struct device *dev) dev_dbg(dev, "Removing sbefifo device...\n"); - misc_deregister(&sbefifo->mdev); - device_for_each_child(dev, NULL, sbefifo_unregister_child); + mutex_lock(&sbefifo->lock); + sbefifo->dead = true; + mutex_unlock(&sbefifo->lock); - ida_simple_remove(&sbefifo_ida, sbefifo->idx); + cdev_device_del(&sbefifo->cdev, &sbefifo->dev); + fsi_free_minor(sbefifo->dev.devt); + device_for_each_child(dev, NULL, sbefifo_unregister_child); + put_device(&sbefifo->dev); return 0; } @@ -1001,8 +1029,6 @@ static int sbefifo_init(void) static void sbefifo_exit(void) { fsi_driver_unregister(&sbefifo_drv); - - ida_destroy(&sbefifo_ida); } module_init(sbefifo_init); -- cgit v1.2.3-58-ga151 From d8f4587655f9682127351a9dc3fca61e70744294 Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Wed, 20 Jun 2018 15:33:03 +1000 Subject: fsi: scom: Convert to use the new chardev This converts FSI scom to use the new fsi-core controlled chardev allocator and use a real cdev instead of a miscdev. Signed-off-by: Benjamin Herrenschmidt --- drivers/fsi/fsi-scom.c | 130 ++++++++++++++++++++++++++++++------------------- 1 file changed, 80 insertions(+), 50 deletions(-) diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c index 39c74351f1bf..0f303a700f69 100644 --- a/drivers/fsi/fsi-scom.c +++ b/drivers/fsi/fsi-scom.c @@ -20,9 +20,8 @@ #include #include #include -#include +#include #include -#include #include @@ -77,18 +76,12 @@ struct scom_device { struct list_head link; struct fsi_device *fsi_dev; - struct miscdevice mdev; + struct device dev; + struct cdev cdev; struct mutex lock; - char name[32]; - int idx; + bool dead; }; -#define to_scom_dev(x) container_of((x), struct scom_device, mdev) - -static struct list_head scom_devices; - -static DEFINE_IDA(scom_ida); - static int __put_scom(struct scom_device *scom_dev, uint64_t value, uint32_t addr, uint32_t *status) { @@ -374,9 +367,7 @@ static int get_scom(struct scom_device *scom, uint64_t *value, static ssize_t scom_read(struct file *filep, char __user *buf, size_t len, loff_t *offset) { - struct miscdevice *mdev = - (struct miscdevice *)filep->private_data; - struct scom_device *scom = to_scom_dev(mdev); + struct scom_device *scom = filep->private_data; struct device *dev = &scom->fsi_dev->dev; uint64_t val; int rc; @@ -385,7 +376,10 @@ static ssize_t scom_read(struct file *filep, char __user *buf, size_t len, return -EINVAL; mutex_lock(&scom->lock); - rc = get_scom(scom, &val, *offset); + if (scom->dead) + rc = -ENODEV; + else + rc = get_scom(scom, &val, *offset); mutex_unlock(&scom->lock); if (rc) { dev_dbg(dev, "get_scom fail:%d\n", rc); @@ -403,8 +397,7 @@ static ssize_t scom_write(struct file *filep, const char __user *buf, size_t len, loff_t *offset) { int rc; - struct miscdevice *mdev = filep->private_data; - struct scom_device *scom = to_scom_dev(mdev); + struct scom_device *scom = filep->private_data; struct device *dev = &scom->fsi_dev->dev; uint64_t val; @@ -418,7 +411,10 @@ static ssize_t scom_write(struct file *filep, const char __user *buf, } mutex_lock(&scom->lock); - rc = put_scom(scom, val, *offset); + if (scom->dead) + rc = -ENODEV; + else + rc = put_scom(scom, val, *offset); mutex_unlock(&scom->lock); if (rc) { dev_dbg(dev, "put_scom failed with:%d\n", rc); @@ -532,12 +528,15 @@ static int scom_check(struct scom_device *scom, void __user *argp) static long scom_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { - struct miscdevice *mdev = file->private_data; - struct scom_device *scom = to_scom_dev(mdev); + struct scom_device *scom = file->private_data; void __user *argp = (void __user *)arg; int rc = -ENOTTY; mutex_lock(&scom->lock); + if (scom->dead) { + mutex_unlock(&scom->lock); + return -ENODEV; + } switch(cmd) { case FSI_SCOM_CHECK: rc = scom_check(scom, argp); @@ -556,48 +555,88 @@ static long scom_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return rc; } +static int scom_open(struct inode *inode, struct file *file) +{ + struct scom_device *scom = container_of(inode->i_cdev, struct scom_device, cdev); + + file->private_data = scom; + + return 0; +} + static const struct file_operations scom_fops = { .owner = THIS_MODULE, + .open = scom_open, .llseek = scom_llseek, .read = scom_read, .write = scom_write, .unlocked_ioctl = scom_ioctl, }; +static void scom_free(struct device *dev) +{ + struct scom_device *scom = container_of(dev, struct scom_device, dev); + + put_device(&scom->fsi_dev->dev); + kfree(scom); +} + static int scom_probe(struct device *dev) { struct fsi_device *fsi_dev = to_fsi_dev(dev); struct scom_device *scom; + int rc, didx; - scom = devm_kzalloc(dev, sizeof(*scom), GFP_KERNEL); + scom = kzalloc(sizeof(*scom), GFP_KERNEL); if (!scom) return -ENOMEM; - + dev_set_drvdata(dev, scom); mutex_init(&scom->lock); - scom->idx = ida_simple_get(&scom_ida, 1, INT_MAX, GFP_KERNEL); - snprintf(scom->name, sizeof(scom->name), "scom%d", scom->idx); - scom->fsi_dev = fsi_dev; - scom->mdev.minor = MISC_DYNAMIC_MINOR; - scom->mdev.fops = &scom_fops; - scom->mdev.name = scom->name; - scom->mdev.parent = dev; - list_add(&scom->link, &scom_devices); - - return misc_register(&scom->mdev); + + /* Grab a reference to the device (parent of our cdev), we'll drop it later */ + if (!get_device(dev)) { + kfree(scom); + return -ENODEV; + } + + /* Create chardev for userspace access */ + scom->dev.type = &fsi_cdev_type; + scom->dev.parent = dev; + scom->dev.release = scom_free; + device_initialize(&scom->dev); + + /* Allocate a minor in the FSI space */ + rc = fsi_get_new_minor(fsi_dev, fsi_dev_scom, &scom->dev.devt, &didx); + if (rc) + goto err; + + dev_set_name(&scom->dev, "scom%d", didx); + cdev_init(&scom->cdev, &scom_fops); + rc = cdev_device_add(&scom->cdev, &scom->dev); + if (rc) { + dev_err(dev, "Error %d creating char device %s\n", + rc, dev_name(&scom->dev)); + goto err_free_minor; + } + + return 0; + err_free_minor: + fsi_free_minor(scom->dev.devt); + err: + put_device(&scom->dev); + return rc; } static int scom_remove(struct device *dev) { - struct scom_device *scom, *scom_tmp; - struct fsi_device *fsi_dev = to_fsi_dev(dev); + struct scom_device *scom = dev_get_drvdata(dev); - list_for_each_entry_safe(scom, scom_tmp, &scom_devices, link) { - if (scom->fsi_dev == fsi_dev) { - list_del(&scom->link); - ida_simple_remove(&scom_ida, scom->idx); - misc_deregister(&scom->mdev); - } - } + mutex_lock(&scom->lock); + scom->dead = true; + mutex_unlock(&scom->lock); + cdev_device_del(&scom->cdev, &scom->dev); + fsi_free_minor(scom->dev.devt); + put_device(&scom->dev); return 0; } @@ -622,20 +661,11 @@ static struct fsi_driver scom_drv = { static int scom_init(void) { - INIT_LIST_HEAD(&scom_devices); return fsi_driver_register(&scom_drv); } static void scom_exit(void) { - struct list_head *pos; - struct scom_device *scom; - - list_for_each(pos, &scom_devices) { - scom = list_entry(pos, struct scom_device, link); - misc_deregister(&scom->mdev); - devm_kfree(&scom->fsi_dev->dev, scom); - } fsi_driver_unregister(&scom_drv); } -- cgit v1.2.3-58-ga151 From d1dcd678257603e71cf3f3d84c70e2b6f0f14bb8 Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Thu, 21 Jun 2018 12:34:22 +1000 Subject: fsi: Add cfam char devices This aims to deprecate the "raw" sysfs file used for directly accessing the CFAM and instead use a char device like the other sub drivers. Since it reworks the slave creation code and adds a cfam device type, we also use the opportunity to convert the attributes to attribute groups and add a couple more. Signed-off-by: Benjamin Herrenschmidt --- drivers/fsi/fsi-core.c | 264 +++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 213 insertions(+), 51 deletions(-) diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c index faa1760a5a40..dea5bd48acc5 100644 --- a/drivers/fsi/fsi-core.c +++ b/drivers/fsi/fsi-core.c @@ -11,6 +11,11 @@ * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. + * + * TODO: + * - Rework topology + * - s/chip_id/chip_loc + * - s/cfam/chip (cfam_id -> chip_id etc...) */ #include @@ -21,6 +26,9 @@ #include #include #include +#include +#include +#include #include "fsi-master.h" @@ -78,8 +86,11 @@ static DEFINE_IDA(master_ida); struct fsi_slave { struct device dev; struct fsi_master *master; - int id; - int link; + struct cdev cdev; + int cdev_idx; + int id; /* FSI address */ + int link; /* FSI link# */ + u32 cfam_id; int chip_id; uint32_t size; /* size of slave address space */ u8 t_send_delay; @@ -607,29 +618,6 @@ static const struct bin_attribute fsi_slave_raw_attr = { .write = fsi_slave_sysfs_raw_write, }; -static ssize_t fsi_slave_sysfs_term_write(struct file *file, - struct kobject *kobj, struct bin_attribute *attr, - char *buf, loff_t off, size_t count) -{ - struct fsi_slave *slave = to_fsi_slave(kobj_to_dev(kobj)); - struct fsi_master *master = slave->master; - - if (!master->term) - return -ENODEV; - - master->term(master, slave->link, slave->id); - return count; -} - -static const struct bin_attribute fsi_slave_term_attr = { - .attr = { - .name = "term", - .mode = 0200, - }, - .size = 0, - .write = fsi_slave_sysfs_term_write, -}; - static void fsi_slave_release(struct device *dev) { struct fsi_slave *slave = to_fsi_slave(dev); @@ -682,6 +670,127 @@ static struct device_node *fsi_slave_find_of_node(struct fsi_master *master, return NULL; } +static ssize_t cfam_read(struct file *filep, char __user *buf, size_t count, + loff_t *offset) +{ + struct fsi_slave *slave = filep->private_data; + size_t total_len, read_len; + loff_t off = *offset; + ssize_t rc; + + if (off < 0) + return -EINVAL; + + if (off > 0xffffffff || count > 0xffffffff || off + count > 0xffffffff) + return -EINVAL; + + for (total_len = 0; total_len < count; total_len += read_len) { + __be32 data; + + read_len = min_t(size_t, count, 4); + read_len -= off & 0x3; + + rc = fsi_slave_read(slave, off, &data, read_len); + if (rc) + goto fail; + rc = copy_to_user(buf + total_len, &data, read_len); + if (rc) { + rc = -EFAULT; + goto fail; + } + off += read_len; + } + rc = count; + fail: + *offset = off; + return count; +} + +static ssize_t cfam_write(struct file *filep, const char __user *buf, + size_t count, loff_t *offset) +{ + struct fsi_slave *slave = filep->private_data; + size_t total_len, write_len; + loff_t off = *offset; + ssize_t rc; + + + if (off < 0) + return -EINVAL; + + if (off > 0xffffffff || count > 0xffffffff || off + count > 0xffffffff) + return -EINVAL; + + for (total_len = 0; total_len < count; total_len += write_len) { + __be32 data; + + write_len = min_t(size_t, count, 4); + write_len -= off & 0x3; + + rc = copy_from_user(&data, buf + total_len, write_len); + if (rc) { + rc = -EFAULT; + goto fail; + } + rc = fsi_slave_write(slave, off, &data, write_len); + if (rc) + goto fail; + off += write_len; + } + rc = count; + fail: + *offset = off; + return count; +} + +static loff_t cfam_llseek(struct file *file, loff_t offset, int whence) +{ + switch (whence) { + case SEEK_CUR: + break; + case SEEK_SET: + file->f_pos = offset; + break; + default: + return -EINVAL; + } + + return offset; +} + +static int cfam_open(struct inode *inode, struct file *file) +{ + struct fsi_slave *slave = container_of(inode->i_cdev, struct fsi_slave, cdev); + + file->private_data = slave; + + return 0; +} + +static const struct file_operations cfam_fops = { + .owner = THIS_MODULE, + .open = cfam_open, + .llseek = cfam_llseek, + .read = cfam_read, + .write = cfam_write, +}; + +static ssize_t send_term_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct fsi_slave *slave = to_fsi_slave(dev); + struct fsi_master *master = slave->master; + + if (!master->term) + return -ENODEV; + + master->term(master, slave->link, slave->id); + return count; +} + +static DEVICE_ATTR_WO(send_term); + static ssize_t slave_send_echo_show(struct device *dev, struct device_attribute *attr, char *buf) @@ -737,6 +846,52 @@ static ssize_t chip_id_show(struct device *dev, static DEVICE_ATTR_RO(chip_id); +static ssize_t cfam_id_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct fsi_slave *slave = to_fsi_slave(dev); + + return sprintf(buf, "0x%x\n", slave->cfam_id); +} + +static DEVICE_ATTR_RO(cfam_id); + +static struct attribute *cfam_attr[] = { + &dev_attr_send_echo_delays.attr, + &dev_attr_chip_id.attr, + &dev_attr_cfam_id.attr, + &dev_attr_send_term.attr, + NULL, +}; + +static const struct attribute_group cfam_attr_group = { + .attrs = cfam_attr, +}; + +static const struct attribute_group *cfam_attr_groups[] = { + &cfam_attr_group, + NULL, +}; + +static char *cfam_devnode(struct device *dev, umode_t *mode, + kuid_t *uid, kgid_t *gid) +{ + struct fsi_slave *slave = to_fsi_slave(dev); + +#ifdef CONFIG_FSI_NEW_DEV_NODE + return kasprintf(GFP_KERNEL, "fsi/cfam%d", slave->cdev_idx); +#else + return kasprintf(GFP_KERNEL, "cfam%d", slave->cdev_idx); +#endif +} + +static const struct device_type cfam_type = { + .name = "cfam", + .devnode = cfam_devnode, + .groups = cfam_attr_groups +}; + static char *fsi_cdev_devnode(struct device *dev, umode_t *mode, kuid_t *uid, kgid_t *gid) { @@ -808,7 +963,7 @@ EXPORT_SYMBOL_GPL(fsi_free_minor); static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) { - uint32_t chip_id; + uint32_t cfam_id; struct fsi_slave *slave; uint8_t crc; __be32 data, llmode; @@ -826,17 +981,17 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) link, id, rc); return -ENODEV; } - chip_id = be32_to_cpu(data); + cfam_id = be32_to_cpu(data); - crc = crc4(0, chip_id, 32); + crc = crc4(0, cfam_id, 32); if (crc) { - dev_warn(&master->dev, "slave %02x:%02x invalid chip id CRC!\n", + dev_warn(&master->dev, "slave %02x:%02x invalid cfam id CRC!\n", link, id); return -EIO; } dev_dbg(&master->dev, "fsi: found chip %08x at %02x:%02x:%02x\n", - chip_id, master->idx, link, id); + cfam_id, master->idx, link, id); /* If we're behind a master that doesn't provide a self-running bus * clock, put the slave into async mode @@ -859,10 +1014,14 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) if (!slave) return -ENOMEM; - slave->master = master; + dev_set_name(&slave->dev, "slave@%02x:%02x", link, id); + slave->dev.type = &cfam_type; slave->dev.parent = &master->dev; slave->dev.of_node = fsi_slave_find_of_node(master, link, id); slave->dev.release = fsi_slave_release; + device_initialize(&slave->dev); + slave->cfam_id = cfam_id; + slave->master = master; slave->link = link; slave->id = id; slave->size = FSI_SLAVE_SIZE_23b; @@ -877,6 +1036,21 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) slave->chip_id = prop; } + + /* Allocate a minor in the FSI space */ + rc = __fsi_get_new_minor(slave, fsi_dev_cfam, &slave->dev.devt, + &slave->cdev_idx); + if (rc) + goto err_free; + + /* Create chardev for userspace access */ + cdev_init(&slave->cdev, &cfam_fops); + rc = cdev_device_add(&slave->cdev, &slave->dev); + if (rc) { + dev_err(&slave->dev, "Error %d creating slave device\n", rc); + goto err_free; + } + rc = fsi_slave_set_smode(slave); if (rc) { dev_warn(&master->dev, @@ -890,30 +1064,11 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) slave->t_send_delay, slave->t_echo_delay); - dev_set_name(&slave->dev, "slave@%02x:%02x", link, id); - rc = device_register(&slave->dev); - if (rc < 0) { - dev_warn(&master->dev, "failed to create slave device: %d\n", - rc); - put_device(&slave->dev); - return rc; - } - + /* Legacy raw file -> to be removed */ rc = device_create_bin_file(&slave->dev, &fsi_slave_raw_attr); if (rc) dev_warn(&slave->dev, "failed to create raw attr: %d\n", rc); - rc = device_create_bin_file(&slave->dev, &fsi_slave_term_attr); - if (rc) - dev_warn(&slave->dev, "failed to create term attr: %d\n", rc); - - rc = device_create_file(&slave->dev, &dev_attr_send_echo_delays); - if (rc) - dev_warn(&slave->dev, "failed to create delay attr: %d\n", rc); - - rc = device_create_file(&slave->dev, &dev_attr_chip_id); - if (rc) - dev_warn(&slave->dev, "failed to create chip id: %d\n", rc); rc = fsi_slave_scan(slave); if (rc) @@ -921,6 +1076,10 @@ static int fsi_slave_init(struct fsi_master *master, int link, uint8_t id) rc); return rc; + + err_free: + put_device(&slave->dev); + return rc; } /* FSI master support */ @@ -1029,7 +1188,10 @@ static int fsi_slave_remove_device(struct device *dev, void *arg) static int fsi_master_remove_slave(struct device *dev, void *arg) { + struct fsi_slave *slave = to_fsi_slave(dev); + device_for_each_child(dev, NULL, fsi_slave_remove_device); + cdev_device_del(&slave->cdev, &slave->dev); put_device(dev); return 0; } -- cgit v1.2.3-58-ga151 From 9840fcd8cc43bfba486a53b4461044f1a1189cdc Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Thu, 21 Jun 2018 18:00:05 +1000 Subject: fsi: Prevent multiple concurrent rescans The bus scanning process isn't terribly good at parallel attempts at rescanning the same bus. Let's have a per-master mutex protecting the scanning process. Signed-off-by: Benjamin Herrenschmidt --- drivers/fsi/fsi-core.c | 16 ++++++++++++++-- drivers/fsi/fsi-master.h | 2 ++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c index dea5bd48acc5..2c31563fdcae 100644 --- a/drivers/fsi/fsi-core.c +++ b/drivers/fsi/fsi-core.c @@ -1203,8 +1203,14 @@ static void fsi_master_unscan(struct fsi_master *master) int fsi_master_rescan(struct fsi_master *master) { + int rc; + + mutex_lock(&master->scan_lock); fsi_master_unscan(master); - return fsi_master_scan(master); + rc = fsi_master_scan(master); + mutex_unlock(&master->scan_lock); + + return rc; } EXPORT_SYMBOL_GPL(fsi_master_rescan); @@ -1240,6 +1246,7 @@ int fsi_master_register(struct fsi_master *master) int rc; struct device_node *np; + mutex_init(&master->scan_lock); master->idx = ida_simple_get(&master_ida, 0, INT_MAX, GFP_KERNEL); dev_set_name(&master->dev, "fsi%d", master->idx); @@ -1264,8 +1271,11 @@ int fsi_master_register(struct fsi_master *master) } np = dev_of_node(&master->dev); - if (!of_property_read_bool(np, "no-scan-on-init")) + if (!of_property_read_bool(np, "no-scan-on-init")) { + mutex_lock(&master->scan_lock); fsi_master_scan(master); + mutex_unlock(&master->scan_lock); + } return 0; } @@ -1278,7 +1288,9 @@ void fsi_master_unregister(struct fsi_master *master) master->idx = -1; } + mutex_lock(&master->scan_lock); fsi_master_unscan(master); + mutex_unlock(&master->scan_lock); device_unregister(&master->dev); } EXPORT_SYMBOL_GPL(fsi_master_unregister); diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h index f653f75da7be..040a7d4cf717 100644 --- a/drivers/fsi/fsi-master.h +++ b/drivers/fsi/fsi-master.h @@ -18,6 +18,7 @@ #define DRIVERS_FSI_MASTER_H #include +#include /* Various protocol delays */ #define FSI_ECHO_DELAY_CLOCKS 16 /* Number clocks for echo delay */ @@ -59,6 +60,7 @@ struct fsi_master { int idx; int n_links; int flags; + struct mutex scan_lock; int (*read)(struct fsi_master *, int link, uint8_t id, uint32_t addr, void *val, size_t size); int (*write)(struct fsi_master *, int link, uint8_t id, -- cgit v1.2.3-58-ga151