Em 19-06-2012 10:31, halli manjunatha escreveu:

Hi Mauro,

Please take the Patch-set 7 which I submitted by removing my set_band
implementation (as per Hans V suggestion).

https://lkml.org/lkml/2012/5/21/294

Manju,

That doesn't solve the issue.

As I pointed on my previous email, the ranges aren't consistent among the
radio devices. The best, IMHO, would be to use several g/s_tuner ranges,
one for each supported one.

An alternative would be to write a set of ioctls specific for radio that
would do the same that g/s_tuner does at radio-cadet, but, IMHO, this is
is overdesign.

In any case, we should not apply a patch for it without having a consensus
about the right way.

Regards,
Mauro

Regards
Manju

On Tue, Jun 19, 2012 at 7:36 AM, Hans de Goede [off-list ref] wrote:

quoted

Hi,


On 06/19/2012 01:09 PM, Mauro Carvalho Chehab wrote:

quoted

Em 19-06-2012 05:27, Hans de Goede escreveu:

quoted

Hi,

On 06/19/2012 02:47 AM, Mauro Carvalho Chehab wrote:

quoted

Em 28-05-2012 07:46, Hans Verkuil escreveu:

quoted

From: Hans Verkuil <redacted>

Signed-off-by: Hans Verkuil <redacted>
Acked-by: Hans de Goede <redacted>
---
    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     0x00010000
+#define V4L2_TUNER_CAP_BAND_FM_JAPAN         0x00020000
+#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN       0x00040000
+#define V4L2_TUNER_CAP_BAND_FM_WEATHER       0x00080000
+#define V4L2_TUNER_CAP_BAND_AM_MW            0x00100000

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;

quoted

tea5759 76 - 91 MHz

        rangelow = 76 * 62500;
        rangehigh = 91 * 62500;

quoted

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 discontinuous ranges as multiple tuners how do we
select the right tuner? Certainly *not* though s_tuner, as that would
violate the spec. Note that changing the spec here is not really an option,
S_TUNER is expected to change the properties of the tuner selected through
the index, and is *not* expected to change the active tuner , esp. since
changing the active tuner would raise the question, change the active tuner
for which input ? The spec is clear on this:
"The tuner is solely determined by the current video input."

iow s_tuner sets tuner parameters (such as the band of a multi-band tuner),
but it does not select a tuner. Making s_tuner actually select 1 of multiple
tuners for radio devices, would cause a large discrepancy between radio and
tv tuners.

For tv tuners we've a 1:1 mapping between tuners and inputs, which makes
sense, because
there are actual dual tuner devices, and the purpose of those is to be able
to watch /
record 2 "shows" at the same time. This is simply not the case with these
radio devices,
they can tune both AM and FM but *not* at the same time, so they have a
*single*
*multiple-band* tuner.

Modeling this as multiple tuners is just wrong. Not only have we already
discussed
this in a long discussion, I've patches to extend the tea575x driver with AM
support,
and the initial revision used the multiple tuner model, but that just does
not work
well, and I'm bad Hans V. intervened and pointed out Halli Manjunatha's
patchset for
limiting hw-freq seek ranges, after which all of this has been discussed
extensively!

quoted

Also, this is generic enough to cover even devices with non-standard
frequency ranges.

All bands can easily be detected via a g_tuner loop, and band switching
is done via s_tuner.

Each band range can have its name ("AM", "FM", "AM-SW", "FM-Japan", ...),
and this is a way more generic than what's being proposed.

It is also very very wrong, there is only a single tuner on these devices,
modeling this as multiple tuners is just wrong!

quoted

It likely makes sense to standardize the band names inside the radio core,
in order to avoid having the same band called with two different names
inside
the drivers.

It should also be noticed that each band may have different properties.
On the above, the FM band can do stereo/mono and RDS, while AM is just
mono So, a change like what's proposed would keep requiring two entries.

With FM we already have a situation where some channels are mono and other
stereo, with AM/FM the tuner capabilities would reflect what the tuner can
do on some bands-frequency combinations, just like it now reflects what
it can do on some frequencies.

<snip>

quoted

quoted

87.5 - 108 MHz is very close to 88 - 108 MHz, I don't think it is worth
creating 2 band defines for this.

Yes, it is very close, but Countries that added the extra 500 kHz
bandwidth
added stations there. On those, older devices can't tune into the new
channels.

On those older devices rangelow would get reported as 88 rather then 87.5,
the
band selection mechanism is there to select a certain range approximately,
the exact resulting range will be hw specific and reported in rangelow /'
rangehigh, as the patch documenting the new fields clearly documents.

<snip>

quoted

quoted

This would be covered by the V4L2_TUNER_BAND_FM_UNIVERSAL, however,
on some devices V4L2_TUNER_BAND_FM_UNIVERSAL may include the weather
band,
thus going all the way from 76 - 163 Mhz, so I guess we should add a
V4L2_TUNER_BAND_FM_JAPAN_WIDE for this. Note that the si470x already
supports this, and indeed calls it "Japan wide band"

That's why giving them name via defines is a bad thing: the concept of
"universal" changes from time to time: 15 years ago, an "universal" radio
is a device that were able to tune at AM-SW, AM-MW, AM-HW and FM
(88-108MHz).

An "universal FM" radio used to be 76-108 MHz, but, with the weather band,
it is now 76-163 Mhz.

If a band like that is described as "FM" with a frequency range from 76
to 163 MHz, this is clearer than calling it as "FM unversal".

We will still have rangelow and rangehigh to report the actual implemented
band. So there is no problem here. An app can select universal and then
figure out what universal is on the specific device it is using with a
G_TUNER.

<snip>

quoted

quoted

So lets get back to the basis, for AM/FM switching / limiting hw-freq
seeking, and on some devices likely even just to be able to tune to
certain frequencies we need to select a band with various radio devices.

On some radio devices we may be able to just program the seek range, but
on
most it is hardcoded based on a band selection register.

Except due to regulatory requirements, the driver could just expose the
broadest range. That's what I did with tea5767, as it allows using either
an "universal" range from 76 to 108 MHz, or to limit it to 88.5-108MHz.

quoted

So we need some way of naming the bands, with approx. expected ranges
(the real range supported by the specific device will be reported on a
G_TUNER).

Looking at:
http://en.wikipedia.org/wiki/FM_broadcast_band

I suggest naming the bands after their standards, except for the Japanese
bands which are special and I suggest just naming them after their
country, resulting in:

#define V4L2_TUNER_BAND_FM_CCIR        1 /* 87.5 - 108 Mhz */

CCIR is a bad (and obsolete) name.

Ok, so we call it V4L2_TUNER_BAND_FM_STANDARD, since it seems to
be what most of the world is either using or moving too (most of the
former USSR has also moved to a range of 87.5 - 108, rather then the
OIRT bands).

quoted

It is a bad name because it is the name of the Radio committee of the ITU,
and this committee standardizes all radio ranges, not only the above.

It is an obsolete name, as CCIR was renamed to ITU-R, back in 1992[1].

Btw, take a look at ITU-R BS.450-3 spec, table 1a[2]: it defines several
ranges there:
        87.5-108
        88-108
        88-100          (Norway)

Standard

quoted

        66-73           (Gambia)
        66-74           (Lithuania)

OIRT

quoted

        87.8-108        (US)
        100-108         (India)

Standard

quoted

        76-90           (Japan)

Japan

Note that currently several drivers already implement a band concept in some
way, ie in the tea5767 driver, you expose this through a config flag called
japan_band,
and that at least the saa7134 and cx88 cards code adds a tea5767 tuner
with the japan_band flag set to 0, resulting in not getting the wide band,
but the
small band, and thus likely not working in japan. Also note that since the
tea5767
radio tuner driver uses the standard tuner framework, it reports a hardcoded
range
of 65-108 (radio_range in drivers/media/video/tuner-core.c) independent of
the
japan_band parameter.

The si470x driver has a band *module* parameter instead, note though that in
both cases
the (average) user ends up with a hardcoded band, where he should be able to
adjust it
to match the country/regio he is in...

So we really need some way to enumerate and set radio-bands, not
radio-tuners, but
radio-bands, and that is exactly what the proposed API gives us in a nice
and simple
way.

Regards,

Hans