[PATCH 0/5] fsi: Convert misc devs to proper chardevs and more

[PATCH 0/5] fsi: Convert misc devs to proper chardevs and more

[Benjamin Herrenschmidt]

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.

[PATCH 1/5] fsi: Add new central chardev support

[Benjamin Herrenschmidt]

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 <benh@kernel.crashing.org>
---
 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 */
-- 
2.17.1

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

[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.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 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..8a185f53c9d8 100644
--- a/drivers/fsi/fsi-sbefifo.c
+++ b/drivers/fsi/fsi-sbefifo.c
@@ -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);
-- 
2.17.1

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

[Benjamin Herrenschmidt]

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 <benh@kernel.crashing.org>
---
 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 <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);
 }
 
-- 
2.17.1

[PATCH 4/5] fsi: Add cfam char devices

[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.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 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 <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;
 }
-- 
2.17.1

[PATCH 5/5] fsi: Prevent multiple concurrent rescans

[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.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 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 <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,
-- 
2.17.1