Message-ID: <20150928082533.GJ7104@lukather>
Date:	Mon, 28 Sep 2015 10:25:33 +0200
From:	Maxime Ripard <maxime.ripard@...e-electrons.com>
To:	codekipper@...il.com
Cc:	lgirdwood@...il.com, linux-arm-kernel@...ts.infradead.org,
	broonie@...nel.org, linux-sunxi@...glegroups.com,
	linux-kernel@...r.kernel.org, alsa-devel@...a-project.org,
	be17068@...rbole.bo.it
Subject: Re: [linux-sunxi][alsa-devel][PATCH 3/3] ASOC: sunxi: Add support
 for the spdif block

On Thu, Sep 24, 2015 at 04:30:05PM +0200, codekipper@...il.com wrote:
> From: Marcus Cooper <codekipper@...il.com>
> 
> The sun4i, sun6i and sun7i SoC families have an SPDIF
> block which is capable of playback and capture.
> 
> This patch enables the playback of this block for
> the sun4i and sun7i families.
> 
> Signed-off-by: Marcus Cooper <codekipper@...il.com>
> ---
>  sound/soc/sunxi/Kconfig               |  10 +
>  sound/soc/sunxi/Makefile              |   4 +
>  sound/soc/sunxi/sunxi-machine-spdif.c | 110 +++++
>  sound/soc/sunxi/sunxi-spdif.c         | 801 ++++++++++++++++++++++++++++++++++
>  4 files changed, 925 insertions(+)
>  create mode 100644 sound/soc/sunxi/sunxi-machine-spdif.c
>  create mode 100644 sound/soc/sunxi/sunxi-spdif.c
> 
> diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
> index 84c72ec..053db02 100644
> --- a/sound/soc/sunxi/Kconfig
> +++ b/sound/soc/sunxi/Kconfig
> @@ -8,4 +8,14 @@ config SND_SUN4I_CODEC
>  	  Select Y or M to add support for the Codec embedded in the Allwinner
>  	  A10 and affiliated SoCs.
>  
> +config SND_SOC_SUNXI_DAI_SPDIF
> +        tristate
> +        select SND_SOC_GENERIC_DMAENGINE_PCM
> +        select REGMAP_MMIO
> +
> +config SND_SOC_SUNXI_MACHINE_SPDIF
> +        tristate "APB on-chip sun4i/sun5i/sun7i SPDIF"
> +        select SND_SOC_SUNXI_DAI_SPDIF
> +        help
> +          Say Y if you want to add support for SoC S/PDIF audio as simple audio card.
>  endmenu
> diff --git a/sound/soc/sunxi/Makefile b/sound/soc/sunxi/Makefile
> index ea8a08c..7849a75 100644
> --- a/sound/soc/sunxi/Makefile
> +++ b/sound/soc/sunxi/Makefile
> @@ -1,2 +1,6 @@
>  obj-$(CONFIG_SND_SUN4I_CODEC) += sun4i-codec.o
>  
> +snd-soc-sunxi-dai-spdif-objs := sunxi-spdif.o
> +obj-$(CONFIG_SND_SOC_SUNXI_DAI_SPDIF) += snd-soc-sunxi-dai-spdif.o
> +snd-soc-sunxi-machine-spdif-objs := sunxi-machine-spdif.o
> +obj-$(CONFIG_SND_SOC_SUNXI_MACHINE_SPDIF) += snd-soc-sunxi-machine-spdif.o
> diff --git a/sound/soc/sunxi/sunxi-machine-spdif.c b/sound/soc/sunxi/sunxi-machine-spdif.c
> new file mode 100644
> index 0000000..f8f6bd8
> --- /dev/null
> +++ b/sound/soc/sunxi/sunxi-machine-spdif.c
> @@ -0,0 +1,110 @@
> +/*
> + * Copyright (C) 2015 Andrea Venturi <be17068@...rbole.bo.it>
> + * From code by (C) 2013 Freescale Semiconductor, Inc.
> + * (sound/soc/fsl/imx-spdif.c)
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <sound/soc.h>
> +
> +struct sunxi_machine_spdif_data {
> +	struct snd_soc_dai_link dai;
> +	struct snd_soc_card card;
> +};
> +
> +static int sunxi_machine_spdif_audio_probe(struct platform_device *pdev)
> +{
> +	struct device_node *spdif_np, *np = pdev->dev.of_node;
> +	struct sunxi_machine_spdif_data *data;
> +	int ret = 0;
> +
> +	dev_dbg(&pdev->dev, "%s: Looking for spdif-controller\n", __func__);
> +	spdif_np = of_parse_phandle(np, "spdif-controller", 0);
> +	if (!spdif_np) {
> +		dev_err(&pdev->dev, "failed to find spdif-controller\n");
> +		ret = -EINVAL;
> +		goto end;
> +	}
> +
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data) {
> +		ret = -ENOMEM;
> +		goto end;
> +	}
> +
> +	data->dai.name = "S/PDIF PCM";
> +	data->dai.stream_name = "S/PDIF PCM";
> +	data->dai.codec_dai_name = "snd-soc-dummy-dai";
> +	data->dai.codec_name = "snd-soc-dummy";
> +	data->dai.cpu_of_node = spdif_np;
> +	data->dai.platform_of_node = spdif_np;
> +	data->dai.playback_only = true;
> +	data->dai.capture_only = true;
> +
> +	if (of_property_read_bool(np, "spdif-out"))
> +		data->dai.capture_only = false;
> +
> +	if (of_property_read_bool(np, "spdif-in"))
> +		data->dai.playback_only = false;
> +
> +	if (data->dai.playback_only && data->dai.capture_only) {
> +		dev_err(&pdev->dev, "no enabled S/PDIF DAI link\n");
> +		goto end;
> +	}
> +
> +	data->card.dev = &pdev->dev;
> +	data->card.dai_link = &data->dai;
> +	data->card.num_links = 1;
> +	data->card.owner = THIS_MODULE;
> +
> +	ret = snd_soc_of_parse_card_name(&data->card, "model");
> +	if (ret)
> +		goto end;
> +
> +	ret = devm_snd_soc_register_card(&pdev->dev, &data->card);
> +	if (ret) {
> +		dev_err(&pdev->dev, "snd_soc_register_card failed: %d\n", ret);
> +		goto end;
> +	}
> +
> +	platform_set_drvdata(pdev, data);
> +
> +end:
> +	of_node_put(spdif_np);
> +
> +	return ret;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id sunxi_machine_spdif_dt_ids[] = {
> +	{ .compatible = "allwinner,sunxi-audio-spdif", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, sunxi_machine_spdif_dt_ids);
> +#endif
> +
> +static struct platform_driver sunxi_machine_spdif_driver = {
> +	.driver = {
> +		.name = "sunxi-machine-spdif",
> +		.of_match_table = sunxi_machine_spdif_dt_ids,
> +	},
> +	.probe = sunxi_machine_spdif_audio_probe,
> +};
> +
> +module_platform_driver(sunxi_machine_spdif_driver);
> +
> +MODULE_AUTHOR("Marcus Cooper <codekipper@...il.com>");
> +MODULE_AUTHOR("Andrea Venturi, <be17068@...rbole.bo.it>");
> +MODULE_DESCRIPTION("Allwinner sunxi S/PDIF machine driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/sound/soc/sunxi/sunxi-spdif.c b/sound/soc/sunxi/sunxi-spdif.c
> new file mode 100644
> index 0000000..cae9261
> --- /dev/null
> +++ b/sound/soc/sunxi/sunxi-spdif.c
> @@ -0,0 +1,801 @@
> +/*
> + * ALSA SoC SPDIF Audio Layer
> + *
> + * Copyright 2015 Andrea Venturi <be17068@...rbole.bo.it>
> + * Copyright 2015 Marcus Cooper <codekipper@...il.com>
> + *
> + * Based on the Allwinner SDK driver, released under the GPL.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +/*
> + * this is SPDIF sunxi simple audio card DAI driver that uses the codec
> + * "dummy driver" as per sound/soc/fsl/imx-spdif.c
> + */
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/regmap.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/ioport.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <sound/dmaengine_pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +
> +#define	SUNXI_SPDIF_CTL		(0x00)
> +	#define SUNXI_SPDIF_CTL_MCLKDIV(v)		((v) << 4) /* v even */
> +	#define SUNXI_SPDIF_CTL_MCLKOUTEN		BIT(2)
> +	#define SUNXI_SPDIF_CTL_GEN			BIT(1)
> +	#define SUNXI_SPDIF_CTL_RESET			BIT(0)
> +
> +#define SUNXI_SPDIF_TXCFG	(0x04)
> +	#define SUNXI_SPDIF_TXCFG_SINGLEMOD		BIT(31)
> +	#define SUNXI_SPDIF_TXCFG_ASS			BIT(17)
> +	#define SUNXI_SPDIF_TXCFG_NONAUDIO		BIT(16)
> +	#define SUNXI_SPDIF_TXCFG_TXRATIO(v)		((v) << 4)
> +	#define SUNXI_SPDIF_TXCFG_TXRATIO_MASK		GENMASK(8, 4)
> +	#define SUNXI_SPDIF_TXCFG_FMTRVD		GENMASK(3, 2)
> +	#define SUNXI_SPDIF_TXCFG_FMT16BIT		(0 << 2)
> +	#define SUNXI_SPDIF_TXCFG_FMT20BIT		(1 << 2)
> +	#define SUNXI_SPDIF_TXCFG_FMT24BIT		(2 << 2)
> +	#define SUNXI_SPDIF_TXCFG_CHSTMODE		BIT(1)
> +	#define SUNXI_SPDIF_TXCFG_TXEN			BIT(0)
> +
> +#define SUNXI_SPDIF_RXCFG	(0x08)
> +	#define SUNXI_SPDIF_RXCFG_LOCKFLAG		BIT(4)
> +	#define SUNXI_SPDIF_RXCFG_CHSTSRC		BIT(3)
> +	#define SUNXI_SPDIF_RXCFG_CHSTCP		BIT(1)
> +	#define SUNXI_SPDIF_RXCFG_RXEN			BIT(0)
> +
> +#define SUNXI_SPDIF_TXFIFO	(0x0C)
> +
> +#define SUNXI_SPDIF_RXFIFO	(0x10)
> +
> +#define SUNXI_SPDIF_FCTL	(0x14)
> +	#define SUNXI_SPDIF_FCTL_FIFOSRC		BIT(31)
> +	#define SUNXI_SPDIF_FCTL_FTX			BIT(17)
> +	#define SUNXI_SPDIF_FCTL_FRX			BIT(16)
> +	#define SUNXI_SPDIF_FCTL_TXTL(v)		((v) << 8)
> +	#define SUNXI_SPDIF_FCTL_TXTL_MASK		GENMASK(12, 8)
> +	#define SUNXI_SPDIF_FCTL_RXTL(v)		((v) << 3)
> +	#define SUNXI_SPDIF_FCTL_RXTL_MASK		GENMASK(7, 3)
> +	#define SUNXI_SPDIF_FCTL_TXIM			BIT(2)
> +	#define SUNXI_SPDIF_FCTL_RXOM(v)		((v) << 0)
> +	#define SUNXI_SPDIF_FCTL_RXOM_MASK		GENMASK(1, 0)
> +
> +#define SUNXI_SPDIF_FSTA	(0x18)
> +	#define SUNXI_SPDIF_FSTA_TXE			BIT(14)
> +	#define SUNXI_SPDIF_FSTA_TXECNTSHT		(8)
> +	#define SUNXI_SPDIF_FSTA_RXA			BIT(6)
> +	#define SUNXI_SPDIF_FSTA_RXACNTSHT		(0)
> +
> +#define SUNXI_SPDIF_INT		(0x1C)
> +	#define SUNXI_SPDIF_INT_RXLOCKEN		BIT(18)
> +	#define SUNXI_SPDIF_INT_RXUNLOCKEN		BIT(17)
> +	#define SUNXI_SPDIF_INT_RXPARERREN		BIT(16)
> +	#define SUNXI_SPDIF_INT_TXDRQEN			BIT(7)
> +	#define SUNXI_SPDIF_INT_TXUIEN			BIT(6)
> +	#define SUNXI_SPDIF_INT_TXOIEN			BIT(5)
> +	#define SUNXI_SPDIF_INT_TXEIEN			BIT(4)
> +	#define SUNXI_SPDIF_INT_RXDRQEN			BIT(2)
> +	#define SUNXI_SPDIF_INT_RXOIEN			BIT(1)
> +	#define SUNXI_SPDIF_INT_RXAIEN			BIT(0)
> +
> +#define SUNXI_SPDIF_ISTA	(0x20)
> +	#define SUNXI_SPDIF_ISTA_RXLOCKSTA		BIT(18)
> +	#define SUNXI_SPDIF_ISTA_RXUNLOCKSTA		BIT(17)
> +	#define SUNXI_SPDIF_ISTA_RXPARERRSTA		BIT(16)
> +	#define SUNXI_SPDIF_ISTA_TXUSTA			BIT(6)
> +	#define SUNXI_SPDIF_ISTA_TXOSTA			BIT(5)
> +	#define SUNXI_SPDIF_ISTA_TXESTA			BIT(4)
> +	#define SUNXI_SPDIF_ISTA_RXOSTA			BIT(1)
> +	#define SUNXI_SPDIF_ISTA_RXASTA			BIT(0)
> +
> +#define SUNXI_SPDIF_TXCNT	(0x24)
> +
> +#define SUNXI_SPDIF_RXCNT	(0x28)
> +
> +#define SUNXI_SPDIF_TXCHSTA0	(0x2C)
> +	#define SUNXI_SPDIF_TXCHSTA0_CLK(v)		((v) << 28)
> +	#define SUNXI_SPDIF_TXCHSTA0_SAMFREQ(v)		((v) << 24)
> +	#define SUNXI_SPDIF_TXCHSTA0_SAMFREQ_MASK	GENMASK(27, 24)
> +	#define SUNXI_SPDIF_TXCHSTA0_CHNUM(v)		((v) << 20)
> +	#define SUNXI_SPDIF_TXCHSTA0_CHNUM_MASK		GENMASK(23, 20)
> +	#define SUNXI_SPDIF_TXCHSTA0_SRCNUM(v)		((v) << 16)
> +	#define SUNXI_SPDIF_TXCHSTA0_CATACOD(v)		((v) << 8)
> +	#define SUNXI_SPDIF_TXCHSTA0_MODE(v)		((v) << 6)
> +	#define SUNXI_SPDIF_TXCHSTA0_EMPHASIS(v)	((v) << 3)
> +	#define SUNXI_SPDIF_TXCHSTA0_CP			BIT(2)
> +	#define SUNXI_SPDIF_TXCHSTA0_AUDIO		BIT(1)
> +	#define SUNXI_SPDIF_TXCHSTA0_PRO		BIT(0)
> +
> +#define SUNXI_SPDIF_TXCHSTA1	(0x30)
> +	#define SUNXI_SPDIF_TXCHSTA1_CGMSA(v)		((v) << 8)
> +	#define SUNXI_SPDIF_TXCHSTA1_ORISAMFREQ(v)	((v) << 4)
> +	#define SUNXI_SPDIF_TXCHSTA1_ORISAMFREQ_MASK	GENMASK(7, 4)
> +	#define SUNXI_SPDIF_TXCHSTA1_SAMWORDLEN(v)	((v) << 1)
> +	#define SUNXI_SPDIF_TXCHSTA1_MAXWORDLEN		BIT(0)
> +
> +#define SUNXI_SPDIF_RXCHSTA0	(0x34)
> +	#define SUNXI_SPDIF_RXCHSTA0_CLK(v)		((v) << 28)
> +	#define SUNXI_SPDIF_RXCHSTA0_SAMFREQ(v)		((v) << 24)
> +	#define SUNXI_SPDIF_RXCHSTA0_CHNUM(v)		((v) << 20)
> +	#define SUNXI_SPDIF_RXCHSTA0_SRCNUM(v)		((v) << 16)
> +	#define SUNXI_SPDIF_RXCHSTA0_CATACOD(v)		((v) << 8)
> +	#define SUNXI_SPDIF_RXCHSTA0_MODE(v)		((v) << 6)
> +	#define SUNXI_SPDIF_RXCHSTA0_EMPHASIS(v)	((v) << 3)
> +	#define SUNXI_SPDIF_RXCHSTA0_CP			BIT(2)
> +	#define SUNXI_SPDIF_RXCHSTA0_AUDIO		BIT(1)
> +	#define SUNXI_SPDIF_RXCHSTA0_PRO		BIT(0)
> +
> +#define SUNXI_SPDIF_RXCHSTA1	(0x38)
> +	#define SUNXI_SPDIF_RXCHSTA1_CGMSA(v)		((v) << 8)
> +	#define SUNXI_SPDIF_RXCHSTA1_ORISAMFREQ(v)	((v) << 4)
> +	#define SUNXI_SPDIF_RXCHSTA1_SAMWORDLEN(v)	((v) << 1)
> +	#define SUNXI_SPDIF_RXCHSTA1_MAXWORDLEN		BIT(0)
> +
> +/* Defines for Sampling Frequency */
> +enum sunxi_spdif_sampling_freq {
> +	SUNXI_SPDIF_SAMFREQ_44_1KHZ		= 0x0,
> +	SUNXI_SPDIF_SAMFREQ_NOT_INDICATED	= 0x1,
> +	SUNXI_SPDIF_SAMFREQ_48KHZ		= 0x2,
> +	SUNXI_SPDIF_SAMFREQ_32KHZ		= 0x3,
> +	SUNXI_SPDIF_SAMFREQ_22_05KHZ		= 0x4,
> +	SUNXI_SPDIF_SAMFREQ_24KHZ		= 0x6,
> +	SUNXI_SPDIF_SAMFREQ_88_2KHZ		= 0x8,
> +	SUNXI_SPDIF_SAMFREQ_768KHZ		= 0x9,
> +	SUNXI_SPDIF_SAMFREQ_96KHZ		= 0xa,
> +	SUNXI_SPDIF_SAMFREQ_176_4KHZ		= 0xc,
> +	SUNXI_SPDIF_SAMFREQ_192KHZ		= 0xe,
> +};
> +
> +/*
> + * Original sampling frequency can be represented by inverting the value of the
> + * sampling frequency.
> + */
> +#define ORIGINAL(v) ((~v) & 0xf)
> +
> +struct sunxi_spdif_dev {
> +	struct platform_device *pdev;
> +	struct clk *clk;
> +	struct clk *apb_clk;
> +	struct clk *audio_clk;
> +	struct snd_soc_dai_driver cpu_dai_drv;
> +	bool running;
> +	struct regmap *regmap;
> +	struct snd_dmaengine_dai_dma_data dma_params_tx;
> +	struct snd_dmaengine_dai_dma_data dma_params_rx;
> +	bool playback_supported;
> +	bool capture_supported;
> +};
> +
> +static void sunxi_spdif_configure(struct sunxi_spdif_dev *host)
> +{
> +	u32 reg_val;
> +
> +	/* soft reset SPDIF */
> +	regmap_write(host->regmap, SUNXI_SPDIF_CTL, SUNXI_SPDIF_CTL_RESET);
> +
> +	/* MCLK OUTPUT enable */
> +	regmap_update_bits(host->regmap, SUNXI_SPDIF_CTL,
> +			SUNXI_SPDIF_CTL_MCLKOUTEN, SUNXI_SPDIF_CTL_MCLKOUTEN);
> +
> +	/* flush TX FIFO */
> +	regmap_update_bits(host->regmap, SUNXI_SPDIF_FCTL, SUNXI_SPDIF_FCTL_FTX,
> +							SUNXI_SPDIF_FCTL_FTX);

The new line should be aligned on an open parenthesis.

> +
> +	/* clear interrupt status */
> +	regmap_read(host->regmap, SUNXI_SPDIF_ISTA, &reg_val);
> +	regmap_write(host->regmap, SUNXI_SPDIF_ISTA, reg_val);
> +
> +	/* clear TX counter */
> +	regmap_write(host->regmap, SUNXI_SPDIF_TXCNT, 0);
> +
> +}
> +
> +void sunxi_snd_txctrl(struct snd_pcm_substream *substream,
> +					struct sunxi_spdif_dev *host, int on)
> +{
> +	u32 tmp;
> +
> +	regmap_read(host->regmap, SUNXI_SPDIF_TXCNT, &tmp);
> +	if (substream->runtime->channels == 1)
> +		regmap_update_bits(host->regmap, SUNXI_SPDIF_TXCFG,
> +						SUNXI_SPDIF_TXCFG_SINGLEMOD,
> +						SUNXI_SPDIF_TXCFG_SINGLEMOD);
> +
> +	if (on) {
> +		/* SPDIF TX ENABLE */
> +		regmap_update_bits(host->regmap, SUNXI_SPDIF_TXCFG,
> +						SUNXI_SPDIF_TXCFG_TXEN,
> +						SUNXI_SPDIF_TXCFG_TXEN);
> +
> +		/* DRQ ENABLE */
> +		regmap_update_bits(host->regmap, SUNXI_SPDIF_INT,
> +						SUNXI_SPDIF_INT_TXDRQEN,
> +						SUNXI_SPDIF_INT_TXDRQEN);
> +
> +		/* Global enable */
> +		regmap_update_bits(host->regmap, SUNXI_SPDIF_CTL,
> +						SUNXI_SPDIF_CTL_GEN,
> +						SUNXI_SPDIF_CTL_GEN);
> +	} else {
> +		/* SPDIF TX DISABLE */
> +		regmap_update_bits(host->regmap, SUNXI_SPDIF_TXCFG,
> +						SUNXI_SPDIF_TXCFG_TXEN, 0);
> +
> +		/* DRQ DISABLE */
> +		regmap_update_bits(host->regmap, SUNXI_SPDIF_INT,
> +						SUNXI_SPDIF_INT_TXDRQEN, 0);
> +
> +		/* Global disable */
> +		regmap_update_bits(host->regmap, SUNXI_SPDIF_CTL,
> +						SUNXI_SPDIF_CTL_GEN, 0);
> +	}
> +}
> +
> +static int sunxi_spdif_startup(struct snd_pcm_substream *substream,
> +		struct snd_soc_dai *cpu_dai)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct sunxi_spdif_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
> +	int ret;
> +
> +	if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
> +		return -EINVAL;
> +
> +	/* Reset module and interrupts only for first initialization */
> +	if (!cpu_dai->active) {
> +		ret = clk_prepare_enable(host->clk);

Can't you just use runtime_pm to deal with that?

> +		if (ret)
> +			return ret;
> +	}
> +
> +	host->running = true;
> +	sunxi_spdif_configure(host);
> +
> +	return 0;
> +}
> +
> +static void sunxi_spdif_shutdown(struct snd_pcm_substream *substream,
> +		struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct sunxi_spdif_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
> +
> +	if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
> +		return;
> +
> +	clk_disable_unprepare(host->clk);
> +	host->running = false;
> +}
> +
> +static int sunxi_spdif_set_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
> +{
> +	u32 reg_val;
> +	struct sunxi_spdif_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
> +
> +	reg_val = 0;
> +	reg_val &= ~SUNXI_SPDIF_TXCFG_SINGLEMOD;
> +	reg_val |= SUNXI_SPDIF_TXCFG_ASS;
> +	reg_val |= fmt; /* set non audio and bit depth */
> +	reg_val |= SUNXI_SPDIF_TXCFG_CHSTMODE;
> +	regmap_write(host->regmap, SUNXI_SPDIF_TXCFG, reg_val);

You can probably used regmap_update_bits for that.

> +	reg_val = 0;
> +	reg_val &= ~SUNXI_SPDIF_FCTL_FIFOSRC;
> +	reg_val |= SUNXI_SPDIF_FCTL_TXTL_MASK;
> +	reg_val |= SUNXI_SPDIF_FCTL_RXTL_MASK;
> +	reg_val |= SUNXI_SPDIF_FCTL_TXIM;
> +	reg_val |= SUNXI_SPDIF_FCTL_RXOM_MASK;
> +	regmap_write(host->regmap, SUNXI_SPDIF_FCTL, reg_val);
> +	reg_val = 0;
> +	reg_val |= (SUNXI_SPDIF_TXCHSTA0_CHNUM(2));
> +	regmap_write(host->regmap, SUNXI_SPDIF_TXCHSTA0, reg_val);
> +
> +	reg_val = 0;
> +	reg_val |= (SUNXI_SPDIF_TXCHSTA1_SAMWORDLEN(1));
> +	if ((fmt & SUNXI_SPDIF_TXCFG_FMT24BIT) == SUNXI_SPDIF_TXCFG_FMT24BIT)
> +		reg_val |= SUNXI_SPDIF_TXCHSTA1_MAXWORDLEN;
> +	regmap_write(host->regmap, SUNXI_SPDIF_TXCHSTA1, reg_val);
> +	return 0;
> +}
> +
> +static int sunxi_spdif_set_sysclk(struct snd_soc_dai *cpu_dai, unsigned int freq)
> +{
> +	struct sunxi_spdif_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
> +	struct platform_device *pdev = host->pdev;
> +
> +	if (clk_set_rate(host->audio_clk, freq))
> +		dev_err(&pdev->dev,
> +			"Setting pll2 clock rate for %d Hz failed!\n", freq);
> +	if (clk_set_rate(host->clk, freq))
> +		dev_err(&pdev->dev,
> +			"Setting SPDIF clock rate for %d Hz failed!\n", freq);
> +	return 0;
> +}
> +
> +static int sunxi_spdif_set_clkdiv(struct snd_soc_dai *cpu_dai,
> +						unsigned int rate, int div)
> +{
> +	struct sunxi_spdif_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
> +	int sample_freq, original_sample_freq;
> +
> +	regmap_update_bits(host->regmap, SUNXI_SPDIF_TXCHSTA0,
> +				SUNXI_SPDIF_TXCHSTA0_SAMFREQ_MASK, 0);
> +
> +	regmap_update_bits(host->regmap, SUNXI_SPDIF_TXCHSTA1,
> +				SUNXI_SPDIF_TXCHSTA1_ORISAMFREQ_MASK, 0);

Why do you need to clear the registers before writing them again?

> +	regmap_update_bits(host->regmap, SUNXI_SPDIF_TXCFG,
> +				SUNXI_SPDIF_TXCFG_TXRATIO_MASK,
> +				SUNXI_SPDIF_TXCFG_TXRATIO(div - 1));
> +
> +	if (rate == 24576000) {
> +		switch (div) {
> +		/* 24KHZ */
> +		case 8:
> +			sample_freq = SUNXI_SPDIF_SAMFREQ_24KHZ;
> +			original_sample_freq
> +				= ORIGINAL(SUNXI_SPDIF_SAMFREQ_24KHZ);

What is this original sample frequency?

> +			break;
> +
> +		/* 32KHZ */
> +		case 6:
> +			sample_freq = SUNXI_SPDIF_SAMFREQ_32KHZ;
> +			original_sample_freq
> +				= ORIGINAL(SUNXI_SPDIF_SAMFREQ_32KHZ);
> +			break;
> +
> +		/* 48KHZ */
> +		case 4:
> +			sample_freq = SUNXI_SPDIF_SAMFREQ_48KHZ;
> +			original_sample_freq
> +				= ORIGINAL(SUNXI_SPDIF_SAMFREQ_48KHZ);
> +			break;
> +
> +		/* 96KHZ */
> +		case 2:
> +			sample_freq = SUNXI_SPDIF_SAMFREQ_96KHZ;
> +			original_sample_freq
> +				= ORIGINAL(SUNXI_SPDIF_SAMFREQ_96KHZ);
> +			break;
> +
> +		/* 192KHZ */
> +		case 1:
> +			sample_freq = SUNXI_SPDIF_SAMFREQ_192KHZ;
> +			original_sample_freq
> +				= ORIGINAL(SUNXI_SPDIF_SAMFREQ_192KHZ);
> +			break;
> +
> +		default:
> +			sample_freq = SUNXI_SPDIF_SAMFREQ_NOT_INDICATED;
> +			original_sample_freq = 0;
> +			break;
> +		}
> +	} else {
> +		/* 22.5792MHz */
> +		switch (div) {
> +		/* 22.05KHZ */
> +		case 8:
> +			sample_freq = SUNXI_SPDIF_SAMFREQ_22_05KHZ;
> +			original_sample_freq
> +				= ORIGINAL(SUNXI_SPDIF_SAMFREQ_22_05KHZ);
> +			break;
> +
> +		/* 44.1KHZ */
> +		case 4:
> +			sample_freq = SUNXI_SPDIF_SAMFREQ_44_1KHZ;
> +			original_sample_freq
> +				= ORIGINAL(SUNXI_SPDIF_SAMFREQ_44_1KHZ);
> +			break;
> +
> +		/* 88.2KHZ */
> +		case 2:
> +			sample_freq = SUNXI_SPDIF_SAMFREQ_88_2KHZ;
> +			original_sample_freq
> +				= ORIGINAL(SUNXI_SPDIF_SAMFREQ_88_2KHZ);
> +			break;
> +
> +		/* 176.4KHZ */
> +		case 1:
> +			sample_freq = SUNXI_SPDIF_SAMFREQ_176_4KHZ;
> +			original_sample_freq
> +				= ORIGINAL(SUNXI_SPDIF_SAMFREQ_176_4KHZ);
> +			break;
> +

You can probably turn this into a funcion that computes it, instead of
having to duplicated and almost identical switch statements.

> +		default:
> +			sample_freq = SUNXI_SPDIF_SAMFREQ_NOT_INDICATED;
> +			original_sample_freq = 0;
> +			break;
> +		}
> +	}

Shouldn't you return an error in such a case?

> +
> +	regmap_update_bits(host->regmap, SUNXI_SPDIF_TXCHSTA0,
> +			SUNXI_SPDIF_TXCHSTA0_SAMFREQ_MASK,
> +			SUNXI_SPDIF_TXCHSTA0_SAMFREQ(sample_freq));
> +
> +	regmap_update_bits(host->regmap, SUNXI_SPDIF_TXCHSTA1,
> +			SUNXI_SPDIF_TXCHSTA1_ORISAMFREQ_MASK,
> +			SUNXI_SPDIF_TXCHSTA1_ORISAMFREQ(original_sample_freq));
> +
> +	return 0;
> +}
> +
> +static int sunxi_spdif_hw_params(struct snd_pcm_substream *substream,
> +					struct snd_pcm_hw_params *params,
> +					struct snd_soc_dai *cpu_dai)
> +{
> +	int ret = 0;
> +	int fmt;
> +	unsigned long rate = params_rate(params);
> +	u32 mclk_div = 0;
> +	unsigned int mclk = 0;
> +
> +	/* Add the PCM and raw data select interface */
> +	switch (params_channels(params)) {
> +	case 1: /* PCM mode */
> +	case 2:
> +		fmt = 0;
> +		break;
> +	case 4: /* raw data mode */
> +		fmt = SUNXI_SPDIF_TXCFG_NONAUDIO;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	switch (params_format(params)) {
> +	case SNDRV_PCM_FORMAT_S16_LE:
> +		fmt |= SUNXI_SPDIF_TXCFG_FMT16BIT;
> +		break;
> +	case SNDRV_PCM_FORMAT_S20_3LE:
> +		fmt |= SUNXI_SPDIF_TXCFG_FMT20BIT;
> +		break;
> +	case SNDRV_PCM_FORMAT_S24_LE:
> +		fmt |= SUNXI_SPDIF_TXCFG_FMT24BIT;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = snd_soc_dai_set_fmt(cpu_dai, fmt);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (rate) {
> +	case 22050:
> +	case 44100:
> +	case 88200:
> +	case 176400:
> +		mclk = 22579200;
> +		break;
> +	case 24000:
> +	case 32000:
> +	case 48000:
> +	case 96000:
> +	case 192000:
> +		mclk = 24576000;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = sunxi_spdif_set_sysclk(cpu_dai, mclk);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (rate) {
> +	case 22050:
> +	case 24000:
> +		mclk_div = 8;
> +		break;
> +	case 32000:
> +		mclk_div = 6;
> +		break;
> +	case 44100:
> +	case 48000:
> +		mclk_div = 4;
> +		break;
> +	case 88200:
> +	case 96000:
> +		mclk_div = 2;
> +		break;
> +	case 176400:
> +	case 192000:
> +		mclk_div = 1;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = sunxi_spdif_set_clkdiv(cpu_dai, mclk, mclk_div);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int sunxi_spdif_trigger(struct snd_pcm_substream *substream, int cmd,
> +		struct snd_soc_dai *dai)
> +{
> +	int ret = 0;
> +	struct sunxi_spdif_dev *host = snd_soc_dai_get_drvdata(dai);
> +
> +	if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
> +		return -EINVAL;
> +
> +	switch (cmd) {
> +	case SNDRV_PCM_TRIGGER_START:
> +	case SNDRV_PCM_TRIGGER_RESUME:
> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +		sunxi_snd_txctrl(substream, host, 1);
> +		break;
> +
> +	case SNDRV_PCM_TRIGGER_STOP:
> +	case SNDRV_PCM_TRIGGER_SUSPEND:
> +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> +		sunxi_snd_txctrl(substream, host, 0);
> +		break;
> +
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +	return ret;
> +}
> +
> +static int sunxi_spdif_soc_dai_probe(struct snd_soc_dai *dai)
> +{
> +	struct sunxi_spdif_dev *host = snd_soc_dai_get_drvdata(dai);
> +
> +	snd_soc_dai_init_dma_data(dai, &host->dma_params_tx,
> +						&host->dma_params_rx);
> +	return 0;
> +}
> +
> +static const struct snd_soc_dai_ops sunxi_spdif_dai_ops = {
> +	.startup	= sunxi_spdif_startup,
> +	.shutdown	= sunxi_spdif_shutdown,
> +	.trigger	= sunxi_spdif_trigger,
> +	.hw_params	= sunxi_spdif_hw_params,
> +	.set_fmt	= sunxi_spdif_set_fmt,
> +};
> +
> +static const struct regmap_config sunxi_spdif_regmap_config = {
> +	.reg_bits = 32,
> +	.reg_stride = 4,
> +	.val_bits = 32,
> +	.max_register = SUNXI_SPDIF_RXCHSTA1,
> +};
> +
> +#define SUNXI_RATES	(SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | \
> +				SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_96000 | \
> +				SNDRV_PCM_RATE_192000)
> +#define SUNXI_FORMATS	(SNDRV_PCM_FORMAT_S16_LE | SNDRV_PCM_FORMAT_S20_3LE | \
> +				SNDRV_PCM_FORMAT_S24_LE)
> +
> +static struct snd_soc_dai_driver sunxi_spdif_dai = {
> +	.playback = {
> +		.channels_min = 2,
> +		.channels_max = 2,
> +		.rates = SUNXI_RATES,
> +		.formats = SUNXI_FORMATS,
> +	},
> +	.probe = sunxi_spdif_soc_dai_probe,
> +	.ops = &sunxi_spdif_dai_ops,
> +	.name = "spdif",
> +};
> +
> +static const struct snd_soc_dapm_widget dit_widgets[] = {
> +	SND_SOC_DAPM_OUTPUT("spdif-out"),
> +};
> +
> +static const struct snd_soc_dapm_route dit_routes[] = {
> +	{ "spdif-out", NULL, "Playback" },
> +};
> +
> +#ifdef CONFIG_OF

You don't support !dt probing. You should probably only depend on
CONFIG_OF in Kconfig.

> +static const struct of_device_id sunxi_spdif_of_match[] = {
> +	{ .compatible = "allwinner,sun4i-a10-spdif", },

I'd prefer to have the driver named sun4i-spdif (in the driver name,
prefixes, Kconfig options, etc.) so that if there's a new spdif IP
introduced, you don't end up with something weird.

> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, sunxi_spdif_of_match);
> +#endif
> +
> +static const struct snd_soc_component_driver sunxi_spdif_component = {
> +	.name		= "sunxi-spdif",
> +};
> +
> +static int sunxi_spdif_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct sunxi_spdif_dev *host;
> +	struct resource *res;
> +	int ret;
> +	const struct of_device_id *of_id;
> +	void __iomem *base;
> +
> +	dev_dbg(&pdev->dev, "Entered %s\n", __func__);
> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	of_id = of_match_device(sunxi_spdif_of_match, &pdev->dev);
> +	if (!of_id) {
> +		dev_err(&pdev->dev, "No matched devices found.\n");
> +		return -EINVAL;
> +	}

Are you using that of_id anywhere?

> +
> +	host = devm_kzalloc(&pdev->dev, sizeof(*host), GFP_KERNEL);
> +	if (!host)
> +		return -ENOMEM;
> +
> +	host->pdev = pdev;
> +
> +	/* Initialize this copy of the CPU DAI driver structure */
> +	memcpy(&host->cpu_dai_drv, &sunxi_spdif_dai, sizeof(sunxi_spdif_dai));
> +	host->cpu_dai_drv.name = dev_name(&pdev->dev);
> +
> +	/* Get the addresses */
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	host->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> +						&sunxi_spdif_regmap_config);
> +
> +	/* Clocks */
> +	host->apb_clk = devm_clk_get(&pdev->dev, "apb");
> +	if (IS_ERR(host->apb_clk)) {
> +		dev_err(&pdev->dev, "failed to get a apb clock.\n");
> +		return PTR_ERR(host->apb_clk);
> +	}
> +
> +	if (clk_prepare_enable(host->apb_clk)) {
> +		dev_err(&pdev->dev, "try to enable apb_spdif_clk failed\n");
> +		return -EINVAL;
> +	}
> +
> +	host->audio_clk = devm_clk_get(&pdev->dev, "audio");
> +	if (IS_ERR(host->audio_clk)) {
> +		dev_err(&pdev->dev, "failed to get an audio clock.\n");
> +		return PTR_ERR(host->audio_clk);
> +	}
> +
> +	host->clk = devm_clk_get(&pdev->dev, "spdif");
> +	if (IS_ERR(host->clk)) {
> +		dev_err(&pdev->dev, "failed to get a spdif clock.\n");
> +		return PTR_ERR(host->clk);
> +	}
> +
> +	ret = clk_set_rate(host->audio_clk, 24576000);
> +	if (ret) {
> +		dev_err(&pdev->dev, "set spdif base clock failed!\n");
> +		return ret;
> +	}

Why do you need to have to poke at PLL2 directly? the spdif clock is
its direct child. If you have to set a rate, do it directly on this
clock. And you don't have to assign a rate at probe time either since
you will like change it when the playback will start

> +	if (clk_prepare_enable(host->audio_clk)) {
> +		dev_err(&pdev->dev, "try to enable audio clk failed\n");
> +		ret = -EINVAL;
> +		goto exit_clkdisable_apb_clk;
> +	}
> +
> +	host->playback_supported = false;
> +	host->capture_supported = false;
> +
> +	if (of_property_read_bool(np, "spdif-out"))
> +		host->playback_supported = true;
> +
> +	if (!host->playback_supported) {
> +		dev_err(&pdev->dev, "no enabled S/PDIF DAI link\n");
> +		goto exit_clkdisable_clk;
> +	}
> +
> +	host->dma_params_tx.addr = res->start + SUNXI_SPDIF_TXFIFO;
> +	host->dma_params_tx.maxburst = 4;
> +	host->dma_params_tx.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +	host->dma_params_rx.addr = res->start + SUNXI_SPDIF_RXFIFO;
> +	host->dma_params_rx.maxburst = 4;
> +	host->dma_params_rx.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +
> +	/* Register with ASoC */
> +	dev_set_drvdata(&pdev->dev, host);

That's not what it does. And you can use platform_set_drvdata here

> +
> +	ret = devm_snd_soc_register_component(&pdev->dev,
> +				&sunxi_spdif_component, &sunxi_spdif_dai, 1);
> +	if (ret)
> +		goto exit_clkdisable_clk;
> +
> +	ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
> +	if (ret)
> +		goto exit_clkdisable_clk;
> +	return 0;
> +
> +exit_clkdisable_clk:
> +	clk_disable_unprepare(host->clk);
> +exit_clkdisable_apb_clk:
> +	clk_disable_unprepare(host->apb_clk);
> +	return ret;
> +}
> +
> +static int sunxi_spdif_remove(struct platform_device *pdev)
> +{
> +	struct sunxi_spdif_dev *host = dev_get_drvdata(&pdev->dev);
> +
> +	snd_soc_unregister_platform(&pdev->dev);
> +	snd_soc_unregister_component(&pdev->dev);
> +
> +	if (!IS_ERR(host->clk)) {
> +		clk_disable_unprepare(host->clk);
> +		clk_disable_unprepare(host->apb_clk);
> +	}
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int sunxi_spdif_suspend(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct sunxi_spdif_dev *host = dev_get_drvdata(&pdev->dev);
> +
> +	if (host->running)
> +		clk_disable(host->clk);
> +
> +	return 0;
> +}
> +
> +static int sunxi_spdif_resume(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct sunxi_spdif_dev *host = dev_get_drvdata(&pdev->dev);
> +
> +	if (host->running) {
> +		clk_enable(host->clk);
> +		sunxi_spdif_configure(host);
> +	}
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(spdif_dev_pm_ops, sunxi_spdif_suspend,
> +						sunxi_spdif_resume);
> +
> +#define SPDIF_DEV_PM_OPS (&spdif_dev_pm_ops)

How did you test this?

> +#else
> +#define SPDIF_DEV_PM_OPS NULL
> +
> +#endif
> +
> +static struct platform_driver sunxi_spdif_driver = {
> +	.driver		= {
> +		.name	= "sunxi-spdif",
> +		.owner	= THIS_MODULE,

That's not needed anymore.

> +		.pm	= SPDIF_DEV_PM_OPS,
> +		.of_match_table = of_match_ptr(sunxi_spdif_of_match),
> +	},
> +	.probe		= sunxi_spdif_probe,
> +	.remove		= sunxi_spdif_remove,
> +};
> +
> +module_platform_driver(sunxi_spdif_driver);
> +
> +MODULE_AUTHOR("Marcus Cooper <codekipper@...il.com>");
> +MODULE_AUTHOR("Andrea Venturi <be17068@...rbole.bo.it>");
> +MODULE_DESCRIPTION("Allwinner Sunxi SPDIF SoC Interface");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:sunxi-spdif");
> -- 
> 1.9.1

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)

Powered by blists - more mailing lists