On 2012年10月25日 12:05, Greg KH wrote:
> On Thu, Oct 25, 2012 at 11:19:05AM +0800, Lan Tianyu wrote:
>> On 2012/10/25 5:49, Greg KH wrote:
>>> On Wed, Oct 24, 2012 at 09:24:04PM +0800, Lan Tianyu wrote:
>>>> 于 2012/10/24 18:55, Sergei Shtylyov 写道:
>>>>>> + /* Create link files between child device and usb port device. */
>>>>>> + if (udev->parent) {
>>>>>> + int no_warn;
>>>>>
>>>>> I think 'ret' or 'result' is a better name...
>>>>>
>>>>>> + struct usb_port *port_dev =
>>>>>> + hdev_to_hub(udev->parent)->ports[udev->portnum - 1];
>>>>>> +
>>>>>> + no_warn = sysfs_create_link(&udev->dev.kobj,
>>>>>> + &port_dev->dev.kobj, "port");
>>>>>> + no_warn = sysfs_create_link(&port_dev->dev.kobj,
>>>>>> + &udev->dev.kobj, "child");
>>>>>
>>>>> I guess you are not supposed to ignore the result if the function requires
>>>>> it not to be ignored.
>>>>>
>>>> Hi Sergei:
>>>> Great thanks for your review. From my opinion, failure to create link will
>>>> not affect usb device function and so the return value can be ignored, Perharps
>>>> producing
>>>> some warning will be better. Do you have some suggestion? :)
>>>
>>> Properly handle the error, don't ignore it.
>>>
>> HI Greg:
>> Ok, I get it. How about following?
>> + /* Create link files between child device and usb port device. */
>> + if (udev->parent) {
>> + struct usb_port *port_dev =
>> + hdev_to_hub(udev->parent)->ports[udev->portnum - 1];
>> +
>> + err = sysfs_create_link(&udev->dev.kobj,
>> + &port_dev->dev.kobj, "port");
>> + if (err)
>> + goto fail;
>> +
>> + err = sysfs_create_link(&port_dev->dev.kobj,
>> + &udev->dev.kobj, "child");
>> + if (err) {
>> + sysfs_remove_link(&udev->dev.kobj, "port");
>> + goto fail;
>> + }
>> + }
>> +
>
> It's a good start, did you test it out?
>
Yeah. I basicly check whether these link files are created under related
sysfs directory or not. It works. Did you find some potential problems?
Or give me more test ways. I'd like to do it. Thanks.
> And why are you calling the devices in a port a "child"? It's a device.
> Otherwise everything is a child, and that doesn't make any sense, right?
Right. Renaming it with "device" is ok for you?
>
> greg k-h
>
--
Best regards
Tianyu Lan
--
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
