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
-

[ message continues ]

Hi Jean,


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
-

[ message continues ]

Hi Dmitry,

Thanks for your answer.



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

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
-

[ message continues ]

Like USB peripheral controllers.  Only one external "B" type


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

-

[ message continues ]

This assumes that we have a better bus than "platform" to dump drivers like

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
-

[ message continues ]

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

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

-

[ message continues ]

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

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
-

[ message continues ]

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,

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)

As a rule, there needs to be a Good Reason to create a new bus
type.  A "feel" is a pretty weak reason...

- Dave

-

[ message continues ]

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

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
-

[ message continues ]

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

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
-

[ message continues ]

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

Actually, DMI is a hint for autoloading, only.  They do probe the


Don't like that one, sorry.  Detection often needs the kind of access to

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
-

[ message continues ]

Hi Henrique,



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
-

[ message continues ]

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
-

[ message continues ]

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


There's really no reason why you would be randomly poking at attributes
without knowing which class device they belong to.

--
Jean Delvare
-

[ message continues ]

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

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
-

[ message continues ]

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
-

[ message continues ]

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
-

[ message continues ]

Sure, that's fine by me, feel free to send a patch.

thanks,

greg k-h
-

[ message continues ]

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 ...
[ message continues ]