| Message ID | 20240827095550.675018-9-wenst@chromium.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | regulator: kerneldoc section fixes | expand |
On Tue, Aug 27, 2024 at 05:55:48PM +0800, Chen-Yu Tsai wrote: > kernel-doc complains about missing "Return" section for the function > regulator_irq_map_event_simple(). > > Add a "Return" section for it based on its behavior. ... > + * Return: 0 "0." > + * I don't think we need this blank line. > + * Actual regulator error and notification are passed back through @rid.
On 8/27/24 12:55, Chen-Yu Tsai wrote: > kernel-doc complains about missing "Return" section for the function > regulator_irq_map_event_simple(). > > Add a "Return" section for it based on its behavior. > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Thank You for improving this! I appreciate it :) > --- > drivers/regulator/irq_helpers.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/regulator/irq_helpers.c b/drivers/regulator/irq_helpers.c > index 5ab1a0befe12..5803ef016b7d 100644 > --- a/drivers/regulator/irq_helpers.c > +++ b/drivers/regulator/irq_helpers.c > @@ -414,6 +414,10 @@ EXPORT_SYMBOL_GPL(regulator_irq_helper_cancel); > * notification helperk. Exactly one rdev and exactly one error (in I just noticed (an existing) typo "helperk". I wonder if it was Ok to fix it while anyways changing the doc. It's not strictly speaking related to the return values though :) > * "common_errs"-field) can be given at IRQ helper registration for > * regulator_irq_map_event_simple() to be viable. > + * > + * Return: 0 > + * Anyways, I agree with Andy about not needing the blank line here - but other than that: Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com> > + * Actual regulator error and notification are passed back through @rid. > */ > int regulator_irq_map_event_simple(int irq, struct regulator_irq_data *rid, > unsigned long *dev_mask)
On Tue, Aug 27, 2024 at 10:44 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Aug 27, 2024 at 05:55:48PM +0800, Chen-Yu Tsai wrote: > > kernel-doc complains about missing "Return" section for the function > > regulator_irq_map_event_simple(). > > > > Add a "Return" section for it based on its behavior. > > ... > > > + * Return: 0 > > "0." Ack. > > + * > > I don't think we need this blank line. This actually changes the output. Without the blank line, they are treated as the same paragraph. With the blank line, the next line is treated as a separate paragraph, and put in the "Description" section. Strictly speaking, the only return value is the 0 integer. The other "return" values are output parameters that have been modified by the function. I believe those should not be in the "Return" section. ChenYu > > + * Actual regulator error and notification are passed back through @rid. > > -- > With Best Regards, > Andy Shevchenko > >
On Wed, Aug 28, 2024 at 1:55 PM Matti Vaittinen <mazziesaccount@gmail.com> wrote: > > On 8/27/24 12:55, Chen-Yu Tsai wrote: > > kernel-doc complains about missing "Return" section for the function > > regulator_irq_map_event_simple(). > > > > Add a "Return" section for it based on its behavior. > > > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > > Thank You for improving this! I appreciate it :) > > > --- > > drivers/regulator/irq_helpers.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/regulator/irq_helpers.c b/drivers/regulator/irq_helpers.c > > index 5ab1a0befe12..5803ef016b7d 100644 > > --- a/drivers/regulator/irq_helpers.c > > +++ b/drivers/regulator/irq_helpers.c > > @@ -414,6 +414,10 @@ EXPORT_SYMBOL_GPL(regulator_irq_helper_cancel); > > * notification helperk. Exactly one rdev and exactly one error (in > > I just noticed (an existing) typo "helperk". I wonder if it was Ok to > fix it while anyways changing the doc. It's not strictly speaking > related to the return values though :) Why not? Only the kerneldoc this function is touched. It looks like the 'k' belongs to the "callbac" on the previous line. > > * "common_errs"-field) can be given at IRQ helper registration for > > * regulator_irq_map_event_simple() to be viable. > > + * > > + * Return: 0 > > + * > Anyways, I agree with Andy about not needing the blank line here - but I disagree because of the formatting result. See my other reply. > other than that: > > Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com> > > > + * Actual regulator error and notification are passed back through @rid. > > */ > > int regulator_irq_map_event_simple(int irq, struct regulator_irq_data *rid, > > unsigned long *dev_mask) > > -- > Matti Vaittinen > Linux kernel developer at ROHM Semiconductors > Oulu Finland > > ~~ When things go utterly wrong vim users can always type :help! ~~ >
ke 28. elok. 2024 klo 10.53 Chen-Yu Tsai (wenst@chromium.org) kirjoitti: > > On Wed, Aug 28, 2024 at 1:55 PM Matti Vaittinen > <mazziesaccount@gmail.com> wrote: > > > > On 8/27/24 12:55, Chen-Yu Tsai wrote: > > > kernel-doc complains about missing "Return" section for the function > > > regulator_irq_map_event_simple(). > > > > > > Add a "Return" section for it based on its behavior. > > > > > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > > > > Thank You for improving this! I appreciate it :) > > > > > --- > > > drivers/regulator/irq_helpers.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/drivers/regulator/irq_helpers.c b/drivers/regulator/irq_helpers.c > > > index 5ab1a0befe12..5803ef016b7d 100644 > > > --- a/drivers/regulator/irq_helpers.c > > > +++ b/drivers/regulator/irq_helpers.c > > > @@ -414,6 +414,10 @@ EXPORT_SYMBOL_GPL(regulator_irq_helper_cancel); > > > * notification helperk. Exactly one rdev and exactly one error (in > > > > I just noticed (an existing) typo "helperk". I wonder if it was Ok to > > fix it while anyways changing the doc. It's not strictly speaking > > related to the return values though :) > > Why not? Only the kerneldoc this function is touched. It looks like the > 'k' belongs to the "callbac" on the previous line. Right. It'd be nice if you fixed the typo as well if you for some reason re-spin this. > > > * "common_errs"-field) can be given at IRQ helper registration for > > > * regulator_irq_map_event_simple() to be viable. > > > + * > > > + * Return: 0 > > > + * > > Anyways, I agree with Andy about not needing the blank line here - but > > I disagree because of the formatting result. See my other reply. Fair enough. The "errors" or "notifications" indeed don't tell if this specific function has failed. So, if you feel the @rid is not documented well enough, maybe add a note in the respective parameter documentation instead of leaving it alone at the bottom of the doc? Well, I don't have a strong opinion on this so you can keep the RB tag also with the blank line. Yours, -- Matti
On Wed, Aug 28, 2024 at 03:50:55PM +0800, Chen-Yu Tsai wrote: > On Tue, Aug 27, 2024 at 10:44 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Aug 27, 2024 at 05:55:48PM +0800, Chen-Yu Tsai wrote: ... > > > + * Return: 0 > > > > "0." > > Ack. > > > > + * > > > > I don't think we need this blank line. > > This actually changes the output. Without the blank line, they are treated > as the same paragraph. With the blank line, the next line is treated as > a separate paragraph, and put in the "Description" section. I see, then please make sure that description either annotated with "Description:" or comes _before_ "Return:". that's why it confused me and I though it's related to the "Return:" section. > Strictly speaking, the only return value is the 0 integer. The other > "return" values are output parameters that have been modified by the > function. I believe those should not be in the "Return" section. > > > > + * Actual regulator error and notification are passed back through @rid.
diff --git a/drivers/regulator/irq_helpers.c b/drivers/regulator/irq_helpers.c index 5ab1a0befe12..5803ef016b7d 100644 --- a/drivers/regulator/irq_helpers.c +++ b/drivers/regulator/irq_helpers.c @@ -414,6 +414,10 @@ EXPORT_SYMBOL_GPL(regulator_irq_helper_cancel); * notification helperk. Exactly one rdev and exactly one error (in * "common_errs"-field) can be given at IRQ helper registration for * regulator_irq_map_event_simple() to be viable. + * + * Return: 0 + * + * Actual regulator error and notification are passed back through @rid. */ int regulator_irq_map_event_simple(int irq, struct regulator_irq_data *rid, unsigned long *dev_mask)
kernel-doc complains about missing "Return" section for the function regulator_irq_map_event_simple(). Add a "Return" section for it based on its behavior. Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> --- drivers/regulator/irq_helpers.c | 4 ++++ 1 file changed, 4 insertions(+)