* [PATCH 0/2] Remove input_call_hotplug @ 2005-01-18 14:56 Hannes Reinecke 2005-01-18 21:30 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: Hannes Reinecke @ 2005-01-18 14:56 UTC (permalink / raw) To: Linux Kernel; +Cc: Vojtech Pawlik Hi all, the input subsystem is using call_usermodehelper directly, which breaks all sorts of assertions especially when using udev. And it's definitely going to fail once someone is trying to use netlink messages for hotplug event delivery. To remedy this I've implemented a new sysfs class 'input_device' which is a representation of 'struct input_dev'. So each device listed in '/proc/bus/input/devices' gets a class device associated with it. And we'll get proper hotplug events for each input_device which can be handled by udev accordingly. Drawback is that a new event type (the said 'input_device') is added, so that hotplug scripts and udev might need to be adapted to handle it properly. And each device driver needs to be touched to write something meaningful as the class_id. A fallback is provided, but by neccessity is not very informative. Patch 1/2 implements the core changes to drivers/input/input.c Patch 2/2 provides proper device names for input drivers. Patches are relative to bk://kernel.bkbits.net/vojtech/input Comments are welcome. Please CC me directly as I'm not on this list. Cheers, Hannes -- Dr. Hannes Reinecke hare@suse.de SuSE Linux AG S390 & zSeries Maxfeldstraße 5 +49 911 74053 688 90409 Nürnberg http://www.suse.de ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Remove input_call_hotplug 2005-01-18 14:56 [PATCH 0/2] Remove input_call_hotplug Hannes Reinecke @ 2005-01-18 21:30 ` Greg KH 2005-01-18 21:49 ` Dmitry Torokhov 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2005-01-18 21:30 UTC (permalink / raw) To: Hannes Reinecke; +Cc: Linux Kernel, Vojtech Pawlik On Tue, Jan 18, 2005 at 03:56:35PM +0100, Hannes Reinecke wrote: > Hi all, > > the input subsystem is using call_usermodehelper directly, which breaks > all sorts of assertions especially when using udev. > And it's definitely going to fail once someone is trying to use netlink > messages for hotplug event delivery. > > To remedy this I've implemented a new sysfs class 'input_device' which > is a representation of 'struct input_dev'. So each device listed in > '/proc/bus/input/devices' gets a class device associated with it. > And we'll get proper hotplug events for each input_device which can be > handled by udev accordingly. Hm, why another input class? We already have /sys/class/input, which we get hotplug events for. We also have the individual input device hotplug events, which is what I think we really want here, right? Hm, in looking at the code some more, I think just adding a struct device to the struct input_device would work. You will have to refactor out the fact that there is a pointer to a struct device in there (that will just become the parent pointer to the input_dev's struct device). If you do that, you can also drop a few of the fields in struct input_dev. That would solve all of the userspace issues your patch causes (hotplug events would come out with the same type as before) and finally the input subsystem would be part of the driver model properly :) But yes, it will take more work than your patch, sorry. thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Remove input_call_hotplug 2005-01-18 21:30 ` Greg KH @ 2005-01-18 21:49 ` Dmitry Torokhov 2005-01-18 21:58 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: Dmitry Torokhov @ 2005-01-18 21:49 UTC (permalink / raw) To: Greg KH; +Cc: Hannes Reinecke, Linux Kernel, Vojtech Pawlik On Tue, 18 Jan 2005 13:30:02 -0800, Greg KH <greg@kroah.com> wrote: > On Tue, Jan 18, 2005 at 03:56:35PM +0100, Hannes Reinecke wrote: > > Hi all, > > > > the input subsystem is using call_usermodehelper directly, which breaks > > all sorts of assertions especially when using udev. > > And it's definitely going to fail once someone is trying to use netlink > > messages for hotplug event delivery. > > > > To remedy this I've implemented a new sysfs class 'input_device' which > > is a representation of 'struct input_dev'. So each device listed in > > '/proc/bus/input/devices' gets a class device associated with it. > > And we'll get proper hotplug events for each input_device which can be > > handled by udev accordingly. > > Hm, why another input class? We already have /sys/class/input, which we > get hotplug events for. We also have the individual input device > hotplug events, which is what I think we really want here, right? These are a bit different classes. One is a generic input device class device. Then you have several class device interfaces (evdev, mousedev, joydev, tsdev, keyboard) that together with generic input device produce concrete input devices (mouse, js, ts) that you have implemented with class_simple. At least that's the picture I had in my mind at some point. -- Dmitry ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Remove input_call_hotplug 2005-01-18 21:49 ` Dmitry Torokhov @ 2005-01-18 21:58 ` Greg KH 2005-01-18 22:20 ` Dmitry Torokhov 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2005-01-18 21:58 UTC (permalink / raw) To: dtor_core; +Cc: Hannes Reinecke, Linux Kernel, Vojtech Pawlik On Tue, Jan 18, 2005 at 04:49:34PM -0500, Dmitry Torokhov wrote: > On Tue, 18 Jan 2005 13:30:02 -0800, Greg KH <greg@kroah.com> wrote: > > On Tue, Jan 18, 2005 at 03:56:35PM +0100, Hannes Reinecke wrote: > > > Hi all, > > > > > > the input subsystem is using call_usermodehelper directly, which breaks > > > all sorts of assertions especially when using udev. > > > And it's definitely going to fail once someone is trying to use netlink > > > messages for hotplug event delivery. > > > > > > To remedy this I've implemented a new sysfs class 'input_device' which > > > is a representation of 'struct input_dev'. So each device listed in > > > '/proc/bus/input/devices' gets a class device associated with it. > > > And we'll get proper hotplug events for each input_device which can be > > > handled by udev accordingly. > > > > Hm, why another input class? We already have /sys/class/input, which we > > get hotplug events for. We also have the individual input device > > hotplug events, which is what I think we really want here, right? > > These are a bit different classes. One is a generic input device class > device. Then you have several class device interfaces (evdev, > mousedev, joydev, tsdev, keyboard) that together with generic input > device produce concrete input devices (mouse, js, ts) that you have > implemented with class_simple. Hm, but we still need to make the input_dev a "real" struct device, right? And if you do that, then you just hooked up your hotplug event properly, with no userspace breakage. Then, if you want to still make the evdev, mousedev, and so on as class_device interfaces, that's fine, but the main point of this patch was to allow the call_usermodehelper call to be removed, so that the input subsytem will work properly with the kernel event and hotplug systems. thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Remove input_call_hotplug 2005-01-18 21:58 ` Greg KH @ 2005-01-18 22:20 ` Dmitry Torokhov 2005-01-19 1:31 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: Dmitry Torokhov @ 2005-01-18 22:20 UTC (permalink / raw) To: Greg KH; +Cc: Hannes Reinecke, Linux Kernel, Vojtech Pawlik On Tue, 18 Jan 2005 13:58:20 -0800, Greg KH <greg@kroah.com> wrote: > On Tue, Jan 18, 2005 at 04:49:34PM -0500, Dmitry Torokhov wrote: > > On Tue, 18 Jan 2005 13:30:02 -0800, Greg KH <greg@kroah.com> wrote: > > > On Tue, Jan 18, 2005 at 03:56:35PM +0100, Hannes Reinecke wrote: > > > > Hi all, > > > > > > > > the input subsystem is using call_usermodehelper directly, which breaks > > > > all sorts of assertions especially when using udev. > > > > And it's definitely going to fail once someone is trying to use netlink > > > > messages for hotplug event delivery. > > > > > > > > To remedy this I've implemented a new sysfs class 'input_device' which > > > > is a representation of 'struct input_dev'. So each device listed in > > > > '/proc/bus/input/devices' gets a class device associated with it. > > > > And we'll get proper hotplug events for each input_device which can be > > > > handled by udev accordingly. > > > > > > Hm, why another input class? We already have /sys/class/input, which we > > > get hotplug events for. We also have the individual input device > > > hotplug events, which is what I think we really want here, right? > > > > These are a bit different classes. One is a generic input device class > > device. Then you have several class device interfaces (evdev, > > mousedev, joydev, tsdev, keyboard) that together with generic input > > device produce concrete input devices (mouse, js, ts) that you have > > implemented with class_simple. > > Hm, but we still need to make the input_dev a "real" struct device, > right? And if you do that, then you just hooked up your hotplug event > properly, with no userspace breakage. I wasn't planning on doing that. The real devices are serio ports, gameport ports and USB devices.They require power and resource management and so forth. input_device is just a product of binding a port to appropriate driver and seems to me like an ideal class_device candidate. Then you add couple of class interfaces and get another class_device layer as a result. > Then, if you want to still make the evdev, mousedev, and so on as > class_device interfaces, that's fine, but the main point of this patch > was to allow the call_usermodehelper call to be removed, so that the > input subsytem will work properly with the kernel event and hotplug > systems. > I was mostly talking about the need of 2 separate classes and this patch lays groundwork for it althou lifetime rules in input system need to be cleaned up before we can go all the way. -- Dmitry ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Remove input_call_hotplug 2005-01-18 22:20 ` Dmitry Torokhov @ 2005-01-19 1:31 ` Greg KH 2005-01-19 11:56 ` Hannes Reinecke 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2005-01-19 1:31 UTC (permalink / raw) To: dtor_core; +Cc: Hannes Reinecke, Linux Kernel, Vojtech Pawlik On Tue, Jan 18, 2005 at 05:20:40PM -0500, Dmitry Torokhov wrote: > On Tue, 18 Jan 2005 13:58:20 -0800, Greg KH <greg@kroah.com> wrote: > > On Tue, Jan 18, 2005 at 04:49:34PM -0500, Dmitry Torokhov wrote: > > > On Tue, 18 Jan 2005 13:30:02 -0800, Greg KH <greg@kroah.com> wrote: > > > > On Tue, Jan 18, 2005 at 03:56:35PM +0100, Hannes Reinecke wrote: > > > > > Hi all, > > > > > > > > > > the input subsystem is using call_usermodehelper directly, which breaks > > > > > all sorts of assertions especially when using udev. > > > > > And it's definitely going to fail once someone is trying to use netlink > > > > > messages for hotplug event delivery. > > > > > > > > > > To remedy this I've implemented a new sysfs class 'input_device' which > > > > > is a representation of 'struct input_dev'. So each device listed in > > > > > '/proc/bus/input/devices' gets a class device associated with it. > > > > > And we'll get proper hotplug events for each input_device which can be > > > > > handled by udev accordingly. > > > > > > > > Hm, why another input class? We already have /sys/class/input, which we > > > > get hotplug events for. We also have the individual input device > > > > hotplug events, which is what I think we really want here, right? > > > > > > These are a bit different classes. One is a generic input device class > > > device. Then you have several class device interfaces (evdev, > > > mousedev, joydev, tsdev, keyboard) that together with generic input > > > device produce concrete input devices (mouse, js, ts) that you have > > > implemented with class_simple. > > > > Hm, but we still need to make the input_dev a "real" struct device, > > right? And if you do that, then you just hooked up your hotplug event > > properly, with no userspace breakage. > > I wasn't planning on doing that. The real devices are serio ports, > gameport ports and USB devices.They require power and resource > management and so forth. input_device is just a product of binding a > port to appropriate driver and seems to me like an ideal class_device > candidate. Then you add couple of class interfaces and get another > class_device layer as a result. Ah, ok, that makes sense. That would work too, although I don't know if udev can handle class_interfaces with a "dev" file in it or not. If not, it shouldn't be that hard to change. > > Then, if you want to still make the evdev, mousedev, and so on as > > class_device interfaces, that's fine, but the main point of this patch > > was to allow the call_usermodehelper call to be removed, so that the > > input subsytem will work properly with the kernel event and hotplug > > systems. > > > > I was mostly talking about the need of 2 separate classes and this > patch lays groundwork for it althou lifetime rules in input system > need to be cleaned up before we can go all the way. I agree. But I think only 1 class is needed, that way we don't break userspace, which is a pretty important thing. thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Remove input_call_hotplug 2005-01-19 1:31 ` Greg KH @ 2005-01-19 11:56 ` Hannes Reinecke 2005-01-19 14:30 ` Dmitry Torokhov 0 siblings, 1 reply; 9+ messages in thread From: Hannes Reinecke @ 2005-01-19 11:56 UTC (permalink / raw) To: Greg KH; +Cc: dtor_core, Linux Kernel, Vojtech Pawlik Greg KH wrote: > On Tue, Jan 18, 2005 at 05:20:40PM -0500, Dmitry Torokhov wrote: > >>On Tue, 18 Jan 2005 13:58:20 -0800, Greg KH <greg@kroah.com> wrote: >> >>>On Tue, Jan 18, 2005 at 04:49:34PM -0500, Dmitry Torokhov wrote: >>> >>>>On Tue, 18 Jan 2005 13:30:02 -0800, Greg KH <greg@kroah.com> wrote: >>>> >>>>>On Tue, Jan 18, 2005 at 03:56:35PM +0100, Hannes Reinecke wrote: >>>>> >>>>>>Hi all, >>>>>> >>>>>>the input subsystem is using call_usermodehelper directly, which breaks >>>>>>all sorts of assertions especially when using udev. >>>>>>And it's definitely going to fail once someone is trying to use netlink >>>>>>messages for hotplug event delivery. >>>>>> >>>>>>To remedy this I've implemented a new sysfs class 'input_device' which >>>>>>is a representation of 'struct input_dev'. So each device listed in >>>>>>'/proc/bus/input/devices' gets a class device associated with it. >>>>>>And we'll get proper hotplug events for each input_device which can be >>>>>>handled by udev accordingly. >>>>> >>>>>Hm, why another input class? We already have /sys/class/input, which we >>>>>get hotplug events for. We also have the individual input device >>>>>hotplug events, which is what I think we really want here, right? >>>> >>>>These are a bit different classes. One is a generic input device class >>>>device. Then you have several class device interfaces (evdev, >>>>mousedev, joydev, tsdev, keyboard) that together with generic input >>>>device produce concrete input devices (mouse, js, ts) that you have >>>>implemented with class_simple. >>> >>>Hm, but we still need to make the input_dev a "real" struct device, >>>right? And if you do that, then you just hooked up your hotplug event >>>properly, with no userspace breakage. >> >>I wasn't planning on doing that. The real devices are serio ports, >>gameport ports and USB devices.They require power and resource >>management and so forth. input_device is just a product of binding a >>port to appropriate driver and seems to me like an ideal class_device >>candidate. Then you add couple of class interfaces and get another >>class_device layer as a result. > > > Ah, ok, that makes sense. That would work too, although I don't know if > udev can handle class_interfaces with a "dev" file in it or not. If > not, it shouldn't be that hard to change. > > >>>Then, if you want to still make the evdev, mousedev, and so on as >>>class_device interfaces, that's fine, but the main point of this patch >>>was to allow the call_usermodehelper call to be removed, so that the >>>input subsytem will work properly with the kernel event and hotplug >>>systems. >>> >> >>I was mostly talking about the need of 2 separate classes and this >>patch lays groundwork for it althou lifetime rules in input system >>need to be cleaned up before we can go all the way. > > > I agree. But I think only 1 class is needed, that way we don't break > userspace, which is a pretty important thing. > Well, if you could show me how to do this with the class_interface thing I'd be happy to comply. The input layer design is like this: - Physical devices present one (or several) abstract input devices (which correspond to struct input_dev) - Each input device can be linked to one or several input handlers (which correspond to struct input_handle) - Each handler is represented to userspace with a device node. The problem with the current input layer is that each 'struct input_handle' is associated with a class_simple device. This class is named 'input', so we're getting 'input' events from it. But each instantiation of struct input_dev is also sending 'input' events as it is doing the call_usermodehelper call directly. Effectively we're having _two_ different hotplug events with the same name, which is inconsistent in itself. I don't mind at all if we're breaking this. But if we were going to implement this with device_interface, we'd be having a /sys/class structure like: class |- isa0060-serio0-input0 | |- event0 | | `dev | |- key | |- ... |- .. So we'd be moving the 'dev' attribute one directory down, again incurring a userland breakage. Plus it would be far more coding involved as the entire input layer structure would have to be redone. But if the powers that be decide for the latter option, who am I to argue ... Only I'd like to have this resolved quite soon as it effectively blocks from switching to netlink messages for hotplug events completely. Cheers, Hannes -- Dr. Hannes Reinecke hare@suse.de SuSE Linux AG S390 & zSeries Maxfeldstraße 5 +49 911 74053 688 90409 Nürnberg http://www.suse.de ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Remove input_call_hotplug 2005-01-19 11:56 ` Hannes Reinecke @ 2005-01-19 14:30 ` Dmitry Torokhov 2005-01-19 21:39 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: Dmitry Torokhov @ 2005-01-19 14:30 UTC (permalink / raw) To: Hannes Reinecke; +Cc: Greg KH, Linux Kernel, Vojtech Pawlik On Wed, 19 Jan 2005 12:56:16 +0100, Hannes Reinecke <hare@suse.de> wrote: > Greg KH wrote: > > On Tue, Jan 18, 2005 at 05:20:40PM -0500, Dmitry Torokhov wrote: > > > >>I was mostly talking about the need of 2 separate classes and this > >>patch lays groundwork for it althou lifetime rules in input system > >>need to be cleaned up before we can go all the way. > > > > > > I agree. But I think only 1 class is needed, that way we don't break > > userspace, which is a pretty important thing. > > > Well, if you could show me how to do this with the class_interface thing > I'd be happy to comply. > > The input layer design is like this: > - Physical devices present one (or several) abstract input devices > (which correspond to struct input_dev) > - Each input device can be linked to one or several input handlers > (which correspond to struct input_handle) > - Each handler is represented to userspace with a device node. > > The problem with the current input layer is that each 'struct > input_handle' is associated with a class_simple device. > This class is named 'input', so we're getting 'input' events from it. > But each instantiation of struct input_dev is also sending 'input' > events as it is doing the call_usermodehelper call directly. > Yes, this is the problem and needs to be resolved. Thankfully most people have keyboard support compiled in so it's not fatal but we probably waht hotplug package be updated first before we commit this change. > > But if we were going to implement this with device_interface, we'd be > having a /sys/class structure like: > > class > |- isa0060-serio0-input0 > | |- event0 > | | `dev > | |- key > | |- ... > |- .. > > So we'd be moving the 'dev' attribute one directory down, again > incurring a userland breakage. > Plus it would be far more coding involved as the entire input layer > structure would have to be redone. > If I understand correctly we do not have subclasses so it will look like class |- input_device | |- input0 | |- input1 | |- input | |-event0 | |-event1 | |-mouse0 So breakage is really minimal. -- Dmitry ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Remove input_call_hotplug 2005-01-19 14:30 ` Dmitry Torokhov @ 2005-01-19 21:39 ` Greg KH 0 siblings, 0 replies; 9+ messages in thread From: Greg KH @ 2005-01-19 21:39 UTC (permalink / raw) To: dtor_core; +Cc: Hannes Reinecke, Linux Kernel, Vojtech Pawlik On Wed, Jan 19, 2005 at 09:30:14AM -0500, Dmitry Torokhov wrote: > If I understand correctly we do not have subclasses so it will look like > class > |- input_device > | |- input0 > | |- input1 > | > |- input > | |-event0 > | |-event1 > | |-mouse0 > > So breakage is really minimal. I really want classes to be able to have "parent" classes someday. It's not that tough of a change to the driver core, if someone wants to do the work. That would enable this input stuff to be cleaner, right? It would also allow us to move the /sys/block stuff to use classes, which it can't right now due to the lack of the parent ability. thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2005-01-19 22:18 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2005-01-18 14:56 [PATCH 0/2] Remove input_call_hotplug Hannes Reinecke 2005-01-18 21:30 ` Greg KH 2005-01-18 21:49 ` Dmitry Torokhov 2005-01-18 21:58 ` Greg KH 2005-01-18 22:20 ` Dmitry Torokhov 2005-01-19 1:31 ` Greg KH 2005-01-19 11:56 ` Hannes Reinecke 2005-01-19 14:30 ` Dmitry Torokhov 2005-01-19 21:39 ` Greg KH
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).