Re: [PATCH 1/4] MFD: twl4030: add twl4030_codec MFD as a new child to the core
On Thursday 22 October 2009 13:55:17 ext Mark Brown wrote: > On Thu, Oct 22, 2009 at 09:57:49AM +0200, Samuel Ortiz wrote: > > It seems to me that this patchset is mostly an asoc one, even though all > > of those patches depend on the MFD one. So I'd perfectly fine if they'd > > all go through Mark's tree, and then I'd have to make sure I'm sending my > > 2.6.33 merge window pull request _after_ Mark's code is in Linus tree. > > Once it's there, I can work on merging conflicts with the few > > twl4030-core pending patches from my tree. Mark, what do you think ? > > Seems reasonable. I'll apply them on a separate branch and merge them > into my 2.6.33 branch so that you can merge them into your tree too and > avoid any cross-tree issues without needing to think about how things > get merged with Linus (though since Takashi generally merges early it'll > probably happen naturally anyway). Note that if you do pull this branch > you won't be able to rebase your tree, you'd have to merge up Linus' > tree instead (but that'd be handy anyway :) ). I just sent the second series, I'll hope the I have not missed anything. I was scratching my head on which tree it should be going, but it is really nice to see that it can be done like this. Thank you Samuel, Mark and Tony! -- Péter -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/4] MFD: twl4030: add twl4030_codec MFD as a new child to the core
On Thu, Oct 22, 2009 at 09:57:49AM +0200, Samuel Ortiz wrote: > It seems to me that this patchset is mostly an asoc one, even though all of > those patches depend on the MFD one. So I'd perfectly fine if they'd all go > through Mark's tree, and then I'd have to make sure I'm sending my 2.6.33 > merge window pull request _after_ Mark's code is in Linus tree. Once it's > there, I can work on merging conflicts with the few twl4030-core pending > patches from my tree. Mark, what do you think ? Seems reasonable. I'll apply them on a separate branch and merge them into my 2.6.33 branch so that you can merge them into your tree too and avoid any cross-tree issues without needing to think about how things get merged with Linus (though since Takashi generally merges early it'll probably happen naturally anyway). Note that if you do pull this branch you won't be able to rebase your tree, you'd have to merge up Linus' tree instead (but that'd be handy anyway :) ). -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/4] MFD: twl4030: add twl4030_codec MFD as a new child to the core
On Thu, Oct 22, 2009 at 09:04:32AM +0300, Peter Ujfalusi wrote: > I'll check, if the MFD patch applies to mfd-2.6:for-next also, but to have > the > soc codec changes the MFD patch should go to the sound-2.6 tree as well to > make > sure it is not braking things. > > All-in-all, how these things can be handled? The OMAP patch has been acked by Tony. Then we I'm fine with the mfd one and Mark is also ok with the remaining asoc one, all 3 patches have to go through one single tree. It seems to me that this patchset is mostly an asoc one, even though all of those patches depend on the MFD one. So I'd perfectly fine if they'd all go through Mark's tree, and then I'd have to make sure I'm sending my 2.6.33 merge window pull request _after_ Mark's code is in Linus tree. Once it's there, I can work on merging conflicts with the few twl4030-core pending patches from my tree. Mark, what do you think ? Cheers, Samuel. > Thanks, > Péter -- Intel Open Source Technology Centre http://oss.intel.com/ -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/4] MFD: twl4030: add twl4030_codec MFD as a new child to the core
On Thursday 22 October 2009 02:13:11 ext Samuel Ortiz wrote:
> As Mark noticed already, you dont really want users to explicitely select
> this obscure mfd driver to get their audio and vibre driver selectable. It
> should be the other way around, and I think you already agreed with that.
Yes, I have already made the modification.
>
> > +static struct device *
> > +twl4030_codec_new_child(const char *name, void *pdata, unsigned
> > pdata_len) +{
...
> This could really use the mfd-core API, and avoid duplicating code.
> You just would have to define a couple cells, and call mfd_add_devices on
> them.
Good point. When I wrote this driver I have been looking at the twl4030-core
driver, which is not using the mfd-core API. I'll make the change.
>
> > +static int __devexit twl4030_codec_remove(struct platform_device *pdev)
> > +{
> > + struct twl4030_codec *codec = platform_get_drvdata(pdev);
> > +
> > + platform_set_drvdata(pdev, NULL);
> > + kfree(codec);
> > + twl4030_codec_dev = NULL;
> > +
> > + return 0;
> > +}
>
> I think you're missing a platform_device_unregister() here (or an
> mfd_remove_devices() if you're going to switch to the mfd-core API)
You are right, although I have used the bool in the Kconfig for the twl4030-
codec in a same way as the twl4030-core (and the core did not unregister the
devices either), but yes, this is not correct so I'll fix it.
I have now question about the practicalities on how this series would be taken,
and via which tree.
The final patch for the soc codec driver depends on the change in MFD and in
the
OMAP board files.
The change in the OMAP board files depends on the MFD changes, obviously.
So in order to have the soc codec changes the MFD and OMAP part has to be
applied before, otherwise the audio will be broken.
the mfd-2.6:for-next branch has some patches against the twl4030-core in
addition to the ones in l-o and in sound-2.6:topic/asoc.
I'll check, if the MFD patch applies to mfd-2.6:for-next also, but to have the
soc codec changes the MFD patch should go to the sound-2.6 tree as well to make
sure it is not braking things.
All-in-all, how these things can be handled?
Thanks,
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/4] MFD: twl4030: add twl4030_codec MFD as a new child to the core
Hi Peter,
On Mon, Oct 19, 2009 at 03:42:17PM +0300, Peter Ujfalusi wrote:
> New MFD child to twl4030 MFD device.
> This MFD device will be used by the drivers, which needs resources
> from the twl4030 codec like audio and vibra.
>
> The platform specific configuration data is passed along to the
> child drivers (audio, vibra).
Some comments on your code:
> +config TWL4030_CODEC
> + bool "Support codec part of the TWL4030 family chips"
> + depends on TWL4030_CORE
> + help
> + Say yes here if you want to use the codec resources on the
> + TWL4030 family chips. The codec in TWL4030 provides the audio
> + functionality and also has the vibrator controls.
> +
> + This driver provides MFD device to be used for drivers, which needs
> + access to the codec part, the board-specific data is passed from this
> + driver to the childs as platform data with the board specific
> + configuration.
> +
As Mark noticed already, you dont really want users to explicitely select this
obscure mfd driver to get their audio and vibre driver selectable. It should
be the other way around, and I think you already agreed with that.
> +static struct device *
> +twl4030_codec_new_child(const char *name, void *pdata, unsigned pdata_len)
> +{
> + struct platform_device *pdev;
> + int status = 0;
> +
> + pdev = platform_device_alloc(name, -1);
> + if (!pdev) {
> + dev_dbg(&twl4030_codec_dev->dev, "can't alloc dev (%s)\n",
> + name);
> + return ERR_PTR(-ENOMEM);
> + }
> + pdev->dev.parent = &twl4030_codec_dev->dev;
> +
> + if (pdata) {
> + status = platform_device_add_data(pdev, pdata, pdata_len);
> + if (status < 0) {
> + dev_dbg(&pdev->dev, "can't add platform_data\n");
> + goto err;
> + }
> + }
> + status = platform_device_add(pdev);
> + if (status < 0) {
> + dev_dbg(&pdev->dev, "Adding platform device failed\n");
> + goto err;
> + }
> +
> + return &pdev->dev;
> +err:
> + platform_device_put(pdev);
> + dev_err(&twl4030_codec_dev->dev, "can't add %s dev\n", name);
> + return ERR_PTR(status);
> +}
This could really use the mfd-core API, and avoid duplicating code.
You just would have to define a couple cells, and call mfd_add_devices on
them.
> +static int __devexit twl4030_codec_remove(struct platform_device *pdev)
> +{
> + struct twl4030_codec *codec = platform_get_drvdata(pdev);
> +
> + platform_set_drvdata(pdev, NULL);
> + kfree(codec);
> + twl4030_codec_dev = NULL;
> +
> + return 0;
> +}
I think you're missing a platform_device_unregister() here (or an
mfd_remove_devices() if you're going to switch to the mfd-core API)
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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] [PATCH 1/4] MFD: twl4030: add twl4030_codec MFD as a new child to the core
On Tuesday 20 October 2009 13:03:34 ext Mark Brown wrote: > On Mon, Oct 19, 2009 at 03:42:17PM +0300, Peter Ujfalusi wrote: > > New MFD child to twl4030 MFD device. > > This MFD device will be used by the drivers, which needs resources > > from the twl4030 codec like audio and vibra. > > > > The platform specific configuration data is passed along to the > > child drivers (audio, vibra). > > > > Signed-off-by: Peter Ujfalusi > > > > +config TWL4030_CODEC > > + bool "Support codec part of the TWL4030 family chips" > > This seems like something that users shouldn't really have to worry > about selecting explicitly - might it be better to have it selected by > the drivers that use it instead? It feels like it's an implementation > detail of the driver. Yes, first I had the TWL4030_CODEC selected in the sound/soc/codecs/Kconfig, but for some reason I have decided to add user selectable config option instead and use the depends on in the codecs Makefile. I'll make the change as you suggested. -- Péter -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/4] MFD: twl4030: add twl4030_codec MFD as a new child to the core
On Mon, Oct 19, 2009 at 03:42:17PM +0300, Peter Ujfalusi wrote: > New MFD child to twl4030 MFD device. > This MFD device will be used by the drivers, which needs resources > from the twl4030 codec like audio and vibra. > The platform specific configuration data is passed along to the > child drivers (audio, vibra). > Signed-off-by: Peter Ujfalusi > +config TWL4030_CODEC > + bool "Support codec part of the TWL4030 family chips" This seems like something that users shouldn't really have to worry about selecting explicitly - might it be better to have it selected by the drivers that use it instead? It feels like it's an implementation detail of the driver. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] MFD: twl4030: add twl4030_codec MFD as a new child to the core
New MFD child to twl4030 MFD device.
This MFD device will be used by the drivers, which needs resources
from the twl4030 codec like audio and vibra.
The platform specific configuration data is passed along to the
child drivers (audio, vibra).
Signed-off-by: Peter Ujfalusi
---
drivers/mfd/Kconfig | 13 ++
drivers/mfd/Makefile |1 +
drivers/mfd/twl4030-codec.c | 245 ++
drivers/mfd/twl4030-core.c| 14 ++
include/linux/i2c/twl4030.h | 18 +++
include/linux/mfd/twl4030-codec.h | 261 +
6 files changed, 552 insertions(+), 0 deletions(-)
create mode 100644 drivers/mfd/twl4030-codec.c
create mode 100644 include/linux/mfd/twl4030-codec.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 570be13..08e4aa6 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -121,6 +121,19 @@ config TWL4030_POWER
and load scripts controling which resources are switched off/on
or reset when a sleep, wakeup or warm reset event occurs.
+config TWL4030_CODEC
+ bool "Support codec part of the TWL4030 family chips"
+ depends on TWL4030_CORE
+ help
+ Say yes here if you want to use the codec resources on the
+ TWL4030 family chips. The codec in TWL4030 provides the audio
+ functionality and also has the vibrator controls.
+
+ This driver provides MFD device to be used for drivers, which needs
+ access to the codec part, the board-specific data is passed from this
+ driver to the childs as platform data with the board specific
+ configuration.
+
config MFD_TMIO
bool
default n
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f3b277b..af0fc90 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_MENELAUS)+= menelaus.o
obj-$(CONFIG_TWL4030_CORE) += twl4030-core.o twl4030-irq.o
obj-$(CONFIG_TWL4030_POWER)+= twl4030-power.o
+obj-$(CONFIG_TWL4030_CODEC)+= twl4030-codec.o
obj-$(CONFIG_MFD_MC13783) += mc13783-core.o
diff --git a/drivers/mfd/twl4030-codec.c b/drivers/mfd/twl4030-codec.c
new file mode 100644
index 000..a459052
--- /dev/null
+++ b/drivers/mfd/twl4030-codec.c
@@ -0,0 +1,245 @@
+/*
+ * MFD driver for twl4030 codec submodule
+ *
+ * Author: Peter Ujfalusi
+ *
+ * Copyright: (C) 2009 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
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static struct platform_device *twl4030_codec_dev;
+
+struct twl4030_codec_resource {
+ int request_count;
+ u8 reg;
+ u8 mask;
+};
+
+struct twl4030_codec {
+ struct mutex mutex;
+ struct twl4030_codec_resource resource[TWL4030_CODEC_RES_MAX];
+};
+
+/*
+ * Modify the resource, the function returns the content of the register
+ * after the modification.
+ */
+static int twl4030_codec_set_resource(enum twl4030_codec_res id, int enable)
+{
+ struct twl4030_codec *codec = platform_get_drvdata(twl4030_codec_dev);
+ u8 val;
+
+ twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &val,
+ codec->resource[id].reg);
+
+ if (enable)
+ val |= codec->resource[id].mask;
+ else
+ val &= ~codec->resource[id].mask;
+
+ twl4030_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE,
+ val, codec->resource[id].reg);
+
+ return val;
+}
+
+static inline int twl4030_codec_get_resource(enum twl4030_codec_res id)
+{
+ struct twl4030_codec *codec = platform_get_drvdata(twl4030_codec_dev);
+ u8 val;
+
+ twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &val,
+ codec->resource[id].reg);
+
+ return val;
+}
+
+/*
+ * Enable the resource.
+ * The function returns with error or the content of the register
+ */
+int twl4030_codec_enable_resource(enum twl4030_codec_res id)
+{
+ struct twl4030_codec *codec = platform_get_drvdata(twl4030_codec_dev);
+ int val;
+
+ if (id >= TWL4030_CODEC_RES_MAX) {
+ dev_err(&twl4030_codec_dev->dev,
+ "Invalid resource ID (%u)\n", id);
+ return -EINVAL;
+ }
+
+ mutex
