diff mbox series

[3/8] regulator: core: Fix incorrectly formatted kerneldoc "Return" sections

Message ID 20240827095550.675018-4-wenst@chromium.org (mailing list archive)
State Superseded, archived
Headers show
Series regulator: kerneldoc section fixes | expand

Commit Message

Chen-Yu Tsai Aug. 27, 2024, 9:55 a.m. UTC
kernel-doc complains about missing "Return" section for many documented
functions in the regulator core. Many of them actually have descriptions
about the return values, just not in the format kernel-doc wants.

Convert these to use the proper "Return:" section header. The existing
descriptions have been reworded and moved around to fit the grammar and
formatting.

In a few cases where the functions don't call even more functions
and the error numbers are known, those are documented in detail.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/regulator/core.c | 101 ++++++++++++++++++++++-----------------
 1 file changed, 56 insertions(+), 45 deletions(-)

Comments

Andy Shevchenko Aug. 27, 2024, 2:41 p.m. UTC | #1
On Tue, Aug 27, 2024 at 05:55:43PM +0800, Chen-Yu Tsai wrote:
> kernel-doc complains about missing "Return" section for many documented
> functions in the regulator core. Many of them actually have descriptions
> about the return values, just not in the format kernel-doc wants.
> 
> Convert these to use the proper "Return:" section header. The existing
> descriptions have been reworded and moved around to fit the grammar and
> formatting.
> 
> In a few cases where the functions don't call even more functions
> and the error numbers are known, those are documented in detail.

...

> + * Return: pointer the &struct device_node corresponding to the regulator if found,

"pointer to the"
Same elsewhere.

> + *	   or %NULL if not found.

...

> + * Return: pointer to a &struct regulator corresponding to the regulator
> + *	   producer, or ERR_PTR() encoded negative error number.

(I'm not sure of definite vs. indefinite article, though. Perhaps you need to
consult with native speaker.)

...

> + *	   producer, or ERR_PTR() encoded negative error number.

Okay, maybe "negative error number" to be used everywhere (see previous email),
the main point is a) to make it clear that it's negative, and b) be consistent
with a term across the subsystem.
Chen-Yu Tsai Aug. 28, 2024, 8:01 a.m. UTC | #2
On Tue, Aug 27, 2024 at 10:42 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Aug 27, 2024 at 05:55:43PM +0800, Chen-Yu Tsai wrote:
> > kernel-doc complains about missing "Return" section for many documented
> > functions in the regulator core. Many of them actually have descriptions
> > about the return values, just not in the format kernel-doc wants.
> >
> > Convert these to use the proper "Return:" section header. The existing
> > descriptions have been reworded and moved around to fit the grammar and
> > formatting.
> >
> > In a few cases where the functions don't call even more functions
> > and the error numbers are known, those are documented in detail.
>
> ...
>
> > + * Return: pointer the &struct device_node corresponding to the regulator if found,
>
> "pointer to the"
> Same elsewhere.

Ack.

> > + *      or %NULL if not found.
>
> ...
>
> > + * Return: pointer to a &struct regulator corresponding to the regulator
> > + *      producer, or ERR_PTR() encoded negative error number.
>
> (I'm not sure of definite vs. indefinite article, though. Perhaps you need to
> consult with native speaker.)

I think "a" makes more sense, because in the case of _regulator_get(),
the |struct regulator| consumer instances are allocated separately on
the fly for each call.

> ...
>
> > + *      producer, or ERR_PTR() encoded negative error number.
>
> Okay, maybe "negative error number" to be used everywhere (see previous email),
> the main point is a) to make it clear that it's negative, and b) be consistent
> with a term across the subsystem.

Ack.


Thanks
ChenYu
Mark Brown Aug. 28, 2024, 8:29 p.m. UTC | #3
On Wed, Aug 28, 2024 at 04:01:52PM +0800, Chen-Yu Tsai wrote:
> On Tue, Aug 27, 2024 at 10:42 PM Andy Shevchenko
> > On Tue, Aug 27, 2024 at 05:55:43PM +0800, Chen-Yu Tsai wrote:

> > > + * Return: pointer to a &struct regulator corresponding to the regulator
> > > + *      producer, or ERR_PTR() encoded negative error number.

> > (I'm not sure of definite vs. indefinite article, though. Perhaps you need to
> > consult with native speaker.)

> I think "a" makes more sense, because in the case of _regulator_get(),
> the |struct regulator| consumer instances are allocated separately on
> the fly for each call.

Your text is perfectly fine and completely intelligible.
diff mbox series

Patch

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 3a1b6fd9780d..b1950cbc046a 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -427,8 +427,9 @@  static void regulator_lock_dependent(struct regulator_dev *rdev,
  *
  * Traverse all child nodes.
  * Extract the child regulator device node corresponding to the supply name.
- * returns the device node corresponding to the regulator if found, else
- * returns NULL.
+ *
+ * Return: pointer the &struct device_node corresponding to the regulator if found,
+ *	   or %NULL if not found.
  */
 static struct device_node *of_get_child_regulator(struct device_node *parent,
 						  const char *prop_name)
@@ -460,8 +461,9 @@  static struct device_node *of_get_child_regulator(struct device_node *parent,
  * @supply: regulator supply name
  *
  * Extract the regulator device node corresponding to the supply name.
- * returns the device node corresponding to the regulator if found, else
- * returns NULL.
+ *
+ * Return: pointer the &struct device_node corresponding to the regulator if found,
+ *	   or %NULL if not found.
  */
 static struct device_node *of_get_regulator(struct device *dev, const char *supply)
 {
@@ -2308,13 +2310,13 @@  struct regulator *_regulator_get(struct device *dev, const char *id,
  * @dev: device for regulator "consumer"
  * @id: Supply name or regulator ID.
  *
- * Returns a struct regulator corresponding to the regulator producer,
- * or IS_ERR() condition containing errno.
- *
  * Use of supply names configured via set_consumer_device_supply() is
  * strongly encouraged.  It is recommended that the supply name used
  * should match the name used for the supply and/or the relevant
  * device pins in the datasheet.
+ *
+ * Return: pointer to a &struct regulator corresponding to the regulator
+ *	   producer, or ERR_PTR() encoded negative error number.
  */
 struct regulator *regulator_get(struct device *dev, const char *id)
 {
@@ -2327,11 +2329,9 @@  EXPORT_SYMBOL_GPL(regulator_get);
  * @dev: device for regulator "consumer"
  * @id: Supply name or regulator ID.
  *
- * Returns a struct regulator corresponding to the regulator producer,
- * or IS_ERR() condition containing errno.  Other consumers will be
- * unable to obtain this regulator while this reference is held and the
- * use count for the regulator will be initialised to reflect the current
- * state of the regulator.
+ * Other consumers will be unable to obtain this regulator while this
+ * reference is held and the use count for the regulator will be
+ * initialised to reflect the current state of the regulator.
  *
  * This is intended for use by consumers which cannot tolerate shared
  * use of the regulator such as those which need to force the
@@ -2342,6 +2342,9 @@  EXPORT_SYMBOL_GPL(regulator_get);
  * strongly encouraged.  It is recommended that the supply name used
  * should match the name used for the supply and/or the relevant
  * device pins in the datasheet.
+ *
+ * Return: pointer to a &struct regulator corresponding to the regulator
+ *	   producer, or ERR_PTR() encoded negative error number.
  */
 struct regulator *regulator_get_exclusive(struct device *dev, const char *id)
 {
@@ -2354,9 +2357,6 @@  EXPORT_SYMBOL_GPL(regulator_get_exclusive);
  * @dev: device for regulator "consumer"
  * @id: Supply name or regulator ID.
  *
- * Returns a struct regulator corresponding to the regulator producer,
- * or IS_ERR() condition containing errno.
- *
  * This is intended for use by consumers for devices which can have
  * some supplies unconnected in normal use, such as some MMC devices.
  * It can allow the regulator core to provide stub supplies for other
@@ -2368,6 +2368,9 @@  EXPORT_SYMBOL_GPL(regulator_get_exclusive);
  * strongly encouraged.  It is recommended that the supply name used
  * should match the name used for the supply and/or the relevant
  * device pins in the datasheet.
+ *
+ * Return: pointer to a &struct regulator corresponding to the regulator
+ *	   producer, or ERR_PTR() encoded negative error number.
  */
 struct regulator *regulator_get_optional(struct device *dev, const char *id)
 {
@@ -2507,12 +2510,12 @@  EXPORT_SYMBOL_GPL(regulator_unregister_supply_alias);
  * lookup the supply
  * @num_id: Number of aliases to register
  *
- * @return 0 on success, an errno on failure.
- *
  * This helper function allows drivers to register several supply
  * aliases in one operation.  If any of the aliases cannot be
  * registered any aliases that were registered will be removed
  * before returning to the caller.
+ *
+ * Return: 0 on success or negative error number on failure.
  */
 int regulator_bulk_register_supply_alias(struct device *dev,
 					 const char *const *id,
@@ -2837,7 +2840,7 @@  static int _regulator_do_enable(struct regulator_dev *rdev)
  * responsible for keeping track of the refcount for a given regulator consumer
  * and applying / unapplying these things.
  *
- * Returns 0 upon no error; -error upon error.
+ * Return: 0 on success or negative error number on failure.
  */
 static int _regulator_handle_consumer_enable(struct regulator *regulator)
 {
@@ -2863,7 +2866,7 @@  static int _regulator_handle_consumer_enable(struct regulator *regulator)
  *
  * The opposite of _regulator_handle_consumer_enable().
  *
- * Returns 0 upon no error; -error upon error.
+ * Return: 0 on success or negative error number on failure.
  */
 static int _regulator_handle_consumer_disable(struct regulator *regulator)
 {
@@ -3271,13 +3274,13 @@  static int _regulator_list_voltage(struct regulator_dev *rdev,
  * regulator_is_enabled - is the regulator output enabled
  * @regulator: regulator source
  *
- * Returns positive if the regulator driver backing the source/client
- * has requested that the device be enabled, zero if it hasn't, else a
- * negative errno code.
- *
  * Note that the device backing this regulator handle can have multiple
  * users, so it might be enabled even if regulator_enable() was never
  * called for this particular source.
+ *
+ * Return: positive if the regulator driver backing the source/client
+ * has requested that the device be enabled, zero if it hasn't, else a
+ * negative error number.
  */
 int regulator_is_enabled(struct regulator *regulator)
 {
@@ -3298,9 +3301,10 @@  EXPORT_SYMBOL_GPL(regulator_is_enabled);
  * regulator_count_voltages - count regulator_list_voltage() selectors
  * @regulator: regulator source
  *
- * Returns number of selectors, or negative errno.  Selectors are
- * numbered starting at zero, and typically correspond to bitfields
- * in hardware registers.
+ * Return: number of selectors for @regulator, or negative error number.
+ *
+ * Selectors are numbered starting at zero, and typically correspond to
+ * bitfields in hardware registers.
  */
 int regulator_count_voltages(struct regulator *regulator)
 {
@@ -3322,9 +3326,9 @@  EXPORT_SYMBOL_GPL(regulator_count_voltages);
  * @selector: identify voltage to list
  * Context: can sleep
  *
- * Returns a voltage that can be passed to @regulator_set_voltage(),
- * zero if this selector code can't be used on this system, or a
- * negative errno.
+ * Return: voltage for @selector that can be passed to regulator_set_voltage(),
+ *	   0 if @selector can't be used on this system, or a negative error
+ *	   number on failure.
  */
 int regulator_list_voltage(struct regulator *regulator, unsigned selector)
 {
@@ -3336,8 +3340,8 @@  EXPORT_SYMBOL_GPL(regulator_list_voltage);
  * regulator_get_regmap - get the regulator's register map
  * @regulator: regulator source
  *
- * Returns the register map for the given regulator, or an ERR_PTR value
- * if the regulator doesn't use regmap.
+ * Return: pointer to &struct regmap for @regulator, or ERR_PTR()
+ *	   encoded -%EOPNOTSUPP if @regulator doesn't use regmap.
  */
 struct regmap *regulator_get_regmap(struct regulator *regulator)
 {
@@ -3387,7 +3391,9 @@  EXPORT_SYMBOL_GPL(regulator_get_hardware_vsel_register);
  * directly written to the regulator registers. The address of the voltage
  * register can be determined by calling @regulator_get_hardware_vsel_register.
  *
- * On error a negative errno is returned.
+ * Return: 0 on success, -%EINVAL if the selector is outside the supported
+ *	   range, or -%EOPNOTSUPP if the regulator does not support voltage
+ *	   selectors.
  */
 int regulator_list_hardware_vsel(struct regulator *regulator,
 				 unsigned selector)
@@ -3414,7 +3420,7 @@  EXPORT_SYMBOL_GPL(regulator_list_hardware_vsel);
  * Request that the regulator be enabled/disabled with the regulator output at
  * the predefined voltage or current value.
  *
- * On success 0 is returned, otherwise a negative errno is returned.
+ * Return: 0 on success or negative error number on failure.
  */
 int regulator_hardware_enable(struct regulator *regulator, bool enable)
 {
@@ -3438,8 +3444,8 @@  EXPORT_SYMBOL_GPL(regulator_hardware_enable);
  * regulator_get_linear_step - return the voltage step size between VSEL values
  * @regulator: regulator source
  *
- * Returns the voltage step size between VSEL values for linear
- * regulators, or return 0 if the regulator isn't a linear regulator.
+ * Return: the voltage step size between VSEL values for linear regulators,
+ *	   or 0 if the regulator isn't a linear regulator.
  */
 unsigned int regulator_get_linear_step(struct regulator *regulator)
 {
@@ -4526,7 +4532,7 @@  EXPORT_SYMBOL_GPL(regulator_get_voltage_rdev);
  * regulator_get_voltage - get regulator output voltage
  * @regulator: regulator source
  *
- * This returns the current regulator voltage in uV.
+ * Return: current regulator voltage in uV, or negative error number on failure.
  *
  * NOTE: If the regulator is disabled it will return the voltage value. This
  * function should not be used to determine regulator state.
@@ -4610,7 +4616,8 @@  static int _regulator_get_current_limit(struct regulator_dev *rdev)
  * regulator_get_current_limit - get regulator output current
  * @regulator: regulator source
  *
- * This returns the current supplied by the specified current sink in uA.
+ * Return: current supplied by the specified current sink in uA,
+ *	   or negative error number on failure.
  *
  * NOTE: If the regulator is disabled it will return the current value. This
  * function should not be used to determine regulator state.
@@ -4778,7 +4785,7 @@  EXPORT_SYMBOL_GPL(regulator_get_error_flags);
  * If a regulator is an always-on regulator then an individual consumer's
  * load will still be removed if that consumer is fully disabled.
  *
- * On error a negative errno is returned.
+ * Return: 0 on success or negative error number on failure.
  */
 int regulator_set_load(struct regulator *regulator, int uA_load)
 {
@@ -4963,12 +4970,12 @@  int _regulator_bulk_get(struct device *dev, int num_consumers,
  * @num_consumers: Number of consumers to register
  * @consumers:     Configuration of consumers; clients are stored here.
  *
- * @return 0 on success, an errno on failure.
- *
  * This helper function allows drivers to get several regulator
  * consumers in one operation.  If any of the regulators cannot be
  * acquired then any regulators that were allocated will be freed
  * before returning to the caller.
+ *
+ * Return: 0 on success or negative error number on failure.
  */
 int regulator_bulk_get(struct device *dev, int num_consumers,
 		       struct regulator_bulk_data *consumers)
@@ -4989,12 +4996,13 @@  static void regulator_bulk_enable_async(void *data, async_cookie_t cookie)
  *
  * @num_consumers: Number of consumers
  * @consumers:     Consumer data; clients are stored here.
- * @return         0 on success, an errno on failure
  *
  * This convenience API allows consumers to enable multiple regulator
  * clients in a single API call.  If any consumers cannot be enabled
  * then any others that were enabled will be disabled again prior to
  * return.
+ *
+ * Return: 0 on success or negative error number on failure.
  */
 int regulator_bulk_enable(int num_consumers,
 			  struct regulator_bulk_data *consumers)
@@ -5038,12 +5046,13 @@  EXPORT_SYMBOL_GPL(regulator_bulk_enable);
  *
  * @num_consumers: Number of consumers
  * @consumers:     Consumer data; clients are stored here.
- * @return         0 on success, an errno on failure
  *
  * This convenience API allows consumers to disable multiple regulator
  * clients in a single API call.  If any consumers cannot be disabled
  * then any others that were disabled will be enabled again prior to
  * return.
+ *
+ * Return: 0 on success or negative error number on failure.
  */
 int regulator_bulk_disable(int num_consumers,
 			   struct regulator_bulk_data *consumers)
@@ -5077,7 +5086,6 @@  EXPORT_SYMBOL_GPL(regulator_bulk_disable);
  *
  * @num_consumers: Number of consumers
  * @consumers:     Consumer data; clients are stored here.
- * @return         0 on success, an errno on failure
  *
  * This convenience API allows consumers to forcibly disable multiple regulator
  * clients in a single API call.
@@ -5085,6 +5093,8 @@  EXPORT_SYMBOL_GPL(regulator_bulk_disable);
  * likely occur if the regulators are not disabled (e.g. over temp).
  * Although regulator_force_disable function call for some consumers can
  * return error numbers, the function is called for all consumers.
+ *
+ * Return: 0 on success or negative error number on failure.
  */
 int regulator_bulk_force_disable(int num_consumers,
 			   struct regulator_bulk_data *consumers)
@@ -5581,8 +5591,9 @@  static struct regulator_coupler generic_regulator_coupler = {
  * @cfg: runtime configuration for regulator
  *
  * Called by regulator drivers to register a regulator.
- * Returns a valid pointer to struct regulator_dev on success
- * or an ERR_PTR() on error.
+ *
+ * Return: pointer to valid &struct regulator_dev on success
+ *	   or ERR_PTR() encoded negative error number on failure.
  */
 struct regulator_dev *
 regulator_register(struct device *dev,