From: Joachim Fenkes <hidden> Date: 2007-08-29 16:15:54
Previously, ibmebus derived a device's bus_id from its location code. The
location code is not guaranteed to be unique, so we might get bus_id
collisions if two devices share the same location code. The OFDT full_name,
however, is unique, so we use that instead.
Signed-off-by: Joachim Fenkes <redacted>
---
The patch has been tested and works fine. If you think it's too much change
for 2.6.23-rc5, please schedule for 2.6.24 instead.
arch/powerpc/kernel/ibmebus.c | 30 +++++++++---------------------
1 files changed, 9 insertions(+), 21 deletions(-)
Previously, ibmebus derived a device's bus_id from its location code. The
location code is not guaranteed to be unique, so we might get bus_id
collisions if two devices share the same location code. The OFDT full_name,
however, is unique, so we use that instead.
This is a userspace-visible change, but I guess it's unavoidable.
Will anything break?
Also, I dislike this approach of duplicating the firmware device tree
path in sysfs. Are GX/ibmebus devices guaranteed to be children of
the same node in the OF device tree? If so, their unit addresses will
be unique, and therefore suitable values for bus_id. I believe this
is what the powerpc vio bus code does.
+ len = strlen(dn->full_name + 1);
+ bus_len = min(len, BUS_ID_SIZE - 1);
+ memcpy(dev->ofdev.dev.bus_id, dn->full_name + 1
+ + (len - bus_len), bus_len);
+ for (i = 0; i < bus_len; i++)
+ if (dev->ofdev.dev.bus_id[i] == '/')
+ dev->ofdev.dev.bus_id[i] = '_';
/* Register with generic device framework. */
if (ibmebus_register_device_common(dev, dn->name) != 0) {
What happens when the full name is > 31 characters? It looks to me that it
will be truncated, which takes away the uniqueness guarantee.
There must be an individual property that is guaranteed to be unique and
less than 32 characters. How about "ibm,my-drc-index"? That looks like a
good candidate.
From: Joachim Fenkes <hidden> Date: 2007-08-30 14:01:17
Nathan Lynch [off-list ref] wrote on 29.08.2007 20:12:32:
quoted
Previously, ibmebus derived a device's bus_id from its location code.
The
quoted
location code is not guaranteed to be unique, so we might get bus_id
collisions if two devices share the same location code. The OFDT
full_name,
quoted
however, is unique, so we use that instead.
This is a userspace-visible change, but I guess it's unavoidable.
Will anything break?
Nope. Userspace programs should not depend on ibmebus' way of naming the
devices; especially since some overly long loc_codes tended to be
truncated and thus rendered useless. I have tested IBM's DLPAR tools
against the changed kernel, and they didn't break.
Also, I dislike this approach of duplicating the firmware device tree
path in sysfs.
Why? Any specific reasons for your dislike?
Are GX/ibmebus devices guaranteed to be children of
the same node in the OF device tree? If so, their unit addresses will
be unique, and therefore suitable values for bus_id. I believe this
is what the powerpc vio bus code does.
While there's no such guarantee (as in "officially signed document"), yes,
I expect future GX devices to also appear beneath the OFDT root node. For
the existing devices, the unit addresses are already part of the device
name, so I save the need to use sprintf() again. Plus, I rather like using
the full_name since it also contains a descriptive name as opposed to
being just nondescript numbers, helping the layman (ie user) to make sense
out of a dev_id.
jschopp [off-list ref] wrote on 29.08.2007 20:33:30:
quoted
+ len = strlen(dn->full_name + 1);
+ bus_len = min(len, BUS_ID_SIZE - 1);
+ memcpy(dev->ofdev.dev.bus_id, dn->full_name + 1
+ + (len - bus_len), bus_len);
+ for (i = 0; i < bus_len; i++)
+ if (dev->ofdev.dev.bus_id[i] == '/')
+ dev->ofdev.dev.bus_id[i] = '_';
/* Register with generic device framework. */
if (ibmebus_register_device_common(dev, dn->name) != 0) {
What happens when the full name is > 31 characters? It looks to me that
it
will be truncated, which takes away the uniqueness guarantee.
There are currently two GX devices, eHCA and eHEA, which both reside
beneath the root node - this is required by architecture for those
devices. Unless they invent a device called
"supercalifragilisticexpialidocious", devices in the root note will have a
full_name of less than 31 chars. Even in that case, the truncation occurs
at the beginning, so the @xxx part that makes the nodes unique will stay
in place.
If any more GX devices appear on the scene, I expect them to appear in the
root node as well. The substitution of "/" by "_" is a safeguard so
possible weird OFDT setups don't break the kernel.
There must be an individual property that is guaranteed to be unique and
less than 32 characters. How about "ibm,my-drc-index"? That looks like
a
good candidate.
On first glance, it does, however the attribute might not be present in
all cases. Architecture states it only needs to be present on systems with
dynamic reconfiguration enabled.
All things considered, I still like the idea of using the full_name most.
No offense.
Regards,
Joachim
Nathan Lynch [off-list ref] wrote on 29.08.2007 20:12:32:
quoted
Will anything break?
Nope. Userspace programs should not depend on ibmebus' way of naming the
devices; especially since some overly long loc_codes tended to be
truncated and thus rendered useless. I have tested IBM's DLPAR tools
against the changed kernel, and they didn't break.
Okay.
quoted
Also, I dislike this approach of duplicating the firmware device tree
path in sysfs.
Why? Any specific reasons for your dislike?
struct device's bus_id field is but 20 bytes in size. Too close for
comfort?
quoted
Are GX/ibmebus devices guaranteed to be children of
the same node in the OF device tree? If so, their unit addresses will
be unique, and therefore suitable values for bus_id. I believe this
is what the powerpc vio bus code does.
While there's no such guarantee (as in "officially signed document"), yes,
I expect future GX devices to also appear beneath the OFDT root node. For
the existing devices, the unit addresses are already part of the device
name, so I save the need to use sprintf() again. Plus, I rather like using
the full_name since it also contains a descriptive name as opposed to
being just nondescript numbers, helping the layman (ie user) to make sense
out of a dev_id.
Okay, but your layman isn't supposed to be relying on any
user-friendly properties of the name :) Hope he doesn't work on a
distro installer.
Anyway, if you're still confident in this approach, I relent. :)
On Wednesday 29 August 2007, Joachim Fenkes wrote:
Previously, ibmebus derived a device's bus_id from its location code. The
location code is not guaranteed to be unique, so we might get bus_id
collisions if two devices share the same location code. The OFDT full_name,
however, is unique, so we use that instead.
Signed-off-by: Joachim Fenkes <redacted>
Actually, I think it would be much better to convert the code to be more
like of_platform_device, or to even replace all of ibmebus with that.
The whole logic of dynamically adding and removing device is rather bogus,
and it prevents autoloading of device drivers. of_platform_make_bus_id
is the function that is responsible for creating unique names over there.
Arnd <><
From: Joel Schopp <hidden> Date: 2007-08-30 20:36:45
There are currently two GX devices, eHCA and eHEA, which both reside
beneath the root node - this is required by architecture for those
devices. Unless they invent a device called
"supercalifragilisticexpialidocious", devices in the root note will have a
full_name of less than 31 chars. Even in that case, the truncation occurs
at the beginning, so the @xxx part that makes the nodes unique will stay
in place.
OK, didn't realize it had to be beneath the root node, and that the
truncation truncated the front and not the back. I would have done it
differently, but this should work.
Acked-by: Joel Schopp <redacted>
On Thu, Aug 30, 2007 at 04:00:56PM +0200, Joachim Fenkes wrote:
Plus, I rather like using
the full_name since it also contains a descriptive name as opposed to
being just nondescript numbers, helping the layman (ie user) to make sense
out of a dev_id.
Yes, well, but no. The location code is useful as a geographical
location: slots and devices are physically labelled with stickers
so you can tell which is which. Handy when you have to unplug stuff.
By contrast, the device-tree full_name is mostly just gobldy-gook,
with some crazy phb numbering in there that, after four years of
staring at them, I still can't reliably do anything useful with.
Location codes are nice.
--linas
From: Joachim Fenkes <hidden> Date: 2007-08-31 08:57:50
linas@austin.ibm.com (Linas Vepstas) wrote on 30.08.2007 23:28:16:
On Thu, Aug 30, 2007 at 04:00:56PM +0200, Joachim Fenkes wrote:
quoted
Plus, I rather like using
the full_name since it also contains a descriptive name as opposed to
being just nondescript numbers, helping the layman (ie user) to make
sense
quoted
out of a dev_id.
[...]
Location codes are nice.
I sure agree with you. Still, they're not unique, so we can't use them as
bus_id. That's why we're having this discussion at all.
What I meant was "I like using the full_name over using the device address
/ DRC index / you-name-it only". Location code is right out.
Cheers,
Joachim
From: Joachim Fenkes <hidden> Date: 2007-08-31 14:35:13
Hi, Arnd,
The whole logic of dynamically adding and removing device is rather
bogus,
and it prevents autoloading of device drivers. of_platform_make_bus_id
is the function that is responsible for creating unique names over
there.
The plaintiff makes a valid point. How about a staging approach: We put
the patch as it is now into 2.6.23 so the problem is fixed, and I'll post
a "nice" version with autoloading support and a generic of_make_bus_id
function for 2.6.24. Agree?
Regards,
Joachim
The whole logic of dynamically adding and removing device is rather
bogus,
quoted
and it prevents autoloading of device drivers. of_platform_make_bus_id
is the function that is responsible for creating unique names over
there.
The plaintiff makes a valid point. How about a staging approach: We put
the patch as it is now into 2.6.23 so the problem is fixed, and I'll post
a "nice" version with autoloading support and a generic of_make_bus_id
function for 2.6.24. Agree?
Ok, sounds fair. Can you make sure that the resulting bus_id is the same
for the final version then?
Arnd <><
From: Joachim Fenkes <hidden> Date: 2007-08-31 17:47:17
quoted
The plaintiff makes a valid point. How about a staging approach: We
put
quoted
the patch as it is now into 2.6.23 so the problem is fixed, and I'll
post
quoted
a "nice" version with autoloading support and a generic of_make_bus_id
quoted
function for 2.6.24. Agree?
Ok, sounds fair. Can you make sure that the resulting bus_id is the same
for the final version then?
No, it would change once more -- the current, minimal-invasive, variant of
the fix uses the full_name -> "ehca@12345678", while of_make_bus_id uses
another format: "12345678.ehca". I don't think this makes much of a
difference, though, and users shouldn't rely on the bus_id having a
certain format anyway, so IMHO we can live with this.
Regards,
Joachim