Em Sun, 05 Jan 2014 10:56:33 -0200
Mauro Carvalho Chehab <m.che...@samsung.com> escreveu:
> Em Sun, 05 Jan 2014 11:47:00 +0100
> Frank Schäfer <fschaefer....@googlemail.com> escreveu:
>
> > Am 04.01.2014 11:55, schrieb Mauro Carvalho Chehab:
> > > Now that all analog-specific code are at em28xx-video, convert
> > > it into an em28xx extension and load it as a separate module.
> > >
> > > Signed-off-by: Mauro Carvalho Chehab <m.che...@samsung.com>
> > > ---
> > > drivers/media/usb/em28xx/Kconfig | 6 ++-
> > > drivers/media/usb/em28xx/Makefile | 5 ++-
> > > drivers/media/usb/em28xx/em28xx-camera.c | 1 +
> > > drivers/media/usb/em28xx/em28xx-cards.c | 45 ++++---------------
> > > drivers/media/usb/em28xx/em28xx-core.c | 12 ++++++
> > > drivers/media/usb/em28xx/em28xx-v4l.h | 20 +++++++++
> > > drivers/media/usb/em28xx/em28xx-vbi.c | 1 +
> > > drivers/media/usb/em28xx/em28xx-video.c | 74
> > > +++++++++++++++++++++++---------
> > > drivers/media/usb/em28xx/em28xx.h | 23 ++--------
> > > 9 files changed, 107 insertions(+), 80 deletions(-)
> > > create mode 100644 drivers/media/usb/em28xx/em28xx-v4l.h
> > >
> > > 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..3f850d5063d0 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-camera.o
> > >
> > > +em28xx-v4l-objs := em28xx-video.o em28xx-vbi.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-camera.c
> > > b/drivers/media/usb/em28xx/em28xx-camera.c
> > > index d666741797d4..c29f5c4e7b40 100644
> > > --- a/drivers/media/usb/em28xx/em28xx-camera.c
> > > +++ b/drivers/media/usb/em28xx/em28xx-camera.c
> > > @@ -454,3 +454,4 @@ int em28xx_init_camera(struct em28xx *dev)
> > >
> > > return ret;
> > > }
> > > +EXPORT_SYMBOL_GPL(em28xx_init_camera);
> > em28xx-camera should also be part of the em28xx-v4l module.
>
> I tried that. Moving em28xx-camera to em28xx-v4l would cause a recursive
> dependency, due to the camera probing logic, that should be called by
> em28xx-cards.c.
>
> Moving that probing part to em28xx-v4l is too complex, due to the code
> that detects the board.
>
> One solution would be to break em28xx-camera into two parts: the detection
> code, to be merged with the core, and the sensor part, to be merged with
> em28xx-v4l, but that sounds ugly.
>
> Other solution would be to use something like the dvb_attach() macro,
> to avoid the recursive dependency, but that would be a dirty solution.
>
> As the code there is not big, the amount of overhead of having everything
> at em28xx-core is not high. So, I opted to keep the simplest path here,
> avoiding the risk of breaking something with a complex changeset.
>
> > > diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
> > > b/drivers/media/usb/em28xx/em28xx-cards.c
> > > index 175cd776e0a1..938daaabd8e0 100644
> > > --- a/drivers/media/usb/em28xx/em28xx-cards.c
> > > +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> > > @@ -2159,6 +2159,8 @@ struct em28xx_board em28xx_boards[] = {
> > > .ir_codes = RC_MAP_PINNACLE_PCTV_HD,
> > > },
> > > };
> > > +EXPORT_SYMBOL_GPL(em28xx_boards);
> > > +
> > > const unsigned int em28xx_bcount = ARRAY_SIZE(em28xx_boards);
> > >
> > > /* table of devices that work with this driver */
> > > @@ -2827,10 +2829,6 @@ static void em28xx_card_setup(struct em28xx *dev)
> > > "tuner", dev->tuner_addr, NULL);
> > > }
> > > }
> > > -
> > > - em28xx_tuner_setup(dev);
> > > -
> > > - em28xx_init_camera(dev);
> > > }
> > Here you are fixing half of the em28xx_card_setup() oopses which you've
> > introduced with patch 3/22.
> > This needs to be done together with patch 5/22 before patch 3/22.
>
> Ok.
>
> > > @@ -2848,11 +2846,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-v4l");
> > > 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 ||
> > > @@ -2881,18 +2880,12 @@ 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);
> > > if (dev->clk)
> > > v4l2_clk_unregister_fixed(dev->clk);
> > >
> > > - v4l2_ctrl_handler_free(&dev->ctrl_handler);
> > > -
> > > - v4l2_device_unregister(&dev->v4l2_dev);
> > > -
> > > usb_put_dev(dev->udev);
> > >
> > > /* Mark device as unused */
> > > @@ -3043,7 +3036,7 @@ static int em28xx_init_dev(struct em28xx *dev,
> > > struct usb_device *udev,
> > > if (retval < 0) {
> > > em28xx_errdev("%s: em28xx_i2c_register bus 0 - error [%d]!\n",
> > > __func__, retval);
> > > - goto unregister_dev;
> > > + return retval;
> > > }
> > >
> > > /* register i2c bus 1 */
> > > @@ -3057,30 +3050,14 @@ static int em28xx_init_dev(struct em28xx *dev,
> > > struct usb_device *udev,
> > > if (retval < 0) {
> > > em28xx_errdev("%s: em28xx_i2c_register bus 1 - error
> > > [%d]!\n",
> > > __func__, retval);
> > > - goto unregister_dev;
> > > + return retval;
> > > }
> > > }
> > Hmm... if registering of bus 1 fails, we need to unregister bus 0.
> > But that's an old bug which we can fix later...
>
> Good point. Yes, this should be on a separate patch.
Patch sent.
>
> > > /* Do board specific init and eeprom reading */
> > > em28xx_card_setup(dev);
> > >
> > > - retval = em28xx_register_analog_devices(dev);
> > > - if (retval < 0) {
> > > - goto fail;
> > > - }
> > > -
> > > return 0;
> > > -
> > > -fail:
> > > - if (dev->def_i2c_bus)
> > > - em28xx_i2c_unregister(dev, 1);
> > > - em28xx_i2c_unregister(dev, 0);
> > > - v4l2_ctrl_handler_free(&dev->ctrl_handler);
> > > -
> > > -unregister_dev:
> > > - v4l2_device_unregister(&dev->v4l2_dev);
> > > -
> > > - return retval;
> > > }
> > >
> > > /* high bandwidth multiplier, as encoded in highspeed endpoint
> > > descriptors */
> > > @@ -3283,6 +3260,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 */
> > > @@ -3322,10 +3300,9 @@ static int em28xx_usb_probe(struct usb_interface
> > > *interface,
> > >
> > > /* 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) {
> > > @@ -3368,7 +3345,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;
> > > }
> > > }
> > >
> > > @@ -3377,13 +3354,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 3012912d2997..1113d4e107d8 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 <cave...@sssup.it>, " \
> > > + "Markus Rechberger <mrechber...@gmail.com>, " \
> > > + "Mauro Carvalho Chehab <mche...@infradead.org>, " \
> > > + "Sascha Sommer <saschasom...@freenet.de>"
> > > +
> > > +#define DRIVER_DESC "Empia em28xx based USB core driver"
> > > +
> > > +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-v4l.h
> > > b/drivers/media/usb/em28xx/em28xx-v4l.h
> > > new file mode 100644
> > > index 000000000000..bce438691e0e
> > > --- /dev/null
> > > +++ b/drivers/media/usb/em28xx/em28xx-v4l.h
> > > @@ -0,0 +1,20 @@
> > > +/*
> > > + em28xx-video.c - driver for Empia EM2800/EM2820/2840 USB
> > > + video capture devices
> > The information about supported chips is outdated everywhere in the driver,
> > but if you introduce a new header file it should really be up to date.
> > Just talk about EM27xx/EM28xx.
>
> We need to do a cleanup on all those headers: they don't follow Kernel
> CodingStyle and the old headers use my previous email.
>
> For the sake of keeping a coherency, I deliberately opted to just use the
> same way as the other headers.
>
> My plan is to write a patch series fixing this and other CodingStyle
> issues on em28xx after merging this changeset.
>
> > > +
> > > + Copyright (C) 2013-2014 Mauro Carvalho Chehab <m.che...@samsung.com>
> > > +
> > > + This program is free software; you can redistribute it and/or modify
> > > + it under the terms of the GNU General Public License as published by
> > > + the Free Software Foundation version 2 of the License.
> > > +
> > > + This program is distributed in the hope that it will be useful,
> > > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > + GNU General Public License for more details.
> > > + */
> > > +
> > > +
> > > +int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int
> > > count);
> > > +int em28xx_stop_vbi_streaming(struct vb2_queue *vq);
> > > +extern struct vb2_ops em28xx_vbi_qops;
> > > diff --git a/drivers/media/usb/em28xx/em28xx-vbi.c
> > > b/drivers/media/usb/em28xx/em28xx-vbi.c
> > > index 39f39c527c13..db3d655600df 100644
> > > --- a/drivers/media/usb/em28xx/em28xx-vbi.c
> > > +++ b/drivers/media/usb/em28xx/em28xx-vbi.c
> > > @@ -27,6 +27,7 @@
> > > #include <linux/init.h>
> > >
> > > #include "em28xx.h"
> > > +#include "em28xx-v4l.h"
> > >
> > > static unsigned int vbibufs = 5;
> > > module_param(vbibufs, int, 0644);
> > > diff --git a/drivers/media/usb/em28xx/em28xx-video.c
> > > b/drivers/media/usb/em28xx/em28xx-video.c
> > > index 85284626dbd6..d615bff8ac09 100644
> > > --- a/drivers/media/usb/em28xx/em28xx-video.c
> > > +++ b/drivers/media/usb/em28xx/em28xx-video.c
> > > @@ -38,6 +38,7 @@
> > > #include <linux/slab.h>
> > >
> > > #include "em28xx.h"
> > > +#include "em28xx-v4l.h"
> > > #include <media/v4l2-common.h>
> > > #include <media/v4l2-ioctl.h>
> > > #include <media/v4l2-event.h>
> > > @@ -141,11 +142,13 @@ static struct em28xx_fmt format[] = {
> > > },
> > > };
> > >
> > > +static int em28xx_vbi_supported(struct em28xx *dev);
> > > +
> > Or move em28xx_vbi_supported() before em28xx_set_outfmt() and
> > em28xx_resolution_set() again.
> > If you had not changed the functions order in patch 1/22, this wouldn't
> > be necessary.
>
> True. I'll put an extra cleanup patch to remove this reference on the
> CodingStyle cleanup patchset I'm planning to do.
Ok, I changed the order on patch 1/22, so this line was removed.
>
> > > /*
> > > * em28xx_wake_i2c()
> > > * configure i2c attached devices
> > > */
> > > -void em28xx_wake_i2c(struct em28xx *dev)
> > > +static void em28xx_wake_i2c(struct em28xx *dev)
> > > {
> > > v4l2_device_call_all(&dev->v4l2_dev, 0, core, reset, 0);
> > > v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing,
> > > @@ -153,7 +156,7 @@ void em28xx_wake_i2c(struct em28xx *dev)
> > > v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
> > > }
> > >
> > > -int em28xx_colorlevels_set_default(struct em28xx *dev)
> > > +static int em28xx_colorlevels_set_default(struct em28xx *dev)
> > > {
> > > em28xx_write_reg(dev, EM28XX_R20_YGAIN, CONTRAST_DEFAULT);
> > > em28xx_write_reg(dev, EM28XX_R21_YOFFSET, BRIGHTNESS_DEFAULT);
> > > @@ -171,7 +174,7 @@ int em28xx_colorlevels_set_default(struct em28xx *dev)
> > > return em28xx_write_reg(dev, EM28XX_R1A_BOFFSET, 0x00);
> > > }
> > >
> > > -int em28xx_set_outfmt(struct em28xx *dev)
> > > +static int em28xx_set_outfmt(struct em28xx *dev)
> > > {
> > > int ret;
> > > u8 fmt, vinctrl;
> > > @@ -278,7 +281,7 @@ static int em28xx_scaler_set(struct em28xx *dev, u16
> > > h, u16 v)
> > > }
> > >
> > > /* FIXME: this only function read values from dev */
> > > -int em28xx_resolution_set(struct em28xx *dev)
> > > +static int em28xx_resolution_set(struct em28xx *dev)
> > > {
> > > int width, height;
> > > width = norm_maxw(dev);
> > > @@ -310,7 +313,7 @@ int em28xx_resolution_set(struct em28xx *dev)
> > > return em28xx_scaler_set(dev, dev->hscale, dev->vscale);
> > > }
> > >
> > > -int em28xx_vbi_supported(struct em28xx *dev)
> > > +static int em28xx_vbi_supported(struct em28xx *dev)
> > > {
> > > /* Modprobe option to manually disable */
> > > if (disable_vbi == 1)
> > > @@ -330,7 +333,7 @@ int em28xx_vbi_supported(struct em28xx *dev)
> > > }
> > >
> > > /* Set USB alternate setting for analog video */
> > > -int em28xx_set_alternate(struct em28xx *dev)
> > > +static int em28xx_set_alternate(struct em28xx *dev)
> > > {
> > > int errCode;
> > > int i;
> > > @@ -1020,7 +1023,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;
> > > @@ -1088,7 +1091,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;
> > >
> > > @@ -1625,7 +1628,7 @@ static int vidioc_g_register(struct file *file,
> > > void *priv,
> > > reg->val = ret;
> > > } else {
> > > __le16 val = 0;
> > > - ret = em28xx_read_reg_req_len(dev, USB_REQ_GET_STATUS,
> > > + ret = dev->em28xx_read_reg_req_len(dev, USB_REQ_GET_STATUS,
> > > reg->reg, (char *)&val, 2);
> > Urgh... is it really necessary to start using these pointers again ?
> > Ok... keep it as is, I'll send a patch to clean this up later.
>
> This is one of the few places where we weren't using those pointers:
>
> $ git grep em28xx_read_reg_req_len drivers/media/usb/em28xx/
> drivers/media/usb/em28xx/em28xx-cards.c: dev->em28xx_read_reg_req_len =
> em28xx_read_reg_req_len;
> drivers/media/usb/em28xx/em28xx-core.c:int em28xx_read_reg_req_len(struct
> em28xx *dev, u8 req, u16 reg,
> drivers/media/usb/em28xx/em28xx-core.c: ret = em28xx_read_reg_req_len(dev,
> req, reg, &val, 1);
> drivers/media/usb/em28xx/em28xx-core.c: ret =
> dev->em28xx_read_reg_req_len(dev, 0, EM28XX_R40_AC97LSB,
> drivers/media/usb/em28xx/em28xx-i2c.c: ret =
> dev->em28xx_read_reg_req_len(dev, 0x00, 4-len, buf2, len);
> drivers/media/usb/em28xx/em28xx-i2c.c: ret =
> dev->em28xx_read_reg_req_len(dev, 2, addr, buf, len);
> drivers/media/usb/em28xx/em28xx-i2c.c: ret =
> dev->em28xx_read_reg_req_len(dev, 0x06, addr, buf, len);
> drivers/media/usb/em28xx/em28xx-input.c: rc =
> dev->em28xx_read_reg_req_len(dev, 0, EM28XX_R45_IR,
> drivers/media/usb/em28xx/em28xx-input.c: rc =
> dev->em28xx_read_reg_req_len(dev, 0, EM2874_R51_IR,
> drivers/media/usb/em28xx/em28xx-video.c: ret =
> dev->em28xx_read_reg_req_len(dev, USB_REQ_GET_STATUS,
> drivers/media/usb/em28xx/em28xx.h: int (*em28xx_read_reg_req_len)
> (struct em28xx *dev, u8 req, u16 reg,
> drivers/media/usb/em28xx/em28xx.h:int em28xx_read_reg_req_len(struct em28xx
> *dev, u8 req, u16 reg,
>
> For the sake of coherency, we should either use one or the other way.
>
> >
> > > if (ret < 0)
> > > return ret;
> > > @@ -1872,11 +1875,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 */
> > > @@ -1906,6 +1909,8 @@ void em28xx_release_analog_resources(struct em28xx
> > > *dev)
> > > video_device_release(dev->vdev);
> > > dev->vdev = NULL;
> > > }
> > > +
> > > + return 0;
> > > }
> > >
> > > /*
> > > @@ -1927,12 +1932,12 @@ static int em28xx_v4l2_close(struct file *filp)
> > > if (dev->users == 1) {
> > > /* the device is already disconnect,
> > > free the remaining resources */
> > > +
> > > if (dev->disconnected) {
> > > - em28xx_release_resources(dev);
> > Who releases the resources now ?
>
> This is tricky.
>
> The hole idea is that em28xx-v4l release the resources allocated for V4L
> only, and not all resources, the same way as the other em28xx modules
> do.
>
> If you take a look at the original em28xx_release_resources(), what it does
> is to call this function, and then to de-allocate and unregister v4l2:
>
> v4l2_ctrl_handler_free(&dev->ctrl_handler);
> v4l2_device_unregister(&dev->v4l2_dev);
>
> after that, it deallocates the rest of allocated data.
>
> Now, em28xx_release_resources() is only called when em28xx is removed
> from memory or when the device is removed.
>
> The code there first calls em28xx_close_extension(dev). So, at device
> removal, em28xx-v4l will release the V4L2 specific resources, and
> em28xx_release_resources() will drop the common dev struct.
>
> I suspect that it might still be a bug there, but, if so, this bug is
> also present on em28xx-dvb, em28xx-alsa and em28xx-rc: what happens if
> the device is removed while the file descriptors is still opened?
>
> Maybe the driver core already prevent such things, but I'm not sure.
>
> If there's a bug out there, the proper fix seems to use kref for
> struct em28xx, increasing refcount for every em28xx extension and/or
> file open, decreasing it at either extension fini call, or at file close.
>
> This way, em28xx_release_resources() would be called only after refcount
> reaching zero, e. g. after being sure that nobody is using it.
>
> My plan is to take a look on it after having this changeset merged,
> as such change, if needed, will be complex and would require lots of
> testing. Also, it is independent on those changes.
>
> >
> > > + v4l2_ctrl_handler_free(&dev->ctrl_handler);
> > > + v4l2_device_unregister(&dev->v4l2_dev);
> > > kfree(dev->alt_max_pkt_size_isoc);
> > > - mutex_unlock(&dev->lock);
> > > - kfree(dev);
> > > - return 0;
> > > + goto exit;
> > > }
> > >
> > > /* Save some power by putting tuner to sleep */
> > > @@ -1951,6 +1956,7 @@ static int em28xx_v4l2_close(struct file *filp)
> > > }
> > > }
> > >
> > > +exit:
> > > dev->users--;
> > Nice bugfix.
> >
> > > mutex_unlock(&dev->lock);
> > > return 0;
> > > @@ -2047,8 +2053,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)
> > > @@ -2122,7 +2126,7 @@ static void em28xx_setup_xc3028(struct em28xx *dev,
> > > struct xc2028_ctrl *ctl)
> > > }
> > > }
> > >
> > > -void em28xx_tuner_setup(struct em28xx *dev)
> > > +static void em28xx_tuner_setup(struct em28xx *dev)
> > > {
> > > struct tuner_setup tun_setup;
> > > struct v4l2_frequency f;
> > > @@ -2181,14 +2185,14 @@ void em28xx_tuner_setup(struct em28xx *dev)
> > > v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, &f);
> > > }
> > >
> > > -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->is_audio_only) {
> > > + if (!dev->has_video) {
> > > /* This device does not support the v4l2 extension */
> > > return 0;
> > > }
> > > @@ -2196,6 +2200,8 @@ int em28xx_register_analog_devices(struct em28xx
> > > *dev)
> > > 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");
> > > @@ -2212,6 +2218,10 @@ int em28xx_register_analog_devices(struct em28xx
> > > *dev)
> > > dev->vinctl = EM28XX_VINCTRL_INTERLACED |
> > > EM28XX_VINCTRL_CCIR656_ENABLE;
> > >
> > > + /* Initialize tuner and camera */
> > > + em28xx_tuner_setup(dev);
> > > + em28xx_init_camera(dev);
> > > +
> > > /* Configure audio */
> > > ret = em28xx_audio_setup(dev);
> > > if (ret < 0) {
> > > @@ -2422,6 +2432,28 @@ int em28xx_register_analog_devices(struct em28xx
> > > *dev)
> > >
> > > /* initialize videobuf2 stuff */
> > > em28xx_vb2_setup(dev);
> > > +
> > > err:
> > > - return 0;
> > > + 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_video_register(void)
> > > +{
> > > + return em28xx_register_extension(&v4l2_ops);
> > > }
> > > +
> > > +static void __exit em28xx_video_unregister(void)
> > > +{
> > > + em28xx_unregister_extension(&v4l2_ops);
> > > +}
> > > +
> > > +module_init(em28xx_video_register);
> > > +module_exit(em28xx_video_unregister);
> > > diff --git a/drivers/media/usb/em28xx/em28xx.h
> > > b/drivers/media/usb/em28xx/em28xx.h
> > > index 7ae05ebc13c1..9d6f43e4681f 100644
> > > --- a/drivers/media/usb/em28xx/em28xx.h
> > > +++ b/drivers/media/usb/em28xx/em28xx.h
> > > @@ -26,7 +26,7 @@
> > > #ifndef _EM28XX_H
> > > #define _EM28XX_H
> > >
> > > -#define EM28XX_VERSION "0.2.0"
> > > +#define EM28XX_VERSION "0.2.1"
> > >
> > > #include <linux/workqueue.h>
> > > #include <linux/i2c.h>
> > > @@ -474,6 +474,7 @@ struct em28xx_eeprom {
> > > #define EM28XX_AUDIO 0x10
> > > #define EM28XX_DVB 0x20
> > > #define EM28XX_RC 0x30
> > > +#define EM28XX_V4L2 0x40
> > >
> > > /* em28xx resource types (used for res_get/res_lock etc */
> > > #define EM28XX_RESOURCE_VIDEO 0x01
> > > @@ -527,6 +528,7 @@ struct em28xx {
> > >
> > > 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;
> > > @@ -723,14 +725,9 @@ int em28xx_write_ac97(struct em28xx *dev, u8 reg,
> > > u16 val);
> > > int em28xx_audio_analog_set(struct em28xx *dev);
> > > int em28xx_audio_setup(struct em28xx *dev);
> > >
> > > -int em28xx_colorlevels_set_default(struct em28xx *dev);
> > > const struct em28xx_led *em28xx_find_led(struct em28xx *dev,
> > > enum em28xx_led_role role);
> > > int em28xx_capture_start(struct em28xx *dev, int start);
> > > -int em28xx_vbi_supported(struct em28xx *dev);
> > > -int em28xx_set_outfmt(struct em28xx *dev);
> > > -int em28xx_resolution_set(struct em28xx *dev);
> > > -int em28xx_set_alternate(struct em28xx *dev);
> > > int em28xx_alloc_urbs(struct em28xx *dev, enum em28xx_mode mode, int
> > > xfer_bulk,
> > > int num_bufs, int max_pkt_size, int packet_multiplier);
> > > int em28xx_init_usb_xfer(struct em28xx *dev, enum em28xx_mode mode,
> > > @@ -742,31 +739,17 @@ void em28xx_uninit_usb_xfer(struct em28xx *dev,
> > > enum em28xx_mode mode);
> > > void em28xx_stop_urbs(struct em28xx *dev);
> > > int em28xx_set_mode(struct em28xx *dev, enum em28xx_mode set_mode);
> > > int em28xx_gpio_set(struct em28xx *dev, struct em28xx_reg_seq *gpio);
> > > -void em28xx_wake_i2c(struct em28xx *dev);
> > > int em28xx_register_extension(struct em28xx_ops *dev);
> > > void em28xx_unregister_extension(struct em28xx_ops *dev);
> > > void em28xx_init_extension(struct em28xx *dev);
> > > void em28xx_close_extension(struct em28xx *dev);
> > >
> > > -/* Provided by em28xx-video.c */
> > > -void em28xx_tuner_setup(struct em28xx *dev);
> > > -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;
> > > -
> > > /* Provided by em28xx-cards.c */
> > > extern struct em28xx_board em28xx_boards[];
> > > extern struct usb_device_id em28xx_id_table[];
> > > int em28xx_tuner_callback(void *ptr, int component, int command, int
> > > arg);
> > > void em28xx_release_resources(struct em28xx *dev);
> > >
> > > -/* Provided by em28xx-vbi.c */
> > > -extern struct vb2_ops em28xx_vbi_qops;
> > > -
> > > /* Provided by em28xx-camera.c */
> > > int em28xx_detect_sensor(struct em28xx *dev);
> > > int em28xx_init_camera(struct em28xx *dev);
> > Nice clean-up ! :)
>
> Thanks!
>
Ok, this is the new version of the patch. It is basically a rebase of the
previous one. Btw, I'm adding those patches on this tree:
http://git.linuxtv.org/mchehab/experimental.git/shortlog/refs/heads/em28xx-v4l2-v5
-
[media] em28xx: make em28xx-video to be a separate module
Now that all analog-specific code are at em28xx-video, convert
it into an em28xx extension and load it as a separate module.
Signed-off-by: Mauro Carvalho Chehab <m.che...@samsung.com>
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
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..3f850d5063d0 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-camera.o
+em28xx-v4l-objs := em28xx-video.o em28xx-vbi.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-camera.c
b/drivers/media/usb/em28xx/em28xx-camera.c
index d666741797d4..c29f5c4e7b40 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -454,3 +454,4 @@ int em28xx_init_camera(struct em28xx *dev)
return ret;
}
+EXPORT_SYMBOL_GPL(em28xx_init_camera);
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c
b/drivers/media/usb/em28xx/em28xx-cards.c
index dbce4dc421f9..b25869331284 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2159,6 +2159,8 @@ struct em28xx_board em28xx_boards[] = {
.ir_codes = RC_MAP_PINNACLE_PCTV_HD,
},
};
+EXPORT_SYMBOL_GPL(em28xx_boards);
+
const unsigned int em28xx_bcount = ARRAY_SIZE(em28xx_boards);
/* table of devices that work with this driver */
@@ -2780,11 +2782,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-v4l");
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 ||
@@ -2813,18 +2816,12 @@ 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);
if (dev->clk)
v4l2_clk_unregister_fixed(dev->clk);
- v4l2_ctrl_handler_free(&dev->ctrl_handler);
-
- v4l2_device_unregister(&dev->v4l2_dev);
-
usb_put_dev(dev->udev);
/* Mark device as unused */
@@ -2999,18 +2996,7 @@ static int em28xx_init_dev(struct em28xx *dev, struct
usb_device *udev,
/* Do board specific init and eeprom reading */
em28xx_card_setup(dev);
- retval = em28xx_register_analog_devices(dev);
- if (retval < 0)
- goto fail;
-
return 0;
-
-fail:
- if (dev->def_i2c_bus)
- em28xx_i2c_unregister(dev, 1);
- em28xx_i2c_unregister(dev, 0);
-
- return retval;
}
/* high bandwidth multiplier, as encoded in highspeed endpoint descriptors */
@@ -3213,6 +3199,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 */
@@ -3252,10 +3239,9 @@ static int em28xx_usb_probe(struct usb_interface
*interface,
/* 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) {
@@ -3298,7 +3284,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;
}
}
@@ -3307,13 +3293,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 f77301773aee..c416dd33af3f 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 <cave...@sssup.it>, " \
+ "Markus Rechberger <mrechber...@gmail.com>, " \
+ "Mauro Carvalho Chehab <mche...@infradead.org>, " \
+ "Sascha Sommer <saschasom...@freenet.de>"
+
+#define DRIVER_DESC "Empia em28xx based USB core driver"
+
+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-v4l.h
b/drivers/media/usb/em28xx/em28xx-v4l.h
new file mode 100644
index 000000000000..bce438691e0e
--- /dev/null
+++ b/drivers/media/usb/em28xx/em28xx-v4l.h
@@ -0,0 +1,20 @@
+/*
+ em28xx-video.c - driver for Empia EM2800/EM2820/2840 USB
+ video capture devices
+
+ Copyright (C) 2013-2014 Mauro Carvalho Chehab <m.che...@samsung.com>
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation version 2 of the License.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+ */
+
+
+int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count);
+int em28xx_stop_vbi_streaming(struct vb2_queue *vq);
+extern struct vb2_ops em28xx_vbi_qops;
diff --git a/drivers/media/usb/em28xx/em28xx-vbi.c
b/drivers/media/usb/em28xx/em28xx-vbi.c
index 39f39c527c13..db3d655600df 100644
--- a/drivers/media/usb/em28xx/em28xx-vbi.c
+++ b/drivers/media/usb/em28xx/em28xx-vbi.c
@@ -27,6 +27,7 @@
#include <linux/init.h>
#include "em28xx.h"
+#include "em28xx-v4l.h"
static unsigned int vbibufs = 5;
module_param(vbibufs, int, 0644);
diff --git a/drivers/media/usb/em28xx/em28xx-video.c
b/drivers/media/usb/em28xx/em28xx-video.c
index 3726af134f39..78a1bfb97c3d 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -38,6 +38,7 @@
#include <linux/slab.h>
#include "em28xx.h"
+#include "em28xx-v4l.h"
#include <media/v4l2-common.h>
#include <media/v4l2-ioctl.h>
#include <media/v4l2-event.h>
@@ -141,7 +142,7 @@ static struct em28xx_fmt format[] = {
},
};
-int em28xx_vbi_supported(struct em28xx *dev)
+static int em28xx_vbi_supported(struct em28xx *dev)
{
/* Modprobe option to manually disable */
if (disable_vbi == 1)
@@ -164,7 +165,7 @@ int em28xx_vbi_supported(struct em28xx *dev)
* em28xx_wake_i2c()
* configure i2c attached devices
*/
-void em28xx_wake_i2c(struct em28xx *dev)
+static void em28xx_wake_i2c(struct em28xx *dev)
{
v4l2_device_call_all(&dev->v4l2_dev, 0, core, reset, 0);
v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing,
@@ -172,7 +173,7 @@ void em28xx_wake_i2c(struct em28xx *dev)
v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
}
-int em28xx_colorlevels_set_default(struct em28xx *dev)
+static int em28xx_colorlevels_set_default(struct em28xx *dev)
{
em28xx_write_reg(dev, EM28XX_R20_YGAIN, CONTRAST_DEFAULT);
em28xx_write_reg(dev, EM28XX_R21_YOFFSET, BRIGHTNESS_DEFAULT);
@@ -190,7 +191,7 @@ int em28xx_colorlevels_set_default(struct em28xx *dev)
return em28xx_write_reg(dev, EM28XX_R1A_BOFFSET, 0x00);
}
-int em28xx_set_outfmt(struct em28xx *dev)
+static int em28xx_set_outfmt(struct em28xx *dev)
{
int ret;
u8 fmt, vinctrl;
@@ -297,7 +298,7 @@ static int em28xx_scaler_set(struct em28xx *dev, u16 h, u16
v)
}
/* FIXME: this only function read values from dev */
-int em28xx_resolution_set(struct em28xx *dev)
+static int em28xx_resolution_set(struct em28xx *dev)
{
int width, height;
width = norm_maxw(dev);
@@ -330,7 +331,7 @@ int em28xx_resolution_set(struct em28xx *dev)
}
/* Set USB alternate setting for analog video */
-int em28xx_set_alternate(struct em28xx *dev)
+static int em28xx_set_alternate(struct em28xx *dev)
{
int errCode;
int i;
@@ -1020,7 +1021,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;
@@ -1088,7 +1089,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;
@@ -1625,7 +1626,7 @@ static int vidioc_g_register(struct file *file, void
*priv,
reg->val = ret;
} else {
__le16 val = 0;
- ret = em28xx_read_reg_req_len(dev, USB_REQ_GET_STATUS,
+ ret = dev->em28xx_read_reg_req_len(dev, USB_REQ_GET_STATUS,
reg->reg, (char *)&val, 2);
if (ret < 0)
return ret;
@@ -1872,15 +1873,12 @@ 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 */
-
if (dev->radio_dev) {
if (video_is_registered(dev->radio_dev))
video_unregister_device(dev->radio_dev);
@@ -1906,6 +1904,8 @@ void em28xx_release_analog_resources(struct em28xx *dev)
video_device_release(dev->vdev);
dev->vdev = NULL;
}
+
+ return 0;
}
/*
@@ -1927,12 +1927,12 @@ static int em28xx_v4l2_close(struct file *filp)
if (dev->users == 1) {
/* the device is already disconnect,
free the remaining resources */
+
if (dev->disconnected) {
- em28xx_release_resources(dev);
+ v4l2_ctrl_handler_free(&dev->ctrl_handler);
+ v4l2_device_unregister(&dev->v4l2_dev);
kfree(dev->alt_max_pkt_size_isoc);
- mutex_unlock(&dev->lock);
- kfree(dev);
- return 0;
+ goto exit;
}
/* Save some power by putting tuner to sleep */
@@ -1951,6 +1951,7 @@ static int em28xx_v4l2_close(struct file *filp)
}
}
+exit:
dev->users--;
mutex_unlock(&dev->lock);
return 0;
@@ -2065,8 +2066,6 @@ static unsigned short msp3400_addrs[] = {
/******************************** usb interface ******************************/
-
-
static struct video_device *em28xx_vdev_init(struct em28xx *dev,
const struct video_device *template,
const char *type_name)
@@ -2140,7 +2139,7 @@ static void em28xx_setup_xc3028(struct em28xx *dev,
struct xc2028_ctrl *ctl)
}
}
-void em28xx_tuner_setup(struct em28xx *dev)
+static void em28xx_tuner_setup(struct em28xx *dev)
{
struct tuner_setup tun_setup;
struct v4l2_frequency f;
@@ -2199,14 +2198,14 @@ void em28xx_tuner_setup(struct em28xx *dev)
v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, &f);
}
-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->is_audio_only) {
+ if (!dev->has_video) {
/* This device does not support the v4l2 extension */
return 0;
}
@@ -2214,6 +2213,8 @@ int em28xx_register_analog_devices(struct em28xx *dev)
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");
@@ -2492,11 +2493,33 @@ int em28xx_register_analog_devices(struct em28xx *dev)
/* initialize videobuf2 stuff */
em28xx_vb2_setup(dev);
+ mutex_unlock(&dev->lock);
return 0;
unregister_dev:
v4l2_ctrl_handler_free(&dev->ctrl_handler);
v4l2_device_unregister(&dev->v4l2_dev);
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_video_register(void)
+{
+ return em28xx_register_extension(&v4l2_ops);
+}
+
+static void __exit em28xx_video_unregister(void)
+{
+ em28xx_unregister_extension(&v4l2_ops);
+}
+
+module_init(em28xx_video_register);
+module_exit(em28xx_video_unregister);
diff --git a/drivers/media/usb/em28xx/em28xx.h
b/drivers/media/usb/em28xx/em28xx.h
index 7ae05ebc13c1..9d6f43e4681f 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -26,7 +26,7 @@
#ifndef _EM28XX_H
#define _EM28XX_H
-#define EM28XX_VERSION "0.2.0"
+#define EM28XX_VERSION "0.2.1"
#include <linux/workqueue.h>
#include <linux/i2c.h>
@@ -474,6 +474,7 @@ struct em28xx_eeprom {
#define EM28XX_AUDIO 0x10
#define EM28XX_DVB 0x20
#define EM28XX_RC 0x30
+#define EM28XX_V4L2 0x40
/* em28xx resource types (used for res_get/res_lock etc */
#define EM28XX_RESOURCE_VIDEO 0x01
@@ -527,6 +528,7 @@ struct em28xx {
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;
@@ -723,14 +725,9 @@ int em28xx_write_ac97(struct em28xx *dev, u8 reg, u16 val);
int em28xx_audio_analog_set(struct em28xx *dev);
int em28xx_audio_setup(struct em28xx *dev);
-int em28xx_colorlevels_set_default(struct em28xx *dev);
const struct em28xx_led *em28xx_find_led(struct em28xx *dev,
enum em28xx_led_role role);
int em28xx_capture_start(struct em28xx *dev, int start);
-int em28xx_vbi_supported(struct em28xx *dev);
-int em28xx_set_outfmt(struct em28xx *dev);
-int em28xx_resolution_set(struct em28xx *dev);
-int em28xx_set_alternate(struct em28xx *dev);
int em28xx_alloc_urbs(struct em28xx *dev, enum em28xx_mode mode, int xfer_bulk,
int num_bufs, int max_pkt_size, int packet_multiplier);
int em28xx_init_usb_xfer(struct em28xx *dev, enum em28xx_mode mode,
@@ -742,31 +739,17 @@ void em28xx_uninit_usb_xfer(struct em28xx *dev, enum
em28xx_mode mode);
void em28xx_stop_urbs(struct em28xx *dev);
int em28xx_set_mode(struct em28xx *dev, enum em28xx_mode set_mode);
int em28xx_gpio_set(struct em28xx *dev, struct em28xx_reg_seq *gpio);
-void em28xx_wake_i2c(struct em28xx *dev);
int em28xx_register_extension(struct em28xx_ops *dev);
void em28xx_unregister_extension(struct em28xx_ops *dev);
void em28xx_init_extension(struct em28xx *dev);
void em28xx_close_extension(struct em28xx *dev);
-/* Provided by em28xx-video.c */
-void em28xx_tuner_setup(struct em28xx *dev);
-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;
-
/* Provided by em28xx-cards.c */
extern struct em28xx_board em28xx_boards[];
extern struct usb_device_id em28xx_id_table[];
int em28xx_tuner_callback(void *ptr, int component, int command, int arg);
void em28xx_release_resources(struct em28xx *dev);
-/* Provided by em28xx-vbi.c */
-extern struct vb2_ops em28xx_vbi_qops;
-
/* Provided by em28xx-camera.c */
int em28xx_detect_sensor(struct em28xx *dev);
int em28xx_init_camera(struct em28xx *dev);
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
