Date:	Mon, 8 Oct 2012 10:57:17 -0700
From:	Andrey Smirnov <andrey.smirnov@...vergeddevices.net>
To:	Hans Verkuil <hverkuil@...all.nl>
CC:	<mchehab@...hat.com>, <sameo@...ux.intel.com>,
	<broonie@...nsource.wolfsonmicro.com>, <perex@...ex.cz>,
	<tiwai@...e.de>, <linux-media@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 5/6] Add a V4L2 driver for SI476X MFD

On 10/08/2012 02:30 AM, Hans Verkuil wrote:
> On Sat October 6 2012 03:55:01 Andrey Smirnov wrote:
>> This commit adds a driver that exposes all the radio related
>> functionality of the Si476x series of chips via the V4L2 subsystem.
>>
>> Signed-off-by: Andrey Smirnov <andrey.smirnov@...vergeddevices.net>
>> ---
>>  drivers/media/radio/Kconfig        |   17 +
>>  drivers/media/radio/Makefile       |    1 +
>>  drivers/media/radio/radio-si476x.c | 1153 ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 1171 insertions(+)
>>  create mode 100644 drivers/media/radio/radio-si476x.c
>>
>> diff --git a/drivers/media/radio/Kconfig b/drivers/media/radio/Kconfig
>> index 8090b87..3c79d09 100644
>> --- a/drivers/media/radio/Kconfig
>> +++ b/drivers/media/radio/Kconfig
>> @@ -16,6 +16,23 @@ config RADIO_SI470X
>>  	bool "Silicon Labs Si470x FM Radio Receiver support"
>>  	depends on VIDEO_V4L2
>>  
>> +config RADIO_SI476X
>> +	tristate "Silicon Laboratories Si476x I2C FM Radio"
>> +	depends on I2C && VIDEO_V4L2
>> +	select MFD_CORE
>> +	select MFD_SI476X_CORE
>> +	select SND_SOC_SI476X
>> +	---help---
>> +	  Choose Y here if you have this FM radio chip.
>> +
>> +	  In order to control your radio card, you will need to use programs
>> +	  that are compatible with the Video For Linux 2 API.  Information on
>> +	  this API and pointers to "v4l2" programs may be found at
>> +	  <file:Documentation/video4linux/API.html>.
>> +
>> +	  To compile this driver as a module, choose M here: the
>> +	  module will be called radio-si476x.
>> +
>>  source "drivers/media/radio/si470x/Kconfig"
>>  
>>  config USB_MR800
>> diff --git a/drivers/media/radio/Makefile b/drivers/media/radio/Makefile
>> index c03ce4f..c4618e0 100644
>> --- a/drivers/media/radio/Makefile
>> +++ b/drivers/media/radio/Makefile
>> @@ -19,6 +19,7 @@ obj-$(CONFIG_RADIO_GEMTEK) += radio-gemtek.o
>>  obj-$(CONFIG_RADIO_TRUST) += radio-trust.o
>>  obj-$(CONFIG_I2C_SI4713) += si4713-i2c.o
>>  obj-$(CONFIG_RADIO_SI4713) += radio-si4713.o
>> +obj-$(CONFIG_RADIO_SI476X) += radio-si476x.o
>>  obj-$(CONFIG_RADIO_MIROPCM20) += radio-miropcm20.o
>>  obj-$(CONFIG_USB_DSBR) += dsbr100.o
>>  obj-$(CONFIG_RADIO_SI470X) += si470x/
>> diff --git a/drivers/media/radio/radio-si476x.c b/drivers/media/radio/radio-si476x.c
>> new file mode 100644
>> index 0000000..2d943da
>> --- /dev/null
>> +++ b/drivers/media/radio/radio-si476x.c
>> @@ -0,0 +1,1153 @@
>> +#include <linux/module.h>
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/slab.h>
>> +#include <linux/atomic.h>
>> +#include <linux/videodev2.h>
>> +#include <linux/mutex.h>
>> +#include <media/v4l2-common.h>
>> +#include <media/v4l2-ioctl.h>
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-event.h>
>> +#include <media/v4l2-device.h>
>> +
>> +#include <linux/mfd/si476x-core.h>
>> +
>> +#define FM_FREQ_RANGE_LOW   64000000
>> +#define FM_FREQ_RANGE_HIGH 108000000
>> +
>> +#define AM_FREQ_RANGE_LOW    520000
>> +#define AM_FREQ_RANGE_HIGH 30000000
>> +
>> +#define PWRLINEFLTR (1 << 8)
>> +
>> +#define FREQ_MUL (10000000 / 625)
>> +
>> +#define DRIVER_NAME "si476x-radio"
>> +#define DRIVER_CARD "SI476x AM/FM Receiver"
>> +
>> +enum si476x_freq_bands {
>> +	SI476X_BAND_FM,
>> +	SI476X_BAND_AM,
>> +};
>> +
>> +static const struct v4l2_frequency_band si476x_bands[] = {
>> +	[SI476X_BAND_FM] = {
>> +		.type		= V4L2_TUNER_RADIO,
>> +		.index		= SI476X_BAND_FM,
>> +		.capability	= V4L2_TUNER_CAP_LOW
>> +		| V4L2_TUNER_CAP_STEREO
>> +		| V4L2_TUNER_CAP_RDS
>> +		| V4L2_TUNER_CAP_RDS_BLOCK_IO
>> +		| V4L2_TUNER_CAP_FREQ_BANDS,
>> +		.rangelow	=  64 * FREQ_MUL,
>> +		.rangehigh	= 108 * FREQ_MUL,
>> +		.modulation	= V4L2_BAND_MODULATION_FM,
>> +	},
>> +	[SI476X_BAND_AM] = {
>> +		.type		= V4L2_TUNER_RADIO,
>> +		.index		= SI476X_BAND_AM,
>> +		.capability	= V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_FREQ_BANDS,
>> +		.rangelow	= 0.52 * FREQ_MUL,
>> +		.rangehigh	= 30 * FREQ_MUL,
>> +		.modulation	= V4L2_BAND_MODULATION_AM,
>> +	},
>> +};
>> +
>> +#define PRIVATE_CTL_IDX(x) (x - V4L2_CID_PRIVATE_BASE)
>> +
>> +static int si476x_s_ctrl(struct v4l2_ctrl *ctrl);
>> +
>> +static const char * const deemphasis[] = {
>> +	"75 us",
>> +	"50 us",
>> +};
>> +
>> +static const struct v4l2_ctrl_ops si476x_ctrl_ops = {
>> +	.s_ctrl = si476x_s_ctrl,
>> +};
>> +
>> +static const struct v4l2_ctrl_config si476x_ctrls[] = {
>> +	/*
>> +	   Tuning parameters
>> +	   'max tune errors' is shared for both AM/FM mode of operation
>> +	*/
>> +	{
>> +		.ops	= &si476x_ctrl_ops,
>> +		.id	= SI476X_CID_RSSI_THRESHOLD,
>> +		.name	= "valid rssi threshold",
>> +		.type	= V4L2_CTRL_TYPE_INTEGER,
>> +		.min	= -128,
>> +		.max	= 127,
>> +		.step	= 1,
>> +	},
>> +	{
>> +		.ops	= &si476x_ctrl_ops,
>> +		.id	= SI476X_CID_SNR_THRESHOLD,
>> +		.type	= V4L2_CTRL_TYPE_INTEGER,
>> +		.name	= "valid snr threshold",
>> +		.min	= -128,
>> +		.max	= 127,
>> +		.step	= 1,
>> +	},
>> +	{
>> +		.ops	= &si476x_ctrl_ops,
>> +		.id	= SI476X_CID_MAX_TUNE_ERROR,
>> +		.type	= V4L2_CTRL_TYPE_INTEGER,
>> +		.name	= "max tune errors",
>> +		.min	= 0,
>> +		.max	= 126 * 2,
>> +		.step	= 2,
>> +	},
>> +	/*
>> +	   Region specific parameters
>> +	*/
>> +	{
>> +		.ops	= &si476x_ctrl_ops,
>> +		.id	= SI476X_CID_HARMONICS_COUNT,
>> +		.type	= V4L2_CTRL_TYPE_INTEGER,
>> +		.name	= "count of harmonics to reject",
>> +		.min	= 0,
>> +		.max	= 20,
>> +		.step	= 1,
>> +	},
>> +	{
>> +		.ops	= &si476x_ctrl_ops,
>> +		.id	= SI476X_CID_DEEMPHASIS,
>> +		.type	= V4L2_CTRL_TYPE_MENU,
>> +		.name	= "de-emphassis",
>> +		.qmenu	= deemphasis,
>> +		.min	= 0,
>> +		.max	= ARRAY_SIZE(deemphasis) - 1,
>> +		.def	= 0,
>> +	},
> I think most if not all of the controls above are candidates for turning into
> standardized controls. I recommend that you make a proposal (RFC) for this.
>
> This may be useful as well:
>
> http://lists-archives.com/linux-kernel/27641304-radio-fixes-and-new-features-for-fm.html
>
> This patch series contains a standardized DEEMPHASIS control.
> Note that this patch series is outdated, but patch 2/5 is OK.

So do you want me to take that patch and make it the part of this patch
set or do you want me to create a separate RFC with a patch set that
contains all those controls?

Just to give some description:

SI476X_CID_RSSI_THRESHOLD, SI476X_CID_SNR_THRESHOLD,
SI476X_CID_MAX_TUNE_ERROR are used to determine at which level of SNR,
RSSI the station station should be considered valid and what margin of
error is to be used(SI476X_CID_MAX_TUNE_ERROR) for those parameters.

SI476X_CID_HARMONICS_COUNT is the amount of AC grid noise harmonics
build-in hardware(or maybe FW) will try to filter out in AM mode.

It seems to me that the controls described above are quite chip specific
should I also include them in the RFC?

>> +	{
>> +		.ops	= &si476x_ctrl_ops,
>> +		.id	= SI476X_CID_RDS_RECEPTION,
>> +		.type	= V4L2_CTRL_TYPE_BOOLEAN,
>> +		.name	= "rds",
>> +		.min	= 0,
>> +		.max	= 1,
>> +		.step	= 1,
>> +	},
> If this control returns whether or not RDS is detected, then this control
> should be removed. VIDIOC_G_TUNER will return that information in rxsubchans.

This control allows to turn on/off RDS processing on the radio chip
itself. In IRQ mode in decreases the amount of
IRQs generated by the chip. And in polling(no-IRQ) mode it decreases I2C
traffic significantly(We've had a run of the boards that had
4-tuners on a single I2C bus, working in polling mode).

>
>> +};
>> +
>> +struct si476x_radio;
>> +
>> +/**
>> + * struct si476x_radio_ops - vtable of tuner functions
>> + *
>> + * This table holds pointers to functions implementing particular
>> + * operations depending on the mode in which the tuner chip was
>> + * configured to start in. If the function is not supported
>> + * corresponding element is set to #NULL.
>> + *
>> + * @tune_freq: Tune chip to a specific frequency
>> + * @seek_start: Star station seeking
>> + * @rsq_status: Get Recieved Signal Quality(RSQ) status
>> + * @rds_blckcnt: Get recived RDS blocks count
>> + * @phase_diversity: Change phase diversity mode of the tuner
>> + * @phase_div_status: Get phase diversity mode status
>> + * @acf_status: Get the status of Automatically Controlled
>> + * Features(ACF)
>> + * @agc_status: Get Automatic Gain Control(AGC) status
>> + */
>> +struct si476x_radio_ops {
>> +	int (*tune_freq)(struct si476x_core *, struct si476x_tune_freq_args *);
>> +	int (*seek_start)(struct si476x_core *, bool, bool);
>> +	int (*rsq_status)(struct si476x_core *, struct si476x_rsq_status_args *,
>> +			  struct si476x_rsq_status_report *);
>> +	int (*rds_blckcnt)(struct si476x_core *, bool,
>> +			   struct si476x_rds_blockcount_report *);
>> +
>> +	int (*phase_diversity)(struct si476x_core *,
>> +			       enum si476x_phase_diversity_mode);
>> +	int (*phase_div_status)(struct si476x_core *);
>> +	int (*acf_status)(struct si476x_core *,
>> +			  struct si476x_acf_status_report *);
>> +	int (*agc_status)(struct si476x_core *,
>> +			  struct si476x_agc_status_report *);
>> +};
>> +
>> +/**
>> + * struct si476x_radio - radio device
>> + *
>> + * @core: Pointer to underlying core device
>> + * @videodev: Pointer to video device created by V4L2 subsystem
>> + * @ops: Vtable of functions. See struct si476x_radio_ops for details
>> + * @kref: Reference counter
>> + * @core_lock: An r/w semaphore to brebvent the deletion of underlying
>> + * core structure is the radio device is being used
>> + */
>> +struct si476x_radio {
>> +	struct v4l2_device v4l2dev;
>> +	struct video_device videodev;
>> +	struct v4l2_ctrl_handler ctrl_handler;
>> +
>> +	struct si476x_core  *core;
>> +	/* This field should not be accesses unless core lock is held */
>> +	const struct si476x_radio_ops *ops;
>> +
>> +	u32 rangelow;
>> +	u32 rangehigh;
>> +	u32 spacing;
>> +	u32 audmode;
>> +};
>> +
>> +static inline struct si476x_radio *v4l2_dev_to_radio(struct v4l2_device *d)
>> +{
>> +	return container_of(d, struct si476x_radio, v4l2dev);
>> +}
>> +
>> +static inline struct si476x_radio *v4l2_ctrl_handler_to_radio(struct v4l2_ctrl_handler *d)
>> +{
>> +	return container_of(d, struct si476x_radio, ctrl_handler);
>> +}
>> +
>> +
>> +static int si476x_radio_initialize_mode(struct si476x_radio *);
>> +
>> +/*
>> + * si476x_vidioc_querycap - query device capabilities
>> + */
>> +static int si476x_querycap(struct file *file, void *priv,
>> +			   struct v4l2_capability *capability)
>> +{
>> +	struct si476x_radio *radio = video_drvdata(file);
>> +
>> +	strlcpy(capability->driver, radio->v4l2dev.name,
>> +		sizeof(capability->driver));
>> +	strlcpy(capability->card,   DRIVER_CARD, sizeof(capability->card));
>> +	strlcpy(capability->bus_info, radio->v4l2dev.name, sizeof(capability->bus_info));
> Bus info needs the proper prefix. See the latest querycap documentation:
>
> http://hverkuil.home.xs4all.nl/spec/media.html#vidioc-querycap
>
>> +
>> +	capability->device_caps = V4L2_CAP_TUNER
>> +		| V4L2_CAP_RADIO
>> +		| V4L2_CAP_RDS_CAPTURE
>> +		| V4L2_CAP_READWRITE
>> +		| V4L2_CAP_HW_FREQ_SEEK;
>> +	capability->capabilities = \
> Remove the trailing \
>
>> +		capability->device_caps | V4L2_CAP_DEVICE_CAPS;
>> +	return 0;
>> +}
>> +
>> +static int si476x_enum_freq_bands(struct file *file, void *priv,
>> +				  struct v4l2_frequency_band *band)
>> +{
>> +	int err;
>> +	struct si476x_radio *radio = video_drvdata(file);
>> +
>> +	if (band->tuner != 0)
>> +		return -EINVAL;
> This needs a check against non-blocking mode and it should return -EAGAIN
> if it is non-blocking. HWSEEK while in non-blocking mode is currently not
> supported by the API.
>
>> +
>> +	switch (radio->core->chip_id) {
>> +		/* AM/FM tuners -- all bands are supported */
>> +	case SI476X_CHIP_SI4761:
>> +	case SI476X_CHIP_SI4762:
>> +	case SI476X_CHIP_SI4763:
>> +	case SI476X_CHIP_SI4764:
>> +		if (band->index < ARRAY_SIZE(si476x_bands)) {
>> +			*band = si476x_bands[band->index];
>> +			err = 0;
>> +		} else {
>> +			err = -EINVAL;
>> +		}
>> +		break;
>> +		/* FM companion tuner chips -- only FM bands are
>> +		 * supported */
>> +	case SI476X_CHIP_SI4768:
>> +	case SI476X_CHIP_SI4769:
>> +		if (band->index == SI476X_BAND_FM) {
>> +			*band = si476x_bands[band->index];
>> +			err = 0;
>> +		} else {
>> +			err = -EINVAL;
>> +		}
>> +		break;
>> +	default:
>> +		err = -EINVAL;
>> +	}
>> +
>> +	return err;
>> +}
>> +
>> +static int si476x_g_tuner(struct file *file, void *priv,
>> +			  struct v4l2_tuner *tuner)
>> +{
>> +	int err;
>> +	struct si476x_rsq_status_report report;
>> +	struct si476x_radio *radio = video_drvdata(file);
>> +
>> +	if (tuner->index != 0)
>> +		return -EINVAL;
>> +
>> +	tuner->type       = V4L2_TUNER_RADIO;
>> +	tuner->capability = V4L2_TUNER_CAP_LOW /* Measure frequncies
> frequncies -> frequencies
>
>> +						 * in multiples of
>> +						 * 62.5 Hz */
>> +		| V4L2_TUNER_CAP_STEREO
>> +		| V4L2_TUNER_CAP_HWSEEK_BOUNDED
>> +		| V4L2_TUNER_CAP_HWSEEK_WRAP;
> You probably should also set V4L2_TUNER_CAP_HWSEEK_PROG_LIM. See
> http://hverkuil.home.xs4all.nl/spec/media.html#vidioc-s-hw-freq-seek
>
>> +
>> +	switch (radio->core->chip_id) {
>> +		/* AM/FM tuners -- all bands are supported */
>> +	case SI476X_CHIP_SI4764:
>> +		if (radio->core->diversity_mode == SI476X_PHDIV_SECONDARY_ANTENNA ||
>> +		    radio->core->diversity_mode == SI476X_PHDIV_SECONDARY_COMBINING) {
>> +			strlcpy(tuner->name, "FM (secondary)", sizeof(tuner->name));
>> +			tuner->capability = 0;
>> +			tuner->rxsubchans = 0;
>> +			break;
>> +		}
>> +
>> +		if (radio->core->diversity_mode == SI476X_PHDIV_PRIMARY_ANTENNA ||
>> +		    radio->core->diversity_mode == SI476X_PHDIV_PRIMARY_COMBINING)
>> +			strlcpy(tuner->name, "AM/FM (primary)", sizeof(tuner->name));
>> +
>> +	case SI476X_CHIP_SI4761: /* FALLTHROUGH */
>> +	case SI476X_CHIP_SI4762:
>> +	case SI476X_CHIP_SI4763:
>> +		if (radio->core->chip_id != SI476X_CHIP_SI4764)
>> +			strlcpy(tuner->name, "AM/FM", sizeof(tuner->name));
>> +
>> +		tuner->rxsubchans = V4L2_TUNER_SUB_MONO | V4L2_TUNER_SUB_STEREO
>> +			| V4L2_TUNER_SUB_RDS;
>> +		tuner->capability |= V4L2_TUNER_CAP_RDS
>> +			| V4L2_TUNER_CAP_RDS_BLOCK_IO
>> +			| V4L2_TUNER_CAP_FREQ_BANDS;
>> +
>> +		tuner->rangelow = si476x_bands[SI476X_BAND_AM].rangelow;
>> +
>> +		break;
>> +		/* FM companion tuner chips -- only FM bands are
>> +		 * supported */
>> +	case SI476X_CHIP_SI4768:
>> +	case SI476X_CHIP_SI4769:
>> +		tuner->rxsubchans = V4L2_TUNER_SUB_RDS;
>> +		tuner->capability |= V4L2_TUNER_CAP_RDS
>> +			| V4L2_TUNER_CAP_RDS_BLOCK_IO
>> +			| V4L2_TUNER_CAP_FREQ_BANDS;
>> +		tuner->rangelow = si476x_bands[SI476X_BAND_FM].rangelow;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	tuner->audmode = radio->audmode;
>> +
>> +	tuner->afc = 1;
>> +	tuner->rangehigh = si476x_bands[SI476X_BAND_FM].rangehigh;
>> +
>> +	si476x_core_lock(radio->core);
>> +	{
>> +		struct si476x_rsq_status_args args = {
>> +			.primary	= false,
>> +			.rsqack		= false,
>> +			.attune		= false,
>> +			.cancel		= false,
>> +			.stcack		= false,
>> +		};
>> +		if (radio->ops->rsq_status) {
>> +			err = radio->ops->rsq_status(radio->core,
>> +						     &args, &report);
>> +			if (err < 0) {
>> +				tuner->signal = 0;
>> +			} else {
>> +				/*
>> +				  tuner->signal value range: 0x0000 .. 0xFFFF,
>> +				  report.rssi: -128 .. 127
>> +				*/
>> +				tuner->signal = (report.rssi + 128) * 257;
>> +			}
>> +		} else {
>> +			tuner->signal = 0;
>> +			err = -EINVAL;
>> +		}
>> +	}
>> +	si476x_core_unlock(radio->core);
>> +
>> +	return err;
>> +}
>> +
>> +static int si476x_s_tuner(struct file *file, void *priv,
>> +				 struct v4l2_tuner *tuner)
>> +{
>> +	struct si476x_radio *radio = video_drvdata(file);
>> +
>> +	if (tuner->index != 0)
>> +		return -EINVAL;
>> +	else if (tuner->audmode == V4L2_TUNER_MODE_MONO ||
> No need for the 'else' keyword.
>
>> +		 tuner->audmode == V4L2_TUNER_MODE_STEREO)
>> +		radio->audmode = tuner->audmode;
> If audmode is neither mono nor stereo, then you should fall back to stereo.
>
>> +
>> +	return 0;
>> +}
>> +
>> +static int si476x_switch_func(struct si476x_radio *radio, enum si476x_func func)
>> +{
>> +	int err;
>> +
>> +	/*
>> +	   Since power/up down is a very time consuming operation,
>> +	   try to avoid doing it if the requested mode matches the one
>> +	   the tuner is in
>> +	*/
>> +	if (func == radio->core->power_up_parameters.func)
>> +		return 0;
>> +
>> +	err = si476x_core_stop(radio->core, true);
>> +	if (err < 0)
>> +		return err;
>> +
>> +	radio->core->power_up_parameters.func = func;
>> +
>> +	err = si476x_core_start(radio->core, true);
>> +	if (!err)
>> +		err = si476x_radio_initialize_mode(radio);
>> +
>> +	return err;
>> +}
>> +
>> +static int si476x_g_frequency(struct file *file, void *priv,
>> +			      struct v4l2_frequency *f)
>> +{
>> +	int err;
>> +	struct si476x_radio *radio = video_drvdata(file);
>> +
>> +	if (f->tuner != 0 ||
>> +	    f->type  != V4L2_TUNER_RADIO)
>> +		return -EINVAL;
>> +
>> +	si476x_core_lock(radio->core);
>> +
>> +	f->type = V4L2_TUNER_RADIO;
> This line is obviously not needed.
>
>> +	if (radio->ops->rsq_status) {
>> +		struct si476x_rsq_status_report report;
>> +		struct si476x_rsq_status_args   args = {
>> +			.primary	= false,
>> +			.rsqack		= false,
>> +			.attune		= true,
>> +			.cancel		= false,
>> +			.stcack		= false,
>> +		};
>> +
>> +		err = radio->ops->rsq_status(radio->core, &args, &report);
>> +		if (!err)
>> +			f->frequency = si476x_to_v4l2(radio->core,
>> +						      report.readfreq);
>> +	} else {
>> +		err = -EINVAL;
>> +	}
>> +
>> +	si476x_core_unlock(radio->core);
>> +
>> +	return err;
>> +}
>> +
>> +static int si476x_s_frequency(struct file *file, void *priv,
>> +			      struct v4l2_frequency *f)
>> +{
>> +	int err;
>> +	struct si476x_radio *radio = video_drvdata(file);
>> +
>> +	if (f->tuner != 0 ||
>> +	    f->type  != V4L2_TUNER_RADIO)
>> +		return -EINVAL;
>> +
>> +	si476x_core_lock(radio->core);
>> +
>> +	/* Remap rangewlow - 1 and rangehigh + 1 */
>> +	if (f->frequency == si476x_bands[SI476X_BAND_FM].rangelow - 1 ||
>> +	    f->frequency == si476x_bands[SI476X_BAND_AM].rangelow - 1)
>> +		f->frequency += 1;
>> +
>> +	if (f->frequency == si476x_bands[SI476X_BAND_FM].rangehigh + 1 ||
>> +	    f->frequency == si476x_bands[SI476X_BAND_AM].rangehigh + 1)
>> +		f->frequency -= 1;
> Huh?
>
> I think you are working around a v4l2-compliance test where I use rangelow-1 and
> rangehigh+1 to test if s_frequency will correctly clamp frequencies. But obviously
> the point is to clamp any out-of-range frequency to the closest valid frequency
> range.
>
> In other words, an out-of-range frequency value shouldn't result in an error,
> but it should be mapped to the closest valid frequency.

Oh, I see. I didn't understand the purpose of that check initially and
thought that this rangehigh + 1/rangelow - 1 was some sort of a special
use-case. I'll fix it in the next version.

Andrey Smirnov

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