On Thu, May 17, 2012 at 09:30:15PM +0800, Lan Tianyu wrote: > 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. That's better, yes. greg k-h -- 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