[PATCH] ALSA: ASoC V2: report error if DMA doesn't start in FSL MPC8610 sound drivers

[PATCH] ALSA: ASoC V2: report error if DMA doesn't start in FSL MPC8610 sound drivers

[Timur Tabi]

Return an error if the DMA's current pointer is not within the bounds of the
DMA buffer.  This can happen if the DMA controller is not programmed correctly,
or if the SSI doesn't communicate with the DMA controller correctly.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

This patch is for ASoC V2.

 sound/soc/fsl/fsl_dma.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c
index cb1e5ae..b4ae486 100644
--- a/sound/soc/fsl/fsl_dma.c
+++ b/sound/soc/fsl/fsl_dma.c
@@ -689,6 +689,15 @@ static snd_pcm_uframes_t fsl_dma_pointer(struct snd_pcm_substream *substream)
 	else
 		position = in_be32(&dma_channel->dar);
 
+	if ((position < dma_private->dma_buf_phys) ||
+	    (position > dma_private->dma_buf_end)) {
+		dev_err(substream->pcm->card->dev,
+			"DMA(%u,%u) pointer is out of range, halting stream\n",
+			dma_private->dma_info->controller_id,
+			dma_private->dma_info->channel_id);
+		return SNDRV_PCM_POS_XRUN;
+	}
+
 	frames = bytes_to_frames(runtime, position - dma_private->dma_buf_phys);
 
 	/*
-- 
1.5.5

[PATCH] ALSA: ASoC V2: disable automatic volume control in the CS4270 sound driver

[Timur Tabi]

Disable the automatic volume control feature of the CS4270 audio codec.  This
feature, which is enabled by default, causes volume change commands to be
delayed.  Sometimes the volume change happens after playback is started.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

This patch is for ASoC V2.

 sound/soc/codecs/cs4270.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/sound/soc/codecs/cs4270.c b/sound/soc/codecs/cs4270.c
index df3294d..e1c3fbd 100644
--- a/sound/soc/codecs/cs4270.c
+++ b/sound/soc/codecs/cs4270.c
@@ -430,6 +430,19 @@ static int cs4270_hw_params(struct snd_pcm_substream *substream,
 		return ret;
 	}
 
+	/* Disable automatic volume control.  It's enabled by default, and
+	 * it causes volume change commands to be delayed, sometimes until
+	 * after playback has started.
+	 */
+
+	reg = cs4270_read_reg_cache(codec, CS4270_TRANS);
+	reg &= ~(CS4270_TRANS_SOFT | CS4270_TRANS_ZERO);
+	ret = cs4270_i2c_write(codec, CS4270_TRANS, reg);
+	if (ret < 0) {
+		dev_err(codec->dev, "I2C write failed\n");
+		return ret;
+	}
+
 	/* Thaw and power-up the codec */
 
 	ret = cs4270_i2c_write(codec, CS4270_PWRCTL, 0);
-- 
1.5.5

[PATCH] ALSA: ASoC V2: trim SSI sysfs statistics in Freescale MPC8610 sound drivers

[Timur Tabi]

Optimize the display of SSI statistics in the Freescale MPC8610 sound driver
to display the status count only of the interrupts that were actually enabled.
Previously, it would display the counts of all SISR status bits, even those
that were not enabled.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

This patch is for ASoC V2.

 sound/soc/fsl/fsl_ssi.c |   78 +++++++++++++++++++++++++++-------------------
 1 files changed, 46 insertions(+), 32 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 146587e..d0e200e 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -63,6 +63,13 @@
 	 SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_LE)
 #endif
 
+/* SIER bitflag of interrupts to enable */
+#define SIER_FLAGS (CCSR_SSI_SIER_TFRC_EN | CCSR_SSI_SIER_TDMAE | \
+		    CCSR_SSI_SIER_TIE | CCSR_SSI_SIER_TUE0_EN | \
+		    CCSR_SSI_SIER_TUE1_EN | CCSR_SSI_SIER_RFRC_EN | \
+		    CCSR_SSI_SIER_RDMAE | CCSR_SSI_SIER_RIE | \
+		    CCSR_SSI_SIER_ROE0_EN | CCSR_SSI_SIER_ROE1_EN)
+
 /**
  * fsl_ssi_isr: SSI interrupt handler
  *
@@ -87,7 +94,7 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id)
 	   were interrupted for.  We mask it with the Interrupt Enable register
 	   so that we only check for events that we're interested in.
 	 */
-	sisr = in_be32(&ssi->sisr) & in_be32(&ssi->sier);
+	sisr = in_be32(&ssi->sisr) & SIER_FLAGS;
 
 	if (sisr & CCSR_SSI_SISR_RFRC) {
 		ssi_info->stats.rfrc++;
@@ -266,12 +273,7 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream,
 		 */
 
 		/* 4. Enable the interrupts and DMA requests */
-		out_be32(&ssi->sier,
-			 CCSR_SSI_SIER_TFRC_EN | CCSR_SSI_SIER_TDMAE |
-			 CCSR_SSI_SIER_TIE | CCSR_SSI_SIER_TUE0_EN |
-			 CCSR_SSI_SIER_TUE1_EN | CCSR_SSI_SIER_RFRC_EN |
-			 CCSR_SSI_SIER_RDMAE | CCSR_SSI_SIER_RIE |
-			 CCSR_SSI_SIER_ROE0_EN | CCSR_SSI_SIER_ROE1_EN);
+		out_be32(&ssi->sier, SIER_FLAGS);
 
 		/*
 		 * Set the watermark for transmit FIFI 0 and receive FIFO 0. We
@@ -520,39 +522,51 @@ static int fsl_ssi_set_fmt(struct snd_soc_dai *dai, unsigned int format)
 	return (format == SND_SOC_DAIFMT_I2S) ? 0 : -EINVAL;
 }
 
+/* Show the statistics of a flag only if its interrupt is enabled.  The
+ * compiler will optimze this code to a no-op if the interrupt is not
+ * enabled.
+ */
+#define SIER_SHOW(flag, name) \
+	do { \
+		if (SIER_FLAGS & CCSR_SSI_SIER_##flag) \
+			length += sprintf(buf + length, #name "=%u\n", \
+				ssi_info->stats.name); \
+	} while (0)
+
 /**
  * fsl_sysfs_ssi_show: display SSI statistics
  *
- * Display the statistics for the current SSI device.
+ * Display the statistics for the current SSI device.  To avoid confusion,
+ * we only show those counts that are enabled.
  */
 static ssize_t fsl_sysfs_ssi_show(struct device *dev,
 	struct device_attribute *attr, char *buf)
 {
 	struct fsl_ssi_info *ssi_info =
-	container_of(attr, struct fsl_ssi_info, dev_attr);
-	ssize_t length;
-
-	length = sprintf(buf, "rfrc=%u", ssi_info->stats.rfrc);
-	length += sprintf(buf + length, "\ttfrc=%u", ssi_info->stats.tfrc);
-	length += sprintf(buf + length, "\tcmdau=%u", ssi_info->stats.cmdau);
-	length += sprintf(buf + length, "\tcmddu=%u", ssi_info->stats.cmddu);
-	length += sprintf(buf + length, "\trxt=%u", ssi_info->stats.rxt);
-	length += sprintf(buf + length, "\trdr1=%u", ssi_info->stats.rdr1);
-	length += sprintf(buf + length, "\trdr0=%u", ssi_info->stats.rdr0);
-	length += sprintf(buf + length, "\ttde1=%u", ssi_info->stats.tde1);
-	length += sprintf(buf + length, "\ttde0=%u", ssi_info->stats.tde0);
-	length += sprintf(buf + length, "\troe1=%u", ssi_info->stats.roe1);
-	length += sprintf(buf + length, "\troe0=%u", ssi_info->stats.roe0);
-	length += sprintf(buf + length, "\ttue1=%u", ssi_info->stats.tue1);
-	length += sprintf(buf + length, "\ttue0=%u", ssi_info->stats.tue0);
-	length += sprintf(buf + length, "\ttfs=%u", ssi_info->stats.tfs);
-	length += sprintf(buf + length, "\trfs=%u", ssi_info->stats.rfs);
-	length += sprintf(buf + length, "\ttls=%u", ssi_info->stats.tls);
-	length += sprintf(buf + length, "\trls=%u", ssi_info->stats.rls);
-	length += sprintf(buf + length, "\trff1=%u", ssi_info->stats.rff1);
-	length += sprintf(buf + length, "\trff0=%u", ssi_info->stats.rff0);
-	length += sprintf(buf + length, "\ttfe1=%u", ssi_info->stats.tfe1);
-	length += sprintf(buf + length, "\ttfe0=%u\n", ssi_info->stats.tfe0);
+		container_of(attr, struct fsl_ssi_info, dev_attr);
+	ssize_t length = 0;
+
+	SIER_SHOW(RFRC_EN, rfrc);
+	SIER_SHOW(TFRC_EN, tfrc);
+	SIER_SHOW(CMDAU_EN, cmdau);
+	SIER_SHOW(CMDDU_EN, cmddu);
+	SIER_SHOW(RXT_EN, rxt);
+	SIER_SHOW(RDR1_EN, rdr1);
+	SIER_SHOW(RDR0_EN, rdr0);
+	SIER_SHOW(TDE1_EN, tde1);
+	SIER_SHOW(TDE0_EN, tde0);
+	SIER_SHOW(ROE1_EN, roe1);
+	SIER_SHOW(ROE0_EN, roe0);
+	SIER_SHOW(TUE1_EN, tue1);
+	SIER_SHOW(TUE0_EN, tue0);
+	SIER_SHOW(TFS_EN, tfs);
+	SIER_SHOW(RFS_EN, rfs);
+	SIER_SHOW(TLS_EN, tls);
+	SIER_SHOW(RLS_EN, rls);
+	SIER_SHOW(RFF1_EN, rff1);
+	SIER_SHOW(RFF0_EN, rff0);
+	SIER_SHOW(TFE1_EN, tfe1);
+	SIER_SHOW(TFE0_EN, tfe0);
 
 	return length;
 }
-- 
1.5.5

Re: [PATCH] ALSA: ASoC V2: disable automatic volume control in the CS4270 sound driver

[Mark Brown]

On Thu, Aug 07, 2008 at 11:22:32AM -0500, Timur Tabi wrote:
> Disable the automatic volume control feature of the CS4270 audio codec.  This
> feature, which is enabled by default, causes volume change commands to be
> delayed.  Sometimes the volume change happens after playback is started.

All applied, thanks.

Should this also be done for current mainline?

Re: [PATCH] ALSA: ASoC V2: disable automatic volume control in the CS4270 sound driver

[Timur Tabi]

Mark Brown wrote:
> On Thu, Aug 07, 2008 at 11:22:32AM -0500, Timur Tabi wrote:
>> Disable the automatic volume control feature of the CS4270 audio codec.  This
>> feature, which is enabled by default, causes volume change commands to be
>> delayed.  Sometimes the volume change happens after playback is started.
> 
> All applied, thanks.
> 
> Should this also be done for current mainline?

No.  I'm only doing critical fixes for mainline now.  Unless someone
specifically asks for this particular fix, I'm not going to put it in.  I have
no more planned fixes for mainline.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] ALSA: ASoC V2: disable automatic volume control in the CS4270 sound driver

[Timur Tabi]

Timur Tabi wrote:
> Disable the automatic volume control feature of the CS4270 audio codec.  This
> feature, which is enabled by default, causes volume change commands to be
> delayed.  Sometimes the volume change happens after playback is started.
> 
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
> 
> This patch is for ASoC V2.

Did you guys forget about this patch?  I don't see it in asoc-v2-dev.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] ALSA: ASoC V2: disable automatic volume control in the CS4270 sound driver

[Mark Brown]

On Fri, Aug 22, 2008 at 04:18:49PM -0500, Timur Tabi wrote:

> Did you guys forget about this patch?  I don't see it in asoc-v2-dev.

Just lost in the shuffle.  I've managed to locate a copy in my archives
and applied it now.  When sending reminders like this the more normal
approach is to resubmit the entire patch - apart from anything else,
there's a reasonable chance that if the patch was lost the original mail
has been lost as well so it'll need to be resent anyway.

It really would be better if changes like this could also be made to
mainline as well - it is much easier to review the differences between
the two branches if there aren't changes like this one that are
unrelated to the API changes mixed in as well.