Re: [RFC] simple_char: New infrastructure to simplify chardev management

Thoughts?  I want to use this for the u2f driver, which will either be
a chardev driver in its own right or use a simple new iso7816 class.

Ideally we could convert a bunch of drivers to use this, at least
where there are no legacy minor number considerations.

I'd really like to see a few consumer posted with it, to see how the
conversion works.

-			   topology.o container.o
+			   topology.o container.o simple_char.o

And the code should probably go into fs/char_dev.c, and have simple
char_ names to that people use it naturally.  A bit of documentation
of all the char interfaces and why you'd want to use this one would
also be useful for driver writers.

+static int simple_char_open(struct inode *inode, struct file *filep)
+{
+	struct simple_char_major *major =
+		container_of(inode->i_cdev, struct simple_char_major,
+			     cdev);
+	void *private;
+	const struct simple_char_ops *ops;
+	int ret = 0;
+
+	mutex_lock(&major->lock);
+
+	{
+		/*
+		 * This is a separate block to make the locking entirely
+		 * clear.  The only thing keeping minor alive is major->lock.
+		 * We need to be completely done with the simple_char_minor
+		 * by the time we release the lock.
+		 */
+		struct simple_char_minor *minor;
+		minor = idr_find(&major->idr, iminor(inode));
+		if (!minor || !minor->ops->reference(minor->private)) {
+			mutex_unlock(&major->lock);
+			return -ENODEV;
+		}
+		private = minor->private;
+		ops = minor->ops;
+	}
+
+	mutex_unlock(&major->lock);
+
+	replace_fops(filep, ops->fops);
+	filep->private_data = private;

So we're back to replace_fops here.  I would much prefer if this
would the regions interface so that we don't have to rely on a full
major allocation and replacing live file operations.

+EXPORT_SYMBOL(simple_char_major_create);

new exported function without any documentation

+
+void simple_char_major_free(struct simple_char_major *major)
+{
+	BUG_ON(!idr_is_empty(&major->idr));
+
+	cdev_del(&major->cdev);
+	unregister_chrdev_region(MKDEV(major->majornum, 0), MAX_MINORS);
+	idr_destroy(&major->idr);
+	kfree(major);
+}

non-static, non-exported function?

+/**
+ * simple_char_minor_create() - create a chardev minor
+ * @major:	Major to use or NULL for a fully dynamic chardev.
+ * @ops:	simple_char_ops to associate with the minor.
+ * @private:	opaque pointer for @ops's use.
+ *
+ * simple_char_minor_create() creates a minor chardev.  For new code,
+ * @major should be NULL; this will create a minor chardev with fully
+ * dynamic major and minor numbers and without a useful name in
+ * /proc/devices.  (All recent user code should be using sysfs
+ * exclusively to map between devices and device numbers.)  For legacy
+ * code, @major can come from simple_char_major_create().

Sounds like you should not pass @major for the main interface then,
and instead have a low-level interface that takes the major pointer.