From patchwork Fri Mar 27 16:44:05 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexey Klimov X-Patchwork-Id: 14759 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n2RGgsv9021036 for ; Fri, 27 Mar 2009 16:42:54 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753606AbZC0Qmy (ORCPT ); Fri, 27 Mar 2009 12:42:54 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754064AbZC0Qmy (ORCPT ); Fri, 27 Mar 2009 12:42:54 -0400 Received: from mail-fx0-f158.google.com ([209.85.220.158]:64198 "EHLO mail-fx0-f158.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753606AbZC0Qmx (ORCPT ); Fri, 27 Mar 2009 12:42:53 -0400 Received: by fxm2 with SMTP id 2so1105002fxm.37 for ; Fri, 27 Mar 2009 09:42:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:subject:from:to:cc :in-reply-to:references:content-type:date:message-id:mime-version :x-mailer:content-transfer-encoding; bh=CmOG0zuvl5MwPQcK5Pio59VYVDpl6XDf75m5ODuF2V0=; b=aagkmz5FDhl5t2veSspsx+b1gV02/bDFnsKCQhHFrG0EF0xTuQGyLuifGFd3SiBg5b Q5FMLh8n4jt42wuo88rvgduFHYaygqL1LtzLpqUeLMx3i0vPgjviYYv+FITy9eAM/ko5 YYMmN4ObKgal7XUSG0//ja/QRfLKqyQ5hlViA= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=subject:from:to:cc:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; b=fpgA9bgDpKLInRMBiprLtC68Gwtyt4pl/B858z6C0l3x3Ieux4CMzc693VQf8g9mXg wCaP3vRZHeXnKpslaHh2QjSDb8vBzNk/lMaVcSZKATwcl1D6bRBcJk0c3pE+AQeWqKY/ P7bH5F0uZ9xgo1pLZgxu81JP/ww7wqrJdxt5c= Received: by 10.103.24.11 with SMTP id b11mr523468muj.76.1238172169603; Fri, 27 Mar 2009 09:42:49 -0700 (PDT) Received: from ?10.55.84.82? ([93.175.15.16]) by mx.google.com with ESMTPS id b9sm3280210mug.2.2009.03.27.09.42.48 (version=SSLv3 cipher=RC4-MD5); Fri, 27 Mar 2009 09:42:49 -0700 (PDT) Subject: Re: [question] about open/release and vidioc_g_input/vidioc_s_input functions From: Alexey Klimov To: Hans Verkuil Cc: linux-media@vger.kernel.org, Mauro Carvalho Chehab , Douglas Schilling Landgraf In-Reply-To: <200903240806.39540.hverkuil@xs4all.nl> References: <1237850047.31041.162.camel@tux.localhost> <200903240806.39540.hverkuil@xs4all.nl> Date: Fri, 27 Mar 2009 19:44:05 +0300 Message-Id: <1238172245.4200.10.camel@tux.localhost> Mime-Version: 1.0 X-Mailer: Evolution 2.24.4 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Hello, Hans On Tue, 2009-03-24 at 08:06 +0100, Hans Verkuil wrote: > On Tuesday 24 March 2009 00:14:07 Alexey Klimov wrote: > > Hello, all > > > > ... > > static int terratec_open(struct file *file) > > { > > return 0; > > } > > > > static int terratec_release(struct file *file) > > { > > return 0; > > } > > ... > > > > ... > > > > Such code used in many radio-drivers as i understand. > > > > Is it good to place this empty and almost empty functions in: > > (here i see two variants) > > > > 1) In header file that be in linux/drivers/media/radio/ directory. > > Later, we can move some generic/or repeating code in this header. > > > > 2) In any v4l header. What header may contain this ? > > > > ? > > > > For what ? Well, as i understand we can decrease amount of lines and > > provide this simple generic functions. It's like > > video_device_release_empty function behaviour. Maybe not only radio > > drivers can use such vidioc_g_input and vidioc_s_input. > > > > Is it worth ? > > I don't think it is worth doing this for g/s_input. I think it is useful to > have them here: it makes it very clear that there is just a single input > and the overhead in both lines and actual bytes is minimal. > > But for the empty open and release functions you could easily handle that in > v4l2-dev.c: if you leave the open and release callbacks to NULL, then > v4l2_open and v4l2_release can just return 0. That would be nice. > > Regards, > > Hans > May i ask help with this ? Hans, should it be looks like: ? Or in v4l2_open function i can check if vdev->fops->open == NULL before video_get(vdev); (increasing the device refcount), and if it's NULL then unlock_mutex and return 0 ? And the same in v4l2_release - just return 0 in the begining of function in case vdev->fops->release == NULL ? What approach is better ? Signed-off-by: Hans Verkuil diff -r 56cf0f1772f7 linux/drivers/media/radio/radio-terratec.c --- a/linux/drivers/media/radio/radio-terratec.c Mon Mar 23 19:18:34 2009 -0300 +++ b/linux/drivers/media/radio/radio-terratec.c Fri Mar 27 19:32:38 2009 +0300 @@ -333,20 +333,8 @@ return a->index ? -EINVAL : 0; } -static int terratec_open(struct file *file) -{ - return 0; -} - -static int terratec_release(struct file *file) -{ - return 0; -} - static const struct v4l2_file_operations terratec_fops = { .owner = THIS_MODULE, - .open = terratec_open, - .release = terratec_release, .ioctl = video_ioctl2, }; diff -r 56cf0f1772f7 linux/drivers/media/video/v4l2-dev.c --- a/linux/drivers/media/video/v4l2-dev.c Mon Mar 23 19:18:34 2009 -0300 +++ b/linux/drivers/media/video/v4l2-dev.c Fri Mar 27 19:32:38 2009 +0300 @@ -264,7 +264,10 @@ /* and increase the device refcount */ video_get(vdev); mutex_unlock(&videodev_lock); - ret = vdev->fops->open(filp); + if (vdev->fops->open == NULL) + ret = 0; + else + ret = vdev->fops->open(filp); /* decrease the refcount in case of an error */ if (ret) video_put(vdev); @@ -275,7 +278,12 @@ static int v4l2_release(struct inode *inode, struct file *filp) { struct video_device *vdev = video_devdata(filp); - int ret = vdev->fops->release(filp); + int ret; + + if (vdev->fops->release == NULL) + ret = 0; + else + ret = vdev->fops->release(filp); /* decrease the refcount unconditionally since the release() return value is ignored. */