Message-Id: <201203101056.41461.hverkuil@xs4all.nl>
Date:	Sat, 10 Mar 2012 10:56:41 +0100
From:	Hans Verkuil <hverkuil@...all.nl>
To:	halli manjunatha <manjunatha_halli@...com>
Cc:	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
	Manjunatha Halli <x0130808@...com>, shahed@...com
Subject: Re: [PATCH 3/3] wl128x: Add sysfs based support for FM features

On Friday, March 09, 2012 21:44:10 halli manjunatha wrote:
> On Fri, Mar 9, 2012 at 2:29 AM, Hans Verkuil <hverkuil@...all.nl> wrote:
> > On Wednesday, March 07, 2012 22:42:05 halli manjunatha wrote:
> >> On Mon, Mar 5, 2012 at 10:24 AM, halli manjunatha <hallimanju@...il.com> wrote:
> >> > On Fri, Mar 2, 2012 at 3:22 AM, Hans Verkuil <hverkuil@...all.nl> wrote:
> >> >> On Wednesday, February 29, 2012 18:19:27 halli manjunatha wrote:
> >> >>> On Wed, Feb 29, 2012 at 5:25 AM, Hans Verkuil <hverkuil@...all.nl> wrote:
> >> >>> > On Tuesday, February 28, 2012 23:52:21 halli manjunatha wrote:
> >> >>> >> On Tue, Feb 28, 2012 at 4:05 AM, Hans Verkuil <hverkuil@...all.nl> wrote:
> >> >>> >> > On Monday, February 27, 2012 17:29:18 halli manjunatha wrote:
> >> >>> >> >> Hi Hans,
> >> >>> >> >>
> >> >>> >> >> Agreed I don't mind to create new controls for below things
> >> >>> >> >>
> >> >>> >> >> 1) FM RX Band selection (US/Europe, Japan and Russian bands)
> >> >>> >> >> 2) FM RX RDS AF turn ON/OFF
> >> >>> >> >> 3) FM RX RSSI level set/get
> >> >>> >> >> 4) FM TX Alternate Frequency set/get
> >> >>> >> >> 5) FM RX De-Emphasis mode set/get
> >> >>> >> >>
> >> >>> >> >> However, previously you have suggested me to hide few controls (like
> >> >>> >> >> band selection) from the user but few of our application wanted
> >> >>> >> >> controls like above and that is why I have created the sysfs entries.
> >> >>> >> >>
> >> >>> >> >> Please suggest me how can I move forward with new controls or with sysfs?
> >> >>> >> >
> >> >>> >> > The first question is which of these controls are general to FM receivers or
> >> >>> >> > transmitters, and which are specific to this chipset. The chipset specific
> >> >>> >> > controls should be private controls (look at V4L2_CID_MPEG_CX2341X_BASE in
> >> >>> >> > videodev2.h how those are defined). The others should be defined as new
> >> >>> >> > controls of the FM_TX class or the FM_RX class. The FM_RX class should be
> >> >>> >> > defined as a new class as it doesn't exist at the moment. Don't forget to
> >> >>> >> > document these controls clearly.
> >> >>> >> >
> >> >>> >> > With regards to the band selection: I remember that there was a discussion,
> >> >>> >> > but not the details. Do you have a link to that discussion? I can't find it
> >> >>> >> > (at least not quickly :-) ).
> >> >>> >>
> >> >>> >> Below features are generic to all FM receivers so we can add new CID's
> >> >>> >> for below features
> >> >>> >> 1) FM RX RDS AF turn ON/OFF
> >> >>> >> 2) FM TX Alternate Frequency set/get
> >> >>> >>
> >> >>> >> About other 3 features its different issue,
> >> >>> >>     1) FM RX Band selection (US/Europe, Japan and Russian bands)
> >> >>> >>     2) FM RX RSSI level set/get
> >> >>> >>     3) FM RX De-Emphasis mode set/get
> >> >>> >>
> >> >>> >> All these 3 features are generic to any FM receiver, only question is
> >> >>> >> does all FM receivers wants to expose these controls to application
> >> >>> >> writer?
> >> >>> >
> >> >>> > Good question, and there is no good answer at the moment. See e.g. this
> >> >>> > IRC discussion:
> >> >>> >
> >> >>> > http://www.spinics.net/lists/linux-media/msg44023.html
> >> >>> >
> >> >>> > In the absence of a good solution to this problem I am inclined to make
> >> >>> > these controls driver specific but marked experimental. The experimental
> >> >>> > tag allows us to eventually make it a generic control without too much
> >> >>> > hassle.
> >> >>>
> >> >>> Agreed, I will make them driver specific and mark them as experimental.
> >> >>>
> >> >>> >
> >> >>> >> Example Band selection, every FM receiver at the minimum supports both
> >> >>> >> Europe and Japan band, now the question is should we add a CID to
> >> >>> >> switch between these two bands?
> >> >>> >
> >> >>> > If we decide to add a band selection control, then that would be a menu
> >> >>> > control (since there are up to three bands) and it would only be implemented
> >> >>> > by drivers that need it.
> >> >>> >
> >> >>> > What I am still not clear on is *why* you would want this control. What
> >> >>> > is the reason your customers want this? What does it allow you to do that
> >> >>> > can't be done today?
> >> >>>
> >> >>> There are 2 reasons for this,
> >> >>>
> >> >>> First, our chip supports weather band, unlike other bands (Europe,
> >> >>> Japan and Russian) user may wants to
> >> >>> switch to weather band and wants to listen to weather report and again
> >> >>> switches back to normal band.
> >> >>
> >> >> OK, that makes sense. Are the RX and TX independent with regards to
> >> >> band selection?
> >> >
> >> > Yes - RX and TX are independent of band selection
> >> >
> >> >> Make sure that when the band is changed the rangelow and rangehigh values
> >> >> are also changed. If the current frequency is out of that range, then the
> >> >> frequency should be clamped to the closest value frequency. Although an
> >> >> alternative strategy might be to remember the last used frequency for each
> >> >> band. That might make more sense in the case of switching between a normal
> >> >> band and the weather band. We need to define and document which strategy
> >> >> should be used here.
> >> >
> >> > As of now when I switch to new band I just set the frequency to lowest
> >> > of the new band.
> >> > In this way user can seek and tune to what ever channel he wants.
> >> >
> >> Hans,
> >>
> >> Which implementation you wants? start with the lowest of the new band
> >> or closer to the frequency of old band? do we need to remember the
> >> present frequency of the band before switching to new band?
> >>
> >> Please let me know your views.
> >
> > The initial frequency of each band is driver dependent. That's how drivers
> > act at the moment, so we keep that.
> >
> > When switching bands it is best to keep the frequency closest to the old band.
> > This makes sense when switching from e.g. US/Europe and Japan band where there
> > is overlap in frequencies, so I think that's the best approach.
> >
> >> Since this feature is required by all FM receivers shall I make this
> >> as a generic CID?
> >
> > Yes, but see my next question below.
> >
> >>
> >> >> BTW, is the receiver for the weather band implemented as a separate receiver?
> >> >> I read that some devices can listen to the normal band and interrupt that
> >> >> when a weather report is broadcast on the weather band. That implies two
> >> >> receivers and it would require a rethink.
> >> >>
> >> >> Also, is this feature really implemented as separate frequency ranges in
> >> >> hardware? Or is the receiver able to receive on the whole range of frequencies
> >> >> from 65.8 (OIRT) to approx. 165 (weather band range)?
> >> >
> >> > Our chip wont have 2 receivers, it has only 1 receiver which can
> >> > receive on whole frequency range from 65 MHz to 165 MHz.
> >
> > If that's the case, then what does the band selector control actually have to do
> > hardware-wise? Does it just clamp the frequency to the correct frequency range?
> 
> The Wilink chip can't able to receive on all frequencies at a time,
> instead first we need to set the band(need to set chip registers to do
> so) on which receiver has to receive, after this the chip can receive
> on frequency range of this band only.
> 
> Eg: When set to Europe band chip can only receive from 87.5 MHz to 108
> MHz, in this case receiver will give band limit reached interrupt if
> tried to set frequency other than within this band.
> 
> Band selector controller will first switch the band of reception and
> then sets the frequency which is within that band.
> 
> >
> >> >
> >> >> Is the datasheet of this device available somewhere?
> >> >
> >> > Sorry our newest chipset supports this feature so yet now we don't
> >> > have any datasheet available on net.
> >> >
> >> >>
> >> >>> Second,  for FM TX, our chip supports band selection for FM
> >> >>> transmitter, so if the same phone is used in different
> >> >>> regions of world then user can switch to the actual band and start
> >> >>> transmitting by choosing a blank frequency in that band.
> >> >>
> >> >> Isn't this something that can be equally easily done in userspace?
> >> >
> >> > you wants me to do this from driver itself without hinting the
> >> > application about the band ?
> >
> > I think my question is really the same as above: what does the band selector
> > control actually have to do for a TX hardware-wise?
> 
> Yes TX case I accept we may not need the band switch controller,
> instead when user wants to set a frequency, driver will see in which
> band this frequency comes and then first switches the FM chip to
> transmit in that band and then sets the frequency.
> 
> So I don't add any private or public CID for this.

Good.

> >
> > None of the existing radio RX and TX drivers need a band selector, because
> > they just cover the whole frequency range. If apps want to create these bands,
> > then they can do so themselves. So I am trying to understand why this device
> > needs a band selector. It's why I was interested in the datasheet...
> 
> In RX case the the above provided solution wont work because
> 1) If user do hardware seek then seek will be performed within the
> present band not throughout from 65MHz to 162 MHz
> 2) If user wants to switch band he should know the frequency range of
> the band to which he is switching and then set the frequency within
> that band.
> 
> Hardware seek functionality is one of the main reason why we need band
> switch, most of the time user may not have knowledge about the
> frequency ranges of bands so in that case use will switch to a band
> and do hardware seek, if stations found then he will continue on the
> same band if not he can check the next available band and do seek
> again.

How about this: rather than adding a band selection control, why not add a
new band field to struct v4l2_hw_freq_seek? The default, 0, means just use the
full frequency range. Then define the other bands (these are pretty much
standardized). If a band is not supported by the hardware, -ERANGE is returned.
Otherwise the current frequency is clamped to the specified band frequency range
and you start searching.

Band selection doesn't really make sense outside of hardware seek.

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists