On 2012/5/16 22:33, Alan Stern wrote:
On Wed, 16 May 2012, Lan Tianyu wrote:
Ick, no, again, make it an iterator, as that's what you really want,
right?
Sorry. That's not really I want. If make it an iterator, how to get one hub's child?
You can write both an iterator and a single-access function.
Yeah. I will do it.
As you know, I will also work on power off mechanism. In some places, I need
to get one port's child or check whether there is a device on the port. So
an iterator doesn't work.
An more question: If I make an iterator, different places have different
process way. That means to add an callback param?
usb_hub_children_iterator(hdev, callback)
{
struct usb_hub *hub = hdev_to_hub(hdev);
struct usb_device *child;
int port1;
for(port1 = 1; port1< hdev->maxchild; port1++) {
child = usb_get_dev(hub->port_data[port1 - 1].child);
if (child) {
usb_lock_device(child);
callback(child);
usb_unlock_device(child);
usb_put_dev(child);
}
}
}
right?
IMO it would be better to copy the pattern used by list_for_each(), if
you can. That pattern doesn't use a callback.
Also, you should check to see whether the usb_lock_device() call is
really needed. If the caller already owns the lock for the parent hub
then you don't need to lock the child -- you don't even need the
usb_{get,put}_dev() calls.
How about this?
#define usb_get_hub_each_child(hdev, port1, child) \
for (port1 = 1, child = usb_get_hub_child(hdev, port1); \
port1 <= hdev->maxchild; child = \
usb_get_hub_child(hdev, ++port1))
usb_get_hub_child(hdev, port1)
{
struct usb_hub *hub = hdev_to_hub(hdev);
return usb_get_dev(hub->port_data[port1 - 1].child);
}
For more general, usb_get_hub_child() increases child refcount
to prevent child from being removed during processing and the caller
should invoke usb_put_dev() when child will never be used.
Alan Stern
--
Best Regards
Tianyu Lan
linux kernel enabling team
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
