[PATCH 03/22] ASoC: ab8500: Inform SoC Core that we have our own I/O arrangements

[PATCH 03/22] ASoC: ab8500: Inform SoC Core that we have our own I/O arrangements

[Lee Jones]

If codec->control_data is not populated SoC Core assumes we want to
use regmap, which fails catastrophically, as we don't have one:

Unable to handle kernel NULL pointer dereference at virtual address 00000080
pgd = c0004000
[00000080] *pgd=00000000
Internal error: Oops: 17 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 1    Not tainted  (3.5.0-rc6-00884-g0b2419e-dirty #130)
PC is at regmap_read+0x10/0x5c
LR is at hw_read+0x80/0x90
pc : [<c01a91b8>]    lr : [<c0216804>]    psr: 60000013

CC: alsa-devel@alsa-project.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 sound/soc/codecs/ab8500-codec.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/soc/codecs/ab8500-codec.c b/sound/soc/codecs/ab8500-codec.c
index 3c79592..23b4018 100644
--- a/sound/soc/codecs/ab8500-codec.c
+++ b/sound/soc/codecs/ab8500-codec.c
@@ -2406,6 +2406,10 @@ static int ab8500_codec_probe(struct snd_soc_codec *codec)
 
 	/* Setup AB8500 according to board-settings */
 	pdata = (struct ab8500_platform_data *)dev_get_platdata(dev->parent);
+
+	/* Inform SoC Core that we have our own I/O arrangements. */
+	codec->control_data = (void *)true;
+
 	status = ab8500_audio_setup_mics(codec, &pdata->codec->amics);
 	if (status < 0) {
 		pr_err("%s: Failed to setup mics (%d)!\n", __func__, status);
-- 
1.7.9.5

Re: [PATCH 03/22] ASoC: ab8500: Inform SoC Core that we have our own I/O arrangements

[Linus Walleij]

On Thu, Aug 9, 2012 at 5:47 PM, Lee Jones <lee.jones@linaro.org> wrote:

> If codec->control_data is not populated SoC Core assumes we want to
> use regmap, which fails catastrophically, as we don't have one:
>
> Unable to handle kernel NULL pointer dereference at virtual address 00000080
> pgd = c0004000
> [00000080] *pgd=00000000
> Internal error: Oops: 17 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 1    Not tainted  (3.5.0-rc6-00884-g0b2419e-dirty #130)
> PC is at regmap_read+0x10/0x5c
> LR is at hw_read+0x80/0x90
> pc : [<c01a91b8>]    lr : [<c0216804>]    psr: 60000013
>
> CC: alsa-devel@alsa-project.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

No clue on this one, have to refer to Mark...

Linus Walleij

Re: [PATCH 03/22] ASoC: ab8500: Inform SoC Core that we have our own I/O arrangements

[Takashi Iwai]

At Thu,  9 Aug 2012 16:47:29 +0100,
Lee Jones wrote:
> 
> If codec->control_data is not populated SoC Core assumes we want to
> use regmap, which fails catastrophically, as we don't have one:
> 
> Unable to handle kernel NULL pointer dereference at virtual address 00000080
> pgd = c0004000
> [00000080] *pgd=00000000
> Internal error: Oops: 17 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 1    Not tainted  (3.5.0-rc6-00884-g0b2419e-dirty #130)
> PC is at regmap_read+0x10/0x5c
> LR is at hw_read+0x80/0x90
> pc : [<c01a91b8>]    lr : [<c0216804>]    psr: 60000013
> 
> CC: alsa-devel@alsa-project.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

This was already fixed in ASoC core on Linus tree...


Takashi

> ---
>  sound/soc/codecs/ab8500-codec.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/sound/soc/codecs/ab8500-codec.c b/sound/soc/codecs/ab8500-codec.c
> index 3c79592..23b4018 100644
> --- a/sound/soc/codecs/ab8500-codec.c
> +++ b/sound/soc/codecs/ab8500-codec.c
> @@ -2406,6 +2406,10 @@ static int ab8500_codec_probe(struct snd_soc_codec *codec)
>  
>  	/* Setup AB8500 according to board-settings */
>  	pdata = (struct ab8500_platform_data *)dev_get_platdata(dev->parent);
> +
> +	/* Inform SoC Core that we have our own I/O arrangements. */
> +	codec->control_data = (void *)true;
> +
>  	status = ab8500_audio_setup_mics(codec, &pdata->codec->amics);
>  	if (status < 0) {
>  		pr_err("%s: Failed to setup mics (%d)!\n", __func__, status);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [PATCH 03/22] ASoC: ab8500: Inform SoC Core that we have our own I/O arrangements

[Mark Brown]

On Thu, Aug 09, 2012 at 04:47:29PM +0100, Lee Jones wrote:
> If codec->control_data is not populated SoC Core assumes we want to
> use regmap, which fails catastrophically, as we don't have one:

*Always* submit code against the current code for the subsystem.

[PATCH 04/22] ASoC: Ux500: Move MSP pinctrl setup into the MSP driver

[Lee Jones]

In the initial submission of the MSP driver msp1 and msp3's associated
pinctrl mechanism was passed back to platform code using a plat_init()
call-back routine, but it has no place in platform code. The MSP driver
should set this up for the appropriate ports. Instead we use a use_pinctrl
identifier which is passed from platform_data/Device Tree which indicates
which ports should use pinctrl.

CC: alsa-devel@alsa-project.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-ux500/board-mop500-msp.c |   75 +-------------------------------
 arch/arm/mach-ux500/include/mach/msp.h |    2 -
 sound/soc/ux500/ux500_msp_i2s.c        |   75 +++++++++++++++++++++++++-------
 sound/soc/ux500/ux500_msp_i2s.h        |    2 -
 4 files changed, 60 insertions(+), 94 deletions(-)

diff --git a/arch/arm/mach-ux500/board-mop500-msp.c b/arch/arm/mach-ux500/board-mop500-msp.c
index df15646..fc78d5d 100644
--- a/arch/arm/mach-ux500/board-mop500-msp.c
+++ b/arch/arm/mach-ux500/board-mop500-msp.c
@@ -23,53 +23,6 @@
 #include "devices-db8500.h"
 #include "pins-db8500.h"
 
-/* MSP1/3 Tx/Rx usage protection */
-static DEFINE_SPINLOCK(msp_rxtx_lock);
-
-/* Reference Count */
-static int msp_rxtx_ref;
-
-/* Pin modes */
-struct pinctrl *msp1_p;
-struct pinctrl_state *msp1_def;
-struct pinctrl_state *msp1_sleep;
-
-int msp13_i2s_init(void)
-{
-	int retval = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(&msp_rxtx_lock, flags);
-	if (msp_rxtx_ref == 0 && !(IS_ERR(msp1_p) || IS_ERR(msp1_def))) {
-		retval = pinctrl_select_state(msp1_p, msp1_def);
-		if (retval)
-			pr_err("could not set MSP1 defstate\n");
-	}
-	if (!retval)
-		msp_rxtx_ref++;
-	spin_unlock_irqrestore(&msp_rxtx_lock, flags);
-
-	return retval;
-}
-
-int msp13_i2s_exit(void)
-{
-	int retval = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(&msp_rxtx_lock, flags);
-	WARN_ON(!msp_rxtx_ref);
-	msp_rxtx_ref--;
-	if (msp_rxtx_ref == 0 && !(IS_ERR(msp1_p) || IS_ERR(msp1_sleep))) {
-		retval = pinctrl_select_state(msp1_p, msp1_sleep);
-		if (retval)
-			pr_err("could not set MSP1 sleepstate\n");
-	}
-	spin_unlock_irqrestore(&msp_rxtx_lock, flags);
-
-	return retval;
-}
-
 static struct stedma40_chan_cfg msp0_dma_rx = {
 	.high_priority = true,
 	.dir = STEDMA40_PERIPH_TO_MEM,
@@ -132,8 +85,6 @@ static struct msp_i2s_platform_data msp1_platform_data = {
 	.id = MSP_I2S_1,
 	.msp_i2s_dma_rx = NULL,
 	.msp_i2s_dma_tx = &msp1_dma_tx,
-	.msp_i2s_init = msp13_i2s_init,
-	.msp_i2s_exit = msp13_i2s_exit,
 };
 
 static struct stedma40_chan_cfg msp2_dma_rx = {
@@ -219,47 +170,23 @@ static struct msp_i2s_platform_data msp3_platform_data = {
 	.id		= MSP_I2S_3,
 	.msp_i2s_dma_rx	= &msp1_dma_rx,
 	.msp_i2s_dma_tx	= NULL,
-	.msp_i2s_init = msp13_i2s_init,
-	.msp_i2s_exit = msp13_i2s_exit,
 };
 
 int mop500_msp_init(struct device *parent)
 {
-	struct platform_device *msp1;
-
 	pr_info("%s: Register platform-device 'snd-soc-mop500'.\n", __func__);
 	platform_device_register(&snd_soc_mop500);
 
 	pr_info("Initialize MSP I2S-devices.\n");
 	db8500_add_msp_i2s(parent, 0, U8500_MSP0_BASE, IRQ_DB8500_MSP0,
 			   &msp0_platform_data);
-	msp1 = db8500_add_msp_i2s(parent, 1, U8500_MSP1_BASE, IRQ_DB8500_MSP1,
+	db8500_add_msp_i2s(parent, 1, U8500_MSP1_BASE, IRQ_DB8500_MSP1,
 			   &msp1_platform_data);
 	db8500_add_msp_i2s(parent, 2, U8500_MSP2_BASE, IRQ_DB8500_MSP2,
 			   &msp2_platform_data);
 	db8500_add_msp_i2s(parent, 3, U8500_MSP3_BASE, IRQ_DB8500_MSP1,
 			   &msp3_platform_data);
 
-	/* Get the pinctrl handle for MSP1 */
-	if (msp1) {
-		msp1_p = pinctrl_get(&msp1->dev);
-		if (IS_ERR(msp1_p))
-			dev_err(&msp1->dev, "could not get MSP1 pinctrl\n");
-		else {
-			msp1_def = pinctrl_lookup_state(msp1_p,
-							PINCTRL_STATE_DEFAULT);
-			if (IS_ERR(msp1_def)) {
-				dev_err(&msp1->dev,
-					"could not get MSP1 defstate\n");
-			}
-			msp1_sleep = pinctrl_lookup_state(msp1_p,
-							  PINCTRL_STATE_SLEEP);
-			if (IS_ERR(msp1_sleep))
-				dev_err(&msp1->dev,
-					"could not get MSP1 idlestate\n");
-		}
-	}
-
 	pr_info("%s: Register platform-device 'ux500-pcm'\n", __func__);
 	platform_device_register(&ux500_pcm);
 
diff --git a/arch/arm/mach-ux500/include/mach/msp.h b/arch/arm/mach-ux500/include/mach/msp.h
index 798be19..3cc7142 100644
--- a/arch/arm/mach-ux500/include/mach/msp.h
+++ b/arch/arm/mach-ux500/include/mach/msp.h
@@ -22,8 +22,6 @@ struct msp_i2s_platform_data {
 	enum msp_i2s_id id;
 	struct stedma40_chan_cfg *msp_i2s_dma_rx;
 	struct stedma40_chan_cfg *msp_i2s_dma_tx;
-	int (*msp_i2s_init) (void);
-	int (*msp_i2s_exit) (void);
 };
 
 #endif
diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c
index 36be11e..2cbfc54 100644
--- a/sound/soc/ux500/ux500_msp_i2s.c
+++ b/sound/soc/ux500/ux500_msp_i2s.c
@@ -15,6 +15,7 @@
 
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
 
@@ -25,6 +26,17 @@
 
 #include "ux500_msp_i2s.h"
 
+/* MSP1/3 Tx/Rx usage protection */
+static DEFINE_SPINLOCK(msp_rxtx_lock);
+
+/* Pin modes */
+struct pinctrl *pinctrl_p;
+struct pinctrl_state *pinctrl_def;
+struct pinctrl_state *pinctrl_sleep;
+
+/* Reference Count */
+int pinctrl_rxtx_ref;
+
  /* Protocol desciptors */
 static const struct msp_protdesc prot_descs[] = {
 	{ /* I2S */
@@ -352,17 +364,23 @@ static int configure_multichannel(struct ux500_msp *msp,
 
 static int enable_msp(struct ux500_msp *msp, struct ux500_msp_config *config)
 {
-	int status = 0;
+	int status = 0, retval = 0;
 	u32 reg_val_DMACR, reg_val_GCR;
+	unsigned long flags;
 
 	/* Check msp state whether in RUN or CONFIGURED Mode */
-	if ((msp->msp_state == MSP_STATE_IDLE) && (msp->plat_init)) {
-		status = msp->plat_init();
-		if (status) {
-			dev_err(msp->dev, "%s: ERROR: Failed to init MSP (%d)!\n",
-				__func__, status);
-			return status;
+	if (msp->msp_state == MSP_STATE_IDLE) {
+		spin_lock_irqsave(&msp_rxtx_lock, flags);
+		if (pinctrl_rxtx_ref == 0 &&
+			!(IS_ERR(pinctrl_p) || IS_ERR(pinctrl_def))) {
+			retval = pinctrl_select_state(pinctrl_p,
+						pinctrl_def);
+			if (retval)
+				pr_err("could not set MSP defstate\n");
 		}
+		if (!retval)
+			pinctrl_rxtx_ref++;
+		spin_unlock_irqrestore(&msp_rxtx_lock, flags);
 	}
 
 	/* Configure msp with protocol dependent settings */
@@ -620,7 +638,8 @@ int ux500_msp_i2s_trigger(struct ux500_msp *msp, int cmd, int direction)
 
 int ux500_msp_i2s_close(struct ux500_msp *msp, unsigned int dir)
 {
-	int status = 0;
+	int status = 0, retval = 0;
+	unsigned long flags;
 
 	dev_dbg(msp->dev, "%s: Enter (dir = 0x%01x).\n", __func__, dir);
 
@@ -631,12 +650,19 @@ int ux500_msp_i2s_close(struct ux500_msp *msp, unsigned int dir)
 		writel((readl(msp->registers + MSP_GCR) &
 			       (~(FRAME_GEN_ENABLE | SRG_ENABLE))),
 			      msp->registers + MSP_GCR);
-		if (msp->plat_exit)
-			status = msp->plat_exit();
-			if (status)
-				dev_warn(msp->dev,
-					"%s: WARN: ux500_msp_i2s_exit failed (%d)!\n",
-					__func__, status);
+
+		spin_lock_irqsave(&msp_rxtx_lock, flags);
+		WARN_ON(!pinctrl_rxtx_ref);
+		pinctrl_rxtx_ref--;
+		if (pinctrl_rxtx_ref == 0 &&
+			!(IS_ERR(pinctrl_p) || IS_ERR(pinctrl_sleep))) {
+			retval = pinctrl_select_state(pinctrl_p,
+						pinctrl_sleep);
+			if (retval)
+				pr_err("could not set MSP sleepstate\n");
+		}
+		spin_unlock_irqrestore(&msp_rxtx_lock, flags);
+
 		writel(0, msp->registers + MSP_GCR);
 		writel(0, msp->registers + MSP_TCF);
 		writel(0, msp->registers + MSP_RCF);
@@ -678,8 +704,6 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,
 
 	msp->id = platform_data->id;
 	msp->dev = &pdev->dev;
-	msp->plat_init = platform_data->msp_i2s_init;
-	msp->plat_exit = platform_data->msp_i2s_exit;
 	msp->dma_cfg_rx = platform_data->msp_i2s_dma_rx;
 	msp->dma_cfg_tx = platform_data->msp_i2s_dma_tx;
 
@@ -717,6 +741,25 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,
 	dev_dbg(&pdev->dev, "I2S device-name: '%s'\n", i2s_cont->name);
 	msp->i2s_cont = i2s_cont;
 
+	pinctrl_p = pinctrl_get(msp->dev);
+	if (IS_ERR(pinctrl_p))
+		dev_err(&pdev->dev, "could not get MSP pinctrl\n");
+	else {
+		pinctrl_def = pinctrl_lookup_state(pinctrl_p,
+						PINCTRL_STATE_DEFAULT);
+		if (IS_ERR(pinctrl_def)) {
+			dev_err(&pdev->dev,
+				"could not get MSP defstate (%li)\n",
+				PTR_ERR(pinctrl_def));
+		}
+		pinctrl_sleep = pinctrl_lookup_state(pinctrl_p,
+						PINCTRL_STATE_SLEEP);
+		if (IS_ERR(pinctrl_sleep))
+			dev_err(&pdev->dev,
+				"could not get MSP idlestate (%li)\n",
+				PTR_ERR(pinctrl_def));
+	}
+
 	return 0;
 
 err_i2s_cont:
diff --git a/sound/soc/ux500/ux500_msp_i2s.h b/sound/soc/ux500/ux500_msp_i2s.h
index 2d9136d..1ce336f 100644
--- a/sound/soc/ux500/ux500_msp_i2s.h
+++ b/sound/soc/ux500/ux500_msp_i2s.h
@@ -524,8 +524,6 @@ struct ux500_msp {
 	struct dma_chan *rx_pipeid;
 	enum msp_state msp_state;
 	int (*transfer) (struct ux500_msp *msp, struct i2s_message *message);
-	int (*plat_init) (void);
-	int (*plat_exit) (void);
 	struct timer_list notify_timer;
 	int def_elem_len;
 	unsigned int dir_busy;
-- 
1.7.9.5

Re: [PATCH 04/22] ASoC: Ux500: Move MSP pinctrl setup into the MSP driver

[Linus Walleij]

On Thu, Aug 9, 2012 at 5:47 PM, Lee Jones <lee.jones@linaro.org> wrote:

> In the initial submission of the MSP driver msp1 and msp3's associated
> pinctrl mechanism was passed back to platform code using a plat_init()
> call-back routine, but it has no place in platform code. The MSP driver
> should set this up for the appropriate ports. Instead we use a use_pinctrl
> identifier which is passed from platform_data/Device Tree which indicates
> which ports should use pinctrl.
>
> CC: alsa-devel@alsa-project.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

This is very good because it rids platform code and makes the driver
self-contained without strange platform data hooks.

Now details...

> +/* MSP1/3 Tx/Rx usage protection */
> +static DEFINE_SPINLOCK(msp_rxtx_lock);
> +
> +/* Pin modes */
> +struct pinctrl *pinctrl_p;
> +struct pinctrl_state *pinctrl_def;
> +struct pinctrl_state *pinctrl_sleep;
> +
> +/* Reference Count */
> +int pinctrl_rxtx_ref;


But wait. These are just local statics. Surelty you can put these into
struct ux500_msp instead? Here it looks like these will be used for
all ports, so MSP2 will enable the pins requested by MSP1 etc
completely broken. So put it into the struct ux500_msp state
container from ux500_msp_i2s.h where it belongs.

Refer to drivers/tty/serial/amba-pl011.c when in trouble. This
one is a good pinctrl example.

Yours,
Linus Walleij

Re: [PATCH 04/22] ASoC: Ux500: Move MSP pinctrl setup into the MSP driver

[Lee Jones]

On Tue, Aug 14, 2012 at 10:51:02AM +0200, Linus Walleij wrote:
> On Thu, Aug 9, 2012 at 5:47 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > In the initial submission of the MSP driver msp1 and msp3's associated
> > pinctrl mechanism was passed back to platform code using a plat_init()
> > call-back routine, but it has no place in platform code. The MSP driver
> > should set this up for the appropriate ports. Instead we use a use_pinctrl
> > identifier which is passed from platform_data/Device Tree which indicates
> > which ports should use pinctrl.
> >
> > CC: alsa-devel@alsa-project.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> This is very good because it rids platform code and makes the driver
> self-contained without strange platform data hooks.
> 
> Now details...
> 
> > +/* MSP1/3 Tx/Rx usage protection */
> > +static DEFINE_SPINLOCK(msp_rxtx_lock);
> > +
> > +/* Pin modes */
> > +struct pinctrl *pinctrl_p;
> > +struct pinctrl_state *pinctrl_def;
> > +struct pinctrl_state *pinctrl_sleep;
> > +
> > +/* Reference Count */
> > +int pinctrl_rxtx_ref;
> 
> 
> But wait. These are just local statics. Surelty you can put these into
> struct ux500_msp instead? Here it looks like these will be used for
> all ports, so MSP2 will enable the pins requested by MSP1 etc
> completely broken. So put it into the struct ux500_msp state
> container from ux500_msp_i2s.h where it belongs.

This was intentional, as in the previous implementation only MSP1 and
MSP3 (which share pins) were enabled for pinctrl. I will move them into
struct ux500_msp instead though, no problem

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 04/22] ASoC: Ux500: Move MSP pinctrl setup into the MSP driver

[Lee Jones]

On Tue, Aug 14, 2012 at 10:51:02AM +0200, Linus Walleij wrote:
> On Thu, Aug 9, 2012 at 5:47 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > In the initial submission of the MSP driver msp1 and msp3's associated
> > pinctrl mechanism was passed back to platform code using a plat_init()
> > call-back routine, but it has no place in platform code. The MSP driver
> > should set this up for the appropriate ports. Instead we use a use_pinctrl
> > identifier which is passed from platform_data/Device Tree which indicates
> > which ports should use pinctrl.
> >
> > CC: alsa-devel@alsa-project.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> This is very good because it rids platform code and makes the driver
> self-contained without strange platform data hooks.
> 
> Now details...
> 
> > +/* MSP1/3 Tx/Rx usage protection */
> > +static DEFINE_SPINLOCK(msp_rxtx_lock);
> > +
> > +/* Pin modes */
> > +struct pinctrl *pinctrl_p;
> > +struct pinctrl_state *pinctrl_def;
> > +struct pinctrl_state *pinctrl_sleep;
> > +
> > +/* Reference Count */
> > +int pinctrl_rxtx_ref;
> 
> 
> But wait. These are just local statics. Surelty you can put these into
> struct ux500_msp instead? Here it looks like these will be used for
> all ports, so MSP2 will enable the pins requested by MSP1 etc
> completely broken. So put it into the struct ux500_msp state
> container from ux500_msp_i2s.h where it belongs.
> 
> Refer to drivers/tty/serial/amba-pl011.c when in trouble. This
> one is a good pinctrl example.

How do you see the MSP1 and MSP3 usage protection working if I hide it
all away in MSP specific structs?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 04/22] ASoC: Ux500: Move MSP pinctrl setup into the MSP driver

[Linus Walleij]

On Mon, Aug 20, 2012 at 4:59 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, Aug 14, 2012 at 10:51:02AM +0200, Linus Walleij wrote:

>> But wait. These are just local statics. Surelty you can put these into
>> struct ux500_msp instead? Here it looks like these will be used for
>> all ports, so MSP2 will enable the pins requested by MSP1 etc
>> completely broken. So put it into the struct ux500_msp state
>> container from ux500_msp_i2s.h where it belongs.
>>
>> Refer to drivers/tty/serial/amba-pl011.c when in trouble. This
>> one is a good pinctrl example.
>
> How do you see the MSP1 and MSP3 usage protection working if I hide it
> all away in MSP specific structs?

Usage protection? Sorry not following. Please elaborate...

Yours,
Linus Walleij

Re: [PATCH 04/22] ASoC: Ux500: Move MSP pinctrl setup into the MSP driver

[Lee Jones]

On Mon, Aug 27, 2012 at 04:09:46PM -0700, Linus Walleij wrote:
> On Mon, Aug 20, 2012 at 4:59 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Tue, Aug 14, 2012 at 10:51:02AM +0200, Linus Walleij wrote:
> 
> >> But wait. These are just local statics. Surelty you can put these into
> >> struct ux500_msp instead? Here it looks like these will be used for
> >> all ports, so MSP2 will enable the pins requested by MSP1 etc
> >> completely broken. So put it into the struct ux500_msp state
> >> container from ux500_msp_i2s.h where it belongs.
> >>
> >> Refer to drivers/tty/serial/amba-pl011.c when in trouble. This
> >> one is a good pinctrl example.
> >
> > How do you see the MSP1 and MSP3 usage protection working if I hide it
> > all away in MSP specific structs?
> 
> Usage protection? Sorry not following. Please elaborate...

Don't worry about it, I think I sorted it.

Please see the lastest version of the patch (sent on 24th Aug).

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 05/22] ASoC: Ux500: Enable MOP500 driver for Device Tree

[Lee Jones]

Here we ensure that the MOP500 audio driver will be probed during a
Device Tree boot. We also parse the sound node to link together the
codec, dma and the CPU-side Digital Audio Interface.

CC: alsa-devel@alsa-project.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 sound/soc/ux500/mop500.c |   40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/sound/soc/ux500/mop500.c b/sound/soc/ux500/mop500.c
index 31c4d26..6840df7 100644
--- a/sound/soc/ux500/mop500.c
+++ b/sound/soc/ux500/mop500.c
@@ -16,6 +16,7 @@
 #include <linux/module.h>
 #include <linux/io.h>
 #include <linux/spi/spi.h>
+#include <linux/of.h>
 
 #include <sound/soc.h>
 #include <sound/initval.h>
@@ -56,8 +57,35 @@ static struct snd_soc_card mop500_card = {
 	.num_links = ARRAY_SIZE(mop500_dai_links),
 };
 
+static int __devinit mop500_of_probe(struct platform_device *pdev,
+				struct device_node *np)
+{
+	struct device_node *codec_np, *msp_np[2];
+	int i;
+
+	msp_np[0] = of_parse_phandle(np, "stericsson,cpu-dai", 0);
+	msp_np[1] = of_parse_phandle(np, "stericsson,cpu-dai", 1);
+	codec_np  = of_parse_phandle(np, "stericsson,audio-codec", 0);
+
+	if (!(msp_np[0] && msp_np[1] && codec_np)) {
+		dev_err(&pdev->dev, "Phandle missing or invalid\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < 2; i++) {
+		mop500_dai_links[i].cpu_of_node = msp_np[i];
+		mop500_dai_links[i].cpu_dai_name = NULL;
+		mop500_dai_links[i].codec_of_node = codec_np;
+		mop500_dai_links[i].codec_name = NULL;
+	}
+
+	snd_soc_of_parse_card_name(&mop500_card, "stericsson,card-name");
+
+	return 0;
+}
 static int __devinit mop500_probe(struct platform_device *pdev)
 {
+	struct device_node *np = pdev->dev.of_node;
 	int ret;
 
 	pr_debug("%s: Enter.\n", __func__);
@@ -66,6 +94,12 @@ static int __devinit mop500_probe(struct platform_device *pdev)
 
 	mop500_card.dev = &pdev->dev;
 
+	if (np) {
+		ret = mop500_of_probe(pdev, np);
+		if (ret)
+			return ret;
+	}
+
 	dev_dbg(&pdev->dev, "%s: Card %s: Set platform drvdata.\n",
 		__func__, mop500_card.name);
 	platform_set_drvdata(pdev, &mop500_card);
@@ -101,10 +135,16 @@ static int __devexit mop500_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id snd_soc_mop500_match[] = {
+	{ .compatible = "stericsson,snd-soc-mop500", },
+	{},
+};
+
 static struct platform_driver snd_soc_mop500_driver = {
 	.driver = {
 		.owner = THIS_MODULE,
 		.name = "snd-soc-mop500",
+		.of_match_table = snd_soc_mop500_match,
 	},
 	.probe = mop500_probe,
 	.remove = __devexit_p(mop500_remove),
-- 
1.7.9.5

Re: [PATCH 05/22] ASoC: Ux500: Enable MOP500 driver for Device Tree

[Linus Walleij]

On Thu, Aug 9, 2012 at 5:47 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Here we ensure that the MOP500 audio driver will be probed during a
> Device Tree boot. We also parse the sound node to link together the
> codec, dma and the CPU-side Digital Audio Interface.
>
> CC: alsa-devel@alsa-project.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

But only if Mark agrees with the overall approach.

Yours,
Linus Walleij

Re: [PATCH 05/22] ASoC: Ux500: Enable MOP500 driver for Device Tree

[Lee Jones]

Ola poke.

On Tue, Aug 14, 2012 at 10:52:23AM +0200, Linus Walleij wrote:
> On Thu, Aug 9, 2012 at 5:47 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > Here we ensure that the MOP500 audio driver will be probed during a
> > Device Tree boot. We also parse the sound node to link together the
> > codec, dma and the CPU-side Digital Audio Interface.
> >
> > CC: alsa-devel@alsa-project.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> But only if Mark agrees with the overall approach.
> 
> Yours,
> Linus Walleij

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 06/22] ASoC: Ux500: Enable ux500 MSP driver for Device Tree

[Lee Jones]

Register both parts of the MSP driver from Device Tree so that they
are probed when Device Tree is enabled. Also, as there is platform
data involved, we ensure that there is allocated memory to place the
configuration into and that the correct information is extracted from
the DT binary.

CC: alsa-devel@alsa-project.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 sound/soc/ux500/ux500_msp_dai.c |    6 ++++++
 sound/soc/ux500/ux500_msp_i2s.c |   22 +++++++++++++++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c
index 772cb19..0f7dd49 100644
--- a/sound/soc/ux500/ux500_msp_dai.c
+++ b/sound/soc/ux500/ux500_msp_dai.c
@@ -833,10 +833,16 @@ static int __devexit ux500_msp_drv_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id ux500_msp_i2c_match[] = {
+	{ .compatible = "stericsson,ux500-msp-i2s", },
+	{},
+};
+
 static struct platform_driver msp_i2s_driver = {
 	.driver = {
 		.name = "ux500-msp-i2s",
 		.owner = THIS_MODULE,
+		.of_match_table = ux500_msp_i2c_match,
 	},
 	.probe = ux500_msp_drv_probe,
 	.remove = ux500_msp_drv_remove,
diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c
index 2cbfc54..5e0bf8c 100644
--- a/sound/soc/ux500/ux500_msp_i2s.c
+++ b/sound/soc/ux500/ux500_msp_i2s.c
@@ -18,6 +18,7 @@
 #include <linux/pinctrl/consumer.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 
 #include <mach/hardware.h>
 #include <mach/msp.h>
@@ -692,16 +693,31 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,
 	int ret = 0;
 	struct resource *res = NULL;
 	struct i2s_controller *i2s_cont;
+	struct device_node *np = pdev->dev.of_node;
 	struct ux500_msp *msp;
 
-	dev_dbg(&pdev->dev, "%s: Enter (name: %s, id: %d).\n", __func__,
-		pdev->name, platform_data->id);
-
 	*msp_p = devm_kzalloc(&pdev->dev, sizeof(struct ux500_msp), GFP_KERNEL);
 	msp = *msp_p;
 	if (!msp)
 		return -ENOMEM;
 
+	if (np) {
+		if (!platform_data) {
+			platform_data = devm_kzalloc(&pdev->dev,
+				sizeof(struct msp_i2s_platform_data), GFP_KERNEL);
+			if (!platform_data)
+				ret = -ENOMEM;
+		}
+	} else
+		if (!platform_data)
+			ret = -EINVAL;
+
+	if (ret)
+		goto err_res;
+
+	dev_dbg(&pdev->dev, "%s: Enter (name: %s, id: %d).\n", __func__,
+		pdev->name, platform_data->id);
+
 	msp->id = platform_data->id;
 	msp->dev = &pdev->dev;
 	msp->dma_cfg_rx = platform_data->msp_i2s_dma_rx;
-- 
1.7.9.5

Re: [PATCH 06/22] ASoC: Ux500: Enable ux500 MSP driver for Device Tree

[Linus Walleij]

On Thu, Aug 9, 2012 at 5:47 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Register both parts of the MSP driver from Device Tree so that they
> are probed when Device Tree is enabled. Also, as there is platform
> data involved, we ensure that there is allocated memory to place the
> configuration into and that the correct information is extracted from
> the DT binary.
>
> CC: alsa-devel@alsa-project.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Looks like it's doing what's intended...
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 06/22] ASoC: Ux500: Enable ux500 MSP driver for Device Tree

[Lee Jones]

Ola poke.

On Tue, Aug 14, 2012 at 10:55:59AM +0200, Linus Walleij wrote:
> On Thu, Aug 9, 2012 at 5:47 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > Register both parts of the MSP driver from Device Tree so that they
> > are probed when Device Tree is enabled. Also, as there is platform
> > data involved, we ensure that there is allocated memory to place the
> > configuration into and that the correct information is extracted from
> > the DT binary.
> >
> > CC: alsa-devel@alsa-project.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Looks like it's doing what's intended...
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Yours,
> Linus Walleij

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Lee Jones]

The PCM is a pseudo-device. It doesn't have any of it's own registers
or hardware. It rather acts as a layer of abstraction for DMA
transfers. Hence, instead of classifying it as a device in its own
right, we call the initialisation from the MSP driver.

CC: alsa-devel@alsa-project.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 sound/soc/ux500/mop500.c        |    4 ++--
 sound/soc/ux500/ux500_msp_dai.c |   14 +++++++++++++-
 sound/soc/ux500/ux500_pcm.c     |   19 ++++---------------
 sound/soc/ux500/ux500_pcm.h     |    3 +++
 4 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/sound/soc/ux500/mop500.c b/sound/soc/ux500/mop500.c
index 6840df7..fe6223a 100644
--- a/sound/soc/ux500/mop500.c
+++ b/sound/soc/ux500/mop500.c
@@ -33,7 +33,7 @@ struct snd_soc_dai_link mop500_dai_links[] = {
 		.stream_name = "ab8500_0",
 		.cpu_dai_name = "ux500-msp-i2s.1",
 		.codec_dai_name = "ab8500-codec-dai.0",
-		.platform_name = "ux500-pcm.0",
+		.platform_name = "ux500-msp-i2s.1",
 		.codec_name = "ab8500-codec.0",
 		.init = mop500_ab8500_machine_init,
 		.ops = mop500_ab8500_ops,
@@ -43,7 +43,7 @@ struct snd_soc_dai_link mop500_dai_links[] = {
 		.stream_name = "ab8500_1",
 		.cpu_dai_name = "ux500-msp-i2s.3",
 		.codec_dai_name = "ab8500-codec-dai.1",
-		.platform_name = "ux500-pcm.0",
+		.platform_name = "ux500-msp-i2s.3",
 		.codec_name = "ab8500-codec.0",
 		.init = NULL,
 		.ops = mop500_ab8500_ops,
diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c
index 0f7dd49..3476d6a 100644
--- a/sound/soc/ux500/ux500_msp_dai.c
+++ b/sound/soc/ux500/ux500_msp_dai.c
@@ -28,6 +28,7 @@
 
 #include "ux500_msp_i2s.h"
 #include "ux500_msp_dai.h"
+#include "ux500_pcm.h"
 
 static int setup_pcm_multichan(struct snd_soc_dai *dai,
 			struct ux500_msp_config *msp_config)
@@ -806,11 +807,20 @@ static int __devinit ux500_msp_drv_probe(struct platform_device *pdev)
 		goto err_init_msp;
 	}
 
+	ret = ux500_pcm_register_platform(pdev);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"Error: %s: Failed to register PCM platform device!\n",
+			__func__);
+		goto err_reg_plat;
+	}
+
 	return 0;
 
+err_reg_plat:
+	snd_soc_unregister_dais(&pdev->dev, ARRAY_SIZE(ux500_msp_dai_drv));
 err_init_msp:
 	clk_put(drvdata->clk);
-
 err_clk:
 	devm_regulator_put(drvdata->reg_vape);
 
@@ -821,6 +831,8 @@ static int __devexit ux500_msp_drv_remove(struct platform_device *pdev)
 {
 	struct ux500_msp_i2s_drvdata *drvdata = dev_get_drvdata(&pdev->dev);
 
+	ux500_pcm_unregister_platform(pdev);
+
 	snd_soc_unregister_dais(&pdev->dev, ARRAY_SIZE(ux500_msp_dai_drv));
 
 	devm_regulator_put(drvdata->reg_vape);
diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c
index 1a04e24..894c9f4 100644
--- a/sound/soc/ux500/ux500_pcm.c
+++ b/sound/soc/ux500/ux500_pcm.c
@@ -282,7 +282,7 @@ static struct snd_soc_platform_driver ux500_pcm_soc_drv = {
 	.pcm_new        = ux500_pcm_new,
 };
 
-static int __devexit ux500_pcm_drv_probe(struct platform_device *pdev)
+int __devinit ux500_pcm_register_platform(struct platform_device *pdev)
 {
 	int ret;
 
@@ -296,23 +296,12 @@ static int __devexit ux500_pcm_drv_probe(struct platform_device *pdev)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(ux500_pcm_register_platform);
 
-static int __devinit ux500_pcm_drv_remove(struct platform_device *pdev)
+int __devexit ux500_pcm_unregister_platform(struct platform_device *pdev)
 {
 	snd_soc_unregister_platform(&pdev->dev);
 
 	return 0;
 }
-
-static struct platform_driver ux500_pcm_driver = {
-	.driver = {
-		.name = "ux500-pcm",
-		.owner = THIS_MODULE,
-	},
-
-	.probe = ux500_pcm_drv_probe,
-	.remove = __devexit_p(ux500_pcm_drv_remove),
-};
-module_platform_driver(ux500_pcm_driver);
-
-MODULE_LICENSE("GPL v2");
+EXPORT_SYMBOL_GPL(ux500_pcm_unregister_platform);
diff --git a/sound/soc/ux500/ux500_pcm.h b/sound/soc/ux500/ux500_pcm.h
index 77ed44d..76d3444 100644
--- a/sound/soc/ux500/ux500_pcm.h
+++ b/sound/soc/ux500/ux500_pcm.h
@@ -32,4 +32,7 @@
 #define UX500_PLATFORM_PERIODS_MAX		48
 #define UX500_PLATFORM_BUFFER_BYTES_MAX		(2048 * PAGE_SIZE)
 
+int ux500_pcm_register_platform(struct platform_device *pdev);
+int ux500_pcm_unregister_platform(struct platform_device *pdev);
+
 #endif
-- 
1.7.9.5

Re: [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Linus Walleij]

On Thu, Aug 9, 2012 at 5:47 PM, Lee Jones <lee.jones@linaro.org> wrote:

> The PCM is a pseudo-device. It doesn't have any of it's own registers
> or hardware. It rather acts as a layer of abstraction for DMA
> transfers. Hence, instead of classifying it as a device in its own
> right, we call the initialisation from the MSP driver.
>
> CC: alsa-devel@alsa-project.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

I'd prefer to have Ola LILJA review this patch, but it looks alright to me.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Lee Jones]

Mark,

> > > I'm sorry but this patch is breaking the design of ASoC.
> > > The ASoC-platform is the DMA-block (in combination with the
> > > MSP-block), and there should be a platform-driver for the DMA/PCM.
> > > The
> > > platform-driver then has a DAI which is the MSP. The ASoC
> > > DAI-link-struct should have one driver for each of these, so the
> > > dummy-driver for PCM should be there.
> > 
> > Just thinking about this now. I converted it to the current format at
> > the request of Mark. If this isn't the correct method I'm not quite
> > sure what is. If you want it to be registered as a device, then it
> > needs to go into the Device Tree, but Mark doesn't want it in there
> > because it doesn't actually represent hardware.

I've just taken a closer look at this with a view to finding the most
suitable solution. My conclusion is that although the PCM doesn't
contain any registers, or represent hardware it should be a device and
therefore be present in the Device Tree.

These are my findings:

PCM devices already represented in DTs:
  fsl,mpc5200-pcm - written by Grant Likely, the author of Device Tree
  phytec,pcm030   - written by Grant Likely, the author of Device Tree

PCM devices which register as actual devices (should be in DT):
  samsung  - samsung-pcm
  sh       - siu-pcm-audio
  omap     - omap-pcm-audio
  pxa      - pxa-pcm-audio
  jz4740   - jz4740-pcm-audio
  kirkwood - kirkwood-pcm-audio
  ep93xx   - ep93xx-pcm-audio
  ...

The later was basically every PCM device bar one I think.

The Open Firmware used to stipulate that each device represented in
the Device Tree to own registers and therefore be an actual hardware
device, but that has since been lifted as it didn't make sense.

I propose to represent the PCM in the Device Tree again and have it
probe just like all the other PCM devices in sound/soc.

If you give me the nod, I'll revert this patch, enable the PCM for DT
again and resent the patch-set in full.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 2186 bytes --]

On Thu, Aug 23, 2012 at 10:22:17AM +0100, Lee Jones wrote:

> > > Just thinking about this now. I converted it to the current format at
> > > the request of Mark. If this isn't the correct method I'm not quite
> > > sure what is. If you want it to be registered as a device, then it
> > > needs to go into the Device Tree, but Mark doesn't want it in there
> > > because it doesn't actually represent hardware.

> I've just taken a closer look at this with a view to finding the most
> suitable solution. My conclusion is that although the PCM doesn't
> contain any registers, or represent hardware it should be a device and
> therefore be present in the Device Tree.

Your assumption that being a device in Linux means that something should
appear in the device tree definitely doesn't follow.

> PCM devices already represented in DTs:
>   fsl,mpc5200-pcm - written by Grant Likely, the author of Device Tree
>   phytec,pcm030   - written by Grant Likely, the author of Device Tree

I suspect Mitch might have a word or two to say about the above...  in
any case, these are *really* old PowerPC bindings which means they're
not always a good model.  Though in this case if you look at the code
you'll also see that the driver is actually directly managing hardware
with register I/O rather than just purely proxying an underlying DMA
API.  Clearly if there's actual hardware control involved a device is
appropriate.

> PCM devices which register as actual devices (should be in DT):

These are all Linux platform devices, things that are OK for purely
internal Linux usage which we can rewrite at will aren't always
appropriate for a cross platform DT.

> I propose to represent the PCM in the Device Tree again and have it
> probe just like all the other PCM devices in sound/soc.

> If you give me the nod, I'll revert this patch, enable the PCM for DT
> again and resent the patch-set in full.

I say I don't understand the motivation for this change.  All the modern
DT bindings are perfectly happy handling this without an explicit shim
in the device tree to bodge things for Linux, adding them in seems like
it'd be a retrograde step.  What benefit do you believe this brings?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Lee Jones]

> I say I don't understand the motivation for this change.  All the modern
> DT bindings are perfectly happy handling this without an explicit shim
> in the device tree to bodge things for Linux, adding them in seems like
> it'd be a retrograde step.  What benefit do you believe this brings?

How do the all the other DT:ed audio drivers handle the PCM then? More
importantly, how would you like to see it handled? Ola has NACKed this
patch and explained why:

"I'm sorry but this patch is breaking the design of ASoC. The ASoC-
platform is the DMA-block (in combination with the MSP-block), and
there should be a platform-driver for the DMA/PCM. The platform-driver 
then has a DAI which is the MSP. The ASoC DAI-link-struct should have
one driver for each of these, so the dummy-driver for PCM should be
there."

So I don't really know where to go with it. Any ideas?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1513 bytes --]

On Thu, Aug 23, 2012 at 01:20:03PM +0100, Lee Jones wrote:
> > I say I don't understand the motivation for this change.  All the modern
> > DT bindings are perfectly happy handling this without an explicit shim
> > in the device tree to bodge things for Linux, adding them in seems like
> > it'd be a retrograde step.  What benefit do you believe this brings?

> How do the all the other DT:ed audio drivers handle the PCM then? More
> importantly, how would you like to see it handled? Ola has NACKed this
> patch and explained why:

They instantiate the PCM driver dynamically from the DAI when it's
probed which is pretty much what you're patch is doing.

> "I'm sorry but this patch is breaking the design of ASoC. The ASoC-
> platform is the DMA-block (in combination with the MSP-block), and
> there should be a platform-driver for the DMA/PCM. The platform-driver 
> then has a DAI which is the MSP. The ASoC DAI-link-struct should have
> one driver for each of these, so the dummy-driver for PCM should be
> there."

> So I don't really know where to go with it. Any ideas?

I think Ola is suggesting probing the DMA driver from the machine which
will also work though I'm not 100% sure if I'm parsing the above
correctly.  The issue in DT terms is that if the DMA controller is
shared with a bunch of other IPs then it should have one node shared
between them all and not a bunch of shim nodes inserted in the middle
which only exists due to the way Linux instantiates stuff.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Lee Jones]

> > "I'm sorry but this patch is breaking the design of ASoC. The ASoC-
> > platform is the DMA-block (in combination with the MSP-block), and
> > there should be a platform-driver for the DMA/PCM. The platform-driver 
> > then has a DAI which is the MSP. The ASoC DAI-link-struct should have
> > one driver for each of these, so the dummy-driver for PCM should be
> > there."
> 
> > So I don't really know where to go with it. Any ideas?
> 
> I think Ola is suggesting probing the DMA driver from the machine which
> will also work though I'm not 100% sure if I'm parsing the above
> correctly.  The issue in DT terms is that if the DMA controller is
> shared with a bunch of other IPs then it should have one node shared
> between them all and not a bunch of shim nodes inserted in the middle
> which only exists due to the way Linux instantiates stuff.

When you say 'machine', do you mean from arch/<arch>/mach-*? If so, I'm
keen for that not to happen.

> > How do the all the other DT:ed audio drivers handle the PCM then? More
> > importantly, how would you like to see it handled? Ola has NACKed this
> > patch and explained why:
> 
> They instantiate the PCM driver dynamically from the DAI when it's
> probed which is pretty much what you're patch is doing.

So they do it in the same why I have with this patch? Do you know why
Ola might think this is a bad idea?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1088 bytes --]

On Thu, Aug 23, 2012 at 02:26:19PM +0100, Lee Jones wrote:

> > I think Ola is suggesting probing the DMA driver from the machine which
> > will also work though I'm not 100% sure if I'm parsing the above
> > correctly.  The issue in DT terms is that if the DMA controller is
> > shared with a bunch of other IPs then it should have one node shared
> > between them all and not a bunch of shim nodes inserted in the middle
> > which only exists due to the way Linux instantiates stuff.

> When you say 'machine', do you mean from arch/<arch>/mach-*? If so, I'm
> keen for that not to happen.

No, sound/soc/ux500/snowball.c or whatever.  At least that's my guess.

> > They instantiate the PCM driver dynamically from the DAI when it's
> > probed which is pretty much what you're patch is doing.

> So they do it in the same why I have with this patch? Do you know why
> Ola might think this is a bad idea?

I'm not 100% sure, I'm guessing it might be down to the fact that you
end up with multiple PCM drivers.  We could avoid that with refcounting
but nobody's really worried about it.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Lee Jones]

On Thu, Aug 23, 2012 at 03:37:57PM +0100, Mark Brown wrote:
> On Thu, Aug 23, 2012 at 02:26:19PM +0100, Lee Jones wrote:
> 
> > > I think Ola is suggesting probing the DMA driver from the machine which
> > > will also work though I'm not 100% sure if I'm parsing the above
> > > correctly.  The issue in DT terms is that if the DMA controller is
> > > shared with a bunch of other IPs then it should have one node shared
> > > between them all and not a bunch of shim nodes inserted in the middle
> > > which only exists due to the way Linux instantiates stuff.
> 
> > When you say 'machine', do you mean from arch/<arch>/mach-*? If so, I'm
> > keen for that not to happen.
> 
> No, sound/soc/ux500/snowball.c or whatever.  At least that's my guess.

Ah, I see. Maybe the mop500.c file then.

> > > They instantiate the PCM driver dynamically from the DAI when it's
> > > probed which is pretty much what you're patch is doing.
> 
> > So they do it in the same why I have with this patch? Do you know why
> > Ola might think this is a bad idea?
> 
> I'm not 100% sure, I'm guessing it might be down to the fact that you
> end up with multiple PCM drivers.  We could avoid that with refcounting
> but nobody's really worried about it.

I think I'll wait for Ola to get back, as he's the expert on this stuff.

I'll attempt to re-jig the patch-set, as this is a blocker atm.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 236 bytes --]

On Thu, Aug 23, 2012 at 03:59:06PM +0100, Lee Jones wrote:

> I'll attempt to re-jig the patch-set, as this is a blocker atm.

OK, if we could get at least the DAI driver and arch/arm changes in
that'd at least make the series smaller.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Lee Jones]

On Thu, Aug 23, 2012 at 01:59:04PM +0100, Mark Brown wrote:
> On Thu, Aug 23, 2012 at 01:20:03PM +0100, Lee Jones wrote:
> > > I say I don't understand the motivation for this change.  All the modern
> > > DT bindings are perfectly happy handling this without an explicit shim
> > > in the device tree to bodge things for Linux, adding them in seems like
> > > it'd be a retrograde step.  What benefit do you believe this brings?
> 
> > How do the all the other DT:ed audio drivers handle the PCM then? More
> > importantly, how would you like to see it handled? Ola has NACKed this
> > patch and explained why:
> 
> They instantiate the PCM driver dynamically from the DAI when it's
> probed which is pretty much what you're patch is doing.

Can we have some closure on this patch please, as it's blocking the
patch-set? I'm fairly sure the patch is doing the correct thing, as
seconded by Mark.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[RESENDING] [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Lee Jones]

Hi Ola,

As requested:

Author: Lee Jones <lee.jones@linaro.org>
Date:   Tue Jul 31 14:06:19 2012 +0100

    ASoC: Ux500: Initialise PCM from MSP probe rather than as a device
    
    The PCM is a pseudo-device. It doesn't have any of it's own registers
    or hardware. It rather acts as a layer of abstraction for DMA
    transfers. Hence, instead of classifying it as a device in its own
    right, we call the initialisation from the MSP driver.
    
    Signed-off-by: Lee Jones <lee.jones@linaro.org>

diff --git a/sound/soc/ux500/mop500.c b/sound/soc/ux500/mop500.c
index 31c4d26..9707f4a 100644
--- a/sound/soc/ux500/mop500.c
+++ b/sound/soc/ux500/mop500.c
@@ -32,7 +32,7 @@ struct snd_soc_dai_link mop500_dai_links[] = {
 		.stream_name = "ab8500_0",
 		.cpu_dai_name = "ux500-msp-i2s.1",
 		.codec_dai_name = "ab8500-codec-dai.0",
-		.platform_name = "ux500-pcm.0",
+		.platform_name = "ux500-msp-i2s.1",
 		.codec_name = "ab8500-codec.0",
 		.init = mop500_ab8500_machine_init,
 		.ops = mop500_ab8500_ops,
@@ -42,7 +42,7 @@ struct snd_soc_dai_link mop500_dai_links[] = {
 		.stream_name = "ab8500_1",
 		.cpu_dai_name = "ux500-msp-i2s.3",
 		.codec_dai_name = "ab8500-codec-dai.1",
-		.platform_name = "ux500-pcm.0",
+		.platform_name = "ux500-msp-i2s.3",
 		.codec_name = "ab8500-codec.0",
 		.init = NULL,
 		.ops = mop500_ab8500_ops,
diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c
index 772cb19..6afab6c 100644
--- a/sound/soc/ux500/ux500_msp_dai.c
+++ b/sound/soc/ux500/ux500_msp_dai.c
@@ -28,6 +28,7 @@
 
 #include "ux500_msp_i2s.h"
 #include "ux500_msp_dai.h"
+#include "ux500_pcm.h"
 
 static int setup_pcm_multichan(struct snd_soc_dai *dai,
 			struct ux500_msp_config *msp_config)
@@ -806,11 +807,20 @@ static int __devinit ux500_msp_drv_probe(struct platform_device *pdev)
 		goto err_init_msp;
 	}
 
+	ret = ux500_pcm_register_platform(pdev);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"Error: %s: Failed to register PCM platform device!\n",
+			__func__);
+		goto err_reg_plat;
+	}
+
 	return 0;
 
+err_reg_plat:
+	snd_soc_unregister_dais(&pdev->dev, ARRAY_SIZE(ux500_msp_dai_drv));
 err_init_msp:
 	clk_put(drvdata->clk);
-
 err_clk:
 	devm_regulator_put(drvdata->reg_vape);
 
@@ -821,6 +831,8 @@ static int __devexit ux500_msp_drv_remove(struct platform_device *pdev)
 {
 	struct ux500_msp_i2s_drvdata *drvdata = dev_get_drvdata(&pdev->dev);
 
+	ux500_pcm_unregister_platform(pdev);
+
 	snd_soc_unregister_dais(&pdev->dev, ARRAY_SIZE(ux500_msp_dai_drv));
 
 	devm_regulator_put(drvdata->reg_vape);
diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c
index 1a04e24..894c9f4 100644
--- a/sound/soc/ux500/ux500_pcm.c
+++ b/sound/soc/ux500/ux500_pcm.c
@@ -282,7 +282,7 @@ static struct snd_soc_platform_driver ux500_pcm_soc_drv = {
 	.pcm_new        = ux500_pcm_new,
 };
 
-static int __devexit ux500_pcm_drv_probe(struct platform_device *pdev)
+int __devinit ux500_pcm_register_platform(struct platform_device *pdev)
 {
 	int ret;
 
@@ -296,23 +296,12 @@ static int __devexit ux500_pcm_drv_probe(struct platform_device *pdev)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(ux500_pcm_register_platform);
 
-static int __devinit ux500_pcm_drv_remove(struct platform_device *pdev)
+int __devexit ux500_pcm_unregister_platform(struct platform_device *pdev)
 {
 	snd_soc_unregister_platform(&pdev->dev);
 
 	return 0;
 }
-
-static struct platform_driver ux500_pcm_driver = {
-	.driver = {
-		.name = "ux500-pcm",
-		.owner = THIS_MODULE,
-	},
-
-	.probe = ux500_pcm_drv_probe,
-	.remove = __devexit_p(ux500_pcm_drv_remove),
-};
-module_platform_driver(ux500_pcm_driver);
-
-MODULE_LICENSE("GPL v2");
+EXPORT_SYMBOL_GPL(ux500_pcm_unregister_platform);
diff --git a/sound/soc/ux500/ux500_pcm.h b/sound/soc/ux500/ux500_pcm.h
index 77ed44d..76d3444 100644
--- a/sound/soc/ux500/ux500_pcm.h
+++ b/sound/soc/ux500/ux500_pcm.h
@@ -32,4 +32,7 @@
 #define UX500_PLATFORM_PERIODS_MAX		48
 #define UX500_PLATFORM_BUFFER_BYTES_MAX		(2048 * PAGE_SIZE)
 
+int ux500_pcm_register_platform(struct platform_device *pdev);
+int ux500_pcm_unregister_platform(struct platform_device *pdev);
+
 #endif

Re: [RESENDING] [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Ola Lilja]

> Hi Ola,

> 
> As requested:
> 
> Author: Lee Jones <lee.jones@linaro.org>
> Date:   Tue Jul 31 14:06:19 2012 +0100
> 
>     ASoC: Ux500: Initialise PCM from MSP probe rather than as a device
>     
>     The PCM is a pseudo-device. It doesn't have any of it's own registers
>     or hardware. It rather acts as a layer of abstraction for DMA
>     transfers. Hence, instead of classifying it as a device in its own
>     right, we call the initialisation from the MSP driver.
>     
>     Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> diff --git a/sound/soc/ux500/mop500.c b/sound/soc/ux500/mop500.c
> index 31c4d26..9707f4a 100644
> --- a/sound/soc/ux500/mop500.c
> +++ b/sound/soc/ux500/mop500.c
> @@ -32,7 +32,7 @@ struct snd_soc_dai_link mop500_dai_links[] = {
>  		.stream_name = "ab8500_0",
>  		.cpu_dai_name = "ux500-msp-i2s.1",
>  		.codec_dai_name = "ab8500-codec-dai.0",
> -		.platform_name = "ux500-pcm.0",
> +		.platform_name = "ux500-msp-i2s.1",
>  		.codec_name = "ab8500-codec.0",
>  		.init = mop500_ab8500_machine_init,
>  		.ops = mop500_ab8500_ops,
> @@ -42,7 +42,7 @@ struct snd_soc_dai_link mop500_dai_links[] = {
>  		.stream_name = "ab8500_1",
>  		.cpu_dai_name = "ux500-msp-i2s.3",
>  		.codec_dai_name = "ab8500-codec-dai.1",
> -		.platform_name = "ux500-pcm.0",
> +		.platform_name = "ux500-msp-i2s.3",
>  		.codec_name = "ab8500-codec.0",
>  		.init = NULL,
>  		.ops = mop500_ab8500_ops,
> diff --git a/sound/soc/ux500/ux500_msp_dai.c b/sound/soc/ux500/ux500_msp_dai.c

>
> Lee Jones - Sept. 19, 2012, 12:29 p.m.
> On Thu, Aug 23, 2012 at 01:59:04PM +0100, Mark Brown wrote:
> > On Thu, Aug 23, 2012 at 01:20:03PM +0100, Lee Jones wrote:
> > > > I say I don't understand the motivation for this change.  All the modern
> > > > DT bindings are perfectly happy handling this without an explicit shim
> > > > in the device tree to bodge things for Linux, adding them in seems like
> > > > it'd be a retrograde step.  What benefit do you believe this brings?
> >
> > > How do the all the other DT:ed audio drivers handle the PCM then? More
> > > importantly, how would you like to see it handled? Ola has NACKed this
> > > patch and explained why:
> >
> > They instantiate the PCM driver dynamically from the DAI when it's
> > probed which is pretty much what you're patch is doing.
>
> Can we have some closure on this patch please, as it's blocking the
> patch-set? I'm fairly sure the patch is doing the correct thing, as
> seconded by Mark.

I still don't like this. It is the dai_link-struct that bothers me. We have
"ux500-msp-i2s.1" as name of the platform AND the cpu_dai. The MSP I2S-block is
not the platform and it is certainly not both platform and cpu-DAI at the same time.
Mark: Did you have a solution for this? Couldn't we just put NULL on the
platform_name instead?

Re: [RESENDING] [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Mark Brown]

On Thu, Sep 20, 2012 at 11:03:34AM +0200, Ola Lilja wrote:

> I still don't like this. It is the dai_link-struct that bothers me. We have
> "ux500-msp-i2s.1" as name of the platform AND the cpu_dai. The MSP I2S-block is
> not the platform and it is certainly not both platform and cpu-DAI at the same time.

> Mark: Did you have a solution for this? Couldn't we just put NULL on the
> platform_name instead?

No, having no platform driver means a direct device<->device link (like
baseband<->CODEC).  You could always register the platform with a -dma
added in the name or something, it'd be a bit more code but it'd work,
but the framework doesn't particularly care what you call these things.

Re: [RESENDING] [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Lee Jones]

> > Can we have some closure on this patch please, as it's blocking the
> > patch-set? I'm fairly sure the patch is doing the correct thing, as
> > seconded by Mark.
> 
> I still don't like this. It is the dai_link-struct that bothers me. We have
> "ux500-msp-i2s.1" as name of the platform AND the cpu_dai. The MSP I2S-block is
> not the platform and it is certainly not both platform and cpu-DAI at the same time.
> Mark: Did you have a solution for this? Couldn't we just put NULL on the
> platform_name instead?

There are other drivers which do this already. 

I don't think it's an issue to do this.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [RESENDING] [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Lee Jones]

On Thu, 20 Sep 2012, Lee Jones wrote:

> > > Can we have some closure on this patch please, as it's blocking the
> > > patch-set? I'm fairly sure the patch is doing the correct thing, as
> > > seconded by Mark.
> > 
> > I still don't like this. It is the dai_link-struct that bothers me. We have
> > "ux500-msp-i2s.1" as name of the platform AND the cpu_dai. The MSP I2S-block is
> > not the platform and it is certainly not both platform and cpu-DAI at the same time.
> > Mark: Did you have a solution for this? Couldn't we just put NULL on the
> > platform_name instead?
> 
> There are other drivers which do this already. 
> 
> I don't think it's an issue to do this.

Has anyone had any more bright ideas on how we might handle this?

The device is still being handled in platform code and I'm desperate
to get it out of there.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RESENDING] [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 989 bytes --]

On Thu, Nov 22, 2012 at 02:05:40PM +0000, Lee Jones wrote:
> On Thu, 20 Sep 2012, Lee Jones wrote:

> > > > Can we have some closure on this patch please, as it's blocking the
> > > > patch-set? I'm fairly sure the patch is doing the correct thing, as
> > > > seconded by Mark.

> > > I still don't like this. It is the dai_link-struct that bothers me. We have
> > > "ux500-msp-i2s.1" as name of the platform AND the cpu_dai. The MSP I2S-block is
> > > not the platform and it is certainly not both platform and cpu-DAI at the same time.
> > > Mark: Did you have a solution for this? Couldn't we just put NULL on the
> > > platform_name instead?

> > There are other drivers which do this already. 

> > I don't think it's an issue to do this.

> Has anyone had any more bright ideas on how we might handle this?

> The device is still being handled in platform code and I'm desperate
> to get it out of there.

Why do we need any ideas?  Just implement it already.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RESENDING] [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Lee Jones]

On Fri, 23 Nov 2012, Mark Brown wrote:

> On Thu, Nov 22, 2012 at 02:05:40PM +0000, Lee Jones wrote:
> > On Thu, 20 Sep 2012, Lee Jones wrote:
> 
> > > > > Can we have some closure on this patch please, as it's blocking the
> > > > > patch-set? I'm fairly sure the patch is doing the correct thing, as
> > > > > seconded by Mark.
> 
> > > > I still don't like this. It is the dai_link-struct that bothers me. We have
> > > > "ux500-msp-i2s.1" as name of the platform AND the cpu_dai. The MSP I2S-block is
> > > > not the platform and it is certainly not both platform and cpu-DAI at the same time.
> > > > Mark: Did you have a solution for this? Couldn't we just put NULL on the
> > > > platform_name instead?
> 
> > > There are other drivers which do this already. 
> 
> > > I don't think it's an issue to do this.
> 
> > Has anyone had any more bright ideas on how we might handle this?
> 
> > The device is still being handled in platform code and I'm desperate
> > to get it out of there.
> 
> Why do we need any ideas?  Just implement it already.

How do you mean? I've written the patch, just accept it. ;)

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [RESENDING] [PATCH 07/22] ASoC: Ux500: Initialise PCM from MSP probe rather than as a device

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 295 bytes --]

On Fri, Nov 23, 2012 at 09:12:36AM +0000, Lee Jones wrote:
> On Fri, 23 Nov 2012, Mark Brown wrote:

> > Why do we need any ideas?  Just implement it already.

> How do you mean? I've written the patch, just accept it. ;)

Whatever the patch was I don't have it any more, you'll need to resend.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 08/22] ASoC: codecs: Enable AB8500 CODEC for Device Tree

[Lee Jones]

We continue to allow the AB8500 CODEC to be registered via the AB8500
Multi Functional Device API, only this time we extract its configuration
from the Device Tree binary.

CC: alsa-devel@alsa-project.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 include/linux/mfd/abx500/ab8500-codec.h |    6 ++-
 sound/soc/codecs/ab8500-codec.c         |   81 +++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/include/linux/mfd/abx500/ab8500-codec.h b/include/linux/mfd/abx500/ab8500-codec.h
index dc65292..d707941 100644
--- a/include/linux/mfd/abx500/ab8500-codec.h
+++ b/include/linux/mfd/abx500/ab8500-codec.h
@@ -23,7 +23,8 @@ enum amic_type {
 /* Mic-biases */
 enum amic_micbias {
 	AMIC_MICBIAS_VAMIC1,
-	AMIC_MICBIAS_VAMIC2
+	AMIC_MICBIAS_VAMIC2,
+	AMIC_MICBIAS_UNKNOWN
 };
 
 /* Bias-voltage */
@@ -31,7 +32,8 @@ enum ear_cm_voltage {
 	EAR_CMV_0_95V,
 	EAR_CMV_1_10V,
 	EAR_CMV_1_27V,
-	EAR_CMV_1_58V
+	EAR_CMV_1_58V,
+	EAR_CMV_UNKNOWN
 };
 
 /* Analog microphone settings */
diff --git a/sound/soc/codecs/ab8500-codec.c b/sound/soc/codecs/ab8500-codec.c
index 23b4018..fa80961 100644
--- a/sound/soc/codecs/ab8500-codec.c
+++ b/sound/soc/codecs/ab8500-codec.c
@@ -34,6 +34,7 @@
 #include <linux/mfd/abx500/ab8500-sysctrl.h>
 #include <linux/mfd/abx500/ab8500-codec.h>
 #include <linux/regulator/consumer.h>
+#include <linux/of.h>
 
 #include <sound/core.h>
 #include <sound/pcm.h>
@@ -2394,9 +2395,65 @@ struct snd_soc_dai_driver ab8500_codec_dai[] = {
 	}
 };
 
+static void ab8500_codec_of_probe(struct device *dev, struct device_node *np,
+				struct ab8500_codec_platform_data *codec)
+{
+	u32 value;
+
+	if (of_get_property(np, "stericsson,amic1-type-single-ended", NULL))
+		codec->amics.mic1_type = AMIC_TYPE_SINGLE_ENDED;
+	else
+		codec->amics.mic1_type = AMIC_TYPE_DIFFERENTIAL;
+
+	if (of_get_property(np, "stericsson,amic2-type-single-ended", NULL))
+		codec->amics.mic2_type = AMIC_TYPE_SINGLE_ENDED;
+	else
+		codec->amics.mic2_type = AMIC_TYPE_DIFFERENTIAL;
+
+	/* Has a non-standard Vamic been requested? */
+	if (of_get_property(np, "stericsson,amic1a-bias-vamic2", NULL))
+		codec->amics.mic1a_micbias = AMIC_MICBIAS_VAMIC2;
+	else
+		codec->amics.mic1a_micbias = AMIC_MICBIAS_VAMIC1;
+
+	if (of_get_property(np, "stericsson,amic1b-bias-vamic2", NULL))
+		codec->amics.mic1b_micbias = AMIC_MICBIAS_VAMIC2;
+	else
+		codec->amics.mic1b_micbias = AMIC_MICBIAS_VAMIC1;
+
+	if (of_get_property(np, "stericsson,amic2-bias-vamic1", NULL))
+		codec->amics.mic2_micbias = AMIC_MICBIAS_VAMIC1;
+	else
+		codec->amics.mic2_micbias = AMIC_MICBIAS_VAMIC2;
+
+	if (!of_property_read_u32(np, "stericsson,earpeice-cmv", &value)) {
+		switch (value) {
+		case 950 :
+			codec->ear_cmv = EAR_CMV_0_95V;
+			break;
+		case 1100 :
+			codec->ear_cmv = EAR_CMV_1_10V;
+			break;
+		case 1270 :
+			codec->ear_cmv = EAR_CMV_1_27V;
+			break;
+		case 1580 :
+			codec->ear_cmv = EAR_CMV_1_58V;
+			break;
+		default :
+			codec->ear_cmv = EAR_CMV_UNKNOWN;
+			dev_err(dev, "Unsuitable earpiece voltage found in DT\n");
+		}
+	} else {
+		dev_warn(dev, "No earpiece voltage found in DT - using default\n");
+		codec->ear_cmv = EAR_CMV_0_95V;
+	}
+}
+
 static int ab8500_codec_probe(struct snd_soc_codec *codec)
 {
 	struct device *dev = codec->dev;
+	struct device_node *np = dev->of_node;
 	struct ab8500_codec_drvdata *drvdata = dev_get_drvdata(dev);
 	struct ab8500_platform_data *pdata;
 	struct filter_control *fc;
@@ -2407,6 +2464,30 @@ static int ab8500_codec_probe(struct snd_soc_codec *codec)
 	/* Setup AB8500 according to board-settings */
 	pdata = (struct ab8500_platform_data *)dev_get_platdata(dev->parent);
 
+	if (np) {
+		if (!pdata)
+			pdata = devm_kzalloc(dev,
+					sizeof(struct ab8500_platform_data),
+					GFP_KERNEL);
+
+		if (!pdata->codec)
+			pdata->codec
+				= devm_kzalloc(dev,
+					sizeof(struct ab8500_codec_platform_data),
+					GFP_KERNEL);
+
+		if (!(pdata && pdata->codec))
+			return -ENOMEM;
+
+		ab8500_codec_of_probe(dev, np, pdata->codec);
+
+	} else {
+		if (!(pdata && pdata->codec)) {
+			dev_err(dev, "No codec platform data or DT found\n");
+			return -EINVAL;
+		}
+	}
+
 	/* Inform SoC Core that we have our own I/O arrangements. */
 	codec->control_data = (void *)true;
 
-- 
1.7.9.5

Re: [PATCH 08/22] ASoC: codecs: Enable AB8500 CODEC for Device Tree

[Linus Walleij]

On Thu, Aug 9, 2012 at 5:47 PM, Lee Jones <lee.jones@linaro.org> wrote:

> We continue to allow the AB8500 CODEC to be registered via the AB8500
> Multi Functional Device API, only this time we extract its configuration
> from the Device Tree binary.
>
> CC: alsa-devel@alsa-project.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Also for Ola to look at, and/or ACK.
I dare not even ACK patches like this.

Yours,
Linus Walleij

Re: [PATCH 08/22] ASoC: codecs: Enable AB8500 CODEC for Device Tree

[Takashi Iwai]

At Thu,  9 Aug 2012 16:47:34 +0100,
Lee Jones wrote:
> @@ -2407,6 +2464,30 @@ static int ab8500_codec_probe(struct snd_soc_codec *codec)
>  	/* Setup AB8500 according to board-settings */
>  	pdata = (struct ab8500_platform_data *)dev_get_platdata(dev->parent);
>  
> +	if (np) {
> +		if (!pdata)
> +			pdata = devm_kzalloc(dev,
> +					sizeof(struct ab8500_platform_data),
> +					GFP_KERNEL);
> +
> +		if (!pdata->codec)

The NULL check of pdata must be before pdata->codec access.


Takashi

> +			pdata->codec
> +				= devm_kzalloc(dev,
> +					sizeof(struct ab8500_codec_platform_data),
> +					GFP_KERNEL);
> +
> +		if (!(pdata && pdata->codec))
> +			return -ENOMEM;
> +
> +		ab8500_codec_of_probe(dev, np, pdata->codec);
> +
> +	} else {
> +		if (!(pdata && pdata->codec)) {
> +			dev_err(dev, "No codec platform data or DT found\n");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	/* Inform SoC Core that we have our own I/O arrangements. */
>  	codec->control_data = (void *)true;
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

[PATCH 1/1] ASoC: codecs: Enable AB8500 CODEC for Device Tree

[Lee Jones]

From: Lee Jones <lee.jones@linaro.org>
Date: Fri, 27 Jul 2012 08:50:05 +0100
Subject: [PATCH 1/1] ASoC: codecs: Enable AB8500 CODEC for Device Tree

We continue to allow the AB8500 CODEC to be registered via the AB8500
Multi Functional Device API, only this time we extract its configuration
from the Device Tree binary.

CC: alsa-devel@alsa-project.org
Acked-by: Ola Lilja <ola.o.lilja@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 include/linux/mfd/abx500/ab8500-codec.h |    6 ++-
 sound/soc/codecs/ab8500-codec.c         |   81 +++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/include/linux/mfd/abx500/ab8500-codec.h b/include/linux/mfd/abx500/ab8500-codec.h
index dc65292..d707941 100644
--- a/include/linux/mfd/abx500/ab8500-codec.h
+++ b/include/linux/mfd/abx500/ab8500-codec.h
@@ -23,7 +23,8 @@ enum amic_type {
 /* Mic-biases */
 enum amic_micbias {
 	AMIC_MICBIAS_VAMIC1,
-	AMIC_MICBIAS_VAMIC2
+	AMIC_MICBIAS_VAMIC2,
+	AMIC_MICBIAS_UNKNOWN
 };
 
 /* Bias-voltage */
@@ -31,7 +32,8 @@ enum ear_cm_voltage {
 	EAR_CMV_0_95V,
 	EAR_CMV_1_10V,
 	EAR_CMV_1_27V,
-	EAR_CMV_1_58V
+	EAR_CMV_1_58V,
+	EAR_CMV_UNKNOWN
 };
 
 /* Analog microphone settings */
diff --git a/sound/soc/codecs/ab8500-codec.c b/sound/soc/codecs/ab8500-codec.c
index 23b4018..99dffcf 100644
--- a/sound/soc/codecs/ab8500-codec.c
+++ b/sound/soc/codecs/ab8500-codec.c
@@ -34,6 +34,7 @@
 #include <linux/mfd/abx500/ab8500-sysctrl.h>
 #include <linux/mfd/abx500/ab8500-codec.h>
 #include <linux/regulator/consumer.h>
+#include <linux/of.h>
 
 #include <sound/core.h>
 #include <sound/pcm.h>
@@ -2394,9 +2395,65 @@ struct snd_soc_dai_driver ab8500_codec_dai[] = {
 	}
 };
 
+static void ab8500_codec_of_probe(struct device *dev, struct device_node *np,
+				struct ab8500_codec_platform_data *codec)
+{
+	u32 value;
+
+	if (of_get_property(np, "stericsson,amic1-type-single-ended", NULL))
+		codec->amics.mic1_type = AMIC_TYPE_SINGLE_ENDED;
+	else
+		codec->amics.mic1_type = AMIC_TYPE_DIFFERENTIAL;
+
+	if (of_get_property(np, "stericsson,amic2-type-single-ended", NULL))
+		codec->amics.mic2_type = AMIC_TYPE_SINGLE_ENDED;
+	else
+		codec->amics.mic2_type = AMIC_TYPE_DIFFERENTIAL;
+
+	/* Has a non-standard Vamic been requested? */
+	if (of_get_property(np, "stericsson,amic1a-bias-vamic2", NULL))
+		codec->amics.mic1a_micbias = AMIC_MICBIAS_VAMIC2;
+	else
+		codec->amics.mic1a_micbias = AMIC_MICBIAS_VAMIC1;
+
+	if (of_get_property(np, "stericsson,amic1b-bias-vamic2", NULL))
+		codec->amics.mic1b_micbias = AMIC_MICBIAS_VAMIC2;
+	else
+		codec->amics.mic1b_micbias = AMIC_MICBIAS_VAMIC1;
+
+	if (of_get_property(np, "stericsson,amic2-bias-vamic1", NULL))
+		codec->amics.mic2_micbias = AMIC_MICBIAS_VAMIC1;
+	else
+		codec->amics.mic2_micbias = AMIC_MICBIAS_VAMIC2;
+
+	if (!of_property_read_u32(np, "stericsson,earpeice-cmv", &value)) {
+		switch (value) {
+		case 950 :
+			codec->ear_cmv = EAR_CMV_0_95V;
+			break;
+		case 1100 :
+			codec->ear_cmv = EAR_CMV_1_10V;
+			break;
+		case 1270 :
+			codec->ear_cmv = EAR_CMV_1_27V;
+			break;
+		case 1580 :
+			codec->ear_cmv = EAR_CMV_1_58V;
+			break;
+		default :
+			codec->ear_cmv = EAR_CMV_UNKNOWN;
+			dev_err(dev, "Unsuitable earpiece voltage found in DT\n");
+		}
+	} else {
+		dev_warn(dev, "No earpiece voltage found in DT - using default\n");
+		codec->ear_cmv = EAR_CMV_0_95V;
+	}
+}
+
 static int ab8500_codec_probe(struct snd_soc_codec *codec)
 {
 	struct device *dev = codec->dev;
+	struct device_node *np = dev->of_node;
 	struct ab8500_codec_drvdata *drvdata = dev_get_drvdata(dev);
 	struct ab8500_platform_data *pdata;
 	struct filter_control *fc;
@@ -2407,6 +2464,30 @@ static int ab8500_codec_probe(struct snd_soc_codec *codec)
 	/* Setup AB8500 according to board-settings */
 	pdata = (struct ab8500_platform_data *)dev_get_platdata(dev->parent);
 
+	if (np) {
+		if (!pdata)
+			pdata = devm_kzalloc(dev,
+					sizeof(struct ab8500_platform_data),
+					GFP_KERNEL);
+
+		if (pdata && !pdata->codec)
+			pdata->codec
+				= devm_kzalloc(dev,
+					sizeof(struct ab8500_codec_platform_data),
+					GFP_KERNEL);
+
+		if (!(pdata && pdata->codec))
+			return -ENOMEM;
+
+		ab8500_codec_of_probe(dev, np, pdata->codec);
+
+	} else {
+		if (!(pdata && pdata->codec)) {
+			dev_err(dev, "No codec platform data or DT found\n");
+			return -EINVAL;
+		}
+	}
+
 	/* Inform SoC Core that we have our own I/O arrangements. */
 	codec->control_data = (void *)true;
 
-- 
1.7.9.5

Re: [PATCH 1/1] ASoC: codecs: Enable AB8500 CODEC for Device Tree

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 588 bytes --]

On Mon, Aug 20, 2012 at 12:34:31PM +0100, Lee Jones wrote:
> From: Lee Jones <lee.jones@linaro.org>
> Date: Fri, 27 Jul 2012 08:50:05 +0100
> Subject: [PATCH 1/1] ASoC: codecs: Enable AB8500 CODEC for Device Tree
> 
> We continue to allow the AB8500 CODEC to be registered via the AB8500
> Multi Functional Device API, only this time we extract its configuration
> from the Device Tree binary.

Please resend this series with all the acks you've got rather than
mixing incremental updates in like this.  Please also send patches in
the format documented in SubmittingPatches.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/1] ASoC: codecs: Enable AB8500 CODEC for Device Tree

[Lee Jones]

On Mon, Aug 20, 2012 at 03:36:53PM +0100, Mark Brown wrote:
> On Mon, Aug 20, 2012 at 12:34:31PM +0100, Lee Jones wrote:
> > From: Lee Jones <lee.jones@linaro.org>
> > Date: Fri, 27 Jul 2012 08:50:05 +0100
> > Subject: [PATCH 1/1] ASoC: codecs: Enable AB8500 CODEC for Device Tree
> > 
> > We continue to allow the AB8500 CODEC to be registered via the AB8500
> > Multi Functional Device API, only this time we extract its configuration
> > from the Device Tree binary.
> 
> Please resend this series with all the acks you've got rather than
> mixing incremental updates in like this.

I'm waiting until I have a few more Acks before I resend the entire
patch-set again. Actually it's you and one other that I'm waiting for
to review (and Ack as necessary) the ones requested by Linus, then 
resend with the corrections.

> Please also send patches in
> the format documented in SubmittingPatches.

That's a big document, most of which I guess I'm adhering to. Care to
be a little more specific?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/1] ASoC: codecs: Enable AB8500 CODEC for Device Tree

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1057 bytes --]

On Tue, Aug 21, 2012 at 12:51:24PM +0100, Lee Jones wrote:
> On Mon, Aug 20, 2012 at 03:36:53PM +0100, Mark Brown wrote:

> > > From: Lee Jones <lee.jones@linaro.org>
> > > Date: Fri, 27 Jul 2012 08:50:05 +0100

> > Please resend this series with all the acks you've got rather than
> > mixing incremental updates in like this.

> I'm waiting until I have a few more Acks before I resend the entire
> patch-set again. Actually it's you and one other that I'm waiting for
> to review (and Ack as necessary) the ones requested by Linus, then 
> resend with the corrections.

Well, I'm the person who's going to apply the patches so I'm unlikely
to ack them...  I was waiting for arch/arm review before I looked at
them due to the number of resends.

> > Please also send patches in
> > the format documented in SubmittingPatches.

> That's a big document, most of which I guess I'm adhering to. Care to
> be a little more specific?

The bit I quoted is the main example, you're including random mail
headers in the body of the mail.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/1] ASoC: codecs: Enable AB8500 CODEC for Device Tree

[Lee Jones]

On Tue, Aug 21, 2012 at 01:39:51PM +0100, Mark Brown wrote:
> On Tue, Aug 21, 2012 at 12:51:24PM +0100, Lee Jones wrote:
> > On Mon, Aug 20, 2012 at 03:36:53PM +0100, Mark Brown wrote:
> 
> > > > From: Lee Jones <lee.jones@linaro.org>
> > > > Date: Fri, 27 Jul 2012 08:50:05 +0100
> 
> > > Please resend this series with all the acks you've got rather than
> > > mixing incremental updates in like this.
> 
> > I'm waiting until I have a few more Acks before I resend the entire
> > patch-set again. Actually it's you and one other that I'm waiting for
> > to review (and Ack as necessary) the ones requested by Linus, then 
> > resend with the corrections.
> 
> Well, I'm the person who's going to apply the patches so I'm unlikely
> to ack them...  I was waiting for arch/arm review before I looked at

I have all of Linus' Acks. The only ones missing are yours and Ola's,
but I think Ola is on vacation still, so he's asked Roger to do it.
I don't know if you saw, but Linus has placed lots of Acked-by's which
are dependent on your say-so, hence why I was waiting for your response.

> them due to the number of resends.

Bingo, thus why I was dubious about resending the entire patch-set too
prematurely.

> > > Please also send patches in
> > > the format documented in SubmittingPatches.
> 
> > That's a big document, most of which I guess I'm adhering to. Care to
> > be a little more specific?
> 
> The bit I quoted is the main example, you're including random mail
> headers in the body of the mail.

They're not mail headers per-say, they're `git format-patch` headers.
I thought this was acceptable for single patches, hence why I've done
it lots of times and had no complaints (until now).

If there are some changes required in a single patch, I usually fix
it up, create a patch with `git format-patch` and send it as a reply
to either the original patch in the series or the mail containing the
suggestion. If this is wrong please educate me as I thought this was
acceptable, as I thought it would be less pain than sending the
entire patch-set again for just one change?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/1] ASoC: codecs: Enable AB8500 CODEC for Device Tree

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1291 bytes --]

On Tue, Aug 21, 2012 at 01:58:12PM +0100, Lee Jones wrote:
> On Tue, Aug 21, 2012 at 01:39:51PM +0100, Mark Brown wrote:

> > The bit I quoted is the main example, you're including random mail
> > headers in the body of the mail.

> They're not mail headers per-say, they're `git format-patch` headers.
> I thought this was acceptable for single patches, hence why I've done
> it lots of times and had no complaints (until now).

> If there are some changes required in a single patch, I usually fix
> it up, create a patch with `git format-patch` and send it as a reply
> to either the original patch in the series or the mail containing the
> suggestion. If this is wrong please educate me as I thought this was

If you're going to do this send the patch properly in the same way
patches are normally sent.  Take a step back and think about this for a
minute - why would it be a good idea to send these incremental patches
in a different format which requires the person applying the patch to
hand edit things to strip out the noise?

> acceptable, as I thought it would be less pain than sending the
> entire patch-set again for just one change?

It makes it harder to work out which versions of things to apply and
causes issues for tools when doing things like applying from a mailbox.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 09/22] Documentation: Define the MOP500 Audio Machine Driver Device Tree bindings

[Lee Jones]

Here we add the required documentation for the new Device Tree
bindings pertaining to the MOP500 Audio Machine driver.

CC: alsa-devel@alsa-project.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 .../devicetree/bindings/sound/ux500-mop500.txt     |   39 ++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/ux500-mop500.txt

diff --git a/Documentation/devicetree/bindings/sound/ux500-mop500.txt b/Documentation/devicetree/bindings/sound/ux500-mop500.txt
new file mode 100644
index 0000000..48e071c
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/ux500-mop500.txt
@@ -0,0 +1,39 @@
+* MOP500 Audio Machine Driver
+
+This node is responsible for linking together all ux500 Audio Driver components.
+
+Required properties:
+ - compatible              : "stericsson,snd-soc-mop500"
+
+Non-standard properties:
+ - stericsson,cpu-dai      : Phandle to the CPU-side DAI
+ - stericsson,audio-codec  : Phandle to the Audio CODEC
+ - stericsson,card-name    : Over-ride default card name
+
+Example:
+
+	sound {
+		compatible = "stericsson,snd-soc-mop500";
+
+		stericsson,cpu-dai = <&msp1 &msp3>;
+		stericsson,audio-codec = <&codec>;
+	};
+
+	msp1: msp@80124000 {
+		compatible = "stericsson,ux500-msp-i2s";
+		reg = <0x80124000 0x1000>;
+		interrupts = <0 62 0x4>;
+		v-ape-supply = <&db8500_vape_reg>;
+	};
+
+	msp3: msp@80125000 {
+		compatible = "stericsson,ux500-msp-i2s";
+		reg = <0x80125000 0x1000>;
+		interrupts = <0 62 0x4>;
+		v-ape-supply = <&db8500_vape_reg>;
+	};
+
+	codec: ab8500-codec {
+		compatible = "stericsson,ab8500-codec";
+		stericsson,earpeice-cmv = <950>; /* Units in mV. */
+	};
-- 
1.7.9.5

[PATCH 10/22] Documentation: Define the MSP Driver Device Tree bindings

[Lee Jones]

Here we add the required documentation for the new Device Tree
bindings pertaining to the MSP CPU-side DAI Driver.

CC: alsa-devel@alsa-project.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 .../devicetree/bindings/sound/ux500-msp.txt        |   43 ++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/ux500-msp.txt

diff --git a/Documentation/devicetree/bindings/sound/ux500-msp.txt b/Documentation/devicetree/bindings/sound/ux500-msp.txt
new file mode 100644
index 0000000..99acd9c
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/ux500-msp.txt
@@ -0,0 +1,43 @@
+* ux500 MSP (CPU-side Digital Audio Interface)
+
+Required properties:
+ - compatible       :"stericsson,ux500-msp-i2s"
+ - reg              : Physical base address and length of the device's registers.
+
+Optional properties:
+ - interrupts       : The interrupt output from the device.
+ - interrupt-parent : The parent interrupt controller.
+ - <name>-supply    : Phandle to the regulator <name> supply
+
+Example:
+
+	sound {
+		compatible = "stericsson,snd-soc-mop500";
+
+		stericsson,platform-pcm-dma = <&pcm>;
+		stericsson,cpu-dai = <&msp1 &msp3>;
+		stericsson,audio-codec = <&codec>;
+	};
+
+	pcm: ux500-pcm {
+		compatible = "stericsson,ux500-pcm";
+	};
+
+	msp1: msp@80124000 {
+		compatible = "stericsson,ux500-msp-i2s";
+		reg = <0x80124000 0x1000>;
+		interrupts = <0 62 0x4>;
+		v-ape-supply = <&db8500_vape_reg>;
+	};
+
+	msp3: msp@80125000 {
+		compatible = "stericsson,ux500-msp-i2s";
+		reg = <0x80125000 0x1000>;
+		interrupts = <0 62 0x4>;
+		v-ape-supply = <&db8500_vape_reg>;
+	};
+
+	codec: ab8500-codec {
+		compatible = "stericsson,ab8500-codec";
+		stericsson,earpeice-cmv = <950>; /* Units in mV. */
+	};
-- 
1.7.9.5

[PATCH 22/22] Documentation: Add the AB8500 CODEC device to the MFD AB8500 doc

[Lee Jones]

Here we document the AB8500 Audio CODEC in the existing MFD AB8500 document.

CC: alsa-devel@alsa-project.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 Documentation/devicetree/bindings/mfd/ab8500.txt |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/ab8500.txt b/Documentation/devicetree/bindings/mfd/ab8500.txt
index 69e757a..ce83c8d 100644
--- a/Documentation/devicetree/bindings/mfd/ab8500.txt
+++ b/Documentation/devicetree/bindings/mfd/ab8500.txt
@@ -23,6 +23,7 @@ Device                     IRQ Names              Supply Names   Description
 ab8500-bm                :                      :              : Battery Manager
 ab8500-btemp             :                      :              : Battery Temperature
 ab8500-charger           :                      :              : Battery Charger
+ab8500-codec             :                      :              : Audio Codec
 ab8500-fg                :                      :              : Fuel Gauge
 ab8500-gpadc             : HW_CONV_END          : vddadc       : Analogue to Digital Converter
                            SW_CONV_END          :              :
@@ -52,6 +53,14 @@ Optional child device properties:
                            supplied in the interrupts property
 - <supply_name>-supply   : contains a phandle to the regulator supply node in Device Tree
 
+Non-standard child device properties:
+ - Audio CODEC:
+   - stericsson,amic[1|2]-type-single-ended : Single-ended Analoge Mic (default: differential)
+   - stericsson,amic1a-bias-vamic2          : Analoge Mic wishes to use a non-standard Vamic
+   - stericsson,amic1b-bias-vamic2          : Analoge Mic wishes to use a non-standard Vamic
+   - stericsson,amic2-bias-vamic1           : Analoge Mic wishes to use a non-standard Vamic
+   - stericsson,earpeice-cmv                : Earpeice voltage (only: 950 | 1100 | 1270 | 1580)
+
 ab8500@5 {
          compatible = "stericsson,ab8500";
          reg = <5>; /* mailbox 5 is i2c */
@@ -110,6 +119,12 @@ ab8500@5 {
                 compatible = "stericsson,ab8500-pwm";
         };
 
+	codec: ab8500-codec {
+		compatible = "stericsson,ab8500-codec";
+
+		stericsson,earpeice-cmv = <950>; /* Units in mV. */
+	};
+
         ab8500-regulators {
                 compatible = "stericsson,ab8500-regulator";
 
-- 
1.7.9.5

Re: [PATCH 22/22] Documentation: Add the AB8500 CODEC device to the MFD AB8500 doc

[Linus Walleij]

On Thu, Aug 9, 2012 at 5:47 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Here we document the AB8500 Audio CODEC in the existing MFD AB8500 document.
>
> CC: alsa-devel@alsa-project.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij