> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
