[PATCH 1/3] MFD: WL1273 FM Radio: MFD driver for the FM radio.
This is a parent driver for two child drivers: the V4L2 driver and
the ALSA codec driver. The MFD part provides the I2C communication
to the device and a couple of functions that are called from both
children.
Signed-off-by: Matti J. Aaltonen
---
drivers/mfd/Kconfig |6 +
drivers/mfd/Makefile|2 +
drivers/mfd/wl1273-core.c | 609 +++
include/linux/mfd/wl1273-core.h | 323 +
4 files changed, 940 insertions(+), 0 deletions(-)
create mode 100644 drivers/mfd/wl1273-core.c
create mode 100644 include/linux/mfd/wl1273-core.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 413576a..c16d500 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -135,6 +135,12 @@ config TWL4030_CODEC
select MFD_CORE
default n
+config WL1273_CORE
+ tristate
+ depends on I2C
+ select MFD_CORE
+ default n
+
config MFD_TMIO
bool
default n
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 78295d6..46e611d 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -30,6 +30,8 @@ obj-$(CONFIG_TWL4030_CORE)+= twl-core.o twl4030-irq.o
twl6030-irq.o
obj-$(CONFIG_TWL4030_POWER)+= twl4030-power.o
obj-$(CONFIG_TWL4030_CODEC)+= twl4030-codec.o
+obj-$(CONFIG_WL1273_CORE) += wl1273-core.o
+
obj-$(CONFIG_MFD_MC13783) += mc13783-core.o
obj-$(CONFIG_MFD_CORE) += mfd-core.o
diff --git a/drivers/mfd/wl1273-core.c b/drivers/mfd/wl1273-core.c
new file mode 100644
index 000..efe8039
--- /dev/null
+++ b/drivers/mfd/wl1273-core.c
@@ -0,0 +1,609 @@
+/*
+ * MFD driver for wl1273 FM radio and audio codec submodules.
+ *
+ * Author: Matti Aaltonen
+ *
+ * Copyright: (C) 2010 Nokia Corporation
+ *
+ * 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., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#undef DEBUG
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DRIVER_DESC "WL1273 FM Radio Core"
+
+#define WL1273_IRQ_MASK (WL1273_FR_EVENT | \
+ WL1273_POW_ENB_EVENT)
+
+static const struct region_info regions[] = {
+ /* Japan */
+ {
+ .bottom_frequency = 76000,
+ .top_frequency = 9,
+ .region = 0,
+ },
+ /* USA & Europe */
+ {
+ .bottom_frequency = 87500,
+ .top_frequency = 108000,
+ .region = 1,
+ },
+};
+
+/*
+ * static unsigned char radio_region - Region
+ *
+ * The regions are 0=Japan, 1=USA-Europe. USA-Europe is the default.
+ */
+static unsigned char radio_region = 1;
+module_param(radio_region, byte, 0);
+MODULE_PARM_DESC(radio_region, "Region: 0=Japan, 1=USA-Europe*");
+
+/*
+ * static unsigned int rds_buf - the number of RDS buffer blocks used.
+ *
+ * The default number is 100.
+ */
+static unsigned int rds_buf = 100;
+module_param(rds_buf, uint, 0);
+MODULE_PARM_DESC(rds_buf, "RDS buffer entries: *100*");
+
+int wl1273_fm_read_reg(struct wl1273_core *core, u8 reg, u16 *value)
+{
+ struct i2c_client *client = core->i2c_dev;
+ u8 b[2];
+ int r;
+
+ r = i2c_smbus_read_i2c_block_data(client, reg, 2, 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;
+}
+EXPORT_SYMBOL(wl1273_fm_read_reg);
+
+int wl1273_fm_write_cmd(struct wl1273_core *core, u8 cmd, u16 param)
+{
+ struct i2c_client *client = core->i2c_dev;
+ u8 buf[] = { (param >> 8) & 0xff, param & 0xff };
+ int r;
+
+ r = i2c_smbus_write_i2c_block_data(client, cmd, 2, buf);
+ if (r) {
+ dev_err(&client->dev, "%s: Cmd: %d fails.\n", __func__, cmd);
+ return r;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(wl1273_fm_write_cmd);
+
+int wl1273_fm_write_data(struct wl1273_core *core, u8 *data, u16 len)
+{
+ struct i2c_client *client = core->i2c_dev;
+ struct i2c_msg msg[1];
+ int r;
+
+ msg[0].addr = client->addr;
+ msg[0].flags = 0;
+ msg[0].buf = data;
+ msg[0].len = len;
+
+ r =
Re: [PATCH 1/3] MFD: WL1273 FM Radio: MFD driver for the FM radio.
On Wed, 21 Apr 2010 12:29:00 +0300 m7aalton wrote: > > So I was taking a quick look at this; it mostly looks OK (though I wonder > > about all those symbol exports - does all that stuff need to be in the > > Some functions get called from both child drivers/modules, but some > stuff could probably be moved from the core to either of the children. > Should I actually do that? Depends. If it's truly only useful to a single child device, the code probably belongs there, without an export. If it's truly a core function, in that (1) it's applicable to multiple devices, or (2) it can't be implemented without exposing stuff you want to keep private to the core, then it should stay in the core. > > What I would suggest you do is remove the completion in favor of a wait > > queue which the interrupt handler can use to signal that something has > > completed. You can then use wait_event() to wait for a wakeup and test > > that the specific condition you are waiting for has come to pass. > > Do you agree with my explanation? Or should I switch to using wait > queue? My belief is that the code would be cleaner with a wait queue; that's the normal pattern for implementing this kind of logic. I'll stop here, though; if others want to take it upstream with the completion, I'll not scream about it. Thanks, jon -- 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 1/3] MFD: WL1273 FM Radio: MFD driver for the FM radio.
Hello.
Thanks for the comments.
On Tue, 2010-04-20 at 21:33 +0200, ext Jonathan Corbet wrote:
> On Tue, 20 Apr 2010 18:20:05 +0300
> "Matti J. Aaltonen" wrote:
>
> > This is a parent driver for two child drivers: the V4L2 driver and
> > the ALSA codec driver. The MFD part provides the I2C communication
> > to the device and the state handling.
>
> So I was taking a quick look at this; it mostly looks OK (though I wonder
> about all those symbol exports - does all that stuff need to be in the
Some functions get called from both child drivers/modules, but some
stuff could probably be moved from the core to either of the children.
Should I actually do that?
> core?). One thing caught my eye, though:
>
> > +int wl1273_fm_rds_off(struct wl1273_core *core)
> > +{
> > + struct device *dev = &core->i2c_dev->dev;
> > + int r;
> > +
> > + if (core->mode == WL1273_MODE_TX)
> > + return 0;
> > +
> > + wait_for_completion_timeout(&core->busy, msecs_to_jiffies(1000));
> The use of a completion here looks like a mistake to me. This isn't a
> normal pattern, and I'm not quite sure what you're trying to do. Here,
> also, you're ignoring the return value, so you don't know if completion
> was signaled or not.
Yes that wait_for_completion is a mistake.
>
> If you look in functions like:
>
> > +int wl1273_fm_set_rx_freq(struct wl1273_core *core, unsigned int freq)
> > +{
>
> [...]
>
> > + INIT_COMPLETION(core->busy);
> > + r = wl1273_fm_write_cmd(core, WL1273_TUNER_MODE_SET,
> > + TUNER_MODE_PRESET);
> > + if (r) {
> > + complete(&core->busy);
> > + goto err;
> > + }
>
> What will prevent multiple threads from doing INIT_COMPLETION()
> simultaneously? It looks racy to me. What happens if multiple
> threads try to wait simultaneously on the completion?
>
> OK, looking further, you're not using it for mutual exclusion. In fact,
> you're not using anything for mutual exclusion; how do you prevent
> concurrent access to the hardware registers?
I have mutexes everywhere that function is called from. My aim was to
have the mutexes in the interface functions. So that the radio use is
serialized on as high a level as possible.
> What I would suggest you do is remove the completion in favor of a wait
> queue which the interrupt handler can use to signal that something has
> completed. You can then use wait_event() to wait for a wakeup and test
> that the specific condition you are waiting for has come to pass.
Do you agree with my explanation? Or should I switch to using wait
queue?
Cheers,
Matti
> jon
--
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 1/3] MFD: WL1273 FM Radio: MFD driver for the FM radio.
On Tue, 20 Apr 2010 18:20:05 +0300
"Matti J. Aaltonen" wrote:
> This is a parent driver for two child drivers: the V4L2 driver and
> the ALSA codec driver. The MFD part provides the I2C communication
> to the device and the state handling.
So I was taking a quick look at this; it mostly looks OK (though I wonder
about all those symbol exports - does all that stuff need to be in the
core?). One thing caught my eye, though:
> +int wl1273_fm_rds_off(struct wl1273_core *core)
> +{
> + struct device *dev = &core->i2c_dev->dev;
> + int r;
> +
> + if (core->mode == WL1273_MODE_TX)
> + return 0;
> +
> + wait_for_completion_timeout(&core->busy, msecs_to_jiffies(1000));
The use of a completion here looks like a mistake to me. This isn't a
normal pattern, and I'm not quite sure what you're trying to do. Here,
also, you're ignoring the return value, so you don't know if completion
was signaled or not.
If you look in functions like:
> +int wl1273_fm_set_rx_freq(struct wl1273_core *core, unsigned int freq)
> +{
[...]
> + INIT_COMPLETION(core->busy);
> + r = wl1273_fm_write_cmd(core, WL1273_TUNER_MODE_SET,
> + TUNER_MODE_PRESET);
> + if (r) {
> + complete(&core->busy);
> + goto err;
> + }
What will prevent multiple threads from doing INIT_COMPLETION()
simultaneously? It looks racy to me. What happens if multiple
threads try to wait simultaneously on the completion?
OK, looking further, you're not using it for mutual exclusion. In fact,
you're not using anything for mutual exclusion; how do you prevent
concurrent access to the hardware registers?
What I would suggest you do is remove the completion in favor of a wait
queue which the interrupt handler can use to signal that something has
completed. You can then use wait_event() to wait for a wakeup and test
that the specific condition you are waiting for has come to pass.
jon
--
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 1/3] MFD: WL1273 FM Radio: MFD driver for the FM radio.
This is a parent driver for two child drivers: the V4L2 driver and the ALSA codec driver. The MFD part provides the I2C communication to the device and the state handling. Signed-off-by: Matti J. Aaltonen --- drivers/mfd/Kconfig |6 + drivers/mfd/Makefile|2 + drivers/mfd/wl1273-core.c | 1825 +++ include/linux/mfd/wl1273-core.h | 265 ++ 4 files changed, 2098 insertions(+), 0 deletions(-) create mode 100644 drivers/mfd/wl1273-core.c create mode 100644 include/linux/mfd/wl1273-core.h diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 413576a..c16d500 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -135,6 +135,12 @@ config TWL4030_CODEC select MFD_CORE default n +config WL1273_CORE + tristate + depends on I2C + select MFD_CORE + default n + config MFD_TMIO bool default n diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 16a349f..5381cd3 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -30,6 +30,8 @@ obj-$(CONFIG_TWL4030_CORE)+= twl-core.o twl4030-irq.o twl6030-irq.o obj-$(CONFIG_TWL4030_POWER)+= twl4030-power.o obj-$(CONFIG_TWL4030_CODEC)+= twl4030-codec.o +obj-$(CONFIG_WL1273_CORE) += wl1273-core.o + obj-$(CONFIG_MFD_MC13783) += mc13783-core.o obj-$(CONFIG_MFD_CORE) += mfd-core.o diff --git a/drivers/mfd/wl1273-core.c b/drivers/mfd/wl1273-core.c new file mode 100644 index 000..77b9442 --- /dev/null +++ b/drivers/mfd/wl1273-core.c @@ -0,0 +1,1825 @@ +/* + * MFD driver for wl1273 FM radio and audio codec submodules. + * + * Author: Matti Aaltonen + * + * Copyright: (C) 2010 Nokia Corporation + * + * 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., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA + * + */ + +#undef DEBUG + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define DRIVER_DESC "WL1273 FM Radio Core" + +#define WL1273_FR_EVENT(1 << 0) +#define WL1273_BL_EVENT(1 << 1) +#define WL1273_RDS_EVENT (1 << 2) +#define WL1273_BBLK_EVENT (1 << 3) +#define WL1273_LSYNC_EVENT (1 << 4) +#define WL1273_LEV_EVENT (1 << 5) +#define WL1273_IFFR_EVENT (1 << 6) +#define WL1273_PI_EVENT(1 << 7) +#define WL1273_PD_EVENT(1 << 8) +#define WL1273_STIC_EVENT (1 << 9) +#define WL1273_MAL_EVENT (1 << 10) +#define WL1273_POW_ENB_EVENT (1 << 11) +#define WL1273_SCAN_OVER_EVENT (1 << 12) +#define WL1273_ERROR_EVENT (1 << 13) + +#define WL1273_IRQ_MASK (WL1273_FR_EVENT | \ + WL1273_POW_ENB_EVENT) + +/* I2S protocol, left channel first, data width 16 bits */ +#define WL1273_PCM_DEF_MODE0x00 + +/* Rx */ +#define WL1273_AUDIO_ENABLE_I2S(1 << 0) +#define WL1273_AUDIO_ENABLE_ANALOG (1 << 1) + +/* Tx */ +#define WL1273_AUDIO_IO_SET_ANALOG 0 +#define WL1273_AUDIO_IO_SET_I2S1 + +#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 TUNER_MODE_STOP_SEARCH 0 +#define TUNER_MODE_PRESET 1 +#define TUNER_MODE_AUTO_SEEK 2 +#define TUNER_MODE_AF 3 + +/* I2S mode */ +#define WL1273_IS2_WIDTH_320x0 +#define WL1273_IS2_WIDTH_400x1 +#define WL1273_IS2_WIDTH_22_23 0x2 +#define WL1273_IS2_WIDTH_23_22 0x3 +#define WL1273_IS2_WIDTH_480x4 +#define WL1273_IS2_WIDTH_500x5 +#define WL1273_IS2_WIDTH_600x6 +#define WL1273_IS2_WIDTH_640x7 +#define WL1273_IS2_WIDTH_800x8 +#define WL1273_IS2_WIDTH_960x9 +#define WL1273_IS2_WIDTH_128 0xa +#define WL1273_IS2_WIDTH 0xf + +#define WL1273_IS2_FORMAT_STD (0x0 << 4) +#define WL1273_IS2_FORMAT_LEFT (0x1 << 4) +#define WL1273_IS2_FORMAT_RIGHT(0x2 << 4) +#define WL1273_IS2_FORMAT_USER (0x3 << 4) + +#define WL1273_IS2_MASTER (0x0 << 6) +#define WL1273_IS2_SLAV
