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
--
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
