[alsa-devel] [PATCH v2 1/2] ASoC: mxs-saif: Fix channel swap for 24-bit format
Playing 24-bit format file leads to channel swap on mx28 and the reason is that the current driver performs one write/read to/from the SAIF_DATA register to trigger the transfer.
This approach works fine for S16_LE case because SAIF_DATA is a 32-bit register and thus is capable of storing the 16-bit left and right channels, but for the S24_LE case it can only store one channel, so in order to not lose the FIFO sync an extra read/write is needed.
Reported-by: Dan Winner DWinner@tc-helicon.com Signed-off-by: Fabio Estevam fabio.estevam@freescale.com Tested-by: Dan Winner DWinner@tc-helicon.com --- Changes since v1: - Clarify that the extra read/write is also safe for non 24-bit formats.
sound/soc/mxs/mxs-saif.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c index aa037b2..0eb2883 100644 --- a/sound/soc/mxs/mxs-saif.c +++ b/sound/soc/mxs/mxs-saif.c @@ -523,16 +523,24 @@ static int mxs_saif_trigger(struct snd_pcm_substream *substream, int cmd,
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { /* - * write a data to saif data register to trigger - * the transfer + * write data to saif data register to trigger + * the transfer. + * For 24-bit format the 32-bit FIFO register stores + * only one channel, so we need to write twice. + * This is also safe for the other non 24-bit formats. */ __raw_writel(0, saif->base + SAIF_DATA); + __raw_writel(0, saif->base + SAIF_DATA); } else { /* - * read a data from saif data register to trigger - * the receive + * read data from saif data register to trigger + * the receive. + * For 24-bit format the 32-bit FIFO register stores + * only one channel, so we need to read twice. + * This is also safe for the other non 24-bit formats. */ __raw_readl(saif->base + SAIF_DATA); + __raw_readl(saif->base + SAIF_DATA); }
master_saif->ongoing = 1;
All MXS users have been converted to device tree and the board files have been removed.
No need to keep platform data in the driver.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com Acked-by: Dong Aisheng dong.aisheng@linaro.org Acked-by: Shawn Guo shawn.guo@linaro.org --- Changes since v3: - Remove pdev->id check Changes since v2: - Also remove 'if (np)' as dt is the only probing mechanism. Changes since v1: - Also remove mxs_saif_platform_data and sound/saif.h, which was used only to include mxs_saif_platform_data definition. include/sound/saif.h | 16 ---------------- sound/soc/mxs/mxs-saif.c | 44 ++++++++++++++++---------------------------- 2 files changed, 16 insertions(+), 44 deletions(-) delete mode 100644 include/sound/saif.h
diff --git a/include/sound/saif.h b/include/sound/saif.h deleted file mode 100644 index f22f3e1..0000000 diff --git a/sound/soc/mxs/mxs-saif.c b/sound/soc/mxs/mxs-saif.c index c294fbb..05dbadb 100644 --- a/sound/soc/mxs/mxs-saif.c +++ b/sound/soc/mxs/mxs-saif.c @@ -32,7 +32,6 @@ #include <sound/pcm.h> #include <sound/pcm_params.h> #include <sound/soc.h> -#include <sound/saif.h> #include <asm/mach-types.h> #include <mach/hardware.h> #include <mach/mxs.h> @@ -660,43 +659,32 @@ static int __devinit mxs_saif_probe(struct platform_device *pdev) struct device_node *np = pdev->dev.of_node; struct resource *iores, *dmares; struct mxs_saif *saif; - struct mxs_saif_platform_data *pdata; struct pinctrl *pinctrl; int ret = 0; + struct device_node *master;
- - if (!np && pdev->id >= ARRAY_SIZE(mxs_saif)) + if (!np) return -EINVAL;
saif = devm_kzalloc(&pdev->dev, sizeof(*saif), GFP_KERNEL); if (!saif) return -ENOMEM;
- if (np) { - struct device_node *master; - saif->id = of_alias_get_id(np, "saif"); - if (saif->id < 0) - return saif->id; - /* - * If there is no "fsl,saif-master" phandle, it's a saif - * master. Otherwise, it's a slave and its phandle points - * to the master. - */ - master = of_parse_phandle(np, "fsl,saif-master", 0); - if (!master) { - saif->master_id = saif->id; - } else { - saif->master_id = of_alias_get_id(master, "saif"); - if (saif->master_id < 0) - return saif->master_id; - } + saif->id = of_alias_get_id(np, "saif"); + if (saif->id < 0) + return saif->id; + /* + * If there is no "fsl,saif-master" phandle, it's a saif + * master. Otherwise, it's a slave and its phandle points + * to the master. + */ + master = of_parse_phandle(np, "fsl,saif-master", 0); + if (!master) { + saif->master_id = saif->id; } else { - saif->id = pdev->id; - pdata = pdev->dev.platform_data; - if (pdata && !pdata->master_mode) - saif->master_id = pdata->master_id; - else - saif->master_id = saif->id; + saif->master_id = of_alias_get_id(master, "saif"); + if (saif->master_id < 0) + return saif->master_id; }
if (saif->master_id < 0 || saif->master_id >= ARRAY_SIZE(mxs_saif)) {
On Thu, Nov 01, 2012 at 03:57:12PM -0200, Fabio Estevam wrote:
All MXS users have been converted to device tree and the board files have been removed.
No need to keep platform data in the driver.
This doesn't apply against v3.7-rc3 (or against my fix/mxs branch) - please check what's going on.
Hi Mark,
On Fri, Nov 2, 2012 at 1:03 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Thu, Nov 01, 2012 at 03:57:12PM -0200, Fabio Estevam wrote:
All MXS users have been converted to device tree and the board files have been removed.
No need to keep platform data in the driver.
This doesn't apply against v3.7-rc3 (or against my fix/mxs branch) - please check what's going on.
fix/mxs branch misses commit 730963f81 (ASoC: mxs-saif: Use devm_clk_get()).
This patch applies cleanly against your for-next and also against linux-next.
Thanks,
Fabio Estevam
On Tue, Nov 13, 2012 at 10:25:06AM -0200, Fabio Estevam wrote:
fix/mxs branch misses commit 730963f81 (ASoC: mxs-saif: Use devm_clk_get()).
This patch applies cleanly against your for-next and also against linux-next.
That commit appears to have been added somewhere prior to v3.6 so should be in mainline now in which case it's surprising that it's not in a branch based off v3.7-rc3, and indeed when I look at both the fix/mxs branch and mainline I can see the commit. This suggests that it isn't the problem.
On 2 November 2012 01:57, Fabio Estevam fabio.estevam@freescale.com wrote:
Playing 24-bit format file leads to channel swap on mx28 and the reason is that the current driver performs one write/read to/from the SAIF_DATA register to trigger the transfer.
This approach works fine for S16_LE case because SAIF_DATA is a 32-bit register and thus is capable of storing the 16-bit left and right channels, but for the S24_LE case it can only store one channel, so in order to not lose the FIFO sync an extra read/write is needed.
Reported-by: Dan Winner DWinner@tc-helicon.com Signed-off-by: Fabio Estevam fabio.estevam@freescale.com Tested-by: Dan Winner DWinner@tc-helicon.com
Acked-by: Dong Aisheng dong.aisheng@linaro.org
Regards Dong Aisheng
On Thu, Nov 01, 2012 at 03:57:11PM -0200, Fabio Estevam wrote:
Playing 24-bit format file leads to channel swap on mx28 and the reason is that the current driver performs one write/read to/from the SAIF_DATA register to trigger the transfer.
Applied, thanks.
participants (4)
-
Dong Aisheng -
Fabio Estevam -
Fabio Estevam -
Mark Brown