Re: [RFC PATCHv2] em28xx: split analog part into a separate module

["Frank Schäfer" <fschaefer.oss@googlemail.com>]

Hi Mauro,

Am 22.12.2013 13:47, schrieb Mauro Carvalho Chehab:
> Now that dvb-only devices start to happen, it makes sense
> to split the analog part on a separate module.
>
> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
>
> -
>
> This is a respin of https://patchwork.linuxtv.org/patch/17967/
>
> v2: add a dev->has_video to signalize if the device has a video interface, and
>     fix some locks
>
> Compile-tested only.
>
> ---
>  drivers/media/usb/em28xx/Kconfig        |   6 +-
>  drivers/media/usb/em28xx/Makefile       |   5 +-
>  drivers/media/usb/em28xx/em28xx-cards.c |  94 ++--------------------
>  drivers/media/usb/em28xx/em28xx-core.c  |  12 +++
>  drivers/media/usb/em28xx/em28xx-video.c | 133 ++++++++++++++++++++++++++++----
>  drivers/media/usb/em28xx/em28xx.h       |  17 ++--
>  6 files changed, 150 insertions(+), 117 deletions(-)
>
> diff --git a/drivers/media/usb/em28xx/Kconfig b/drivers/media/usb/em28xx/Kconfig
> index d6ba514d31eb..838fc9dbb747 100644
> --- a/drivers/media/usb/em28xx/Kconfig
> +++ b/drivers/media/usb/em28xx/Kconfig
> @@ -1,8 +1,12 @@
>  config VIDEO_EM28XX
> -	tristate "Empia EM28xx USB video capture support"
> +	tristate "Empia EM28xx USB devices support"
>  	depends on VIDEO_DEV && I2C
>  	select VIDEO_TUNER
>  	select VIDEO_TVEEPROM
> +
> +config VIDEO_EM28XX_V4L2
> +	tristate "Empia EM28xx analog TV, video capture and/or webcam support"
> +	depends on VIDEO_EM28XX && I2C
VIDEO_EM28XX already depends on I2C.

>  	select VIDEOBUF2_VMALLOC
>  	select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECT
>  	select VIDEO_TVP5150 if MEDIA_SUBDRV_AUTOSELECT
> diff --git a/drivers/media/usb/em28xx/Makefile b/drivers/media/usb/em28xx/Makefile
> index ad6d48557940..3e2b6b54817d 100644
> --- a/drivers/media/usb/em28xx/Makefile
> +++ b/drivers/media/usb/em28xx/Makefile
> @@ -1,10 +1,11 @@
> -em28xx-y +=	em28xx-video.o em28xx-i2c.o em28xx-cards.o
> -em28xx-y +=	em28xx-core.o  em28xx-vbi.o em28xx-camera.o
> +em28xx-y +=	em28xx-core.o em28xx-i2c.o em28xx-cards.o
>  
> +em28xx-v4l-objs := em28xx-video.o em28xx-vbi.o em28xx-camera.o
>  em28xx-alsa-objs := em28xx-audio.o
>  em28xx-rc-objs := em28xx-input.o
>  
>  obj-$(CONFIG_VIDEO_EM28XX) += em28xx.o
> +obj-$(CONFIG_VIDEO_EM28XX_V4L2) += em28xx-v4l.o
>  obj-$(CONFIG_VIDEO_EM28XX_ALSA) += em28xx-alsa.o
>  obj-$(CONFIG_VIDEO_EM28XX_DVB) += em28xx-dvb.o
>  obj-$(CONFIG_VIDEO_EM28XX_RC) += em28xx-rc.o
> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
> index 36853f16bf97..e1ffd9c6e79d 100644
> --- a/drivers/media/usb/em28xx/em28xx-cards.c
> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> @@ -2106,7 +2106,7 @@ struct em28xx_board em28xx_boards[] = {
>  	},
>  	/* 1b80:e1cc Delock 61959
>  	 * Empia EM2874B + Micronas DRX 3913KA2 + NXP TDA18271HDC2
> -         * mostly the same as MaxMedia UB-425-TC but different remote */
> +	 * mostly the same as MaxMedia UB-425-TC but different remote */
>  	[EM2874_BOARD_DELOCK_61959] = {
>  		.name          = "Delock 61959",
>  		.tuner_type    = TUNER_ABSENT,
> @@ -2955,11 +2955,12 @@ static void request_module_async(struct work_struct *work)
>  	em28xx_init_extension(dev);
>  
>  #if defined(CONFIG_MODULES) && defined(MODULE)
> +	if (!dev->has_video)
> +		request_module("em28xx-v4l2");
if (dev->has_video)
What about audio devices ?

>  	if (dev->has_audio_class)
>  		request_module("snd-usb-audio");
>  	else if (dev->has_alsa_audio)
>  		request_module("em28xx-alsa");
> -
>  	if (dev->board.has_dvb)
>  		request_module("em28xx-dvb");
>  	if (dev->board.buttons ||
> @@ -2988,8 +2989,6 @@ void em28xx_release_resources(struct em28xx *dev)
>  {
>  	/*FIXME: I2C IR should be disconnected */
>  
> -	em28xx_release_analog_resources(dev);
> -
>  	if (dev->def_i2c_bus)
>  		em28xx_i2c_unregister(dev, 1);
>  	em28xx_i2c_unregister(dev, 0);
> @@ -3014,7 +3013,6 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
>  			   struct usb_interface *interface,
>  			   int minor)
>  {
> -	struct v4l2_ctrl_handler *hdl = &dev->ctrl_handler;
>  	int retval;
>  	static const char *default_chip_name = "em28xx";
>  	const char *chip_name = default_chip_name;
> @@ -3141,15 +3139,6 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
>  		}
>  	}
>  
> -	retval = v4l2_device_register(&interface->dev, &dev->v4l2_dev);
> -	if (retval < 0) {
> -		em28xx_errdev("Call to v4l2_device_register() failed!\n");
> -		return retval;
> -	}
> -
> -	v4l2_ctrl_handler_init(hdl, 8);
> -	dev->v4l2_dev.ctrl_handler = hdl;
> -
>  	rt_mutex_init(&dev->i2c_bus_lock);
>  
>  	/* register i2c bus 0 */
> @@ -3178,75 +3167,11 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
>  		}
>  	}
>  
> -	/*
> -	 * Default format, used for tvp5150 or saa711x output formats
> -	 */
> -	dev->vinmode = 0x10;
> -	dev->vinctl  = EM28XX_VINCTRL_INTERLACED |
> -		       EM28XX_VINCTRL_CCIR656_ENABLE;
> -
>  	/* Do board specific init and eeprom reading */
>  	em28xx_card_setup(dev);
>  
> -	/* Configure audio */
> -	retval = em28xx_audio_setup(dev);
> -	if (retval < 0) {
> -		em28xx_errdev("%s: Error while setting audio - error [%d]!\n",
> -			__func__, retval);
> -		goto fail;
> -	}
> -	if (dev->audio_mode.ac97 != EM28XX_NO_AC97) {
> -		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
> -			V4L2_CID_AUDIO_MUTE, 0, 1, 1, 1);
> -		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
> -			V4L2_CID_AUDIO_VOLUME, 0, 0x1f, 1, 0x1f);
> -	} else {
> -		/* install the em28xx notify callback */
> -		v4l2_ctrl_notify(v4l2_ctrl_find(hdl, V4L2_CID_AUDIO_MUTE),
> -				em28xx_ctrl_notify, dev);
> -		v4l2_ctrl_notify(v4l2_ctrl_find(hdl, V4L2_CID_AUDIO_VOLUME),
> -				em28xx_ctrl_notify, dev);
> -	}
> -
> -	/* wake i2c devices */
> -	em28xx_wake_i2c(dev);
> -
> -	/* init video dma queues */
> -	INIT_LIST_HEAD(&dev->vidq.active);
> -	INIT_LIST_HEAD(&dev->vbiq.active);
> -
> -	if (dev->board.has_msp34xx) {
> -		/* Send a reset to other chips via gpio */
> -		retval = em28xx_write_reg(dev, EM2820_R08_GPIO_CTRL, 0xf7);
> -		if (retval < 0) {
> -			em28xx_errdev("%s: em28xx_write_reg - "
> -				      "msp34xx(1) failed! error [%d]\n",
> -				      __func__, retval);
> -			goto fail;
> -		}
> -		msleep(3);
> -
> -		retval = em28xx_write_reg(dev, EM2820_R08_GPIO_CTRL, 0xff);
> -		if (retval < 0) {
> -			em28xx_errdev("%s: em28xx_write_reg - "
> -				      "msp34xx(2) failed! error [%d]\n",
> -				      __func__, retval);
> -			goto fail;
> -		}
> -		msleep(3);
> -	}
> -
> -	retval = em28xx_register_analog_devices(dev);
> -	if (retval < 0) {
> -		goto fail;
> -	}
> -
> -	/* Save some power by putting tuner to sleep */
> -	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_power, 0);
> -
>  	return 0;
Now the following code is unrechable...
> -fail:
>  	if (dev->def_i2c_bus)
>  		em28xx_i2c_unregister(dev, 1);
>  	em28xx_i2c_unregister(dev, 0);
... up to the "unregister_dev" label.
Either keep the "fail" label and check the success of
em28xx_card_setup() or remove this dead code.

> @@ -3458,6 +3383,7 @@ static int em28xx_usb_probe(struct usb_interface *interface,
>  	dev->alt   = -1;
>  	dev->is_audio_only = has_audio && !(has_video || has_dvb);
>  	dev->has_alsa_audio = has_audio;
> +	dev->has_video = has_video;
>  	dev->audio_ifnum = ifnum;
>  
>  	/* Checks if audio is provided by some interface */
> @@ -3495,15 +3421,11 @@ static int em28xx_usb_probe(struct usb_interface *interface,
>  	/* save our data pointer in this interface device */
>  	usb_set_intfdata(interface, dev);
>  
> -	/* initialize videobuf2 stuff */
> -	em28xx_vb2_setup(dev);
> -
>  	/* allocate device struct */
>  	mutex_init(&dev->lock);
> -	mutex_lock(&dev->lock);
>  	retval = em28xx_init_dev(dev, udev, interface, nr);
>  	if (retval) {
> -		goto unlock_and_free;
> +		goto err_free;
>  	}
>  
>  	if (usb_xfer_mode < 0) {
> @@ -3546,7 +3468,7 @@ static int em28xx_usb_probe(struct usb_interface *interface,
>  		if (retval) {
>  			printk(DRIVER_NAME
>  			       ": Failed to pre-allocate USB transfer buffers for DVB.\n");
> -			goto unlock_and_free;
> +			goto err_free;
>  		}
>  	}
>  
> @@ -3555,13 +3477,9 @@ static int em28xx_usb_probe(struct usb_interface *interface,
>  	/* Should be the last thing to do, to avoid newer udev's to
>  	   open the device before fully initializing it
>  	 */
> -	mutex_unlock(&dev->lock);
>  
>  	return 0;
>  
> -unlock_and_free:
> -	mutex_unlock(&dev->lock);
> -
>  err_free:
>  	kfree(dev->alt_max_pkt_size_isoc);
>  	kfree(dev);
> diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
> index f6076a512e8f..0f61532d2612 100644
> --- a/drivers/media/usb/em28xx/em28xx-core.c
> +++ b/drivers/media/usb/em28xx/em28xx-core.c
> @@ -33,6 +33,18 @@
>  
>  #include "em28xx.h"
>  
> +#define DRIVER_AUTHOR "Ludovico Cavedon <cavedon@sssup.it>, " \
> +		      "Markus Rechberger <mrechberger@gmail.com>, " \
> +		      "Mauro Carvalho Chehab <mchehab@infradead.org>, " \
> +		      "Sascha Sommer <saschasommer@freenet.de>"
> +
> +#define DRIVER_DESC         "Empia em28xx based USB core driver"
You should update the driver description string for the new v4l2 module
in em28xx-video.c.
...
> +
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(DRIVER_DESC);
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION(EM28XX_VERSION);
> +
...
>  /* #define ENABLE_DEBUG_ISOC_FRAMES */
>  
>  static unsigned int core_debug;
> diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
> index dd19c9ff76e0..b20e4e7116d3 100644
> --- a/drivers/media/usb/em28xx/em28xx-video.c
> +++ b/drivers/media/usb/em28xx/em28xx-video.c
> @@ -51,8 +51,6 @@
>  
>  #define DRIVER_DESC         "Empia em28xx based USB video device driver"
>  
> -#define EM28XX_VERSION "0.2.0"
> -
Some of these DRIVER_XYZ #defines are kept locally, this one is moved to
the header file, other extension modules don't use ist all (strings
hardcoded or corresponding MODULE macros unused)...
Time for a clean-up / unification. :)

>  #define em28xx_videodbg(fmt, arg...) do {\
>  	if (video_debug) \
>  		printk(KERN_INFO "%s %s :"fmt, \
> @@ -763,7 +761,7 @@ static struct vb2_ops em28xx_video_qops = {
>  	.wait_finish    = vb2_ops_wait_finish,
>  };
>  
> -int em28xx_vb2_setup(struct em28xx *dev)
> +static int em28xx_vb2_setup(struct em28xx *dev)
>  {
>  	int rc;
>  	struct vb2_queue *q;
> @@ -831,7 +829,7 @@ static void video_mux(struct em28xx *dev, int index)
>  	em28xx_audio_analog_set(dev);
>  }
>  
> -void em28xx_ctrl_notify(struct v4l2_ctrl *ctrl, void *priv)
> +static void em28xx_ctrl_notify(struct v4l2_ctrl *ctrl, void *priv)
>  {
>  	struct em28xx *dev = priv;
>  
> @@ -1615,11 +1613,11 @@ static int em28xx_v4l2_open(struct file *filp)
>  }
>  
>  /*
> - * em28xx_realease_resources()
> + * em28xx_v4l2_fini()
>   * unregisters the v4l2,i2c and usb devices
>   * called when the device gets disconected or at module unload
>  */
> -void em28xx_release_analog_resources(struct em28xx *dev)
> +static int em28xx_v4l2_fini(struct em28xx *dev)
>  {
>  
>  	/*FIXME: I2C IR should be disconnected */
> @@ -1649,6 +1647,8 @@ void em28xx_release_analog_resources(struct em28xx *dev)
>  			video_device_release(dev->vdev);
>  		dev->vdev = NULL;
>  	}
> +
> +	return 0;
>  }
>  
>  /*
> @@ -1790,8 +1790,6 @@ static struct video_device em28xx_radio_template = {
>  
>  /******************************** usb interface ******************************/
>  
> -
> -
>  static struct video_device *em28xx_vdev_init(struct em28xx *dev,
>  					const struct video_device *template,
>  					const char *type_name)
> @@ -1817,15 +1815,85 @@ static struct video_device *em28xx_vdev_init(struct em28xx *dev,
>  	return vfd;
>  }
>  
> -int em28xx_register_analog_devices(struct em28xx *dev)
> +static int em28xx_v4l2_init(struct em28xx *dev)
>  {
>  	u8 val;
>  	int ret;
>  	unsigned int maxw;
> +	struct v4l2_ctrl_handler *hdl = &dev->ctrl_handler;
> +
> +	if (!dev->has_video) {
Same as above, what about audio devices ?

> +		/* This device does not support the v4l2 extension */
> +		return 0;
-ENODEV

> +	}
>  
>  	printk(KERN_INFO "%s: v4l2 driver version %s\n",
>  		dev->name, EM28XX_VERSION);
>  
> +	mutex_lock(&dev->lock);
> +
> +	ret = v4l2_device_register(&dev->udev->dev, &dev->v4l2_dev);
> +	if (ret < 0) {
> +		em28xx_errdev("Call to v4l2_device_register() failed!\n");
> +		goto err;
> +	}
> +
> +	v4l2_ctrl_handler_init(hdl, 8);
> +	dev->v4l2_dev.ctrl_handler = hdl;
> +
> +	/*
> +	 * Default format, used for tvp5150 or saa711x output formats
> +	 */
> +	dev->vinmode = 0x10;
> +	dev->vinctl  = EM28XX_VINCTRL_INTERLACED |
> +		       EM28XX_VINCTRL_CCIR656_ENABLE;
> +
> +	/* Configure audio */
> +	ret = em28xx_audio_setup(dev);
> +	if (ret < 0) {
> +		em28xx_errdev("%s: Error while setting audio - error [%d]!\n",
> +			__func__, ret);
> +		goto err;
> +	}
> +	if (dev->audio_mode.ac97 != EM28XX_NO_AC97) {
> +		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
> +			V4L2_CID_AUDIO_MUTE, 0, 1, 1, 1);
> +		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
> +			V4L2_CID_AUDIO_VOLUME, 0, 0x1f, 1, 0x1f);
> +	} else {
> +		/* install the em28xx notify callback */
> +		v4l2_ctrl_notify(v4l2_ctrl_find(hdl, V4L2_CID_AUDIO_MUTE),
> +				em28xx_ctrl_notify, dev);
> +		v4l2_ctrl_notify(v4l2_ctrl_find(hdl, V4L2_CID_AUDIO_VOLUME),
> +				em28xx_ctrl_notify, dev);
> +	}
> +
> +	/* wake i2c devices */
> +	em28xx_wake_i2c(dev);
> +
> +	/* init video dma queues */
> +	INIT_LIST_HEAD(&dev->vidq.active);
> +	INIT_LIST_HEAD(&dev->vbiq.active);
> +
> +	if (dev->board.has_msp34xx) {
> +		/* Send a reset to other chips via gpio */
> +		ret = em28xx_write_reg(dev, EM2820_R08_GPIO_CTRL, 0xf7);
> +		if (ret < 0) {
> +			em28xx_errdev("%s: em28xx_write_reg - msp34xx(1) failed! error [%d]\n",
> +				      __func__, ret);
> +			goto err;
> +		}
> +		msleep(3);
> +
> +		ret = em28xx_write_reg(dev, EM2820_R08_GPIO_CTRL, 0xff);
> +		if (ret < 0) {
> +			em28xx_errdev("%s: em28xx_write_reg - msp34xx(2) failed! error [%d]\n",
> +				      __func__, ret);
> +			goto err;
> +		}
> +		msleep(3);
> +	}
> +
>  	/* set default norm */
>  	dev->norm = V4L2_STD_PAL;
>  	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std, dev->norm);
> @@ -1888,14 +1956,16 @@ int em28xx_register_analog_devices(struct em28xx *dev)
>  	/* Reset image controls */
>  	em28xx_colorlevels_set_default(dev);
>  	v4l2_ctrl_handler_setup(&dev->ctrl_handler);
> -	if (dev->ctrl_handler.error)
> -		return dev->ctrl_handler.error;
> +	ret = dev->ctrl_handler.error;
> +	if (ret)
> +		goto err;
>  
>  	/* allocate and fill video video_device struct */
>  	dev->vdev = em28xx_vdev_init(dev, &em28xx_video_template, "video");
>  	if (!dev->vdev) {
>  		em28xx_errdev("cannot allocate video_device.\n");
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto err;
>  	}
>  	dev->vdev->queue = &dev->vb_vidq;
>  	dev->vdev->queue->lock = &dev->vb_queue_lock;
> @@ -1925,7 +1995,7 @@ int em28xx_register_analog_devices(struct em28xx *dev)
>  	if (ret) {
>  		em28xx_errdev("unable to register video device (error=%i).\n",
>  			      ret);
> -		return ret;
> +		goto err;
>  	}
>  
>  	/* Allocate and fill vbi video_device struct */
> @@ -1954,7 +2024,7 @@ int em28xx_register_analog_devices(struct em28xx *dev)
>  					    vbi_nr[dev->devno]);
>  		if (ret < 0) {
>  			em28xx_errdev("unable to register vbi device\n");
> -			return ret;
> +			goto err;
>  		}
>  	}
>  
> @@ -1963,13 +2033,14 @@ int em28xx_register_analog_devices(struct em28xx *dev)
>  						  "radio");
>  		if (!dev->radio_dev) {
>  			em28xx_errdev("cannot allocate video_device.\n");
> -			return -ENODEV;
> +			ret = -ENODEV;
> +			goto err;
>  		}
>  		ret = video_register_device(dev->radio_dev, VFL_TYPE_RADIO,
>  					    radio_nr[dev->devno]);
>  		if (ret < 0) {
>  			em28xx_errdev("can't register radio device\n");
> -			return ret;
> +			goto err;
>  		}
>  		em28xx_info("Registered radio device as %s\n",
>  			    video_device_node_name(dev->radio_dev));
> @@ -1982,5 +2053,33 @@ int em28xx_register_analog_devices(struct em28xx *dev)
>  		em28xx_info("V4L2 VBI device registered as %s\n",
>  			    video_device_node_name(dev->vbi_dev));
>  
> -	return 0;
> +	/* Save some power by putting tuner to sleep */
> +	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_power, 0);
> +
> +	/* initialize videobuf2 stuff */
> +	em28xx_vb2_setup(dev);
Are you 100% sure it is safe to do this that late now ?

> +
> +err:
> +	mutex_unlock(&dev->lock);
> +	return ret;
>  }
> +
> +static struct em28xx_ops v4l2_ops = {
> +	.id   = EM28XX_V4L2,
> +	.name = "Em28xx v4l2 Extension",
> +	.init = em28xx_v4l2_init,
> +	.fini = em28xx_v4l2_fini,
> +};
> +
> +static int __init em28xx_dvb_register(void)
> +{
> +	return em28xx_register_extension(&v4l2_ops);
> +}
> +
> +static void __exit em28xx_dvb_unregister(void)
> +{
> +	em28xx_unregister_extension(&v4l2_ops);
> +}
> +
> +module_init(em28xx_dvb_register);
> +module_exit(em28xx_dvb_unregister);
Should be em28xx_v4l2_register and em28xx_v4l2_unregister.

> diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
> index 191ef3593891..bc37d4833dbe 100644
> --- a/drivers/media/usb/em28xx/em28xx.h
> +++ b/drivers/media/usb/em28xx/em28xx.h
> @@ -26,6 +26,8 @@
>  #ifndef _EM28XX_H
>  #define _EM28XX_H
>  
> +#define EM28XX_VERSION "0.2.1"
> +
>  #include <linux/workqueue.h>
>  #include <linux/i2c.h>
>  #include <linux/mutex.h>
> @@ -472,6 +474,7 @@ struct em28xx_eeprom {
>  #define EM28XX_AUDIO   0x10
>  #define EM28XX_DVB     0x20
>  #define EM28XX_RC      0x30
> +#define EM28XX_V4L2    0x40
Reorder ?

>  /* em28xx resource types (used for res_get/res_lock etc */
>  #define EM28XX_RESOURCE_VIDEO 0x01
> @@ -522,9 +525,13 @@ struct em28xx {
>  	int model;		/* index in the device_data struct */
>  	int devno;		/* marks the number of this device */
>  	enum em28xx_chip_id chip_id;
> -	unsigned int is_em25xx:1;	/* em25xx/em276x/7x/8x family bridge */
>  
> +	unsigned int is_em25xx:1;	/* em25xx/em276x/7x/8x family bridge */
>  	unsigned char disconnected:1;	/* device has been diconnected */
> +	unsigned int has_video:1;
> +	unsigned int has_audio_class:1;
> +	unsigned int has_alsa_audio:1;
> +	unsigned int is_audio_only:1;
>  
>  	int audio_ifnum;
>  
> @@ -544,10 +551,6 @@ struct em28xx {
>  	/* Vinmode/Vinctl used at the driver */
>  	int vinmode, vinctl;
>  
> -	unsigned int has_audio_class:1;
> -	unsigned int has_alsa_audio:1;
> -	unsigned int is_audio_only:1;
> -
>  	/* Controls audio streaming */
>  	struct work_struct wq_trigger;	/* Trigger to start/stop audio for alsa module */
>  	atomic_t       stream_started;	/* stream should be running if true */
> @@ -748,10 +751,6 @@ void em28xx_init_extension(struct em28xx *dev);
>  void em28xx_close_extension(struct em28xx *dev);
>  
>  /* Provided by em28xx-video.c */
> -int em28xx_vb2_setup(struct em28xx *dev);
> -int em28xx_register_analog_devices(struct em28xx *dev);
> -void em28xx_release_analog_resources(struct em28xx *dev);
> -void em28xx_ctrl_notify(struct v4l2_ctrl *ctrl, void *priv);
>  int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count);
>  int em28xx_stop_vbi_streaming(struct vb2_queue *vq);
>  extern const struct v4l2_ctrl_ops em28xx_ctrl_ops;

I'll be glad to test the changes with the hardware I have once the
changed code compiles.

Regards,
Frank