[PATCH v21 0/3] ASoC/MFD/V4L2: WL1273 FM Radio Driver
Hi. And thanks for the comment. Samuel wrote: Remove two unnecessary calls to i2c_set_clientdata. Provided that you add a changelog relevant to the patch itself, and not to the v1-v2 diff: Replaced the incremental changelog stuff with the original descriptions. Acked-by: Samuel Ortiz sa...@linux.intel.com Cheers, Matti Matti J. Aaltonen (3): MFD: WL1273 FM Radio: MFD driver for the FM radio. V4L2: WL1273 FM Radio: TI WL1273 FM radio driver ASoC: WL1273 FM radio: Access I2C IO functions through pointers. drivers/media/radio/radio-wl1273.c | 360 +++- drivers/mfd/Kconfig|2 +- drivers/mfd/wl1273-core.c | 149 +++- include/linux/mfd/wl1273-core.h|2 + sound/soc/codecs/Kconfig |2 +- sound/soc/codecs/wl1273.c | 11 +- 6 files changed, 264 insertions(+), 262 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v21 2/3] V4L2: WL1273 FM Radio: TI WL1273 FM radio driver
This module implements V4L2 controls for the Texas Instruments
WL1273 FM Radio and handles the communication with the chip.
Signed-off-by: Matti J. Aaltonen matti.j.aalto...@nokia.com
---
drivers/media/radio/radio-wl1273.c | 360 +++-
1 files changed, 106 insertions(+), 254 deletions(-)
diff --git a/drivers/media/radio/radio-wl1273.c
b/drivers/media/radio/radio-wl1273.c
index 7ecc8e6..9e177dc 100644
--- a/drivers/media/radio/radio-wl1273.c
+++ b/drivers/media/radio/radio-wl1273.c
@@ -1,7 +1,7 @@
/*
* Driver for the Texas Instruments WL1273 FM radio.
*
- * Copyright (C) 2010 Nokia Corporation
+ * Copyright (C) 2011 Nokia Corporation
* Author: Matti J. Aaltonen matti.j.aalto...@nokia.com
*
* This program is free software; you can redistribute it and/or
@@ -104,58 +104,6 @@ static unsigned int rds_buf = 100;
module_param(rds_buf, uint, 0);
MODULE_PARM_DESC(rds_buf, Number of RDS buffer entries. Default = 100);
-static int wl1273_fm_read_reg(struct wl1273_core *core, u8 reg, u16 *value)
-{
- struct i2c_client *client = core-client;
- u8 b[2];
- int r;
-
- r = i2c_smbus_read_i2c_block_data(client, reg, sizeof(b), b);
- if (r != 2) {
- dev_err(client-dev, %s: Read: %d fails.\n, __func__, reg);
- return -EREMOTEIO;
- }
-
- *value = (u16)b[0] 8 | b[1];
-
- return 0;
-}
-
-static int wl1273_fm_write_cmd(struct wl1273_core *core, u8 cmd, u16 param)
-{
- struct i2c_client *client = core-client;
- u8 buf[] = { (param 8) 0xff, param 0xff };
- int r;
-
- r = i2c_smbus_write_i2c_block_data(client, cmd, sizeof(buf), buf);
- if (r) {
- dev_err(client-dev, %s: Cmd: %d fails.\n, __func__, cmd);
- return r;
- }
-
- return 0;
-}
-
-static int wl1273_fm_write_data(struct wl1273_core *core, u8 *data, u16 len)
-{
- struct i2c_client *client = core-client;
- struct i2c_msg msg;
- int r;
-
- msg.addr = client-addr;
- msg.flags = 0;
- msg.buf = data;
- msg.len = len;
-
- r = i2c_transfer(client-adapter, msg, 1);
- if (r != 1) {
- dev_err(client-dev, %s: write error.\n, __func__);
- return -EREMOTEIO;
- }
-
- return 0;
-}
-
static int wl1273_fm_write_fw(struct wl1273_core *core,
__u8 *fw, int len)
{
@@ -188,94 +136,6 @@ static int wl1273_fm_write_fw(struct wl1273_core *core,
return r;
}
-/**
- * wl1273_fm_set_audio() - Set audio mode.
- * @core: A pointer to the device struct.
- * @new_mode: The new audio mode.
- *
- * Audio modes are WL1273_AUDIO_DIGITAL and WL1273_AUDIO_ANALOG.
- */
-static int wl1273_fm_set_audio(struct wl1273_core *core, unsigned int new_mode)
-{
- int r = 0;
-
- if (core-mode == WL1273_MODE_OFF ||
- core-mode == WL1273_MODE_SUSPENDED)
- return -EPERM;
-
- if (core-mode == WL1273_MODE_RX new_mode == WL1273_AUDIO_DIGITAL) {
- r = wl1273_fm_write_cmd(core, WL1273_PCM_MODE_SET,
- WL1273_PCM_DEF_MODE);
- if (r)
- goto out;
-
- r = wl1273_fm_write_cmd(core, WL1273_I2S_MODE_CONFIG_SET,
- core-i2s_mode);
- if (r)
- goto out;
-
- r = wl1273_fm_write_cmd(core, WL1273_AUDIO_ENABLE,
- WL1273_AUDIO_ENABLE_I2S);
- if (r)
- goto out;
-
- } else if (core-mode == WL1273_MODE_RX
- new_mode == WL1273_AUDIO_ANALOG) {
- r = wl1273_fm_write_cmd(core, WL1273_AUDIO_ENABLE,
- WL1273_AUDIO_ENABLE_ANALOG);
- if (r)
- goto out;
-
- } else if (core-mode == WL1273_MODE_TX
- new_mode == WL1273_AUDIO_DIGITAL) {
- r = wl1273_fm_write_cmd(core, WL1273_I2S_MODE_CONFIG_SET,
- core-i2s_mode);
- if (r)
- goto out;
-
- r = wl1273_fm_write_cmd(core, WL1273_AUDIO_IO_SET,
- WL1273_AUDIO_IO_SET_I2S);
- if (r)
- goto out;
-
- } else if (core-mode == WL1273_MODE_TX
- new_mode == WL1273_AUDIO_ANALOG) {
- r = wl1273_fm_write_cmd(core, WL1273_AUDIO_IO_SET,
- WL1273_AUDIO_IO_SET_ANALOG);
- if (r)
- goto out;
- }
-
- core-audio_mode = new_mode;
-out:
- return r;
-}
-
-/**
- * wl1273_fm_set_volume() -Set volume.
- * @core: A pointer to the device struct.
- * @volume:The new volume value.
- */
-static int
[PATCH v22 0/3] ASoC/MFD/V4L2: WL1273 FM Radio Driver
Hello. Thanks for the comment Mark. On Tue, 2011-03-01 at 11:54 +, ext Mark Brown wrote: On Tue, Mar 01, 2011 at 10:00:50AM +0200, Matti J. Aaltonen wrote: These changes are needed to keep up with the changes in the MFD core and V4L2 parts of the wl1273 FM radio driver. Use function pointers instead of exported functions for I2C IO. Also move all preprocessor constants from the wl1273.h to include/linux/mfd/wl1273-core.h. Also update the year in the copyright statement. It's not actually doing that: - * Copyright: (C) 2010 Nokia Corporation + * Copyright: (C) 2011 Nokia Corporation It's replacing it - portions are still 2010. Kept also the year 2010 on the copyright line. Acked-by: Mark Brown broo...@opensource.wolfsonmicro.com On Tue, 2011-03-01 at 12:43 +0100, ext Samuel Ortiz wrote: Acked-by: Samuel Ortiz sa...@linux.intel.com Cheers, Matti Matti J. Aaltonen (3): MFD: WL1273 FM Radio: MFD driver for the FM radio. V4L2: WL1273 FM Radio: TI WL1273 FM radio driver ASoC: WL1273 FM radio: Access I2C IO functions through pointers. drivers/media/radio/radio-wl1273.c | 360 +++- drivers/mfd/Kconfig|2 +- drivers/mfd/wl1273-core.c | 149 +++- include/linux/mfd/wl1273-core.h|2 + sound/soc/codecs/Kconfig |2 +- sound/soc/codecs/wl1273.c | 11 +- 6 files changed, 264 insertions(+), 262 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v22 2/3] V4L2: WL1273 FM Radio: TI WL1273 FM radio driver
This module implements V4L2 controls for the Texas Instruments
WL1273 FM Radio and handles the communication with the chip.
Signed-off-by: Matti J. Aaltonen matti.j.aalto...@nokia.com
---
drivers/media/radio/radio-wl1273.c | 360 +++-
1 files changed, 106 insertions(+), 254 deletions(-)
diff --git a/drivers/media/radio/radio-wl1273.c
b/drivers/media/radio/radio-wl1273.c
index 7ecc8e6..9e177dc 100644
--- a/drivers/media/radio/radio-wl1273.c
+++ b/drivers/media/radio/radio-wl1273.c
@@ -1,7 +1,7 @@
/*
* Driver for the Texas Instruments WL1273 FM radio.
*
- * Copyright (C) 2010 Nokia Corporation
+ * Copyright (C) 2011 Nokia Corporation
* Author: Matti J. Aaltonen matti.j.aalto...@nokia.com
*
* This program is free software; you can redistribute it and/or
@@ -104,58 +104,6 @@ static unsigned int rds_buf = 100;
module_param(rds_buf, uint, 0);
MODULE_PARM_DESC(rds_buf, Number of RDS buffer entries. Default = 100);
-static int wl1273_fm_read_reg(struct wl1273_core *core, u8 reg, u16 *value)
-{
- struct i2c_client *client = core-client;
- u8 b[2];
- int r;
-
- r = i2c_smbus_read_i2c_block_data(client, reg, sizeof(b), b);
- if (r != 2) {
- dev_err(client-dev, %s: Read: %d fails.\n, __func__, reg);
- return -EREMOTEIO;
- }
-
- *value = (u16)b[0] 8 | b[1];
-
- return 0;
-}
-
-static int wl1273_fm_write_cmd(struct wl1273_core *core, u8 cmd, u16 param)
-{
- struct i2c_client *client = core-client;
- u8 buf[] = { (param 8) 0xff, param 0xff };
- int r;
-
- r = i2c_smbus_write_i2c_block_data(client, cmd, sizeof(buf), buf);
- if (r) {
- dev_err(client-dev, %s: Cmd: %d fails.\n, __func__, cmd);
- return r;
- }
-
- return 0;
-}
-
-static int wl1273_fm_write_data(struct wl1273_core *core, u8 *data, u16 len)
-{
- struct i2c_client *client = core-client;
- struct i2c_msg msg;
- int r;
-
- msg.addr = client-addr;
- msg.flags = 0;
- msg.buf = data;
- msg.len = len;
-
- r = i2c_transfer(client-adapter, msg, 1);
- if (r != 1) {
- dev_err(client-dev, %s: write error.\n, __func__);
- return -EREMOTEIO;
- }
-
- return 0;
-}
-
static int wl1273_fm_write_fw(struct wl1273_core *core,
__u8 *fw, int len)
{
@@ -188,94 +136,6 @@ static int wl1273_fm_write_fw(struct wl1273_core *core,
return r;
}
-/**
- * wl1273_fm_set_audio() - Set audio mode.
- * @core: A pointer to the device struct.
- * @new_mode: The new audio mode.
- *
- * Audio modes are WL1273_AUDIO_DIGITAL and WL1273_AUDIO_ANALOG.
- */
-static int wl1273_fm_set_audio(struct wl1273_core *core, unsigned int new_mode)
-{
- int r = 0;
-
- if (core-mode == WL1273_MODE_OFF ||
- core-mode == WL1273_MODE_SUSPENDED)
- return -EPERM;
-
- if (core-mode == WL1273_MODE_RX new_mode == WL1273_AUDIO_DIGITAL) {
- r = wl1273_fm_write_cmd(core, WL1273_PCM_MODE_SET,
- WL1273_PCM_DEF_MODE);
- if (r)
- goto out;
-
- r = wl1273_fm_write_cmd(core, WL1273_I2S_MODE_CONFIG_SET,
- core-i2s_mode);
- if (r)
- goto out;
-
- r = wl1273_fm_write_cmd(core, WL1273_AUDIO_ENABLE,
- WL1273_AUDIO_ENABLE_I2S);
- if (r)
- goto out;
-
- } else if (core-mode == WL1273_MODE_RX
- new_mode == WL1273_AUDIO_ANALOG) {
- r = wl1273_fm_write_cmd(core, WL1273_AUDIO_ENABLE,
- WL1273_AUDIO_ENABLE_ANALOG);
- if (r)
- goto out;
-
- } else if (core-mode == WL1273_MODE_TX
- new_mode == WL1273_AUDIO_DIGITAL) {
- r = wl1273_fm_write_cmd(core, WL1273_I2S_MODE_CONFIG_SET,
- core-i2s_mode);
- if (r)
- goto out;
-
- r = wl1273_fm_write_cmd(core, WL1273_AUDIO_IO_SET,
- WL1273_AUDIO_IO_SET_I2S);
- if (r)
- goto out;
-
- } else if (core-mode == WL1273_MODE_TX
- new_mode == WL1273_AUDIO_ANALOG) {
- r = wl1273_fm_write_cmd(core, WL1273_AUDIO_IO_SET,
- WL1273_AUDIO_IO_SET_ANALOG);
- if (r)
- goto out;
- }
-
- core-audio_mode = new_mode;
-out:
- return r;
-}
-
-/**
- * wl1273_fm_set_volume() -Set volume.
- * @core: A pointer to the device struct.
- * @volume:The new volume value.
- */
-static int
[PATCH v20 0/3] ASoC/MFD/V4L2: WL1273 FM Radio driver
Hello. And thanks for the comments. If we now move quickly enough we can get this thing in before it becomes deprecated... Samuel wrote: for Mauro to take this one you'd have to provide a diff against the already existing wl1273-core. I've made these patches against the existing stuff. + * Copyright (C) 2010 Nokia Corporation 2011. Changed. +} I'm confused with this one: Isn't WL1273_VOLUME_SET a command ? Also, how can reading from it set the volume ? It cannot... so I've changed it (back) to write. + i2c_set_clientdata(client, NULL) Not needed. Removed. +err: + i2c_set_clientdata(client, NULL); Ditto. Ditto. Cheers, Matti Matti J. Aaltonen (3): MFD: Wl1273 FM radio core: Add I2C IO functions. V4L2: Wl1273 FM Radio: Remove I2C IO functions. ASoC: WL1273 FM radio: Access I2C IO functions through pointers. drivers/media/radio/radio-wl1273.c | 360 +++- drivers/mfd/Kconfig|2 +- drivers/mfd/wl1273-core.c | 149 +++- include/linux/mfd/wl1273-core.h|2 + sound/soc/codecs/Kconfig |2 +- sound/soc/codecs/wl1273.c | 11 +- 6 files changed, 264 insertions(+), 262 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v19 0/3] TI Wl1273 FM radio driver.
Hi Mauro, On Tue, Feb 15, 2011 at 05:59:52PM -0200, Mauro Carvalho Chehab wrote: Em 15-02-2011 06:13, Matti J. Aaltonen escreveu: Hello. Now I've refactored the code so that the I2C I/O functions are in the MFD core. Also now the codec can be compiled without compiling the V4L2 driver. I haven't implemented the audio routing (yet), but I've added a TODO comment about it in the codec file. Matti, It looks ok on my eyes. As most of the changes is at the V4L part, it is probably better to merge this patch via my tree. I'm fine with that, yes. I'll add my Acked-by once Matti has fixed the minor issues I found. Cheers, Samuel. -- Intel Open Source Technology Centre http://oss.intel.com/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v19 0/3] TI Wl1273 FM radio driver.
Hello. Now I've refactored the code so that the I2C I/O functions are in the MFD core. Also now the codec can be compiled without compiling the V4L2 driver. I haven't implemented the audio routing (yet), but I've added a TODO comment about it in the codec file. Cheers, Matti Matti J. Aaltonen (3): MFD: WL1273 FM Radio: MFD driver for the FM radio. V4L2: WL1273 FM Radio: TI WL1273 FM radio driver ASoC: WL1273 FM radio: Access I2C IO functions through pointers. drivers/media/radio/Kconfig| 16 + drivers/media/radio/Makefile |1 + drivers/media/radio/radio-wl1273.c | 2183 drivers/mfd/Kconfig| 10 + drivers/mfd/Makefile |1 + drivers/mfd/wl1273-core.c | 295 + include/linux/mfd/wl1273-core.h| 291 + sound/soc/codecs/Kconfig |2 +- sound/soc/codecs/wl1273.c | 38 +- sound/soc/codecs/wl1273.h | 71 -- 10 files changed, 2817 insertions(+), 91 deletions(-) create mode 100644 drivers/media/radio/radio-wl1273.c create mode 100644 drivers/mfd/wl1273-core.c create mode 100644 include/linux/mfd/wl1273-core.h -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v19 2/3] V4L2: WL1273 FM Radio: TI WL1273 FM radio driver
This module implements V4L2 controls for the Texas Instruments
WL1273 FM Radio and handles the communication with the chip.
Signed-off-by: Matti J. Aaltonen matti.j.aalto...@nokia.com
---
drivers/media/radio/Kconfig| 16 +
drivers/media/radio/Makefile |1 +
drivers/media/radio/radio-wl1273.c | 2183
3 files changed, 2200 insertions(+), 0 deletions(-)
create mode 100644 drivers/media/radio/radio-wl1273.c
diff --git a/drivers/media/radio/Kconfig b/drivers/media/radio/Kconfig
index 83567b8..3c5a473 100644
--- a/drivers/media/radio/Kconfig
+++ b/drivers/media/radio/Kconfig
@@ -452,4 +452,20 @@ config RADIO_TIMBERDALE
found behind the Timberdale FPGA on the Russellville board.
Enabling this driver will automatically select the DSP and tuner.
+config RADIO_WL1273
+ tristate Texas Instruments WL1273 I2C FM Radio
+ depends on I2C VIDEO_V4L2
+ select MFD_WL1273_CORE
+ select FW_LOADER
+ ---help---
+ Choose Y here if you have this FM radio chip.
+
+ In order to control your radio card, you will need to use programs
+ that are compatible with the Video For Linux 2 API. Information on
+ this API and pointers to v4l2 programs may be found at
+ file:Documentation/video4linux/API.html.
+
+ To compile this driver as a module, choose M here: the
+ module will be called radio-wl1273.
+
endif # RADIO_ADAPTERS
diff --git a/drivers/media/radio/Makefile b/drivers/media/radio/Makefile
index f615583..d297074 100644
--- a/drivers/media/radio/Makefile
+++ b/drivers/media/radio/Makefile
@@ -26,5 +26,6 @@ obj-$(CONFIG_RADIO_TEA5764) += radio-tea5764.o
obj-$(CONFIG_RADIO_SAA7706H) += saa7706h.o
obj-$(CONFIG_RADIO_TEF6862) += tef6862.o
obj-$(CONFIG_RADIO_TIMBERDALE) += radio-timb.o
+obj-$(CONFIG_RADIO_WL1273) += radio-wl1273.o
EXTRA_CFLAGS += -Isound
diff --git a/drivers/media/radio/radio-wl1273.c
b/drivers/media/radio/radio-wl1273.c
new file mode 100644
index 000..37a7cc7
--- /dev/null
+++ b/drivers/media/radio/radio-wl1273.c
@@ -0,0 +1,2183 @@
+/*
+ * Driver for the Texas Instruments WL1273 FM radio.
+ *
+ * Copyright (C) 2010 Nokia Corporation
+ * Author: Matti J. Aaltonen matti.j.aalto...@nokia.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include linux/delay.h
+#include linux/firmware.h
+#include linux/interrupt.h
+#include linux/mfd/wl1273-core.h
+#include linux/slab.h
+#include media/v4l2-common.h
+#include media/v4l2-ctrls.h
+#include media/v4l2-device.h
+#include media/v4l2-ioctl.h
+
+#define DRIVER_DESC Wl1273 FM Radio
+
+#define WL1273_POWER_SET_OFF 0
+#define WL1273_POWER_SET_FMBIT(0)
+#define WL1273_POWER_SET_RDS BIT(1)
+#define WL1273_POWER_SET_RETENTION BIT(4)
+
+#define WL1273_PUPD_SET_OFF0x00
+#define WL1273_PUPD_SET_ON 0x01
+#define WL1273_PUPD_SET_RETENTION 0x10
+
+#define WL1273_FREQ(x) (x * 1 / 625)
+#define WL1273_INV_FREQ(x) (x * 625 / 1)
+
+/*
+ * static int radio_nr - The number of the radio device
+ *
+ * The default is 0.
+ */
+static int radio_nr;
+module_param(radio_nr, int, 0);
+MODULE_PARM_DESC(radio_nr, The number of the radio device. Default = 0);
+
+struct wl1273_device {
+ char *bus_type;
+
+ u8 forbidden;
+ unsigned int preemphasis;
+ unsigned int spacing;
+ unsigned int tx_power;
+ unsigned int rx_frequency;
+ unsigned int tx_frequency;
+ unsigned int rangelow;
+ unsigned int rangehigh;
+ unsigned int band;
+ bool stereo;
+
+ /* RDS */
+ unsigned int rds_on;
+ struct delayed_work work;
+
+ wait_queue_head_t read_queue;
+ struct mutex lock; /* for serializing fm radio operations */
+ struct completion busy;
+
+ unsigned char *buffer;
+ unsigned int buf_size;
+ unsigned int rd_index;
+ unsigned int wr_index;
+
+ /* Selected interrupts */
+ u16 irq_flags;
+ u16 irq_received;
+
+ struct v4l2_ctrl_handler ctrl_handler;
+ struct v4l2_device v4l2dev;
+ struct video_device videodev;
+ struct device *dev;
+ struct wl1273_core *core;
+ struct file *owner;
+ char *write_buf;
+ unsigned int rds_users;
+};
+
+#define WL1273_IRQ_MASK
Re: [PATCH v19 0/3] TI Wl1273 FM radio driver.
Em 15-02-2011 06:13, Matti J. Aaltonen escreveu: Hello. Now I've refactored the code so that the I2C I/O functions are in the MFD core. Also now the codec can be compiled without compiling the V4L2 driver. I haven't implemented the audio routing (yet), but I've added a TODO comment about it in the codec file. Matti, It looks ok on my eyes. As most of the changes is at the V4L part, it is probably better to merge this patch via my tree. Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [alsa-devel] WL1273 FM Radio driver...
On Tue, 2011-02-08 at 13:02 +0200, Peter Ujfalusi wrote: For both configuration we have a set of HCI commands to configure the FM audio path and one of my concerns is to know if the wl1273_codec should handle the audio path configuration and the switch between FM and BT SCO? It would be better if the codec could handle the configuration, depending on which DAI is in use. If we can send HCI commands from kernel, I think that would be the cleanest way. Yes, I would have done just that - and we talked a lot about it locally - if I had known how to do it. I started to work on it and also talked to some BT people but it didn't seem feasible at the time. Advice welcome of course... Cheers, Matti Hi, Below the set of HCI commands that we have identified to configure the FM and BT SCO audio paths: - External Audio Connection o START BT SCO Connection # BT audio Path PCMFM audio Path I2S hcitool cmd 0x3F 0X195 FF FF FF FF FF FF FF FF 01 02 FF 00 00 00 00 # BT AUDIO Codec Configuration: MASTER hcitool cmd 0x3F 0X106 00 03 00 40 1F 00 00 01 00 00 00 00 10 00 02 00 00 10 00 02 00 01 00 00 00 00 00 00 00 00 00 00 o STOP BT SCO Connection # BT AUDIO Codec Configuration: SLAVE hcitool cmd 0x3F 0X106 00 03 01 40 1F 00 00 01 00 00 00 00 10 00 02 00 00 10 00 02 00 01 00 00 00 00 00 00 00 00 00 00 - Internal Audio Connection o START BT SCO Connection # BT audio Path PCM # FM audio Path None hcitool cmd 0x3F 0X195 FF FF FF FF FF FF FF FF 01 00 FF 00 00 00 00 # BT AUDIO Codec Configuration: MASTER hcitool cmd 0x3F 0X106 00 03 00 40 1F 00 00 01 00 00 00 00 10 00 02 00 00 10 00 02 00 01 00 00 00 00 00 00 00 00 00 00 o STOP BT SCO Connection # BT audio Path None FM audio Path None hcitool cmd 0x3F 0X195 FF FF FF FF FF FF FF FF 00 00 FF 00 00 00 00 o FM Audio Path configuration # BT audio Path None FM audio Path PCM hcitool cmd 0x3F 0X195 FF FF FF FF FF FF FF FF 00 01 FF 00 00 00 00 Please note that the BT SCO Codec settings are platform specific. Selma. - Intel Corporation SAS (French simplified joint stock company) Registered headquarters: Les Montalets- 2, rue de Paris, 92196 Meudon Cedex, France Registration Number: 302 456 199 R.C.S. NANTERRE Capital: 4,572,000 Euros This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
RE: [alsa-devel] WL1273 FM Radio driver...
Hello. On Thu, 2011-02-10 at 09:35 +, ext Bensaid, Selma wrote: On Tue, 2011-02-08 at 13:02 +0200, Peter Ujfalusi wrote: For both configuration we have a set of HCI commands to configure the FM audio path and one of my concerns is to know if the wl1273_codec should handle the audio path configuration and the switch between FM and BT SCO? It would be better if the codec could handle the configuration, depending on which DAI is in use. If we can send HCI commands from kernel, I think that would be the cleanest way. Yes, I would have done just that - and we talked a lot about it locally - if I had known how to do it. I started to work on it and also talked to some BT people but it didn't seem feasible at the time. Advice welcome of course... Cheers, Matti Hi, Below the set of HCI commands that we have identified to configure the FM and BT SCO audio paths: - External Audio Connection o START BT SCO Connection # BT audio Path PCMFM audio Path I2S hcitool cmd 0x3F 0X195 FF FF FF FF FF FF FF FF 01 02 FF 00 00 00 00 # BT AUDIO Codec Configuration: MASTER hcitool cmd 0x3F 0X106 00 03 00 40 1F 00 00 01 00 00 00 00 10 00 02 00 00 10 00 02 00 01 00 00 00 00 00 00 00 00 00 00 o STOP BT SCO Connection # BT AUDIO Codec Configuration: SLAVE hcitool cmd 0x3F 0X106 00 03 01 40 1F 00 00 01 00 00 00 00 10 00 02 00 00 10 00 02 00 01 00 00 00 00 00 00 00 00 00 00 - Internal Audio Connection o START BT SCO Connection # BT audio Path PCM # FM audio Path None hcitool cmd 0x3F 0X195 FF FF FF FF FF FF FF FF 01 00 FF 00 00 00 00 # BT AUDIO Codec Configuration: MASTER hcitool cmd 0x3F 0X106 00 03 00 40 1F 00 00 01 00 00 00 00 10 00 02 00 00 10 00 02 00 01 00 00 00 00 00 00 00 00 00 00 o STOP BT SCO Connection # BT audio Path None FM audio Path None hcitool cmd 0x3F 0X195 FF FF FF FF FF FF FF FF 00 00 FF 00 00 00 00 o FM Audio Path configuration # BT audio Path None FM audio Path PCM hcitool cmd 0x3F 0X195 FF FF FF FF FF FF FF FF 00 01 FF 00 00 00 00 Please note that the BT SCO Codec settings are platform specific. Selma. We know these messages, they are mentioned in the documentation and we use them already, but we send them from the user space. The problem is how to send the messages from the driver within the kernel. Thanks, Matti - Intel Corporation SAS (French simplified joint stock company) Registered headquarters: Les Montalets- 2, rue de Paris, 92196 Meudon Cedex, France Registration Number: 302 456 199 R.C.S. NANTERRE Capital: 4,572,000 Euros This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] WL1273 FM Radio driver...
On Thu, Feb 10, 2011 at 02:10:40PM +0200, Matti J. Aaltonen wrote: But I got the following quick comment from a local BT expert: No you cannot change line discipline if bt is already in use. And it's not uart interface but hci interface. uart can be replaced with sdio for example and you still have the same hci interface. Yes, it only works when the device is idle - if your device needs to be shared with other bits of the system while doing this you'd need to present a virtual interface up the way. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] WL1273 FM Radio driver...
On Thu, 2011-02-10 at 12:28 +, ext Mark Brown wrote: On Thu, Feb 10, 2011 at 02:10:40PM +0200, Matti J. Aaltonen wrote: But I got the following quick comment from a local BT expert: No you cannot change line discipline if bt is already in use. And it's not uart interface but hci interface. uart can be replaced with sdio for example and you still have the same hci interface. Yes, it only works when the device is idle - if your device needs to be shared with other bits of the system while doing this you'd need to present a virtual interface up the way. And a comment to the above from the earlier mentioned local BT expert: It would need some hack to generic hci code. Or maybe some kind of management api extension. That should be a few line only.. but getting it to upstream sounds like mission impossible. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] WL1273 FM Radio driver...
On Thu, Feb 10, 2011 at 02:57:47PM +0200, Matti J. Aaltonen wrote: And a comment to the above from the earlier mentioned local BT expert: It would need some hack to generic hci code. Or maybe some kind of management api extension. That should be a few line only.. but getting it to upstream sounds like mission impossible. I don't see any particular problem getting something like that upstream if it works over multiple devices; we've already got stuff like that for the Amstrad Delta platform. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
On Mon, 2011-02-07 at 14:00 -0200, ext Mauro Carvalho Chehab wrote: Em 07-02-2011 12:17, Matti J. Aaltonen escreveu: On Mon, 2011-02-07 at 13:52 +, ext Mark Brown wrote: On Mon, Feb 07, 2011 at 11:48:17AM -0200, Mauro Carvalho Chehab wrote: Em 07-02-2011 11:10, Mark Brown escreveu: There is an audio driver for this chip and it is using those functions. Where are the other drivers that depend on it? Nothing's been merged yet to my knowledge, Matti can comment on any incoming boards which will use it (rx51?). Yes, nothing's been merged yet. There are only dependencies between the parts of this driver... I cannot comment on upcoming boards, I just hope we could agree on a sensible structure for this thing. We don't need any brand names or specific details, but it would be good to have an overview, in general lines, about the architecture, in order to help you to map how this would fit. In particular, the architecturre of things that are tightly coupled and can't be splitted by some bus abstraction. I understand what you are saying but obviously I cannot think like a sub-system maintainer:-) What I see here is a piece of hardware with lots of capabilities: FM RX and TX with analog and digital audio, I2C and UART control... I would have thought that the goal is to make a driver that's as general as possible and to make it possible to use the chip in all kinds of architectures and scenarios. But we have been using the driver in principle in its current form. But if the interface to the users has to be split in a different way, I don't see that as a major problem. But on the other hand I can't see the ASoC / V4L2 division going away completely. The users won't probably care if we have MFD or not. Cheers, Matti Mauro. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [alsa-devel] WL1273 FM Radio driver...
The wl1273 as such is designed for embedded systems. It can be connected in several ways to the system: - analog only In this way the RX/TX is connected to some codec's Line IN/OUT For this to work, we don't need any audio driver for the FM chip (basically the same configuration as rx51 has in regards of FM radio) - Digital interfaces The I2S lines are connected to the main processor. In this way the wl1273 acts as a codec. In order to provide platform independent driver we need to use ASoC framework. ASoC have broad main processor side support, and it is easy to switch the arch, if we have proper ASoC codec driver for the wl1273. It is also better to keep the codec implementation under sound/soc/codecs. 2 Digital interfaces are possible for FM WL1273: - the external connection: the I2S lines are used for the FM PCM samples - the internal connection: the BT PCM interface is used for the FM PCM samples For both configuration we have a set of HCI commands to configure the FM audio path and one of my concerns is to know if the wl1273_codec should handle the audio path configuration and the switch between FM and BT SCO? I have not looked deeply into the wl1273 datasheets, but I'm sure there's a way to nicely divide the parts between the MFD, V4L, and ASoC. -- Péter ___ Alsa-devel mailing list alsa-de...@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel - Intel Corporation SAS (French simplified joint stock company) Registered headquarters: Les Montalets- 2, rue de Paris, 92196 Meudon Cedex, France Registration Number: 302 456 199 R.C.S. NANTERRE Capital: 4,572,000 Euros This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] WL1273 FM Radio driver...
On 02/08/11 12:09, ext Bensaid, Selma wrote: 2 Digital interfaces are possible for FM WL1273: - the external connection: the I2S lines are used for the FM PCM samples - the internal connection: the BT PCM interface is used for the FM PCM samples Yes, that is correct, I just did not wanted to go into details. Currently the ASoC codec driver only supports the BT/FM PCM interface. Adding the dedicated I2S interface for the FM radio should not be a big effort, we just need to add another dai to the codec driver, and connect that to the host processor. As I said before, we only implemented the BT/FM PCM interface support, since we do not have HW, where we could verify the dedicated FM I2S lines. But adding the support should not be a big deal IMHO (and can be done even without any means of testing it). For both configuration we have a set of HCI commands to configure the FM audio path and one of my concerns is to know if the wl1273_codec should handle the audio path configuration and the switch between FM and BT SCO? It would be better if the codec could handle the configuration, depending on which DAI is in use. If we can send HCI commands from kernel, I think that would be the cleanest way. -- Péter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [alsa-devel] WL1273 FM Radio driver...
For both configuration we have a set of HCI commands to configure the FM audio path and one of my concerns is to know if the wl1273_codec should handle the audio path configuration and the switch between FM and BT SCO? It would be better if the codec could handle the configuration, depending on which DAI is in use. If we can send HCI commands from kernel, I think that would be the cleanest way. If we use the Combined Interface Mode (host controls both the BT and FM radio via BT HCI) this could be possible. However, you use the Separate Interface (FM controlled vi I2C). Is there a plan to handle also the Combined Interface Mode for WL1273 FM Radio driver? Selma. -- Péter - Intel Corporation SAS (French simplified joint stock company) Registered headquarters: Les Montalets- 2, rue de Paris, 92196 Meudon Cedex, France Registration Number: 302 456 199 R.C.S. NANTERRE Capital: 4,572,000 Euros This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] WL1273 FM Radio driver...
On Tue, 2011-02-08 at 13:02 +0200, Peter Ujfalusi wrote: For both configuration we have a set of HCI commands to configure the FM audio path and one of my concerns is to know if the wl1273_codec should handle the audio path configuration and the switch between FM and BT SCO? It would be better if the codec could handle the configuration, depending on which DAI is in use. If we can send HCI commands from kernel, I think that would be the cleanest way. Yes, I would have done just that - and we talked a lot about it locally - if I had known how to do it. I started to work on it and also talked to some BT people but it didn't seem feasible at the time. Advice welcome of course... Cheers, Matti -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] WL1273 FM Radio driver...
On Tue, 2011-02-08 at 11:15 +0200, Peter Ujfalusi I have not looked deeply into the wl1273 datasheets, but I'm sure there's a way to nicely divide the parts between the MFD, V4L, and ASoC. I don't think there's much to be moved between the sub-systems after moving the I2C communication to the MFD driver (which has almost been agreed on). Cheers, Matti -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
Hello. Mark Brown wrote: On Wed, Feb 02, 2011 at 01:35:01PM -0200, Mauro Carvalho Chehab wrote: [Reflowed into 80 columns.] My concerns is that the V4L2-specific part of the code should be at drivers/media. I prefer that the specific MFD I/O part to be at drivers/mfd, just like the other drivers. Currently that's not the case - the I/O functionality is not in any meaningful sense included in the MFD, it's provided by the V4L portion. I've been away for two and a half weeks so I haven't been able to comment... But before I start to make changes, I'd still like to ask for a comment on my original plan, which was to have the I/O functions in the MFD driver and also have there things like interrupt handling etc. My vision was that the MFD part would have the application logic and the child drivers would be just true interfaces to the core functionality, because I kind of saw the children to be of equal importance and because the codec and the v4l2 driver share some controls like for example the volume control. If you'd care to take a look an earlier version of the MFD driver here: http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/23602/match=aaltonen So the question is if I put only the I/O stuff into the MFD driver or can I have the other application logic there as well? Thanks, Matti -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
On Mon, Feb 07, 2011 at 10:00:16AM -0200, Mauro Carvalho Chehab wrote: the MFD part (for example, wl1273_fm_read_reg/wl1273_fm_write_cmd/wl1273_fm_write_data). The logic that are related to control the radio (wl1273_fm_set_audio, wl1273_fm_set_volume, etc) are not related to access the device via the MFD bus. They should be at the media part of the driver, where they belong. Those functions are being used by the audio driver. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
Em 07-02-2011 10:02, Mark Brown escreveu: On Mon, Feb 07, 2011 at 10:00:16AM -0200, Mauro Carvalho Chehab wrote: the MFD part (for example, wl1273_fm_read_reg/wl1273_fm_write_cmd/wl1273_fm_write_data). The logic that are related to control the radio (wl1273_fm_set_audio, wl1273_fm_set_volume, etc) are not related to access the device via the MFD bus. They should be at the media part of the driver, where they belong. Those functions are being used by the audio driver. Not sure if I understood your comments. Several media drivers have alsa drivers: saa7134, em28xx, cx231xx, etc. The audio drivers for them are also under /drivers/media, as it is not easy to de-couple audio and video/radio part on those devices. For bttv and some USB boards (that use snd-usb-audio), the audio part is at /sound, as the audio part on them are independent and don't need to share anything, as audio is provided by a completely independent group of registers. I suggest to use the same logic for wl1273. Cheers, Mauro. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
On Mon, Feb 07, 2011 at 10:48:03AM -0200, Mauro Carvalho Chehab wrote: Em 07-02-2011 10:02, Mark Brown escreveu: On Mon, Feb 07, 2011 at 10:00:16AM -0200, Mauro Carvalho Chehab wrote: the MFD part (for example, wl1273_fm_read_reg/wl1273_fm_write_cmd/wl1273_fm_write_data). The logic that are related to control the radio (wl1273_fm_set_audio, wl1273_fm_set_volume, etc) are not related to access the device via the MFD bus. They should be at the media part of the driver, where they belong. Those functions are being used by the audio driver. Not sure if I understood your comments. Several media drivers have alsa drivers: There is an audio driver for this chip and it is using those functions. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
On Mon, 2011-02-07 at 10:48 -0200, ext Mauro Carvalho Chehab wrote: Em 07-02-2011 10:02, Mark Brown escreveu: On Mon, Feb 07, 2011 at 10:00:16AM -0200, Mauro Carvalho Chehab wrote: the MFD part (for example, wl1273_fm_read_reg/wl1273_fm_write_cmd/wl1273_fm_write_data). The logic that are related to control the radio (wl1273_fm_set_audio, wl1273_fm_set_volume, etc) are not related to access the device via the MFD bus. They should be at the media part of the driver, where they belong. Those functions are being used by the audio driver. Not sure if I understood your comments. Several media drivers have alsa drivers: saa7134, em28xx, cx231xx, etc. The audio drivers for them are also under /drivers/media, as it is not easy to de-couple audio and video/radio part on those devices. For bttv and some USB boards (that use snd-usb-audio), the audio part is at /sound, as the audio part on them are independent and don't need to share anything, as audio is provided by a completely independent group of registers. I suggest to use the same logic for wl1273. So you are suggesting a more or less complete rewrite, so that I'd create a directory like media/radio/wl1273 and then place there all of the driver files: wl1273-radio.c, wl1273-alsa.c and maybe wl1273-core.c? Cheers, Matti Cheers, Mauro. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
On Mon, Feb 07, 2011 at 03:27:12PM +0200, Matti J. Aaltonen wrote: So you are suggesting a more or less complete rewrite, so that I'd create a directory like media/radio/wl1273 and then place there all of the driver files: wl1273-radio.c, wl1273-alsa.c and maybe wl1273-core.c? Don't move the ASoC driver out of the ASoC code, it's a complete pain for maintaining it and only makes the problems with having to build v4l worse. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
Em 07-02-2011 11:10, Mark Brown escreveu: On Mon, Feb 07, 2011 at 10:48:03AM -0200, Mauro Carvalho Chehab wrote: Em 07-02-2011 10:02, Mark Brown escreveu: On Mon, Feb 07, 2011 at 10:00:16AM -0200, Mauro Carvalho Chehab wrote: the MFD part (for example, wl1273_fm_read_reg/wl1273_fm_write_cmd/wl1273_fm_write_data). The logic that are related to control the radio (wl1273_fm_set_audio, wl1273_fm_set_volume, etc) are not related to access the device via the MFD bus. They should be at the media part of the driver, where they belong. Those functions are being used by the audio driver. Not sure if I understood your comments. Several media drivers have alsa drivers: There is an audio driver for this chip and it is using those functions. Where are the other drivers that depend on it? Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
On Mon, Feb 07, 2011 at 11:48:17AM -0200, Mauro Carvalho Chehab wrote: Em 07-02-2011 11:10, Mark Brown escreveu: There is an audio driver for this chip and it is using those functions. Where are the other drivers that depend on it? Nothing's been merged yet to my knowledge, Matti can comment on any incoming boards which will use it (rx51?). Note that due to the decomposed nature of embedded audio hardware the audio part of the chip needs to be represended within the audio subsystem even if the control were all in the media side - this isn't an isolated bit of hardware on an expansion card, it's fairly tightly coupled into the rest of the system. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
On Mon, 2011-02-07 at 11:48 -0200, ext Mauro Carvalho Chehab wrote: Em 07-02-2011 11:10, Mark Brown escreveu: On Mon, Feb 07, 2011 at 10:48:03AM -0200, Mauro Carvalho Chehab wrote: Em 07-02-2011 10:02, Mark Brown escreveu: On Mon, Feb 07, 2011 at 10:00:16AM -0200, Mauro Carvalho Chehab wrote: the MFD part (for example, wl1273_fm_read_reg/wl1273_fm_write_cmd/wl1273_fm_write_data). The logic that are related to control the radio (wl1273_fm_set_audio, wl1273_fm_set_volume, etc) are not related to access the device via the MFD bus. They should be at the media part of the driver, where they belong. Those functions are being used by the audio driver. Not sure if I understood your comments. Several media drivers have alsa drivers: There is an audio driver for this chip and it is using those functions. Where are the other drivers that depend on it? There's the MFD driver driver/mfd/wl1273-core.c, which is to offer the (I2C) I/O functions to the child drivers: drivers/media/radio/radio-wl1273.c and sound/soc/codecs/wl1273.c. Both children depend on the MFD driver for I/O and the codec also depends on the presence of the radio-wl1273 driver because without the v4l2 part nothing can be done... Matti Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
On Mon, 2011-02-07 at 13:52 +, ext Mark Brown wrote: On Mon, Feb 07, 2011 at 11:48:17AM -0200, Mauro Carvalho Chehab wrote: Em 07-02-2011 11:10, Mark Brown escreveu: There is an audio driver for this chip and it is using those functions. Where are the other drivers that depend on it? Nothing's been merged yet to my knowledge, Matti can comment on any incoming boards which will use it (rx51?). Yes, nothing's been merged yet. There are only dependencies between the parts of this driver... I cannot comment on upcoming boards, I just hope we could agree on a sensible structure for this thing. Matti. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
Em 07-02-2011 12:09, Matti J. Aaltonen escreveu: On Mon, 2011-02-07 at 11:48 -0200, ext Mauro Carvalho Chehab wrote: Em 07-02-2011 11:10, Mark Brown escreveu: On Mon, Feb 07, 2011 at 10:48:03AM -0200, Mauro Carvalho Chehab wrote: Em 07-02-2011 10:02, Mark Brown escreveu: On Mon, Feb 07, 2011 at 10:00:16AM -0200, Mauro Carvalho Chehab wrote: the MFD part (for example, wl1273_fm_read_reg/wl1273_fm_write_cmd/wl1273_fm_write_data). The logic that are related to control the radio (wl1273_fm_set_audio, wl1273_fm_set_volume, etc) are not related to access the device via the MFD bus. They should be at the media part of the driver, where they belong. Those functions are being used by the audio driver. Not sure if I understood your comments. Several media drivers have alsa drivers: There is an audio driver for this chip and it is using those functions. Where are the other drivers that depend on it? There's the MFD driver driver/mfd/wl1273-core.c, which is to offer the (I2C) I/O functions to the child drivers: drivers/media/radio/radio-wl1273.c and sound/soc/codecs/wl1273.c. Both children depend on the MFD driver for I/O and the codec also depends on the presence of the radio-wl1273 driver because without the v4l2 part nothing can be done... I think that the better would be to move the audio part (sound/soc/codecs/wl1273.c) as drivers/media/radio/wl1273/wl1273-alsa.c. Is there any problem on moving it, or the alsa driver is also tightly coupled on the rest of the sound/soc stuff? I remember that, in the past, there were someone that proposed to move /sound into /media/sound, and move some common stuff between them into /media/common. Btw, there are(where?) some problems between -alsa and -media subsystems: basically, the audio core needs to be initialized before the drivers. However, this sometimes don't happen (I can't remember the exact situation - perhaps builtin compilations?), but we ended by needing to explicitly delaying the init of some drivers with: late_initcall(saa7134_alsa_init); To avoid some OOPS conditions. Matti Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
Em 07-02-2011 12:17, Matti J. Aaltonen escreveu: On Mon, 2011-02-07 at 13:52 +, ext Mark Brown wrote: On Mon, Feb 07, 2011 at 11:48:17AM -0200, Mauro Carvalho Chehab wrote: Em 07-02-2011 11:10, Mark Brown escreveu: There is an audio driver for this chip and it is using those functions. Where are the other drivers that depend on it? Nothing's been merged yet to my knowledge, Matti can comment on any incoming boards which will use it (rx51?). Yes, nothing's been merged yet. There are only dependencies between the parts of this driver... I cannot comment on upcoming boards, I just hope we could agree on a sensible structure for this thing. We don't need any brand names or specific details, but it would be good to have an overview, in general lines, about the architecture, in order to help you to map how this would fit. In particular, the architecturre of things that are tightly coupled and can't be splitted by some bus abstraction. Mauro. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
On Mon, Feb 07, 2011 at 01:57:10PM -0200, Mauro Carvalho Chehab wrote: Em 07-02-2011 12:09, Matti J. Aaltonen escreveu: Both children depend on the MFD driver for I/O and the codec also depends on the presence of the radio-wl1273 driver because without the v4l2 part nothing can be done... I think that the better would be to move the audio part (sound/soc/codecs/wl1273.c) as drivers/media/radio/wl1273/wl1273-alsa.c. Is there any problem on moving it, or the alsa driver is also tightly coupled on the rest of the sound/soc stuff? As I said in my previous e-mail it's tightly coupled. I remember that, in the past, there were someone that proposed to move /sound into /media/sound, and move some common stuff between them into /media/common. This is the first embedded audio driver that's had interface with media stuff, the driver situation for embedded audio is very different to that for PCs. Embedded audio subsystems are tightly coupled integrations of many different devices, the sound card userspace sees is produced by coordinating the actions of several different drivers. Btw, there are(where?) some problems between -alsa and -media subsystems: basically, the audio core needs to be initialized before the drivers. However, this sometimes don't happen (I can't remember the exact situation - perhaps builtin compilations?), but we ended by needing to explicitly delaying the init of some drivers with: late_initcall(saa7134_alsa_init); To avoid some OOPS conditions. This isn't a problem for embedded audio, instantiation of the cards is deferred until all the components for the card have registered with the core so nothing will happen until dependencies are satisfied, though it is a problem with the wl1273 driver as it currently stands due to the lack of a functional MFD. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
Em 30-01-2011 21:23, Samuel Ortiz escreveu: Hi Matti, On Tue, Jan 18, 2011 at 05:04:23PM +0200, Matti J. Aaltonen wrote: Hello I have been trying to get the WL1273 FM radio driver into the kernel for some time. It has been kind of difficult, one of the reasons is that I didn't realize I should have tried to involve all relevant maintainers to the discussion form the beginning (AsoC, Media and MFD). At Mark's suggestion I'm trying to reopen the discussion now. The driver consists of an MFD core and two child drivers (the audio codec and the V4L2 driver). And the question is mainly about the role of the MFD driver: the original design had the IO functions in the core. Currently the core is practically empty mainly because Mauro very strongly wanted to have “everything” in the V4L2 driver. What was Mauro main concerns with having the IO part in the core ? A lot of MFD drivers are going that path already. My concerns is that the V4L2-specific part of the code should be at drivers/media. I prefer that the specific MFD I/O part to be at drivers/mfd, just like the other drivers. Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
On Wed, Feb 02, 2011 at 01:35:01PM -0200, Mauro Carvalho Chehab wrote: [Reflowed into 80 columns.] My concerns is that the V4L2-specific part of the code should be at drivers/media. I prefer that the specific MFD I/O part to be at drivers/mfd, just like the other drivers. Currently that's not the case - the I/O functionality is not in any meaningful sense included in the MFD, it's provided by the V4L portion. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
Hi Mauro, On Wed, Feb 02, 2011 at 01:35:01PM -0200, Mauro Carvalho Chehab wrote: Em 30-01-2011 21:23, Samuel Ortiz escreveu: Hi Matti, On Tue, Jan 18, 2011 at 05:04:23PM +0200, Matti J. Aaltonen wrote: Hello I have been trying to get the WL1273 FM radio driver into the kernel for some time. It has been kind of difficult, one of the reasons is that I didn't realize I should have tried to involve all relevant maintainers to the discussion form the beginning (AsoC, Media and MFD). At Mark's suggestion I'm trying to reopen the discussion now. The driver consists of an MFD core and two child drivers (the audio codec and the V4L2 driver). And the question is mainly about the role of the MFD driver: the original design had the IO functions in the core. Currently the core is practically empty mainly because Mauro very strongly wanted to have “everything” in the V4L2 driver. What was Mauro main concerns with having the IO part in the core ? A lot of MFD drivers are going that path already. My concerns is that the V4L2-specific part of the code should be at drivers/media. I prefer that the specific MFD I/O part to be at drivers/mfd, just like the other drivers. Agreed, but it seems that's not the case currently. Would you be ok with Matti refactoring those 2 drivers a bit so that the actual core I/O parts should be handled by the MFD driver ? Cheers, Samuel. -- Intel Open Source Technology Centre http://oss.intel.com/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
Hi Matti, On Tue, Jan 18, 2011 at 05:04:23PM +0200, Matti J. Aaltonen wrote: Hello I have been trying to get the WL1273 FM radio driver into the kernel for some time. It has been kind of difficult, one of the reasons is that I didn't realize I should have tried to involve all relevant maintainers to the discussion form the beginning (AsoC, Media and MFD). At Mark's suggestion I'm trying to reopen the discussion now. The driver consists of an MFD core and two child drivers (the audio codec and the V4L2 driver). And the question is mainly about the role of the MFD driver: the original design had the IO functions in the core. Currently the core is practically empty mainly because Mauro very strongly wanted to have “everything” in the V4L2 driver. What was Mauro main concerns with having the IO part in the core ? A lot of MFD drivers are going that path already. Cheers, Samuel. -- Intel Open Source Technology Centre http://oss.intel.com/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WL1273 FM Radio driver...
On Tue, Jan 18, 2011 at 05:04:23PM +0200, Matti J. Aaltonen wrote: The driver consists of an MFD core and two child drivers (the audio codec and the V4L2 driver). And the question is mainly about the role of the MFD driver: the original design had the IO functions in the core. Currently the core is practically empty mainly because Mauro very strongly wanted to have “everything” in the V4L2 driver. I liked the original design because it didn't have the bug that the current MFD has: the codec can be initialized before the V4L2 part sets the IO function pointers. Having in principle equally capable interface drivers is symmetrical and esthetically pleasing:-) Also somebody could easily leave out the existing interfaces and create a completely new one based for example to sysfs or something... Having the IO in the core would also conveniently hide the physical communication layer and make it easy to change I2C to UART, which is also supported by the chip. The above pattern with the core taking responsibility for register I/O is used by all the other MFD drivers in part because it's much less fragile against initialisation ordering issues. It ensures that before any subdevices can instantiate and try to do register I/O all the infrastructure required to do that is present. Is there any great reason for following a different pattern, and if we are going to do so how do we deal with the init ordering? -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
WL1273 FM Radio driver...
Hello I have been trying to get the WL1273 FM radio driver into the kernel for some time. It has been kind of difficult, one of the reasons is that I didn't realize I should have tried to involve all relevant maintainers to the discussion form the beginning (AsoC, Media and MFD). At Mark's suggestion I'm trying to reopen the discussion now. The driver consists of an MFD core and two child drivers (the audio codec and the V4L2 driver). And the question is mainly about the role of the MFD driver: the original design had the IO functions in the core. Currently the core is practically empty mainly because Mauro very strongly wanted to have “everything” in the V4L2 driver. I liked the original design because it didn't have the bug that the current MFD has: the codec can be initialized before the V4L2 part sets the IO function pointers. Having in principle equally capable interface drivers is symmetrical and esthetically pleasing:-) Also somebody could easily leave out the existing interfaces and create a completely new one based for example to sysfs or something... Having the IO in the core would also conveniently hide the physical communication layer and make it easy to change I2C to UART, which is also supported by the chip. Thanks, Matti -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git:v4l-dvb/for_v2.6.38] [media] [v18, 2/2] V4L2: WL1273 FM Radio: TI WL1273 FM radio driver
On Tue, 2010-12-21 at 15:15 +0100, ext Hans Verkuil wrote:
On Tuesday, December 21, 2010 15:05:08 Mauro Carvalho Chehab wrote:
This is an automatic generated email to let you know that the following
patch were queued at the
http://git.linuxtv.org/media_tree.git tree:
Subject: [media] [v18,2/2] V4L2: WL1273 FM Radio: TI WL1273 FM radio driver
Author: Matti Aaltonen matti.j.aalto...@nokia.com
Date:Fri Dec 10 11:41:34 2010 -0300
This module implements V4L2 controls for the Texas Instruments
WL1273 FM Radio and handles the communication with the chip.
Signed-off-by: Matti J. Aaltonen matti.j.aalto...@nokia.com
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
drivers/media/radio/Kconfig| 16 +
drivers/media/radio/Makefile |1 +
drivers/media/radio/radio-wl1273.c | 2331
3 files changed, 2348 insertions(+), 0 deletions(-)
snip
+
+static const struct v4l2_file_operations wl1273_fops = {
+ .owner = THIS_MODULE,
+ .read = wl1273_fm_fops_read,
+ .write = wl1273_fm_fops_write,
+ .poll = wl1273_fm_fops_poll,
+ .ioctl = video_ioctl2,
Matti,
Can you make a patch that replaces .ioctl with .unlocked_ioctl?
This should be done for 2.6.38.
Yes, no problem...
Cheers,
Matti
See this thread for more information on how to do this:
http://www.spinics.net/lists/linux-media/msg26604.html
Thanks!
Hans
+ .open = wl1273_fm_fops_open,
+ .release= wl1273_fm_fops_release,
+};
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git:v4l-dvb/for_v2.6.38] [media] [v18, 2/2] V4L2: WL1273 FM Radio: TI WL1273 FM radio driver
On Tuesday, December 21, 2010 15:05:08 Mauro Carvalho Chehab wrote:
This is an automatic generated email to let you know that the following patch
were queued at the
http://git.linuxtv.org/media_tree.git tree:
Subject: [media] [v18,2/2] V4L2: WL1273 FM Radio: TI WL1273 FM radio driver
Author: Matti Aaltonen matti.j.aalto...@nokia.com
Date:Fri Dec 10 11:41:34 2010 -0300
This module implements V4L2 controls for the Texas Instruments
WL1273 FM Radio and handles the communication with the chip.
Signed-off-by: Matti J. Aaltonen matti.j.aalto...@nokia.com
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
drivers/media/radio/Kconfig| 16 +
drivers/media/radio/Makefile |1 +
drivers/media/radio/radio-wl1273.c | 2331
3 files changed, 2348 insertions(+), 0 deletions(-)
snip
+
+static const struct v4l2_file_operations wl1273_fops = {
+ .owner = THIS_MODULE,
+ .read = wl1273_fm_fops_read,
+ .write = wl1273_fm_fops_write,
+ .poll = wl1273_fm_fops_poll,
+ .ioctl = video_ioctl2,
Matti,
Can you make a patch that replaces .ioctl with .unlocked_ioctl?
This should be done for 2.6.38.
See this thread for more information on how to do this:
http://www.spinics.net/lists/linux-media/msg26604.html
Thanks!
Hans
+ .open = wl1273_fm_fops_open,
+ .release= wl1273_fm_fops_release,
+};
--
Hans Verkuil - video4linux developer - sponsored by Cisco
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v18 0/2] WL1273 FM Radio driver...
Hi. Thank you for the comments and the conditional ACK. On Thu, 2010-12-09 at 15:17 +0100, ext Samuel Ortiz wrote: with the device. The ALSA codec offers digital audio, without it only analog audio is available. The driver looks much better now. If that goes through Mauro's tree, please add my: Acked-by: Samuel Ortiz sa...@linux.intel.com OK. \ No newline at end of file Please add a new line here. Added... Cheers, Matti Matti J. Aaltonen (2): MFD: WL1273 FM Radio: MFD driver for the FM radio. V4L2: WL1273 FM Radio: TI WL1273 FM radio driver drivers/media/radio/Kconfig| 16 + drivers/media/radio/Makefile |1 + drivers/media/radio/radio-wl1273.c | 2331 drivers/mfd/Kconfig| 10 + drivers/mfd/Makefile |1 + drivers/mfd/wl1273-core.c | 148 +++ include/linux/mfd/wl1273-core.h| 288 + 7 files changed, 2795 insertions(+), 0 deletions(-) create mode 100644 drivers/media/radio/radio-wl1273.c create mode 100644 drivers/mfd/wl1273-core.c create mode 100644 include/linux/mfd/wl1273-core.h -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v17 0/2] WL1273 FM radio driver...
Hello,
and thank you for the comments.
On Fri, 2010-11-26 at 12:56 +0100, ext Samuel Ortiz wrote:
+config WL1273_CORE
+ tristate
+ depends on I2C
+ select MFD_CORE
+ default n
+
You need to be a lot more verbose here. Nobody knows what this wl1273 core
driver is for.
Also, please rename your config to MFD_WL1273_CORE.
I added a bit of explanation and also renamed the config.
+#include linux/delay.h
Not needed
Removed all unnecessary includes.
+ struct mfd_cell *cell = core-cells[children];
Please add a line between your variable declarations and the rest of the code.
Eventually I moved these definitions to the beginning of the function.
+ } else {
+ dev_err(client-dev, Cannot function without radio
child.\n);
+ r = -EINVAL;
+ goto err_radio_child;
+ }
So I'd prefer something like:
if (!pdata-children WL1273_RADIO_CHILD) {
dev_err(...);
return -EINVAL;
}
before you actually allocate the core pointer.
Moved this test to the beginning of the function.
+ if (!r)
+ return 0;
I'd prefere:
if (children == 0) {
dev_err();
r = -ENODEV;
goto err;
}
dev_dbg();
return mfd_add_devices();
err:
I did more or less what Samuel suggested but kept the test for the
mfd_add_devices return value to be able to free resources in case
of error.
+ return 0;
Here you can just return r.
Yes.
+ flush_scheduled_work();
What is that for ?
I forgot that when refactoring the code. Removed.
+ * include/media/radio/radio-wl1273.h
This is include/linux/mfd/wl1273-core.h
Correct.
+ * Copyright (C) Nokia Corporation
You want to specify a year here.
Added year to all copyright statements.
+#ifndef RADIO_WL1273_H
#ifndef WL1273_CORE_H, please.
OK.
+#include linux/i2c.h
+#include linux/mfd/core.h
You don't want all your drivers to include mfd/core.h. And that may apply to
i2c.h as well.
Both are directly needed in this header so I kept these includes.
+struct wl1273_fw_packet{
+ int len;
+ __u8 *buf;
+};
Do you really need to export this structure ?
No. It actually become completely obsoleted so: removed.
+ int (*write)(struct wl1273_core *core, u8, u16);
+ int (*set_audio)(struct wl1273_core *core, unsigned int);
+ int (*set_volume)(struct wl1273_core *core, unsigned int);
So who is defining those routines ?
They are defined in the V4L2 driver. Previously they were defined here
in the core...
On Fri, 2010-11-19 at 18:33 +0530, ext Raja Mani wrote:
+static int radio_nr = -1;
You have assigned -1 to radio_nr here. But your description says The
default is 0.
I am bit confused...
I was also confused, so I removed the initialization.
+MODULE_PARM_DESC(radio_nr, Radio Nr);
Can we add clear comments instead Radio Nr ?. I know other driver
has this kind of description
What you added for rds_buf is more clear. This is only a suggestion..
Wrote a longer explanation.
+ struct i2c_client *client = core-client;
+
No space here
+ u8 buf[] = { (param 8) 0xff, param 0xff }
OK.
+ r = i2c_transfer(client-adapter, msg, 1);
+
No space here
+ if (r != 1) {
OK.
+ val = volume;
Really do we need val variable here ?
I wanted to have a pointer to u16...
+ r = wl1273_fm_read_reg(core, WL1273_RSSI_LVL_GET, level);
+
No space here
+ if (r)
OK.
+ INIT_COMPLETION(radio-busy);
+ /* Set the current tx channel */
+ r = wl1273_fm_write_cmd(core, WL1273_CHANL_SET, freq / 10);
+ if (r)
+ return r;
+
Better , radio-busy init can be moved here.. there can be a chance to
hit above return..
OK. I also did the other similar changes.
+ INIT_COMPLETION(radio-busy);
+ r = wl1273_fm_write_cmd(core, WL1273_TUNER_MODE_SET,
TUNER_MODE_PRESET);
+ if (r) {
+ dev_err(radio-dev, TUNER_MODE_SET fails\n);
why complete here ? radio-busy is just initialized .. who is waiting
on radio-busy ?
A good question. I removed the complete call and moved the init after the test.
+
+ return 0;
probably, you have to return negative error . not Zero. Isn't it
mandatory to download FM Firmware ?
I kept this as it was because we are uploading only a firmware patch,
which may not be necessary. Added a comment, however.
+ goto out;
+ }
should we need packets ? . Can't we deal with fw_p-data itself for
firmware download?
we can avoid eating system memory.. Give a thought on this..
I got rid of the packets.
Cheers,
Matti
Matti J. Aaltonen (2):
MFD: WL1273 FM Radio: MFD driver for the FM radio.
V4L2: WL1273 FM Radio: TI WL1273 FM radio driver
drivers/media/radio/Kconfig| 16 +
drivers/media/radio/Makefile
Re: [GIT PULL REQUEST v2.] WL1273 FM Radio driver.
Em 16-11-2010 11:35, Matti J. Aaltonen escreveu: Hi. The radio pull request, now using http protocol. The following changes since commit f6614b7bb405a9b35dd28baea989a749492c46b2: Linus Torvalds (1): Merge git://git.kernel.org/.../sfrench/cifs-2.6 are available in the git repository at: http://git.gitorious.org/wl1273-fm-driver/wl1273-fm-driver.git master Matti J. Aaltonen (2): MFD: WL1273 FM Radio: MFD driver for the FM radio. V4L2: WL1273 FM Radio: Controls for the FM radio. Hi Matti, The proper way is to add the core stuff to drivers/media/radio, adding just the mfd glue at drivers/mfd. I also want mfd's maintainer ack of the mfd patch. I just added an mfd driver right now. You may use it as an example: http://git.linuxtv.org/media_tree.git?a=commit;h=ef6ce9acc5f87e253c97dfd5301f8036f937f595 http://git.linuxtv.org/media_tree.git?a=commit;h=552faf8580766b6fc944cb966f690ed0624a5564 drivers/media/radio/Kconfig| 16 + drivers/media/radio/Makefile |1 + drivers/media/radio/radio-wl1273.c | 1841 drivers/mfd/Kconfig|6 + drivers/mfd/Makefile |1 + drivers/mfd/wl1273-core.c | 617 include/linux/mfd/wl1273-core.h| 330 +++ 7 files changed, 2812 insertions(+), 0 deletions(-) create mode 100644 drivers/media/radio/radio-wl1273.c create mode 100644 drivers/mfd/wl1273-core.c create mode 100644 include/linux/mfd/wl1273-core.h B.R. Matti A. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL REQUEST v2.] WL1273 FM Radio driver.
Hi. The radio pull request, now using http protocol. The following changes since commit f6614b7bb405a9b35dd28baea989a749492c46b2: Linus Torvalds (1): Merge git://git.kernel.org/.../sfrench/cifs-2.6 are available in the git repository at: http://git.gitorious.org/wl1273-fm-driver/wl1273-fm-driver.git master Matti J. Aaltonen (2): MFD: WL1273 FM Radio: MFD driver for the FM radio. V4L2: WL1273 FM Radio: Controls for the FM radio. drivers/media/radio/Kconfig| 16 + drivers/media/radio/Makefile |1 + drivers/media/radio/radio-wl1273.c | 1841 drivers/mfd/Kconfig|6 + drivers/mfd/Makefile |1 + drivers/mfd/wl1273-core.c | 617 include/linux/mfd/wl1273-core.h| 330 +++ 7 files changed, 2812 insertions(+), 0 deletions(-) create mode 100644 drivers/media/radio/radio-wl1273.c create mode 100644 drivers/mfd/wl1273-core.c create mode 100644 include/linux/mfd/wl1273-core.h B.R. Matti A. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL REQUEST] WL1273 FM Radio driver.
Hello Mauro. Here is the pull request for the TI wl1273 FM radio driver. The following changes since commit f6614b7bb405a9b35dd28baea989a749492c46b2: Linus Torvalds (1): Merge git://git.kernel.org/.../sfrench/cifs-2.6 are available in the git repository at: g...@gitorious.org:wl1273-fm-driver/wl1273-fm-driver.git master Matti J. Aaltonen (2): MFD: WL1273 FM Radio: MFD driver for the FM radio. V4L2: WL1273 FM Radio: Controls for the FM radio. drivers/media/radio/Kconfig| 16 + drivers/media/radio/Makefile |1 + drivers/media/radio/radio-wl1273.c | 1841 drivers/mfd/Kconfig|6 + drivers/mfd/Makefile |1 + drivers/mfd/wl1273-core.c | 635 + include/linux/mfd/wl1273-core.h| 330 +++ 7 files changed, 2830 insertions(+), 0 deletions(-) create mode 100644 drivers/media/radio/radio-wl1273.c create mode 100644 drivers/mfd/wl1273-core.c create mode 100644 include/linux/mfd/wl1273-core.h B.R. Matti A. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v12 0/3] TI WL1273 FM Radio driver...
Hi Matti, I've acked patches 1 and 2 and added some comments to patch 3. Two other patches are needed, though: the new BLOCK_IO and CONTROLS capabilities also need to be set in existing RDS drivers: video/saa6588.c radio/radio-cadet.c radio/si4713-i2c.c radio/si470x/radio-si470x-common.c The other patch is for v4l2-ctl.cpp in the v4l-utils repository where support for these new caps needs to be added as well. This second patch can be provided once the other patches are merged, although it would be nice if you can have it earlier. Regards, Hans On Thursday, October 07, 2010 15:16:10 Matti J. Aaltonen wrote: Hello Mauro, Hans and others. I haven't gotten any comments to the latest patch set. The audio part of the driver has already been accepted so I'm now trying to apply a similar approach as with the codec. I've abstracted out the physical control layer from the driver, it could use I2c or UART but the driver now has only read and write calls (and a couple of other calls). Also the driver doesn't export anything and it doesn't expose the FM radio bands it uses internally to the outsize world. Cheers, Matti Matti J. Aaltonen (3): V4L2: Add seek spacing and RDS CAP bits. V4L2: WL1273 FM Radio: Controls for the FM radio. Documentation: v4l: Add hw_seek spacing and two TUNER_RDS_CAP flags. Documentation/DocBook/v4l/dev-rds.xml | 10 +- .../DocBook/v4l/vidioc-s-hw-freq-seek.xml | 10 +- drivers/media/radio/Kconfig| 16 + drivers/media/radio/Makefile |1 + drivers/media/radio/radio-wl1273.c | 1848 include/linux/videodev2.h |5 +- 6 files changed, 1886 insertions(+), 4 deletions(-) create mode 100644 drivers/media/radio/radio-wl1273.c -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 0/3] TI WL1273 FM Radio driver...
Hello Mauro, Hans and others. I haven't gotten any comments to the latest patch set. The audio part of the driver has already been accepted so I'm now trying to apply a similar approach as with the codec. I've abstracted out the physical control layer from the driver, it could use I2c or UART but the driver now has only read and write calls (and a couple of other calls). Also the driver doesn't export anything and it doesn't expose the FM radio bands it uses internally to the outsize world. Cheers, Matti Matti J. Aaltonen (3): V4L2: Add seek spacing and RDS CAP bits. V4L2: WL1273 FM Radio: Controls for the FM radio. Documentation: v4l: Add hw_seek spacing and two TUNER_RDS_CAP flags. Documentation/DocBook/v4l/dev-rds.xml | 10 +- .../DocBook/v4l/vidioc-s-hw-freq-seek.xml | 10 +- drivers/media/radio/Kconfig| 16 + drivers/media/radio/Makefile |1 + drivers/media/radio/radio-wl1273.c | 1848 include/linux/videodev2.h |5 +- 6 files changed, 1886 insertions(+), 4 deletions(-) create mode 100644 drivers/media/radio/radio-wl1273.c -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v11 0/4] WL1273 FM Radio driver.
Hello again! I've only received one comment from Hans (thank you) and I'm still expecting to get comments also from Mauro. But I'm sending the eleventh version anyway to keep the wheels rolling so to speak... Hans wrote: + V4L2_CAP_RDS_OUTPUT | V4L2_TUNER_CAP_RDS_BLOCK_IO; + + return 0; +} V4L2_TUNER_CAP_RDS_BLOCK_IO is a tuner/modulator capability! Not a querycap capability! It's added at the wrong place. Moved the BLOCK_IO flag to g_tuner and g_modulator. I also made a small fix to driver/media/radio/Kconfig so that the driver can actually be built without the digital audio codec. And in addition removed a reference to FM radio bands from the mfd file. B.R. Matti Matti J. Aaltonen (4): V4L2: Add seek spacing and RDS CAP bits. MFD: WL1273 FM Radio: MFD driver for the FM radio. V4L2: WL1273 FM Radio: Controls for the FM radio. Documentation: v4l: Add hw_seek spacing and two TUNER_RDS_CAP flags. Documentation/DocBook/v4l/dev-rds.xml | 10 +- .../DocBook/v4l/vidioc-s-hw-freq-seek.xml | 10 +- drivers/media/radio/Kconfig| 16 + drivers/media/radio/Makefile |1 + drivers/media/radio/radio-wl1273.c | 1859 drivers/mfd/Kconfig|5 + drivers/mfd/Makefile |2 + drivers/mfd/wl1273-core.c | 583 ++ include/linux/mfd/wl1273-core.h| 320 include/linux/videodev2.h |5 +- 10 files changed, 2807 insertions(+), 4 deletions(-) create mode 100644 drivers/media/radio/radio-wl1273.c create mode 100644 drivers/mfd/wl1273-core.c create mode 100644 include/linux/mfd/wl1273-core.h -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 0/4] WL1273 FM Radio driver.
Hello all.
This is the tenth version of this patch set. Thank you for
comments, especially to Mauro.
I'll go through Mauro's comments one by one.
Patch 1/4:
+ case V4L2_CID_FM_BAND: return FM Band;
There's no need for a FM control, as there's already an ioctl pair
that allows get/set the frequency bandwidth: VIDIOC_S_TUNER and
VIDIOC_G_TUNER. So, the entire patch here seems uneeded/unwanted.
OK, I've taken this approach to bands.
2/4:
+MODULE_PARM_DESC(radio_band, Band: 0=USA-Europe, 1=Japan);
There's no need for a parameter to set the bandwidth.
Agreed removed...
+MODULE_PARM_DESC(rds_buf, RDS buffer entries: *100*);
Hmm... it would be better to use, instead:
MODULE_PARM_DESC(rds_buf, Number of RDS buffer entries. Default = 100);
Yes that's a better wording. Changed.
+EXPORT_SYMBOL(wl1273_fm_read_reg);
Hmm... why do you need to export a symbol here?
This is something I didn't change (yet). I'll try to explain. Our original
idea for the driver structure was like this.
codec v4l2 controls
\ /
\ /
\ /
the mfd core
Mauro's idea is to merge the control child to the core and get rid of
the function exports. But because of the codec child many of the
exports would still be needed. And it makes sense to have the codec as
a separate module because the radio could be used without digital
audio. So I'm kind of against a major rewrite between v9 and v10
because in my view having v4l2 part as a separate module is not that
bad...
+static void wl1273_fm_rds_work(struct wl1273_core *core)
+{
+ wl1273_fm_rds(core);
+}
Wouldn't be better to just use wl1273_fm_rds() instead of this?
Yes... fixed.
+ /* RDS buffer allocation */
+ core-buf_size = rds_buf * 3;
Why it should be multiplied by a randomvalue of 3?
Added a #define for the RDS block length.
+obj-$(CONFIG_RADIO_WL1273) += radio-wl1273.o
From what I saw, wl1273-core and radio-wl1273 are just part of the
same radio driver. So, the better would be to just merge them into
one driver module, with something like:
obj-wl1273.o = radio-wl1273.o wl1273-core.o
obj-$(CONFIG_RADIO_WL1273) += radio-wl1273.o
And remove all those export_symbol' s from wl1273-core.
Didn't do this (yet) for the reasons mentioned above
3/4:
+#define DRIVER_DESC Wl1273 FM Radio - V4L2
Why V4L2??? Just call it as Wl1273 FM Radio.
That is - I guess - a good question, and I removed the - V4L2. My
idea was to differentiate between the core, codec and the v4l2 part,
that idea evidently wasn't a good one...
+ msgs = kmalloc((packet_num + 1)*sizeof(struct i2c_msg), GFP_KERNEL);
Small CodingStyle issue: please use spaces between * operator: msgs
= kmalloc((packet_num + 1) * sizeof(struct i2c_msg),
GFP_KERNEL);
Fixed...
+static int wl1273_fm_set_band(struct wl1273_core *core, unsigned int band)
+{
Should be adjusted to use VIDIOC_[G|S]_TUNER for RX.
Done. Internally the driver still kind of has two bands because that's
how the chip is. g_tuner returns the range 76 - 108MHz, which is the
combination of the two internal ranges.
s_tuner select either of the ranges, and returns an error if the requested
range doesn't fit into either supported ranges. Are you happy with this
approach?
+ /* calculate block count from byte count */
+ count /= 3;
Why the magic value of 3? Instead, please define a constant, naming
it in a way that a casual code reviewer could understand why you're
multiplying/dividing by 3.
Replaced all the magic numbers with the define constant.
+ r = wl1273_fm_set_band(core, ctrl-val);
+ break;
Not needed. Instead, please implement it via VIDIOC_S_TUNER.
Done...
+ dev_dbg(radio-dev, tuner-rangehigh: %d\n, tuner-rangehigh);
Ranges should be using tuner-rangelow/rangehigh to change band limits.
Yes. Fixed.
+ }
Please do the registration of the device at the end. Applications may
try to open (and udev, in fact does it) the device while you're still
initializing it. If you need to do it earlier, you'll need a lock
protecting open/close/init, to avoid race conditions.
Moved the registration to the end of the probe function.
4/4:
new FM RX control class.
Same comment as patch 1/4: FM bandwidth can already be defined via
VIDIOC_[G|S]_TUNER. So, this patch is just creating a duplicated API
for something already defined.
Changed the documentation to reflect the changes in the code.
Cheers,
Matti
Matti J. Aaltonen (4): V4L2: Add seek spacing and RDS CAP bits. MFD:
WL1273 FM Radio: MFD driver for the FM radio. V4L2: WL1273 FM
Radio: Controls for the FM radio. Documentation: v4l: Add hw_seek
spacing and two TUNER_RDS_CAP flags.
Documentation/DocBook/v4l/dev-rds.xml | 10 +-
.../DocBook/v4l/vidioc-s-hw-freq-seek.xml | 10 +-
drivers/media/radio/Kconfig
[PATCH v8 0/5] TI WL1273 FM Radio driver.
Hello,
and thank you for the comments.
The audio codec has been accepted on the ALSA list...
I've converted the driver to the new control framework
as Hans strongly suggested.
P.S. I thought that I sent the patches on the day I created them,
but something clearly went wrong here...
Hans wrote:
Use ERANGE instead of EDOM. EDOM is for math functions only.
Changed EDOM to ERANGE.
+ if (r)
+ core-mode = old_mode ;
Remove space before ';'.
Space removed...
+ if (radio-rds_on) {
+ if (mutex_lock_interruptible(core-lock))
+ return -EINTR;
+
+ core-irq_flags = ~WL1273_RDS_EVENT;
This is dangerous: you probably want to use a usecount instead. With this
code opening the device one will turn on the RDS events, but opening and
closing it via another application (e.g. v4l2-ctl) will disable it while
the first still needs it.
Replaced the bool variable with a usage counter.
Alexey wrote:
+ if (!radio-write_buf)
+ return -ENOMEM;
I'm not sure but it looks like possible memory leak. Shouldn't you
call to kfree(radio) before returning ENOMEM?
and
+err_device_alloc:
+ kfree(radio);
And i'm not sure about this error path.. Before kfree(radio) it's
needed to call kfree(radio-write_buf), rigth?
Looks like all erorr paths in this probe function have to be checked.
Rewrote the error handling in the probe function.
Pramodh wrote:
+r = wl1273_fm_write_cmd(core, WL1273_POWER_LEV_SET, power);
Output power level is specified in units of dBuV (as explained at
http://www.linuxtv.org/downloads/v4l-dvb-apis/ch01s09.html#fm-tx-controls).
Shouldn't it be converted to WL1273 specific power level value?
My understanding:
If output power level specified using V4L2_CID_TUNE_POWER_LEVEL is 122
(dB/uV), then
power level value to be passed for WL1273 should be '0'.
Please correct me, if I got this conversion wrong.
Fixed the TX power level handling...
Thanks
Matti
Matti J. Aaltonen (5):
V4L2: Add seek spacing and FM RX class.
MFD: WL1273 FM Radio: MFD driver for the FM radio.
ASoC: WL1273 FM Radio Digital audio codec.
V4L2: WL1273 FM Radio: Controls for the FM radio.
Documentation: v4l: Add hw_seek spacing and FM_RX class
Documentation/DocBook/v4l/controls.xml | 71 +
.../DocBook/v4l/vidioc-s-hw-freq-seek.xml | 10 +-
drivers/media/radio/Kconfig| 15 +
drivers/media/radio/Makefile |1 +
drivers/media/radio/radio-wl1273.c | 1947
drivers/media/video/v4l2-ctrls.c | 12 +
drivers/mfd/Kconfig|5 +
drivers/mfd/Makefile |2 +
drivers/mfd/wl1273-core.c | 612 ++
include/linux/mfd/wl1273-core.h| 314
include/linux/videodev2.h | 15 +-
sound/soc/codecs/Kconfig |6 +
sound/soc/codecs/Makefile |2 +
sound/soc/codecs/wl1273.c | 593 ++
sound/soc/codecs/wl1273.h | 42 +
15 files changed, 3644 insertions(+), 3 deletions(-)
create mode 100644 drivers/media/radio/radio-wl1273.c
create mode 100644 drivers/mfd/wl1273-core.c
create mode 100644 include/linux/mfd/wl1273-core.h
create mode 100644 sound/soc/codecs/wl1273.c
create mode 100644 sound/soc/codecs/wl1273.h
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 0/5] TI WL1273 FM Radio driver.
Hello. It starts to look like the ALSA codec could be accepted on the ALSA list pretty soon. So I'd be very interested to hear from you if the rest of the driver still needs fixes... By the way, now the newest wl1273 firmware supports a special form of hardware seek, they call it the 'bulk search' mode. It can be used to search for all stations that are available and at first the number of found stations is returned. Then the frequencies can be fetched one by one. Should we add a 'seek mode' field to hardware seek? Or do you have a vision of how it should be handled? Thanks, Matti -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 0/5] TI WL1273 FM Radio driver.
Hello. It starts to look like the ALSA codec could be accepted on the ALSA list pretty soon. So I'd be very interested to hear from you if the rest of the driver still needs fixes... Thanks for reminding me. I'll do a final review this evening. By the way, now the newest wl1273 firmware supports a special form of hardware seek, they call it the 'bulk search' mode. It can be used to search for all stations that are available and at first the number of found stations is returned. Then the frequencies can be fetched one by one. Should we add a 'seek mode' field to hardware seek? Or do you have a vision of how it should be handled? It sounds very hardware specific. We should postpone support for this until we have support for subdev device nodes. That will make it possible to create custom ioctls for hw specific features. This should be merged if all goes well for 2.6.37. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 0/5] TI WL1273 FM Radio driver.
Hello all,
and thanks for the comments Hans.
Now I've done a couple of iterations with the codec on the ALSA mailing
list and that still continues... I've removed all #undef DEBUG lines,
because the ALSA people didn't like them.
I'll go through the comments and the rest of the changes:
+ tuner-capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_RDS |
+ V4L2_TUNER_CAP_STEREO;
+
audmode must be set even when the device is in TX mode. Best is to just set it
to the last set audmode.
I added a state field for the audmode. I used a boolean variable because
that seemed to lead to clearer code than using an int. Other two valued
entities - like the digital/analog mode - could also be modeled with booleans
but I didn't do it because it could be condemned by the community :-)
Should there also be audmode for the modulator?
+ if (val == WL1273_RX_MONO) {
+ tuner-rxsubchans = V4L2_TUNER_SUB_MONO;
+ tuner-audmode = V4L2_TUNER_MODE_MONO;
+ } else {
+ tuner-rxsubchans = V4L2_TUNER_SUB_STEREO;
+ tuner-audmode = V4L2_TUNER_MODE_STEREO;
+ }
There are two separate things: detecting whether the signal is stereo or mono
and selecting the audio mode (this is the format of the audio that is sent to
userspace). The first is set in rxsubchans and is dynamic, the second is fixed
and set by the application.
If the device can detect mono vs stereo signals, then rxsubchans should be set
accordingly. If the device cannot do this, then both mono and stereo should be
specified in rxsubchans.
The audmode field is like a control: it does not automatically change if the
signal switches from mono to stereo or vice versa. Unless the hardware is
unable to map a mono signal to a stereo audio stream or a stereo signal to a
mono audio stream.
The fact that the code above sets both rxsubchans and audmode suggests either
that the hardware cannot map stereo to mono or vice versa, or a program bug.
In the first case we need a comment here, in the second case it should be
fixed.
I kind of new I was doing something wrong here... but then I thought: why
isn't there a control variable for the RDS? Anyway, now I've made the
distinction between subchans flags and the audmode field.
+
+ if (core-rds_on)
+ modulator-txsubchans |= V4L2_TUNER_SUB_RDS;
+ else
+ modulator-txsubchans = ~V4L2_TUNER_SUB_RDS;
This else is not needed.
Else removed...
Just make this Hz. There is no need to restrict the precision to
kHz. S_FREQUENCY supports units of 67.5 Hz, so using kHz for the
spacing seems unnecessary.
Alternatively the same resolution as S_FREQUENCY can be used (67.5 Hz
or 67.5 kHz, depending on the CAP_LOW capability). Not sure which is
best, though.
I think using Hz is the most straightforward policy here so I chose that.
Cheers,
Matti
Matti J. Aaltonen (5):
V4L2: Add seek spacing and FM RX class.
MFD: WL1273 FM Radio: MFD driver for the FM radio.
ASoC: WL1273 FM Radio Digital audio codec.
V4L2: WL1273 FM Radio: Controls for the FM radio.
Documentation: v4l: Add hw_seek spacing and FM_RX class
Documentation/DocBook/v4l/controls.xml | 71 +
.../DocBook/v4l/vidioc-s-hw-freq-seek.xml | 10 +-
drivers/media/radio/Kconfig| 15 +
drivers/media/radio/Makefile |1 +
drivers/media/radio/radio-wl1273.c | 1972
drivers/media/video/v4l2-common.c | 12 +
drivers/mfd/Kconfig|6 +
drivers/mfd/Makefile |2 +
drivers/mfd/wl1273-core.c | 612 ++
include/linux/mfd/wl1273-core.h| 314
include/linux/videodev2.h | 15 +-
sound/soc/codecs/Kconfig |6 +
sound/soc/codecs/Makefile |2 +
sound/soc/codecs/wl1273.c | 591 ++
sound/soc/codecs/wl1273.h | 42 +
15 files changed, 3668 insertions(+), 3 deletions(-)
create mode 100644 drivers/media/radio/radio-wl1273.c
create mode 100644 drivers/mfd/wl1273-core.c
create mode 100644 include/linux/mfd/wl1273-core.h
create mode 100644 sound/soc/codecs/wl1273.c
create mode 100644 sound/soc/codecs/wl1273.h
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 0/5] WL1273 FM Radio driver...
Hello All!
And thank for comments Hans and Mauro.
Here is the list of changes from v5:
include/linux/videodev2.h
Hans wrote:
+
+#define V4L2_CID_FM_RX_BAND (V4L2_CID_FM_TX_CLASS_BASE + 1)
+enum v4l2_fm_rx_band {
Just a very small change: rename v4l2_fm_rx_band to v4l2_fm_band. We might
need
this enum later for transmitter devices as well so it is better to give it a
slightly more generic name.
Changed the name to v4l2_fm_band. Also changed the define constant.
Hans wrote:
Note: you also need to add support for the new class and control to
v4l2-common.c.
The following functions should be extended:
v4l2_ctrl_get_menu()
v4l2_ctrl_get_name()
v4l2_ctrl_query_fill()
Added code...
sound/soc/codecs/wl1273.c
Hans wrote:
You might want to have this reviewed on the alsa mailinglist. I don't think
anyone on the linux-media list has the expertise to review this audio codec.
Yes, good idea, I'll send it to the ALSA list...
drivers/media/radio/radio-wl1273.c
+ return POLLIN | POLLOUT | POLLRDNORM;
You also need to add POLLWRNORM.
I wonder if this code is correct. Doesn't this depend on whether the device
is in receive or transmit mode? So either poll returns POLLIN|POLLRDNORM or
POLLOUT|POLLWRNORM. Or am I missing something?
I don't think you are missing anything. I've added code for RX and TX.
+ strcpy(tuner-name, WL1273_FM_DRIVER_NAME);
strlcpy
Fixed. There was also another strcpy. Fixed also that.
+ tuner-rxsubchans = V4L2_TUNER_SUB_MONO | V4L2_TUNER_SUB_STEREO;
You can't detect whether mono or stereo is received? Does the alsa codec
always
receive two channel audio? How does it handle mono vs stereo?
Stereo and mono modes can be detected. ALSA can also handle both...
The codec conforms automatically to the audio source/sink.
+ tuner-capability = V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_RDS;
Shouldn't CAP_STEREO be added?
Added.
+ tuner-rxsubchans |= V4L2_TUNER_SUB_RDS;
tuner-audmode isn't filled!
Filled...
+ r = wl1273_fm_set_rds(core, WL1273_RDS_OFF);
There is no support for SUB_MONO or SUB_STEREO?
Actually there is...
+ modulator-capability = V4L2_TUNER_CAP_RDS;
Shouldn't CAP_LOW and CAP_STEREO be added here?
I agree.
+ modulator-txsubchans = ~V4L2_TUNER_SUB_RDS;
The SUB_MONO/SUB_STEREO flags aren't handled here.
Handling added.
The g/s_tuner and g/s_modulator functions are always hard to get right. Lots
of
tricky flags and settings...
I agree... I kind of left the stereo/mono stuff to alsa...
As Mauro's review mentioned: please specify the unit of this spacing field!
(It should be Hz).
Don't forget to read and reply to Mauro's review as well!
OK. I put kHz as the spacing unit, that should be fine grained enough? I also
reformulated the text somewhat. And yes, I didn't notice Mauro's comments on
v4 patch set. But now I've read his comments and also given some comments
back.
Cheers,
Matti A.
Matti J. Aaltonen (5):
V4L2: Add seek spacing and FM RX class.
MFD: WL1273 FM Radio: MFD driver for the FM radio.
ASoC: WL1273 FM Radio Digital audio codec.
V4L2: WL1273 FM Radio: Controls for the FM radio.
Documentation: v4l: Add hw_seek spacing and FM_RX class
Documentation/DocBook/v4l/controls.xml | 71 +
.../DocBook/v4l/vidioc-s-hw-freq-seek.xml | 10 +-
drivers/media/radio/Kconfig| 15 +
drivers/media/radio/Makefile |1 +
drivers/media/radio/radio-wl1273.c | 1960
drivers/media/video/v4l2-common.c | 12 +
drivers/mfd/Kconfig|6 +
drivers/mfd/Makefile |2 +
drivers/mfd/wl1273-core.c | 613 ++
include/linux/mfd/wl1273-core.h| 313
include/linux/videodev2.h | 15 +-
sound/soc/codecs/Kconfig |6 +
sound/soc/codecs/Makefile |2 +
sound/soc/codecs/wl1273.c | 593 ++
sound/soc/codecs/wl1273.h | 42 +
15 files changed, 3658 insertions(+), 3 deletions(-)
create mode 100644 drivers/media/radio/radio-wl1273.c
create mode 100644 drivers/mfd/wl1273-core.c
create mode 100644 include/linux/mfd/wl1273-core.h
create mode 100644 sound/soc/codecs/wl1273.c
create mode 100644 sound/soc/codecs/wl1273.h
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/5] WL1273 FM Radio driver.
Hello again, and thank you for the comments. New in this version of the patch set: General headers: I removed the seek level stuff and added the FM RX class. And I've added the BAND IOCTL and I defined the three existing bands: I also added the OIRT band because I think it's nicer to have three bands than only two. Hans wrote: Is there a difference in power consumption between the 'off' and 'suspend' state of the device? I assume that 'off' has the lowest power consumption. In that case I would have the driver go to suspend when no one has opened the radioX device. And if the driver is asked to go to suspend (that's the linux suspend state), then the driver can turn off the radio and on resume it will reload the firmware. Does that sound reasonable? Yes that's reasonable... But I have had one problem here: the suspend does not work, but we are working on it. As mentioned I don't think the level stuff should be added at the moment. The spacing field is no problem, but don't forget to update the V4L2 spec as well. Also document there what should happen if spacing == 0 (which is the case for existing apps). It basically boils down to the fact that the driver uses the spacing as a hint only and will adjust it to whatever the hardware supports. I've fixed the spacing handling. I've dropped the seek level stuff. drivers/mfd/wl1273-core.c: Don't use bitfields! How bitfields are ordered is compiler specific. I've dropped the bitfields. Does the data you copy here conform to the v4l2_rds_data struct? In particular the block byte. It is well documented in the Spec in the section on 'Reading RDS data'. The error bits are not same as in the spec so I now copy them to the correct positions and use the v4l2_rds_data struct. drivers/media/radio/radio-wl1273.c: I understood that the band was relevant for receiving only? That's true, I've removed the band stuff here. Rather than continually allocating and freeing this buffer I would just make it a field in struct wl1273_device. It's never more than 256 bytes, so that's no problem. Now the buffer gets allocated and freed only once. Don't. Just make sure there can be only one reader. This is the same principle used by video: there can be multiple file handles open on a video node, but only one can be used for streaming at a time. Trying to handle multiple readers or writers in a driver will lead to chaos. And this can be done much better by userspace. Now only one reader or writer is accepted. Where are the other FM_TX controls? Added the missing controls. However, there's read-only control - I didn't add that. Use strlcpy here as well. Now I'm using strlcpy everywhere... I strongly recommend using v4l2_ctrl_query_fill() instead (defined in v4l2-common.c). This ensures consistent naming. It will also make it easier to convert to the upcoming new control framework. Replaced the old code with code that uses v4l2_ctrl_query_fill etc. Make this dev_dbg. I think it is probably better to pick the closest spacing rather than falling back to 50. I've changed spacing handling quite a bit. Cheers, Matti J. Aaltonen (5): V4L2: Add seek spacing and FM RX class. MFD: WL1273 FM Radio: MFD driver for the FM radio. ASoC: WL1273 FM Radio Digital audio codec. V4L2: WL1273 FM Radio: Controls for the FM radio. Documentation: v4l: Add hw_seek spacing. .../DocBook/v4l/vidioc-s-hw-freq-seek.xml | 10 +- drivers/media/radio/Kconfig| 15 + drivers/media/radio/Makefile |1 + drivers/media/radio/radio-wl1273.c | 1907 drivers/mfd/Kconfig|6 + drivers/mfd/Makefile |2 + drivers/mfd/wl1273-core.c | 616 +++ include/linux/mfd/wl1273-core.h| 326 include/linux/videodev2.h | 15 +- sound/soc/codecs/Kconfig |6 + sound/soc/codecs/Makefile |2 + sound/soc/codecs/wl1273.c | 594 ++ sound/soc/codecs/wl1273.h | 40 + 13 files changed, 3537 insertions(+), 3 deletions(-) create mode 100644 drivers/media/radio/radio-wl1273.c create mode 100644 drivers/mfd/wl1273-core.c create mode 100644 include/linux/mfd/wl1273-core.h create mode 100644 sound/soc/codecs/wl1273.c create mode 100644 sound/soc/codecs/wl1273.h -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] WL1273 FM Radio Driver
On Monday 24 May 2010 14:21:39 Matti J. Aaltonen wrote: Hello again. And thanks for the comments. It the first patch I'm kind of suggesting a couple of additions to the general interface: signal level stuff in the hw seek struct and then a function / IOCTL for asking for minimum and maximum for the level. There are many changes I'll follow by commenting to Hans's comments: 1. WL1273_CID_FM_REGION for setting the region. This may not be a good candidate for standardization as the region control shouldn't exist in the kernel in general... Is this region relevant for receive, transmit or both? Region is relevant for receiving only. Now I've changes the naming to band because TI uses that in their latest document version. Based on this article: http://en.wikipedia.org/wiki/FM_broadcasting, there seem to be only three bands for FM radio in practice: 87.5-108, 76-90 (Japan) and 65-74 (former Soviet republics, some former eastern bloc countries). So this should be a standard control. Since the latter band is being phased out I think a menu control with just the two other bands is sufficient. And I also think this should be a control of a new FM_RX class. I know, that's not what I said last time. So sue me :-) BTW: don't forget to update the V4L2 spec when adding new controls! 2. WL1273_CID_FM_SEEK_SPACING: defines what resolution is used when scanning automatically for stations (50KHz, 100KHz or 200KHz). This could be useful in general. Could this be a field in the v4l2_hw_freq_seek struct? I think this belongs in v4l2_hw_freq_seek. I've added spacing to the hw seek struct. 3. WL1273_CID_FM_RDS_CTRL for turning on and off the RDS reception / transmission. To me this seems like a useful standard control... This already exists. You can enable/disable RDS by setting the V4L2_TUNER_SUB_RDS subchannel bit when calling S_TUNER or S_MODULATOR. I did this. 4. WL1273_CID_SEARCH_LVL for setting the threshold level when detecting radio channels when doing automatic scan. This could be useful for fine tuning because automatic scanning seems to be kind of problematic... This could also be a field in the v4l2_hw_freq_seek struct? This too seems reasonable to add to v4l2_hw_freq_seek. Although what sort of unit this level would have might be tricky. What is the unit for your hardware? I've added this as well. The unit is some kind of dB value: 8 bit signed number in 2أ�s complement format Each LSB = 1.5051 dBuV. I also added min and max values for the level. I'm uncertain about this. I wonder if this shouldn't be something that is hardcoded in the driver. Setting these levels to sensible values is not a trivial exercise. It is also very much device specific. I think it is best to do hardcode it now and once we can expose controls that are sub-device specific (this is being worked on), then we can add WL1273-specific controls for this. Could the VIDIOC_S_MODULATOR and VIDIOC_S_TUNER IOCTLs be used for setting the TX/RX mode? Not entirely sure what you want to achieve here. I gather that the radio is either receiving, transmitting or off? So it can't receive and transmit at the same time, right? Yes the radio can only transmit or receive at a time. And the states are: On (RX or TX), Off and Suspended. In the suspended mode that firmware patch is kept in the memory and it doesn't need to get uploaded again when operation resumes. would expect in that case that calling S_TUNER or S_MODULATOR would switch it to either receive or transmit mode. S_HW_FREQ_SEEK would of course also switch it to receive mode. I added this... There isn't anything to turn off the radio at the moment. Perhaps you can just automatically turn it off if nobody has the radio device or alsa device open? Yes that can be done. Also volume control could be used. But also there's nothing to put the radio to stand-by (suspension). Is there a difference in power consumption between the 'off' and 'suspend' state of the device? I assume that 'off' has the lowest power consumption. In that case I would have the driver go to suspend when no one has opened the radioX device. And if the driver is asked to go to suspend (that's the linux suspend state), then the driver can turn off the radio and on resume it will reload the firmware. Does that sound reasonable? Now there already exits a class for fm transmitters: V4L2_CTRL_CLASS_FM_TX. Should a corresponding class be created for FM tuners? I'm not sure that we need any. I think the only control that we need is to set the region, and I think that is more appropriate as a private (?) user control since it is definitely something that users should be easily able to change. OK, that's fine by me This probably should be discussed a bit more. Ok. There's also two new things. The chip supports in addition to the normal HW seek a HW block seek, which
[PATCH v3 0/4] WL1273 FM Radio Driver
Hello again. And thanks for the comments. It the first patch I'm kind of suggesting a couple of additions to the general interface: signal level stuff in the hw seek struct and then a function / IOCTL for asking for minimum and maximum for the level. There are many changes I'll follow by commenting to Hans's comments: 1. WL1273_CID_FM_REGION for setting the region. This may not be a good candidate for standardization as the region control shouldn't exist in the kernel in general... Is this region relevant for receive, transmit or both? Region is relevant for receiving only. Now I've changes the naming to band because TI uses that in their latest document version. 2. WL1273_CID_FM_SEEK_SPACING: defines what resolution is used when scanning automatically for stations (50KHz, 100KHz or 200KHz). This could be useful in general. Could this be a field in the v4l2_hw_freq_seek struct? I think this belongs in v4l2_hw_freq_seek. I've added spacing to the hw seek struct. 3. WL1273_CID_FM_RDS_CTRL for turning on and off the RDS reception / transmission. To me this seems like a useful standard control... This already exists. You can enable/disable RDS by setting the V4L2_TUNER_SUB_RDS subchannel bit when calling S_TUNER or S_MODULATOR. I did this. 4. WL1273_CID_SEARCH_LVL for setting the threshold level when detecting radio channels when doing automatic scan. This could be useful for fine tuning because automatic scanning seems to be kind of problematic... This could also be a field in the v4l2_hw_freq_seek struct? This too seems reasonable to add to v4l2_hw_freq_seek. Although what sort of unit this level would have might be tricky. What is the unit for your hardware? I've added this as well. The unit is some kind of dB value: 8 bit signed number in 2âs complement format Each LSB = 1.5051 dBuV. I also added min and max values for the level. Could the VIDIOC_S_MODULATOR and VIDIOC_S_TUNER IOCTLs be used for setting the TX/RX mode? Not entirely sure what you want to achieve here. I gather that the radio is either receiving, transmitting or off? So it can't receive and transmit at the same time, right? Yes the radio can only transmit or receive at a time. And the states are: On (RX or TX), Off and Suspended. In the suspended mode that firmware patch is kept in the memory and it doesn't need to get uploaded again when operation resumes. would expect in that case that calling S_TUNER or S_MODULATOR would switch it to either receive or transmit mode. S_HW_FREQ_SEEK would of course also switch it to receive mode. I added this... There isn't anything to turn off the radio at the moment. Perhaps you can just automatically turn it off if nobody has the radio device or alsa device open? Yes that can be done. Also volume control could be used. But also there's nothing to put the radio to stand-by (suspension). Now there already exits a class for fm transmitters: V4L2_CTRL_CLASS_FM_TX. Should a corresponding class be created for FM tuners? I'm not sure that we need any. I think the only control that we need is to set the region, and I think that is more appropriate as a private (?) user control since it is definitely something that users should be easily able to change. OK, that's fine by me This probably should be discussed a bit more. Ok. There's also two new things. The chip supports in addition to the normal HW seek a HW block seek, which finds all receivable channels at once. And then there's a RSSI block scan that be used in both RX and TX modes to find transmitting radio stations. Should we try to get these also into the general interface? Cheers, Matti Matti J. Aaltonen (4): V4L2: Add features to the interface. MFD: WL1273 FM Radio: MFD driver for the FM radio. ASoC: WL1273 FM Radio Digital audio codec. V4L2: WL1273 FM Radio: Controls for the FM radio. drivers/media/radio/Kconfig| 15 + drivers/media/radio/Makefile |1 + drivers/media/radio/radio-wl1273.c | 1876 drivers/mfd/Kconfig|6 + drivers/mfd/Makefile |2 + drivers/mfd/wl1273-core.c | 606 include/linux/mfd/wl1273-core.h| 326 +++ include/linux/videodev2.h |6 +- include/media/v4l2-ioctl.h |2 + sound/soc/codecs/Kconfig |6 + sound/soc/codecs/Makefile |2 + sound/soc/codecs/wl1273.c | 588 +++ sound/soc/codecs/wl1273.h | 40 + 13 files changed, 3475 insertions(+), 1 deletions(-) create mode 100644 drivers/media/radio/radio-wl1273.c create mode 100644 drivers/mfd/wl1273-core.c create mode 100644 include/linux/mfd/wl1273-core.h create mode 100644 sound/soc/codecs/wl1273.c create mode 100644 sound/soc/codecs/wl1273.h -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: [PATCH 0/3] TI WL1273 FM Radio Driver v2.
On Friday 30 April 2010 14:59:45 Matti J. Aaltonen wrote: Hello. I've implemented most of the changes proposed on the previous review round. There are some things to be done in the RDS handling... I've left the region handling as it was because neither of the chip's regions cover the complete range. Japan is from 76 - 90MHz and Europe/US is 87.5 to 108 MHz. Some of the private IOCTL were not necessary because corresponding standardized controls already exist. And for setting the audio mode to digital or analog I created an ALSA control because that's an audio thing anyway. A couple of private IOCTLs are still there: 1. WL1273_CID_FM_REGION for setting the region. This may not be a good candidate for standardization as the region control shouldn't exist in the kernel in general... Is this region relevant for receive, transmit or both? 2. WL1273_CID_FM_SEEK_SPACING: defines what resolution is used when scanning automatically for stations (50KHz, 100KHz or 200KHz). This could be useful in genaral. Could this be a field in the v4l2_hw_freq_seek struct? I think this belongs in v4l2_hw_freq_seek. 3. WL1273_CID_FM_RDS_CTRL for turning on and off the RDS reception / transmission. To me this seems like a useful standard control... This already exists. You can enable/disable RDS by setting the V4L2_TUNER_SUB_RDS subchannel bit when calling S_TUNER or S_MODULATOR. 4. WL1273_CID_SEARCH_LVL for setting the threshold level when detecting radio channels when doing automatic scan. This could be useful for fine tuning because automatic scanning seems to be kind of problematic... This could also be a field in the v4l2_hw_freq_seek struct? This too seems reasonable to add to v4l2_hw_freq_seek. Although what sort of unit this level would have might be tricky. What is the unit for your hardware? 5. WL1273_CID_FM_RADIO_MODE: Now the radio has the following modes: off, suspend, rx and tx. It probably would be better to separate the powering state (off, on, suspend) from the FM radio state (rx, tx)... Could the VIDIOC_S_MODULATOR and VIDIOC_S_TUNER IOCTLs be used for setting the TX/RX mode? Not entirely sure what you want to achieve here. I gather that the radio is either receiving, transmitting or off? So it can't receive and transmit at the same time, right? I would expect in that case that calling S_TUNER or S_MODULATOR would switch it to either receive or transmit mode. S_HW_FREQ_SEEK would of course also switch it to receive mode. There isn't anything to turn off the radio at the moment. Perhaps you can just automatically turn it off if nobody has the radio device or alsa device open? Now there already exits a class for fm transmitters: V4L2_CTRL_CLASS_FM_TX. Should a corresponding class be created for FM tuners? I'm not sure that we need any. I think the only control that we need is to set the region, and I think that is more appropriate as a private (?) user control since it is definitely something that users should be easily able to change. This probably should be discussed a bit more. Regards, Hans B.R. Matti A. Matti J. Aaltonen (3): MFD: WL1273 FM Radio: MFD driver for the FM radio. WL1273 FM Radio: Digital audio codec. V4L2: WL1273 FM Radio: Controls for the FM radio. drivers/media/radio/Kconfig| 15 + drivers/media/radio/Makefile |1 + drivers/media/radio/radio-wl1273.c | 1849 drivers/mfd/Kconfig|6 + drivers/mfd/Makefile |2 + drivers/mfd/wl1273-core.c | 609 include/linux/mfd/wl1273-core.h| 323 +++ sound/soc/codecs/Kconfig |6 + sound/soc/codecs/Makefile |2 + sound/soc/codecs/wl1273.c | 587 sound/soc/codecs/wl1273.h | 40 + 11 files changed, 3440 insertions(+), 0 deletions(-) create mode 100644 drivers/media/radio/radio-wl1273.c create mode 100644 drivers/mfd/wl1273-core.c create mode 100644 include/linux/mfd/wl1273-core.h create mode 100644 sound/soc/codecs/wl1273.c create mode 100644 sound/soc/codecs/wl1273.h -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] TI WL1273 FM Radio Driver v2.
Hello. I've implemented most of the changes proposed on the previous review round. There are some things to be done in the RDS handling... I've left the region handling as it was because neither of the chip's regions cover the complete range. Japan is from 76 - 90MHz and Europe/US is 87.5 to 108 MHz. Some of the private IOCTL were not necessary because corresponding standardized controls already exist. And for setting the audio mode to digital or analog I created an ALSA control because that's an audio thing anyway. A couple of private IOCTLs are still there: 1. WL1273_CID_FM_REGION for setting the region. This may not be a good candidate for standardization as the region control shouldn't exist in the kernel in general... 2. WL1273_CID_FM_SEEK_SPACING: defines what resolution is used when scanning automatically for stations (50KHz, 100KHz or 200KHz). This could be useful in genaral. Could this be a field in the v4l2_hw_freq_seek struct? 3. WL1273_CID_FM_RDS_CTRL for turning on and off the RDS reception / transmission. To me this seems like a useful standard control... 4. WL1273_CID_SEARCH_LVL for setting the threshold level when detecting radio channels when doing automatic scan. This could be useful for fine tuning because automatic scanning seems to be kind of problematic... This could also be a field in the v4l2_hw_freq_seek struct? 5. WL1273_CID_FM_RADIO_MODE: Now the radio has the following modes: off, suspend, rx and tx. It probably would be better to separate the powering state (off, on, suspend) from the FM radio state (rx, tx)... Could the VIDIOC_S_MODULATOR and VIDIOC_S_TUNER IOCTLs be used for setting the TX/RX mode? Now there already exits a class for fm transmitters: V4L2_CTRL_CLASS_FM_TX. Should a corresponding class be created for FM tuners? B.R. Matti A. Matti J. Aaltonen (3): MFD: WL1273 FM Radio: MFD driver for the FM radio. WL1273 FM Radio: Digital audio codec. V4L2: WL1273 FM Radio: Controls for the FM radio. drivers/media/radio/Kconfig| 15 + drivers/media/radio/Makefile |1 + drivers/media/radio/radio-wl1273.c | 1849 drivers/mfd/Kconfig|6 + drivers/mfd/Makefile |2 + drivers/mfd/wl1273-core.c | 609 include/linux/mfd/wl1273-core.h| 323 +++ sound/soc/codecs/Kconfig |6 + sound/soc/codecs/Makefile |2 + sound/soc/codecs/wl1273.c | 587 sound/soc/codecs/wl1273.h | 40 + 11 files changed, 3440 insertions(+), 0 deletions(-) create mode 100644 drivers/media/radio/radio-wl1273.c create mode 100644 drivers/mfd/wl1273-core.c create mode 100644 include/linux/mfd/wl1273-core.h create mode 100644 sound/soc/codecs/wl1273.c create mode 100644 sound/soc/codecs/wl1273.h -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
