Re: [PATCH v7 4/5] V4L2: WL1273 FM Radio: Controls for the FM radio.

2010-08-28 Thread Hans Verkuil
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.

2010-08-23 Thread Matti J. Aaltonen
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.

2010-08-20 Thread pramodh ag
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.

2010-08-12 Thread Matti J. Aaltonen
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.

2010-08-12 Thread pramodh ag
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.

2010-08-12 Thread Matti J. Aaltonen
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.

2010-08-11 Thread Matti J. Aaltonen
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.

2010-08-09 Thread Hans Verkuil
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.

2010-08-02 Thread Matti J. Aaltonen
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 /