Merge tag 'fsi-updates-2018-07-27' of...

Merge tag 'fsi-updates-2018-07-27' of git://git.kernel.org/pub/scm/linux/kernel/git/benh/linux-fsi into char-misc-next

Ben writes:

Last round of FSI updates for 4.19

This adds a few fixes for things reported since the last merge,
and the latch batch of changes pending for FSI for 4.19.

That batch is a rather mechanical conversion of the misc devices
into proper char devices.

The misc devices were ill suited, the minor space for them is
limited and we can have a lot of chips in a system creating FSI
devices.

This also allows us to better control (and fix) object lifetime
getting rid of the bad devm_kzalloc() of the structures containing
the devices etc...

Finally, we add a chardev to the core FSI that provides raw CFAM
access to FSI slaves as a replacement for the current "raw" binary
sysfs file which will be ultimately deprecated and removed.

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

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;
@@ -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 @@ static int handle_response(struct fsi_master_acf *master,
 		}
 		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 @@ static int handle_response(struct fsi_master_acf *master,
 		 */
 		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 @@ static int handle_response(struct fsi_master_acf *master,
 	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;
 	}
@@ -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);
@@ -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;
 }
 

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,

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)) {
+		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);

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,6 +376,9 @@ static ssize_t scom_read(struct file *filep, char __user *buf, size_t len,
 		return -EINVAL;
 
 	mutex_lock(&scom->lock);
+	if (scom->dead)
+		rc = -ENODEV;
+	else
 		rc = get_scom(scom, &val, *offset);
 	mutex_unlock(&scom->lock);
 	if (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,6 +411,9 @@ static ssize_t scom_write(struct file *filep, const char __user *buf,
 	}
 
 	mutex_lock(&scom->lock);
+	if (scom->dead)
+		rc = -ENODEV;
+	else
 		rc = put_scom(scom, val, *offset);
 	mutex_unlock(&scom->lock);
 	if (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);
 }
 

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 */