From: natechancellor@gmail.com (Nathan Chancellor)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] staging: bcm2835-camera: Avoid unneeded internal declaration warning
Date: Fri, 28 Sep 2018 07:48:56 -0700 [thread overview]
Message-ID: <20180928144856.GA22524@flashbox> (raw)
In-Reply-To: <CAAoAYcMMiiDXCcnZ=DP9Y9Xs+xdq-jasj8G_W0jrnQV9so79=A@mail.gmail.com>
On Fri, Sep 28, 2018 at 10:04:29AM +0100, Dave Stevenson wrote:
> Hi Nate
>
> Thanks for the patch.
>
> On Fri, 28 Sep 2018 at 01:53, Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > Clang warns:
> >
> > drivers/staging/vc04_services/bcm2835-camera/controls.c:59:18: warning:
> > variable 'mains_freq_qmenu' is not needed and will not be emitted
> > [-Wunneeded-internal-declaration]
> > static const s64 mains_freq_qmenu[] = {
> > ^
> > 1 warning generated.
> >
> > This is because mains_freq_qmenu is currently only used in an ARRAY_SIZE
> > macro, which is a compile time evaluation in this case. Avoid this by
> > adding mains_freq_qmenu as the imenu member of this structure, which
> > matches all other controls that uses the ARRAY_SIZE macro in v4l2_ctrls.
> > This turns out to be a no-op because V4L2_CID_MPEG_VIDEO_BITRATE_MODE is
> > defined as a MMAL_CONTROL_TYPE_STD_MENU, which does not pass the imenu
> > definition along to v4l2_ctrl_new in bm2835_mmal_init_controls.
>
> There's a slight confusion betwen V4L2_CID_MPEG_VIDEO_BITRATE_MODE and
> V4L2_CID_POWER_LINE_FREQUENCY in your description.
>
> However you're right that MMAL_CONTROL_TYPE_STD_MENU calls
> v4l2_ctrl_new_std_menu which doesn't need a menu array (it's
> v4l2_ctrl_new_int_menu that does). That means the correct fix is to
> define the max value using the relevant enum (in this case
> V4L2_CID_POWER_LINE_FREQUENCY_AUTO) and remove the array.
>
> The same is true for V4L2_CID_MPEG_VIDEO_BITRATE_MODE - max should be
> V4L2_MPEG_VIDEO_BITRATE_MODE_CBR, and remove the bitrate_mode_qmenu
> array.
>
> Thanks,
> Dave
>
Hi Dave,
Thank you for the review, I appreciate it. I had certainly considered
that solution first butnfigured this was the more conservative change.
I will send a v2 soon with the suggested change.
Nathan
> > Link: https://github.com/ClangBuiltLinux/linux/issues/122
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> > drivers/staging/vc04_services/bcm2835-camera/controls.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > index cff7b1e07153..a2c55cb2192a 100644
> > --- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > +++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > @@ -1106,7 +1106,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
> > {
> > V4L2_CID_POWER_LINE_FREQUENCY, MMAL_CONTROL_TYPE_STD_MENU,
> > 0, ARRAY_SIZE(mains_freq_qmenu) - 1,
> > - 1, 1, NULL,
> > + 1, 1, mains_freq_qmenu,
> > MMAL_PARAMETER_FLICKER_AVOID,
> > &ctrl_set_flicker_avoidance,
> > false
> > --
> > 2.19.0
> >
> >
> > _______________________________________________
> > linux-rpi-kernel mailing list
> > linux-rpi-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Nathan Chancellor <natechancellor@gmail.com>
To: Dave Stevenson <dave.stevenson@raspberrypi.org>
Cc: Eric Anholt <eric@anholt.net>,
Stefan Wahren <stefan.wahren@i2se.com>,
Greg KH <gregkh@linuxfoundation.org>,
devel@driverdev.osuosl.org, ndesaulniers@google.com,
linux-kernel@vger.kernel.org,
"moderated list:BROADCOM BCM2835 ARM ARCHITECTURE"
<linux-rpi-kernel@lists.infradead.org>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] staging: bcm2835-camera: Avoid unneeded internal declaration warning
Date: Fri, 28 Sep 2018 07:48:56 -0700 [thread overview]
Message-ID: <20180928144856.GA22524@flashbox> (raw)
In-Reply-To: <CAAoAYcMMiiDXCcnZ=DP9Y9Xs+xdq-jasj8G_W0jrnQV9so79=A@mail.gmail.com>
On Fri, Sep 28, 2018 at 10:04:29AM +0100, Dave Stevenson wrote:
> Hi Nate
>
> Thanks for the patch.
>
> On Fri, 28 Sep 2018 at 01:53, Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > Clang warns:
> >
> > drivers/staging/vc04_services/bcm2835-camera/controls.c:59:18: warning:
> > variable 'mains_freq_qmenu' is not needed and will not be emitted
> > [-Wunneeded-internal-declaration]
> > static const s64 mains_freq_qmenu[] = {
> > ^
> > 1 warning generated.
> >
> > This is because mains_freq_qmenu is currently only used in an ARRAY_SIZE
> > macro, which is a compile time evaluation in this case. Avoid this by
> > adding mains_freq_qmenu as the imenu member of this structure, which
> > matches all other controls that uses the ARRAY_SIZE macro in v4l2_ctrls.
> > This turns out to be a no-op because V4L2_CID_MPEG_VIDEO_BITRATE_MODE is
> > defined as a MMAL_CONTROL_TYPE_STD_MENU, which does not pass the imenu
> > definition along to v4l2_ctrl_new in bm2835_mmal_init_controls.
>
> There's a slight confusion betwen V4L2_CID_MPEG_VIDEO_BITRATE_MODE and
> V4L2_CID_POWER_LINE_FREQUENCY in your description.
>
> However you're right that MMAL_CONTROL_TYPE_STD_MENU calls
> v4l2_ctrl_new_std_menu which doesn't need a menu array (it's
> v4l2_ctrl_new_int_menu that does). That means the correct fix is to
> define the max value using the relevant enum (in this case
> V4L2_CID_POWER_LINE_FREQUENCY_AUTO) and remove the array.
>
> The same is true for V4L2_CID_MPEG_VIDEO_BITRATE_MODE - max should be
> V4L2_MPEG_VIDEO_BITRATE_MODE_CBR, and remove the bitrate_mode_qmenu
> array.
>
> Thanks,
> Dave
>
Hi Dave,
Thank you for the review, I appreciate it. I had certainly considered
that solution first butnfigured this was the more conservative change.
I will send a v2 soon with the suggested change.
Nathan
> > Link: https://github.com/ClangBuiltLinux/linux/issues/122
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> > drivers/staging/vc04_services/bcm2835-camera/controls.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > index cff7b1e07153..a2c55cb2192a 100644
> > --- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > +++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > @@ -1106,7 +1106,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
> > {
> > V4L2_CID_POWER_LINE_FREQUENCY, MMAL_CONTROL_TYPE_STD_MENU,
> > 0, ARRAY_SIZE(mains_freq_qmenu) - 1,
> > - 1, 1, NULL,
> > + 1, 1, mains_freq_qmenu,
> > MMAL_PARAMETER_FLICKER_AVOID,
> > &ctrl_set_flicker_avoidance,
> > false
> > --
> > 2.19.0
> >
> >
> > _______________________________________________
> > linux-rpi-kernel mailing list
> > linux-rpi-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel
next prev parent reply other threads:[~2018-09-28 14:48 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-28 0:50 [PATCH] staging: bcm2835-camera: Avoid unneeded internal declaration warning Nathan Chancellor
2018-09-28 0:50 ` Nathan Chancellor
2018-09-28 9:04 ` Dave Stevenson
2018-09-28 9:04 ` Dave Stevenson
2018-09-28 14:48 ` Nathan Chancellor [this message]
2018-09-28 14:48 ` Nathan Chancellor
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=20180928144856.GA22524@flashbox \
--to=natechancellor@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.