[PATCH 0/2] staging: greybus: fix warnings reported by checkpatch

[PATCH 0/2] staging: greybus: fix warnings reported by checkpatch

[Jaehee Park]

The first patch corrects a typo in a comment. The second patch fixes 
'void function return statements are not generally useful' warning.

Jaehee Park (2):
  staging: greybus: correct typo in comment 'Atleast' to 'At least'
  staging: greybus: remove return in an empty void function

 drivers/staging/greybus/arche-apb-ctrl.c | 2 +-
 drivers/staging/greybus/audio_codec.c    | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

-- 
2.25.1

[PATCH 1/2] staging: greybus: correct typo in comment 'Atleast' to 'At least'

[Jaehee Park]

Correct spelling typo.

Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
---
 drivers/staging/greybus/arche-apb-ctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/greybus/arche-apb-ctrl.c b/drivers/staging/greybus/arche-apb-ctrl.c
index bbf3ba744fc4..45afa208d004 100644
--- a/drivers/staging/greybus/arche-apb-ctrl.c
+++ b/drivers/staging/greybus/arche-apb-ctrl.c
@@ -445,7 +445,7 @@ static int __maybe_unused arche_apb_ctrl_suspend(struct device *dev)
 static int __maybe_unused arche_apb_ctrl_resume(struct device *dev)
 {
 	/*
-	 * Atleast for ES2 we have to meet the delay requirement between
+	 * At least for ES2 we have to meet the delay requirement between
 	 * unipro switch and AP bridge init, depending on whether bridge is in
 	 * OFF state or standby state.
 	 *
-- 
2.25.1

[PATCH 2/2] staging: greybus: remove return in an empty void function

[Jaehee Park]

Issue found by checkpatch:
WARNING: void function return statements are not generally useful

Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
---
 drivers/staging/greybus/audio_codec.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c
index 0f50d1e51e2c..3e3a16568def 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -1032,7 +1032,6 @@ static int gbcodec_probe(struct snd_soc_component *comp)
 static void gbcodec_remove(struct snd_soc_component *comp)
 {
 	/* Empty function for now */
-	return;
 }
 
 static int gbcodec_write(struct snd_soc_component *comp, unsigned int reg,
-- 
2.25.1

Re: [PATCH 0/2] staging: greybus: fix warnings reported by checkpatch

[Alex Elder]

On 4/11/22 8:50 PM, Jaehee Park wrote:
> The first patch corrects a typo in a comment. The second patch fixes
> 'void function return statements are not generally useful' warning.
> 
> Jaehee Park (2):
>    staging: greybus: correct typo in comment 'Atleast' to 'At least'
>    staging: greybus: remove return in an empty void function
> 
>   drivers/staging/greybus/arche-apb-ctrl.c | 2 +-
>   drivers/staging/greybus/audio_codec.c    | 1 -
>   2 files changed, 1 insertion(+), 2 deletions(-)
> 

These are really trivial patches.  Whether the second
one is even worth changing is questionable (it represents
"code churn" without really adding value).

That said, you've properly submitted the patches and I
have no problem with either one.  So someone else can
object, but if they do not:

Reviewed-by: Alex Elder <elder@linaro.org>

Re: [PATCH 2/2] staging: greybus: remove return in an empty void function

[Julia Lawall]

On Mon, 11 Apr 2022, Jaehee Park wrote:

> Issue found by checkpatch:
> WARNING: void function return statements are not generally useful

Even for a simple change, it wouldbe better to follow the model that the
subject line should concisely orient the reader (eg "remove unneeded
return") and the log message should give a more complete description.
Simply repeating what checkpatch tells you is rarely a good idea.  It says
what the problem is in a general way, whereas you should be explaining the
reasoning behind your fix in a specific case.  Here you could say that an
empty function with void return type does not need an explicit return, and
that the problem was detected with checkpatch.

julia

>
> Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
> ---
>  drivers/staging/greybus/audio_codec.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c
> index 0f50d1e51e2c..3e3a16568def 100644
> --- a/drivers/staging/greybus/audio_codec.c
> +++ b/drivers/staging/greybus/audio_codec.c
> @@ -1032,7 +1032,6 @@ static int gbcodec_probe(struct snd_soc_component *comp)
>  static void gbcodec_remove(struct snd_soc_component *comp)
>  {
>  	/* Empty function for now */
> -	return;
>  }
>
>  static int gbcodec_write(struct snd_soc_component *comp, unsigned int reg,
> --
> 2.25.1
>
>
>

Re: [PATCH 2/2] staging: greybus: remove return in an empty void function

[Dan Carpenter]

On Mon, Apr 11, 2022 at 09:50:22PM -0400, Jaehee Park wrote:
> Issue found by checkpatch:
> WARNING: void function return statements are not generally useful
> 
> Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
> ---
>  drivers/staging/greybus/audio_codec.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c
> index 0f50d1e51e2c..3e3a16568def 100644
> --- a/drivers/staging/greybus/audio_codec.c
> +++ b/drivers/staging/greybus/audio_codec.c
> @@ -1032,7 +1032,6 @@ static int gbcodec_probe(struct snd_soc_component *comp)
>  static void gbcodec_remove(struct snd_soc_component *comp)
>  {
>  	/* Empty function for now */
> -	return;
>  }

This function is called from snd_soc_component_remove().  You can safely
remove the whole function.  We do not like empty stub functions.

regards,
dan carpenter

Re: [PATCH 2/2] staging: greybus: remove return in an empty void function

[Jaehee Park]

On Tue, Apr 12, 2022 at 09:04:08AM +0200, Julia Lawall wrote:
> 
> 
> On Mon, 11 Apr 2022, Jaehee Park wrote:
> 
> > Issue found by checkpatch:
> > WARNING: void function return statements are not generally useful
> 
> Even for a simple change, it wouldbe better to follow the model that the
> subject line should concisely orient the reader (eg "remove unneeded
> return") and the log message should give a more complete description.
> Simply repeating what checkpatch tells you is rarely a good idea.  It says
> what the problem is in a general way, whereas you should be explaining the
> reasoning behind your fix in a specific case.  Here you could say that an
> empty function with void return type does not need an explicit return, and
> that the problem was detected with checkpatch.
> 
> julia
> 

Thank you for your edits. I've submitted version 2 of the patchset just
now.

> >
> > Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
> > ---
> >  drivers/staging/greybus/audio_codec.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c
> > index 0f50d1e51e2c..3e3a16568def 100644
> > --- a/drivers/staging/greybus/audio_codec.c
> > +++ b/drivers/staging/greybus/audio_codec.c
> > @@ -1032,7 +1032,6 @@ static int gbcodec_probe(struct snd_soc_component *comp)
> >  static void gbcodec_remove(struct snd_soc_component *comp)
> >  {
> >  	/* Empty function for now */
> > -	return;
> >  }
> >
> >  static int gbcodec_write(struct snd_soc_component *comp, unsigned int reg,
> > --
> > 2.25.1
> >
> >
> >

Re: [PATCH 2/2] staging: greybus: remove return in an empty void function

[Jaehee Park]

On Tue, Apr 12, 2022 at 10:28:43AM +0300, Dan Carpenter wrote:
> On Mon, Apr 11, 2022 at 09:50:22PM -0400, Jaehee Park wrote:
> > Issue found by checkpatch:
> > WARNING: void function return statements are not generally useful
> > 
> > Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
> > ---
> >  drivers/staging/greybus/audio_codec.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c
> > index 0f50d1e51e2c..3e3a16568def 100644
> > --- a/drivers/staging/greybus/audio_codec.c
> > +++ b/drivers/staging/greybus/audio_codec.c
> > @@ -1032,7 +1032,6 @@ static int gbcodec_probe(struct snd_soc_component *comp)
> >  static void gbcodec_remove(struct snd_soc_component *comp)
> >  {
> >  	/* Empty function for now */
> > -	return;
> >  }
> 
> This function is called from snd_soc_component_remove().  You can safely
> remove the whole function.  We do not like empty stub functions.

I'll make this into another patch. Thank you for your suggestion.

> 
> regards,
> dan carpenter
>