Re: [PATCH v6 2/8] usb: Register usb port's acpi power resources

[Lan Tianyu <tianyu.lan@intel.com>]

Hi Greg:
	Great thanks for your review. I learn a lot from your comments which I
have not noticed before.

On 2013年01月22日 05:20, Greg KH wrote:
> On Mon, Jan 21, 2013 at 10:18:01PM +0800, Lan Tianyu wrote:
>> This patch is to register usb port's acpi power resources. Create
>> link between usb port device and its acpi power resource.
>>
>> Acked-by: Alan Stern <stern@rowland.harvard.edu>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>> ---
>>  drivers/usb/core/port.c     |    3 +++
>>  drivers/usb/core/usb-acpi.c |   20 ++++++++++++++++++++
>>  drivers/usb/core/usb.h      |    6 ++++++
>>  3 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
>> index fe5959f..4dfa254 100644
>> --- a/drivers/usb/core/port.c
>> +++ b/drivers/usb/core/port.c
>> @@ -66,6 +66,7 @@ static void usb_port_device_release(struct device *dev)
>>  {
>>  	struct usb_port *port_dev = to_usb_port(dev);
>>  
>> +	usb_acpi_unregister_power_resources(dev);
>>  	kfree(port_dev);
>>  }
>>  
>> @@ -95,6 +96,8 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
>>  	if (retval)
>>  		goto error_register;
>>  
>> +	usb_acpi_register_power_resources(&port_dev->dev);
>> +
>>  	return 0;
>>  
>>  error_register:
>> diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
>> index cef4252..558ab01 100644
>> --- a/drivers/usb/core/usb-acpi.c
>> +++ b/drivers/usb/core/usb-acpi.c
>> @@ -216,6 +216,26 @@ static struct acpi_bus_type usb_acpi_bus = {
>>  	.find_device = usb_acpi_find_device,
>>  };
>>  
>> +int usb_acpi_register_power_resources(struct device *dev)
>> +{
>> +	acpi_handle port_handle = DEVICE_ACPI_HANDLE(dev);
>> +
>> +	if (!port_handle)
>> +		return -ENODEV;
>> +
>> +	if (acpi_power_resource_register_device(dev, port_handle) < 0)
>> +		return -ENODEV;
>> +	return 0;
>> +}
> 
> Why return a value if you never check it?  Either properly handle the
> error in the place you call this function, or don't have a return value.
> 
> I'd prefer you to handle the error properly.
Not all system will provide acpi power resouces for usb port and this
will not damage usb device function. So I think I should produce some
warning if the return value is not ENODEV.
> 
> thanks,
> 
> greg k-h
> 


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