Input: prepare to sysfs integration
Add struct class_device to input_dev; add input_allocate_dev()
to dynamically allocate input devices; dynamically allocated
devices are automatically registered with sysfs.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/input.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++----
include/linux/input.h | 24 ++++++++++++++-
2 files changed, 95 insertions(+), 6 deletions(-)
Index: work/include/linux/input.h
===================================================================
--- work.orig/include/linux/input.h
+++ work/include/linux/input.h
@@ -12,6 +12,7 @@
#ifdef __KERNEL__
#include <linux/time.h>
#include <linux/list.h>
+#include <linux/device.h>
#else
#include <sys/time.h>
#include <sys/ioctl.h>
@@ -889,11 +890,15 @@ struct input_dev {
struct semaphore sem; /* serializes open and close operations */
unsigned int users;
- struct device *dev;
+ struct class_device cdev;
+ struct device *dev; /* will be removed soon */
+
+ int dynalloc; /* temporarily */
struct list_head h_list;
struct list_head node;
};
+#define to_input_dev(d) container_of(d, struct input_dev, cdev)
/*
* Structure for hotplug & device<->driver matching.
@@ -984,6 +989,23 @@ static inline void init_input_dev(struct
INIT_LIST_HEAD(&dev->node);
}
+struct input_dev *input_allocate_device(void);
+
+static inline void input_free_device(struct input_dev *dev)
+{
+ kfree(dev);
+}
+
+static inline struct input_dev *input_get_device(struct input_dev *dev)
+{
+ return to_input_dev(class_device_get(&dev->cdev));
+}
+
+static inline void input_put_device(struct input_dev *dev)
+{
+ class_device_put(&dev->cdev);
+}
+
void input_register_device(struct input_dev *);
void input_unregister_device(struct input_dev *);
Index: work/drivers/input/input.c
===================================================================
--- work.orig/drivers/input/input.c
+++ work/drivers/input/input.c
@@ -27,6 +27,7 @@ MODULE_AUTHOR("Vojtech Pavlik <vojtech@s
MODULE_DESCRIPTION("Input core");
MODULE_LICENSE("GPL");
+EXPORT_SYMBOL(input_allocate_device);
EXPORT_SYMBOL(input_register_device);
EXPORT_SYMBOL(input_unregister_device);
EXPORT_SYMBOL(input_register_handler);
@@ -604,6 +605,56 @@ static inline int input_proc_init(void)
static inline void input_proc_exit(void) { }
#endif
+static void input_dev_release(struct class_device *class_dev)
+{
+ struct input_dev *dev = to_input_dev(class_dev);
+
+ kfree(dev);
+ module_put(THIS_MODULE);
+}
+
+static struct class input_dev_class = {
+ .name = "input_dev",
+ .release = input_dev_release,
+};
+
+struct input_dev *input_allocate_device(void)
+{
+ struct input_dev *dev;
+
+ dev = kzalloc(sizeof(struct input_dev), GFP_KERNEL);
+ if (dev) {
+ dev->dynalloc = 1;
+ dev->cdev.class = &input_dev_class;
+ class_device_initialize(&dev->cdev);
+ INIT_LIST_HEAD(&dev->h_list);
+ INIT_LIST_HEAD(&dev->node);
+ }
+
+ return dev;
+}
+
+static void input_register_classdevice(struct input_dev *dev)
+{
+ static atomic_t input_no = ATOMIC_INIT(0);
+ const char *path;
+
+ __module_get(THIS_MODULE);
+
+ dev->dev = dev->cdev.dev;
+
+ snprintf(dev->cdev.class_id, sizeof(dev->cdev.class_id),
+ "input%ld", (unsigned long) atomic_inc_return(&input_no) - 1);
+
+ path = kobject_get_path(&dev->cdev.class->subsys.kset.kobj, GFP_KERNEL);
+ printk(KERN_INFO "input: %s/%s as %s\n",
+ dev->name ? dev->name : "Unspecified device",
+ path ? path : "", dev->cdev.class_id);
+ kfree(path);
+
+ class_device_add(&dev->cdev);
+}
+
void input_register_device(struct input_dev *dev)
{
struct input_handle *handle;
@@ -636,6 +687,10 @@ void input_register_device(struct input_
if ((handle = handler->connect(handler, dev, id)))
input_link_handle(handle);
+
+ if (dev->dynalloc)
+ input_register_classdevice(dev);
+
#ifdef CONFIG_HOTPLUG
input_call_hotplug("add", dev);
#endif
@@ -664,6 +719,9 @@ void input_unregister_device(struct inpu
list_del_init(&dev->node);
+ if (dev->dynalloc)
+ class_device_unregister(&dev->cdev);
+
input_wakeup_procfs_readers();
}
@@ -752,26 +810,34 @@ static int __init input_init(void)
{
int err;
+ err = class_register(&input_dev_class);
+ if (err) {
+ printk(KERN_ERR "input: unable to register input_dev class\n");
+ return err;
+ }
+
input_class = class_create(THIS_MODULE, "input");
if (IS_ERR(input_class)) {
printk(KERN_ERR "input: unable to register input class\n");
- return PTR_ERR(input_class);
+ err = PTR_ERR(input_class);
+ goto fail1;
}
err = input_proc_init();
if (err)
- goto fail1;
+ goto fail2;
err = register_chrdev(INPUT_MAJOR, "input", &input_fops);
if (err) {
printk(KERN_ERR "input: unable to register char major %d", INPUT_MAJOR);
- goto fail2;
+ goto fail3;
}
return 0;
- fail2: input_proc_exit();
- fail1: class_destroy(input_class);
+ fail3: input_proc_exit();
+ fail2: class_destroy(input_class);
+ fail1: class_unregister(&input_dev_class);
return err;
}
@@ -780,6 +846,7 @@ static void __exit input_exit(void)
input_proc_exit();
unregister_chrdev(INPUT_MAJOR, "input");
class_destroy(input_class);
+ class_unregister(&input_dev_class);
}
subsys_initcall(input_init);
On Thu, Sep 15, 2005 at 02:01:39AM -0500, Dmitry Torokhov wrote:
> Input: prepare to sysfs integration
>
> Add struct class_device to input_dev; add input_allocate_dev()
> to dynamically allocate input devices; dynamically allocated
> devices are automatically registered with sysfs.
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
Ok, I've applied this one, and the other "convert the input drivers to
be dynamic" to my tree, as this is all great work.
I'll work on the last few patches that you have, with regard to how to
tie it into sysfs "properly" now, and get back to you, just wanted to
apply all of these, so we have a common base to work on.
Oh, I did change one thing in this patch:
>
> +EXPORT_SYMBOL(input_allocate_device);
I made that EXPORT_SYMBOL_GPL(). Let me know if you object to this.
thanks,
greg k-h
On 10/5/05, Greg KH <[email protected]> wrote:
> On Thu, Sep 15, 2005 at 02:01:39AM -0500, Dmitry Torokhov wrote:
> > Input: prepare to sysfs integration
> >
> > Add struct class_device to input_dev; add input_allocate_dev()
> > to dynamically allocate input devices; dynamically allocated
> > devices are automatically registered with sysfs.
> >
> > Signed-off-by: Dmitry Torokhov <[email protected]>
>
> Ok, I've applied this one, and the other "convert the input drivers to
> be dynamic" to my tree, as this is all great work.
>
> I'll work on the last few patches that you have, with regard to how to
> tie it into sysfs "properly" now, and get back to you, just wanted to
> apply all of these, so we have a common base to work on.
>
Greg,
Could you please drop these patches for a while? Or maybe just don't
push them to Linus yet. The reason is that I want to change
input_allocate_device to take bitmap of supported events. This way I
could allocate ABS tables dynamically at the same time I allocate
input_dev itself and it will simplify error handling logic in drivers
and it will save I think 1260 bytes per input_dev structure which is
nice. And I don't want to go through all subsystems yet again soI want
to fold into my input dynalloc patch...
> Oh, I did change one thing in this patch:
>
> >
> > +EXPORT_SYMBOL(input_allocate_device);
>
> I made that EXPORT_SYMBOL_GPL(). Let me know if you object to this.
>
That's fine with me.
--
Dmitry
On Wed, Oct 05, 2005 at 05:17:00PM -0500, Dmitry Torokhov wrote:
> On 10/5/05, Greg KH <[email protected]> wrote:
> > On Thu, Sep 15, 2005 at 02:01:39AM -0500, Dmitry Torokhov wrote:
> > > Input: prepare to sysfs integration
> > >
> > > Add struct class_device to input_dev; add input_allocate_dev()
> > > to dynamically allocate input devices; dynamically allocated
> > > devices are automatically registered with sysfs.
> > >
> > > Signed-off-by: Dmitry Torokhov <[email protected]>
> >
> > Ok, I've applied this one, and the other "convert the input drivers to
> > be dynamic" to my tree, as this is all great work.
> >
> > I'll work on the last few patches that you have, with regard to how to
> > tie it into sysfs "properly" now, and get back to you, just wanted to
> > apply all of these, so we have a common base to work on.
> >
>
> Greg,
>
> Could you please drop these patches for a while? Or maybe just don't
> push them to Linus yet.
How about I hold on to them, until you send me replacements for them?
I'm using quilt so I can just drop them and add your new ones very
easily. I will not push them to Linus until you and Vojtech say it's ok
for me to do so.
> The reason is that I want to change input_allocate_device to take
> bitmap of supported events. This way I could allocate ABS tables
> dynamically at the same time I allocate input_dev itself and it will
> simplify error handling logic in drivers and it will save I think 1260
> bytes per input_dev structure which is nice. And I don't want to go
> through all subsystems yet again soI want to fold into my input
> dynalloc patch...
That sounds good.
thanks,
greg k-h
On 10/5/05, Greg KH <[email protected]> wrote:
> On Wed, Oct 05, 2005 at 05:17:00PM -0500, Dmitry Torokhov wrote:
>
> > The reason is that I want to change input_allocate_device to take
> > bitmap of supported events. This way I could allocate ABS tables
> > dynamically at the same time I allocate input_dev itself and it will
> > simplify error handling logic in drivers and it will save I think 1260
> > bytes per input_dev structure which is nice. And I don't want to go
> > through all subsystems yet again soI want to fold into my input
> > dynalloc patch...
>
> That sounds good.
>
Well, I tried implementing the proposal above and interface came out
pretty awkward to use. My next option is to move abs table into
"->private" structure, much like keytable was moved, or (for HID-like
devices) allocate it when actually needed and adjust individual
drivers. So I guess the patches that you have right now are good after
all.
--
Dmitry
On Thu, Oct 06, 2005 at 12:46:59PM -0500, Dmitry Torokhov wrote:
> On 10/5/05, Greg KH <[email protected]> wrote:
> > On Wed, Oct 05, 2005 at 05:17:00PM -0500, Dmitry Torokhov wrote:
> >
> > > The reason is that I want to change input_allocate_device to take
> > > bitmap of supported events. This way I could allocate ABS tables
> > > dynamically at the same time I allocate input_dev itself and it will
> > > simplify error handling logic in drivers and it will save I think 1260
> > > bytes per input_dev structure which is nice. And I don't want to go
> > > through all subsystems yet again soI want to fold into my input
> > > dynalloc patch...
> >
> > That sounds good.
> >
>
> Well, I tried implementing the proposal above and interface came out
> pretty awkward to use. My next option is to move abs table into
> "->private" structure, much like keytable was moved, or (for HID-like
> devices) allocate it when actually needed and adjust individual
> drivers. So I guess the patches that you have right now are good after
> all.
The problem is that the ->abs tables are accessed in the input core and
in the handlers, too, which means they have to share the lifetime rules
with the input_dev struct itself.
That means we probably have a problem with the drivers deallocating the
keytable, while the device still exists, because there is a reference to
it from say sysfs, and keyboard.c tries to access the keytable because
of an ioctl.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
On Thursday 06 October 2005 18:05, Vojtech Pavlik wrote:
> On Thu, Oct 06, 2005 at 12:46:59PM -0500, Dmitry Torokhov wrote:
> > On 10/5/05, Greg KH <[email protected]> wrote:
> > > On Wed, Oct 05, 2005 at 05:17:00PM -0500, Dmitry Torokhov wrote:
> > >
> > > > The reason is that I want to change input_allocate_device to take
> > > > bitmap of supported events. This way I could allocate ABS tables
> > > > dynamically at the same time I allocate input_dev itself and it will
> > > > simplify error handling logic in drivers and it will save I think 1260
> > > > bytes per input_dev structure which is nice. And I don't want to go
> > > > through all subsystems yet again soI want to fold into my input
> > > > dynalloc patch...
> > >
> > > That sounds good.
> > >
> >
> > Well, I tried implementing the proposal above and interface came out
> > pretty awkward to use. My next option is to move abs table into
> > "->private" structure, much like keytable was moved, or (for HID-like
> > devices) allocate it when actually needed and adjust individual
> > drivers. So I guess the patches that you have right now are good after
> > all.
>
> The problem is that the ->abs tables are accessed in the input core and
> in the handlers, too, which means they have to share the lifetime rules
> with the input_dev struct itself.
>
> That means we probably have a problem with the drivers deallocating the
> keytable, while the device still exists, because there is a reference to
> it from say sysfs, and keyboard.c tries to access the keytable because
> of an ioctl.
>
Not necessarily, you just need to make sure that you don't try to access
these fields when input_dev is "half-dead". But we have many issues with
locking/lifetime rules in input core so that's just another item that
needs to be considered.
I wanted to get basic sysfs support in and then work on locking...
--
Dmitry
On Thu, Oct 06, 2005 at 10:58:07PM -0500, Dmitry Torokhov wrote:
> Not necessarily, you just need to make sure that you don't try to access
> these fields when input_dev is "half-dead". But we have many issues with
> locking/lifetime rules in input core so that's just another item that
> needs to be considered.
>
> I wanted to get basic sysfs support in and then work on locking...
... when the first one to fix should be the lifetime rules. Both sysfs
stuff and locking depend on those...
On Friday 07 October 2005 01:41, Al Viro wrote:
> ... when the first one to fix should be the lifetime rules. ?Both sysfs
> stuff and locking depend on those...
FWIW for me it is easier to think about lifetime rules when I already have
sysfs support. I mean sysfs itself dictates basic lifetime rules, additional
constraints are just that - additional constraints.
--
Dmitry