Message-ID: <54368AF8.1060807@opensource.altera.com>
Date: Thu, 9 Oct 2014 08:17:44 -0500
From: Dinh Nguyen <dinguyen@opensource.altera.com>
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.6.0
MIME-Version: 1.0
To: Philipp Zabel <p.zabel@pengutronix.de>
CC: <dinh.linux@gmail.com>, <atull@opensource.altera.com>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] reset: add reset_control_status helper function
References: <1412808351-4598-1-git-send-email-dinguyen@opensource.altera.com> <1412844240.6809.1.camel@pengutronix.de>
In-Reply-To: <1412844240.6809.1.camel@pengutronix.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org

Hi Philipp,

On 10/9/14, 3:44 AM, Philipp Zabel wrote:
> Hi Dinh,
> 
> thank you for the patch. Just two small issues below:
> 
> Am Mittwoch, den 08.10.2014, 17:45 -0500 schrieb dinguyen@opensource.altera.com:
> [...]
>> @@ -126,6 +126,19 @@ int reset_control_deassert(struct reset_control *rstc)
>>  EXPORT_SYMBOL_GPL(reset_control_deassert);
>>  
>>  /**
>> + * reset_control_status - returns a status of a reset bit
>> + * @rstc: reset controller
>> + */
>> +unsigned int reset_control_status(struct reset_control *rstc)
>> +{
>> +	if (rstc->rcdev->ops->status)
>> +		return rstc->rcdev->ops->status(rstc->rcdev, rstc->id);
>> +
>> +	return -ENOSYS;
>> +}
>> +EXPORT_SYMBOL_GPL(reset_control_status);
> 
> Since this function can return negative error numbers, please make the
> return value of reset_control_status int. That also means that drivers
> must not set the MSB when returning status.

Ah yes...Don't why I changed it to unsigned int for the return.

> 
>> +
>> +/**
>>   * of_reset_control_get - Lookup and obtain a reference to a reset controller.
>>   * @node: device to be reset by the controller
>>   * @id: reset line name
>> diff --git a/include/linux/reset-controller.h b/include/linux/reset-controller.h
>> index 41a4695..8e659d5 100644
>> --- a/include/linux/reset-controller.h
>> +++ b/include/linux/reset-controller.h
>> @@ -17,6 +17,7 @@ struct reset_control_ops {
>>  	int (*reset)(struct reset_controller_dev *rcdev, unsigned long id);
>>  	int (*assert)(struct reset_controller_dev *rcdev, unsigned long id);
>>  	int (*deassert)(struct reset_controller_dev *rcdev, unsigned long id);
>> +	unsigned int (*status)(struct reset_controller_dev *rcdev, unsigned long id);
>>  };
> 
> Please change the return value of the status callback to int and
> describe it in the kerneldoc comment above. It should probably be
> mentioned that the returned status value must be >= 0 except in error
> cases.
>  
Will do for V2...

Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/