public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* 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 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 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-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  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-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

* 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-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

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