Re: [PATCH 2/2] Remove input_call_hotplug
On Wed, Jan 19, 2005 at 04:50:39PM +0100, Hannes Reinecke wrote: > Vojtech Pavlik wrote: > >On Wed, Jan 19, 2005 at 03:56:33PM +0100, Hannes Reinecke wrote: > > > > > >>oops. hadn't thought of that. But of course, you are correct. > >>So better be using monotonically increasing numbers. > >> > >>New patch attached. > > > > > >This one looks quite OK to me. > > > Will you put it into your bk-input tree so that it will eventually find > its way towards akpm/linus ? Unless anyone objects in a while, yes. -- Vojtech Pavlik SuSE Labs, SuSE CR - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] Remove input_call_hotplug
Vojtech Pavlik wrote: On Wed, Jan 19, 2005 at 03:56:33PM +0100, Hannes Reinecke wrote: oops. hadn't thought of that. But of course, you are correct. So better be using monotonically increasing numbers. New patch attached. This one looks quite OK to me. Will you put it into your bk-input tree so that it will eventually find its way towards akpm/linus ? Cheers, Hannes -- Dr. Hannes Reinecke [EMAIL PROTECTED] SuSE Linux AG S390 & zSeries Maxfeldstraße 5 +49 911 74053 688 90409 Nürnberg http://www.suse.de - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] Remove input_call_hotplug
On Wed, Jan 19, 2005 at 03:56:33PM +0100, Hannes Reinecke wrote: > oops. hadn't thought of that. But of course, you are correct. > So better be using monotonically increasing numbers. > > New patch attached. This one looks quite OK to me. -- Vojtech Pavlik SuSE Labs, SuSE CR - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] Remove input_call_hotplug
Dmitry Torokhov wrote: On Wed, 19 Jan 2005 15:16:08 +0100, Hannes Reinecke <[EMAIL PROTECTED]> wrote: [ ... ] I'm not too happy about this 'inputX' thing (as this doesn't carry any information, whereas 'phys' gives you at least a rough guess what this device's about), but if phys is to go it would be the logical choice. The problem with encoding phys in class ID is the following: you have to guarantee that the moment you destroy underlying hw device yur input_device has to be gone too. Imagine you have input_device for your PS/2 mouse and you decide to unload psmouse module. You also have one of user processes holding any of the class device attributes open. This causes input_device to be pinned into memory so when you load psmouse module back again it will not be able to create new input_device and mouse will be dead. With monotonicaly increasing inputX name you will never have this issue. oops. hadn't thought of that. But of course, you are correct. So better be using monotonically increasing numbers. New patch attached. Cheers, Hannes -- Dr. Hannes Reinecke [EMAIL PROTECTED] SuSE Linux AG S390 & zSeries Maxfeldstraße 5 +49 911 74053 688 90409 Nürnberg http://www.suse.de # This is a BitKeeper generated diff -Nru style patch. # # ChangeSet # 2005/01/19 15:54:26+01:00 [EMAIL PROTECTED] # As discussed with Dmitry we should be using a monotonically increasing # numbering scheme for 'input_device' devices, as persistent names would # break locking rules. # # drivers/input/input.c # 2005/01/19 15:54:20+01:00 [EMAIL PROTECTED] +3 -7 # Use inputX as device names instead of the 'phys' attribute. # # ChangeSet # 2005/01/19 14:34:48+01:00 [EMAIL PROTECTED] # Cset include: [EMAIL PROTECTED]|ChangeSet|20050119133405|27423 # Cset include: [EMAIL PROTECTED]|ChangeSet|20050119132157|27436 # # include/linux/input.h # 2005/01/19 14:34:45+01:00 [EMAIL PROTECTED] +0 -0 # Include # # drivers/input/input.c # 2005/01/19 14:34:45+01:00 [EMAIL PROTECTED] +0 -0 # Include # # ChangeSet # 2005/01/19 14:34:05+01:00 [EMAIL PROTECTED] # Remove left-over variables. # # include/linux/input.h # 2005/01/19 14:27:07+01:00 [EMAIL PROTECTED] +0 -2 # Remove left-over variables. # # ChangeSet # 2005/01/19 14:21:57+01:00 [EMAIL PROTECTED] # Implement sysfs class_device 'input_device' as abstraction of # struct input_dev. This allows us to get rid of the explicit call # to call_usermodehelper in drivers/input/input.c and makes the input # layer driver-model compliant. # # drivers/input/input.c # 2005/01/19 14:21:51+01:00 [EMAIL PROTECTED] +6 -6 # Use ->phys as class_id for the device. # # include/linux/input.h # 2005/01/18 15:49:00+01:00 [EMAIL PROTECTED] +4 -0 # Added class_device to the input_dev struct. # # drivers/input/input.c # 2005/01/18 15:47:37+01:00 [EMAIL PROTECTED] +147 -54 # Implement 'input_device' class as abstraction of # struct input_dev. Default class IDs are being generated # if the driver does not provide one. # diff -Nru a/drivers/input/input.c b/drivers/input/input.c --- a/drivers/input/input.c 2005-01-19 15:55:14 +01:00 +++ b/drivers/input/input.c 2005-01-19 15:55:14 +01:00 @@ -47,6 +47,7 @@ static LIST_HEAD(input_handler_list); static struct input_handler *input_table[8]; +static atomic_t input_device_num = ATOMIC_INIT(0); #ifdef CONFIG_PROC_FS static struct proc_dir_entry *proc_bus_input_dir; @@ -332,52 +333,27 @@ SPRINTF_BIT_A(bit, name, max); \ } while (0) -static void input_call_hotplug(char *verb, struct input_dev *dev) +static int __input_hotplug(struct input_dev *dev, char **envp, int num_envp, + char *buffer, int buffer_size) { - char *argv[3], **envp, *buf, *scratch; - int i = 0, j, value; + char *scratch; + int i = 0, j; + scratch = buffer; - if (!hotplug_path[0]) { - printk(KERN_ERR "input.c: calling hotplug without a hotplug agent defined\n"); - return; - } - if (in_interrupt()) { - printk(KERN_ERR "input.c: calling hotplug from interrupt\n"); - return; - } - if (!current->fs->root) { - printk(KERN_WARNING "input.c: calling hotplug without valid filesystem\n"); - return; - } - if (!(envp = (char **) kmalloc(20 * sizeof(char *), GFP_KERNEL))) { - printk(KERN_ERR "input.c: not enough memory allocating hotplug environment\n"); - return; - } - if (!(buf = kmalloc(1024, GFP_KERNEL))) { - kfree (envp); - printk(KERN_ERR "input.c: not enough memory allocating hotplug environment\n"); - return; - } - - argv[0] = hotplug_path; - argv[1] = "input"; - argv[2] = NULL; - - envp[i++] = "HOME=/"; - envp[i++] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin"; - - scratch = buf; - - envp[i++] = scratch; - scratch += sprintf(scratch, "ACTION=%s", verb) + 1; + if (!dev) + return -ENODEV; envp[i++] = scratch; scratch += sprintf(scratch, "PRODUCT=%x/%x/%x/%x", dev->id.bustype, dev->id.vendor, dev->id.product, dev->id.version) +
Re: [PATCH 2/2] Remove input_call_hotplug
On Wed, 19 Jan 2005 15:16:08 +0100, Hannes Reinecke <[EMAIL PROTECTED]> wrote: > Dmitry Torokhov wrote: > > Hi Hannes, > > > > On Wed, 19 Jan 2005 10:59:30 +0100, Hannes Reinecke <[EMAIL PROTECTED]> > > wrote: > > > >>Dmitry Torokhov wrote: > >> > >>>But the real question is whether we really need class devices have > >>>unique names or we could do with inputX thus leaving individual > >>>drivers intact and only modifying the input core. As far as I > >>>understand userspace should be concerned only with device > >>>capabilities, not particular name, besides, it gets PRODUCT string > >>>which has all needed data encoded. > >>> > >> > >>Indeed. What about using 'phys' (with all '/' replaced by '-') as the > >>class_id? This way we'll retain compability with /proc/bus/input/devices > >>and do not have to touch every single driver. > >> > > > > > > I want to kill phys at some point - we have topology information > > already present in sysfs in much better form. Can we have a new > > hotplug variable HWDEV= which is kobject_path(input_dev->dev). If > > input_dev is not set then we can just dump phys in it. And the class > > id will still be inputX. Will this work? > > > Sure. And we don't need a special HWDEV variable, as there is already a > PHYSDEVPATH variable providing exactly this information. Oh right! Even better. > I'm not too happy about this 'inputX' thing (as this doesn't carry any > information, whereas 'phys' gives you at least a rough guess what this > device's about), but if phys is to go it would be the logical choice. > The problem with encoding phys in class ID is the following: you have to guarantee that the moment you destroy underlying hw device yur input_device has to be gone too. Imagine you have input_device for your PS/2 mouse and you decide to unload psmouse module. You also have one of user processes holding any of the class device attributes open. This causes input_device to be pinned into memory so when you load psmouse module back again it will not be able to create new input_device and mouse will be dead. With monotonicaly increasing inputX name you will never have this issue. You had a workaround for this problem with your original path but when you fold it all in core you lost it. -- Dmitry - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] Remove input_call_hotplug
Dmitry Torokhov wrote: Hi Hannes, On Wed, 19 Jan 2005 10:59:30 +0100, Hannes Reinecke <[EMAIL PROTECTED]> wrote: Dmitry Torokhov wrote: But the real question is whether we really need class devices have unique names or we could do with inputX thus leaving individual drivers intact and only modifying the input core. As far as I understand userspace should be concerned only with device capabilities, not particular name, besides, it gets PRODUCT string which has all needed data encoded. Indeed. What about using 'phys' (with all '/' replaced by '-') as the class_id? This way we'll retain compability with /proc/bus/input/devices and do not have to touch every single driver. I want to kill phys at some point - we have topology information already present in sysfs in much better form. Can we have a new hotplug variable HWDEV= which is kobject_path(input_dev->dev). If input_dev is not set then we can just dump phys in it. And the class id will still be inputX. Will this work? Sure. And we don't need a special HWDEV variable, as there is already a PHYSDEVPATH variable providing exactly this information. I'm not too happy about this 'inputX' thing (as this doesn't carry any information, whereas 'phys' gives you at least a rough guess what this device's about), but if phys is to go it would be the logical choice. Btw, I really doubt that topology information is important here as the only thing that one needs to do when new "input_device" appears is to load one or more input handler modules based on device's capability bits. The decision whether a device is "good enough" to create a device node should be done by hotplug handler for the other "input" class. Yes, topology is not an issue when loading modules. Cheers, Hannes -- Dr. Hannes Reinecke [EMAIL PROTECTED] SuSE Linux AG S390 & zSeries Maxfeldstraße 5 +49 911 74053 688 90409 Nürnberg http://www.suse.de - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] Remove input_call_hotplug
Hi Hannes, On Wed, 19 Jan 2005 10:59:30 +0100, Hannes Reinecke <[EMAIL PROTECTED]> wrote: > Dmitry Torokhov wrote: > > But the real question is whether we really need class devices have > > unique names or we could do with inputX thus leaving individual > > drivers intact and only modifying the input core. As far as I > > understand userspace should be concerned only with device > > capabilities, not particular name, besides, it gets PRODUCT string > > which has all needed data encoded. > > > Indeed. What about using 'phys' (with all '/' replaced by '-') as the > class_id? This way we'll retain compability with /proc/bus/input/devices > and do not have to touch every single driver. > I want to kill phys at some point - we have topology information already present in sysfs in much better form. Can we have a new hotplug variable HWDEV= which is kobject_path(input_dev->dev). If input_dev is not set then we can just dump phys in it. And the class id will still be inputX. Will this work? Btw, I really doubt that topology information is important here as the only thing that one needs to do when new "input_device" appears is to load one or more input handler modules based on device's capability bits. The decision whether a device is "good enough" to create a device node should be done by hotplug handler for the other "input" class. -- Dmitry - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] Remove input_call_hotplug
Dmitry Torokhov wrote: Hi, On Tue, 18 Jan 2005 15:59:35 +0100, Hannes Reinecke <[EMAIL PROTECTED]> wrote: Implement proper class names for input drivers. This patch probably should probably use atomic_inc in case we ever have non-serialized probe functions. True. But the real question is whether we really need class devices have unique names or we could do with inputX thus leaving individual drivers intact and only modifying the input core. As far as I understand userspace should be concerned only with device capabilities, not particular name, besides, it gets PRODUCT string which has all needed data encoded. Indeed. What about using 'phys' (with all '/' replaced by '-') as the class_id? This way we'll retain compability with /proc/bus/input/devices and do not have to touch every single driver. Better idea? Cheers, Hannes -- Dr. Hannes Reinecke [EMAIL PROTECTED] SuSE Linux AG S390 & zSeries Maxfeldstraße 5 +49 911 74053 688 90409 Nürnberg http://www.suse.de - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] Remove input_call_hotplug
Hi, On Tue, 18 Jan 2005 15:59:35 +0100, Hannes Reinecke <[EMAIL PROTECTED]> wrote: > Implement proper class names for input drivers. > This patch probably should probably use atomic_inc in case we ever have non-serialized probe functions. But the real question is whether we really need class devices have unique names or we could do with inputX thus leaving individual drivers intact and only modifying the input core. As far as I understand userspace should be concerned only with device capabilities, not particular name, besides, it gets PRODUCT string which has all needed data encoded. What do you think? -- Dmitry - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/