Hi Mark,
We have some patches for Blackfin ASoC updates.
Please take a look.
Thanks a lot
-Bryan
From: Cliff Cai <[email protected]>
set constraint only if the value is not 0, change the configuring way for sport
Signed-off-by: Cliff Cai <[email protected]>
Signed-off-by: Bryan Wu <[email protected]>
---
sound/soc/codecs/ssm2602.c | 29 ++++++++++++++---------------
1 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/sound/soc/codecs/ssm2602.c b/sound/soc/codecs/ssm2602.c
index cac3736..3fdba6a 100644
--- a/sound/soc/codecs/ssm2602.c
+++ b/sound/soc/codecs/ssm2602.c
@@ -49,8 +49,8 @@ struct snd_soc_codec_device soc_codec_dev_ssm2602;
/* codec private data */
struct ssm2602_priv {
unsigned int sysclk;
- struct snd_pcm_substream *master_substream;
- struct snd_pcm_substream *slave_substream;
+ unsigned int master_rate;
+ unsigned int master_sample_bits;
};
/*
@@ -339,31 +339,30 @@ static int ssm2602_startup(struct snd_pcm_substream *substream,
struct snd_soc_codec *codec = socdev->codec;
struct ssm2602_priv *ssm2602 = codec->private_data;
struct i2c_client *i2c = codec->control_data;
- struct snd_pcm_runtime *master_runtime;
/* The DAI has shared clocks so if we already have a playback or
* capture going then constrain this substream to match it.
* TODO: the ssm2602 allows pairs of non-matching PB/REC rates
*/
- if (ssm2602->master_substream) {
- master_runtime = ssm2602->master_substream->runtime;
+ if (ssm2602->master_rate) {
dev_dbg(&i2c->dev, "Constraining to %d bits at %dHz\n",
- master_runtime->sample_bits,
- master_runtime->rate);
+ ssm2602->master_sample_bits,
+ ssm2602->master_rate);
snd_pcm_hw_constraint_minmax(substream->runtime,
SNDRV_PCM_HW_PARAM_RATE,
- master_runtime->rate,
- master_runtime->rate);
-
+ ssm2602->master_rate,
+ ssm2602->master_rate);
+ } else
+ ssm2602->master_rate = substream->runtime->rate;
+
+ if (ssm2602->master_sample_bits) {
snd_pcm_hw_constraint_minmax(substream->runtime,
SNDRV_PCM_HW_PARAM_SAMPLE_BITS,
- master_runtime->sample_bits,
- master_runtime->sample_bits);
-
- ssm2602->slave_substream = substream;
+ ssm2602->master_sample_bits,
+ ssm2602->master_sample_bits);
} else
- ssm2602->master_substream = substream;
+ ssm2602->master_sample_bits = substream->runtime->sample_bits;
return 0;
}
--
1.5.6.3
From: Cliff Cai <[email protected]>
set constraint only if the value is not 0, change the configuring way for sport
Signed-off-by: Cliff Cai <[email protected]>
Signed-off-by: Bryan Wu <[email protected]>
---
sound/soc/blackfin/bf5xx-i2s.c | 25 ++++---------------------
1 files changed, 4 insertions(+), 21 deletions(-)
diff --git a/sound/soc/blackfin/bf5xx-i2s.c b/sound/soc/blackfin/bf5xx-i2s.c
index d1d95d2..29cd5a4 100644
--- a/sound/soc/blackfin/bf5xx-i2s.c
+++ b/sound/soc/blackfin/bf5xx-i2s.c
@@ -49,7 +49,7 @@ struct bf5xx_i2s_port {
u16 rcr1;
u16 tcr2;
u16 rcr2;
- int counter;
+ int configured;
};
static struct bf5xx_i2s_port bf5xx_i2s;
@@ -132,16 +132,6 @@ static int bf5xx_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
return ret;
}
-static int bf5xx_i2s_startup(struct snd_pcm_substream *substream,
- struct snd_soc_dai *dai)
-{
- pr_debug("%s enter\n", __func__);
-
- /*this counter is used for counting how many pcm streams are opened*/
- bf5xx_i2s.counter++;
- return 0;
-}
-
static int bf5xx_i2s_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,
struct snd_soc_dai *dai)
@@ -168,7 +158,7 @@ static int bf5xx_i2s_hw_params(struct snd_pcm_substream *substream,
break;
}
- if (bf5xx_i2s.counter == 1) {
+ if (!bf5xx_i2s.configured) {
/*
* TX and RX are not independent,they are enabled at the
* same time, even if only one side is running. So, we
@@ -195,13 +185,6 @@ static int bf5xx_i2s_hw_params(struct snd_pcm_substream *substream,
return 0;
}
-static void bf5xx_i2s_shutdown(struct snd_pcm_substream *substream,
- struct snd_soc_dai *dai)
-{
- pr_debug("%s enter\n", __func__);
- bf5xx_i2s.counter--;
-}
-
static int bf5xx_i2s_probe(struct platform_device *pdev,
struct snd_soc_dai *dai)
{
@@ -219,6 +202,8 @@ static int bf5xx_i2s_probe(struct platform_device *pdev,
return -ENODEV;
}
+ bf5xx_i2s.configured = 1;
+
return 0;
}
@@ -305,8 +290,6 @@ struct snd_soc_dai bf5xx_i2s_dai = {
.rates = BF5XX_I2S_RATES,
.formats = BF5XX_I2S_FORMATS,},
.ops = {
- .startup = bf5xx_i2s_startup,
- .shutdown = bf5xx_i2s_shutdown,
.hw_params = bf5xx_i2s_hw_params,
.set_fmt = bf5xx_i2s_set_dai_fmt,
},
--
1.5.6.3
From: Mike Frysinger <[email protected]>
Signed-off-by: Mike Frysinger <[email protected]>
Signed-off-by: Bryan Wu <[email protected]>
---
sound/soc/blackfin/bf5xx-ac97.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/blackfin/bf5xx-ac97.c b/sound/soc/blackfin/bf5xx-ac97.c
index 5885702..8a935f2 100644
--- a/sound/soc/blackfin/bf5xx-ac97.c
+++ b/sound/soc/blackfin/bf5xx-ac97.c
@@ -357,8 +357,8 @@ sport_config_err:
sport_err:
#ifdef CONFIG_SND_BF5XX_HAVE_COLD_RESET
gpio_free(CONFIG_SND_BF5XX_RESET_GPIO_NUM);
-#endif
gpio_err:
+#endif
peripheral_free_list(sport_req[sport_num]);
peripheral_err:
free_page((unsigned long)cmd_count);
--
1.5.6.3
From: Mike Frysinger <[email protected]>
Reported-by: Rob Maris <[email protected]>
Signed-off-by: Mike Frysinger <[email protected]>
Signed-off-by: Bryan Wu <[email protected]>
---
sound/soc/codecs/ad73311.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/codecs/ad73311.h b/sound/soc/codecs/ad73311.h
index 507ce0c..569573d 100644
--- a/sound/soc/codecs/ad73311.h
+++ b/sound/soc/codecs/ad73311.h
@@ -70,7 +70,7 @@
#define REGD_IGS(x) (x & 0x7)
#define REGD_RMOD (1 << 3)
#define REGD_OGS(x) ((x & 0x7) << 4)
-#define REGD_MUTE (x << 7)
+#define REGD_MUTE (1 << 7)
/* Control register E */
#define CTRL_REG_E (4 << 8)
--
1.5.6.3
From: Mike Frysinger <[email protected]>
Signed-off-by: Mike Frysinger <[email protected]>
Signed-off-by: Bryan Wu <[email protected]>
---
sound/blackfin/bf53x_sport.c | 55 ++++++++++++++++++-----------------------
1 files changed, 24 insertions(+), 31 deletions(-)
diff --git a/sound/blackfin/bf53x_sport.c b/sound/blackfin/bf53x_sport.c
index 8dfa694..23ef579 100644
--- a/sound/blackfin/bf53x_sport.c
+++ b/sound/blackfin/bf53x_sport.c
@@ -165,7 +165,7 @@ static void setup_desc(struct dmasg *desc, void *buf, int fragcount,
int i;
for (i=0; i<fragcount; ++i) {
- desc[i].next_desc_addr = (unsigned long)&(desc[i + 1]);
+ desc[i].next_desc_addr = &desc[i + 1];
desc[i].start_addr = (unsigned long)buf + i*fragsize;
desc[i].cfg = cfg;
desc[i].x_count = x_count;
@@ -175,7 +175,7 @@ static void setup_desc(struct dmasg *desc, void *buf, int fragcount,
}
/* make circular */
- desc[fragcount-1].next_desc_addr = (unsigned long)desc;
+ desc[fragcount-1].next_desc_addr = desc;
/* printk(KERN_ERR "setup desc: desc0=%p, next0=%lx, desc1=%p,"
"next1=%lx\nx_count=%x,y_count=%x,addr=0x%lx,cfs=0x%x\n",
@@ -217,8 +217,7 @@ static inline int sport_hook_rx_dummy(struct bf53x_sport *sport)
SPORT_ASSERT(sport->dummy_rx_desc != NULL);
SPORT_ASSERT(sport->curr_rx_desc != sport->dummy_rx_desc);
- sport->dummy_rx_desc->next_desc_addr = \
- (unsigned long)(sport->dummy_rx_desc+1);
+ sport->dummy_rx_desc->next_desc_addr = sport->dummy_rx_desc + 1;
local_irq_save(flags);
desc = (struct dmasg *)get_dma_next_desc_ptr(sport->dma_rx_chan);
@@ -226,12 +225,12 @@ static inline int sport_hook_rx_dummy(struct bf53x_sport *sport)
temp_desc = *desc;
desc->x_count=0x10;
desc->y_count=0;
- desc->next_desc_addr = (unsigned long)(sport->dummy_rx_desc);
+ desc->next_desc_addr = sport->dummy_rx_desc;
local_irq_restore(flags);
/* Waiting for dummy buffer descriptor is already hooked*/
while ((get_dma_curr_desc_ptr(sport->dma_rx_chan) - \
- sizeof(struct dmasg)) != \
- (unsigned long)sport->dummy_rx_desc) {}
+ sizeof(struct dmasg)) != sport->dummy_rx_desc)
+ continue;
sport->curr_rx_desc = sport->dummy_rx_desc;
/* Restore the damaged descriptor */
*desc = temp_desc;
@@ -244,13 +243,12 @@ static inline int sport_rx_dma_start(struct bf53x_sport *sport, int dummy)
struct dma_register *dma = sport->dma_rx;
if (dummy) {
- sport->dummy_rx_desc->next_desc_addr = \
- (unsigned long) sport->dummy_rx_desc;
+ sport->dummy_rx_desc->next_desc_addr = sport->dummy_rx_desc;
sport->curr_rx_desc = sport->dummy_rx_desc;
} else
sport->curr_rx_desc = sport->dma_rx_desc;
- dma->next_desc_ptr = (unsigned long)(sport->curr_rx_desc);
+ dma->next_desc_ptr = sport->curr_rx_desc;
dma->cfg = DMAFLOW_LARGE | NDSIZE_9 | WDSIZE_32 | WNR;
dma->x_count = 0;
dma->x_modify = 0;
@@ -267,13 +265,12 @@ static inline int sport_tx_dma_start(struct bf53x_sport *sport, int dummy)
struct dma_register *dma = sport->dma_tx;
if (dummy) {
- sport->dummy_tx_desc->next_desc_addr = \
- (unsigned long) sport->dummy_tx_desc;
+ sport->dummy_tx_desc->next_desc_addr = sport->dummy_tx_desc;
sport->curr_tx_desc = sport->dummy_tx_desc;
} else
sport->curr_tx_desc = sport->dma_tx_desc;
- dma->next_desc_ptr = (unsigned long)(sport->curr_tx_desc);
+ dma->next_desc_ptr = sport->curr_tx_desc;
dma->cfg = DMAFLOW_LARGE | NDSIZE_9 | WDSIZE_32 ;
dma->x_count = 0;
dma->x_modify = 0;
@@ -298,10 +295,9 @@ int bf53x_sport_rx_start(struct bf53x_sport *sport)
SPORT_ASSERT(sport->curr_rx_desc == sport->dummy_rx_desc);
local_irq_save(flags);
while ((get_dma_curr_desc_ptr(sport->dma_rx_chan) - \
- sizeof(struct dmasg)) != \
- (unsigned long)sport->dummy_rx_desc) {}
- sport->dummy_rx_desc->next_desc_addr = \
- (unsigned long)(sport->dma_rx_desc);
+ sizeof(struct dmasg)) != sport->dummy_rx_desc)
+ continue;
+ sport->dummy_rx_desc->next_desc_addr = sport->dma_rx_desc;
local_irq_restore(flags);
sport->curr_rx_desc = sport->dma_rx_desc;
} else {
@@ -343,8 +339,7 @@ static inline int sport_hook_tx_dummy(struct bf53x_sport *sport)
SPORT_ASSERT(sport->dummy_tx_desc != NULL);
SPORT_ASSERT(sport->curr_tx_desc != sport->dummy_tx_desc);
- sport->dummy_tx_desc->next_desc_addr = \
- (unsigned long)(sport->dummy_tx_desc+1);
+ sport->dummy_tx_desc->next_desc_addr = sport->dummy_tx_desc + 1;
/* Shorten the time on last normal descriptor */
local_irq_save(flags);
@@ -353,12 +348,12 @@ static inline int sport_hook_tx_dummy(struct bf53x_sport *sport)
temp_desc = *desc;
desc->x_count = 0x10;
desc->y_count = 0;
- desc->next_desc_addr = (unsigned long)(sport->dummy_tx_desc);
+ desc->next_desc_addr = sport->dummy_tx_desc;
local_irq_restore(flags);
/* Waiting for dummy buffer descriptor is already hooked*/
while ((get_dma_curr_desc_ptr(sport->dma_tx_chan) - \
- sizeof(struct dmasg)) != \
- (unsigned long)sport->dummy_tx_desc) {}
+ sizeof(struct dmasg)) != sport->dummy_tx_desc)
+ continue;
sport->curr_tx_desc = sport->dummy_tx_desc;
/* Restore the damaged descriptor */
*desc = temp_desc;
@@ -381,10 +376,9 @@ int bf53x_sport_tx_start(struct bf53x_sport *sport)
/* Hook the normal buffer descriptor */
local_irq_save(flags);
while ((get_dma_curr_desc_ptr(sport->dma_tx_chan) - \
- sizeof(struct dmasg)) != \
- (unsigned long)sport->dummy_tx_desc) {}
- sport->dummy_tx_desc->next_desc_addr = \
- (unsigned long)(sport->dma_tx_desc);
+ sizeof(struct dmasg)) != sport->dummy_tx_desc)
+ continue;
+ sport->dummy_tx_desc->next_desc_addr = sport->dma_tx_desc;
local_irq_restore(flags);
sport->curr_tx_desc = sport->dma_tx_desc;
} else {
@@ -587,8 +581,8 @@ static int sport_config_rx_dummy(struct bf53x_sport *sport, size_t size)
desc->y_count = 0;
desc->y_modify = 0;
memcpy(desc+1, desc, sizeof(*desc));
- desc->next_desc_addr = (unsigned long)(desc+1);
- desc[1].next_desc_addr = (unsigned long)desc;
+ desc->next_desc_addr = desc + 1;
+ desc[1].next_desc_addr = desc;
return 0;
}
@@ -623,8 +617,8 @@ static int sport_config_tx_dummy(struct bf53x_sport *sport, size_t size)
desc->y_count = 0;
desc->y_modify = 0;
memcpy(desc+1, desc, sizeof(*desc));
- desc->next_desc_addr = (unsigned long)(desc+1);
- desc[1].next_desc_addr = (unsigned long)desc;
+ desc->next_desc_addr = desc + 1;
+ desc[1].next_desc_addr = desc;
return 0;
}
@@ -918,6 +912,5 @@ void bf53x_sport_done(struct bf53x_sport *sport)
free_dma(sport->dma_tx_chan);
free_irq(sport->err_irq, sport);
-
kfree(sport);
}
--
1.5.6.3
Bryan Wu wrote:
> From: Cliff Cai <[email protected]>
>
> set constraint only if the value is not 0, change the configuring way for sport
>
> Signed-off-by: Cliff Cai <[email protected]>
> Signed-off-by: Bryan Wu <[email protected]>
> ---
> sound/soc/codecs/ssm2602.c | 29 ++++++++++++++---------------
> 1 files changed, 14 insertions(+), 15 deletions(-)
>
What tree is this against ?
--
Karl Beldan
On Fri, Mar 06, 2009 at 03:53:28PM +0800, Bryan Wu wrote:
> From: Mike Frysinger <[email protected]>
>
> Signed-off-by: Mike Frysinger <[email protected]>
> Signed-off-by: Bryan Wu <[email protected]>
Applied, thanks.
On Fri, Mar 06, 2009 at 03:53:30PM +0800, Bryan Wu wrote:
> From: Mike Frysinger <[email protected]>
>
> Reported-by: Rob Maris <[email protected]>
> Signed-off-by: Mike Frysinger <[email protected]>
> Signed-off-by: Bryan Wu <[email protected]>
Applied, thanks.
On Fri, Mar 06, 2009 at 03:53:29PM +0800, Bryan Wu wrote:
> From: Mike Frysinger <[email protected]>
>
> Signed-off-by: Mike Frysinger <[email protected]>
> Signed-off-by: Bryan Wu <[email protected]>
> ---
> sound/blackfin/bf53x_sport.c | 55 ++++++++++++++++++-----------------------
> 1 files changed, 24 insertions(+), 31 deletions(-)
Mainline doesn't have a sound/blackfin directory at all as far as I can
tell so this patch doesn't apply? The only mainline support I can see
for blackfin audio is sound/soc/blackfin which only has a generic
bf5xx-sport file. A similar cleanup was done there a while ago.
On Fri, Mar 06, 2009 at 03:53:26PM +0800, Bryan Wu wrote:
> From: Cliff Cai <[email protected]>
>
> set constraint only if the value is not 0, change the configuring way for sport
Hrm. As far as I can tell the actual effect of this patch is to not do
any of the per-format configuration for the sport if the sport has been
configured once already - as far as I can tell nothing ever resets your
'configured' variable and this is the only place that the data format is
taken into account. Won't this mean that if a second data format is
played the audio will be mishandled since the hardware will not have
been configured for the new audio format?
If it's really not possible to reconfigure the hardware (I'm assuming
that this is what the actual crash is?) I would expect to see code added
which remembers the format that has been configured and then adds a
constraint in the startup() function enforcing that.
On Fri, Mar 06, 2009 at 03:53:27PM +0800, Bryan Wu wrote:
> set constraint only if the value is not 0, change the configuring way for sport
This changelog doesn't appear to apply to this patch - it looks like
was copied from the previous one? This patch appears to suffer from a
similar issue where only the first format set by an application can ever
be used, though since this changes the constraints that are set by the
driver there's no possibility that applications will end up trying to
use a sample format that the hardware is not configured for.
Do we really need such a heavy handed fix here - what is the issue
that's being fixed?
On Fri, Mar 6, 2009 at 7:10 PM, Mark Brown <[email protected]> wrote:
> On Fri, Mar 06, 2009 at 03:53:29PM +0800, Bryan Wu wrote:
>> From: Mike Frysinger <[email protected]>
>>
>> Signed-off-by: Mike Frysinger <[email protected]>
>> Signed-off-by: Bryan Wu <[email protected]>
>> ---
>> ?sound/blackfin/bf53x_sport.c | ? 55 ++++++++++++++++++-----------------------
>> ?1 files changed, 24 insertions(+), 31 deletions(-)
>
> Mainline doesn't have a sound/blackfin directory at all as far as I can
> tell so this patch doesn't apply? ?The only mainline support I can see
> for blackfin audio is sound/soc/blackfin which only has a generic
> bf5xx-sport file. ?A similar cleanup was done there a while ago.
>
Oh, god. Sorry, my script sent out this in a mess. Please ignore this one
Thanks a lot
-Bryan
This patch should be named :set constraint only if the value is not 0.
I found that sometimes substream may be not NULL,but runtime can be
NULL,So the previous way will cause
Kernel to crash.
Cliff
>-----Original Message-----
>From: Mark Brown [mailto:[email protected]]
>Sent: Friday, March 06, 2009 8:35 PM
>To: Bryan Wu
>Cc: Cliff Cai; [email protected];
>[email protected]
>Subject: Re: [alsa-devel] [PATCH 2/5] ASoC: ssm2602 codec: fix
>bug - kernelwill crash when record and play in bf527-ezkit
>
>On Fri, Mar 06, 2009 at 03:53:27PM +0800, Bryan Wu wrote:
>
>> set constraint only if the value is not 0, change the
>configuring way
>> for sport
>
>This changelog doesn't appear to apply to this patch - it
>looks like was copied from the previous one? This patch
>appears to suffer from a similar issue where only the first
>format set by an application can ever be used, though since
>this changes the constraints that are set by the driver
>there's no possibility that applications will end up trying to
>use a sample format that the hardware is not configured for.
>
>Do we really need such a heavy handed fix here - what is the
>issue that's being fixed?
>
This patch should be named : change the configuring way for sport,
Because I found that the previous way is not reliable sometimes.
Application like "tone" ,will call starup() twice before entering
hw_params() to configure SPORT.
in this case SPORT won't be configured at all.
Cliff
>-----Original Message-----
>From: Mark Brown [mailto:[email protected]]
>Sent: Friday, March 06, 2009 8:02 PM
>To: Bryan Wu
>Cc: Cliff Cai; [email protected];
>[email protected]
>Subject: Re: [alsa-devel] [PATCH 1/5] ASoC: Blackfin: fix bug
>- kernel willcrash when record and play in bf527-ezkit
>
>On Fri, Mar 06, 2009 at 03:53:26PM +0800, Bryan Wu wrote:
>> From: Cliff Cai <[email protected]>
>>
>> set constraint only if the value is not 0, change the
>configuring way
>> for sport
>
>Hrm. As far as I can tell the actual effect of this patch is
>to not do any of the per-format configuration for the sport if
>the sport has been configured once already - as far as I can
>tell nothing ever resets your 'configured' variable and this
>is the only place that the data format is taken into account.
>Won't this mean that if a second data format is played the
>audio will be mishandled since the hardware will not have been
>configured for the new audio format?
>
>If it's really not possible to reconfigure the hardware (I'm
>assuming that this is what the actual crash is?) I would
>expect to see code added which remembers the format that has
>been configured and then adds a constraint in the startup()
>function enforcing that.
>
On Mon, Mar 09, 2009 at 06:58:19PM +0800, Cai, Cliff wrote:
> This patch should be named : change the configuring way for sport,
> Because I found that the previous way is not reliable sometimes.
> Application like "tone" ,will call starup() twice before entering
> hw_params() to configure SPORT.
> in this case SPORT won't be configured at all.
This doesn't address the concerns I raised with the patch:
> >Won't this mean that if a second data format is played the
> >audio will be mishandled since the hardware will not have been
> >configured for the new audio format?
On Mon, Mar 09, 2009 at 07:07:26PM +0800, Cai, Cliff wrote:
> This patch should be named :set constraint only if the value is not 0.
> I found that sometimes substream may be not NULL,but runtime can be
> NULL,So the previous way will cause
> Kernel to crash.
Again, this doesn't address the concern about the fact that you're only
supporting one time configuration here.
>-----Original Message-----
>From: Mark Brown [mailto:[email protected]]
>Sent: Monday, March 09, 2009 7:21 PM
>To: Cai, Cliff
>Cc: Bryan Wu; [email protected]; [email protected]
>Subject: Re: [alsa-devel] [PATCH 1/5] ASoC: Blackfin: fix bug
>- kernelwillcrash when record and play in bf527-ezkit
>
>On Mon, Mar 09, 2009 at 06:58:19PM +0800, Cai, Cliff wrote:
>
>> This patch should be named : change the configuring way for sport,
>> Because I found that the previous way is not reliable sometimes.
>> Application like "tone" ,will call starup() twice before entering
>> hw_params() to configure SPORT.
>> in this case SPORT won't be configured at all.
>
>This doesn't address the concerns I raised with the patch:
>
>> >Won't this mean that if a second data format is played the
>audio will
>> >be mishandled since the hardware will not have been configured for
>> >the new audio format?
>
I think I understand your concerns,I will send a new patch if I'm free.
Thanks
Cliff