RE: [PATCH 7/9] ARM: DMA: steal memory for DMA coherent mappings
On Wed, 2011-08-17 at 15:06 +0200, Marek Szyprowski wrote: [...] > > > Maybe for the first version a static pool with reasonably small size > > > (like 128KiB) will be more than enough? This size can be even board > > > depended or changed with kernel command line for systems that really > > > needs more memory. > > > > For a first version that sounds good enough. Maybe we could use a fraction > > of the CONSISTENT_DMA_SIZE as an estimate? > > Ok, good. For the initial values I will probably use 1/8 of > CONSISTENT_DMA_SIZE for coherent allocations. Writecombine atomic allocations > are extremely rare and rather ARM specific. 1/32 of CONSISTENT_DMA_SIZE should > be more than enough for them. For people who aren't aware, we have a patch to remove the define CONSISTENT_DMA_SIZE and replace it with a runtime call to an initialisation function [1]. I don't believe this fundamentally changes anything being discussed though. -- Tixy [1] http://www.spinics.net/lists/arm-kernel/msg135589.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
[PATCH] adp1653: make ->power() method optional
The ->power() could be absent or not used on some platforms. This patch makes
its presence optional.
Signed-off-by: Andy Shevchenko
Cc: Sakari Ailus
---
drivers/media/video/adp1653.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c
index 0fd9579..65f6f3f 100644
--- a/drivers/media/video/adp1653.c
+++ b/drivers/media/video/adp1653.c
@@ -309,6 +309,9 @@ __adp1653_set_power(struct adp1653_flash *flash, int on)
{
int ret;
+ if (flash->platform_data->power == NULL)
+ return 0;
+
ret = flash->platform_data->power(&flash->subdev, on);
if (ret < 0)
return ret;
--
1.7.5.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] Media controller: Define media_entity_init() and media_entity_cleanup() conditionally
On Wednesday 17 August 2011 12:53:46 Andy Shevchenko wrote: > On Wed, 2011-08-17 at 16:04 +0530, Deepthy Ravi wrote: > > From: Vaibhav Hiremath > > > > Defines the two functions only when CONFIG_MEDIA_CONTROLLER > > is enabled. > > Is it not a driver's option to be dependent on MEDIA_CONTROLLER? Yes it is. -- 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] adp1653: make ->power() method optional
On Thu, Aug 18, 2011 at 11:53:03AM +0300, Andy Shevchenko wrote:
> The ->power() could be absent or not used on some platforms. This patch makes
> its presence optional.
Hi Andy,
Thanks for the patch!
> Signed-off-by: Andy Shevchenko
> Cc: Sakari Ailus
> ---
> drivers/media/video/adp1653.c |3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c
> index 0fd9579..65f6f3f 100644
> --- a/drivers/media/video/adp1653.c
> +++ b/drivers/media/video/adp1653.c
> @@ -309,6 +309,9 @@ __adp1653_set_power(struct adp1653_flash *flash, int on)
> {
> int ret;
>
> + if (flash->platform_data->power == NULL)
> + return 0;
> +
> ret = flash->platform_data->power(&flash->subdev, on);
> if (ret < 0)
> return ret;
> --
> 1.7.5.4
>
How about doing this in adp1653_set_power() instead of
__adp1653_set_power()? At least I don't see any ill effects from this.
There's no need to keep track of the power state (flash->power_count) if
there isn't one. :-)
--
Sakari Ailus
sakari.ai...@iki.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] Media controller: Define media_entity_init() and media_entity_cleanup() conditionally
Thanks,
Deepthy Ravi.
From: Hiremath, Vaibhav
Sent: Wednesday, August 17, 2011 9:00 PM
To: Ravi, Deepthy; mche...@infradead.org; linux-media@vger.kernel.org;
linux-ker...@vger.kernel.org
Cc: linux-o...@vger.kernel.org
Subject: RE: [PATCH] Media controller: Define media_entity_init() and
media_entity_cleanup() conditionally
> -Original Message-
> From: Ravi, Deepthy
> Sent: Wednesday, August 17, 2011 4:05 PM
> To: mche...@infradead.org; linux-media@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Cc: linux-o...@vger.kernel.org; Hiremath, Vaibhav; Ravi, Deepthy
> Subject: [PATCH] Media controller: Define media_entity_init() and
> media_entity_cleanup() conditionally
>
> From: Vaibhav Hiremath
>
> Defines the two functions only when CONFIG_MEDIA_CONTROLLER
> is enabled.
[Hiremath, Vaibhav] Deepthy,
You may want to mention about build failure without MEDIA_CONTROLLER option
being enabled, especially if any sensor driver is being used between MC and
non-MC framework compatible devices.
For example, OMAP3 and AM3517, where TVP5146 is being used but OMAP3 is based
on MC framework and AM3517 is based on simple sub-dev based interface.
[Deepthy Ravi] Ok. I will change description to include that.
Thanks,
Vaibhav
>
> Signed-off-by: Vaibhav Hiremath
> Signed-off-by: Deepthy Ravi
> ---
> include/media/media-entity.h |9 +
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index cd8bca6..c90916e 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -121,9 +121,18 @@ struct media_entity_graph {
> int top;
> };
>
> +#ifdef CONFIG_MEDIA_CONTROLLER
> int media_entity_init(struct media_entity *entity, u16 num_pads,
> struct media_pad *pads, u16 extra_links);
> void media_entity_cleanup(struct media_entity *entity);
> +#else
> +static inline int media_entity_init(struct media_entity *entity, u16
> num_pads,
> + struct media_pad *pads, u16 extra_links)
> +{
> + return 0;
> +}
> +static inline void media_entity_cleanup(struct media_entity *entity) {}
> +#endif
>
> int media_entity_create_link(struct media_entity *source, u16 source_pad,
> struct media_entity *sink, u16 sink_pad, u32 flags);
> --
> 1.7.0.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] adp1653: make ->power() method optional
On Thu, 2011-08-18 at 12:21 +0300, Sakari Ailus wrote:
> On Thu, Aug 18, 2011 at 11:53:03AM +0300, Andy Shevchenko wrote:
> > The ->power() could be absent or not used on some platforms. This patch
> > makes
> > its presence optional.
>
> Hi Andy,
>
> Thanks for the patch!
>
> > Signed-off-by: Andy Shevchenko
> > Cc: Sakari Ailus
> > ---
> > drivers/media/video/adp1653.c |3 +++
> > 1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c
> > index 0fd9579..65f6f3f 100644
> > --- a/drivers/media/video/adp1653.c
> > +++ b/drivers/media/video/adp1653.c
> > @@ -309,6 +309,9 @@ __adp1653_set_power(struct adp1653_flash *flash, int on)
> > {
> > int ret;
> >
> > + if (flash->platform_data->power == NULL)
> > + return 0;
> > +
> > ret = flash->platform_data->power(&flash->subdev, on);
> > if (ret < 0)
> > return ret;
> How about doing this in adp1653_set_power() instead of
> __adp1653_set_power()? At least I don't see any ill effects from this.
> There's no need to keep track of the power state (flash->power_count) if
> there isn't one. :-)
It was my first assumption. However, the __adp1653_set_power() is used
directly from suspend/resume methods.
--
Andy Shevchenko
Intel Finland Oy
--
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] adp1653: make ->power() method optional
On Thu, Aug 18, 2011 at 01:30:50PM +0300, Andy Shevchenko wrote:
> On Thu, 2011-08-18 at 12:21 +0300, Sakari Ailus wrote:
> > On Thu, Aug 18, 2011 at 11:53:03AM +0300, Andy Shevchenko wrote:
> > > The ->power() could be absent or not used on some platforms. This patch
> > > makes
> > > its presence optional.
> >
> > Hi Andy,
> >
> > Thanks for the patch!
> >
> > > Signed-off-by: Andy Shevchenko
> > > Cc: Sakari Ailus
> > > ---
> > > drivers/media/video/adp1653.c |3 +++
> > > 1 files changed, 3 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c
> > > index 0fd9579..65f6f3f 100644
> > > --- a/drivers/media/video/adp1653.c
> > > +++ b/drivers/media/video/adp1653.c
> > > @@ -309,6 +309,9 @@ __adp1653_set_power(struct adp1653_flash *flash, int
> > > on)
> > > {
> > > int ret;
> > >
> > > + if (flash->platform_data->power == NULL)
> > > + return 0;
> > > +
> > > ret = flash->platform_data->power(&flash->subdev, on);
> > > if (ret < 0)
> > > return ret;
>
> > How about doing this in adp1653_set_power() instead of
> > __adp1653_set_power()? At least I don't see any ill effects from this.
> > There's no need to keep track of the power state (flash->power_count) if
> > there isn't one. :-)
> It was my first assumption. However, the __adp1653_set_power() is used
> directly from suspend/resume methods.
It is but it won't get called: power_count will be always zero when the
power() callback doesn't exist.
--
Sakari Ailus
sakari.ai...@iki.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] adp1653: make ->power() method optional
On Thu, 2011-08-18 at 13:53 +0300, Sakari Ailus wrote:
> On Thu, Aug 18, 2011 at 01:30:50PM +0300, Andy Shevchenko wrote:
> > On Thu, 2011-08-18 at 12:21 +0300, Sakari Ailus wrote:
> > > On Thu, Aug 18, 2011 at 11:53:03AM +0300, Andy Shevchenko wrote:
> > > > The ->power() could be absent or not used on some platforms. This patch
> > > > makes
> > > > its presence optional.
> > >
> > > Hi Andy,
> > >
> > > Thanks for the patch!
> > >
> > > > Signed-off-by: Andy Shevchenko
> > > > Cc: Sakari Ailus
> > > > ---
> > > > drivers/media/video/adp1653.c |3 +++
> > > > 1 files changed, 3 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/drivers/media/video/adp1653.c
> > > > b/drivers/media/video/adp1653.c
> > > > index 0fd9579..65f6f3f 100644
> > > > --- a/drivers/media/video/adp1653.c
> > > > +++ b/drivers/media/video/adp1653.c
> > > > @@ -309,6 +309,9 @@ __adp1653_set_power(struct adp1653_flash *flash,
> > > > int on)
> > > > {
> > > > int ret;
> > > >
> > > > + if (flash->platform_data->power == NULL)
> > > > + return 0;
> > > > +
> > > > ret = flash->platform_data->power(&flash->subdev, on);
> > > > if (ret < 0)
> > > > return ret;
> >
> > > How about doing this in adp1653_set_power() instead of
> > > __adp1653_set_power()? At least I don't see any ill effects from this.
> > > There's no need to keep track of the power state (flash->power_count) if
> > > there isn't one. :-)
> > It was my first assumption. However, the __adp1653_set_power() is used
> > directly from suspend/resume methods.
>
> It is but it won't get called: power_count will be always zero when the
> power() callback doesn't exist.
Ah, now I got the full picture. Yes, if we leave adp1653_set_power()
immediately, then power_count stays 0.
I will send patch v2 soon.
--
Andy Shevchenko
Intel Finland Oy
--
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
[PATCHv2] adp1653: make ->power() method optional
The ->power() could be absent or not used on some platforms. This patch makes its presence optional. Signed-off-by: Andy Shevchenko Cc: Sakari Ailus --- drivers/media/video/adp1653.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c index 0fd9579..f830313 100644 --- a/drivers/media/video/adp1653.c +++ b/drivers/media/video/adp1653.c @@ -329,6 +329,11 @@ adp1653_set_power(struct v4l2_subdev *subdev, int on) struct adp1653_flash *flash = to_adp1653_flash(subdev); int ret = 0; + /* There is no need to switch power in case of absence ->power() +* method. */ + if (flash->platform_data->power == NULL) + return 0; + mutex_lock(&flash->power_lock); /* If the power count is modified from 0 to != 0 or from != 0 to 0, -- 1.7.5.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: [PATCHv2] adp1653: make ->power() method optional
On Thu, 2011-08-18 at 14:22 +0300, Andy Shevchenko wrote: > The ->power() could be absent or not used on some platforms. This patch makes > its presence optional. > > Signed-off-by: Andy Shevchenko > Cc: Sakari Ailus > --- > drivers/media/video/adp1653.c |5 + > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c > index 0fd9579..f830313 100644 > --- a/drivers/media/video/adp1653.c > +++ b/drivers/media/video/adp1653.c > @@ -329,6 +329,11 @@ adp1653_set_power(struct v4l2_subdev *subdev, int on) > struct adp1653_flash *flash = to_adp1653_flash(subdev); > int ret = 0; > > + /* There is no need to switch power in case of absence ->power() > + * method. */ > + if (flash->platform_data->power == NULL) > + return 0; > + > mutex_lock(&flash->power_lock); > > /* If the power count is modified from 0 to != 0 or from != 0 to 0, He-h, I guess you are not going to apply this one. The patch breaks init logic of the device. If we have no ->power(), we still need to bring the device to the known state. I have no good idea how to do this. -- Andy Shevchenko Intel Finland Oy -- 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: [PATCHv2] adp1653: make ->power() method optional
On Thu, Aug 18, 2011 at 02:32:02PM +0300, Andy Shevchenko wrote: > On Thu, 2011-08-18 at 14:22 +0300, Andy Shevchenko wrote: > > The ->power() could be absent or not used on some platforms. This patch > > makes > > its presence optional. > > > > Signed-off-by: Andy Shevchenko > > Cc: Sakari Ailus > > --- > > drivers/media/video/adp1653.c |5 + > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c > > index 0fd9579..f830313 100644 > > --- a/drivers/media/video/adp1653.c > > +++ b/drivers/media/video/adp1653.c > > @@ -329,6 +329,11 @@ adp1653_set_power(struct v4l2_subdev *subdev, int on) > > struct adp1653_flash *flash = to_adp1653_flash(subdev); > > int ret = 0; > > > > + /* There is no need to switch power in case of absence ->power() > > +* method. */ > > + if (flash->platform_data->power == NULL) > > + return 0; > > + > > mutex_lock(&flash->power_lock); > > > > /* If the power count is modified from 0 to != 0 or from != 0 to 0, > > He-h, I guess you are not going to apply this one. > The patch breaks init logic of the device. If we have no ->power(), we > still need to bring the device to the known state. I have no good idea > how to do this. I don't think it breaks anything actually. Albeit in practice one is still likely to put the adp1653 reset line to the board since that lowers its power consumption significantly. Instead of being in power-up state after opening the flash subdev, it will reach this state already when the system is powered up. At subdev open all the relevant registers are written to anyway, so I don't see an issue here. I think either this one, or one should check in probe() that the power() callback is non-NULL. The board code is going away in the near future so this callback will disappear eventually anyway. The gpio code in the board file should likely be moved to the driver itself. That assumes that there will be a gpio which can be used to enable and disable the device and I'm not fully certain this is generic enough. Hopefully it is, but I don't know where else the adp1653 would be used than on the N900. Cheers, -- Sakari Ailus sakari.ai...@iki.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
Record DVB-T from command line
Hi. I need to record from DVB-T using the command line. I'm looking for some commands to make that saving the recording to a .ts file. Could you help me please? Thank you very much! Bye. -- 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: Kernel oops with Kopete due to DVB device
On 03/08/11 23:06, Laurent Grawet wrote: > On 02/08/11 21:37, Laurent Grawet wrote: >> Bug initially submitted to https://bugs.kde.org/show_bug.cgi?id=279202 >> >> Hello, >> >> Kopete crash and kernel oopses when opening "Settings -> Configure" dialog >> due to presence of DVB-S PCI card (av7110) as /dev/video0. This happens >> everytime I try to configure Kopete in presence of my DVB PCI card. >> (see attachment) >> >> Reproducible: Always >> >> Steps to Reproduce: >> Own a DVB card and try to configure Kopete. >> >> Actual Results: >> Kopete crash and Kernel oopses. >> >> Expected Results: >> Configure dialog opening. > Cheese gnome app also get into trouble. No kernel oops but the machine > get unresponsive and quickly begins to swap like hell until I kill > cheese. I don't have this problem when I plug my USB uvc webcam. > Probably because this is the first device picked up by kopete or cheese. > But as soon I try to configure the DVB device in kopete, kernel oops... The bug is also present in 3.0 kernel. Laurent -- 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: Fix a UVC performance problem on systems with non-coherent DMA.
The UVC driver uses usb_alloc_coherent() to allocate DMA data buffers.
On systems without coherent DMA this ends up allocating buffers in
uncached memory. The subsequent memcpy's done to coalesce the DMA
chunks into contiguous buffers then run VERY slowly. On a MIPS test
system the memcpy is about 200 times slower. This issue prevents the
system from keeping up with 720p YUYV data at 10fps.
The following patch uses kmalloc to alloc the DMA buffers instead of
uab_alloc_coherent on systems without coherent DMA. With this patch
the system was easily able to keep up with 720p at 10fps.
Signed-off-by: Al Cooper
---
drivers/media/video/uvc/uvc_video.c | 18 +-
1 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/uvc/uvc_video.c
b/drivers/media/video/uvc/uvc_video.c
index 4999479..30c18b4 100644
--- a/drivers/media/video/uvc/uvc_video.c
+++ b/drivers/media/video/uvc/uvc_video.c
@@ -790,8 +790,12 @@ static void uvc_free_urb_buffers(struct uvc_streaming
*stream)
for (i = 0; i < UVC_URBS; ++i) {
if (stream->urb_buffer[i]) {
+#ifndef CONFIG_DMA_NONCOHERENT
usb_free_coherent(stream->dev->udev, stream->urb_size,
stream->urb_buffer[i], stream->urb_dma[i]);
+#else
+ kfree(stream->urb_buffer[i]);
+#endif
stream->urb_buffer[i] = NULL;
}
}
@@ -831,9 +835,15 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming
*stream,
for (; npackets > 1; npackets /= 2) {
for (i = 0; i < UVC_URBS; ++i) {
stream->urb_size = psize * npackets;
+#ifndef CONFIG_DMA_NONCOHERENT
stream->urb_buffer[i] = usb_alloc_coherent(
stream->dev->udev, stream->urb_size,
gfp_flags | __GFP_NOWARN, &stream->urb_dma[i]);
+#else
+ stream->urb_buffer[i] =
+ kmalloc(stream->urb_size, gfp_flags | __GFP_NOWARN);
+#endif
+
if (!stream->urb_buffer[i]) {
uvc_free_urb_buffers(stream);
break;
@@ -908,10 +918,14 @@ static int uvc_init_video_isoc(struct uvc_streaming
*stream,
urb->context = stream;
urb->pipe = usb_rcvisocpipe(stream->dev->udev,
ep->desc.bEndpointAddress);
+#ifndef CONFIG_DMA_NONCOHERENT
urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP;
+ urb->transfer_dma = stream->urb_dma[i];
+#else
+ urb->transfer_flags = URB_ISO_ASAP;
+#endif
urb->interval = ep->desc.bInterval;
urb->transfer_buffer = stream->urb_buffer[i];
- urb->transfer_dma = stream->urb_dma[i];
urb->complete = uvc_video_complete;
urb->number_of_packets = npackets;
urb->transfer_buffer_length = size;
@@ -969,8 +983,10 @@ static int uvc_init_video_bulk(struct uvc_streaming
*stream,
usb_fill_bulk_urb(urb, stream->dev->udev, pipe,
stream->urb_buffer[i], size, uvc_video_complete,
stream);
+#ifndef CONFIG_DMA_NONCOHERENT
urb->transfer_flags = URB_NO_TRANSFER_DMA_MAP;
urb->transfer_dma = stream->urb_dma[i];
+#endif
stream->urb[i] = urb;
}
--
1.7.3.2
--
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: [PATCHv2] adp1653: make ->power() method optional
On Thu, 2011-08-18 at 14:51 +0300, Sakari Ailus wrote: > On Thu, Aug 18, 2011 at 02:32:02PM +0300, Andy Shevchenko wrote: > > On Thu, 2011-08-18 at 14:22 +0300, Andy Shevchenko wrote: > > > The ->power() could be absent or not used on some platforms. This patch > > > makes > > > its presence optional. > > > > > > Signed-off-by: Andy Shevchenko > > > Cc: Sakari Ailus > > > --- > > > drivers/media/video/adp1653.c |5 + > > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > > > diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c > > > index 0fd9579..f830313 100644 > > > --- a/drivers/media/video/adp1653.c > > > +++ b/drivers/media/video/adp1653.c > > > @@ -329,6 +329,11 @@ adp1653_set_power(struct v4l2_subdev *subdev, int on) > > > struct adp1653_flash *flash = to_adp1653_flash(subdev); > > > int ret = 0; > > > > > > + /* There is no need to switch power in case of absence ->power() > > > + * method. */ > > > + if (flash->platform_data->power == NULL) > > > + return 0; > > > + > > > mutex_lock(&flash->power_lock); > > > > > > /* If the power count is modified from 0 to != 0 or from != 0 to 0, > > > > He-h, I guess you are not going to apply this one. > > The patch breaks init logic of the device. If we have no ->power(), we > > still need to bring the device to the known state. I have no good idea > > how to do this. > > I don't think it breaks anything actually. Albeit in practice one is still > likely to put the adp1653 reset line to the board since that lowers its power > consumption significantly. Yeah, even in practice we might see various ways of a chip connection. > Instead of being in power-up state after opening the flash subdev, it will > reach this state already when the system is powered up. At subdev open all > the relevant registers are written to anyway, so I don't see an issue here. You mean at first writing to the V4L2 value, do you? Because ->open() uses set_power() which will be skipped in case of no ->power method defined. > I think either this one, or one should check in probe() that the power() > callback is non-NULL. > The board code is going away in the near future so this callback will > disappear eventually anyway. So, it's up to you to include or not my last patch. > The gpio code in the board file should likely > be moved to the driver itself. The line could be different, the hw could be used in environment w/o gpio, but with (for example) external gate, and so on. I think current generic driver is pretty okay. And what to do with limits? Pass them as the module parameters? > That assumes that there will be a gpio which > can be used to enable and disable the device and I'm not fully certain this > is generic enough. Hopefully it is, but I don't know where else the adp1653 > would be used than on the N900. Don't narrow a chip application to the one device. -- Andy Shevchenko Intel Finland Oy -- 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: Record DVB-T from command line
2011/8/18 shacky : > Hi. > > I need to record from DVB-T using the command line. > I'm looking for some commands to make that saving the recording to a .ts file. > Could you help me please? > > Thank you very much! > Bye. > -- > 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 > You can try this: szap -a 0 -c channels_astra.conf -r "TV3 CAT" cat /dev/dvb/adapter0/dvr0 > testvideo.mpg mplayer testvideo.mpg This is for DVB-S, you can change it with tzap, first you need to scan your local channels. Regards. -- Josu Lazkano -- 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: [PATCHv2] adp1653: make ->power() method optional
On Thu, Aug 18, 2011 at 04:32:21PM +0300, Andy Shevchenko wrote: > On Thu, 2011-08-18 at 14:51 +0300, Sakari Ailus wrote: > > On Thu, Aug 18, 2011 at 02:32:02PM +0300, Andy Shevchenko wrote: > > > On Thu, 2011-08-18 at 14:22 +0300, Andy Shevchenko wrote: > > > > The ->power() could be absent or not used on some platforms. This patch > > > > makes > > > > its presence optional. > > > > > > > > Signed-off-by: Andy Shevchenko > > > > Cc: Sakari Ailus > > > > --- > > > > drivers/media/video/adp1653.c |5 + > > > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > > > > > diff --git a/drivers/media/video/adp1653.c > > > > b/drivers/media/video/adp1653.c > > > > index 0fd9579..f830313 100644 > > > > --- a/drivers/media/video/adp1653.c > > > > +++ b/drivers/media/video/adp1653.c > > > > @@ -329,6 +329,11 @@ adp1653_set_power(struct v4l2_subdev *subdev, int > > > > on) > > > > struct adp1653_flash *flash = to_adp1653_flash(subdev); > > > > int ret = 0; > > > > > > > > + /* There is no need to switch power in case of absence ->power() > > > > +* method. */ > > > > + if (flash->platform_data->power == NULL) > > > > + return 0; > > > > + > > > > mutex_lock(&flash->power_lock); > > > > > > > > /* If the power count is modified from 0 to != 0 or from != 0 > > > > to 0, > > > > > > He-h, I guess you are not going to apply this one. > > > The patch breaks init logic of the device. If we have no ->power(), we > > > still need to bring the device to the known state. I have no good idea > > > how to do this. > > > > I don't think it breaks anything actually. Albeit in practice one is still > > likely to put the adp1653 reset line to the board since that lowers its > > power > > consumption significantly. > Yeah, even in practice we might see various ways of a chip connection. > > > Instead of being in power-up state after opening the flash subdev, it will > > reach this state already when the system is powered up. At subdev open all > > the relevant registers are written to anyway, so I don't see an issue here. > You mean at first writing to the V4L2 value, do you? Because ->open() > uses set_power() which will be skipped in case of no ->power method > defined. Oh. I missed that adp1653_init_device() is being called in __adp1653_set_power(). That's quite important. It still needs to be called, at least once between probe() and first open(). I'm beginning to think we should require power() callback and fail in probe() if it doesn't exist, or directly make it a gpio. My plan is to get the N900 board code with the rest of the subdev drivers to mainline at some point but that will likely take quite a bit of calendar time, unfortunately. The adp1653 driver and the flash interface was just a first part of it. > > I think either this one, or one should check in probe() that the power() > > callback is non-NULL. > > The board code is going away in the near future so this callback will > > disappear eventually anyway. > So, it's up to you to include or not my last patch. > > > The gpio code in the board file should likely > > be moved to the driver itself. > The line could be different, the hw could be used in environment w/o > gpio, but with (for example) external gate, and so on. I think current > generic driver is pretty okay. > > And what to do with limits? Pass them as the module parameters? Limits still do belong to platform data. They are board dependent. Bad limits may cause the all-important smoke to escape from the board. :-) > > That assumes that there will be a gpio which > > can be used to enable and disable the device and I'm not fully certain this > > is generic enough. Hopefully it is, but I don't know where else the adp1653 > > would be used than on the N900. > Don't narrow a chip application to the one device. It's not useful to generalise this until it is useful for something. I assume the chip will be connected to a gpio in practice. If there are exceptions then we need to think again what to do. Board code which contains functions is a problem in general since it cannot be meaningfully converted to the device tree which can only contain static data. This is an issue for complex systems such as digital cameras in general, I hope a means to have board code will be left even after the device tree conversion. Regrads, -- Sakari Ailus sakari.ai...@iki.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: [PATCHv2] adp1653: make ->power() method optional
On Thu, Aug 18, 2011 at 04:55:56PM +0300, Sakari Ailus wrote: [clip] > I'm beginning to think we should require power() callback and fail in > probe() if it doesn't exist, or directly make it a gpio. > > My plan is to get the N900 board code with the rest of the subdev drivers to > mainline at some point but that will likely take quite a bit of calendar > time, unfortunately. The adp1653 driver and the flash interface was just a > first part of it. FYI: the code is available here: git://gitorious.org/omap3camera/mainline.git rx51-005/002-devel -- Sakari Ailus sakari.ai...@iki.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: Record DVB-T from command line
> szap -a 0 -c channels_astra.conf -r "TV3 CAT" > cat /dev/dvb/adapter0/dvr0 > testvideo.mpg > mplayer testvideo.mpg I tried that, but cat tells me that the device is busy: root@werecit1:/opt/utils/tv# tzap -c /etc/channels.conf -r "Rai 1" using '/dev/dvb/adapter0/frontend0' and '/dev/dvb/adapter0/demux0' reading channels from file '/etc/channels.conf' tuning to 17750 Hz video pid 0x0200, audio pid 0x028a status 00 | signal 1b1b | snr 000c | ber | unc | status 1f | signal fefe | snr 00f6 | ber 00bd | unc 0282 | FE_HAS_LOCK [last line repeated several times] In another console: root@werecit1:~# cat /dev/dvb/adapter0/dvr0 > prova.mpg cat: /dev/dvb/adapter0/dvr0: Dispositivo o risorsa occupata Could you help me please? -- 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: Record DVB-T from command line
Em 18-08-2011 08:47, shacky escreveu: >> szap -a 0 -c channels_astra.conf -r "TV3 CAT" >> cat /dev/dvb/adapter0/dvr0 > testvideo.mpg >> mplayer testvideo.mpg > > I tried that, but cat tells me that the device is busy: > > root@werecit1:/opt/utils/tv# tzap -c /etc/channels.conf -r "Rai 1" > using '/dev/dvb/adapter0/frontend0' and '/dev/dvb/adapter0/demux0' > reading channels from file '/etc/channels.conf' > tuning to 17750 Hz > video pid 0x0200, audio pid 0x028a > status 00 | signal 1b1b | snr 000c | ber | unc | > status 1f | signal fefe | snr 00f6 | ber 00bd | unc 0282 | FE_HAS_LOCK > [last line repeated several times] > > In another console: > > root@werecit1:~# cat /dev/dvb/adapter0/dvr0 > prova.mpg > cat: /dev/dvb/adapter0/dvr0: Dispositivo o risorsa occupata > > Could you help me please? You can use gnometv for that. It is also part of dvb-utils package. > -- > 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: Record DVB-T from command line
Em 18-08-2011 09:09, Mauro Carvalho Chehab escreveu: > Em 18-08-2011 08:47, shacky escreveu: >>> szap -a 0 -c channels_astra.conf -r "TV3 CAT" >>> cat /dev/dvb/adapter0/dvr0 > testvideo.mpg >>> mplayer testvideo.mpg >> >> I tried that, but cat tells me that the device is busy: >> >> root@werecit1:/opt/utils/tv# tzap -c /etc/channels.conf -r "Rai 1" >> using '/dev/dvb/adapter0/frontend0' and '/dev/dvb/adapter0/demux0' >> reading channels from file '/etc/channels.conf' >> tuning to 17750 Hz >> video pid 0x0200, audio pid 0x028a >> status 00 | signal 1b1b | snr 000c | ber | unc | >> status 1f | signal fefe | snr 00f6 | ber 00bd | unc 0282 | >> FE_HAS_LOCK >> [last line repeated several times] >> >> In another console: >> >> root@werecit1:~# cat /dev/dvb/adapter0/dvr0 > prova.mpg >> cat: /dev/dvb/adapter0/dvr0: Dispositivo o risorsa occupata >> >> Could you help me please? > > You can use gnometv for that. It is also part of dvb-utils package. Sorry... the name of the application is: gnutv You'll use it with something like: $ gnutv -out file test.mpeg -channels channel.conf mychannel PS.: I'm fighting with a gnome application here that is doing something bad when called fom xfce... so, "gnome" come to my mind, instead of "gnu" ;) >> -- >> 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: Record DVB-T from command line
> You'll use it with something like: > $ gnutv -out file test.mpeg -channels channel.conf mychannel Thank you very very much!!! It works!! :-) -- 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: [PATCHv2] adp1653: make ->power() method optional
Hi, On 08/18/2011 03:32 PM, Andy Shevchenko wrote: > On Thu, 2011-08-18 at 14:51 +0300, Sakari Ailus wrote: >> On Thu, Aug 18, 2011 at 02:32:02PM +0300, Andy Shevchenko wrote: >>> On Thu, 2011-08-18 at 14:22 +0300, Andy Shevchenko wrote: The ->power() could be absent or not used on some platforms. This patch makes its presence optional. Signed-off-by: Andy Shevchenko Cc: Sakari Ailus --- drivers/media/video/adp1653.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c index 0fd9579..f830313 100644 --- a/drivers/media/video/adp1653.c +++ b/drivers/media/video/adp1653.c @@ -329,6 +329,11 @@ adp1653_set_power(struct v4l2_subdev *subdev, int on) struct adp1653_flash *flash = to_adp1653_flash(subdev); int ret = 0; + /* There is no need to switch power in case of absence ->power() + * method. */ + if (flash->platform_data->power == NULL) + return 0; + mutex_lock(&flash->power_lock); /* If the power count is modified from 0 to != 0 or from != 0 to 0, >>> >>> He-h, I guess you are not going to apply this one. >>> The patch breaks init logic of the device. If we have no ->power(), we >>> still need to bring the device to the known state. I have no good idea >>> how to do this. >> >> I don't think it breaks anything actually. Albeit in practice one is still >> likely to put the adp1653 reset line to the board since that lowers its power >> consumption significantly. > Yeah, even in practice we might see various ways of a chip connection. > >> Instead of being in power-up state after opening the flash subdev, it will >> reach this state already when the system is powered up. At subdev open all >> the relevant registers are written to anyway, so I don't see an issue here. > You mean at first writing to the V4L2 value, do you? Because ->open() > uses set_power() which will be skipped in case of no ->power method > defined. > >> I think either this one, or one should check in probe() that the power() >> callback is non-NULL. >> The board code is going away in the near future so this callback will >> disappear eventually anyway. > So, it's up to you to include or not my last patch. > >> The gpio code in the board file should likely >> be moved to the driver itself. > The line could be different, the hw could be used in environment w/o > gpio, but with (for example) external gate, and so on. I think current > generic driver is pretty okay. Would it make sense to use the regulator API in place of the platform_data callback? If there is only one GPIO then it's easy to create a 'fixed voltage regulator' for this. Does the 'platform_data->power' callback control power supply on pin 14 (VDD) or does it do something else? Also, what do you mean by an external gate? > > And what to do with limits? Pass them as the module parameters? > >> That assumes that there will be a gpio which >> can be used to enable and disable the device and I'm not fully certain this >> is generic enough. Hopefully it is, but I don't know where else the adp1653 >> would be used than on the N900. > Don't narrow a chip application to the one device. We don't want this, but on the other hand there is a need to replace custom callbacks in driver's platform_data with something else. -- Regards, 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
dvb-apps: update hr-All
Hi, Attached updated patch for Croatia. Most of the updates are for MUX D as it's in phase of implementation. Removed MUX C (experimental HD mux) as it's no longer emitted. Best regards, Samuel hr-All.patch Description: Binary data
[PATCH] Latest version of em28xx / em28xx-dvb patch for PCTV 290e
Hi,
Here's my latest patch for the em28xx / em28xx-dvb modules, which addresses the
following problems:
a) Locking problem when unplugging and then replugging USB adapter.
b) Race conditions between adding/removing devices from the devlist, while
simultaneously loading/unloading extension modules.
c) Resource leaks on error paths.
d) Preventing the DVB framework from trying to put the adapter into sleep mode
when the adapter has been physically unplugged. (This results in occasional
"-19" errors from I2C functions when disconnecting.)
e) Use atomic bit operations to manage "device in use" slots, and enforce upper
limit of EM28XX_MAXBOARDS slots.
BTW, was there a reason why the em28xx-dvb module doesn't use dvb-usb?
Any review comments welcome,
Cheers,
Chris
--- linux-3.0/drivers/media/dvb/frontends/cxd2820r_core.c.orig 2011-08-18 16:56:02.0 +0100
+++ linux-3.0/drivers/media/dvb/frontends/cxd2820r_core.c 2011-08-18 16:56:08.0 +0100
@@ -778,7 +778,7 @@
}
EXPORT_SYMBOL(cxd2820r_get_tuner_i2c_adapter);
-static struct dvb_frontend_ops cxd2820r_ops[2];
+static const struct dvb_frontend_ops cxd2820r_ops[2];
struct dvb_frontend *cxd2820r_attach(const struct cxd2820r_config *cfg,
struct i2c_adapter *i2c, struct dvb_frontend *fe)
@@ -844,7 +844,7 @@
}
EXPORT_SYMBOL(cxd2820r_attach);
-static struct dvb_frontend_ops cxd2820r_ops[2] = {
+static const struct dvb_frontend_ops cxd2820r_ops[2] = {
{
/* DVB-T/T2 */
.info = {
--- linux-3.0/drivers/media/common/tuners/tda18271-fe.c.orig 2011-08-18 16:55:53.0 +0100
+++ linux-3.0/drivers/media/common/tuners/tda18271-fe.c 2011-08-18 16:56:08.0 +0100
@@ -1230,7 +1230,7 @@
return 0;
}
-static struct dvb_tuner_ops tda18271_tuner_ops = {
+static const struct dvb_tuner_ops tda18271_tuner_ops = {
.info = {
.name = "NXP TDA18271HD",
.frequency_min = 4500,
--- linux-3.0/drivers/media/video/em28xx/em28xx-video.c.orig 2011-08-18 17:20:10.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-video.c 2011-08-18 17:20:33.0 +0100
@@ -2202,6 +2202,7 @@
free the remaining resources */
if (dev->state & DEV_DISCONNECTED) {
em28xx_release_resources(dev);
+ kfree(dev->alt_max_pkt_size);
kfree(dev);
return 0;
}
--- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig 2011-08-17 08:52:19.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c 2011-08-18 16:56:08.0 +0100
@@ -60,7 +60,7 @@
module_param_array(card, int, NULL, 0444);
MODULE_PARM_DESC(card, "card type");
-/* Bitmask marking allocated devices from 0 to EM28XX_MAXBOARDS */
+/* Bitmask marking allocated devices from 0 to EM28XX_MAXBOARDS - 1 */
static unsigned long em28xx_devused;
struct em28xx_hash_table {
@@ -2738,9 +2738,9 @@
#endif /* CONFIG_MODULES */
/*
- * em28xx_realease_resources()
+ * em28xx_release_resources()
* unregisters the v4l2,i2c and usb devices
- * called when the device gets disconected or at module unload
+ * called when the device gets disconnected or at module unload
*/
void em28xx_release_resources(struct em28xx *dev)
{
@@ -2754,8 +2754,6 @@
em28xx_release_analog_resources(dev);
- em28xx_remove_from_devlist(dev);
-
em28xx_i2c_unregister(dev);
v4l2_device_unregister(&dev->v4l2_dev);
@@ -2763,7 +2761,7 @@
usb_put_dev(dev->udev);
/* Mark device as unused */
- em28xx_devused &= ~(1 << dev->devno);
+ clear_bit(dev->devno, &em28xx_devused);
};
/*
@@ -2776,7 +2774,6 @@
{
struct em28xx *dev = *devhandle;
int retval;
- int errCode;
dev->udev = udev;
mutex_init(&dev->ctrl_urb_lock);
@@ -2872,12 +2869,11 @@
}
/* register i2c bus */
- errCode = em28xx_i2c_register(dev);
- if (errCode < 0) {
- v4l2_device_unregister(&dev->v4l2_dev);
+ retval = em28xx_i2c_register(dev);
+ if (retval < 0) {
em28xx_errdev("%s: em28xx_i2c_register - errCode [%d]!\n",
- __func__, errCode);
- return errCode;
+ __func__, retval);
+ goto fail_reg_i2c;
}
/*
@@ -2891,11 +2887,11 @@
em28xx_card_setup(dev);
/* Configure audio */
- errCode = em28xx_audio_setup(dev);
- if (errCode < 0) {
- v4l2_device_unregister(&dev->v4l2_dev);
+ retval = em28xx_audio_setup(dev);
+ if (retval < 0) {
em28xx_errdev("%s: Error while setting audio - errCode [%d]!\n",
- __func__, errCode);
+ __func__, retval);
+ goto fail_setup_audio;
}
/* wake i2c devices */
@@ -2909,31 +2905,28 @@
if (dev->board.has_msp34xx) {
/* Send a reset to other chips via gpio */
- errCode = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xf7);
- if (errCode < 0) {
- em28xx_errdev("%s: em28xx_write_regs_req - "
+ retval = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xf7);
+ if (retval < 0) {
+ em28xx_errdev("%s: em28xx_write_reg - "
"msp34xx(1) failed! errCode [%d]\n",
- __func__, errCode);
- return errCode;
+ __func__, retval);
+ goto fail_write_reg;
}
msleep(3);
- errCode = em28xx_write_reg(dev, EM28XX_R08_GPI
Re: [PATCH] Latest version of em28xx / em28xx-dvb patch for PCTV 290e
On Thu, Aug 18, 2011 at 1:52 PM, Chris Rankin wrote: > Hi, > > Here's my latest patch for the em28xx / em28xx-dvb modules, which addresses > the following problems: > > a) Locking problem when unplugging and then replugging USB adapter. > b) Race conditions between adding/removing devices from the devlist, while > simultaneously loading/unloading extension modules. > c) Resource leaks on error paths. > d) Preventing the DVB framework from trying to put the adapter into sleep > mode when the adapter has been physically unplugged. (This results in > occasional "-19" errors from I2C functions when disconnecting.) > e) Use atomic bit operations to manage "device in use" slots, and enforce > upper limit of EM28XX_MAXBOARDS slots. Hi Chris, You would be well served to break this into a patch series, as it tends to be difficult to review a whole series of changes in a single patch. You seem to be mixed in a bunch of "useless" changes alongside functional changes. For example, if you're trying to add in a missing goto inside an exception block, doing that at the same time as renaming instances of "errCode" to "retval" just creates confusion. And finally, the mutex structure used for the modules is somewhat complicated due to to the need to keep the analog side of the board locked while initializing the digital side. This code was added specifically to prevent race conditions that were seen during initialization as things like udev and dbus attempted to connect to the newly created V4L device while the em28xx-dvb module was still coming up. In other words, I don't doubt there are bugs, and I cannot say whether your fixes are appropriate without giving a hard look at the logic. But you should be aware of the thinking behind the way it was done and it would be very worthwhile if you could test with at least one hybrid product to ensure the changes you are making don't break anything (the em2874 used in the 290e is a poor test case since it doesn't have analog support). > BTW, was there a reason why the em28xx-dvb module doesn't use dvb-usb? This is largely a product of the history of the devices using the framework. The em28xx driver was originally analog only, and DVB support was added later as new chips came out which needed it. The dvb-usb driver came from dedicated DVB devices that had no analog support. In fact, even today the lack of analog support is a huge deficiency in that framework which is why we don't support the analog side of any hybrid devices that use dvb-usb. In other words, if we were reinventing this stuff today, there would probably be only a single framework shared by dvb-usb and em28xx. But at this point it's too much cost and too little benefit to go through the work to attempt to merge them. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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] Latest version of em28xx / em28xx-dvb patch for PCTV 290e
On Thu, Aug 18, 2011 at 2:43 PM, Devin Heitmueller wrote: > On Thu, Aug 18, 2011 at 1:52 PM, Chris Rankin wrote: >> Hi, >> >> Here's my latest patch for the em28xx / em28xx-dvb modules, which addresses >> the following problems: >> >> a) Locking problem when unplugging and then replugging USB adapter. >> b) Race conditions between adding/removing devices from the devlist, while >> simultaneously loading/unloading extension modules. >> c) Resource leaks on error paths. >> d) Preventing the DVB framework from trying to put the adapter into sleep >> mode when the adapter has been physically unplugged. (This results in >> occasional "-19" errors from I2C functions when disconnecting.) >> e) Use atomic bit operations to manage "device in use" slots, and enforce >> upper limit of EM28XX_MAXBOARDS slots. > > Hi Chris, > > You would be well served to break this into a patch series, as it > tends to be difficult to review a whole series of changes in a single > patch. > > You seem to be mixed in a bunch of "useless" changes alongside > functional changes. For example, if you're trying to add in a missing > goto inside an exception block, doing that at the same time as > renaming instances of "errCode" to "retval" just creates confusion. > > And finally, the mutex structure used for the modules is somewhat > complicated due to to the need to keep the analog side of the board > locked while initializing the digital side. This code was added > specifically to prevent race conditions that were seen during > initialization as things like udev and dbus attempted to connect to > the newly created V4L device while the em28xx-dvb module was still > coming up. > > In other words, I don't doubt there are bugs, and I cannot say whether > your fixes are appropriate without giving a hard look at the logic. > But you should be aware of the thinking behind the way it was done and > it would be very worthwhile if you could test with at least one hybrid > product to ensure the changes you are making don't break anything (the > em2874 used in the 290e is a poor test case since it doesn't have > analog support). > >> BTW, was there a reason why the em28xx-dvb module doesn't use dvb-usb? > > This is largely a product of the history of the devices using the > framework. The em28xx driver was originally analog only, and DVB > support was added later as new chips came out which needed it. The > dvb-usb driver came from dedicated DVB devices that had no analog > support. In fact, even today the lack of analog support is a huge > deficiency in that framework which is why we don't support the analog > side of any hybrid devices that use dvb-usb. > > In other words, if we were reinventing this stuff today, there would > probably be only a single framework shared by dvb-usb and em28xx. But > at this point it's too much cost and too little benefit to go through > the work to attempt to merge them. > > Devin > > -- > Devin J. Heitmueller - Kernel Labs > http://www.kernellabs.com > Probably one more point worth making: I definitely appreciate that you've take the time to focus on these particular problems. I've been complaining about them for months but just never got around to rolling up my sleeves to debug them myself. In other words, don't interpret anything in my previous email as discouragement. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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: [PATCHv2] adp1653: make ->power() method optional
Sylwester Nawrocki wrote: Hi, Hi Sylwester, On 08/18/2011 03:32 PM, Andy Shevchenko wrote: On Thu, 2011-08-18 at 14:51 +0300, Sakari Ailus wrote: On Thu, Aug 18, 2011 at 02:32:02PM +0300, Andy Shevchenko wrote: On Thu, 2011-08-18 at 14:22 +0300, Andy Shevchenko wrote: The ->power() could be absent or not used on some platforms. This patch makes its presence optional. Signed-off-by: Andy Shevchenko Cc: Sakari Ailus --- drivers/media/video/adp1653.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c index 0fd9579..f830313 100644 --- a/drivers/media/video/adp1653.c +++ b/drivers/media/video/adp1653.c @@ -329,6 +329,11 @@ adp1653_set_power(struct v4l2_subdev *subdev, int on) struct adp1653_flash *flash = to_adp1653_flash(subdev); int ret = 0; + /* There is no need to switch power in case of absence ->power() +* method. */ + if (flash->platform_data->power == NULL) + return 0; + mutex_lock(&flash->power_lock); /* If the power count is modified from 0 to != 0 or from != 0 to 0, He-h, I guess you are not going to apply this one. The patch breaks init logic of the device. If we have no ->power(), we still need to bring the device to the known state. I have no good idea how to do this. I don't think it breaks anything actually. Albeit in practice one is still likely to put the adp1653 reset line to the board since that lowers its power consumption significantly. Yeah, even in practice we might see various ways of a chip connection. Instead of being in power-up state after opening the flash subdev, it will reach this state already when the system is powered up. At subdev open all the relevant registers are written to anyway, so I don't see an issue here. You mean at first writing to the V4L2 value, do you? Because ->open() uses set_power() which will be skipped in case of no ->power method defined. I think either this one, or one should check in probe() that the power() callback is non-NULL. The board code is going away in the near future so this callback will disappear eventually anyway. So, it's up to you to include or not my last patch. The gpio code in the board file should likely be moved to the driver itself. The line could be different, the hw could be used in environment w/o gpio, but with (for example) external gate, and so on. I think current generic driver is pretty okay. Would it make sense to use the regulator API in place of the platform_data callback? If there is only one GPIO then it's easy to create a 'fixed voltage regulator' for this. I don't know the regulator framework very well, but do you mean creating a new regulator which just controls a gpio? It would be preferrable that this wouldn't create a new driver nor add any board core. Does the 'platform_data->power' callback control power supply on pin 14 (VDD) or does it do something else? No. The chip is always powered on the N900 but pulling down (or up, I don't remember) its reset pin puts the chip to reset and causes the current draw to reach almost zero. I think it's in the class of some or few tens of µA. Someone still might implement a board containing the adp1653 which would require enabling a regulator for it. -- Sakari Ailus sakari.ai...@iki.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
[cron job] v4l-dvb daily build: WARNINGS
This message is generated daily by a cron job that builds v4l-dvb for the kernels and architectures in the list below. Results of the daily build of v4l-dvb: date:Thu Aug 18 19:00:33 CEST 2011 git hash:9bed77ee2fb46b74782d0d9d14b92e9d07f3df6e gcc version: i686-linux-gcc (GCC) 4.6.1 host hardware:x86_64 host os: 2.6.32.5 linux-git-armv5: WARNINGS linux-git-armv5-davinci: WARNINGS linux-git-armv5-ixp: WARNINGS linux-git-armv5-omap2: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: OK linux-git-mips: WARNINGS linux-git-powerpc64: WARNINGS linux-git-x86_64: WARNINGS 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-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-rc1-x86_64: WARNINGS spec-git: WARNINGS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.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] Latest version of em28xx / em28xx-dvb patch for PCTV 290e
On 18/08/11 19:43, Devin Heitmueller wrote: You would be well served to break this into a patch series, as it tends to be difficult to review a whole series of changes in a single patch. You seem to be mixed in a bunch of "useless" changes alongside functional changes. For example, if you're trying to add in a missing goto inside an exception block, doing that at the same time as renaming instances of "errCode" to "retval" just creates confusion. Actually, those two particular changes go together because I'm replacing "return errCode" with a goto to "return retval". Ultimately, errCode becomes an unused variable. And finally, the mutex structure used for the modules is somewhat complicated due to to the need to keep the analog side of the board locked while initializing the digital side. This code was added specifically to prevent race conditions that were seen during initialization as things like udev and dbus attempted to connect to the newly created V4L device while the em28xx-dvb module was still coming up. OK, thanks. I've been tackling this problem from the "We must always take lock A before lock B, and never vice versa" point of view. So the order is: - take device mutex - enter em28xx_init_dev() - enter em28xx_init_extension() - take device list mutex - call init() function for every extension with this device Since dvb_init() is the init() function for the em28xx-dvb extension, it follows that it cannot take the device's mutex again. The problem is therefore moved to em28xx_dvb_register(), which takes the device list mutex and yet MUST not take the mutex for any device in the list. Combining em28xx_add_into_devlist() with em28xx_init_extension() (and similarly em28xx_remove_from_devlist() with em28xx_close_extension()) means that the device list must always contain a list of devices that has been initialised against every extension in the extension list. I can probably factor out the simpler patches first, such as using the bit operations on em28xx_devused, and the memory leak in em28xx_v4l2_close(). And the spelling fixes... Cheers, Chris -- 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: Negotiating frame buffer size between sensor subdevs and bridge devices
On Wed, Aug 17, 2011 at 10:22:26PM +0200, Sylwester Nawrocki wrote: > On 08/17/2011 12:25 AM, Sakari Ailus wrote: > > On Thu, Jul 28, 2011 at 07:04:11PM +0200, Sylwester Nawrocki wrote: > >> Hello, > > > > Hi Sylwester, > > Hi Sakari, > > kiitos commentti ;) Hi Sylwester! Ole hyvä! :) > > > >> Trying to capture images in JPEG format with regular "image sensor -> > >> mipi-csi receiver -> host interface" H/W configuration I've found there > >> is no standard way to communicate between the sensor subdev and the host > >> driver what is exactly a required maximum buffer size to capture a frame. > >> > >> For the raw formats there is no issue as the buffer size can be easily > >> determined from the pixel format and resolution (or sizeimage set on > >> a video node). > >> However compressed data formats are a bit more complicated, the required > >> memory buffer size depends on multiple factors, like compression ratio, > >> exact file header structure etc. > >> > >> Often it is at the sensor driver where all information required to > >> determine size of the allocated memory is present. Bridge/host devices > >> just do plain DMA without caring much what is transferred. I know of > >> hardware which, for some pixel formats, once data capture is started, > >> writes to memory whatever amount of data the sensor is transmitting, > >> without any means to interrupt on the host side. So it is critical > >> to assure the buffer allocation is done right, according to the sensor > >> requirements, to avoid buffer overrun. > >> > >> > >> Here is a link to somehow related discussion I could find: > >> [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg27138.html > >> > >> > >> In order to let the host drivers query or configure subdevs with required > >> frame buffer size one of the following changes could be done at V4L2 API: > >> > >> 1. Add a 'sizeimage' field in struct v4l2_mbus_framefmt and make subdev > >> drivers optionally set/adjust it when setting or getting the format with > >> set_fmt/get_fmt pad level ops (and s/g_mbus_fmt ?) > >> There could be two situations: > >> - effective required frame buffer size is specified by the sensor and the > >> host driver relies on that value when allocating a buffer; > >> - the host driver forces some arbitrary buffer size and the sensor > >> performs > >> any required action to limit transmitted amount of data to that amount > >> of data; > >> Both cases could be covered similarly as it's done with VIDIOC_S_FMT. > >> > >> Introducing 'sizeimage' field is making the media bus format struct looking > >> more similar to struct v4l2_pix_format and not quite in line with media bus > >> format meaning, i.e. describing data on a physical bus, not in the memory. > >> The other option I can think of is to create separate subdev video ops. > >> 2. Add new s/g_sizeimage subdev video operations > >> > >> The best would be to make this an optional callback, not sure if it makes > >> sense > >> though. It has an advantage of not polluting the user space API. Although > >> 'sizeimage' in user space might be useful for some purposes I rather tried > >> to > >> focus on "in-kernel" calls. > > > > I prefer this second approach over the first once since the maxiumu size of > > the image in bytes really isn't a property of the bus. > > After thinking some more about it I came to similar conclusion. I intended to > find some better name for s/g_sizeimage callbacks and post relevant patch > for consideration. > Although I haven't yet found some time to carry on with this. That sounds a possible solution to me as well. The upside would be that v4l2_mbus_framefmt would be left to describe relatively low level bus and format properties. That said, I'm not anymore quite certain it should not be part of that structure. Is the size always the same, or is this maximum? > > How about a regular V4L2 control? You would also have minimum and maximum > > values, I'm not quite sure whather this is a plus, though. :) > > My intention was to have these calls used only internally in the kernel and > do not allow the userspace to mess with it. All in all, if anything had > interfered and the host driver would have allocated too small buffer the > system > would crash miserably due to buffer overrun. The user space wouldn't be allowed to do anything like that. E.g. the control would become read-only during streaming and the bridge driver would need to check its value against the sizes of the video buffers. Although this might not be relevant at all if there are no direct ways to affect the maximum size of the resulting image. > The final buffer size for a JFIF/EXIF file will depend on other factors, like > main image resolution, JPEG compression ratio, the thumbnail inclusion and its > format/resolution, etc. I imagine we should be rather creating controls > for those parameters. > > Also the driver would most likely have to validate the buffer size duri
Re: [PATCHv2] adp1653: make ->power() method optional
On 08/18/2011 09:02 PM, Sakari Ailus wrote: > > He-h, I guess you are not going to apply this one. > The patch breaks init logic of the device. If we have no ->power(), we > still need to bring the device to the known state. I have no good idea > how to do this. I don't think it breaks anything actually. Albeit in practice one is still likely to put the adp1653 reset line to the board since that lowers its power consumption significantly. >>> Yeah, even in practice we might see various ways of a chip connection. >>> Instead of being in power-up state after opening the flash subdev, it will reach this state already when the system is powered up. At subdev open all the relevant registers are written to anyway, so I don't see an issue here. >>> You mean at first writing to the V4L2 value, do you? Because ->open() >>> uses set_power() which will be skipped in case of no ->power method >>> defined. >>> I think either this one, or one should check in probe() that the power() callback is non-NULL. The board code is going away in the near future so this callback will disappear eventually anyway. >>> So, it's up to you to include or not my last patch. >>> The gpio code in the board file should likely be moved to the driver itself. >>> The line could be different, the hw could be used in environment w/o >>> gpio, but with (for example) external gate, and so on. I think current >>> generic driver is pretty okay. >> >> Would it make sense to use the regulator API in place of the platform_data >> callback? If there is only one GPIO then it's easy to create a 'fixed voltage >> regulator' for this. > > I don't know the regulator framework very well, but do you mean creating a new > regulator which just controls a gpio? It would be preferable that this > wouldn't > create a new driver nor add any board core. I'm afraid your requirements are too demanding :) Yes, I meant creating a new regulator. In case the ADP1635 voltage regulator is inhibited through a GPIO at a host processor such regulator would in fact be only flipping a GPIO (and its driver would request the GPIO and set it into a default inactive state during its initialization). But the LDO for ADP1635 could be also a part of larger PMIC device, which are often configured through I2C and have their generic drivers (in drivers/regulator). So the regulator API in some extent abstracts the power supply details. There is a common API at the client driver side regardless of the details how the power is actually enabled/ disabled. I've seen some patches for the device tree bindings for the regulator API but I guess this is not in the mainline yet. Currently the 'fixed voltage regulator' is instantiated by creating a platform device, with an appropriate 'id', in the board code. And you have to create... a platform data for it :) The driver is common for all such devices (drivers/regulator/fixed.c). > >> Does the 'platform_data->power' callback control power supply on pin 14 (VDD) >> or does it do something else? > > No. The chip is always powered on the N900 but pulling down (or up, I don't > remember) > its reset pin puts the chip to reset and causes the current draw to reach > almost zero. > I think it's in the class of some or few tens of µA. Someone still might > implement According to the datasheet it's even less, below 1 uA :) "Shutdown Current (EN = GND, TJ = −40°C to +85°C) = 0.1μA (Typ.), 1μA (Max)" So the reset (EN) pin is basically a GPIO, but it could be as well some signal provided by a glue FPGA, driven by a memory mapped register(s). Then the callback is most flexible, but it's a little bit ugly at the same time:) If I were you I, would probably originally put a GPIO number in platform_data, instead of a function callback. But that depends on priorities. > a board containing the adp1653 which would require enabling a regulator for > it. Indeed, I guess there is no point in adding support for the power supply control over the regulator API now. When someone needs it on some other board, it can be added in the driver then. -- Regards, 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] Latest version of em28xx / em28xx-dvb patch for PCTV 290e
On 18/08/11 19:44, Devin Heitmueller wrote:
You would be well served to break this into a patch series, as it tends to be
difficult to review a whole series of changes in a single patch.
Here are the first three patches:
- release the alt_max_pkt_size buffer.
- use atomic bit operations for em28xx_devused.
- use the correct amount of memory for snprintf().
Cheers,
Chris
Signed-of-by: Chris Rankin
--- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig 2011-08-18 22:24:12.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c 2011-08-18 22:22:14.0 +0100
@@ -60,7 +60,7 @@
module_param_array(card, int, NULL, 0444);
MODULE_PARM_DESC(card, "card type");
-/* Bitmask marking allocated devices from 0 to EM28XX_MAXBOARDS */
+/* Bitmask marking allocated devices from 0 to EM28XX_MAXBOARDS - 1 */
static unsigned long em28xx_devused;
struct em28xx_hash_table {
@@ -2763,7 +2763,7 @@
usb_put_dev(dev->udev);
/* Mark device as unused */
- em28xx_devused &= ~(1 << dev->devno);
+ clear_bit(dev->devno, &em28xx_devused);
};
/*
@@ -2967,8 +2967,16 @@
ifnum = interface->altsetting[0].desc.bInterfaceNumber;
/* Check to see next free device and mark as used */
- nr = find_first_zero_bit(&em28xx_devused, EM28XX_MAXBOARDS);
- em28xx_devused |= 1<= EM28XX_MAXBOARDS) {
+ /* No free device slots */
+ printk(DRIVER_NAME ": Supports only %i em28xx boards.\n",
+ EM28XX_MAXBOARDS);
+ retval = -ENOMEM;
+ goto err_no_slot;
+ }
+ } while (test_and_set_bit(nr, &em28xx_devused));
/* Don't register audio interfaces */
if (interface->altsetting[0].desc.bInterfaceClass == USB_CLASS_AUDIO) {
@@ -2979,7 +2987,6 @@
ifnum,
interface->altsetting[0].desc.bInterfaceClass);
- em28xx_devused &= ~(1<= EM28XX_MAXBOARDS) {
- printk(DRIVER_NAME ": Supports only %i em28xx boards.\n",
-EM28XX_MAXBOARDS);
- em28xx_devused &= ~(1alt_max_pkt_size);
mutex_unlock(&dev->lock);
kfree(dev);
@@ -3147,6 +3141,10 @@
return 0;
err:
+ clear_bit(nr, &em28xx_devused);
+
+err_no_slot:
+ usb_put_dev(udev);
return retval;
}
--- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig 2011-08-17 08:52:19.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c 2011-08-18 22:03:05.0 +0100
@@ -3085,7 +3085,7 @@
goto err;
}
- snprintf(dev->name, 29, "em28xx #%d", nr);
+ snprintf(dev->name, sizeof(dev->name), "em28xx #%d", nr);
dev->devno = nr;
dev->model = id->driver_info;
dev->alt = -1;
--- linux-3.0/drivers/media/video/em28xx/em28xx-video.c.orig 2011-08-18 17:20:10.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-video.c 2011-08-18 17:20:33.0 +0100
@@ -2202,6 +2202,7 @@
free the remaining resources */
if (dev->state & DEV_DISCONNECTED) {
em28xx_release_resources(dev);
+ kfree(dev->alt_max_pkt_size);
kfree(dev);
return 0;
}
--- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig 2011-08-17 08:52:19.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c 2011-08-18 22:09:32.0 +0100
@@ -3128,6 +3128,7 @@
retval = em28xx_init_dev(&dev, udev, interface, nr);
if (retval) {
em28xx_devused &= ~(1alt_max_pkt_size);
mutex_unlock(&dev->lock);
kfree(dev);
goto err;
Re: [PATCH] Latest version of em28xx / em28xx-dvb patch for PCTV 290e
Two more patches:
a) clean up resources more reliably if em28xx_init_dev() fails,
b) move two printk() statements outside the mutex lock
Cheers,
Chris
Signed-off-by: Chris Rankin
--- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig 2011-08-18 22:42:03.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c 2011-08-18 22:55:52.0 +0100
@@ -2776,7 +2776,6 @@
{
struct em28xx *dev = *devhandle;
int retval;
- int errCode;
dev->udev = udev;
mutex_init(&dev->ctrl_urb_lock);
@@ -2858,7 +2857,7 @@
/* Resets I2C speed */
em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, dev->board.i2c_speed);
if (retval < 0) {
- em28xx_errdev("%s: em28xx_write_regs_req failed!"
+ em28xx_errdev("%s: em28xx_write_reg failed!"
" retval [%d]\n",
__func__, retval);
return retval;
@@ -2872,12 +2871,11 @@
}
/* register i2c bus */
- errCode = em28xx_i2c_register(dev);
- if (errCode < 0) {
- v4l2_device_unregister(&dev->v4l2_dev);
+ retval = em28xx_i2c_register(dev);
+ if (retval < 0) {
em28xx_errdev("%s: em28xx_i2c_register - errCode [%d]!\n",
- __func__, errCode);
- return errCode;
+ __func__, retval);
+ goto fail_reg_i2c;
}
/*
@@ -2891,11 +2889,11 @@
em28xx_card_setup(dev);
/* Configure audio */
- errCode = em28xx_audio_setup(dev);
- if (errCode < 0) {
- v4l2_device_unregister(&dev->v4l2_dev);
+ retval = em28xx_audio_setup(dev);
+ if (retval < 0) {
em28xx_errdev("%s: Error while setting audio - errCode [%d]!\n",
- __func__, errCode);
+ __func__, retval);
+ goto fail_setup_audio;
}
/* wake i2c devices */
@@ -2909,31 +2907,28 @@
if (dev->board.has_msp34xx) {
/* Send a reset to other chips via gpio */
- errCode = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xf7);
- if (errCode < 0) {
- em28xx_errdev("%s: em28xx_write_regs_req - "
+ retval = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xf7);
+ if (retval < 0) {
+ em28xx_errdev("%s: em28xx_write_reg - "
"msp34xx(1) failed! errCode [%d]\n",
- __func__, errCode);
- return errCode;
+ __func__, retval);
+ goto fail_write_reg;
}
msleep(3);
- errCode = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xff);
- if (errCode < 0) {
- em28xx_errdev("%s: em28xx_write_regs_req - "
+ retval = em28xx_write_reg(dev, EM28XX_R08_GPIO, 0xff);
+ if (retval < 0) {
+ em28xx_errdev("%s: em28xx_write_reg - "
"msp34xx(2) failed! errCode [%d]\n",
- __func__, errCode);
- return errCode;
+ __func__, retval);
+ goto fail_write_reg;
}
msleep(3);
}
- em28xx_add_into_devlist(dev);
-
retval = em28xx_register_analog_devices(dev);
if (retval < 0) {
- em28xx_release_resources(dev);
- goto fail_reg_devices;
+ goto fail_reg_analog_devices;
}
em28xx_init_extension(dev);
@@ -2943,7 +2938,14 @@
return 0;
-fail_reg_devices:
+fail_setup_audio:
+fail_write_reg:
+fail_reg_analog_devices:
+ em28xx_i2c_unregister(dev);
+
+fail_reg_i2c:
+ v4l2_device_unregister(&dev->v4l2_dev);
+
return retval;
}
--- linux-3.0/drivers/media/video/em28xx/em28xx-core.c.orig 2011-08-17 08:52:25.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-core.c 2011-08-18 22:51:59.0 +0100
@@ -1171,13 +1171,6 @@
mutex_unlock(&em28xx_devlist_mutex);
};
-void em28xx_add_into_devlist(struct em28xx *dev)
-{
- mutex_lock(&em28xx_devlist_mutex);
- list_add_tail(&dev->devlist, &em28xx_devlist);
- mutex_unlock(&em28xx_devlist_mutex);
-};
-
/*
* Extension interface
*/
@@ -1215,14 +1208,13 @@
void em28xx_init_extension(struct em28xx *dev)
{
- struct em28xx_ops *ops = NULL;
+ const struct em28xx_ops *ops = NULL;
mutex_lock(&em28xx_devlist_mutex);
- if (!list_empty(&em28xx_extension_devlist)) {
- list_for_each_entry(ops, &em28xx_extension_devlist, next) {
- if (ops->init)
-ops->init(dev);
- }
+ list_add_tail(&dev->devlist, &em28xx_devlist);
+ list_for_each_entry(ops, &em28xx_extension_devlist, next) {
+ if (ops->init)
+ ops->init(dev);
}
mutex_unlock(&em28xx_devlist_mutex);
}
--- linux-3.0/drivers/media/video/em28xx/em28xx-core.c.orig 2011-08-18 23:05:50.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-core.c 2011-08-18 23:07:02.0 +0100
@@ -1186,8 +1186,8 @@
list_for_each_entry(dev, &em28xx_devlist, devlist) {
ops->init(dev);
}
- printk(KERN_INFO "Em28xx: Initialized (%s) extension\n", ops->name);
mutex_unlock(&em28xx_devlist_mutex);
+ printk(KERN_INFO "Em28xx: Initialized (%s) extension\n", ops->name);
return 0;
}
EXPORT_SYMBOL(em28xx_register_extension);
@@ -1200,9 +1200,9 @@
list_for_each_entry(dev, &em28xx_devlist, devlist) {
ops->fini(dev);
}
- printk(KERN_INFO "Em28xx: Removed (%s) extension\n", ops->name);
list_del(&ops->next);
mutex_unlock(&em28xx_devlist_mutex);
+ printk(KERN_INFO "Em28xx: Removed (%s) extension\n", ops->name);
}
EXPORT_SYMBOL(em28xx_unregister_extension);
Re: [PATCH] Latest version of em28xx / em28xx-dvb patch for PCTV 290e
Next patch: By default, the DVB framework tries to put a frontend to sleep after
it has been shut down. This obviously doesn't work for a USB device that has
been disconnected, and can result in occasional errors in dmesg about I2C writes
failing with error code -19.
The patch works by nulling out the function pointers that the DVB framework
would otherwise try to call. I have therefore declared the structs in the
tda18271 and cxd2820r modules to be "const", so that we know that they are
supposed only to be templates.
Signed-off-by: Chris Rankin
--- linux-3.0/drivers/media/common/tuners/tda18271-fe.c.orig 2011-08-18 16:55:53.0 +0100
+++ linux-3.0/drivers/media/common/tuners/tda18271-fe.c 2011-08-18 23:12:55.0 +0100
@@ -1230,7 +1230,7 @@
return 0;
}
-static struct dvb_tuner_ops tda18271_tuner_ops = {
+static const struct dvb_tuner_ops tda18271_tuner_ops = {
.info = {
.name = "NXP TDA18271HD",
.frequency_min = 4500,
--- linux-3.0/drivers/media/dvb/frontends/cxd2820r_core.c.orig 2011-08-18 16:56:02.0 +0100
+++ linux-3.0/drivers/media/dvb/frontends/cxd2820r_core.c 2011-08-18 23:14:06.0 +0100
@@ -778,7 +778,7 @@
}
EXPORT_SYMBOL(cxd2820r_get_tuner_i2c_adapter);
-static struct dvb_frontend_ops cxd2820r_ops[2];
+static const struct dvb_frontend_ops cxd2820r_ops[2];
struct dvb_frontend *cxd2820r_attach(const struct cxd2820r_config *cfg,
struct i2c_adapter *i2c, struct dvb_frontend *fe)
@@ -844,7 +844,7 @@
}
EXPORT_SYMBOL(cxd2820r_attach);
-static struct dvb_frontend_ops cxd2820r_ops[2] = {
+static const struct dvb_frontend_ops cxd2820r_ops[2] = {
{
/* DVB-T/T2 */
.info = {
--- linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c.orig 2011-08-17 08:52:30.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c 2011-08-18 23:17:42.0 +0100
@@ -720,6 +720,12 @@
goto ret;
}
+static inline void prevent_sleep(struct dvb_frontend_ops *ops) {
+ ops->set_voltage = NULL;
+ ops->sleep = NULL;
+ ops->tuner_ops.sleep = NULL;
+}
+
static int dvb_fini(struct em28xx *dev)
{
if (!dev->board.has_dvb) {
@@ -728,8 +734,17 @@
}
if (dev->dvb) {
- unregister_dvb(dev->dvb);
- kfree(dev->dvb);
+ struct em28xx_dvb *dvb = dev->dvb;
+
+ if (dev->state & DEV_DISCONNECTED) {
+ /* We cannot tell the device to sleep
+ * once it has been unplugged. */
+ prevent_sleep(&dvb->fe[0]->ops);
+ prevent_sleep(&dvb->fe[1]->ops);
+ }
+
+ unregister_dvb(dvb);
+ kfree(dvb);
dev->dvb = NULL;
}
Re: [PATCH] Latest version of em28xx / em28xx-dvb patch for PCTV 290e
This patch closes the race on the device and extension lists at USB disconnect
time. Previously, the device was removed from the device list during
em28xx_release_resources(), and then passed to the em28xx_close_extension()
function so that all extensions could run their fini() operations. However, this
left a (brief, theoretical, highly unlikely ;-)) window between these two calls
during which a new module could call em28xx_register_extension(). The result
would have been that the em28xx_usb_disconnect() function would also have passed
the device to the new extension's fini() function, despite never having called
the extension's init() function.
This patch also restores em28xx_close_extension()'s symmetry with
em28xx_init_extension(), and establishes the property that every device in the
device list must have been initialised for every extension in the extension list.
Signed-of-by: Chris Rankin
--- linux-3.0/drivers/media/video/em28xx/em28xx-cards.c.orig 2011-08-19 00:23:17.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-cards.c 2011-08-19 00:32:40.0 +0100
@@ -2738,9 +2738,9 @@
#endif /* CONFIG_MODULES */
/*
- * em28xx_realease_resources()
+ * em28xx_release_resources()
* unregisters the v4l2,i2c and usb devices
- * called when the device gets disconected or at module unload
+ * called when the device gets disconnected or at module unload
*/
void em28xx_release_resources(struct em28xx *dev)
{
@@ -2754,8 +2754,6 @@
em28xx_release_analog_resources(dev);
- em28xx_remove_from_devlist(dev);
-
em28xx_i2c_unregister(dev);
v4l2_device_unregister(&dev->v4l2_dev);
@@ -3152,7 +3150,7 @@
/*
* em28xx_usb_disconnect()
- * called when the device gets diconencted
+ * called when the device gets disconnected
* video device will be unregistered on v4l2_close in case it is still open
*/
static void em28xx_usb_disconnect(struct usb_interface *interface)
--- linux-3.0/drivers/media/video/em28xx/em28xx-core.c.orig 2011-08-18 23:07:51.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-core.c 2011-08-19 00:27:00.0 +0100
@@ -1160,18 +1160,6 @@
static DEFINE_MUTEX(em28xx_devlist_mutex);
/*
- * em28xx_realease_resources()
- * unregisters the v4l2,i2c and usb devices
- * called when the device gets disconected or at module unload
-*/
-void em28xx_remove_from_devlist(struct em28xx *dev)
-{
- mutex_lock(&em28xx_devlist_mutex);
- list_del(&dev->devlist);
- mutex_unlock(&em28xx_devlist_mutex);
-};
-
-/*
* Extension interface
*/
@@ -1221,14 +1209,13 @@
void em28xx_close_extension(struct em28xx *dev)
{
- struct em28xx_ops *ops = NULL;
+ const struct em28xx_ops *ops = NULL;
mutex_lock(&em28xx_devlist_mutex);
- if (!list_empty(&em28xx_extension_devlist)) {
- list_for_each_entry(ops, &em28xx_extension_devlist, next) {
- if (ops->fini)
-ops->fini(dev);
- }
+ list_for_each_entry(ops, &em28xx_extension_devlist, next) {
+ if (ops->fini)
+ ops->fini(dev);
}
+ list_del(&dev->devlist);
mutex_unlock(&em28xx_devlist_mutex);
}
Re: [PATCH] Latest version of em28xx / em28xx-dvb patch for PCTV 290e
The final patch removes the unplug/replug deadlock by not holding the device
mutex during dvb_init(). However, this mutex has already been locked during
device initialisation by em28xx_usb_probe() and is not released again until all
extensions have been initialised successfully.
The device mutex is not held during either em28xx_register_extension() or
em28xx_unregister_extension() any more. More importantly, I don't believe it can
safely be held by these functions because they must both - by their nature -
acquire the device list mutex before they can iterate through the device list.
In other words, while usb_probe() and usb_disconnect() acquire the device mutex
followed by the device list mutex, the register/unregister_extension() functions
would need to acquire these mutexes in the opposite order. And that sounds like
a potential deadlock.
On the other hand, the new situation is a definite improvement :-).
Signed-off-by: Chris Rankin
--- linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c.orig 2011-08-19 00:50:41.0 +0100
+++ linux-3.0/drivers/media/video/em28xx/em28xx-dvb.c 2011-08-19 00:51:03.0 +0100
@@ -542,7 +542,6 @@
dev->dvb = dvb;
dvb->fe[0] = dvb->fe[1] = NULL;
- mutex_lock(&dev->lock);
em28xx_set_mode(dev, EM28XX_DIGITAL_MODE);
/* init frontend */
switch (dev->model) {
@@ -711,7 +710,6 @@
em28xx_info("Successfully loaded em28xx-dvb\n");
ret:
em28xx_set_mode(dev, EM28XX_SUSPEND);
- mutex_unlock(&dev->lock);
return result;
out_free:
offer for Conax CAM exchange
I ordered two different Conax CAMs for Anysee development but unfortunately got just same CAMs with different brands. As a CI/CAM driver development there is no idea having two similar CAMs. Anyone willing to exchange Conax CAMs or maybe some other encryption? Those are branded as SMIT and the other is LA Digital (SMIT seems to be original brand). SMITDVB CA Module DVB_CI_V1.00 CAM Application manufacturer: cafe CAM Manufacturer code: babe 1. Software version: CNX-STD-2.5.9 2. Hardware version: 2.2.1 8. CA_SYS_ID: 0x0b00 9. CIM Build Date : Jul 28 2009 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] Latest version of em28xx / em28xx-dvb patch for PCTV 290e
Em 18-08-2011 12:56, Chris Rankin escreveu: > On 18/08/11 19:43, Devin Heitmueller wrote: >> You would be well served to break this into a patch series, as it tends to >> be difficult to review a whole series of changes in a single patch. You seem >> to be mixed in a bunch of "useless" changes alongside functional changes. >> For example, if you're trying to add in a missing goto inside an exception >> block, doing that at the same time as renaming instances of "errCode" to >> "retval" just creates confusion. > > Actually, those two particular changes go together because I'm replacing > "return errCode" with a goto to "return retval". Ultimately, errCode becomes > an unused variable. > >> And finally, the mutex structure used for the modules is somewhat >> complicated due to to the need to keep the analog side of the board locked >> while initializing the digital side. This code was added specifically to >> prevent race conditions that were seen during initialization as things like >> udev and dbus attempted to connect to the newly created V4L device while the >> em28xx-dvb module was still coming up. > > OK, thanks. I've been tackling this problem from the "We must always take > lock A before lock B, and never vice versa" point of view. So the order is: > > - take device mutex > - enter em28xx_init_dev() > - enter em28xx_init_extension() > - take device list mutex > - call init() function for every extension with this device > > Since dvb_init() is the init() function for the em28xx-dvb extension, it > follows that it cannot take the device's mutex again. The problem is > therefore moved to em28xx_dvb_register(), which takes the device list mutex > and yet MUST not take the mutex for any device in the list. > > Combining em28xx_add_into_devlist() with em28xx_init_extension() (and > similarly em28xx_remove_from_devlist() with em28xx_close_extension()) means > that the device list must always contain a list of devices that has been > initialised against every extension in the extension list. > > I can probably factor out the simpler patches first, such as using the bit > operations on em28xx_devused, and the memory leak in em28xx_v4l2_close(). And > the spelling fixes... I agree with Devin: please break it into one patch with just the mutex fixes, and another one(s) with the cleanups. The basic rule is one patch per logic change. For the mutex patch, please add: "Cc: sta...@kernel.org" at the line before your signed-off-by, as such patch should be backported to stable kernels. With respect to return errCode, maybe the better would be to rename it as just "rc". It is very common to call the return code as that on several places, and it is smaller. PS.: I didn't make a full review of your patches yet. I'll probably do it by the next week, after returning from LinuxCon. Thanks! 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: [beagleboard] Re: [PATCH v7 1/2] Add driver for Aptina (Micron) mt9p031 sensor.
Hi, I am trying to get the mt9p031 working from nand with a ubifs file system and I am having a few problems. /dev/media0 is not present unless I run: #mknod /dev/media0 c 251 0 #chown root:video /dev/media0 #media-ctl -p Enumerating entities media_open: Unable to enumerate entities for device /dev/media0 (Inappropriate ioctl for device) With the same rig/files it works fine running from EXT4 on an SD card. Any idea why this does not work on nand with ubifs? Regards, CJ On 13/06/11 22:39, javier Martin wrote: On 8 June 2011 18:24, Laurent Pinchart wrote: That works much better, thank you. -- Regards, Laurent Pinchart So, how is it going? Are you finally accepting the patches for mainline? Thank you, -- 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] Latest version of em28xx / em28xx-dvb patch for PCTV 290e
Em 18-08-2011 15:11, Chris Rankin escreveu: > Two more patches: > > a) clean up resources more reliably if em28xx_init_dev() fails, > b) move two printk() statements outside the mutex lock > > Cheers, > Chris > > Signed-off-by: Chris Rankin > Chris, Please, never send two patches at the same email. This doesn't work: patchwork and all tools used by me and other maintainers will either discard one or both patches, or even merge both into one. Also, when you're sending us a series of patches, be sure to number the patch series with PATCH 01/xx (where xx is the last patch), and put a different subject on each patch. There's no need to reply the original thread. See how all your patches will appear for me: https://patchwork.kernel.org/project/linux-media/list/?submitter=4542 It is very hard to know that you have a complese series of patches, instead of just a new version of a patch that you've already sent. By default, the last case is assumed, as it is somewhat common to reply to a patch with a new version of it (yet, the best practice would be to rename the new patch series as PATCH v2, PATCH v3, etc). If I wouldn't notice this before I would simply mark all patches except by the last one as superseded, and just get the last one. Thanks! Mauro PS.: I suggest you to take a look at the wiki page bellow. It helps people about what's the expected way for you to submit your work http://linuxtv.org/wiki/index.php/Development:_How_to_submit_patches -- 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] v4l: Add V4L2_PIX_FMT_NV24 and V4L2_PIX_FMT_NV42 formats
NV24 and NV42 are planar YCbCr 4:4:4 and YCrCb 4:4:4 formats with a
luma plane followed by an interleaved chroma plane.
Signed-off-by: Laurent Pinchart
---
Documentation/DocBook/media/v4l/pixfmt-nv24.xml | 128 +++
Documentation/DocBook/media/v4l/pixfmt.xml |1 +
include/linux/videodev2.h |2 +
3 files changed, 131 insertions(+), 0 deletions(-)
create mode 100644 Documentation/DocBook/media/v4l/pixfmt-nv24.xml
This format will be used by an fbdev driver. I'm already posting the patch for
for review and will send a pull request later.
diff --git a/Documentation/DocBook/media/v4l/pixfmt-nv24.xml
b/Documentation/DocBook/media/v4l/pixfmt-nv24.xml
new file mode 100644
index 000..e77301d
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/pixfmt-nv24.xml
@@ -0,0 +1,128 @@
+
+
+ V4L2_PIX_FMT_NV24 ('NV24'), V4L2_PIX_FMT_NV42
('NV42')
+ &manvol;
+
+
+ V4L2_PIX_FMT_NV24
+ V4L2_PIX_FMT_NV42
+ Formats with full horizontal and vertical
+chroma resolutions, also known as YUV 4:4:4. One luminance and one
+chrominance plane with alternating chroma samples as opposed to
+V4L2_PIX_FMT_YVU420
+
+
+ Description
+
+ These are two-plane versions of the YUV 4:4:4 format.
+The three components are separated into two sub-images or planes. The
+Y plane is first. The Y plane has one byte per pixel. For
+V4L2_PIX_FMT_NV24, a combined CbCr plane
+immediately follows the Y plane in memory. The CbCr plane is the same
+width and height, in pixels, as the Y plane (and of the image).
+Each line contains one CbCr pair per pixel.
+V4L2_PIX_FMT_NV42 is the same except the Cb and
+Cr bytes are swapped, the CrCb plane starts with a Cr byte.
+
+ If the Y plane has pad bytes after each row, then the
+CbCr plane has twice as many pad bytes after its rows.
+
+
+ V4L2_PIX_FMT_NV24 4 × 4
+pixel image
+
+
+ Byte Order.
+ Each cell is one byte.
+
+
+
+
+
+ start + 0:
+ Y'00
+ Y'01
+ Y'02
+ Y'03
+
+
+ start + 4:
+ Y'10
+ Y'11
+ Y'12
+ Y'13
+
+
+ start + 8:
+ Y'20
+ Y'21
+ Y'22
+ Y'23
+
+
+ start + 12:
+ Y'30
+ Y'31
+ Y'32
+ Y'33
+
+
+ start + 16:
+ Cb00
+ Cr00
+ Cb01
+ Cr01
+ Cb02
+ Cr02
+ Cb03
+ Cr03
+
+
+ start + 24:
+ Cb10
+ Cr10
+ Cb11
+ Cr11
+ Cb12
+ Cr12
+ Cb13
+ Cr13
+
+
+ start + 32:
+ Cb20
+ Cr20
+ Cb21
+ Cr21
+ Cb22
+ Cr22
+ Cb23
+ Cr23
+
+
+ start + 40:
+ Cb30
+ Cr30
+ Cb31
+ Cr31
+ Cb32
+ Cr32
+ Cb33
+ Cr33
+
+
+
+
+
+
+
+
+
+
+
diff --git a/Documentation/DocBook/media/v4l/pixfmt.xml
b/Documentation/DocBook/media/v4l/pixfmt.xml
index 2ff6b77..aef4615 100644
--- a/Documentation/DocBook/media/v4l/pixfmt.xml
+++ b/Documentation/DocBook/media/v4l/pixfmt.xml
@@ -714,6 +714,7 @@ information.
&sub-nv12m;
&sub-nv12mt;
&sub-nv16;
+&sub-nv24;
&sub-m420;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index fca24cc..8225163 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -343,6 +343,8 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_NV21v4l2_fourcc('N', 'V', '2', '1') /* 12 Y/CrCb
4:2:0 */
#define V4L2_PIX_FMT_NV16v4l2_fourcc('N', 'V', '1', '6') /* 16 Y/CbCr
4:2:2 */
#define V4L2_PIX_FMT_NV61v4l2_fourcc('N', 'V', '6', '1') /* 16 Y/CrCb
4:2:2 */
+#define V4L2_PIX_FMT_NV24v4l2_fourcc('N', 'V', '2', '4') /* 24 Y/CbC
Re: Record DVB-T from command line
One question. What if I need to record two channels in the same multiplex in the same time? Gnutv does not let me it, because the second recording command returns me that the device is busy, but I know this is possible (I know a Windows program - TV Scheduler Pro - that make it. Could you help me please? Thank you very much! Bye. -- 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
