Re: [PATCH v7 4/5] V4L2: WL1273 FM Radio: Controls for the FM radio.
On Monday, August 23, 2010 08:53:37 Matti J. Aaltonen wrote: Hi. On Fri, 2010-08-20 at 14:04 +0200, ext pramodh ag wrote: Hello, +static ssize_t wl1273_fm_fops_write(struct file *file, const char __user *buf, +size_t count, loff_t *ppos) +{ +struct wl1273_device *radio = video_get_drvdata(video_devdata(file)); +u16 val; +int r; + +dev_dbg(radio-dev, %s\n, __func__); + +if (radio-core-mode != WL1273_MODE_TX) +return count; + +if (!radio-rds_on) { +dev_warn(radio-dev, %s: RDS not on.\n, __func__); +return 0; +} Aren't you planning to use extended controls V4L2_CID_RDS_TX_RADIO_TEXT, V4L2_CID_RDS_TX_PI, etc to handle FM TX RDS data? In principle yes, but we haven't yet decided to implement those now, at the moment the RDS interpretation is left completely to user space applications. Matti, is it even possible to use the current FM TX RDS API for this chip? That API expects that the chip can generate the correct RDS packets based on high-level data. If the chip can only handle 'raw' RDS packets (requiring a userspace RDS encoder), then that API will never work. But if this chip can indeed handle raw RDS only, then we need to add some capability flags to signal that to userspace. Regards, Hans Best Regards, Matti Thanks and regards, Pramodh -- 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
Re: [PATCH v7 4/5] V4L2: WL1273 FM Radio: Controls for the FM radio.
Hi. On Fri, 2010-08-20 at 14:04 +0200, ext pramodh ag wrote: Hello, +static ssize_t wl1273_fm_fops_write(struct file *file, const char __user *buf, +size_t count, loff_t *ppos) +{ +struct wl1273_device *radio = video_get_drvdata(video_devdata(file)); +u16 val; +int r; + +dev_dbg(radio-dev, %s\n, __func__); + +if (radio-core-mode != WL1273_MODE_TX) +return count; + +if (!radio-rds_on) { +dev_warn(radio-dev, %s: RDS not on.\n, __func__); +return 0; +} Aren't you planning to use extended controls V4L2_CID_RDS_TX_RADIO_TEXT, V4L2_CID_RDS_TX_PI, etc to handle FM TX RDS data? In principle yes, but we haven't yet decided to implement those now, at the moment the RDS interpretation is left completely to user space applications. Best Regards, Matti Thanks and regards, Pramodh -- 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: [PATCH v7 4/5] V4L2: WL1273 FM Radio: Controls for the FM radio.
Hello, +static ssize_t wl1273_fm_fops_write(struct file *file, const char __user *buf, + size_t count, loff_t *ppos) +{ + struct wl1273_device *radio = video_get_drvdata(video_devdata(file)); + u16 val; + int r; + + dev_dbg(radio-dev, %s\n, __func__); + + if (radio-core-mode != WL1273_MODE_TX) + return count; + + if (!radio-rds_on) { + dev_warn(radio-dev, %s: RDS not on.\n, __func__); + return 0; + } Aren't you planning to use extended controls V4L2_CID_RDS_TX_RADIO_TEXT, V4L2_CID_RDS_TX_PI, etc to handle FM TX RDS data? Thanks and regards, Pramodh -- 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 4/5] V4L2: WL1273 FM Radio: Controls for the FM radio.
Hello Alexey On Wed, 2010-08-11 at 10:06 +0200, ext Alexey Klimov wrote: + + radio = kzalloc(sizeof(*radio), GFP_KERNEL); + if (!radio) + return -ENOMEM; + + radio-write_buf = kmalloc(256, GFP_KERNEL); + 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? Yes you're right... et_drvdata(radio-videodev, radio); + platform_set_drvdata(pdev, radio); + + return 0; + +err_video_register: + v4l2_device_unregister(radio-v4l2dev); +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. Yes, I'll the the error handling here... Thanks, Matti -- Best regards, Klimov -- 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 4/5] V4L2: WL1273 FM Radio: Controls for the FM radio.
Matti, +/** + * wl1273_fm_set_tx_power() - Set the transmission power value. + * @core: A pointer to the device struct. + * @power: The new power value. + */ +static int wl1273_fm_set_tx_power(struct wl1273_core *core, u16 power) +{ + int r; + + if (core-mode == WL1273_MODE_OFF || + core-mode == WL1273_MODE_SUSPENDED) + return -EPERM; + + mutex_lock(core-lock); + + 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. Thanks and regards, Pramodh -- 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 4/5] V4L2: WL1273 FM Radio: Controls for the FM radio.
Hello. On Thu, 2010-08-12 at 14:10 +0200, ext pramodh ag wrote: Matti, +/** + * wl1273_fm_set_tx_power() -Set the transmission power value. + * @core:A pointer to the device struct. + * @power:The new power value. + */ +static int wl1273_fm_set_tx_power(struct wl1273_core *core, u16 power) +{ +int r; + +if (core-mode == WL1273_MODE_OFF || +core-mode == WL1273_MODE_SUSPENDED) +return -EPERM; + +mutex_lock(core-lock); + +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. Thank you for pointing that out. I'll check and fix it... Regards, Matti Thanks and regards, Pramodh -- 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 4/5] V4L2: WL1273 FM Radio: Controls for the FM radio.
Hello Alexey On Wed, 2010-08-11 at 10:06 +0200, ext Alexey Klimov wrote: + + radio = kzalloc(sizeof(*radio), GFP_KERNEL); + if (!radio) + return -ENOMEM; + + radio-write_buf = kmalloc(256, GFP_KERNEL); + 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? Yes you're right... et_drvdata(radio-videodev, radio); + platform_set_drvdata(pdev, radio); + + return 0; + +err_video_register: + v4l2_device_unregister(radio-v4l2dev); +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. Yes, I'll the the error handling here... 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 4/5] V4L2: WL1273 FM Radio: Controls for the FM radio.
Hi Matti, Just a few comments: On Monday 02 August 2010 16:06:42 Matti J. Aaltonen wrote: This driver implements V4L2 controls for the Texas Instruments WL1273 FM Radio. Signed-off-by: Matti J. Aaltonen matti.j.aalto...@nokia.com --- drivers/media/radio/Kconfig| 15 + drivers/media/radio/Makefile |1 + drivers/media/radio/radio-wl1273.c | 1972 drivers/mfd/Kconfig|6 + drivers/mfd/Makefile |2 + 5 files changed, 1996 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..209fd37 100644 --- a/drivers/media/radio/Kconfig +++ b/drivers/media/radio/Kconfig @@ -452,4 +452,19 @@ 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 SND + 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..9c7f01f --- /dev/null +++ b/drivers/media/radio/radio-wl1273.c @@ -0,0 +1,1972 @@ +/* + * Driver for the Texas Instruments WL1273 FM radio. + * + * Copyright (C) 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/mfd/wl1273-core.h +#include linux/slab.h +#include media/v4l2-device.h +#include media/v4l2-ioctl.h + +#define DRIVER_DESC Wl1273 FM Radio - V4L2 + +#define WL1273_POWER_SET_OFF 0 +#define WL1273_POWER_SET_FM (1 0) +#define WL1273_POWER_SET_RDS (1 1) +#define WL1273_POWER_SET_RETENTION (1 4) + +#define WL1273_PUPD_SET_OFF 0x00 +#define WL1273_PUPD_SET_ON 0x01 +#define WL1273_PUPD_SET_RETENTION0x10 + +#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 = -1; +module_param(radio_nr, int, 0); +MODULE_PARM_DESC(radio_nr, Radio Nr); + +struct wl1273_device { + struct v4l2_device v4l2dev; + struct video_device videodev; + struct device *dev; + struct wl1273_core *core; + struct file *owner; + char *write_buf; + bool rds_on; +}; + +static int wl1273_fm_set_tx_freq(struct wl1273_core *core, unsigned int freq) +{ + int r = 0; + + if (freq 76000) { + dev_err(core-i2c_dev-dev, + Frequency out of range: %d %d\n, + freq, core-bands[core-band].bottom_frequency); + return -EDOM; Use ERANGE instead of EDOM. EDOM is for math functions only. + } + + if (freq 108000) { + dev_err(core-i2c_dev-dev, + Frequency out of range: %d %d\n, + freq, core-bands[core-band].top_frequency); + return -EDOM; + } + + /* + * The driver works better with this msleep, + * the documentation doesn't mention it. + */ + msleep(5); +
[PATCH v7 4/5] V4L2: WL1273 FM Radio: Controls for the FM radio.
This driver implements V4L2 controls for the Texas Instruments WL1273 FM Radio. Signed-off-by: Matti J. Aaltonen matti.j.aalto...@nokia.com --- drivers/media/radio/Kconfig| 15 + drivers/media/radio/Makefile |1 + drivers/media/radio/radio-wl1273.c | 1972 drivers/mfd/Kconfig|6 + drivers/mfd/Makefile |2 + 5 files changed, 1996 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..209fd37 100644 --- a/drivers/media/radio/Kconfig +++ b/drivers/media/radio/Kconfig @@ -452,4 +452,19 @@ 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 SND + 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..9c7f01f --- /dev/null +++ b/drivers/media/radio/radio-wl1273.c @@ -0,0 +1,1972 @@ +/* + * Driver for the Texas Instruments WL1273 FM radio. + * + * Copyright (C) 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/mfd/wl1273-core.h +#include linux/slab.h +#include media/v4l2-device.h +#include media/v4l2-ioctl.h + +#define DRIVER_DESC Wl1273 FM Radio - V4L2 + +#define WL1273_POWER_SET_OFF 0 +#define WL1273_POWER_SET_FM(1 0) +#define WL1273_POWER_SET_RDS (1 1) +#define WL1273_POWER_SET_RETENTION (1 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 = -1; +module_param(radio_nr, int, 0); +MODULE_PARM_DESC(radio_nr, Radio Nr); + +struct wl1273_device { + struct v4l2_device v4l2dev; + struct video_device videodev; + struct device *dev; + struct wl1273_core *core; + struct file *owner; + char *write_buf; + bool rds_on; +}; + +static int wl1273_fm_set_tx_freq(struct wl1273_core *core, unsigned int freq) +{ + int r = 0; + + if (freq 76000) { + dev_err(core-i2c_dev-dev, + Frequency out of range: %d %d\n, + freq, core-bands[core-band].bottom_frequency); + return -EDOM; + } + + if (freq 108000) { + dev_err(core-i2c_dev-dev, + Frequency out of range: %d %d\n, + freq, core-bands[core-band].top_frequency); + return -EDOM; + } + + /* +* The driver works better with this msleep, +* the documentation doesn't mention it. +*/ + msleep(5); + dev_dbg(core-i2c_dev-dev, %s: freq: %d kHz\n, __func__, freq); + + INIT_COMPLETION(core-busy); + /* Set the current tx channel */ + r = wl1273_fm_write_cmd(core, WL1273_CHANL_SET, freq /