From 1fab6cafc798c987caa6e98ee8e04991e9171cd0 Mon Sep 17 00:00:00 2001 From: Timur Tabi Date: Tue, 16 Aug 2011 18:47:45 -0400 Subject: ASoC: claim the IRQ when the fsl_ssi device is probed, not opened The PowerPC Freescale SSI driver is claiming the IRQ when the IRQ when the device is opened, which means that the /proc/interrupts entry for the SSI exists only during playback or capture. This also meant that the user won't know that the IRQ number is wrong until he tries to use the device. Instead, we should claim the IRQ when the device is probed. Signed-off-by: Timur Tabi Signed-off-by: Mark Brown --- sound/soc/fsl/fsl_ssi.c | 61 ++++++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 24 deletions(-) (limited to 'sound/soc/fsl') diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index d48afea5d93d..06ac2b92faf3 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -289,16 +289,6 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, */ if (!ssi_private->playback && !ssi_private->capture) { struct ccsr_ssi __iomem *ssi = ssi_private->ssi; - int ret; - - /* The 'name' should not have any slashes in it. */ - ret = request_irq(ssi_private->irq, fsl_ssi_isr, 0, - ssi_private->name, ssi_private); - if (ret < 0) { - dev_err(substream->pcm->card->dev, - "could not claim irq %u\n", ssi_private->irq); - return ret; - } /* * Section 16.5 of the MPC8610 reference manual says that the @@ -522,15 +512,12 @@ static void fsl_ssi_shutdown(struct snd_pcm_substream *substream, ssi_private->second_stream = NULL; /* - * If this is the last active substream, disable the SSI and release - * the IRQ. + * If this is the last active substream, disable the SSI. */ if (!ssi_private->playback && !ssi_private->capture) { struct ccsr_ssi __iomem *ssi = ssi_private->ssi; clrbits32(&ssi->scr, CCSR_SSI_SCR_SSIEN); - - free_irq(ssi_private->irq, ssi_private); } } @@ -675,17 +662,30 @@ static int __devinit fsl_ssi_probe(struct platform_device *pdev) ret = of_address_to_resource(np, 0, &res); if (ret) { dev_err(&pdev->dev, "could not determine device resources\n"); - kfree(ssi_private); - return ret; + goto error_kmalloc; } ssi_private->ssi = of_iomap(np, 0); if (!ssi_private->ssi) { dev_err(&pdev->dev, "could not map device resources\n"); - kfree(ssi_private); - return -ENOMEM; + ret = -ENOMEM; + goto error_kmalloc; } ssi_private->ssi_phys = res.start; + ssi_private->irq = irq_of_parse_and_map(np, 0); + if (ssi_private->irq == NO_IRQ) { + dev_err(&pdev->dev, "no irq for node %s\n", np->full_name); + ret = -ENXIO; + goto error_iomap; + } + + /* The 'name' should not have any slashes in it. */ + ret = request_irq(ssi_private->irq, fsl_ssi_isr, 0, ssi_private->name, + ssi_private); + if (ret < 0) { + dev_err(&pdev->dev, "could not claim irq %u\n", ssi_private->irq); + goto error_irqmap; + } /* Are the RX and the TX clocks locked? */ if (of_find_property(np, "fsl,ssi-asynchronous", NULL)) @@ -711,7 +711,7 @@ static int __devinit fsl_ssi_probe(struct platform_device *pdev) if (ret) { dev_err(&pdev->dev, "could not create sysfs %s file\n", ssi_private->dev_attr.attr.name); - goto error; + goto error_irq; } /* Register with ASoC */ @@ -720,7 +720,7 @@ static int __devinit fsl_ssi_probe(struct platform_device *pdev) ret = snd_soc_register_dai(&pdev->dev, &ssi_private->cpu_dai_drv); if (ret) { dev_err(&pdev->dev, "failed to register DAI: %d\n", ret); - goto error; + goto error_dev; } /* Trigger the machine driver's probe function. The platform driver @@ -741,18 +741,28 @@ static int __devinit fsl_ssi_probe(struct platform_device *pdev) if (IS_ERR(ssi_private->pdev)) { ret = PTR_ERR(ssi_private->pdev); dev_err(&pdev->dev, "failed to register platform: %d\n", ret); - goto error; + goto error_dai; } return 0; -error: +error_dai: snd_soc_unregister_dai(&pdev->dev); + +error_dev: dev_set_drvdata(&pdev->dev, NULL); - if (dev_attr) - device_remove_file(&pdev->dev, dev_attr); + device_remove_file(&pdev->dev, dev_attr); + +error_irq: + free_irq(ssi_private->irq, ssi_private); + +error_irqmap: irq_dispose_mapping(ssi_private->irq); + +error_iomap: iounmap(ssi_private->ssi); + +error_kmalloc: kfree(ssi_private); return ret; @@ -766,6 +776,9 @@ static int fsl_ssi_remove(struct platform_device *pdev) snd_soc_unregister_dai(&pdev->dev); device_remove_file(&pdev->dev, &ssi_private->dev_attr); + free_irq(ssi_private->irq, ssi_private); + irq_dispose_mapping(ssi_private->irq); + kfree(ssi_private); dev_set_drvdata(&pdev->dev, NULL); -- cgit v1.2.3 From 96af5c6a8266003a2212d9d0b383603f1af9b109 Mon Sep 17 00:00:00 2001 From: Timur Tabi Date: Tue, 16 Aug 2011 21:58:29 -0400 Subject: ASoC: fsl: fix build warning in fsl_dma The previous patch to fsl_dma.c ("fix initialization of DMA buffers") left behind an unused local variable that causes a build warning. Signed-off-by: Timur Tabi Signed-off-by: Mark Brown --- sound/soc/fsl/fsl_dma.c | 1 - 1 file changed, 1 deletion(-) (limited to 'sound/soc/fsl') diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c index 732208c8c0b4..0efc04af8f15 100644 --- a/sound/soc/fsl/fsl_dma.c +++ b/sound/soc/fsl/fsl_dma.c @@ -297,7 +297,6 @@ static irqreturn_t fsl_dma_isr(int irq, void *dev_id) static int fsl_dma_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; struct snd_pcm *pcm = rtd->pcm; static u64 fsl_dma_dmamask = DMA_BIT_MASK(36); int ret; -- cgit v1.2.3 From 1e3ad571d56ab96e5fab87cea71c0e657d4708cb Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Mon, 29 Aug 2011 14:10:42 +0100 Subject: ASoC: Remove redundant -codec from WM8776 driver name Signed-off-by: Mark Brown Acked-by: Timur Tabi --- sound/soc/codecs/wm8776.c | 4 ++-- sound/soc/fsl/p1022_ds.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'sound/soc/fsl') diff --git a/sound/soc/codecs/wm8776.c b/sound/soc/codecs/wm8776.c index 8e7953b1b790..367a990e6cc4 100644 --- a/sound/soc/codecs/wm8776.c +++ b/sound/soc/codecs/wm8776.c @@ -481,7 +481,7 @@ static int __devexit wm8776_spi_remove(struct spi_device *spi) static struct spi_driver wm8776_spi_driver = { .driver = { - .name = "wm8776-codec", + .name = "wm8776", .owner = THIS_MODULE, }, .probe = wm8776_spi_probe, @@ -525,7 +525,7 @@ MODULE_DEVICE_TABLE(i2c, wm8776_i2c_id); static struct i2c_driver wm8776_i2c_driver = { .driver = { - .name = "wm8776-codec", + .name = "wm8776", .owner = THIS_MODULE, }, .probe = wm8776_i2c_probe, diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c index fcb862eb0c73..e8849ed36cbd 100644 --- a/sound/soc/fsl/p1022_ds.c +++ b/sound/soc/fsl/p1022_ds.c @@ -267,7 +267,7 @@ static int codec_node_dev_name(struct device_node *np, char *buf, size_t len) if (bus < 0) return bus; - snprintf(buf, len, "%s-codec.%u-%04x", temp, bus, addr); + snprintf(buf, len, "%s.%u-%04x", temp, bus, addr); return 0; } -- cgit v1.2.3 From 5e538ecade22a5ec4c8e18d494db0ecf924254eb Mon Sep 17 00:00:00 2001 From: Timur Tabi Date: Tue, 13 Sep 2011 12:59:37 -0500 Subject: ASoC: improve asynchronous mode support in the fsl_ssi driver The Freescale SSI audio controller supports "synchronous" and "asynchronous" modes. In synchronous mode, playback and capture use the same input clock, so sample rates must be the same during simultaneous playback and capture. Unfortunately, the code which supports asynchronous mode is just broken in various ways. In particular, it was constraining sample sizes as well as the sample rate. The fix also allows us to simplify the code by eliminating the 'asynchronous', 'playback', and 'capture' variables that were used to keep track of playback and capture streams. Unfortunately, it turns out that simulataneous playback and record does not actually work on the only platform that supports asynchronous mode: the Freescale P1022DS reference board. If a second stream is started, the SSI grinds to halt for both streams. This is true even if the P1022 is configured for synchronous mode, so it's likely a hardware problem that needs to be worked around. Signed-off-by: Timur Tabi Acked-by: Liam Girdwood Signed-off-by: Mark Brown --- sound/soc/fsl/fsl_ssi.c | 145 +++++++++++++++++++++++------------------------- 1 file changed, 68 insertions(+), 77 deletions(-) (limited to 'sound/soc/fsl') diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 06ac2b92faf3..0268cf989736 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -78,7 +78,6 @@ * @second_stream: pointer to second stream * @playback: the number of playback streams opened * @capture: the number of capture streams opened - * @asynchronous: 0=synchronous mode, 1=asynchronous mode * @cpu_dai: the CPU DAI for this device * @dev_attr: the sysfs device attribute structure * @stats: SSI statistics @@ -90,9 +89,6 @@ struct fsl_ssi_private { unsigned int irq; struct snd_pcm_substream *first_stream; struct snd_pcm_substream *second_stream; - unsigned int playback; - unsigned int capture; - int asynchronous; unsigned int fifo_depth; struct snd_soc_dai_driver cpu_dai_drv; struct device_attribute dev_attr; @@ -281,15 +277,19 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(rtd->cpu_dai); + struct fsl_ssi_private *ssi_private = + snd_soc_dai_get_drvdata(rtd->cpu_dai); + int synchronous = ssi_private->cpu_dai_drv.symmetric_rates; /* * If this is the first stream opened, then request the IRQ * and initialize the SSI registers. */ - if (!ssi_private->playback && !ssi_private->capture) { + if (!ssi_private->first_stream) { struct ccsr_ssi __iomem *ssi = ssi_private->ssi; + ssi_private->first_stream = substream; + /* * Section 16.5 of the MPC8610 reference manual says that the * SSI needs to be disabled before updating the registers we set @@ -306,7 +306,7 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, clrsetbits_be32(&ssi->scr, CCSR_SSI_SCR_I2S_MODE_MASK | CCSR_SSI_SCR_SYN, CCSR_SSI_SCR_TFR_CLK_DIS | CCSR_SSI_SCR_I2S_MODE_SLAVE - | (ssi_private->asynchronous ? 0 : CCSR_SSI_SCR_SYN)); + | (synchronous ? CCSR_SSI_SCR_SYN : 0)); out_be32(&ssi->stcr, CCSR_SSI_STCR_TXBIT0 | CCSR_SSI_STCR_TFEN0 | @@ -323,7 +323,7 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, * master. */ - /* 4. Enable the interrupts and DMA requests */ + /* Enable the interrupts and DMA requests */ out_be32(&ssi->sier, SIER_FLAGS); /* @@ -352,58 +352,47 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, * this is bad is because at this point, the PCM driver has not * finished initializing the DMA controller. */ - } - - if (!ssi_private->first_stream) - ssi_private->first_stream = substream; - else { - /* This is the second stream open, so we need to impose sample - * rate and maybe sample size constraints. Note that this can - * cause a race condition if the second stream is opened before - * the first stream is fully initialized. - * - * We provide some protection by checking to make sure the first - * stream is initialized, but it's not perfect. ALSA sometimes - * re-initializes the driver with a different sample rate or - * size. If the second stream is opened before the first stream - * has received its final parameters, then the second stream may - * be constrained to the wrong sample rate or size. - * - * FIXME: This code does not handle opening and closing streams - * repeatedly. If you open two streams and then close the first - * one, you may not be able to open another stream until you - * close the second one as well. - */ - struct snd_pcm_runtime *first_runtime = - ssi_private->first_stream->runtime; - - if (!first_runtime->sample_bits) { - dev_err(substream->pcm->card->dev, - "set sample size in %s stream first\n", - substream->stream == SNDRV_PCM_STREAM_PLAYBACK - ? "capture" : "playback"); - return -EAGAIN; - } + } else { + if (synchronous) { + struct snd_pcm_runtime *first_runtime = + ssi_private->first_stream->runtime; + /* + * This is the second stream open, and we're in + * synchronous mode, so we need to impose sample + * sample size constraints. This is because STCCR is + * used for playback and capture in synchronous mode, + * so there's no way to specify different word + * lengths. + * + * Note that this can cause a race condition if the + * second stream is opened before the first stream is + * fully initialized. We provide some protection by + * checking to make sure the first stream is + * initialized, but it's not perfect. ALSA sometimes + * re-initializes the driver with a different sample + * rate or size. If the second stream is opened + * before the first stream has received its final + * parameters, then the second stream may be + * constrained to the wrong sample rate or size. + */ + if (!first_runtime->sample_bits) { + dev_err(substream->pcm->card->dev, + "set sample size in %s stream first\n", + substream->stream == + SNDRV_PCM_STREAM_PLAYBACK + ? "capture" : "playback"); + return -EAGAIN; + } - /* If we're in synchronous mode, then we need to constrain - * the sample size as well. We don't support independent sample - * rates in asynchronous mode. - */ - if (!ssi_private->asynchronous) snd_pcm_hw_constraint_minmax(substream->runtime, SNDRV_PCM_HW_PARAM_SAMPLE_BITS, first_runtime->sample_bits, first_runtime->sample_bits); + } ssi_private->second_stream = substream; } - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - ssi_private->playback++; - - if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) - ssi_private->capture++; - return 0; } @@ -424,24 +413,35 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *hw_params, struct snd_soc_dai *cpu_dai) { struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(cpu_dai); + struct ccsr_ssi __iomem *ssi = ssi_private->ssi; + unsigned int sample_size = + snd_pcm_format_width(params_format(hw_params)); + u32 wl = CCSR_SSI_SxCCR_WL(sample_size); + int enabled = in_be32(&ssi->scr) & CCSR_SSI_SCR_SSIEN; - if (substream == ssi_private->first_stream) { - struct ccsr_ssi __iomem *ssi = ssi_private->ssi; - unsigned int sample_size = - snd_pcm_format_width(params_format(hw_params)); - u32 wl = CCSR_SSI_SxCCR_WL(sample_size); + /* + * If we're in synchronous mode, and the SSI is already enabled, + * then STCCR is already set properly. + */ + if (enabled && ssi_private->cpu_dai_drv.symmetric_rates) + return 0; - /* The SSI should always be disabled at this points (SSIEN=0) */ + /* + * FIXME: The documentation says that SxCCR[WL] should not be + * modified while the SSI is enabled. The only time this can + * happen is if we're trying to do simultaneous playback and + * capture in asynchronous mode. Unfortunately, I have been enable + * to get that to work at all on the P1022DS. Therefore, we don't + * bother to disable/enable the SSI when setting SxCCR[WL], because + * the SSI will stop anyway. Maybe one day, this will get fixed. + */ - /* In synchronous mode, the SSI uses STCCR for capture */ - if ((substream->stream == SNDRV_PCM_STREAM_PLAYBACK) || - !ssi_private->asynchronous) - clrsetbits_be32(&ssi->stccr, - CCSR_SSI_SxCCR_WL_MASK, wl); - else - clrsetbits_be32(&ssi->srccr, - CCSR_SSI_SxCCR_WL_MASK, wl); - } + /* In synchronous mode, the SSI uses STCCR for capture */ + if ((substream->stream == SNDRV_PCM_STREAM_PLAYBACK) || + ssi_private->cpu_dai_drv.symmetric_rates) + clrsetbits_be32(&ssi->stccr, CCSR_SSI_SxCCR_WL_MASK, wl); + else + clrsetbits_be32(&ssi->srccr, CCSR_SSI_SxCCR_WL_MASK, wl); return 0; } @@ -464,7 +464,6 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd, switch (cmd) { case SNDRV_PCM_TRIGGER_START: - clrbits32(&ssi->scr, CCSR_SSI_SCR_SSIEN); case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) setbits32(&ssi->scr, @@ -500,12 +499,6 @@ static void fsl_ssi_shutdown(struct snd_pcm_substream *substream, struct snd_soc_pcm_runtime *rtd = substream->private_data; struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(rtd->cpu_dai); - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - ssi_private->playback--; - - if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) - ssi_private->capture--; - if (ssi_private->first_stream == substream) ssi_private->first_stream = ssi_private->second_stream; @@ -514,7 +507,7 @@ static void fsl_ssi_shutdown(struct snd_pcm_substream *substream, /* * If this is the last active substream, disable the SSI. */ - if (!ssi_private->playback && !ssi_private->capture) { + if (!ssi_private->first_stream) { struct ccsr_ssi __iomem *ssi = ssi_private->ssi; clrbits32(&ssi->scr, CCSR_SSI_SCR_SSIEN); @@ -688,9 +681,7 @@ static int __devinit fsl_ssi_probe(struct platform_device *pdev) } /* Are the RX and the TX clocks locked? */ - if (of_find_property(np, "fsl,ssi-asynchronous", NULL)) - ssi_private->asynchronous = 1; - else + if (!of_find_property(np, "fsl,ssi-asynchronous", NULL)) ssi_private->cpu_dai_drv.symmetric_rates = 1; /* Determine the FIFO depth. */ -- cgit v1.2.3 From d890a1a42dff2e6987f04f18fc9e467b10e99cc9 Mon Sep 17 00:00:00 2001 From: Axel Lin Date: Tue, 20 Sep 2011 15:09:00 +0800 Subject: ASoC: fsl: Fix error handling if platform_device_add fails Call platform_device_put() instead of platform_device_unregister() if platform_device_add() fails. Signed-off-by: Axel Lin Acked-by: Liam Girdwood Signed-off-by: Mark Brown --- sound/soc/fsl/mpc8610_hpcd.c | 2 +- sound/soc/fsl/p1022_ds.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'sound/soc/fsl') diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c index 358f0baaf71b..31af405bda84 100644 --- a/sound/soc/fsl/mpc8610_hpcd.c +++ b/sound/soc/fsl/mpc8610_hpcd.c @@ -505,7 +505,7 @@ static int mpc8610_hpcd_probe(struct platform_device *pdev) return 0; error_sound: - platform_device_unregister(sound_device); + platform_device_put(sound_device); error: kfree(machine_data); error_alloc: diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c index e8849ed36cbd..2c064a9824ad 100644 --- a/sound/soc/fsl/p1022_ds.c +++ b/sound/soc/fsl/p1022_ds.c @@ -506,7 +506,7 @@ static int p1022_ds_probe(struct platform_device *pdev) error: if (sound_device) - platform_device_unregister(sound_device); + platform_device_put(sound_device); kfree(mdata); error_put: -- cgit v1.2.3