Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
Hi, On 06/22/2012 06:15 PM, Mauro Carvalho Chehab wrote: Em 22-06-2012 11:07, Hans Verkuil escreveu: 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.
Em 22-06-2012 11:07, Hans Verkuil escreveu: > Sorry for the late reply, but I've been quite busy the last few days... > > On Tue June 19 2012 16:14:26 Mauro Carvalho Chehab wrote: >> Em 19-06-2012 09:36, Hans de Goede escreveu: >>> 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 >>> >>> Signed-off-by: Hans Verkuil >>> Acked-by: Hans de Goede >>> --- >>> 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
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
Sorry for the late reply, but I've been quite busy the last few days... On Tue June 19 2012 16:14:26 Mauro Carvalho Chehab wrote: > Em 19-06-2012 09:36, Hans de Goede escreveu: > > 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 > > > > Signed-off-by: Hans Verkuil > > Acked-by: Hans de Goede > > --- > > 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. > > > > , this has been discussed extensively between me
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
Hi, On 06/19/2012 07:43 PM, halli manjunatha wrote: On Tue, Jun 19, 2012 at 12:33 PM, Hans de Goede wrote: Hi, On 06/19/2012 06:47 PM, Hans de Goede wrote: Hi, 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
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
On 19/06/12 19:33, Hans de Goede wrote: Hi, On 06/19/2012 06:47 PM, Hans de Goede wrote: Hi, 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 I need to think about all these proposals. I know that when I worked with the cadet driver I didn't like those multiple tuners at all. But I need to read up on these new discussions and think about it. I doubt I'll have time tomorrow, so it's probably going to be Thursday or Friday. 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.
On Tue, Jun 19, 2012 at 12:33 PM, Hans de Goede wrote: > Hi, > > > On 06/19/2012 06:47 PM, Hans de Goede wrote: >> >> Hi, >> >> > 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. This approach looks good to me. > > Regards, > > Hans -- Regards Halli -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
Hi, On 06/19/2012 06:47 PM, Hans de Goede wrote: Hi, Ok, you've convinced me. I agree that having a way to actually enumerate ranges, rather then having a fixed set of ranges, is better. Which brings us back many weeks to the proposal for making it possible to enumerate bands on radio devices. Rather then digging up the old mails lets start anew, I propose the following API for this: 1. A radio device can have multiple tuners, but only 1 can be active (streaming audio to the associated audio input) at the same time. 2. Radio device tuners are enumerated by calling G_TUNER with an increasing index until EINVAL gets returned 3. G_FREQUENCY will always return the frequency and index of the currently active tuner 4. When calling S_TUNER on a radio device, the active tuner will be set to the v4l2_tuner index field 5. When calling S_FREQUENCY on a radio device, the active tuner will be set to the v4l2_frequency tuner field 6. On a G_TUNER call on a radio device the rxsubchans, audmode, signal and afc v4l2_tuner fields are only filled on for the active tuner (as returned by G_FREQUENCY) for inactive tuners these fields are reported as 0. p.s. I forgot: 7. When calling VIDIOC_S_HW_FREQ_SEEK on a radio device, the active tuner will be set to the v4l2_hw_freq_seek tuner field 8. When changing the active tuner with S_TUNER or S_HW_FREQ_SEEK, the current frequency may be changed to fit in the range of the new active tuner 9. For backwards compatibility reasons tuner 0 should be the tuner with the broadest possible FM range Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
Hi, 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.
On Tue, Jun 19, 2012 at 10:41 AM, Mauro Carvalho Chehab wrote: > 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. I agree with you that we should not apply a patch till we come to a conclusion about the design. But the patches which I have sent (PATCH-SET 7) that doesn't deal with FM band selection, instead it adds few other features like below 1) FM RX RDS AF turn ON/OFF 2) FM RX De-Emphasis mode set/get 3) FM TX Alternate Frequency set/get So since these are other features which are not related to Band selection I think you can merge these to K3.6 kernel. > > Regards, > Mauro > >> >> Regards >> Manju >> >> On Tue, Jun 19, 2012 at 7:36 AM, Hans de Goede wrote: >>> 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 >>> >>> Signed-off-by: Hans Verkuil >>> Acked-by: Hans de Goede >>> --- >>> 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_MW 0x0010 >> >> >> 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: >>>
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
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 wrote: >> 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 >> >> Signed-off-by: Hans Verkuil >> Acked-by: Hans de Goede >> --- >> 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
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
Em 19-06-2012 09:36, Hans de Goede escreveu: > 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 > > Signed-off-by: Hans Verkuil > Acked-by: Hans de Goede > --- > 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. > > , 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_tune
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
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 Regards Manju On Tue, Jun 19, 2012 at 7:36 AM, Hans de Goede wrote: > 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 > > Signed-off-by: Hans Verkuil > Acked-by: Hans de Goede > --- > 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_MW 0x0010 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
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
Hi, On 06/19/2012 01:09 PM, Mauro Carvalho Chehab wrote: Em 19-06-2012 05:27, Hans de Goede escreveu: Hi, On 06/19/2012 02:47 AM, Mauro Carvalho Chehab wrote: Em 28-05-2012 07:46, Hans Verkuil escreveu: From: Hans Verkuil Signed-off-by: Hans Verkuil Acked-by: Hans de Goede --- 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. , 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
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
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 >>> >>> Signed-off-by: Hans Verkuil >>> Acked-by: Hans de Goede >>> --- >>>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. 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 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, t
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
Hi, On 06/19/2012 02:47 AM, Mauro Carvalho Chehab wrote: Em 28-05-2012 07:46, Hans Verkuil escreveu: From: Hans Verkuil Signed-off-by: Hans Verkuil Acked-by: Hans de Goede --- 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, but wikipedia points that they may increase it (from 76MHz
Re: [RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
Em 28-05-2012 07:46, Hans Verkuil escreveu: > From: Hans Verkuil > > Signed-off-by: Hans Verkuil > Acked-by: Hans de Goede > --- > 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? > > /* 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 What does "default" mean? > +#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. > +#define V4L2_TUNER_BAND_FM_JAPAN 2 /* 76 MHz - 90 MHz */ This is currently true, but wikipedia points that they may increase it (from 76MHz to 108MHz?) after the end of NTSC broadcast. The DTV range there starts at channel 14 (473 MHz and upper). Maybe they may reserve the channel 7-13 range (VHF High - starting at 177 MHz) like Brazil for DTV. Anyway, what I mean is that calling a frequency range with a Country name is dangerous, as frequency ranges can vary from time to time. > +#define V4L2_TUNER_BAND_FM_RUSSIAN3 /* 65.8 MHz - 74 MHz */ AFAIKT, this is wrong. The range used there is 65.8-104MHz. It used to be 65.8 to 100 MHz. Also, other ex-soviet countries are still using such range. > +#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 */ > 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
[RFCv2 PATCH 4/6] videodev2.h: add frequency band information.
From: Hans Verkuil Signed-off-by: Hans Verkuil Acked-by: Hans de Goede --- 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_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 /* Flags for the 'rxsubchans' field */ #define V4L2_TUNER_SUB_MONO0x0001 @@ -2065,6 +2072,14 @@ struct v4l2_modulator { #define V4L2_TUNER_MODE_LANG1 0x0003 #define V4L2_TUNER_MODE_LANG1_LANG20x0004 +/* 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 */ -- 1.7.10 -- 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