Re: [PATCH v3 4/4] omap3isp: lane shifter support
Hi Sakari, Thanks for the review. On 03/16/2011 06:46 PM, Laurent Pinchart wrote: Hi Sakari, On Wednesday 16 March 2011 18:08:04 Sakari Ailus wrote: Laurent Pinchart wrote: Hi Sakari, + return in_info-bpp - out_info-bpp + additional_shift = 6; Currently there are no formats that would behave badly in this check? Perhaps it'd be good idea to take that into consideration. The shift that can be done is even. I've asked Michael to remove the check because we have no misbehaving formats :-) Do you think we need to add a check back ? I think it would be helpful in debugging if someone decides to attach a sensor which supports a shift of non-even bits (8 and 9 bits, for example). In any case an invalid configuration is possible in such case, and I don't think that should be allowed, should it? I agree it shouldn't be allowed, but the ISP driver doesn't support non-even widths at the moment, so there's no big risk. There could be an issue when a non-even width is added to the driver if the developer forgets to update the shift code. Maybe a comment in ispvideo.c above the big formats array would help making sure this is not forgotten ? I think now that additional_shift is also being considered which comes from the board file, it makes sense to reintroduce the check for an even shift. As Sakari points out, this would be helpful for debugging if someone tries using .data_lane_shift which is odd. @@ -247,6 +296,7 @@ static int isp_video_validate_pipeline(struct isp_pipeline *pipe) return -EPIPE; while (1) { + unsigned int link_has_shifter; link_has_shifter is only used in one place. Would it be cleaner to test below if it's the CCDC? A comment there could be nice, too. I would like that better as well, but between the line where link_has_shifter is set and the line where it is checked, the subdev variable changes so we can't just check subdev == isp-isp_ccdc.subdev there. That's definitely valid. I take my comment back. The variable could be called is_ccdc, though, since only the CCDC has that feature. No need to generalise. :-) But this is not a feature of the CCDC, the lane shifter is outside of the CCDC. Each 'while (1)' iteration handles 2 subdevs on each side of one link, so I think it makes sense for a particular iteration to say this link has, especially when the subdev ptr changes values between the assignment of this var and its usage. is_ccdc is vague as to which side of the CCDC we're on. 'link_has_shifter' wasn't intended to be general, it was supposed to mean 'this_is_the_link_with_the_shifter'. If you want to be more specific where that is in the pipeline, maybe 'ccdc_sink_link'? If you just want it to sound less like this is one of the links with a shifter and more like We've found _the_ link with _the_ shifter, it could just be 'shifter_link'. After we iron these two things out, are you guys ready to see v4? -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] omap3isp: lane shifter support
On Thursday 17 March 2011 11:07:40 Michael Jones wrote: On 03/16/2011 06:46 PM, Laurent Pinchart wrote: On Wednesday 16 March 2011 18:08:04 Sakari Ailus wrote: Laurent Pinchart wrote: Hi Sakari, + return in_info-bpp - out_info-bpp + additional_shift = 6; Currently there are no formats that would behave badly in this check? Perhaps it'd be good idea to take that into consideration. The shift that can be done is even. I've asked Michael to remove the check because we have no misbehaving formats :-) Do you think we need to add a check back ? I think it would be helpful in debugging if someone decides to attach a sensor which supports a shift of non-even bits (8 and 9 bits, for example). In any case an invalid configuration is possible in such case, and I don't think that should be allowed, should it? I agree it shouldn't be allowed, but the ISP driver doesn't support non-even widths at the moment, so there's no big risk. There could be an issue when a non-even width is added to the driver if the developer forgets to update the shift code. Maybe a comment in ispvideo.c above the big formats array would help making sure this is not forgotten ? I think now that additional_shift is also being considered which comes from the board file, it makes sense to reintroduce the check for an even shift. As Sakari points out, this would be helpful for debugging if someone tries using .data_lane_shift which is odd. How should we handle such a broken .data_lane_shift value ? Always refuse to start streaming (maybe with a kernel log message) ? Or should we catch it in isp_register_entities() instead ? @@ -247,6 +296,7 @@ static int isp_video_validate_pipeline(struct isp_pipeline *pipe) return -EPIPE; while (1) { + unsigned int link_has_shifter; link_has_shifter is only used in one place. Would it be cleaner to test below if it's the CCDC? A comment there could be nice, too. I would like that better as well, but between the line where link_has_shifter is set and the line where it is checked, the subdev variable changes so we can't just check subdev == isp-isp_ccdc.subdev there. That's definitely valid. I take my comment back. The variable could be called is_ccdc, though, since only the CCDC has that feature. No need to generalise. :-) But this is not a feature of the CCDC, the lane shifter is outside of the CCDC. Each 'while (1)' iteration handles 2 subdevs on each side of one link, so I think it makes sense for a particular iteration to say this link has, especially when the subdev ptr changes values between the assignment of this var and its usage. is_ccdc is vague as to which side of the CCDC we're on. 'link_has_shifter' wasn't intended to be general, it was supposed to mean 'this_is_the_link_with_the_shifter'. If you want to be more specific where that is in the pipeline, maybe 'ccdc_sink_link'? If you just want it to sound less like this is one of the links with a shifter and more like We've found _the_ link with _the_ shifter, it could just be 'shifter_link'. shifter_link sounds good to me. After we iron these two things out, are you guys ready to see v4? That's fine with me. -- 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] ignore Documentation/DocBook/media/
Hi Michael, Thanks for the patch. On Wednesday 16 March 2011 15:22:53 Michael Jones wrote: From 81a09633855b88d19f013d7e559e0c4f602ba711 Mon Sep 17 00:00:00 2001 From: Michael Jones michael.jo...@matrix-vision.de Date: Thu, 10 Mar 2011 16:16:38 +0100 Subject: [PATCH] ignore Documentation/DocBook/media/ It is created and populated by 'make htmldocs' Signed-off-by: Michael Jones michael.jo...@matrix-vision.de Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- Documentation/DocBook/.gitignore |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/Documentation/DocBook/.gitignore b/Documentation/DocBook/.gitignore index c6def35..679034c 100644 --- a/Documentation/DocBook/.gitignore +++ b/Documentation/DocBook/.gitignore @@ -8,3 +8,4 @@ *.dvi *.log *.out +media/ -- 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: [ANNOUNCE] usbmon capture and parser script
Em 16-03-2011 17:20, Paul Bolle escreveu: On Wed, 2011-03-16 at 12:47 -0700, Greg KH wrote: Very cool stuff. You are away of: http://vusb-analyzer.sourceforge.net/ right? Thanks for pointing it. It seems very interesting. Paul, On a quick test, it seems that it doesn't recognize the tcpdump file format (at least, it was not able to capture the dump files I got with the beagleboard). Adding support for it could be an interesting addition to your code. Btw, it seems that most of your work is focused on getting VMware logs. Most USB adapters I handle are USB 2.0 only, and have very high bandwidth requirements (something like 40%-60% of the total bandwidth available at the USB bus). It would be nice to be capable of using a VM on some cases, but the last time I tested VMWare, kvm, etc, none of them were capable of properly support USB 2.0 with isoc data transfers. Do you know if any of them are now capable of properly emulate USB 2.0 isoc transfers and give enough performance for the devices to actually work with such high-bandwidth requirements? I know you are doing this in console mode, but it looks close to the same idea. Yes, there are some similarities, although I think that the tools complement each other. Doing it via console is nice, as I can just use the serial interface of the beagleboard to capture and parse data in real time. An offline graphic analyser is interesting, especially when you need to filter data and check event timings. Perhaps there should be some references to vusb-analyzer and similar tools in Documentation/usb/usbmon.txt (it now only mentions usbdump and USBMon). I remember looking for a tool like that (ie, a parser) for quite some time before stumbling onto vusb-analyzer. Yeah, that seems a good idea to me too. Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] lirc_zilog: error out if buffer read bytes != chunk size
On Wed, Mar 16, 2011 at 08:07:22PM -0400, Andy Walls wrote: On Wed, 2011-03-16 at 16:24 -0400, Jarod Wilson wrote: Signed-off-by: Jarod Wilson ja...@redhat.com --- drivers/staging/lirc/lirc_zilog.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index 407d4b4..5ada643 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -950,6 +950,10 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos) ret = copy_to_user((void *)outbuf+written, buf, rbuf-chunk_size); written += rbuf-chunk_size; + } else { + zilog_error(Buffer read failed!\n); + ret = -EIO; + break; No need to break, just let the non-0 ret value drop you out of the while loop. Ah, indeed. I think I mindlessly copied what the tests just a few lines above were doing without looking at the actual reason for them. I'll remove that break from the patch here locally. -- Jarod Wilson ja...@redhat.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 5/6] lirc_zilog: error out if buffer read bytes != chunk size
Jarod Wilson ja...@redhat.com wrote: On Wed, Mar 16, 2011 at 08:07:22PM -0400, Andy Walls wrote: On Wed, 2011-03-16 at 16:24 -0400, Jarod Wilson wrote: Signed-off-by: Jarod Wilson ja...@redhat.com --- drivers/staging/lirc/lirc_zilog.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index 407d4b4..5ada643 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -950,6 +950,10 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos) ret = copy_to_user((void *)outbuf+written, buf, rbuf-chunk_size); written += rbuf-chunk_size; + } else { + zilog_error(Buffer read failed!\n); + ret = -EIO; + break; No need to break, just let the non-0 ret value drop you out of the while loop. Ah, indeed. I think I mindlessly copied what the tests just a few lines above were doing without looking at the actual reason for them. I'll remove that break from the patch here locally. -- Jarod Wilson ja...@redhat.com You might also want to take a look at that test to ensure it doesn't break blocking read() behavior. (man 2 read). I'm swamped ATM and didn't look too hard. It seems odd that the lirc buffer object can have data ready (the first branch of the big if() in the while() loop), and yet the read of that lirc buffer object fails. -Andy -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] omap3isp: lane shifter support
Hi Laurent and Michael, Laurent Pinchart wrote: On Thursday 17 March 2011 11:07:40 Michael Jones wrote: On 03/16/2011 06:46 PM, Laurent Pinchart wrote: On Wednesday 16 March 2011 18:08:04 Sakari Ailus wrote: Laurent Pinchart wrote: Hi Sakari, + return in_info-bpp - out_info-bpp + additional_shift = 6; Currently there are no formats that would behave badly in this check? Perhaps it'd be good idea to take that into consideration. The shift that can be done is even. I've asked Michael to remove the check because we have no misbehaving formats :-) Do you think we need to add a check back ? I think it would be helpful in debugging if someone decides to attach a sensor which supports a shift of non-even bits (8 and 9 bits, for example). In any case an invalid configuration is possible in such case, and I don't think that should be allowed, should it? I agree it shouldn't be allowed, but the ISP driver doesn't support non-even widths at the moment, so there's no big risk. There could be an issue when a non-even width is added to the driver if the developer forgets to update the shift code. Maybe a comment in ispvideo.c above the big formats array would help making sure this is not forgotten ? I think now that additional_shift is also being considered which comes from the board file, it makes sense to reintroduce the check for an even shift. As Sakari points out, this would be helpful for debugging if someone tries using .data_lane_shift which is odd. How should we handle such a broken .data_lane_shift value ? Always refuse to start streaming (maybe with a kernel log message) ? Or should we catch it in isp_register_entities() instead ? If I understand correctly it's not possible to shift odd bits in any case. It's a hardware limitation. I'd perhaps have just the appropriate register bits in the platform data so that leaves no room for accidental misconfiguration, but this is perhaps just too much work for not much gain. @@ -247,6 +296,7 @@ static int isp_video_validate_pipeline(struct isp_pipeline *pipe) return -EPIPE; while (1) { + unsigned int link_has_shifter; link_has_shifter is only used in one place. Would it be cleaner to test below if it's the CCDC? A comment there could be nice, too. I would like that better as well, but between the line where link_has_shifter is set and the line where it is checked, the subdev variable changes so we can't just check subdev == isp-isp_ccdc.subdev there. That's definitely valid. I take my comment back. The variable could be called is_ccdc, though, since only the CCDC has that feature. No need to generalise. :-) But this is not a feature of the CCDC, the lane shifter is outside of the CCDC. Each 'while (1)' iteration handles 2 subdevs on each side of one link, so I think it makes sense for a particular iteration to say this link has, especially when the subdev ptr changes values between the assignment of this var and its usage. is_ccdc is vague as to which side of the CCDC we're on. 'link_has_shifter' wasn't intended to be general, it was supposed to mean 'this_is_the_link_with_the_shifter'. If you want to be more specific where that is in the pipeline, maybe 'ccdc_sink_link'? If you just want it to sound less like this is one of the links with a shifter and more like We've found _the_ link with _the_ shifter, it could just be 'shifter_link'. shifter_link sounds good to me. I agree. After we iron these two things out, are you guys ready to see v4? That's fine with me. For me, too! Cheers, -- Sakari Ailus sakari.ai...@maxwell.research.nokia.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 5/6] lirc_zilog: error out if buffer read bytes != chunk size
On Thu, Mar 17, 2011 at 11:29:08AM -0400, Andy Walls wrote: Jarod Wilson ja...@redhat.com wrote: On Wed, Mar 16, 2011 at 08:07:22PM -0400, Andy Walls wrote: On Wed, 2011-03-16 at 16:24 -0400, Jarod Wilson wrote: Signed-off-by: Jarod Wilson ja...@redhat.com --- drivers/staging/lirc/lirc_zilog.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index 407d4b4..5ada643 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -950,6 +950,10 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos) ret = copy_to_user((void *)outbuf+written, buf, rbuf-chunk_size); written += rbuf-chunk_size; +} else { +zilog_error(Buffer read failed!\n); +ret = -EIO; +break; No need to break, just let the non-0 ret value drop you out of the while loop. Ah, indeed. I think I mindlessly copied what the tests just a few lines above were doing without looking at the actual reason for them. I'll remove that break from the patch here locally. -- Jarod Wilson ja...@redhat.com You might also want to take a look at that test to ensure it doesn't break blocking read() behavior. (man 2 read). I'm swamped ATM and didn't look too hard. It seems odd that the lirc buffer object can have data ready (the first branch of the big if() in the while() loop), and yet the read of that lirc buffer object fails. Generally, it shouldn't, but lirc_buffer_read uses kfifo underneath, and in the pre-2.6.33 kfifo implementation, the retval from lirc_buffer_read (as backported by way of media_build) is always 0, which is of course not equal to chunk_size. So I think that in current kernels, this should never trigger, and its partially just a note-to-self that this check will go sideways when running on an older kernel, but not a bad check to have if something really does go wrong. -- Jarod Wilson ja...@redhat.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 v3 4/4] omap3isp: lane shifter support
On 03/17/2011 04:40 PM, Sakari Ailus wrote: Hi Laurent and Michael, Laurent Pinchart wrote: On Thursday 17 March 2011 11:07:40 Michael Jones wrote: On 03/16/2011 06:46 PM, Laurent Pinchart wrote: On Wednesday 16 March 2011 18:08:04 Sakari Ailus wrote: Laurent Pinchart wrote: Hi Sakari, + return in_info-bpp - out_info-bpp + additional_shift = 6; Currently there are no formats that would behave badly in this check? Perhaps it'd be good idea to take that into consideration. The shift that can be done is even. I've asked Michael to remove the check because we have no misbehaving formats :-) Do you think we need to add a check back ? I think it would be helpful in debugging if someone decides to attach a sensor which supports a shift of non-even bits (8 and 9 bits, for example). In any case an invalid configuration is possible in such case, and I don't think that should be allowed, should it? I agree it shouldn't be allowed, but the ISP driver doesn't support non-even widths at the moment, so there's no big risk. There could be an issue when a non-even width is added to the driver if the developer forgets to update the shift code. Maybe a comment in ispvideo.c above the big formats array would help making sure this is not forgotten ? I think now that additional_shift is also being considered which comes from the board file, it makes sense to reintroduce the check for an even shift. As Sakari points out, this would be helpful for debugging if someone tries using .data_lane_shift which is odd. How should we handle such a broken .data_lane_shift value ? Always refuse to start streaming (maybe with a kernel log message) ? Or should we catch it in isp_register_entities() instead ? If I understand correctly it's not possible to shift odd bits in any case. It's a hardware limitation. I'd perhaps have just the appropriate register bits in the platform data so that leaves no room for accidental misconfiguration, but this is perhaps just too much work for not much gain. Actually, that's the way .data_lane_shift was originally defined (0,1,2,3), and I left it that way to minimize confusion. I was mistaken above when I said that .data_lane_shift could sneak an odd shift to isp_video_is_shiftable(), because .data_lane_shift is multiplied by 2 before getting passed there. So I would like to leave this as is, and it sounds like we have a consensus on this. I'll submit v4 soon, then. thanks, Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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 5/6] lirc_zilog: error out if buffer read bytes != chunk size
Jarod Wilson ja...@redhat.com wrote: On Thu, Mar 17, 2011 at 11:29:08AM -0400, Andy Walls wrote: Jarod Wilson ja...@redhat.com wrote: On Wed, Mar 16, 2011 at 08:07:22PM -0400, Andy Walls wrote: On Wed, 2011-03-16 at 16:24 -0400, Jarod Wilson wrote: Signed-off-by: Jarod Wilson ja...@redhat.com --- drivers/staging/lirc/lirc_zilog.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index 407d4b4..5ada643 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -950,6 +950,10 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos) ret = copy_to_user((void *)outbuf+written, buf, rbuf-chunk_size); written += rbuf-chunk_size; + } else { + zilog_error(Buffer read failed!\n); + ret = -EIO; + break; No need to break, just let the non-0 ret value drop you out of the while loop. Ah, indeed. I think I mindlessly copied what the tests just a few lines above were doing without looking at the actual reason for them. I'll remove that break from the patch here locally. -- Jarod Wilson ja...@redhat.com You might also want to take a look at that test to ensure it doesn't break blocking read() behavior. (man 2 read). I'm swamped ATM and didn't look too hard. It seems odd that the lirc buffer object can have data ready (the first branch of the big if() in the while() loop), and yet the read of that lirc buffer object fails. Generally, it shouldn't, but lirc_buffer_read uses kfifo underneath, and in the pre-2.6.33 kfifo implementation, the retval from lirc_buffer_read (as backported by way of media_build) is always 0, which is of course not equal to chunk_size. So I think that in current kernels, this should never trigger, and its partially just a note-to-self that this check will go sideways when running on an older kernel, but not a bad check to have if something really does go wrong. -- Jarod Wilson ja...@redhat.com But the orignal intent of the check I put in was to avoid passing partial/junk data to userspace, and go around again to see if good data could be provided. Your check bails when good data that might be sitting there still. That doesn't seem like a good trade for supporting backward compat for old kernels. -Andy -- 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: Compro VideoMate S350
Ken Smith wrote: First post to this list, but not new to Linux. I'm trying to get a Compro VideoMate S350 card to work in a 32bit Centos 5.5 system that also has a Hauppauge WinTV-NOVA-T-500 that uses the dvb-usb-dib0700.ko kernel module as its driver. The Nova-T appears to be working. I've used the v4l source RPM from ATRPMS to build an RPM of the v4l package. The RPM I compiled/built is called video4linux-kmdl-2.6.18-194.32.1.el5.centos.plus-20090907-93.RHL5.i386.rpm The build runs fine without issue. When I try to load the driver for the Compro with modprobe -v saa7134 card=169 I get insmod /lib/modules/2.6.18-194.32.1.el5.centos.plus/kernel/drivers/media/video/video-buf.ko WARNING: Error inserting video_buf (/lib/modules/2.6.18-194.32.1.el5.centos.plus/kernel/drivers/media/video/video-buf.ko): Invalid module format WARNING: Error inserting videobuf_dma_sg (/lib/modules/2.6.18-194.32.1.el5.centos.plus/updates/drivers/media/video/videobuf-dma-sg.ko): Unknown symbol in module, or unknown parameter (see dmesg) WARNING: Error inserting v4l1_compat (/lib/modules/2.6.18-194.32.1.el5.centos.plus/updates/drivers/media/video/v4l1-compat.ko): Unknown symbol in module, or unknown parameter (see dmesg) WARNING: Error inserting videodev (/lib/modules/2.6.18-194.32.1.el5.centos.plus/updates/drivers/media/video/videodev.ko): Unknown symbol in module, or unknown parameter (see dmesg) WARNING: Error inserting v4l2_common (/lib/modules/2.6.18-194.32.1.el5.centos.plus/updates/drivers/media/video/v4l2-common.ko): Unknown symbol in module, or unknown parameter (see dmesg) WARNING: Error inserting ir_common (/lib/modules/2.6.18-194.32.1.el5.centos.plus/updates/drivers/media/common/ir-common.ko): Unknown symbol in module, or unknown parameter (see dmesg) FATAL: Error inserting saa7134 (/lib/modules/2.6.18-194.32.1.el5.centos.plus/updates/drivers/media/video/saa7134/saa7134.ko): Unknown symbol in module, or unknown parameter (see dmesg) The line referred to in /var/log/messages is kernel: video_buf: exports duplicate symbol videobuf_mmap_mapper (owned by videobuf_core) I know that the centos.plus distribution includes some (if not all of) the v4l modules, so I tried my build of v4l with the plain Centos kernel build. The Nova T also runs fine, but I still get the error insmod /lib/modules/2.6.18-194.32.1.el5/kernel/drivers/media/video/video-buf.ko WARNING: Error inserting video_buf (/lib/modules/2.6.18-194.32.1.el5/kernel/drivers/media/video/video-buf.ko): Invalid module format I'm sure I have missed something obvious, I'd appreciate any suggestions. Thanks Ken Apologies - have I sent this to the wrong list. Is there a more appropriate forum? Thanks Ken -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean. -- 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 5/6] lirc_zilog: error out if buffer read bytes != chunk size
On Thu, Mar 17, 2011 at 12:16:31PM -0400, Andy Walls wrote: Jarod Wilson ja...@redhat.com wrote: On Thu, Mar 17, 2011 at 11:29:08AM -0400, Andy Walls wrote: Jarod Wilson ja...@redhat.com wrote: On Wed, Mar 16, 2011 at 08:07:22PM -0400, Andy Walls wrote: On Wed, 2011-03-16 at 16:24 -0400, Jarod Wilson wrote: Signed-off-by: Jarod Wilson ja...@redhat.com --- drivers/staging/lirc/lirc_zilog.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/staging/lirc/lirc_zilog.c b/drivers/staging/lirc/lirc_zilog.c index 407d4b4..5ada643 100644 --- a/drivers/staging/lirc/lirc_zilog.c +++ b/drivers/staging/lirc/lirc_zilog.c @@ -950,6 +950,10 @@ static ssize_t read(struct file *filep, char *outbuf, size_t n, loff_t *ppos) ret = copy_to_user((void *)outbuf+written, buf, rbuf-chunk_size); written += rbuf-chunk_size; + } else { + zilog_error(Buffer read failed!\n); + ret = -EIO; + break; No need to break, just let the non-0 ret value drop you out of the while loop. Ah, indeed. I think I mindlessly copied what the tests just a few lines above were doing without looking at the actual reason for them. I'll remove that break from the patch here locally. -- Jarod Wilson ja...@redhat.com You might also want to take a look at that test to ensure it doesn't break blocking read() behavior. (man 2 read). I'm swamped ATM and didn't look too hard. It seems odd that the lirc buffer object can have data ready (the first branch of the big if() in the while() loop), and yet the read of that lirc buffer object fails. Generally, it shouldn't, but lirc_buffer_read uses kfifo underneath, and in the pre-2.6.33 kfifo implementation, the retval from lirc_buffer_read (as backported by way of media_build) is always 0, which is of course not equal to chunk_size. So I think that in current kernels, this should never trigger, and its partially just a note-to-self that this check will go sideways when running on an older kernel, but not a bad check to have if something really does go wrong. But the orignal intent of the check I put in was to avoid passing partial/junk data to userspace, and go around again to see if good data could be provided. Your check bails when good data that might be sitting there still. That doesn't seem like a good trade for supporting backward compat for old kernels. Ah. Another thing I neglected to notice then. :) Perhaps there should be a retry count check as well then, as otherwise, its possible to get stuck in that loop forever (which is what was happening on older kernels). Its conceivable that similar could happen on a newer kernel for some reason. -- Jarod Wilson ja...@redhat.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
[cron job] v4l-dvb daily build: ERRORS
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 Mar 17 19:00:52 CET 2011 git hash:41f3becb7bef489f9e8c35284dd88a1ff59b190c gcc version: i686-linux-gcc (GCC) 4.5.1 host hardware:x86_64 host os: 2.6.32.5 linux-git-armv5: OK linux-git-armv5-davinci: OK linux-git-armv5-ixp: OK linux-git-armv5-omap2: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: WARNINGS linux-git-powerpc64: OK linux-git-x86_64: OK linux-2.6.31.12-i686: ERRORS linux-2.6.32.6-i686: ERRORS 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.31.12-x86_64: ERRORS linux-2.6.32.6-x86_64: ERRORS 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 spec-git: OK 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: add dvb-t fr-All frequency
Le Wed, 16 Mar 2011 16:46:38 +0100, Christoph Pfister christophpfis...@gmail.com a écrit : 2011/3/15 matthieu castet castet.matth...@free.fr: Hi, can this file be added to dvb-apps/util/scan/dvb-t No. Use auto-Normal [1] (or the +-167kHz file if needed). Ok, thanks I wasn't aware of the auto files. How does the offset version works ? The scan tools is clever enough to not duplicate freq, freq-offset, freq+offset and choose the one with best signal ? BTW in theory in France, offset can be : - 0,166 MHz /+ 0,166 MHz /+ 0,332 MHz /+ 0,498 MHz. Matthieu -- 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] New Jeilin dual-mode camera support
Le 17/03/2011 11:28, Jean-Francois Moine a écrit : On Wed, 16 Mar 2011 21:06:13 +0100 Patrice Chotard patrice.chot...@sfr.fr wrote: This patch add a new jeilin dual mode camera support and some specific controls settings. Hi Patrice and Theodore, Here are somme comments about Patrice's patch. #include linux/workqueue.h +#include linux/delay.h #include linux/slab.h It is not a good idea to use mdelay(): it is a loop. Better use msleep(). -u8 quality; /* image quality */ -u8 jpegqual;/* webcam quality */ +u8 camquality; /* webcam quality */ +u8 jpegquality; /* jpeg quality */ The webcam (encoding) quality and the jpeg (decoding) quality must be the same. Then, looking carefully, jpegquality is not used! +u8 freq; +u8 type; +/* below variables are only used for SPORTSCAM_DV15 */ +u8 autogain; +u8 cyan; +u8 magenta; +u8 yellow; You should use the new control mechanism (see stk014, sonixj, zc3xx...). +#define V4L2_CID_CAMQUALITY (V4L2_CID_USER_BASE + 1) +.id = V4L2_CID_CAMQUALITY, +.type= V4L2_CTRL_TYPE_INTEGER, +.name= Image quality, The JPEG quality must be get/set by the VIDIOC_G_JPEGCOMP / VIDIOC_S_JPEGCOMP ioctl's. +#define V4L2_CID_CYAN_BALANCE (V4L2_CID_USER_BASE + 2) [snip] +#define V4L2_CID_MAGENTA_BALANCE (V4L2_CID_USER_BASE + 3) [snip] +#define V4L2_CID_YELLOW_BALANCE (V4L2_CID_USER_BASE + 4) These values redefine V4L2_CID_SATURATION and V4L2_CID_HUE (user_base + 4 is no more defined). You should use V4L2_CID_RED_BALANCE, V4L2_CID_BLUE_BALANCE and V4L2_CID_GAIN to set these controls. +if (sd-type == SPORTSCAM_DV15) +start_commands_size = 9; +else +start_commands_size = ARRAY_SIZE(start_commands); Don't use magic values ('9'). +mdelay(start_commands[i].delay); See above. BTW, Theodore, as there is no USB command in the loop, there is no need to have a work queue (look at the SENSOR_OV772x in ov534). Best regards. Hi, Following Jean Francois's remark, here is a new version of my previous patch for a new jeilin dual mode camera support with specific control settings. Regards. Signed-off-by: Patrice CHOTARD patricechot...@free.fr Signed-off-by: Theodore Kilgore kilg...@auburn.edu --- Documentation/video4linux/gspca.txt |1 + drivers/media/video/gspca/jeilinj.c | 405 ++- 2 files changed, 354 insertions(+), 52 deletions(-) diff --git a/Documentation/video4linux/gspca.txt b/Documentation/video4linux/gspca.txt index 261776e..c4245d2 100644 --- a/Documentation/video4linux/gspca.txt +++ b/Documentation/video4linux/gspca.txt @@ -265,6 +265,7 @@ pac7302 093a:2629 Genious iSlim 300 pac7302093a:262a Webcam 300k pac7302093a:262c Philips SPC 230 NC jeilinj0979:0280 Sakar 57379 +jeilinj0979:0270 Sportscam DV15 zc3xx 0ac8:0302 Z-star Vimicro zc0302 vc032x 0ac8:0321 Vimicro generic vc0321 vc032x 0ac8:0323 Vimicro Vc0323 diff --git a/drivers/media/video/gspca/jeilinj.c b/drivers/media/video/gspca/jeilinj.c index 06b777f..b417589 100644 --- a/drivers/media/video/gspca/jeilinj.c +++ b/drivers/media/video/gspca/jeilinj.c @@ -6,6 +6,9 @@ * * Copyright (C) 2009 Theodore Kilgore * + * Sportscam DV15 support and control settings are + * Copyright (C) 2011 Patrice Chotard + * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2 of the License, or @@ -34,6 +37,7 @@ MODULE_LICENSE(GPL); /* Default timeouts, in ms */ #define JEILINJ_CMD_TIMEOUT 500 +#define JEILINJ_CMD_DELAY 160 #define JEILINJ_DATA_TIMEOUT 1000 /* Maximum transfer size to use. */ @@ -41,30 +45,40 @@ MODULE_LICENSE(GPL); #define FRAME_HEADER_LEN 0x10 +enum { + SAKAR_57379, + SPORTSCAM_DV15, +}; + +#define CAMQUALITY_MIN 0 /* highest cam quality */ +#define CAMQUALITY_MAX 97 /* lowest cam quality */ +#define SPORTSCAM_DV15_CMD_SIZE 9 + /* Structure to hold all of our device specific stuff */ struct sd { struct gspca_dev gspca_dev; /* !! must be the first item */ - const struct v4l2_pix_format *cap_mode; /* Driver stuff */ struct work_struct work_struct; struct workqueue_struct *work_thread; - u8 quality; /* image quality */ - u8 jpegqual;/* webcam quality */ + u8 quality; +#define QUALITY_MIN 35 +#define QUALITY_MAX 85 +#define QUALITY_DEF 85 + u8 jpeg_hdr[JPEG_HDR_SZ]; + u8 freq; + u8 type; + /* below variables are only
[PATCH] tm6000: fix s-video input
Hi Add compatibility for composite and s-video inputs. Some TV cards hasn't it. Fix S-Video input, the s-video cable has only video signals no audio. Call the function of audio configure kill chroma in signal. only b/w video. Known bugs: after s-video the audio for radio didn't work, TV crashed hardly after composite TV crashed hardly too. P.S. After this patch I'll want to rework the procedure of configure video. Now it has a lot of junk and dubles. diff --git a/drivers/staging/tm6000/tm6000-cards.c b/drivers/staging/tm6000/tm6000-cards.c index 88144a1..146c7e8 100644 --- a/drivers/staging/tm6000/tm6000-cards.c +++ b/drivers/staging/tm6000/tm6000-cards.c @@ -235,11 +235,13 @@ struct tm6000_board tm6000_boards[] = { .avideo = TM6000_AIP_SIF1, .aradio = TM6000_AIP_LINE1, .caps = { - .has_tuner= 1, - .has_dvb = 1, - .has_zl10353 = 1, - .has_eeprom = 1, - .has_remote = 1, + .has_tuner = 1, + .has_dvb= 1, + .has_zl10353= 1, + .has_eeprom = 1, + .has_remote = 1, + .has_input_comp = 1, + .has_input_svid = 1, }, .gpio = { .tuner_reset= TM6010_GPIO_0, @@ -255,11 +257,13 @@ struct tm6000_board tm6000_boards[] = { .avideo = TM6000_AIP_SIF1, .aradio = TM6000_AIP_LINE1, .caps = { - .has_tuner= 1, - .has_dvb = 0, - .has_zl10353 = 0, - .has_eeprom = 1, - .has_remote = 1, + .has_tuner = 1, + .has_dvb= 0, + .has_zl10353= 0, + .has_eeprom = 1, + .has_remote = 1, + .has_input_comp = 1, + .has_input_svid = 1, }, .gpio = { .tuner_reset= TM6010_GPIO_0, @@ -327,10 +331,13 @@ struct tm6000_board tm6000_boards[] = { .avideo = TM6000_AIP_SIF1, .aradio = TM6000_AIP_LINE1, .caps = { - .has_tuner= 1, - .has_dvb = 1, - .has_zl10353 = 1, - .has_eeprom = 1, + .has_tuner = 1, + .has_dvb= 1, + .has_zl10353= 1, + .has_eeprom = 1, + .has_remote = 0, + .has_input_comp = 0, + .has_input_svid = 0, }, .gpio = { .tuner_reset= TM6010_GPIO_0, @@ -346,10 +353,13 @@ struct tm6000_board tm6000_boards[] = { .avideo = TM6000_AIP_SIF1, .aradio = TM6000_AIP_LINE1, .caps = { - .has_tuner= 1, - .has_dvb = 0, - .has_zl10353 = 0, - .has_eeprom = 1, + .has_tuner = 1, + .has_dvb= 0, + .has_zl10353= 0, + .has_eeprom = 1, + .has_remote = 0, + .has_input_comp = 0, + .has_input_svid = 0, }, .gpio = { .tuner_reset= TM6010_GPIO_0, diff --git a/drivers/staging/tm6000/tm6000-stds.c b/drivers/staging/tm6000/tm6000-stds.c index a4c07e5..da3e51b 100644 --- a/drivers/staging/tm6000/tm6000-stds.c +++ b/drivers/staging/tm6000/tm6000-stds.c @@ -1161,8 +1161,6 @@ int tm6000_set_standard(struct tm6000_core *dev, v4l2_std_id * norm) rc = tm6000_load_std(dev, svideo_stds[i].common, sizeof(svideo_stds[i]. common)); - tm6000_set_audio_std(dev, svideo_stds[i].audio_default_std); - goto ret; } } diff --git a/drivers/staging/tm6000/tm6000-video.c b/drivers/staging/tm6000/tm6000-video.c index b550340..c80a316 100644 --- a/drivers/staging/tm6000/tm6000-video.c +++ b/drivers/staging/tm6000/tm6000-video.c @@ -1080,18 +1080,27 @@ static int vidioc_s_std (struct file *file, void *priv, v4l2_std_id *norm) static int vidioc_enum_input(struct file *file, void *priv, struct v4l2_input *inp) {
Re: [PATCH 5/6] lirc_zilog: error out if buffer read bytes != chunk size
On Thu, 2011-03-17 at 15:08 -0400, Jarod Wilson wrote: On Thu, Mar 17, 2011 at 12:16:31PM -0400, Andy Walls wrote: Jarod Wilson ja...@redhat.com wrote: . But the orignal intent of the check I put in was to avoid passing partial/junk data to userspace, and go around again to see if good data could be provided. Your check bails when good data that might be sitting there still. That doesn't seem like a good trade for supporting backward compat for old kernels. Ah. Another thing I neglected to notice then. :) Perhaps there should be a retry count check as well then, as otherwise, its possible to get stuck in that loop forever (which is what was happening on older kernels). Its conceivable that similar could happen on a newer kernel for some reason. Well, lets see, From the perspective of userspace lircd: 1. A specification compliance failure for a corner case isn't too bad (bailing out on junk and leaving good data behind) 2. An unrecoverable failure for any case is very bad (spinning/hanging on a result that won't change) 3. Sending unitialized bytes out to userspace with copy_to_user() is very bad. (I recall the old code would do the copy to user and always tell userspace it got a code whether it read anything out of the buffer or not. IIRC, that leaked information off the stack.) If the code as patched avoids the two very bad things (#2 and #3), then the patch is OK by me. Regards, Andy -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-dvb] Problem with saa7134: Asus Tiger revision 1.0, subsys 1043:4857
Hi Jason, Am Samstag, den 12.03.2011, 10:43 +1100 schrieb Jason Hecker: I just bought a pair of what are a version of the My Cinema 7131 Hybrid cards. The kernel reports it as saa7134: Asus Tiger revision 1.0, subsys 1043:4857 I did inititially try Mythbuntu 10.04 but the firmware upload seemed to fail fairly consistently. Restarting with v10.10 the firmware loads but I can't seem to scan the channels with Mythbackend - it has a 0% signal and 100% signal to noise. I am using MythTV 0.24 with Avenard's latest patches. This version of the card has written on the silkscreen Tiger rev 3.02, a sticker that says Tiger_8M AA.F7.C0.01 (which would appear to be the latest firmware for this card on Asus's support site) but there is only one RF connector on CON1. CON2 is not fitted nor is the IR receiver. Now I saw mentioned on a list that to get DVB working on this card in Linux you need to connect the TV antenna to the FM port, which I suspect is the one not fitted. The latest Windows drivers for this card is circa 2009. Two questions: - Is there some sort of SAA7134 module argument I need to use to get the card working on the TV RF input? - Why does the kernel show the firmware is being reloaded every time MythTV seems to want to talk to the card? This slows down access as it seems to take about 30 seconds for the firmware to install each time. I am happy to provide whatever debug dumps or more info if need be. this hits me only by accident, reading through backlash, but I added that Asus Tiger Revision 1.0 with subsys 1043:4857, with a huge delay only. (approximately 1 1/2 years) The development and testing for the new tuner types was done only much later on freely available stuff, a so called Asus Dual _non_ OEM variant. Not to tell what we did all see thereafter, but that all was at least, with only one exception, valid using the PCI subsystem as unique identifier. Luckily, as far as I can see, we have only a fictional radio device on your new variant left over. This can still be very annoying, but won't do any harm, except wasting a users time, bad enough, but at least not any radiation from that sort of radio flaw. Since the PCI subsystem is identical with mine, still around somewhere, with radio support, either take that dead radio device for now or a last chance is to discover, if any eeprom differences are there to eventually filter that minor, but unpleasant shortcoming for those trying in vain on the radio. Cheers, Hermann To restore the power on a failing power plant in urgent need of it seems to be a good idea, after six or seven days ... All my excuses for the failing radio device on that not yet seen OEM stuff, but I can ensure, to piss on it doesn't help any further. Hopefully it does help in that other case. -- 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