Re: [PATCH] media: Fix Media Controller API config checks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/15/21 10:55 AM, Shuah Khan wrote:
On 6/15/21 7:36 AM, Hans Verkuil wrote:
Hi Shuah,

On 11/06/2021 03:58, Shuah Khan wrote:
Smatch static checker warns that "mdev" can be null:

sound/usb/media.c:287 snd_media_device_create()
     warn: 'mdev' can also be NULL

If CONFIG_MEDIA_CONTROLLER is disabled, this file should not be included
in the build.

The below conditions in the sound/usb/Makefile are in place to ensure that
media.c isn't included in the build.

sound/usb/Makefile:
snd-usb-audio-$(CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER) += media.o

select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER &&
        (MEDIA_SUPPORT=y || MEDIA_SUPPORT=SND_USB_AUDIO)

The following config check in include/media/media-dev-allocator.h is
in place to enable the API only when CONFIG_MEDIA_CONTROLLER and
CONFIG_USB are enabled.

  #if defined(CONFIG_MEDIA_CONTROLLER) && defined(CONFIG_USB)

This check doesn't work as intended when CONFIG_USB=m. When CONFIG_USB=m, CONFIG_USB_MODULE is defined and CONFIG_USB is not. The above config check doesn't catch that CONFIG_USB is defined as a module and disables the API.
This results in sound/usb enabling Media Controller specific ALSA driver
code, while Media disables the Media Controller API.

Fix the problem requires two changes:

1. Change the check to use IS_ENABLED to detect when CONFIG_USB is enabled     as a module or static. Since CONFIG_MEDIA_CONTROLLER is a bool, leave
    the check unchanged to be consistent with drivers/media/Makefile.

2. Change the drivers/media/mc/Makefile to include mc-dev-allocator.o
    in mc-objs when CONFIG_USB is y or m.

If I test this patch, then I get:

drivers/media/mc/mc-dev-allocator.c:97:22: error: redefinition of 'media_device_usb_allocate'     97 | struct media_device *media_device_usb_allocate(struct usb_device *udev,
       |                      ^~~~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/media/mc/mc-dev-allocator.c:24:
include/media/media-dev-allocator.h:55:36: note: previous definition of 'media_device_usb_allocate' was here
    55 | static inline struct media_device *media_device_usb_allocate(
       |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/media/mc/mc-dev-allocator.c:119:6: error: redefinition of 'media_device_delete'    119 | void media_device_delete(struct media_device *mdev, const char *module_name,
       |      ^~~~~~~~~~~~~~~~~~~
In file included from drivers/media/mc/mc-dev-allocator.c:24:
include/media/media-dev-allocator.h:59:20: note: previous definition of 'media_device_delete' was here
    59 | static inline void media_device_delete(
       |                    ^~~~~~~~~~~~~~~~~~~

The .config has:

# CONFIG_USB_SUPPORT is not set
CONFIG_MEDIA_CONTROLLER=y



Hi Hans,

I don't know why I made the logic fancy and complex. Fixed now and
sending v2 shortly. I made sure all the combinations are tested now.

thanks,
-- Shuah




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux