Re: [PATCH V2 1/3] usb: add kerneldoc for usb_get_hub_child_device function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux