Skip to contentSkip to conversation

fsi: Convert misc devs to proper chardevs and more

PATCH New Benjamin Herrenschmidt 1 participant 6 messages Jul 24, 2018 FSI

lore
Share & tools
lore
#0 lore raw

cover fsi: Convert misc devs to proper chardevs and more

This converts the various FSI devices from misc dev to chardev, as there can potentially be too much of them for misc devs limited minors, and because there are some lifetime issues with the current support.

This provide a common infrastructure to allocate an FSI major and distribute minors in a way that keeps it compatible with existing userspace. A new representation grouping FSI devices under a /dev/fsi directory is optinally provided, which will work in conjunction with new udev scripts aimed at providing fixed ID based symlinks.

A side effect of those conversions is to fix some object lifetime issues caused by a mixup between devm_kzalloc and proper object lifetime.

This series also adds a /dev{/fsi}/cfamN chardev for raw CFAM access that will superseed the existing "raw" sysfs file.

Finally there's also a locking fix to avoid horrible mixups if the "rescan" file is poked while a rescan is already in progress.

#1 lore raw

PATCH 1/5 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.

---
 drivers/fsi/Kconfig    | 15 +++++++
 drivers/fsi/fsi-core.c | 95 +++++++++++++++++++++++++++++++++++++++++-
 include/linux/fsi.h    | 12 +++++-
 3 files changed, 119 insertions(+), 3 deletions(-)
drivers/fsi/Kconfig +15 -0
index 8d82b1e60514..af3a20dd5aa4 100644
@@ -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
drivers/fsi/fsi-core.c +93 -2

131 lines not on this page · raw message

include/linux/fsi.h +11 -1
index 141fd38d061f..ec3be0d5b786 100644
@@ -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 */
Signature · 1 line
2.17.1
#2 lore raw

PATCH 2/5 fsi: sbefifo: Convert to use the new chardev

In reply to Benjamin Herrenschmidt

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.

---
 drivers/fsi/fsi-sbefifo.c | 84 +++++++++++++++++++++++++--------------
 1 file changed, 55 insertions(+), 29 deletions(-)
drivers/fsi/fsi-sbefifo.c +55 -29
index 33a5d9a43a07..8a185f53c9d8 100644
@@ -17,9 +17,8 @@
 #include <linux/fs.h>
 #include <linux/fsi.h>
 #include <linux/fsi-sbefifo.h>
-#include <linux/idr.h>
 #include <linux/kernel.h>
-#include <linux/miscdevice.h>
+#include <linux/cdev.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
@@ -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)) {
+		return -ENODEV;
+		kfree(sbefifo);
+	}
+
 	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);
Signature · 1 line
2.17.1
#3 lore raw

PATCH 3/5 fsi: scom: Convert to use the new chardev

In reply to Benjamin Herrenschmidt

This converts FSI scom to use the new fsi-core controlled chardev allocator and use a real cdev instead of a miscdev.

---
 drivers/fsi/fsi-scom.c | 130 +++++++++++++++++++++++++----------------
 1 file changed, 80 insertions(+), 50 deletions(-)
drivers/fsi/fsi-scom.c +80 -50
index 39c74351f1bf..0f303a700f69 100644
@@ -20,9 +20,8 @@
 #include <linux/fs.h>
 #include <linux/uaccess.h>
 #include <linux/slab.h>
-#include <linux/miscdevice.h>
+#include <linux/cdev.h>
 #include <linux/list.h>
-#include <linux/idr.h>
 
 #include <uapi/linux/fsi.h>
 
@@ -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);
 }
 
Signature · 1 line
2.17.1
#4 lore raw

PATCH 4/5 fsi: Add cfam char devices

In reply to Benjamin Herrenschmidt

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.

---
 drivers/fsi/fsi-core.c | 264 +++++++++++++++++++++++++++++++++--------
 1 file changed, 213 insertions(+), 51 deletions(-)
drivers/fsi/fsi-core.c +213 -51
index faa1760a5a40..dea5bd48acc5 100644
@@ -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 <linux/crc4.h>
@@ -21,6 +26,9 @@
 #include <linux/of.h>
 #include <linux/slab.h>
 #include <linux/bitops.h>
+#include <linux/cdev.h>
+#include <linux/fs.h>
+#include <linux/uaccess.h>
 
 #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;
 }
Signature · 1 line
2.17.1
#5 lore raw

PATCH 5/5 fsi: Prevent multiple concurrent rescans

In reply to Benjamin Herrenschmidt

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.

---
 drivers/fsi/fsi-core.c   | 16 ++++++++++++++--
 drivers/fsi/fsi-master.h |  2 ++
 2 files changed, 16 insertions(+), 2 deletions(-)
drivers/fsi/fsi-core.c +14 -2
index dea5bd48acc5..2c31563fdcae 100644
@@ -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);
drivers/fsi/fsi-master.h +2 -0
index f653f75da7be..040a7d4cf717 100644
@@ -18,6 +18,7 @@
 #define DRIVERS_FSI_MASTER_H
 
 #include <linux/device.h>
+#include <linux/mutex.h>
 
 /* 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,
Signature · 1 line
2.17.1