[alsa-devel] [PATCH 0/3] Few patches to recent OMAP McBSP and ASoC changes
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.
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);
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
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.
On Mon, Aug 24, 2009 at 09:35:03AM +0200, Jarkko Nikula wrote:
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.
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".
I'll send a new version later this day.
-- Jarkko
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);
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
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;
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;
break; case SND_SOC_DAIFMT_DSP_A: /* 1-bit data delay */ regs->rcr2 |= RDATDLY(1); regs->xcr2 |= XDATDLY(1);regs->xccr |= (DXENDLY(1) | XDMAEN | XDISABLE);
regs->rccr |= RFULL_CYCLE | RDMAEN | RDISABLE;
/* Invert FS polarity configuration */ temp_fmt ^= SND_SOC_DAIFMT_NB_IF; break;regs->xccr |= (DXENDLY(1) | XDMAEN | XDISABLE);
-- 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
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:
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
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
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!
participants (3)
-
Eduardo Valentin
-
Jarkko Nikula
-
Mark Brown