diff mbox series

[v2] thermal: devfreq_cooling: Fix kernel-doc

Message ID 1623223350-128104-1-git-send-email-yang.lee@linux.alibaba.com (mailing list archive)
State New
Delegated to: Daniel Lezcano
Headers show
Series [v2] thermal: devfreq_cooling: Fix kernel-doc | expand

Commit Message

Yang Li June 9, 2021, 7:22 a.m. UTC
Fix function name in devfreq_cooling.c comment to remove a 
warning found by kernel-doc.

drivers/thermal/devfreq_cooling.c:479: warning: expecting prototype for
devfreq_cooling_em_register_power(). Prototype was for
devfreq_cooling_em_register() instead.

Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
---

Change in v2:
--replaced s/clang(make W=1 LLVM=1)/kernel-doc/ in commit.
https://lore.kernel.org/patchwork/patch/1442639/

 drivers/thermal/devfreq_cooling.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Nick Desaulniers June 9, 2021, 6:02 p.m. UTC | #1
On Wed, Jun 9, 2021 at 12:22 AM Yang Li <yang.lee@linux.alibaba.com> wrote:
>
> Fix function name in devfreq_cooling.c comment to remove a
> warning found by kernel-doc.
>
> drivers/thermal/devfreq_cooling.c:479: warning: expecting prototype for
> devfreq_cooling_em_register_power(). Prototype was for
> devfreq_cooling_em_register() instead.
>
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>

I'm ok with leaving my reviewed by on _this_ patch because it's so simple but...

In general, when sending a follow up version of a patch, it's _not_ ok
to add a reviewed by tag when a reviewer has not explicitly responded
with "Reviewed-by: ...".  That provides a false sense that a patch has
been thoroughly reviewed.  Responding to a patch does not constitute a
"Reviewed-by:" tag.

And I might be fine with _this_ patch, but that says nothing about
Nathan, whom you've also falsely attributed a reviewed by tag here.

For such a trivial patch, it's not a big deal, but in the future
please do not do that again.  It's ok to send v2, v3, etc, but wait
for reviewers to explicitly state such reviewed by tag. The maintainer
will collect those responses (and can be done so in an automated
fashion via a tool like b4 (https://pypi.org/project/b4/)) when
applying patches.

> ---
>
> Change in v2:
> --replaced s/clang(make W=1 LLVM=1)/kernel-doc/ in commit.
> https://lore.kernel.org/patchwork/patch/1442639/
>
>  drivers/thermal/devfreq_cooling.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c
> index 3a788ac..5a86cff 100644
> --- a/drivers/thermal/devfreq_cooling.c
> +++ b/drivers/thermal/devfreq_cooling.c
> @@ -458,7 +458,7 @@ struct thermal_cooling_device *devfreq_cooling_register(struct devfreq *df)
>  EXPORT_SYMBOL_GPL(devfreq_cooling_register);
>
>  /**
> - * devfreq_cooling_em_register_power() - Register devfreq cooling device with
> + * devfreq_cooling_em_register() - Register devfreq cooling device with
>   *             power information and automatically register Energy Model (EM)
>   * @df:                Pointer to devfreq device.
>   * @dfc_power: Pointer to devfreq_cooling_power.
> --
> 1.8.3.1
>
Nathan Chancellor June 9, 2021, 6:15 p.m. UTC | #2
On 6/9/2021 11:02 AM, Nick Desaulniers wrote:
> On Wed, Jun 9, 2021 at 12:22 AM Yang Li <yang.lee@linux.alibaba.com> wrote:
>>
>> Fix function name in devfreq_cooling.c comment to remove a
>> warning found by kernel-doc.
>>
>> drivers/thermal/devfreq_cooling.c:479: warning: expecting prototype for
>> devfreq_cooling_em_register_power(). Prototype was for
>> devfreq_cooling_em_register() instead.
>>
>> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
>> Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
>> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> 
> I'm ok with leaving my reviewed by on _this_ patch because it's so simple but...
> 
> In general, when sending a follow up version of a patch, it's _not_ ok
> to add a reviewed by tag when a reviewer has not explicitly responded
> with "Reviewed-by: ...".  That provides a false sense that a patch has
> been thoroughly reviewed.  Responding to a patch does not constitute a
> "Reviewed-by:" tag.
> 
> And I might be fine with _this_ patch, but that says nothing about
> Nathan, whom you've also falsely attributed a reviewed by tag here.
> 
> For such a trivial patch, it's not a big deal, but in the future
> please do not do that again.  It's ok to send v2, v3, etc, but wait
> for reviewers to explicitly state such reviewed by tag. The maintainer
> will collect those responses (and can be done so in an automated
> fashion via a tool like b4 (https://pypi.org/project/b4/)) when
> applying patches.

+1 with all that was said above. Tags should be explicitly given, except 
for maybe the "Reported-by" and "Suggested-by" tags if the report or 
suggestion was done in the public forum but it is still polite to ask if 
it is okay to add.

For the record, my reviewed-by tag can stand:

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

>> ---
>>
>> Change in v2:
>> --replaced s/clang(make W=1 LLVM=1)/kernel-doc/ in commit.
>> https://lore.kernel.org/patchwork/patch/1442639/
>>
>>   drivers/thermal/devfreq_cooling.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c
>> index 3a788ac..5a86cff 100644
>> --- a/drivers/thermal/devfreq_cooling.c
>> +++ b/drivers/thermal/devfreq_cooling.c
>> @@ -458,7 +458,7 @@ struct thermal_cooling_device *devfreq_cooling_register(struct devfreq *df)
>>   EXPORT_SYMBOL_GPL(devfreq_cooling_register);
>>
>>   /**
>> - * devfreq_cooling_em_register_power() - Register devfreq cooling device with
>> + * devfreq_cooling_em_register() - Register devfreq cooling device with
>>    *             power information and automatically register Energy Model (EM)
>>    * @df:                Pointer to devfreq device.
>>    * @dfc_power: Pointer to devfreq_cooling_power.
>> --
>> 1.8.3.1
>>
> 
>
Lukasz Luba June 11, 2021, 2:46 p.m. UTC | #3
On 6/9/21 8:22 AM, Yang Li wrote:
> Fix function name in devfreq_cooling.c comment to remove a
> warning found by kernel-doc.
> 
> drivers/thermal/devfreq_cooling.c:479: warning: expecting prototype for
> devfreq_cooling_em_register_power(). Prototype was for
> devfreq_cooling_em_register() instead.
> 
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>

Everything was said regarding these Reviewed-by tags
and you got them in this thread, so should be OK
Thank you Nathan and Nick for explaining this and sorting
out.

Small hint for next time, the Signed-off-by should be at the bottom of
that list.


> ---
> 
> Change in v2:
> --replaced s/clang(make W=1 LLVM=1)/kernel-doc/ in commit.
> https://lore.kernel.org/patchwork/patch/1442639/
> 
>   drivers/thermal/devfreq_cooling.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c
> index 3a788ac..5a86cff 100644
> --- a/drivers/thermal/devfreq_cooling.c
> +++ b/drivers/thermal/devfreq_cooling.c
> @@ -458,7 +458,7 @@ struct thermal_cooling_device *devfreq_cooling_register(struct devfreq *df)
>   EXPORT_SYMBOL_GPL(devfreq_cooling_register);
>   
>   /**
> - * devfreq_cooling_em_register_power() - Register devfreq cooling device with
> + * devfreq_cooling_em_register() - Register devfreq cooling device with
>    *		power information and automatically register Energy Model (EM)
>    * @df:		Pointer to devfreq device.
>    * @dfc_power:	Pointer to devfreq_cooling_power.
> 

Apart from that, the change itself is OK. I forgot about this comment
function name in some iteration... my apologies for that.

LGTM

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

Regards,
Lukasz Luba
diff mbox series

Patch

diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c
index 3a788ac..5a86cff 100644
--- a/drivers/thermal/devfreq_cooling.c
+++ b/drivers/thermal/devfreq_cooling.c
@@ -458,7 +458,7 @@  struct thermal_cooling_device *devfreq_cooling_register(struct devfreq *df)
 EXPORT_SYMBOL_GPL(devfreq_cooling_register);
 
 /**
- * devfreq_cooling_em_register_power() - Register devfreq cooling device with
+ * devfreq_cooling_em_register() - Register devfreq cooling device with
  *		power information and automatically register Energy Model (EM)
  * @df:		Pointer to devfreq device.
  * @dfc_power:	Pointer to devfreq_cooling_power.