Re: RFC: Core + Radio profile

2012-08-22 Thread Hans de Goede

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

2012-08-22 Thread Hans de Goede

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 ?

2012-08-20 Thread Hans de Goede

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 ?

2012-08-20 Thread Hans de Goede

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

2012-08-18 Thread Hans de Goede

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

2012-08-17 Thread Hans de Goede

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

2012-08-17 Thread Hans de Goede

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

2012-08-15 Thread Hans de Goede

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

2012-08-14 Thread Hans de Goede

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

2012-08-14 Thread Hans de Goede

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

2012-08-14 Thread Hans de Goede

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

2012-08-14 Thread Hans de Goede

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

2012-08-14 Thread Hans de Goede

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

2012-08-13 Thread Hans de Goede

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

2012-08-13 Thread Hans de Goede

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

2012-08-12 Thread Hans de Goede

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

2012-08-11 Thread Hans de Goede

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

2012-08-11 Thread Hans de Goede
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

2012-08-11 Thread Hans de Goede
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

2012-08-11 Thread Hans de Goede
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

2012-08-11 Thread Hans de Goede
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

2012-08-11 Thread Hans de Goede
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

2012-08-10 Thread Hans de Goede

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)

2012-08-10 Thread Hans de Goede

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)

2012-08-10 Thread Hans de Goede

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

2012-08-10 Thread Hans de Goede
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

2012-08-10 Thread Hans de Goede
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

2012-08-09 Thread Hans de Goede

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

2012-08-09 Thread Hans de Goede

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)

2012-08-09 Thread Hans de Goede

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

2012-08-09 Thread Hans de Goede

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)

2012-08-09 Thread Hans de Goede

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

2012-08-06 Thread Hans de Goede

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)

2012-08-01 Thread Hans de Goede

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

2012-07-28 Thread Hans de Goede

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

2012-07-28 Thread Hans de Goede

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

2012-07-28 Thread Hans de Goede

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

2012-07-12 Thread Hans de Goede

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

2012-07-12 Thread Hans de Goede

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)

2012-07-12 Thread Hans de Goede
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.

2012-07-12 Thread Hans de Goede
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

2012-07-12 Thread Hans de Goede
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

2012-07-12 Thread Hans de Goede
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

2012-07-12 Thread Hans de Goede
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

2012-07-12 Thread Hans de Goede
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

2012-07-11 Thread Hans de Goede

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

2012-07-11 Thread Hans de Goede
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

2012-07-11 Thread Hans de Goede
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

2012-07-11 Thread Hans de Goede
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

2012-07-11 Thread Hans de Goede
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

2012-07-11 Thread Hans de Goede
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

2012-07-11 Thread Hans de Goede
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

2012-07-11 Thread Hans de Goede

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)

2012-07-09 Thread Hans de Goede

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

2012-07-09 Thread Hans de Goede

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

2012-07-09 Thread Hans de Goede

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

2012-07-09 Thread Hans de Goede

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

2012-07-09 Thread Hans de Goede

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

2012-07-08 Thread 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?

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

2012-07-08 Thread Hans de Goede

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

2012-07-07 Thread Hans de Goede

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

2012-07-07 Thread Hans de Goede
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

2012-07-07 Thread Hans de Goede
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

2012-07-07 Thread Hans de Goede

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

2012-07-07 Thread Hans de Goede

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

2012-07-07 Thread Hans de Goede

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

2012-07-05 Thread Hans de Goede

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

2012-07-05 Thread Hans de Goede

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

2012-07-05 Thread Hans de Goede

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.

2012-07-04 Thread Hans de Goede

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

2012-07-02 Thread Hans de Goede

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

2012-06-29 Thread Hans de Goede

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.

2012-06-23 Thread Hans de Goede

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.

2012-06-19 Thread Hans de Goede

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.

2012-06-19 Thread Hans de Goede

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.

2012-06-19 Thread Hans de Goede

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.

2012-06-19 Thread Hans de Goede

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.

2012-06-19 Thread Hans de Goede

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

2012-06-14 Thread Hans de Goede
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

2012-06-14 Thread Hans de Goede
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

2012-06-14 Thread Hans de Goede
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

2012-06-14 Thread Hans de Goede
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

2012-06-12 Thread Hans de Goede

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

2012-06-12 Thread Hans de Goede

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)

2012-06-11 Thread Hans de Goede

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)

2012-06-09 Thread Hans de Goede

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?

2012-06-05 Thread Hans de Goede

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?

2012-06-04 Thread Hans de Goede

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

2012-06-02 Thread Hans de Goede

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

2012-06-01 Thread Hans de Goede

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

2012-05-29 Thread Hans de Goede

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)

2012-05-29 Thread Hans de Goede

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

2012-05-29 Thread Hans de Goede

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

2012-05-28 Thread Hans de Goede

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

2012-05-27 Thread Hans de Goede

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.

2012-05-27 Thread Hans de Goede

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

2012-05-27 Thread Hans de Goede

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.

2012-05-27 Thread Hans de Goede

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.

2012-05-27 Thread Hans de Goede

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.

2012-05-27 Thread Hans de Goede

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 

<    1   2   3   4   5   6   7   8   9   10   >