On Thu, Feb 6, 2014 at 12:57 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, 31 Jan 2014, Dan Williams wrote:
>
>> For example:
>
> Topic sentence? A reader seeing this could be forgiven for wondering
> "Example of what?".
>
Ok, fixed.
>> usb2/2-1/2-1:1.0/port1/peer => ../../../../usb3/3-1/3-1:1.0/port1
>> usb2/2-1/2-1:1.0/port2/peer => ../../../../usb3/3-1/3-1:1.0/port2
>> usb2/2-1/2-1:1.0/port3/peer => ../../../../usb3/3-1/3-1:1.0/port3
>> usb2/2-1/2-1:1.0/port4/peer => ../../../../usb3/3-1/3-1:1.0/port4
>> usb2/2-0:1.0/port1/peer => ../../../usb3/3-0:1.0/port1
>> usb2/2-0:1.0/port2/peer => ../../../usb3/3-0:1.0/port2
>> usb2/2-0:1.0/port3/peer => ../../../usb3/3-0:1.0/port3
>> usb2/2-0:1.0/port4/peer => ../../../usb3/3-0:1.0/port4
>>
>> usb3/3-1/3-1:1.0/port1/peer => ../../../../usb2/2-1/2-1:1.0/port1
>> usb3/3-1/3-1:1.0/port2/peer => ../../../../usb2/2-1/2-1:1.0/port2
>> usb3/3-1/3-1:1.0/port3/peer => ../../../../usb2/2-1/2-1:1.0/port3
>> usb3/3-1/3-1:1.0/port4/peer => ../../../../usb2/2-1/2-1:1.0/port4
>> usb3/3-0:1.0/port1/peer => ../../../usb2/2-0:1.0/port1
>> usb3/3-0:1.0/port2/peer => ../../../usb2/2-0:1.0/port2
>> usb3/3-0:1.0/port3/peer => ../../../usb2/2-0:1.0/port3
>> usb3/3-0:1.0/port4/peer => ../../../usb2/2-0:1.0/port4
>
>> + mutex_lock(&peer_lock);
>> + peer = port_dev->peer;
>> + do if (peer) {
>> + retval = sysfs_create_link(&port_dev->dev.kobj,
>> + &peer->dev.kobj, "peer");
>> + if (retval)
>> + break;
>> + retval = sysfs_create_link(&peer->dev.kobj,
>> + &port_dev->dev.kobj, "peer");
>> + } while (0);
>
> This is an unusual idiom; I don't recall seeing it before. How about
> writing this in a more conventional fashion? It would require one
> less line of code...
I moved it into the new link_peers() helper, so it's moot now.
>
>> + mutex_unlock(&peer_lock);
>> + if (retval)
>> + goto error_links;
>> +
>> pm_runtime_set_active(&port_dev->dev);
>>
>> /* It would be dangerous if user space couldn't
>> @@ -339,6 +357,8 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
>>
>> error_register:
>> put_device(&port_dev->dev);
>> +error_links:
>> + device_unregister(&port_dev->dev);
>
> You don't explicitly remove the first link if the second link fails. I
> guess sysfs is smart enough to do this for you, right?
I went ahead and added this to link_peers() as well.
--
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
