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
prev parent 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.