[PATCH v7 0/5] TI WL1273 FM Radio driver.

["Matti J. Aaltonen" <matti.j.aaltonen@nokia.com>]

Hello all,

and thanks for the comments Hans.

Now I've done a couple of iterations with the codec on the ALSA mailing
list and that still continues... I've removed all "#undef DEBUG" lines,
because the ALSA people didn't like them.

I'll go through the comments and the rest of the changes:

>> +     tuner->capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_RDS |
>> +             V4L2_TUNER_CAP_STEREO;
>> +
>
> audmode must be set even when the device is in TX mode. Best is to just set it
> to the last set audmode.

I added a state field for the audmode. I used a boolean variable because
that seemed to lead to clearer code than using an int. Other two valued
entities - like the digital/analog mode - could also be modeled with booleans
but I didn't do it because it could be condemned by the community :-)

Should there also be audmode for the modulator?

>> +     if (val == WL1273_RX_MONO) {
>> +             tuner->rxsubchans = V4L2_TUNER_SUB_MONO;
>> +             tuner->audmode = V4L2_TUNER_MODE_MONO;
>> +     } else {
>> +             tuner->rxsubchans = V4L2_TUNER_SUB_STEREO;
>> +             tuner->audmode = V4L2_TUNER_MODE_STEREO;
>> +     }
>
> There are two separate things: detecting whether the signal is stereo or mono
> and selecting the audio mode (this is the format of the audio that is sent to
> userspace). The first is set in rxsubchans and is dynamic, the second is fixed
> and set by the application.
>
> If the device can detect mono vs stereo signals, then rxsubchans should be set
> accordingly. If the device cannot do this, then both mono and stereo should be
> specified in rxsubchans.
>
> The audmode field is like a control: it does not automatically change if the
> signal switches from mono to stereo or vice versa. Unless the hardware is
> unable to map a mono signal to a stereo audio stream or a stereo signal to a
> mono audio stream.
>
> The fact that the code above sets both rxsubchans and audmode suggests either
> that the hardware cannot map stereo to mono or vice versa, or a program bug.
> In the first case we need a comment here, in the second case it should be
> fixed.

I kind of new I was doing something wrong here... but then I thought: why
isn't there a control variable for the RDS? Anyway, now I've made the
distinction between subchans flags and the audmode field.

>> +
>> +     if (core->rds_on)
>> +             modulator->txsubchans |= V4L2_TUNER_SUB_RDS;
>> +     else
>> +             modulator->txsubchans &= ~V4L2_TUNER_SUB_RDS;
>
> This else is not needed.

Else removed...

> Just make this Hz. There is no need to restrict the precision to
> kHz. S_FREQUENCY supports units of 67.5 Hz, so using kHz for the
> spacing seems unnecessary.
> 
> Alternatively the same resolution as S_FREQUENCY can be used (67.5 Hz
> or 67.5 kHz, depending on the CAP_LOW capability). Not sure which is
> best, though.

I think using Hz is the most straightforward policy here so I chose that.

Cheers,
Matti

Matti J. Aaltonen (5):
  V4L2: Add seek spacing and FM RX class.
  MFD: WL1273 FM Radio: MFD driver for the FM radio.
  ASoC: WL1273 FM Radio Digital audio codec.
  V4L2: WL1273 FM Radio: Controls for the FM radio.
  Documentation: v4l: Add hw_seek spacing and FM_RX class

 Documentation/DocBook/v4l/controls.xml             |   71 +
 .../DocBook/v4l/vidioc-s-hw-freq-seek.xml          |   10 +-
 drivers/media/radio/Kconfig                        |   15 +
 drivers/media/radio/Makefile                       |    1 +
 drivers/media/radio/radio-wl1273.c                 | 1972 ++++++++++++++++++++
 drivers/media/video/v4l2-common.c                  |   12 +
 drivers/mfd/Kconfig                                |    6 +
 drivers/mfd/Makefile                               |    2 +
 drivers/mfd/wl1273-core.c                          |  612 ++++++
 include/linux/mfd/wl1273-core.h                    |  314 ++++
 include/linux/videodev2.h                          |   15 +-
 sound/soc/codecs/Kconfig                           |    6 +
 sound/soc/codecs/Makefile                          |    2 +
 sound/soc/codecs/wl1273.c                          |  591 ++++++
 sound/soc/codecs/wl1273.h                          |   42 +
 15 files changed, 3668 insertions(+), 3 deletions(-)
 create mode 100644 drivers/media/radio/radio-wl1273.c
 create mode 100644 drivers/mfd/wl1273-core.c
 create mode 100644 include/linux/mfd/wl1273-core.h
 create mode 100644 sound/soc/codecs/wl1273.c
 create mode 100644 sound/soc/codecs/wl1273.h