Re: [RFC PATCH 1/4] usb: add struct usb_hub_port to store port related members.

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

 



On 2012年03月20日 22:04, Alan Stern wrote:
On Tue, 20 Mar 2012, Lan Tianyu wrote:

On 2012年03月20日 00:04, Alan Stern wrote:
On Mon, 19 Mar 2012, Lan Tianyu wrote:

Add struct usb_hub_port pointer port_data in the struct usb_hub and allocate
struct usb_hub_port perspectively for each ports to store private data.

You might as well add the child device pointer into your new data
structure.  This will require changes to at least three other files in
addition to hub.c:
hi alan:
	Great thanks for your review.
	But I still confuse about "You might as well add the child device pointer
into your new data structure." Could you help me to make it clear? :)
	Do you mean add struct usb_device pointer toward the device attached to the
port in the
struct usb_hub_port? If yes,Why?

Yes, that's what I mean.  It's a natural thing to do; the child device
is directly associated with the port.  It's better than allocating a
separate array for hdev->children.
OK. That' mean that I should replace the hdev->children with port_data->child_device. In the hub.c, the struct usb_hub_port can be visited and the child_device can be accessed directly. Out of hub.c, I should provide function to return child pointer. Is that right?

	devices.c, host/r8a66597-hcd.c,
	drivers/staging/usbip/usbip_common.c

Maybe some others; I didn't look through the entire kernel source.  It
also means you will have to export a function to get a pointer to the
child device, given the port number.
	If it was necessary, could I fill the child pointer in the
hub_port_connect_change()? just
after a new usb device being created.

static void hub_port_connect_change(struct usb_hub *hub, int port1,
					u16 portstatus, u16 portchange) {
		...
		/* Run it through the hoops (find a driver, etc) */
		if (!status) {
			status = usb_new_device(udev);
			if (status) {
				spin_lock_irq(&device_state_lock);
				hdev->children[port1-1] = NULL;
				spin_unlock_irq(&device_state_lock);
			}
			/* like here?*/
		}
		...
}

That's the right idea, but the wrong place.  The right place to fill
in the pointer is a few lines higher, where the code already does

			hdev->children[port1-1] = udev;

while holding the device_state_lock.

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


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

  Powered by Linux