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日 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?

	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?*/
		}
		...
}

Thanks again. Look forward for your reply.

@@ -1451,13 +1456,12 @@ void usb_hub_release_all_ports(struct usb_device *hdev, void *owner)
  	int n;
  	void **powner;

-	n = find_port_owner(hdev, 1,&powner);
-	if (n == 0) {
-		for (; n<  hdev->maxchild; (++n, ++powner)) {
-			if (*powner == owner)
-				*powner = NULL;
-		}
+	for (n = 1; n<= hdev->maxchild; n++) {
+		find_port_owner(hdev, n,&powner);
+		if (*powner == owner)
+			*powner = NULL;
  	}

This is wrong; find_port_owner() doesn't set powner to anything if an
error occurs.

You can just set the pointer field directly to NULL, instead of looking
it up in find_port_owner().  I don't remember why it was written that
way originally.

Alan Stern



--
Best Regards
Tianyu Lan
linux kernel enabling team
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux