[PATCH V6 0/5] [Media] Radio: Fixes and New features for FM

[PATCH V6 0/5] [Media] Radio: Fixes and New features for FM

[manjunatha_halli]

From: Manjunatha Halli <x0130808@ti.com>

Mauro and the list,

Fixed version 6 of patchset 2/5.
 
This version 6 of patchset resolves the comments received from
Han's on patchset v5.
     
This patchset creates new control class 'V4L2_CTRL_CLASS_FM_RX' for FM RX,
introduces 2 new CID's for FM RX and and 1 new CID for FM TX. Also adds 1
field in struct v4l2_hw_freq_seek.
      
This patch adds few new features to TI's FM driver features
are listed below,
       
1) FM TX RDS Support (RT, PS, AF, PI, PTY)          
2) FM RX Russian band support
3) FM RX AF set/get
4) FM RX De-emphasis mode set/get
        
Along with new features this patch also fixes few issues in the driver
like default rssi level for seek, unnecessory logs etc.

Manjunatha Halli (5):
  WL128x: Add support for FM TX RDS
  New control class and features for FM RX
  Add new CID for FM TX RDS Alternate Frequency
  Media: Update docs for V4L2 FM new features
  WL12xx: Add support for FM new features.

 Documentation/DocBook/media/v4l/compat.xml         |    3 +
 Documentation/DocBook/media/v4l/controls.xml       |   77 ++++++++++
 Documentation/DocBook/media/v4l/dev-rds.xml        |    5 +-
 .../DocBook/media/v4l/vidioc-g-ext-ctrls.xml       |    7 +
 Documentation/DocBook/media/v4l/vidioc-g-tuner.xml |   25 ++++
 .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml    |   38 +++++-
 drivers/media/radio/wl128x/fmdrv.h                 |    3 +-
 drivers/media/radio/wl128x/fmdrv_common.c          |   55 ++++++--
 drivers/media/radio/wl128x/fmdrv_common.h          |   43 +++++-
 drivers/media/radio/wl128x/fmdrv_rx.c              |   92 ++++++++++---
 drivers/media/radio/wl128x/fmdrv_rx.h              |    2 +-
 drivers/media/radio/wl128x/fmdrv_tx.c              |   41 +++---
 drivers/media/radio/wl128x/fmdrv_tx.h              |    3 +-
 drivers/media/radio/wl128x/fmdrv_v4l2.c            |  148 +++++++++++++++++++-
 drivers/media/video/v4l2-ctrls.c                   |   18 ++-
 include/linux/videodev2.h                          |   25 +++-
 16 files changed, 503 insertions(+), 82 deletions(-)

-- 
1.7.4.1

[PATCH V6 1/5] WL128x: Add support for FM TX RDS

[manjunatha_halli]

From: Manjunatha Halli <x0130808@ti.com>

This patch adds support for following FM TX RDS features,
     1. Radio Text
     2. PS Name
     3. PI Code
     4. PTY Code.

Along with above this patch fixes few other minor issues(like
fm tx get frequency, unnecessary error messages etc).

Signed-off-by: Manjunatha Halli <x0130808@ti.com>
---
 drivers/media/radio/wl128x/fmdrv_common.c |   17 +++++++---
 drivers/media/radio/wl128x/fmdrv_common.h |   17 +++++++----
 drivers/media/radio/wl128x/fmdrv_rx.c     |    2 +-
 drivers/media/radio/wl128x/fmdrv_tx.c     |   41 ++++++++++---------------
 drivers/media/radio/wl128x/fmdrv_tx.h     |    3 +-
 drivers/media/radio/wl128x/fmdrv_v4l2.c   |   47 +++++++++++++++++++++++++++++
 6 files changed, 90 insertions(+), 37 deletions(-)

diff --git a/drivers/media/radio/wl128x/fmdrv_common.c b/drivers/media/radio/wl128x/fmdrv_common.c
index bf867a6..fcce61a 100644
--- a/drivers/media/radio/wl128x/fmdrv_common.c
+++ b/drivers/media/radio/wl128x/fmdrv_common.c
@@ -354,7 +354,7 @@ static void send_tasklet(unsigned long arg)
 
 	/* Check, is there any timeout happened to last transmitted packet */
 	if ((jiffies - fmdev->last_tx_jiffies) > FM_DRV_TX_TIMEOUT) {
-		fmerr("TX timeout occurred\n");
+		fmdbg("TX timeout occurred\n");
 		atomic_set(&fmdev->tx_cnt, 1);
 	}
 
@@ -615,7 +615,11 @@ static void fm_irq_handle_rds_start(struct fmdev *fmdev)
 {
 	if (fmdev->irq_info.flag & FM_RDS_EVENT & fmdev->irq_info.mask) {
 		fmdbg("irq: rds threshold reached\n");
-		fmdev->irq_info.stage = FM_RDS_SEND_RDS_GETCMD_IDX;
+		/* If RSSI reached below threshold then dont get RDS data */
+		if (fmdev->irq_info.flag & FM_LEV_EVENT)
+			fmdev->irq_info.stage = FM_HW_TUNE_OP_ENDED_IDX;
+		else
+			fmdev->irq_info.stage = FM_RDS_SEND_RDS_GETCMD_IDX;
 	} else {
 		/* Continue next function in interrupt handler table */
 		fmdev->irq_info.stage = FM_HW_TUNE_OP_ENDED_IDX;
@@ -1129,8 +1133,9 @@ int fmc_set_freq(struct fmdev *fmdev, u32 freq_to_set)
 
 int fmc_get_freq(struct fmdev *fmdev, u32 *cur_tuned_frq)
 {
-	if (fmdev->rx.freq == FM_UNDEFINED_FREQ) {
-		fmerr("RX frequency is not set\n");
+	if (fmdev->rx.freq == FM_UNDEFINED_FREQ &&
+			fmdev->tx_data.tx_frq == FM_UNDEFINED_FREQ) {
+		fmerr("RX/TX frequency is not set\n");
 		return -EPERM;
 	}
 	if (cur_tuned_frq == NULL) {
@@ -1144,7 +1149,7 @@ int fmc_get_freq(struct fmdev *fmdev, u32 *cur_tuned_frq)
 		return 0;
 
 	case FM_MODE_TX:
-		*cur_tuned_frq = 0;	/* TODO : Change this later */
+		*cur_tuned_frq = fmdev->tx_data.tx_frq;
 		return 0;
 
 	default:
@@ -1574,6 +1579,8 @@ int fmc_prepare(struct fmdev *fmdev)
 	fmdev->rx.af_mode = FM_RX_RDS_AF_SWITCH_MODE_OFF;
 	fmdev->irq_info.retry = 0;
 
+	fmdev->tx_data.tx_frq = FM_UNDEFINED_FREQ;
+
 	fm_rx_reset_rds_cache(fmdev);
 	init_waitqueue_head(&fmdev->rx.rds.read_queue);
 
diff --git a/drivers/media/radio/wl128x/fmdrv_common.h b/drivers/media/radio/wl128x/fmdrv_common.h
index d9b9c6c..196ff7d 100644
--- a/drivers/media/radio/wl128x/fmdrv_common.h
+++ b/drivers/media/radio/wl128x/fmdrv_common.h
@@ -48,8 +48,8 @@ struct fm_reg_table {
 #define SEARCH_LVL_SET           15
 #define BAND_SET                 16
 #define MUTE_STATUS_SET          17
-#define RDS_PAUSE_LVL_SET        18
-#define RDS_PAUSE_DUR_SET        19
+#define AUD_PAUSE_LVL_SET        18
+#define AUD_PAUSE_DUR_SET        19
 #define RDS_MEM_SET              20
 #define RDS_BLK_B_SET            21
 #define RDS_MSK_B_SET            22
@@ -84,11 +84,12 @@ struct fm_reg_table {
 
 #define FM_POWER_MODE            254
 #define FM_INTERRUPT             255
+#define STATION_VALID		 123
 
 /* Transmitter API */
 
 #define CHANL_SET                55
-#define CHANL_BW_SET		56
+#define SCAN_SPACING_SET         56
 #define REF_SET                  57
 #define POWER_ENB_SET            90
 #define POWER_ATT_SET            58
@@ -103,7 +104,8 @@ struct fm_reg_table {
 #define MONO_SET                 66
 #define MUTE                     92
 #define MPX_LMT_ENABLE           67
-#define PI_SET                   93
+#define REF_ERR_SET		 93
+#define PI_SET                   68
 #define ECC_SET                  69
 #define PTY                      70
 #define AF                       71
@@ -120,6 +122,10 @@ struct fm_reg_table {
 #define TX_AUDIO_LEVEL_TEST      96
 #define TX_AUDIO_LEVEL_TEST_THRESHOLD    73
 #define TX_AUDIO_INPUT_LEVEL_RANGE_SET   54
+#define TX_AUDIO_LEVEL_GET		 7
+#define READ_FMANT_TUNE_VALUE            104
+
+/* New FM APIs (Rx and Tx) */
 #define RX_ANTENNA_SELECT        87
 #define I2C_DEV_ADDR_SET         86
 #define REF_ERR_CALIB_PARAM_SET          88
@@ -131,7 +137,6 @@ struct fm_reg_table {
 #define RSSI_BLOCK_SCAN_FREQ_SET 95
 #define RSSI_BLOCK_SCAN_START    97
 #define RSSI_BLOCK_SCAN_DATA_GET  5
-#define READ_FMANT_TUNE_VALUE            104
 
 /* SKB helpers */
 struct fm_skb_cb {
@@ -348,7 +353,7 @@ struct fm_event_msg_hdr {
  * with this default values after loading RX firmware.
  */
 #define FM_DEFAULT_RX_VOLUME		10
-#define FM_DEFAULT_RSSI_THRESHOLD	3
+#define FM_DEFAULT_RSSI_THRESHOLD	20
 
 /* Range for TX power level in units for dB/uV */
 #define FM_PWR_LVL_LOW			91
diff --git a/drivers/media/radio/wl128x/fmdrv_rx.c b/drivers/media/radio/wl128x/fmdrv_rx.c
index 43fb722..a806bda 100644
--- a/drivers/media/radio/wl128x/fmdrv_rx.c
+++ b/drivers/media/radio/wl128x/fmdrv_rx.c
@@ -156,7 +156,7 @@ static int fm_rx_set_channel_spacing(struct fmdev *fmdev, u32 spacing)
 
 	/* set channel spacing */
 	payload = spacing;
-	ret = fmc_send_cmd(fmdev, CHANL_BW_SET, REG_WR, &payload,
+	ret = fmc_send_cmd(fmdev, SCAN_SPACING_SET, REG_WR, &payload,
 			sizeof(payload), NULL, NULL);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/media/radio/wl128x/fmdrv_tx.c b/drivers/media/radio/wl128x/fmdrv_tx.c
index 6ea33e0..6d879fb 100644
--- a/drivers/media/radio/wl128x/fmdrv_tx.c
+++ b/drivers/media/radio/wl128x/fmdrv_tx.c
@@ -51,6 +51,7 @@ static int set_rds_text(struct fmdev *fmdev, u8 *rds_text)
 	u16 payload;
 	int ret;
 
+	*(u16 *)rds_text = cpu_to_be16(*(u16 *)rds_text);
 	ret = fmc_send_cmd(fmdev, RDS_DATA_SET, REG_WR, rds_text,
 			strlen(rds_text), NULL, NULL);
 	if (ret < 0)
@@ -66,26 +67,31 @@ static int set_rds_text(struct fmdev *fmdev, u8 *rds_text)
 	return 0;
 }
 
-static int set_rds_data_mode(struct fmdev *fmdev, u8 mode)
+int set_rds_picode(struct fmdev *fmdev, u16 pi_val)
 {
 	u16 payload;
 	int ret;
 
-	/* Setting unique PI TODO: how unique? */
-	payload = (u16)0xcafe;
+	payload = pi_val;
 	ret = fmc_send_cmd(fmdev, PI_SET, REG_WR, &payload,
 			sizeof(payload), NULL, NULL);
 	if (ret < 0)
 		return ret;
 
-	/* Set decoder id */
-	payload = (u16)0xa;
-	ret = fmc_send_cmd(fmdev, DI_SET, REG_WR, &payload,
+	return 0;
+}
+
+int set_rds_pty(struct fmdev *fmdev, u16 pty)
+{
+	u16 payload;
+	u32 ret;
+
+	payload = pty;
+	ret = fmc_send_cmd(fmdev, PTY, REG_WR, &payload,
 			sizeof(payload), NULL, NULL);
 	if (ret < 0)
 		return ret;
 
-	/* TODO: RDS_MODE_GET? */
 	return 0;
 }
 
@@ -101,7 +107,6 @@ static int set_rds_len(struct fmdev *fmdev, u8 type, u16 len)
 	if (ret < 0)
 		return ret;
 
-	/* TODO: LENGTH_GET? */
 	return 0;
 }
 
@@ -109,20 +114,17 @@ int fm_tx_set_rds_mode(struct fmdev *fmdev, u8 rds_en_dis)
 {
 	u16 payload;
 	int ret;
-	u8 rds_text[] = "Zoom2\n";
+	u8 rds_text[] = "WL12XX Radio\n";
 
 	fmdbg("rds_en_dis:%d(E:%d, D:%d)\n", rds_en_dis,
 		   FM_RDS_ENABLE, FM_RDS_DISABLE);
 
 	if (rds_en_dis == FM_RDS_ENABLE) {
 		/* Set RDS length */
-		set_rds_len(fmdev, 0, strlen(rds_text));
+		set_rds_len(fmdev, 2, strlen(rds_text));
 
 		/* Set RDS text */
 		set_rds_text(fmdev, rds_text);
-
-		/* Set RDS mode */
-		set_rds_data_mode(fmdev, 0x0);
 	}
 
 	/* Send command to enable RDS */
@@ -136,13 +138,6 @@ int fm_tx_set_rds_mode(struct fmdev *fmdev, u8 rds_en_dis)
 	if (ret < 0)
 		return ret;
 
-	if (rds_en_dis == FM_RDS_ENABLE) {
-		/* Set RDS length */
-		set_rds_len(fmdev, 0, strlen(rds_text));
-
-		/* Set RDS text */
-		set_rds_text(fmdev, rds_text);
-	}
 	fmdev->tx_data.rds.flag = rds_en_dis;
 
 	return 0;
@@ -156,7 +151,6 @@ int fm_tx_set_radio_text(struct fmdev *fmdev, u8 *rds_text, u8 rds_type)
 	if (fmdev->curr_fmmode != FM_MODE_TX)
 		return -EPERM;
 
-	fm_tx_set_rds_mode(fmdev, 0);
 
 	/* Set RDS length */
 	set_rds_len(fmdev, rds_type, strlen(rds_text));
@@ -164,9 +158,6 @@ int fm_tx_set_radio_text(struct fmdev *fmdev, u8 *rds_text, u8 rds_type)
 	/* Set RDS text */
 	set_rds_text(fmdev, rds_text);
 
-	/* Set RDS mode */
-	set_rds_data_mode(fmdev, 0x0);
-
 	payload = 1;
 	ret = fmc_send_cmd(fmdev, RDS_DATA_ENB, REG_WR, &payload,
 			sizeof(payload), NULL, NULL);
@@ -421,6 +412,8 @@ int fm_tx_set_freq(struct fmdev *fmdev, u32 freq_to_set)
 	tx->aud_mode = FM_STEREO_MODE;
 	tx->rds.flag = FM_RDS_DISABLE;
 
+	tx->tx_frq = freq_to_set * 1000; /* in KHz */
+
 	return 0;
 }
 
diff --git a/drivers/media/radio/wl128x/fmdrv_tx.h b/drivers/media/radio/wl128x/fmdrv_tx.h
index 11ae2e4..8ed71bd 100644
--- a/drivers/media/radio/wl128x/fmdrv_tx.h
+++ b/drivers/media/radio/wl128x/fmdrv_tx.h
@@ -32,6 +32,7 @@ int fm_tx_set_radio_text(struct fmdev *, u8 *, u8);
 int fm_tx_set_af(struct fmdev *, u32);
 int fm_tx_set_preemph_filter(struct fmdev *, u32);
 int fm_tx_get_tune_cap_val(struct fmdev *);
-
+int set_rds_picode(struct fmdev *, u16);
+int set_rds_pty(struct fmdev *, u16);
 #endif
 
diff --git a/drivers/media/radio/wl128x/fmdrv_v4l2.c b/drivers/media/radio/wl128x/fmdrv_v4l2.c
index 077d369..494faaf 100644
--- a/drivers/media/radio/wl128x/fmdrv_v4l2.c
+++ b/drivers/media/radio/wl128x/fmdrv_v4l2.c
@@ -210,6 +210,8 @@ static int fm_v4l2_s_ctrl(struct v4l2_ctrl *ctrl)
 	struct fmdev *fmdev = container_of(ctrl->handler,
 			struct fmdev, ctrl_handler);
 
+	int ret;
+
 	switch (ctrl->id) {
 	case V4L2_CID_AUDIO_VOLUME:	/* set volume */
 		return fm_rx_set_volume(fmdev, (u16)ctrl->val);
@@ -224,6 +226,38 @@ static int fm_v4l2_s_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_TUNE_PREEMPHASIS:
 		return fm_tx_set_preemph_filter(fmdev, (u8) ctrl->val);
 
+	case V4L2_CID_RDS_TX_PI:
+		ret = set_rds_picode(fmdev, ctrl->val);
+		if (ret < 0) {
+			fmerr("Failed to set RDS Radio PS Name\n");
+			return ret;
+		}
+		return 0;
+
+	case V4L2_CID_RDS_TX_PTY:
+		ret = set_rds_pty(fmdev, ctrl->val);
+		if (ret < 0) {
+			fmerr("Failed to set RDS Radio PS Name\n");
+			return ret;
+		}
+		return 0;
+
+	case V4L2_CID_RDS_TX_PS_NAME:
+		ret = fm_tx_set_radio_text(fmdev, ctrl->string, 1);
+		if (ret < 0) {
+			fmerr("Failed to set RDS Radio PS Name\n");
+			return ret;
+		}
+		return 0;
+
+	case V4L2_CID_RDS_TX_RADIO_TEXT:
+		ret = fm_tx_set_radio_text(fmdev, ctrl->string, 2);
+		if (ret < 0) {
+			fmerr("Failed to set RDS Radio Text\n");
+			return ret;
+		}
+		return 0;
+
 	default:
 		return -EINVAL;
 	}
@@ -455,6 +489,7 @@ static int fm_v4l2_vidioc_s_modulator(struct file *file, void *priv,
 		fmerr("Failed to set mono/stereo mode for TX\n");
 		return ret;
 	}
+
 	ret = fm_tx_set_rds_mode(fmdev, rds_mode);
 	if (ret < 0)
 		fmerr("Failed to set rds mode for TX\n");
@@ -549,6 +584,18 @@ int fm_v4l2_init_video_device(struct fmdev *fmdev, int radio_nr)
 	v4l2_ctrl_new_std(&fmdev->ctrl_handler, &fm_ctrl_ops,
 			V4L2_CID_AUDIO_MUTE, 0, 1, 1, 1);
 
+	v4l2_ctrl_new_std(&fmdev->ctrl_handler, &fm_ctrl_ops,
+			V4L2_CID_RDS_TX_PI, 0x0, 0xf, 1, 0x0);
+
+	v4l2_ctrl_new_std(&fmdev->ctrl_handler, &fm_ctrl_ops,
+			V4L2_CID_RDS_TX_PTY, 0, 32, 1, 0);
+
+	v4l2_ctrl_new_std(&fmdev->ctrl_handler, &fm_ctrl_ops,
+			V4L2_CID_RDS_TX_PS_NAME, 0, 0xf, 1, 0);
+
+	v4l2_ctrl_new_std(&fmdev->ctrl_handler, &fm_ctrl_ops,
+			V4L2_CID_RDS_TX_RADIO_TEXT, 0, 0xff, 1, 0);
+
 	v4l2_ctrl_new_std_menu(&fmdev->ctrl_handler, &fm_ctrl_ops,
 			V4L2_CID_TUNE_PREEMPHASIS, V4L2_PREEMPHASIS_75_uS,
 			0, V4L2_PREEMPHASIS_75_uS);
-- 
1.7.4.1

[PATCH V6 2/5] New control class and features for FM RX

[manjunatha_halli]

From: Manjunatha Halli <x0130808@ti.com>

This patch creates new ctrl class for FM RX and adds new CID's for
below FM features,
      1) De-Emphasis filter mode
      2) RDS Alternate Frequency switch

Also this patch adds a field for band selection in struct v4l2_hw_freq_seek
and adds new capability flags for all below FM bands
	1) V4L2_TUNER_CAP_BAND_TYPE_DEFAULT -> Default Band
	2) V4L2_TUNER_CAP_BAND_TYPE_EUROPE_US -> Europe/US Band
	3) V4L2_TUNER_CAP_BAND_TYPE_JAPAN   -> Japan Band
	4) V4L2_TUNER_CAP_BAND_TYPE_RUSSIAN -> Russian Band
	5) V4L2_TUNER_CAP_BAND_TYPE_WEATHER -> Weather Band

Signed-off-by: Manjunatha Halli <x0130808@ti.com>
---
 drivers/media/video/v4l2-ctrls.c |   17 ++++++++++++++---
 include/linux/videodev2.h        |   24 +++++++++++++++++++++++-
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index 18015c0..9d7608e 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -243,8 +243,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 		"Vivid",
 		NULL
 	};
-	static const char * const tune_preemphasis[] = {
-		"No Preemphasis",
+	static const char * const tune_emphasis[] = {
+		"None",
 		"50 Microseconds",
 		"75 Microseconds",
 		NULL,
@@ -413,7 +413,9 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 	case V4L2_CID_COLORFX:
 		return colorfx;
 	case V4L2_CID_TUNE_PREEMPHASIS:
-		return tune_preemphasis;
+		return tune_emphasis;
+	case V4L2_CID_TUNE_DEEMPHASIS:
+		return tune_emphasis;
 	case V4L2_CID_FLASH_LED_MODE:
 		return flash_led_mode;
 	case V4L2_CID_FLASH_STROBE_SOURCE:
@@ -644,6 +646,12 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_JPEG_COMPRESSION_QUALITY:	return "Compression Quality";
 	case V4L2_CID_JPEG_ACTIVE_MARKER:	return "Active Markers";
 
+	/* FM Radio Receiver control */
+	/* Keep the order of the 'case's the same as in videodev2.h! */
+	case V4L2_CID_FM_RX_CLASS:		return "FM Radio Receiver Controls";
+	case V4L2_CID_RDS_AF_SWITCH:		return "RDS Alternate Frequency Switch";
+	case V4L2_CID_TUNE_DEEMPHASIS:		return "De-Emphasis";
+
 	default:
 		return NULL;
 	}
@@ -688,6 +696,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_H264_8X8_TRANSFORM:
 	case V4L2_CID_MPEG_VIDEO_H264_VUI_SAR_ENABLE:
 	case V4L2_CID_MPEG_VIDEO_MPEG4_QPEL:
+	case V4L2_CID_RDS_AF_SWITCH:
 		*type = V4L2_CTRL_TYPE_BOOLEAN;
 		*min = 0;
 		*max = *step = 1;
@@ -733,6 +742,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL:
 	case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
 	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
+	case V4L2_CID_TUNE_DEEMPHASIS:
 		*type = V4L2_CTRL_TYPE_MENU;
 		break;
 	case V4L2_CID_RDS_TX_PS_NAME:
@@ -745,6 +755,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_FM_TX_CLASS:
 	case V4L2_CID_FLASH_CLASS:
 	case V4L2_CID_JPEG_CLASS:
+	case V4L2_CID_FM_RX_CLASS:
 		*type = V4L2_CTRL_TYPE_CTRL_CLASS;
 		/* You can neither read not write these */
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index c9c9a46..7b3dd95 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1137,6 +1137,7 @@ struct v4l2_ext_controls {
 #define V4L2_CTRL_CLASS_FM_TX 0x009b0000	/* FM Modulator control class */
 #define V4L2_CTRL_CLASS_FLASH 0x009c0000	/* Camera flash controls */
 #define V4L2_CTRL_CLASS_JPEG 0x009d0000		/* JPEG-compression controls */
+#define V4L2_CTRL_CLASS_FM_RX 0x009e0000	/* FM Receiver control class */
 
 #define V4L2_CTRL_ID_MASK      	  (0x0fffffff)
 #define V4L2_CTRL_ID2CLASS(id)    ((id) & 0x0fff0000UL)
@@ -1782,6 +1783,13 @@ enum v4l2_jpeg_chroma_subsampling {
 #define	V4L2_JPEG_ACTIVE_MARKER_DQT		(1 << 17)
 #define	V4L2_JPEG_ACTIVE_MARKER_DHT		(1 << 18)
 
+/* FM Receiver class control IDs */
+#define V4L2_CID_FM_RX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_RX | 0x900)
+#define V4L2_CID_FM_RX_CLASS			(V4L2_CTRL_CLASS_FM_RX | 1)
+
+#define V4L2_CID_RDS_AF_SWITCH			(V4L2_CID_FM_RX_CLASS_BASE + 1)
+#define V4L2_CID_TUNE_DEEMPHASIS		(V4L2_CID_FM_RX_CLASS_BASE + 2)
+
 /*
  *	T U N I N G
  */
@@ -1819,6 +1827,12 @@ 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_TYPE_DEFAULT	0x00000000	/* Default band */
+#define V4L2_TUNER_CAP_BAND_TYPE_EUROPE_US	0x00010000	/* Europe/US band */
+#define V4L2_TUNER_CAP_BAND_TYPE_JAPAN		0x00020000	/* Japan band */
+#define V4L2_TUNER_CAP_BAND_TYPE_RUSSIAN	0x00030000	/* Russian band */
+#define V4L2_TUNER_CAP_BAND_TYPE_WEATHER	0x00040000	/* Weather band */
+
 
 /*  Flags for the 'rxsubchans' field */
 #define V4L2_TUNER_SUB_MONO		0x0001
@@ -1843,13 +1857,21 @@ struct v4l2_frequency {
 	__u32		      reserved[8];
 };
 
+
+#define V4L2_FM_BAND_DEFAULT	0
+#define V4L2_FM_BAND_EUROPE_US	1	/* 87.5 Mhz - 108 MHz */
+#define V4L2_FM_BAND_JAPAN	2	/* 76 MHz - 90 MHz */
+#define V4L2_FM_BAND_RUSSIAN	3	/* 65.8 MHz - 74 MHz */
+#define V4L2_FM_BAND_WEATHER	4	/* 162.4 MHz - 162.55 MHz */
+
 struct v4l2_hw_freq_seek {
 	__u32		      tuner;
 	enum v4l2_tuner_type  type;
 	__u32		      seek_upward;
 	__u32		      wrap_around;
 	__u32		      spacing;
-	__u32		      reserved[7];
+	__u32		      band;
+	__u32		      reserved[6];
 };
 
 /*
-- 
1.7.4.1

[PATCH V6 3/5] Add new CID for FM TX RDS Alternate Frequency

[manjunatha_halli]

From: Manjunatha Halli <x0130808@ti.com>

Signed-off-by: Manjunatha Halli <x0130808@ti.com>
---
 drivers/media/video/v4l2-ctrls.c |    1 +
 include/linux/videodev2.h        |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index 9d7608e..610076c 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -608,6 +608,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_RDS_TX_PTY:		return "RDS Program Type";
 	case V4L2_CID_RDS_TX_PS_NAME:		return "RDS PS Name";
 	case V4L2_CID_RDS_TX_RADIO_TEXT:	return "RDS Radio Text";
+	case V4L2_CID_RDS_TX_AF_FREQ:		return "RDS Alternate Frequency";
 	case V4L2_CID_AUDIO_LIMITER_ENABLED:	return "Audio Limiter Feature Enabled";
 	case V4L2_CID_AUDIO_LIMITER_RELEASE_TIME: return "Audio Limiter Release Time";
 	case V4L2_CID_AUDIO_LIMITER_DEVIATION:	return "Audio Limiter Deviation";
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 7b3dd95..b16d603 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1699,6 +1699,7 @@ enum  v4l2_exposure_auto_type {
 #define V4L2_CID_RDS_TX_PTY			(V4L2_CID_FM_TX_CLASS_BASE + 3)
 #define V4L2_CID_RDS_TX_PS_NAME			(V4L2_CID_FM_TX_CLASS_BASE + 5)
 #define V4L2_CID_RDS_TX_RADIO_TEXT		(V4L2_CID_FM_TX_CLASS_BASE + 6)
+#define V4L2_CID_RDS_TX_AF_FREQ			(V4L2_CID_FM_TX_CLASS_BASE + 7)
 
 #define V4L2_CID_AUDIO_LIMITER_ENABLED		(V4L2_CID_FM_TX_CLASS_BASE + 64)
 #define V4L2_CID_AUDIO_LIMITER_RELEASE_TIME	(V4L2_CID_FM_TX_CLASS_BASE + 65)
-- 
1.7.4.1

[PATCH V6 4/5] Media: Update docs for V4L2 FM new features

[manjunatha_halli]

From: Manjunatha Halli <x0130808@ti.com>

The list of new features -
	1) New control class for FM RX
	2) New FM RX CID's - De-Emphasis filter mode and RDS AF switch
	3) New FM TX CID - RDS Alternate frequency set.
	4) New capability struct v4l2_tuner flags for band selection

Signed-off-by: Manjunatha Halli <x0130808@ti.com>
---
 Documentation/DocBook/media/v4l/compat.xml         |    3 +
 Documentation/DocBook/media/v4l/controls.xml       |   77 ++++++++++++++++++++
 Documentation/DocBook/media/v4l/dev-rds.xml        |    5 +-
 .../DocBook/media/v4l/vidioc-g-ext-ctrls.xml       |    7 ++
 Documentation/DocBook/media/v4l/vidioc-g-tuner.xml |   25 +++++++
 .../DocBook/media/v4l/vidioc-s-hw-freq-seek.xml    |   38 +++++++++-
 6 files changed, 151 insertions(+), 4 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml
index bce97c5..df1f345 100644
--- a/Documentation/DocBook/media/v4l/compat.xml
+++ b/Documentation/DocBook/media/v4l/compat.xml
@@ -2311,6 +2311,9 @@ more information.</para>
 	  <para>Added FM Modulator (FM TX) Extended Control Class: <constant>V4L2_CTRL_CLASS_FM_TX</constant> and their Control IDs.</para>
 	</listitem>
 	<listitem>
+	<para>Added FM Receiver (FM RX) Extended Control Class: <constant>V4L2_CTRL_CLASS_FM_RX</constant> and their Control IDs.</para>
+	</listitem>
+	<listitem>
 	  <para>Added Remote Controller chapter, describing the default Remote Controller mapping for media devices.</para>
 	</listitem>
       </orderedlist>
diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
index b84f25e..ced2bee 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -3018,6 +3018,12 @@ to find receivers which can scroll strings sized as 32 x N or 64 x N characters.
 with steps of 32 or 64 characters. The result is it must always contain a string with size multiple of 32 or 64. </entry>
 	  </row>
 	  <row>
+	  <entry spanname="id"><constant>V4L2_CID_RDS_TX_AF_FREQ</constant>&nbsp;</entry>
+	  <entry>integer</entry>
+	  </row>
+	  <row><entry spanname="descr">Sets the RDS Alternate Frequency value which allows a receiver to re-tune to a different frequency providing the same station when the first signal becomes too weak (e.g., when moving out of range). </entry>
+	  </row>
+	  <row>
 	    <entry spanname="id"><constant>V4L2_CID_AUDIO_LIMITER_ENABLED</constant>&nbsp;</entry>
 	    <entry>boolean</entry>
 	  </row>
@@ -3146,6 +3152,77 @@ manually or automatically if set to zero. Unit, range and step are driver-specif
 <xref linkend="en50067" /> document, from CENELEC.</para>
     </section>
 
+    <section id="fm-rx-controls">
+      <title>FM Receiver Control Reference</title>
+
+      <para>The FM Receiver (FM_RX) class includes controls for common features of
+FM Reception capable devices.</para>
+
+      <table pgwide="1" frame="none" id="fm-rx-control-id">
+      <title>FM_RX Control IDs</title>
+
+      <tgroup cols="4">
+        <colspec colname="c1" colwidth="1*" />
+        <colspec colname="c2" colwidth="6*" />
+        <colspec colname="c3" colwidth="2*" />
+        <colspec colname="c4" colwidth="6*" />
+        <spanspec namest="c1" nameend="c2" spanname="id" />
+        <spanspec namest="c2" nameend="c4" spanname="descr" />
+        <thead>
+          <row>
+            <entry spanname="id" align="left">ID</entry>
+            <entry align="left">Type</entry>
+          </row><row rowsep="1"><entry spanname="descr" align="left">Description</entry>
+          </row>
+        </thead>
+        <tbody valign="top">
+          <row><entry></entry></row>
+          <row>
+            <entry spanname="id"><constant>V4L2_CID_FM_RX_CLASS</constant>&nbsp;</entry>
+            <entry>class</entry>
+          </row><row><entry spanname="descr">The FM_RX class
+descriptor. Calling &VIDIOC-QUERYCTRL; for this control will return a
+description of this control class.</entry>
+          </row>
+          <row>
+            <entry spanname="id"><constant>V4L2_CID_RDS_AF_SWITCH</constant>&nbsp;</entry>
+            <entry>boolean</entry>
+          </row>
+          <row><entry spanname="descr">Enable or Disable the RDS Alternate frequency feature. When enabled the driver will decode the RDS AF field and tries to switch to this AF frequency once the current frequency RSSI (Received signal strength indication) level goes below the threshold. If the frequency is switched, then &VIDIOC-G-FREQUENCY; will return the new frequency.</entry>
+          </row>
+          <row>
+	    <entry spanname="id"><constant>V4L2_CID_TUNE_DEEMPHASIS</constant>&nbsp;</entry>
+	    <entry>integer</entry>
+	  </row>
+	  <row id="v4l2-deemphasis"><entry spanname="descr">Configures the de-emphasis value for reception.
+A pre-emphasis filter is applied to the broadcast to accentuate the high audio frequencies.
+Depending on the region, a time constant of either 50 or 75 useconds is used. The enum&nbsp;v4l2_preemphasis
+defines possible values for pre-emphasis. Here they are:</entry>
+	</row><row>
+	<entrytbl spanname="descr" cols="2">
+		  <tbody valign="top">
+		    <row>
+		      <entry><constant>V4L2_PREEMPHASIS_DISABLED</constant>&nbsp;</entry>
+		      <entry>No de-emphasis is applied.</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_PREEMPHASIS_50_uS</constant>&nbsp;</entry>
+		      <entry>A de-emphasis of 50 uS is used.</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_PREEMPHASIS_75_uS</constant>&nbsp;</entry>
+		      <entry>A de-emphasis of 75 uS is used.</entry>
+		    </row>
+		  </tbody>
+		</entrytbl>
+
+	  </row>
+          <row><entry></entry></row>
+        </tbody>
+      </tgroup>
+      </table>
+
+      </section>
     <section id="flash-controls">
       <title>Flash Control Reference</title>
 
diff --git a/Documentation/DocBook/media/v4l/dev-rds.xml b/Documentation/DocBook/media/v4l/dev-rds.xml
index 38883a4..8188161 100644
--- a/Documentation/DocBook/media/v4l/dev-rds.xml
+++ b/Documentation/DocBook/media/v4l/dev-rds.xml
@@ -55,8 +55,9 @@ If the driver only passes RDS blocks without interpreting the data
 the <constant>V4L2_TUNER_CAP_RDS_BLOCK_IO</constant> flag has to be set. If the
 tuner is capable of handling RDS entities like program identification codes and radio
 text, the flag <constant>V4L2_TUNER_CAP_RDS_CONTROLS</constant> should be set,
-see <link linkend="writing-rds-data">Writing RDS data</link> and
-<link linkend="fm-tx-controls">FM Transmitter Control Reference</link>.</para>
+see <link linkend="writing-rds-data">Writing RDS data</link>,
+<link linkend="fm-tx-controls">FM Transmitter Control Reference</link>
+<link linkend="fm-rx-controls">FM Receiver Control Reference</link>.</para>
   </section>
 
   <section  id="reading-rds-data">
diff --git a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
index b17a7aa..2a8b44e 100644
--- a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
@@ -258,6 +258,13 @@ These controls are described in <xref
 These controls are described in <xref
 		linkend="fm-tx-controls" />.</entry>
 	  </row>
+          <row>
+            <entry><constant>V4L2_CTRL_CLASS_FM_RX</constant></entry>
+             <entry>0x9c0000</entry>
+             <entry>The class containing FM Receiver (FM RX) controls.
+These controls are described in <xref
+                 linkend="fm-rx-controls" />.</entry>
+           </row>
 	  <row>
 	    <entry><constant>V4L2_CTRL_CLASS_FLASH</constant></entry>
 	    <entry>0x9c0000</entry>
diff --git a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml
index 91ec2fb..edad182 100644
--- a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml
@@ -328,6 +328,31 @@ radio tuners.</entry>
 	<entry>0x0200</entry>
 	<entry>The RDS data is parsed by the hardware and set via controls.</entry>
 	  </row>
+	  <row>
+	<entry><constant>V4L2_TUNER_CAP_BAND_TYPE_DEFAULT</constant></entry>
+	<entry>0x010000</entry>
+	<entry>This is the default band, which should be the widest frequency range supported by the hardware.</entry>
+	  </row>
+	  <row>
+	<entry><constant>V4L2_TUNER_CAP_BAND_TYPE_EUROPE_US</constant></entry>
+	<entry>0x020000</entry>
+	<entry>Europe or US band (87.5 Mhz - 108 MHz).</entry>
+	  </row>
+	  <row>
+	<entry><constant>V4L2_TUNER_CAP_BAND_TYPE_JAPAN</constant></entry>
+	<entry>0x030000</entry>
+	<entry>Japan band (76 MHz - 90 MHz).</entry>
+	  </row>
+	  <row>
+	<entry><constant>V4L2_TUNER_CAP_BAND_TYPE_RUSSIAN</constant></entry>
+	<entry>0x040000</entry>
+	<entry>OIRT or Russian band (65.8 MHz - 74 MHz).</entry>
+	  </row>
+	  <row>
+	<entry><constant>V4L2_TUNER_CAP_BAND_TYPE_WEATHER</constant></entry>
+	<entry>0x050000</entry>
+	<entry>Weather band (162.4 MHz - 162.55 MHz).</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
diff --git a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
index 18b1a82..b86c1db 100644
--- a/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-s-hw-freq-seek.xml
@@ -95,7 +95,12 @@ field and the &v4l2-tuner; <structfield>index</structfield> field.</entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
-	    <entry><structfield>reserved</structfield>[7]</entry>
+	    <entry><structfield>band</structfield></entry>
+	    <entry>Configure the FM chip to a specific band before starting seek operation. Please refer to table 'Radio Band Types'.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved</structfield>[6]</entry>
 	    <entry>Reserved for future extensions. Applications
 	    must set the array to zero.</entry>
 	  </row>
@@ -103,6 +108,35 @@ field and the &v4l2-tuner; <structfield>index</structfield> field.</entry>
       </tgroup>
     </table>
   </refsect1>
+    <table pgwide="1" frame="none" id="Radio band Types">
+      <title>Radio Band Types</title>
+      <tgroup cols="2">
+	&cs-str;
+	<tbody valign="top">
+		    <row>
+		      <entry><constant>V4L2_FM_BAND_DEFAULT</constant>&nbsp;</entry>
+		      <entry>This is the default band, which should be the widest frequency range supported by
+		      the hardware.</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_FM_BAND_EUROPE_US</constant>&nbsp;</entry>
+		      <entry>Europe or US band (87.5 Mhz - 108 MHz).</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_FM_BAND_JAPAN</constant>&nbsp;</entry>
+		      <entry>Japan band (76 MHz - 90 MHz).</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_FM_BAND_RUSSIAN</constant>&nbsp;</entry>
+		      <entry>OIRT or Russian band (65.8 MHz - 74 MHz).</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_FM_BAND_WEATHER</constant>&nbsp;</entry>
+		      <entry>Weather band (162.4 MHz - 162.55 MHz).</entry>
+		    </row>
+	</tbody>
+      </tgroup>
+    </table>
 
   <refsect1>
     &return-value;
@@ -112,7 +146,7 @@ field and the &v4l2-tuner; <structfield>index</structfield> field.</entry>
 	<term><errorcode>EINVAL</errorcode></term>
 	<listitem>
 	  <para>The <structfield>tuner</structfield> index is out of
-bounds, the wrap_around value is not supported or the value in the <structfield>type</structfield> field is
+bounds, the band is not supported, the wrap_around value is not supported or the value in the <structfield>type</structfield> field is
 wrong.</para>
 	</listitem>
       </varlistentry>
-- 
1.7.4.1

[PATCH V6 5/5] WL12xx: Add support for FM new features.

[manjunatha_halli]

From: Manjunatha Halli <x0130808@ti.com>

This patch adds below features to TI's V4l2 FM driver for Wilink
chipsets,

	1) FM RX Set frequency allows to set frequency anywhere between
	65.5 MHz till 108 MHz (if chip is Wilink8 then till 164.55 MHz)
	2) FM RX seek caters for band switch
	3) FM RX RDS AF turn ON/OFF
	4) FM RX De-Emphasis mode set/get
	5) FM TX Alternate Frequency set/get

Also this patch fixes below issues

	1) FM audio volume gain setting
	2) Default rssi level is set to 8 instead of 20
	3) Issue related to audio mute/unmute
	4) Enable FM RX AF support in driver
	5) In wrap_around seek mode try for once

Signed-off-by: Manjunatha Halli <x0130808@ti.com>
---
 drivers/media/radio/wl128x/fmdrv.h        |    3 +-
 drivers/media/radio/wl128x/fmdrv_common.c |   38 +++++++++--
 drivers/media/radio/wl128x/fmdrv_common.h |   28 +++++++-
 drivers/media/radio/wl128x/fmdrv_rx.c     |   90 ++++++++++++++++++++-----
 drivers/media/radio/wl128x/fmdrv_rx.h     |    2 +-
 drivers/media/radio/wl128x/fmdrv_v4l2.c   |  101 +++++++++++++++++++++++++++--
 6 files changed, 224 insertions(+), 38 deletions(-)

diff --git a/drivers/media/radio/wl128x/fmdrv.h b/drivers/media/radio/wl128x/fmdrv.h
index d84ad9d..c806c85 100644
--- a/drivers/media/radio/wl128x/fmdrv.h
+++ b/drivers/media/radio/wl128x/fmdrv.h
@@ -150,6 +150,7 @@ struct fm_rx {
 	struct region_info region;	/* Current selected band */
 	u32 freq;	/* Current RX frquency */
 	u8 mute_mode;	/* Current mute mode */
+	u8 bl_flag;	/* Band limit reached flag */
 	u8 deemphasis_mode; /* Current deemphasis mode */
 	/* RF dependent soft mute mode */
 	u8 rf_depend_mute;
@@ -203,7 +204,7 @@ struct fmtx_data {
 struct fmdev {
 	struct video_device *radio_dev;	/* V4L2 video device pointer */
 	struct snd_card *card;	/* Card which holds FM mixer controls */
-	u16 asci_id;
+	u16 asic_id;
 	spinlock_t rds_buff_lock; /* To protect access to RDS buffer */
 	spinlock_t resp_skb_lock; /* To protect access to received SKB */
 
diff --git a/drivers/media/radio/wl128x/fmdrv_common.c b/drivers/media/radio/wl128x/fmdrv_common.c
index fcce61a..ac20556 100644
--- a/drivers/media/radio/wl128x/fmdrv_common.c
+++ b/drivers/media/radio/wl128x/fmdrv_common.c
@@ -44,20 +44,41 @@
 
 /* Region info */
 static struct region_info region_configs[] = {
+	/* Super set of all bands */
+	{
+	 .chanl_space = FM_CHANNEL_SPACING_200KHZ * FM_FREQ_MUL,
+	 .bot_freq = 65800,	/* 87.5 MHz */
+	 .top_freq = 162550,	/* 108 MHz */
+	 .fm_band = 0,
+	 },
 	/* Europe/US */
 	{
 	 .chanl_space = FM_CHANNEL_SPACING_200KHZ * FM_FREQ_MUL,
 	 .bot_freq = 87500,	/* 87.5 MHz */
 	 .top_freq = 108000,	/* 108 MHz */
-	 .fm_band = 0,
+	 .fm_band = 1,
 	 },
 	/* Japan */
 	{
 	 .chanl_space = FM_CHANNEL_SPACING_200KHZ * FM_FREQ_MUL,
 	 .bot_freq = 76000,	/* 76 MHz */
 	 .top_freq = 90000,	/* 90 MHz */
-	 .fm_band = 1,
+	 .fm_band = 2,
 	 },
+	/* Russian (OIRT) band */
+	{
+	.chanl_space = FM_CHANNEL_SPACING_50KHZ * FM_FREQ_MUL_RUS,
+	.bot_freq = 65800,     /* 65.8 MHz */
+	.top_freq = 74000,     /* 74 MHz */
+	.fm_band = 3,
+	},
+	/* Wether Band */
+	{
+	.chanl_space = FM_CHANNEL_SPACING_50KHZ * FM_FREQ_MUL_WB,
+	.bot_freq = 162400,     /* 162.4 MHz */
+	.top_freq = 162550,     /* 162.55 MHz */
+	.fm_band = 4,
+	}
 };
 
 /* Band selection */
@@ -596,7 +617,6 @@ static void fm_irq_handle_flag_getcmd_resp(struct fmdev *fmdev)
 	memcpy(&fmdev->irq_info.flag, skb->data, fm_evt_hdr->dlen);
 
 	fmdev->irq_info.flag = be16_to_cpu(fmdev->irq_info.flag);
-	fmdbg("irq: flag register(0x%x)\n", fmdev->irq_info.flag);
 
 	/* Continue next function in interrupt handler table */
 	fm_irq_call_stage(fmdev, FM_HW_MAL_FUNC_IDX);
@@ -702,7 +722,7 @@ static void fm_rdsparse_swapbytes(struct fmdev *fmdev,
 	 * in Dolphin they are in big endian, the parsing of the RDS data
 	 * is chip dependent
 	 */
-	if (fmdev->asci_id != 0x6350) {
+	if (fmdev->asic_id != 0x6350) {
 		rds_buff = &rds_format->data.groupdatabuff.buff[0];
 		while (index + 1 < FM_RX_RDS_INFO_FIELD_MAX) {
 			byte1 = rds_buff[index];
@@ -1353,11 +1373,13 @@ static int fm_power_up(struct fmdev *fmdev, u8 mode)
 			sizeof(asic_ver), &asic_ver, &resp_len))
 		goto rel;
 
+	fmdev->asic_id = be16_to_cpu(asic_id);
+
 	fmdbg("ASIC ID: 0x%x , ASIC Version: %d\n",
-		be16_to_cpu(asic_id), be16_to_cpu(asic_ver));
+		fmdev->asic_id, be16_to_cpu(asic_ver));
 
 	sprintf(fw_name, "%s_%x.%d.bts", FM_FMC_FW_FILE_START,
-		be16_to_cpu(asic_id), be16_to_cpu(asic_ver));
+		fmdev->asic_id, be16_to_cpu(asic_ver));
 
 	ret = fm_download_firmware(fmdev, fw_name);
 	if (ret < 0) {
@@ -1366,7 +1388,7 @@ static int fm_power_up(struct fmdev *fmdev, u8 mode)
 	}
 	sprintf(fw_name, "%s_%x.%d.bts", (mode == FM_MODE_RX) ?
 			FM_RX_FW_FILE_START : FM_TX_FW_FILE_START,
-			be16_to_cpu(asic_id), be16_to_cpu(asic_ver));
+			fmdev->asic_id, be16_to_cpu(asic_ver));
 
 	ret = fm_download_firmware(fmdev, fw_name);
 	if (ret < 0) {
@@ -1576,7 +1598,7 @@ int fmc_prepare(struct fmdev *fmdev)
 	fmdev->rx.rds.flag = FM_RDS_DISABLE;
 	fmdev->rx.freq = FM_UNDEFINED_FREQ;
 	fmdev->rx.rds_mode = FM_RDS_SYSTEM_RDS;
-	fmdev->rx.af_mode = FM_RX_RDS_AF_SWITCH_MODE_OFF;
+	fmdev->rx.af_mode = FM_RX_RDS_AF_SWITCH_MODE_ON;
 	fmdev->irq_info.retry = 0;
 
 	fmdev->tx_data.tx_frq = FM_UNDEFINED_FREQ;
diff --git a/drivers/media/radio/wl128x/fmdrv_common.h b/drivers/media/radio/wl128x/fmdrv_common.h
index 196ff7d..b25c49e 100644
--- a/drivers/media/radio/wl128x/fmdrv_common.h
+++ b/drivers/media/radio/wl128x/fmdrv_common.h
@@ -201,8 +201,24 @@ struct fm_event_msg_hdr {
 #define FM_UNDEFINED_FREQ		   0xFFFFFFFF
 
 /* Band types */
-#define FM_BAND_EUROPE_US	0
-#define FM_BAND_JAPAN		1
+#define FM_BAND_ALL		0
+#define FM_BAND_EUROPE_US	1
+#define FM_BAND_JAPAN		2
+#define FM_BAND_RUSSIAN		3
+#define FM_BAND_WEATHER		4
+
+/* Frequencies */
+#define FM_RUSSIAN_BAND_LOW	65800
+#define FM_RUSSIAN_BAND_HIGH	74000
+
+#define FM_JAPAN_BAND_LOW       76000
+#define FM_JAPAN_BAND_HIGH      90000
+
+#define FM_US_BAND_LOW          87500
+#define FM_US_BAND_HIGH         108000
+
+#define FM_WEATHER_BAND_LOW	162400
+#define FM_WEATHER_BAND_HIGH	162550
 
 /* Seek directions */
 #define FM_SEARCH_DIRECTION_DOWN	0
@@ -213,6 +229,10 @@ struct fm_event_msg_hdr {
 #define FM_TUNER_PRESET_MODE		1
 #define FM_TUNER_AUTONOMOUS_SEARCH_MODE	2
 #define FM_TUNER_AF_JUMP_MODE		3
+#define FM_TUNER_PI_MATCH_MODE		4
+#define FM_TUNER_BULK_SEARCH_MODE	5
+#define FM_TUNER_WRAP_SEARCH_MODE	6
+#define FM_TUNER_WEATHER_MODE		7
 
 /* Min and Max volume */
 #define FM_RX_VOLUME_MIN	0
@@ -353,7 +373,7 @@ struct fm_event_msg_hdr {
  * with this default values after loading RX firmware.
  */
 #define FM_DEFAULT_RX_VOLUME		10
-#define FM_DEFAULT_RSSI_THRESHOLD	20
+#define FM_DEFAULT_RSSI_THRESHOLD	8
 
 /* Range for TX power level in units for dB/uV */
 #define FM_PWR_LVL_LOW			91
@@ -402,6 +422,8 @@ int fmc_get_mode(struct fmdev *, u8 *);
 #define FM_CHANNEL_SPACING_100KHZ 2
 #define FM_CHANNEL_SPACING_200KHZ 4
 #define FM_FREQ_MUL 50
+#define FM_FREQ_MUL_RUS 10
+#define FM_FREQ_MUL_WB 25
 
 #endif
 
diff --git a/drivers/media/radio/wl128x/fmdrv_rx.c b/drivers/media/radio/wl128x/fmdrv_rx.c
index a806bda..77824d14 100644
--- a/drivers/media/radio/wl128x/fmdrv_rx.c
+++ b/drivers/media/radio/wl128x/fmdrv_rx.c
@@ -51,7 +51,19 @@ int fm_rx_set_freq(struct fmdev *fmdev, u32 freq)
 	u32 resp_len;
 	int ret;
 
-	if (freq < fmdev->rx.region.bot_freq || freq > fmdev->rx.region.top_freq) {
+
+	if (freq >= FM_RUSSIAN_BAND_LOW && freq <= FM_RUSSIAN_BAND_HIGH)
+		fm_rx_set_region(fmdev, FM_BAND_RUSSIAN);
+	else if (freq >= FM_JAPAN_BAND_LOW && freq <= FM_JAPAN_BAND_HIGH)
+		fm_rx_set_region(fmdev, FM_BAND_JAPAN);
+	else if (freq >= FM_US_BAND_LOW && freq <= FM_US_BAND_HIGH)
+		fm_rx_set_region(fmdev, FM_BAND_EUROPE_US);
+	else if (freq >= FM_WEATHER_BAND_LOW && freq <= FM_WEATHER_BAND_HIGH) {
+		if (fmdev->asic_id < 0x1800)
+			return -ERANGE;
+
+		fm_rx_set_region(fmdev, FM_BAND_WEATHER);
+	} else {
 		fmerr("Invalid frequency %d\n", freq);
 		return -EINVAL;
 	}
@@ -72,7 +84,12 @@ int fm_rx_set_freq(struct fmdev *fmdev, u32 freq)
 		return ret;
 
 	/* Calculate frequency index and set*/
-	payload = (freq - fmdev->rx.region.bot_freq) / FM_FREQ_MUL;
+	if (fmdev->rx.region.fm_band == FM_BAND_RUSSIAN)
+		payload = (freq - fmdev->rx.region.bot_freq) / FM_FREQ_MUL_RUS;
+	else if (fmdev->rx.region.fm_band == FM_BAND_WEATHER)
+		payload = (freq - fmdev->rx.region.bot_freq) / FM_FREQ_MUL_WB;
+	else
+		payload = (freq - fmdev->rx.region.bot_freq) / FM_FREQ_MUL;
 
 	ret = fmc_send_cmd(fmdev, FREQ_SET, REG_WR, &payload,
 			sizeof(payload), NULL, NULL);
@@ -117,7 +134,13 @@ int fm_rx_set_freq(struct fmdev *fmdev, u32 freq)
 		goto exit;
 
 	curr_frq = be16_to_cpu(curr_frq);
-	curr_frq_in_khz = (fmdev->rx.region.bot_freq + ((u32)curr_frq * FM_FREQ_MUL));
+
+	if (fmdev->rx.region.fm_band == FM_BAND_RUSSIAN)
+		curr_frq_in_khz = (fmdev->rx.region.bot_freq + ((u32)curr_frq * FM_FREQ_MUL_RUS));
+	else if (fmdev->rx.region.fm_band == FM_BAND_WEATHER)
+		curr_frq_in_khz = (fmdev->rx.region.bot_freq + ((u32)curr_frq * FM_FREQ_MUL_WB));
+	else
+		curr_frq_in_khz = (fmdev->rx.region.bot_freq + ((u32)curr_frq * FM_FREQ_MUL));
 
 	if (curr_frq_in_khz != freq) {
 		pr_info("Frequency is set to (%d) but "
@@ -167,15 +190,22 @@ static int fm_rx_set_channel_spacing(struct fmdev *fmdev, u32 spacing)
 }
 
 int fm_rx_seek(struct fmdev *fmdev, u32 seek_upward,
-		u32 wrap_around, u32 spacing)
+		u32 wrap_around, u32 spacing, u32 fm_band)
 {
 	u32 resp_len;
-	u16 curr_frq, next_frq, last_frq;
+	u16 curr_frq, next_frq, last_frq, fm_frq_mul;
 	u16 payload, int_reason, intr_flag;
 	u16 offset, space_idx;
 	unsigned long timeleft;
 	int ret;
 
+	if (fmdev->rx.region.fm_band != fm_band) {
+		if ((fmdev->asic_id < 0x1800) && (fm_band == FM_BAND_WEATHER))
+			return -EINVAL;
+
+		fm_rx_set_region(fmdev, fm_band);
+	}
+
 	/* Set channel spacing */
 	ret = fm_rx_set_channel_spacing(fmdev, spacing);
 	if (ret < 0) {
@@ -190,10 +220,22 @@ int fm_rx_seek(struct fmdev *fmdev, u32 seek_upward,
 		return ret;
 
 	curr_frq = be16_to_cpu(curr_frq);
-	last_frq = (fmdev->rx.region.top_freq - fmdev->rx.region.bot_freq) / FM_FREQ_MUL;
 
 	/* Check the offset in order to be aligned to the channel spacing*/
-	space_idx = fmdev->rx.region.chanl_space / FM_FREQ_MUL;
+	if (fmdev->rx.region.fm_band == FM_BAND_RUSSIAN) {
+		last_frq = (fmdev->rx.region.top_freq - fmdev->rx.region.bot_freq) / FM_FREQ_MUL_RUS;
+		space_idx = fmdev->rx.region.chanl_space / FM_FREQ_MUL_RUS;
+		fm_frq_mul = FM_FREQ_MUL_RUS;
+	} else if (fmdev->rx.region.fm_band == FM_BAND_WEATHER) {
+		last_frq = (fmdev->rx.region.top_freq - fmdev->rx.region.bot_freq) / FM_FREQ_MUL_WB;
+		space_idx = 1;
+		fm_frq_mul = FM_FREQ_MUL_WB;
+	} else {
+		last_frq = (fmdev->rx.region.top_freq - fmdev->rx.region.bot_freq) / FM_FREQ_MUL;
+		space_idx = fmdev->rx.region.chanl_space / FM_FREQ_MUL;
+		fm_frq_mul = FM_FREQ_MUL;
+	}
+
 	offset = curr_frq % space_idx;
 
 	next_frq = seek_upward ? curr_frq + space_idx /* Seek Up */ :
@@ -238,7 +280,11 @@ again:
 		return ret;
 
 	/* Start seek */
-	payload = FM_TUNER_AUTONOMOUS_SEARCH_MODE;
+	if (fmdev->rx.region.fm_band == FM_BAND_WEATHER)
+		payload = FM_TUNER_WEATHER_MODE;
+	else
+		payload = FM_TUNER_AUTONOMOUS_SEARCH_MODE;
+
 	ret = fmc_send_cmd(fmdev, TUNER_MODE_SET, REG_WR, &payload,
 			sizeof(payload), NULL, NULL);
 	if (ret < 0)
@@ -265,6 +311,7 @@ again:
 		return ret;
 
 	if (int_reason & FM_BL_EVENT) {
+		fmdev->rx.bl_flag = 1;
 		if (wrap_around == 0) {
 			fmdev->rx.freq = seek_upward ?
 				fmdev->rx.region.top_freq :
@@ -274,8 +321,11 @@ again:
 				fmdev->rx.region.bot_freq :
 				fmdev->rx.region.top_freq;
 			/* Calculate frequency index to write */
-			next_frq = (fmdev->rx.freq -
-					fmdev->rx.region.bot_freq) / FM_FREQ_MUL;
+			next_frq = (fmdev->rx.freq - fmdev->rx.region.bot_freq) / fm_frq_mul;
+
+			/* If no valid chanel then report default frequency */
+			wrap_around = 0;
+
 			goto again;
 		}
 	} else {
@@ -286,9 +336,7 @@ again:
 			return ret;
 
 		curr_frq = be16_to_cpu(curr_frq);
-		fmdev->rx.freq = (fmdev->rx.region.bot_freq +
-				((u32)curr_frq * FM_FREQ_MUL));
-
+		fmdev->rx.freq = (fmdev->rx.region.bot_freq + ((u32)curr_frq * fm_frq_mul));
 	}
 	/* Reset RDS cache and current station pointers */
 	fm_rx_reset_rds_cache(fmdev);
@@ -364,18 +412,21 @@ int fm_rx_set_region(struct fmdev *fmdev, u8 region_to_set)
 	int ret;
 
 	if (region_to_set != FM_BAND_EUROPE_US &&
-	    region_to_set != FM_BAND_JAPAN) {
+	    region_to_set != FM_BAND_JAPAN &&
+	    region_to_set != FM_BAND_RUSSIAN &&
+	    region_to_set != FM_BAND_WEATHER) {
 		fmerr("Invalid band\n");
 		return -EINVAL;
 	}
 
 	if (fmdev->rx.region.fm_band == region_to_set) {
-		fmerr("Requested band is already configured\n");
+		fmdbg("Requested band is already configured\n");
 		return 0;
 	}
 
 	/* Send cmd to set the band  */
-	payload = (u16)region_to_set;
+	/* Since 0 indicates super band actual payload should be payload - 1*/
+	payload = (u16)(region_to_set - 1);
 	ret = fmc_send_cmd(fmdev, BAND_SET, REG_WR, &payload,
 			sizeof(payload), NULL, NULL);
 	if (ret < 0)
@@ -636,8 +687,11 @@ int fm_rx_set_deemphasis_mode(struct fmdev *fmdev, u16 mode)
 	if (fmdev->curr_fmmode != FM_MODE_RX)
 		return -EPERM;
 
-	if (mode != FM_RX_EMPHASIS_FILTER_50_USEC &&
-			mode != FM_RX_EMPHASIS_FILTER_75_USEC) {
+	if (mode == V4L2_PREEMPHASIS_DISABLED)
+		return 0;
+
+	if (mode != V4L2_PREEMPHASIS_50_uS &&
+			mode != V4L2_PREEMPHASIS_75_uS) {
 		fmerr("Invalid rx de-emphasis mode (%d)\n", mode);
 		return -EINVAL;
 	}
diff --git a/drivers/media/radio/wl128x/fmdrv_rx.h b/drivers/media/radio/wl128x/fmdrv_rx.h
index 32add81..261420e 100644
--- a/drivers/media/radio/wl128x/fmdrv_rx.h
+++ b/drivers/media/radio/wl128x/fmdrv_rx.h
@@ -37,7 +37,7 @@ int fm_rx_set_af_switch(struct fmdev *, u8);
 void fm_rx_reset_rds_cache(struct fmdev *);
 void fm_rx_reset_station_info(struct fmdev *);
 
-int fm_rx_seek(struct fmdev *, u32, u32, u32);
+int fm_rx_seek(struct fmdev *, u32, u32, u32, u32);
 
 int fm_rx_get_rds_mode(struct fmdev *, u8 *);
 int fm_rx_get_rds_system(struct fmdev *, u8 *);
diff --git a/drivers/media/radio/wl128x/fmdrv_v4l2.c b/drivers/media/radio/wl128x/fmdrv_v4l2.c
index 494faaf..65ba08d 100644
--- a/drivers/media/radio/wl128x/fmdrv_v4l2.c
+++ b/drivers/media/radio/wl128x/fmdrv_v4l2.c
@@ -212,6 +212,7 @@ static int fm_v4l2_s_ctrl(struct v4l2_ctrl *ctrl)
 
 	int ret;
 
+	fmdbg(" fm_v4l2_s_ctrl - AF bit = %d\n", ctrl->val);
 	switch (ctrl->id) {
 	case V4L2_CID_AUDIO_VOLUME:	/* set volume */
 		return fm_rx_set_volume(fmdev, (u16)ctrl->val);
@@ -258,6 +259,22 @@ static int fm_v4l2_s_ctrl(struct v4l2_ctrl *ctrl)
 		}
 		return 0;
 
+	case V4L2_CID_RDS_TX_AF_FREQ:
+		ret = fm_tx_set_af(fmdev, ctrl->val);
+		if (ret < 0) {
+			fmerr("Failed to set FM TX AF Frequency\n");
+			return ret;
+		}
+		return 0;
+
+	case V4L2_CID_RDS_AF_SWITCH:	/* enable/disable FM RX RDS AF*/
+		fmdbg("V4L2_CID_RDS_AF_SWITCH:\n");
+		return fm_rx_set_af_switch(fmdev, (u8)ctrl->val);
+
+	case V4L2_CID_TUNE_DEEMPHASIS:
+		fmdbg("V4L2_CID_TUNE_DEEMPHASIS\n");
+		return fm_rx_set_deemphasis_mode(fmdev, (u8) ctrl->val);
+
 	default:
 		return -EINVAL;
 	}
@@ -319,7 +336,15 @@ static int fm_v4l2_vidioc_g_tuner(struct file *file, void *priv,
 	tuner->rxsubchans = V4L2_TUNER_SUB_MONO | V4L2_TUNER_SUB_STEREO |
 	((fmdev->rx.rds.flag == FM_RDS_ENABLE) ? V4L2_TUNER_SUB_RDS : 0);
 	tuner->capability = V4L2_TUNER_CAP_STEREO | V4L2_TUNER_CAP_RDS |
-			    V4L2_TUNER_CAP_LOW;
+			    V4L2_TUNER_CAP_LOW |
+			    V4L2_TUNER_CAP_BAND_TYPE_DEFAULT |
+			    V4L2_TUNER_CAP_BAND_TYPE_EUROPE_US |
+			    V4L2_TUNER_CAP_BAND_TYPE_JAPAN |
+			    V4L2_TUNER_CAP_BAND_TYPE_RUSSIAN;
+
+	if (fmdev->asic_id > 0x1800)
+		tuner->capability = tuner->capability | V4L2_TUNER_CAP_BAND_TYPE_WEATHER;
+
 	tuner->audmode = (stereo_mono_mode ?
 			  V4L2_TUNER_MODE_MONO : V4L2_TUNER_MODE_STEREO);
 
@@ -420,7 +445,7 @@ static int fm_v4l2_vidioc_s_hw_freq_seek(struct file *file, void *priv,
 		struct v4l2_hw_freq_seek *seek)
 {
 	struct fmdev *fmdev = video_drvdata(file);
-	int ret;
+	int ret, curr_band = fmdev->rx.region.fm_band;
 
 	if (fmdev->curr_fmmode != FM_MODE_RX) {
 		ret = fmc_set_mode(fmdev, FM_MODE_RX);
@@ -430,12 +455,64 @@ static int fm_v4l2_vidioc_s_hw_freq_seek(struct file *file, void *priv,
 		}
 	}
 
-	ret = fm_rx_seek(fmdev, seek->seek_upward, seek->wrap_around,
-			seek->spacing);
-	if (ret < 0)
-		fmerr("RX seek failed - %d\n", ret);
+	if (seek->band != 0) {
+		ret = fm_rx_seek(fmdev, seek->seek_upward, seek->wrap_around,
+			seek->spacing, seek->band);
+		if (ret < 0)
+			fmerr("RX seek failed - %d\n", ret);
 
-	return ret;
+		return ret;
+	} else {
+		do {
+			ret = fm_rx_seek(fmdev, seek->seek_upward, seek->wrap_around,
+					seek->spacing, curr_band);
+			if (ret < 0)
+				fmerr("RX seek failed - %d\n", ret);
+
+			if (fmdev->rx.bl_flag != 1) {
+				return ret;
+			}
+
+			/* If reached the 162.55 or 65.8 then return the same*/
+			if (!seek->seek_upward && (fmdev->rx.region.fm_band == FM_BAND_RUSSIAN))
+				return ret;
+
+			if (seek->seek_upward && (fmdev->rx.region.fm_band == FM_BAND_WEATHER)) {
+				if (fmdev->asic_id < 0x1800) {
+					fmdev->rx.freq = FM_US_BAND_HIGH;
+					return FM_US_BAND_HIGH;
+				}
+
+				return ret;
+			}
+
+			if (seek->seek_upward) {
+				switch (fmdev->rx.region.fm_band) {
+				case FM_BAND_RUSSIAN:
+					curr_band = FM_BAND_JAPAN;
+					break;
+				case FM_BAND_JAPAN:
+					curr_band = FM_BAND_EUROPE_US;
+					break;
+				case FM_BAND_EUROPE_US:
+					curr_band = FM_BAND_WEATHER;
+					break;
+				}
+			} else {
+				switch (fmdev->rx.region.fm_band) {
+				case FM_BAND_WEATHER:
+					curr_band = FM_BAND_EUROPE_US;
+					break;
+				case FM_BAND_EUROPE_US:
+					curr_band = FM_BAND_JAPAN;
+					break;
+				case FM_BAND_JAPAN:
+					curr_band = FM_BAND_RUSSIAN;
+					break;
+				}
+			}
+		} while (1);
+	}
 }
 /* Get modulator attributes. If mode is not TX, return no attributes. */
 static int fm_v4l2_vidioc_g_modulator(struct file *file, void *priv,
@@ -596,6 +673,9 @@ int fm_v4l2_init_video_device(struct fmdev *fmdev, int radio_nr)
 	v4l2_ctrl_new_std(&fmdev->ctrl_handler, &fm_ctrl_ops,
 			V4L2_CID_RDS_TX_RADIO_TEXT, 0, 0xff, 1, 0);
 
+	v4l2_ctrl_new_std(&fmdev->ctrl_handler, &fm_ctrl_ops,
+			V4L2_CID_RDS_TX_AF_FREQ, 76000, 180000, 1, 87500);
+
 	v4l2_ctrl_new_std_menu(&fmdev->ctrl_handler, &fm_ctrl_ops,
 			V4L2_CID_TUNE_PREEMPHASIS, V4L2_PREEMPHASIS_75_uS,
 			0, V4L2_PREEMPHASIS_75_uS);
@@ -611,6 +691,13 @@ int fm_v4l2_init_video_device(struct fmdev *fmdev, int radio_nr)
 	if (ctrl)
 		ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
 
+	v4l2_ctrl_new_std(&fmdev->ctrl_handler, &fm_ctrl_ops,
+			V4L2_CID_RDS_AF_SWITCH, 0, 1, 1, 0);
+
+	v4l2_ctrl_new_std_menu(&fmdev->ctrl_handler, &fm_ctrl_ops,
+			V4L2_CID_TUNE_PREEMPHASIS, V4L2_PREEMPHASIS_75_uS,
+			0, V4L2_PREEMPHASIS_75_uS);
+
 	return 0;
 }
 
-- 
1.7.4.1

Re: [PATCH V6 2/5] New control class and features for FM RX

[Hans Verkuil]

On Tue May 15 2012 00:01:50 manjunatha_halli@ti.com wrote:
> From: Manjunatha Halli <x0130808@ti.com>
> 
> This patch creates new ctrl class for FM RX and adds new CID's for
> below FM features,
>       1) De-Emphasis filter mode
>       2) RDS Alternate Frequency switch
> 
> Also this patch adds a field for band selection in struct v4l2_hw_freq_seek
> and adds new capability flags for all below FM bands
> 	1) V4L2_TUNER_CAP_BAND_TYPE_DEFAULT -> Default Band
> 	2) V4L2_TUNER_CAP_BAND_TYPE_EUROPE_US -> Europe/US Band
> 	3) V4L2_TUNER_CAP_BAND_TYPE_JAPAN   -> Japan Band
> 	4) V4L2_TUNER_CAP_BAND_TYPE_RUSSIAN -> Russian Band
> 	5) V4L2_TUNER_CAP_BAND_TYPE_WEATHER -> Weather Band
> 
> Signed-off-by: Manjunatha Halli <x0130808@ti.com>
> ---
>  drivers/media/video/v4l2-ctrls.c |   17 ++++++++++++++---
>  include/linux/videodev2.h        |   24 +++++++++++++++++++++++-
>  2 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index c9c9a46..7b3dd95 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h

...

> @@ -1819,6 +1827,12 @@ 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_TYPE_DEFAULT	0x00000000	/* Default band */
> +#define V4L2_TUNER_CAP_BAND_TYPE_EUROPE_US	0x00010000	/* Europe/US band */
> +#define V4L2_TUNER_CAP_BAND_TYPE_JAPAN		0x00020000	/* Japan band */
> +#define V4L2_TUNER_CAP_BAND_TYPE_RUSSIAN	0x00030000	/* Russian band */
> +#define V4L2_TUNER_CAP_BAND_TYPE_WEATHER	0x00040000	/* Weather band */

Argh! These capabilities are wrong: these should have been 0x10000, 0x20000,
0x40000 and 0x80000! Bits that you can OR together.

Also, I think _TYPE can be dropped here, just as we did for the V4L2_FM_BAND
defines below.

V4L2_TUNER_CAP_BAND_TYPE_DEFAULT is useless as a capability and should be
dropped.

Manju, I would recommend that you split off the frequency band handling from
the other patches. Based on an RFC from Hans de Goede regarding work on the AM
band I believe we need to postpone this part for 3.6. The other patches in this
series not related to frequency bands are fine and you can keep my Acked-by
there.

Mauro, if you intended to merge Manjunatha's patches for 3.5, then please wait
with merging them until he had a chance to split off the frequency band bits.

Regards,

	Hans

> +
>  
>  /*  Flags for the 'rxsubchans' field */
>  #define V4L2_TUNER_SUB_MONO		0x0001
> @@ -1843,13 +1857,21 @@ struct v4l2_frequency {
>  	__u32		      reserved[8];
>  };
>  
> +
> +#define V4L2_FM_BAND_DEFAULT	0
> +#define V4L2_FM_BAND_EUROPE_US	1	/* 87.5 Mhz - 108 MHz */
> +#define V4L2_FM_BAND_JAPAN	2	/* 76 MHz - 90 MHz */
> +#define V4L2_FM_BAND_RUSSIAN	3	/* 65.8 MHz - 74 MHz */
> +#define V4L2_FM_BAND_WEATHER	4	/* 162.4 MHz - 162.55 MHz */
> +
>  struct v4l2_hw_freq_seek {
>  	__u32		      tuner;
>  	enum v4l2_tuner_type  type;
>  	__u32		      seek_upward;
>  	__u32		      wrap_around;
>  	__u32		      spacing;
> -	__u32		      reserved[7];
> +	__u32		      band;
> +	__u32		      reserved[6];
>  };
>  
>  /*
> 

Re: [PATCH V6 2/5] New control class and features for FM RX

[halli manjunatha]

On Sun, May 20, 2012 at 4:52 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On Tue May 15 2012 00:01:50 manjunatha_halli@ti.com wrote:
>> From: Manjunatha Halli <x0130808@ti.com>
>>
>> This patch creates new ctrl class for FM RX and adds new CID's for
>> below FM features,
>>       1) De-Emphasis filter mode
>>       2) RDS Alternate Frequency switch
>>
>> Also this patch adds a field for band selection in struct v4l2_hw_freq_seek
>> and adds new capability flags for all below FM bands
>>       1) V4L2_TUNER_CAP_BAND_TYPE_DEFAULT -> Default Band
>>       2) V4L2_TUNER_CAP_BAND_TYPE_EUROPE_US -> Europe/US Band
>>       3) V4L2_TUNER_CAP_BAND_TYPE_JAPAN   -> Japan Band
>>       4) V4L2_TUNER_CAP_BAND_TYPE_RUSSIAN -> Russian Band
>>       5) V4L2_TUNER_CAP_BAND_TYPE_WEATHER -> Weather Band
>>
>> Signed-off-by: Manjunatha Halli <x0130808@ti.com>
>> ---
>>  drivers/media/video/v4l2-ctrls.c |   17 ++++++++++++++---
>>  include/linux/videodev2.h        |   24 +++++++++++++++++++++++-
>>  2 files changed, 37 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>> index c9c9a46..7b3dd95 100644
>> --- a/include/linux/videodev2.h
>> +++ b/include/linux/videodev2.h
>
> ...
>
>> @@ -1819,6 +1827,12 @@ 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_TYPE_DEFAULT     0x00000000      /* Default band */
>> +#define V4L2_TUNER_CAP_BAND_TYPE_EUROPE_US   0x00010000      /* Europe/US band */
>> +#define V4L2_TUNER_CAP_BAND_TYPE_JAPAN               0x00020000      /* Japan band */
>> +#define V4L2_TUNER_CAP_BAND_TYPE_RUSSIAN     0x00030000      /* Russian band */
>> +#define V4L2_TUNER_CAP_BAND_TYPE_WEATHER     0x00040000      /* Weather band */
>
> Argh! These capabilities are wrong: these should have been 0x10000, 0x20000,
> 0x40000 and 0x80000! Bits that you can OR together.
>
> Also, I think _TYPE can be dropped here, just as we did for the V4L2_FM_BAND
> defines below.
>
> V4L2_TUNER_CAP_BAND_TYPE_DEFAULT is useless as a capability and should be
> dropped.
>
> Manju, I would recommend that you split off the frequency band handling from
> the other patches. Based on an RFC from Hans de Goede regarding work on the AM
> band I believe we need to postpone this part for 3.6. The other patches in this
> series not related to frequency bands are fine and you can keep my Acked-by
> there.
>
> Mauro, if you intended to merge Manjunatha's patches for 3.5, then please wait
> with merging them until he had a chance to split off the frequency band bits.
>
> Regards,
>
>        Hans

Hans, Now I have sent the patch set V7 without the frequency handling part.
I will send the frequency handling patches once we come to a
conclusion for AM and FM
bands with Hans de Goede :)

>
>> +
>>
>>  /*  Flags for the 'rxsubchans' field */
>>  #define V4L2_TUNER_SUB_MONO          0x0001
>> @@ -1843,13 +1857,21 @@ struct v4l2_frequency {
>>       __u32                 reserved[8];
>>  };
>>
>> +
>> +#define V4L2_FM_BAND_DEFAULT 0
>> +#define V4L2_FM_BAND_EUROPE_US       1       /* 87.5 Mhz - 108 MHz */
>> +#define V4L2_FM_BAND_JAPAN   2       /* 76 MHz - 90 MHz */
>> +#define V4L2_FM_BAND_RUSSIAN 3       /* 65.8 MHz - 74 MHz */
>> +#define V4L2_FM_BAND_WEATHER 4       /* 162.4 MHz - 162.55 MHz */
>> +
>>  struct v4l2_hw_freq_seek {
>>       __u32                 tuner;
>>       enum v4l2_tuner_type  type;
>>       __u32                 seek_upward;
>>       __u32                 wrap_around;
>>       __u32                 spacing;
>> -     __u32                 reserved[7];
>> +     __u32                 band;
>> +     __u32                 reserved[6];
>>  };
>>
>>  /*
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Regards
Halli

Discussion: How to deal with radio tuners which can tune to multiple bands

[Hans de Goede]

Hi,

As discussed before 2 different use-cases have come up where we want some
knowledge of there being different radio bands added to the v4l2 API.

In Manjunatha Halli's case, if I understand things correctly, he wants
to limit hw_freq_seek to a certain band, rather then the receiver happily
trying to seek to frequencies which are not relevant for the use case in
question. To that purpose his patch proposes adding a band field to the
v4l2_hw_freq_seek struct, which can have one of the following values:

FM_BAND_TYPE_ALL	All Bands from 65.8 MHz till 108 Mhz or 162.55 MHz if weather band
FM_BAND_TYPE_EUROPE_US	Europe or US band(87.5 Mhz - 108 MHz)
FM_BAND_TYPE_JAPAN	Japan band(76 MHz - 90 MHz)
FM_BAND_TYPE_RUSSIAN	OIRT or Russian band(65.8 MHz - 74 MHz)
FM_BAND_TYPE_WEATHER	Weather band(162.4 MHz - 162.55 MHz)

In my case the problem is that the TEA5757 tuner can tune both AM
and FM (but note at the same time, it is a single tuner):
AM  530   - 1710 kHz
FM   87.5 -  108 MHz

In my case part of the problem is that the userspace UI for the tuner
cannot simply depict the frequency range as one large slider.

For FM devices having a slider going from 65.8 - 162.55 MHz is
far from ideal, esp. as there is a large whole of nothing in the
108-162 Mhz making the non functional area of the sliders range
larger then the functional area.

For AM and FM capable devices representing the entire range
(530 Khz - 162.55 Mhz) is not just not ideal it is simply unworkable!

So we don't just want to limit the range a VIDIOC_S_HW_FREQ_SEEK can
seek over, we also want to let user space know for manual tuning
that we've several ranges, and allow it to query information
such as min / max freq, stereo capable, etc. per range.

As discussed with Hans V. this can best be done by extending
struct v4l2_tuner with a band field.

Our (Hans V. and me) first idea here was to let this field
work like an index, where userspace can enumerate available
bands by calling VIDIOC_G_TUNER, incrementing band each time until
-EINVAL gets returned.

So that completes the intro, also known as setting the stage :)

###

Taking the intersection between the 2 proposals and the 2 problems
makes things interesting :)

Dividing the VIDIOC_G_TUNER results into bands also makes sense for
the FM case, at least in a 65.8 - 108 Mhz and a  162 Mhz band, to
avoid having a not tunable gap in the range reported to userspace.

But, allowing a EU citizen to tune below 87.5 is also not really
useful, nor allowing a Japanese citizen to tune above the 90 Mhz, etc.

So from presenting the user with a sensible UI pov, it makes sense
to not use 2 bands with FM, but to expose all supported bands
to userspace as they really are. This also makes sense from a demod
pov, since FM demodulation for Japanese FM is different then for
EU/US FM, so maybe the hardware needs to be poked to switch modes.

Note that some radio chip drivers already do this effectively by having
a module parameter to select which band to use.

So lets expose all the FM bands from Manjunatha Halli's proposal
in VIDIOC_G_TUNER results. If we then go for the classic enumeration
strategy where userspace can enumerate available bands by calling
VIDIOC_G_TUNER, incrementing band each time until -EINVAL gets
returned, we get another problem...

How does user space know which band is which (other then checking
min/max frequency which is ugly!) ? We could make the tuner name field
different for each band, and let the app display a menu with tuner names
for the user to select a band, but that is ugly too.

Not only would doing something like that be ugly, it also makes it
as good as impossible for userspace to automatically select the
right band based on location.

So doing the classic v4l2 enum trick where we increment band each
time until we get -EINVAL is not a good idea IMHO.

Luckily Manjunatha Halli's proposal already gives us a solution,
we can define a fixed set of bands (adding SW/MW/LW bands for AM),
and userspace can enumerate by trying a G_TUNER for all
bands it is interested in.

###

So given all of the above I would like to propose the following:

1) Add a "band" field to struct v4l2_tuner, and a capability
    indicating if the driver understands / uses this field
2) This field is only valid for radio tuners, for tv tuners it
should always be 0 (as it was sofar as it is reserved atm)
3) This field can have a number of fixed values, for now we have:

0 RADIO_BAND_DEFAULT	Entire FM band supported by the tuner, or "default"
                         band if different bands require switching the tuner to
                         a different mode, or entire AM band supported by the
			tuner for AM only tuners.
1 RADIO_BAND_FM_EUROPE_US Europe or US band(87.5 Mhz - 108 MHz) *
2 RADIO_BAND_FM_JAPAN	Japan band(76 MHz - 90 MHz) *
3 RADIO_BAND_FM_RUSSIAN	OIRT or Russian band(65.8 MHz - 74 MHz) *
4 RADIO_BAND_FM_WEATHER	Weather band(162.4 MHz - 162.55 MHz) *

256 RADIO_BAND_AM_MW	Mid Wave AM band, covered frequencies are tuner dependent
257 RADIO_BAND_AM_LW	Long Wave AM band, covered frequencies are tuner dependent
258 RADIO_BAND_AM_SW	Short Wave AM band, covered frequencies are tuner dependent

*) Reported (and available) frequency range might be different based on hardware
capabilities

Notice how 0, which the current reserved field should be set to for old apps,
should always cover as much of FM as possible, or AM for AM only tuners, to
preserve functionality for old non band aware v4l2 radio apps.

A (radio) tuner should always support RADIO_BAND_DEFAULT

4) Apps can find out which bands are supported by doing a VIDIOC_G_TUNER
with band set to the desired value. If the passed band is not available
-EINVAL will be returned.

5) Apps can select the active band by doing a VIDIOC_S_TUNER with the band
field set to the desired band.

6) Doing a VIDIOC_S_FREQUENCY with a frequency which falls outside of the
current band will *not* result in an automatic band switch, instead the
passed frequency will be clamped to fit into the current band.

7) Doing a VIDIOC_S_HW_FREQ_SEEK will seek in the currently active band,
this matches existing behavior where the seek starts at the currently
active frequency.

I think / hope that covers everything we need. Suggestions ? Comments ?

Regards,

Hans

Re: Discussion: How to deal with radio tuners which can tune to multiple bands

[halli manjunatha]

On Wed, May 23, 2012 at 1:29 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> As discussed before 2 different use-cases have come up where we want some
> knowledge of there being different radio bands added to the v4l2 API.
>
> In Manjunatha Halli's case, if I understand things correctly, he wants
> to limit hw_freq_seek to a certain band, rather then the receiver happily
> trying to seek to frequencies which are not relevant for the use case in
> question. To that purpose his patch proposes adding a band field to the
> v4l2_hw_freq_seek struct, which can have one of the following values:
>
> FM_BAND_TYPE_ALL        All Bands from 65.8 MHz till 108 Mhz or 162.55 MHz
> if weather band
> FM_BAND_TYPE_EUROPE_US  Europe or US band(87.5 Mhz - 108 MHz)
> FM_BAND_TYPE_JAPAN      Japan band(76 MHz - 90 MHz)
> FM_BAND_TYPE_RUSSIAN    OIRT or Russian band(65.8 MHz - 74 MHz)
> FM_BAND_TYPE_WEATHER    Weather band(162.4 MHz - 162.55 MHz)
>
> In my case the problem is that the TEA5757 tuner can tune both AM
> and FM (but note at the same time, it is a single tuner):
> AM  530   - 1710 kHz
> FM   87.5 -  108 MHz
>
> In my case part of the problem is that the userspace UI for the tuner
> cannot simply depict the frequency range as one large slider.
>
> For FM devices having a slider going from 65.8 - 162.55 MHz is
> far from ideal, esp. as there is a large whole of nothing in the
> 108-162 Mhz making the non functional area of the sliders range
> larger then the functional area.
>
> For AM and FM capable devices representing the entire range
> (530 Khz - 162.55 Mhz) is not just not ideal it is simply unworkable!
>
> So we don't just want to limit the range a VIDIOC_S_HW_FREQ_SEEK can
> seek over, we also want to let user space know for manual tuning
> that we've several ranges, and allow it to query information
> such as min / max freq, stereo capable, etc. per range.
>
> As discussed with Hans V. this can best be done by extending
> struct v4l2_tuner with a band field.
>
> Our (Hans V. and me) first idea here was to let this field
> work like an index, where userspace can enumerate available
> bands by calling VIDIOC_G_TUNER, incrementing band each time until
> -EINVAL gets returned.
>
> So that completes the intro, also known as setting the stage :)
>
> ###
>
> Taking the intersection between the 2 proposals and the 2 problems
> makes things interesting :)
>
> Dividing the VIDIOC_G_TUNER results into bands also makes sense for
> the FM case, at least in a 65.8 - 108 Mhz and a  162 Mhz band, to
> avoid having a not tunable gap in the range reported to userspace.
>
> But, allowing a EU citizen to tune below 87.5 is also not really
> useful, nor allowing a Japanese citizen to tune above the 90 Mhz, etc.
>
> So from presenting the user with a sensible UI pov, it makes sense
> to not use 2 bands with FM, but to expose all supported bands
> to userspace as they really are. This also makes sense from a demod
> pov, since FM demodulation for Japanese FM is different then for
> EU/US FM, so maybe the hardware needs to be poked to switch modes.
>
> Note that some radio chip drivers already do this effectively by having
> a module parameter to select which band to use.
>
> So lets expose all the FM bands from Manjunatha Halli's proposal
> in VIDIOC_G_TUNER results. If we then go for the classic enumeration
> strategy where userspace can enumerate available bands by calling
> VIDIOC_G_TUNER, incrementing band each time until -EINVAL gets
> returned, we get another problem...
>
> How does user space know which band is which (other then checking
> min/max frequency which is ugly!) ? We could make the tuner name field
> different for each band, and let the app display a menu with tuner names
> for the user to select a band, but that is ugly too.
>
> Not only would doing something like that be ugly, it also makes it
> as good as impossible for userspace to automatically select the
> right band based on location.
>
> So doing the classic v4l2 enum trick where we increment band each
> time until we get -EINVAL is not a good idea IMHO.
>
> Luckily Manjunatha Halli's proposal already gives us a solution,
> we can define a fixed set of bands (adding SW/MW/LW bands for AM),
> and userspace can enumerate by trying a G_TUNER for all
> bands it is interested in.
>
> ###
>
> So given all of the above I would like to propose the following:
>
> 1) Add a "band" field to struct v4l2_tuner, and a capability
>   indicating if the driver understands / uses this field
> 2) This field is only valid for radio tuners, for tv tuners it
> should always be 0 (as it was sofar as it is reserved atm)
> 3) This field can have a number of fixed values, for now we have:
>
> 0 RADIO_BAND_DEFAULT    Entire FM band supported by the tuner, or "default"
>                        band if different bands require switching the tuner
> to
>                        a different mode, or entire AM band supported by the
>                        tuner for AM only tuners.
> 1 RADIO_BAND_FM_EUROPE_US Europe or US band(87.5 Mhz - 108 MHz) *
> 2 RADIO_BAND_FM_JAPAN   Japan band(76 MHz - 90 MHz) *
> 3 RADIO_BAND_FM_RUSSIAN OIRT or Russian band(65.8 MHz - 74 MHz) *
> 4 RADIO_BAND_FM_WEATHER Weather band(162.4 MHz - 162.55 MHz) *
>
> 256 RADIO_BAND_AM_MW    Mid Wave AM band, covered frequencies are tuner
> dependent
> 257 RADIO_BAND_AM_LW    Long Wave AM band, covered frequencies are tuner
> dependent
> 258 RADIO_BAND_AM_SW    Short Wave AM band, covered frequencies are tuner
> dependent

First - Here driver will add the list of Band which it supports in the
VIDEOC_G_TUNER capability flag
>
> *) Reported (and available) frequency range might be different based on
> hardware
> capabilities
>
> Notice how 0, which the current reserved field should be set to for old
> apps,
> should always cover as much of FM as possible, or AM for AM only tuners, to
> preserve functionality for old non band aware v4l2 radio apps.
>
> A (radio) tuner should always support RADIO_BAND_DEFAULT
>
> 4) Apps can find out which bands are supported by doing a VIDIOC_G_TUNER
> with band set to the desired value. If the passed band is not available
> -EINVAL will be returned.
Second - User Application do VIDEOC_G_TUNER and checks the struct
v4l2_tuner.capability for the list of bands supported by Driver.
>
> 5) Apps can select the active band by doing a VIDIOC_S_TUNER with the band
> field set to the desired band.
Third - Here App sets the chip  to the required band.
>
> 6) Doing a VIDIOC_S_FREQUENCY with a frequency which falls outside of the
> current band will *not* result in an automatic band switch, instead the
> passed frequency will be clamped to fit into the current band.
Fourth - As already chip is set to a band set_frequency for frequency
within the activated band will work.

But I think its better to report 'ERANGE' to the frequencies which are
out of band limit. So that UI app also knows that set frequency failed
and it will show the bar to the last frequency.
>
> 7) Doing a VIDIOC_S_HW_FREQ_SEEK will seek in the currently active band,
> this matches existing behavior where the seek starts at the currently
> active frequency.
I agree.
>
> I think / hope that covers everything we need. Suggestions ? Comments ?

Solution seems fine for me
>
> Regards,
>
> Hans



-- 
Regards
Halli

Re: Discussion: How to deal with radio tuners which can tune to multiple bands

[Hans de Goede]

Hi,

On 05/23/2012 09:24 PM, halli manjunatha wrote:
> On Wed, May 23, 2012 at 1:29 PM, Hans de Goede<hdegoede@redhat.com>  wrote:

< snip my super long proposal intro :) >

>> ###
>>
>> So given all of the above I would like to propose the following:
>>
>> 1) Add a "band" field to struct v4l2_tuner, and a capability
>>    indicating if the driver understands / uses this field
>> 2) This field is only valid for radio tuners, for tv tuners it
>> should always be 0 (as it was sofar as it is reserved atm)
>> 3) This field can have a number of fixed values, for now we have:
>>
>> 0 RADIO_BAND_DEFAULT    Entire FM band supported by the tuner, or "default"
>>                         band if different bands require switching the tuner
>> to
>>                         a different mode, or entire AM band supported by the
>>                         tuner for AM only tuners.
>> 1 RADIO_BAND_FM_EUROPE_US Europe or US band(87.5 Mhz - 108 MHz) *
>> 2 RADIO_BAND_FM_JAPAN   Japan band(76 MHz - 90 MHz) *
>> 3 RADIO_BAND_FM_RUSSIAN OIRT or Russian band(65.8 MHz - 74 MHz) *
>> 4 RADIO_BAND_FM_WEATHER Weather band(162.4 MHz - 162.55 MHz) *
>>
>> 256 RADIO_BAND_AM_MW    Mid Wave AM band, covered frequencies are tuner
>> dependent
>> 257 RADIO_BAND_AM_LW    Long Wave AM band, covered frequencies are tuner
>> dependent
>> 258 RADIO_BAND_AM_SW    Short Wave AM band, covered frequencies are tuner
>> dependent
>
> First - Here driver will add the list of Band which it supports in the
> VIDEOC_G_TUNER capability flag

We could do that, although then the indexes for the AM bands need to be
lowered. I'm not sure if we should do this though. It will save a number
of ioctl calls, but those are cheap (as long as they don't hit the hardware)
and they are only done during enumeration by the app (iow not repeatedly)

So we've a trade-off here between making it easier for the app (the app
has to do less ioctls and can just test capability bits) and between
being conservative with capability bits.

I've a slight preference for the just let the app call g_tuner a number
of times to find out about supported bands because we don't know what
sort of bands we will add in the future and if we get too many bands
we may run out of capability bits.

With that said I'm not against the capability bits approach, just explaining
my reasoning. If Hans V. says he prefers the capability bits too, I'm fine
with going that way.

>>
>> *) Reported (and available) frequency range might be different based on
>> hardware
>> capabilities
>>
>> Notice how 0, which the current reserved field should be set to for old
>> apps,
>> should always cover as much of FM as possible, or AM for AM only tuners, to
>> preserve functionality for old non band aware v4l2 radio apps.
>>
>> A (radio) tuner should always support RADIO_BAND_DEFAULT
>>
>> 4) Apps can find out which bands are supported by doing a VIDIOC_G_TUNER
>> with band set to the desired value. If the passed band is not available
>> -EINVAL will be returned.
> Second - User Application do VIDEOC_G_TUNER and checks the struct
> v4l2_tuner.capability for the list of bands supported by Driver.

See above :)

>>
>> 5) Apps can select the active band by doing a VIDIOC_S_TUNER with the band
>> field set to the desired band.
> Third - Here App sets the chip  to the required band.

Ack.

>> 6) Doing a VIDIOC_S_FREQUENCY with a frequency which falls outside of the
>> current band will *not* result in an automatic band switch, instead the
>> passed frequency will be clamped to fit into the current band.
> Fourth - As already chip is set to a band set_frequency for frequency
> within the activated band will work.
>
> But I think its better to report 'ERANGE' to the frequencies which are
> out of band limit. So that UI app also knows that set frequency failed
> and it will show the bar to the last frequency.

The spec currently states that the behavior for s_freq is to set the
frequency to the nearest supported frequency, rather then report an error,
so we're stuck with that. Also note that the driver may also change the
frequency even if it is inside the range, because the hardware may not
be able to do the exact requested frequency. So any good written app
should always do a g_freq after a s_freq and show the user what he
actually got.

>>
>> 7) Doing a VIDIOC_S_HW_FREQ_SEEK will seek in the currently active band,
>> this matches existing behavior where the seek starts at the currently
>> active frequency.
> I agree.
>>
>> I think / hope that covers everything we need. Suggestions ? Comments ?
>
> Solution seems fine for me

I'm happy to hear that!

Regards,

Hans

Re: Discussion: How to deal with radio tuners which can tune to multiple bands

[Hans Verkuil]

On Wed 23 May 2012 20:29:20 Hans de Goede wrote:
> Hi,
> 
> As discussed before 2 different use-cases have come up where we want some
> knowledge of there being different radio bands added to the v4l2 API.
> 
> In Manjunatha Halli's case, if I understand things correctly, he wants
> to limit hw_freq_seek to a certain band, rather then the receiver happily
> trying to seek to frequencies which are not relevant for the use case in
> question. To that purpose his patch proposes adding a band field to the
> v4l2_hw_freq_seek struct, which can have one of the following values:
> 
> FM_BAND_TYPE_ALL	All Bands from 65.8 MHz till 108 Mhz or 162.55 MHz if weather band

I suggested that this be renamed to FM_BAND_DEFAULT (and that the _TYPE part
of the names will be removed). The default band is usually the widest band
(often 76-108) that the hardware can handle.

> FM_BAND_TYPE_EUROPE_US	Europe or US band(87.5 Mhz - 108 MHz)
> FM_BAND_TYPE_JAPAN	Japan band(76 MHz - 90 MHz)
> FM_BAND_TYPE_RUSSIAN	OIRT or Russian band(65.8 MHz - 74 MHz)
> FM_BAND_TYPE_WEATHER	Weather band(162.4 MHz - 162.55 MHz)
> 
> In my case the problem is that the TEA5757 tuner can tune both AM
> and FM (but note at the same time, it is a single tuner):
> AM  530   - 1710 kHz
> FM   87.5 -  108 MHz
> 
> In my case part of the problem is that the userspace UI for the tuner
> cannot simply depict the frequency range as one large slider.
> 
> For FM devices having a slider going from 65.8 - 162.55 MHz is
> far from ideal, esp. as there is a large whole of nothing in the
> 108-162 Mhz making the non functional area of the sliders range
> larger then the functional area.
> 
> For AM and FM capable devices representing the entire range
> (530 Khz - 162.55 Mhz) is not just not ideal it is simply unworkable!
> 
> So we don't just want to limit the range a VIDIOC_S_HW_FREQ_SEEK can
> seek over, we also want to let user space know for manual tuning
> that we've several ranges, and allow it to query information
> such as min / max freq, stereo capable, etc. per range.
> 
> As discussed with Hans V. this can best be done by extending
> struct v4l2_tuner with a band field.
> 
> Our (Hans V. and me) first idea here was to let this field
> work like an index, where userspace can enumerate available
> bands by calling VIDIOC_G_TUNER, incrementing band each time until
> -EINVAL gets returned.

Not quite. The available bands are signaled through the v4l2_tuner capabilities
field, so the app will know which bands there are. So there is no need to
increment the band.

> So that completes the intro, also known as setting the stage :)
> 
> ###
> 
> Taking the intersection between the 2 proposals and the 2 problems
> makes things interesting :)
> 
> Dividing the VIDIOC_G_TUNER results into bands also makes sense for
> the FM case, at least in a 65.8 - 108 Mhz and a  162 Mhz band, to
> avoid having a not tunable gap in the range reported to userspace.
> 
> But, allowing a EU citizen to tune below 87.5 is also not really
> useful, nor allowing a Japanese citizen to tune above the 90 Mhz, etc.
> 
> So from presenting the user with a sensible UI pov, it makes sense
> to not use 2 bands with FM, but to expose all supported bands
> to userspace as they really are. This also makes sense from a demod
> pov, since FM demodulation for Japanese FM is different then for
> EU/US FM, so maybe the hardware needs to be poked to switch modes.

Right.

> Note that some radio chip drivers already do this effectively by having
> a module parameter to select which band to use.
> 
> So lets expose all the FM bands from Manjunatha Halli's proposal
> in VIDIOC_G_TUNER results. If we then go for the classic enumeration
> strategy where userspace can enumerate available bands by calling
> VIDIOC_G_TUNER, incrementing band each time until -EINVAL gets
> returned, we get another problem...
> 
> How does user space know which band is which (other then checking
> min/max frequency which is ugly!) ? We could make the tuner name field
> different for each band, and let the app display a menu with tuner names
> for the user to select a band, but that is ugly too.
> 
> Not only would doing something like that be ugly, it also makes it
> as good as impossible for userspace to automatically select the
> right band based on location.
> 
> So doing the classic v4l2 enum trick where we increment band each
> time until we get -EINVAL is not a good idea IMHO.
> 
> Luckily Manjunatha Halli's proposal already gives us a solution,
> we can define a fixed set of bands (adding SW/MW/LW bands for AM),
> and userspace can enumerate by trying a G_TUNER for all
> bands it is interested in.
> 
> ###
> 
> So given all of the above I would like to propose the following:
> 
> 1) Add a "band" field to struct v4l2_tuner, and a capability
>     indicating if the driver understands / uses this field
> 2) This field is only valid for radio tuners, for tv tuners it
> should always be 0 (as it was sofar as it is reserved atm)
> 3) This field can have a number of fixed values, for now we have:
> 
> 0 RADIO_BAND_DEFAULT	Entire FM band supported by the tuner, or "default"
>                          band if different bands require switching the tuner to
>                          a different mode, or entire AM band supported by the
> 			tuner for AM only tuners.
> 1 RADIO_BAND_FM_EUROPE_US Europe or US band(87.5 Mhz - 108 MHz) *
> 2 RADIO_BAND_FM_JAPAN	Japan band(76 MHz - 90 MHz) *
> 3 RADIO_BAND_FM_RUSSIAN	OIRT or Russian band(65.8 MHz - 74 MHz) *
> 4 RADIO_BAND_FM_WEATHER	Weather band(162.4 MHz - 162.55 MHz) *
> 
> 256 RADIO_BAND_AM_MW	Mid Wave AM band, covered frequencies are tuner dependent
> 257 RADIO_BAND_AM_LW	Long Wave AM band, covered frequencies are tuner dependent
> 258 RADIO_BAND_AM_SW	Short Wave AM band, covered frequencies are tuner dependent

I wouldn't add LW and SW as long as we don't have hardware that supports it.

> 
> *) Reported (and available) frequency range might be different based on hardware
> capabilities
> 
> Notice how 0, which the current reserved field should be set to for old apps,
> should always cover as much of FM as possible, or AM for AM only tuners, to
> preserve functionality for old non band aware v4l2 radio apps.
> 
> A (radio) tuner should always support RADIO_BAND_DEFAULT
> 
> 4) Apps can find out which bands are supported by doing a VIDIOC_G_TUNER
> with band set to the desired value. If the passed band is not available
> -EINVAL will be returned.

I would propose to add capability flags signaling the presence of each bands.
There are 24 bits available, and the number of bands is very limited. I see
no problem here.

This way an application doesn't need to cycle through all possible bands, but
it can select one immediately.

> 
> 5) Apps can select the active band by doing a VIDIOC_S_TUNER with the band
> field set to the desired band.

OK. Note that the current frequency will have to be clamped to the new band.

> 6) Doing a VIDIOC_S_FREQUENCY with a frequency which falls outside of the
> current band will *not* result in an automatic band switch, instead the
> passed frequency will be clamped to fit into the current band.
 
OK.

> 7) Doing a VIDIOC_S_HW_FREQ_SEEK will seek in the currently active band,
> this matches existing behavior where the seek starts at the currently
> active frequency.

Sounds good. Then we don't need to add a band field here as was in Halli's
first proposal.

> I think / hope that covers everything we need. Suggestions ? Comments ?

Modulators. v4l2_modulator needs a band field as well. The capabilities are
already shared with v4l2_tuner, so that doesn't need to change.

Other than that it looks OK to me.

Regards,

	Hans

Re: Discussion: How to deal with radio tuners which can tune to multiple bands

[Hans de Goede]

Hi,

On 05/24/2012 05:00 PM, Hans Verkuil wrote:
> On Wed 23 May 2012 20:29:20 Hans de Goede wrote:

<snip>

>> ###
>>
>> So given all of the above I would like to propose the following:
>>
>> 1) Add a "band" field to struct v4l2_tuner, and a capability
>>      indicating if the driver understands / uses this field
>> 2) This field is only valid for radio tuners, for tv tuners it
>> should always be 0 (as it was sofar as it is reserved atm)
>> 3) This field can have a number of fixed values, for now we have:
>>
>> 0 RADIO_BAND_DEFAULT	Entire FM band supported by the tuner, or "default"
>>                           band if different bands require switching the tuner to
>>                           a different mode, or entire AM band supported by the
>> 			tuner for AM only tuners.
>> 1 RADIO_BAND_FM_EUROPE_US Europe or US band(87.5 Mhz - 108 MHz) *
>> 2 RADIO_BAND_FM_JAPAN	Japan band(76 MHz - 90 MHz) *
>> 3 RADIO_BAND_FM_RUSSIAN	OIRT or Russian band(65.8 MHz - 74 MHz) *
>> 4 RADIO_BAND_FM_WEATHER	Weather band(162.4 MHz - 162.55 MHz) *
>>
>> 256 RADIO_BAND_AM_MW	Mid Wave AM band, covered frequencies are tuner dependent
>> 257 RADIO_BAND_AM_LW	Long Wave AM band, covered frequencies are tuner dependent
>> 258 RADIO_BAND_AM_SW	Short Wave AM band, covered frequencies are tuner dependent
>
> I wouldn't add LW and SW as long as we don't have hardware that supports it.

Ok.

>>
>> *) Reported (and available) frequency range might be different based on hardware
>> capabilities
>>
>> Notice how 0, which the current reserved field should be set to for old apps,
>> should always cover as much of FM as possible, or AM for AM only tuners, to
>> preserve functionality for old non band aware v4l2 radio apps.
>>
>> A (radio) tuner should always support RADIO_BAND_DEFAULT
>>
>> 4) Apps can find out which bands are supported by doing a VIDIOC_G_TUNER
>> with band set to the desired value. If the passed band is not available
>> -EINVAL will be returned.
>
> I would propose to add capability flags signaling the presence of each bands.
> There are 24 bits available, and the number of bands is very limited. I see
> no problem here.
>
> This way an application doesn't need to cycle through all possible bands, but
> it can select one immediately.

Ok so that is 2 votes for using capability bits, so lets go with that solution
rather then requiring the app to do a g_tuner with all possible bands to
find out which bands are available.

>> 5) Apps can select the active band by doing a VIDIOC_S_TUNER with the band
>> field set to the desired band.
>
> OK. Note that the current frequency will have to be clamped to the new band.
>
>> 6) Doing a VIDIOC_S_FREQUENCY with a frequency which falls outside of the
>> current band will *not* result in an automatic band switch, instead the
>> passed frequency will be clamped to fit into the current band.
>
> OK.
>
>> 7) Doing a VIDIOC_S_HW_FREQ_SEEK will seek in the currently active band,
>> this matches existing behavior where the seek starts at the currently
>> active frequency.
>
> Sounds good. Then we don't need to add a band field here as was in Halli's
> first proposal.
>
>> I think / hope that covers everything we need. Suggestions ? Comments ?
>
> Modulators. v4l2_modulator needs a band field as well. The capabilities are
> already shared with v4l2_tuner, so that doesn't need to change.

Ah, yes modulators, good one, ack.

Manjunatha, since the final proposal is close to yours, and you already have
a patch for that including all the necessary documentation updates, can I ask
you to update your patch to implement this proposal?

I must admit another reason is that I don't really have a lot of time to work
on this atm, and it would be good to get this finalized soon, so that we will
be ready well in advance of the 3.6 cycle start :)

Thanks & Regards,

Hans

Re: Discussion: How to deal with radio tuners which can tune to multiple bands

[halli manjunatha]

On Thu, May 24, 2012 at 2:12 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 05/24/2012 05:00 PM, Hans Verkuil wrote:
>>
>> On Wed 23 May 2012 20:29:20 Hans de Goede wrote:
>
>
> <snip>
>
>
>>> ###
>>>
>>> So given all of the above I would like to propose the following:
>>>
>>> 1) Add a "band" field to struct v4l2_tuner, and a capability
>>>     indicating if the driver understands / uses this field
>>> 2) This field is only valid for radio tuners, for tv tuners it
>>> should always be 0 (as it was sofar as it is reserved atm)
>>> 3) This field can have a number of fixed values, for now we have:
>>>
>>> 0 RADIO_BAND_DEFAULT    Entire FM band supported by the tuner, or
>>> "default"
>>>                          band if different bands require switching the
>>> tuner to
>>>                          a different mode, or entire AM band supported by
>>> the
>>>                        tuner for AM only tuners.
>>> 1 RADIO_BAND_FM_EUROPE_US Europe or US band(87.5 Mhz - 108 MHz) *
>>> 2 RADIO_BAND_FM_JAPAN   Japan band(76 MHz - 90 MHz) *
>>> 3 RADIO_BAND_FM_RUSSIAN OIRT or Russian band(65.8 MHz - 74 MHz) *
>>> 4 RADIO_BAND_FM_WEATHER Weather band(162.4 MHz - 162.55 MHz) *
>>>
>>> 256 RADIO_BAND_AM_MW    Mid Wave AM band, covered frequencies are tuner
>>> dependent
>>> 257 RADIO_BAND_AM_LW    Long Wave AM band, covered frequencies are tuner
>>> dependent
>>> 258 RADIO_BAND_AM_SW    Short Wave AM band, covered frequencies are tuner
>>> dependent
>>
>>
>> I wouldn't add LW and SW as long as we don't have hardware that supports
>> it.
>
>
> Ok.
>
>
>>>
>>> *) Reported (and available) frequency range might be different based on
>>> hardware
>>> capabilities
>>>
>>> Notice how 0, which the current reserved field should be set to for old
>>> apps,
>>> should always cover as much of FM as possible, or AM for AM only tuners,
>>> to
>>> preserve functionality for old non band aware v4l2 radio apps.
>>>
>>> A (radio) tuner should always support RADIO_BAND_DEFAULT
>>>
>>> 4) Apps can find out which bands are supported by doing a VIDIOC_G_TUNER
>>> with band set to the desired value. If the passed band is not available
>>> -EINVAL will be returned.
>>
>>
>> I would propose to add capability flags signaling the presence of each
>> bands.
>> There are 24 bits available, and the number of bands is very limited. I
>> see
>> no problem here.
>>
>> This way an application doesn't need to cycle through all possible bands,
>> but
>> it can select one immediately.
>
>
> Ok so that is 2 votes for using capability bits, so lets go with that
> solution
> rather then requiring the app to do a g_tuner with all possible bands to
> find out which bands are available.
>
>
>>> 5) Apps can select the active band by doing a VIDIOC_S_TUNER with the
>>> band
>>> field set to the desired band.
>>
>>
>> OK. Note that the current frequency will have to be clamped to the new
>> band.
>>
>>> 6) Doing a VIDIOC_S_FREQUENCY with a frequency which falls outside of the
>>> current band will *not* result in an automatic band switch, instead the
>>> passed frequency will be clamped to fit into the current band.
>>
>>
>> OK.
>>
>>> 7) Doing a VIDIOC_S_HW_FREQ_SEEK will seek in the currently active band,
>>> this matches existing behavior where the seek starts at the currently
>>> active frequency.
>>
>>
>> Sounds good. Then we don't need to add a band field here as was in Halli's
>> first proposal.
>>
>>> I think / hope that covers everything we need. Suggestions ? Comments ?
>>
>>
>> Modulators. v4l2_modulator needs a band field as well. The capabilities
>> are
>> already shared with v4l2_tuner, so that doesn't need to change.
>
>
> Ah, yes modulators, good one, ack.
>
> Manjunatha, since the final proposal is close to yours, and you already have
> a patch for that including all the necessary documentation updates, can I
> ask
> you to update your patch to implement this proposal?
>
> I must admit another reason is that I don't really have a lot of time to
> work
> on this atm, and it would be good to get this finalized soon, so that we
> will
> be ready well in advance of the 3.6 cycle start :)

Sure... I will implement this proposal and send the patches :)
>
> Thanks & Regards,
>
> Hans



-- 
Regards
Halli

Re: Discussion: How to deal with radio tuners which can tune to multiple bands

[Hans de Goede]

Hi,

On 05/24/2012 09:12 PM, Hans de Goede wrote:
> Hi,
>
> On 05/24/2012 05:00 PM, Hans Verkuil wrote:
>>> I think / hope that covers everything we need. Suggestions ? Comments ?
>>
>> Modulators. v4l2_modulator needs a band field as well. The capabilities are
>> already shared with v4l2_tuner, so that doesn't need to change.
>
> Ah, yes modulators, good one, ack.
>
> Manjunatha, since the final proposal is close to yours, and you already have
> a patch for that including all the necessary documentation updates, can I ask
> you to update your patch to implement this proposal?
>

So I've been working a bit on adding AM support to the tea575x driver using
the agreed upon API, some observations from this:

1) There is no way to get which band is currently active

This is IMHO a big problem, a GUI radio app will quite likely when it starts get
all the current settings and display them without modifying any settings, so
it needs a way to find out which band is active.

2) What if first a band aware radio app sets a non default band, and then a
non band aware radio app comes along, it does a g_tuner on the default-band,
using the lo / high freq-s to build its UI, then the users picks a frequency,
and the app does a s_freq, and the result is a frequency outside of what the
app thinks are the lo / high freq limits because a different band is active ->
not good.

So I think we need to slightly modify the proposal, esp. to deal with 1), 2)
is a corner case and not really all that important on its own IMHO.

I suggest fixing 1) by not only adding a band field to v4l2_tuner, so that
the different ranges for different bands can be queried, but also adding
a band field to v4l2_frequency, so that the current active band can be
reported by g_frequency. Once we make g_frequency report the active band,
it makes sense to make s_frequency set the active band.

We would then need no changes to s_tuner at all, it will still only have
audmode as writeable setting and thus *not* set the active band. Effectively
s_tuner would just completely ignore the passed in band. Keeping audmode as
a global (not band specific) setting, and likewise g_tuner would always
return CAP_STEREO stereo if some bands are stereo capable.

This also nicely fixes 2). Since the reserved fields should be 0, so a
s_frequency by a non band aware app will set the band to the default band.

###

So here is a new / amended version of my band proposal:

1) Introduce the concept of bands, for radio tuners only

2) Define the following bands:

#define V4L2_BAND_DEFAULT       0
#define V4L2_BAND_FM_EUROPE_US  1       /* 87.5 Mhz - 108 MHz */
#define V4L2_BAND_FM_JAPAN      2       /* 76 MHz - 90 MHz */
#define V4L2_BAND_FM_RUSSIAN    3       /* 65.8 MHz - 74 MHz */
#define V4L2_BAND_FM_WEATHER    4       /* 162.4 MHz - 162.55 MHz */
#define V4L2_BAND_AM_MW         5

3) radio tuners indicate if they understand any of the non default bands
with the following tuner caps:

#define V4L2_TUNER_CAP_BAND_FM_EUROPE_US        0x00010000
#define V4L2_TUNER_CAP_BAND_FM_JAPAN    	0x00020000
#define V4L2_TUNER_CAP_BAND_FM_RUSSIAN  	0x00040000
#define V4L2_TUNER_CAP_BAND_FM_WEATHER  	0x00080000
#define V4L2_TUNER_CAP_BAND_AM_MW       	0x00100000

A (radio) tuner should always support RADIO_BAND_DEFAULT, so there is no
capability flag for this

4) Add a band field to v4l2_tuner, apps can query the exact rangelow and
rangehigh values for a specific band by doing a g_tuner with band set to
that band. All v4l2_tuner fields returned by g_tuner will be independent
of the selected band (iow constant) except for: rangelow, rangehigh,
rxsubchans and signal.
4a) rangelow, rangehigh will be the actual values for that band
4b) rxsubchans and signal will be 0 if a g_tuner is done for a band different
then the active band, for the active band they will reflect the actual values.

5) s_tuner will be completely unchanged, the band field will not influence
it, audmode will be a per tuner global, not a per band value

6) Add a band field to v4l2_frequency, on a g_frequency this will reflect the
current band, on a s_frequency this will set the current band

7) Doing a VIDIOC_S_HW_FREQ_SEEK will seek in the currently active band,
iow the band last set by a s_frequency call, this matches existing behavior where
the seek starts at the currently active frequency (so the frequency set by the
last s_frequency call, or the frequency from the last seek).

Regards,

Hans

Re: Discussion: How to deal with radio tuners which can tune to multiple bands

[Hans Verkuil]

On Sat May 26 2012 18:02:34 Hans de Goede wrote:
> Hi,
> 
> On 05/24/2012 09:12 PM, Hans de Goede wrote:
> > Hi,
> >
> > On 05/24/2012 05:00 PM, Hans Verkuil wrote:
> >>> I think / hope that covers everything we need. Suggestions ? Comments ?
> >>
> >> Modulators. v4l2_modulator needs a band field as well. The capabilities are
> >> already shared with v4l2_tuner, so that doesn't need to change.
> >
> > Ah, yes modulators, good one, ack.
> >
> > Manjunatha, since the final proposal is close to yours, and you already have
> > a patch for that including all the necessary documentation updates, can I ask
> > you to update your patch to implement this proposal?
> >
> 
> So I've been working a bit on adding AM support to the tea575x driver using
> the agreed upon API, some observations from this:
> 
> 1) There is no way to get which band is currently active

Huh? Didn't G_TUNER return the current band? That's how I interpreted the
proposal. G_TUNER returns the available bands in capabilities and the current
band and its frequency range. You want to find the frequency range of another
band you call have to call S_TUNER first to select that other band, and then
G_TUNER to discover its range.

That also solves case 2. No need for an extra band in v4l2_frequency.

> This is IMHO a big problem, a GUI radio app will quite likely when it starts get
> all the current settings and display them without modifying any settings, so
> it needs a way to find out which band is active.
> 
> 2) What if first a band aware radio app sets a non default band, and then a
> non band aware radio app comes along, it does a g_tuner on the default-band,
> using the lo / high freq-s to build its UI, then the users picks a frequency,
> and the app does a s_freq, and the result is a frequency outside of what the
> app thinks are the lo / high freq limits because a different band is active ->
> not good.
> 
> So I think we need to slightly modify the proposal, esp. to deal with 1), 2)
> is a corner case and not really all that important on its own IMHO.
> 
> I suggest fixing 1) by not only adding a band field to v4l2_tuner, so that
> the different ranges for different bands can be queried, but also adding
> a band field to v4l2_frequency, so that the current active band can be
> reported by g_frequency. Once we make g_frequency report the active band,
> it makes sense to make s_frequency set the active band.

I don't really like this. You run into the same weird situation with G_TUNER
that I did (as that was one of my ideas as well) where you set band to e.g.
the weather band and get back the corresponding frequency range, but the other
fields like signal and rxsubchans still refer to the *current* band. That's
confusing and not logical.

> We would then need no changes to s_tuner at all, it will still only have
> audmode as writeable setting and thus *not* set the active band. Effectively
> s_tuner would just completely ignore the passed in band. Keeping audmode as
> a global (not band specific) setting, and likewise g_tuner would always
> return CAP_STEREO stereo if some bands are stereo capable.
> 
> This also nicely fixes 2). Since the reserved fields should be 0, so a
> s_frequency by a non band aware app will set the band to the default band.
> 
> ###
> 
> So here is a new / amended version of my band proposal:
> 
> 1) Introduce the concept of bands, for radio tuners only
> 
> 2) Define the following bands:
> 
> #define V4L2_BAND_DEFAULT       0
> #define V4L2_BAND_FM_EUROPE_US  1       /* 87.5 Mhz - 108 MHz */
> #define V4L2_BAND_FM_JAPAN      2       /* 76 MHz - 90 MHz */
> #define V4L2_BAND_FM_RUSSIAN    3       /* 65.8 MHz - 74 MHz */
> #define V4L2_BAND_FM_WEATHER    4       /* 162.4 MHz - 162.55 MHz */
> #define V4L2_BAND_AM_MW         5
> 
> 3) radio tuners indicate if they understand any of the non default bands
> with the following tuner caps:
> 
> #define V4L2_TUNER_CAP_BAND_FM_EUROPE_US        0x00010000
> #define V4L2_TUNER_CAP_BAND_FM_JAPAN    	0x00020000
> #define V4L2_TUNER_CAP_BAND_FM_RUSSIAN  	0x00040000
> #define V4L2_TUNER_CAP_BAND_FM_WEATHER  	0x00080000
> #define V4L2_TUNER_CAP_BAND_AM_MW       	0x00100000
> 
> A (radio) tuner should always support RADIO_BAND_DEFAULT, so there is no
> capability flag for this
> 
> 4) Add a band field to v4l2_tuner, apps can query the exact rangelow and
> rangehigh values for a specific band by doing a g_tuner with band set to
> that band. All v4l2_tuner fields returned by g_tuner will be independent
> of the selected band (iow constant) except for: rangelow, rangehigh,
> rxsubchans and signal.
> 4a) rangelow, rangehigh will be the actual values for that band
> 4b) rxsubchans and signal will be 0 if a g_tuner is done for a band different
> then the active band, for the active band they will reflect the actual values.

So I would do this as:

4) Add a band field to v4l2_tuner. Calling g_tuner will set this to the
current band. You change it by calling s_tuner. CAP_STEREO and audmode are
global properties, not per-band. CAP_STEREO really refers to whether the
hardware can do stereo at all.

> 5) s_tuner will be completely unchanged, the band field will not influence
> it, audmode will be a per tuner global, not a per band value

Drop this.

> 6) Add a band field to v4l2_frequency, on a g_frequency this will reflect the
> current band, on a s_frequency this will set the current band

Drop this.

> 7) Doing a VIDIOC_S_HW_FREQ_SEEK will seek in the currently active band,
> iow the band last set by a s_frequency call, this matches existing behavior where
> the seek starts at the currently active frequency (so the frequency set by the
> last s_frequency call, or the frequency from the last seek).

5) Doing a VIDIOC_S_HW_FREQ_SEEK will seek in the currently active band,
iow the band last set by a s_tuner call.

Two fewer items on this list :-)

Regards,

	Hans

Re: Discussion: How to deal with radio tuners which can tune to multiple bands

[Hans de Goede]

Hi,

On 05/26/2012 06:40 PM, Hans Verkuil wrote:
> On Sat May 26 2012 18:02:34 Hans de Goede wrote:
>> Hi,
>>
>> On 05/24/2012 09:12 PM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 05/24/2012 05:00 PM, Hans Verkuil wrote:
>>>>> I think / hope that covers everything we need. Suggestions ? Comments ?
>>>>
>>>> Modulators. v4l2_modulator needs a band field as well. The capabilities are
>>>> already shared with v4l2_tuner, so that doesn't need to change.
>>>
>>> Ah, yes modulators, good one, ack.
>>>
>>> Manjunatha, since the final proposal is close to yours, and you already have
>>> a patch for that including all the necessary documentation updates, can I ask
>>> you to update your patch to implement this proposal?
>>>
>>
>> So I've been working a bit on adding AM support to the tea575x driver using
>> the agreed upon API, some observations from this:
>>
>> 1) There is no way to get which band is currently active
>
> Huh? Didn't G_TUNER return the current band? That's how I interpreted the
> proposal. G_TUNER returns the available bands in capabilities and the current
> band and its frequency range. You want to find the frequency range of another
> band you call have to call S_TUNER first to select that other band, and then
> G_TUNER to discover its range.

Ah, we misunderstood each other there, I thought G_TUNER would honor the band
passed in and return info on that band.

> That also solves case 2. No need for an extra band in v4l2_frequency.

Right, the downside to this is that there is no way to just enumerate things
without actually changing anything. It would be nice if for example v4l2-ctl
could lists all bands including ranges as a 100% read-only operation.

Note I'm ok with the way you propose to handle things, just pointing out
one (obvious) shortcoming of doing things this way. I think it is a short coming
we can live with, but we should be aware of it.

>> This is IMHO a big problem, a GUI radio app will quite likely when it starts get
>> all the current settings and display them without modifying any settings, so
>> it needs a way to find out which band is active.
>>
>> 2) What if first a band aware radio app sets a non default band, and then a
>> non band aware radio app comes along, it does a g_tuner on the default-band,
>> using the lo / high freq-s to build its UI, then the users picks a frequency,
>> and the app does a s_freq, and the result is a frequency outside of what the
>> app thinks are the lo / high freq limits because a different band is active ->
>> not good.
>>
>> So I think we need to slightly modify the proposal, esp. to deal with 1), 2)
>> is a corner case and not really all that important on its own IMHO.
>>
>> I suggest fixing 1) by not only adding a band field to v4l2_tuner, so that
>> the different ranges for different bands can be queried, but also adding
>> a band field to v4l2_frequency, so that the current active band can be
>> reported by g_frequency. Once we make g_frequency report the active band,
>> it makes sense to make s_frequency set the active band.
>
> I don't really like this. You run into the same weird situation with G_TUNER
> that I did (as that was one of my ideas as well) where you set band to e.g.
> the weather band and get back the corresponding frequency range, but the other
> fields like signal and rxsubchans still refer to the *current* band. That's
> confusing and not logical.

Right, notice how my proposal says that signal and rxsubchans will always
report 0 except when doing a g_tuner for the active band. This is not ideal,
but IMHO better then not being able to do read-only enumeration of the bands.

>> We would then need no changes to s_tuner at all, it will still only have
>> audmode as writeable setting and thus *not* set the active band. Effectively
>> s_tuner would just completely ignore the passed in band. Keeping audmode as
>> a global (not band specific) setting, and likewise g_tuner would always
>> return CAP_STEREO stereo if some bands are stereo capable.
>>
>> This also nicely fixes 2). Since the reserved fields should be 0, so a
>> s_frequency by a non band aware app will set the band to the default band. >>
>> ###
>>
>> So here is a new / amended version of my band proposal:
>>
>> 1) Introduce the concept of bands, for radio tuners only
>>
>> 2) Define the following bands:
>>
>> #define V4L2_BAND_DEFAULT       0
>> #define V4L2_BAND_FM_EUROPE_US  1       /* 87.5 Mhz - 108 MHz */
>> #define V4L2_BAND_FM_JAPAN      2       /* 76 MHz - 90 MHz */
>> #define V4L2_BAND_FM_RUSSIAN    3       /* 65.8 MHz - 74 MHz */
>> #define V4L2_BAND_FM_WEATHER    4       /* 162.4 MHz - 162.55 MHz */
>> #define V4L2_BAND_AM_MW         5
>>
>> 3) radio tuners indicate if they understand any of the non default bands
>> with the following tuner caps:
>>
>> #define V4L2_TUNER_CAP_BAND_FM_EUROPE_US        0x00010000
>> #define V4L2_TUNER_CAP_BAND_FM_JAPAN    	0x00020000
>> #define V4L2_TUNER_CAP_BAND_FM_RUSSIAN  	0x00040000
>> #define V4L2_TUNER_CAP_BAND_FM_WEATHER  	0x00080000
>> #define V4L2_TUNER_CAP_BAND_AM_MW       	0x00100000
>>
>> A (radio) tuner should always support RADIO_BAND_DEFAULT, so there is no
>> capability flag for this
>>
>> 4) Add a band field to v4l2_tuner, apps can query the exact rangelow and
>> rangehigh values for a specific band by doing a g_tuner with band set to
>> that band. All v4l2_tuner fields returned by g_tuner will be independent
>> of the selected band (iow constant) except for: rangelow, rangehigh,
>> rxsubchans and signal.
>> 4a) rangelow, rangehigh will be the actual values for that band
>> 4b) rxsubchans and signal will be 0 if a g_tuner is done for a band different
>> then the active band, for the active band they will reflect the actual values.
>
> So I would do this as:
>
> 4) Add a band field to v4l2_tuner. Calling g_tuner will set this to the
> current band. You change it by calling s_tuner. CAP_STEREO and audmode are
> global properties, not per-band. CAP_STEREO really refers to whether the
> hardware can do stereo at all.

Right, I can see the logic in this, and I think it is an elegant solution in
a way. But as said that looses read only enumeration of the band ranges, which
I think is something which we should allow...

I'm happy that we agree that CAP_STEREO and audmode shoul be global and not per
band :)

>> 5) s_tuner will be completely unchanged, the band field will not influence
>> it, audmode will be a per tuner global, not a per band value
>
> Drop this.
>
>> 6) Add a band field to v4l2_frequency, on a g_frequency this will reflect the
>> current band, on a s_frequency this will set the current band
>
> Drop this.
>
>> 7) Doing a VIDIOC_S_HW_FREQ_SEEK will seek in the currently active band,
>> iow the band last set by a s_frequency call, this matches existing behavior where
>> the seek starts at the currently active frequency (so the frequency set by the
>> last s_frequency call, or the frequency from the last seek).
>
> 5) Doing a VIDIOC_S_HW_FREQ_SEEK will seek in the currently active band,
> iow the band last set by a s_tuner call.
>
> Two fewer items on this list :-)

He he, I'm ok with either way, but I still have a slight preference to adding
a band field to v4l2_frequency because of the enumeration issue.

Regards,

Hans

Re: Discussion: How to deal with radio tuners which can tune to multiple bands

[Hans Verkuil]

On Sat May 26 2012 20:09:53 Hans de Goede wrote:
> Hi,
> 
> On 05/26/2012 06:40 PM, Hans Verkuil wrote:
> > On Sat May 26 2012 18:02:34 Hans de Goede wrote:
> >> Hi,
> >>
> >> On 05/24/2012 09:12 PM, Hans de Goede wrote:
> >>> Hi,
> >>>
> >>> On 05/24/2012 05:00 PM, Hans Verkuil wrote:
> >>>>> I think / hope that covers everything we need. Suggestions ? Comments ?
> >>>>
> >>>> Modulators. v4l2_modulator needs a band field as well. The capabilities are
> >>>> already shared with v4l2_tuner, so that doesn't need to change.
> >>>
> >>> Ah, yes modulators, good one, ack.
> >>>
> >>> Manjunatha, since the final proposal is close to yours, and you already have
> >>> a patch for that including all the necessary documentation updates, can I ask
> >>> you to update your patch to implement this proposal?
> >>>
> >>
> >> So I've been working a bit on adding AM support to the tea575x driver using
> >> the agreed upon API, some observations from this:
> >>
> >> 1) There is no way to get which band is currently active
> >
> > Huh? Didn't G_TUNER return the current band? That's how I interpreted the
> > proposal. G_TUNER returns the available bands in capabilities and the current
> > band and its frequency range. You want to find the frequency range of another
> > band you call have to call S_TUNER first to select that other band, and then
> > G_TUNER to discover its range.
> 
> Ah, we misunderstood each other there, I thought G_TUNER would honor the band
> passed in and return info on that band.
> 
> > That also solves case 2. No need for an extra band in v4l2_frequency.
> 
> Right, the downside to this is that there is no way to just enumerate things
> without actually changing anything. It would be nice if for example v4l2-ctl
> could lists all bands including ranges as a 100% read-only operation.
> 
> Note I'm ok with the way you propose to handle things, just pointing out
> one (obvious) shortcoming of doing things this way. I think it is a short coming
> we can live with, but we should be aware of it.

Well, the important thing is that you know what bands are supported, and you
know because of the capabilities. I don't really think you have to query the
exact range of other than the current band: the band's name is already a good
indication of what the range will be, you just don't know the exact boundaries
until you switch to that band. Which I believe is perfectly reasonable.

I had the same idea originally as you, but then I realized that it only made
the API confusing and that you really don't need to know the precise range as
long as you know which bands are supported.

Regards,

	Hans

Re: Discussion: How to deal with radio tuners which can tune to multiple bands

[Hans de Goede]

Hi,

On 05/26/2012 08:22 PM, Hans Verkuil wrote:
> On Sat May 26 2012 20:09:53 Hans de Goede wrote:
>> Hi,
>>
>> On 05/26/2012 06:40 PM, Hans Verkuil wrote:
>>> On Sat May 26 2012 18:02:34 Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 05/24/2012 09:12 PM, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 05/24/2012 05:00 PM, Hans Verkuil wrote:
>>>>>>> I think / hope that covers everything we need. Suggestions ? Comments ?
>>>>>>
>>>>>> Modulators. v4l2_modulator needs a band field as well. The capabilities are
>>>>>> already shared with v4l2_tuner, so that doesn't need to change.
>>>>>
>>>>> Ah, yes modulators, good one, ack.
>>>>>
>>>>> Manjunatha, since the final proposal is close to yours, and you already have
>>>>> a patch for that including all the necessary documentation updates, can I ask
>>>>> you to update your patch to implement this proposal?
>>>>>
>>>>
>>>> So I've been working a bit on adding AM support to the tea575x driver using
>>>> the agreed upon API, some observations from this:
>>>>
>>>> 1) There is no way to get which band is currently active
>>>
>>> Huh? Didn't G_TUNER return the current band? That's how I interpreted the
>>> proposal. G_TUNER returns the available bands in capabilities and the current
>>> band and its frequency range. You want to find the frequency range of another
>>> band you call have to call S_TUNER first to select that other band, and then
>>> G_TUNER to discover its range.
>>
>> Ah, we misunderstood each other there, I thought G_TUNER would honor the band
>> passed in and return info on that band.
>>
>>> That also solves case 2. No need for an extra band in v4l2_frequency.
>>
>> Right, the downside to this is that there is no way to just enumerate things
>> without actually changing anything. It would be nice if for example v4l2-ctl
>> could lists all bands including ranges as a 100% read-only operation.
>>
>> Note I'm ok with the way you propose to handle things, just pointing out
>> one (obvious) shortcoming of doing things this way. I think it is a short coming
>> we can live with, but we should be aware of it.
>
> Well, the important thing is that you know what bands are supported, and you
> know because of the capabilities.

I agree.

> I don't really think you have to query the
> exact range of other than the current band: the band's name is already a good
> indication of what the range will be, you just don't know the exact boundaries
> until you switch to that band. Which I believe is perfectly reasonable.

I agree, so lets go with your proposal / my original proposal as interpreted by
you :)

Regards,

Hans

Re: Discussion: How to deal with radio tuners which can tune to multiple bands

[Hans Verkuil]

Just a warning to avoid double work: I'll post a patch with all the needed
changes tomorrow (or Monday at the latest). That will also include changes
from my RFC on improving the hardware seek.

Regards,

	Hans

On Sat May 26 2012 20:38:52 Hans de Goede wrote:
> Hi,
> 
> On 05/26/2012 08:22 PM, Hans Verkuil wrote:
> > On Sat May 26 2012 20:09:53 Hans de Goede wrote:
> >> Hi,
> >>
> >> On 05/26/2012 06:40 PM, Hans Verkuil wrote:
> >>> On Sat May 26 2012 18:02:34 Hans de Goede wrote:
> >>>> Hi,
> >>>>
> >>>> On 05/24/2012 09:12 PM, Hans de Goede wrote:
> >>>>> Hi,
> >>>>>
> >>>>> On 05/24/2012 05:00 PM, Hans Verkuil wrote:
> >>>>>>> I think / hope that covers everything we need. Suggestions ? Comments ?
> >>>>>>
> >>>>>> Modulators. v4l2_modulator needs a band field as well. The capabilities are
> >>>>>> already shared with v4l2_tuner, so that doesn't need to change.
> >>>>>
> >>>>> Ah, yes modulators, good one, ack.
> >>>>>
> >>>>> Manjunatha, since the final proposal is close to yours, and you already have
> >>>>> a patch for that including all the necessary documentation updates, can I ask
> >>>>> you to update your patch to implement this proposal?
> >>>>>
> >>>>
> >>>> So I've been working a bit on adding AM support to the tea575x driver using
> >>>> the agreed upon API, some observations from this:
> >>>>
> >>>> 1) There is no way to get which band is currently active
> >>>
> >>> Huh? Didn't G_TUNER return the current band? That's how I interpreted the
> >>> proposal. G_TUNER returns the available bands in capabilities and the current
> >>> band and its frequency range. You want to find the frequency range of another
> >>> band you call have to call S_TUNER first to select that other band, and then
> >>> G_TUNER to discover its range.
> >>
> >> Ah, we misunderstood each other there, I thought G_TUNER would honor the band
> >> passed in and return info on that band.
> >>
> >>> That also solves case 2. No need for an extra band in v4l2_frequency.
> >>
> >> Right, the downside to this is that there is no way to just enumerate things
> >> without actually changing anything. It would be nice if for example v4l2-ctl
> >> could lists all bands including ranges as a 100% read-only operation.
> >>
> >> Note I'm ok with the way you propose to handle things, just pointing out
> >> one (obvious) shortcoming of doing things this way. I think it is a short coming
> >> we can live with, but we should be aware of it.
> >
> > Well, the important thing is that you know what bands are supported, and you
> > know because of the capabilities.
> 
> I agree.
> 
> > I don't really think you have to query the
> > exact range of other than the current band: the band's name is already a good
> > indication of what the range will be, you just don't know the exact boundaries
> > until you switch to that band. Which I believe is perfectly reasonable.
> 
> I agree, so lets go with your proposal / my original proposal as interpreted by
> you :)
> 
> Regards,
> 
> Hans
> 

Re: Discussion: How to deal with radio tuners which can tune to multiple bands

[Hans de Goede]

Hi,

Just noticed the following on:
http://linuxtv.org/downloads/v4l-dvb-apis/tuner.html#id2570531

"This specification does not define radio output devices.", iow no
radio modulators, but we agreed upon making the band changes to
the modulator too, and this makes sense because AFAIK we do support
radio modulators.

I hit this while working on adding support for the proposed API to
v4l2-ctl, as I wanted to only print the band stuff for radio type
devices, but the modulator struct has no type!

Regards,

Hans

Re: Discussion: How to deal with radio tuners which can tune to multiple bands

[Hans Verkuil]

On Sun May 27 2012 11:06:27 Hans de Goede wrote:
> Hi,
> 
> Just noticed the following on:
> http://linuxtv.org/downloads/v4l-dvb-apis/tuner.html#id2570531
> 
> "This specification does not define radio output devices.", iow no
> radio modulators, but we agreed upon making the band changes to
> the modulator too, and this makes sense because AFAIK we do support
> radio modulators.
> 
> I hit this while working on adding support for the proposed API to
> v4l2-ctl, as I wanted to only print the band stuff for radio type
> devices, but the modulator struct has no type!
> 
> Regards,
> 
> Hans
> 

Yeah, that section needs to be rewritten. We have no TV modulators at all.
If a video/vbi node supports G/S_MODULATOR, then it is a TV modulator. If
a radio node supports it, then it is a radio modulator.

Even for g/s_freq and g/s_tuner the type field is now determined strictly
by the type of the node. It is no longer possible to e.g. query the radio
tuner from a video node. It used to be possible (as least in theory) in the
past, but that was crazy.

Regards,

	Hans