public inbox for linux-staging@lists.linux.dev
 help / color / mirror / Atom feed
From: Jaehee Park <jhpark1013@gmail.com>
To: Alex Elder <elder@ieee.org>
Cc: johan@kernel.org, elder@kernel.org, gregkh@linuxfoundation.org,
	greybus-dev@lists.linaro.org, linux-staging@lists.linux.dev,
	linux-kernel@vger.kernel.org, outreachy@lists.linux.dev
Subject: Re: [PATCH v2 2/2] staging: greybus: remove unneeded return
Date: Wed, 13 Apr 2022 00:43:34 -0400	[thread overview]
Message-ID: <20220413044334.GA3357359@jaehee-ThinkPad-X1-Extreme> (raw)
In-Reply-To: <a509099e-2b19-7a27-693c-b418bd2f7b28@ieee.org>

On Tue, Apr 12, 2022 at 03:35:04PM -0500, Alex Elder wrote:
> On 4/12/22 2:59 PM, Jaehee Park wrote:
> > An empty function with void return type does not need an explicit
> > return. Issue found by checkpatch.
> > 
> > Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
> 
> Dan's suggestion here was to simply remove this function
> entirely.  It is only used as the ->remove callback
> for the soc_codec_dev_gbaudio structure.
> 
> You can see that soc_codec_dev_gbaudio is only used in the
> call to devm_snd_soc_register_component() near the end of
> "audio_codec.c".  When a sound component is registered
> that way, the ->remove callback is optional.  You can see
> that because the only place in "sound/soc/soc-component.c"
> that it is referenced is snd_soc_component_remove() (as
> Dan said), and it only calls the function if it the pointer
> is non-null.  Allowing null function pointers in places
> like this. to allow them to be optionally omitted is not
> an uncommon pattern you'll see in the kernel.
> 
> Anyway, please don't just add another small patch to remove
> the function.  Just replace *this* patch with one that
> removes the function, and omits the assignment if its
> address to soc_codec_dev_gbaudio->remove.
> 
> 					-Alex

Hi Alex, Thank you for explaining where the functions are called from!
It makes a lot more sense now. I've used your explanation for my patch
log. Please let me know if I'm misunderstanding things. I've sent patch 
v3 for your review. The first patch (typo patch) already has your 
"review-by" -- thank you for the advice.
 
Thanks,
Jaehee
> 
> > ---
> >   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,
> 

  reply	other threads:[~2022-04-13  4:43 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-12 19:59 [PATCH v2 0/2] staging: greybus: fix warnings reported by checkpatch Jaehee Park
2022-04-12 19:59 ` [PATCH v2 1/2] staging: greybus: correct typo in comment Jaehee Park
2022-04-12 20:38   ` Alex Elder
2022-04-12 19:59 ` [PATCH v2 2/2] staging: greybus: remove unneeded return Jaehee Park
2022-04-12 20:35   ` Alex Elder
2022-04-13  4:43     ` Jaehee Park [this message]
2022-04-12 20:35   ` Fabio M. De Francesco
2022-04-13  4:44     ` Jaehee Park
2022-04-13  6:16     ` Julia Lawall
2022-04-13  6:35       ` Fabio M. De Francesco

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=20220413044334.GA3357359@jaehee-ThinkPad-X1-Extreme \
    --to=jhpark1013@gmail.com \
    --cc=elder@ieee.org \
    --cc=elder@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=greybus-dev@lists.linaro.org \
    --cc=johan@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=outreachy@lists.linux.dev \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox