Some devices do not implement all their controls in a way that complies
with the UVC specification. This is for instance the case for several
devices that do not support the disabled mode for the power line
frequency control. Add a mechanism to allow per-device control mapping
overrides to avoid errors when accessing non-compliant controls.
Signed-off-by: Ricardo Ribalda <[email protected]>
Signed-off-by: Laurent Pinchart <[email protected]>
---
drivers/media/usb/uvc/uvc_ctrl.c | 35 ++++++++++++++++++++++++++------
drivers/media/usb/uvc/uvcvideo.h | 1 +
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index a709ebbb4d69..092decfdaa62 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2403,9 +2403,8 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
{
const struct uvc_control_info *info = uvc_ctrls;
const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
- const struct uvc_control_mapping *mapping = uvc_ctrl_mappings;
- const struct uvc_control_mapping *mend =
- mapping + ARRAY_SIZE(uvc_ctrl_mappings);
+ const struct uvc_control_mapping *mapping;
+ const struct uvc_control_mapping *mend;
/* XU controls initialization requires querying the device for control
* information. As some buggy UVC devices will crash when queried
@@ -2433,14 +2432,38 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
if (!ctrl->initialized)
return;
- /* Process common mappings first. */
- for (; mapping < mend; ++mapping) {
+ /*
+ * First check if the device provides a custom mapping for this control,
+ * used to override standard mappings for non-conformant devices. Don't
+ * process standard mappings if a custom mapping is found. This
+ * mechanism doesn't support combining standard and custom mappings for
+ * a single control.
+ */
+ if (chain->dev->info->mappings) {
+ bool custom = false;
+ unsigned int i;
+
+ for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) {
+ if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
+ ctrl->info.selector == mapping->selector) {
+ __uvc_ctrl_add_mapping(chain, ctrl, mapping);
+ custom = true;
+ }
+ }
+
+ if (custom)
+ return;
+ }
+
+ /* Process common mappings next. */
+ mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
+ for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
ctrl->info.selector == mapping->selector)
__uvc_ctrl_add_mapping(chain, ctrl, mapping);
}
- /* And then version-specific mappings. */
+ /* Finally process version-specific mappings. */
if (chain->dev->uvc_version < 0x0150) {
mapping = uvc_ctrl_mappings_uvc11;
mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11);
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index c5b4febd2d94..fff5c5c99a3d 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -667,6 +667,7 @@ struct uvc_device_info {
u32 quirks;
u32 meta_format;
u16 uvc_version;
+ const struct uvc_control_mapping **mappings;
};
struct uvc_device {
--
2.36.1.476.g0c4daa206d-goog
Hi Ricardo,
Thank you for the patch.
On Fri, Jun 17, 2022 at 12:36:39PM +0200, Ricardo Ribalda wrote:
> Some devices do not implement all their controls in a way that complies
> with the UVC specification. This is for instance the case for several
> devices that do not support the disabled mode for the power line
> frequency control. Add a mechanism to allow per-device control mapping
> overrides to avoid errors when accessing non-compliant controls.
>
> Signed-off-by: Ricardo Ribalda <[email protected]>
> Signed-off-by: Laurent Pinchart <[email protected]>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 35 ++++++++++++++++++++++++++------
> drivers/media/usb/uvc/uvcvideo.h | 1 +
> 2 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index a709ebbb4d69..092decfdaa62 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2403,9 +2403,8 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> {
> const struct uvc_control_info *info = uvc_ctrls;
> const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
> - const struct uvc_control_mapping *mapping = uvc_ctrl_mappings;
> - const struct uvc_control_mapping *mend =
> - mapping + ARRAY_SIZE(uvc_ctrl_mappings);
> + const struct uvc_control_mapping *mapping;
> + const struct uvc_control_mapping *mend;
>
> /* XU controls initialization requires querying the device for control
> * information. As some buggy UVC devices will crash when queried
> @@ -2433,14 +2432,38 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> if (!ctrl->initialized)
> return;
>
> - /* Process common mappings first. */
> - for (; mapping < mend; ++mapping) {
> + /*
> + * First check if the device provides a custom mapping for this control,
> + * used to override standard mappings for non-conformant devices. Don't
> + * process standard mappings if a custom mapping is found. This
> + * mechanism doesn't support combining standard and custom mappings for
> + * a single control.
> + */
> + if (chain->dev->info->mappings) {
> + bool custom = false;
> + unsigned int i;
> +
> + for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) {
> + if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> + ctrl->info.selector == mapping->selector) {
> + __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> + custom = true;
> + }
> + }
> +
> + if (custom)
> + return;
> + }
> +
> + /* Process common mappings next. */
> + mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
I don't think mapping has the right value here, it could even be
uninitialized. Let's make this
mapping = uvc_ctrl_mappings;
mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
for (; mapping < mend; ++mapping) {
to match the code below.
> + for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
> if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> ctrl->info.selector == mapping->selector)
> __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> }
>
> - /* And then version-specific mappings. */
> + /* Finally process version-specific mappings. */
> if (chain->dev->uvc_version < 0x0150) {
> mapping = uvc_ctrl_mappings_uvc11;
> mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11);
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index c5b4febd2d94..fff5c5c99a3d 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -667,6 +667,7 @@ struct uvc_device_info {
> u32 quirks;
> u32 meta_format;
> u16 uvc_version;
> + const struct uvc_control_mapping **mappings;
> };
>
> struct uvc_device {
--
Regards,
Laurent Pinchart
Hi Laurent
On Fri, 17 Jun 2022 at 15:44, Laurent Pinchart
<[email protected]> wrote:
>
> Hi Ricardo,
>
> Thank you for the patch.
>
> On Fri, Jun 17, 2022 at 12:36:39PM +0200, Ricardo Ribalda wrote:
> > Some devices do not implement all their controls in a way that complies
> > with the UVC specification. This is for instance the case for several
> > devices that do not support the disabled mode for the power line
> > frequency control. Add a mechanism to allow per-device control mapping
> > overrides to avoid errors when accessing non-compliant controls.
> >
> > Signed-off-by: Ricardo Ribalda <[email protected]>
> > Signed-off-by: Laurent Pinchart <[email protected]>
> > ---
> > drivers/media/usb/uvc/uvc_ctrl.c | 35 ++++++++++++++++++++++++++------
> > drivers/media/usb/uvc/uvcvideo.h | 1 +
> > 2 files changed, 30 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index a709ebbb4d69..092decfdaa62 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -2403,9 +2403,8 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> > {
> > const struct uvc_control_info *info = uvc_ctrls;
> > const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
> > - const struct uvc_control_mapping *mapping = uvc_ctrl_mappings;
> > - const struct uvc_control_mapping *mend =
> > - mapping + ARRAY_SIZE(uvc_ctrl_mappings);
> > + const struct uvc_control_mapping *mapping;
> > + const struct uvc_control_mapping *mend;
> >
> > /* XU controls initialization requires querying the device for control
> > * information. As some buggy UVC devices will crash when queried
> > @@ -2433,14 +2432,38 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> > if (!ctrl->initialized)
> > return;
> >
> > - /* Process common mappings first. */
> > - for (; mapping < mend; ++mapping) {
> > + /*
> > + * First check if the device provides a custom mapping for this control,
> > + * used to override standard mappings for non-conformant devices. Don't
> > + * process standard mappings if a custom mapping is found. This
> > + * mechanism doesn't support combining standard and custom mappings for
> > + * a single control.
> > + */
> > + if (chain->dev->info->mappings) {
> > + bool custom = false;
> > + unsigned int i;
> > +
> > + for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) {
> > + if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> > + ctrl->info.selector == mapping->selector) {
> > + __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> > + custom = true;
> > + }
> > + }
> > +
> > + if (custom)
> > + return;
> > + }
> > +
> > + /* Process common mappings next. */
> > + mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
>
> I don't think mapping has the right value here, it could even be
> uninitialized. Let's make this
>
> mapping = uvc_ctrl_mappings;
> mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
>
> for (; mapping < mend; ++mapping) {
>
> to match the code below.
ups, sorry about that :(
. Will send as a new version
>
> > + for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
> > if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> > ctrl->info.selector == mapping->selector)
> > __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> > }
> >
> > - /* And then version-specific mappings. */
> > + /* Finally process version-specific mappings. */
> > if (chain->dev->uvc_version < 0x0150) {
> > mapping = uvc_ctrl_mappings_uvc11;
> > mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11);
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > index c5b4febd2d94..fff5c5c99a3d 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -667,6 +667,7 @@ struct uvc_device_info {
> > u32 quirks;
> > u32 meta_format;
> > u16 uvc_version;
> > + const struct uvc_control_mapping **mappings;
> > };
> >
> > struct uvc_device {
>
> --
> Regards,
>
> Laurent Pinchart
--
Ricardo Ribalda
Hi Ricardo,
I love your patch! Perhaps something to improve:
[auto build test WARNING on media-tree/master]
[also build test WARNING on linus/master v5.19-rc2 next-20220617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Ricardo-Ribalda/uvcvideo-Fix-handling-of-power_line_frequency/20220617-185644
base: git://linuxtv.org/media_tree.git master
config: hexagon-randconfig-r045-20220617 (https://download.01.org/0day-ci/archive/20220617/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d764aa7fc6b9cc3fbe960019018f5f9e941eb0a6)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/10bdca4191d7a8be97c77dbe4ba89c05713ee0e2
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Ricardo-Ribalda/uvcvideo-Fix-handling-of-power_line_frequency/20220617-185644
git checkout 10bdca4191d7a8be97c77dbe4ba89c05713ee0e2
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/media/usb/uvc/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> drivers/media/usb/uvc/uvc_ctrl.c:2442:6: warning: variable 'mapping' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
if (chain->dev->info->mappings) {
^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media/usb/uvc/uvc_ctrl.c:2459:9: note: uninitialized use occurs here
mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
^~~~~~~
drivers/media/usb/uvc/uvc_ctrl.c:2442:2: note: remove the 'if' if its condition is always true
if (chain->dev->info->mappings) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media/usb/uvc/uvc_ctrl.c:2406:43: note: initialize the variable 'mapping' to silence this warning
const struct uvc_control_mapping *mapping;
^
= NULL
1 warning generated.
vim +2442 drivers/media/usb/uvc/uvc_ctrl.c
2396
2397 /*
2398 * Add control information and hardcoded stock control mappings to the given
2399 * device.
2400 */
2401 static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
2402 struct uvc_control *ctrl)
2403 {
2404 const struct uvc_control_info *info = uvc_ctrls;
2405 const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
2406 const struct uvc_control_mapping *mapping;
2407 const struct uvc_control_mapping *mend;
2408
2409 /* XU controls initialization requires querying the device for control
2410 * information. As some buggy UVC devices will crash when queried
2411 * repeatedly in a tight loop, delay XU controls initialization until
2412 * first use.
2413 */
2414 if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
2415 return;
2416
2417 for (; info < iend; ++info) {
2418 if (uvc_entity_match_guid(ctrl->entity, info->entity) &&
2419 ctrl->index == info->index) {
2420 uvc_ctrl_add_info(chain->dev, ctrl, info);
2421 /*
2422 * Retrieve control flags from the device. Ignore errors
2423 * and work with default flag values from the uvc_ctrl
2424 * array when the device doesn't properly implement
2425 * GET_INFO on standard controls.
2426 */
2427 uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
2428 break;
2429 }
2430 }
2431
2432 if (!ctrl->initialized)
2433 return;
2434
2435 /*
2436 * First check if the device provides a custom mapping for this control,
2437 * used to override standard mappings for non-conformant devices. Don't
2438 * process standard mappings if a custom mapping is found. This
2439 * mechanism doesn't support combining standard and custom mappings for
2440 * a single control.
2441 */
> 2442 if (chain->dev->info->mappings) {
2443 bool custom = false;
2444 unsigned int i;
2445
2446 for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) {
2447 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
2448 ctrl->info.selector == mapping->selector) {
2449 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
2450 custom = true;
2451 }
2452 }
2453
2454 if (custom)
2455 return;
2456 }
2457
2458 /* Process common mappings next. */
2459 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
2460 for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
2461 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
2462 ctrl->info.selector == mapping->selector)
2463 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
2464 }
2465
2466 /* Finally process version-specific mappings. */
2467 if (chain->dev->uvc_version < 0x0150) {
2468 mapping = uvc_ctrl_mappings_uvc11;
2469 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11);
2470 } else {
2471 mapping = uvc_ctrl_mappings_uvc15;
2472 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc15);
2473 }
2474
2475 for (; mapping < mend; ++mapping) {
2476 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
2477 ctrl->info.selector == mapping->selector)
2478 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
2479 }
2480 }
2481
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Hi Ricardo,
url: https://github.com/intel-lab-lkp/linux/commits/Ricardo-Ribalda/uvcvideo-Fix-handling-of-power_line_frequency/20220617-185644
base: git://linuxtv.org/media_tree.git master
config: microblaze-randconfig-m031-20220629 (https://download.01.org/0day-ci/archive/20220630/[email protected]/config)
compiler: microblaze-linux-gcc (GCC) 11.3.0
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
smatch warnings:
drivers/media/usb/uvc/uvc_ctrl.c:2459 uvc_ctrl_init_ctrl() error: uninitialized symbol 'mapping'.
vim +/mapping +2459 drivers/media/usb/uvc/uvc_ctrl.c
866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2401 static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2402 struct uvc_control *ctrl)
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2403 {
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2404 const struct uvc_control_info *info = uvc_ctrls;
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2405 const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2406 const struct uvc_control_mapping *mapping;
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2407 const struct uvc_control_mapping *mend;
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2408
52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2409 /* XU controls initialization requires querying the device for control
52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2410 * information. As some buggy UVC devices will crash when queried
52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2411 * repeatedly in a tight loop, delay XU controls initialization until
52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2412 * first use.
52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2413 */
52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2414 if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2415 return;
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2416
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2417 for (; info < iend; ++info) {
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2418 if (uvc_entity_match_guid(ctrl->entity, info->entity) &&
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2419 ctrl->index == info->index) {
866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2420 uvc_ctrl_add_info(chain->dev, ctrl, info);
93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2421 /*
93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2422 * Retrieve control flags from the device. Ignore errors
93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2423 * and work with default flag values from the uvc_ctrl
93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2424 * array when the device doesn't properly implement
93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2425 * GET_INFO on standard controls.
93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2426 */
866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2427 uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2428 break;
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2429 }
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2430 }
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2431
071c8bb827c80a drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2432 if (!ctrl->initialized)
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2433 return;
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2434
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2435 /*
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2436 * First check if the device provides a custom mapping for this control,
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2437 * used to override standard mappings for non-conformant devices. Don't
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2438 * process standard mappings if a custom mapping is found. This
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2439 * mechanism doesn't support combining standard and custom mappings for
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2440 * a single control.
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2441 */
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2442 if (chain->dev->info->mappings) {
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2443 bool custom = false;
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2444 unsigned int i;
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2445
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2446 for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) {
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2447 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2448 ctrl->info.selector == mapping->selector) {
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2449 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2450 custom = true;
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2451 }
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2452 }
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2453
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2454 if (custom)
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2455 return;
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2456 }
"mapping" uninitialized if chain->dev->info->mappings is NULL.
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2457
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2458 /* Process common mappings next. */
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 @2459 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
^^^^^^^
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2460 for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
I'm surprised this made it through testing... Anyway, the easier way to
to iterate through an array of structs is just say:
for (i = 0; i < ARRAY_SIZE(uvc_ctrl_mappings); i++) {
mapping = &uvc_ctrl_mappings[i];
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2461 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2462 ctrl->info.selector == mapping->selector)
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2463 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2464 }
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2465
10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2466 /* Finally process version-specific mappings. */
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2467 if (chain->dev->uvc_version < 0x0150) {
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2468 mapping = uvc_ctrl_mappings_uvc11;
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2469 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11);
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2470 } else {
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2471 mapping = uvc_ctrl_mappings_uvc15;
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2472 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc15);
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2473 }
87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2474
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2475 for (; mapping < mend; ++mapping) {
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2476 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
071c8bb827c80a drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2477 ctrl->info.selector == mapping->selector)
866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2478 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2479 }
ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2480 }
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Hi Dan
Thanks for your mail
On Thu, 30 Jun 2022 at 13:08, Dan Carpenter <[email protected]> wrote:
>
> Hi Ricardo,
>
> url: https://github.com/intel-lab-lkp/linux/commits/Ricardo-Ribalda/uvcvideo-Fix-handling-of-power_line_frequency/20220617-185644
> base: git://linuxtv.org/media_tree.git master
> config: microblaze-randconfig-m031-20220629 (https://download.01.org/0day-ci/archive/20220630/[email protected]/config)
> compiler: microblaze-linux-gcc (GCC) 11.3.0
>
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <[email protected]>
> Reported-by: Dan Carpenter <[email protected]>
>
> smatch warnings:
> drivers/media/usb/uvc/uvc_ctrl.c:2459 uvc_ctrl_init_ctrl() error: uninitialized symbol 'mapping'.
>
> vim +/mapping +2459 drivers/media/usb/uvc/uvc_ctrl.c
>
> 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2401 static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2402 struct uvc_control *ctrl)
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2403 {
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2404 const struct uvc_control_info *info = uvc_ctrls;
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2405 const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2406 const struct uvc_control_mapping *mapping;
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2407 const struct uvc_control_mapping *mend;
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2408
> 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2409 /* XU controls initialization requires querying the device for control
> 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2410 * information. As some buggy UVC devices will crash when queried
> 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2411 * repeatedly in a tight loop, delay XU controls initialization until
> 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2412 * first use.
> 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2413 */
> 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2414 if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2415 return;
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2416
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2417 for (; info < iend; ++info) {
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2418 if (uvc_entity_match_guid(ctrl->entity, info->entity) &&
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2419 ctrl->index == info->index) {
> 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2420 uvc_ctrl_add_info(chain->dev, ctrl, info);
> 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2421 /*
> 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2422 * Retrieve control flags from the device. Ignore errors
> 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2423 * and work with default flag values from the uvc_ctrl
> 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2424 * array when the device doesn't properly implement
> 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2425 * GET_INFO on standard controls.
> 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2426 */
> 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2427 uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2428 break;
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2429 }
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2430 }
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2431
> 071c8bb827c80a drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2432 if (!ctrl->initialized)
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2433 return;
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2434
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2435 /*
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2436 * First check if the device provides a custom mapping for this control,
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2437 * used to override standard mappings for non-conformant devices. Don't
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2438 * process standard mappings if a custom mapping is found. This
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2439 * mechanism doesn't support combining standard and custom mappings for
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2440 * a single control.
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2441 */
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2442 if (chain->dev->info->mappings) {
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2443 bool custom = false;
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2444 unsigned int i;
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2445
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2446 for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) {
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2447 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2448 ctrl->info.selector == mapping->selector) {
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2449 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2450 custom = true;
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2451 }
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2452 }
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2453
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2454 if (custom)
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2455 return;
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2456 }
>
> "mapping" uninitialized if chain->dev->info->mappings is NULL.
>
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2457
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2458 /* Process common mappings next. */
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 @2459 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
> ^^^^^^^
>
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2460 for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
>
> I'm surprised this made it through testing... Anyway, the easier way to
> to iterate through an array of structs is just say:
There is already a new version under review that fixes this:
https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/
>
> for (i = 0; i < ARRAY_SIZE(uvc_ctrl_mappings); i++) {
> mapping = &uvc_ctrl_mappings[i];
>
I agree, and that is what I normally used, but that driver iterate this way...
Thanks again!
PS: Any idea why the test was triggered 14+ days after the patch was sent?
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2461 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2462 ctrl->info.selector == mapping->selector)
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2463 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2464 }
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2465
> 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2466 /* Finally process version-specific mappings. */
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2467 if (chain->dev->uvc_version < 0x0150) {
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2468 mapping = uvc_ctrl_mappings_uvc11;
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2469 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11);
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2470 } else {
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2471 mapping = uvc_ctrl_mappings_uvc15;
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2472 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc15);
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2473 }
> 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2474
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2475 for (; mapping < mend; ++mapping) {
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2476 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> 071c8bb827c80a drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2477 ctrl->info.selector == mapping->selector)
> 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2478 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2479 }
> ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2480 }
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp
>
--
Ricardo Ribalda
On Fri, Jul 01, 2022 at 01:16:23AM +0200, Ricardo Ribalda wrote:
> Hi Dan
>
> Thanks for your mail
>
> On Thu, 30 Jun 2022 at 13:08, Dan Carpenter <[email protected]> wrote:
> >
> > Hi Ricardo,
> >
> > url: https://github.com/intel-lab-lkp/linux/commits/Ricardo-Ribalda/uvcvideo-Fix-handling-of-power_line_frequency/20220617-185644
> > base: git://linuxtv.org/media_tree.git master
> > config: microblaze-randconfig-m031-20220629 (https://download.01.org/0day-ci/archive/20220630/[email protected]/config)
> > compiler: microblaze-linux-gcc (GCC) 11.3.0
> >
> > If you fix the issue, kindly add following tag where applicable
> > Reported-by: kernel test robot <[email protected]>
> > Reported-by: Dan Carpenter <[email protected]>
> >
> > smatch warnings:
> > drivers/media/usb/uvc/uvc_ctrl.c:2459 uvc_ctrl_init_ctrl() error: uninitialized symbol 'mapping'.
> >
> > vim +/mapping +2459 drivers/media/usb/uvc/uvc_ctrl.c
> >
> > 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2401 static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
> > 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2402 struct uvc_control *ctrl)
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2403 {
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2404 const struct uvc_control_info *info = uvc_ctrls;
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2405 const struct uvc_control_info *iend = info + ARRAY_SIZE(uvc_ctrls);
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2406 const struct uvc_control_mapping *mapping;
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2407 const struct uvc_control_mapping *mend;
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2408
> > 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2409 /* XU controls initialization requires querying the device for control
> > 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2410 * information. As some buggy UVC devices will crash when queried
> > 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2411 * repeatedly in a tight loop, delay XU controls initialization until
> > 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2412 * first use.
> > 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2413 */
> > 52c58ad6f95ff6 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2414 if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2415 return;
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2416
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2417 for (; info < iend; ++info) {
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2418 if (uvc_entity_match_guid(ctrl->entity, info->entity) &&
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2419 ctrl->index == info->index) {
> > 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2420 uvc_ctrl_add_info(chain->dev, ctrl, info);
> > 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2421 /*
> > 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2422 * Retrieve control flags from the device. Ignore errors
> > 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2423 * and work with default flag values from the uvc_ctrl
> > 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2424 * array when the device doesn't properly implement
> > 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2425 * GET_INFO on standard controls.
> > 93df48d37c3f03 drivers/media/usb/uvc/uvc_ctrl.c Hans de Goede 2020-07-28 2426 */
> > 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2427 uvc_ctrl_get_flags(chain->dev, ctrl, &ctrl->info);
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2428 break;
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2429 }
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2430 }
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2431
> > 071c8bb827c80a drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2432 if (!ctrl->initialized)
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2433 return;
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2434
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2435 /*
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2436 * First check if the device provides a custom mapping for this control,
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2437 * used to override standard mappings for non-conformant devices. Don't
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2438 * process standard mappings if a custom mapping is found. This
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2439 * mechanism doesn't support combining standard and custom mappings for
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2440 * a single control.
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2441 */
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2442 if (chain->dev->info->mappings) {
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2443 bool custom = false;
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2444 unsigned int i;
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2445
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2446 for (i = 0; (mapping = chain->dev->info->mappings[i]); ++i) {
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2447 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2448 ctrl->info.selector == mapping->selector) {
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2449 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2450 custom = true;
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2451 }
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2452 }
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2453
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2454 if (custom)
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2455 return;
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2456 }
> >
> > "mapping" uninitialized if chain->dev->info->mappings is NULL.
> >
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2457
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2458 /* Process common mappings next. */
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 @2459 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
> > ^^^^^^^
> >
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2460 for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
> >
> > I'm surprised this made it through testing... Anyway, the easier way to
> > to iterate through an array of structs is just say:
>
> There is already a new version under review that fixes this:
>
> https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/
>
>
> >
> > for (i = 0; i < ARRAY_SIZE(uvc_ctrl_mappings); i++) {
> > mapping = &uvc_ctrl_mappings[i];
> >
>
> I agree, and that is what I normally used, but that driver iterate this way...
>
> Thanks again!
>
> PS: Any idea why the test was triggered 14+ days after the patch was sent?
Thanks, the config is a random one, which is generated on 6/29 "microblaze-randconfig-m031-20220629",
which is probably the case that only this randconfig has chance to trigger the issue
and bisect successfully.
>
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2461 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2462 ctrl->info.selector == mapping->selector)
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2463 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2464 }
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2465
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2466 /* Finally process version-specific mappings. */
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2467 if (chain->dev->uvc_version < 0x0150) {
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2468 mapping = uvc_ctrl_mappings_uvc11;
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2469 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc11);
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2470 } else {
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2471 mapping = uvc_ctrl_mappings_uvc15;
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2472 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings_uvc15);
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2473 }
> > 87c205087585a0 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2474
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2475 for (; mapping < mend; ++mapping) {
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2476 if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
> > 071c8bb827c80a drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29 2477 ctrl->info.selector == mapping->selector)
> > 866c6bdd5663d4 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2021-06-18 2478 __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2479 }
> > ba2fa99668bb9b drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-20 2480 }
> >
> > --
> > 0-DAY CI Kernel Test Service
> > https://01.org/lkp
> >
>
>
> --
> Ricardo Ribalda
> _______________________________________________
> kbuild-all mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
On Fri, Jul 01, 2022 at 01:16:23AM +0200, Ricardo Ribalda wrote:
> >
> > "mapping" uninitialized if chain->dev->info->mappings is NULL.
> >
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2457
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2458 /* Process common mappings next. */
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 @2459 mend = mapping + ARRAY_SIZE(uvc_ctrl_mappings);
> > ^^^^^^^
> >
> > 10bdca4191d7a8 drivers/media/usb/uvc/uvc_ctrl.c Ricardo Ribalda 2022-06-17 2460 for (mapping = uvc_ctrl_mappings; mapping < mend; ++mapping) {
> >
> > I'm surprised this made it through testing... Anyway, the easier way to
> > to iterate through an array of structs is just say:
>
> There is already a new version under review that fixes this:
>
> https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/
>
>
> >
> > for (i = 0; i < ARRAY_SIZE(uvc_ctrl_mappings); i++) {
> > mapping = &uvc_ctrl_mappings[i];
> >
>
> I agree, and that is what I normally used, but that driver iterate this way...
That's not really an excuse. Imagine you were a lemming and Disney
employees were trying to chase you off a cliff!
>
> Thanks again!
>
> PS: Any idea why the test was triggered 14+ days after the patch was sent?
I don't know how kbuild works. I run Smatch on linux-next so I would
have caught it eventually. Plus the clang people would have caught it.
regards,
dan carpenter