Re: RFC: Core + Radio profile
Hi, On 08/22/2012 11:40 AM, Hans Verkuil wrote: Hi all! Snip While writing down these profiles I noticed one thing that was very much missing for radio devices: there is no capability bit to tell the application whether there is an associated ALSA device or whether the audio goes through a line-in/out. Personally I think that this should be fixed. The question with generic tuner drivers like the tea57XX series is do we always know this ? One could argue to proper way to find this out for applications is by looking at the device topology, either through the media controller framework, or through sysfs. This is for example what xawtv currently does. We need a better library to handle this, which also hides from the user whether the media controller or sysfs is used. Comments are welcome! Looks good! Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] media/radio/shark2: Fix build error caused by missing dependencies
Hi, I've a better fix for this here: http://git.linuxtv.org/hgoede/gspca.git/shortlog/refs/heads/media-for_v3.6 I already send a pull-req for this to Mauro a while ago, Mauro? Regards, Hans On 08/22/2012 05:16 PM, Guenter Roeck wrote: Fix build error: ERROR: led_classdev_register [drivers/media/radio/shark2.ko] undefined! ERROR: led_classdev_unregister [drivers/media/radio/shark2.ko] undefined! which is seen if RADIO_SHARK2 is enabled, but LEDS_CLASS is not. Since RADIO_SHARK2 depends on NEW_LEDS and LEDS_CLASS, select both if it is enabled. Cc: Hans de Goede hdego...@redhat.com Signed-off-by: Guenter Roeck li...@roeck-us.net --- drivers/media/radio/Kconfig |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/radio/Kconfig b/drivers/media/radio/Kconfig index 8090b87..bee4bee 100644 --- a/drivers/media/radio/Kconfig +++ b/drivers/media/radio/Kconfig @@ -77,6 +77,8 @@ config RADIO_SHARK config RADIO_SHARK2 tristate Griffin radioSHARK2 USB radio receiver depends on USB + select NEW_LEDS + select LEDS_CLASS ---help--- Choose Y here if you have this radio receiver. -- 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: How to add support for the em2765 webcam Speedlink VAD Laplace to the kernel ?
Hi, On 08/20/2012 01:41 PM, Frank Schäfer wrote: Hi, after a break of 2 1/2 kernel releases (sorry, I was busy with another project), I would like to bring up again the question how to add support for this device to the kernel. See http://www.mail-archive.com/linux-media@vger.kernel.org/msg44417.html (Move em27xx/em28xx webcams to a gspca subdriver ?) for the previous discussion. Current status is, that I've reverse-engineered the Windows driver and written a new gspca-subdriver for testing, which is feature complete and working stable (will send a patch shortly !). The device uses an em2765-bridge, so my first idea was of course to modify/extend the em28xx-driver. But during the reverse-engineering-process, it turned out that writing a new gspca-subdriver was much easier than modifying the em28xx-driver. The device has the following special characteristics: - supports only bulk transfers (em28xx driver supports ISOC only) - uses proprietary read/write procedures for the sensor - uses 16bit eeprom - em25xx-eeprom with different layout - sensor OV2640 - different frame processing - 3 buttons (snapshot, mute, light) which need special treatment (GPIO-polling, status-reseting, ...) Another important point to mention: you can see from the USB-logs (sensor probing) that there must be at least 3 other webcam devices. Some pros and cons for both solutions: em28xx: + one driver for all 25xx/26xx/27xx/28xx devices + no duplicate code (bridge register defines, bridge read/write fcns) + other devices COULD benefit from the new functions/code - big task/lots of work - code gets bloated with stuff, which is only needed by a few special devices gspca: + driver already exists (see patch) + default driver for webcams + much easier to understand and extend + same or even less amount of new code lines + keeps em28xx-code simple - code duplication - support for em28xx-webcams spread over to 2 different drivers I have no strong opinion whether support for this device should finally be added to em28xx or gspca and I'm willing to continue working on both solutions as much as my time permits and as long as I'm having fun (I'm doing this as a hobby !). Anyway, the em28xx driver is very complex and I really think it would take several further kernel releases to get the job done... I would also be willing to spend some time for moving the em28xx-webcam code to a gspca subdriver, but I don't have any of these devices for testing. What do you think ? I think given the special way this camera uses the bridge (not using standard i2c interface, weird button layout, etc.). That it is likely server better by a specialized driver. As the (new) gspca maintainer I'm fine with taking it as a gspca sub-driver, but given the code duplication issue, that is a call Mauro should make. Note that luckily these devices do use a unique USB id and not one of the generic em28xx ids so from that pov having a specialized driver for them is not an issue. Regards, Hans p.s. Frank have you seen this mail, it seems another Linux user has the same camera, perhaps he can run some tests for you: http://osdir.com/ml/linux-media/2009-05/msg00186.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: How to add support for the em2765 webcam Speedlink VAD Laplace to the kernel ?
Hi, On 08/20/2012 09:21 PM, Mauro Carvalho Chehab wrote: Em 20-08-2012 10:02, Hans de Goede escreveu: snip Note that luckily these devices do use a unique USB id and not one of the generic em28xx ids so from that pov having a specialized driver for them is not an issue. Hans, Not sure if all em2765 cameras will have unique USB id's: at em28xx, the known em2710/em2750 cameras that don't have unique ID's; detecting between them requires to probe for the type of sensor. Right, like the one I gave to Douglas and you or Douglas (don't remember) added support for. But that one was a regular em28xx using camera, and this one appears to be a bit funky in places... I'll let Frank answer your other remarks. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL FOR 3.7] Misc. fixes + shark improvements
Hi Mauro, Please pull from my tree for a bunch of assorted fixes, as well as AM tuning and suspend/resume for the radio-shark and radio-shark2 drivers. The following changes since commit 9b78c5a3007e10a172d4e83bea18509fdff2e8e3: [media] b2c2: export b2c2_flexcop_debug symbol (2012-08-17 11:09:19 -0300) are available in the git repository at: git://linuxtv.org/hgoede/gspca.git media-for_v3.7-wip for you to fetch changes up to 7fd59dce24e2c57d2537ec827e0c7e09c7c1e77b: media-api-docs: Documented V4L2_TUNER_CAP_HWSEEK_PROG_LIM in G_TUNER docs (2012-08-18 17:35:48 +0200) Emil Goode (1): gspca: dubious one-bit signed bitfield Ezequiel Garcia (1): pwc: Use vb2 queue mutex through a single name Hans de Goede (10): radio-shark*: Remove work-around for dangling pointer in usb intfdata radio-shark*: Call cancel_work_sync from disconnect rather then release radio-shark: Only compile led support when CONFIG_LED_CLASS is set radio-shark2: Only compile led support when CONFIG_LED_CLASS is set snd_tea575x: Add support for tuning AM radio-tea5777.c: Get rid of do_div usage radio-tea5777: Add support for tuning AM radio-shark2: Add support for suspend resume radio-shark: Add support for suspend resume media-api-docs: Documented V4L2_TUNER_CAP_HWSEEK_PROG_LIM in G_TUNER docs Documentation/DocBook/media/v4l/vidioc-g-tuner.xml | 6 + drivers/media/radio/radio-shark.c | 195 drivers/media/radio/radio-shark2.c | 176 +++--- drivers/media/radio/radio-tea5777.c| 197 +++- drivers/media/radio/radio-tea5777.h| 3 + drivers/media/usb/gspca/ov519.c| 16 +- drivers/media/usb/pwc/pwc-if.c | 2 +- include/sound/tea575x-tuner.h | 4 + sound/i2c/other/tea575x-tuner.c| 200 + 9 files changed, 570 insertions(+), 229 deletions(-) Thanks, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: V4L2 API ambiguities: workshop presentation
Hi, On 08/17/2012 12:35 PM, Hans Verkuil wrote: Hi all, I've prepared a presentation for the upcoming workshop based on my RFC and the comments I received. It is available here: http://hverkuil.home.xs4all.nl/presentations/v4l2-workshop-2012.odp http://hverkuil.home.xs4all.nl/presentations/v4l2-workshop-2012.pdf Attendees of the workshop: please review this before the workshop starts. I want to go through this list fairly quickly (particularly slides 1-14) so we can have more time for other topics. A note on the Pixel Aspect Ratio from me, since I won't be attending: I'm not sure if having a VIDIOC_G_PIXELASPECT is enough, it will work to get the current mode, but not for enumerating. Also it will not work with TRY_FMT, that is one cannot find out the actual pixelaspect until after a S_FMT. As mentioned in previous mail I think at a minimum the results of ENUM_FRAMESIZES should contain the pixel aspect per framesize, there is enough reserved space in the relevant structs to make this happen Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Workshop-2011] V4L2 API ambiguities: workshop presentation
Hi, On 08/17/2012 02:57 PM, Laurent Pinchart wrote: Snip Regarding ENUM_FRAMESIZES: it makes sense to add an aspect ratio here for use with sensors. But for video receivers ENUM_FRAMESIZES isn't applicable. Do we have sensors with non-square pixels ? Short answer: not that I know of. Long answer: that depends on the optics (so the sensor pixels may be square, but the optics could make them non-square from a proper mapping to a real world picture pov). As I've done too much with weird old webcams I actually now webcams which do this, the vicam cameras to be precise. The 3 com HomeConnect (04c1:009) has a sensor with a native resolution of 512x244, yeah widescreen baby! But it stems from an area where widescreen was unheard of in computer graphics, so it actually has optics which force that cool widescreen resolution back into a 4x3 field of view. So for a proper square pixels image form that camera its image needs to be scaled from 512x244 to 512x384 (*). But with that one exception proving the rule (Dutch expression), I think all sensors have square pixels. Regards, Hans *) Really? Yes really! -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT-PULL FIXES for 3.6] radio-shark*: Only compile led support when CONFIG_LED_CLASS is set
Hi, Please pull the shark build fixes from my tree for 3.6: The following changes since commit f9cd49033b349b8be3bb1f01b39eed837853d880: Merge tag 'v3.6-rc1' into staging/for_v3.6 (2012-08-03 22:41:33 -0300) are available in the git repository at: git://linuxtv.org/hgoede/gspca.git media-for_v3.6 for you to fetch changes up to 74e8155de5331ef7f707ed22432a79f1477a7d22: radio-shark2: Only compile led support when CONFIG_LED_CLASS is set (2012-08-15 12:05:49 +0200) Hans de Goede (4): radio-shark*: Remove work-around for dangling pointer in usb intfdata radio-shark*: Call cancel_work_sync from disconnect rather then release radio-shark: Only compile led support when CONFIG_LED_CLASS is set radio-shark2: Only compile led support when CONFIG_LED_CLASS is set drivers/media/radio/radio-shark.c | 151 +++-- drivers/media/radio/radio-shark2.c | 137 + 2 files changed, 150 insertions(+), 138 deletions(-) Thanks Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Workshop-2011] RFC: V4L2 API ambiguities
Hi, On 08/13/2012 09:15 PM, Sylwester Nawrocki wrote: snip And if a driver also supports single-plane formats in addition to 1 plane formats, should V4L2_CAP_VIDEO_CAPTURE be compulsary? Yes, so that non multi-plane aware apps keep working. There is the multi-planar API and there are multi-planar formats. Single- and multi-planar formats can be handled with the multi-planar API. So if a driver supports single- and multi-planar formats by means on multi-planar APIs, there shouldn't be a need for signalling V4L2_CAP_VIDEO_CAPTURE, which normally indicates single-planar API. The driver may choose to not support it, in order to handle single-planar formats. Thus, in my opinion making V4L2_CAP_VIDEO_CAPTURE compulsory wouldn't make sense. Unless the driver supports both types of ioctls (_mplane and regular versions), we shouldn't flag V4L2_CAP_VIDEO_CAPTURE. Ok. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Workshop-2011] RFC: V4L2 API ambiguities
Hi, On 08/14/2012 02:00 AM, Laurent Pinchart wrote: Hi Hans, On Monday 13 August 2012 15:13:34 Hans de Goede wrote: [snip] 4) What should a driver return in TRY_FMT/S_FMT if the requested format is not supported (possible behaviours include returning the currently selected format or a default format). The spec says this: Drivers should not return an error code unless the input is ambiguous, but it does not explain what constitutes an ambiguous input. Frankly, I can't think of any and in my opinion TRY/S_FMT should never return an error other than EINVAL (if the buffer type is unsupported) or EBUSY (for S_FMT if streaming is in progress). Returning an error for any other reason doesn't help the application since the app will have no way of knowing what to do next. Ack on not returning an error for requesting an unavailable format. As for what the driver should do (default versus current format) I've no preference, I vote for letting this be decided by the driver implementation. That's exactly the point that I wanted to clarify :-) I don't see a good reason to let the driver decide on this, and would prefer returning a default format I see. as TRY_FMT would then always return the same result for a given input format regardless of the currently selected format. That argument makes sense, so ack from me on always returning a default format. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Copyright issues, do not copy code and add your own copyrights
Hi, On 08/14/2012 11:10 AM, Manu Abraham wrote: Hi, The subject line says it. Please fix the offending Copyright header. Offending one. http://git.linuxtv.org/media_tree.git/blob/staging/for_v3.7:/drivers/media/dvb-frontends/stb6100_proc.h Original one. http://git.linuxtv.org/media_tree.git/blob/staging/for_v3.7:/drivers/media/dvb-frontends/stb6100_cfg.h Or even better, get rid of the offending one and add a i2c_gate_ctrl parameters to the inline functions defined in stb6100_cfg.h, as this seems a typical case of unnecessary code-duplication. I would also like to point out that things like these are pretty much wrong: 27 if (fe-ops) 28 frontend_ops = fe-ops; 29 if (frontend_ops-tuner_ops) 30 tuner_ops = frontend_ops-tuner_ops; 31 if (tuner_ops-get_state) { The last check de-references tuner_ops, which only is non-NULL if fe-ops and fe-ops-tuner_ops are non NULL. So either the last check needs to be: if (tuner_ops tuner_ops-get_state) { Or we assume that fe-ops and fe-ops-tuner_ops are always non NULL when this helper gets called and all the previous checks can be removed. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Copyright issues, do not copy code and add your own copyrights
Hi, On 08/14/2012 11:42 AM, Manu Abraham wrote: Hi, On Tue, Aug 14, 2012 at 2:51 PM, Hans de Goede hdego...@redhat.com wrote: Hi, On 08/14/2012 11:10 AM, Manu Abraham wrote: Hi, The subject line says it. Please fix the offending Copyright header. Offending one. http://git.linuxtv.org/media_tree.git/blob/staging/for_v3.7:/drivers/media/dvb-frontends/stb6100_proc.h Original one. http://git.linuxtv.org/media_tree.git/blob/staging/for_v3.7:/drivers/media/dvb-frontends/stb6100_cfg.h Or even better, get rid of the offending one and add a i2c_gate_ctrl parameters to the inline functions defined in stb6100_cfg.h, as this seems a typical case of unnecessary code-duplication. i2c_gate_ctrl is not provided by stb6100 hardware, but by the demodulator used in conjunction such as a stb0899 as can be seen. Right, I was merely pointing out that the only difference between the original function wrappers in stb6100_cfg.h and the ones in stb6100_proc.h, is the calling of the i2c_gate_ctrl frontend-op if defined. So the 2 files could be merged into one, with the wrappers getting an extra boolean parameter making them call the frontend-op when that parameter is true. Note that if the i2c_gate_ctrl frontend-op should always be called when present then the extra parameter could be omitted. snip I would also like to point out that things like these are pretty much wrong: 27 if (fe-ops) 28 frontend_ops = fe-ops; 29 if (frontend_ops-tuner_ops) 30 tuner_ops = frontend_ops-tuner_ops; 31 if (tuner_ops-get_state) { The last check de-references tuner_ops, which only is non-NULL if fe-ops and fe-ops-tuner_ops are non NULL. So either the last check needs to be: if (tuner_ops tuner_ops-get_state) { Or we assume that fe-ops and fe-ops-tuner_ops are always non NULL when this helper gets called and all the previous checks can be removed. fe-ops is not NULL in any case, when we reach here, but that conditionality check causes a slight additional delay. The additional check you proposed presents no harm, though not bringing any new advantage/disadvantage. Well if we know that fe-ops and fe-ops-tuner_ops are never NULL, then the if (fe-ops) and if (frontend_ops-tuner_ops) are superfluous and should be removed, on the other hand if we don't know that, then the get_state check should be: if (tuner_ops tuner_ops-get_state) { Either know fe-ops and fe-ops-tuner_ops are never NULL and then all checks should be removed, or we don't know and we should check them in *all* places where they are used. What we've now is somewhat of the former, and then some of the latter, which makes no sense at all. Regards, Hans Regards, Manu -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Workshop-2011] RFC: V4L2 API ambiguities
Hi, On 08/14/2012 12:54 PM, Hans Verkuil wrote: On Tue August 14 2012 01:54:16 Laurent Pinchart wrote: Hi Hans, On Monday 13 August 2012 14:27:56 Hans Verkuil wrote: Hi all! As part of the 2012 Kernel Summit V4L2 workshop I will be discussing a bunch of V4L2 ambiguities/improvements. I've made a list of all the V4L2 issues and put them in two categories: issues that I think are easy to resolve (within a few minutes at most), and those that are harder. If you think I put something in the easy category that you believe is actually hard, then please let me know. If you attend the workshop, then please read through this and think about it a bit, particularly for the second category. If something is unclear, or you think another topic should be added, then let me know as well. Easy: [snip] 4) What should a driver return in TRY_FMT/S_FMT if the requested format is not supported (possible behaviours include returning the currently selected format or a default format). The spec says this: Drivers should not return an error code unless the input is ambiguous, but it does not explain what constitutes an ambiguous input. Frankly, I can't think of any and in my opinion TRY/S_FMT should never return an error other than EINVAL (if the buffer type is unsupported) or EBUSY (for S_FMT if streaming is in progress). Returning an error for any other reason doesn't help the application since the app will have no way of knowing what to do next. That wasn't my point. Drivers should obviously not return an error. Let's consider the case of a driver supporting YUYV and MJPEG. If the user calls TRY_FMT or S_FMT with the pixel format set to RGB565, should the driver return a hardcoded default format (one of YUYV or MJPEG), or the currently selected format ? In other words, should the pixel format returned by TRY_FMT or S_FMT when the requested pixel format is not valid be a fixed default pixel format, or should it depend on the currently selected pixel format ? Actually, in this case I would probably choose a YUYV format that is closest to the requested size. If a driver supports both compressed and uncompressed formats, then it should only select a compressed format if the application explicitly asked for it. Handling compressed formats is more complex than uncompressed formats, so that seems a sensible rule. The next heuristic I would apply is to choose a format that is closest to the requested size. Size as in resolution or size as in bpp? So I guess my guidelines would be: 1) If the pixelformat is not supported, then choose an uncompressed format (if possible) instead. 2) Next choose a format closest to, but smaller than (if possible) the requested size. But this would be a guideline only, and in the end it should be up to the driver. Just as long TRY/S_FMT always returns a format. I think we're making this way too complicated. I agree that TRY/S_FMT should always returns a format and not EINVAL, but other then that lets just document that if the driver does not know the passed in format it should return a default format and not make this dependent on the passed in fmt, esp. since otherwise the driver would need to know about all formats it does not support to best map that to a one which it does support, which is just crazy. So I suggest adding the following to the spec: When a driver receives an unsupported pixfmt as input on a TRY/S_FMT call it should replace this with a default pixfmt, independent of input pixfmt and current driver state. Preferably a driver uses a well known uncompressed pixfmt as its default. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Workshop-2011] RFC: V4L2 API ambiguities
Hi, snip Easy: 1) Split off the control part from videodev2.h. Controls are almost 30% of videodev2.h. I think maintaining controls would be easier if they are moved to e.g. linux/v4l2-controls.h which is included by videodev2.h. Ack. 2) Currently there are three types of controls: standard controls, controls that are specific to a chipset (e.g. cx2341x, mfc51) and driver-specific controls. The controls of the first two types have well defined and unique IDs. For driver-specific controls however there are no clear rules. It all depends on one question: should driver-specific controls have a unique control ID as well, or can they overlap with other drivers? If the answer is that they should be unique as well, then all driver-specific controls will have to be defined in a single header so you can be certain that there is no overlap. If the answer is that they may overlap, then each driver can either define their controls inside their own driver, or in a driver-specific public header. In both cases a control ID range has to be defined for such controls, to ensure that they never clash with standard or chipset-specific control IDs. E.g. = V4L2_CTRL_CLASS_XXX + 0x8000 (no overlap) or + 0xf000 (overlap allowed). My preference is to allow overlap. +1 for allowing overlap, and only when it is expected that some special app will actually use the device specific controls (versus the user changing them in a generic v4l2 control panel app), then the private controls should be defined in a public header. If no such special app is expected, the private controls should be defined inside a private header of the driver. 3) What should VIDIOC_STREAMON/OFF do if the stream is already started/stopped? I believe they should do nothing and just return 0. The main reason for that is it can be useful if an application can just call VIDIOC_STREAMOFF without having to check whether streaming is in progress. +1 for just returning 0 4) What should a driver return in TRY_FMT/S_FMT if the requested format is not supported (possible behaviours include returning the currently selected format or a default format). The spec says this: Drivers should not return an error code unless the input is ambiguous, but it does not explain what constitutes an ambiguous input. Frankly, I can't think of any and in my opinion TRY/S_FMT should never return an error other than EINVAL (if the buffer type is unsupported) or EBUSY (for S_FMT if streaming is in progress). Returning an error for any other reason doesn't help the application since the app will have no way of knowing what to do next. Ack on not returning an error for requesting an unavailable format. As for what the driver should do (default versus current format) I've no preference, I vote for letting this be decided by the driver implementation. 5) VIDIOC_QUERYCAP allows bus_info to be empty. Since the purpose of bus_info is to distinguish multiple identical devices this makes no sense. I propose to make the spec more strict and require that bus_info is always filled in with a unique string. Ack. 6) Deprecate V4L2_BUF_TYPE_PRIVATE. None of the kernel drivers use it, and I cannot see any good use-case for this. If some new type of buffer is needed, then that should be added instead of allowing someone to abuse this buffer type. Ack. 7) A driver that has both a video node and a vbi node (for example) that uses the same struct v4l2_ioctl_ops for both nodes will have to check in e.g. vidioc_g_fmt_vid_cap or vidioc_g_fmt_vbi_cap whether it is called from the correct node (video or vbi) and return an error if it isn't. That's an annoying test that can be done in the V4L2 core as well. Especially since few drivers actually test for that. Should such checks be added to the V4L2 core? And if so, should we add some additional VFL types? Currently we have GRABBER (video nodes), VBI, RADIO and SUBDEV. But if we want to do proper core checks, then we would also need OUTPUT, VBI_OUT and M2M. I'm in favor of adding checks to the core. 8) Remove the experimental tag from the following old drivers: VIDEO_TLV320AIC23B USB_STKWEBCAM VIDEO_CX18 VIDEO_CX18_ALSA VIDEO_ZORAN_AVS6EYES DVB_USB_AF9005 MEDIA_TUNER_TEA5761 ACK. Removing this tag from these drivers might be too soon, though: VIDEO_NOON010PC30 VIDEO_OMAP3 I've no opinion on these. 9) What should VIDIOC_G_STD/DV_PRESET/DV_TIMINGS return if the current input or output does not support that particular timing approach? EINVAL? ENODATA? This is relevant for the case where a driver has multiple inputs/outputs where some are SDTV (and support the STD API) and others are HDTV (and support the DV_TIMINGS API). I propose ENODATA.
Re: [Workshop-2011] RFC: V4L2 API ambiguities
Hi, On 08/13/2012 04:52 PM, Hans Verkuil wrote: On Mon August 13 2012 15:13:34 Hans de Goede wrote: Hi, snip 5) How to handle tuner ownership if both a video and radio node share the same tuner? Obvious rules: - Calling S_FREQ, S_TUNER, S_MODULATOR or S_HW_FREQ_SEEK will change owner or return EBUSY if streaming is in progress. That won't work, as there is no such thing as streaming from a radio node, There is, actually: read() for RDS data and alsa streaming (although that might be hard to detect in the case of USB audio). I suggest we go with the simple approach we discussed at our last meeting in your Dutch House: Calling S_FREQ, S_TUNER, S_MODULATOR or S_HW_FREQ_SEEK will make an app the tuner-owner, and *closing* the device handle makes an app release its tuner ownership. If an other app already is the tuner owner -EBUSY is returned. So the ownership is associated with a filehandle? Yes, that is how it works for videobuf streams too, right? The only difference being that with videobuf streams there is an expilict way to release the ownership, where as for tuner ownership there is none, so the ownership is released on device close. - Ditto for STREAMON, read/write and polling for read/write. No, streaming and tuning are 2 different things, if an app does both, it will likely tune before streaming, but in some cases a user may use a streaming only app together with say v4l2-ctl to do the actual tuning. I think keeping things simple here is key. Lets just treat the tuner and stream as 2 separate entities with a separate ownership. That would work provided the ownership is associated with a filehandle. Right. - Ditto for ioctls that expect a valid tuner configuration like QUERYSTD. QUERY is a read only ioctl, so it should not be influenced by any ownership, nor imply ownership. It is definitely influenced by ownership, since if the tuner is in radio mode, then it can't detect a standard. Neither is this necessarily a passive call as some (mostly older) drivers need to switch the receiver to different modes in order to try and detect the current standard. Hmm, then I guess that this call should fail with EBUSY if: The tuner is owned by another app *and* 1) The tuner is in radio mode; or 2) The tuner is in tv mode *and* doing QUERYSTD requires prodding the device snip Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Add core TMC (Traffic Message Channel) support
Hi, Looks good to me. Regards, Hans On 08/10/2012 07:04 PM, Konke Radlow wrote: Signed-off-by: Konke Radlow korad...@gmail.com --- lib/include/libv4l2rds.h| 64 lib/libv4l2rds/libv4l2rds.c | 340 ++- utils/rds-ctl/rds-ctl.cpp | 31 +++- 3 files changed, 432 insertions(+), 3 deletions(-) diff --git a/lib/include/libv4l2rds.h b/lib/include/libv4l2rds.h index 37bdd2f..caefc1a 100644 --- a/lib/include/libv4l2rds.h +++ b/lib/include/libv4l2rds.h @@ -63,6 +63,9 @@ extern C { #define V4L2_RDS_AF 0x800 /* AF (alternative freq) available */ #define V4L2_RDS_ECC 0x1000 /* Extended County Code */ #define V4L2_RDS_LC 0x2000 /* Language Code */ +#define V4L2_RDS_TMC_SG0x4000 /* RDS-TMC single group */ +#define V4L2_RDS_TMC_MG0x8000 /* RDS-TMC multi group */ +#define V4L2_RDS_TMC_SYS 0x1 /* RDS-TMC system information */ /* Define Constants for the state of the RDS decoding process * used to address the relevant bit in the decode_information bitmask */ @@ -76,6 +79,11 @@ extern C { #define V4L2_RDS_FLAG_COMPRESSED 0x04 #define V4L2_RDS_FLAG_STATIC_PTY 0x08 +/* TMC related codes + * used to extract TMC fields from RDS groups */ +#define V4L2_TMC_TUNING_INFO 0x08 +#define V4L2_TMC_SINGLE_GROUP 0x04 + /* struct to encapsulate one complete RDS group */ /* This structure is used internally to store data until a complete RDS * group was received and group id dependent decoding can be done. @@ -137,6 +145,61 @@ struct v4l2_rds_af_set { uint32_t af[MAX_AF_CNT];/* AFs defined in Hz */ }; +/* struct to encapsulate an additional data field in a TMC message */ +struct v4l2_tmc_additional { + uint8_t label; + uint16_t data; +}; + +/* struct to encapsulate an arbitrary number of additional data fields + * belonging to one TMC message */ +struct v4l2_tmc_additional_set { + uint8_t size; + /* 28 is the maximal possible number of fields. Additional data +* is limited to 112 bit, and the smallest optional tuple has +* a size of 4 bit (4 bit identifier + 0 bits of data) */ + struct v4l2_tmc_additional fields[28]; +}; + +/* struct to encapsulate a decoded TMC message with optional additional + * data field (in case of a multi-group TMC message) */ +struct v4l2_rds_tmc_msg { + uint8_t length; /* length of multi-group message (0..4) */ + uint8_t sid;/* service identifier at time of reception */ + uint8_t extent; + uint8_t dp; /* duration and persistence */ + uint16_t event; /* TMC event code */ + uint16_t location; /* TMC event location */ + bool follow_diversion; /* indicates if the driver is adviced to +* follow the diversion */ + bool neg_direction; /* indicates negative / positive direction */ + + /* decoded additional information (only available in multi-group +* messages) */ + struct v4l2_tmc_additional_set additional; +}; + +/* struct to encapsulate all TMC related information, including TMC System + * Information, TMC Tuning information and a buffer for the last decoded + * TMC messages */ +struct v4l2_rds_tmc { + uint8_t ltn;/* location_table_number */ + bool afi; /* alternative frequency indicator */ + bool enhanced_mode; /* mode of transmission, +* if false - basic = gaps between tmc groups +* gap defines timing behavior +* if true - enhanced = t_a, t_w and t_d +* define timing behavior of tmc groups */ + uint8_t mgs;/* message geographical scope */ + uint8_t sid;/* service identifier (unique ID on national level) */ + uint8_t gap;/* Gap parameters */ + uint8_t t_a;/* activity time (only if mode = enhanced) */ + uint8_t t_w;/* window time (only if mode = enhanced */ + uint8_t t_d;/* delay time (only if mode = enhanced */ + uint8_t spn[9]; /* service provider name */ + struct v4l2_rds_tmc_msg tmc_msg; +}; + /* struct to encapsulate state and RDS information for current decoding process */ /* This is the structure that will be used by external applications, to * communicate with the library and get access to RDS data */ @@ -172,6 +235,7 @@ struct v4l2_rds { struct v4l2_rds_statistics rds_statistics; struct v4l2_rds_oda_set rds_oda;/* Open Data Services */ struct v4l2_rds_af_set rds_af; /* Alternative Frequencies */ + struct v4l2_rds_tmc tmc;/* TMC information */ }; /* v4l2_rds_init() - initializes a new decoding process diff --git a/lib/libv4l2rds/libv4l2rds.c
Re: [PATCH 1/2] radio-shark: Only compile led support when CONFIG_LED_CLASS is set
Hi, On 08/10/2012 10:15 PM, Mauro Carvalho Chehab wrote: Em 10-08-2012 16:58, Hans de Goede escreveu: Reported-by: Dadiv Rientjes rient...@google.com Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/radio-shark.c | 26 -- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/drivers/media/radio/radio-shark.c b/drivers/media/radio/radio-shark.c index c2ead23..f746ed0 100644 --- a/drivers/media/radio/radio-shark.c +++ b/drivers/media/radio/radio-shark.c @@ -27,7 +27,6 @@ #include linux/init.h #include linux/kernel.h -#include linux/leds.h #include linux/module.h #include linux/slab.h #include linux/usb.h @@ -35,6 +34,12 @@ #include media/v4l2-device.h #include sound/tea575x-tuner.h +#if defined(CONFIG_LEDS_CLASS) || \ +(defined(CONFIG_LEDS_CLASS_MODULE) defined(CONFIG_RADIO_SHARK_MODULE)) +#include linux/leds.h Conditionally including headers is not a good thing. Ok, I will make it unconditional then. ... static void usb_shark_disconnect(struct usb_interface *intf) { struct v4l2_device *v4l2_dev = usb_get_intfdata(intf); struct shark_device *shark = v4l2_dev_to_shark(v4l2_dev); +#ifdef SHARK_USE_LEDS int i; +#endif mutex_lock(shark-tea.mutex); v4l2_device_disconnect(shark-v4l2_dev); snd_tea575x_exit(shark-tea); mutex_unlock(shark-tea.mutex); +#ifdef SHARK_USE_LEDS for (i = 0; i NO_LEDS; i++) led_classdev_unregister(shark-leds[i]); +#endif v4l2_device_put(shark-v4l2_dev); } That looks ugly. Agreed. Maybe you could code it on a different way. You could be move all shark_use_leds together into the same place at the code, like: Sounds good, although I will still need to set a SHARK_USE_LEDS define at the top of the file, to avoid having unused members in struct shark_device, not that the compiler will complain but having them there for no good reason seems undesirable. #if defined(CONFIG_LEDS_CLASS) || \ (defined(CONFIG_LEDS_CLASS_MODULE) defined(CONFIG_RADIO_SHARK_MODULE)) static void shark_led_set_blue(struct led_classdev *led_cdev, ... .brightness_set = shark_led_set_red, }, }; static void shark_led_disconnect(...) { ... } static void shark_led_release(...) { ... } static void shark_led_register(...) { ... } #else static inline void shark_led_disconnect(...) { }; static inline void shark_led_release(...) { }; static inline void shark_led_register(...) { printk(KERN_WARN radio-shark: CONFIG_LED_CLASS not enabled. LEDs won't work\n); } #endif And let the rest of the code to call the shark_led functions, as if LEDS aren't enabled, the function stubs won't produce any code (well, except for the above error notice). The same comment also applies to patch 2. Ok new versions of the 2 patches coming up (should be there in 2 hours or so, I want to both compile and run-time test them both with / without leds). Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] radio-shark*: Remove work-around for dangling pointer in usb intfdata
Recent kernels properly clear the usb intfdata pointer when another driver fails to bind (in the radio-shark* case the usbhid driver would try to bind first. Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/radio-shark.c | 9 - drivers/media/radio/radio-shark2.c | 9 - 2 files changed, 18 deletions(-) diff --git a/drivers/media/radio/radio-shark.c b/drivers/media/radio/radio-shark.c index c2ead23..6117132 100644 --- a/drivers/media/radio/radio-shark.c +++ b/drivers/media/radio/radio-shark.c @@ -286,15 +286,6 @@ static int usb_shark_probe(struct usb_interface *intf, if (!shark-transfer_buffer) goto err_alloc_buffer; - /* -* Work around a bug in usbhid/hid-core.c, where it leaves a dangling -* pointer in intfdata causing v4l2-device.c to not set it. Which -* results in usb_shark_disconnect() referencing the dangling pointer -* -* REMOVE (as soon as the above bug is fixed, patch submitted) -*/ - usb_set_intfdata(intf, NULL); - shark-v4l2_dev.release = usb_shark_release; v4l2_device_set_name(shark-v4l2_dev, DRV_NAME, shark_instance); retval = v4l2_device_register(intf-dev, shark-v4l2_dev); diff --git a/drivers/media/radio/radio-shark2.c b/drivers/media/radio/radio-shark2.c index b9575de..fc0289d 100644 --- a/drivers/media/radio/radio-shark2.c +++ b/drivers/media/radio/radio-shark2.c @@ -258,15 +258,6 @@ static int usb_shark_probe(struct usb_interface *intf, if (!shark-transfer_buffer) goto err_alloc_buffer; - /* -* Work around a bug in usbhid/hid-core.c, where it leaves a dangling -* pointer in intfdata causing v4l2-device.c to not set it. Which -* results in usb_shark_disconnect() referencing the dangling pointer -* -* REMOVE (as soon as the above bug is fixed, patch submitted) -*/ - usb_set_intfdata(intf, NULL); - shark-v4l2_dev.release = usb_shark_release; v4l2_device_set_name(shark-v4l2_dev, DRV_NAME, shark_instance); retval = v4l2_device_register(intf-dev, shark-v4l2_dev); -- 1.7.11.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
[PATCH 4/4] radio-shark2: Only compile led support when CONFIG_LED_CLASS is set
Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/radio-shark2.c | 122 ++--- 1 file changed, 73 insertions(+), 49 deletions(-) diff --git a/drivers/media/radio/radio-shark2.c b/drivers/media/radio/radio-shark2.c index 217483c..7b4efdf 100644 --- a/drivers/media/radio/radio-shark2.c +++ b/drivers/media/radio/radio-shark2.c @@ -35,6 +35,11 @@ #include media/v4l2-device.h #include radio-tea5777.h +#if defined(CONFIG_LEDS_CLASS) || \ +(defined(CONFIG_LEDS_CLASS_MODULE) defined(CONFIG_RADIO_SHARK2_MODULE)) +#define SHARK_USE_LEDS 1 +#endif + MODULE_AUTHOR(Hans de Goede hdego...@redhat.com); MODULE_DESCRIPTION(Griffin radioSHARK2, USB radio receiver driver); MODULE_LICENSE(GPL); @@ -43,7 +48,6 @@ static int debug; module_param(debug, int, 0); MODULE_PARM_DESC(debug, Debug level (0-1)); - #define SHARK_IN_EP0x83 #define SHARK_OUT_EP 0x05 @@ -54,36 +58,18 @@ MODULE_PARM_DESC(debug, Debug level (0-1)); enum { BLUE_LED, RED_LED, NO_LEDS }; -static void shark_led_set_blue(struct led_classdev *led_cdev, - enum led_brightness value); -static void shark_led_set_red(struct led_classdev *led_cdev, - enum led_brightness value); - -static const struct led_classdev shark_led_templates[NO_LEDS] = { - [BLUE_LED] = { - .name = %s:blue:, - .brightness = LED_OFF, - .max_brightness = 127, - .brightness_set = shark_led_set_blue, - }, - [RED_LED] = { - .name = %s:red:, - .brightness = LED_OFF, - .max_brightness = 1, - .brightness_set = shark_led_set_red, - }, -}; - struct shark_device { struct usb_device *usbdev; struct v4l2_device v4l2_dev; struct radio_tea5777 tea; +#ifdef SHARK_USE_LEDS struct work_struct led_work; struct led_classdev leds[NO_LEDS]; char led_names[NO_LEDS][32]; atomic_t brightness[NO_LEDS]; unsigned long brightness_new; +#endif u8 *transfer_buffer; }; @@ -161,6 +147,7 @@ static struct radio_tea5777_ops shark_tea_ops = { .read_reg = shark_read_reg, }; +#ifdef SHARK_USE_LEDS static void shark_led_work(struct work_struct *work) { struct shark_device *shark = @@ -208,21 +195,72 @@ static void shark_led_set_red(struct led_classdev *led_cdev, schedule_work(shark-led_work); } +static const struct led_classdev shark_led_templates[NO_LEDS] = { + [BLUE_LED] = { + .name = %s:blue:, + .brightness = LED_OFF, + .max_brightness = 127, + .brightness_set = shark_led_set_blue, + }, + [RED_LED] = { + .name = %s:red:, + .brightness = LED_OFF, + .max_brightness = 1, + .brightness_set = shark_led_set_red, + }, +}; + +static int shark_register_leds(struct shark_device *shark, struct device *dev) +{ + int i, retval; + + INIT_WORK(shark-led_work, shark_led_work); + for (i = 0; i NO_LEDS; i++) { + shark-leds[i] = shark_led_templates[i]; + snprintf(shark-led_names[i], sizeof(shark-led_names[0]), +shark-leds[i].name, shark-v4l2_dev.name); + shark-leds[i].name = shark-led_names[i]; + retval = led_classdev_register(dev, shark-leds[i]); + if (retval) { + v4l2_err(shark-v4l2_dev, +couldn't register led: %s\n, +shark-led_names[i]); + return retval; + } + } + return 0; +} + +static void shark_unregister_leds(struct shark_device *shark) +{ + int i; + + for (i = 0; i NO_LEDS; i++) + led_classdev_unregister(shark-leds[i]); + + cancel_work_sync(shark-led_work); +} +#else +static int shark_register_leds(struct shark_device *shark, struct device *dev) +{ + v4l2_warn(shark-v4l2_dev, + CONFIG_LED_CLASS not enabled, LED support disabled\n); + return 0; +} +static inline void shark_unregister_leds(struct shark_device *shark) { } +#endif + static void usb_shark_disconnect(struct usb_interface *intf) { struct v4l2_device *v4l2_dev = usb_get_intfdata(intf); struct shark_device *shark = v4l2_dev_to_shark(v4l2_dev); - int i; mutex_lock(shark-tea.mutex); v4l2_device_disconnect(shark-v4l2_dev); radio_tea5777_exit(shark-tea); mutex_unlock(shark-tea.mutex); - for (i = 0; i NO_LEDS; i++) - led_classdev_unregister(shark-leds[i]); - - cancel_work_sync(shark-led_work); + shark_unregister_leds(shark); v4l2_device_put(shark-v4l2_dev); } @@ -240,7 +278,7 @@ static int
[PATCH 3/4] radio-shark: Only compile led support when CONFIG_LED_CLASS is set
Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/radio-shark.c | 135 ++ 1 file changed, 79 insertions(+), 56 deletions(-) diff --git a/drivers/media/radio/radio-shark.c b/drivers/media/radio/radio-shark.c index 05e12bf..e1970bf 100644 --- a/drivers/media/radio/radio-shark.c +++ b/drivers/media/radio/radio-shark.c @@ -35,6 +35,11 @@ #include media/v4l2-device.h #include sound/tea575x-tuner.h +#if defined(CONFIG_LEDS_CLASS) || \ +(defined(CONFIG_LEDS_CLASS_MODULE) defined(CONFIG_RADIO_SHARK_MODULE)) +#define SHARK_USE_LEDS 1 +#endif + /* * Version Information */ @@ -56,44 +61,18 @@ MODULE_LICENSE(GPL); enum { BLUE_LED, BLUE_PULSE_LED, RED_LED, NO_LEDS }; -static void shark_led_set_blue(struct led_classdev *led_cdev, - enum led_brightness value); -static void shark_led_set_blue_pulse(struct led_classdev *led_cdev, -enum led_brightness value); -static void shark_led_set_red(struct led_classdev *led_cdev, - enum led_brightness value); - -static const struct led_classdev shark_led_templates[NO_LEDS] = { - [BLUE_LED] = { - .name = %s:blue:, - .brightness = LED_OFF, - .max_brightness = 127, - .brightness_set = shark_led_set_blue, - }, - [BLUE_PULSE_LED] = { - .name = %s:blue-pulse:, - .brightness = LED_OFF, - .max_brightness = 255, - .brightness_set = shark_led_set_blue_pulse, - }, - [RED_LED] = { - .name = %s:red:, - .brightness = LED_OFF, - .max_brightness = 1, - .brightness_set = shark_led_set_red, - }, -}; - struct shark_device { struct usb_device *usbdev; struct v4l2_device v4l2_dev; struct snd_tea575x tea; +#ifdef SHARK_USE_LEDS struct work_struct led_work; struct led_classdev leds[NO_LEDS]; char led_names[NO_LEDS][32]; atomic_t brightness[NO_LEDS]; unsigned long brightness_new; +#endif u8 *transfer_buffer; u32 last_val; @@ -175,6 +154,7 @@ static struct snd_tea575x_ops shark_tea_ops = { .read_val = shark_read_val, }; +#ifdef SHARK_USE_LEDS static void shark_led_work(struct work_struct *work) { struct shark_device *shark = @@ -235,21 +215,78 @@ static void shark_led_set_red(struct led_classdev *led_cdev, schedule_work(shark-led_work); } +static const struct led_classdev shark_led_templates[NO_LEDS] = { + [BLUE_LED] = { + .name = %s:blue:, + .brightness = LED_OFF, + .max_brightness = 127, + .brightness_set = shark_led_set_blue, + }, + [BLUE_PULSE_LED] = { + .name = %s:blue-pulse:, + .brightness = LED_OFF, + .max_brightness = 255, + .brightness_set = shark_led_set_blue_pulse, + }, + [RED_LED] = { + .name = %s:red:, + .brightness = LED_OFF, + .max_brightness = 1, + .brightness_set = shark_led_set_red, + }, +}; + +static int shark_register_leds(struct shark_device *shark, struct device *dev) +{ + int i, retval; + + INIT_WORK(shark-led_work, shark_led_work); + for (i = 0; i NO_LEDS; i++) { + shark-leds[i] = shark_led_templates[i]; + snprintf(shark-led_names[i], sizeof(shark-led_names[0]), +shark-leds[i].name, shark-v4l2_dev.name); + shark-leds[i].name = shark-led_names[i]; + retval = led_classdev_register(dev, shark-leds[i]); + if (retval) { + v4l2_err(shark-v4l2_dev, +couldn't register led: %s\n, +shark-led_names[i]); + return retval; + } + } + return 0; +} + +static void shark_unregister_leds(struct shark_device *shark) +{ + int i; + + for (i = 0; i NO_LEDS; i++) + led_classdev_unregister(shark-leds[i]); + + cancel_work_sync(shark-led_work); +} +#else +static int shark_register_leds(struct shark_device *shark, struct device *dev) +{ + v4l2_warn(shark-v4l2_dev, + CONFIG_LED_CLASS not enabled, LED support disabled\n); + return 0; +} +static inline void shark_unregister_leds(struct shark_device *shark) { } +#endif + static void usb_shark_disconnect(struct usb_interface *intf) { struct v4l2_device *v4l2_dev = usb_get_intfdata(intf); struct shark_device *shark = v4l2_dev_to_shark(v4l2_dev); - int i; mutex_lock(shark-tea.mutex); v4l2_device_disconnect(shark-v4l2_dev); snd_tea575x_exit(shark
[PATCH 0/4] radio-shark*: Only compile led support when CONFIG_LED_CLA
Hi All, Here is the second revision of my patch-set to fix the build breakage when the radio-shark* drivers are enabled and CONFIG_LED_CLASS is not enabled. This new version introduces 2 new cleanup / preparation patches, and take into account the remarks from Mauro's review of v1. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] radio-shark*: Call cancel_work_sync from disconnect rather then release
This removes the need for shark_led_work to take the v4l2 lock. Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/radio-shark.c | 13 ++--- drivers/media/radio/radio-shark2.c | 12 ++-- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/drivers/media/radio/radio-shark.c b/drivers/media/radio/radio-shark.c index 6117132..05e12bf 100644 --- a/drivers/media/radio/radio-shark.c +++ b/drivers/media/radio/radio-shark.c @@ -181,14 +181,6 @@ static void shark_led_work(struct work_struct *work) container_of(work, struct shark_device, led_work); int i, res, brightness, actual_len; - /* -* We use the v4l2_dev lock and registered bit to ensure the device -* does not get unplugged and unreffed while we're running. -*/ - mutex_lock(shark-tea.mutex); - if (!video_is_registered(shark-tea.vd)) - goto leave; - for (i = 0; i 3; i++) { if (!test_and_clear_bit(i, shark-brightness_new)) continue; @@ -208,8 +200,6 @@ static void shark_led_work(struct work_struct *work) v4l2_err(shark-v4l2_dev, set LED %s error: %d\n, shark-led_names[i], res); } -leave: - mutex_unlock(shark-tea.mutex); } static void shark_led_set_blue(struct led_classdev *led_cdev, @@ -259,6 +249,8 @@ static void usb_shark_disconnect(struct usb_interface *intf) for (i = 0; i NO_LEDS; i++) led_classdev_unregister(shark-leds[i]); + cancel_work_sync(shark-led_work); + v4l2_device_put(shark-v4l2_dev); } @@ -266,7 +258,6 @@ static void usb_shark_release(struct v4l2_device *v4l2_dev) { struct shark_device *shark = v4l2_dev_to_shark(v4l2_dev); - cancel_work_sync(shark-led_work); v4l2_device_unregister(shark-v4l2_dev); kfree(shark-transfer_buffer); kfree(shark); diff --git a/drivers/media/radio/radio-shark2.c b/drivers/media/radio/radio-shark2.c index fc0289d..217483c 100644 --- a/drivers/media/radio/radio-shark2.c +++ b/drivers/media/radio/radio-shark2.c @@ -166,13 +166,6 @@ static void shark_led_work(struct work_struct *work) struct shark_device *shark = container_of(work, struct shark_device, led_work); int i, res, brightness, actual_len; - /* -* We use the v4l2_dev lock and registered bit to ensure the device -* does not get unplugged and unreffed while we're running. -*/ - mutex_lock(shark-tea.mutex); - if (!video_is_registered(shark-tea.vd)) - goto leave; for (i = 0; i 2; i++) { if (!test_and_clear_bit(i, shark-brightness_new)) @@ -191,8 +184,6 @@ static void shark_led_work(struct work_struct *work) v4l2_err(shark-v4l2_dev, set LED %s error: %d\n, shark-led_names[i], res); } -leave: - mutex_unlock(shark-tea.mutex); } static void shark_led_set_blue(struct led_classdev *led_cdev, @@ -231,6 +222,8 @@ static void usb_shark_disconnect(struct usb_interface *intf) for (i = 0; i NO_LEDS; i++) led_classdev_unregister(shark-leds[i]); + cancel_work_sync(shark-led_work); + v4l2_device_put(shark-v4l2_dev); } @@ -238,7 +231,6 @@ static void usb_shark_release(struct v4l2_device *v4l2_dev) { struct shark_device *shark = v4l2_dev_to_shark(v4l2_dev); - cancel_work_sync(shark-led_work); v4l2_device_unregister(shark-v4l2_dev); kfree(shark-transfer_buffer); kfree(shark); -- 1.7.11.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: [GIT PULL for 3.6-rc1] media updates part 2
Hi, On 08/09/2012 10:03 PM, David Rientjes wrote: On Thu, 9 Aug 2012, Mauro Carvalho Chehab wrote: Yeah, that would work as well, although the code would look uglier. IMHO, using select/depend is better. Agreed, I think it should be depends on LEDS_CLASS rather than select it if there is a hard dependency that cannot be fixed with extracting the led support in the driver to #ifdef CONFIG_LEDS_CLASS code. The led support could be #ifdef CONFIG_LEDS_CLASS, the problem with that approach is the whole module versus build-in thing: led-class shark enable-led-support build-inbuild-inyes build-inmodule yes module build-inno module module yes Now this can be coded into #ifdef magic, but it won't be pretty, of course we only need the non pretty version once at the top to set a SHARK_USE_LEDS define, but still. I'm fine with either solution (depends or ifdef magic), although I think that people will get unpleasantly surprised if they want to use the shark driver and they don't get to see it in the menu because they don't have leds enabled. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/2] Add libv4l2rds library (with changes proposed in RFC)
Hi, On 08/09/2012 02:14 PM, Hans Verkuil wrote: On Thu August 9 2012 13:58:07 Hans de Goede wrote: Hi Konke, As Gregor already mentioned there is no need to define libv4l2rdssubdir in configure.ac , so please drop that. Other then that I've some minor remarks (comments inline), with all those fixed, this one is could to go. So hopefully the next version can be added to git master! On 08/07/2012 05:11 PM, Konke Radlow wrote: --- Makefile.am |3 +- configure.ac|7 +- lib/include/libv4l2rds.h| 228 ++ lib/libv4l2rds/Makefile.am | 11 + lib/libv4l2rds/libv4l2rds.c | 953 +++ lib/libv4l2rds/libv4l2rds.pc.in | 11 + 6 files changed, 1211 insertions(+), 2 deletions(-) create mode 100644 lib/include/libv4l2rds.h create mode 100644 lib/libv4l2rds/Makefile.am create mode 100644 lib/libv4l2rds/libv4l2rds.c create mode 100644 lib/libv4l2rds/libv4l2rds.pc.in snip diff --git a/lib/include/libv4l2rds.h b/lib/include/libv4l2rds.h new file mode 100644 index 000..4aa8593 --- /dev/null +++ b/lib/include/libv4l2rds.h @@ -0,0 +1,228 @@ +/* + * Copyright 2012 Cisco Systems, Inc. and/or its affiliates. All rights reserved. + * Author: Konke Radlow korad...@gmail.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; either version 2.1 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA 02110-1335 USA + */ + +#ifndef __LIBV4L2RDS +#define __LIBV4L2RDS + +#include errno.h +#include stdio.h +#include stdlib.h +#include string.h +#include stdbool.h +#include unistd.h +#include stdint.h +#include time.h +#include sys/types.h +#include sys/mman.h +#include config.h You should never include config.h in a public header, also are all the headers really needed for the prototypes in this header? I don't think so! Please move all the unneeded ones to the libv4l2rds.c file! + +#include linux/videodev2.h + +#ifdef __cplusplus +extern C { +#endif /* __cplusplus */ + +#if HAVE_VISIBILITY +#define LIBV4L_PUBLIC __attribute__ ((visibility(default))) +#else +#define LIBV4L_PUBLIC +#endif + +/* used to define the current version (version field) of the v4l2_rds struct */ +#define V4L2_RDS_VERSION (1) + What is the purpose of this field? Once we've released a v4l-utils with this library we are stuck to the API we've defined, having a version field changing it, won't stop us from breaking existing apps, so once we've an official release we simply cannot make ABI breaking changes, which is why most of my review sofar has concentrated on the API side :) I suggest dropping this define and the version field from the struct. I think it is useful, actually. The v4l2_rds struct is allocated by the v4l2_rds_create so at least in theory it is possible to extend the struct in the future without breaking existing apps, provided you have a version number to check. I disagree, if it gets extended only, then existing apps will just work, if an apps gets compiled against a newer version with the extension then it is safe to assume it will run against that newer version. The only reason I can see a version define being useful is to make a newer app compile with an older version of the librarry, but that only requires a version define, not a version field in the struct. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/2] Add libv4l2rds library (with changes proposed in RFC)
Hi, On 08/10/2012 09:36 AM, Hans Verkuil wrote: On Fri 10 August 2012 09:16:34 Hans de Goede wrote: Hi, On 08/09/2012 02:14 PM, Hans Verkuil wrote: On Thu August 9 2012 13:58:07 Hans de Goede wrote: Hi Konke, As Gregor already mentioned there is no need to define libv4l2rdssubdir in configure.ac , so please drop that. Other then that I've some minor remarks (comments inline), with all those fixed, this one is could to go. So hopefully the next version can be added to git master! On 08/07/2012 05:11 PM, Konke Radlow wrote: --- Makefile.am |3 +- configure.ac|7 +- lib/include/libv4l2rds.h| 228 ++ lib/libv4l2rds/Makefile.am | 11 + lib/libv4l2rds/libv4l2rds.c | 953 +++ lib/libv4l2rds/libv4l2rds.pc.in | 11 + 6 files changed, 1211 insertions(+), 2 deletions(-) create mode 100644 lib/include/libv4l2rds.h create mode 100644 lib/libv4l2rds/Makefile.am create mode 100644 lib/libv4l2rds/libv4l2rds.c create mode 100644 lib/libv4l2rds/libv4l2rds.pc.in snip diff --git a/lib/include/libv4l2rds.h b/lib/include/libv4l2rds.h new file mode 100644 index 000..4aa8593 --- /dev/null +++ b/lib/include/libv4l2rds.h @@ -0,0 +1,228 @@ +/* + * Copyright 2012 Cisco Systems, Inc. and/or its affiliates. All rights reserved. + * Author: Konke Radlow korad...@gmail.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; either version 2.1 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA 02110-1335 USA + */ + +#ifndef __LIBV4L2RDS +#define __LIBV4L2RDS + +#include errno.h +#include stdio.h +#include stdlib.h +#include string.h +#include stdbool.h +#include unistd.h +#include stdint.h +#include time.h +#include sys/types.h +#include sys/mman.h +#include config.h You should never include config.h in a public header, also are all the headers really needed for the prototypes in this header? I don't think so! Please move all the unneeded ones to the libv4l2rds.c file! + +#include linux/videodev2.h + +#ifdef __cplusplus +extern C { +#endif /* __cplusplus */ + +#if HAVE_VISIBILITY +#define LIBV4L_PUBLIC __attribute__ ((visibility(default))) +#else +#define LIBV4L_PUBLIC +#endif + +/* used to define the current version (version field) of the v4l2_rds struct */ +#define V4L2_RDS_VERSION (1) + What is the purpose of this field? Once we've released a v4l-utils with this library we are stuck to the API we've defined, having a version field changing it, won't stop us from breaking existing apps, so once we've an official release we simply cannot make ABI breaking changes, which is why most of my review sofar has concentrated on the API side :) I suggest dropping this define and the version field from the struct. I think it is useful, actually. The v4l2_rds struct is allocated by the v4l2_rds_create so at least in theory it is possible to extend the struct in the future without breaking existing apps, provided you have a version number to check. I disagree, if it gets extended only, then existing apps will just work, if an apps gets compiled against a newer version with the extension then it is safe to assume it will run against that newer version. The only reason I can see a version define being useful is to make a newer app compile with an older version of the librarry, but that only requires a version define, not a version field in the struct. That's true, you only need the define, not the version field. So let's keep the define and ditch the version field. I think that should do it. Ack. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] radio-shark2: Only compile led support when CONFIG_LED_CLASS is set
Reported-by: Dadiv Rientjes rient...@google.com Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/radio-shark2.c | 27 --- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/drivers/media/radio/radio-shark2.c b/drivers/media/radio/radio-shark2.c index b9575de..e593d5a 100644 --- a/drivers/media/radio/radio-shark2.c +++ b/drivers/media/radio/radio-shark2.c @@ -27,7 +27,6 @@ #include linux/init.h #include linux/kernel.h -#include linux/leds.h #include linux/module.h #include linux/slab.h #include linux/usb.h @@ -35,6 +34,12 @@ #include media/v4l2-device.h #include radio-tea5777.h +#if defined(CONFIG_LEDS_CLASS) || \ +(defined(CONFIG_LEDS_CLASS_MODULE) defined(CONFIG_RADIO_SHARK2_MODULE)) +#include linux/leds.h +#define SHARK_USE_LEDS 1 +#endif + MODULE_AUTHOR(Hans de Goede hdego...@redhat.com); MODULE_DESCRIPTION(Griffin radioSHARK2, USB radio receiver driver); MODULE_LICENSE(GPL); @@ -43,7 +48,6 @@ static int debug; module_param(debug, int, 0); MODULE_PARM_DESC(debug, Debug level (0-1)); - #define SHARK_IN_EP0x83 #define SHARK_OUT_EP 0x05 @@ -52,6 +56,7 @@ MODULE_PARM_DESC(debug, Debug level (0-1)); #define v4l2_dev_to_shark(d) container_of(d, struct shark_device, v4l2_dev) +#ifdef SHARK_USE_LEDS enum { BLUE_LED, RED_LED, NO_LEDS }; static void shark_led_set_blue(struct led_classdev *led_cdev, @@ -73,17 +78,20 @@ static const struct led_classdev shark_led_templates[NO_LEDS] = { .brightness_set = shark_led_set_red, }, }; +#endif struct shark_device { struct usb_device *usbdev; struct v4l2_device v4l2_dev; struct radio_tea5777 tea; +#ifdef SHARK_USE_LEDS struct work_struct led_work; struct led_classdev leds[NO_LEDS]; char led_names[NO_LEDS][32]; atomic_t brightness[NO_LEDS]; unsigned long brightness_new; +#endif u8 *transfer_buffer; }; @@ -161,6 +169,7 @@ static struct radio_tea5777_ops shark_tea_ops = { .read_reg = shark_read_reg, }; +#ifdef SHARK_USE_LEDS static void shark_led_work(struct work_struct *work) { struct shark_device *shark = @@ -216,20 +225,25 @@ static void shark_led_set_red(struct led_classdev *led_cdev, set_bit(RED_LED, shark-brightness_new); schedule_work(shark-led_work); } +#endif static void usb_shark_disconnect(struct usb_interface *intf) { struct v4l2_device *v4l2_dev = usb_get_intfdata(intf); struct shark_device *shark = v4l2_dev_to_shark(v4l2_dev); +#ifdef SHARK_USE_LEDS int i; +#endif mutex_lock(shark-tea.mutex); v4l2_device_disconnect(shark-v4l2_dev); radio_tea5777_exit(shark-tea); mutex_unlock(shark-tea.mutex); +#ifdef SHARK_USE_LEDS for (i = 0; i NO_LEDS; i++) led_classdev_unregister(shark-leds[i]); +#endif v4l2_device_put(shark-v4l2_dev); } @@ -238,7 +252,9 @@ static void usb_shark_release(struct v4l2_device *v4l2_dev) { struct shark_device *shark = v4l2_dev_to_shark(v4l2_dev); +#ifdef SHARK_USE_LEDS cancel_work_sync(shark-led_work); +#endif v4l2_device_unregister(shark-v4l2_dev); kfree(shark-transfer_buffer); kfree(shark); @@ -248,7 +264,10 @@ static int usb_shark_probe(struct usb_interface *intf, const struct usb_device_id *id) { struct shark_device *shark; - int i, retval = -ENOMEM; + int retval = -ENOMEM; +#ifdef SHARK_USE_LEDS + int i; +#endif shark = kzalloc(sizeof(struct shark_device), GFP_KERNEL); if (!shark) @@ -292,6 +311,7 @@ static int usb_shark_probe(struct usb_interface *intf, goto err_init_tea; } +#ifdef SHARK_USE_LEDS INIT_WORK(shark-led_work, shark_led_work); for (i = 0; i NO_LEDS; i++) { shark-leds[i] = shark_led_templates[i]; @@ -312,6 +332,7 @@ static int usb_shark_probe(struct usb_interface *intf, couldn't register led: %s\n, shark-led_names[i]); } +#endif return 0; -- 1.7.11.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
[PATCH 1/2] radio-shark: Only compile led support when CONFIG_LED_CLASS is set
Reported-by: Dadiv Rientjes rient...@google.com Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/radio-shark.c | 26 -- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/drivers/media/radio/radio-shark.c b/drivers/media/radio/radio-shark.c index c2ead23..f746ed0 100644 --- a/drivers/media/radio/radio-shark.c +++ b/drivers/media/radio/radio-shark.c @@ -27,7 +27,6 @@ #include linux/init.h #include linux/kernel.h -#include linux/leds.h #include linux/module.h #include linux/slab.h #include linux/usb.h @@ -35,6 +34,12 @@ #include media/v4l2-device.h #include sound/tea575x-tuner.h +#if defined(CONFIG_LEDS_CLASS) || \ +(defined(CONFIG_LEDS_CLASS_MODULE) defined(CONFIG_RADIO_SHARK_MODULE)) +#include linux/leds.h +#define SHARK_USE_LEDS 1 +#endif + /* * Version Information */ @@ -54,6 +59,7 @@ MODULE_LICENSE(GPL); #define v4l2_dev_to_shark(d) container_of(d, struct shark_device, v4l2_dev) +#ifdef SHARK_USE_LEDS enum { BLUE_LED, BLUE_PULSE_LED, RED_LED, NO_LEDS }; static void shark_led_set_blue(struct led_classdev *led_cdev, @@ -83,17 +89,20 @@ static const struct led_classdev shark_led_templates[NO_LEDS] = { .brightness_set = shark_led_set_red, }, }; +#endif struct shark_device { struct usb_device *usbdev; struct v4l2_device v4l2_dev; struct snd_tea575x tea; +#ifdef SHARK_USE_LEDS struct work_struct led_work; struct led_classdev leds[NO_LEDS]; char led_names[NO_LEDS][32]; atomic_t brightness[NO_LEDS]; unsigned long brightness_new; +#endif u8 *transfer_buffer; u32 last_val; @@ -175,6 +184,7 @@ static struct snd_tea575x_ops shark_tea_ops = { .read_val = shark_read_val, }; +#ifdef SHARK_USE_LEDS static void shark_led_work(struct work_struct *work) { struct shark_device *shark = @@ -244,20 +254,25 @@ static void shark_led_set_red(struct led_classdev *led_cdev, set_bit(RED_LED, shark-brightness_new); schedule_work(shark-led_work); } +#endif static void usb_shark_disconnect(struct usb_interface *intf) { struct v4l2_device *v4l2_dev = usb_get_intfdata(intf); struct shark_device *shark = v4l2_dev_to_shark(v4l2_dev); +#ifdef SHARK_USE_LEDS int i; +#endif mutex_lock(shark-tea.mutex); v4l2_device_disconnect(shark-v4l2_dev); snd_tea575x_exit(shark-tea); mutex_unlock(shark-tea.mutex); +#ifdef SHARK_USE_LEDS for (i = 0; i NO_LEDS; i++) led_classdev_unregister(shark-leds[i]); +#endif v4l2_device_put(shark-v4l2_dev); } @@ -266,7 +281,9 @@ static void usb_shark_release(struct v4l2_device *v4l2_dev) { struct shark_device *shark = v4l2_dev_to_shark(v4l2_dev); +#ifdef SHARK_USE_LEDS cancel_work_sync(shark-led_work); +#endif v4l2_device_unregister(shark-v4l2_dev); kfree(shark-transfer_buffer); kfree(shark); @@ -276,7 +293,10 @@ static int usb_shark_probe(struct usb_interface *intf, const struct usb_device_id *id) { struct shark_device *shark; - int i, retval = -ENOMEM; + int retval = -ENOMEM; +#ifdef SHARK_USE_LEDS + int i; +#endif shark = kzalloc(sizeof(struct shark_device), GFP_KERNEL); if (!shark) @@ -321,6 +341,7 @@ static int usb_shark_probe(struct usb_interface *intf, goto err_init_tea; } +#ifdef SHARK_USE_LEDS INIT_WORK(shark-led_work, shark_led_work); for (i = 0; i NO_LEDS; i++) { shark-leds[i] = shark_led_templates[i]; @@ -341,6 +362,7 @@ static int usb_shark_probe(struct usb_interface *intf, couldn't register led: %s\n, shark-led_names[i]); } +#endif return 0; -- 1.7.11.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] pwc: Use vb2 queue mutex through a single name
Hi, Thanks for the patch, I've added it to my tree for 3.7: http://git.linuxtv.org/hgoede/gspca.git/shortlog/refs/heads/media-for_v3.7-wip Regards, Hans On 07/15/2012 08:00 AM, Ezequiel Garcia wrote: This lock was being taken using two different names (pointers) in the same function. Both names refer to the same lock, so this wasn't an error; but it looked very strange. Cc: Hans Verkuil hverk...@xs4all.nl Signed-off-by: Ezequiel Garcia elezegar...@gmail.com --- drivers/media/video/pwc/pwc-if.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/video/pwc/pwc-if.c b/drivers/media/video/pwc/pwc-if.c index de7c7ba..b5d0729 100644 --- a/drivers/media/video/pwc/pwc-if.c +++ b/drivers/media/video/pwc/pwc-if.c @@ -1127,7 +1127,7 @@ static void usb_pwc_disconnect(struct usb_interface *intf) v4l2_device_disconnect(pdev-v4l2_dev); video_unregister_device(pdev-vdev); mutex_unlock(pdev-v4l2_lock); - mutex_unlock(pdev-vb_queue.lock); + mutex_unlock(pdev-vb_queue_lock); #ifdef CONFIG_USB_PWC_INPUT_EVDEV if (pdev-button_dev) -- 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] gspca: dubious one-bit signed bitfield
Hi, Thanks for the patch, I've added it to my tree for 3.7: http://git.linuxtv.org/hgoede/gspca.git/shortlog/refs/heads/media-for_v3.7-wip Regards, Hans On 08/05/2012 02:34 PM, Emil Goode wrote: This patch changes some signed integers to unsigned because they are not intended for negative values and sparse is making noise about it. Sparse gives eight of these errors: drivers/media/video/gspca/ov519.c:144:29: error: dubious one-bit signed bitfield Signed-off-by: Emil Goode emilgo...@gmail.com --- drivers/media/video/gspca/ov519.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/media/video/gspca/ov519.c b/drivers/media/video/gspca/ov519.c index bfc7cef..c1a21bf 100644 --- a/drivers/media/video/gspca/ov519.c +++ b/drivers/media/video/gspca/ov519.c @@ -141,14 +141,14 @@ enum sensors { /* table of the disabled controls */ struct ctrl_valid { - int has_brightness:1; - int has_contrast:1; - int has_exposure:1; - int has_autogain:1; - int has_sat:1; - int has_hvflip:1; - int has_autobright:1; - int has_freq:1; + unsigned int has_brightness:1; + unsigned int has_contrast:1; + unsigned int has_exposure:1; + unsigned int has_autogain:1; + unsigned int has_sat:1; + unsigned int has_hvflip:1; + unsigned int has_autobright:1; + unsigned int has_freq:1; }; static const struct ctrl_valid valid_controls[] = { -- 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 PATCH 1/2] Add libv4l2rds library (with changes proposed in RFC)
Hi Konke, As Gregor already mentioned there is no need to define libv4l2rdssubdir in configure.ac , so please drop that. Other then that I've some minor remarks (comments inline), with all those fixed, this one is could to go. So hopefully the next version can be added to git master! On 08/07/2012 05:11 PM, Konke Radlow wrote: --- Makefile.am |3 +- configure.ac|7 +- lib/include/libv4l2rds.h| 228 ++ lib/libv4l2rds/Makefile.am | 11 + lib/libv4l2rds/libv4l2rds.c | 953 +++ lib/libv4l2rds/libv4l2rds.pc.in | 11 + 6 files changed, 1211 insertions(+), 2 deletions(-) create mode 100644 lib/include/libv4l2rds.h create mode 100644 lib/libv4l2rds/Makefile.am create mode 100644 lib/libv4l2rds/libv4l2rds.c create mode 100644 lib/libv4l2rds/libv4l2rds.pc.in snip diff --git a/lib/include/libv4l2rds.h b/lib/include/libv4l2rds.h new file mode 100644 index 000..4aa8593 --- /dev/null +++ b/lib/include/libv4l2rds.h @@ -0,0 +1,228 @@ +/* + * Copyright 2012 Cisco Systems, Inc. and/or its affiliates. All rights reserved. + * Author: Konke Radlow korad...@gmail.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; either version 2.1 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA 02110-1335 USA + */ + +#ifndef __LIBV4L2RDS +#define __LIBV4L2RDS + +#include errno.h +#include stdio.h +#include stdlib.h +#include string.h +#include stdbool.h +#include unistd.h +#include stdint.h +#include time.h +#include sys/types.h +#include sys/mman.h +#include config.h You should never include config.h in a public header, also are all the headers really needed for the prototypes in this header? I don't think so! Please move all the unneeded ones to the libv4l2rds.c file! + +#include linux/videodev2.h + +#ifdef __cplusplus +extern C { +#endif /* __cplusplus */ + +#if HAVE_VISIBILITY +#define LIBV4L_PUBLIC __attribute__ ((visibility(default))) +#else +#define LIBV4L_PUBLIC +#endif + +/* used to define the current version (version field) of the v4l2_rds struct */ +#define V4L2_RDS_VERSION (1) + What is the purpose of this field? Once we've released a v4l-utils with this library we are stuck to the API we've defined, having a version field changing it, won't stop us from breaking existing apps, so once we've an official release we simply cannot make ABI breaking changes, which is why most of my review sofar has concentrated on the API side :) I suggest dropping this define and the version field from the struct. +/* Constants used to define the size of arrays used to store RDS information */ +#define MAX_ODA_CNT 18 /* there are 16 groups each with type a or b. Of these +* 32 distinct groups, 18 can be used for ODA purposes */ +#define MAX_AF_CNT 25 /* AF Method A allows a maximum of 25 AFs to be defined +* AF Method B does not impose a limit on the number of AFs +* but it is not fully supported at the moment and will +* not receive more than 25 AFs */ + +/* Define Constants for the possible types of RDS information + * used to address the relevant bit in the valid_fields bitmask */ +#define V4L2_RDS_PI0x01/* Program Identification */ +#define V4L2_RDS_PTY 0x02/* Program Type */ +#define V4L2_RDS_TP0x04/* Traffic Program */ +#define V4L2_RDS_PS0x08/* Program Service Name */ +#define V4L2_RDS_TA0x10/* Traffic Announcement */ +#define V4L2_RDS_DI0x20/* Decoder Information */ +#define V4L2_RDS_MS0x40/* Music / Speech flag */ +#define V4L2_RDS_PTYN 0x80/* Program Type Name */ +#define V4L2_RDS_RT0x100 /* Radio-Text */ +#define V4L2_RDS_TIME 0x200 /* Date and Time information */ +#define V4L2_RDS_TMC 0x400 /* TMC availability */ +#define V4L2_RDS_AF0x800 /* AF (alternative freq) available */ +#define V4L2_RDS_ECC 0x1000 /* Extended County Code */ +#define V4L2_RDS_LC0x2000 /* Language Code */ + +/* Define Constants for the state of the RDS decoding process + * used to address the relevant bit in the decode_information bitmask */ +#define V4L2_RDS_GROUP_NEW 0x01/* New group received */
Re: [GIT PULL for 3.6-rc1] media updates part 2
Hi, My bad, sorry about this. Mauro's patch looks good. An alternative fix would be to #ifdefify the led code in the drivers themselves. Regards, Hans On 08/09/2012 01:38 PM, Mauro Carvalho Chehab wrote: Em 08-08-2012 19:28, David Rientjes escreveu: On Tue, 31 Jul 2012, Mauro Carvalho Chehab wrote: [media] radio-shark: New driver for the Griffin radioSHARK USB radio receiver This one gives me a build warning if CONFIG_LEDS_CLASS is disabled: ERROR: led_classdev_register [drivers/media/radio/shark2.ko] undefined! ERROR: led_classdev_unregister [drivers/media/radio/shark2.ko] undefined! Could you please test the enclosed patch? Thanks! Mauro - [media] radio-shark: make sure LEDS_CLASS is selected As reported by David: ERROR: led_classdev_register [drivers/media/radio/shark2.ko] undefined! ERROR: led_classdev_unregister [drivers/media/radio/shark2.ko] undefined! Reported-by: Dadiv Rientjes rient...@google.com Cc: Hans de Goede hdego...@redhat.com Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com diff --git a/drivers/media/radio/Kconfig b/drivers/media/radio/Kconfig index 8090b87..be68ec2 100644 --- a/drivers/media/radio/Kconfig +++ b/drivers/media/radio/Kconfig @@ -60,6 +60,7 @@ config RADIO_MAXIRADIO config RADIO_SHARK tristate Griffin radioSHARK USB radio receiver depends on USB SND + select LEDS_CLASS ---help--- Choose Y here if you have this radio receiver. @@ -77,6 +78,7 @@ config RADIO_SHARK config RADIO_SHARK2 tristate Griffin radioSHARK2 USB radio receiver depends on USB + select LEDS_CLASS ---help--- Choose Y here if you have this radio receiver. -- 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 PATCH 2/2] Add rds-ctl tool (with changes proposed in RFC)
Hi, Comments inline. On 08/07/2012 05:11 PM, Konke Radlow wrote: --- Makefile.am |3 +- configure.ac |1 + utils/rds-ctl/Makefile.am |5 + utils/rds-ctl/rds-ctl.cpp | 959 + 4 files changed, 967 insertions(+), 1 deletion(-) create mode 100644 utils/rds-ctl/Makefile.am create mode 100644 utils/rds-ctl/rds-ctl.cpp diff --git a/Makefile.am b/Makefile.am index 621d8d9..8ef0d00 100644 --- a/Makefile.am +++ b/Makefile.am Snip +static void print_rds_data(const struct v4l2_rds *handle, uint32_t updated_fields) +{ + if (params.options[OptPrintBlock]) + updated_fields = 0x; + + if ((updated_fields V4L2_RDS_PI) + (handle-valid_fields V4L2_RDS_PI)) { + printf(\nPI: %04x, handle-pi); + print_rds_pi(handle); + } + + if (updated_fields V4L2_RDS_PS + handle-valid_fields V4L2_RDS_PS) { + printf(\nPS: ); + for (int i = 0; i 8; ++i) { + /* filter out special characters */ + if (handle-ps[i] = 0x20 handle-ps[i] = 0x7E) + printf(%lc,handle-ps[i]); + else + printf( ); + } Since ps now is a 0 terminated UTF-8 string you should be able to just do: printf(\nPS: %s, handle-ps); And likewise for the other strings. + } + + if (updated_fields V4L2_RDS_PTY handle-valid_fields V4L2_RDS_PTY) + printf(\nPTY: %0u - %s,handle-pty, v4l2_rds_get_pty_str(handle)); + + if (updated_fields V4L2_RDS_PTYN handle-valid_fields V4L2_RDS_PTYN) { + printf(\nPTYN: ); + for (int i = 0; i 8; ++i) { + /* filter out special characters */ + if (handle-ptyn[i] = 0x20 handle-ptyn[i] = 0x7E) + printf(%lc,handle-ptyn[i]); + else + printf( ); + } Likewise. + } + + if (updated_fields V4L2_RDS_TIME) { + printf(\nTime: %s, ctime(handle-time)); + } + if (updated_fields V4L2_RDS_RT handle-valid_fields V4L2_RDS_RT) { + printf(\nRT: ); + for (int i = 0; i handle-rt_length; ++i) { + /* filter out special characters */ + if (handle-rt[i] = 0x20 handle-rt[i] = 0x7E) + printf(%lc,handle-rt[i]); + else + printf( ); + } Likewise. + } + + if (updated_fields V4L2_RDS_TP handle-valid_fields V4L2_RDS_TP) + printf(\nTP: %s TA: %s, (handle-tp)? yes:no, + handle-ta? yes:no); + if (updated_fields V4L2_RDS_MS handle-valid_fields V4L2_RDS_MS) + printf(\nMS Flag: %s, (handle-ms)? Music : Speech); + if (updated_fields V4L2_RDS_ECC handle-valid_fields V4L2_RDS_ECC) + printf(\nECC: %X%x, Country: %u - %s, + handle-ecc 4, handle-ecc 0x0f, handle-pi 12, + v4l2_rds_get_country_str(handle)); + if (updated_fields V4L2_RDS_LC handle-valid_fields V4L2_RDS_LC) + printf(\nLanguage: %u - %s , handle-lc, + v4l2_rds_get_language_str(handle)); + if (updated_fields V4L2_RDS_DI handle-valid_fields V4L2_RDS_DI) + print_decoder_info(handle-di); + if (updated_fields V4L2_RDS_ODA + handle-decode_information V4L2_RDS_ODA) { + for (int i = 0; i handle-rds_oda.size; ++i) + printf(\nODA Group: %02u%c, AID: %08x,handle-rds_oda.oda[i].group_id, + handle-rds_oda.oda[i].group_version, handle-rds_oda.oda[i].aid); + } + if (updated_fields V4L2_RDS_AF handle-valid_fields V4L2_RDS_AF) + print_rds_af(handle-rds_af); + if (params.options[OptPrintBlock]) + printf(\n); +} + +static void read_rds(struct v4l2_rds *handle, const int fd, const int wait_limit) +{ + int byte_cnt = 0; + int error_cnt = 0; + uint32_t updated_fields = 0x00; + struct v4l2_rds_data rds_data; /* read buffer for rds blocks */ + + while (!params.terminate_decoding) { + memset(rds_data, 0, sizeof(rds_data)); + if ((byte_cnt=read(fd, rds_data, 3)) != 3) { + if (byte_cnt == 0) { + printf(\nEnd of input file reached \n); + break; + } else if(++error_cnt 2) { + fprintf(stderr, \nError reading from + device (no RDS data available)\n); +
Re: [PATCH] [media] gspca: dubious one-bit signed bitfield
Hi, On 08/05/2012 02:34 PM, Emil Goode wrote: This patch changes some signed integers to unsigned because they are not intended for negative values and sparse is making noise about it. Sparse gives eight of these errors: drivers/media/video/gspca/ov519.c:144:29: error: dubious one-bit signed bitfield Signed-off-by: Emil Goode emilgo...@gmail.com Thanks, I'll add this to my gspca tree. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: Tree for July 31 (media/radio-tea5777)
Thanks for fixing this for me! Acked-by: Hans de Goede hdego...@redhat.com On 07/31/2012 09:56 PM, Mauro Carvalho Chehab wrote: Em 31-07-2012 14:22, Randy Dunlap escreveu: drivers/built-in.o: In function `radio_tea5777_set_freq': radio-tea5777.c:(.text+0x4d8704): undefined reference to `__udivdi3' The patch below should fix it. Thanks for reporting it! Regards, Mauro [media] radio-tea5777: use library for 64bits div From: Mauro Carvalho Chehab mche...@redhat.com drivers/built-in.o: In function `radio_tea5777_set_freq': radio-tea5777.c:(.text+0x4d8704): undefined reference to `__udivdi3' Reported-by: Randy Dunlap rdun...@xenotime.net Cc: Hans de Goede hdego...@redhat.com Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com diff --git a/drivers/media/radio/radio-tea5777.c b/drivers/media/radio/radio-tea5777.c index 3e12179..5bc9fa6 100644 --- a/drivers/media/radio/radio-tea5777.c +++ b/drivers/media/radio/radio-tea5777.c @@ -33,6 +33,7 @@ #include media/v4l2-fh.h #include media/v4l2-ioctl.h #include media/v4l2-event.h +#include asm/div64.h #include radio-tea5777.h MODULE_AUTHOR(Hans de Goede pe...@perex.cz); @@ -158,10 +159,11 @@ static int radio_tea5777_set_freq(struct radio_tea5777 *tea) int res; freq = clamp_t(u32, tea-freq, - TEA5777_FM_RANGELOW, TEA5777_FM_RANGEHIGH); - freq = (freq + 8) / 16; /* to kHz */ + TEA5777_FM_RANGELOW, TEA5777_FM_RANGEHIGH) + 8; + do_div(freq, 16); /* to kHz */ - freq = (freq - TEA5777_FM_IF) / TEA5777_FM_FREQ_STEP; + freq -= TEA5777_FM_IF; + do_div(freq, TEA5777_FM_FREQ_STEP); tea-write_reg = ~(TEA5777_W_FM_PLL_MASK | TEA5777_W_FM_FREF_MASK); tea-write_reg |= freq TEA5777_W_FM_PLL_SHIFT; -- 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: Advice on extending libv4l for media controller support
Hi, On 07/26/2012 10:49 PM, Robert Abel wrote: Hi, Sorry to be late to the party... I wanted to follow up on this discussion, but forgot and haven't read anything about it since... On 10.05.2012 17:09, Ivan T. Ivanov wrote: On Wed, May 9, 2012 at 7:08 PM, Sergio Aguirre sergio.a.agui...@gmail.com mailto:sergio.a.agui...@gmail.com wrote: I want to create some sort of plugin with specific media controller configurations, to avoid userspace to worry about component names and specific usecases (use sensor resizer, or SoC ISP resizer, etc.). Probably following links can help you. They have been tested with the OMAP3 ISP. Regards, iivanov [1]http://www.spinics.net/lists/linux-media/msg31901.html [2] http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/32704 I recently extended Yordan Kamenov's libv4l-mcplugin to support multiple trees per device with extended configurations (-stolen from- inspired by media-ctl) not tied to specific device nodes (but to device names instead). I uploaded the patches here https://sites.google.com/site/rawbdagslair/libv4l-mcplugin.7z?attredirects=0d=1(16kB). Basically, I used Yordan's patches as a base and worked from there to fix up his source code and Makefile for cross-compiling using OpenEmbedded/Yocto. There are a ton of minor issues with this, starting with the fact that I did not put proper copyright notices in any of these files. Please advise if this poses a problem. Only integral frame size support and no support for native read() calls. There's a dummy read() function, because for some reason this is required in libv4l2 0.9.0-test though it's not mentioned anywhere. As the original plug-in by Yordan, there is currently no cleaning-up of the internal data structures. I used this in conjunction with the Gumstix CASPA FS (MT9V032) camera using some of Laurent's patches and some custom patches which add ENUM_FMT support to the driver. Basically, upon opening a given device, all trees are configured once to load the respective end-point's formats for emulation of setting and getting formats. Then regular format negotiation by the user application takes place. As discussed higher up in this thread, since the initial libv4l-mcplugin was done for the omap3, we've had several meetings on the topic of libv4l and media-controller using devices and we came to the following conclusions: 1) The existing mediactl lib would be extended with a libmediactlvideo lib, which would be able to control media-ctrl video chains, ie it can: -give a list of possibly supported formats / sizes / framerates -setup the chain to deliver a requested format Since the optimal setup will be hardware specific the idea was to give this libs per soc plugins, and a generic plugin for simple socs / as fallback. 2) A cmdline utility to set up a chain using libmediactlvideo, so that things can be tested using raw devices, ie without libv4l2 coming into play, just like apps like v4l2-ctl allow low level control mostly for testing purposes 3) There would then be a libv4l2 plugin much like the above linked omap3 plugin, but then generic for any mediactl using video devices, which would use libmediactlvideo to do the work of setting up the chain (and which will fail to init when the to be opened device is not part of a mediactl controlled chain). And AFAIK some work was done in this direction. Sakari? Laurent? Eitherway it is about time someone started working on this, and I would greatly prefer the above plan to be implemented. Once we have this in place, then we can do a new v4l-utils release which officially supports the plugin API (which currently only lives in master, not in any releases). Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/2] Initial version of the RDS-decoder library Signed-off-by: Konke Radlow krad...@cisco.com
Hi, First of all many thanks for working on this! Note I've also taken a quick look at the original patch with the actual implementation and that looks good. I'm replying here because in my mind the API is the most interesting thing to discuss. Comments inline. On 07/26/2012 06:21 PM, Konke Radlow wrote: PATCH 1/2 was missing the public header for the rds library. I'm sorry for that mistake: diff --git a/lib/include/libv4l2rds.h b/lib/include/libv4l2rds.h new file mode 100644 index 000..04843d3 --- /dev/null +++ b/lib/include/libv4l2rds.h @@ -0,0 +1,203 @@ +/* + * Copyright 2012 Cisco Systems, Inc. and/or its affiliates. All rights reserved. + * Author: Konke Radlow korad...@gmail.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; either version 2.1 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA 02110-1335 USA + */ + +#ifndef __LIBV4L2RDS +#define __LIBV4L2RDS + +#include errno.h +#include stdio.h +#include stdlib.h +#include string.h +#include stdbool.h +#include unistd.h +#include stdint.h +#include sys/types.h +#include sys/mman.h + +#include linux/videodev2.h + +#ifdef __cplusplus +extern C { +#endif /* __cplusplus */ + +#if __GNUC__ = 4 +#define LIBV4L_PUBLIC __attribute__ ((visibility(default))) +#else +#define LIBV4L_PUBLIC +#endif + +/* used to define the current version (version field) of the v4l2_rds struct */ +#define V4L2_RDS_VERSION (1) + +/* Constants used to define the size of arrays used to store RDS information */ +#define MAX_ODA_CNT 18 /* there are 16 groups each with type a or b. Of these +* 32 distinct groups, 18 can be used for ODA purposes*/ +#define MAX_AF_CNT 25 /* AF Method A allows a maximum of 25 AFs to be defined +* AF Method B does not impose a limit on the number of AFs +* but it is not fully supported at the moment and will +* not receive more than 25 AFs */ + +/* Define Constants for the possible types of RDS information + * used to address the relevant bit in the valid_bitmask */ +#define V4L2_RDS_PI0x01/* Program Identification */ +#define V4L2_RDS_PTY 0x02/* Program Type */ +#define V4L2_RDS_TP0x04/* Traffic Program */ +#define V4L2_RDS_PS0x08/* Program Service Name */ +#define V4L2_RDS_TA0x10/* Traffic Announcement */ +#define V4L2_RDS_DI0x20/* Decoder Information */ +#define V4L2_RDS_MS0x40/* Music / Speech code */ +#define V4L2_RDS_PTYN 0x80/* Program Type Name */ +#define V4L2_RDS_RT0x100 /* Radio-Text */ +#define V4L2_RDS_TIME 0x200 /* Date and Time information */ +#define V4L2_RDS_TMC 0x400 /* TMC availability */ +#define V4L2_RDS_AF0x800 /* AF (alternative freq) available */ +#define V4L2_RDS_ECC 0x1000 /* Extended County Code */ +#define V4L2_RDS_LC0x2000 /* Language Code */ + +/* Define Constants for the state of the RDS decoding process + * used to address the relevant bit in the state_bitmask */ +#define V4L2_RDS_GROUP_NEW 0x01/* New group received */ +#define V4L2_RDS_ODA 0x02/* Open Data Group announced */ + +/* Decoder Information (DI) codes + * used to decode the DI information according to the RDS standard */ +#define V4L2_RDS_FLAG_STEREO 0x01 +#define V4L2_RDS_FLAG_ARTIFICIAL_HEAD 0x02 +#define V4L2_RDS_FLAG_COMPRESSED 0x04 +#define V4L2_RDS_FLAG_STATIC_PTY 0x08 + +/* struct to encapsulate one complete RDS group */ +struct v4l2_rds_group { + uint16_t pi; + char group_version; + bool traffic_prog; + uint8_t group_id; + uint8_t pty; + uint8_t data_b_lsb; + uint8_t data_c_msb; + uint8_t data_c_lsb; + uint8_t data_d_msb; + uint8_t data_d_lsb; +}; + +/* struct to encapsulate some statistical information about the decoding process */ +struct v4l2_rds_statistics { + uint32_t block_cnt; + uint32_t group_cnt; + uint32_t block_error_cnt; + uint32_t group_error_cnt; + uint32_t block_corrected_cnt; + uint32_t group_type_cnt[16]; +}; + +/* struct to encapsulate the definition of one ODA (Open Data Application) type */ +struct v4l2_rds_oda { + uint8_t group_id; + char group_version; + uint16_t
Re: [RFC PATCH 2/2] Initial version of RDS Control utility Signed-off-by: Konke Radlow krad...@cisco.com
Hi, Overall this looks good, but some of the code, for example the set/get freq and get/set tuner stuff seems to be a 1 on 1 copy of code from v4l2-ctrl, please factor this out into a common file which can be shared between both utilities (I think Hans V's recent work on splitting v4l2-ctl into multiple files comes a long way towards this). Regards, Hans On 07/25/2012 07:44 PM, Konke Radlow wrote: --- Makefile.am |3 +- configure.ac |1 + utils/rds-ctl/Makefile.am |5 + utils/rds-ctl/rds-ctl.cpp | 978 + 4 files changed, 986 insertions(+), 1 deletion(-) create mode 100644 utils/rds-ctl/Makefile.am create mode 100644 utils/rds-ctl/rds-ctl.cpp diff --git a/Makefile.am b/Makefile.am index 6707f5f..47103a1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -18,7 +18,8 @@ SUBDIRS += \ utils/v4l2-compliance \ utils/v4l2-ctl \ utils/v4l2-dbg \ - utils/v4l2-sysfs-path + utils/v4l2-sysfs-path \ + utils/rds-ctl if LINUX_OS SUBDIRS += \ diff --git a/configure.ac b/configure.ac index 1d7eb29..1ad99e6 100644 --- a/configure.ac +++ b/configure.ac @@ -28,6 +28,7 @@ AC_CONFIG_FILES([Makefile utils/v4l2-sysfs-path/Makefile utils/xc3028-firmware/Makefile utils/qv4l2/Makefile + utils/rds-ctl/Makefile contrib/freebsd/Makefile contrib/test/Makefile diff --git a/utils/rds-ctl/Makefile.am b/utils/rds-ctl/Makefile.am new file mode 100644 index 000..9a84257 --- /dev/null +++ b/utils/rds-ctl/Makefile.am @@ -0,0 +1,5 @@ +bin_PROGRAMS = rds-ctl + +rds_ctl_SOURCES = rds-ctl.cpp +rds_ctl_LDADD = ../../lib/libv4l2/libv4l2.la ../../lib/libv4l2rds/libv4l2rds.la + diff --git a/utils/rds-ctl/rds-ctl.cpp b/utils/rds-ctl/rds-ctl.cpp new file mode 100644 index 000..8ddb969 --- /dev/null +++ b/utils/rds-ctl/rds-ctl.cpp @@ -0,0 +1,978 @@ +/* + * rds-ctl.cpp is based on v4l2-ctl.cpp + * + * the following applies for all RDS related parts: + * Copyright 2012 Cisco Systems, Inc. and/or its affiliates. All rights reserved. + * Author: Konke Radlow korad...@gmail.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; either version 2.1 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA 02110-1335 USA + */ + +#include unistd.h +#include stdlib.h +#include stdio.h +#include string.h +#include wchar.h +#include locale.h +#include inttypes.h +#include getopt.h +#include sys/types.h +#include fcntl.h +#include errno.h +#include sys/ioctl.h +#include sys/time.h +#include dirent.h +#include config.h +#include signal.h + +#ifdef HAVE_SYS_KLOG_H +#include sys/klog.h +#endif + +#include linux/videodev2.h +#include libv4l2.h +#include libv4l2rds.h + +#include list +#include vector +#include map +#include string +#include algorithm + +#define ARRAY_SIZE(arr) ((int)(sizeof(arr) / sizeof((arr)[0]))) + +typedef std::vectorstd::string dev_vec; +typedef std::mapstd::string, std::string dev_map; + +/* Short option list + + Please keep in alphabetical order. + That makes it easier to see which short options are still free. + + In general the lower case is used to set something and the upper + case is used to retrieve a setting. */ +enum Option { + OptSetDevice = 'd', + OptGetDriverInfo = 'D', + OptGetFreq = 'F', + OptSetFreq = 'f', + OptHelp = 'h', + OptReadRds = 'R', + OptGetTuner = 'T', + OptSetTuner = 't', + OptUseWrapper = 'w', + OptAll = 128, + OptFreqSeek, + OptListDevices, + OptOpenFile, + OptPrintBlock, + OptSilent, + OptTunerIndex, + OptVerbose, + OptWaitLimit, + OptLast = 256 +}; + +struct ctl_parameters { + bool terminate_decoding; + char options[OptLast]; + char fd_name[80]; + bool filemode_active; + double freq; + uint32_t wait_limit; + uint8_t tuner_index; + struct v4l2_hw_freq_seek freq_seek; +}; + +static struct ctl_parameters params; +static int app_result; + +static struct option long_options[] = { + {all, no_argument, 0, OptAll}, + {device, required_argument, 0, OptSetDevice}, + {file, required_argument, 0, OptOpenFile}, + {freq-seek, required_argument, 0, OptFreqSeek}, + {get-freq, no_argument, 0, OptGetFreq}, + {get-tuner, no_argument, 0, OptGetTuner}, +
Vacation hansg July 13th - July 20th
Hi All, I'm going on vacation for a week starting tomorrow, and I will *not* be reading email, etc. during that time. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] v4l2: Add rangelow and rangehigh fields to the v4l2_hw_freq_seek struct
Hi, On 07/11/2012 08:37 PM, halli manjunatha wrote: On Wed, Jul 11, 2012 at 1:01 PM, Hans Verkuil hverk...@xs4all.nl wrote: Hi Hans, Thanks for the patch. I've CC-ed Halli as well. On Wed July 11 2012 17:47:34 Hans de Goede wrote: To allow apps to limit a hw-freq-seek to a specific band, for further info see the documentation this patch adds for these new fields. Signed-off-by: Hans de Goede hdego...@redhat.com --- .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml| 44 include/linux/videodev2.h |5 ++- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml index f4db44d..50dc9f8 100644 --- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml +++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml @@ -52,11 +52,21 @@ paraStart a hardware frequency seek from the current frequency. To do this applications initialize the structfieldtuner/structfield, structfieldtype/structfield, structfieldseek_upward/structfield, -structfieldspacing/structfield and -structfieldwrap_around/structfield fields, and zero out the -structfieldreserved/structfield array of a v4l2-hw-freq-seek; and -call the constantVIDIOC_S_HW_FREQ_SEEK/constant ioctl with a pointer -to this structure./para +structfieldwrap_around/structfield, structfieldspacing/structfield, +structfieldrangelow/structfield and structfieldrangehigh/structfield +fields, and zero out the structfieldreserved/structfield array of a +v4l2-hw-freq-seek; and call the constantVIDIOC_S_HW_FREQ_SEEK/constant +ioctl with a pointer to this structure./para + +paraThe structfieldrangelow/structfield and +structfieldrangehigh/structfield fields can be set to a non-zero value to +tell the driver to search a specific band. If the v4l2-tuner; +structfieldcapability/structfield field has the +constantV4L2_TUNER_CAP_HWSEEK_PROG_LIM/constant flag set, these values +must fall within one of the bands returned by VIDIOC-ENUM-FREQ-BANDS;. If +the constantV4L2_TUNER_CAP_HWSEEK_PROG_LIM/constant flag is not set, +then these values must exactly match those of one of the bands returned by +VIDIOC-ENUM-FREQ-BANDS;./para OK, I have some questions here: 1) If you have a multiband tuner, what should happen if both low and high are zero? Currently it is undefined, other than that the seek should start from the current frequency until it reaches some limit. Halli, what does your hardware do? In particular, is the hwseek limited by the US/Europe or Japan band range or can it do the full range? If I'm not mistaken it is the former, right? You are right... my hardware seek is limited by the japan/US band range If it is the former, then you need to explicitly set low + high to ensure that the hwseek uses the correct range because the driver can't guess which of the overlapping bands to use. Yes in my driver I will take care of this :) I think you misunderstood Hans here, not the driver but userspace will need to fill in the rangelow / rangehigh fields of struct v4l2_hw_freq_seek, because if the current freq is in the overlapping area of the bands, the driver cannot know which band to seek, so it will just have to guess, I think it is best to just leave the band at its current setting in that case. The way the new API works (which was done this way to preserve backward compat) is that the bands returned from ENUM_BANDS are there as information only. userspace never explicitly sets a band, so an old app will just see the entire 76-108 MHZ range in the tuner struct and may do a S_FREQUENCY for any of those frequencies, and the driver must automatically switch bands when necessary. With S_HW_FREQ_SEEK we've the 2 new fields to indicate the band to seek for new apps, but with old apps these fields will be 0, and the driver needs to just pick a band to search on a best effort basis, for the si470x IE, if no band is specified in struct v4l2_hw_freq_seek, I simply always switch to the Japan wide band of 76-108 Mhz as that includes all other bands supported by the si470x. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RFC: Add support for limiting hw freq seeks to a certain band (v2)
This patchset, which builds on top of hverkuil's bands2 branch, which adds the VIDIOC_ENUM_FREQ_BANDS API, add support for limiting hw freq seeks to one of the bands from VIDIOC_ENUM_FREQ_BANDS, or a subset there of. The first patch introduces the new API and documents its, the other patches are patches to the si470x radio driver, implementing the new API, and removing the band module parameter as this is no longer needed with this new API. A git tree with all hverkuils patches included is here: http://git.linuxtv.org/hgoede/gspca.git/shortlog/refs/heads/media-for_v3.6-wip A git tree of xawtv3 with its radio app modified to support the new API is here: http://git.linuxtv.org/hgoede/xawtv3.git/shortlog/refs/heads/band-support Changes in v2: -improve / clarify the documentation on how the rangelow and rangehigh fields of the v4l2_hw_freq_seek struct are used Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] v4l2 spec: add VIDIOC_ENUM_FREQ_BANDS documentation.
From: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- Documentation/DocBook/media/v4l/compat.xml | 12 ++ Documentation/DocBook/media/v4l/v4l2.xml |6 + .../DocBook/media/v4l/vidioc-enum-freq-bands.xml | 179 .../DocBook/media/v4l/vidioc-g-frequency.xml |7 +- Documentation/DocBook/media/v4l/vidioc-g-tuner.xml | 26 ++- 5 files changed, 221 insertions(+), 9 deletions(-) create mode 100644 Documentation/DocBook/media/v4l/vidioc-enum-freq-bands.xml diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml index 97b8951..aa28015 100644 --- a/Documentation/DocBook/media/v4l/compat.xml +++ b/Documentation/DocBook/media/v4l/compat.xml @@ -2471,6 +2471,15 @@ that used it. It was originally scheduled for removal in 2.6.35. /orderedlist /section +section + titleV4L2 in Linux 3.6/title + orderedlist +listitem + paraAdded support for frequency band enumerations: VIDIOC-ENUM-FREQ-BANDS;./para +/listitem + /orderedlist +/section + section id=other titleRelation of V4L2 to other Linux multimedia APIs/title @@ -2600,6 +2609,9 @@ ioctls./para paralink linkend=v4l2-auto-focus-areaconstant V4L2_CID_AUTO_FOCUS_AREA/constant/link control./para /listitem +listitem + paraSupport for frequency band enumeration: VIDIOC-ENUM-FREQ-BANDS; ioctl./para +/listitem /itemizedlist /section diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml index 36bafc4..eee6908 100644 --- a/Documentation/DocBook/media/v4l/v4l2.xml +++ b/Documentation/DocBook/media/v4l/v4l2.xml @@ -140,6 +140,11 @@ structs, ioctls) must be noted in more detail in the history chapter applications. -- revision + revnumber3.6/revnumber + date2012-07-02/date + authorinitialshv/authorinitials + revremarkAdded VIDIOC_ENUM_FREQ_BANDS. + /revremark revnumber3.5/revnumber date2012-05-07/date authorinitialssa, sn/authorinitials @@ -534,6 +539,7 @@ and discussions on the V4L mailing list./revremark sub-enum-fmt; sub-enum-framesizes; sub-enum-frameintervals; +sub-enum-freq-bands; sub-enuminput; sub-enumoutput; sub-enumstd; diff --git a/Documentation/DocBook/media/v4l/vidioc-enum-freq-bands.xml b/Documentation/DocBook/media/v4l/vidioc-enum-freq-bands.xml new file mode 100644 index 000..6541ba0 --- /dev/null +++ b/Documentation/DocBook/media/v4l/vidioc-enum-freq-bands.xml @@ -0,0 +1,179 @@ +refentry id=vidioc-enum-freq-bands + refmeta +refentrytitleioctl VIDIOC_ENUM_FREQ_BANDS/refentrytitle +manvol; + /refmeta + + refnamediv +refnameVIDIOC_ENUM_FREQ_BANDS/refname +refpurposeEnumerate supported frequency bands/refpurpose + /refnamediv + + refsynopsisdiv +funcsynopsis + funcprototype + funcdefint functionioctl/function/funcdef + paramdefint parameterfd/parameter/paramdef + paramdefint parameterrequest/parameter/paramdef + paramdefstruct v4l2_frequency_band +*parameterargp/parameter/paramdef + /funcprototype +/funcsynopsis + /refsynopsisdiv + + refsect1 +titleArguments/title + +variablelist + varlistentry + termparameterfd/parameter/term + listitem + parafd;/para + /listitem + /varlistentry + varlistentry + termparameterrequest/parameter/term + listitem + paraVIDIOC_ENUM_FREQ_BANDS/para + /listitem + /varlistentry + varlistentry + termparameterargp/parameter/term + listitem + para/para + /listitem + /varlistentry +/variablelist + /refsect1 + + refsect1 +titleDescription/title + +note + titleExperimental/title + paraThis is an link linkend=experimental experimental /link + interface and may change in the future./para +/note + +paraEnumerates the frequency bands that a tuner or modulator supports. +To do this applications initialize the structfieldtuner/structfield, +structfieldtype/structfield and structfieldindex/structfield fields, +and zero out the structfieldreserved/structfield array of a v4l2-frequency-band; and +call the constantVIDIOC_ENUM_FREQ_BANDS/constant ioctl with a pointer +to this structure./para + +paraThis ioctl is supported if the constantV4L2_TUNER_CAP_FREQ_BANDS/constant capability +of the corresponding tuner/modulator is set./para + +table pgwide=1 frame=none id=v4l2-frequency-band + titlestruct structnamev4l2_frequency_band/structname/title + tgroup cols=3 + cs-str; + tbody valign=top + row + entry__u32/entry + entrystructfieldtuner/structfield/entry + entryThe tuner or modulator index number. This is the +same value as in the v4l2-input;
[PATCH 2/5] v4l2: Add rangelow and rangehigh fields to the v4l2_hw_freq_seek struct
To allow apps to limit a hw-freq-seek to a specific band, for further info see the documentation this patch adds for these new fields. Signed-off-by: Hans de Goede hdego...@redhat.com --- .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml| 50 include/linux/videodev2.h |5 +- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml index f4db44d..3dd1bec 100644 --- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml +++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml @@ -52,11 +52,23 @@ paraStart a hardware frequency seek from the current frequency. To do this applications initialize the structfieldtuner/structfield, structfieldtype/structfield, structfieldseek_upward/structfield, -structfieldspacing/structfield and -structfieldwrap_around/structfield fields, and zero out the -structfieldreserved/structfield array of a v4l2-hw-freq-seek; and -call the constantVIDIOC_S_HW_FREQ_SEEK/constant ioctl with a pointer -to this structure./para +structfieldwrap_around/structfield, structfieldspacing/structfield, +structfieldrangelow/structfield and structfieldrangehigh/structfield +fields, and zero out the structfieldreserved/structfield array of a +v4l2-hw-freq-seek; and call the constantVIDIOC_S_HW_FREQ_SEEK/constant +ioctl with a pointer to this structure./para + +paraThe structfieldrangelow/structfield and +structfieldrangehigh/structfield fields can be set to a non-zero value to +tell the driver to search a specific band. If the v4l2-tuner; +structfieldcapability/structfield field has the +constantV4L2_TUNER_CAP_HWSEEK_PROG_LIM/constant flag set, these values +must fall within one of the bands returned by VIDIOC-ENUM-FREQ-BANDS;. If +the constantV4L2_TUNER_CAP_HWSEEK_PROG_LIM/constant flag is not set, +then these values must exactly match those of one of the bands returned by +VIDIOC-ENUM-FREQ-BANDS;. If the current frequency of the tuner does not fall +within the selected band it will be clamped to fit in the band before the +seek is started./para paraIf an error is returned, then the original frequency will be restored./para @@ -102,7 +114,27 @@ field and the v4l2-tuner; structfieldindex/structfield field./entry /row row entry__u32/entry - entrystructfieldreserved/structfield[7]/entry + entrystructfieldrangelow/structfield/entry + entryIf non-zero, the lowest tunable frequency of the band to +search in units of 62.5 kHz, or if the v4l2-tuner; +structfieldcapability/structfield field has the +constantV4L2_TUNER_CAP_LOW/constant flag set, in units of 62.5 Hz. +If structfieldrangelow/structfield is zero a reasonable default value +is used./entry + /row + row + entry__u32/entry + entrystructfieldrangehigh/structfield/entry + entryIf non-zero, the highest tunable frequency of the band to +search in units of 62.5 kHz, or if the v4l2-tuner; +structfieldcapability/structfield field has the +constantV4L2_TUNER_CAP_LOW/constant flag set, in units of 62.5 Hz. +If structfieldrangehigh/structfield is zero a reasonable default value +is used./entry + /row + row + entry__u32/entry + entrystructfieldreserved/structfield[5]/entry entryReserved for future extensions. Applications must set the array to zero./entry /row @@ -119,8 +151,10 @@ field and the v4l2-tuner; structfieldindex/structfield field./entry termerrorcodeEINVAL/errorcode/term listitem paraThe structfieldtuner/structfield index is out of -bounds, the wrap_around value is not supported or the value in the structfieldtype/structfield field is -wrong./para +bounds, the structfieldwrap_around/structfield value is not supported or +one of the values in the structfieldtype/structfield, +structfieldrangelow/structfield or structfieldrangehigh/structfield +fields is wrong./para /listitem /varlistentry varlistentry diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 9fa822a..1c6aa63 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -2029,6 +2029,7 @@ struct v4l2_modulator { #define V4L2_TUNER_CAP_RDS_BLOCK_IO0x0100 #define V4L2_TUNER_CAP_RDS_CONTROLS0x0200 #define V4L2_TUNER_CAP_FREQ_BANDS 0x0400 +#define V4L2_TUNER_CAP_HWSEEK_PROG_LIM 0x0800 /* Flags for the 'rxsubchans' field */ #define V4L2_TUNER_SUB_MONO0x0001 @@ -2074,7 +2075,9 @@ struct v4l2_hw_freq_seek { __u32 seek_upward; __u32 wrap_around; __u32 spacing; - __u32 reserved[7]; + __u32 rangelow; + __u32 rangehigh; + __u32 reserved[5]; }; /* -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media
[PATCH 4/5] radio-si470x: Fix band selection
The mask was wrong resulting in band 0 and 1 always ending up as band 0 in the register. Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/si470x/radio-si470x.h |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/radio/si470x/radio-si470x.h b/drivers/media/radio/si470x/radio-si470x.h index b3b612f..11f14b6 100644 --- a/drivers/media/radio/si470x/radio-si470x.h +++ b/drivers/media/radio/si470x/radio-si470x.h @@ -87,7 +87,7 @@ #define SYSCONFIG2 5 /* System Configuration 2 */ #define SYSCONFIG2_SEEKTH 0xff00 /* bits 15..08: RSSI Seek Threshold */ -#define SYSCONFIG2_BAND0x0080 /* bits 07..06: Band Select */ +#define SYSCONFIG2_BAND0x00c0 /* bits 07..06: Band Select */ #define SYSCONFIG2_SPACE 0x0030 /* bits 05..04: Channel Spacing */ #define SYSCONFIG2_VOLUME 0x000f /* bits 03..00: Volume */ -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] radio-si470x: restore ctrl settings after suspend/resume
Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/si470x/radio-si470x-usb.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/media/radio/si470x/radio-si470x-usb.c b/drivers/media/radio/si470x/radio-si470x-usb.c index 40b963c..0204cf4 100644 --- a/drivers/media/radio/si470x/radio-si470x-usb.c +++ b/drivers/media/radio/si470x/radio-si470x-usb.c @@ -792,11 +792,16 @@ static int si470x_usb_driver_suspend(struct usb_interface *intf, static int si470x_usb_driver_resume(struct usb_interface *intf) { struct si470x_device *radio = usb_get_intfdata(intf); + int ret; dev_info(intf-dev, resuming now...\n); /* start radio */ - return si470x_start_usb(radio); + ret = si470x_start_usb(radio); + if (ret == 0) + v4l2_ctrl_handler_setup(radio-hdl); + + return ret; } -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] radio-si470x: Add support for the new band APIs
Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/si470x/radio-si470x-common.c | 215 +- drivers/media/radio/si470x/radio-si470x-i2c.c|1 + drivers/media/radio/si470x/radio-si470x-usb.c|1 + drivers/media/radio/si470x/radio-si470x.h|1 + 4 files changed, 126 insertions(+), 92 deletions(-) diff --git a/drivers/media/radio/si470x/radio-si470x-common.c b/drivers/media/radio/si470x/radio-si470x-common.c index 84ab3d57..9e38132 100644 --- a/drivers/media/radio/si470x/radio-si470x-common.c +++ b/drivers/media/radio/si470x/radio-si470x-common.c @@ -4,6 +4,7 @@ * Driver for radios with Silicon Labs Si470x FM Radio Receivers * * Copyright (c) 2009 Tobias Lorenz tobias.lor...@gmx.net + * Copyright (c) 2012 Hans de Goede hdego...@redhat.com * * 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 @@ -127,14 +128,6 @@ static unsigned short space = 2; module_param(space, ushort, 0444); MODULE_PARM_DESC(space, Spacing: 0=200kHz 1=100kHz *2=50kHz*); -/* Bottom of Band (MHz) */ -/* 0: 87.5 - 108 MHz (USA, Europe)*/ -/* 1: 76 - 108 MHz (Japan wide band) */ -/* 2: 76 - 90 MHz (Japan) */ -static unsigned short band = 1; -module_param(band, ushort, 0444); -MODULE_PARM_DESC(band, Band: 0=87.5..108MHz *1=76..108MHz* 2=76..90MHz); - /* De-emphasis */ /* 0: 75 us (USA) */ /* 1: 50 us (Europe, Australia, Japan) */ @@ -152,13 +145,61 @@ static unsigned int seek_timeout = 5000; module_param(seek_timeout, uint, 0644); MODULE_PARM_DESC(seek_timeout, Seek timeout: *5000*); - +static const struct v4l2_frequency_band bands[] = { + { + .type = V4L2_TUNER_RADIO, + .index = 0, + .capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO | + V4L2_TUNER_CAP_RDS | V4L2_TUNER_CAP_RDS_BLOCK_IO | + V4L2_TUNER_CAP_HWSEEK_BOUNDED | + V4L2_TUNER_CAP_HWSEEK_WRAP, + .rangelow = 87500 * 16, + .rangehigh = 108000 * 16, + .modulation = V4L2_BAND_MODULATION_FM, + }, + { + .type = V4L2_TUNER_RADIO, + .index = 1, + .capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO | + V4L2_TUNER_CAP_RDS | V4L2_TUNER_CAP_RDS_BLOCK_IO | + V4L2_TUNER_CAP_HWSEEK_BOUNDED | + V4L2_TUNER_CAP_HWSEEK_WRAP, + .rangelow = 76000 * 16, + .rangehigh = 108000 * 16, + .modulation = V4L2_BAND_MODULATION_FM, + }, + { + .type = V4L2_TUNER_RADIO, + .index = 2, + .capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO | + V4L2_TUNER_CAP_RDS | V4L2_TUNER_CAP_RDS_BLOCK_IO | + V4L2_TUNER_CAP_HWSEEK_BOUNDED | + V4L2_TUNER_CAP_HWSEEK_WRAP, + .rangelow = 76000 * 16, + .rangehigh = 9 * 16, + .modulation = V4L2_BAND_MODULATION_FM, + }, +}; /** * Generic Functions **/ /* + * si470x_set_band - set the band + */ +static int si470x_set_band(struct si470x_device *radio, int band) +{ + if (radio-band == band) + return 0; + + radio-band = band; + radio-registers[SYSCONFIG2] = ~SYSCONFIG2_BAND; + radio-registers[SYSCONFIG2] |= radio-band 6; + return si470x_set_register(radio, SYSCONFIG2); +} + +/* * si470x_set_chan - set the channel */ static int si470x_set_chan(struct si470x_device *radio, unsigned short chan) @@ -194,48 +235,39 @@ done: return retval; } - /* - * si470x_get_freq - get the frequency + * si470x_get_step - get channel spacing */ -static int si470x_get_freq(struct si470x_device *radio, unsigned int *freq) +static unsigned int si470x_get_step(struct si470x_device *radio) { - unsigned int spacing, band_bottom; - unsigned short chan; - int retval; - /* Spacing (kHz) */ switch ((radio-registers[SYSCONFIG2] SYSCONFIG2_SPACE) 4) { /* 0: 200 kHz (USA, Australia) */ case 0: - spacing = 0.200 * FREQ_MUL; break; + return 200 * 16; /* 1: 100 kHz (Europe, Japan) */ case 1: - spacing = 0.100 * FREQ_MUL; break; + return 100 * 16; /* 2: 50 kHz */ default: - spacing = 0.050 * FREQ_MUL; break; + return 50 * 16; }; +} - /* Bottom of Band (MHz) */ - switch ((radio-registers[SYSCONFIG2] SYSCONFIG2_BAND) 6) { - /* 0: 87.5 - 108 MHz (USA, Europe) */ - case 0
Re: video: USB webcam fails since kernel 3.2
Hi, On 07/11/2012 02:01 PM, Martin-Éric Racine wrote: 2012/7/11 Jean-Francois Moine moin...@free.fr: On Wed, 11 Jul 2012 14:14:24 +0300 Martin-Éric Racine martin-eric.rac...@iki.fi wrote: CC [M] /home/perkelix/gspca-2.15.18/build/ov534_9.o /home/perkelix/gspca-2.15.18/build/ov534_9.c: In function ‘sd_init’: /home/perkelix/gspca-2.15.18/build/ov534_9.c:1353:3: error: implicit declaration of function ‘err’ [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors make[2]: *** [/home/perkelix/gspca-2.15.18/build/ov534_9.o] Virhe 1 make[1]: *** [_module_/home/perkelix/gspca-2.15.18/build] Error 2 make[1]: Leaving directory `/usr/src/linux-headers-3.5.0-rc6+' make: *** [modules] Error 2 Sorry, I did not compile yet with kernel = 3.4. So, please, edit the file build/ov534_9.c (and possibly other sources), changing the calls to 'err' to 'pr_err'. This was was required for both build/ov534_9.c and build/spca505.c to build agaist 3.5. Sure enough, this seems to fix support for this camera in both Cheese and Skype. Hurray! :-) Ok, so it seems that increasing the bandwidth we claim for the camera (which is what my suggested return 2000 * 2000 * 120; change does, helps a bit, where as the changes to vc032x which are in Jean-Francois Moine's gspca-2.15.18 tarbal fix the problem entirely, correct? Now, the only thing that remains is for this to be merged in the 3.5 tree, then backported to the 3.2 tree that is used for Debian's upcoming Wheezy stable release (and for Ubuntu's recently released Precise also). Well we first need to turn the changes made in gspca-2.15.18 into a patch will which apply to the latest gspca tree: http://git.linuxtv.org/hgoede/gspca.git/shortlog/refs/heads/media-for_v3.6 And then apply them there, before the can be backported to older kernels. Unfortunately I'm leaving for a week vacation Friday, and I probably won't get around to this before then. Jean-Francois, can you perhaps make a patch against my latest tree for the po / PO3130 changes in your tarbal? Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RFC: Add support for limiting hw freq seeks to a certain band
This patchset, which builds on top of hverkuil's bands2 branch, which adds the VIDIOC_ENUM_FREQ_BANDS API, add support for limiting hw freq seeks to one of the bands from VIDIOC_ENUM_FREQ_BANDS, or a subset there of. The first patch introduces the new API and documents its, the other patches are patches to the si470x radio driver, implementing the new API, and removing the band module parameter as this is no longer needed with this new API. A git tree with all hverkuils patches included is here: http://git.linuxtv.org/hgoede/gspca.git/shortlog/refs/heads/media-for_v3.6-wip A git tree of xawtv3 with its radio app modified to support the new API is here: http://git.linuxtv.org/hgoede/xawtv3.git/shortlog/refs/heads/band-support Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] v4l2: Add rangelow and rangehigh fields to the v4l2_hw_freq_seek struct
To allow apps to limit a hw-freq-seek to a specific band, for further info see the documentation this patch adds for these new fields. Signed-off-by: Hans de Goede hdego...@redhat.com --- .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml| 44 include/linux/videodev2.h |5 ++- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml index f4db44d..50dc9f8 100644 --- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml +++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml @@ -52,11 +52,21 @@ paraStart a hardware frequency seek from the current frequency. To do this applications initialize the structfieldtuner/structfield, structfieldtype/structfield, structfieldseek_upward/structfield, -structfieldspacing/structfield and -structfieldwrap_around/structfield fields, and zero out the -structfieldreserved/structfield array of a v4l2-hw-freq-seek; and -call the constantVIDIOC_S_HW_FREQ_SEEK/constant ioctl with a pointer -to this structure./para +structfieldwrap_around/structfield, structfieldspacing/structfield, +structfieldrangelow/structfield and structfieldrangehigh/structfield +fields, and zero out the structfieldreserved/structfield array of a +v4l2-hw-freq-seek; and call the constantVIDIOC_S_HW_FREQ_SEEK/constant +ioctl with a pointer to this structure./para + +paraThe structfieldrangelow/structfield and +structfieldrangehigh/structfield fields can be set to a non-zero value to +tell the driver to search a specific band. If the v4l2-tuner; +structfieldcapability/structfield field has the +constantV4L2_TUNER_CAP_HWSEEK_PROG_LIM/constant flag set, these values +must fall within one of the bands returned by VIDIOC-ENUM-FREQ-BANDS;. If +the constantV4L2_TUNER_CAP_HWSEEK_PROG_LIM/constant flag is not set, +then these values must exactly match those of one of the bands returned by +VIDIOC-ENUM-FREQ-BANDS;./para paraIf an error is returned, then the original frequency will be restored./para @@ -102,7 +112,23 @@ field and the v4l2-tuner; structfieldindex/structfield field./entry /row row entry__u32/entry - entrystructfieldreserved/structfield[7]/entry + entrystructfieldrangelow/structfield/entry + entryIf non-zero, the lowest tunable frequency of the band to +search in units of 62.5 kHz, or if the v4l2-tuner; +structfieldcapability/structfield field has the +constantV4L2_TUNER_CAP_LOW/constant flag set, in units of 62.5 Hz./entry + /row + row + entry__u32/entry + entrystructfieldrangehigh/structfield/entry + entryIf non-zero, the highest tunable frequency of the band to +search in units of 62.5 kHz, or if the v4l2-tuner; +structfieldcapability/structfield field has the +constantV4L2_TUNER_CAP_LOW/constant flag set, in units of 62.5 Hz./entry + /row + row + entry__u32/entry + entrystructfieldreserved/structfield[5]/entry entryReserved for future extensions. Applications must set the array to zero./entry /row @@ -119,8 +145,10 @@ field and the v4l2-tuner; structfieldindex/structfield field./entry termerrorcodeEINVAL/errorcode/term listitem paraThe structfieldtuner/structfield index is out of -bounds, the wrap_around value is not supported or the value in the structfieldtype/structfield field is -wrong./para +bounds, the structfieldwrap_around/structfield value is not supported or +one of the values in the structfieldtype/structfield, +structfieldrangelow/structfield or structfieldrangehigh/structfield +fields is wrong./para /listitem /varlistentry varlistentry diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 9fa822a..1c6aa63 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -2029,6 +2029,7 @@ struct v4l2_modulator { #define V4L2_TUNER_CAP_RDS_BLOCK_IO0x0100 #define V4L2_TUNER_CAP_RDS_CONTROLS0x0200 #define V4L2_TUNER_CAP_FREQ_BANDS 0x0400 +#define V4L2_TUNER_CAP_HWSEEK_PROG_LIM 0x0800 /* Flags for the 'rxsubchans' field */ #define V4L2_TUNER_SUB_MONO0x0001 @@ -2074,7 +2075,9 @@ struct v4l2_hw_freq_seek { __u32 seek_upward; __u32 wrap_around; __u32 spacing; - __u32 reserved[7]; + __u32 rangelow; + __u32 rangehigh; + __u32 reserved[5]; }; /* -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] radio-si470x: Fix band selection
The mask was wrong resulting in band 0 and 1 always ending up as band 0 in the register. Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/si470x/radio-si470x.h |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/radio/si470x/radio-si470x.h b/drivers/media/radio/si470x/radio-si470x.h index b3b612f..11f14b6 100644 --- a/drivers/media/radio/si470x/radio-si470x.h +++ b/drivers/media/radio/si470x/radio-si470x.h @@ -87,7 +87,7 @@ #define SYSCONFIG2 5 /* System Configuration 2 */ #define SYSCONFIG2_SEEKTH 0xff00 /* bits 15..08: RSSI Seek Threshold */ -#define SYSCONFIG2_BAND0x0080 /* bits 07..06: Band Select */ +#define SYSCONFIG2_BAND0x00c0 /* bits 07..06: Band Select */ #define SYSCONFIG2_SPACE 0x0030 /* bits 05..04: Channel Spacing */ #define SYSCONFIG2_VOLUME 0x000f /* bits 03..00: Volume */ -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] radio-si470x: Lower firmware version requirements
Testing with a firmware version 12 usb radio stick has shown version 12 to work fine too. Reported-by: Antti Palosaari cr...@iki.fi Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/si470x/radio-si470x.h |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/radio/si470x/radio-si470x.h b/drivers/media/radio/si470x/radio-si470x.h index 8e3a62f..2f089b4 100644 --- a/drivers/media/radio/si470x/radio-si470x.h +++ b/drivers/media/radio/si470x/radio-si470x.h @@ -190,7 +190,7 @@ struct si470x_device { * Firmware Versions **/ -#define RADIO_FW_VERSION 14 +#define RADIO_FW_VERSION 12 -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] radio-si470x: Add support for the new band APIs
Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/si470x/radio-si470x-common.c | 215 +- drivers/media/radio/si470x/radio-si470x-i2c.c|1 + drivers/media/radio/si470x/radio-si470x-usb.c|1 + drivers/media/radio/si470x/radio-si470x.h|1 + 4 files changed, 126 insertions(+), 92 deletions(-) diff --git a/drivers/media/radio/si470x/radio-si470x-common.c b/drivers/media/radio/si470x/radio-si470x-common.c index 84ab3d57..9e38132 100644 --- a/drivers/media/radio/si470x/radio-si470x-common.c +++ b/drivers/media/radio/si470x/radio-si470x-common.c @@ -4,6 +4,7 @@ * Driver for radios with Silicon Labs Si470x FM Radio Receivers * * Copyright (c) 2009 Tobias Lorenz tobias.lor...@gmx.net + * Copyright (c) 2012 Hans de Goede hdego...@redhat.com * * 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 @@ -127,14 +128,6 @@ static unsigned short space = 2; module_param(space, ushort, 0444); MODULE_PARM_DESC(space, Spacing: 0=200kHz 1=100kHz *2=50kHz*); -/* Bottom of Band (MHz) */ -/* 0: 87.5 - 108 MHz (USA, Europe)*/ -/* 1: 76 - 108 MHz (Japan wide band) */ -/* 2: 76 - 90 MHz (Japan) */ -static unsigned short band = 1; -module_param(band, ushort, 0444); -MODULE_PARM_DESC(band, Band: 0=87.5..108MHz *1=76..108MHz* 2=76..90MHz); - /* De-emphasis */ /* 0: 75 us (USA) */ /* 1: 50 us (Europe, Australia, Japan) */ @@ -152,13 +145,61 @@ static unsigned int seek_timeout = 5000; module_param(seek_timeout, uint, 0644); MODULE_PARM_DESC(seek_timeout, Seek timeout: *5000*); - +static const struct v4l2_frequency_band bands[] = { + { + .type = V4L2_TUNER_RADIO, + .index = 0, + .capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO | + V4L2_TUNER_CAP_RDS | V4L2_TUNER_CAP_RDS_BLOCK_IO | + V4L2_TUNER_CAP_HWSEEK_BOUNDED | + V4L2_TUNER_CAP_HWSEEK_WRAP, + .rangelow = 87500 * 16, + .rangehigh = 108000 * 16, + .modulation = V4L2_BAND_MODULATION_FM, + }, + { + .type = V4L2_TUNER_RADIO, + .index = 1, + .capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO | + V4L2_TUNER_CAP_RDS | V4L2_TUNER_CAP_RDS_BLOCK_IO | + V4L2_TUNER_CAP_HWSEEK_BOUNDED | + V4L2_TUNER_CAP_HWSEEK_WRAP, + .rangelow = 76000 * 16, + .rangehigh = 108000 * 16, + .modulation = V4L2_BAND_MODULATION_FM, + }, + { + .type = V4L2_TUNER_RADIO, + .index = 2, + .capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO | + V4L2_TUNER_CAP_RDS | V4L2_TUNER_CAP_RDS_BLOCK_IO | + V4L2_TUNER_CAP_HWSEEK_BOUNDED | + V4L2_TUNER_CAP_HWSEEK_WRAP, + .rangelow = 76000 * 16, + .rangehigh = 9 * 16, + .modulation = V4L2_BAND_MODULATION_FM, + }, +}; /** * Generic Functions **/ /* + * si470x_set_band - set the band + */ +static int si470x_set_band(struct si470x_device *radio, int band) +{ + if (radio-band == band) + return 0; + + radio-band = band; + radio-registers[SYSCONFIG2] = ~SYSCONFIG2_BAND; + radio-registers[SYSCONFIG2] |= radio-band 6; + return si470x_set_register(radio, SYSCONFIG2); +} + +/* * si470x_set_chan - set the channel */ static int si470x_set_chan(struct si470x_device *radio, unsigned short chan) @@ -194,48 +235,39 @@ done: return retval; } - /* - * si470x_get_freq - get the frequency + * si470x_get_step - get channel spacing */ -static int si470x_get_freq(struct si470x_device *radio, unsigned int *freq) +static unsigned int si470x_get_step(struct si470x_device *radio) { - unsigned int spacing, band_bottom; - unsigned short chan; - int retval; - /* Spacing (kHz) */ switch ((radio-registers[SYSCONFIG2] SYSCONFIG2_SPACE) 4) { /* 0: 200 kHz (USA, Australia) */ case 0: - spacing = 0.200 * FREQ_MUL; break; + return 200 * 16; /* 1: 100 kHz (Europe, Japan) */ case 1: - spacing = 0.100 * FREQ_MUL; break; + return 100 * 16; /* 2: 50 kHz */ default: - spacing = 0.050 * FREQ_MUL; break; + return 50 * 16; }; +} - /* Bottom of Band (MHz) */ - switch ((radio-registers[SYSCONFIG2] SYSCONFIG2_BAND) 6) { - /* 0: 87.5 - 108 MHz (USA, Europe) */ - case 0
[PATCH 2/5] radio-si470x: restore ctrl settings after suspend/resume
Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/si470x/radio-si470x-usb.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/media/radio/si470x/radio-si470x-usb.c b/drivers/media/radio/si470x/radio-si470x-usb.c index 40b963c..0204cf4 100644 --- a/drivers/media/radio/si470x/radio-si470x-usb.c +++ b/drivers/media/radio/si470x/radio-si470x-usb.c @@ -792,11 +792,16 @@ static int si470x_usb_driver_suspend(struct usb_interface *intf, static int si470x_usb_driver_resume(struct usb_interface *intf) { struct si470x_device *radio = usb_get_intfdata(intf); + int ret; dev_info(intf-dev, resuming now...\n); /* start radio */ - return si470x_start_usb(radio); + ret = si470x_start_usb(radio); + if (ret == 0) + v4l2_ctrl_handler_setup(radio-hdl); + + return ret; } -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] v4l2: Add rangelow and rangehigh fields to the v4l2_hw_freq_seek struct
Hi Hans, On 07/11/2012 08:01 PM, Hans Verkuil wrote: Hi Hans, Thanks for the patch. I've CC-ed Halli as well. On Wed July 11 2012 17:47:34 Hans de Goede wrote: To allow apps to limit a hw-freq-seek to a specific band, for further info see the documentation this patch adds for these new fields. Signed-off-by: Hans de Goede hdego...@redhat.com --- .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml| 44 include/linux/videodev2.h |5 ++- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml index f4db44d..50dc9f8 100644 --- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml +++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml @@ -52,11 +52,21 @@ paraStart a hardware frequency seek from the current frequency. To do this applications initialize the structfieldtuner/structfield, structfieldtype/structfield, structfieldseek_upward/structfield, -structfieldspacing/structfield and -structfieldwrap_around/structfield fields, and zero out the -structfieldreserved/structfield array of a v4l2-hw-freq-seek; and -call the constantVIDIOC_S_HW_FREQ_SEEK/constant ioctl with a pointer -to this structure./para +structfieldwrap_around/structfield, structfieldspacing/structfield, +structfieldrangelow/structfield and structfieldrangehigh/structfield +fields, and zero out the structfieldreserved/structfield array of a +v4l2-hw-freq-seek; and call the constantVIDIOC_S_HW_FREQ_SEEK/constant +ioctl with a pointer to this structure./para + +paraThe structfieldrangelow/structfield and +structfieldrangehigh/structfield fields can be set to a non-zero value to +tell the driver to search a specific band. If the v4l2-tuner; +structfieldcapability/structfield field has the +constantV4L2_TUNER_CAP_HWSEEK_PROG_LIM/constant flag set, these values +must fall within one of the bands returned by VIDIOC-ENUM-FREQ-BANDS;. If +the constantV4L2_TUNER_CAP_HWSEEK_PROG_LIM/constant flag is not set, +then these values must exactly match those of one of the bands returned by +VIDIOC-ENUM-FREQ-BANDS;./para OK, I have some questions here: 1) If you have a multiband tuner, what should happen if both low and high are zero? Currently it is undefined, other than that the seek should start from the current frequency until it reaches some limit. That would be driver specific, we could add the same If rangelow/high is zero a reasonable default value is used. language as used for the spacing. For example for the si470x if both are zero I simply switch to the Japan wide band which covers all frequencies handled by the other bands, but if there is no such covers all ranges band, then the logical thing todo would just keep the band as is (so as determined by the last s_freq). Halli, what does your hardware do? In particular, is the hwseek limited by the US/Europe or Japan band range or can it do the full range? If I'm not mistaken it is the former, right? If it is the former, then you need to explicitly set low + high to ensure that the hwseek uses the correct range because the driver can't guess which of the overlapping bands to use. 2) What happens if the current frequency is outside the low/high range? The hwseek spec says that the seek starts from the current frequency, so that might mean that hwseek returns -ERANGE in this case. What the si470x code currently does is just clamp the frequency to the new range before seeking, but -ERANGE works for me too. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH-3.5 ] Fix sn9c20x regression (was [regression 3.4-3.5, bisected] kernel oops when starting capturing from gspca-sn9c20x webcams)
Hi Linus, I'm sending this patch (attached) directly to you, since it fixes a regression in 3.5-rc6, and Mauro is on vacation. Please add this to the next 3.5 rc. Thanks Regards, Hans On 07/08/2012 10:04 PM, Frank Schäfer wrote: Am 08.07.2012 19:43, schrieb Hans de Goede: Hi, Thanks for reporting this! On 07/08/2012 06:25 PM, Frank Schäfer wrote: Hi, running kernel 3.5.rc6 with the two gspca-sn9c20x webcams 0c45:62b3 Microdia PC Camera with Microphone (SN9C202 + OV9655) 0c45:6270 Microdia PC Camera (SN9C201 + MI0360/MT9V011 or MI0360SOC/MT9V111) U-CAM PC Camera NE878, Whitcom WHC017, ... I'm getting the following kernel oops when I start capturing with qv4l2 (and also Kopete and others): [ 81.719973] gspca_sn9c20x: Set 640x480 [ 81.736805] BUG: unable to handle kernel NULL pointer dereference at 002c [ 81.736877] IP: [f7aebb59] v4l2_ctrl_g_ctrl+0x9/0x50 [videodev] [ 81.736901] *pdpt = 2c4fa001 *pde = [ 81.736922] Oops: [#1] PREEMPT SMP [ 81.737055] [ 81.737071] Pid: 4026, comm: qv4l2 Not tainted 3.4.0-0.1-desktop+ #19 System manufacturer System Product Name/M2N-VM DH [ 81.737101] EIP: 0060:[f7aebb59] EFLAGS: 00010292 CPU: 1 [ 81.737130] EIP is at v4l2_ctrl_g_ctrl+0x9/0x50 [videodev] [ 81.737147] EAX: EBX: ECX: f6c000c0 EDX: 0001 [ 81.737165] ESI: 0028 EDI: 0028 EBP: f5af9c84 ESP: f5af9c7c [ 81.737183] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 [ 81.737200] CR0: 80050033 CR2: 002c CR3: 2c941000 CR4: 07f0 [ 81.737219] DR0: DR1: DR2: DR3: [ 81.737236] DR6: 0ff0 DR7: 0400 [ 81.737252] Process qv4l2 (pid: 4026, ti=f5af8000 task=f594c030 task.ti=f5af8000) [ 81.737271] Stack: [ 81.737290] 0028 f61c2000 f5af9cd0 f7d1c74c 007f f5af9ca4 c0586ba1 [ 81.737318] ef190444 007f 0080 01e0 0280 0002 02000100 [ 81.737346] 003c2800 00f000a0 f61c2000 f5af9d84 f7b682c0 f71d8bec [ 81.737366] Call Trace: [ 81.737390] [f7d1c74c] sd_start+0x2dc/0x450 [gspca_sn9c20x] [ 81.737421] [c0586ba1] ? usb_alloc_coherent+0x21/0x30 [ 81.737448] [f7b682c0] gspca_init_transfer+0x260/0x420 [gspca_main] [ 81.737479] [c02ea322] ? __alloc_pages_nodemask+0x142/0x7b0 [ 81.737504] [f7b6926b] vidioc_streamon+0x9b/0xb0 [gspca_main] [ 81.737535] [f7ae4919] __video_do_ioctl+0x2ba9/0x5780 [videodev] [ 81.737565] [c031b7ee] ? alloc_pages_current+0x8e/0x100 [ 81.737588] [c0230998] ? kmap_atomic_prot+0xe8/0x110 [ 81.737611] [c04824ea] ? __copy_from_user_ll+0x1a/0x30 [ 81.737630] [c0482530] ? _copy_from_user+0x30/0x50 [ 81.737656] [f7ae1b0e] video_usercopy+0x1de/0x270 [videodev] [ 81.737679] [c0309fc8] ? mmap_region+0x1e8/0x4b0 [ 81.737706] [f7ae1bb2] video_ioctl2+0x12/0x20 [videodev] [ 81.737725] [f7ae1d70] ? v4l2_norm_to_name+0x50/0x50 [videodev] [ 81.737725] [f7ae064a] v4l2_ioctl+0xca/0x190 [videodev] [ 81.737725] [c030a401] ? do_mmap_pgoff+0x171/0x2e0 [ 81.737725] [f7ae0580] ? v4l2_open+0x120/0x120 [videodev] [ 81.737725] [c0344074] do_vfs_ioctl+0x74/0x2e0 [ 81.737725] [c0344347] sys_ioctl+0x67/0x80 [ 81.737725] [c07242dd] syscall_call+0x7/0xb [ 81.737725] Code: 55 f0 89 02 8b 46 10 8b 40 14 e8 03 61 c3 c8 89 f8 83 c4 04 5b 5e 5f 5d c3 89 f6 8d bc 27 00 00 00 00 55 89 e5 53 89 c3 83 ec 04 8b 40 2c c7 45 f8 00 00 00 00 8d 50 fb 83 fa 02 77 09 80 b8 d3 [ 81.737725] EIP: [f7aebb59] v4l2_ctrl_g_ctrl+0x9/0x50 [videodev] SS:ESP 0068:f5af9c7c [ 81.737725] CR2: 002c [ 81.743027] ---[ end trace 1c291740d69b151f ]--- This is a regression from kernel 3.4.x. The causing commit seems to be commit 63069da1c8ef0abcdb74b0ea1c461d23fb9181d9 Author: Hans Verkuil hans.verk...@cisco.com Date: Sun May 6 09:28:29 2012 -0300 [media] gcpca_sn9c20x: Convert to the control framework HdG: Small fix: don't register some controls for sensors which don't have an implementation for them. Signed-off-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Hans de Goede hdego...@redhat.com Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com Should I open a bug report at bugzilla, too ? That won't be necessary, the attached patch should fix this, can you give it a try please? Thank you Hans, I can confirm that your patch fixes the oops for both devices. Regards, Frank Thanks, Hans From e3d5933cf00270768b202041adf463ffc4fc9544 Mon Sep 17 00:00:00 2001 From: Hans de Goede hdego...@redhat.com Date: Sun, 8 Jul 2012 19:41:14 +0200 Subject: [PATCH] gspca_sn9c20x: Fix NULL pointer dereference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don't call v4l2_ctrl_g_ctrl on ctrls which the model cam in question does not have. Reported-by: Frank Schäfer fschaefer@googlemail.com Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/video/gspca/sn9c20x.c | 13
Re: video: USB webcam fails since kernel 3.2
Hi, On 07/08/2012 08:33 PM, Jean-Francois Moine wrote: On Sun, 08 Jul 2012 19:58:08 +0200 Hans de Goede hdego...@redhat.com wrote: Hmm, this is then likely caused by the new isoc bandwidth negotiation code in 3.2, unfortunately the vc032x driver is one of the few gspca drivers for which I don't have a cam to test with. Can you try to build your own kernel from source? Hi Martin-Éric, Instead of re-building the gspca driver from a kernel source, you may try the gspca test tarball from my web site http://moinejf.free.fr/gspca-2.15.18.tar.gz That is a good option too and easier then building a whole new kernel, but: It contains most of the bug fixes, including the one about the bandwidth problem. Right, but the problem with the vc032x driver is that there no bandwidth related bugfix for it yet, which is why I asked Martin-Éric, not only to build a new gspca driver from source, but also to try some modifications. Martin-Éric, Building the gspca test-tarbal also is a good way to test this: http://moinejf.free.fr/gspca-2.15.18.tar.gz But once you've confirmed the problem still happens with that version you will still need to try the changes I suggested to gspca.c to help us confirm that this is a bandwidth issue and try to come up with a fix. Thanks Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] radio-si470x: Lower firmware version requirements
Hi, On 07/09/2012 11:17 AM, Antti Palosaari wrote: On 07/07/2012 09:53 PM, Hans de Goede wrote: Hi, On 07/07/2012 11:00 AM, Antti Palosaari wrote: Hello Hans, On 07/07/2012 10:58 AM, Hans de Goede wrote: So is your device working properly now? The reason I'm asking it because it is still causing a firmware version warning, and if it works fine I would like to lower the firmware version warning point, so that the warning goes away. I don't know what is definition of properly in that case. Problem is that when I use radio application from xawtv3 with that new loopback I hear very often cracks and following errors are printed to the radio screen: ALSA lib pcm.c:7339:(snd_pcm_recover) underrun occurred or ALSA lib pcm.c:7339:(snd_pcm_recover) overrun occurred Looks like those does not appear, at least it does not crack so often nor errors seen, when I use Rhythmbox to tune and arecord -D hw:2,0 -r96000 -c2 -f S16_LE | aplay - to listen. I can guess those are not firmware related so warning texts could be removed. Actually they may very well be firmware related. At least with my firmware there is a bug where actively asking the device for its register contents, it lets its audio stream drop. My patches fix this by waiting for the device to volunteer it register contents through its usb interrupt in endpoint, which it does at xx times / sec. So the first question would be, does this dropping of sound happen approx 1 / sec when using radio? If so this is caused by radio upating the signal strength it displays 1 / sec. If you look at radio.c line 981 you will see a radio_getsignal_n_stereo(fd); call there in the main loop which gets called 1/sec. Try commenting this out. If commenting this out fixes your sound issues with radio, then the next question is are you using my 4 recent si470x patches, if not please give them a try. If you are already using them then I'm afraid that your older firmware may be broken even more then my also not so new firmware. I suspect these signal strength update pops are different thing. Those are almost so minor you cannot even hear. I recorded small sample: http://palosaari.fi/linux/v4l-dvb/xawtv3_radio.m4v And I am almost 100% sure those cracks are coming ALSA underrun/overrun as error text is seen just same time when crack happens. The signal updates are what is causing the ALSA under-runs (*), the over-runs are the result of catching up after a under-run. *) Or at least an important cause of them Commenting out line did not help. Are you sure about this? Did you do a make after commenting, are you sure you were using the new build to test? But I think I was also hearing those small pops too and likely new four patches fixes those - but it is hard to say because it cracks audio all time. One other thing you can try is increasing the buffer size, using: radio -L 1000 For example will increase it from the 500 millisecs default to 1 second Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] radio-si470x: Lower firmware version requirements
Hi, On 07/09/2012 01:58 PM, Antti Palosaari wrote: On 07/09/2012 01:03 PM, Hans de Goede wrote: If I tune using old radio it works. If I tune using latest radio but with option -l 0 (./console/radio -l 0) it also works. Using arecord -D hw:2,0 -r96000 -c2 -f S16_LE | aplay - to listen. So it seems that latest radio with alsa loopback is only having those problems. Ok. These are the patches: radio-si470x: Don't unnecesarily read registers on G_TUNER radio-si470x: Lower hardware freq seek signal treshold radio-si470x: Always use interrupt to wait for tune/seek completion radio-si470x: Lower firmware version requirements And from that I can see it loads new driver as it does not warn about software version - only firmware. Right, so what I want to do is to lower the firmware requirement to 12, so that it won't complain for your device since that seems to be working fine. Does that sound like a good idea to you? Jul 9 14:28:29 localhost kernel: [13403.017920] Linux media interface: v0.10 Jul 9 14:28:29 localhost kernel: [13403.020866] Linux video capture interface: v2.00 Jul 9 14:28:29 localhost kernel: [13403.022744] radio-si470x 5-2:1.2: DeviceID=0x1242 ChipID=0x060c Jul 9 14:28:29 localhost kernel: [13403.022747] radio-si470x 5-2:1.2: This driver is known to work with firmware version 14, Jul 9 14:28:29 localhost kernel: [13403.022749] radio-si470x 5-2:1.2: but the device has firmware version 12. Jul 9 14:28:29 localhost kernel: [13403.024715] radio-si470x 5-2:1.2: software version 1, hardware version 7 Jul 9 14:28:29 localhost kernel: [13403.024717] radio-si470x 5-2:1.2: If you have some trouble using this driver, Jul 9 14:28:29 localhost kernel: [13403.024719] radio-si470x 5-2:1.2: please report to V4L ML at linux-media@vger.kernel.org Jul 9 14:28:29 localhost kernel: [13403.114583] usbcore: registered new interface driver radio-si470x Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: video: USB webcam fails since kernel 3.2
Hi, On 07/09/2012 01:33 PM, Martin-Éric Racine wrote: snip Hmm, this is then likely caused by the new isoc bandwidth negotiation code in 3.2, unfortunately the vc032x driver is one of the few gspca drivers for which I don't have a cam to test with. Can you try to build your own kernel from source? Boot into your own kernel, and verify the regression is still there, then edit drivers/media/video/gspca/gspca.c and go to the which_bandwidth function, and at the beginning of this function add the following line: return 2000 * 2000 * 120; Then rebuild and re-install the kernel and try again. If that helps, remove the added return 2000 * 2000 * 120; line, and also remove the following lines from which_bandwidth: /* if the image is compressed, estimate its mean size */ if (!gspca_dev-cam.needs_full_bandwidth bandwidth gspca_dev-cam.cam_mode[i].width * gspca_dev-cam.cam_mode[i].height) bandwidth = bandwidth * 3 / 8; /* 0.375 */ And try again if things still work this way. Once you've tested this I can try to write a fix for this. Hans, Thank you for your reply. Just to eliminate the possibility of mistakes on my part while trying to perform the above changes, could you send me a patch against Linux 3.2.21 that I could apply as-is, before building myself a test kernel package? Erm, that is quite a bit of work from my side for something which you can easily do yourself, edit gspca.c, search for which_bandwidth and then under the following lines: u32 bandwidth; int i; Add a line like this: return 2000 * 2000 * 120; Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [regression 3.4-3.5, bisected] kernel oops when starting capturing from gspca-sn9c20x webcams
Hi, Thanks for reporting this! On 07/08/2012 06:25 PM, Frank Schäfer wrote: Hi, running kernel 3.5.rc6 with the two gspca-sn9c20x webcams 0c45:62b3 Microdia PC Camera with Microphone (SN9C202 + OV9655) 0c45:6270 Microdia PC Camera (SN9C201 + MI0360/MT9V011 or MI0360SOC/MT9V111) U-CAM PC Camera NE878, Whitcom WHC017, ... I'm getting the following kernel oops when I start capturing with qv4l2 (and also Kopete and others): [ 81.719973] gspca_sn9c20x: Set 640x480 [ 81.736805] BUG: unable to handle kernel NULL pointer dereference at 002c [ 81.736877] IP: [f7aebb59] v4l2_ctrl_g_ctrl+0x9/0x50 [videodev] [ 81.736901] *pdpt = 2c4fa001 *pde = [ 81.736922] Oops: [#1] PREEMPT SMP [ 81.737055] [ 81.737071] Pid: 4026, comm: qv4l2 Not tainted 3.4.0-0.1-desktop+ #19 System manufacturer System Product Name/M2N-VM DH [ 81.737101] EIP: 0060:[f7aebb59] EFLAGS: 00010292 CPU: 1 [ 81.737130] EIP is at v4l2_ctrl_g_ctrl+0x9/0x50 [videodev] [ 81.737147] EAX: EBX: ECX: f6c000c0 EDX: 0001 [ 81.737165] ESI: 0028 EDI: 0028 EBP: f5af9c84 ESP: f5af9c7c [ 81.737183] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 [ 81.737200] CR0: 80050033 CR2: 002c CR3: 2c941000 CR4: 07f0 [ 81.737219] DR0: DR1: DR2: DR3: [ 81.737236] DR6: 0ff0 DR7: 0400 [ 81.737252] Process qv4l2 (pid: 4026, ti=f5af8000 task=f594c030 task.ti=f5af8000) [ 81.737271] Stack: [ 81.737290] 0028 f61c2000 f5af9cd0 f7d1c74c 007f f5af9ca4 c0586ba1 [ 81.737318] ef190444 007f 0080 01e0 0280 0002 02000100 [ 81.737346] 003c2800 00f000a0 f61c2000 f5af9d84 f7b682c0 f71d8bec [ 81.737366] Call Trace: [ 81.737390] [f7d1c74c] sd_start+0x2dc/0x450 [gspca_sn9c20x] [ 81.737421] [c0586ba1] ? usb_alloc_coherent+0x21/0x30 [ 81.737448] [f7b682c0] gspca_init_transfer+0x260/0x420 [gspca_main] [ 81.737479] [c02ea322] ? __alloc_pages_nodemask+0x142/0x7b0 [ 81.737504] [f7b6926b] vidioc_streamon+0x9b/0xb0 [gspca_main] [ 81.737535] [f7ae4919] __video_do_ioctl+0x2ba9/0x5780 [videodev] [ 81.737565] [c031b7ee] ? alloc_pages_current+0x8e/0x100 [ 81.737588] [c0230998] ? kmap_atomic_prot+0xe8/0x110 [ 81.737611] [c04824ea] ? __copy_from_user_ll+0x1a/0x30 [ 81.737630] [c0482530] ? _copy_from_user+0x30/0x50 [ 81.737656] [f7ae1b0e] video_usercopy+0x1de/0x270 [videodev] [ 81.737679] [c0309fc8] ? mmap_region+0x1e8/0x4b0 [ 81.737706] [f7ae1bb2] video_ioctl2+0x12/0x20 [videodev] [ 81.737725] [f7ae1d70] ? v4l2_norm_to_name+0x50/0x50 [videodev] [ 81.737725] [f7ae064a] v4l2_ioctl+0xca/0x190 [videodev] [ 81.737725] [c030a401] ? do_mmap_pgoff+0x171/0x2e0 [ 81.737725] [f7ae0580] ? v4l2_open+0x120/0x120 [videodev] [ 81.737725] [c0344074] do_vfs_ioctl+0x74/0x2e0 [ 81.737725] [c0344347] sys_ioctl+0x67/0x80 [ 81.737725] [c07242dd] syscall_call+0x7/0xb [ 81.737725] Code: 55 f0 89 02 8b 46 10 8b 40 14 e8 03 61 c3 c8 89 f8 83 c4 04 5b 5e 5f 5d c3 89 f6 8d bc 27 00 00 00 00 55 89 e5 53 89 c3 83 ec 04 8b 40 2c c7 45 f8 00 00 00 00 8d 50 fb 83 fa 02 77 09 80 b8 d3 [ 81.737725] EIP: [f7aebb59] v4l2_ctrl_g_ctrl+0x9/0x50 [videodev] SS:ESP 0068:f5af9c7c [ 81.737725] CR2: 002c [ 81.743027] ---[ end trace 1c291740d69b151f ]--- This is a regression from kernel 3.4.x. The causing commit seems to be commit 63069da1c8ef0abcdb74b0ea1c461d23fb9181d9 Author: Hans Verkuil hans.verk...@cisco.com Date: Sun May 6 09:28:29 2012 -0300 [media] gcpca_sn9c20x: Convert to the control framework HdG: Small fix: don't register some controls for sensors which don't have an implementation for them. Signed-off-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Hans de Goede hdego...@redhat.com Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com Should I open a bug report at bugzilla, too ? That won't be necessary, the attached patch should fix this, can you give it a try please? Thanks, Hans From e3d5933cf00270768b202041adf463ffc4fc9544 Mon Sep 17 00:00:00 2001 From: Hans de Goede hdego...@redhat.com Date: Sun, 8 Jul 2012 19:41:14 +0200 Subject: [PATCH] gspca_sn9c20x: Fix NULL pointer dereference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don't call v4l2_ctrl_g_ctrl on ctrls which the model cam in question does not have. Reported-by: Frank Schäfer fschaefer@googlemail.com Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/video/gspca/sn9c20x.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/media/video/gspca/sn9c20x.c b/drivers/media/video/gspca/sn9c20x.c index 6c31e46..b9c6f17 100644 --- a/drivers/media/video/gspca/sn9c20x.c +++ b/drivers/media/video/gspca/sn9c20x.c @@ -2070,10 +2070,13 @@ static int sd_start(struct gspca_dev *gspca_dev) set_gamma(gspca_dev, v4l2_ctrl_g_ctrl(sd-gamma
Re: video: USB webcam fails since kernel 3.2
Hi, On 07/08/2012 03:01 PM, Martin-Éric Racine wrote: 2012/6/17 Martin-Éric Racine martin-eric.rac...@iki.fi: pe, 2012-06-15 kello 23:41 -0500, Jonathan Nieder kirjoitti: Martin-Éric Racine wrote: usb 1-7: new high-speed USB device number 3 using ehci_hcd [...] usb 1-7: New USB device found, idVendor=0ac8, idProduct=0321 usb 1-7: New USB device strings: Mfr=1, Product=2, SerialNumber=0 usb 1-7: Product: USB2.0 Web Camera usb 1-7: Manufacturer: Vimicro Corp. [...] Linux media interface: v0.10 Linux video capture interface: v2.00 gspca_main: v2.14.0 registered gspca_main: vc032x-2.14.0 probing 0ac8:0321 usbcore: registered new interface driver vc032x The device of interest is discovered. gspca_main: ISOC data error: [36] len=0, status=-71 gspca_main: ISOC data error: [65] len=0, status=-71 [...] gspca_main: ISOC data error: [48] len=0, status=-71 video_source:sr[3246]: segfault at 0 ip (null) sp ab36de1c error 14 in cheese[8048000+21000] gspca_main: ISOC data error: [17] len=0, status=-71 (The above data error spew starts around t=121 seconds and continues at a rate of about 15 messages per second. The segfault is around t=154.) The vc032x code hasn't changed since 3.4.1, so please report your symptoms to Jean-François Moine moin...@free.fr, cc-ing linux-media@vger.kernel.org, linux-ker...@vger.kernel.org, and either me or this bug log so we can track it. Be sure to mention: - steps to reproduce, expected result, actual result, and how the difference indicates a bug (should be simple enough in this case) 1. Ensure that user 'myself' is a member of the 'video' group. 2. Launch the webcam application Cheese from the GNOME desktop. Expected result: Cheese displays whatever this laptop's camera sees. Actual result: Cheese crashes while attempting to access the camera. - how reproducible the bug is (100%?) 100% - which kernel versions you have tested and result with each (what is the newest kernel version that worked?) It probably was 3.1.0 or some earlier 3.2 release (the upcoming Debian will release with 3.2.x; 3.4 was only used here for testing purposes), but I wouldn't know for sure since I don't use my webcam too often. I finally found time to perform further testing, using kernel packages from snapshots.debian.org, and the last one that positively worked (at least using GNOME's webcam application Cheese) was: linux-image-3.1.0-1-686-pae 3.1.8-2 Linux 3.1 for modern PCs This loaded the following video modules: gspca_vc032x gspca_main videodev media Tests using 3.2.1-1 or more recent crashed as described before. This at least gives us a time frame for when the regression started. Hmm, this is then likely caused by the new isoc bandwidth negotiation code in 3.2, unfortunately the vc032x driver is one of the few gspca drivers for which I don't have a cam to test with. Can you try to build your own kernel from source? Boot into your own kernel, and verify the regression is still there, then edit drivers/media/video/gspca/gspca.c and go to the which_bandwidth function, and at the beginning of this function add the following line: return 2000 * 2000 * 120; Then rebuild and re-install the kernel and try again. If that helps, remove the added return 2000 * 2000 * 120; line, and also remove the following lines from which_bandwidth: /* if the image is compressed, estimate its mean size */ if (!gspca_dev-cam.needs_full_bandwidth bandwidth gspca_dev-cam.cam_mode[i].width * gspca_dev-cam.cam_mode[i].height) bandwidth = bandwidth * 3 / 8; /* 0.375 */ And try again if things still work this way. Once you've tested this I can try to write a fix for this. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] radio-si470x: Lower firmware version requirements
Hi, So is your device working properly now? The reason I'm asking it because it is still causing a firmware version warning, and if it works fine I would like to lower the firmware version warning point, so that the warning goes away. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] media/video: Add v4l2-common.h to userspace headers
The user header CHECK in usr/include/linux was (rightfully) failing because of v4l2-common.h missing, this patch fixes this. Signed-off-by: Hans de Goede hdego...@redhat.com --- include/linux/Kbuild |1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/Kbuild b/include/linux/Kbuild index d38b3a8..ef4cc94 100644 --- a/include/linux/Kbuild +++ b/include/linux/Kbuild @@ -382,6 +382,7 @@ header-y += usbdevice_fs.h header-y += utime.h header-y += utsname.h header-y += uvcvideo.h +header-y += v4l2-common.h header-y += v4l2-dv-timings.h header-y += v4l2-mediabus.h header-y += v4l2-subdev.h -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] v4l2-ioctl: Don't assume file-private_data always points to a v4l2_fh
Commit efbceecd4522a41b8442c6b4f68b4508d57d1ccf, adds a number of helper functions for ctrl related ioctls to v4l2-ioctl.c, these helpers assume that if file-private_data != NULL, it points to a v4l2_fh, which is only the case for drivers which actually use v4l2_fh. This breaks for example bttv which use the filedata pointer for its own uses, and now all the ctrl ioctls try to use whatever its filedata points to as v4l2_fh and think it has a ctrl_handler, leading to: [ 142.499214] BUG: unable to handle kernel NULL pointer dereference at 0021 [ 142.499270] IP: [a01cb959] v4l2_queryctrl+0x29/0x230 [videodev] [ 142.514649] [a01c7a77] v4l_queryctrl+0x47/0x90 [videodev] [ 142.517417] [a01c58b1] __video_do_ioctl+0x2c1/0x420 [videodev] [ 142.520116] [a01c7ee6] video_usercopy+0x1a6/0x470 [videodev] ... This patch adds the missing test_bit(V4L2_FL_USES_V4L2_FH, vfd-flags) tests to the ctrl ioctl helpers v4l2_fh paths, fixing the issues with for example the bttv driver. Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/video/v4l2-ioctl.c | 21 ++--- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c index 70e0efb..3c498b2 100644 --- a/drivers/media/video/v4l2-ioctl.c +++ b/drivers/media/video/v4l2-ioctl.c @@ -1488,7 +1488,8 @@ static int v4l_queryctrl(const struct v4l2_ioctl_ops *ops, struct v4l2_queryctrl *p = arg; struct v4l2_fh *vfh = fh; - if (vfh vfh-ctrl_handler) + if (test_bit(V4L2_FL_USES_V4L2_FH, vfd-flags) + vfh vfh-ctrl_handler) return v4l2_queryctrl(vfh-ctrl_handler, p); if (vfd-ctrl_handler) return v4l2_queryctrl(vfd-ctrl_handler, p); @@ -1504,7 +1505,8 @@ static int v4l_querymenu(const struct v4l2_ioctl_ops *ops, struct v4l2_querymenu *p = arg; struct v4l2_fh *vfh = fh; - if (vfh vfh-ctrl_handler) + if (test_bit(V4L2_FL_USES_V4L2_FH, vfd-flags) + vfh vfh-ctrl_handler) return v4l2_querymenu(vfh-ctrl_handler, p); if (vfd-ctrl_handler) return v4l2_querymenu(vfd-ctrl_handler, p); @@ -1522,7 +1524,8 @@ static int v4l_g_ctrl(const struct v4l2_ioctl_ops *ops, struct v4l2_ext_controls ctrls; struct v4l2_ext_control ctrl; - if (vfh vfh-ctrl_handler) + if (test_bit(V4L2_FL_USES_V4L2_FH, vfd-flags) + vfh vfh-ctrl_handler) return v4l2_g_ctrl(vfh-ctrl_handler, p); if (vfd-ctrl_handler) return v4l2_g_ctrl(vfd-ctrl_handler, p); @@ -1555,7 +1558,8 @@ static int v4l_s_ctrl(const struct v4l2_ioctl_ops *ops, struct v4l2_ext_controls ctrls; struct v4l2_ext_control ctrl; - if (vfh vfh-ctrl_handler) + if (test_bit(V4L2_FL_USES_V4L2_FH, vfd-flags) + vfh vfh-ctrl_handler) return v4l2_s_ctrl(vfh, vfh-ctrl_handler, p); if (vfd-ctrl_handler) return v4l2_s_ctrl(NULL, vfd-ctrl_handler, p); @@ -1582,7 +1586,8 @@ static int v4l_g_ext_ctrls(const struct v4l2_ioctl_ops *ops, struct v4l2_fh *vfh = fh; p-error_idx = p-count; - if (vfh vfh-ctrl_handler) + if (test_bit(V4L2_FL_USES_V4L2_FH, vfd-flags) + vfh vfh-ctrl_handler) return v4l2_g_ext_ctrls(vfh-ctrl_handler, p); if (vfd-ctrl_handler) return v4l2_g_ext_ctrls(vfd-ctrl_handler, p); @@ -1600,7 +1605,8 @@ static int v4l_s_ext_ctrls(const struct v4l2_ioctl_ops *ops, struct v4l2_fh *vfh = fh; p-error_idx = p-count; - if (vfh vfh-ctrl_handler) + if (test_bit(V4L2_FL_USES_V4L2_FH, vfd-flags) + vfh vfh-ctrl_handler) return v4l2_s_ext_ctrls(vfh, vfh-ctrl_handler, p); if (vfd-ctrl_handler) return v4l2_s_ext_ctrls(NULL, vfd-ctrl_handler, p); @@ -1618,7 +1624,8 @@ static int v4l_try_ext_ctrls(const struct v4l2_ioctl_ops *ops, struct v4l2_fh *vfh = fh; p-error_idx = p-count; - if (vfh vfh-ctrl_handler) + if (test_bit(V4L2_FL_USES_V4L2_FH, vfd-flags) + vfh vfh-ctrl_handler) return v4l2_try_ext_ctrls(vfh-ctrl_handler, p); if (vfd-ctrl_handler) return v4l2_try_ext_ctrls(vfd-ctrl_handler, p); -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] radio-si470x: Lower firmware version requirements
Hi, On 07/07/2012 11:00 AM, Antti Palosaari wrote: Hello Hans, On 07/07/2012 10:58 AM, Hans de Goede wrote: So is your device working properly now? The reason I'm asking it because it is still causing a firmware version warning, and if it works fine I would like to lower the firmware version warning point, so that the warning goes away. I don't know what is definition of properly in that case. Problem is that when I use radio application from xawtv3 with that new loopback I hear very often cracks and following errors are printed to the radio screen: ALSA lib pcm.c:7339:(snd_pcm_recover) underrun occurred or ALSA lib pcm.c:7339:(snd_pcm_recover) overrun occurred Looks like those does not appear, at least it does not crack so often nor errors seen, when I use Rhythmbox to tune and arecord -D hw:2,0 -r96000 -c2 -f S16_LE | aplay - to listen. I can guess those are not firmware related so warning texts could be removed. Actually they may very well be firmware related. At least with my firmware there is a bug where actively asking the device for its register contents, it lets its audio stream drop. My patches fix this by waiting for the device to volunteer it register contents through its usb interrupt in endpoint, which it does at xx times / sec. So the first question would be, does this dropping of sound happen approx 1 / sec when using radio? If so this is caused by radio upating the signal strength it displays 1 / sec. If you look at radio.c line 981 you will see a radio_getsignal_n_stereo(fd); call there in the main loop which gets called 1/sec. Try commenting this out. If commenting this out fixes your sound issues with radio, then the next question is are you using my 4 recent si470x patches, if not please give them a try. If you are already using them then I'm afraid that your older firmware may be broken even more then my also not so new firmware. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] v4l2-ioctl: Don't assume file-private_data always points to a v4l2_fh
Hi, On 07/07/2012 09:11 PM, Hans Verkuil wrote: On Sat July 7 2012 20:46:21 Hans de Goede wrote: Commit efbceecd4522a41b8442c6b4f68b4508d57d1ccf, adds a number of helper functions for ctrl related ioctls to v4l2-ioctl.c, these helpers assume that if file-private_data != NULL, it points to a v4l2_fh, which is only the case for drivers which actually use v4l2_fh. This breaks for example bttv which use the filedata pointer for its own uses, and now all the ctrl ioctls try to use whatever its filedata points to as v4l2_fh and think it has a ctrl_handler, leading to: [ 142.499214] BUG: unable to handle kernel NULL pointer dereference at 0021 [ 142.499270] IP: [a01cb959] v4l2_queryctrl+0x29/0x230 [videodev] [ 142.514649] [a01c7a77] v4l_queryctrl+0x47/0x90 [videodev] [ 142.517417] [a01c58b1] __video_do_ioctl+0x2c1/0x420 [videodev] [ 142.520116] [a01c7ee6] video_usercopy+0x1a6/0x470 [videodev] ... This patch adds the missing test_bit(V4L2_FL_USES_V4L2_FH, vfd-flags) tests to the ctrl ioctl helpers v4l2_fh paths, fixing the issues with for example the bttv driver. Urgh, I didn't test with a 'old' driver. Thanks for catching this. But I would prefer a simpler patch (although effectively the same) like this: diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c index 70e0efb..bbcb4f6 100644 --- a/drivers/media/video/v4l2-ioctl.c +++ b/drivers/media/video/v4l2-ioctl.c @@ -1486,7 +1486,7 @@ static int v4l_queryctrl(const struct v4l2_ioctl_ops *ops, { struct video_device *vfd = video_devdata(file); struct v4l2_queryctrl *p = arg; - struct v4l2_fh *vfh = fh; + struct v4l2_fh *vfh = test_bit(V4L2_FL_USES_V4L2_FH, vfd-flags) ? fh : NULL; if (vfh vfh-ctrl_handler) return v4l2_queryctrl(vfh-ctrl_handler, p); snip I agree that that is better, so I've added that version to my tree now, for which I expect / hope to send a pullreq later tonight. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL for 3.6] gspca control-framework conversions and radio drivers work
Hi Mauro, Please pull from my git tree for: -A ton of gspca control-framework conversions (mostly done By Hans V, reviewed and tested by me) -Some radio driver fixes -2 new drivers for USB radio devices -2 v4l2-core regression fixes for your current for_v3.6 tree The following changes since commit b7e386360922a15f943b2fbe8d77a19bb86f2e6f: media: Revert [media] Terratec Cinergy S2 USB HD Rev.2 (2012-07-07 00:19:20 -0300) are available in the git repository at: git://linuxtv.org/hgoede/gspca.git media-for_v3.6 for you to fetch changes up to 93330babccc2e0777430834435bdd4bb34db3ff6: shark2: New driver for the Griffin radioSHARK v2 USB radio receiver (2012-07-07 23:36:44 +0200) Antonio Ospite (2): gspca_kinect: remove traces of the gspca control mechanism gspca_ov534: Convert to the control framework Hans Verkuil (30): gspca-conex: convert to the control framework. gspca-cpia1: convert to the control framework. gspca-etoms: convert to the control framework. gspca-jeilinj: convert to the control framework. gspca-konica: convert to the control framework. gspca-mr97310a: convert to the control framework. nw80x: convert to the control framework. ov519: convert to the control framework. ov534_9: convert to the control framework. es401: convert to the control framework. spca1528: convert to the control framework. spca500: convert to the control framework. spca501: convert to the control framework. spca505: convert to the control framework. spca506: convert to the control framework. spca508: convert to the control framework. spca561: convert to the control framework. sq930x: convert to the control framework. stk014: convert to the control framework. sunplus: convert to the control framework. gspca_t613: convert to the control framework tv8532: convert to the control framework. vicam: convert to the control framework. xirlink_cit: convert to the control framework. vc032x: convert to the control framework. gspca-topro: convert to the control framework. gspca: clear priv field and disable relevant ioctls. gspca: always call v4l2_ctrl_handler_setup after start. gspca-spca501: remove old function prototypes. v4l2-ioctl: Don't assume file-private_data always points to a v4l2_fh Hans de Goede (24): snd_tea575x: Add write_/read_val operations snd_tea575x: Add a cannot_mute flag radio-shark: New driver for the Griffin radioSHARK USB radio receiver radio-si470x: Don't unnecesarily read registers on G_TUNER radio-si470x: Always use interrupt to wait for tune/seek completion radio-si470x: Lower hardware freq seek signal treshold radio-si470x: Lower firmware version requirements gspca_pac7302: Convert to the control framework gscpa_sonixb: Use usb_err for error handling gscpa_sonixb: Convert to the control framework gspca_sonixb: Fix OV7630 gain control gspca: Remove bogus JPEG quality controls from various sub-drivers gspca_benq: Remove empty ctrls array gspca: Add reset_resume callback to all sub-drivers gspca_konica: Fix init sequence gspca_sn9c2028: Remove empty ctrls array gscpa_spca561: Add brightness control for rev12a cams gspca_stv0680: Remove empty ctrls array gspca_t613: Disable CIF resolutions gspca_xirlink_cit: Grab backlight compensation control while streaming gspca: Don't use video_device_node_name in v4l2_device release handler v4l2-ctrls: Teach v4l2-ctrls that V4L2_CID_AUTOBRIGHTNESS is a boolean media/video: Add v4l2-common.h to userspace headers shark2: New driver for the Griffin radioSHARK v2 USB radio receiver Documentation/DocBook/media/v4l/controls.xml |5 + drivers/hid/hid-core.c |1 + drivers/hid/hid-ids.h|1 + drivers/media/radio/Kconfig | 33 + drivers/media/radio/Makefile |4 + drivers/media/radio/radio-shark.c| 376 ++ drivers/media/radio/radio-shark2.c | 348 + drivers/media/radio/radio-tea5777.c | 489 + drivers/media/radio/radio-tea5777.h | 87 +++ drivers/media/radio/si470x/radio-si470x-common.c | 68 +- drivers/media/radio/si470x/radio-si470x-i2c.c|5 +- drivers/media/radio/si470x/radio-si470x-usb.c| 39 +- drivers/media/radio/si470x/radio-si470x.h|4 +- drivers/media/video/gspca/benq.c |7 +- drivers/media/video/gspca/conex.c| 208 ++ drivers/media/video/gspca/cpia1.c| 486 - drivers/media/video/gspca/etoms.c| 221 ++ drivers/media/video/gspca/finepix.c
Re: [PATCH 0/6] Add frequency band information
Series looks good to me, ack series: Acked-by: Hans de Goede hdego...@redhat.com On 07/05/2012 12:25 PM, Hans Verkuil wrote: Hi Mauro, This should be the final patch series for adding multiband support to the kernel. This patch series assumes that this pull request was merged first: http://patchwork.linuxtv.org/patch/13180/ Changes since the previous RFC patch series: (See http://www.mail-archive.com/linux-media@vger.kernel.org/msg48549.html) - The name field was dropped. - A new modulation field was added that describes the possible modulation systems for that frequency band (currently only one modulation system can be supported per band). - Compat code was added to allow VIDIOC_ENUM_FREQ_BANDS to be used for existing drivers. A note regarding the cadet driver: I want to do a follow-up patch to this at a later date so that it uses the tea575x-tuner framework. But with these patches it will at least work correctly again. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] radio-si470x: Lower firmware version requirements
Hi, On 07/05/2012 03:35 PM, Antti Palosaari wrote: snip I believe you're doing something wrong ... I compiled radio from http://git.linuxtv.org/xawtv3.git to tune and arecord -r96000 -c2 -f S16_LE | aplay - to play sound. Just silent white noise is heard. You're not specifying which device arecord should record from so likely it is taking the default input of your soundcard (line/mic in), rather then recording from the radio device. I tried to define hw:1,0 etc. but only hw:0,0 exists. Note the latest radio from http://git.linuxtv.org/xawtv3.git will do the digital loopback of the sound itself, so try things again with running arecord / aplay, if you then start radio and exit again (so that you can see its startup messages) you should see something like this: Using alsa loopback: cap: hw:1,0 (/dev/radio0), out: default Note radio will automatically select the correct alsa device to record from for the radio-usb-stick. For some reason I don't see that happening. Hmm, so it seems that for some reason alsa is not working with the usb sound-card part of the usb-stick. Can you try doing: ls /dev/snd/ Before and after plugging in the device, you should get a new PCMC?D0c device there. Otherwise see if you can enable some debugging options for snd-usb-audio and find out why it is not liking your device (and maybe at a quirk for it somewhere) ? If you do end up adding a quirk please let me know and I'll test with mine to ensure the quirck does not break working versions :) Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] radio-si470x: Lower firmware version requirements
Hi, On 07/05/2012 04:13 PM, Antti Palosaari wrote: On 07/05/2012 04:41 PM, Hans de Goede wrote: Hi, On 07/05/2012 03:35 PM, Antti Palosaari wrote: snip I believe you're doing something wrong ... I compiled radio from http://git.linuxtv.org/xawtv3.git to tune and arecord -r96000 -c2 -f S16_LE | aplay - to play sound. Just silent white noise is heard. You're not specifying which device arecord should record from so likely it is taking the default input of your soundcard (line/mic in), rather then recording from the radio device. I tried to define hw:1,0 etc. but only hw:0,0 exists. Note the latest radio from http://git.linuxtv.org/xawtv3.git will do the digital loopback of the sound itself, so try things again with running arecord / aplay, if you then start radio and exit again (so that you can see its startup messages) you should see something like this: Using alsa loopback: cap: hw:1,0 (/dev/radio0), out: default Note radio will automatically select the correct alsa device to record from for the radio-usb-stick. For some reason I don't see that happening. Hmm, so it seems that for some reason alsa is not working with the usb sound-card part of the usb-stick. Can you try doing: ls /dev/snd/ Before and after plugging in the device, you should get a new PCMC?D0c device there. Two files appears, controlC2 and pcmC2D0c. Otherwise see if you can enable some debugging options for snd-usb-audio and find out why it is not liking your device (and maybe at a quirk for it somewhere) ? If you do end up adding a quirk please let me know and I'll test with mine to ensure the quirck does not break working versions :) And now I can hear the voice too using arecord -D hw:2,0 -r96000 -c2 -f S16_LE | aplay -. But loopback is still missing. So if you start radio before starting the arecord, it won't do the loopback for you? Have you compiled xawtv with alsa support? (this requires the libalsa headers to be installed) Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/6] videodev2.h: add VIDIOC_ENUM_FREQ_BANDS.
Hi, On 07/04/2012 10:35 AM, Hans Verkuil wrote: snip snip Can we have a (hopefully short) irc discussion today? I'd really like to get this API finalized. +1, I'm available the entire day (CET office hours + evening if needed that is) snip snip So my current proposal is: use a bitfield in v4l2_frequency_band to describe possible (de)modulators and add compat code to the v4l2-ioctl.c to automatically create a vidioc_enum_freq_bands op if no such op was supplied, using the data from g_tuner or g_modulator and which device node was used to fill in the fields. +1 Regards, The other Hans :) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/6] Add frequency band information
Hi, Series looks good to me, ack series: Acked-by: Hans de Goede hdego...@redhat.com Regards, Hans On 07/02/2012 04:15 PM, Hans Verkuil wrote: Hi all, This patch series adds support for the new VIDIOC_ENUM_FREQ_BANDS ioctl that tells userspace if a tuner supports multiple frequency bands. This API is based on earlier attempts: http://www.spinics.net/lists/linux-media/msg48391.html http://www.spinics.net/lists/linux-media/msg48435.html And an irc discussion: http://linuxtv.org/irc/v4l/index.php?date=2012-06-26 This irc discussion also talked about adding rangelow/high to the v4l2_hw_freq_seek struct, but I decided not to do that. The hwseek additions are independent to this patch series, and I think it is best if Hans de Goede does that part so that that API change can be made together with a driver that actually uses it. In order to show how the new ioctl is used, this patch series adds support for it to the very, very old radio-cadet driver. Comments are welcome! Regards, Hans PS: Mauro, I haven't been able to work on the radio profile yet, so that's not included. -- 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: [git:v4l-dvb/for_v3.6] [media] gspca-core: Fix buffers staying in queued state after a stream_off
Hi, On 06/29/2012 07:10 AM, Ben Hutchings wrote: On Mon, 2012-06-11 at 21:06 +0200, Mauro Carvalho Chehab wrote: This is an automatic generated email to let you know that the following patch were queued at the http://git.linuxtv.org/media_tree.git tree: Subject: [media] gspca-core: Fix buffers staying in queued state after a stream_off Author: Hans de Goede hdego...@redhat.com Date:Tue May 22 11:24:05 2012 -0300 This fixes a regression introduced by commit f7059ea and should be backported to all supported stable kernels which have this commit. Signed-off-by: Hans de Goede hdego...@redhat.com Tested-by: Antonio Ospite osp...@studenti.unina.it CC: sta...@vger.kernel.org Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com [...] This surely can't both be so important that it should go into stable updates, yet so unimportant that it can wait for 3.6. You're right. This patch was part of a pull-request titled: [GIT PULL FIXES FOR 3.5]: gspca radio fixes Mauro, as the title of the mail suggested this pull-req contains only fixes, which should really go into 3.5. Esp. The one pointed out by Ben, but for example also the bttv fixes really belong in 3.5 (another user has already independently verified that they fix the radio on his bttv card too). They really are all bug-fixes :) Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
Hi, On 06/22/2012 06:15 PM, Mauro Carvalho Chehab wrote: Em 22-06-2012 11:07, Hans Verkuil escreveu: snip Reusing G_TUNER/S_TUNER or not, the issue is that a bitfield parameter for frequency range is not actually able to express what are the supported ranges. As I said before, the tuner ranges can only be properly expressed by an array with: - range low/high; - modulation (AM, FM, ...); - sub-carriers (mono, stereo, lang1, lang2); - properties (RDS, seek caps, ...). Agreed. So, in my opinion we still need the band field, but instead of this being a fixed band it is an index. In order to enumerate over all bands I propose a new ioctl: VIDIOC_ENUM_TUNER_BAND with struct: struct v4l2_tuner_band { __u32 tuner; __u32 index; char name[32]; __u32 capability; /* The same as in v4l2_tuner */ __u32 rangelow; __u32 rangehigh; __u32 reserved[7]; }; It enumerates the supported bands by the tuner, each with a human readable name, frequency range and capabilities. If you change the band using S_TUNER, then G_TUNER will return the frequency range and capabilities from the corresponding v4l2_tuner_band struct. The only capability that needs to be added is one that tells the application that the tuner has the capability to do band enumeration (V4L2_TUNER_CAP_HAS_BANDS or something). I am not 100% certain about the name field: it is very nice for apps, but we do need some guidelines here. A similar struct would be needed for modulators if we ever need to add support for modulators with multiple bands. We could perhaps rename v4l2_tuner_band to just v4l2_band to make it tuner/modulator agnostic. I've not replied before because I've been thinking about Hans V's proposal for a bit, I've come to the conclusion that Hans V's proposal is better, because it avoids a discrepancy in how tuners work between tv and radio, which is something which worried me about my own proposal. Hans V's proposal also has the benefit that it will work fine for tv-tuners too, so if we ever need bands for tv tuners we can use the *same* API. The above proposal would be great if we were starting to write the radio API today, but your proposal is not backward compatible with the status quo. Huh? Hans V. is proposing adding a band field to the tuner struct (using one of the reserved fields) and adding a new ioctl to enumerate bands. Old apps will have that field set to 0 on a S_TUNER, selecting band 0, and G_TUNER will give info on the current band, where-as S/G_FREQ will be unmodified (they will work on the current band). I don't see how this breaks current apps... Anyways both proposals seem workable to me, although I prefer Hans V.'s one. Lets just pick one and get on with this. Regards, Hans Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
Hi, On 06/19/2012 02:47 AM, Mauro Carvalho Chehab wrote: Em 28-05-2012 07:46, Hans Verkuil escreveu: From: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Hans de Goede hdego...@redhat.com --- include/linux/videodev2.h | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 2339678..013ee46 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -2023,7 +2023,8 @@ struct v4l2_tuner { __u32 audmode; __s32 signal; __s32 afc; - __u32 reserved[4]; + __u32 band; + __u32 reserved[3]; }; struct v4l2_modulator { @@ -2033,7 +2034,8 @@ struct v4l2_modulator { __u32 rangelow; __u32 rangehigh; __u32 txsubchans; - __u32 reserved[4]; + __u32 band; + __u32 reserved[3]; }; /* Flags for the 'capability' field */ @@ -2048,6 +2050,11 @@ struct v4l2_modulator { #define V4L2_TUNER_CAP_RDS 0x0080 #define V4L2_TUNER_CAP_RDS_BLOCK_IO 0x0100 #define V4L2_TUNER_CAP_RDS_CONTROLS 0x0200 +#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US 0x0001 +#define V4L2_TUNER_CAP_BAND_FM_JAPAN 0x0002 +#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN 0x0004 +#define V4L2_TUNER_CAP_BAND_FM_WEATHER 0x0008 +#define V4L2_TUNER_CAP_BAND_AM_MW0x0010 Frequency band is already specified by rangelow/rangehigh. Why do you need to duplicate this information? Because radio tuners may support multiple non overlapping bands, this is why this patch also adds a band member to the tuner struct, which can be used to set/get the current band. One example of this are the tea5757 / tea5759 radio tuner chips: FM: tea5757 87.5 - 108 MHz tea5759 76 - 91 MHz AM: Both: 530 - 1710 kHz So an app would set as band one of DEFAULT, EUROPE_US (or JAPAN depending on the model) and AM_MW, and then get the actual range supported reported in rangelow / rangehigh on a subsequent G_TUNER. Note that setting ie a band of FM_JAPAN on a 5757 would result in the S_TUNER failing with -EINVAL. /* Flags for the 'rxsubchans' field */ #define V4L2_TUNER_SUB_MONO 0x0001 @@ -2065,6 +2072,14 @@ struct v4l2_modulator { #define V4L2_TUNER_MODE_LANG10x0003 #define V4L2_TUNER_MODE_LANG1_LANG2 0x0004 +/* Values for the 'band' field */ +#define V4L2_TUNER_BAND_DEFAULT 0 What does default mean? Default means default. This is for compatibility with old apps which don't know about the new tuner band API extension so they will set this field to 0 (as reserved fields should be set to 0 by userspace). In this case we don't want to fail with -EINVAL based on the band value, so we need some value all tuners will accept. Some tuners, ie the si470x support both selecting a specific FM band, as well as selecting a universal FM band of 76 - 108 MHz. For those default would be the universal FM band. For the tea575x devices discussed above default would have the range of whatever FM band they support. Note that even on devices with a universal band being able to select a certain band is quite useful to limit hardware freq-seek to this band since searching freqs below 87.5 is useless in europe / US for example. Thinking more about this I think we should rename V4L2_TUNER_BAND_DEFAULT to V4L2_TUNER_BAND_FM_UNIVERSAL, and document that this means the widest FM band the device supports, with the actual limits being reported in rangelow and rangehigh. Note that the mentioned ranges by the bands are indications of the expected range only the true range will still be reported through rangelow and rangehigh, and this is what apps are expected to use. Defining 0 as V4L2_TUNER_BAND_FM_UNIVERSAL does cause a -EINVAL when doing a S_TUNER with a band value of 0 on AM only tuners, but: 1) We don't support AM only tuners atm, and I don't expect we will in the future either 2) Non band aware apps don't work well with AM tuners anyways (as they must take much smaller frequency steps for one). +#define V4L2_TUNER_BAND_FM_EUROPE_US 1 /* 87.5 Mhz - 108 MHz */ EUROPE_US is a bad name for this range. According with Wikipedia, this range is used at ITU region 1 (Europe/Africa), while America uses ITU region 2 (88-108). In Brazil, the range from 87.5-88 were added several years ago, so it is currently at the ITU region 1 range, just like in US. I don't doubt that there are still some places at the 88-108 MHz range. 87.5 - 108 MHz is very close to 88 - 108 MHz, I don't think it is worth creating 2 band defines for this. +#define V4L2_TUNER_BAND_FM_JAPAN 2 /* 76 MHz - 90 MHz */ This is currently true
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
Hi, On 06/19/2012 01:09 PM, Mauro Carvalho Chehab wrote: Em 19-06-2012 05:27, Hans de Goede escreveu: Hi, On 06/19/2012 02:47 AM, Mauro Carvalho Chehab wrote: Em 28-05-2012 07:46, Hans Verkuil escreveu: From: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Hans de Goede hdego...@redhat.com --- include/linux/videodev2.h | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 2339678..013ee46 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -2023,7 +2023,8 @@ struct v4l2_tuner { __u32audmode; __s32signal; __s32afc; -__u32reserved[4]; +__u32band; +__u32reserved[3]; }; struct v4l2_modulator { @@ -2033,7 +2034,8 @@ struct v4l2_modulator { __u32rangelow; __u32rangehigh; __u32txsubchans; -__u32reserved[4]; +__u32band; +__u32reserved[3]; }; /* Flags for the 'capability' field */ @@ -2048,6 +2050,11 @@ struct v4l2_modulator { #define V4L2_TUNER_CAP_RDS0x0080 #define V4L2_TUNER_CAP_RDS_BLOCK_IO0x0100 #define V4L2_TUNER_CAP_RDS_CONTROLS0x0200 +#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US 0x0001 +#define V4L2_TUNER_CAP_BAND_FM_JAPAN 0x0002 +#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN 0x0004 +#define V4L2_TUNER_CAP_BAND_FM_WEATHER 0x0008 +#define V4L2_TUNER_CAP_BAND_AM_MW0x0010 Frequency band is already specified by rangelow/rangehigh. Why do you need to duplicate this information? Because radio tuners may support multiple non overlapping bands, this is why this patch also adds a band member to the tuner struct, which can be used to set/get the current band. One example of this are the tea5757 / tea5759 radio tuner chips: FM: tea5757 87.5 - 108 MHz rangelow = 87.5 * 62500; rangehigh = 108 * 62500; tea5759 76 - 91 MHz rangelow = 76 * 62500; rangehigh = 91 * 62500; AM: Both: 530 - 1710 kHz rangelow = 0.530 * 62500; rangehigh = 0.1710 * 62500; See radio-cadet.c: static int vidioc_g_tuner(struct file *file, void *priv, struct v4l2_tuner *v) { struct cadet *dev = video_drvdata(file); v-type = V4L2_TUNER_RADIO; switch (v-index) { case 0: strlcpy(v-name, FM, sizeof(v-name)); v-capability = V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS | V4L2_TUNER_CAP_RDS_BLOCK_IO; v-rangelow = 1400; /* 87.5 MHz */ v-rangehigh = 1728;/* 108.0 MHz */ v-rxsubchans = cadet_getstereo(dev); switch (v-rxsubchans) { case V4L2_TUNER_SUB_MONO: v-audmode = V4L2_TUNER_MODE_MONO; break; case V4L2_TUNER_SUB_STEREO: v-audmode = V4L2_TUNER_MODE_STEREO; break; default: break; } v-rxsubchans |= V4L2_TUNER_SUB_RDS; break; case 1: strlcpy(v-name, AM, sizeof(v-name)); v-capability = V4L2_TUNER_CAP_LOW; v-rangelow = 8320; /* 520 kHz */ v-rangehigh = 26400;/* 1650 kHz */ v-rxsubchans = V4L2_TUNER_SUB_MONO; v-audmode = V4L2_TUNER_MODE_MONO; break; default: return -EINVAL; } v-signal = dev-sigstrength; /* We might need to modify scaling of this */ return 0; } static int vidioc_s_tuner(struct file *file, void *priv, struct v4l2_tuner *v) { struct cadet *dev = video_drvdata(file); if (v-index != 0 v-index != 1) return -EINVAL; dev-curtuner = v-index; return 0; } Band switching are made via g_tuner/s_tuner calls. If a device have several non-overlapping bands, just implement it there. There's no need for a new API. sigh, this has been discussed extensively between me, Hans V and Halli Manjunatha on both irc and on the list. What the cadet driver is doing is an ugly hack, and really a poor match for what we want. Not to mention that it is a clear violation of the v4l2 spec: http://linuxtv.org/downloads/v4l-dvb-apis/tuner.html Radio input devices have exactly one tuner with index zero, no video inputs. So there is supposed to be only one tuner, and s_tuner / g_tuner on radio devices always expect a tuner index of 0. Also from the same page: Note that VIDIOC_S_TUNER does not switch the current tuner, when there is more than one at all. So if we model
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
Hi, snip long discussion about having a fixed set of bands versus a way to enumerate bands, including their rangelow, rangehigh and capabilities Ok, you've convinced me. I agree that having a way to actually enumerate ranges, rather then having a fixed set of ranges, is better. Which brings us back many weeks to the proposal for making it possible to enumerate bands on radio devices. Rather then digging up the old mails lets start anew, I propose the following API for this: 1. A radio device can have multiple tuners, but only 1 can be active (streaming audio to the associated audio input) at the same time. 2. Radio device tuners are enumerated by calling G_TUNER with an increasing index until EINVAL gets returned 3. G_FREQUENCY will always return the frequency and index of the currently active tuner 4. When calling S_TUNER on a radio device, the active tuner will be set to the v4l2_tuner index field 5. When calling S_FREQUENCY on a radio device, the active tuner will be set to the v4l2_frequency tuner field 6. On a G_TUNER call on a radio device the rxsubchans, audmode, signal and afc v4l2_tuner fields are only filled on for the active tuner (as returned by G_FREQUENCY) for inactive tuners these fields are reported as 0. This is pretty close to what the cadet driver currently does, the differences are point 5 and 6, currently wrt point 5 the cadet driver just ignores the tuner field, and wrt point 6 it always tries to fill in these fields, reporting ie the FM signal strength when the FM tuner is active as signal for the AM tuner too. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
Hi, On 06/19/2012 06:47 PM, Hans de Goede wrote: Hi, snip long discussion about having a fixed set of bands versus a way to enumerate bands, including their rangelow, rangehigh and capabilities Ok, you've convinced me. I agree that having a way to actually enumerate ranges, rather then having a fixed set of ranges, is better. Which brings us back many weeks to the proposal for making it possible to enumerate bands on radio devices. Rather then digging up the old mails lets start anew, I propose the following API for this: 1. A radio device can have multiple tuners, but only 1 can be active (streaming audio to the associated audio input) at the same time. 2. Radio device tuners are enumerated by calling G_TUNER with an increasing index until EINVAL gets returned 3. G_FREQUENCY will always return the frequency and index of the currently active tuner 4. When calling S_TUNER on a radio device, the active tuner will be set to the v4l2_tuner index field 5. When calling S_FREQUENCY on a radio device, the active tuner will be set to the v4l2_frequency tuner field 6. On a G_TUNER call on a radio device the rxsubchans, audmode, signal and afc v4l2_tuner fields are only filled on for the active tuner (as returned by G_FREQUENCY) for inactive tuners these fields are reported as 0. p.s. I forgot: 7. When calling VIDIOC_S_HW_FREQ_SEEK on a radio device, the active tuner will be set to the v4l2_hw_freq_seek tuner field 8. When changing the active tuner with S_TUNER or S_HW_FREQ_SEEK, the current frequency may be changed to fit in the range of the new active tuner 9. For backwards compatibility reasons tuner 0 should be the tuner with the broadest possible FM range Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
Hi, On 06/19/2012 07:43 PM, halli manjunatha wrote: On Tue, Jun 19, 2012 at 12:33 PM, Hans de Goede hdego...@redhat.com wrote: Hi, On 06/19/2012 06:47 PM, Hans de Goede wrote: Hi, snip long discussion about having a fixed set of bands versus a way to enumerate bands, including their rangelow, rangehigh and capabilities Ok, you've convinced me. I agree that having a way to actually enumerate ranges, rather then having a fixed set of ranges, is better. Which brings us back many weeks to the proposal for making it possible to enumerate bands on radio devices. Rather then digging up the old mails lets start anew, I propose the following API for this: 1. A radio device can have multiple tuners, but only 1 can be active (streaming audio to the associated audio input) at the same time. 2. Radio device tuners are enumerated by calling G_TUNER with an increasing index until EINVAL gets returned 3. G_FREQUENCY will always return the frequency and index of the currently active tuner 4. When calling S_TUNER on a radio device, the active tuner will be set to the v4l2_tuner index field 5. When calling S_FREQUENCY on a radio device, the active tuner will be set to the v4l2_frequency tuner field 6. On a G_TUNER call on a radio device the rxsubchans, audmode, signal and afc v4l2_tuner fields are only filled on for the active tuner (as returned by G_FREQUENCY) for inactive tuners these fields are reported as 0. p.s. I forgot: 7. When calling VIDIOC_S_HW_FREQ_SEEK on a radio device, the active tuner will be set to the v4l2_hw_freq_seek tuner field 8. When changing the active tuner with S_TUNER or S_HW_FREQ_SEEK, the current frequency may be changed to fit in the range of the new active tuner 9. For backwards compatibility reasons tuner 0 should be the tuner with the broadest possible FM range So with this approach every time during S_FREQ/S_HW_SEEK/S_TUNER driver will check which tuner mode it is set to and change the tuner mode (or band) according to tuner field. So in my case I will have to support 5 tuner modes (EUROPE, JAPAN, RUSSIAN, WEATHER and DEFAULT) just like bands. Correct. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] radio-si470x: Don't unnecesarily read registers on G_TUNER
Reading registers from the pcear USB dongles with the si470x causes a loud pop (and an alsa buffer overrun). Since most radio apps periodically call G_TUNER to update mono/stereo, signal and afc status this leads to the music . pop . music . pop . music - not good. On the internet there is an howto for flashing the pcear with a newer firmware from the silabs reference boardto fix this, but: 1) This howto relies on a special version of the driver which allows firmware flashing 2) We should try to avoid the answer to a bug report being upgrade your firmware, if at all possible 3) Windows does not suffer from the pop sounds After a quick look at the driver I found at that the register reads are not necessary at all, as the device gives us the necessary status through usb interrupt packets, and the driver already uses these! Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/si470x/radio-si470x-common.c | 10 ++ drivers/media/radio/si470x/radio-si470x-usb.c| 12 +--- drivers/media/radio/si470x/radio-si470x.h|1 + 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/drivers/media/radio/si470x/radio-si470x-common.c b/drivers/media/radio/si470x/radio-si470x-common.c index d485b79..5dbb897 100644 --- a/drivers/media/radio/si470x/radio-si470x-common.c +++ b/drivers/media/radio/si470x/radio-si470x-common.c @@ -583,14 +583,16 @@ static int si470x_vidioc_g_tuner(struct file *file, void *priv, struct v4l2_tuner *tuner) { struct si470x_device *radio = video_drvdata(file); - int retval; + int retval = 0; if (tuner-index != 0) return -EINVAL; - retval = si470x_get_register(radio, STATUSRSSI); - if (retval 0) - return retval; + if (!radio-status_rssi_auto_update) { + retval = si470x_get_register(radio, STATUSRSSI); + if (retval 0) + return retval; + } /* driver constants */ strcpy(tuner-name, FM); diff --git a/drivers/media/radio/si470x/radio-si470x-usb.c b/drivers/media/radio/si470x/radio-si470x-usb.c index f412f7a..0da5c98 100644 --- a/drivers/media/radio/si470x/radio-si470x-usb.c +++ b/drivers/media/radio/si470x/radio-si470x-usb.c @@ -399,12 +399,16 @@ static void si470x_int_in_callback(struct urb *urb) } } - if ((radio-registers[SYSCONFIG1] SYSCONFIG1_RDS) == 0) + /* Sometimes the device returns len 0 packets */ + if (urb-actual_length != RDS_REPORT_SIZE) goto resubmit; - if (urb-actual_length 0) { + radio-registers[STATUSRSSI] = + get_unaligned_be16(radio-int_in_buffer[1]); + + if ((radio-registers[SYSCONFIG1] SYSCONFIG1_RDS)) { /* Update RDS registers with URB data */ - for (regnr = 0; regnr RDS_REGISTER_NUM; regnr++) + for (regnr = 1; regnr RDS_REGISTER_NUM; regnr++) radio-registers[STATUSRSSI + regnr] = get_unaligned_be16(radio-int_in_buffer[ regnr * RADIO_REGISTER_SIZE + 1]); @@ -480,6 +484,7 @@ resubmit: radio-int_in_running = 0; } } + radio-status_rssi_auto_update = radio-int_in_running; } @@ -560,6 +565,7 @@ static int si470x_start_usb(struct si470x_device *radio) submitting int urb failed (%d)\n, retval); radio-int_in_running = 0; } + radio-status_rssi_auto_update = radio-int_in_running; return retval; } diff --git a/drivers/media/radio/si470x/radio-si470x.h b/drivers/media/radio/si470x/radio-si470x.h index 4921cab..2a0a46f 100644 --- a/drivers/media/radio/si470x/radio-si470x.h +++ b/drivers/media/radio/si470x/radio-si470x.h @@ -161,6 +161,7 @@ struct si470x_device { struct completion completion; bool stci_enabled; /* Seek/Tune Complete Interrupt */ + bool status_rssi_auto_update; /* Does RSSI get updated automatic? */ #if defined(CONFIG_USB_SI470X) || defined(CONFIG_USB_SI470X_MODULE) /* reference to USB and video device */ -- 1.7.10.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
[PATCH 3/4] radio-si470x: Lower hardware freq seek signal treshold
The previous value made hardware freq seek not work for me, despite having good reception of almost all Dutch radio stations. Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/si470x/radio-si470x-common.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/radio/si470x/radio-si470x-common.c b/drivers/media/radio/si470x/radio-si470x-common.c index 9f8b675..84ab3d57 100644 --- a/drivers/media/radio/si470x/radio-si470x-common.c +++ b/drivers/media/radio/si470x/radio-si470x-common.c @@ -359,7 +359,7 @@ int si470x_start(struct si470x_device *radio) /* sysconfig 2 */ radio-registers[SYSCONFIG2] = - (0x3f 8) | /* SEEKTH */ + (0x1f 8) | /* SEEKTH */ ((band 6) SYSCONFIG2_BAND) | /* BAND */ ((space 4) SYSCONFIG2_SPACE) | /* SPACE */ 15; /* VOLUME (max) */ -- 1.7.10.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
[PATCH 4/4] radio-si470x: Lower firmware version requirements
With the changes from the previous patches device firmware version 14 + usb microcontroller software version 1 works fine too. Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/si470x/radio-si470x-usb.c |2 +- drivers/media/radio/si470x/radio-si470x.h |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/radio/si470x/radio-si470x-usb.c b/drivers/media/radio/si470x/radio-si470x-usb.c index 66b1ba8..40b963c 100644 --- a/drivers/media/radio/si470x/radio-si470x-usb.c +++ b/drivers/media/radio/si470x/radio-si470x-usb.c @@ -143,7 +143,7 @@ MODULE_PARM_DESC(max_rds_errors, RDS maximum block errors: *1*); * Software/Hardware Versions from Scratch Page **/ #define RADIO_SW_VERSION_NOT_BOOTLOADABLE 6 -#define RADIO_SW_VERSION 7 +#define RADIO_SW_VERSION 1 #define RADIO_HW_VERSION 1 diff --git a/drivers/media/radio/si470x/radio-si470x.h b/drivers/media/radio/si470x/radio-si470x.h index fbf713d..b3b612f 100644 --- a/drivers/media/radio/si470x/radio-si470x.h +++ b/drivers/media/radio/si470x/radio-si470x.h @@ -189,7 +189,7 @@ struct si470x_device { * Firmware Versions **/ -#define RADIO_FW_VERSION 15 +#define RADIO_FW_VERSION 14 -- 1.7.10.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
[PATCH 2/4] radio-si470x: Always use interrupt to wait for tune/seek completion
Since USB receives STATUS_RSSI updates through the interrupt endpoint, there is no need to poll with USB, so get rid of the polling. Note this also changes the order in which the probing of USB devices is done, to avoid si470x_set_chan getting called before the interrupt endpoint is being monitored. Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/media/radio/si470x/radio-si470x-common.c | 56 +- drivers/media/radio/si470x/radio-si470x-i2c.c|5 +- drivers/media/radio/si470x/radio-si470x-usb.c| 25 ++ drivers/media/radio/si470x/radio-si470x.h|1 - 4 files changed, 28 insertions(+), 59 deletions(-) diff --git a/drivers/media/radio/si470x/radio-si470x-common.c b/drivers/media/radio/si470x/radio-si470x-common.c index 5dbb897..9f8b675 100644 --- a/drivers/media/radio/si470x/radio-si470x-common.c +++ b/drivers/media/radio/si470x/radio-si470x-common.c @@ -164,7 +164,6 @@ MODULE_PARM_DESC(seek_timeout, Seek timeout: *5000*); static int si470x_set_chan(struct si470x_device *radio, unsigned short chan) { int retval; - unsigned long timeout; bool timed_out = 0; /* start tuning */ @@ -174,26 +173,12 @@ static int si470x_set_chan(struct si470x_device *radio, unsigned short chan) if (retval 0) goto done; - /* currently I2C driver only uses interrupt way to tune */ - if (radio-stci_enabled) { - INIT_COMPLETION(radio-completion); - - /* wait till tune operation has completed */ - retval = wait_for_completion_timeout(radio-completion, - msecs_to_jiffies(tune_timeout)); - if (!retval) - timed_out = true; - } else { - /* wait till tune operation has completed */ - timeout = jiffies + msecs_to_jiffies(tune_timeout); - do { - retval = si470x_get_register(radio, STATUSRSSI); - if (retval 0) - goto stop; - timed_out = time_after(jiffies, timeout); - } while (((radio-registers[STATUSRSSI] STATUSRSSI_STC) == 0) -(!timed_out)); - } + /* wait till tune operation has completed */ + INIT_COMPLETION(radio-completion); + retval = wait_for_completion_timeout(radio-completion, + msecs_to_jiffies(tune_timeout)); + if (!retval) + timed_out = true; if ((radio-registers[STATUSRSSI] STATUSRSSI_STC) == 0) dev_warn(radio-videodev.dev, tune does not complete\n); @@ -201,7 +186,6 @@ static int si470x_set_chan(struct si470x_device *radio, unsigned short chan) dev_warn(radio-videodev.dev, tune timed out after %u ms\n, tune_timeout); -stop: /* stop tuning */ radio-registers[CHANNEL] = ~CHANNEL_TUNE; retval = si470x_set_register(radio, CHANNEL); @@ -312,7 +296,6 @@ static int si470x_set_seek(struct si470x_device *radio, unsigned int wrap_around, unsigned int seek_upward) { int retval = 0; - unsigned long timeout; bool timed_out = 0; /* start seeking */ @@ -329,26 +312,12 @@ static int si470x_set_seek(struct si470x_device *radio, if (retval 0) return retval; - /* currently I2C driver only uses interrupt way to seek */ - if (radio-stci_enabled) { - INIT_COMPLETION(radio-completion); - - /* wait till seek operation has completed */ - retval = wait_for_completion_timeout(radio-completion, - msecs_to_jiffies(seek_timeout)); - if (!retval) - timed_out = true; - } else { - /* wait till seek operation has completed */ - timeout = jiffies + msecs_to_jiffies(seek_timeout); - do { - retval = si470x_get_register(radio, STATUSRSSI); - if (retval 0) - goto stop; - timed_out = time_after(jiffies, timeout); - } while (((radio-registers[STATUSRSSI] STATUSRSSI_STC) == 0) -(!timed_out)); - } + /* wait till tune operation has completed */ + INIT_COMPLETION(radio-completion); + retval = wait_for_completion_timeout(radio-completion, + msecs_to_jiffies(seek_timeout)); + if (!retval) + timed_out = true; if ((radio-registers[STATUSRSSI] STATUSRSSI_STC) == 0) dev_warn(radio-videodev.dev, seek does not complete\n); @@ -356,7 +325,6 @@ static int si470x_set_seek(struct si470x_device *radio, dev_warn(radio-videodev.dev, seek failed / band limit reached\n); -stop
Re: [git:v4l-utils/master] libv4l: Move dev ops to libv4lconvert
Hi, On 06/11/2012 09:59 PM, Gregor Jasny wrote: This is an automatic generated email to let you know that the following patch were queued at the http://git.linuxtv.org/v4l-utils.git tree: Subject: libv4l: Move dev ops to libv4lconvert Author: Gregor Jasny gja...@googlemail.com Date:Mon Jun 11 21:59:25 2012 +0200 As discussed with Hans de Goede, this patch moves the plugin dev-ops structure to libv4lconvert. It was also renamed to libv4l_dev_ops. As a positive side effect we restored SONAME compatibility with the 0.8.x releases. Nice, good work! So I guess it is about time for a 0.10 release? Before doing a 0.10 release I would like to revisit the plugin API and mmap issue though. I've made a 180 wrt my opinion on this, and I think it would be good to have mmap in the plugin API to allow plugins to intercept it if they want (so that we can do ie a an IEEE1394 converter plugin like http://dv4l.berlios.de/) The mmap callbacks would be optional, so a not interested plugin does not need to worry about them. Here is what I wrote on this before: The problem with mmap is that we've 3 kinds of mmap buffers: 1) Real mmap-ed device buffers, used directly by the app in the no conversion path. 2) Faked mmap-ed device buffers, seen by the app when doing conversion, this is basically convert_mmap_buf, IMHO if we add mmap plugin ops, the mmap / munmap of convert_mmap_buf should not go through it, is is basically just a malloc/free, but done through mmap to make sure we get the right alignment, etc. 3) memory not managed by v4l at all, this happens only in the munmap call, when used in combination with LD_PRELOAD note that currently in v4l2_munmap, the code paths for 1 3 are the same. If we allow plugins to intercept the munmap call (and make no further changes) then the plugin will get the munmap call and if the memory is not owned by the plugin it should do a SYS_MUNMAP instead!! So if we keep using the real mmap / munmap for the fake buffers (2), then the only problem is that when used with LD_PRELOAD (ie skype), the plugin can get munmap calls for memory it never returned from mmap. I suggest we document this, as well as that in this case the plugin should forward the call to SYS_MUNMAP. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 PATCH 00/32] Core and vb2 enhancements
Hi, On 06/12/2012 01:35 PM, Mauro Carvalho Chehab wrote: Em 10-06-2012 16:27, Hans Verkuil escreveu: On Sun June 10 2012 19:32:36 Hans Verkuil wrote: On Sun June 10 2012 18:46:52 Mauro Carvalho Chehab wrote: 3) it would be interesting if you could benchmark the previous code and the new one, to see what gains this change introduced, in terms of v4l2-core footprint and performance. I'll try that, should be interesting. Actually, my prediction is that I won't notice any difference. Todays CPUs are so fast that the overhead of the switch is probably hard to measure. I did some tests, calling various ioctls 100,000,000 times. The actual call into the driver was disabled so that I only measure the time spent in v4l2-ioctl.c. I ran the test program with 'time ./t' and measured the sys time. For each ioctl I tested 5 times and averaged the results. Times are in seconds. Old New QUERYCAP24.86 24.37 UNSUBSCRIBE_EVENT 23.40 23.10 LOG_STATUS 18.84 18.76 ENUMINPUT 28.82 28.90 Particularly for QUERYCAP and UNSUBSCRIBE_EVENT I found a small but reproducible improvement in speed. The results for LOG_STATUS and ENUMINPUT are too close to call. After looking at the assembly code that the old code produces I suspect (but it is hard to be sure) that LOG_STATUS and ENUMINPUT are tested quite early on, whereas QUERYCAP and UNSUBSCRIBE_EVENT are tested quite late. The order in which the compiler tests definitely has no relationship with the order of the case statements in the switch. The ioctl's are reordered, as gcc optimizes them in order to do a tree search and to avoid cache flush. The worse case is likely converted into 7 CMP asm calls (log2(128)). On your code, gcc may not be able to predict the JMP's, so it may actually have cache flushes, depending on the cache size, and if the caller functions are before of after the video_ioctl2 handler. I suspect that, if you compare the code with debug enabled, the new code can actually be worse than the previous one. It would be good if you could test what happens with QBUF/DQBUF. This would certainly explain what I am seeing. I'm actually a bit surprised that this is measurable at all. The timing difference is not significant, especially because those ioctl's aren't the ones used inside the streaming loop. The only ioctl's that are more time-sensitive are the streaming ones, especially QBUF/DQBUF. Even QBUF / DQBUF are called max circa 100 times / second. I think Hans V's patchset should not be seen from a performance pov (other then that it should not cause performance regressions), but more as a nice code cleanup / simplification. It certainly makes things a lot more readable by avoiding a lot of code duplication. Not sure if in the end it actually saves any lines of code, but readability, and being able to understand the intent of the code is key here IMHO. Regards, Hans (who likes Hans V's patchset :) -- 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: PWC ioctl inappropriate for device (Regression)
Hi, On 06/11/2012 04:34 PM, Bernard GODARD wrote: Hi Hans, Thank you for your reply. I will try to do the fix in qastrocam-g2 myself as this program does not currently have a maintainer. Thanks! Please let me know if you need any help. If programs like qastrocam-g2 are going to depend on some of the custom v4l2 ctrls pwc has (so controls not using a standard ctrl id), then one of the first steps would be to make the ctrl ids for all the custom controls available in a public header file. Which means writing a (simple) kernel patch, currently drivers/media/video/pwc/pwc-v4l.c in the kernel has: #define PWC_CID_CUSTOM(ctrl) ((V4L2_CID_USER_BASE | 0xf000) + custom_ ## ctrl) and: enum { custom_autocontour, custom_contour, custom_noise_reduction, custom_awb_speed, custom_awb_delay, custom_save_user, custom_restore_user, custom_restore_factory }; And then in various places uses things like: PWC_CID_CUSTOM(autocontour) I think it would be best to make a new include/media/pwc.h file, which then would contain things like: PWC_CID_AUTOCONTOUR (V4L2_CID_USER_BASE | 0xf000) PWC_CID_CONTOUR (V4L2_CID_USER_BASE | 0xf001) And then in drivers/media/video/pwc/pwc-v4l.c replace PWC_CID_CUSTOM(autocontour) with PWC_CID_AUTOCONTOUR, etc. It would be good to keep the order the same, as in the enum, this way your modified qastrocam-g2 will work with the current kernel too, as the ids are then unchanged. Another something to look at is the V4L2_CID_AUTO_WHITE_BALANCE control, which uses a menu, rather then being the standard boolean. The very latest kernel code has a new standardized ctrl for auto-whitebalance controls which are a menu, see: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=e40a05736d4503950ec303610a51f838bd59cdc1 It would be nice if you could do a patch to move pwc over to this too. Note that pwc will move over to this sooner or later (probably sooner, so if you don't feel up to doing a patch for this yourself let me know and I'll do one), as making qastrocam-g2 work only with the current V4L2_CID_AUTO_WHITE_BALANCE and not with the future V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE control means it will break again in the future :/ This does mean that if you want the modified qastrocam-g2 to work both with current kernels and with newer kernels where pwc has moved over to V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE, you need to support both. You can simply do a VIDIOC_QUERYCTRL on both to see which one is present to support both. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PWC ioctl inappropriate for device (Regression)
Hi, On 06/09/2012 07:06 PM, Bernard GODARD wrote: Dear all, I am using a Philips Toucam Pro 2 webcam with the program qastrocam-g2 (astronomy program that use some specific functions of the PWC driver). I have been using this program with this camera for a long time on different Linux distributions without a problem. With Ubuntu 12.04, I now get a kernel oops. See bug report: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1010028 I have installed mainline kernel 3.4rc6 on my Ubuntu box to check if the oops was fixed upstream. Now I am not getting the oops anymore Good! but the IOCTL used to get/set the camera parameters are failing: astro@saturn:~$ qastrocam-g2 init : Avifile RELEASE-0.7.48-120122-05:53-../src/configure init : Available CPU flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt rdtscp lm 3dnowext 3dnow rep_good nopl extd_apicid pni cx16 la init : 2200.00 MHz AMD Athlon(tm) 64 X2 Dual Core Processor 4200+ detected Getting Standard: Inappropriate ioctl for device setWhiteBalance: Inappropriate ioctl for device getWhiteBalance: Inappropriate ioctl for device getWhiteBalance: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device ioctl (VIDIOCGWIN): Inappropriate ioctl for device mmap: Invalid argument VIDIOCPWCGDYNNOISE: Inappropriate ioctl for device VIDIOCPWCGCONTOUR: Inappropriate ioctl for device VIDIOCPWCSCONTOUR: Inappropriate ioctl for device VIDIOCPWCGDYNNOISE: Inappropriate ioctl for device VIDIOCPWCGCONTOUR: Inappropriate ioctl for device VIDIOCPWCGDYNNOISE: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCSAGC: Inappropriate ioctl for device getWhiteBalance: Inappropriate ioctl for device VIDIOCPWCSAGC: Inappropriate ioctl for device VIDIOCPWCSSHUTTER: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device getWhiteBalance: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device VIDIOCPWCGAGC: Inappropriate ioctl for device As the names of the ioctls imply these are (were) custom pwc ioctls, these were added in the v4l1 days as the v4l1 api did not have a way to expose the desired functionality in a standard manner. Support for the v4l1 API has been removed a number of kernel releases ago and at the same time the pwc specific ioctls have been marked as deprecated. And with kernel 3.2 they have finally been removed. The same results can be achieved with the standard v4l2 VIDIOC_S_CTRL and VIDIOC_G_CTRL ioctls. I'm sorry to hear that the removal of the custom pwc ioctls is causing problems for you, but we really don't want to have any unneeded driver specific ioctls with v4l2 devices. So the qastrocam-g2 program needs to be modified to use the standard controls interface to modify these settings on newer kernels. Can you please send a bug report to qastrocam-g2 about this and add me in the CC ? Thanks, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Doing a new upstream / linuxtv.org xawtv3 release?
Hi, On 06/04/2012 08:01 PM, Gregor Jasny wrote: Hello, On 6/4/12 12:57 PM, Hans de Goede wrote: I've been doing a lot of work on xawtv3 lately, mostly on the radio app but also some on xawtv itself. I'm no done and IMHO it would be good to do a new upstream release to get all those changes out there. So any comments / suggestions? Note go for it also is a valid comment :) The Debian patch tracker contains four patches for xawtv: http://patch-tracker.debian.org/package/xawtv/3.102-3 Three of them are already in the git tree, this one is not: Thanks for checking this! http://patch-tracker.debian.org/patch/series/view/xawtv/3.102-3/mtt_only_in_linux Could you please add it before the release? Done! Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Doing a new upstream / linuxtv.org xawtv3 release?
Hi All, I've been doing a lot of work on xawtv3 lately, mostly on the radio app but also some on xawtv itself. I'm no done and IMHO it would be good to do a new upstream release to get all those changes out there. So any comments / suggestions? Note go for it also is a valid comment :) Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git:v4l-utils/master] Add HW_SEEK and TUNER_BAND capabilities to videodev2.h
Hi, On 06/02/2012 02:15 PM, Hans Verkuil wrote: On Sat June 2 2012 11:11:53 Hans de Goede wrote: This is an automatic generated email to let you know that the following patch were queued at the http://git.linuxtv.org/v4l-utils.git tree: Subject: Add HW_SEEK and TUNER_BAND capabilities to videodev2.h Author: Hans de Goedehdego...@redhat.com Date:Sat Jun 2 11:11:53 2012 +0200 Bring in the pending (reviewed and acked) changes from: But not merged. I think this is a bit too quick. It is good practice to wait with making such changes to v4l-utils until Mauro has merged the videodev2.h changes as well. Ok, next time around I'll wait. I also have a small request: +static const char *band_names[] = { + default, + fm-eur_us, + fm-japan, + fm-russian, + fm-weather, + am-mw, +}; Can you rename fm-eur_us to fm-eur-us? That mix of '-' and '_' is very jarring and awkward to type IMHO. Done. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Support for old Intel webcam
Hi, On 05/29/2012 08:46 PM, Jason Miller wrote: I have an old Intel webcam that shows up as: Bus 002 Device 005: ID 8086:0431 Intel Corp. Intel Pro Video PC Camera My hazy memory says that this used to work on linux with the spca_50x driver, though I think I had to add the vendor/device ID manually to the driver. Is there any way to determine which chip is used in the camera so I can try doing so again? Looking at the inf file from the windows drivers it is indeed quite likely an spca50x based chip, what you can try doing (as root) is: modprobe gspca_spca501 cd /sys/bus/usb/drivers/spca501 echo '0x8086 0x0431' new_id And see if it works, if not you can also try the spca500 and spca508 drivers. Please unplug, rmmod the last tried driver, and then replug the device between different attempts. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv2 PATCH 0/5] Add hwseek caps and frequency bands
Hi, On 05/28/2012 01:58 PM, Hans Verkuil wrote: On Mon May 28 2012 13:20:47 Hans de Goede wrote: Hi, Looks good, the entire series is: Acked-by: Hans de Goedehdego...@redhat.com I was thinking that it would be a good idea to add a: #define V4L2_TUNER_CAP_BANDS_MASK 0x001f to videodev2.h, which apps can then easily use to test if the driver supports any bands other then the default, and decide to show band selection elements of the UI or not based on a test on the tuner-caps using that mask. This can be done in a separate patch, or merged into PATCH 4/6 videodev2.h: add frequency band information Good idea, I've merged it into patch 4 and 5 (documenting it). It's here: http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/bands New version still look good, so the entire series still is: Acked-by: Hans de Goedehdego...@redhat.com :) Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL FIXES FOR 3.5]: gspca radio fixes (updated 3x)
Hi Mauro et al, updated again to include 2 gspca fixes from Jean-François Moine Here is a bunch of fixes for gspca and a couple of fixes for good old radio support :) The following changes since commit abed623ca59a7d1abed6c4e7459be03e25a90a1e: [media] radio-sf16fmi: add support for SF16-FMD (2012-05-20 16:10:05 -0300) are available in the git repository at: git://linuxtv.org/hgoede/gspca.git media-for_v3.5 for you to fetch changes up to 2856acfae0a84b3cb4af049d17c09593d6b1b2d3: gspca - sonixj: Fix bad values of webcam 0458:7025 (2012-05-29 11:11:10 +0200) Antonio Ospite (1): gspca_ov534: make AGC and AWB controls independent Hans de Goede (10): radio/si470x: Add support for the Axentia ALERT FM USB Receiver snd_tea575x: Report correct frequency range for EU/US versus JA models snd_tea575x: Make the module using snd_tea575x the fops owner snd_tea575x: set_freq: update cached freq to the actual achieved frequency bttv: Use btv-has_radio rather then the card info when registering the tuner bttv: Remove unused needs_tvaudio card variable bttv: The Hauppauge 61334 needs the msp3410 to do radio demodulation gspca_pac7311: Correct number of controls gscpa_sn9c20x: Move clustering of controls to after error checking gspca-core: Fix buffers staying in queued state after a stream_off Jean-Francois Moine (2): gspca - ov534/ov534_9: Fix sccd_read/write errors gspca - sonixj: Fix bad values of webcam 0458:7025 drivers/hid/hid-core.c|1 + drivers/hid/hid-ids.h |3 + drivers/media/radio/radio-maxiradio.c |2 +- drivers/media/radio/radio-sf16fmr2.c |2 +- drivers/media/radio/si470x/radio-si470x-usb.c |2 + drivers/media/video/bt8xx/bttv-cards.c| 84 ++--- drivers/media/video/bt8xx/bttv-driver.c |5 ++ drivers/media/video/bt8xx/bttv.h |1 - drivers/media/video/bt8xx/bttvp.h |1 + drivers/media/video/gspca/gspca.c |4 +- drivers/media/video/gspca/ov534.c | 32 +- drivers/media/video/gspca/ov534_9.c |1 + drivers/media/video/gspca/pac7311.c |2 +- drivers/media/video/gspca/sn9c20x.c | 24 --- drivers/media/video/gspca/sonixj.c|2 +- include/sound/tea575x-tuner.h |3 +- sound/i2c/other/tea575x-tuner.c | 21 --- sound/pci/es1968.c|2 +- sound/pci/fm801.c |4 +- 19 files changed, 62 insertions(+), 134 deletions(-) Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gspca - ov534/ov534_9: Fix sccd_read/write errors
Hi, Thanks I've added this and the sonixj fixes to my tree and send an updated pullreq to Mauro to include these in the next round of fixes for 3.5 Regards, Hans On 05/28/2012 07:04 PM, Jean-Francois Moine wrote: The ov534 bridge is too slow to handle the sensor accesses requested by fast hosts giving 'sccb_reg_write failed'. A small delay fixes the problem. Signed-off-by: Jean-François Moinemoin...@free.fr --- drivers/media/video/gspca/ov534.c |1 + drivers/media/video/gspca/ov534_9.c |1 + 2 files changed, 2 insertions(+) diff --git a/drivers/media/video/gspca/ov534.c b/drivers/media/video/gspca/ov534.c index b5acb1e..d5a7873 100644 --- a/drivers/media/video/gspca/ov534.c +++ b/drivers/media/video/gspca/ov534.c @@ -851,6 +851,7 @@ static int sccb_check_status(struct gspca_dev *gspca_dev) int i; for (i = 0; i 5; i++) { + msleep(10); data = ov534_reg_read(gspca_dev, OV534_REG_STATUS); switch (data) { diff --git a/drivers/media/video/gspca/ov534_9.c b/drivers/media/video/gspca/ov534_9.c index e6601b8..0120f94 100644 --- a/drivers/media/video/gspca/ov534_9.c +++ b/drivers/media/video/gspca/ov534_9.c @@ -1008,6 +1008,7 @@ static int sccb_check_status(struct gspca_dev *gspca_dev) int i; for (i = 0; i 5; i++) { + msleep(10); data = reg_r(gspca_dev, OV534_REG_STATUS); switch (data) { -- 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: [RFCv2 PATCH 0/5] Add hwseek caps and frequency bands
Hi, Looks good, the entire series is: Acked-by: Hans de Goede hdego...@redhat.com I was thinking that it would be a good idea to add a: #define V4L2_TUNER_CAP_BANDS_MASK 0x001f to videodev2.h, which apps can then easily use to test if the driver supports any bands other then the default, and decide to show band selection elements of the UI or not based on a test on the tuner-caps using that mask. This can be done in a separate patch, or merged into PATCH 4/6 videodev2.h: add frequency band information Regards, Hans On 05/28/2012 12:46 PM, Hans Verkuil wrote: Changes since v1: - Fixed typo in second patch - Patch 5 now only contains the part about frequency bands - Patch 6 contains only (I hope) a non-controversial clarification regarding modulators (and a small change making a line more understandable). Regards, Hans This patch series adds improved hwseek support as discussed here: http://www.mail-archive.com/linux-media@vger.kernel.org/msg45957.html and on irc: http://linuxtv.org/irc/v4l/index.php?date=2012-05-26 From the RFC I have implemented/documented items 1-4 and 6a. I decided not to go with option 6b. This may be added in the future if there is a clear need. The addition of the frequency band came out of this discussion: http://www.spinics.net/lists/linux-media/msg48272.html Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Discussion: How to deal with radio tuners which can tune to multiple bands
Hi, Just noticed the following on: http://linuxtv.org/downloads/v4l-dvb-apis/tuner.html#id2570531 This specification does not define radio output devices., iow no radio modulators, but we agreed upon making the band changes to the modulator too, and this makes sense because AFAIK we do support radio modulators. I hit this while working on adding support for the proposed API to v4l2-ctl, as I wanted to only print the band stuff for radio type devices, but the modulator struct has no type! Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 PATCH 1/5] videodev2.h: add new hwseek capability bits.
Looks good: Acked-by: Hans de Goede hdego...@redhat.com On 05/27/2012 01:50 PM, Hans Verkuil wrote: From: Hans Verkuilhans.verk...@cisco.com Tell the application whether the hardware seek is bounded and/or wraps around. Signed-off-by: Hans Verkuilhans.verk...@cisco.com --- include/linux/videodev2.h |2 ++ 1 file changed, 2 insertions(+) diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 370d111..2339678 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -2039,6 +2039,8 @@ struct v4l2_modulator { /* Flags for the 'capability' field */ #define V4L2_TUNER_CAP_LOW0x0001 #define V4L2_TUNER_CAP_NORM 0x0002 +#define V4L2_TUNER_CAP_HWSEEK_BOUNDED 0x0004 +#define V4L2_TUNER_CAP_HWSEEK_WRAP 0x0008 #define V4L2_TUNER_CAP_STEREO 0x0010 #define V4L2_TUNER_CAP_LANG2 0x0020 #define V4L2_TUNER_CAP_SAP0x0020 -- 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: [RFCv1 PATCH 2/5] v4l2 spec: document the new v4l2_tuner capabilities
Small typo, see comment inline, with that fixed: Acked-by: Hans de Goede hdego...@redhat.com On 05/27/2012 01:50 PM, Hans Verkuil wrote: From: Hans Verkuilhans.verk...@cisco.com Update the spec with the new capabilities and specify new error codes for S_HW_FREQ_SEEK. Signed-off-by: Hans Verkuilhans.verk...@cisco.com --- .../DocBook/media/v4l/vidioc-g-frequency.xml |6 ++ Documentation/DocBook/media/v4l/vidioc-g-tuner.xml | 12 .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml | 18 +++--- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/Documentation/DocBook/media/v4l/vidioc-g-frequency.xml b/Documentation/DocBook/media/v4l/vidioc-g-frequency.xml index 69c178a..40e58a4 100644 --- a/Documentation/DocBook/media/v4l/vidioc-g-frequency.xml +++ b/Documentation/DocBook/media/v4l/vidioc-g-frequency.xml @@ -135,6 +135,12 @@ bounds or the value in thestructfieldtype/structfield field is wrong./para /listitem /varlistentry +varlistentry + termerrorcodeEBUSY/errorcode/term + listitem + paraA hardware seek is in progress./para + /listitem +/varlistentry /variablelist /refsect1 /refentry diff --git a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml index 62a1aa2..95d5371 100644 --- a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml +++ b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml @@ -276,6 +276,18 @@ can or must be switched. (B/G PAL tuners for example are typically not constantV4L2_TUNER_ANALOG_TV/constant tuners can have this capability./entry /row row + entryconstantV4L2_TUNER_CAP_HWSEEK_BOUNDED/constant/entry + entry0x0004/entry + entryIf set, then this tuner supports the hardware seek functionality + where the seek stops when it reaches the end of the frequency range./entry + /row + row + entryconstantV4L2_TUNER_CAP_HWSEEK_WRAP/constant/entry + entry0x0008/entry + entryIf set, then this tuner supports the hardware seek functionality + where the seek wraps around when it reaches the end of the frequency range./entry + /row + row entryconstantV4L2_TUNER_CAP_STEREO/constant/entry entry0x0010/entry entryStereo audio reception is supported./entry diff --git a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml index 407dfce..d58b648 100644 --- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml +++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml @@ -58,6 +58,9 @@ To do this applications initialize thestructfieldtuner/structfield, call theconstantVIDIOC_S_HW_FREQ_SEEK/constant ioctl with a pointer to this structure./para +paraIf an error is returned, then the frequency original frequency will +be restored./para + One frequency too many in that sentence :) paraThis ioctl is supported if theconstantV4L2_CAP_HW_FREQ_SEEK/constant capability is set./para table pgwide=1 frame=none id=v4l2-hw-freq-seek @@ -87,7 +90,10 @@ field and thev4l2-tuner;structfieldindex/structfield field./entry row entry__u32/entry entrystructfieldwrap_around/structfield/entry - entryIf non-zero, wrap around when at the end of the frequency range, else stop seeking./entry + entryIf non-zero, wrap around when at the end of the frequency range, else stop seeking. + Thev4l2-tuner;structfieldcapability/structfield field will tell you what the + hardware supports. + /entry /row row entry__u32/entry @@ -118,9 +124,15 @@ wrong./para /listitem /varlistentry varlistentry - termerrorcodeEAGAIN/errorcode/term + termerrorcodeENODATA/errorcode/term + listitem + paraThe hardware seek found no channels./para + /listitem +/varlistentry +varlistentry + termerrorcodeEBUSY/errorcode/term listitem - paraThe ioctl timed-out. Try again./para + paraAnother hardware seek is already in progress./para /listitem /varlistentry /variablelist -- 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: [RFCv1 PATCH 3/5] S_HW_FREQ_SEEK: set capability flags and return ENODATA instead of EAGAIN.
Looks good: Acked-by: Hans de Goede hdego...@redhat.com On 05/27/2012 01:50 PM, Hans Verkuil wrote: From: Hans Verkuilhans.verk...@cisco.com Set the new capability flags in G_TUNER and return ENODATA if no channels were found. Signed-off-by: Hans Verkuilhans.verk...@cisco.com --- drivers/media/radio/radio-mr800.c|5 +++-- drivers/media/radio/radio-wl1273.c |3 ++- drivers/media/radio/si470x/radio-si470x-common.c |6 -- drivers/media/radio/wl128x/fmdrv_rx.c|2 +- drivers/media/radio/wl128x/fmdrv_v4l2.c |4 +++- sound/i2c/other/tea575x-tuner.c |4 +++- 6 files changed, 16 insertions(+), 8 deletions(-) diff --git a/drivers/media/radio/radio-mr800.c b/drivers/media/radio/radio-mr800.c index 94cb6bc..3182b26 100644 --- a/drivers/media/radio/radio-mr800.c +++ b/drivers/media/radio/radio-mr800.c @@ -295,7 +295,8 @@ static int vidioc_g_tuner(struct file *file, void *priv, v-type = V4L2_TUNER_RADIO; v-rangelow = FREQ_MIN * FREQ_MUL; v-rangehigh = FREQ_MAX * FREQ_MUL; - v-capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO; + v-capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO | + V4L2_TUNER_CAP_HWSEEK_WRAP; v-rxsubchans = is_stereo ? V4L2_TUNER_SUB_STEREO : V4L2_TUNER_SUB_MONO; v-audmode = radio-stereo ? V4L2_TUNER_MODE_STEREO : V4L2_TUNER_MODE_MONO; @@ -372,7 +373,7 @@ static int vidioc_s_hw_freq_seek(struct file *file, void *priv, timeout = jiffies + msecs_to_jiffies(3); for (;;) { if (time_after(jiffies, timeout)) { - retval = -EAGAIN; + retval = -ENODATA; break; } if (schedule_timeout_interruptible(msecs_to_jiffies(10))) { diff --git a/drivers/media/radio/radio-wl1273.c b/drivers/media/radio/radio-wl1273.c index f1b6070..e8428f5 100644 --- a/drivers/media/radio/radio-wl1273.c +++ b/drivers/media/radio/radio-wl1273.c @@ -1514,7 +1514,8 @@ static int wl1273_fm_vidioc_g_tuner(struct file *file, void *priv, tuner-rangehigh = WL1273_FREQ(WL1273_BAND_OTHER_HIGH); tuner-capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_RDS | - V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS_BLOCK_IO; + V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS_BLOCK_IO | + V4L2_TUNER_CAP_HWSEEK_BOUNDED | V4L2_TUNER_CAP_HWSEEK_WRAP; if (radio-stereo) tuner-audmode = V4L2_TUNER_MODE_STEREO; diff --git a/drivers/media/radio/si470x/radio-si470x-common.c b/drivers/media/radio/si470x/radio-si470x-common.c index 969cf49..d485b79 100644 --- a/drivers/media/radio/si470x/radio-si470x-common.c +++ b/drivers/media/radio/si470x/radio-si470x-common.c @@ -363,7 +363,7 @@ stop: /* try again, if timed out */ if (retval == 0 timed_out) - return -EAGAIN; + return -ENODATA; return retval; } @@ -596,7 +596,9 @@ static int si470x_vidioc_g_tuner(struct file *file, void *priv, strcpy(tuner-name, FM); tuner-type = V4L2_TUNER_RADIO; tuner-capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO | - V4L2_TUNER_CAP_RDS | V4L2_TUNER_CAP_RDS_BLOCK_IO; + V4L2_TUNER_CAP_RDS | V4L2_TUNER_CAP_RDS_BLOCK_IO | + V4L2_TUNER_CAP_HWSEEK_BOUNDED | + V4L2_TUNER_CAP_HWSEEK_WRAP; /* range limits */ switch ((radio-registers[SYSCONFIG2] SYSCONFIG2_BAND) 6) { diff --git a/drivers/media/radio/wl128x/fmdrv_rx.c b/drivers/media/radio/wl128x/fmdrv_rx.c index 43fb722..3dd9fc0 100644 --- a/drivers/media/radio/wl128x/fmdrv_rx.c +++ b/drivers/media/radio/wl128x/fmdrv_rx.c @@ -251,7 +251,7 @@ again: if (!timeleft) { fmerr(Timeout(%d sec),didn't get tune ended int\n, jiffies_to_msecs(FM_DRV_RX_SEEK_TIMEOUT) / 1000); - return -ETIMEDOUT; + return -ENODATA; } int_reason = fmdev-irq_info.flag (FM_TUNE_COMPLETE | FM_BAND_LIMIT); diff --git a/drivers/media/radio/wl128x/fmdrv_v4l2.c b/drivers/media/radio/wl128x/fmdrv_v4l2.c index 080b96a..49a11ec 100644 --- a/drivers/media/radio/wl128x/fmdrv_v4l2.c +++ b/drivers/media/radio/wl128x/fmdrv_v4l2.c @@ -285,7 +285,9 @@ static int fm_v4l2_vidioc_g_tuner(struct file *file, void *priv, tuner-rxsubchans = V4L2_TUNER_SUB_MONO | V4L2_TUNER_SUB_STEREO | ((fmdev-rx.rds.flag == FM_RDS_ENABLE) ? V4L2_TUNER_SUB_RDS : 0); tuner-capability = V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS | - V4L2_TUNER_CAP_LOW; + V4L2_TUNER_CAP_LOW | + V4L2_TUNER_CAP_HWSEEK_BOUNDED | + V4L2_TUNER_CAP_HWSEEK_WRAP; tuner-audmode = (stereo_mono_mode
Re: [RFCv1 PATCH 4/5] videodev2.h: add frequency band information.
Looks good: Acked-by: Hans de Goede hdego...@redhat.com On 05/27/2012 01:50 PM, Hans Verkuil wrote: From: Hans Verkuilhans.verk...@cisco.com Signed-off-by: Hans Verkuilhans.verk...@cisco.com --- include/linux/videodev2.h | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 2339678..013ee46 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -2023,7 +2023,8 @@ struct v4l2_tuner { __u32 audmode; __s32 signal; __s32 afc; - __u32 reserved[4]; + __u32 band; + __u32 reserved[3]; }; struct v4l2_modulator { @@ -2033,7 +2034,8 @@ struct v4l2_modulator { __u32 rangelow; __u32 rangehigh; __u32 txsubchans; - __u32 reserved[4]; + __u32 band; + __u32 reserved[3]; }; /* Flags for the 'capability' field */ @@ -2048,6 +2050,11 @@ struct v4l2_modulator { #define V4L2_TUNER_CAP_RDS0x0080 #define V4L2_TUNER_CAP_RDS_BLOCK_IO 0x0100 #define V4L2_TUNER_CAP_RDS_CONTROLS 0x0200 +#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US 0x0001 +#define V4L2_TUNER_CAP_BAND_FM_JAPAN 0x0002 +#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN 0x0004 +#define V4L2_TUNER_CAP_BAND_FM_WEATHER 0x0008 +#define V4L2_TUNER_CAP_BAND_AM_MW0x0010 /* Flags for the 'rxsubchans' field */ #define V4L2_TUNER_SUB_MONO 0x0001 @@ -2065,6 +2072,14 @@ struct v4l2_modulator { #define V4L2_TUNER_MODE_LANG1 0x0003 #define V4L2_TUNER_MODE_LANG1_LANG2 0x0004 +/* Values for the 'band' field */ +#define V4L2_TUNER_BAND_DEFAULT 0 +#define V4L2_TUNER_BAND_FM_EUROPE_US 1 /* 87.5 Mhz - 108 MHz */ +#define V4L2_TUNER_BAND_FM_JAPAN 2 /* 76 MHz - 90 MHz */ +#define V4L2_TUNER_BAND_FM_RUSSIAN3 /* 65.8 MHz - 74 MHz */ +#define V4L2_TUNER_BAND_FM_WEATHER4 /* 162.4 MHz - 162.55 MHz */ +#define V4L2_TUNER_BAND_AM_MW 5 + struct v4l2_frequency { __u32 tuner; __u32 type; /* enum v4l2_tuner_type */ -- 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: [RFCv1 PATCH 5/5] V4L2 spec: add frequency band documentation.
Hi, Comments inline. On 05/27/2012 01:50 PM, Hans Verkuil wrote: From: Hans Verkuilhans.verk...@cisco.com Based in part on an earlier patch fromhallima...@gmail.com. Signed-off-by: Hans Verkuilhans.verk...@cisco.com --- Documentation/DocBook/media/v4l/common.xml | 28 -- .../DocBook/media/v4l/vidioc-g-modulator.xml | 38 +--- Documentation/DocBook/media/v4l/vidioc-g-tuner.xml | 97 +--- .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml|3 +- 4 files changed, 131 insertions(+), 35 deletions(-) diff --git a/Documentation/DocBook/media/v4l/common.xml b/Documentation/DocBook/media/v4l/common.xml index 4101aeb..4e7082d 100644 --- a/Documentation/DocBook/media/v4l/common.xml +++ b/Documentation/DocBook/media/v4l/common.xml @@ -464,17 +464,18 @@ Thestructfieldtype/structfield field of the respective structfieldtuner/structfield field contains the index number of the tuner./para -paraRadio devices have exactly one tuner with index zero, no -video inputs./para +paraRadio input devices have one or more tuners, but these are +obviously not associated with any video inputs./para This is about having multiple tuners for radio devices, not about the band support, IMHO as such this belongs in a different patch. Also it seems we never finished the earlier discussions of how to handle radio devices which really have multiple tuners, so it seems premature to change this at all atm. paraTo query and change tuner properties applications use the VIDIOC-G-TUNER; andVIDIOC-S-TUNER; ioctl, respectively. The v4l2-tuner; returned byconstantVIDIOC_G_TUNER/constant also contains signal status information applicable when the tuner of the -current video input, or a radio tuner is queried. Note that +current video input or a radio tuner is queried. Note that constantVIDIOC_S_TUNER/constant does not switch the current tuner, when there is more than one at all. The tuner is solely determined by -the current video input. Drivers must support both ioctls and set the +the current video input or by callingVIDIOC-S-FREQUENCY; for radio +tuners. Drivers must support both ioctls and set the constantV4L2_CAP_TUNER/constant flag in thev4l2-capability; returned by theVIDIOC-QUERYCAP; ioctl when the device has one or more tuners./para Again this seems about having multiple tuners on radio devices. If a radio device has multiple tuners, I would expect both to be able to be active at the same time (ie for recording one show and listening an other), so I would expect there to be a mapping between audio-inputs and tuners, just like we have one between video inputs and tuners for video. Which means that the language of S_FREQ selecting a tuner makes no sense, as both can be active at the same time ... All in all I think the whole what to do with radio devices with multiple tuners discussion can best be deferred until we actually encounter such a device. @@ -491,14 +492,24 @@ the modulator. Thestructfieldtype/structfield field of the respectivev4l2-output; returned by theVIDIOC-ENUMOUTPUT; ioctl is set toconstantV4L2_OUTPUT_TYPE_MODULATOR/constant and its structfieldmodulator/structfield field contains the index number -of the modulator. This specification does not define radio output -devices./para +of the modulator./para + +paraRadio output devices have one or more modulators, but these +are obviously not associated with any video outputs./para + +paraA video or radio device cannot support both a tuner and a +modulator. Two separate device nodes will have to be used for such +hardware, one that supports the tuner functionality and one that supports +the modulator functionality. The reason is a limitation with the +VIDIOC-S-FREQUENCY; ioctl where you cannot specify whether the frequency +is for a tuner or a modulator./para paraTo query and change modulator properties applications use theVIDIOC-G-MODULATOR; andVIDIOC-S-MODULATOR; ioctl. Note that constantVIDIOC_S_MODULATOR/constant does not switch the current modulator, when there is more than one at all. The modulator is solely -determined by the current video output. Drivers must support both +determined by the current video output or by callingVIDIOC-S-FREQUENCY; +for radio modulators. Drivers must support both ioctls and set theconstantV4L2_CAP_MODULATOR/constant flag in thev4l2-capability; returned by theVIDIOC-QUERYCAP; ioctl when the device has one or more modulators./para Same again, who says if there are 2 modulators they cannot be both active at the same time, which means the whole notion of selecting one is wrong. @@ -511,8 +522,7 @@ device has one or more modulators./para applications use theVIDIOC-G-FREQUENCY; andVIDIOC-S-FREQUENCY; ioctl which both take a pointer to av4l2-frequency;. These ioctls are used for TV and radio devices alike. Drivers must support both -ioctls when the tuner or modulator ioctls are supported, or -when the device