bus_id collisions

bus_id collisions

[Benjamin Herrenschmidt]

Hi Greg !

It occurs to me (after some trouble I had with custom bus types) that
this comment is incorrect in device.h, in the definition of struct
device :

        char    bus_id[BUS_ID_SIZE];    /* position on parent bus */

As the bus_id needs to be unique for a given bus_type, not only under a
given parent, due to the symlinks in /sys/bus/<bus_type>/.

This has caused me some trouble with of_platform devices, which are
sort-of platform devices but linked to the Open Firmware device-tree, as
I generate their names based on the nodes in the tree which need not be
unique as long as they are unique under a given parent.

I've worked around it, but I though the comment might need to be
clarified.

Also, I don't suppose you have any plan to move away from the bus_id
being a fixed size array inside struct device ? I would very much like
to be able to have larger names ... Among others, in order to handle the
above problem, I tend to include the fully translated 64 bits address of
the device in the name :-) (Hopefully, it's generally smaller and I
don't have leading zero's but still, I have little room left for the
device name which is annoying).

Cheers,
Ben.

Re: bus_id collisions

[David Miller]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Tue, 21 Nov 2006 15:02:16 +1100

> This has caused me some trouble with of_platform devices, which are
> sort-of platform devices but linked to the Open Firmware device-tree, as
> I generate their names based on the nodes in the tree which need not be
> unique as long as they are unique under a given parent.
> 
> I've worked around it, but I though the comment might need to be
> clarified.

BTW Ben, on sparc64 for of devices I use "%08x" and the PROM
node ID as the bus_id[] to deal with this.

Re: bus_id collisions

[Benjamin Herrenschmidt]

On Mon, 2006-11-20 at 22:13 -0800, David Miller wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Tue, 21 Nov 2006 15:02:16 +1100
> 
> > This has caused me some trouble with of_platform devices, which are
> > sort-of platform devices but linked to the Open Firmware device-tree, as
> > I generate their names based on the nodes in the tree which need not be
> > unique as long as they are unique under a given parent.
> > 
> > I've worked around it, but I though the comment might need to be
> > clarified.
> 
> BTW Ben, on sparc64 for of devices I use "%08x" and the PROM
> node ID as the bus_id[] to deal with this.

Yes, the phandle would have been a good option... 

Unfortunately, when we defined the simplified device-tree format for use
by platforms without a real Open Firmware (embedded etc...), we made the
phandle optional (the flat device-tree format that we defined doesn't
have it, it's added as an optional property linux,phandle only when a
node needs to be referenced by another one, like interrupt-map's
etc...). Part of the reason there was to please embedded folks who
scream at every single byte added anywhere :-)

I suppose I can still decide that it's also mandatory for nodes that are
to be used as of_platform_device's though... I need to discuss that with
the embedded folks.

(BTW. I still need to look into back-porting some of your changes to
that stuff and possibly having some of that code moved to a common
location... I hope I'll have some time for that early next year).

Ben.

Re: bus_id collisions

[Greg KH]

On Tue, Nov 21, 2006 at 03:02:16PM +1100, Benjamin Herrenschmidt wrote:
> Hi Greg !
> 
> It occurs to me (after some trouble I had with custom bus types) that
> this comment is incorrect in device.h, in the definition of struct
> device :
> 
>         char    bus_id[BUS_ID_SIZE];    /* position on parent bus */
> 
> As the bus_id needs to be unique for a given bus_type, not only under a
> given parent, due to the symlinks in /sys/bus/<bus_type>/.

Yes, sorry, that comment is _very_ old...

> This has caused me some trouble with of_platform devices, which are
> sort-of platform devices but linked to the Open Firmware device-tree, as
> I generate their names based on the nodes in the tree which need not be
> unique as long as they are unique under a given parent.
> 
> I've worked around it, but I though the comment might need to be
> clarified.

Ok.

> Also, I don't suppose you have any plan to move away from the bus_id
> being a fixed size array inside struct device ?

No, I had not heard of anyone having any problems with the size of it.
We could just do it like the kobject does, and have a function to set it
and handle the pointer vs. array issue there if you _really_ need a
bigger size.

> I would very much like to be able to have larger names ... Among
> others, in order to handle the above problem, I tend to include the
> fully translated 64 bits address of the device in the name :-)
> (Hopefully, it's generally smaller and I don't have leading zero's but
> still, I have little room left for the device name which is annoying).

Oh, that's a very annoying bus id, but I guess it's legal.

I'll poke around and see if I can make it bigger.  You don't want it
bigger for static 'struct device' types, right?

thanks,

greg k-h

Re: bus_id collisions

[Benjamin Herrenschmidt]

> Oh, that's a very annoying bus id, but I guess it's legal.
> 
> I'll poke around and see if I can make it bigger.  You don't want it
> bigger for static 'struct device' types, right?

Don't bother too much if it's complicated. I'm trying other options as
well, like possibly using the Open Firmware "phandle" (sort of object
ID) which is only 32 bits, provided I can get the embedded folks to
agree to have one at all...

Cheers,
Ben.

Re: bus_id collisions

[Kumar Gala]

On Nov 21, 2006, at 12:49 AM, Benjamin Herrenschmidt wrote:

>
>> Oh, that's a very annoying bus id, but I guess it's legal.
>>
>> I'll poke around and see if I can make it bigger.  You don't want it
>> bigger for static 'struct device' types, right?
>
> Don't bother too much if it's complicated. I'm trying other options as
> well, like possibly using the Open Firmware "phandle" (sort of object
> ID) which is only 32 bits, provided I can get the embedded folks to
> agree to have one at all...

Hopefully, we will not complain too much.  I can't imagine an extra  
32-bits for the two dozen or so nodes we have in a embedded tree is  
going to cause any heart burn.

- k