From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mx1.redhat.com ([209.132.183.28]:60689 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751076Ab2E0SLt (ORCPT ); Sun, 27 May 2012 14:11:49 -0400 Message-ID: <4FC26E68.1020704@redhat.com> Date: Sun, 27 May 2012 20:11:52 +0200 From: Hans de Goede MIME-Version: 1.0 To: Hans Verkuil CC: linux-media@vger.kernel.org, halli manjunatha , Hans Verkuil Subject: Re: [RFCv1 PATCH 5/5] V4L2 spec: add frequency band documentation. References: <1338119425-17274-1-git-send-email-hverkuil@xs4all.nl> <04d04591632a7db503981b27335eaad27c73d332.1338118975.git.hans.verkuil@cisco.com> In-Reply-To: <04d04591632a7db503981b27335eaad27c73d332.1338118975.git.hans.verkuil@cisco.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: Hi, Comments inline. On 05/27/2012 01:50 PM, Hans Verkuil wrote: > From: Hans Verkuil > > Based in part on an earlier patch from. > > Signed-off-by: Hans Verkuil > --- > 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 @@ Thetype field of the respective > tuner field contains the index number of > the tuner. > > -Radio devices have exactly one tuner with index zero, no > -video inputs. > +Radio input devices have one or more tuners, but these are > +obviously not associated with any video inputs. > 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. > To query and change tuner properties applications use the > &VIDIOC-G-TUNER; and&VIDIOC-S-TUNER; ioctl, respectively. The > &v4l2-tuner; returned byVIDIOC_G_TUNER 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 > VIDIOC_S_TUNER 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 calling&VIDIOC-S-FREQUENCY; for radio > +tuners. Drivers must support both ioctls and set the > V4L2_CAP_TUNER flag in the&v4l2-capability; > returned by the&VIDIOC-QUERYCAP; ioctl when the device has one or > more tuners. 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. Thetype field of the > respective&v4l2-output; returned by the&VIDIOC-ENUMOUTPUT; ioctl is > set toV4L2_OUTPUT_TYPE_MODULATOR and its > modulator field contains the index number > -of the modulator. This specification does not define radio output > -devices. > +of the modulator. > + > +Radio output devices have one or more modulators, but these > +are obviously not associated with any video outputs. > + > +A 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. > > To query and change modulator properties applications use > the&VIDIOC-G-MODULATOR; and&VIDIOC-S-MODULATOR; ioctl. Note that > VIDIOC_S_MODULATOR 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 calling&VIDIOC-S-FREQUENCY; > +for radio modulators. Drivers must support both > ioctls and set theV4L2_CAP_MODULATOR flag in > the&v4l2-capability; returned by the&VIDIOC-QUERYCAP; ioctl when the > device has one or more modulators. 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. > applications use the&VIDIOC-G-FREQUENCY; and&VIDIOC-S-FREQUENCY; > ioctl which both take a pointer to a&v4l2-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 is a radio device. > +ioctls when the tuner or modulator ioctls are supported. > > > > diff --git a/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml b/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml > index 7f4ac7e..713ba06 100644 > --- a/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml > +++ b/Documentation/DocBook/media/v4l/vidioc-g-modulator.xml > @@ -68,17 +68,17 @@ to this structure. Drivers fill the rest of the structure or return an > applications shall begin at index zero, incrementing by one until the > driver returnsEINVAL. > > -Modulators have two writable properties, an audio > -modulation set and the radio frequency. To change the modulated audio > -subprograms, applications initialize theindex > - andtxsubchans fields and the > -reserved array and call the > -VIDIOC_S_MODULATOR ioctl. Drivers may choose a > -different audio modulation if the request cannot be satisfied. However > -this is a write-only ioctl, it does not return the actual audio > +Modulators have three writable properties, an audio > +modulation set, the frequency band and the radio frequency. To change the > +modulated audio subprograms or frequency band, applications initialize the > +index,band, > +txsubchans andreserved > +fields and call theVIDIOC_S_MODULATOR ioctl. Drivers > +may choose a different audio modulation if the request cannot be satisfied. > +However this is a write-only ioctl, it does not return the actual audio > modulation selected. > > -To change the radio frequency the&VIDIOC-S-FREQUENCY; ioctl > +To change the frequency the&VIDIOC-S-FREQUENCY; ioctl > is available. > > > @@ -110,16 +110,16 @@ change for example with the current video standard. > > __u32 > rangelow > - The lowest tunable frequency in units of 62.5 > -KHz, or if thecapability flag > + The lowest tunable frequency of the current frequency band > +in units of 62.5 kHz, or if thecapability flag > V4L2_TUNER_CAP_LOW is set, in units of 62.5 > Hz. > > > __u32 > rangehigh > - The highest tunable frequency in units of 62.5 > -KHz, or if thecapability flag > + The highest tunable frequency of the current frequency band > +in units of 62.5 kHz, or if thecapability flag > V4L2_TUNER_CAP_LOW is set, in units of 62.5 > Hz. > > @@ -138,7 +138,17 @@ indicator, for example a stereo pilot tone. > > > __u32 > - reserved[4] > + band > + The frequency band. The available bands are > + defined in thecapability field. The band > + V4L2_TUNER_BAND_DEFAULT is always available. After changing > + the band the current frequency will be clamped to the new frequency range. > + See for valid band values. > + > + > + > + __u32 > + reserved[3] > Reserved for future extensions. Drivers and > applications must set the array to zero. > > diff --git a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml > index 95d5371..27a8916 100644 > --- a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml > +++ b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml > @@ -68,10 +68,10 @@ structure. Drivers fill the rest of the structure or return an > applications shall begin at index zero, incrementing by one until the > driver returnsEINVAL. > > -Tuners have two writable properties, the audio mode and > -the radio frequency. To change the audio mode, applications initialize > -theindex, > -audmode and > +Tuners have three writable properties, the audio mode, the frequency > +band and the radio frequency. To change the audio mode and band, applications > +initialize theindex, > +audmode,band and > reserved fields and call the > VIDIOC_S_TUNER ioctl. This will > not change the current tuner, which is determined > @@ -80,7 +80,7 @@ if the requested mode is invalid or unsupported. Since this is a > write-only ioctl, it does not return the actually > selected audio mode. > > -To change the radio frequency the&VIDIOC-S-FREQUENCY; ioctl > +To change the frequency the&VIDIOC-S-FREQUENCY; ioctl > is available. > >
> @@ -127,16 +127,16 @@ the structure refers to a radio tuner only the > > __u32 > rangelow > - The lowest tunable frequency in > -units of 62.5 kHz, or if thecapability > + The lowest tunable frequency of the current > +frequency band in units of 62.5 kHz, or if thecapability > flagV4L2_TUNER_CAP_LOW is set, in units of 62.5 > Hz. > > > __u32 > rangehigh > - The highest tunable frequency in > -units of 62.5 kHz, or if thecapability > + The highest tunable frequency of the current > +frequency band in units of 62.5 kHz, or if thecapability > flagV4L2_TUNER_CAP_LOW is set, in units of 62.5 > Hz. > > @@ -226,7 +226,17 @@ settles at zero,&ie; range is what? --> > > > __u32 > - reserved[4] > + band > + The frequency band. The available bands are > + defined in thecapability field. The band > + V4L2_TUNER_BAND_DEFAULT is always available. After changing > + the band the current frequency will be clamped to the new frequency range. > + See for valid band values. > + > + > + > + __u32 > + reserved[3] > Reserved for future extensions. Drivers and > applications must set the array to zero. > > @@ -340,6 +350,31 @@ radio tuners. > 0x0200 > The RDS data is parsed by the hardware and set via controls. > > + > + V4L2_TUNER_CAP_BAND_FM_EUROPE_US > + 0x010000 > + FM radio European or US band (87.5 Mhz - 108 MHz, exact range is hardware dependent). > + > + > + V4L2_TUNER_CAP_BAND_FM_JAPAN > + 0x020000 > + FM radio Japan band (76 MHz - 90 MHz, exact range is hardware dependent). > + > + > + V4L2_TUNER_CAP_BAND_FM_RUSSIAN > + 0x040000 > + FM radio OIRT or Russian band (65.8 MHz - 74 MHz, exact range is hardware dependent). > + > + > + V4L2_TUNER_CAP_BAND_FM_WEATHER > + 0x080000 > + FM radio weather band (162.4 MHz - 162.55 MHz, exact range is hardware dependent). > + > + > + V4L2_TUNER_CAP_BAND_AM_MW > + 0x100000 > + AM radio medium wave band (520 kHz - 1710 kHz, exact range is hardware dependent). > + > > >
> @@ -532,6 +567,39 @@ Lang1/Lang1 > > > > + > +Radio Band Types > + > + &cs-str; > + > + > + V4L2_TUNER_BAND_DEFAULT  > + This is the default band, which should be the widest frequency range supported by > + the hardware. This band is always available. > + > + > + V4L2_TUNER_BAND_FM_EUROPE_US  > + FM radio European or US band (87.5 Mhz - 108 MHz, exact range is hardware dependent). > + > + > + V4L2_TUNER_BAND_FM_JAPAN  > + FM radio Japan band (76 MHz - 90 MHz, exact range is hardware dependent). > + > + > + V4L2_TUNER_BAND_FM_RUSSIAN  > + FM radio OIRT or Russian band (65.8 MHz - 74 MHz, exact range is hardware dependent). > + > + > + V4L2_TUNER_BAND_FM_WEATHER  > + FM radio weather band (162.4 MHz - 162.55 MHz, exact range is hardware dependent). > + > + > + V4L2_TUNER_BAND_AM_MW  > + AM radio medium wave band (520 kHz - 1710 kHz, exact range is hardware dependent). > + > + > + > +
> > > &return-value; > @@ -541,7 +609,14 @@ Lang1/Lang1 > EINVAL > > The&v4l2-tuner;index is > -out of bounds. > +out of bounds or theband is invalid. > + > + > + > + EBUSY > + > + An attempt was made to change the frequency band while a hardware > +frequency seek was in progress. > > > > 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 d58b648..d893d67 100644 > --- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml > +++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml > @@ -49,7 +49,8 @@ > > Description > > -Start a hardware frequency seek from the current frequency. > +Start a hardware frequency seek from the current frequency covering > +the current frequency band. > To do this applications initialize thetuner, > type,seek_upward, > spacing and The actual frequency band documentation part looks good :) Regards, Hans