[PATCH 0/3] Few patches to recent OMAP McBSP and ASoC changes

[PATCH 0/3] Few patches to recent OMAP McBSP and ASoC changes

[Jarkko Nikula]

Hi

I would like to add these tree patches to recent set:

[PATCHv5 00/20] OMAP ASoC changes in DMA utilization
http://mailman.alsa-project.org/pipermail/alsa-devel/2009-August/020619.html

These are actually independent patches but probably easier to handle as with
the set above since some of the patches are touching both ASoC and OMAP
subsystems. First patch is the most important since it changes the user space
API for McBSP DMA operating mode selection. It would be more difficult to
change it later if software starts to use current numeric value based
operating mode selection in sysfs files.


-- 
Jarkko

[PATCH 1/3] OMAP: McBSP: Use textual values in DMA operating mode sysfs files

[Jarkko Nikula]

Use more descriptive than numerical value when showing and storing the
McBSP DMA operating mode. Show function is using similar syntax than e.g.
the led triggers so that all possible values for store function are
printed but with current value surrounded with square brackets.

Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
Cc: Peter Ujfalusi <peter.ujfalusi@nokia.com>
Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
---
 arch/arm/plat-omap/mcbsp.c |   52 ++++++++++++++++++++++++-------------------
 1 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index b63a720..28e29b9 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -1161,25 +1161,31 @@ static DEVICE_ATTR(prop, 0644, prop##_show, prop##_store);
 THRESHOLD_PROP_BUILDER(max_tx_thres);
 THRESHOLD_PROP_BUILDER(max_rx_thres);
 
+static const char *dma_op_modes[] = {
+	"element", "threshold", "frame",
+};
+
 static ssize_t dma_op_mode_show(struct device *dev,
 			struct device_attribute *attr, char *buf)
 {
 	struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
-	int dma_op_mode;
+	int dma_op_mode, i = 0;
+	ssize_t len = 0;
+	const char * const *s;
 
 	spin_lock_irq(&mcbsp->lock);
 	dma_op_mode = mcbsp->dma_op_mode;
 	spin_unlock_irq(&mcbsp->lock);
 
-	return sprintf(buf, "current mode: %d\n"
-			"possible mode values are:\n"
-			"%d - %s\n"
-			"%d - %s\n"
-			"%d - %s\n",
-			dma_op_mode,
-			MCBSP_DMA_MODE_ELEMENT, "element mode",
-			MCBSP_DMA_MODE_THRESHOLD, "threshold mode",
-			MCBSP_DMA_MODE_FRAME, "frame mode");
+	for (s = &dma_op_modes[i]; i < ARRAY_SIZE(dma_op_modes); s++, i++) {
+		if (dma_op_mode == i)
+			len += sprintf(buf + len, "[%s] ", *s);
+		else
+			len += sprintf(buf + len, "%s ", *s);
+	}
+	len += sprintf(len+buf, "\n");
+
+	return len;
 }
 
 static ssize_t dma_op_mode_store(struct device *dev,
@@ -1187,26 +1193,26 @@ static ssize_t dma_op_mode_store(struct device *dev,
 				const char *buf, size_t size)
 {
 	struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
-	unsigned long val;
-	int status;
+	const char * const *s;
+	char *p;
+	int len, i = 0;
 
-	status = strict_strtoul(buf, 0, &val);
-	if (status)
-		return status;
+	p = memchr(buf, '\n', size);
+	len = p ? p - buf : size;
 
-	spin_lock_irq(&mcbsp->lock);
+	for (s = &dma_op_modes[i]; i < ARRAY_SIZE(dma_op_modes); s++, i++)
+		if (*s && len == strlen(*s) && !strncmp(buf, *s, len))
+			break;
 
+	if (i == ARRAY_SIZE(dma_op_modes))
+		return -EINVAL;
+
+	spin_lock_irq(&mcbsp->lock);
 	if (!mcbsp->free) {
 		size = -EBUSY;
 		goto unlock;
 	}
-
-	if (val > MCBSP_DMA_MODE_FRAME || val < MCBSP_DMA_MODE_ELEMENT) {
-		size = -EINVAL;
-		goto unlock;
-	}
-
-	mcbsp->dma_op_mode = val;
+	mcbsp->dma_op_mode = i;
 
 unlock:
 	spin_unlock_irq(&mcbsp->lock);
-- 
1.6.3.3

[PATCH 2/3] ASoC: OMAP: Fix setup of XCCR and RCCR registers in McBSP DAI

[Jarkko Nikula]

Commit ca6e2ce08679c094878d7f39a0349a7db1d13675 is setting up few XCCR and
RCCR bits for I2S and DPS_A formats. Part of the bits are already set
for all formats and I believe that XDISABLE and RDISABLE bits are
format independent.

As XCCR and RCCR are found only from OMAP2430 and OMAP34xx, I move setup
of XDISABLE and RDISABLE to where those cpu's are tested and remove format
dependent part for simplicity.

Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
Cc: Eero Nurkkala <ext-eero.nurkkala@nokia.com>
Cc: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 sound/soc/omap/omap-mcbsp.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
index f5387d9..89e8bce 100644
--- a/sound/soc/omap/omap-mcbsp.c
+++ b/sound/soc/omap/omap-mcbsp.c
@@ -379,8 +379,8 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		regs->xcr2	|= XFIG;
 	}
 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
-		regs->xccr = DXENDLY(1) | XDMAEN;
-		regs->rccr = RFULL_CYCLE | RDMAEN;
+		regs->xccr = DXENDLY(1) | XDMAEN | XDISABLE;
+		regs->rccr = RFULL_CYCLE | RDMAEN | RDISABLE;
 	}
 
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
@@ -388,15 +388,11 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		/* 1-bit data delay */
 		regs->rcr2	|= RDATDLY(1);
 		regs->xcr2	|= XDATDLY(1);
-		regs->rccr	|= RFULL_CYCLE | RDMAEN | RDISABLE;
-		regs->xccr	|= (DXENDLY(1) | XDMAEN | XDISABLE);
 		break;
 	case SND_SOC_DAIFMT_DSP_A:
 		/* 1-bit data delay */
 		regs->rcr2      |= RDATDLY(1);
 		regs->xcr2      |= XDATDLY(1);
-		regs->rccr	|= RFULL_CYCLE | RDMAEN | RDISABLE;
-		regs->xccr	|= (DXENDLY(1) | XDMAEN | XDISABLE);
 		/* Invert FS polarity configuration */
 		temp_fmt ^= SND_SOC_DAIFMT_NB_IF;
 		break;
-- 
1.6.3.3

[PATCH 3/3] ARM: OMAP: McBSP: Merge two functions into omap_mcbsp_start/_stop

[Jarkko Nikula]

Functionality of functions omap_mcbsp_xmit_enable and omap_mcbsp_recv_enable
can be merged into omap_mcbsp_start and omap_mcbsp_stop since API of
those omap_mcbsp_start and omap_mcbsp_stop was changed recently allowing
to start and stop individually the transmitter and receiver.

This cleans up the code in arch/arm/plat-omap/mcbsp.c and in
sound/soc/omap/omap-mcbsp.c which was the only user for those removed
functions.

Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
Cc: Eero Nurkkala <ext-eero.nurkkala@nokia.com>
Cc: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 arch/arm/plat-omap/include/mach/mcbsp.h |    2 -
 arch/arm/plat-omap/mcbsp.c              |   84 ++++++++++--------------------
 sound/soc/omap/omap-mcbsp.c             |    5 --
 3 files changed, 28 insertions(+), 63 deletions(-)

diff --git a/arch/arm/plat-omap/include/mach/mcbsp.h b/arch/arm/plat-omap/include/mach/mcbsp.h
index 70e950e..63a3f25 100644
--- a/arch/arm/plat-omap/include/mach/mcbsp.h
+++ b/arch/arm/plat-omap/include/mach/mcbsp.h
@@ -436,8 +436,6 @@ int omap_mcbsp_request(unsigned int id);
 void omap_mcbsp_free(unsigned int id);
 void omap_mcbsp_start(unsigned int id, int tx, int rx);
 void omap_mcbsp_stop(unsigned int id, int tx, int rx);
-void omap_mcbsp_xmit_enable(unsigned int id, u8 enable);
-void omap_mcbsp_recv_enable(unsigned int id, u8 enable);
 void omap_mcbsp_xmit_word(unsigned int id, u32 word);
 u32 omap_mcbsp_recv_word(unsigned int id);
 
diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index 28e29b9..7e7d435 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -529,11 +529,13 @@ void omap_mcbsp_start(unsigned int id, int tx, int rx)
 	}
 
 	/* Enable transmitter and receiver */
+	tx &= 1;
 	w = OMAP_MCBSP_READ(io_base, SPCR2);
-	OMAP_MCBSP_WRITE(io_base, SPCR2, w | (tx & 1));
+	OMAP_MCBSP_WRITE(io_base, SPCR2, w | tx);
 
+	rx &= 1;
 	w = OMAP_MCBSP_READ(io_base, SPCR1);
-	OMAP_MCBSP_WRITE(io_base, SPCR1, w | (rx & 1));
+	OMAP_MCBSP_WRITE(io_base, SPCR1, w | rx);
 
 	/*
 	 * Worst case: CLKSRG*2 = 8000khz: (1/8000) * 2 * 2 usec
@@ -549,6 +551,16 @@ void omap_mcbsp_start(unsigned int id, int tx, int rx)
 		OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 7));
 	}
 
+	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
+		/* Release the transmitter and receiver */
+		w = OMAP_MCBSP_READ(io_base, XCCR);
+		w &= ~(tx ? XDISABLE : 0);
+		OMAP_MCBSP_WRITE(io_base, XCCR, w);
+		w = OMAP_MCBSP_READ(io_base, RCCR);
+		w &= ~(rx ? RDISABLE : 0);
+		OMAP_MCBSP_WRITE(io_base, RCCR, w);
+	}
+
 	/* Dump McBSP Regs */
 	omap_mcbsp_dump_reg(id);
 }
@@ -570,12 +582,24 @@ void omap_mcbsp_stop(unsigned int id, int tx, int rx)
 	io_base = mcbsp->io_base;
 
 	/* Reset transmitter */
+	tx &= 1;
+	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
+		w = OMAP_MCBSP_READ(io_base, XCCR);
+		w |= (tx ? XDISABLE : 0);
+		OMAP_MCBSP_WRITE(io_base, XCCR, w);
+	}
 	w = OMAP_MCBSP_READ(io_base, SPCR2);
-	OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(tx & 1));
+	OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~tx);
 
 	/* Reset receiver */
+	rx &= 1;
+	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
+		w = OMAP_MCBSP_READ(io_base, RCCR);
+		w |= (tx ? RDISABLE : 0);
+		OMAP_MCBSP_WRITE(io_base, RCCR, w);
+	}
 	w = OMAP_MCBSP_READ(io_base, SPCR1);
-	OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~(rx & 1));
+	OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~rx);
 
 	idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
 		  OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
@@ -588,58 +612,6 @@ void omap_mcbsp_stop(unsigned int id, int tx, int rx)
 }
 EXPORT_SYMBOL(omap_mcbsp_stop);
 
-void omap_mcbsp_xmit_enable(unsigned int id, u8 enable)
-{
-	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
-	u16 w;
-
-	if (!(cpu_is_omap2430() || cpu_is_omap34xx()))
-		return;
-
-	if (!omap_mcbsp_check_valid_id(id)) {
-		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
-		return;
-	}
-
-	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
-
-	w = OMAP_MCBSP_READ(io_base, XCCR);
-
-	if (enable)
-		OMAP_MCBSP_WRITE(io_base, XCCR, w & ~(XDISABLE));
-	else
-		OMAP_MCBSP_WRITE(io_base, XCCR, w | XDISABLE);
-}
-EXPORT_SYMBOL(omap_mcbsp_xmit_enable);
-
-void omap_mcbsp_recv_enable(unsigned int id, u8 enable)
-{
-	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
-	u16 w;
-
-	if (!(cpu_is_omap2430() || cpu_is_omap34xx()))
-		return;
-
-	if (!omap_mcbsp_check_valid_id(id)) {
-		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
-		return;
-	}
-
-	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
-
-	w = OMAP_MCBSP_READ(io_base, RCCR);
-
-	if (enable)
-		OMAP_MCBSP_WRITE(io_base, RCCR, w & ~(RDISABLE));
-	else
-		OMAP_MCBSP_WRITE(io_base, RCCR, w | RDISABLE);
-}
-EXPORT_SYMBOL(omap_mcbsp_recv_enable);
-
 /* polled mcbsp i/o operations */
 int omap_mcbsp_pollwrite(unsigned int id, u16 buf)
 {
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
index 89e8bce..0e173e7 100644
--- a/sound/soc/omap/omap-mcbsp.c
+++ b/sound/soc/omap/omap-mcbsp.c
@@ -231,11 +231,6 @@ static int omap_mcbsp_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		mcbsp_data->active++;
 		omap_mcbsp_start(mcbsp_data->bus_id, play, !play);
-		/* Make sure data transfer is frame synchronized */
-		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-			omap_mcbsp_xmit_enable(mcbsp_data->bus_id, 1);
-		else
-			omap_mcbsp_recv_enable(mcbsp_data->bus_id, 1);
 		break;
 
 	case SNDRV_PCM_TRIGGER_STOP:
-- 
1.6.3.3

Re: [PATCH 1/3] OMAP: McBSP: Use textual values in DMA operating mode sysfs files

[Eduardo Valentin]

Hello Jarkko,

On Sun, Aug 23, 2009 at 11:24:25AM +0200, Jarkko Nikula wrote:
> Use more descriptive than numerical value when showing and storing the
> McBSP DMA operating mode. Show function is using similar syntax than e.g.
> the led triggers so that all possible values for store function are
> printed but with current value surrounded with square brackets.

I liked the idea as I said before. But let's try to improve it a bit.

> 
> Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
> Cc: Peter Ujfalusi <peter.ujfalusi@nokia.com>
> Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
> ---
>  arch/arm/plat-omap/mcbsp.c |   52 ++++++++++++++++++++++++-------------------
>  1 files changed, 29 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
> index b63a720..28e29b9 100644
> --- a/arch/arm/plat-omap/mcbsp.c
> +++ b/arch/arm/plat-omap/mcbsp.c
> @@ -1161,25 +1161,31 @@ static DEVICE_ATTR(prop, 0644, prop##_show, prop##_store);
>  THRESHOLD_PROP_BUILDER(max_tx_thres);
>  THRESHOLD_PROP_BUILDER(max_rx_thres);
>  
> +static const char *dma_op_modes[] = {
> +	"element", "threshold", "frame",
> +};
> +
>  static ssize_t dma_op_mode_show(struct device *dev,
>  			struct device_attribute *attr, char *buf)
>  {
>  	struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
> -	int dma_op_mode;
> +	int dma_op_mode, i = 0;
> +	ssize_t len = 0;
> +	const char * const *s;
>  
>  	spin_lock_irq(&mcbsp->lock);
>  	dma_op_mode = mcbsp->dma_op_mode;
>  	spin_unlock_irq(&mcbsp->lock);
>  
> -	return sprintf(buf, "current mode: %d\n"
> -			"possible mode values are:\n"
> -			"%d - %s\n"
> -			"%d - %s\n"
> -			"%d - %s\n",
> -			dma_op_mode,
> -			MCBSP_DMA_MODE_ELEMENT, "element mode",
> -			MCBSP_DMA_MODE_THRESHOLD, "threshold mode",
> -			MCBSP_DMA_MODE_FRAME, "frame mode");
> +	for (s = &dma_op_modes[i]; i < ARRAY_SIZE(dma_op_modes); s++, i++) {
> +		if (dma_op_mode == i)
> +			len += sprintf(buf + len, "[%s] ", *s);
> +		else
> +			len += sprintf(buf + len, "%s ", *s);
> +	}
> +	len += sprintf(len+buf, "\n");

Just a tiny thing, add spaces before and after + operator:
+	len += sprintf(len + buf, "\n");

> +
> +	return len;
>  }
>  
>  static ssize_t dma_op_mode_store(struct device *dev,
> @@ -1187,26 +1193,26 @@ static ssize_t dma_op_mode_store(struct device *dev,
>  				const char *buf, size_t size)
>  {
>  	struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
> -	unsigned long val;
> -	int status;
> +	const char * const *s;
> +	char *p;
> +	int len, i = 0;
>  
> -	status = strict_strtoul(buf, 0, &val);
> -	if (status)
> -		return status;
> +	p = memchr(buf, '\n', size);
> +	len = p ? p - buf : size;

I guess here we have two better options, please use one of these:
* strstrip
* sysfs_streq

>  
> -	spin_lock_irq(&mcbsp->lock);
> +	for (s = &dma_op_modes[i]; i < ARRAY_SIZE(dma_op_modes); s++, i++)
> +		if (*s && len == strlen(*s) && !strncmp(buf, *s, len))

how about using strnicmp ??

> +			break;
>  
> +	if (i == ARRAY_SIZE(dma_op_modes))
> +		return -EINVAL;
> +
> +	spin_lock_irq(&mcbsp->lock);
>  	if (!mcbsp->free) {
>  		size = -EBUSY;
>  		goto unlock;
>  	}
> -
> -	if (val > MCBSP_DMA_MODE_FRAME || val < MCBSP_DMA_MODE_ELEMENT) {
> -		size = -EINVAL;
> -		goto unlock;
> -	}
> -
> -	mcbsp->dma_op_mode = val;
> +	mcbsp->dma_op_mode = i;
>  
>  unlock:
>  	spin_unlock_irq(&mcbsp->lock);
> -- 
> 1.6.3.3

-- 
Eduardo Valentin

Re: [PATCH 1/3] OMAP: McBSP: Use textual values in DMA operating mode sysfs files

[Jarkko Nikula]

On Mon, 24 Aug 2009 09:49:41 +0300
Eduardo Valentin <eduardo.valentin@nokia.com> wrote:

> > +			len += sprintf(buf + len, "[%s] ", *s);
> > +		else
> > +			len += sprintf(buf + len, "%s ", *s);
> > +	}
> > +	len += sprintf(len+buf, "\n");
> 
> Just a tiny thing, add spaces before and after + operator:
> +	len += sprintf(len + buf, "\n");
> 
> > +	p = memchr(buf, '\n', size);
> > +	len = p ? p - buf : size;
> 
> I guess here we have two better options, please use one of these:
> * strstrip
> * sysfs_streq
> 
Thanks Eduardo. I'll fix the first the one and use the sysfs_streq as
it's much cleaner to use.

> >  
> > -	spin_lock_irq(&mcbsp->lock);
> > +	for (s = &dma_op_modes[i]; i < ARRAY_SIZE(dma_op_modes); s++, i++)
> > +		if (*s && len == strlen(*s) && !strncmp(buf, *s, len))
> 
> how about using strnicmp ??
> 
Are there need for case insensitive check? The sysfs_streq is not.

I'll send a new version later this day.


-- 
Jarkko

Re: [PATCH 0/3] Few patches to recent OMAP McBSP and ASoC changes

[Eduardo Valentin]

On Sun, Aug 23, 2009 at 11:24:24AM +0200, Jarkko Nikula wrote:
> Hi
> 
> I would like to add these tree patches to recent set:
> 
> [PATCHv5 00/20] OMAP ASoC changes in DMA utilization
> http://mailman.alsa-project.org/pipermail/alsa-devel/2009-August/020619.html
> 
> These are actually independent patches but probably easier to handle as with
> the set above since some of the patches are touching both ASoC and OMAP
> subsystems. First patch is the most important since it changes the user space
> API for McBSP DMA operating mode selection. It would be more difficult to
> change it later if software starts to use current numeric value based
> operating mode selection in sysfs files.


For the other two changes (patches 02 and 03) I'm giving my ack here.
Tested them and after running several playback/capture tasks, looks like the
streaming was ok.

And for the channel swapping issue, I think the thing remains the same.
If I increase the max threshold to something closer to its maximum (0x500)
than, channels swap quite easily. Just:

# aplay -c2 -fdat -I /dev/urandom /dev/zero

But that wouldn't block your two patch Jarkko. I liked this merge.
Code looks cleaner now.

> 
> 
> -- 
> Jarkko
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Eduardo Valentin

Re: [PATCH 1/3] OMAP: McBSP: Use textual values in DMA operating mode sysfs files

[Jarkko Nikula]

On Mon, 24 Aug 2009 10:34:22 +0300
Eduardo Valentin <eduardo.valentin@nokia.com> wrote:

> > Are there need for case insensitive check? The sysfs_streq is not.
> 
> Yes, sysfs_streq is not. There is no need for the insensitive, but
> it can give user more options (ELEMENT or element)?
> 
> If using strcicmp, then strstrip would be required.
> 
> Just to reminder that strstrip does a wider job by cleaning any kind of
> spaces. sysfs_streq deals only with 1 leading "\n".
> 
Sounds too fancy :-)

Here with sysfs_streq. Clean and simple.

============================== CUT HERE ==============================
From: Jarkko Nikula <jhnikula@gmail.com>
Subject: [PATCH 1/3] OMAP: McBSP: Use textual values in DMA operating mode sysfs files

Use more descriptive than numerical value when showing and storing the
McBSP DMA operating mode. Show function is using similar syntax than e.g.
the led triggers so that all possible values for store function are
printed but with current value surrounded with square brackets.

Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
Cc: Peter Ujfalusi <peter.ujfalusi@nokia.com>
Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
---
 arch/arm/plat-omap/mcbsp.c |   48 ++++++++++++++++++++++---------------------
 1 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index b63a720..ee60ab6 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -1161,25 +1161,31 @@ static DEVICE_ATTR(prop, 0644, prop##_show, prop##_store);
 THRESHOLD_PROP_BUILDER(max_tx_thres);
 THRESHOLD_PROP_BUILDER(max_rx_thres);
 
+static const char *dma_op_modes[] = {
+	"element", "threshold", "frame",
+};
+
 static ssize_t dma_op_mode_show(struct device *dev,
 			struct device_attribute *attr, char *buf)
 {
 	struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
-	int dma_op_mode;
+	int dma_op_mode, i = 0;
+	ssize_t len = 0;
+	const char * const *s;
 
 	spin_lock_irq(&mcbsp->lock);
 	dma_op_mode = mcbsp->dma_op_mode;
 	spin_unlock_irq(&mcbsp->lock);
 
-	return sprintf(buf, "current mode: %d\n"
-			"possible mode values are:\n"
-			"%d - %s\n"
-			"%d - %s\n"
-			"%d - %s\n",
-			dma_op_mode,
-			MCBSP_DMA_MODE_ELEMENT, "element mode",
-			MCBSP_DMA_MODE_THRESHOLD, "threshold mode",
-			MCBSP_DMA_MODE_FRAME, "frame mode");
+	for (s = &dma_op_modes[i]; i < ARRAY_SIZE(dma_op_modes); s++, i++) {
+		if (dma_op_mode == i)
+			len += sprintf(buf + len, "[%s] ", *s);
+		else
+			len += sprintf(buf + len, "%s ", *s);
+	}
+	len += sprintf(buf + len, "\n");
+
+	return len;
 }
 
 static ssize_t dma_op_mode_store(struct device *dev,
@@ -1187,26 +1193,22 @@ static ssize_t dma_op_mode_store(struct device *dev,
 				const char *buf, size_t size)
 {
 	struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
-	unsigned long val;
-	int status;
+	const char * const *s;
+	int i = 0;
 
-	status = strict_strtoul(buf, 0, &val);
-	if (status)
-		return status;
+	for (s = &dma_op_modes[i]; i < ARRAY_SIZE(dma_op_modes); s++, i++)
+		if (sysfs_streq(buf, *s))
+			break;
 
-	spin_lock_irq(&mcbsp->lock);
+	if (i == ARRAY_SIZE(dma_op_modes))
+		return -EINVAL;
 
+	spin_lock_irq(&mcbsp->lock);
 	if (!mcbsp->free) {
 		size = -EBUSY;
 		goto unlock;
 	}
-
-	if (val > MCBSP_DMA_MODE_FRAME || val < MCBSP_DMA_MODE_ELEMENT) {
-		size = -EINVAL;
-		goto unlock;
-	}
-
-	mcbsp->dma_op_mode = val;
+	mcbsp->dma_op_mode = i;
 
 unlock:
 	spin_unlock_irq(&mcbsp->lock);
-- 
1.6.3.3

Re: [PATCH 1/3] OMAP: McBSP: Use textual values in DMA operating mode sysfs files

[Eduardo Valentin]

On Mon, Aug 24, 2009 at 04:45:50PM +0200, Jarkko Nikula wrote:
> On Mon, 24 Aug 2009 10:34:22 +0300
> Eduardo Valentin <eduardo.valentin@nokia.com> wrote:
> 
> > > Are there need for case insensitive check? The sysfs_streq is not.
> > 
> > Yes, sysfs_streq is not. There is no need for the insensitive, but
> > it can give user more options (ELEMENT or element)?
> > 
> > If using strcicmp, then strstrip would be required.
> > 
> > Just to reminder that strstrip does a wider job by cleaning any kind of
> > spaces. sysfs_streq deals only with 1 leading "\n".
> > 
> Sounds too fancy :-)
> 
> Here with sysfs_streq. Clean and simple.
> 
> ============================== CUT HERE ==============================
> From: Jarkko Nikula <jhnikula@gmail.com>
> Subject: [PATCH 1/3] OMAP: McBSP: Use textual values in DMA operating mode sysfs files
> 
> Use more descriptive than numerical value when showing and storing the
> McBSP DMA operating mode. Show function is using similar syntax than e.g.
> the led triggers so that all possible values for store function are
> printed but with current value surrounded with square brackets.
> 
> Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
> Cc: Peter Ujfalusi <peter.ujfalusi@nokia.com>
> Cc: Eduardo Valentin <eduardo.valentin@nokia.com>

Acked-by: Eduardo Valentin <eduardo.valentin@nokia.com>

> ---
>  arch/arm/plat-omap/mcbsp.c |   48 ++++++++++++++++++++++---------------------
>  1 files changed, 25 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
> index b63a720..ee60ab6 100644
> --- a/arch/arm/plat-omap/mcbsp.c
> +++ b/arch/arm/plat-omap/mcbsp.c
> @@ -1161,25 +1161,31 @@ static DEVICE_ATTR(prop, 0644, prop##_show, prop##_store);
>  THRESHOLD_PROP_BUILDER(max_tx_thres);
>  THRESHOLD_PROP_BUILDER(max_rx_thres);
>  
> +static const char *dma_op_modes[] = {
> +	"element", "threshold", "frame",
> +};
> +
>  static ssize_t dma_op_mode_show(struct device *dev,
>  			struct device_attribute *attr, char *buf)
>  {
>  	struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
> -	int dma_op_mode;
> +	int dma_op_mode, i = 0;
> +	ssize_t len = 0;
> +	const char * const *s;
>  
>  	spin_lock_irq(&mcbsp->lock);
>  	dma_op_mode = mcbsp->dma_op_mode;
>  	spin_unlock_irq(&mcbsp->lock);
>  
> -	return sprintf(buf, "current mode: %d\n"
> -			"possible mode values are:\n"
> -			"%d - %s\n"
> -			"%d - %s\n"
> -			"%d - %s\n",
> -			dma_op_mode,
> -			MCBSP_DMA_MODE_ELEMENT, "element mode",
> -			MCBSP_DMA_MODE_THRESHOLD, "threshold mode",
> -			MCBSP_DMA_MODE_FRAME, "frame mode");
> +	for (s = &dma_op_modes[i]; i < ARRAY_SIZE(dma_op_modes); s++, i++) {
> +		if (dma_op_mode == i)
> +			len += sprintf(buf + len, "[%s] ", *s);
> +		else
> +			len += sprintf(buf + len, "%s ", *s);
> +	}
> +	len += sprintf(buf + len, "\n");
> +
> +	return len;
>  }
>  
>  static ssize_t dma_op_mode_store(struct device *dev,
> @@ -1187,26 +1193,22 @@ static ssize_t dma_op_mode_store(struct device *dev,
>  				const char *buf, size_t size)
>  {
>  	struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
> -	unsigned long val;
> -	int status;
> +	const char * const *s;
> +	int i = 0;
>  
> -	status = strict_strtoul(buf, 0, &val);
> -	if (status)
> -		return status;
> +	for (s = &dma_op_modes[i]; i < ARRAY_SIZE(dma_op_modes); s++, i++)
> +		if (sysfs_streq(buf, *s))
> +			break;
>  
> -	spin_lock_irq(&mcbsp->lock);
> +	if (i == ARRAY_SIZE(dma_op_modes))
> +		return -EINVAL;
>  
> +	spin_lock_irq(&mcbsp->lock);
>  	if (!mcbsp->free) {
>  		size = -EBUSY;
>  		goto unlock;
>  	}
> -
> -	if (val > MCBSP_DMA_MODE_FRAME || val < MCBSP_DMA_MODE_ELEMENT) {
> -		size = -EINVAL;
> -		goto unlock;
> -	}
> -
> -	mcbsp->dma_op_mode = val;
> +	mcbsp->dma_op_mode = i;
>  
>  unlock:
>  	spin_unlock_irq(&mcbsp->lock);
> -- 
> 1.6.3.3

-- 
Eduardo Valentin

Re: [PATCH 2/3] ASoC: OMAP: Fix setup of XCCR and RCCR registers in McBSP DAI

[Eduardo Valentin]

On Sun, Aug 23, 2009 at 11:24:26AM +0200, Jarkko Nikula wrote:
> Commit ca6e2ce08679c094878d7f39a0349a7db1d13675 is setting up few XCCR and
> RCCR bits for I2S and DPS_A formats. Part of the bits are already set
> for all formats and I believe that XDISABLE and RDISABLE bits are
> format independent.
> 
> As XCCR and RCCR are found only from OMAP2430 and OMAP34xx, I move setup
> of XDISABLE and RDISABLE to where those cpu's are tested and remove format
> dependent part for simplicity.
> 
> Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
> Cc: Eero Nurkkala <ext-eero.nurkkala@nokia.com>
> Cc: Peter Ujfalusi <peter.ujfalusi@nokia.com>

Acked-by: Eduardo Valentin <eduardo.valentin@nokia.com>

> ---
>  sound/soc/omap/omap-mcbsp.c |    8 ++------
>  1 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
> index f5387d9..89e8bce 100644
> --- a/sound/soc/omap/omap-mcbsp.c
> +++ b/sound/soc/omap/omap-mcbsp.c
> @@ -379,8 +379,8 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai,
>  		regs->xcr2	|= XFIG;
>  	}
>  	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
> -		regs->xccr = DXENDLY(1) | XDMAEN;
> -		regs->rccr = RFULL_CYCLE | RDMAEN;
> +		regs->xccr = DXENDLY(1) | XDMAEN | XDISABLE;
> +		regs->rccr = RFULL_CYCLE | RDMAEN | RDISABLE;
>  	}
>  
>  	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> @@ -388,15 +388,11 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai,
>  		/* 1-bit data delay */
>  		regs->rcr2	|= RDATDLY(1);
>  		regs->xcr2	|= XDATDLY(1);
> -		regs->rccr	|= RFULL_CYCLE | RDMAEN | RDISABLE;
> -		regs->xccr	|= (DXENDLY(1) | XDMAEN | XDISABLE);
>  		break;
>  	case SND_SOC_DAIFMT_DSP_A:
>  		/* 1-bit data delay */
>  		regs->rcr2      |= RDATDLY(1);
>  		regs->xcr2      |= XDATDLY(1);
> -		regs->rccr	|= RFULL_CYCLE | RDMAEN | RDISABLE;
> -		regs->xccr	|= (DXENDLY(1) | XDMAEN | XDISABLE);
>  		/* Invert FS polarity configuration */
>  		temp_fmt ^= SND_SOC_DAIFMT_NB_IF;
>  		break;
> -- 
> 1.6.3.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Eduardo Valentin

Re: [PATCH 3/3] ARM: OMAP: McBSP: Merge two functions into omap_mcbsp_start/_stop

[Eduardo Valentin]

On Sun, Aug 23, 2009 at 11:24:27AM +0200, Jarkko Nikula wrote:
> Functionality of functions omap_mcbsp_xmit_enable and omap_mcbsp_recv_enable
> can be merged into omap_mcbsp_start and omap_mcbsp_stop since API of
> those omap_mcbsp_start and omap_mcbsp_stop was changed recently allowing
> to start and stop individually the transmitter and receiver.
> 
> This cleans up the code in arch/arm/plat-omap/mcbsp.c and in
> sound/soc/omap/omap-mcbsp.c which was the only user for those removed
> functions.
> 
> Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
> Cc: Eero Nurkkala <ext-eero.nurkkala@nokia.com>
> Cc: Peter Ujfalusi <peter.ujfalusi@nokia.com>

Acked-by: Eduardo Valentin <eduardo.valentin@nokia.com>

> ---
>  arch/arm/plat-omap/include/mach/mcbsp.h |    2 -
>  arch/arm/plat-omap/mcbsp.c              |   84 ++++++++++--------------------
>  sound/soc/omap/omap-mcbsp.c             |    5 --
>  3 files changed, 28 insertions(+), 63 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/mach/mcbsp.h b/arch/arm/plat-omap/include/mach/mcbsp.h
> index 70e950e..63a3f25 100644
> --- a/arch/arm/plat-omap/include/mach/mcbsp.h
> +++ b/arch/arm/plat-omap/include/mach/mcbsp.h
> @@ -436,8 +436,6 @@ int omap_mcbsp_request(unsigned int id);
>  void omap_mcbsp_free(unsigned int id);
>  void omap_mcbsp_start(unsigned int id, int tx, int rx);
>  void omap_mcbsp_stop(unsigned int id, int tx, int rx);
> -void omap_mcbsp_xmit_enable(unsigned int id, u8 enable);
> -void omap_mcbsp_recv_enable(unsigned int id, u8 enable);
>  void omap_mcbsp_xmit_word(unsigned int id, u32 word);
>  u32 omap_mcbsp_recv_word(unsigned int id);
>  
> diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
> index 28e29b9..7e7d435 100644
> --- a/arch/arm/plat-omap/mcbsp.c
> +++ b/arch/arm/plat-omap/mcbsp.c
> @@ -529,11 +529,13 @@ void omap_mcbsp_start(unsigned int id, int tx, int rx)
>  	}
>  
>  	/* Enable transmitter and receiver */
> +	tx &= 1;
>  	w = OMAP_MCBSP_READ(io_base, SPCR2);
> -	OMAP_MCBSP_WRITE(io_base, SPCR2, w | (tx & 1));
> +	OMAP_MCBSP_WRITE(io_base, SPCR2, w | tx);
>  
> +	rx &= 1;
>  	w = OMAP_MCBSP_READ(io_base, SPCR1);
> -	OMAP_MCBSP_WRITE(io_base, SPCR1, w | (rx & 1));
> +	OMAP_MCBSP_WRITE(io_base, SPCR1, w | rx);
>  
>  	/*
>  	 * Worst case: CLKSRG*2 = 8000khz: (1/8000) * 2 * 2 usec
> @@ -549,6 +551,16 @@ void omap_mcbsp_start(unsigned int id, int tx, int rx)
>  		OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 7));
>  	}
>  
> +	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
> +		/* Release the transmitter and receiver */
> +		w = OMAP_MCBSP_READ(io_base, XCCR);
> +		w &= ~(tx ? XDISABLE : 0);
> +		OMAP_MCBSP_WRITE(io_base, XCCR, w);
> +		w = OMAP_MCBSP_READ(io_base, RCCR);
> +		w &= ~(rx ? RDISABLE : 0);
> +		OMAP_MCBSP_WRITE(io_base, RCCR, w);
> +	}
> +
>  	/* Dump McBSP Regs */
>  	omap_mcbsp_dump_reg(id);
>  }
> @@ -570,12 +582,24 @@ void omap_mcbsp_stop(unsigned int id, int tx, int rx)
>  	io_base = mcbsp->io_base;
>  
>  	/* Reset transmitter */
> +	tx &= 1;
> +	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
> +		w = OMAP_MCBSP_READ(io_base, XCCR);
> +		w |= (tx ? XDISABLE : 0);
> +		OMAP_MCBSP_WRITE(io_base, XCCR, w);
> +	}
>  	w = OMAP_MCBSP_READ(io_base, SPCR2);
> -	OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(tx & 1));
> +	OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~tx);
>  
>  	/* Reset receiver */
> +	rx &= 1;
> +	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
> +		w = OMAP_MCBSP_READ(io_base, RCCR);
> +		w |= (tx ? RDISABLE : 0);
> +		OMAP_MCBSP_WRITE(io_base, RCCR, w);
> +	}
>  	w = OMAP_MCBSP_READ(io_base, SPCR1);
> -	OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~(rx & 1));
> +	OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~rx);
>  
>  	idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
>  		  OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
> @@ -588,58 +612,6 @@ void omap_mcbsp_stop(unsigned int id, int tx, int rx)
>  }
>  EXPORT_SYMBOL(omap_mcbsp_stop);
>  
> -void omap_mcbsp_xmit_enable(unsigned int id, u8 enable)
> -{
> -	struct omap_mcbsp *mcbsp;
> -	void __iomem *io_base;
> -	u16 w;
> -
> -	if (!(cpu_is_omap2430() || cpu_is_omap34xx()))
> -		return;
> -
> -	if (!omap_mcbsp_check_valid_id(id)) {
> -		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
> -		return;
> -	}
> -
> -	mcbsp = id_to_mcbsp_ptr(id);
> -	io_base = mcbsp->io_base;
> -
> -	w = OMAP_MCBSP_READ(io_base, XCCR);
> -
> -	if (enable)
> -		OMAP_MCBSP_WRITE(io_base, XCCR, w & ~(XDISABLE));
> -	else
> -		OMAP_MCBSP_WRITE(io_base, XCCR, w | XDISABLE);
> -}
> -EXPORT_SYMBOL(omap_mcbsp_xmit_enable);
> -
> -void omap_mcbsp_recv_enable(unsigned int id, u8 enable)
> -{
> -	struct omap_mcbsp *mcbsp;
> -	void __iomem *io_base;
> -	u16 w;
> -
> -	if (!(cpu_is_omap2430() || cpu_is_omap34xx()))
> -		return;
> -
> -	if (!omap_mcbsp_check_valid_id(id)) {
> -		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
> -		return;
> -	}
> -
> -	mcbsp = id_to_mcbsp_ptr(id);
> -	io_base = mcbsp->io_base;
> -
> -	w = OMAP_MCBSP_READ(io_base, RCCR);
> -
> -	if (enable)
> -		OMAP_MCBSP_WRITE(io_base, RCCR, w & ~(RDISABLE));
> -	else
> -		OMAP_MCBSP_WRITE(io_base, RCCR, w | RDISABLE);
> -}
> -EXPORT_SYMBOL(omap_mcbsp_recv_enable);
> -
>  /* polled mcbsp i/o operations */
>  int omap_mcbsp_pollwrite(unsigned int id, u16 buf)
>  {
> diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
> index 89e8bce..0e173e7 100644
> --- a/sound/soc/omap/omap-mcbsp.c
> +++ b/sound/soc/omap/omap-mcbsp.c
> @@ -231,11 +231,6 @@ static int omap_mcbsp_dai_trigger(struct snd_pcm_substream *substream, int cmd,
>  	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>  		mcbsp_data->active++;
>  		omap_mcbsp_start(mcbsp_data->bus_id, play, !play);
> -		/* Make sure data transfer is frame synchronized */
> -		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> -			omap_mcbsp_xmit_enable(mcbsp_data->bus_id, 1);
> -		else
> -			omap_mcbsp_recv_enable(mcbsp_data->bus_id, 1);
>  		break;
>  
>  	case SNDRV_PCM_TRIGGER_STOP:
> -- 
> 1.6.3.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Eduardo Valentin

Re: [PATCH 0/3] Few patches to recent OMAP McBSP and ASoC changes

[Mark Brown]

On Sun, Aug 23, 2009 at 12:24:24PM +0300, Jarkko Nikula wrote:

> These are actually independent patches but probably easier to handle as with
> the set above since some of the patches are touching both ASoC and OMAP
> subsystems. First patch is the most important since it changes the user space
> API for McBSP DMA operating mode selection. It would be more difficult to
> change it later if software starts to use current numeric value based
> operating mode selection in sysfs files.

Applied all of these, thanks!