Calling device_for_each_child() on a device that has yet to be added
(with device_add()) used to work. UWB subsystem currently tries to do this.
Since "driver core: move klist_children into private structure"[1] this
no longer works (an oops occurs in device_for_each_chid()). Is it
something that ought to work? Or should the UWB subsystem be changed to
not do this?
David
[1] f791b8c836307b58cbf62133a6a772ed1a92fb33
--
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park, Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ http://www.csr.com/
On Tue, Apr 14, 2009 at 05:54:40PM +0100, David Vrabel wrote:
> Calling device_for_each_child() on a device that has yet to be added
> (with device_add()) used to work. UWB subsystem currently tries to do this.
>
> Since "driver core: move klist_children into private structure"[1] this
> no longer works (an oops occurs in device_for_each_chid()). Is it
> something that ought to work? Or should the UWB subsystem be changed to
> not do this?
Why would you call device_for_each_child() if you have never added it to
the bus? How would you expect that to work, as there will never be any
children for it?
thanks,
greg k-h
Greg KH wrote:
> On Tue, Apr 14, 2009 at 05:54:40PM +0100, David Vrabel wrote:
>> Calling device_for_each_child() on a device that has yet to be added
>> (with device_add()) used to work. UWB subsystem currently tries to do this.
>>
>> Since "driver core: move klist_children into private structure"[1] this
>> no longer works (an oops occurs in device_for_each_chid()). Is it
>> something that ought to work? Or should the UWB subsystem be changed to
>> not do this?
>
> Why would you call device_for_each_child() if you have never added it to
> the bus? How would you expect that to work, as there will never be any
> children for it?
The children of a UWB radio controller (RC) are it's neighbors. When
assigning a random DevAddr to an RC we need to check that the address is
unique within the beacon group. We use device_for_each_child() here
to check that the generated DevAddr is not used by any of its neighbors.
This address must be assigned during initialization and during normal
operation and in the past we didn't need to special case the address
assignment during initialization.
David
On Tue, Apr 14, 2009 at 07:16:31PM +0100, David Vrabel wrote:
> Greg KH wrote:
> > On Tue, Apr 14, 2009 at 05:54:40PM +0100, David Vrabel wrote:
> >> Calling device_for_each_child() on a device that has yet to be added
> >> (with device_add()) used to work. UWB subsystem currently tries to do this.
> >>
> >> Since "driver core: move klist_children into private structure"[1] this
> >> no longer works (an oops occurs in device_for_each_chid()). Is it
> >> something that ought to work? Or should the UWB subsystem be changed to
> >> not do this?
> >
> > Why would you call device_for_each_child() if you have never added it to
> > the bus? How would you expect that to work, as there will never be any
> > children for it?
>
> The children of a UWB radio controller (RC) are it's neighbors. When
> assigning a random DevAddr to an RC we need to check that the address is
> unique within the beacon group. We use device_for_each_child() here
> to check that the generated DevAddr is not used by any of its neighbors.
So the problem is for the first device being added, right? Can't you
just have a flag for this?
> This address must be assigned during initialization and during normal
> operation and in the past we didn't need to special case the address
> assignment during initialization.
Does the patch below fix the problem for you?
thanks,
greg k-h
---------------
drivers/base/core.c | 3 +++
1 file changed, 3 insertions(+)
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1142,6 +1142,9 @@ int device_for_each_child(struct device
struct device *child;
int error = 0;
+ if (!parent->p)
+ return 0;
+
klist_iter_init(&parent->p->klist_children, &i);
while ((child = next_device(&i)) && !error)
error = fn(child, data);
Greg KH wrote:
>
> So the problem is for the first device being added, right? Can't you
> just have a flag for this?
It's trivial to fix in the UWB stack.
>> This address must be assigned during initialization and during normal
>> operation and in the past we didn't need to special case the address
>> assignment during initialization.
>
> Does the patch below fix the problem for you?
Yes.
> drivers/base/core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1142,6 +1142,9 @@ int device_for_each_child(struct device
> struct device *child;
> int error = 0;
>
> + if (!parent->p)
> + return 0;
> +
> klist_iter_init(&parent->p->klist_children, &i);
> while ((child = next_device(&i)) && !error)
> error = fn(child, data);
>
>
David
--
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park, Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ http://www.csr.com/
On Wed, Apr 15, 2009 at 03:51:03PM +0100, David Vrabel wrote:
> Greg KH wrote:
> >
> > So the problem is for the first device being added, right? Can't you
> > just have a flag for this?
>
> It's trivial to fix in the UWB stack.
I'd recommend doing that :)
> >> This address must be assigned during initialization and during normal
> >> operation and in the past we didn't need to special case the address
> >> assignment during initialization.
> >
> > Does the patch below fix the problem for you?
>
> Yes.
Thanks for testing, I've now added the patch to my tree.
greg k-h