* Platform device id @ 2007-09-07 13:35 Jean Delvare 2007-09-07 14:58 ` Dmitry Torokhov 2007-09-08 13:30 ` Greg KH 0 siblings, 2 replies; 19+ messages in thread From: Jean Delvare @ 2007-09-07 13:35 UTC (permalink / raw) To: Greg KH; +Cc: LKML, David Brownell Hi Greg, all, While platform_device.id is a u32, platform_device_add() handles "-1" as a special id value. This has potential for confusion and bugs. One such bug was reported to me by David Brownell: http://lists.lm-sensors.org/pipermail/i2c/2007-September/001787.html And since then I've found two other drivers affected (uartlite and i2c-pxa). Could we at least make platform_device.id an int so as to clear up the confusion? I doubt that the id will ever be a large number anyway. To go one step further, I am questioning the real value of this naming exception for these "unique" platform devices. On top of the bugs I mentioned above, it has potential for compatibility breakage: adding a second device of the same type will rename the first one from "foo" to "foo.0". It also requires specific checks in many individual platform drivers. All this, as I understand it, for a purely aesthetic reason. I don't think this is worth it. Would there be any objection to simply getting rid of this exception and having all platform devices named "foo.%d"? -- Jean Delvare ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-07 13:35 Platform device id Jean Delvare @ 2007-09-07 14:58 ` Dmitry Torokhov 2007-09-07 16:36 ` Jean Delvare ` (2 more replies) 2007-09-08 13:30 ` Greg KH 1 sibling, 3 replies; 19+ messages in thread From: Dmitry Torokhov @ 2007-09-07 14:58 UTC (permalink / raw) To: Jean Delvare; +Cc: Greg KH, LKML, David Brownell Hi Jean, On 9/7/07, Jean Delvare <khali@linux-fr.org> wrote: > Hi Greg, all, > > While platform_device.id is a u32, platform_device_add() handles "-1" as > a special id value. This has potential for confusion and bugs. One such > bug was reported to me by David Brownell: > > http://lists.lm-sensors.org/pipermail/i2c/2007-September/001787.html > > And since then I've found two other drivers affected (uartlite and > i2c-pxa). > > Could we at least make platform_device.id an int so as to clear up the > confusion? I doubt that the id will ever be a large number anyway. > > To go one step further, I am questioning the real value of this naming > exception for these "unique" platform devices. On top of the bugs I > mentioned above, it has potential for compatibility breakage: adding a > second device of the same type will rename the first one from "foo" to > "foo.0". It also requires specific checks in many individual platform > drivers. All this, as I understand it, for a purely aesthetic reason. I > don't think this is worth it. Would there be any objection to simply > getting rid of this exception and having all platform devices named > "foo.%d"? > If a device has a <name>.<instance> scheme this implies possibility of having several instances of said device in a box. There are a few of platform devices that can only have one instance - for example i8042 keyboard controller (the -1 special handling came from me because i80420 name was very confusing - there wasn't a dot separator in the name back then). Drivers that allow multiple devices should not attempt to use -1 for the very first instance - this should eliminate potential for error and special handling that you are talking about. -- Dmitry ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-07 14:58 ` Dmitry Torokhov @ 2007-09-07 16:36 ` Jean Delvare 2007-09-07 16:41 ` David Brownell 2007-09-07 20:56 ` Henrique de Moraes Holschuh 2 siblings, 0 replies; 19+ messages in thread From: Jean Delvare @ 2007-09-07 16:36 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Greg KH, LKML, David Brownell Hi Dmitry, Thanks for your answer. On Fri, 7 Sep 2007 10:58:31 -0400, Dmitry Torokhov wrote: > On 9/7/07, Jean Delvare <khali@linux-fr.org> wrote: > > While platform_device.id is a u32, platform_device_add() handles "-1" as > > a special id value. This has potential for confusion and bugs. One such > > bug was reported to me by David Brownell: > > > > http://lists.lm-sensors.org/pipermail/i2c/2007-September/001787.html > > > > And since then I've found two other drivers affected (uartlite and > > i2c-pxa). > > > > Could we at least make platform_device.id an int so as to clear up the > > confusion? I doubt that the id will ever be a large number anyway. > > > > To go one step further, I am questioning the real value of this naming > > exception for these "unique" platform devices. On top of the bugs I > > mentioned above, it has potential for compatibility breakage: adding a > > second device of the same type will rename the first one from "foo" to > > "foo.0". It also requires specific checks in many individual platform > > drivers. All this, as I understand it, for a purely aesthetic reason. I > > don't think this is worth it. Would there be any objection to simply > > getting rid of this exception and having all platform devices named > > "foo.%d"? > > If a device has a <name>.<instance> scheme this implies possibility of > having several instances of said device in a box. This "allows" more than "implies". > There are a few of > platform devices that can only have one instance - for example i8042 > keyboard controller (the -1 special handling came from me because > i80420 name was very confusing - there wasn't a dot separator in the > name back then). I agree that in general there is a single i8042 keyboard controller on a system, but what prevents someone to build a system with several of these (e.g. in a multi-console computer)? I agree that "i80420" was a confusing name, but now that a dot was inserted between the name and the id, it wouldn't be a problem to have a device named "i8042.0", would it? > Drivers that allow multiple devices should not > attempt to use -1 for the very first instance - this should eliminate > potential for error and special handling that you are talking about. This isn't that easy. For a given kind of device, some systems might have only one, it might even be strictly impossible to ever have more than one by design, but other systems might not have this limitation and may actually have several instances of said device. As we try to make our drivers as platform-independent as possible, the drivers themselves can't assume that only either scheme is used, they have to support both. -- Jean Delvare ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-07 14:58 ` Dmitry Torokhov 2007-09-07 16:36 ` Jean Delvare @ 2007-09-07 16:41 ` David Brownell 2007-09-07 21:00 ` Henrique de Moraes Holschuh 2007-09-07 20:56 ` Henrique de Moraes Holschuh 2 siblings, 1 reply; 19+ messages in thread From: David Brownell @ 2007-09-07 16:41 UTC (permalink / raw) To: khali, dmitry.torokhov; +Cc: linux-kernel, gregkh > If a device has a <name>.<instance> scheme this implies possibility of > having several instances of said device in a box. There are a few of > platform devices that can only have one instance Like USB peripheral controllers. Only one external "B" type connector is allowed. > - for example i8042 > keyboard controller (the -1 special handling came from me because > i80420 name was very confusing - there wasn't a dot separator in the > name back then). There were other devices with similar issues. > Drivers that allow multiple devices should not > attempt to use -1 for the very first instance - this should eliminate > potential for error and special handling that you are talking about. For that matter, a *driver* should never create its own device node(s) in the first place. Device creation belongs elsewhere, like as part of platform setup or, for busses with integral enumeration support like PCI or USB, bus glue. Linux is moving away from that legacy model. I realize that may be more easily said than done in some cases, like i8042 on non-PNP systems. - Dave ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-07 16:41 ` David Brownell @ 2007-09-07 21:00 ` Henrique de Moraes Holschuh 2007-09-07 21:41 ` David Brownell 0 siblings, 1 reply; 19+ messages in thread From: Henrique de Moraes Holschuh @ 2007-09-07 21:00 UTC (permalink / raw) To: David Brownell; +Cc: khali, dmitry.torokhov, linux-kernel, gregkh On Fri, 07 Sep 2007, David Brownell wrote: > For that matter, a *driver* should never create its own device node(s) > in the first place. Device creation belongs elsewhere, like as part of > platform setup or, for busses with integral enumeration support like > PCI or USB, bus glue. Linux is moving away from that legacy model. This assumes that we have a better bus than "platform" to dump drivers like thinkpad-acpi, hdaps, and a host of other host-specific stuff. > I realize that may be more easily said than done in some cases, > like i8042 on non-PNP systems. Yes, and there is a LOT of non-PNP stuff involved, since platform became the dumping ground for host-specific devices (as opposed to platform-specific devices). -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-07 21:00 ` Henrique de Moraes Holschuh @ 2007-09-07 21:41 ` David Brownell 2007-09-07 22:04 ` Henrique de Moraes Holschuh 0 siblings, 1 reply; 19+ messages in thread From: David Brownell @ 2007-09-07 21:41 UTC (permalink / raw) To: hmh; +Cc: linux-kernel, khali, gregkh, dmitry.torokhov > > For that matter, a *driver* should never create its own device node(s) > > in the first place. Device creation belongs elsewhere, like as part of > > platform setup or, for busses with integral enumeration support like > > PCI or USB, bus glue. Linux is moving away from that legacy model. > > This assumes that we have a better bus than "platform" to dump drivers like > thinkpad-acpi, hdaps, and a host of other host-specific stuff. I don't follow. If it's host-specific, then it's easy enough to have a host-specific routine creating those platform devices. A different host wouldn't call that routine. Embedded Linux platforms do that *ALL* the time. ARM keys on a board ID provided early in boot (e.g. by U-Boot). PowerPC uses a device tree, which ISTR evolved from the OpenBoot as first used on SPARC. Worst comes to worst, the kernel command line can say which board is involved, and thus which setup code to run. (Also, note that "platform", "host", and "board" are ambiguous. In some contexts each is synonymous; in others, not. I avoid using "host" except in the protocol sense. Usually "board" is pretty specific -- this cpu, those peripherals -- although it gets messy when the system is really a board stack, or when the CPU may be socketed or be in a customizable FPGA etc.) > > I realize that may be more easily said than done in some cases, > > like i8042 on non-PNP systems. > > Yes, and there is a LOT of non-PNP stuff involved, since platform became the > dumping ground for host-specific devices (as opposed to platform-specific > devices). See above ... most embedded systems aren't x86, so lack of PNP is less of an issue than plain old legacy system designs -- designed in ways that complicate or prevent probe/discovery schemes, which gets to be a mess (like the one preceding PNP with DOS/x86/ISA). Less clear cases include orphaned drivers, especially ones for hardware that's on its way out or already obsolete. Most folk don't want to touch those, for fear of getting stuck to them. :) - Dave ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-07 21:41 ` David Brownell @ 2007-09-07 22:04 ` Henrique de Moraes Holschuh 2007-09-08 0:18 ` David Brownell 0 siblings, 1 reply; 19+ messages in thread From: Henrique de Moraes Holschuh @ 2007-09-07 22:04 UTC (permalink / raw) To: David Brownell; +Cc: linux-kernel, khali, gregkh, dmitry.torokhov On Fri, 07 Sep 2007, David Brownell wrote: > > > For that matter, a *driver* should never create its own device node(s) > > > in the first place. Device creation belongs elsewhere, like as part of > > > platform setup or, for busses with integral enumeration support like > > > PCI or USB, bus glue. Linux is moving away from that legacy model. > > > > This assumes that we have a better bus than "platform" to dump drivers like > > thinkpad-acpi, hdaps, and a host of other host-specific stuff. > > I don't follow. If it's host-specific, then it's easy enough to > have a host-specific routine creating those platform devices. > A different host wouldn't call that routine. We do that in the module that also provides the device driver. E.g. hdaps or thikpad-acpi will provide both the platform device (and register it), and the driver. > (Also, note that "platform", "host", and "board" are ambiguous. > In some contexts each is synonymous; in others, not. I avoid In this specific case I am talking about, they're not. ThinkPads are the host. The platform for a ThinkPad is either i386 or amd64. But there are many more hosts that are i386 or amd64 than ThinkPads, and the devices in my example are thinkpad-specific. I don't feel like drivers like hdaps, thinkpad-acpi, dock, bay, and many others really belong in the platform bus. But that's what happens right now. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-07 22:04 ` Henrique de Moraes Holschuh @ 2007-09-08 0:18 ` David Brownell 2007-09-08 3:50 ` Henrique de Moraes Holschuh 0 siblings, 1 reply; 19+ messages in thread From: David Brownell @ 2007-09-08 0:18 UTC (permalink / raw) To: hmh; +Cc: linux-kernel, khali, gregkh, dmitry.torokhov > > (Also, note that "platform", "host", and "board" are ambiguous. > > In some contexts each is synonymous; in others, not. I avoid > > In this specific case I am talking about, they're not. That is, in *YOUR* usage context they're not. I had to parse what you wrote a few times before your comments about $SUBJECT started to make sense. I've *never* heard "host" used that way, and rarely hear "platform" used that way either. > The platform for a ThinkPad is either i386 or amd64. Both i386 and x86_64 are clearly an "arch". They even live in an "arch" directory: linux/arch/{i386,x86_64}. When folk talk about a "PC Platform", they're talking about a thing that doesn't quite exist in today's Linux tree. If we ever get to an arch/x86, that could have a plat-pc (or mach-pc) subdirectory. ThinkPads should then be a variant of that. > I don't feel like drivers like hdaps, thinkpad-acpi, dock, bay, > and many others really belong in the platform bus. But that's > what happens right now. As a rule, there needs to be a Good Reason to create a new bus type. A "feel" is a pretty weak reason... - Dave ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-08 0:18 ` David Brownell @ 2007-09-08 3:50 ` Henrique de Moraes Holschuh 2007-09-08 8:55 ` Jean Delvare 0 siblings, 1 reply; 19+ messages in thread From: Henrique de Moraes Holschuh @ 2007-09-08 3:50 UTC (permalink / raw) To: David Brownell; +Cc: linux-kernel, khali, gregkh, dmitry.torokhov On Fri, 07 Sep 2007, David Brownell wrote: > > The platform for a ThinkPad is either i386 or amd64. > > Both i386 and x86_64 are clearly an "arch". They even live in > an "arch" directory: linux/arch/{i386,x86_64}. Well, I stand corrected on the "platform" term, then. > When folk talk about a "PC Platform", they're talking about a > thing that doesn't quite exist in today's Linux tree. If we > ever get to an arch/x86, that could have a plat-pc (or mach-pc) > subdirectory. ThinkPads should then be a variant of that. You'd have so many, it wouldn't be funny. It would also cause some headaches for distros, unless one can have an "all platform" kernel or somesuch. > > I don't feel like drivers like hdaps, thinkpad-acpi, dock, bay, > > and many others really belong in the platform bus. But that's > > what happens right now. > > As a rule, there needs to be a Good Reason to create a new bus > type. A "feel" is a pretty weak reason... The "feel" is there because: 1. Comments about how what we do is wrong for the platform bus (i.e. adding the devices and the driver in the same module). Even the documentation for platform devices make it quite clear we are abusing it. There was one of those comments in this very thread. 2. The fact that a module that has a number of different devices has to register itself a number of times as a driver, if it wants to name the devices something different... -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-08 3:50 ` Henrique de Moraes Holschuh @ 2007-09-08 8:55 ` Jean Delvare 2007-09-10 22:52 ` Henrique de Moraes Holschuh 0 siblings, 1 reply; 19+ messages in thread From: Jean Delvare @ 2007-09-08 8:55 UTC (permalink / raw) To: Henrique de Moraes Holschuh Cc: David Brownell, linux-kernel, gregkh, dmitry.torokhov On Sat, 8 Sep 2007 00:50:22 -0300, Henrique de Moraes Holschuh wrote: > On Fri, 07 Sep 2007, David Brownell wrote: > > > I don't feel like drivers like hdaps, thinkpad-acpi, dock, bay, > > > and many others really belong in the platform bus. But that's > > > what happens right now. > > > > As a rule, there needs to be a Good Reason to create a new bus > > type. A "feel" is a pretty weak reason... > > The "feel" is there because: > > 1. Comments about how what we do is wrong for the platform bus (i.e. adding > the devices and the driver in the same module). Even the documentation > for platform devices make it quite clear we are abusing it. There was > one of those comments in this very thread. This more general than just the platform bus. It's how the Linux 2.6 device driver model is designed. There are alternatives to the way hdaps and thinkpad_acpi instantiate their devices: * These drivers use DMI to find out whether they run on supported hardware. The same detection code could be moved to a separate driver (possibly the same one for all DMI-detected devices) and that separate driver would instantiate the devices as needed. Then the usual hotplug/coldplug rules apply. * Detection could be moved to user-space entirely, then we would only need a way to instantiate these specific devices from user-space. This exists in other areas (scsi, network) for quite some times already so it shouldn't be too difficult. > 2. The fact that a module that has a number of different devices has to > register itself a number of times as a driver, if it wants to name the > devices something different... Sounds like you have a design issue here to start with. Supporting several significantly different devices in the same module is not something that we do usually. -- Jean Delvare ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-08 8:55 ` Jean Delvare @ 2007-09-10 22:52 ` Henrique de Moraes Holschuh 2007-09-11 7:43 ` Jean Delvare 0 siblings, 1 reply; 19+ messages in thread From: Henrique de Moraes Holschuh @ 2007-09-10 22:52 UTC (permalink / raw) To: Jean Delvare; +Cc: David Brownell, linux-kernel, gregkh, dmitry.torokhov On Sat, 08 Sep 2007, Jean Delvare wrote: > This more general than just the platform bus. It's how the Linux 2.6 > device driver model is designed. No issues about that. It is just that the platform bus sucks a bit if you need to "abuse it" (no wonder!) to hang various different devices that are provided *or* driven by the same module. > There are alternatives to the way hdaps and thinkpad_acpi instantiate > their devices: > > * These drivers use DMI to find out whether they run on supported > hardware. The same detection code could be moved to a separate driver Actually, DMI is a hint for autoloading, only. They do probe the hardware/firmware for the needed functionality to know for sure. > (possibly the same one for all DMI-detected devices) and that separate > driver would instantiate the devices as needed. Then the usual > hotplug/coldplug rules apply. That could work, yes. > * Detection could be moved to user-space entirely, then we would only > need a way to instantiate these specific devices from user-space. This > exists in other areas (scsi, network) for quite some times already so > it shouldn't be too difficult. Don't like that one, sorry. Detection often needs the kind of access to hardware that is better off contained in the kernel. > > 2. The fact that a module that has a number of different devices has to > > register itself a number of times as a driver, if it wants to name the > > devices something different... > > Sounds like you have a design issue here to start with. Supporting > several significantly different devices in the same module is not > something that we do usually. I will see what I can do about breaking it up in various modules. But this can be unoptimal. If I took it too seriously, thinkpad-acpi would break into at least five different modules, if not more, and at least one or two modules would need to be there for the common code. There has to be a middle ground somewhere, I think. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-10 22:52 ` Henrique de Moraes Holschuh @ 2007-09-11 7:43 ` Jean Delvare 2007-09-11 13:44 ` Henrique de Moraes Holschuh 0 siblings, 1 reply; 19+ messages in thread From: Jean Delvare @ 2007-09-11 7:43 UTC (permalink / raw) To: hmh Cc: David Brownell, linux-kernel@vger.kernel.org, gregkh@suse.de, dmitry.torokhov@gmail.com Hi Henrique, On 9/10/2007, "Henrique de Moraes Holschuh" <hmh@hmh.eng.br> wrote: >On Sat, 08 Sep 2007, Jean Delvare wrote: >> * Detection could be moved to user-space entirely, then we would only >> need a way to instantiate these specific devices from user-space. This >> exists in other areas (scsi, network) for quite some times already so >> it shouldn't be too difficult. > >Don't like that one, sorry. Detection often needs the kind of access to >hardware that is better off contained in the kernel. Yes, good point. >(...) >I will see what I can do about breaking it up in various modules. But this >can be unoptimal. If I took it too seriously, thinkpad-acpi would break into >at least five different modules, if not more, and at least one or two >modules would need to be there for the common code. There has to be a >middle ground somewhere, I think. I don't know your code and I don't really have the time to look at it in depth, but I'm a bit surprised. Presumably your driver is implementing a number of interfaces (e.g. hwmon) and you create a class device for each one. You can have as many class devices hanging of a (physical) device, so I fail to see why you would need to register several (physical) devices. -- Jean Delvare ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-11 7:43 ` Jean Delvare @ 2007-09-11 13:44 ` Henrique de Moraes Holschuh 2007-09-11 14:05 ` Jean Delvare 0 siblings, 1 reply; 19+ messages in thread From: Henrique de Moraes Holschuh @ 2007-09-11 13:44 UTC (permalink / raw) To: Jean Delvare Cc: David Brownell, linux-kernel@vger.kernel.org, gregkh@suse.de, dmitry.torokhov@gmail.com On Tue, 11 Sep 2007, Jean Delvare wrote: > >I will see what I can do about breaking it up in various modules. But this > >can be unoptimal. If I took it too seriously, thinkpad-acpi would break into > >at least five different modules, if not more, and at least one or two > >modules would need to be there for the common code. There has to be a > >middle ground somewhere, I think. > > I don't know your code and I don't really have the time to look at it > in depth, but I'm a bit surprised. Presumably your driver is > implementing a number of interfaces (e.g. hwmon) and you create a class > device for each one. You can have as many class devices hanging of a > (physical) device, so I fail to see why you would need to register > several (physical) devices. Namespace clashes. Now that class attributes are gone (or going, whatever), everything lives in one namespace: the device. I dread the day I find out one class I need has attribute name clashes with another, or (more likely) one of my driver-specific attributes clash with one from a generic class. Even if clashes never happen, it can make quite a mess when you have four or more classes in the same device. And you can have semanthic clashes, if one looks at various attributes (from different classes or driver-specific) and think they are related in some obvious way (only, it is not obvious at all, and the user didn't read the docs to know it). It is nothing too serious, but something to keep in mind when deciding what to do with a big driver. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-11 13:44 ` Henrique de Moraes Holschuh @ 2007-09-11 14:05 ` Jean Delvare 0 siblings, 0 replies; 19+ messages in thread From: Jean Delvare @ 2007-09-11 14:05 UTC (permalink / raw) To: hmh Cc: David Brownell, linux-kernel@vger.kernel.org, gregkh@suse.de, dmitry.torokhov@gmail.com On 9/11/2007, "Henrique de Moraes Holschuh" <hmh@hmh.eng.br> wrote: >On Tue, 11 Sep 2007, Jean Delvare wrote: >> I don't know your code and I don't really have the time to look at it >> in depth, but I'm a bit surprised. Presumably your driver is >> implementing a number of interfaces (e.g. hwmon) and you create a class >> device for each one. You can have as many class devices hanging of a >> (physical) device, so I fail to see why you would need to register >> several (physical) devices. > >Namespace clashes. Now that class attributes are gone (or going, whatever), >everything lives in one namespace: the device. I dread the day I find out >one class I need has attribute name clashes with another, or (more likely) >one of my driver-specific attributes clash with one from a generic class. This shouldn't be the case. Ex-class devices still have their own directory in sysfs. For example, the radeonfb driver spawns 4 i2c buses (i2c-adapter class), each gets its own directory. One problem at the moment is the hwmon subsystem, because we create the attributes in the physical device rather than in the hwmon class device. This is for historical reasons, but I presume that we'll have to change this someday to work around the name space collision issue you mentioned. If other subsystems are doing the same mistake, this can explain your problem, but this is a subsystem implementation issue, not a problem with class devices themselves. >Even if clashes never happen, it can make quite a mess when you have four or >more classes in the same device. I don't agree. This is how things were designed. >And you can have semanthic clashes, if one >looks at various attributes (from different classes or driver-specific) and >think they are related in some obvious way (only, it is not obvious at all, >and the user didn't read the docs to know it). There's really no reason why you would be randomly poking at attributes without knowing which class device they belong to. -- Jean Delvare ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-07 14:58 ` Dmitry Torokhov 2007-09-07 16:36 ` Jean Delvare 2007-09-07 16:41 ` David Brownell @ 2007-09-07 20:56 ` Henrique de Moraes Holschuh 2007-09-08 8:40 ` Jean Delvare 2 siblings, 1 reply; 19+ messages in thread From: Henrique de Moraes Holschuh @ 2007-09-07 20:56 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Jean Delvare, Greg KH, LKML, David Brownell On Fri, 07 Sep 2007, Dmitry Torokhov wrote: > On 9/7/07, Jean Delvare <khali@linux-fr.org> wrote: > > To go one step further, I am questioning the real value of this naming > > exception for these "unique" platform devices. On top of the bugs I > > mentioned above, it has potential for compatibility breakage: adding a Agreed. But the breakage might happen anyway, if you need to move attributes from foo.0 to foo.1. After that first time, userspace will learn to hunt down all foo.* after what it wants, but still... > If a device has a <name>.<instance> scheme this implies possibility of > having several instances of said device in a box. There are a few of No, it doesn't. It allows for, but it does not imply anything. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-07 20:56 ` Henrique de Moraes Holschuh @ 2007-09-08 8:40 ` Jean Delvare 2007-09-10 22:45 ` Henrique de Moraes Holschuh 0 siblings, 1 reply; 19+ messages in thread From: Jean Delvare @ 2007-09-08 8:40 UTC (permalink / raw) To: Henrique de Moraes Holschuh Cc: Dmitry Torokhov, Greg KH, LKML, David Brownell On Fri, 7 Sep 2007 17:56:59 -0300, Henrique de Moraes Holschuh wrote: > On 9/7/07, Jean Delvare <khali@linux-fr.org> wrote: > > To go one step further, I am questioning the real value of this naming > > exception for these "unique" platform devices. On top of the bugs I > > mentioned above, it has potential for compatibility breakage: adding a > > Agreed. But the breakage might happen anyway, if you need to move > attributes from foo.0 to foo.1. After that first time, userspace will learn > to hunt down all foo.* after what it wants, but still... Moving attributes from one device to another? This doesn't make any sense to me, sorry. foo.0 and foo.1 represent different instances of the same device type, they typically have the same attributes. -- Jean Delvare ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-08 8:40 ` Jean Delvare @ 2007-09-10 22:45 ` Henrique de Moraes Holschuh 0 siblings, 0 replies; 19+ messages in thread From: Henrique de Moraes Holschuh @ 2007-09-10 22:45 UTC (permalink / raw) To: Jean Delvare; +Cc: Dmitry Torokhov, Greg KH, LKML, David Brownell On Sat, 08 Sep 2007, Jean Delvare wrote: > On Fri, 7 Sep 2007 17:56:59 -0300, Henrique de Moraes Holschuh wrote: > > On 9/7/07, Jean Delvare <khali@linux-fr.org> wrote: > > > To go one step further, I am questioning the real value of this naming > > > exception for these "unique" platform devices. On top of the bugs I > > > mentioned above, it has potential for compatibility breakage: adding a > > > > Agreed. But the breakage might happen anyway, if you need to move > > attributes from foo.0 to foo.1. After that first time, userspace will learn > > to hunt down all foo.* after what it wants, but still... > > Moving attributes from one device to another? This doesn't make any > sense to me, sorry. foo.0 and foo.1 represent different instances of > the same device type, they typically have the same attributes. Yeah, I came across that need in an attempt to fix an issue in thinkpad-acpi which will be fixed in another way (separate modules, or separate platform devices/platform drivers in the same module). I must have been sleep-deprieved and in a weird mind state of sorts (and not in a nice way) to come up with it. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Platform device id 2007-09-07 13:35 Platform device id Jean Delvare 2007-09-07 14:58 ` Dmitry Torokhov @ 2007-09-08 13:30 ` Greg KH 2007-09-09 10:54 ` [PATCH] Make platform_device.id an int Jean Delvare 1 sibling, 1 reply; 19+ messages in thread From: Greg KH @ 2007-09-08 13:30 UTC (permalink / raw) To: Jean Delvare; +Cc: LKML, David Brownell On Fri, Sep 07, 2007 at 03:35:59PM +0200, Jean Delvare wrote: > Hi Greg, all, > > While platform_device.id is a u32, platform_device_add() handles "-1" as > a special id value. This has potential for confusion and bugs. One such > bug was reported to me by David Brownell: > > http://lists.lm-sensors.org/pipermail/i2c/2007-September/001787.html > > And since then I've found two other drivers affected (uartlite and > i2c-pxa). > > Could we at least make platform_device.id an int so as to clear up the > confusion? I doubt that the id will ever be a large number anyway. Sure, that's fine by me, feel free to send a patch. thanks, greg k-h ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] Make platform_device.id an int 2007-09-08 13:30 ` Greg KH @ 2007-09-09 10:54 ` Jean Delvare 0 siblings, 0 replies; 19+ messages in thread From: Jean Delvare @ 2007-09-09 10:54 UTC (permalink / raw) To: Greg KH; +Cc: LKML, David Brownell, Dmitry Torokhov While platform_device.id is a u32, platform_device_add() handles "-1" as a special id value. This has potential for confusion and bugs. Making it an int instead should prevent problems from happening in the future. Signed-off-by: Jean Delvare <khali@linux-fr.org> --- I still believe that we can further clean up this area, but that will do for now. drivers/base/platform.c | 7 ++++--- include/linux/platform_device.h | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) --- linux-2.6.23-rc5.orig/drivers/base/platform.c 2007-07-09 14:49:46.000000000 +0200 +++ linux-2.6.23-rc5/drivers/base/platform.c 2007-09-09 12:15:24.000000000 +0200 @@ -166,7 +166,7 @@ static void platform_device_release(stru * the device isn't being dynamically allocated as a legacy "probe the * hardware" driver, infrastructure code should reverse this marking. */ -struct platform_device *platform_device_alloc(const char *name, unsigned int id) +struct platform_device *platform_device_alloc(const char *name, int id) { struct platform_object *pa; @@ -256,7 +256,8 @@ int platform_device_add(struct platform_ pdev->dev.bus = &platform_bus_type; if (pdev->id != -1) - snprintf(pdev->dev.bus_id, BUS_ID_SIZE, "%s.%u", pdev->name, pdev->id); + snprintf(pdev->dev.bus_id, BUS_ID_SIZE, "%s.%d", pdev->name, + pdev->id); else strlcpy(pdev->dev.bus_id, pdev->name, BUS_ID_SIZE); @@ -370,7 +371,7 @@ EXPORT_SYMBOL_GPL(platform_device_unregi * the Linux driver model. In particular, when such drivers are built * as modules, they can't be "hotplugged". */ -struct platform_device *platform_device_register_simple(char *name, unsigned int id, +struct platform_device *platform_device_register_simple(char *name, int id, struct resource *res, unsigned int num) { struct platform_device *pdev; --- linux-2.6.23-rc5.orig/include/linux/platform_device.h 2007-02-04 19:44:54.000000000 +0100 +++ linux-2.6.23-rc5/include/linux/platform_device.h 2007-09-09 12:15:41.000000000 +0200 @@ -15,7 +15,7 @@ struct platform_device { const char * name; - u32 id; + int id; struct device dev; u32 num_resources; struct resource * resource; @@ -35,9 +35,10 @@ extern struct resource *platform_get_res extern int platform_get_irq_byname(struct platform_device *, char *); extern int platform_add_devices(struct platform_device **, int); -extern struct platform_device *platform_device_register_simple(char *, unsigned int, struct resource *, unsigned int); +extern struct platform_device *platform_device_register_simple(char *, int id, + struct resource *, unsigned int); -extern struct platform_device *platform_device_alloc(const char *name, unsigned int id); +extern struct platform_device *platform_device_alloc(const char *name, int id); extern int platform_device_add_resources(struct platform_device *pdev, struct resource *res, unsigned int num); extern int platform_device_add_data(struct platform_device *pdev, const void *data, size_t size); extern int platform_device_add(struct platform_device *pdev); -- Jean Delvare ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2007-09-11 14:05 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-09-07 13:35 Platform device id Jean Delvare 2007-09-07 14:58 ` Dmitry Torokhov 2007-09-07 16:36 ` Jean Delvare 2007-09-07 16:41 ` David Brownell 2007-09-07 21:00 ` Henrique de Moraes Holschuh 2007-09-07 21:41 ` David Brownell 2007-09-07 22:04 ` Henrique de Moraes Holschuh 2007-09-08 0:18 ` David Brownell 2007-09-08 3:50 ` Henrique de Moraes Holschuh 2007-09-08 8:55 ` Jean Delvare 2007-09-10 22:52 ` Henrique de Moraes Holschuh 2007-09-11 7:43 ` Jean Delvare 2007-09-11 13:44 ` Henrique de Moraes Holschuh 2007-09-11 14:05 ` Jean Delvare 2007-09-07 20:56 ` Henrique de Moraes Holschuh 2007-09-08 8:40 ` Jean Delvare 2007-09-10 22:45 ` Henrique de Moraes Holschuh 2007-09-08 13:30 ` Greg KH 2007-09-09 10:54 ` [PATCH] Make platform_device.id an int Jean Delvare
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox