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. > 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. Alan Stern -- 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