Re: [PATCH FOR v3.8] omap3isp: Don't include deleted OMAP plat/ header files
Hi Laurent, On Mon, Dec 31, 2012 at 01:26:33PM +0100, Laurent Pinchart wrote: The plat/iommu.h, plat/iovmm.h and plat/omap-pm.h have been deleted. Don't include them. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com I think I first confused this with the other patch with a similar subject... Acked-by: Sakari Ailus sakari.ai...@iki.fi --- drivers/media/platform/omap3isp/ispvideo.c |3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c index e0d73a6..8dac175 100644 --- a/drivers/media/platform/omap3isp/ispvideo.c +++ b/drivers/media/platform/omap3isp/ispvideo.c @@ -35,9 +35,6 @@ #include linux/vmalloc.h #include media/v4l2-dev.h #include media/v4l2-ioctl.h -#include plat/iommu.h -#include plat/iovmm.h -#include plat/omap-pm.h #include ispvideo.h #include isp.h -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- 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
Re: [PATCHv16 5/7] fbmon: add of_videomode helpers
Hi Afzal,
On Mon, Jan 07, 2013 at 06:10:13AM +, Mohammed, Afzal wrote:
Hi Steffen,
On Tue, Dec 18, 2012 at 22:34:14, Steffen Trumtrar wrote:
Add helper to get fb_videomode from devicetree.
drivers/video/fbmon.c | 42 ++
include/linux/fb.h|4
This breaks DaVinci (da8xx_omapl_defconfig), following change was
required to get it build if OF_VIDEOMODE or/and FB_MODE_HELPERS
is not defined. There may be better solutions, following was the
one that was used by me to test this series.
---8--
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 58b9860..0ce30d1 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -716,9 +716,19 @@ extern void fb_destroy_modedb(struct fb_videomode
*modedb);
extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
+#if defined(CONFIG_OF_VIDEOMODE) defined(CONFIG_FB_MODE_HELPERS)
extern int of_get_fb_videomode(struct device_node *np,
struct fb_videomode *fb,
int index);
+#else
+static inline int of_get_fb_videomode(struct device_node *np,
+ struct fb_videomode *fb,
+ int index)
+{
+ return -EINVAL;
+}
+#endif
+
extern int fb_videomode_from_videomode(const struct videomode *vm,
struct fb_videomode *fbmode);
---8--
I just did a quick make da8xx_omapl_defconfig make and it builds just fine.
On what version did you apply the series?
At the moment I have the series sitting on 3.7. Didn't try any 3.8-rcx yet.
But fixing this shouldn't be a problem.
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
As _OF_VIDEOMODE is a bool type CONFIG, isn't,
#ifdef CONFIG_OF_VIDEOMODE
sufficient ?
Yes, that is right. But I think IS_ENABLED is the preferred way to do it, isn't
it?
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
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
RE: [PATCHv16 5/7] fbmon: add of_videomode helpers
Hi Steffen, On Mon, Jan 07, 2013 at 13:36:48, Steffen Trumtrar wrote: On Mon, Jan 07, 2013 at 06:10:13AM +, Mohammed, Afzal wrote: This breaks DaVinci (da8xx_omapl_defconfig), following change was required to get it build if OF_VIDEOMODE or/and FB_MODE_HELPERS is not defined. There may be better solutions, following was the one that was used by me to test this series. I just did a quick make da8xx_omapl_defconfig make and it builds just fine. On what version did you apply the series? At the moment I have the series sitting on 3.7. Didn't try any 3.8-rcx yet. But fixing this shouldn't be a problem. You are right, me idiot, error will happen only upon try to make use of of_get_fb_videomode() (defined in this patch) in the da8xx-fb driver (with da8xx_omapl_defconfig), to be exact upon adding, video: da8xx-fb: obtain fb_videomode info from dt of my patch series. The change as I mentioned or something similar would be required as any driver that is going to make use of of_get_fb_videomode() would break if CONFIG_OF_VIDEOMODE or CONFIG_FB_MODE_HELPERS is not defined. And testing was done over v3.8-rc2. +#if IS_ENABLED(CONFIG_OF_VIDEOMODE) As _OF_VIDEOMODE is a bool type CONFIG, isn't, #ifdef CONFIG_OF_VIDEOMODE sufficient ? Yes, that is right. But I think IS_ENABLED is the preferred way to do it, isn't it? Now I realize it is. Regards Afzal
[PATCH 1/2] drivers/media/platform/soc_camera/pxa_camera.c: reposition free_irq to avoid access to invalid data
From: Julia Lawall julia.law...@lip6.fr
The data referenced by an interrupt handler should not be freed before the
interrupt is ended. The handler is pxa_camera_irq. This handler may call
pxa_dma_start_channels, which references the channels that are freed on the
lines before the call to free_irq.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// smpl
@fn exists@
expression list es;
expression a,b;
identifier f;
@@
if (...) {
... when any
free_irq(a,b);
... when any
f(es);
... when any
return ...;
}
@@
expression list fn.es;
expression fn.a,fn.b;
identifier fn.f;
@@
*f(es);
... when any
*free_irq(a,b);
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
Not compiled. I have not observed the problem in practice; the code just
looks suspicious.
drivers/media/platform/soc_camera/pxa_camera.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/soc_camera/pxa_camera.c
b/drivers/media/platform/soc_camera/pxa_camera.c
index f91f7bf..2a19aba 100644
--- a/drivers/media/platform/soc_camera/pxa_camera.c
+++ b/drivers/media/platform/soc_camera/pxa_camera.c
@@ -1810,10 +1810,10 @@ static int pxa_camera_remove(struct platform_device
*pdev)
clk_put(pcdev-clk);
+ free_irq(pcdev-irq, pcdev);
pxa_free_dma(pcdev-dma_chans[0]);
pxa_free_dma(pcdev-dma_chans[1]);
pxa_free_dma(pcdev-dma_chans[2]);
- free_irq(pcdev-irq, pcdev);
soc_camera_host_unregister(soc_host);
--
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
Re: [PATCH V4 2/3] sta2x11_vip: convert to videobuf2 and control framework
Hi Frederico!
Just one comment, see below:
On Sun January 6 2013 18:29:02 Federico Vaga wrote:
This patch re-write the driver and use the videobuf2
interface instead of the old videobuf. Moreover, it uses also
the control framework which allows the driver to inherit
controls from its subdevice (ADV7180)
Signed-off-by: Federico Vaga federico.v...@gmail.com
Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com
---
drivers/media/pci/sta2x11/Kconfig |2 +-
drivers/media/pci/sta2x11/sta2x11_vip.c | 1244
++-
2 file modificati, 414 inserzioni(+), 832 rimozioni(-)
diff --git a/drivers/media/pci/sta2x11/Kconfig
b/drivers/media/pci/sta2x11/Kconfig
index 6749f67..a94ccad 100644
--- a/drivers/media/pci/sta2x11/Kconfig
+++ b/drivers/media/pci/sta2x11/Kconfig
@@ -2,7 +2,7 @@ config STA2X11_VIP
tristate STA2X11 VIP Video For Linux
depends on STA2X11
select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
- select VIDEOBUF_DMA_CONTIG
+ select VIDEOBUF2_DMA_CONTIG
depends on PCI VIDEO_V4L2 VIRT_TO_BUS
help
Say Y for support for STA2X11 VIP (Video Input Port) capture
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index ed1337a..e379e03 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
snip
-/**
- * vidioc_try_fmt_vid_cap - set video capture format
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @f: new format
- *
- * new video format is set which includes width and
- * field type. width is fixed to 720, no scaling.
- * Only UYVY is supported by this hardware.
- * the minimum height is 200, the maximum is 576 (PAL)
- *
- * return value: 0, no error
- *
- * -EINVAL, pixel or field format not supported
- *
- */
static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
struct v4l2_format *f)
{
- struct video_device *dev = priv;
- struct sta2x11_vip *vip = video_get_drvdata(dev);
+ struct sta2x11_vip *vip = video_drvdata(file);
int interlace_lim;
- if (V4L2_PIX_FMT_UYVY != f-fmt.pix.pixelformat)
- return -EINVAL;
-
You should keep this check for now. See this discussion:
http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html
I'm going to change v4l2-compliance to make this a warning instead of an
error for now.
if (V4L2_STD_525_60 vip-std)
interlace_lim = 240;
else
interlace_lim = 288;
switch (f-fmt.pix.field) {
+ default:
case V4L2_FIELD_ANY:
if (interlace_lim f-fmt.pix.height)
f-fmt.pix.field = V4L2_FIELD_INTERLACED;
After updating v4l2-compliance (I've just made the change to v4l2-compliance)
can you also post the output of v4l2-compliance for this driver?
Thanks,
Hans
--
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
Re: Status of the patches under review at LMML (35 patches)
On 01/06/2013 02:34 PM, Mauro Carvalho Chehab wrote: This is the summary of the patches that are currently under review at Linux Media Mailing List linux-media@vger.kernel.org. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email). P.S.: This email is c/c to the developers where some action is expected. If you were copied, please review the patches, acking/nacking or submitting an update. == New patches == ... == Sylwester Nawrocki s.nawro...@samsung.com == Dec,28 2012: [1/3,media] s5p-mfc: use mfc_err instead of printk http://patchwork.linuxtv.org/patch/16012 Sachin Kamat sachin.ka...@linaro.org This patch doesn't apply any more, it's superseded by this patch from Kamil, that includes same change - http://patchwork.linuxtv.org/patch/16073 Jan, 6 2013: s5p-tv: mixer: fix handling of VIDIOC_S_FMT http://patchwork.linuxtv.org/patch/16143 Tomasz Stanislawski t.stanisl...@samsung.com And it's been decided to postpone merging of this one for some time, so I've marked it as Under review. -- Thanks, Sylwester -- 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
Re: [PATCH 1/3] [media] s5p-mfc: use mfc_err instead of printk
Hi Sachin, On 01/07/2013 05:09 AM, Sachin Kamat wrote: Hi Sylwester, On 3 January 2013 00:00, Kamil Debski k.deb...@samsung.com wrote: Hi Sachin, Thank you for your patch. Best wishes, -- Kamil Debski Linux Platform Group Samsung Poland RD Center From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Sachin Kamat Sent: Friday, December 28, 2012 11:18 AM To: linux-media@vger.kernel.org Cc: k.deb...@samsung.com; s.nawro...@samsung.com; sylvester.nawro...@gmail.com; sachin.ka...@linaro.org; patc...@linaro.org Subject: [PATCH 1/3] [media] s5p-mfc: use mfc_err instead of printk Use mfc_err for consistency. Also silences checkpatch warning. Acked-by: Kamil Debski k.deb...@samsung.com Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- Probably you missed to apply this patch to your tree. Hmm, not really, I intended it for a second v3.9 pull request. However I checked it yesterday and it doesn't apply any more. Since one of Kamil's patches includes same change. Thanks, Sylwester -- 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
[PATCH] [media] coda: Fix build due to iram.h rename
commit c045e3f13 (ARM: imx: include iram.h rather than mach/iram.h) changed the
location of iram.h, which causes the following build error when building the
coda
driver:
drivers/media/platform/coda.c:27:23: error: mach/iram.h: No such file or
directory
drivers/media/platform/coda.c: In function 'coda_probe':
drivers/media/platform/coda.c:2000: error: implicit declaration of function
'iram_alloc'
drivers/media/platform/coda.c:2001: warning: assignment makes pointer from
integer without a cast
drivers/media/platform/coda.c: In function 'coda_remove':
drivers/media/platform/coda.c:2024: error: implicit declaration of function
'iram_free'
Since the content of iram.h is not imx specific, move it to
include/linux/platform_data/imx-iram.h instead. This is an intermediate solution
until the i.MX iram allocator is converted to the generic SRAM allocator.
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
---
Based on an earlier version from Fabio Estevam, but this one moves iram.h
to include/linux/platform_data/imx-iram.h instead of include/linux/iram.h
which is a less generic name.
arch/arm/mach-imx/iram.h | 41
arch/arm/mach-imx/iram_alloc.c |3 +--
drivers/media/platform/coda.c |2 +-
include/linux/platform_data/imx-iram.h | 41
4 files changed, 43 insertions(+), 44 deletions(-)
delete mode 100644 arch/arm/mach-imx/iram.h
create mode 100644 include/linux/platform_data/imx-iram.h
diff --git a/arch/arm/mach-imx/iram.h b/arch/arm/mach-imx/iram.h
deleted file mode 100644
index 022690c..000
--- a/arch/arm/mach-imx/iram.h
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * Copyright (C) 2010 Freescale Semiconductor, Inc. All Rights Reserved.
- *
- * 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; either version 2
- * of the License, or (at your option) any later version.
- *
- * 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.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
- * MA 02110-1301, USA.
- */
-#include linux/errno.h
-
-#ifdef CONFIG_IRAM_ALLOC
-
-int __init iram_init(unsigned long base, unsigned long size);
-void __iomem *iram_alloc(unsigned int size, unsigned long *dma_addr);
-void iram_free(unsigned long dma_addr, unsigned int size);
-
-#else
-
-static inline int __init iram_init(unsigned long base, unsigned long size)
-{
- return -ENOMEM;
-}
-
-static inline void __iomem *iram_alloc(unsigned int size, unsigned long
*dma_addr)
-{
- return NULL;
-}
-
-static inline void iram_free(unsigned long base, unsigned long size) {}
-
-#endif
diff --git a/arch/arm/mach-imx/iram_alloc.c b/arch/arm/mach-imx/iram_alloc.c
index 6c80424..e05cf40 100644
--- a/arch/arm/mach-imx/iram_alloc.c
+++ b/arch/arm/mach-imx/iram_alloc.c
@@ -22,8 +22,7 @@
#include linux/module.h
#include linux/spinlock.h
#include linux/genalloc.h
-
-#include iram.h
+#include linux/platform_data/imx-iram.h
static unsigned long iram_phys_base;
static void __iomem *iram_virt_base;
diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 7b8b547..afadd3a 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -23,8 +23,8 @@
#include linux/slab.h
#include linux/videodev2.h
#include linux/of.h
+#include linux/platform_data/imx-iram.h
-#include mach/iram.h
#include media/v4l2-ctrls.h
#include media/v4l2-device.h
#include media/v4l2-ioctl.h
diff --git a/include/linux/platform_data/imx-iram.h
b/include/linux/platform_data/imx-iram.h
new file mode 100644
index 000..022690c
--- /dev/null
+++ b/include/linux/platform_data/imx-iram.h
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2010 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * 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; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+#include linux/errno.h
+
+#ifdef
[PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00 2001
From: Guennadi Liakhovetski g.liakhovet...@gmx.de
Date: Fri, 19 Oct 2012 23:40:44 +0200
Subject: [PATCH] media: V4L2: support asynchronous subdevice registration
Currently bridge device drivers register devices for all subdevices
synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor
is attached to a video bridge device, the bridge driver will create an I2C
device and wait for the respective I2C driver to probe. This makes linking
of devices straight forward, but this approach cannot be used with
intrinsically asynchronous and unordered device registration systems like
the Flattened Device Tree. To support such systems this patch adds an
asynchronous subdevice registration framework to V4L2. To use it respective
(e.g. I2C) subdevice drivers must request deferred probing as long as their
bridge driver hasn't probed. The bridge driver during its probing submits a
an arbitrary number of subdevice descriptor groups to the framework to
manage. After that it can add callbacks to each of those groups to be
called at various stages during subdevice probing, e.g. after completion.
Then the bridge driver can request single groups to be probed, finish its
own probing and continue its video subsystem configuration from its
callbacks.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
v4: Fixed v4l2_async_notifier_register() for the case, when subdevices
probe successfully before the bridge, thanks to Prabhakar for reporting
drivers/media/v4l2-core/Makefile |3 +-
drivers/media/v4l2-core/v4l2-async.c | 284 ++
include/media/v4l2-async.h | 113 ++
3 files changed, 399 insertions(+), 1 deletions(-)
create mode 100644 drivers/media/v4l2-core/v4l2-async.c
create mode 100644 include/media/v4l2-async.h
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index d065c01..b667ced 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -5,7 +5,8 @@
tuner-objs := tuner-core.o
videodev-objs := v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
- v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o
+ v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
+ v4l2-async.o
ifeq ($(CONFIG_COMPAT),y)
videodev-objs += v4l2-compat-ioctl32.o
endif
diff --git a/drivers/media/v4l2-core/v4l2-async.c
b/drivers/media/v4l2-core/v4l2-async.c
new file mode 100644
index 000..55c2ad0
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -0,0 +1,284 @@
+/*
+ * V4L2 asynchronous subdevice registration API
+ *
+ * Copyright (C) 2012, Guennadi Liakhovetski g.liakhovet...@gmx.de
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include linux/device.h
+#include linux/err.h
+#include linux/i2c.h
+#include linux/list.h
+#include linux/module.h
+#include linux/mutex.h
+#include linux/notifier.h
+#include linux/platform_device.h
+#include linux/slab.h
+#include linux/types.h
+
+#include media/v4l2-async.h
+#include media/v4l2-device.h
+#include media/v4l2-subdev.h
+
+static bool match_i2c(struct device *dev, struct v4l2_async_hw_device *hw_dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ return hw_dev-bus_type == V4L2_ASYNC_BUS_I2C
+ hw_dev-match.i2c.adapter_id == client-adapter-nr
+ hw_dev-match.i2c.address == client-addr;
+}
+
+static bool match_platform(struct device *dev, struct v4l2_async_hw_device
*hw_dev)
+{
+ return hw_dev-bus_type == V4L2_ASYNC_BUS_PLATFORM
+ !strcmp(hw_dev-match.platform.name, dev_name(dev));
+}
+
+static LIST_HEAD(subdev_list);
+static LIST_HEAD(notifier_list);
+static DEFINE_MUTEX(list_lock);
+
+static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier
*notifier,
+ struct
v4l2_async_subdev_list *asdl)
+{
+ struct v4l2_async_subdev *asd = NULL;
+ bool (*match)(struct device *,
+ struct v4l2_async_hw_device *);
+
+ list_for_each_entry (asd, notifier-waiting, list) {
+ struct v4l2_async_hw_device *hw = asd-hw;
+ switch (hw-bus_type) {
+ case V4L2_ASYNC_BUS_SPECIAL:
+ match = hw-match.special.match;
+ if (!match)
+ /* Match always */
+ return asd;
+ break;
+ case V4L2_ASYNC_BUS_PLATFORM:
+ match = match_platform;
+ break;
+ case V4L2_ASYNC_BUS_I2C:
+ match = match_i2c;
+ break;
+
Re: [PATCH] [media] coda: Fix build due to iram.h rename
Hi Sascha, On Mon, Jan 7, 2013 at 8:03 AM, Sascha Hauer s.ha...@pengutronix.de wrote: commit c045e3f13 (ARM: imx: include iram.h rather than mach/iram.h) changed the location of iram.h, which causes the following build error when building the coda driver: drivers/media/platform/coda.c:27:23: error: mach/iram.h: No such file or directory drivers/media/platform/coda.c: In function 'coda_probe': drivers/media/platform/coda.c:2000: error: implicit declaration of function 'iram_alloc' drivers/media/platform/coda.c:2001: warning: assignment makes pointer from integer without a cast drivers/media/platform/coda.c: In function 'coda_remove': drivers/media/platform/coda.c:2024: error: implicit declaration of function 'iram_free' Since the content of iram.h is not imx specific, move it to include/linux/platform_data/imx-iram.h instead. This is an intermediate solution until the i.MX iram allocator is converted to the generic SRAM allocator. Signed-off-by: Sascha Hauer s.ha...@pengutronix.de --- Based on an earlier version from Fabio Estevam, but this one moves iram.h to include/linux/platform_data/imx-iram.h instead of include/linux/iram.h which is a less generic name. arch/arm/mach-imx/iram.h | 41 arch/arm/mach-imx/iram_alloc.c |3 +-- drivers/media/platform/coda.c |2 +- include/linux/platform_data/imx-iram.h | 41 4 files changed, 43 insertions(+), 44 deletions(-) delete mode 100644 arch/arm/mach-imx/iram.h create mode 100644 include/linux/platform_data/imx-iram.h It would be better to use git mv /git format-patch -M, so that git can detect the file rename. Regards, Fabio Estevam -- 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
RFC: add parameters to V4L controls
Hi, I have included this proposition already in the post [PATCH RFC 0/2] V4L: Add auto focus area control and selection but it left unanswered. I repost it again in a separate e-mail, I hope this way it will be easier to attract attention. Problem description Currently V4L2 controls can have only single value (of type int, int64, string). Some hardware controls require more than single int parameter, for example to set auto-focus (AF) rectangle four coordinates should be passed, to set auto-focus spot two coordinates should be passed. Current solution In case of AF rectangle we can reuse selection API as in [PATCH RFC 0/2] V4L: Add auto focus area control and selection post. Pros: - reuse existing API, Cons: - two IOCTL's to perform one action, - non-atomic operation, - fits well only for rectangles and spots (but with unused fields width, height), in case of other parameters we should find a different way. Proposed solution The solution takes an advantage of the fact VIDIOC_(G/S/TRY)_EXT_CTRLS ioctls can be called with multiple controls per call. I will present it using AF area control example. There could be added four pseudo-controls, lets call them for short: LEFT, TOP, WIDTH, HEIGHT. Those controls could be passed together with V4L2_AUTO_FOCUS_AREA_RECTANGLE control in one ioctl as a kind of parameters. For example setting auto-focus spot would require calling VIDIOC_S_EXT_CTRLS with the following controls: - V4L2_CID_AUTO_FOCUS_AREA = V4L2_AUTO_FOCUS_AREA_RECTANGLE - LEFT = ... - RIGHT = ... Setting AF rectangle: - V4L2_CID_AUTO_FOCUS_AREA = V4L2_AUTO_FOCUS_AREA_RECTANGLE - LEFT = ... - TOP = ... - WIDTH = ... - HEIGHT = ... Setting AF object detection (no parameters required): - V4L2_CID_AUTO_FOCUS_AREA = V4L2_AUTO_FOCUS_AREA_OBJECT_DETECTION I have presented all three cases to show the advantages of this solution: - atomicity - control and its parameters are passed in one call, - flexibility - we are not limited by a fixed number of parameters, - no-redundancy - we can pass only required parameters (no need to pass null width and height in case of spot selection), - extensibility - it is possible to extend parameters in the future, for example add parameters to V4L2_AUTO_FOCUS_AREA_OBJECT_DETECTION, without breaking API, - backward compatibility, - re-usability - this schema could be used in other controls, pseudo-controls could be re-used in other controls as well. - API backward compatibility. Regards Andrzej Hajda -- 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
Re: [RFC] Initial scan files troubles and brainstorming
On 12/18/2012 11:01 PM, Oliver Schinagl wrote: Unfortunatly, I have had zero replies. Hmm, it's sad there is a silence in this thread from linux-media guys :/. So why bring it up again? On 2012/11/30 Jakub Kasprzycki provided us with updated polish DVB-T frequencies for his region. This has yet to be merged, almost 3 weeks later. I sent a patch for cz data too: https://patchwork.kernel.org/patch/1844201/ I'll quickly repeat why I think this approach would be quite reasonable. * dvb-apps binary changes don't result in unnecessary releases * frequency updates don't result in unnecessary dvb-app releases * Less strict requirements for commits (code reviews etc) Well the code should be reviewed still. See commit a2e96055db297 in your repo, it's bogus. The freq added is in kHz instead of MHz. * Possibly easier entry for new submitters * much easier to package (tag it once per month if an update was) * Separate maintainer possible * just seems more logical to have it separated ;) The downside is you have to change the URL in kaffeine sources as kaffeine generates its scan file from that repo (on the server side and the client downloads then http://kaffeine.kde.org/scanfile.dvb.qz). This obviously should find a nice home on linuxtv where it belongs! At best. Here is my personal repository with the work I mentioned done. git://git.schinagl.nl/dvb_frequency_scanfiles.git If an issue is that none of the original maintainers are not looking forward to do the job, I am willing to take maintenance on me for now. Ping? Anybody? I would help with that too as I hate resending patches. But the first step I would do is a conversion to GIT. HG is demented to begin with. -- js -- 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
Re: [PATCH 1/2] drivers/media/platform/soc_camera/pxa_camera.c: reposition free_irq to avoid access to invalid data
(adding Robert to CC)
Hi Julia
Thanks for the patch.
On Mon, 7 Jan 2013, Julia Lawall wrote:
From: Julia Lawall julia.law...@lip6.fr
The data referenced by an interrupt handler should not be freed before the
interrupt is ended. The handler is pxa_camera_irq. This handler may call
pxa_dma_start_channels, which references the channels that are freed on the
lines before the call to free_irq.
I don't think any data is freed by pxa_free_dma(), it only disables DMA on
a certain channel. Theoretically there could be a different problem:
pxa_free_dma() deactivates DMA, whereas pxa_dma_start_channels() activates
it. But I think we're also protected against that: by the time
pxa_camera_remove() is called, and operation on the interface has been
stopped, client devices have been detached, pxa_camera_remove_device() has
been called, which has also stopped the interface clock. And with clock
stopped no interrupts can be generated. And the case of interrupt having
been generated before clk_disabled() and only delivered to the driver so
much later, that we're already unloading the module, seems really
impossible to me. Robert, you agree?
OTOH, it would be nice to convert also this driver to managed allocations,
which also would include devm_request(_threaded)_irq(), but that would
mean, that free_irq() would be called even later than now, also after
pxa_free_dma().
Speaking about managed allocations, those can be dangerous too: if you
request an IRQ before, say, remapping memory, or if you only use managed
IRQ requesting and ioremap() memory in your driver manually, that would be
wrong. But from a quick grep looks like most (all?) drivers get ir right -
first ioremap(), then request IRQ, but to be certain maybe coccinelle
could run a test for that too;-)
Thanks
Guennadi
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// smpl
@fn exists@
expression list es;
expression a,b;
identifier f;
@@
if (...) {
... when any
free_irq(a,b);
... when any
f(es);
... when any
return ...;
}
@@
expression list fn.es;
expression fn.a,fn.b;
identifier fn.f;
@@
*f(es);
... when any
*free_irq(a,b);
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
Not compiled. I have not observed the problem in practice; the code just
looks suspicious.
drivers/media/platform/soc_camera/pxa_camera.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/soc_camera/pxa_camera.c
b/drivers/media/platform/soc_camera/pxa_camera.c
index f91f7bf..2a19aba 100644
--- a/drivers/media/platform/soc_camera/pxa_camera.c
+++ b/drivers/media/platform/soc_camera/pxa_camera.c
@@ -1810,10 +1810,10 @@ static int pxa_camera_remove(struct platform_device
*pdev)
clk_put(pcdev-clk);
+ free_irq(pcdev-irq, pcdev);
pxa_free_dma(pcdev-dma_chans[0]);
pxa_free_dma(pcdev-dma_chans[1]);
pxa_free_dma(pcdev-dma_chans[2]);
- free_irq(pcdev-irq, pcdev);
soc_camera_host_unregister(soc_host);
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
Re: [BUG] Problem with LV5TDLX DVB-T USB and the 3.7.1 kernel
On Sat, 05 Jan 2013 16:28:04 +0200 Antti Palosaari cr...@iki.fi wrote: Take USB sniffs, make scripts to generate e4000 register write code from the sniffs, copy paste that code from the sniffs until it starts working. After it starts working it is quite easy to comment out / tweak with driver in order to find problem. With the experience and luck it is only few hours to fix, but without a experience you will likely need to learn a lot of stuff first. I have not experience with the linux media drivers coding, so it probably would take me much more than a few hours or require lots of luck. Of course those sniffs needed to take from working case, which just makes successful tuning to 74600 or 69800. Also you could use to attenuate or amplifier signal to see if it helps. Already tried that, with various levels of attenuation and amplification, the results vary from snr always to, at best, approximately every second line of tzap output shows non-zero snr. I don't have much time / money, no interest, no equipment (DVB-T modulator) to start optimizing it currently. I see. Can sending the device to you help in any way? In case I cannot make it work, I can, as well, send it to someone who could do good use of it. But first, I will try to fix it myself somehow. I'll try my luck with code. Maybe comparing the drivers with those from Realtek, which used to work for me, will help. Thanks for all the hints. Greets, Jacek -- 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
Re: [PATCH 1/2] drivers/media/platform/soc_camera/pxa_camera.c: reposition free_irq to avoid access to invalid data
On Mon, 7 Jan 2013, Guennadi Liakhovetski wrote:
(adding Robert to CC)
Hi Julia
Thanks for the patch.
On Mon, 7 Jan 2013, Julia Lawall wrote:
From: Julia Lawall julia.law...@lip6.fr
The data referenced by an interrupt handler should not be freed before the
interrupt is ended. The handler is pxa_camera_irq. This handler may call
pxa_dma_start_channels, which references the channels that are freed on the
lines before the call to free_irq.
I don't think any data is freed by pxa_free_dma(), it only disables DMA on
a certain channel.
OK, I seem to have been thrown off by the clearing fo the name field, but
that doesn't seem to be very important.
Theoretically there could be a different problem:
pxa_free_dma() deactivates DMA, whereas pxa_dma_start_channels() activates
it. But I think we're also protected against that: by the time
pxa_camera_remove() is called, and operation on the interface has been
stopped, client devices have been detached, pxa_camera_remove_device() has
been called, which has also stopped the interface clock. And with clock
stopped no interrupts can be generated. And the case of interrupt having
been generated before clk_disabled() and only delivered to the driver so
much later, that we're already unloading the module, seems really
impossible to me. Robert, you agree?
OK, thanks for the explanation.
OTOH, it would be nice to convert also this driver to managed allocations,
which also would include devm_request(_threaded)_irq(), but that would
mean, that free_irq() would be called even later than now, also after
pxa_free_dma().
OK, if it is safe to call free_irq much later, then I can propose a patch
for that.
Speaking about managed allocations, those can be dangerous too: if you
request an IRQ before, say, remapping memory, or if you only use managed
IRQ requesting and ioremap() memory in your driver manually, that would be
wrong. But from a quick grep looks like most (all?) drivers get ir right -
first ioremap(), then request IRQ, but to be certain maybe coccinelle
could run a test for that too;-)
Sure. Thanks for the suggestion!
julia
Thanks
Guennadi
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// smpl
@fn exists@
expression list es;
expression a,b;
identifier f;
@@
if (...) {
... when any
free_irq(a,b);
... when any
f(es);
... when any
return ...;
}
@@
expression list fn.es;
expression fn.a,fn.b;
identifier fn.f;
@@
*f(es);
... when any
*free_irq(a,b);
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
Not compiled. I have not observed the problem in practice; the code just
looks suspicious.
drivers/media/platform/soc_camera/pxa_camera.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/soc_camera/pxa_camera.c
b/drivers/media/platform/soc_camera/pxa_camera.c
index f91f7bf..2a19aba 100644
--- a/drivers/media/platform/soc_camera/pxa_camera.c
+++ b/drivers/media/platform/soc_camera/pxa_camera.c
@@ -1810,10 +1810,10 @@ static int pxa_camera_remove(struct platform_device
*pdev)
clk_put(pcdev-clk);
+ free_irq(pcdev-irq, pcdev);
pxa_free_dma(pcdev-dma_chans[0]);
pxa_free_dma(pcdev-dma_chans[1]);
pxa_free_dma(pcdev-dma_chans[2]);
- free_irq(pcdev-irq, pcdev);
soc_camera_host_unregister(soc_host);
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe kernel-janitors in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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
Re: [PATCH 1/6] uvcvideo: Set error_idx properly for extended controls API failures
On Thu December 27 2012 12:59:15 Hans Verkuil wrote:
On Wed December 26 2012 12:33:58 Laurent Pinchart wrote:
Hi Hans,
On Tuesday 25 December 2012 12:50:51 Hans Verkuil wrote:
On Tue December 25 2012 12:23:00 Laurent Pinchart wrote:
On Tuesday 25 December 2012 12:15:25 Hans Verkuil wrote:
On Mon December 24 2012 13:27:08 Laurent Pinchart wrote:
On Thursday 27 September 2012 17:16:15 Laurent Pinchart wrote:
When one of the requested controls doesn't exist the error_idx
field
must reflect that situation. For G_EXT_CTRLS and S_EXT_CTRLS,
error_idx must be set to the control count. For TRY_EXT_CTRLS, it
must be set to the index of the unexisting control.
This issue was found by the v4l2-compliance tool.
I'm revisiting this patch as it has been reverted in v3.8-rc1.
Signed-off-by: Laurent Pinchart
laurent.pinch...@ideasonboard.com
---
drivers/media/usb/uvc/uvc_ctrl.c | 17 ++---
drivers/media/usb/uvc/uvc_v4l2.c | 19 ---
2 files changed, 22 insertions(+), 14 deletions(-)
[snip]
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
b/drivers/media/usb/uvc/uvc_v4l2.c index f00db30..e5817b9 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -591,8 +591,10 @@ static long uvc_v4l2_do_ioctl(struct file
*file,
[snip]
@@ -637,8 +639,9 @@ static long uvc_v4l2_do_ioctl(struct file
*file,
unsigned int cmd, void *arg) ret = uvc_ctrl_get(chain, ctrl);
if (ret 0) {
uvc_ctrl_rollback(handle);
- ctrls-error_idx = i;
- return ret;
+ ctrls-error_idx = ret == -ENOENT
+ ? ctrls-count : i;
+ return ret == -ENOENT ? -EINVAL : ret;
}
}
ctrls-error_idx = 0;
@@ -661,8 +664,10 @@ static long uvc_v4l2_do_ioctl(struct file
*file,
unsigned int cmd, void *arg) ret = uvc_ctrl_set(chain, ctrl);
if (ret 0) {
uvc_ctrl_rollback(handle);
- ctrls-error_idx = i;
- return ret;
+ ctrls-error_idx = (ret == -ENOENT
+ cmd ==
VIDIOC_S_EXT_CTRLS)
+ ? ctrls-count : i;
+ return ret == -ENOENT ? -EINVAL : ret;
}
}
I've reread the V4L2 specification, and the least I can say is that
the text is pretty ambiguous. Let's clarify it.
Is there a reason to differentiate between invalid control IDs and
other errors as far as error_idx is concerned ? It would be simpler
if
error_idx was set to the index of the first error for get and try
operations, regardless of the error type. What do you think ?
There is a good reason for doing this: the G/S_EXT_CTRLS ioctls have
to
be as atomic as possible, i.e. it should try hard to prevent leaving
the
hardware in an inconsistent state because not all controls could be
set.
It can never be fully atomic since writing multiple registers over usb
or i2c can always return errors for one of those writes, but it should
certainly check for all the obvious errors first that do not require
actually writing to the hardware, such as whether all the controls in
the control list actually exist.
And for such errors error_idx should be set to the number of controls
to
indicate that none of the controls were actually set but that there
was
a problem with the list of controls itself.
For S_EXT_CTRLS, sure, but G_EXT_CTRLS doesn't modify the hardware
state,
so it could get all controls up to the erroneous one.
I have thought about that but I decided against it. One reason is to have
get and set behave the same since both access the hardware. The other
reason is that even getting a control value might change the hardware
state, for example by resetting some internal hardware counter when a
register is read (it's rare but there is hardware like that). Furthermore,
reading hardware registers can be slow so why not do the sanity check
first?
Get can indeed change the device state in rare cases, but the information
won't be lost, as the value of all controls before error_idx will be
returned.
What bothers me with the current G_EXT_CTRLS implementation (beside that
it's
very
Re: [PATCH 1/2] drivers/media/platform/soc_camera/pxa_camera.c: reposition free_irq to avoid access to invalid data
On Mon, 7 Jan 2013, Julia Lawall wrote:
On Mon, 7 Jan 2013, Guennadi Liakhovetski wrote:
(adding Robert to CC)
Hi Julia
Thanks for the patch.
On Mon, 7 Jan 2013, Julia Lawall wrote:
From: Julia Lawall julia.law...@lip6.fr
The data referenced by an interrupt handler should not be freed before the
interrupt is ended. The handler is pxa_camera_irq. This handler may call
pxa_dma_start_channels, which references the channels that are freed on
the
lines before the call to free_irq.
I don't think any data is freed by pxa_free_dma(), it only disables DMA on
a certain channel.
OK, I seem to have been thrown off by the clearing fo the name field, but
that doesn't seem to be very important.
Exactly.
Theoretically there could be a different problem:
pxa_free_dma() deactivates DMA, whereas pxa_dma_start_channels() activates
it. But I think we're also protected against that: by the time
pxa_camera_remove() is called, and operation on the interface has been
stopped, client devices have been detached, pxa_camera_remove_device() has
been called, which has also stopped the interface clock. And with clock
stopped no interrupts can be generated. And the case of interrupt having
been generated before clk_disabled() and only delivered to the driver so
much later, that we're already unloading the module, seems really
impossible to me. Robert, you agree?
OK, thanks for the explanation.
OTOH, it would be nice to convert also this driver to managed allocations,
which also would include devm_request(_threaded)_irq(), but that would
mean, that free_irq() would be called even later than now, also after
pxa_free_dma().
OK, if it is safe to call free_irq much later, then I can propose a patch
for that.
I think it should be safe. In any case we cannot rely on the fact, that
free_irq() in the current code happens soon after pxa_free_dma(), so,
putting it even later will either emphasise our certainty, that we're safe
there, or help up catch the bug, since statistically the window will
become larger;-) Of course, all of clk_get(), request_mem_region() +
ioremap(), kzalloc(), request_irq() would have to be replaced.
Thanks
Guennadi
Speaking about managed allocations, those can be dangerous too: if you
request an IRQ before, say, remapping memory, or if you only use managed
IRQ requesting and ioremap() memory in your driver manually, that would be
wrong. But from a quick grep looks like most (all?) drivers get ir right -
first ioremap(), then request IRQ, but to be certain maybe coccinelle
could run a test for that too;-)
Sure. Thanks for the suggestion!
julia
Thanks
Guennadi
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// smpl
@fn exists@
expression list es;
expression a,b;
identifier f;
@@
if (...) {
... when any
free_irq(a,b);
... when any
f(es);
... when any
return ...;
}
@@
expression list fn.es;
expression fn.a,fn.b;
identifier fn.f;
@@
*f(es);
... when any
*free_irq(a,b);
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
Not compiled. I have not observed the problem in practice; the code just
looks suspicious.
drivers/media/platform/soc_camera/pxa_camera.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/soc_camera/pxa_camera.c
b/drivers/media/platform/soc_camera/pxa_camera.c
index f91f7bf..2a19aba 100644
--- a/drivers/media/platform/soc_camera/pxa_camera.c
+++ b/drivers/media/platform/soc_camera/pxa_camera.c
@@ -1810,10 +1810,10 @@ static int pxa_camera_remove(struct
platform_device *pdev)
clk_put(pcdev-clk);
+ free_irq(pcdev-irq, pcdev);
pxa_free_dma(pcdev-dma_chans[0]);
pxa_free_dma(pcdev-dma_chans[1]);
pxa_free_dma(pcdev-dma_chans[2]);
- free_irq(pcdev-irq, pcdev);
soc_camera_host_unregister(soc_host);
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe kernel-janitors
in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
cx88 WRITERM instruction
Hi, I'm trying to add precise timestamps to the cx88 driver but I somehow fail to use the WRITERM instruction correctly. Has anyone ever successfully used that RISC instruction? Is there a bit that needs to be flipped to actually make it perform writes to the PCI bus? I know it is executed because I get a RISC_RD_BERR_INT if I replace the third word with junk but no matter what I do the target memory stays untouched (yes, it is coherent memory and I use the DMA address). Thanks, Daniel -- 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
[PATCH] drivers/media/platform/soc_camera/pxa_camera.c: use devm_ functions
From: Julia Lawall julia.law...@lip6.fr
This patch uses various devm_ functions for data that is allocated in the
probe function of a platform driver and is only freed in the remove
function.
This also fixes a checkpatch warning, removing a space before a \n in a
string.
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
Not compiled. This assumes that it is safe to move the free of the clk and
the irq after the calls to pxa_free_dma.
drivers/media/platform/soc_camera/pxa_camera.c | 65 +
1 file changed, 15 insertions(+), 50 deletions(-)
diff --git a/drivers/media/platform/soc_camera/pxa_camera.c
b/drivers/media/platform/soc_camera/pxa_camera.c
index f91f7bf..5ebb2a1 100644
--- a/drivers/media/platform/soc_camera/pxa_camera.c
+++ b/drivers/media/platform/soc_camera/pxa_camera.c
@@ -1661,23 +1661,18 @@ static int pxa_camera_probe(struct platform_device
*pdev)
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
irq = platform_get_irq(pdev, 0);
- if (!res || irq 0) {
- err = -ENODEV;
- goto exit;
- }
+ if (!res || irq 0)
+ return -ENODEV;
- pcdev = kzalloc(sizeof(*pcdev), GFP_KERNEL);
+ pcdev = devm_kzalloc(pdev-dev, sizeof(*pcdev), GFP_KERNEL);
if (!pcdev) {
dev_err(pdev-dev, Could not allocate pcdev\n);
- err = -ENOMEM;
- goto exit;
+ return -ENOMEM;
}
- pcdev-clk = clk_get(pdev-dev, NULL);
- if (IS_ERR(pcdev-clk)) {
- err = PTR_ERR(pcdev-clk);
- goto exit_kfree;
- }
+ pcdev-clk = devm_clk_get(pdev-dev, NULL);
+ if (IS_ERR(pcdev-clk))
+ return PTR_ERR(pcdev-clk);
pcdev-res = res;
@@ -1715,17 +1710,9 @@ static int pxa_camera_probe(struct platform_device *pdev)
/*
* Request the regions.
*/
- if (!request_mem_region(res-start, resource_size(res),
- PXA_CAM_DRV_NAME)) {
- err = -EBUSY;
- goto exit_clk;
- }
-
- base = ioremap(res-start, resource_size(res));
- if (!base) {
- err = -ENOMEM;
- goto exit_release;
- }
+ base = devm_request_and_ioremap(pdev-dev, res);
+ if (!base)
+ return -ENOMEM;
pcdev-irq = irq;
pcdev-base = base;
@@ -1734,7 +1721,7 @@ static int pxa_camera_probe(struct platform_device *pdev)
pxa_camera_dma_irq_y, pcdev);
if (err 0) {
dev_err(pdev-dev, Can't request DMA for Y\n);
- goto exit_iounmap;
+ return err;
}
pcdev-dma_chans[0] = err;
dev_dbg(pdev-dev, got DMA channel %d\n, pcdev-dma_chans[0]);
@@ -1762,10 +1749,10 @@ static int pxa_camera_probe(struct platform_device
*pdev)
DRCMR(70) = pcdev-dma_chans[2] | DRCMR_MAPVLD;
/* request irq */
- err = request_irq(pcdev-irq, pxa_camera_irq, 0, PXA_CAM_DRV_NAME,
- pcdev);
+ err = devm_request_irq(pdev-dev, pcdev-irq, pxa_camera_irq, 0,
+ PXA_CAM_DRV_NAME, pcdev);
if (err) {
- dev_err(pdev-dev, Camera interrupt register failed \n);
+ dev_err(pdev-dev, Camera interrupt register failed\n);
goto exit_free_dma;
}
@@ -1777,27 +1764,16 @@ static int pxa_camera_probe(struct platform_device
*pdev)
err = soc_camera_host_register(pcdev-soc_host);
if (err)
- goto exit_free_irq;
+ goto exit_free_dma;
return 0;
-exit_free_irq:
- free_irq(pcdev-irq, pcdev);
exit_free_dma:
pxa_free_dma(pcdev-dma_chans[2]);
exit_free_dma_u:
pxa_free_dma(pcdev-dma_chans[1]);
exit_free_dma_y:
pxa_free_dma(pcdev-dma_chans[0]);
-exit_iounmap:
- iounmap(base);
-exit_release:
- release_mem_region(res-start, resource_size(res));
-exit_clk:
- clk_put(pcdev-clk);
-exit_kfree:
- kfree(pcdev);
-exit:
return err;
}
@@ -1806,24 +1782,13 @@ static int pxa_camera_remove(struct platform_device
*pdev)
struct soc_camera_host *soc_host = to_soc_camera_host(pdev-dev);
struct pxa_camera_dev *pcdev = container_of(soc_host,
struct pxa_camera_dev, soc_host);
- struct resource *res;
-
- clk_put(pcdev-clk);
pxa_free_dma(pcdev-dma_chans[0]);
pxa_free_dma(pcdev-dma_chans[1]);
pxa_free_dma(pcdev-dma_chans[2]);
- free_irq(pcdev-irq, pcdev);
soc_camera_host_unregister(soc_host);
- iounmap(pcdev-base);
-
- res = pcdev-res;
- release_mem_region(res-start, resource_size(res));
-
- kfree(pcdev);
-
dev_info(pdev-dev, PXA Camera driver unloaded\n);
return 0;
--
To unsubscribe from this list: send the line unsubscribe
[REVIEW PATCHv1 0/2] DocBook fixes
Two patches: the first clarifies how the error_idx is used, the second fixes a number of validation errors in the DocBook code. Regards, Hans -- 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
[REVIEW PATCHv1 1/2] DocBook: improve the error_idx field documentation.
From: Hans Verkuil hans.verk...@cisco.com The documentation of the error_idx field was incomplete and confusing. This patch improves it. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- .../DocBook/media/v4l/vidioc-g-ext-ctrls.xml | 51 +--- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml index 0a4b90f..c07c657 100644 --- a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml +++ b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml @@ -199,13 +199,50 @@ also be zero./entry row entry__u32/entry entrystructfielderror_idx/structfield/entry - entrySet by the driver in case of an error. If it is equal -to structfieldcount/structfield, then no actual changes were made to -controls. In other words, the error was not associated with setting a particular -control. If it is another value, then only the controls up to structfielderror_idx-1/structfield -were modified and control structfielderror_idx/structfield is the one that -caused the error. The structfielderror_idx/structfield value is undefined -if the ioctl returned 0 (success)./entry + entryparaSet by the driver in case of an error. If the error is +associated with a particular control, then structfielderror_idx/structfield +is set to the index of that control. If the error is not related to a specific +control, then structfielderror_idx/structfield is set to structfieldcount/structfield./para + +paraThe behavior is different for constantVIDIOC_G_EXT_CTRLS/constant and +constantVIDIOC_S_EXT_CTRLS/constant: if +structfielderror_idx/structfield is equal to structfieldcount/structfield, +then no actual changes were made to the controls. For example, if you try to +write to a read-only control, then constantVIDIOC_TRY_EXT_CTRLS/constant +will set structfielderror_idx/structfield to the index of that read-only +control, but constantVIDIOC_S_EXT_CTRLS/constant will set +structfielderror_idx/structfield to structfieldcount/structfield instead +and none of the controls in the list will be set./para + +paraThe same is true when trying to e.g. read a write-only control: +constantVIDIOC_G_EXT_CTRLS/constant will set structfielderror_idx/structfield +to structfieldcount/structfield and none of the controls in the list will +have been retrieved./para + +paraThe reason for this behavior is that it is important when setting and getting +controls to do this as atomically as possible, so simple sanity checks like testing +for read-only controls are done first before an attempt is made to apply/retrieve the new +control values to/from the hardware. It is important for an application to know whether +constantVIDIOC_S_EXT_CTRLS/constant or constantVIDIOC_G_EXT_CTRLS/constant actually +made changes to controls or not. So if structfielderror_idx/structfield is equal +to structfieldcount/structfield, then you know that no actual controls were set or +retrieved. In the case of constantVIDIOC_S_EXT_CTRLS/constant you can call +constantVIDIOC_TRY_EXT_CTRLS/constant with the same control list to see if the +problem was with a specific control or not (constantVIDIOC_TRY_EXT_CTRLS/constant +never modifies controls, so that ioctl will just set structfielderror_idx/structfield +to the control that caused the problem). No such option exists for constantVIDIOC_G_EXT_CTRLS/constant +though, unfortunately./para + +paraIf structfielderror_idx/structfield as returned by constantVIDIOC_S_EXT_CTRLS/constant +or constantVIDIOC_G_EXT_CTRLS/constant is less than structfieldcount/structfield, +then only the controls up to structfielderror_idx-1/structfield were modified and control +structfielderror_idx/structfield is the one that caused the error. In the case of +constantVIDIOC_S_EXT_CTRLS/constant this might have left the hardware in an +inconsistent state. These types of errors should not normally happen, and such +errors are typically caused by problems in communicating with the hardware./para + +paraFinally, note that the structfielderror_idx/structfield value is undefined +if the ioctl returned 0 (success)./para/entry /row row entry__u32/entry -- 1.7.10.4 -- 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
Re: RFC: add parameters to V4L controls
On Mon January 7 2013 11:46:38 Andrzej Hajda wrote: Hi, I have included this proposition already in the post [PATCH RFC 0/2] V4L: Add auto focus area control and selection but it left unanswered. I repost it again in a separate e-mail, I hope this way it will be easier to attract attention. Problem description Currently V4L2 controls can have only single value (of type int, int64, string). Some hardware controls require more than single int parameter, for example to set auto-focus (AF) rectangle four coordinates should be passed, to set auto-focus spot two coordinates should be passed. Current solution In case of AF rectangle we can reuse selection API as in [PATCH RFC 0/2] V4L: Add auto focus area control and selection post. Pros: - reuse existing API, Cons: - two IOCTL's to perform one action, - non-atomic operation, - fits well only for rectangles and spots (but with unused fields width, height), in case of other parameters we should find a different way. Proposed solution The solution takes an advantage of the fact VIDIOC_(G/S/TRY)_EXT_CTRLS ioctls can be called with multiple controls per call. I will present it using AF area control example. There could be added four pseudo-controls, lets call them for short: LEFT, TOP, WIDTH, HEIGHT. Those controls could be passed together with V4L2_AUTO_FOCUS_AREA_RECTANGLE control in one ioctl as a kind of parameters. For example setting auto-focus spot would require calling VIDIOC_S_EXT_CTRLS with the following controls: - V4L2_CID_AUTO_FOCUS_AREA = V4L2_AUTO_FOCUS_AREA_RECTANGLE - LEFT = ... - RIGHT = ... Setting AF rectangle: - V4L2_CID_AUTO_FOCUS_AREA = V4L2_AUTO_FOCUS_AREA_RECTANGLE - LEFT = ... - TOP = ... - WIDTH = ... - HEIGHT = ... Setting AF object detection (no parameters required): - V4L2_CID_AUTO_FOCUS_AREA = V4L2_AUTO_FOCUS_AREA_OBJECT_DETECTION If you want to do this, then you have to make LEFT/TOP/WIDTH/HEIGHT real controls. There is no such thing as a pseudo control. So you need five new controls in total: V4L2_CID_AUTO_FOCUS_AREA V4L2_CID_AUTO_FOCUS_LEFT V4L2_CID_AUTO_FOCUS_RIGHT V4L2_CID_AUTO_FOCUS_WIDTH V4L2_CID_AUTO_FOCUS_HEIGHT I have no problem with this from the point of view of the control API, but whether this is the best solution for implementing auto-focus is a different issue and input from sensor specialists is needed as well (added Laurent and Sakari to the CC list). The primary concern I have is that this does not scale to multiple focus rectangles. This might not be relevant to auto focus, though. Regards, Hans I have presented all three cases to show the advantages of this solution: - atomicity - control and its parameters are passed in one call, - flexibility - we are not limited by a fixed number of parameters, - no-redundancy - we can pass only required parameters (no need to pass null width and height in case of spot selection), - extensibility - it is possible to extend parameters in the future, for example add parameters to V4L2_AUTO_FOCUS_AREA_OBJECT_DETECTION, without breaking API, - backward compatibility, - re-usability - this schema could be used in other controls, pseudo-controls could be re-used in other controls as well. - API backward compatibility. Regards Andrzej Hajda -- 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 -- 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
Re: Status of the patches under review at LMML (35 patches)
Hi Prabhakar, On Monday 07 January 2013 11:26:01 Prabhakar Lad wrote: On Sun, Jan 6, 2013 at 7:04 PM, Mauro Carvalho Chehab wrote: This is the summary of the patches that are currently under review at Linux Media Mailing List linux-media@vger.kernel.org. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email). Snip == Prabhakar Lad prabhakar@ti.com == Aug,24 2012: Corrected Oops on omap_vout when no manager is connected http://patchwork.linuxtv.org/patch/14033 Federico Fuga f...@studiofuga.com Tomi can you take care of this patch ? Tomi is on parental leave until beginning of February. Beside, he doesn't have much experience with the omap_vout driver. We need an Acked-by on this patch before he can take it in his tree. -- Regards, Laurent Pinchart -- 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
Re: [patch] [media] dvb: unlock on error in dvb_ca_en50221_io_do_ioctl()
On Fri, Jan 04, 2013 at 09:56:02PM +0300, Dan Carpenter wrote: We recently pushed the locking down into this function, but there was an error path where the unlock was missed. Ugh, indeed. Thanks for catching this! Nikolaus. -- 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
Re: [PATCH] omap3isp: Add support for interlaced input data
Hi William, On Friday 04 January 2013 11:52:28 William Swanson wrote: Laurent Pinchart laurent.pinch...@ideasonboard.com escreveu: On Monday 17 December 2012 18:12:19 William Swanson wrote: If the remote video sensor reports an interlaced video mode, the CCDC block should configure itself appropriately. What will the CCDC do in that case ? Will it capture fields or frames to memory ? If frames, what's the field layout ? You will most likely need to modify ispvideo.c as well, to support interlacing in the V4L2 API, and possibly add interlaced formats support to the media bus API. Sorry for the delay in responding; today is my first day back at the office. I do not know the answers to these questions, and the documentation doesn't discuss interlacing much. Our application has the following pipeline: composite video - TVP5146 decoder - CCDC parallel interface - memory - application One of the wires in the parallel interface, cam_fld, indicates the current field, and this patch simply enables that wire. Without the patch, every other line in our memory buffer is garbage; with the patch, the image comes out correctly. What do you get in the memory buffers ? Are fields captured in separate buffers or combined in a single buffer ? If they're combined, are they interleaved or sequential in memory ? As a matter of fact, an earlier version of the ISP driver actually contained code for dealing with this flag; it was removed in cf7a3d91ade6c56bfd860b377f84bd58132f7a81 along with a bunch of other cleanup work. This patch simply adds the code back, but in a way that is compatible with the new media pipeline stuff. I believe that the CCDC simply captures image data a line at a time and writes it directly to memory, at least in our use case. The CCDC_SDOFST register controls the layout, and the default value (which is what the driver uses now) is basically correct. I am not familiar enough with the V4L2 architecture to tell you how the driver decides that it now has a complete frame, or what that even means in an interlaced case. On 12/27/2012 12:27 PM, Mauro Carvalho Chehab wrote: Btw, you missed to add a Signed-off-by: line on it. Oops, this was a problem with my git setup. Both email addresses are mine; I can re-send the patch with them both set to the same address if you would prefer that. -- Regards, Laurent Pinchart -- 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
Re: Status of the patches under review at LMML (35 patches)
Hi Mauro, On Sunday 06 January 2013 11:34:55 Mauro Carvalho Chehab wrote: This is the summary of the patches that are currently under review at Linux Media Mailing List linux-media@vger.kernel.org. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email). P.S.: This email is c/c to the developers where some action is expected. If you were copied, please review the patches, acking/nacking or submitting an update. [snip] == Laurent Pinchart laurent.pinch...@ideasonboard.com == Dec,12 2012: [v2] ad5820: Voice coil motor controller driver http://patchwork.linuxtv.org/patch/15881 Florian Neuhaus florian.neuh...@reberinformatik.ch Still under review, I haven't had time to look into it yet. Jan, 4 2013: omap3isp: Add support for interlaced input data http://patchwork.linuxtv.org/patch/16133 William Swanson william.swan...@fuel7.com The patch is still under review, I've asked for more information today. Sep, 4 2012: [5/5] drivers/media/platform/omap3isp/isp.c: fix error return code http://patchwork.linuxtv.org/patch/14169 Peter Senna Tschudin peter.se...@gmail.com This one is already in mainline. -- Regards, Laurent Pinchart -- 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
Re: Status of the patches under review at LMML (35 patches)
Hi Mauro On Sun, 6 Jan 2013, Mauro Carvalho Chehab wrote: == Guennadi Liakhovetski g.liakhovet...@gmx.de == Oct,30 2012: [v2,2/4] media: mx2_camera: Add image size HW limits. http://patchwork.linuxtv.org/patch/15298 Javier Martin javier.mar...@vista-silicon.com In the mainline as commit 6ec5575c381de50b17e68796435f20ce1b27de79 Nov,13 2012: sh_vou: Move from videobuf to videobuf2 http://patchwork.linuxtv.org/patch/15433 Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com Patch has to be fixed. Nov,16 2012: [05/14,media] atmel-isi: Update error check for unsigned variables http://patchwork.linuxtv.org/patch/15475 Tushar Behera tushar.beh...@linaro.org Hmm, I'll push it (or an equivalent of it) in the second 3.9 pull request Jan, 3 2013: [1/3] sh_vou: Don't modify const variable in sh_vou_s_crop() http://patchwork.linuxtv.org/patch/16095 Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com Jan, 3 2013: [2/3] sh_vou: Use video_drvdata() http://patchwork.linuxtv.org/patch/16097 Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com Jan, 3 2013: [3/3] sh_vou: Use vou_dev instead of vou_file wherever possible http://patchwork.linuxtv.org/patch/16096 Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com These ones arrived after my pull request, I'll take care of them in the next pull round. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
Re: Status of the patches under review at LMML (35 patches)
Hi Laurent, On Mon, Jan 7, 2013 at 5:43 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Prabhakar, On Monday 07 January 2013 11:26:01 Prabhakar Lad wrote: On Sun, Jan 6, 2013 at 7:04 PM, Mauro Carvalho Chehab wrote: This is the summary of the patches that are currently under review at Linux Media Mailing List linux-media@vger.kernel.org. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email). Snip == Prabhakar Lad prabhakar@ti.com == Aug,24 2012: Corrected Oops on omap_vout when no manager is connected http://patchwork.linuxtv.org/patch/14033 Federico Fuga f...@studiofuga.com Tomi can you take care of this patch ? Tomi is on parental leave until beginning of February. Beside, he doesn't have much experience with the omap_vout driver. We need an Acked-by on this patch before he can take it in his tree. Thanks for the info, I did contact Vaibhav even he is on leave till Jan-20. Murali can you Ack/Review this patch so that Tomi can pick it up when hes back from vacation (this patch doesnt apply on 3.8) Regards, --Prabhakar -- Regards, Laurent Pinchart -- 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
Re: [RFC] Initial scan files troubles and brainstorming
On 07-01-13 11:46, Jiri Slaby wrote: On 12/18/2012 11:01 PM, Oliver Schinagl wrote: Unfortunatly, I have had zero replies. Hmm, it's sad there is a silence in this thread from linux-media guys :/. In their defense, they are very very busy people ;) chatter on this thread does bring it up however. So why bring it up again? On 2012/11/30 Jakub Kasprzycki provided us with updated polish DVB-T frequencies for his region. This has yet to be merged, almost 3 weeks later. I sent a patch for cz data too: https://patchwork.kernel.org/patch/1844201/ I see that patch lives on the kernel patchwork not the linuxtv one. Did it pass this ML? I have applied it to my tree for now. I'll quickly repeat why I think this approach would be quite reasonable. * dvb-apps binary changes don't result in unnecessary releases * frequency updates don't result in unnecessary dvb-app releases * Less strict requirements for commits (code reviews etc) Well the code should be reviewed still. See commit a2e96055db297 in your repo, it's bogus. The freq added is in kHz instead of MHz. You are absolutely right, on both accounts. But its far less important. Yes I committed it wrongfully. I missed it. I should have spotted it. You 'fixed' this bug though ;) Reviewing is still needed. 'Rules' in that regard can always be brought up later, when things actually do change. Auto apply after a week for example? Again, 'bugs' aren't critical and don't break functionality (but of course do cause headaches if they don't work). But updates are required to happen quickly if they do (for dvb-t at the very least). If the broadcaster changes settings, users probably want to be able to tune right away. * Possibly easier entry for new submitters * much easier to package (tag it once per month if an update was) * Separate maintainer possible * just seems more logical to have it separated ;) The downside is you have to change the URL in kaffeine sources as kaffeine generates its scan file from that repo (on the server side and the client downloads then http://kaffeine.kde.org/scanfile.dvb.qz). IF a proper change goes through, then that's a one time change only though and while kaffeine can still use its own mirror, it's less needed :p This obviously should find a nice home on linuxtv where it belongs! At best. Here is my personal repository with the work I mentioned done. git://git.schinagl.nl/dvb_frequency_scanfiles.git If an issue is that none of the original maintainers are not looking forward to do the job, I am willing to take maintenance on me for now. Ping? Anybody? I would help with that too as I hate resending patches. But the first step I would do is a conversion to GIT. HG is demented to begin with. -- 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
Re: [RFC] Initial scan files troubles and brainstorming
Okay guys, I think this is a good time to discuss scan file maintenance [ should have taken care of it earlier, but well ...]. I've been updating the scan data for the past few years, but found barely no time in the last (many!) months. This won't change in future, so I've decided to step down from the task; may others do what is necessary ... @Johannes: As I don't need the account anymore, please delete it. Christoph 2013/1/7 Jiri Slaby jirisl...@gmail.com: On 12/18/2012 11:01 PM, Oliver Schinagl wrote: Unfortunatly, I have had zero replies. Hmm, it's sad there is a silence in this thread from linux-media guys :/. So why bring it up again? On 2012/11/30 Jakub Kasprzycki provided us with updated polish DVB-T frequencies for his region. This has yet to be merged, almost 3 weeks later. I sent a patch for cz data too: https://patchwork.kernel.org/patch/1844201/ I'll quickly repeat why I think this approach would be quite reasonable. * dvb-apps binary changes don't result in unnecessary releases * frequency updates don't result in unnecessary dvb-app releases * Less strict requirements for commits (code reviews etc) Well the code should be reviewed still. See commit a2e96055db297 in your repo, it's bogus. The freq added is in kHz instead of MHz. * Possibly easier entry for new submitters * much easier to package (tag it once per month if an update was) * Separate maintainer possible * just seems more logical to have it separated ;) The downside is you have to change the URL in kaffeine sources as kaffeine generates its scan file from that repo (on the server side and the client downloads then http://kaffeine.kde.org/scanfile.dvb.qz). This obviously should find a nice home on linuxtv where it belongs! At best. Here is my personal repository with the work I mentioned done. git://git.schinagl.nl/dvb_frequency_scanfiles.git If an issue is that none of the original maintainers are not looking forward to do the job, I am willing to take maintenance on me for now. Ping? Anybody? I would help with that too as I hate resending patches. But the first step I would do is a conversion to GIT. HG is demented to begin with. -- js -- 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
Re: [PATCH] [media] coda: Fix build due to iram.h rename
Hello Fabio, On Mon, Jan 07, 2013 at 08:16:02AM -0200, Fabio Estevam wrote: Hi Sascha, On Mon, Jan 7, 2013 at 8:03 AM, Sascha Hauer s.ha...@pengutronix.de wrote: commit c045e3f13 (ARM: imx: include iram.h rather than mach/iram.h) changed the location of iram.h, which causes the following build error when building the coda driver: drivers/media/platform/coda.c:27:23: error: mach/iram.h: No such file or directory drivers/media/platform/coda.c: In function 'coda_probe': drivers/media/platform/coda.c:2000: error: implicit declaration of function 'iram_alloc' drivers/media/platform/coda.c:2001: warning: assignment makes pointer from integer without a cast drivers/media/platform/coda.c: In function 'coda_remove': drivers/media/platform/coda.c:2024: error: implicit declaration of function 'iram_free' Since the content of iram.h is not imx specific, move it to include/linux/platform_data/imx-iram.h instead. This is an intermediate solution until the i.MX iram allocator is converted to the generic SRAM allocator. Signed-off-by: Sascha Hauer s.ha...@pengutronix.de --- Based on an earlier version from Fabio Estevam, but this one moves iram.h to include/linux/platform_data/imx-iram.h instead of include/linux/iram.h which is a less generic name. arch/arm/mach-imx/iram.h | 41 arch/arm/mach-imx/iram_alloc.c |3 +-- drivers/media/platform/coda.c |2 +- include/linux/platform_data/imx-iram.h | 41 4 files changed, 43 insertions(+), 44 deletions(-) delete mode 100644 arch/arm/mach-imx/iram.h create mode 100644 include/linux/platform_data/imx-iram.h It would be better to use git mv /git format-patch -M, so that git can detect the file rename. Note that git-mv is not needed here. Using it doesn't change anything and you can use git-format-patch -M independant of it. Best regards Uwe -- 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
Re: [PATCH RFC v1 2/2] V4L: Add driver for OV9650/52 image sensors
Hi Sylwester, A quick review below: On Sat January 5 2013 00:10:23 Sylwester Nawrocki wrote: This patch adds V4L2 sub-device driver for OV9650/OV9652 image sensors. The driver exposes following V4L2 controls: - auto/manual exposure, auto exposure reference area, - auto/manual white balance, - auto/manual gain, auto gain ceiling, - brightness, saturation, sharpness, - horizontal/vertical flip, - color bar test pattern, - banding filter (power line frequency), - g/s_frame_interval pad level op. TODO: - add support for more resolutions (QQVGA, CIF, QCIF, ...), This driver has been tested only with 12 and 24 MHz external clock frequency. Signed-off-by: Sylwester Nawrocki sylvester.nawro...@gmail.com --- drivers/media/i2c/Kconfig |7 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/ov9650.c | 1684 include/media/ov9650.h | 20 + 4 files changed, 1712 insertions(+), 0 deletions(-) create mode 100644 drivers/media/i2c/ov9650.c create mode 100644 include/media/ov9650.h diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 24d78e2..182852f 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -421,6 +421,13 @@ config VIDEO_OV7670 OV7670 VGA camera. It currently only works with the M88ALP01 controller. +config VIDEO_OV9650 + tristate OmniVision OV9650/OV9652 sensor support + depends on I2C VIDEO_V4L2 VIDEO_V4L2_SUBDEV_API + ---help--- + This is a V4L2 sensor-level driver for the Omnivision + OV9650 and OV9652 camera sensors. + config VIDEO_VS6624 tristate ST VS6624 sensor support depends on VIDEO_V4L2 I2C diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index b1d62df..8b62e54 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -47,6 +47,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o obj-$(CONFIG_VIDEO_OV7670) += ov7670.o +obj-$(CONFIG_VIDEO_OV9650) += ov9650.o obj-$(CONFIG_VIDEO_TCM825X) += tcm825x.o obj-$(CONFIG_VIDEO_TVEEPROM) += tveeprom.o obj-$(CONFIG_VIDEO_MT9M032) += mt9m032.o diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c new file mode 100644 index 000..9ff960d --- /dev/null +++ b/drivers/media/i2c/ov9650.c @@ -0,0 +1,1684 @@ +/* + * Omnivision OV9650/OV9652 CMOS Image Sensor driver + * + * Copyright (C) 2012, Sylwester Nawrocki sylvester.nawro...@gmail.com + * + * Register definitions and initial settings based on a driver written + * by Vladimir Fonov. + * Copyright (c) 2010, Vladimir Fonov + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +#define pr_fmt(fmt) %s:%d fmt, __func__, __LINE__ + +#include linux/delay.h +#include linux/gpio.h +#include linux/i2c.h +#include linux/kernel.h +#include linux/media.h +#include linux/module.h +#include linux/ratelimit.h +#include linux/slab.h +#include linux/string.h + +#include media/image-sizes.h +#include media/media-entity.h +#include media/v4l2-ctrls.h +#include media/v4l2-device.h +#include media/v4l2-subdev.h +#include media/v4l2-mediabus.h +#include media/ov9650.h + +static int debug = 2; +module_param(debug, int, 0644); +MODULE_PARM_DESC(debug, Debug level (0-2)); + +#define DRIVER_NAME OV9650 + +/* + * OV9650/OV9652 register definitions + */ +#define REG_GAIN 0x00/* Gain control, AGC[7:0] */ +#define REG_BLUE 0x01/* AWB - Blue chanel gain */ +#define REG_RED 0x02/* AWB - Red chanel gain */ +#define REG_VREF 0x03/* [7:6] - AGC[9:8], [5:3]/[2:0] */ +#define VREF_GAIN_MASK 0xc0/* - VREF end/start low 3 bits */ +#define REG_COM1 0x04 +#define COM1_CCIR6560x40 +#define REG_B_AVE0x05 +#define REG_GB_AVE 0x06 +#define REG_GR_AVE 0x07 +#define REG_R_AVE0x08 +#define REG_COM2 0x09 +#define REG_PID 0x0a/* Product ID MSB */ +#define REG_VER 0x0b/* Product ID LSB */ +#define REG_COM3 0x0c +#define COM3_SWAP 0x40 +#define COM3_VARIOPIXEL10x04 +#define REG_COM4 0x0d/* Vario Pixels */ +#define COM4_VARIOPIXEL20x80 +#define REG_COM5 0x0e/* System clock options */ +#define COM5_SLAVE_MODE 0x10 +#define COM5_SYSTEMCLOCK48MHZ 0x80 +#define REG_COM6 0x0f/* HREF ADBLC options */ +#define REG_AECH 0x10/* Exposure value, AEC[9:2] */ +#define REG_CLKRC0x11/* Clock control */ +#define CLK_EXT 0x40/* Use
Re: [PATCH] omap_vout: find_vma() needs -mmap_sem held
Hi Mauro, On Sunday 06 January 2013 11:02:25 Mauro Carvalho Chehab wrote: Em Sat, 15 Dec 2012 20:38:29 + Al Viro escreveu: On Sat, Dec 15, 2012 at 08:12:37PM +, Al Viro wrote: Walking rbtree while it's modified is a Bad Idea(tm); besides, the result of find_vma() can be freed just as it's getting returned to caller. Fortunately, it's easy to fix - just take -mmap_sem a bit earlier (and don't bother with find_vma() at all if virtp = PAGE_OFFSET - in that case we don't even look at its result). While we are at it, what prevents VIDIOC_PREPARE_BUF calling v4l_prepare_buf() - (e.g) vb2_ioctl_prepare_buf() - vb2_prepare_buf() - __buf_prepare() - __qbuf_userptr() - vb2_vmalloc_get_userptr() - find_vma(), AFAICS without having taken -mmap_sem anywhere in process? The code flow is bloody convoluted and depends on a bunch of things done by initialization, so I certainly might've missed something... This driver is currently missing an active maintainer, as it is for an old hardware (AFAIK, omap is now at version 4, and this is for the first one), The omap_vout driver is for OMAP2+ if I'm not mistaken. I use it with the OMAP3. but I'm c/c a few developers that might help to test and analyze it. In any case, /me is assuming that your patch is right (as nobody complained), and I'm applying it right now on my tree. This will hopefully allow some people to test. -- Regards, Laurent Pinchart -- 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
Re: [PATCH] omap_vout: find_vma() needs -mmap_sem held
Hi Mauro, On Sunday 06 January 2013 11:02:25 Mauro Carvalho Chehab wrote: Em Sat, 15 Dec 2012 20:38:29 + Al Viro escreveu: On Sat, Dec 15, 2012 at 08:12:37PM +, Al Viro wrote: Walking rbtree while it's modified is a Bad Idea(tm); besides, the result of find_vma() can be freed just as it's getting returned to caller. Fortunately, it's easy to fix - just take -mmap_sem a bit earlier (and don't bother with find_vma() at all if virtp = PAGE_OFFSET - in that case we don't even look at its result). While we are at it, what prevents VIDIOC_PREPARE_BUF calling v4l_prepare_buf() - (e.g) vb2_ioctl_prepare_buf() - vb2_prepare_buf() - __buf_prepare() - __qbuf_userptr() - vb2_vmalloc_get_userptr() - find_vma(), AFAICS without having taken -mmap_sem anywhere in process? The code flow is bloody convoluted and depends on a bunch of things done by initialization, so I certainly might've missed something... This driver is currently missing an active maintainer, as it is for an old hardware (AFAIK, omap is now at version 4, and this is for the first one), but I'm c/c a few developers that might help to test and analyze it. In any case, /me is assuming that your patch is right (as nobody complained), and I'm applying it right now on my tree. This will hopefully allow some people to test. Please make sure you apply v2 (posted as Re: [PATCH] omap_vout: find_vma() needs -mmap_sem held). -- Regards, Laurent Pinchart -- 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
Re: [PATCH] omap_vout: find_vma() needs -mmap_sem held
On Sunday 16 December 2012 20:04:46 Al Viro wrote:
On Sun, Dec 16, 2012 at 09:01:10PM +0100, Paul Bolle wrote:
+ vma = find_vma(mm, virtp);
} else if (vma (vma-vm_flags VM_IO) vma-vm_pgoff) {
Shouldn't that line become
if (vma (vma-vm_flags VM_IO) vma-vm_pgoff) {
so that this actually compiles?
*Do'h*
Yes, it should. Mea culpa...
Signed-off-by: Al Viro v...@zeniv.linux.org.uk
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
diff --git a/drivers/media/platform/omap/omap_vout.c
b/drivers/media/platform/omap/omap_vout.c index 9935040..cb564d0 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -207,19 +207,21 @@ static u32 omap_vout_uservirt_to_phys(u32 virtp)
struct vm_area_struct *vma;
struct mm_struct *mm = current-mm;
- vma = find_vma(mm, virtp);
/* For kernel direct-mapped memory, take the easy way */
- if (virtp = PAGE_OFFSET) {
- physp = virt_to_phys((void *) virtp);
- } else if (vma (vma-vm_flags VM_IO) vma-vm_pgoff) {
+ if (virtp = PAGE_OFFSET)
+ return virt_to_phys((void *) virtp);
+
+ down_read(current-mm-mmap_sem);
+ vma = find_vma(mm, virtp);
+ if (vma (vma-vm_flags VM_IO) vma-vm_pgoff) {
/* this will catch, kernel-allocated, mmaped-to-usermode
addresses */
physp = (vma-vm_pgoff PAGE_SHIFT) + (virtp - vma-vm_start);
+ up_read(current-mm-mmap_sem);
} else {
/* otherwise, use get_user_pages() for general userland pages */
int res, nr_pages = 1;
struct page *pages;
- down_read(current-mm-mmap_sem);
res = get_user_pages(current, current-mm, virtp, nr_pages, 1,
0, pages, NULL);
--
Regards,
Laurent Pinchart
--
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
Fwd: [Bug 51991] ioctl(VIDIOC_QUERYCAP) may return non-ASCII characters
Hi everybody, Any opinion on this ? -- Forwarded Message -- Subject: [Bug 51991] ioctl(VIDIOC_QUERYCAP) may return non-ASCII characters Date: Wednesday 02 January 2013, 14:45:48 From: bugzilla-dae...@bugzilla.kernel.org To: laurent.pinchart+bugzilla-ker...@ideasonboard.com https://bugzilla.kernel.org/show_bug.cgi?id=51991 Alan a...@lxorguk.ukuu.org.uk changed: What|Removed |Added CC||a...@lxorguk.ukuu.org.uk --- Comment #2 from Alan a...@lxorguk.ukuu.org.uk 2013-01-02 14:45:48 --- It should probably be documented as UTF-8 IMHO - -- Regards, Laurent Pinchart -- 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
Re: Fwd: [Bug 51991] ioctl(VIDIOC_QUERYCAP) may return non-ASCII characters
On Mon January 7 2013 15:25:30 Laurent Pinchart wrote: Hi everybody, Any opinion on this ? UTF-8 makes sense to me. Regards, Hans -- Forwarded Message -- Subject: [Bug 51991] ioctl(VIDIOC_QUERYCAP) may return non-ASCII characters Date: Wednesday 02 January 2013, 14:45:48 From: bugzilla-dae...@bugzilla.kernel.org To: laurent.pinchart+bugzilla-ker...@ideasonboard.com https://bugzilla.kernel.org/show_bug.cgi?id=51991 Alan a...@lxorguk.ukuu.org.uk changed: What|Removed |Added CC||a...@lxorguk.ukuu.org.uk --- Comment #2 from Alan a...@lxorguk.ukuu.org.uk 2013-01-02 14:45:48 --- It should probably be documented as UTF-8 IMHO - -- 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
[PATCH RFCv7] dvb: Add DVBv5 stats properties for Quality of Service
The DVBv3 quality parameters are limited on several ways:
- Doesn't provide any way to indicate the used measure,
so userspace need to guess how to calculate the measure;
- Only a limited set of stats are supported;
- Can't be called in a way to require them to be filled
all at once (atomic reads from the hardware), with may
cause troubles on interpreting them on userspace;
- On some OFDM delivery systems, the carriers can be
independently modulated, having different properties.
Currently, there's no way to report per-layer stats.
To address the above issues, adding a new DVBv5-based stats
API.
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
---
v6: Add DocBook documentation.
v7: Some fixes as suggested by Antti
TODO:
- Add methods at the core to periodically collect, store
the statistics and reset the counters;
- Add a driver implementation.
---
Documentation/DocBook/media/dvb/dvbproperty.xml | 97 -
include/uapi/linux/dvb/frontend.h | 70 +-
2 files changed, 164 insertions(+), 3 deletions(-)
diff --git a/Documentation/DocBook/media/dvb/dvbproperty.xml
b/Documentation/DocBook/media/dvb/dvbproperty.xml
index 957e3ac..9168808 100644
--- a/Documentation/DocBook/media/dvb/dvbproperty.xml
+++ b/Documentation/DocBook/media/dvb/dvbproperty.xml
@@ -7,16 +7,29 @@ the capability ioctls weren't implemented yet via the new
way./para
paraThe typical usage for the
constantFE_GET_PROPERTY/FE_SET_PROPERTY/constant
API is to replace the ioctl's were the link linkend=dvb-frontend-parameters
struct constantdvb_frontend_parameters/constant/link were used./para
+section id=dtv-stats
+titleDTV stats type/title
+programlisting
+struct dtv_stats {
+__u16 value;
+__u8 scale;
+} __attribute__ ((packed));
+/programlisting
+/section
section id=dtv-property
titleDTV property type/title
programlisting
/* Reserved fields should be set to 0 */
+
struct dtv_property {
__u32 cmd;
union {
__u32 data;
struct {
- __u8 data[32];
+ union {
+ __u8 data[32];
+ __u16 data[16];
+ }
__u32 len;
__u32 reserved1[3];
void *reserved2;
@@ -850,6 +863,78 @@ enum fe_interleaving {
parause the special macro LNA_AUTO to set LNA auto/para
/section
/section
+
+ section id=frontend-qos-properties
+ titleFrontend Quality of Service/Statistics indicators/title
+ paraExcept for link
linkend=DTV-QOS-ENUMconstantDTV_QOS_ENUM/constant/link,
+ the values are returned via
constantdtv_property.stat/constant./para
+ paraFor most delivery systems, this will return a single value for
each parameter./para
+ paraIt should be noticed, however, that new OFDM delivery systems
+ like ISDB can use different modulation types for each group of carriers.
+ On such standards, up to 3 groups of statistics can be provided, one
+ for each carrier group (called layer on ISDB).
+ In order to be consistent with other delivery systems, the first
+ value at link
linkend=dtv-statsconstantdtv_property.stat.dtv_stats/constant/link
array refers to
+ a global indicator, if any. The other elements of the array represent
+ each layer, starting from layer A(index 1), layer B (index 2) and so
on/para
+ paraThe number of filled elements are stored at
constantdtv_property.stat.len/constant./para
+ paraEach element of the
constantdtv_property.stat.dtv_stats/constant array consists on two
elements:/para
+ itemizedlist mark='opencircle'
+ listitemparaconstantvalue/constant - Value of the
measure/para/listitem
+ listitemparaconstantscale/constant - Scale for the
value. It can be:/para
+ section id = fecap-scale-params
+ itemizedlist mark='bullet'
+
listitemparaconstantFE_SCALE_NOT_AVAILABLE/constant - If it is not
possible to collect a given parameter (could be a transitory or permanent
condition)/para/listitem
+
listitemparaconstantFE_SCALE_DECIBEL/constant - parameter is a signed
value, measured in 0.1 dB/para/listitem
+
listitemparaconstantFE_SCALE_RELATIVE/constant - parameter is a
unsigned value, where 0 means 0% and 65535 means 100%./para/listitem
+
listitemparaconstantFE_SCALE_COUNTER/constant - parameter is a unsigned
value that counts the occurrence of an event, like bit error, block error, or
lapsed time./para/listitem
+ /itemizedlist
+ /section
+ /listitem
+
Re: [PATCH] [media] coda: Fix build due to iram.h rename
Em Mon, 7 Jan 2013 08:16:02 -0200 Fabio Estevam feste...@gmail.com escreveu: Hi Sascha, On Mon, Jan 7, 2013 at 8:03 AM, Sascha Hauer s.ha...@pengutronix.de wrote: commit c045e3f13 (ARM: imx: include iram.h rather than mach/iram.h) changed the location of iram.h, which causes the following build error when building the coda driver: drivers/media/platform/coda.c:27:23: error: mach/iram.h: No such file or directory drivers/media/platform/coda.c: In function 'coda_probe': drivers/media/platform/coda.c:2000: error: implicit declaration of function 'iram_alloc' drivers/media/platform/coda.c:2001: warning: assignment makes pointer from integer without a cast drivers/media/platform/coda.c: In function 'coda_remove': drivers/media/platform/coda.c:2024: error: implicit declaration of function 'iram_free' Since the content of iram.h is not imx specific, move it to include/linux/platform_data/imx-iram.h instead. This is an intermediate solution until the i.MX iram allocator is converted to the generic SRAM allocator. Signed-off-by: Sascha Hauer s.ha...@pengutronix.de --- Based on an earlier version from Fabio Estevam, but this one moves iram.h to include/linux/platform_data/imx-iram.h instead of include/linux/iram.h which is a less generic name. arch/arm/mach-imx/iram.h | 41 arch/arm/mach-imx/iram_alloc.c |3 +-- drivers/media/platform/coda.c |2 +- include/linux/platform_data/imx-iram.h | 41 4 files changed, 43 insertions(+), 44 deletions(-) delete mode 100644 arch/arm/mach-imx/iram.h create mode 100644 include/linux/platform_data/imx-iram.h It would be better to use git mv /git format-patch -M, so that git can detect the file rename. Agreed. Anyway, I applied here and did a git show -M: From: Sascha Hauer s.ha...@pengutronix.de Date: Mon, 7 Jan 2013 11:03:45 +0100 Subject: [PATCH] coda: Fix build due to iram.h rename commit c045e3f13 (ARM: imx: include iram.h rather than mach/iram.h) changed the location of iram.h, which causes the following build error when building the coda driver: drivers/media/platform/coda.c:27:23: error: mach/iram.h: No such file or directory drivers/media/platform/coda.c: In function 'coda_probe': drivers/media/platform/coda.c:2000: error: implicit declaration of function 'iram_alloc' drivers/media/platform/coda.c:2001: warning: assignment makes pointer from integer without a cast drivers/media/platform/coda.c: In function 'coda_remove': drivers/media/platform/coda.c:2024: error: implicit declaration of function 'iram_free' Since the content of iram.h is not imx specific, move it to include/linux/platform_data/imx-iram.h instead. This is an intermediate solution until the i.MX iram allocator is converted to the generic SRAM allocator. Signed-off-by: Sascha Hauer s.ha...@pengutronix.de diff --git a/arch/arm/mach-imx/iram_alloc.c b/arch/arm/mach-imx/iram_alloc.c index 6c80424..e05cf40 100644 --- a/arch/arm/mach-imx/iram_alloc.c +++ b/arch/arm/mach-imx/iram_alloc.c @@ -22,8 +22,7 @@ #include linux/module.h #include linux/spinlock.h #include linux/genalloc.h - -#include iram.h +#include linux/platform_data/imx-iram.h static unsigned long iram_phys_base; static void __iomem *iram_virt_base; diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index 2721f83..16a243d 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -23,8 +23,8 @@ #include linux/slab.h #include linux/videodev2.h #include linux/of.h +#include linux/platform_data/imx-iram.h -#include mach/iram.h #include media/v4l2-ctrls.h #include media/v4l2-device.h #include media/v4l2-ioctl.h diff --git a/arch/arm/mach-imx/iram.h b/include/linux/platform_data/imx-iram.h similarity index 100% rename from arch/arm/mach-imx/iram.h rename to include/linux/platform_data/imx-iram.h Cheers, Mauro -- 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
Re: [PATCH] [media] coda: Fix build due to iram.h rename
Em Mon, 7 Jan 2013 13:46:05 -0200 Mauro Carvalho Chehab mche...@infradead.org escreveu: Em Mon, 7 Jan 2013 08:16:02 -0200 Fabio Estevam feste...@gmail.com escreveu: Hi Sascha, ... It would be better to use git mv /git format-patch -M, so that git can detect the file rename. Agreed. Anyway, I applied here and did a git show -M: From: Sascha Hauer s.ha...@pengutronix.de Date: Mon, 7 Jan 2013 11:03:45 +0100 Subject: [PATCH] coda: Fix build due to iram.h rename commit c045e3f13 (ARM: imx: include iram.h rather than mach/iram.h) changed the location of iram.h, which causes the following build error when building the coda driver: drivers/media/platform/coda.c:27:23: error: mach/iram.h: No such file or directory drivers/media/platform/coda.c: In function 'coda_probe': drivers/media/platform/coda.c:2000: error: implicit declaration of function 'iram_alloc' drivers/media/platform/coda.c:2001: warning: assignment makes pointer from integer without a cast drivers/media/platform/coda.c: In function 'coda_remove': drivers/media/platform/coda.c:2024: error: implicit declaration of function 'iram_free' Since the content of iram.h is not imx specific, move it to include/linux/platform_data/imx-iram.h instead. This is an intermediate solution until the i.MX iram allocator is converted to the generic SRAM allocator. Signed-off-by: Sascha Hauer s.ha...@pengutronix.de Patch looks OK on my eyes. Acked-by: Mauro Carvalho Chehab mche...@redhat.com diff --git a/arch/arm/mach-imx/iram_alloc.c b/arch/arm/mach-imx/iram_alloc.c index 6c80424..e05cf40 100644 --- a/arch/arm/mach-imx/iram_alloc.c +++ b/arch/arm/mach-imx/iram_alloc.c @@ -22,8 +22,7 @@ #include linux/module.h #include linux/spinlock.h #include linux/genalloc.h - -#include iram.h +#include linux/platform_data/imx-iram.h static unsigned long iram_phys_base; static void __iomem *iram_virt_base; diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index 2721f83..16a243d 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -23,8 +23,8 @@ #include linux/slab.h #include linux/videodev2.h #include linux/of.h +#include linux/platform_data/imx-iram.h -#include mach/iram.h #include media/v4l2-ctrls.h #include media/v4l2-device.h #include media/v4l2-ioctl.h diff --git a/arch/arm/mach-imx/iram.h b/include/linux/platform_data/imx-iram.h similarity index 100% rename from arch/arm/mach-imx/iram.h rename to include/linux/platform_data/imx-iram.h Cheers, Mauro -- 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
Re: [PATCH 0/4] Some IR fixes for I2C devices on em28xx
Em Sun, 06 Jan 2013 21:26:46 +0100 Frank Schäfer fschaefer@googlemail.com escreveu: Am 05.01.2013 16:35, schrieb Mauro Carvalho Chehab: Em Sat, 05 Jan 2013 14:42:10 +0100 Frank Schäfer fschaefer@googlemail.com escreveu: Am 05.01.2013 14:22, schrieb Frank Schäfer: Am 04.01.2013 22:15, schrieb Mauro Carvalho Chehab: Frank pointed that IR was not working with I2C devices. So, I took some time to fix them. Tested with Hauppauge WinTV USB2. Mauro Carvalho Chehab (4): [media] em28xx: initialize button/I2C IR earlier [media] em28xx: autoload em28xx-rc if the device has an I2C IR [media] em28xx: simplify IR names on I2C devices [media] em28xx: tell ir-kbd-i2c that WinTV uses an RC5 protocol drivers/media/usb/em28xx/em28xx-cards.c | 2 +- drivers/media/usb/em28xx/em28xx-input.c | 29 - 2 files changed, 17 insertions(+), 14 deletions(-) While these patches make I2C IR remote controls working again, they leave several issues unaddressed which should really be fixed: 1) the i2c client isn't unregistered on module unload. This was the reason for patch 2 in my series. There is also a FIXME comment about this in em28xx_release_resources() (although this is the wrong place to do it). 2) there is no error checking in em28xx_register_i2c_ir(). em28xx_ir_init should really bail out if no i2c device is found. 3) All RC maps should be assigned at the same place, no matter if the receiver/demodulator is built in or external. Spreading them over the code is inconsistent and makes the code bug prone. 4) the list of known i2c devices in em28xx-i2c.c misses client address 0x3e 1 = 0x1f. See client list in em28xx_register_i2c_ir(). 5) there should be a warning message for the case that we call ir-kbd-i2c with an unknown rc device. 6) because we use our own key polling functions with ir-kbd-i2c, we should also select the polling interval value manually. That makes things consistent and avoids confusion. The rest is a matter of taste / prefered code layout. I'm fine with it. Regards, Frank It seems like already applied them... :( While I certainly appreciate patches beeing applied as soon as possible, I think there should really be a chance to review them before this happens. Especially when the changes are non-trivial and someone else has posted patches addressing the same issues before (other contributers might feel offended ;) ). All the 4 applied patches are really trivial: - patch 1: just reorder existing code; - patch 2: one-line patch adding another condition to an existing if; - patch 3: pure string rename; - patch 4: one line patch properly reporting the RC5 protocol on WinTV. Just because a patch just reorders existing code or just changes a single line it's not automatically trivial. True, but this is not the case of all the above ones: all of them are obvious, and do what are described there. I'm sure you have seen more cases than me in which it were patches like this who caused big trouble. ;) Trivial patches are known to cause troubles, just like any other code change. The thing is that trivial patches are: 1) easy to review, especially when properly described; 2) when they're wrong, easy to fix; 3) easy to get them accepted/merged at stable kernels. In any case, if you found any breakage caused by the above patches, feel free to send a patch fixing it. If you're concerned with other things that aren't there, send in separate. The rule is one patch by each logical change. Yeah, I understand your time problems and I really appreciate patches beeing applied as soon as possible (after they have been reviewed). But delaying a patch for a few days really shouldn't cause too much extra work. It causes, as my main work is unrelated to drivers/media maintenance. So, it may take weeks for me to be able to get a time slice big enough for installing a kernel on my test machines and re-test some code. Cheers, Mauro -- 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
Re: [RFC] Initial scan files troubles and brainstorming
On Mon, Jan 7, 2013 at 6:23 PM, Christoph Pfister christophpfis...@gmail.com wrote: Okay guys, I think this is a good time to discuss scan file maintenance [ should have taken care of it earlier, but well ...]. I've been updating the scan data for the past few years, but found barely no time in the last (many!) months. This won't change in future, so I've decided to step down from the task; may others do what is necessary ... Christoph, It's unfortunate that you decided to drop the ball. I will try to take care of it in the meanwhile.. Regards, Manu -- 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
[GIT PULL FOR v3.7] Fix modulator regression in kernel 3.7.
Hi all, This patch fixes a regression that was introduced in 3.7 and made these four drivers useless. It's the same problem for all: the new vfl_dir field wasn't set correctly for these radio modulator drivers. Tested with the radio-keene driver. Regards, Hans The following changes since commit 8cd7085ff460ead3aba6174052a408f4ad52ac36: [media] get_dvb_firmware: Fix the location of firmware for Terratec HTC (2013-01-01 11:18:26 -0200) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git modulators for you to fetch changes up to 86552330c91fff094a07c0018ca34a9f45362a64: radio: set vfl_dir correctly to fix modulator regression. (2013-01-05 13:01:30 +0100) Hans Verkuil (1): radio: set vfl_dir correctly to fix modulator regression. drivers/media/radio/radio-keene.c |1 + drivers/media/radio/radio-si4713.c |1 + drivers/media/radio/radio-wl1273.c |1 + drivers/media/radio/wl128x/fmdrv_v4l2.c | 10 ++ 4 files changed, 13 insertions(+) -- 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
Re: Status of the patches under review at LMML (35 patches)
On Sun, Jan 6, 2013 at 7:04 PM, Mauro Carvalho Chehab mche...@redhat.com wrote: This is the summary of the patches that are currently under review at Linux Media Mailing List linux-media@vger.kernel.org. Each patch is represented by its submission date, the subject (up to 70 chars) and the patchwork link (if submitted via email). P.S.: This email is c/c to the developers where some action is expected. If you were copied, please review the patches, acking/nacking or submitting an update. == New patches == Those patches require some review from the community: This one could break again DVB-S-DVB-S2 support, so, it needs to be carefully reviewed and tested: Jun,21 2012: [media] dvb frontend core: tuning in ISDB-T using DVB API v3 http://patchwork.linuxtv.org/patch/12988 Olivier Grenie olivier.gre...@parrot.com This one fix a code that, IMHO, should, instead be replaced by something better: Sep,17 2012: [3/3] cx25821: Cleanup filename assignment code http://patchwork.linuxtv.org/patch/14445 Peter Senna Tschudin peter.se...@gmail.com This one doesn't seem right for me. Anybody can test/work with it? Sep, 2 2012: fix: iMon Knob event interpretation issues http://patchwork.linuxtv.org/patch/16030 Alexandre Lissy alexandreli...@free.fr I'm not sure if we should apply this one or not, as it will increase the probability of miss-interpreting a nec IR protocol. Comments? Jul,26 2012: media: rc: Add support to decode Remotes using NECx IR protocol http://patchwork.linuxtv.org/patch/13480 Ravi Kumar V kumar...@codeaurora.org == Manu Abraham abraham.m...@gmail.com == Those patches are there for a long time. I think I'll simply apply all of them, if they're not reviewed on the next couple weeks: Mar,11 2012: [2/3] stv090x: use error counter 1 for BER estimation http://patchwork.linuxtv.org/patch/10301 Andreas Regel andreas.re...@gmx.de Mar,11 2012: [3/3] stv090x: On STV0903 do not set registers of the second path. http://patchwork.linuxtv.org/patch/10302 Andreas Regel andreas.re...@gmx.de Nov,29 2011: stv090x: implement function for reading uncorrected blocks count http://patchwork.linuxtv.org/patch/8656 Mariusz Bia?o?czyk ma...@skyboo.net Jun, 8 2011: Add remote control support for mantis http://patchwork.linuxtv.org/patch/7217 Christoph Pinkl christoph.pi...@gmail.com Apr, 1 2012: [05/11] Slightly more friendly debugging output. http://patchwork.linuxtv.org/patch/10520 Steinar H. Gunderson se...@samfundet.no Apr, 1 2012: [06/11] Replace ca_lock by a slightly more general int_stat_lock. http://patchwork.linuxtv.org/patch/10521 Steinar H. Gunderson se...@samfundet.no Apr, 1 2012: [07/11] Fix a ton of SMP-unsafe accesses. http://patchwork.linuxtv.org/patch/10523 Steinar H. Gunderson se...@samfundet.no Apr, 1 2012: [08/11] Remove some unused structure members. http://patchwork.linuxtv.org/patch/10525 Steinar H. Gunderson se...@samfundet.no Apr, 1 2012: [09/11] Correct wait_event_timeout error return check. http://patchwork.linuxtv.org/patch/10526 Steinar H. Gunderson se...@samfundet.no Apr, 1 2012: [10/11] Ignore timeouts waiting for the IRQ0 flag. http://patchwork.linuxtv.org/patch/10527 Steinar H. Gunderson se...@samfundet.no Apr, 1 2012: [11/11] Enable Mantis CA support. http://patchwork.linuxtv.org/patch/10524 Steinar H. Gunderson se...@samfundet.no Somehow, these patches missed me. This weekend, I am traveling. I will take a look at it during the weekend after that one. Regards, Manu -- 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
Re: [PATCH 6/6] ir-kbd-i2c: fix get_key_knc1()
Em Sun, 06 Jan 2013 21:32:31 +0100
Frank Schäfer fschaefer@googlemail.com escreveu:
Am 05.01.2013 16:25, schrieb Mauro Carvalho Chehab:
Em Sat, 05 Jan 2013 14:32:30 +0100
Frank Schäfer fschaefer@googlemail.com escreveu:
Am 05.01.2013 03:39, schrieb Mauro Carvalho Chehab:
Em Fri, 28 Dec 2012 00:02:48 +0100
Frank Schäfer fschaefer@googlemail.com escreveu:
- return valid key code when button is hold
- debug: print key code only when a button is pressed
Tested with device Terratec Cinergy 200 USB (em28xx).
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
---
drivers/media/i2c/ir-kbd-i2c.c | 15 +--
1 Datei geändert, 5 Zeilen hinzugefügt(+), 10 Zeilen entfernt(-)
diff --git a/drivers/media/i2c/ir-kbd-i2c.c
b/drivers/media/i2c/ir-kbd-i2c.c
index 08ae067..2984b7d 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -184,18 +184,13 @@ static int get_key_knc1(struct IR_i2c *ir, u32
*ir_key, u32 *ir_raw)
return -EIO;
}
-/* it seems that 0xFE indicates that a button is still hold
- down, while 0xff indicates that no button is hold
- down. 0xfe sequences are sometimes interrupted by 0xFF */
-
-dprintk(2,key %02x\n, b);
-
-if (b == 0xff)
+if (b == 0xff) /* no button */
return 0;
-if (b == 0xfe)
-/* keep old data */
-return 1;
+if (b == 0xfe) /* button is still hold */
+b = ir-rc-last_scancode; /* keep old data */
+
+dprintk(2,key %02x\n, b);
*ir_key = b;
*ir_raw = b;
Don't do that. This piece of code is old, and it was added there
before the em28xx driver. Originally, the ir-i2c-kbd were used by
bttv and saa7134 drivers and the code there were auto-detecting the
I2C IR hardware decoding chips that used to be very common on media
devices. I'm almost sure that the original device that started using
this code is this model:
drivers/media/pci/bt8xx/bttv-cards.c: .name =
Typhoon TView RDS + FM Stereo / KNC1 TV Station RDS,
That's why it is called as KNC1, but there are other cards that use
it as well. I think I have one bttv using it. Not sure.
The routine on em28xx is a fork of the original one, that was changed
to work with the devices there.
Indeed, it's a fork, 100% identical:
static int em28xx_get_key_terratec(struct IR_i2c *ir, u32 *ir_key, u32
*ir_raw)
{
unsigned char b;
/* poll IR chip */
if (1 != i2c_master_recv(ir-c, b, 1)) {
i2cdprintk(read error\n);
return -EIO;
}
/* it seems that 0xFE indicates that a button is still hold
down, while 0xff indicates that no button is hold
down. 0xfe sequences are sometimes interrupted by 0xFF */
i2cdprintk(key %02x\n, b);
if (b == 0xff)
return 0;
if (b == 0xfe)
/* keep old data */
return 1;
*ir_key = b;
*ir_raw = b;
return 1;
}
static int get_key_knc1(struct IR_i2c *ir, u32 *ir_key, u32 *ir_raw)
{
unsigned char b;
/* poll IR chip */
if (1 != i2c_master_recv(ir-c, b, 1)) {
dprintk(1,read error\n);
return -EIO;
}
/* it seems that 0xFE indicates that a button is still hold
down, while 0xff indicates that no button is hold
down. 0xfe sequences are sometimes interrupted by 0xFF */
dprintk(2,key %02x\n, b);
if (b == 0xff)
return 0;
if (b == 0xfe)
/* keep old data */
return 1;
*ir_key = b;
*ir_raw = b;
return 1;
}
Why should we keep two 100% identical functions ? See patch 4/6.
I'm 99% sure that both devices are absolutely identical.
99% sure is not enough. A simple firmware difference at the microprocessor
is enough to make the devices different.
I agree, but that's irrelevant. What counts is that the _code_ ist 100%
identical.
Also, this was widely discussed several years ago, when we decided to
cleanup
the I2C code. We then invested lot of efforts to move those get_keys away
from ir-i2c-kbd. The only things left there are the ones we identified that
were needed by auto-detection mode on old devices that we don't have.
What was decided is to move everything that we know to the *-input driver,
keeping there only the legacy stuff.
Uhm... ok.
My assumption was, that the goal is the opposite (move as much common
code as possible to i2c-ir-kbd).
I'm a bit puzzled about this decision...
Okay but then... why do we still use ir-kbd-i2c in em28xx ?
We can easily get rid of it. Everything we need is already on board.
I can send a patch if you want.
No. We don't want to mix I2C client code inside the I2C bus drivers.
If we
Re: [PATCH 0/4] Some IR fixes for I2C devices on em28xx
Em Sun, 06 Jan 2013 21:20:31 +0100 Frank Schäfer fschaefer@googlemail.com escreveu: Am 05.01.2013 16:06, schrieb Mauro Carvalho Chehab: Em Sat, 05 Jan 2013 14:22:08 +0100 Frank Schäfer fschaefer@googlemail.com escreveu: Am 04.01.2013 22:15, schrieb Mauro Carvalho Chehab: Frank pointed that IR was not working with I2C devices. So, I took some time to fix them. Tested with Hauppauge WinTV USB2. Mauro Carvalho Chehab (4): [media] em28xx: initialize button/I2C IR earlier [media] em28xx: autoload em28xx-rc if the device has an I2C IR [media] em28xx: simplify IR names on I2C devices [media] em28xx: tell ir-kbd-i2c that WinTV uses an RC5 protocol drivers/media/usb/em28xx/em28xx-cards.c | 2 +- drivers/media/usb/em28xx/em28xx-input.c | 29 - 2 files changed, 17 insertions(+), 14 deletions(-) While these patches make I2C IR remote controls working again, they leave several issues unaddressed which should really be fixed: 1) the i2c client isn't unregistered on module unload. This was the reason for patch 2 in my series. There is also a FIXME comment about this in em28xx_release_resources() (although this is the wrong place to do it). AFAIKT, this is not really needed, as the I2C clients are unregistered when the I2C bus is unregistered. So, a device disconnect will release it. Also, an em28xx driver unload. The only difference might be if just ir-kbd-i2c and em28xx-rc are unloaded, but em28xx is still loaded, but I think that, even on this case, calling the .release code for an I2C bus will release it. So, I don't see any need for such patch. I might be wrong, of course, but, in order to proof that a release code is needed, you'll need to check if some memory are lost after module load/unload. Mauro, just because code luckily 'works' in the current constellation, it isn't necessarily good code. It doesn't luckly 'works'. It is rock solid. There were really few bug reports for ir-kbd-i2c during all those years. It's this kind of issues that can easily cause regressions if the code changes somewhere else. Nah. What causes regression is touching on a code that works for no good reason and without enough testing. Btw, I was told that audio on HVR-950 seemed to stop working. Not sure what broke it, but, as I tested it some time ago, I suspect was due to one of those recent patches (v4l2-compliance, vb2 or your patches - we need to bisect to discover what broke it). None of those touched on em28xx-alsa directly, but perhaps one of the patches had a bad side effect. em28xx-input registers the i2c device, so it should unregister it on uninit/close/unload, too. Pretty simple and easy to fix (+5 - 2 = 3 additional lines of code). Fair enough. Please send us a patch, after enough testing. 2) there is no error checking in em28xx_register_i2c_ir(). em28xx_ir_init should really bail out if no i2c device is found. A failure to initialize IR should not be fatal for the driver, as the rest of the hardware still works. I'm talking about em28xx-input and the RC part only. Of course not the whole driver. Do you really want to load module ir-kbd-i2c even though there is no device ? Also, there's no way to warrant that the I2C code is actually running, as ir-i2c-kbd may not even be compiled. So, returning 0 there doesn't mean that IR is working. You can check the success of request_module. The whole thing is really easy to fix, I fail to see why you don't want to do it. Ok, such change may make sense, but only as a separate patch, and not as a big fix patch that does something else. 3) All RC maps should be assigned at the same place, no matter if the receiver/demodulator is built in or external. Spreading them over the code is inconsistent and makes the code bug prone. I don't agree. It is better to keep RC maps for those devices together with the RC protocol setting, get_key config, etc. At boards config, it is very easy to identify I2C IR's, as there's an special field there to mark those devices (has_ir_i2c). So, if the board has_ir_i2c, the IR config is inside em28xx-input. ... which is exactly what made it so easy to cause this regression !!! It's not obvious for programmers that no RC map has to be specified for i2c RCs in the board data. It's also not obvious that em28xx-input silently overwrites the rc-map assigned at board level. In general, it's not obvious that two completely different code areas have to be touched for these devices. That's why we really should avoid those board specific code parts spread all over the driver as much as possible. In case of the RC map it's really easy. I also fail to see what you would loose in em28xx-input. We would still assign the RC map to dev-init_data. If you prefer seeing the used RC map in the em28xx-input code directly, then the same should apply for devices with built-in
Re: BUG: bttv does not load module ir-kbd-i2c for Hauppauge model 37284, rev B421
Em Sun, 06 Jan 2013 21:36:45 +0100 Frank Schäfer fschaefer@googlemail.com escreveu: Am 05.01.2013 16:57, schrieb Mauro Carvalho Chehab: Em Sat, 05 Jan 2013 15:00:18 +0100 Frank Schäfer fschaefer@googlemail.com escreveu: While we are at it ;) : [ 15.280772] bttv: Bt8xx card found (0) [ 15.281349] bttv: 0: Bt878 (rev 17) at :01:08.0, irq: 18, latency: 32, mmio: 0xfdfff000 [ 15.281386] bttv: 0: detected: Hauppauge WinTV [card=10], PCI subsystem ID is 0070:13eb [ 15.281391] bttv: 0: using: Hauppauge (bt878) [card=10,insmod option] [ 15.283964] bttv: 0: Hauppauge/Voodoo msp34xx: reset line init [5] [ 15.316043] tveeprom 4-0050: Hauppauge model 37284, rev B421, serial# 5111944 [ 15.316049] tveeprom 4-0050: tuner model is Philips FM1216 (idx 21, type 5) [ 15.316054] tveeprom 4-0050: TV standards PAL(B/G) (eeprom 0x04) [ 15.316059] tveeprom 4-0050: audio processor is MSP3410D (idx 5) [ 15.316063] tveeprom 4-0050: has radio [ 15.316066] bttv: 0: Hauppauge eeprom indicates model#37284 [ 15.316071] bttv: 0: tuner type=5 [ 16.178816] bttv: 0: registered device video0 [ 16.179071] bttv: 0: registered device vbi0 [ 16.180587] bttv: 0: registered device radio0 When I load module ir-kbd-i2c manually: Registered IR keymap rc-hauppauge input: i2c IR (Hauppauge) as /devices/virtual/rc/rc0/input6 rc0: i2c IR (Hauppauge) as /devices/virtual/rc/rc0 ir-kbd-i2c: i2c IR (Hauppauge) detected at i2c-4/4-0018/ir0 [bt878 #0 [sw]] Remote control works fine then. Yeah, this is a known misfeature of bttv, and very likely documented on several wiki pages like those: http://linuxtv.org/wiki/index.php/Remote_controllers-V4L (btw, this wiki page is very likely outdated) http://www.mythtv.org/wiki/Ir-kbd-i2c http://www.linuxtv.org/wiki/index.php/Prolink_Pixelview_PlayTV_Pro I can't remember if this were always this way, or if this was caused by the I2C core redesign (2006?). I think it was always like that, and, as I2C comes with a cost (polling can affect video processing with old machines), so, we decided to not do the auto-load on the older drivers that weren't doing it already. I'm not sure I understand you. Is it a misfeature or intentional ? I think it was intentional, but I can't remember for sure. If it wasn't intentional, its behavior changed when the I2C 'application type' flags got removed (~5-6 years ago). Does polling 3 bytes every 100ms really affect the performance of video processing in a perceptable manner ? On that time, yes. The big kernel lock were removed only on recent versions. Also, CPUs/GPUs weren't that fast 7 years ago. Btw, the fact that there's no clear indication about what bttv boards require I2C made hard to remove the get_key codes from ir-kbd-i2c. See my previous post, I thought the intention is to do the opposite. No. We tried hard to identify what board were using what get_key code, and moved the drivers specifics code to the driver. It probably makes sense to add a has_ir_i2c field at bttv, add a flag there at modprobe to prevent the autoload, and start tagging the boards with I2C IR with such tag. Without having looked into the code, it seems that the driver detects the i2c rc already without a board flag. Otherwise it wouldn't register the i2c device. Unfortunately, it doesn't display a message. No. In the past (kernel 2.4 and upper), I2C bus used to work with 0-len reads to scan the used I2C addresses. The I2C drivers like tuners, demods, IR's etc used to register to the I2C core saying that they were to be used on TV boards. The I2C logic binds them to the I2C bus driver when they were detected, during the scanning process. That's why it is so hard to know what boards are using I2C remotes, on those older drivers. We spent a lot of time seeking at the git logs to identify, before being able to move the code away from ir-i2c-kbd. If you care enough, feel free to send us such patch. I care enough, but it has a low priority for me at the moment. Regards, Frank Cheers, Mauro -- Cheers, Mauro -- 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
[PATCH RFCv8 2/2] dvb: the core logic to handle the DVBv5 QoS properties
Add the logic to poll, reset counters and report the QoS stats
to the end user.
The idea is that the core will periodically poll the frontend for
the stats. The frontend may return -EBUSY, if the previous collect
didn't finish, or it may fill the cached data.
The value returned to the end user is always the cached data.
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
---
drivers/media/dvb-core/dvb_frontend.c | 36 +++
drivers/media/dvb-core/dvb_frontend.h | 12
2 files changed, 48 insertions(+)
diff --git a/drivers/media/dvb-core/dvb_frontend.c
b/drivers/media/dvb-core/dvb_frontend.c
index dd35fa9..f8be7be 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -260,6 +260,9 @@ static int dvb_frontend_get_event(struct dvb_frontend *fe,
return ret;
}
+ if (fe-ops.get_qos_stats)
+ fe-ops.get_qos_stats(fe);
+
mutex_lock(events-mtx);
*event = events-events[events-eventr];
events-eventr = (events-eventr + 1) % MAX_EVENT;
@@ -1053,6 +1056,15 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] =
{
_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_B, 0, 0),
_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_C, 0, 0),
_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_D, 0, 0),
+
+ /* Statistics API */
+ _DTV_CMD(DTV_QOS_ENUM, 0, 0),
+ _DTV_CMD(DTV_QOS_SIGNAL_STRENGTH, 0, 0),
+ _DTV_CMD(DTV_QOS_CNR, 0, 0),
+ _DTV_CMD(DTV_QOS_BIT_ERROR_COUNT, 0, 0),
+ _DTV_CMD(DTV_QOS_TOTAL_BITS_COUNT, 0, 0),
+ _DTV_CMD(DTV_QOS_ERROR_BLOCK_COUNT, 0, 0),
+ _DTV_CMD(DTV_QOS_TOTAL_BLOCKS_COUNT, 0, 0),
};
static void dtv_property_dump(struct dvb_frontend *fe, struct dtv_property
*tvp)
@@ -1443,6 +1455,25 @@ static int dtv_property_process_get(struct dvb_frontend
*fe,
tvp-u.data = c-lna;
break;
+ /* Fill quality measures */
+ case DTV_QOS_SIGNAL_STRENGTH:
+ tvp-u.st = c-strength;
+ break;
+ case DTV_QOS_CNR:
+ tvp-u.st = c-cnr;
+ break;
+ case DTV_QOS_BIT_ERROR_COUNT:
+ tvp-u.st = c-bit_error;
+ break;
+ case DTV_QOS_TOTAL_BITS_COUNT:
+ tvp-u.st = c-bit_count;
+ break;
+ case DTV_QOS_ERROR_BLOCK_COUNT:
+ tvp-u.st = c-block_error;
+ break;
+ case DTV_QOS_TOTAL_BLOCKS_COUNT:
+ tvp-u.st = c-block_count;
+ break;
default:
return -EINVAL;
}
@@ -1705,6 +1736,8 @@ static int dtv_property_process_set(struct dvb_frontend
*fe,
break;
case DTV_DELIVERY_SYSTEM:
r = set_delivery_system(fe, tvp-u.data);
+ if (r = 0 fe-ops.reset_qos_counters)
+ fe-ops.reset_qos_counters(fe);
break;
case DTV_VOLTAGE:
c-voltage = tvp-u.data;
@@ -2305,6 +2338,9 @@ static int dvb_frontend_ioctl_legacy(struct file *file,
if (err)
break;
err = dtv_set_frontend(fe);
+ if (err = 0 fe-ops.reset_qos_counters)
+ fe-ops.reset_qos_counters(fe);
+
break;
case FE_GET_EVENT:
err = dvb_frontend_get_event (fe, parg, file-f_flags);
diff --git a/drivers/media/dvb-core/dvb_frontend.h
b/drivers/media/dvb-core/dvb_frontend.h
index 97112cd..7bb49f1 100644
--- a/drivers/media/dvb-core/dvb_frontend.h
+++ b/drivers/media/dvb-core/dvb_frontend.h
@@ -315,6 +315,10 @@ struct dvb_frontend_ops {
int (*set_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
int (*get_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
+
+ /* QoS statistics callbacks */
+ int (*get_qos_stats)(struct dvb_frontend *fe);
+ int (*reset_qos_counters)(struct dvb_frontend *fe);
};
#ifdef __DVB_CORE__
@@ -393,6 +397,14 @@ struct dtv_frontend_properties {
u8 atscmh_sccc_code_mode_d;
u32 lna;
+
+ /* QoS statistics data */
+ struct dtv_fe_stats strength;
+ struct dtv_fe_stats cnr;
+ struct dtv_fe_stats bit_error;
+ struct dtv_fe_stats bit_count;
+ struct dtv_fe_stats block_error;
+ struct dtv_fe_stats block_count;
};
struct dvb_frontend {
--
1.7.11.7
--
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
[PATCH RFCv8 1/2] dvb: Add DVBv5 stats properties for Quality of Service
The DVBv3 quality parameters are limited on several ways:
- Doesn't provide any way to indicate the used measure,
so userspace need to guess how to calculate the measure;
- Only a limited set of stats are supported;
- Can't be called in a way to require them to be filled
all at once (atomic reads from the hardware), with may
cause troubles on interpreting them on userspace;
- On some OFDM delivery systems, the carriers can be
independently modulated, having different properties.
Currently, there's no way to report per-layer stats.
To address the above issues, adding a new DVBv5-based stats
API.
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
---
v6: Add DocBook documentation.
v7: Some fixes as suggested by Antti
V8: Documentation fix, compilation fix and name the stats struct,
for its reusage inside the core
TODO:
- Add a driver implementation.
---
Documentation/DocBook/media/dvb/dvbproperty.xml | 97 -
include/uapi/linux/dvb/frontend.h | 83 -
2 files changed, 177 insertions(+), 3 deletions(-)
diff --git a/Documentation/DocBook/media/dvb/dvbproperty.xml
b/Documentation/DocBook/media/dvb/dvbproperty.xml
index 957e3ac..9168808 100644
--- a/Documentation/DocBook/media/dvb/dvbproperty.xml
+++ b/Documentation/DocBook/media/dvb/dvbproperty.xml
@@ -7,16 +7,29 @@ the capability ioctls weren't implemented yet via the new
way./para
paraThe typical usage for the
constantFE_GET_PROPERTY/FE_SET_PROPERTY/constant
API is to replace the ioctl's were the link linkend=dvb-frontend-parameters
struct constantdvb_frontend_parameters/constant/link were used./para
+section id=dtv-stats
+titleDTV stats type/title
+programlisting
+struct dtv_stats {
+__u16 value;
+__u8 scale;
+} __attribute__ ((packed));
+/programlisting
+/section
section id=dtv-property
titleDTV property type/title
programlisting
/* Reserved fields should be set to 0 */
+
struct dtv_property {
__u32 cmd;
union {
__u32 data;
struct {
- __u8 data[32];
+ union {
+ __u8 data[32];
+ __u16 data[16];
+ }
__u32 len;
__u32 reserved1[3];
void *reserved2;
@@ -850,6 +863,78 @@ enum fe_interleaving {
parause the special macro LNA_AUTO to set LNA auto/para
/section
/section
+
+ section id=frontend-qos-properties
+ titleFrontend Quality of Service/Statistics indicators/title
+ paraExcept for link
linkend=DTV-QOS-ENUMconstantDTV_QOS_ENUM/constant/link,
+ the values are returned via
constantdtv_property.stat/constant./para
+ paraFor most delivery systems, this will return a single value for
each parameter./para
+ paraIt should be noticed, however, that new OFDM delivery systems
+ like ISDB can use different modulation types for each group of carriers.
+ On such standards, up to 3 groups of statistics can be provided, one
+ for each carrier group (called layer on ISDB).
+ In order to be consistent with other delivery systems, the first
+ value at link
linkend=dtv-statsconstantdtv_property.stat.dtv_stats/constant/link
array refers to
+ a global indicator, if any. The other elements of the array represent
+ each layer, starting from layer A(index 1), layer B (index 2) and so
on/para
+ paraThe number of filled elements are stored at
constantdtv_property.stat.len/constant./para
+ paraEach element of the
constantdtv_property.stat.dtv_stats/constant array consists on two
elements:/para
+ itemizedlist mark='opencircle'
+ listitemparaconstantvalue/constant - Value of the
measure/para/listitem
+ listitemparaconstantscale/constant - Scale for the
value. It can be:/para
+ section id = fecap-scale-params
+ itemizedlist mark='bullet'
+
listitemparaconstantFE_SCALE_NOT_AVAILABLE/constant - If it is not
possible to collect a given parameter (could be a transitory or permanent
condition)/para/listitem
+
listitemparaconstantFE_SCALE_DECIBEL/constant - parameter is a signed
value, measured in 0.1 dB/para/listitem
+
listitemparaconstantFE_SCALE_RELATIVE/constant - parameter is a
unsigned value, where 0 means 0% and 65535 means 100%./para/listitem
+
listitemparaconstantFE_SCALE_COUNTER/constant - parameter is a unsigned
value that counts the occurrence of an event, like bit error, block error, or
lapsed time./para/listitem
+ /itemizedlist
+ /section
+ /listitem
+ /itemizedlist
+
Re: [PATCH] usb id addition for Terratec Cinergy T Stick Dual rev. 2
On 10/06/2012 06:40 PM, Mauro Carvalho Chehab wrote: Em Mon, 01 Oct 2012 14:21:34 +0300 Antti Palosaari cr...@iki.fi escreveu: On 10/01/2012 02:15 PM, Mauro Carvalho Chehab wrote: Em Sun, 30 Sep 2012 19:36:50 +0200 Damien Bally bir...@free.fr escreveu: Le 29/09/2012 19:33, Mauro Carvalho Chehab a écrit : It seems that the it931x variant has bcdDevice equal to 2.00, from Damien's email: idVendor 0x0ccd TerraTec Electronic GmbH idProduct 0x0099 bcdDevice2.00 iManufacturer 1 ITE Technologies, Inc. iProduct2 DVB-T TV Stick iSerial 0 If the af9015 variant uses another bcdDevice, the fix should be simple. Alas, according to http://www.linuxtv.org/wiki/index.php/TerraTec_Cinergy_T_USB_Dual_RC the af9015 variant appears to have the same bcdDevice. I join both lsusb outputs for comparison. Well, then the alternative is to let both drivers to handle this USB ID, and add a code there on each of them that will check if the device is the right one, perhaps by looking at iProduct string. If the driver doesn't recognize it, it should return -ENODEV at .probe() time. The USB core will call the second driver. It is the easiest solution, but there should be very careful. Those strings could change from device to device. I used earlier af9015 eeprom hash (those string as coming from the eeprom) to map TerraTec dual remote controller and git bug report quite soon as it didn't worked. After I looked the reason I found out they was changed some not meaningful value. Yeah, those strings can change, especially when vendors don't care enough to use a different USB ID/bcdDevice for different models. Yet, seems to be the cleaner approach, among the alternatives. Damien, care to test? http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_tuner I split tuner out from IT9135 driver and due to that AF9035 driver supports IT9135 too (difference between AF9035 and IT9135 is integrated RF-tuner). I added iManufacturer based checks for both AF9015 and AF9035 drivers regards Antti -- http://palosaari.fi/ -- 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
Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
On Mon, 7 Jan 2013 00:09:47 +0100 Alessandro Rubini rub...@gnudd.com wrote: I don't expect you'll see serious performance differences on the PC. I think ARM users will have better benefits, due to the different cache architecture. You told me Jon measured meaningful figures on a Marvel CPU. It made the difference between 10 frames per second with the CPU running flat out and 30fps mostly idle. I think that probably counts as meaningful, yeah...:) jon -- 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
Re: [REVIEW PATCHv1 1/2] DocBook: improve the error_idx field documentation.
Hi Hans, Thanks for the patch. On Monday 07 January 2013 13:09:47 Hans Verkuil wrote: From: Hans Verkuil hans.verk...@cisco.com The documentation of the error_idx field was incomplete and confusing. This patch improves it. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- .../DocBook/media/v4l/vidioc-g-ext-ctrls.xml | 51 ++--- 1 file changed, 44 insertions(+), 7 deletions(-) (Text reflowed for easier review) diff --git a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml index 0a4b90f..c07c657 100644 --- a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml +++ b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml @@ -199,13 +199,50 @@ also be zero./entry row entry__u32/entry entrystructfielderror_idx/structfield/entry - entrySet by the driver in case of an error. If it is equal -to structfieldcount/structfield, then no actual changes were made to -controls. In other words, the error was not associated with setting a -particular control. If it is another value, then only the controls up to -structfielderror_idx-1/structfield were modified and control -structfielderror_idx/structfield is the one that caused the error. The -structfielderror_idx/structfield value is undefined if the ioctl -returned 0 (success)./entry + entryparaSet by the driver in case of an error. If the error is +associated with a particular control, then +structfielderror_idx/structfield is set to the index of that control. +If the error is not related to a specific control, then +structfielderror_idx/structfield is set to +structfieldcount/structfield./para +paraThe behavior is different for constantVIDIOC_G_EXT_CTRLS/constant +and constantVIDIOC_S_EXT_CTRLS/constant: if +structfielderror_idx/structfield is equal to +structfieldcount/structfield, then no actual changes were made to the +controls. For example, if you try to write to a read-only control, then +constantVIDIOC_TRY_EXT_CTRLS/constant will set +structfielderror_idx/structfield to the index of that read-only +control, but constantVIDIOC_S_EXT_CTRLS/constant will set +structfielderror_idx/structfield to structfieldcount/structfield +instead and none of the controls in the list will be set./para I find this a bit confusing (although I understand what you mean, as I'm familiar with the API). You start by mentioning [GS]_EXT_CTRLS only, and then you introduce TRY_EXT_CTRLS in the example. I think it would be clearer if you restructed the explanation as follows: - explain the error_idx principle globally, withotu examples - explain that [GS]_EXT_CTRLS will perform pre-validation and will thus set error_idx to count in specific cases (they should be listed) - explain that TRY_EXT_CTRLS doesn't perform pre-validation and will thus set error_idx to the index of the erroneous control in all cases, including the specific cases listed for [GS]_EXT_CTRLS +paraThe same is true when trying to e.g. read a write-only control: +constantVIDIOC_G_EXT_CTRLS/constant will set +structfielderror_idx/structfield to structfieldcount/structfield +and none of the controls in the list will have been retrieved./para + +paraThe reason for this behavior is that it is important when setting and +getting controls to do this as atomically as possible, so simple sanity +checks like testing for read-only controls are done first before an +attempt is made to apply/retrieve the new control values to/from the +hardware. It is important for an application to know whether +constantVIDIOC_S_EXT_CTRLS/constant or +constantVIDIOC_G_EXT_CTRLS/constant actually made changes to controls +or not. So if structfielderror_idx/structfield is equal to +structfieldcount/structfield, then you know that no actual controls +were set or retrieved. In the case of +constantVIDIOC_S_EXT_CTRLS/constant you can call +constantVIDIOC_TRY_EXT_CTRLS/constant with the same control list to +see if the problem was with a specific control or not +(constantVIDIOC_TRY_EXT_CTRLS/constant never modifies controls, so +that ioctl will just set structfielderror_idx/structfield to the +control that caused the problem). No such option exists for +constantVIDIOC_G_EXT_CTRLS/constant though, unfortunately./para It's very unfortunate indeed :-) Given that the hardware state must not be changed by a read operation, are you sure G_EXT_CTRLS couldn't retrieve controls up to the erroneous one ? ;-) I think some flexibility should still probably be left to drivers (and I'm not talking about UVC here), as some drivers might not be able to know that a control is write-only before trying to read it and getting an error. +paraIf structfielderror_idx/structfield as returned by +constantVIDIOC_S_EXT_CTRLS/constant or +constantVIDIOC_G_EXT_CTRLS/constant is less than +structfieldcount/structfield, then only the controls up to
Re: RFC: add parameters to V4L controls
Hi Hans, On Monday 07 January 2013 13:10:54 Hans Verkuil wrote: On Mon January 7 2013 11:46:38 Andrzej Hajda wrote: Hi, I have included this proposition already in the post [PATCH RFC 0/2] V4L: Add auto focus area control and selection but it left unanswered. I repost it again in a separate e-mail, I hope this way it will be easier to attract attention. Problem description Currently V4L2 controls can have only single value (of type int, int64, string). Some hardware controls require more than single int parameter, for example to set auto-focus (AF) rectangle four coordinates should be passed, to set auto-focus spot two coordinates should be passed. Current solution In case of AF rectangle we can reuse selection API as in [PATCH RFC 0/2] V4L: Add auto focus area control and selection post. Pros: - reuse existing API, Cons: - two IOCTL's to perform one action, - non-atomic operation, Is it really an issue in your use cases, or is this only a theoretical problem ? - fits well only for rectangles and spots (but with unused fields width, height), in case of other parameters we should find a different way. Proposed solution The solution takes an advantage of the fact VIDIOC_(G/S/TRY)_EXT_CTRLS ioctls can be called with multiple controls per call. I will present it using AF area control example. There could be added four pseudo-controls, lets call them for short: LEFT, TOP, WIDTH, HEIGHT. Those controls could be passed together with V4L2_AUTO_FOCUS_AREA_RECTANGLE control in one ioctl as a kind of parameters. For example setting auto-focus spot would require calling VIDIOC_S_EXT_CTRLS with the following controls: - V4L2_CID_AUTO_FOCUS_AREA = V4L2_AUTO_FOCUS_AREA_RECTANGLE - LEFT = ... - RIGHT = ... Setting AF rectangle: - V4L2_CID_AUTO_FOCUS_AREA = V4L2_AUTO_FOCUS_AREA_RECTANGLE - LEFT = ... - TOP = ... - WIDTH = ... - HEIGHT = ... Setting AF object detection (no parameters required): - V4L2_CID_AUTO_FOCUS_AREA = V4L2_AUTO_FOCUS_AREA_OBJECT_DETECTION If you want to do this, then you have to make LEFT/TOP/WIDTH/HEIGHT real controls. There is no such thing as a pseudo control. So you need five new controls in total: V4L2_CID_AUTO_FOCUS_AREA V4L2_CID_AUTO_FOCUS_LEFT V4L2_CID_AUTO_FOCUS_RIGHT V4L2_CID_AUTO_FOCUS_WIDTH V4L2_CID_AUTO_FOCUS_HEIGHT I have no problem with this from the point of view of the control API, but whether this is the best solution for implementing auto-focus is a different issue and input from sensor specialists is needed as well (added Laurent and Sakari to the CC list). The primary concern I have is that this does not scale to multiple focus rectangles. This might not be relevant to auto focus, though. Time to implement a rectangle control type ? Or to make it possible to issue atomic transactions across ioctls ? We've been postponing this discussion for ages. I have presented all three cases to show the advantages of this solution: - atomicity - control and its parameters are passed in one call, - flexibility - we are not limited by a fixed number of parameters, - no-redundancy - we can pass only required parameters (no need to pass null width and height in case of spot selection), - extensibility - it is possible to extend parameters in the future, for example add parameters to V4L2_AUTO_FOCUS_AREA_OBJECT_DETECTION, without breaking API, - backward compatibility, - re-usability - this schema could be used in other controls, pseudo-controls could be re-used in other controls as well. - API backward compatibility. -- Regards, Laurent Pinchart -- 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
Re: [PATCHv16 5/7] fbmon: add of_videomode helpers
On Mon, Jan 7, 2013 at 2:46 AM, Mohammed, Afzal af...@ti.com wrote: Hi Steffen, On Mon, Jan 07, 2013 at 13:36:48, Steffen Trumtrar wrote: On Mon, Jan 07, 2013 at 06:10:13AM +, Mohammed, Afzal wrote: This breaks DaVinci (da8xx_omapl_defconfig), following change was required to get it build if OF_VIDEOMODE or/and FB_MODE_HELPERS is not defined. There may be better solutions, following was the one that was used by me to test this series. I just did a quick make da8xx_omapl_defconfig make and it builds just fine. On what version did you apply the series? At the moment I have the series sitting on 3.7. Didn't try any 3.8-rcx yet. But fixing this shouldn't be a problem. You are right, me idiot, error will happen only upon try to make use of of_get_fb_videomode() (defined in this patch) in the da8xx-fb driver (with da8xx_omapl_defconfig), to be exact upon adding, video: da8xx-fb: obtain fb_videomode info from dt of my patch series. The change as I mentioned or something similar would be required as any driver that is going to make use of of_get_fb_videomode() would break if CONFIG_OF_VIDEOMODE or CONFIG_FB_MODE_HELPERS is not defined. Shouldn't the driver that depends on CONFIG_OF_VIDEOMODE and CONFIG_FB_MODE_HELPERS, explicitly select them? I don't really see the point of having the static-inline fallbacks. fwiw, using 'select' is what I was doing for lcd panel support for lcdc/da8xx drm driver (which was using the of videomode helpers, albeit a slightly earlier version of the patches): https://github.com/robclark/kernel-omap4/commit/e2aef5f281348afaaaeaa132699efc2831aa8384 BR, -R And testing was done over v3.8-rc2. +#if IS_ENABLED(CONFIG_OF_VIDEOMODE) As _OF_VIDEOMODE is a bool type CONFIG, isn't, #ifdef CONFIG_OF_VIDEOMODE sufficient ? Yes, that is right. But I think IS_ENABLED is the preferred way to do it, isn't it? Now I realize it is. Regards Afzal -- 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
Re: [PATCH 1/2] drivers/media/platform/soc_camera/pxa_camera.c: reposition free_irq to avoid access to invalid data
Guennadi Liakhovetski g.liakhovet...@gmx.de writes: (adding Robert to CC) I don't think any data is freed by pxa_free_dma(), it only disables DMA on a certain channel. Theoretically there could be a different problem: pxa_free_dma() deactivates DMA, whereas pxa_dma_start_channels() activates it. But I think we're also protected against that: by the time pxa_camera_remove() is called, and operation on the interface has been stopped, client devices have been detached, pxa_camera_remove_device() has been called, which has also stopped the interface clock. And with clock stopped no interrupts can be generated. And the case of interrupt having been generated before clk_disabled() and only delivered to the driver so much later, that we're already unloading the module, seems really impossible to me. Robert, you agree? Agreed that pxa_free_dma() doesn't free anything, that one is easy :) And agreed too for the second part, with a slighly different explanation : - pxa_camera_remove_device() has been called as you said - inside this function, check comment /* disable capture, disable interrupts */ = this ensures no interrupt can be generated anymore So after pxa_camera_remove_device() has been called, no interrupts can be generated. Yet as you said, it leaves the almost impossible scenario : - a user begins a capture - the user closes the capture device and unloads pxa-camera.ko: soc_camera_close() pxa_camera_remove_device() the IRQ line is asserted but doesn't trigger yet the interrupt handler (yes I know, improbable) meanwhile, IRQs are disabled, DMA channels are stopped switch_to(rmmod) = yes I know, impossible, the interrupt handler must be run before, but let's continue for love of discussion ... rmmod pxa-camera pxa_camera_remove() pxa_free_dma() * 3 here the IRQ handler kicks in !!! = pxa_camera_irq() pxa_dma_start_channels() it hurts ! My call is that this is impossible because the switch_to() should run the IRQ handler before pxa_camera_remove() is called. So all this to say that I think we're safe, unless a heavy ion or a cosmic ray strikes the PXA :) Cheers. -- Robert -- 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
Re: [PATCH] usb id addition for Terratec Cinergy T Stick Dual rev. 2
On Mon, 2013-01-07 at 21:37 +0200, Antti Palosaari wrote: On 10/06/2012 06:40 PM, Mauro Carvalho Chehab wrote: Em Mon, 01 Oct 2012 14:21:34 +0300 Antti Palosaari cr...@iki.fi escreveu: On 10/01/2012 02:15 PM, Mauro Carvalho Chehab wrote: Em Sun, 30 Sep 2012 19:36:50 +0200 Damien Bally bir...@free.fr escreveu: Le 29/09/2012 19:33, Mauro Carvalho Chehab a écrit : It seems that the it931x variant has bcdDevice equal to 2.00, from Damien's email: idVendor 0x0ccd TerraTec Electronic GmbH idProduct 0x0099 bcdDevice2.00 iManufacturer 1 ITE Technologies, Inc. iProduct2 DVB-T TV Stick iSerial 0 If the af9015 variant uses another bcdDevice, the fix should be simple. Alas, according to http://www.linuxtv.org/wiki/index.php/TerraTec_Cinergy_T_USB_Dual_RC the af9015 variant appears to have the same bcdDevice. I join both lsusb outputs for comparison. Well, then the alternative is to let both drivers to handle this USB ID, and add a code there on each of them that will check if the device is the right one, perhaps by looking at iProduct string. If the driver doesn't recognize it, it should return -ENODEV at .probe() time. The USB core will call the second driver. It is the easiest solution, but there should be very careful. Those strings could change from device to device. I used earlier af9015 eeprom hash (those string as coming from the eeprom) to map TerraTec dual remote controller and git bug report quite soon as it didn't worked. After I looked the reason I found out they was changed some not meaningful value. Yeah, those strings can change, especially when vendors don't care enough to use a different USB ID/bcdDevice for different models. Yet, seems to be the cleaner approach, among the alternatives. Damien, care to test? http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_tuner I split tuner out from IT9135 driver and due to that AF9035 driver supports IT9135 too (difference between AF9035 and IT9135 is integrated RF-tuner). I added iManufacturer based checks for both AF9015 and AF9035 drivers I can't see the point of adding this to the af9035/af9033 driver. It is going to turn into one enormous blob. The it913x is a stable driver and has it own entity moving forward. The only thing that needs to happen is the id is added to it913x driver and if it doesn't apply drop it. Nack. Regards Malcolm -- 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
Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
Em Mon, 7 Jan 2013 12:40:50 -0700 Jonathan Corbet cor...@lwn.net escreveu: On Mon, 7 Jan 2013 00:09:47 +0100 Alessandro Rubini rub...@gnudd.com wrote: I don't expect you'll see serious performance differences on the PC. I think ARM users will have better benefits, due to the different cache architecture. You told me Jon measured meaningful figures on a Marvel CPU. It made the difference between 10 frames per second with the CPU running flat out and 30fps mostly idle. I think that probably counts as meaningful, yeah...:) Couldn't this performance difference be due to the usage of GFP_DMA inside the VB2 code, like Federico's new patch series is proposing? If not, why are there a so large performance penalty? Regards, Mauro -- 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
[PATCH] DocBook: media: struct v4l2_capability card field is a UTF-8 string
The struct v4l2_capability card field stores the device name. That name can be hardcoded in drivers, or be retrieved directly from the device. The later is very common with USB devices. As several devices already report names that include non-ASCII characters, update the field description to use UTF-8. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- .../DocBook/media/v4l/vidioc-querycap.xml |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Documentation/DocBook/media/v4l/vidioc-querycap.xml b/Documentation/DocBook/media/v4l/vidioc-querycap.xml index 4c70215..d5a3c97 100644 --- a/Documentation/DocBook/media/v4l/vidioc-querycap.xml +++ b/Documentation/DocBook/media/v4l/vidioc-querycap.xml @@ -76,7 +76,7 @@ make sure the strings are properly NUL-terminated./para/entry row entry__u8/entry entrystructfieldcard/structfield[32]/entry - entryName of the device, a NUL-terminated ASCII string. + entryName of the device, a NUL-terminated UTF-8 string. For example: Yoyodyne TV/FM. One driver may support different brands or models of video hardware. This information is intended for users, for example in a menu of available devices. Since multiple TV cards of -- Regards, Laurent Pinchart -- 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
Driver support for Creatix Polymedia CTX967
Dear all, being a Newbie in Linux, I desparately try to get my PCIE card working to enable DVB-T reception on my Notebook. I'm running UBUNTU 12.04LTS The Video card is: Creatix Poymedia CTX967 v7.2.2 PCIe Device based on Philips SAA7160 (rev. 03) component As I was so far not successful in finding any suited driver for the device, it would be highly appreciated if someone might find the time to provide support on this. Effectively the Media controller is listed up if running lspci. The output is: 06:00.0 Multimedia controller: Philips Semiconductors SAA7160 (rev 03) However neither any appropriate Linux FW is available nor any support by the manufacturer. After having searched intesively on the web, it seems there are quite plenty of folks out there with the same issue. Best regards Markus Hermsen -- 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
Re: [PATCH] usb id addition for Terratec Cinergy T Stick Dual rev. 2
On 01/07/2013 10:13 PM, Malcolm Priestley wrote: On Mon, 2013-01-07 at 21:37 +0200, Antti Palosaari wrote: On 10/06/2012 06:40 PM, Mauro Carvalho Chehab wrote: Em Mon, 01 Oct 2012 14:21:34 +0300 Antti Palosaari cr...@iki.fi escreveu: On 10/01/2012 02:15 PM, Mauro Carvalho Chehab wrote: Em Sun, 30 Sep 2012 19:36:50 +0200 Damien Bally bir...@free.fr escreveu: Le 29/09/2012 19:33, Mauro Carvalho Chehab a écrit : It seems that the it931x variant has bcdDevice equal to 2.00, from Damien's email: idVendor 0x0ccd TerraTec Electronic GmbH idProduct 0x0099 bcdDevice2.00 iManufacturer 1 ITE Technologies, Inc. iProduct2 DVB-T TV Stick iSerial 0 If the af9015 variant uses another bcdDevice, the fix should be simple. Alas, according to http://www.linuxtv.org/wiki/index.php/TerraTec_Cinergy_T_USB_Dual_RC the af9015 variant appears to have the same bcdDevice. I join both lsusb outputs for comparison. Well, then the alternative is to let both drivers to handle this USB ID, and add a code there on each of them that will check if the device is the right one, perhaps by looking at iProduct string. If the driver doesn't recognize it, it should return -ENODEV at .probe() time. The USB core will call the second driver. It is the easiest solution, but there should be very careful. Those strings could change from device to device. I used earlier af9015 eeprom hash (those string as coming from the eeprom) to map TerraTec dual remote controller and git bug report quite soon as it didn't worked. After I looked the reason I found out they was changed some not meaningful value. Yeah, those strings can change, especially when vendors don't care enough to use a different USB ID/bcdDevice for different models. Yet, seems to be the cleaner approach, among the alternatives. Damien, care to test? http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/it9135_tuner I split tuner out from IT9135 driver and due to that AF9035 driver supports IT9135 too (difference between AF9035 and IT9135 is integrated RF-tuner). I added iManufacturer based checks for both AF9015 and AF9035 drivers I can't see the point of adding this to the af9035/af9033 driver. It is going to turn into one enormous blob. Stop speaking bullshit! It increases AF9035/AF9033 driver size marginally. I could guess total binary size of AF9035+AF9033+IT913X (IT913X == that new tuner driver) is smaller than size of your IT9135 blob. The it913x is a stable driver and has it own entity moving forward. The only thing that needs to happen is the id is added to it913x driver and if it doesn't apply drop it. Nack. If you remember about year back when IT9135 was mainlined I reviewed that stuff and criticized it wasn't split correctly. I also pointed out all what is needed is new RF-tuner driver as AF9035/AF9033 is just same silicon, but without a integrated tuner. You said on one mail something like it is too much different than AF9035 family, I didn't cared to start stupid yes/no discussion on that time. You don't have any technical reason to NACK it. Last time I was discussing with DS3000+TS2020 split with you and you were against. Same happens now. I ask you to look driver guidelines [1] and stop speaking BS, instead start fixing your own stuff and you will never end up situation like that. [1] http://lwn.net/Articles/529490/ Antti -- http://palosaari.fi/ -- 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
Re: [PATCH 1/2] drivers/media/platform/soc_camera/pxa_camera.c: reposition free_irq to avoid access to invalid data
On Mon, 7 Jan 2013, Robert Jarzmik wrote: Guennadi Liakhovetski g.liakhovet...@gmx.de writes: (adding Robert to CC) I don't think any data is freed by pxa_free_dma(), it only disables DMA on a certain channel. Theoretically there could be a different problem: pxa_free_dma() deactivates DMA, whereas pxa_dma_start_channels() activates it. But I think we're also protected against that: by the time pxa_camera_remove() is called, and operation on the interface has been stopped, client devices have been detached, pxa_camera_remove_device() has been called, which has also stopped the interface clock. And with clock stopped no interrupts can be generated. And the case of interrupt having been generated before clk_disabled() and only delivered to the driver so much later, that we're already unloading the module, seems really impossible to me. Robert, you agree? Agreed that pxa_free_dma() doesn't free anything, that one is easy :) And agreed too for the second part, with a slighly different explanation : - pxa_camera_remove_device() has been called as you said - inside this function, check comment /* disable capture, disable interrupts */ = this ensures no interrupt can be generated anymore So after pxa_camera_remove_device() has been called, no interrupts can be generated. Yet as you said, it leaves the almost impossible scenario : - a user begins a capture - the user closes the capture device and unloads pxa-camera.ko: soc_camera_close() pxa_camera_remove_device() the IRQ line is asserted but doesn't trigger yet the interrupt handler (yes I know, improbable) meanwhile, IRQs are disabled, DMA channels are stopped switch_to(rmmod) = yes I know, impossible, the interrupt handler must be run before, but let's continue for love of discussion ... rmmod pxa-camera pxa_camera_remove() pxa_free_dma() * 3 here the IRQ handler kicks in !!! = pxa_camera_irq() pxa_dma_start_channels() it hurts ! My call is that this is impossible because the switch_to() should run the IRQ handler before pxa_camera_remove() is called. So all this to say that I think we're safe, unless a heavy ion or a cosmic ray strikes the PXA :) Thanks for the explanation. julia -- 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
Re: [PATCH v2 4/6] media: soc-camera: split struct soc_camera_link into host and subdevice parts
Hi Guennadi,
Thanks for the patch.
On Thursday 03 January 2013 17:13:15 Guennadi Liakhovetski wrote:
struct soc_camera_link currently contains fields, used both by sensor and
bridge drivers. To make subdevice driver re-use simpler, split it into a
host and a subdevice parts.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
v2: following an off-list discussion with Laurent, .add_device() and
.del_device() callbacks are moved over from the subdevice part of the
platform data to the host part.
drivers/media/i2c/soc_camera/imx074.c | 14 ++--
drivers/media/i2c/soc_camera/mt9m001.c | 38
drivers/media/i2c/soc_camera/mt9m111.c | 21 ++--
drivers/media/i2c/soc_camera/mt9t031.c | 22 ++--
drivers/media/i2c/soc_camera/mt9t112.c | 18 ++--
drivers/media/i2c/soc_camera/mt9v022.c | 36
drivers/media/i2c/soc_camera/ov2640.c | 12 +-
drivers/media/i2c/soc_camera/ov5642.c | 16 ++--
drivers/media/i2c/soc_camera/ov6650.c | 16 ++--
drivers/media/i2c/soc_camera/ov772x.c | 14 ++--
drivers/media/i2c/soc_camera/ov9640.c | 12 +-
drivers/media/i2c/soc_camera/ov9740.c | 14 ++--
drivers/media/i2c/soc_camera/rj54n1cb0c.c | 24 +++---
drivers/media/i2c/soc_camera/tw9910.c | 18 ++--
drivers/media/platform/soc_camera/soc_camera.c | 98 +
.../platform/soc_camera/soc_camera_platform.c |2 +-
include/media/soc_camera.h | 102 +
include/media/soc_camera_platform.h| 10 ++-
18 files changed, 279 insertions(+), 208 deletions(-)
[snip]
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c index 04ce718..8ec9805
100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -50,22 +50,22 @@ static LIST_HEAD(hosts);
static LIST_HEAD(devices);
static DEFINE_MUTEX(list_lock); /* Protects the list of hosts */
-int soc_camera_power_on(struct device *dev, struct soc_camera_link *icl)
+int soc_camera_power_on(struct device *dev, struct soc_camera_subdev_desc
*ssdd) {
- int ret = regulator_bulk_enable(icl-num_regulators,
- icl-regulators);
+ int ret = regulator_bulk_enable(ssdd-num_regulators,
+ ssdd-regulators);
if (ret 0) {
dev_err(dev, Cannot enable regulators\n);
return ret;
}
- if (icl-power) {
- ret = icl-power(dev, 1);
+ if (ssdd-power) {
+ ret = ssdd-power(dev, 1);
While we're at it, do you plan to get rid of this callback ? What do the
supported board need to perform there ?
if (ret 0) {
dev_err(dev,
Platform failed to power-on the camera.\n);
- regulator_bulk_disable(icl-num_regulators,
-icl-regulators);
+ regulator_bulk_disable(ssdd-num_regulators,
+ssdd-regulators);
}
}
[snip]
@@ -552,8 +552,8 @@ static int soc_camera_open(struct file *file)
};
/* The camera could have been already on, try to reset */
- if (icl-reset)
- icl-reset(icd-pdev);
+ if (sdesc-subdev_desc.reset)
+ sdesc-subdev_desc.reset(icd-pdev);
ret = ici-ops-add(icd);
if (ret 0) {
Same question here, would a GPIO do ?
@@ -1072,23 +1072,24 @@ static void scan_add_host(struct soc_camera_host
*ici)
#ifdef CONFIG_I2C_BOARDINFO
static int soc_camera_init_i2c(struct soc_camera_device *icd,
-struct soc_camera_link *icl)
+struct soc_camera_desc *sdesc)
{
struct i2c_client *client;
struct soc_camera_host *ici = to_soc_camera_host(icd-parent);
- struct i2c_adapter *adap = i2c_get_adapter(icl-i2c_adapter_id);
+ struct soc_camera_host_desc *shd = sdesc-host_desc;
+ struct i2c_adapter *adap = i2c_get_adapter(shd-i2c_adapter_id);
struct v4l2_subdev *subdev;
if (!adap) {
dev_err(icd-pdev, Cannot get I2C adapter #%d. No driver?\n,
- icl-i2c_adapter_id);
+ shd-i2c_adapter_id);
goto ei2cga;
}
- icl-board_info-platform_data = icl;
+ shd-board_info-platform_data = sdesc-subdev_desc;
I'm looking forward to async registration here (yes, I know, I need to review
your latest patches :-)).
subdev = v4l2_i2c_new_subdev_board(ici-v4l2_dev, adap,
-
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date:Mon Jan 7 19:00:18 CET 2013 git hash:73ec66c000e9816806c7380ca3420f4e0638c40e gcc version: i686-linux-gcc (GCC) 4.7.1 host hardware:x86_64 host os: 3.4.07-marune linux-git-arm-eabi-davinci: WARNINGS linux-git-arm-eabi-exynos: WARNINGS linux-git-arm-eabi-omap: ERRORS linux-git-i686: OK linux-git-m32r: OK linux-git-mips: WARNINGS linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.31.12-i686: WARNINGS linux-2.6.32.6-i686: WARNINGS linux-2.6.33-i686: WARNINGS linux-2.6.34-i686: WARNINGS linux-2.6.35.3-i686: WARNINGS linux-2.6.36-i686: WARNINGS linux-2.6.37-i686: WARNINGS linux-2.6.38.2-i686: WARNINGS linux-2.6.39.1-i686: WARNINGS linux-3.0-i686: WARNINGS linux-3.1-i686: WARNINGS linux-3.2.1-i686: WARNINGS linux-3.3-i686: WARNINGS linux-3.4-i686: WARNINGS linux-3.5-i686: WARNINGS linux-3.6-i686: WARNINGS linux-3.7-i686: WARNINGS linux-3.8-rc1-i686: WARNINGS linux-2.6.31.12-x86_64: WARNINGS linux-2.6.32.6-x86_64: WARNINGS linux-2.6.33-x86_64: WARNINGS linux-2.6.34-x86_64: WARNINGS linux-2.6.35.3-x86_64: WARNINGS linux-2.6.36-x86_64: WARNINGS linux-2.6.37-x86_64: WARNINGS linux-2.6.38.2-x86_64: WARNINGS linux-2.6.39.1-x86_64: WARNINGS linux-3.0-x86_64: WARNINGS linux-3.1-x86_64: WARNINGS linux-3.2.1-x86_64: WARNINGS linux-3.3-x86_64: WARNINGS linux-3.4-x86_64: WARNINGS linux-3.5-x86_64: WARNINGS linux-3.6-x86_64: WARNINGS linux-3.7-x86_64: WARNINGS linux-3.8-rc1-x86_64: WARNINGS apps: WARNINGS spec-git: OK sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.tar.bz2 The V4L-DVB specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- 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
Re: [PATCH] omap3isp: ispqueue: Fix uninitialized variable compiler warnings
Hi Sakari,
On Saturday 05 January 2013 00:51:36 Sakari Ailus wrote:
On Mon, Dec 17, 2012 at 09:52:48AM +0100, Laurent Pinchart wrote:
drivers/media/platform/omap3isp/ispqueue.c:399:18: warning: 'pa' may be
used uninitialized in this function [-Wuninitialized]
This is a false positive but the compiler has no way to know about it,
so initialize the variable to 0.
drivers/media/platform/omap3isp/ispqueue.c:445:6: warning:
'vm_page_prot' may be used uninitialized in this function
[-Wuninitialized]
This is a false positive and the compiler should know better. Use
uninitialized_var().
Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
drivers/media/platform/omap3isp/ispqueue.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/omap3isp/ispqueue.c
b/drivers/media/platform/omap3isp/ispqueue.c index 15bf3ea..1388eb7
100644
--- a/drivers/media/platform/omap3isp/ispqueue.c
+++ b/drivers/media/platform/omap3isp/ispqueue.c
@@ -366,7 +366,7 @@ static int isp_video_buffer_prepare_pfnmap(struct
isp_video_buffer *buf)
unsigned long this_pfn;
unsigned long start;
unsigned long end;
- dma_addr_t pa;
+ dma_addr_t pa = 0;
Why 0 and not something else arbitrary? :-)
The value will not be used as far as I can tell. If it gets used it
certainly is a bug.
If buf-vbuf.length == 0 pa will be used uninitialized. This is clearly a bug,
but I thought it would be easier to debug if pa was set to 0 in that case
instead of a random value through uninitialized_var().
int ret = -EFAULT;
start = buf-vbuf.m.userptr;
@@ -419,7 +419,7 @@ done:
static int isp_video_buffer_prepare_vm_flags(struct isp_video_buffer
*buf)
{
struct vm_area_struct *vma;
- pgprot_t vm_page_prot;
+ pgprot_t uninitialized_var(vm_page_prot);
unsigned long start;
unsigned long end;
int ret = -EFAULT;
--
Regards,
Laurent Pinchart
--
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
Re: [linux-uvc-devel] [PATCH 0/2] uvcvideo: Support Logitech RGB Bayer formats
Hi Oleksij, (CC'ing linux-media) On Wednesday 02 January 2013 11:50:51 Oleksij Rempel wrote: Am 02.01.2013 10:03, schrieb Peter Ross: On Tue, Jan 01, 2013 at 05:16:44PM +0100, Oleksij Rempel wrote: Hi Peter, thank you for your work, but most of it belongs to uvcdynctrl. You will need to add it here: /usr/share/uvcdynctrl/data/046d/logitech.xml Using uvcdynctrl is problematic for two reasons. 1. Setting the Logitech 'disable video processing' and 'raw data bpp' controls independently from uvcvideo causes the driver to *always* drop frames. uvc_video_decode_isoc() performs a sanity check on the size of incoming uncompressed frames. It expects to receive frame sizes reported by the 8-bit yuyv 4:2:2 format description. The check fails when video processing is disabled, because the 8-/10-bit RGB Bayer frame sizes are always smaller. 2. Userspace applications need to distinguish the yuyv 4:2:2 format from the the additional RGB Bayer formats. Currently, when video processing is disabled, user applications expect to receive yuyv 4:2:2 pixels, because thats what it and uvcvideo has agreed to (mediate by V4L2). Instead the application receives RGB Bayer pixels and attempts to process them as yuyv resulting in visual garbage. One could argue that an application that explicitly disables processing would be able to handle that. However, I agree that point 1 is problematic, and point 2 is definitely not clean. Some of this has been discussed previously, see http://article.gmane.org/gmane.linux.drivers.uvc.devel/3061/. The patch tries to solve both of these problems: 1) making uvcvideo aware of the Logitech controls and recalculating the expected frame sizes, and 2) presenting the RGB Bayer formats through the V4L2 interface, so user application can request them. If, the problem seems to be bigger. We have: 1. Formats provided by usb descriptor and controlled over uvc protocol. 2. Formats provided by usb descriptor and controlled over XU Do we really have devices that expose formats through the standard UVC descriptors but require XU access to select them ? 3. Formats controlled over XU and provided by documentation other kind of knowledge. Last case in not properly handled by uvcvideo.ko, but IMHO it is not good way to have all possible XU in driver. Both problems you described should be fixed, but not in this way. If it is possible - uvcdynctl should provide/update format descriptor over uvc_xu_control_query interface or some other way. Beside, i was working on kernel space plugin system for uvcvideo driver, which was probably not the best idea. How about to do some part of it in userspace? For example, uvcdynctl can provide bandwidth information too. This way we can fix many buggy cams without needing to permanently update kernel driver. Laurent? I'm really undecided here. On one hand I agree with you, from a theoretical point of view the driver should not know about all possible XUs. This just can't scale. On the other hand, it's pretty clear that we don't need to scale. We only have a single public dynamic controls XML file, and looking at the descriptors of most devices it's pretty clear that they reuse the same XUs, as they are based on only a dozen or so different chips. The dynamic controls API brings additional complexity to the driver, and I think that it was a good design decision. However, in some cases, the implications of changing an XU control value go well beyond what is usually expected of a control. I'm thus tempted to allow handling of XU controls in the kernel when there's a good reason to do so. Of course I want the code to be clean, without lots of hooks all over the place that would make the driver sources impossible to read and understand :-) Comments and thoughts will be appreciated. -- Regards, Laurent Pinchart -- 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
Re: [RFC] Initial scan files troubles and brainstorming
Hi Christoph, On Mon, 7 Jan 2013 13:53:25 +0100, Christoph Pfister wrote: I've been updating the scan data for the past few years, but found barely no time in the last (many!) months. This won't change in future, so I've decided to step down from the task; may others do what is necessary ... Thanks for your work over the past few years. If nobody else is willing, I'd be happy to take over this role, and if Oliver's offer still stands, I'd be happy to share DVB scanfile maintenance with him. Jon signature.asc Description: Digital signature
Re: [BUG] crystalhd git.linuxtv.org kernel driver: No more Oops or kernel crashes with Linux 3.2
Hi People, -topic- be careful using the bleeding edge stable kernel series 3.2 with this driver. y tom On 05.01.2013 13:44, thomas schorpp wrote: -Removed Broadcom kernel module authors pras...@broadcom.com, nsan...@broadcom.com from CC list, unreachable, see att. Trying listed official Press contact instead- Hi Oliver, hi crystalhd users and devs, hello Broadcom Crystal HD staff, 1. sorry for the delay, I had to upgrade my old debian i386 stable...squeeze-backports userspace on the old core2duo machine to amd64 by full reinstall, otherwise the driver interface of libcrystalhd3 i686 to 3.6.10...3.7.1amd64 SMP kernel.org kernels has failed permanently, please (anyone still running such a setup or You) try to confirm this and report to this thread. lspci still shows the same PCI-E errors (see my other posts to this list) with the working libcrystalhd3 amd64 and broadcom designed crystalhd driver now, so this data reported from the chipset or lspci has to be considered faulty or stale and irelevant now, I will build the latest lspci from source to crosscheck this. Please build ffmpeg rel. 1.0.1(non-MT version, not later version, git master showed up with an audio format bug, presenting wrong audio sample format as planar (sfltp, fltp, s16p) which bino cannot handle and makes mplayer cry for not having libavresample access but which is disabled by default in ffmpeg configure defaults and debian dmo source packages ) from Your distro source package with --disable-decoders='h264, h264_vdpau, h264_vda' and leave only h264_crystalhd need as h.264 decoder and so trigger crystalhd by every app on your system accessing h.264 content for parsing or decoding and linked to libavcodec (check binaries with ldd if linked against this libavcodec54, , in libavcodec53 h264_crystalhd is flagged CAP_EXPERIMENTAL, which makes it unaccesible by other apps than the ffmpeg program (-strict -2), mplayer: -vc ffh264crystalhd, or will fail --disable-decoders='h264, h264_vdpau, h264_vda') like mplayer (not statically linked), kaffeine, vlc, gnome nautilus media file properties (uses mplayer -identify) sequentially called and then in parallel and record and post the Ooopses in this thread here until the authors return from winter sports ;-) Note for Bino users: System requirement is at least debian squeeze-backports X and DRI, otherwise bino will segfault the dri driver, i965 here and you need to build libGLEW(mx) 1.6 from source for debian stable systems, see http://savannah.nongnu.org/bugs/?33368 http://bino3d.org/help-wanted.html 2. With the new amd64 userspace on 3.7.1 SMP PREEMPT kernel things got even more worse here now, got 5 kernel panics in IRQ handler of the crystalhd driver in 1h while watching h.264 with mplayer2/1 (single threaded decoding mode, stereo3d filter) resulting in system halting kernel crashes, I need to setup serial console debugging to get the logs, on my Pentium M i686 vdr machine, kernel has been able to continue at least after the null pointer oopses. We need to have confirmation for this, too. 3. Since the source code still states broadcom staff as module authors and the download on the broadcom website is packaged broken tar.bz2 and does not build here with recent kernels, I'm CC'ing them now, too, and because it's their basic driver skeleton design and the quality and performance of this driver is far below the windows driver, which performs h.264 1080 great with http://mpc-hc.sourceforge.net at 5-10% cpu usage even on xp x64 on a i965, this should be the reference development target. 4. The driver Makefile won't compile it with debian squeeze-backports 3.2 and 3.2 bpo kbuild infrastructure, missing helper scripts and includes, it needs a full ready build kernel from sources: schorpp@tom3:/usr/local/src/crystalhd/driver/linux$ LC_ALL=C make make -C /lib/modules/3.2.0-0.bpo.4-amd64/build SUBDIRS=/mnt/data/usr/local/src/crystalhd/driver/linux modules make[1]: Entering directory `/mnt/data/usr/src/linux-headers-3.2.0-0.bpo.4-amd64' /mnt/data/usr/src/linux-headers-3.2.0-0.bpo.4-common/Makefile:287: /mnt/data/usr/src/linux-headers-3.2.0-0.bpo.4-common/scripts/Kbuild.include: Datei oder Verzeichnis nicht gefunden /bin/bash: /mnt/data/usr/src/linux-headers-3.2.0-0.bpo.4-common/scripts/gcc-x86_64-has-stack-protector.sh: Datei oder Verzeichnis nicht gefunden /mnt/data/usr/src/linux-headers-3.2.0-0.bpo.4-common/arch/x86/Makefile:81: stack protector enabled but no compiler support /bin/bash: /mnt/data/usr/src/linux-headers-3.2.0-0.bpo.4-common/scripts/gcc-goto.sh: Datei oder Verzeichnis nicht gefunden make: *** Leerer Variablenname. Schluss. make[3]: *** [_module_/mnt/data/usr/local/src/crystalhd/driver/linux] Fehler 2 make[2]: *** [sub-make] Error 2 make[1]: *** [all] Error 2 make[1]: Leaving directory `/mnt/data/usr/src/linux-headers-3.2.0-0.bpo.4-amd64' make: *** [all] Error 2 schorpp@tom3:/usr/local/src/crystalhd/driver/linux$ 5. I'm
[PATCH RFCv9 2/4] dvb: the core logic to handle the DVBv5 QoS properties
Add the logic to poll, reset counters and report the QoS stats
to the end user.
The idea is that the core will periodically poll the frontend for
the stats. The frontend may return -EBUSY, if the previous collect
didn't finish, or it may fill the cached data.
The value returned to the end user is always the cached data.
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
---
drivers/media/dvb-core/dvb_frontend.c | 56 +++
drivers/media/dvb-core/dvb_frontend.h | 12
2 files changed, 68 insertions(+)
diff --git a/drivers/media/dvb-core/dvb_frontend.c
b/drivers/media/dvb-core/dvb_frontend.c
index dd35fa9..66e066b 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -260,6 +260,9 @@ static int dvb_frontend_get_event(struct dvb_frontend *fe,
return ret;
}
+ if (fe-ops.get_qos_stats)
+ fe-ops.get_qos_stats(fe);
+
mutex_lock(events-mtx);
*event = events-events[events-eventr];
events-eventr = (events-eventr + 1) % MAX_EVENT;
@@ -1053,6 +1056,15 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] =
{
_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_B, 0, 0),
_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_C, 0, 0),
_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_D, 0, 0),
+
+ /* Statistics API */
+ _DTV_CMD(DTV_QOS_ENUM, 0, 0),
+ _DTV_CMD(DTV_QOS_SIGNAL_STRENGTH, 0, 0),
+ _DTV_CMD(DTV_QOS_CNR, 0, 0),
+ _DTV_CMD(DTV_QOS_BIT_ERROR_COUNT, 0, 0),
+ _DTV_CMD(DTV_QOS_TOTAL_BITS_COUNT, 0, 0),
+ _DTV_CMD(DTV_QOS_ERROR_BLOCK_COUNT, 0, 0),
+ _DTV_CMD(DTV_QOS_TOTAL_BLOCKS_COUNT, 0, 0),
};
static void dtv_property_dump(struct dvb_frontend *fe, struct dtv_property
*tvp)
@@ -1443,6 +1455,25 @@ static int dtv_property_process_get(struct dvb_frontend
*fe,
tvp-u.data = c-lna;
break;
+ /* Fill quality measures */
+ case DTV_QOS_SIGNAL_STRENGTH:
+ tvp-u.st = c-strength;
+ break;
+ case DTV_QOS_CNR:
+ tvp-u.st = c-cnr;
+ break;
+ case DTV_QOS_BIT_ERROR_COUNT:
+ tvp-u.st = c-bit_error;
+ break;
+ case DTV_QOS_TOTAL_BITS_COUNT:
+ tvp-u.st = c-bit_count;
+ break;
+ case DTV_QOS_ERROR_BLOCK_COUNT:
+ tvp-u.st = c-block_error;
+ break;
+ case DTV_QOS_TOTAL_BLOCKS_COUNT:
+ tvp-u.st = c-block_count;
+ break;
default:
return -EINVAL;
}
@@ -1646,6 +1677,26 @@ static int set_delivery_system(struct dvb_frontend *fe,
u32 desired_system)
return 0;
}
+static int reset_qos_counters(struct dvb_frontend *fe)
+{
+ struct dtv_frontend_properties *c = fe-dtv_property_cache;
+
+ /* Reset QoS cache */
+
+ memset (c-strength, 0, sizeof(c-strength));
+ memset (c-cnr, 0, sizeof(c-cnr));
+ memset (c-bit_error, 0, sizeof(c-bit_error));
+ memset (c-bit_count, 0, sizeof(c-bit_count));
+ memset (c-block_error, 0, sizeof(c-block_error));
+ memset (c-block_count, 0, sizeof(c-block_count));
+
+ /* Call frontend reset counter method, if available */
+ if (fe-ops.reset_qos_counters)
+ return fe-ops.reset_qos_counters(fe);
+
+ return 0;
+}
+
static int dtv_property_process_set(struct dvb_frontend *fe,
struct dtv_property *tvp,
struct file *file)
@@ -1705,6 +1756,8 @@ static int dtv_property_process_set(struct dvb_frontend
*fe,
break;
case DTV_DELIVERY_SYSTEM:
r = set_delivery_system(fe, tvp-u.data);
+ if (r = 0)
+ reset_qos_counters(fe);
break;
case DTV_VOLTAGE:
c-voltage = tvp-u.data;
@@ -2305,6 +2358,9 @@ static int dvb_frontend_ioctl_legacy(struct file *file,
if (err)
break;
err = dtv_set_frontend(fe);
+ if (err = 0)
+ reset_qos_counters(fe);
+
break;
case FE_GET_EVENT:
err = dvb_frontend_get_event (fe, parg, file-f_flags);
diff --git a/drivers/media/dvb-core/dvb_frontend.h
b/drivers/media/dvb-core/dvb_frontend.h
index 97112cd..7bb49f1 100644
--- a/drivers/media/dvb-core/dvb_frontend.h
+++ b/drivers/media/dvb-core/dvb_frontend.h
@@ -315,6 +315,10 @@ struct dvb_frontend_ops {
int (*set_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
int (*get_property)(struct dvb_frontend* fe, struct dtv_property* tvp);
+
+ /* QoS statistics callbacks */
+ int (*get_qos_stats)(struct dvb_frontend *fe);
+ int (*reset_qos_counters)(struct dvb_frontend *fe);
};
#ifdef __DVB_CORE__
@@ -393,6 +397,14 @@ struct dtv_frontend_properties {
[PATCH RFCv9 0/4] DVB QoS statistics API
This is the version 9 of the stats API. On this patchset, there are DocBooks, API headers, dvb-core and one driver example for the new API usage. Currently, the driver example is too simple: it adds just 2 QoS indicators, being one global (signal strength) and one per-layer. It is simple like that to help others to review this patch series. Adding the code for the remaining indicators (CNR, UCB, per-layer CNR) is easy, coding them are a little more complex, as table lookups are needed to convert from MER to CNR, and from global CNR into dB. So, I'll code that later. Mauro Carvalho Chehab (4): dvb: Add DVBv5 stats properties for Quality of Service dvb: the core logic to handle the DVBv5 QoS properties mb86a20s: provide signal strength via DVBv5 stats API mb86a20s: add BER measure Documentation/DocBook/media/dvb/dvbproperty.xml | 97 - drivers/media/dvb-core/dvb_frontend.c | 56 ++ drivers/media/dvb-core/dvb_frontend.h | 12 ++ drivers/media/dvb-frontends/mb86a20s.c | 257 ++-- include/uapi/linux/dvb/frontend.h | 84 +++- 5 files changed, 484 insertions(+), 22 deletions(-) -- 1.7.11.7 -- 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
[PATCH RFCv9 3/4] mb86a20s: provide signal strength via DVBv5 stats API
Implement DVBv5 API status handler functions.
The counter reset code there is complete for all stats provided
by this frontend.
The get_stats callback currently handles only the existing stat
(signal strength), although the code is already prepared for the
per-layer stats.
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
---
drivers/media/dvb-frontends/mb86a20s.c | 166 +
1 file changed, 148 insertions(+), 18 deletions(-)
diff --git a/drivers/media/dvb-frontends/mb86a20s.c
b/drivers/media/dvb-frontends/mb86a20s.c
index fade566..c2cc207 100644
--- a/drivers/media/dvb-frontends/mb86a20s.c
+++ b/drivers/media/dvb-frontends/mb86a20s.c
@@ -119,8 +119,8 @@ static struct regdata mb86a20s_init[] = {
{ 0x50, 0xb6 }, { 0x51, 0xff },
{ 0x50, 0xb7 }, { 0x51, 0xff },
{ 0x50, 0x50 }, { 0x51, 0x02 },
- { 0x50, 0x51 }, { 0x51, 0x04 },
- { 0x45, 0x04 },
+ { 0x50, 0x51 }, { 0x51, 0x04 }, /* MER symbol 4 */
+ { 0x45, 0x04 }, /* CN symbol 4 */
{ 0x48, 0x04 },
{ 0x50, 0xd5 }, { 0x51, 0x01 }, /* Serial */
{ 0x50, 0xd6 }, { 0x51, 0x1f },
@@ -176,6 +176,18 @@ static struct regdata mb86a20s_reset_reception[] = {
{ 0x08, 0x00 },
};
+static struct regdata mb86a20s_clear_stats[] = {
+ { 0x53, 0x00 }, /* VBER Counter reset */
+ { 0x53, 0x07 },
+
+ { 0x5f, 0x00 }, /* SBER Counter reset */
+ { 0x5f, 0x07 },
+
+ { 0x50, 0xb1 }, /* PBER Counter reset */
+ { 0x51, 0x07 },
+ { 0x51, 0x01 },
+};
+
static int mb86a20s_i2c_writereg(struct mb86a20s_state *state,
u8 i2c_addr, int reg, int data)
{
@@ -223,7 +235,7 @@ static int mb86a20s_i2c_readreg(struct mb86a20s_state
*state,
if (rc != 2) {
rc(%s: reg=0x%x (error=%d)\n, __func__, reg, rc);
- return rc;
+ return (rc 0) ? rc : -EIO;
}
return val;
@@ -278,29 +290,34 @@ err:
return rc;
}
-static int mb86a20s_read_signal_strength(struct dvb_frontend *fe, u16
*strength)
+static int __mb86a20s_read_signal_strength(struct dvb_frontend *fe, u16
*strength)
{
struct mb86a20s_state *state = fe-demodulator_priv;
+ int rc;
unsigned rf_max, rf_min, rf;
- u8 val;
-
- dprintk(\n);
-
- if (fe-ops.i2c_gate_ctrl)
- fe-ops.i2c_gate_ctrl(fe, 0);
/* Does a binary search to get RF strength */
rf_max = 0xfff;
rf_min = 0;
do {
rf = (rf_max + rf_min) / 2;
- mb86a20s_writereg(state, 0x04, 0x1f);
- mb86a20s_writereg(state, 0x05, rf 8);
- mb86a20s_writereg(state, 0x04, 0x20);
- mb86a20s_writereg(state, 0x04, rf);
+ rc = mb86a20s_writereg(state, 0x04, 0x1f);
+ if (rc 0)
+ return rc;
+ rc = mb86a20s_writereg(state, 0x05, rf 8);
+ if (rc 0)
+ return rc;
+ rc = mb86a20s_writereg(state, 0x04, 0x20);
+ if (rc 0)
+ return rc;
+ rc = mb86a20s_writereg(state, 0x04, rf);
+ if (rc 0)
+ return rc;
- val = mb86a20s_readreg(state, 0x02);
- if (val 0x08)
+ rc = mb86a20s_readreg(state, 0x02);
+ if (rc 0)
+ return rc;
+ if (rc 0x08)
rf_min = (rf_max + rf_min) / 2;
else
rf_max = (rf_max + rf_min) / 2;
@@ -310,12 +327,22 @@ static int mb86a20s_read_signal_strength(struct
dvb_frontend *fe, u16 *strength)
}
} while (1);
- dprintk(signal strength = %d\n, *strength);
+ return 0;
+}
+
+static int mb86a20s_read_signal_strength(struct dvb_frontend *fe, u16
*strength)
+{
+ int rc;
+
+ if (fe-ops.i2c_gate_ctrl)
+ fe-ops.i2c_gate_ctrl(fe, 0);
+
+ rc = __mb86a20s_read_signal_strength(fe, strength);
if (fe-ops.i2c_gate_ctrl)
fe-ops.i2c_gate_ctrl(fe, 1);
- return 0;
+ return rc;
}
static int mb86a20s_read_status(struct dvb_frontend *fe, fe_status_t *status)
@@ -615,6 +642,106 @@ static int mb86a20s_tune(struct dvb_frontend *fe,
return rc;
}
+static int mb86a20s_reset_counters(struct dvb_frontend *fe)
+{
+
+ struct mb86a20s_state *state = fe-demodulator_priv;
+ struct dtv_frontend_properties *c = fe-dtv_property_cache;
+
+ int rc, val;
+
+ if (fe-ops.i2c_gate_ctrl)
+ fe-ops.i2c_gate_ctrl(fe, 0);
+
+ /* Set the QoS clear status for most stats */
+ rc = mb86a20s_writeregdata(state, mb86a20s_clear_stats);
+ if (rc 0)
+ goto err;
+
+ /* CNR counter reset */
+ rc = mb86a20s_readreg(state, 0x45);
+ if
[PATCH RFCv9 1/4] dvb: Add DVBv5 stats properties for Quality of Service
The DVBv3 quality parameters are limited on several ways:
- Doesn't provide any way to indicate the used measure,
so userspace need to guess how to calculate the measure;
- Only a limited set of stats are supported;
- Can't be called in a way to require them to be filled
all at once (atomic reads from the hardware), with may
cause troubles on interpreting them on userspace;
- On some OFDM delivery systems, the carriers can be
independently modulated, having different properties.
Currently, there's no way to report per-layer stats.
To address the above issues, adding a new DVBv5-based stats
API.
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
---
v6: Add DocBook documentation.
v7: Some fixes as suggested by Antti
v8: Documentation fix, compilation fix and name the stats struct,
for its reusage inside the core
v9: counters need 32 bits. So, change the return data types to
s32/u32 types
---
Documentation/DocBook/media/dvb/dvbproperty.xml | 97 -
include/uapi/linux/dvb/frontend.h | 84 -
2 files changed, 178 insertions(+), 3 deletions(-)
diff --git a/Documentation/DocBook/media/dvb/dvbproperty.xml
b/Documentation/DocBook/media/dvb/dvbproperty.xml
index 957e3ac..9168808 100644
--- a/Documentation/DocBook/media/dvb/dvbproperty.xml
+++ b/Documentation/DocBook/media/dvb/dvbproperty.xml
@@ -7,16 +7,29 @@ the capability ioctls weren't implemented yet via the new
way./para
paraThe typical usage for the
constantFE_GET_PROPERTY/FE_SET_PROPERTY/constant
API is to replace the ioctl's were the link linkend=dvb-frontend-parameters
struct constantdvb_frontend_parameters/constant/link were used./para
+section id=dtv-stats
+titleDTV stats type/title
+programlisting
+struct dtv_stats {
+__u16 value;
+__u8 scale;
+} __attribute__ ((packed));
+/programlisting
+/section
section id=dtv-property
titleDTV property type/title
programlisting
/* Reserved fields should be set to 0 */
+
struct dtv_property {
__u32 cmd;
union {
__u32 data;
struct {
- __u8 data[32];
+ union {
+ __u8 data[32];
+ __u16 data[16];
+ }
__u32 len;
__u32 reserved1[3];
void *reserved2;
@@ -850,6 +863,78 @@ enum fe_interleaving {
parause the special macro LNA_AUTO to set LNA auto/para
/section
/section
+
+ section id=frontend-qos-properties
+ titleFrontend Quality of Service/Statistics indicators/title
+ paraExcept for link
linkend=DTV-QOS-ENUMconstantDTV_QOS_ENUM/constant/link,
+ the values are returned via
constantdtv_property.stat/constant./para
+ paraFor most delivery systems, this will return a single value for
each parameter./para
+ paraIt should be noticed, however, that new OFDM delivery systems
+ like ISDB can use different modulation types for each group of carriers.
+ On such standards, up to 3 groups of statistics can be provided, one
+ for each carrier group (called layer on ISDB).
+ In order to be consistent with other delivery systems, the first
+ value at link
linkend=dtv-statsconstantdtv_property.stat.dtv_stats/constant/link
array refers to
+ a global indicator, if any. The other elements of the array represent
+ each layer, starting from layer A(index 1), layer B (index 2) and so
on/para
+ paraThe number of filled elements are stored at
constantdtv_property.stat.len/constant./para
+ paraEach element of the
constantdtv_property.stat.dtv_stats/constant array consists on two
elements:/para
+ itemizedlist mark='opencircle'
+ listitemparaconstantvalue/constant - Value of the
measure/para/listitem
+ listitemparaconstantscale/constant - Scale for the
value. It can be:/para
+ section id = fecap-scale-params
+ itemizedlist mark='bullet'
+
listitemparaconstantFE_SCALE_NOT_AVAILABLE/constant - If it is not
possible to collect a given parameter (could be a transitory or permanent
condition)/para/listitem
+
listitemparaconstantFE_SCALE_DECIBEL/constant - parameter is a signed
value, measured in 0.1 dB/para/listitem
+
listitemparaconstantFE_SCALE_RELATIVE/constant - parameter is a
unsigned value, where 0 means 0% and 65535 means 100%./para/listitem
+
listitemparaconstantFE_SCALE_COUNTER/constant - parameter is a unsigned
value that counts the occurrence of an event, like bit error, block error, or
lapsed time./para/listitem
+ /itemizedlist
+ /section
+
[PATCH RFCv9 4/4] mb86a20s: add BER measure
Add per-layer BER measure. In order to provide some data for
applications not prepared for layers support, calculate BER for
the worse-case scenario, e. g. sums the BER values for all layers
and provide it as a global BER value.
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
---
drivers/media/dvb-frontends/mb86a20s.c | 99 --
1 file changed, 94 insertions(+), 5 deletions(-)
diff --git a/drivers/media/dvb-frontends/mb86a20s.c
b/drivers/media/dvb-frontends/mb86a20s.c
index c2cc207..7ecee12 100644
--- a/drivers/media/dvb-frontends/mb86a20s.c
+++ b/drivers/media/dvb-frontends/mb86a20s.c
@@ -94,7 +94,7 @@ static struct regdata mb86a20s_init[] = {
{ 0x04, 0x13 }, { 0x05, 0xff },
{ 0x04, 0x15 }, { 0x05, 0x4e },
{ 0x04, 0x16 }, { 0x05, 0x20 },
- { 0x52, 0x01 },
+ { 0x52, 0x01 }, /* Turn on BER before Viterbi */
{ 0x50, 0xa7 }, { 0x51, 0xff },
{ 0x50, 0xa8 }, { 0x51, 0xff },
{ 0x50, 0xa9 }, { 0x51, 0xff },
@@ -705,13 +705,76 @@ err:
return rc;
}
-static int mb86a20s_get_stats(struct dvb_frontend *fe)
+static int mb86a20s_get_ber_before_vterbi(struct dvb_frontend *fe,
+ unsigned layer,
+ u32 *error, u32 *count)
{
- int rc, i;
- __u16 strength;
+ struct mb86a20s_state *state = fe-demodulator_priv;
+ u8 byte[3];
+ int rc;
+
+ if (layer = 3)
+ return -EINVAL;
+
+ /* Check if it is available */
+ rc = mb86a20s_readreg(state, 0x54);
+ if (rc 0)
+ return rc;
+ /* Check if data is available for that layer */
+ if (!(rc (1 layer)))
+ return -EBUSY;
+
+ /* Read Bit Error Count */
+ rc = mb86a20s_readreg(state, 0x55 + layer);
+ if (rc 0)
+ return rc;
+ byte[0] = rc;
+ rc = mb86a20s_readreg(state, 0x56 + layer);
+ if (rc 0)
+ return rc;
+ byte[1] = rc;
+ rc = mb86a20s_readreg(state, 0x57 + layer);
+ if (rc 0)
+ return rc;
+ byte[2] = rc;
+ *error = byte[0] 16 | byte[1] 8 | byte[2];
+ /* Read Bit Count */
+ rc = mb86a20s_writereg(state, 0x50, 0xa7 + layer);
+ if (rc 0)
+ return rc;
+ rc = mb86a20s_readreg(state, 0x51);
+ if (rc 0)
+ return rc;
+ byte[0] = rc;
+ rc = mb86a20s_writereg(state, 0x50, 0xa8 + layer);
+ if (rc 0)
+ return rc;
+ rc = mb86a20s_readreg(state, 0x51);
+ if (rc 0)
+ return rc;
+ byte[1] = rc;
+ rc = mb86a20s_writereg(state, 0x50, 0xa9 + layer);
+ if (rc 0)
+ return rc;
+ rc = mb86a20s_readreg(state, 0x51);
+ if (rc 0)
+ return rc;
+ byte[2] = rc;
+ *count = byte[0] 16 | byte[1] 8 | byte[2];
+
+ return rc;
+}
+
+static int mb86a20s_get_stats(struct dvb_frontend *fe)
+{
struct mb86a20s_state *state = fe-demodulator_priv;
struct dtv_frontend_properties *c = fe-dtv_property_cache;
+ int rc, i;
+ u16 strength;
+ u32 bit_error = 0, bit_count = 0;
+ u32 t_bit_error = 0, t_bit_count = 0;
+ bool has_total_ber = true;
if (fe-ops.i2c_gate_ctrl)
fe-ops.i2c_gate_ctrl(fe, 0);
@@ -732,10 +795,36 @@ static int mb86a20s_get_stats(struct dvb_frontend *fe)
if (rc 0 rc 14) {
/* Layer is active and has rc segments */
- /* FIXME: add a per-layer stats logic */
+ /* Handle BER before vterbi */
+ rc = mb86a20s_get_ber_before_vterbi(fe, i, bit_error,
+ bit_count);
+ if (rc = 0) {
+ c-bit_error.stat[1 + i].scale =
FE_SCALE_COUNTER;
+ c-bit_error.stat[1 + i].uvalue = bit_error;
+ c-bit_count.stat[1 + i].scale =
FE_SCALE_COUNTER;
+ c-bit_count.stat[1 + i].uvalue = bit_count;
+ }
+ if (c-bit_error.stat[1 + i].scale != FE_SCALE_COUNTER)
+ has_total_ber = false;
+ else {
+ t_bit_error += c-bit_error.stat[1 + i].uvalue;
+ t_bit_count += c-bit_count.stat[1 + i].uvalue;
+ }
+
}
}
+ if (has_total_ber) {
+ /*
+* Total Bit Error/Count is calculated as the sum of the
+* bit errors on all active layers.
+*/
+ c-bit_error.stat[0].scale = FE_SCALE_COUNTER;
+ c-bit_error.stat[0].uvalue = t_bit_error;
+ c-bit_count.stat[0].scale
Re: [PATCH RFCv9] mb86a20s: add BER measure
Add per-layer BER measure. In order to provide some data for
applications not prepared for layers support, calculate BER for
the worse-case scenario, e. g. sums the BER values for all layers
and provide it as a global BER value.
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
---
v2: by mistake, I sent the wrong version that weren't properly incrementing
the per-layer registers
drivers/media/dvb-frontends/mb86a20s.c | 104 +++--
1 file changed, 99 insertions(+), 5 deletions(-)
diff --git a/drivers/media/dvb-frontends/mb86a20s.c
b/drivers/media/dvb-frontends/mb86a20s.c
index c2cc207..8a10111 100644
--- a/drivers/media/dvb-frontends/mb86a20s.c
+++ b/drivers/media/dvb-frontends/mb86a20s.c
@@ -94,7 +94,7 @@ static struct regdata mb86a20s_init[] = {
{ 0x04, 0x13 }, { 0x05, 0xff },
{ 0x04, 0x15 }, { 0x05, 0x4e },
{ 0x04, 0x16 }, { 0x05, 0x20 },
- { 0x52, 0x01 },
+ { 0x52, 0x01 }, /* Turn on BER before Viterbi */
{ 0x50, 0xa7 }, { 0x51, 0xff },
{ 0x50, 0xa8 }, { 0x51, 0xff },
{ 0x50, 0xa9 }, { 0x51, 0xff },
@@ -705,13 +705,76 @@ err:
return rc;
}
-static int mb86a20s_get_stats(struct dvb_frontend *fe)
+static int mb86a20s_get_ber_before_vterbi(struct dvb_frontend *fe,
+ unsigned layer,
+ u32 *error, u32 *count)
{
- int rc, i;
- __u16 strength;
+ struct mb86a20s_state *state = fe-demodulator_priv;
+ u8 byte[3];
+ int rc;
+
+ if (layer = 3)
+ return -EINVAL;
+
+ /* Check if it is available */
+ rc = mb86a20s_readreg(state, 0x54);
+ if (rc 0)
+ return rc;
+ /* Check if data is available for that layer */
+ if (!(rc (1 layer)))
+ return -EBUSY;
+
+ /* Read Bit Error Count */
+ rc = mb86a20s_readreg(state, 0x55 + layer * 3);
+ if (rc 0)
+ return rc;
+ byte[0] = rc;
+ rc = mb86a20s_readreg(state, 0x56 + layer * 3);
+ if (rc 0)
+ return rc;
+ byte[1] = rc;
+ rc = mb86a20s_readreg(state, 0x57 + layer * 3);
+ if (rc 0)
+ return rc;
+ byte[2] = rc;
+ *error = byte[0] 16 | byte[1] 8 | byte[2];
+
+ /* Read Bit Count */
+ rc = mb86a20s_writereg(state, 0x50, 0xa7 + layer * 3);
+ if (rc 0)
+ return rc;
+ rc = mb86a20s_readreg(state, 0x51);
+ if (rc 0)
+ return rc;
+ byte[0] = rc;
+ rc = mb86a20s_writereg(state, 0x50, 0xa8 + layer * 3);
+ if (rc 0)
+ return rc;
+ rc = mb86a20s_readreg(state, 0x51);
+ if (rc 0)
+ return rc;
+ byte[1] = rc;
+ rc = mb86a20s_writereg(state, 0x50, 0xa9 + layer * 3);
+ if (rc 0)
+ return rc;
+ rc = mb86a20s_readreg(state, 0x51);
+ if (rc 0)
+ return rc;
+ byte[2] = rc;
+ *count = byte[0] 16 | byte[1] 8 | byte[2];
+
+ return rc;
+}
+static int mb86a20s_get_stats(struct dvb_frontend *fe)
+{
struct mb86a20s_state *state = fe-demodulator_priv;
struct dtv_frontend_properties *c = fe-dtv_property_cache;
+ int rc, i;
+ u16 strength;
+ u32 bit_error = 0, bit_count = 0;
+ u32 t_bit_error = 0, t_bit_count = 0;
+ bool has_total_ber = true;
if (fe-ops.i2c_gate_ctrl)
fe-ops.i2c_gate_ctrl(fe, 0);
@@ -732,10 +795,41 @@ static int mb86a20s_get_stats(struct dvb_frontend *fe)
if (rc 0 rc 14) {
/* Layer is active and has rc segments */
- /* FIXME: add a per-layer stats logic */
+ /* Handle BER before vterbi */
+ rc = mb86a20s_get_ber_before_vterbi(fe, i, bit_error,
+ bit_count);
+ if (rc = 0) {
+ c-bit_error.stat[1 + i].scale =
FE_SCALE_COUNTER;
+ c-bit_error.stat[1 + i].uvalue = bit_error;
+ c-bit_count.stat[1 + i].scale =
FE_SCALE_COUNTER;
+ c-bit_count.stat[1 + i].uvalue = bit_count;
+ }
+ if (c-bit_error.stat[1 + i].scale != FE_SCALE_COUNTER)
+ has_total_ber = false;
+ else {
+ t_bit_error += c-bit_error.stat[1 + i].uvalue;
+ t_bit_count += c-bit_count.stat[1 + i].uvalue;
+ }
+
}
}
+ /*
+* FIXME: Some logic is likely need to ask the frontend to measure
+*BER again
+*/
+
+ if (has_total_ber) {
+ /*
+* Total Bit Error/Count is
[PATCH] drivers/media/pci: use memmove for overlapping regions
Change several memcpy() to memmove() in cases when the regions are
definitely overlapping; memcpy() of overlapping regions is undefined
behavior in C and can produce different results depending on the compiler,
the memcpy implementation, etc.
Signed-off-by: Nickolai Zeldovich nicko...@csail.mit.edu
---
drivers/media/pci/bt8xx/dst_ca.c |4 ++--
drivers/media/pci/cx18/cx18-vbi.c |2 +-
drivers/media/pci/ivtv/ivtv-vbi.c |4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/pci/bt8xx/dst_ca.c b/drivers/media/pci/bt8xx/dst_ca.c
index 7d96fab..0e788fc 100644
--- a/drivers/media/pci/bt8xx/dst_ca.c
+++ b/drivers/media/pci/bt8xx/dst_ca.c
@@ -180,11 +180,11 @@ static int ca_get_app_info(struct dst_state *state)
put_command_and_length(state-messages[0], CA_APP_INFO, length);
// Copy application_type, application_manufacturer and manufacturer_code
- memcpy(state-messages[4], state-messages[7], 5);
+ memmove(state-messages[4], state-messages[7], 5);
// Set string length and copy string
state-messages[9] = str_length;
- memcpy(state-messages[10], state-messages[12], str_length);
+ memmove(state-messages[10], state-messages[12], str_length);
return 0;
}
diff --git a/drivers/media/pci/cx18/cx18-vbi.c
b/drivers/media/pci/cx18/cx18-vbi.c
index 6d3121f..add9964 100644
--- a/drivers/media/pci/cx18/cx18-vbi.c
+++ b/drivers/media/pci/cx18/cx18-vbi.c
@@ -84,7 +84,7 @@ static void copy_vbi_data(struct cx18 *cx, int lines, u32
pts_stamp)
(the max size of the VBI data is 36 * 43 + 4 bytes).
So in this case we use the magic number 'ITV0'. */
memcpy(dst + sd, ITV0, 4);
- memcpy(dst + sd + 4, dst + sd + 12, line * 43);
+ memmove(dst + sd + 4, dst + sd + 12, line * 43);
size = 4 + ((43 * line + 3) ~3);
} else {
memcpy(dst + sd, itv0, 4);
diff --git a/drivers/media/pci/ivtv/ivtv-vbi.c
b/drivers/media/pci/ivtv/ivtv-vbi.c
index 293db80..3c156bc 100644
--- a/drivers/media/pci/ivtv/ivtv-vbi.c
+++ b/drivers/media/pci/ivtv/ivtv-vbi.c
@@ -224,7 +224,7 @@ static void copy_vbi_data(struct ivtv *itv, int lines, u32
pts_stamp)
(the max size of the VBI data is 36 * 43 + 4 bytes).
So in this case we use the magic number 'ITV0'. */
memcpy(dst + sd, ITV0, 4);
- memcpy(dst + sd + 4, dst + sd + 12, line * 43);
+ memmove(dst + sd + 4, dst + sd + 12, line * 43);
size = 4 + ((43 * line + 3) ~3);
} else {
memcpy(dst + sd, itv0, 4);
@@ -532,7 +532,7 @@ void ivtv_vbi_work_handler(struct ivtv *itv)
while (vi-cc_payload_idx) {
cc = vi-cc_payload[0];
- memcpy(vi-cc_payload, vi-cc_payload + 1,
+ memmove(vi-cc_payload, vi-cc_payload + 1,
sizeof(vi-cc_payload) -
sizeof(vi-cc_payload[0]));
vi-cc_payload_idx--;
if (vi-cc_payload_idx cc.odd[0] == 0x80
cc.odd[1] == 0x80)
--
1.7.10.4
--
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
Re: [PATCH 6/6] ARM: shmobile: convert ap4evb to asynchronously register camera subdevices
On Wed, Dec 26, 2012 at 06:49:11PM +0100, Guennadi Liakhovetski wrote: Register the imx074 camera I2C and the CSI-2 platform devices directly in board platform data instead of letting the sh_mobile_ceu_camera driver and the soc-camera framework register them at their run-time. This uses the V4L2 asynchronous subdevice probing capability. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Hi Guennadi, could you let me know what if any dependencies this patch has. And the status of any dependencies. -- 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
RE: [PATCHv16 5/7] fbmon: add of_videomode helpers
Hi Rob, On Tue, Jan 08, 2013 at 01:36:50, Rob Clark wrote: On Mon, Jan 7, 2013 at 2:46 AM, Mohammed, Afzal af...@ti.com wrote: On Mon, Jan 07, 2013 at 13:36:48, Steffen Trumtrar wrote: I just did a quick make da8xx_omapl_defconfig make and it builds just fine. On what version did you apply the series? At the moment I have the series sitting on 3.7. Didn't try any 3.8-rcx yet. But fixing this shouldn't be a problem. The change as I mentioned or something similar would be required as any driver that is going to make use of of_get_fb_videomode() would break if CONFIG_OF_VIDEOMODE or CONFIG_FB_MODE_HELPERS is not defined. Shouldn't the driver that depends on CONFIG_OF_VIDEOMODE and CONFIG_FB_MODE_HELPERS, explicitly select them? I don't really see the point of having the static-inline fallbacks. But here da8xx-fb driver does not depend on _OF_VIDEOMODE and _FB_MODE_HELPERS, currently it works as a pure platform driver for DaVinci SoC's without those CONFIG's. It is only upon enhancing the driver to make use of of_get_fb_videomode() for DT support those CONFIG's are being made use of. As the driver can work w/o these CONFIG's and so as it is not a dependency for driver on non-DT boot (as in the case of DaVinci), I disagree in selecting those options always, but rather giving user an option to select. And selecting these options always will bring in some amount of code onto Kernel image w/o any purpose in the case of DaVinci builds. Another option would be to sprinkle driver with ifdef's to avoid inline fallbacks, which is not a good thing to do. Moreover having a static inline fallback is more in line with other of_*'s. fwiw, using 'select' is what I was doing for lcd panel support for lcdc/da8xx drm driver (which was using the of videomode helpers, albeit a slightly earlier version of the patches): In your case as it is a new driver is meant only for DT, that is fine, but here it is an existing driver that works w/o these. Regards Afzal
global mutex in dvb_usercopy (dvbdev.c)
Hi Everyone, I have a doubt regarding about the global mutex lock in dvb_usercopy(drivers/media/dvb-core/dvbdev.c, line 382) . /* call driver */ mutex_lock(dvbdev_mutex); if ((err = func(file, cmd, parg)) == -ENOIOCTLCMD) err = -EINVAL; mutex_unlock(dvbdev_mutex); Why is this mutex needed? When I check similar functions like video_usercopy, this kind of global locking is not present when func() is called. This global lock will prevent any other ioctl call from being executed unless the previous blocking ioctl call has returned. If we need to have a lock why not make it file handle specific ? Thanks for your help. Best Regards Soby Mathew Best Regards -- 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
Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
On 1/7/2013 9:15 PM, Mauro Carvalho Chehab wrote: Em Mon, 7 Jan 2013 12:40:50 -0700 Jonathan Corbet cor...@lwn.net escreveu: On Mon, 7 Jan 2013 00:09:47 +0100 Alessandro Rubini rub...@gnudd.com wrote: I don't expect you'll see serious performance differences on the PC. I think ARM users will have better benefits, due to the different cache architecture. You told me Jon measured meaningful figures on a Marvel CPU. It made the difference between 10 frames per second with the CPU running flat out and 30fps mostly idle. I think that probably counts as meaningful, yeah...:) Couldn't this performance difference be due to the usage of GFP_DMA inside the VB2 code, like Federico's new patch series is proposing? If not, why are there a so large performance penalty? Nope, this was caused rather by a very poor CPU access to non-cached (aka 'coherent') memory and the way the video data has been accessed/read with CPU. Best regards -- Marek Szyprowski Samsung Poland RD Center -- 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
Re: [PATCH v4 1/3] videobuf2-dma-contig: user can specify GFP flags
Hello,
On 1/6/2013 6:29 PM, Federico Vaga wrote:
This is useful when you need to specify specific GFP flags during memory
allocation (e.g. GFP_DMA).
Signed-off-by: Federico Vaga federico.v...@gmail.com
---
drivers/media/v4l2-core/videobuf2-dma-contig.c | 7 ++-
include/media/videobuf2-dma-contig.h | 5 +
2 file modificati, 7 inserzioni(+), 5 rimozioni(-)
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 10beaee..bb411c0 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -21,10 +21,6 @@
#include media/videobuf2-dma-contig.h
#include media/videobuf2-memops.h
-struct vb2_dc_conf {
- struct device *dev;
-};
-
struct vb2_dc_buf {
struct device *dev;
void*vaddr;
@@ -165,7 +161,8 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long
size)
/* align image size to PAGE_SIZE */
size = PAGE_ALIGN(size);
- buf-vaddr = dma_alloc_coherent(dev, size, buf-dma_addr, GFP_KERNEL);
+ buf-vaddr = dma_alloc_coherent(dev, size, buf-dma_addr,
+ GFP_KERNEL
| conf-mem_flags);
I think we can add GFP_DMA flag unconditionally to the vb2_dc_contig
allocator.
It won't hurt existing clients as most of nowadays platforms doesn't
have DMA
zone (GFP_DMA is ignored in such case), but it should fix the issues
with some
older and non-standard systems.
if (!buf-vaddr) {
dev_err(dev, dma_alloc_coherent of size %ld failed\n, size);
kfree(buf);
diff --git a/include/media/videobuf2-dma-contig.h
b/include/media/videobuf2-dma-contig.h
index 8197f87..22733f4 100644
--- a/include/media/videobuf2-dma-contig.h
+++ b/include/media/videobuf2-dma-contig.h
@@ -16,6 +16,11 @@
#include media/videobuf2-core.h
#include linux/dma-mapping.h
+struct vb2_dc_conf {
+ struct device *dev;
+ gfp_t mem_flags;
+};
+
static inline dma_addr_t
vb2_dma_contig_plane_dma_addr(struct vb2_buffer *vb, unsigned int plane_no)
{
Best regards
--
Marek Szyprowski
Samsung Poland RD Center
--
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
[PATCH 1/1] [media] s5k6aa: Use devm_regulator_bulk_get API
devm_regulator_bulk_get is device managed and saves some cleanup
and exit code.
Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
---
drivers/media/i2c/s5k6aa.c |7 ++-
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/media/i2c/s5k6aa.c b/drivers/media/i2c/s5k6aa.c
index 57cd4fa..bdf5e3d 100644
--- a/drivers/media/i2c/s5k6aa.c
+++ b/drivers/media/i2c/s5k6aa.c
@@ -1598,7 +1598,7 @@ static int s5k6aa_probe(struct i2c_client *client,
for (i = 0; i S5K6AA_NUM_SUPPLIES; i++)
s5k6aa-supplies[i].supply = s5k6aa_supply_names[i];
- ret = regulator_bulk_get(client-dev, S5K6AA_NUM_SUPPLIES,
+ ret = devm_regulator_bulk_get(client-dev, S5K6AA_NUM_SUPPLIES,
s5k6aa-supplies);
if (ret) {
dev_err(client-dev, Failed to get regulators\n);
@@ -1607,7 +1607,7 @@ static int s5k6aa_probe(struct i2c_client *client,
ret = s5k6aa_initialize_ctrls(s5k6aa);
if (ret)
- goto out_err4;
+ goto out_err3;
s5k6aa_presets_data_init(s5k6aa);
@@ -1618,8 +1618,6 @@ static int s5k6aa_probe(struct i2c_client *client,
return 0;
-out_err4:
- regulator_bulk_free(S5K6AA_NUM_SUPPLIES, s5k6aa-supplies);
out_err3:
s5k6aa_free_gpios(s5k6aa);
out_err2:
@@ -1635,7 +1633,6 @@ static int s5k6aa_remove(struct i2c_client *client)
v4l2_device_unregister_subdev(sd);
v4l2_ctrl_handler_free(sd-ctrl_handler);
media_entity_cleanup(sd-entity);
- regulator_bulk_free(S5K6AA_NUM_SUPPLIES, s5k6aa-supplies);
s5k6aa_free_gpios(s5k6aa);
return 0;
--
1.7.4.1
--
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
[PATCH 1/1] [media] s5p-csis: Use devm_regulator_bulk_get API
devm_regulator_bulk_get is device managed and saves some cleanup
and exit code.
Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
---
drivers/media/platform/s5p-fimc/mipi-csis.c |7 ++-
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/s5p-fimc/mipi-csis.c
b/drivers/media/platform/s5p-fimc/mipi-csis.c
index cde510f..7e36ad9 100644
--- a/drivers/media/platform/s5p-fimc/mipi-csis.c
+++ b/drivers/media/platform/s5p-fimc/mipi-csis.c
@@ -743,7 +743,7 @@ static int s5pcsis_probe(struct platform_device *pdev)
for (i = 0; i CSIS_NUM_SUPPLIES; i++)
state-supplies[i].supply = csis_supply_name[i];
- ret = regulator_bulk_get(pdev-dev, CSIS_NUM_SUPPLIES,
+ ret = devm_regulator_bulk_get(pdev-dev, CSIS_NUM_SUPPLIES,
state-supplies);
if (ret)
return ret;
@@ -762,7 +762,7 @@ static int s5pcsis_probe(struct platform_device *pdev)
0, dev_name(pdev-dev), state);
if (ret) {
dev_err(pdev-dev, Interrupt request failed\n);
- goto e_regput;
+ goto e_clkput;
}
v4l2_subdev_init(state-sd, s5pcsis_subdev_ops);
@@ -793,8 +793,6 @@ static int s5pcsis_probe(struct platform_device *pdev)
pm_runtime_enable(pdev-dev);
return 0;
-e_regput:
- regulator_bulk_free(CSIS_NUM_SUPPLIES, state-supplies);
e_clkput:
clk_disable(state-clock[CSIS_CLK_MUX]);
s5pcsis_clk_put(state);
@@ -903,7 +901,6 @@ static int s5pcsis_remove(struct platform_device *pdev)
clk_disable(state-clock[CSIS_CLK_MUX]);
pm_runtime_set_suspended(pdev-dev);
s5pcsis_clk_put(state);
- regulator_bulk_free(CSIS_NUM_SUPPLIES, state-supplies);
media_entity_cleanup(state-sd.entity);
--
1.7.4.1
--
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
Re: [linux-uvc-devel] [PATCH 0/2] uvcvideo: Support Logitech RGB Bayer formats
Am 07.01.2013 23:45, schrieb Laurent Pinchart: Hi Oleksij, (CC'ing linux-media) On Wednesday 02 January 2013 11:50:51 Oleksij Rempel wrote: Am 02.01.2013 10:03, schrieb Peter Ross: On Tue, Jan 01, 2013 at 05:16:44PM +0100, Oleksij Rempel wrote: Hi Peter, thank you for your work, but most of it belongs to uvcdynctrl. You will need to add it here: /usr/share/uvcdynctrl/data/046d/logitech.xml Using uvcdynctrl is problematic for two reasons. 1. Setting the Logitech 'disable video processing' and 'raw data bpp' controls independently from uvcvideo causes the driver to *always* drop frames. uvc_video_decode_isoc() performs a sanity check on the size of incoming uncompressed frames. It expects to receive frame sizes reported by the 8-bit yuyv 4:2:2 format description. The check fails when video processing is disabled, because the 8-/10-bit RGB Bayer frame sizes are always smaller. 2. Userspace applications need to distinguish the yuyv 4:2:2 format from the the additional RGB Bayer formats. Currently, when video processing is disabled, user applications expect to receive yuyv 4:2:2 pixels, because thats what it and uvcvideo has agreed to (mediate by V4L2). Instead the application receives RGB Bayer pixels and attempts to process them as yuyv resulting in visual garbage. One could argue that an application that explicitly disables processing would be able to handle that. However, I agree that point 1 is problematic, and point 2 is definitely not clean. Some of this has been discussed previously, see http://article.gmane.org/gmane.linux.drivers.uvc.devel/3061/. The patch tries to solve both of these problems: 1) making uvcvideo aware of the Logitech controls and recalculating the expected frame sizes, and 2) presenting the RGB Bayer formats through the V4L2 interface, so user application can request them. If, the problem seems to be bigger. We have: 1. Formats provided by usb descriptor and controlled over uvc protocol. 2. Formats provided by usb descriptor and controlled over XU Do we really have devices that expose formats through the standard UVC descriptors but require XU access to select them ? h264? Can be started over uvc, but mostly use less without XU. I'll bet, if you will accept this XUs, some body will ask to add XU for h264. 3. Formats controlled over XU and provided by documentation other kind of knowledge. Last case in not properly handled by uvcvideo.ko, but IMHO it is not good way to have all possible XU in driver. Both problems you described should be fixed, but not in this way. If it is possible - uvcdynctl should provide/update format descriptor over uvc_xu_control_query interface or some other way. Beside, i was working on kernel space plugin system for uvcvideo driver, which was probably not the best idea. How about to do some part of it in userspace? For example, uvcdynctl can provide bandwidth information too. This way we can fix many buggy cams without needing to permanently update kernel driver. Laurent? I'm really undecided here. On one hand I agree with you, from a theoretical point of view the driver should not know about all possible XUs. This just can't scale. On the other hand, it's pretty clear that we don't need to scale. We only have a single public dynamic controls XML file, and looking at the descriptors of most devices it's pretty clear that they reuse the same XUs, as they are based on only a dozen or so different chips. The dynamic controls API brings additional complexity to the driver, and I think that it was a good design decision. However, in some cases, the implications of changing an XU control value go well beyond what is usually expected of a control. I'm thus tempted to allow handling of XU controls in the kernel when there's a good reason to do so. Of course I want the code to be clean, without lots of hooks all over the place that would make the driver sources impossible to read and understand :-) Comments and thoughts will be appreciated. I had my doubts in that because: - we don't know if same XU was used for some thing different in ... older version of some hardware. - we can't guarantee anything here. This XUs are not documented and not a part of sail strategy. If some thing will not work, or will brake because of it - only we will be responsible. - In this case we will not have any win in case of bandwidth[1]. To enable this stream we should ask YUV and webcam wil allocate bandwidth for it. - but all this is just FUD. I always happy to see, if some body goes beyond it and revers undocumented XUs and brings patches to make use of it for all :) Peter thx! [1] - bandwidth seems to be number one in our top 3 of always coming problems :) -- Regards, Oleksij -- 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
