public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dinh Nguyen <dinguyen@opensource.altera.com>
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
Date: Thu, 9 Oct 2014 08:17:44 -0500	[thread overview]
Message-ID: <54368AF8.1060807@opensource.altera.com> (raw)
In-Reply-To: <1412844240.6809.1.camel@pengutronix.de>

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

      reply	other threads:[~2014-10-09 13:18 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-08 22:45 [PATCH] reset: add reset_control_status helper function dinguyen
2014-10-09  8:44 ` Philipp Zabel
2014-10-09 13:17   ` Dinh Nguyen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54368AF8.1060807@opensource.altera.com \
    --to=dinguyen@opensource.altera.com \
    --cc=atull@opensource.altera.com \
    --cc=dinh.linux@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=p.zabel@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox