From patchwork Sun Jan 5 15:18:52 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Mauro Carvalho Chehab X-Patchwork-Id: 3434631 Return-Path: X-Original-To: patchwork-linux-media@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 1FD2AC02DC for ; Sun, 5 Jan 2014 15:19:28 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D496220170 for ; Sun, 5 Jan 2014 15:19:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 540462016C for ; Sun, 5 Jan 2014 15:19:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751196AbaAEPTD (ORCPT ); Sun, 5 Jan 2014 10:19:03 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:47090 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751185AbaAEPTC convert rfc822-to-8bit (ORCPT ); Sun, 5 Jan 2014 10:19:02 -0500 Received: from [177.143.150.208] (helo=localhost.localdomain) by bombadil.infradead.org with esmtpsa (Exim 4.80.1 #2 (Red Hat Linux)) id 1VzpTc-0007AG-0J; Sun, 05 Jan 2014 15:18:57 +0000 Date: Sun, 5 Jan 2014 13:18:52 -0200 From: Mauro Carvalho Chehab To: Mauro Carvalho Chehab Cc: Frank =?UTF-8?B?U2Now6RmZXI=?= , Linux Media Mailing List Subject: Re: [PATCH v4 04/22] [media] em28xx: make em28xx-video to be a separate module Message-ID: <20140105131852.21bd3056@infradead.org> In-Reply-To: <20140105105633.12cd6e4b@samsung.com> References: <1388832951-11195-1-git-send-email-m.chehab@samsung.com> <1388832951-11195-5-git-send-email-m.chehab@samsung.com> <52C93824.1000206@googlemail.com> <20140105105633.12cd6e4b@samsung.com> X-Mailer: Claws Mail 3.9.2 (GTK+ 2.24.22; x86_64-redhat-linux-gnu) Mime-Version: 1.0 X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org See http://www.infradead.org/rpr.html Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Spam-Status: No, score=-7.0 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Em Sun, 05 Jan 2014 10:56:33 -0200 Mauro Carvalho Chehab escreveu: > Em Sun, 05 Jan 2014 11:47:00 +0100 > Frank Schäfer 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 > > > --- > > > 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 , " \ > > > + "Markus Rechberger , " \ > > > + "Mauro Carvalho Chehab , " \ > > > + "Sascha Sommer " > > > + > > > +#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 > > > + > > > + 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 > > > > > > #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 > > > > > > #include "em28xx.h" > > > +#include "em28xx-v4l.h" > > > #include > > > #include > > > #include > > > @@ -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 > > > #include > > > @@ -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 Reviewed-by: Frank Schäfer Tested-by: Frank Schäfer --- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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 , " \ + "Markus Rechberger , " \ + "Mauro Carvalho Chehab , " \ + "Sascha Sommer " + +#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 + + 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 #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 #include "em28xx.h" +#include "em28xx-v4l.h" #include #include #include @@ -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 #include @@ -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);