[PATCH v2 0/4] Add SPDIF support for Allwinner SoCs

[PATCH v2 0/4] Add SPDIF support for Allwinner SoCs

[codekipper at gmail.com]

From: Marcus Cooper <codekipper@gmail.com>

This patch set adds support for the Allwinner SPDIF transceiver as present
on the A10, A20 and A31 SoC boards.

For now just the SPDIF transmitter has been tested on a Mele A2000.

In order for this patch set to be functional we require audio clock patches
which will be delivered separately. For those that are interested I've pushed
the patches here with all the required changes to get SPDIF audio out of the
device.

https://github.com/codekipper/linux-sunxi/commits/spdif_delivery

This is version 2 of the patch series. After Mark's initial comments I realised
that I shouldn't of blindly copied the register set-up from the original SDK
that was released. It doesn't help also that the datasheet that finally emerged
lacked a serious amount of detail. If anybody does recognise the IP used for the
block then please speak out.

In short, much of the code that was in the first release is no longer here. 

I'm still unsure about the machine driver setup. Simple card seems a bit overkill
and what I've added is pretty much a copy of what is done in imx-spdif. I'm not
planning on arguing the case for my method so if you can advise then I'm all
ears.

Thanks in advance,
CK

---
Changes since v1:
- Moved sunxi-machine-spdif.c to seperate patch
- replaced sunxi in naming scheme with sun4i in the sun4i-spdif driver.
- split tx controller into seperate enable/disable functions
- moved setclk and setfmt functionality into hw params
- added support for mono signals.
- cleaned up probe clock set up.
- removed all writes to transmit status registers.
- removed of_id
- removed power management code.
- Added support for more rates.
---
Marcus Cooper (4):
  dt-bindings: add sunxi SPDIF transceiver bindings
  dt-binding: Add sunxi S/PDIF machine driver
  ASoC: sunxi: Add S/PDIF machine driver.
  ASOC: sunxi: Add support for the spdif block

 .../devicetree/bindings/sound/sunxi,spdif.txt      |  49 ++
 .../bindings/sound/sunxi-audio-spdif.txt           |  36 ++
 sound/soc/sunxi/Kconfig                            |  12 +
 sound/soc/sunxi/Makefile                           |   4 +
 sound/soc/sunxi/sun4i-spdif.c                      | 612 +++++++++++++++++++++
 sound/soc/sunxi/sunxi-machine-spdif.c              | 108 ++++
 6 files changed, 821 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/sunxi,spdif.txt
 create mode 100644 Documentation/devicetree/bindings/sound/sunxi-audio-spdif.txt
 create mode 100644 sound/soc/sunxi/sun4i-spdif.c
 create mode 100644 sound/soc/sunxi/sunxi-machine-spdif.c

-- 
1.9.1

[PATCH v2 1/4] dt-bindings: add sunxi SPDIF transceiver bindings

[codekipper at gmail.com]

From: Marcus Cooper <codekipper@gmail.com>

Add devicetree bindings for the SPDIF transceiver found on
found on Allwinners A10, A20 and A31 SoCs.

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
---
 .../devicetree/bindings/sound/sunxi,spdif.txt      | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/sunxi,spdif.txt

diff --git a/Documentation/devicetree/bindings/sound/sunxi,spdif.txt b/Documentation/devicetree/bindings/sound/sunxi,spdif.txt
new file mode 100644
index 0000000..1868722
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/sunxi,spdif.txt
@@ -0,0 +1,49 @@
+Allwinner Sony/Philips Digital Interface Format (S/PDIF) Controller
+
+The Allwinner S/PDIF audio block is a transceiver that allows the
+processor to receive and transmit digital audio via an coaxial cable or
+a fibre cable.
+
+Required properties:
+
+  - compatible		: should be one of the following:
+    - "allwinner,sun4i-a10-spdif": for the Allwinner A10 SoC
+    - "allwinner,sun7i-a20-spdif": for the Allwinner A20 SoC
+    - "allwinner,sun6i-a31-spdif": for the Allwinner A31 SoC
+
+  - reg			: Offset and length of the register set for the device.
+
+  - interrupts		: Contains the spdif interrupt.
+
+  - dmas		: Generic dma devicetree binding as described in
+			  Documentation/devicetree/bindings/dma/dma.txt.
+
+  - dma-names		: Two dmas have to be defined, "tx" and "rx".
+
+  - clocks		: Contains an entry for each entry in clock-names.
+
+  - clock-names		: Includes the following entries:
+	"apb"		  clock for the spdif bus.
+	"audio"		  clock from the audio pll.
+	"spdif"		  clock for spdif controller.
+
+Optional:
+
+  - spdif-in		: Enable block for capturing an SPDIF signal.
+
+  - spdif-out		: Enable block for transmitting an SPDIF signal.
+
+Example:
+
+spdif: spdif at 01c21000 {
+	compatible = "allwinner,sun4i-a10-spdif";
+	reg = <0x01c21000 0x40>;
+	interrupts = <13>;
+	clocks = <&apb0_gates 1>, <&pll2 0>, <&spdif_clk>;
+	clock-names = "apb", "audio", "spdif";
+	dmas = <&dma 0 2>, <&dma 0 2>;
+	dma-names = "rx", "tx";
+	spdif-in = "disabled";
+	spdif-out = "okay";
+	status = "okay";
+};
-- 
1.9.1

[PATCH v2 1/4] dt-bindings: add sunxi SPDIF transceiver bindings

[Maxime Ripard]

On Wed, Sep 30, 2015 at 07:50:55PM +0200, codekipper at gmail.com wrote:
> From: Marcus Cooper <codekipper@gmail.com>
> 
> Add devicetree bindings for the SPDIF transceiver found on
> found on Allwinners A10, A20 and A31 SoCs.
> 
> Signed-off-by: Marcus Cooper <codekipper@gmail.com>
> ---
>  .../devicetree/bindings/sound/sunxi,spdif.txt      | 49 ++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/sunxi,spdif.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/sunxi,spdif.txt b/Documentation/devicetree/bindings/sound/sunxi,spdif.txt
> new file mode 100644
> index 0000000..1868722
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/sunxi,spdif.txt
> @@ -0,0 +1,49 @@
> +Allwinner Sony/Philips Digital Interface Format (S/PDIF) Controller
> +
> +The Allwinner S/PDIF audio block is a transceiver that allows the
> +processor to receive and transmit digital audio via an coaxial cable or
> +a fibre cable.
> +
> +Required properties:
> +
> +  - compatible		: should be one of the following:
> +    - "allwinner,sun4i-a10-spdif": for the Allwinner A10 SoC
> +    - "allwinner,sun7i-a20-spdif": for the Allwinner A20 SoC
> +    - "allwinner,sun6i-a31-spdif": for the Allwinner A31 SoC

Are all these compatibles really work? Is there any significant
difference between the controller on all these SoCs?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151001/30054a00/attachment.sig>

[PATCH v2 1/4] dt-bindings: add sunxi SPDIF transceiver bindings

[Code Kipper]

>> +
>> +  - compatible               : should be one of the following:
>> +    - "allwinner,sun4i-a10-spdif": for the Allwinner A10 SoC
>> +    - "allwinner,sun7i-a20-spdif": for the Allwinner A20 SoC
>> +    - "allwinner,sun6i-a31-spdif": for the Allwinner A31 SoC
>
> Are all these compatibles really work? Is there any significant
> difference between the controller on all these SoCs?
Let us assume that there isn't any difference. Remember SPDIF details
for all of these devices is sketchy. In the A10 User Manual, it's not
even mentioned although devices such as the Mele A2000 which I use
come with the physical connector. It's only when the A20 Manual was
released that we see the pin details and related components. We didn't
see a SPDIF block spec until the H3 User Manual was released.

Looking at the SDK code I've only seen fifo level settings to be
different for the sun6i family. It was this release that also showed
Rx rotines. The fact of the matter is we won't know until these SoCs
have been tested and with that in mind I'm happy to remove all
capabilities for now until then.

BR,
CK

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

[PATCH v2 1/4] dt-bindings: add sunxi SPDIF transceiver bindings

[Maxime Ripard]

On Fri, Oct 02, 2015 at 07:24:20AM +0200, Code Kipper wrote:
> >> +
> >> +  - compatible               : should be one of the following:
> >> +    - "allwinner,sun4i-a10-spdif": for the Allwinner A10 SoC
> >> +    - "allwinner,sun7i-a20-spdif": for the Allwinner A20 SoC
> >> +    - "allwinner,sun6i-a31-spdif": for the Allwinner A31 SoC
> >
> > Are all these compatibles really work? Is there any significant
> > difference between the controller on all these SoCs?
>
> Let us assume that there isn't any difference. Remember SPDIF details
> for all of these devices is sketchy. In the A10 User Manual, it's not
> even mentioned although devices such as the Mele A2000 which I use
> come with the physical connector. It's only when the A20 Manual was
> released that we see the pin details and related components. We didn't
> see a SPDIF block spec until the H3 User Manual was released.
> 
> Looking at the SDK code I've only seen fifo level settings to be
> different for the sun6i family. It was this release that also showed
> Rx rotines. The fact of the matter is we won't know until these SoCs
> have been tested and with that in mind I'm happy to remove all
> capabilities for now until then.

The point was more that you document compatibles that you are not
actually supporting.

You've only tested it on one SoC (and it actually works only on one
SoC, or at least with one compatible), so only document that.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151005/fb5f71b9/attachment.sig>

[PATCH v2 1/4] dt-bindings: add sunxi SPDIF transceiver bindings

[Code Kipper]

On 5 October 2015 at 10:41, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Fri, Oct 02, 2015 at 07:24:20AM +0200, Code Kipper wrote:
>> >> +
>> >> +  - compatible               : should be one of the following:
>> >> +    - "allwinner,sun4i-a10-spdif": for the Allwinner A10 SoC
>> >> +    - "allwinner,sun7i-a20-spdif": for the Allwinner A20 SoC
>> >> +    - "allwinner,sun6i-a31-spdif": for the Allwinner A31 SoC
>> >
>> > Are all these compatibles really work? Is there any significant
>> > difference between the controller on all these SoCs?
>>
>> Let us assume that there isn't any difference. Remember SPDIF details
>> for all of these devices is sketchy. In the A10 User Manual, it's not
>> even mentioned although devices such as the Mele A2000 which I use
>> come with the physical connector. It's only when the A20 Manual was
>> released that we see the pin details and related components. We didn't
>> see a SPDIF block spec until the H3 User Manual was released.
>>
>> Looking at the SDK code I've only seen fifo level settings to be
>> different for the sun6i family. It was this release that also showed
>> Rx rotines. The fact of the matter is we won't know until these SoCs
>> have been tested and with that in mind I'm happy to remove all
>> capabilities for now until then.
>
> The point was more that you document compatibles that you are not
> actually supporting.
>
> You've only tested it on one SoC (and it actually works only on one
> SoC, or at least with one compatible), so only document that.

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

[PATCH v2 2/4] dt-binding: Add sunxi S/PDIF machine driver

[codekipper at gmail.com]

From: Marcus Cooper <codekipper@gmail.com>

Add device tree bindings for the SPDIF machine driver for Allwinner SoC
devices.

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
---
 .../bindings/sound/sunxi-audio-spdif.txt           | 36 ++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/sunxi-audio-spdif.txt

diff --git a/Documentation/devicetree/bindings/sound/sunxi-audio-spdif.txt b/Documentation/devicetree/bindings/sound/sunxi-audio-spdif.txt
new file mode 100644
index 0000000..b9e8152
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/sunxi-audio-spdif.txt
@@ -0,0 +1,36 @@
+Allwinner audio complex with S/PDIF transceiver
+
+Required properties:
+
+  - compatible		: "Allwinner,sunxi-audio-spdif"
+
+  - model		: The user-visible name of this sound complex
+
+  - spdif-controller	: The phandle of the Allwinner S/PDIF controller
+
+
+Optional properties:
+
+  - spdif-out		: This is a boolean property. If present, the
+			  transmitting function of S/PDIF will be enabled,
+			  indicating there's a physical S/PDIF out connector
+			  or jack on the board or it's connecting to some
+			  other IP block, such as an HDMI encoder or
+			  display-controller.
+
+  - spdif-in		: This is a boolean property. If present, the receiving
+			  function of S/PDIF will be enabled, indicating there
+			  is a physical S/PDIF in connector/jack on the board.
+
+* Note: At least one of these two properties should be set in the DT binding.
+
+
+Example:
+
+sound-spdif {
+	compatible = "allwinner,sunxi-audio-spdif";
+	model = "sunxi-spdif";
+	spdif-controller = <&spdif>;
+	spdif-out;
+	spdif-in;
+};
-- 
1.9.1

[PATCH v2 3/4] ASoC: sunxi: Add S/PDIF machine driver.

[codekipper at gmail.com]

From: Marcus Cooper <codekipper@gmail.com>

Signed-off-by: Marcus Cooper <codekipper@gmail.com>
---
 sound/soc/sunxi/sunxi-machine-spdif.c | 108 ++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)
 create mode 100644 sound/soc/sunxi/sunxi-machine-spdif.c

diff --git a/sound/soc/sunxi/sunxi-machine-spdif.c b/sound/soc/sunxi/sunxi-machine-spdif.c
new file mode 100644
index 0000000..2adb727
--- /dev/null
+++ b/sound/soc/sunxi/sunxi-machine-spdif.c
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2015 Andrea Venturi <be17068@iperbole.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;
+}
+
+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);
+
+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@gmail.com>");
+MODULE_AUTHOR("Andrea Venturi, <be17068@iperbole.bo.it>");
+MODULE_DESCRIPTION("Allwinner sunxi S/PDIF machine driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH v2 4/4] ASOC: sunxi: Add support for the spdif block

[codekipper at gmail.com]

From: Marcus Cooper <codekipper@gmail.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@gmail.com>
---
 sound/soc/sunxi/Kconfig       |  12 +
 sound/soc/sunxi/Makefile      |   4 +
 sound/soc/sunxi/sun4i-spdif.c | 612 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 628 insertions(+)
 create mode 100644 sound/soc/sunxi/sun4i-spdif.c

diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
index 84c72ec..2ebf43d 100644
--- a/sound/soc/sunxi/Kconfig
+++ b/sound/soc/sunxi/Kconfig
@@ -8,4 +8,16 @@ 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
+	depends on OF
+        select SND_SOC_GENERIC_DMAENGINE_PCM
+        select REGMAP_MMIO
+
+config SND_SOC_SUNXI_MACHINE_SPDIF
+        tristate "APB on-chip sun4i/sun5i/sun7i SPDIF"
+	depends on OF
+        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..c8c0a00 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 := sun4i-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/sun4i-spdif.c b/sound/soc/sunxi/sun4i-spdif.c
new file mode 100644
index 0000000..5fff6f6
--- /dev/null
+++ b/sound/soc/sunxi/sun4i-spdif.c
@@ -0,0 +1,612 @@
+/*
+ * ALSA SoC SPDIF Audio Layer
+ *
+ * Copyright 2015 Andrea Venturi <be17068@iperbole.bo.it>
+ * Copyright 2015 Marcus Cooper <codekipper@gmail.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 sun4i 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	SUN4I_SPDIF_CTL		(0x00)
+	#define SUN4I_SPDIF_CTL_MCLKDIV(v)		((v) << 4) /* v even */
+	#define SUN4I_SPDIF_CTL_MCLKOUTEN		BIT(2)
+	#define SUN4I_SPDIF_CTL_GEN			BIT(1)
+	#define SUN4I_SPDIF_CTL_RESET			BIT(0)
+
+#define SUN4I_SPDIF_TXCFG	(0x04)
+	#define SUN4I_SPDIF_TXCFG_SINGLEMOD		BIT(31)
+	#define SUN4I_SPDIF_TXCFG_ASS			BIT(17)
+	#define SUN4I_SPDIF_TXCFG_NONAUDIO		BIT(16)
+	#define SUN4I_SPDIF_TXCFG_TXRATIO(v)		((v) << 4)
+	#define SUN4I_SPDIF_TXCFG_TXRATIO_MASK		GENMASK(8, 4)
+	#define SUN4I_SPDIF_TXCFG_FMTRVD		GENMASK(3, 2)
+	#define SUN4I_SPDIF_TXCFG_FMT16BIT		(0 << 2)
+	#define SUN4I_SPDIF_TXCFG_FMT20BIT		(1 << 2)
+	#define SUN4I_SPDIF_TXCFG_FMT24BIT		(2 << 2)
+	#define SUN4I_SPDIF_TXCFG_CHSTMODE		BIT(1)
+	#define SUN4I_SPDIF_TXCFG_TXEN			BIT(0)
+
+#define SUN4I_SPDIF_RXCFG	(0x08)
+	#define SUN4I_SPDIF_RXCFG_LOCKFLAG		BIT(4)
+	#define SUN4I_SPDIF_RXCFG_CHSTSRC		BIT(3)
+	#define SUN4I_SPDIF_RXCFG_CHSTCP		BIT(1)
+	#define SUN4I_SPDIF_RXCFG_RXEN			BIT(0)
+
+#define SUN4I_SPDIF_TXFIFO	(0x0C)
+
+#define SUN4I_SPDIF_RXFIFO	(0x10)
+
+#define SUN4I_SPDIF_FCTL	(0x14)
+	#define SUN4I_SPDIF_FCTL_FIFOSRC		BIT(31)
+	#define SUN4I_SPDIF_FCTL_FTX			BIT(17)
+	#define SUN4I_SPDIF_FCTL_FRX			BIT(16)
+	#define SUN4I_SPDIF_FCTL_TXTL(v)		((v) << 8)
+	#define SUN4I_SPDIF_FCTL_TXTL_MASK		GENMASK(12, 8)
+	#define SUN4I_SPDIF_FCTL_RXTL(v)		((v) << 3)
+	#define SUN4I_SPDIF_FCTL_RXTL_MASK		GENMASK(7, 3)
+	#define SUN4I_SPDIF_FCTL_TXIM			BIT(2)
+	#define SUN4I_SPDIF_FCTL_RXOM(v)		((v) << 0)
+	#define SUN4I_SPDIF_FCTL_RXOM_MASK		GENMASK(1, 0)
+
+#define SUN4I_SPDIF_FSTA	(0x18)
+	#define SUN4I_SPDIF_FSTA_TXE			BIT(14)
+	#define SUN4I_SPDIF_FSTA_TXECNTSHT		(8)
+	#define SUN4I_SPDIF_FSTA_RXA			BIT(6)
+	#define SUN4I_SPDIF_FSTA_RXACNTSHT		(0)
+
+#define SUN4I_SPDIF_INT		(0x1C)
+	#define SUN4I_SPDIF_INT_RXLOCKEN		BIT(18)
+	#define SUN4I_SPDIF_INT_RXUNLOCKEN		BIT(17)
+	#define SUN4I_SPDIF_INT_RXPARERREN		BIT(16)
+	#define SUN4I_SPDIF_INT_TXDRQEN			BIT(7)
+	#define SUN4I_SPDIF_INT_TXUIEN			BIT(6)
+	#define SUN4I_SPDIF_INT_TXOIEN			BIT(5)
+	#define SUN4I_SPDIF_INT_TXEIEN			BIT(4)
+	#define SUN4I_SPDIF_INT_RXDRQEN			BIT(2)
+	#define SUN4I_SPDIF_INT_RXOIEN			BIT(1)
+	#define SUN4I_SPDIF_INT_RXAIEN			BIT(0)
+
+#define SUN4I_SPDIF_ISTA	(0x20)
+	#define SUN4I_SPDIF_ISTA_RXLOCKSTA		BIT(18)
+	#define SUN4I_SPDIF_ISTA_RXUNLOCKSTA		BIT(17)
+	#define SUN4I_SPDIF_ISTA_RXPARERRSTA		BIT(16)
+	#define SUN4I_SPDIF_ISTA_TXUSTA			BIT(6)
+	#define SUN4I_SPDIF_ISTA_TXOSTA			BIT(5)
+	#define SUN4I_SPDIF_ISTA_TXESTA			BIT(4)
+	#define SUN4I_SPDIF_ISTA_RXOSTA			BIT(1)
+	#define SUN4I_SPDIF_ISTA_RXASTA			BIT(0)
+
+#define SUN4I_SPDIF_TXCNT	(0x24)
+
+#define SUN4I_SPDIF_RXCNT	(0x28)
+
+#define SUN4I_SPDIF_TXCHSTA0	(0x2C)
+	#define SUN4I_SPDIF_TXCHSTA0_CLK(v)		((v) << 28)
+	#define SUN4I_SPDIF_TXCHSTA0_SAMFREQ(v)		((v) << 24)
+	#define SUN4I_SPDIF_TXCHSTA0_SAMFREQ_MASK	GENMASK(27, 24)
+	#define SUN4I_SPDIF_TXCHSTA0_CHNUM(v)		((v) << 20)
+	#define SUN4I_SPDIF_TXCHSTA0_CHNUM_MASK		GENMASK(23, 20)
+	#define SUN4I_SPDIF_TXCHSTA0_SRCNUM(v)		((v) << 16)
+	#define SUN4I_SPDIF_TXCHSTA0_CATACOD(v)		((v) << 8)
+	#define SUN4I_SPDIF_TXCHSTA0_MODE(v)		((v) << 6)
+	#define SUN4I_SPDIF_TXCHSTA0_EMPHASIS(v)	((v) << 3)
+	#define SUN4I_SPDIF_TXCHSTA0_CP			BIT(2)
+	#define SUN4I_SPDIF_TXCHSTA0_AUDIO		BIT(1)
+	#define SUN4I_SPDIF_TXCHSTA0_PRO		BIT(0)
+
+#define SUN4I_SPDIF_TXCHSTA1	(0x30)
+	#define SUN4I_SPDIF_TXCHSTA1_CGMSA(v)		((v) << 8)
+	#define SUN4I_SPDIF_TXCHSTA1_ORISAMFREQ(v)	((v) << 4)
+	#define SUN4I_SPDIF_TXCHSTA1_ORISAMFREQ_MASK	GENMASK(7, 4)
+	#define SUN4I_SPDIF_TXCHSTA1_SAMWORDLEN(v)	((v) << 1)
+	#define SUN4I_SPDIF_TXCHSTA1_MAXWORDLEN		BIT(0)
+
+#define SUN4I_SPDIF_RXCHSTA0	(0x34)
+	#define SUN4I_SPDIF_RXCHSTA0_CLK(v)		((v) << 28)
+	#define SUN4I_SPDIF_RXCHSTA0_SAMFREQ(v)		((v) << 24)
+	#define SUN4I_SPDIF_RXCHSTA0_CHNUM(v)		((v) << 20)
+	#define SUN4I_SPDIF_RXCHSTA0_SRCNUM(v)		((v) << 16)
+	#define SUN4I_SPDIF_RXCHSTA0_CATACOD(v)		((v) << 8)
+	#define SUN4I_SPDIF_RXCHSTA0_MODE(v)		((v) << 6)
+	#define SUN4I_SPDIF_RXCHSTA0_EMPHASIS(v)	((v) << 3)
+	#define SUN4I_SPDIF_RXCHSTA0_CP			BIT(2)
+	#define SUN4I_SPDIF_RXCHSTA0_AUDIO		BIT(1)
+	#define SUN4I_SPDIF_RXCHSTA0_PRO		BIT(0)
+
+#define SUN4I_SPDIF_RXCHSTA1	(0x38)
+	#define SUN4I_SPDIF_RXCHSTA1_CGMSA(v)		((v) << 8)
+	#define SUN4I_SPDIF_RXCHSTA1_ORISAMFREQ(v)	((v) << 4)
+	#define SUN4I_SPDIF_RXCHSTA1_SAMWORDLEN(v)	((v) << 1)
+	#define SUN4I_SPDIF_RXCHSTA1_MAXWORDLEN		BIT(0)
+
+/* Defines for Sampling Frequency */
+#define SUN4I_SPDIF_SAMFREQ_44_1KHZ		0x0
+#define SUN4I_SPDIF_SAMFREQ_NOT_INDICATED	0x1
+#define SUN4I_SPDIF_SAMFREQ_48KHZ		0x2
+#define SUN4I_SPDIF_SAMFREQ_32KHZ		0x3
+#define SUN4I_SPDIF_SAMFREQ_22_05KHZ		0x4
+#define SUN4I_SPDIF_SAMFREQ_24KHZ		0x6
+#define SUN4I_SPDIF_SAMFREQ_88_2KHZ		0x8
+#define SUN4I_SPDIF_SAMFREQ_76_8KHZ		0x9
+#define SUN4I_SPDIF_SAMFREQ_96KHZ		0xa
+#define SUN4I_SPDIF_SAMFREQ_176_4KHZ		0xc
+#define SUN4I_SPDIF_SAMFREQ_192KHZ		0xe
+
+/*
+ * Original sampling frequency can be represented by inverting the value of the
+ * sampling frequency.
+ */
+#define ORIGINAL(v) ((~v) & 0xf)
+
+struct sun4i_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;
+	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 sun4i_spdif_configure(struct sun4i_spdif_dev *host)
+{
+	u32 reg_val;
+
+	/* soft reset SPDIF */
+	regmap_write(host->regmap, SUN4I_SPDIF_CTL, SUN4I_SPDIF_CTL_RESET);
+
+	/* MCLK OUTPUT enable */
+	regmap_update_bits(host->regmap, SUN4I_SPDIF_CTL,
+			SUN4I_SPDIF_CTL_MCLKOUTEN, SUN4I_SPDIF_CTL_MCLKOUTEN);
+
+	/* flush TX FIFO */
+	regmap_update_bits(host->regmap, SUN4I_SPDIF_FCTL,
+			SUN4I_SPDIF_FCTL_FTX, SUN4I_SPDIF_FCTL_FTX);
+
+	/* clear interrupt status */
+	regmap_read(host->regmap, SUN4I_SPDIF_ISTA, &reg_val);
+	regmap_write(host->regmap, SUN4I_SPDIF_ISTA, reg_val);
+
+	/* clear TX counter */
+	regmap_write(host->regmap, SUN4I_SPDIF_TXCNT, 0);
+
+}
+
+void sun4i_snd_txctrl_on(struct snd_pcm_substream *substream,
+				struct sun4i_spdif_dev *host)
+{
+	if (substream->runtime->channels == 1)
+		regmap_update_bits(host->regmap, SUN4I_SPDIF_TXCFG,
+						SUN4I_SPDIF_TXCFG_SINGLEMOD,
+						SUN4I_SPDIF_TXCFG_SINGLEMOD);
+
+	/* SPDIF TX ENABLE */
+	regmap_update_bits(host->regmap, SUN4I_SPDIF_TXCFG,
+					SUN4I_SPDIF_TXCFG_TXEN,
+					SUN4I_SPDIF_TXCFG_TXEN);
+
+	/* DRQ ENABLE */
+	regmap_update_bits(host->regmap, SUN4I_SPDIF_INT,
+					SUN4I_SPDIF_INT_TXDRQEN,
+					SUN4I_SPDIF_INT_TXDRQEN);
+
+	/* Global enable */
+	regmap_update_bits(host->regmap, SUN4I_SPDIF_CTL,
+					SUN4I_SPDIF_CTL_GEN,
+					SUN4I_SPDIF_CTL_GEN);
+}
+
+void sun4i_snd_txctrl_off(struct snd_pcm_substream *substream,
+				struct sun4i_spdif_dev *host)
+{
+	/* SPDIF TX DISABLE */
+	regmap_update_bits(host->regmap, SUN4I_SPDIF_TXCFG,
+					SUN4I_SPDIF_TXCFG_TXEN, 0);
+
+	/* DRQ DISABLE */
+	regmap_update_bits(host->regmap, SUN4I_SPDIF_INT,
+					SUN4I_SPDIF_INT_TXDRQEN, 0);
+
+	/* Global disable */
+	regmap_update_bits(host->regmap, SUN4I_SPDIF_CTL,
+					SUN4I_SPDIF_CTL_GEN, 0);
+}
+
+static int sun4i_spdif_startup(struct snd_pcm_substream *substream,
+				struct snd_soc_dai *cpu_dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct sun4i_spdif_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
+
+	if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
+		return -EINVAL;
+
+	sun4i_spdif_configure(host);
+
+	return clk_prepare_enable(host->clk);
+}
+
+static void sun4i_spdif_shutdown(struct snd_pcm_substream *substream,
+				struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct sun4i_spdif_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
+
+	if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
+		return;
+
+	clk_disable_unprepare(host->clk);
+}
+
+static int sun4i_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;
+	u32 reg_val;
+	struct sun4i_spdif_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
+	struct platform_device *pdev = host->pdev;
+
+	/* 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 = SUN4I_SPDIF_TXCFG_NONAUDIO;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+		fmt |= SUN4I_SPDIF_TXCFG_FMT16BIT;
+		break;
+	case SNDRV_PCM_FORMAT_S20_3LE:
+		fmt |= SUN4I_SPDIF_TXCFG_FMT20BIT;
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		fmt |= SUN4I_SPDIF_TXCFG_FMT24BIT;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	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 = clk_set_rate(host->audio_clk, mclk);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"Setting pll2 clock rate for %d Hz failed!\n", mclk);
+		return ret;
+	}
+
+	ret = clk_set_rate(host->clk, mclk);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"Setting SPDIF clock rate for %d Hz failed!\n", mclk);
+		return ret;
+	}
+
+	reg_val = 0;
+	reg_val &= ~SUN4I_SPDIF_FCTL_FIFOSRC;
+	reg_val |= SUN4I_SPDIF_FCTL_TXTL_MASK;
+	reg_val |= SUN4I_SPDIF_FCTL_RXTL_MASK;
+	reg_val |= SUN4I_SPDIF_FCTL_TXIM;
+	reg_val |= SUN4I_SPDIF_FCTL_RXOM_MASK;
+	regmap_write(host->regmap, SUN4I_SPDIF_FCTL, reg_val);
+
+	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;
+	}
+
+	reg_val = 0;
+	reg_val &= ~SUN4I_SPDIF_TXCFG_SINGLEMOD;
+	reg_val |= SUN4I_SPDIF_TXCFG_ASS;
+	reg_val |= fmt; /* set non audio and bit depth */
+	reg_val |= SUN4I_SPDIF_TXCFG_CHSTMODE;
+	reg_val |= SUN4I_SPDIF_TXCFG_TXRATIO(mclk_div - 1);
+	regmap_write(host->regmap, SUN4I_SPDIF_TXCFG, reg_val);
+
+	return 0;
+}
+
+static int sun4i_spdif_trigger(struct snd_pcm_substream *substream, int cmd,
+				struct snd_soc_dai *dai)
+{
+	int ret = 0;
+	struct sun4i_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:
+		sun4i_snd_txctrl_on(substream, host);
+		break;
+
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		sun4i_snd_txctrl_off(substream, host);
+		break;
+
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	return ret;
+}
+
+static int sun4i_spdif_soc_dai_probe(struct snd_soc_dai *dai)
+{
+	struct sun4i_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 sun4i_spdif_dai_ops = {
+	.startup	= sun4i_spdif_startup,
+	.shutdown	= sun4i_spdif_shutdown,
+	.trigger	= sun4i_spdif_trigger,
+	.hw_params	= sun4i_spdif_hw_params,
+};
+
+static const struct regmap_config sun4i_spdif_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = SUN4I_SPDIF_RXCHSTA1,
+};
+
+#define SUN4I_RATES	SNDRV_PCM_RATE_8000_192000
+
+#define SUN4I_FORMATS	(SNDRV_PCM_FORMAT_S16_LE | \
+				SNDRV_PCM_FORMAT_S20_3LE | \
+				SNDRV_PCM_FORMAT_S24_LE)
+
+static struct snd_soc_dai_driver sun4i_spdif_dai = {
+	.playback = {
+		.channels_min = 1,
+		.channels_max = 2,
+		.rates = SUN4I_RATES,
+		.formats = SUN4I_FORMATS,
+	},
+	.probe = sun4i_spdif_soc_dai_probe,
+	.ops = &sun4i_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" },
+};
+
+static const struct of_device_id sun4i_spdif_of_match[] = {
+	{ .compatible = "allwinner,sun4i-a10-spdif", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, sun4i_spdif_of_match);
+
+static const struct snd_soc_component_driver sun4i_spdif_component = {
+	.name		= "sun4i-spdif",
+};
+
+static int sun4i_spdif_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct sun4i_spdif_dev *host;
+	struct resource *res;
+	int ret;
+	void __iomem *base;
+
+	dev_dbg(&pdev->dev, "Entered %s\n", __func__);
+
+	if (!np)
+		return -ENODEV;
+
+	if (!of_match_device(sun4i_spdif_of_match, &pdev->dev)) {
+		dev_err(&pdev->dev, "No matched devices found.\n");
+		return -EINVAL;
+	}
+
+	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, &sun4i_spdif_dai, sizeof(sun4i_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,
+						&sun4i_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);
+	}
+
+	ret = clk_prepare_enable(host->apb_clk);
+	if (ret) {
+		dev_err(&pdev->dev, "try to enable apb_spdif_clk failed\n");
+		return ret;
+	}
+
+	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_prepare_enable(host->audio_clk);
+	if (ret) {
+		dev_err(&pdev->dev, "try to enable audio clk failed\n");
+		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 + SUN4I_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 + SUN4I_SPDIF_RXFIFO;
+	host->dma_params_rx.maxburst = 4;
+	host->dma_params_rx.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+
+	platform_set_drvdata(pdev, host);
+
+	ret = devm_snd_soc_register_component(&pdev->dev,
+				&sun4i_spdif_component, &sun4i_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 sun4i_spdif_remove(struct platform_device *pdev)
+{
+	struct sun4i_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;
+}
+
+static struct platform_driver sun4i_spdif_driver = {
+	.driver		= {
+		.name	= "sun4i-spdif",
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(sun4i_spdif_of_match),
+	},
+	.probe		= sun4i_spdif_probe,
+	.remove		= sun4i_spdif_remove,
+};
+
+module_platform_driver(sun4i_spdif_driver);
+
+MODULE_AUTHOR("Marcus Cooper <codekipper@gmail.com>");
+MODULE_AUTHOR("Andrea Venturi <be17068@iperbole.bo.it>");
+MODULE_DESCRIPTION("Allwinner sun4i SPDIF SoC Interface");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:sun4i-spdif");
-- 
1.9.1

[PATCH v2 4/4] ASOC: sunxi: Add support for the spdif block

[kbuild test robot]

Hi Marcus,

[auto build test results on next-20150930 -- if it's inappropriate base, please ignore]


coccinelle warnings: (new ones prefixed by >>)

>> sound/soc/sunxi/sun4i-spdif.c:599:3-8: No need to set .owner here. The core will do it.

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[PATCH] ASOC: sunxi: fix platform_no_drv_owner.cocci warnings

[kbuild test robot]

sound/soc/sunxi/sun4i-spdif.c:599:3-8: No need to set .owner here. The core will do it.

 Remove .owner field if calls are used which set it automatically

Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci

CC: Marcus Cooper <codekipper@gmail.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 sun4i-spdif.c |    1 -
 1 file changed, 1 deletion(-)

--- a/sound/soc/sunxi/sun4i-spdif.c
+++ b/sound/soc/sunxi/sun4i-spdif.c
@@ -596,7 +596,6 @@ static int sun4i_spdif_remove(struct pla
 static struct platform_driver sun4i_spdif_driver = {
 	.driver		= {
 		.name	= "sun4i-spdif",
-		.owner	= THIS_MODULE,
 		.of_match_table = of_match_ptr(sun4i_spdif_of_match),
 	},
 	.probe		= sun4i_spdif_probe,

[PATCH v2 4/4] ASOC: sunxi: Add support for the spdif block

[kbuild test robot]

Hi Marcus,

[auto build test results on next-20150930 -- if it's inappropriate base, please ignore]

reproduce:
  # apt-get install sparse
  make ARCH=x86_64 allmodconfig
  make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> sound/soc/sunxi/sun4i-spdif.c:207:6: sparse: symbol 'sun4i_snd_txctrl_on' was not declared. Should it be static?
>> sound/soc/sunxi/sun4i-spdif.c:231:6: sparse: symbol 'sun4i_snd_txctrl_off' was not declared. Should it be static?
>> sound/soc/sunxi/sun4i-spdif.c:451:28: sparse: incorrect type in initializer (different base types)
   sound/soc/sunxi/sun4i-spdif.c:451:28:    expected unsigned long long [unsigned] [usertype] formats
   sound/soc/sunxi/sun4i-spdif.c:451:28:    got restricted snd_pcm_format_t

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[RFC PATCH] ASOC: sunxi: sun4i_snd_txctrl_on() can be static

[kbuild test robot]

Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
 sun4i-spdif.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-spdif.c b/sound/soc/sunxi/sun4i-spdif.c
index 5fff6f6..cf0ed80 100644
--- a/sound/soc/sunxi/sun4i-spdif.c
+++ b/sound/soc/sunxi/sun4i-spdif.c
@@ -204,7 +204,7 @@ static void sun4i_spdif_configure(struct sun4i_spdif_dev *host)
 
 }
 
-void sun4i_snd_txctrl_on(struct snd_pcm_substream *substream,
+static void sun4i_snd_txctrl_on(struct snd_pcm_substream *substream,
 				struct sun4i_spdif_dev *host)
 {
 	if (substream->runtime->channels == 1)
@@ -228,7 +228,7 @@ void sun4i_snd_txctrl_on(struct snd_pcm_substream *substream,
 					SUN4I_SPDIF_CTL_GEN);
 }
 
-void sun4i_snd_txctrl_off(struct snd_pcm_substream *substream,
+static void sun4i_snd_txctrl_off(struct snd_pcm_substream *substream,
 				struct sun4i_spdif_dev *host)
 {
 	/* SPDIF TX DISABLE */

[PATCH v2 4/4] ASOC: sunxi: Add support for the spdif block

[Maxime Ripard]

On Wed, Sep 30, 2015 at 07:50:58PM +0200, codekipper at gmail.com wrote:
> From: Marcus Cooper <codekipper@gmail.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@gmail.com>
> ---
>  sound/soc/sunxi/Kconfig       |  12 +
>  sound/soc/sunxi/Makefile      |   4 +
>  sound/soc/sunxi/sun4i-spdif.c | 612 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 628 insertions(+)
>  create mode 100644 sound/soc/sunxi/sun4i-spdif.c
> 
> diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
> index 84c72ec..2ebf43d 100644
> --- a/sound/soc/sunxi/Kconfig
> +++ b/sound/soc/sunxi/Kconfig
> @@ -8,4 +8,16 @@ 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
> +	depends on OF
> +        select SND_SOC_GENERIC_DMAENGINE_PCM
> +        select REGMAP_MMIO
> +
> +config SND_SOC_SUNXI_MACHINE_SPDIF
> +        tristate "APB on-chip sun4i/sun5i/sun7i SPDIF"
> +	depends on OF
> +        select SND_SOC_SUNXI_DAI_SPDIF
> +        help
> +          Say Y if you want to add support for SoC S/PDIF audio as simple audio card.

You still haven't said why you can't use simple-card...

> +static void sun4i_spdif_configure(struct sun4i_spdif_dev *host)
> +{
> +	u32 reg_val;
> +
> +	/* soft reset SPDIF */
> +	regmap_write(host->regmap, SUN4I_SPDIF_CTL, SUN4I_SPDIF_CTL_RESET);
> +
> +	/* MCLK OUTPUT enable */
> +	regmap_update_bits(host->regmap, SUN4I_SPDIF_CTL,
> +			SUN4I_SPDIF_CTL_MCLKOUTEN, SUN4I_SPDIF_CTL_MCLKOUTEN);

The alignment is still not right....

> +	/* flush TX FIFO */
> +	regmap_update_bits(host->regmap, SUN4I_SPDIF_FCTL,
> +			SUN4I_SPDIF_FCTL_FTX, SUN4I_SPDIF_FCTL_FTX);
> +
> +	/* clear interrupt status */
> +	regmap_read(host->regmap, SUN4I_SPDIF_ISTA, &reg_val);
> +	regmap_write(host->regmap, SUN4I_SPDIF_ISTA, reg_val);

You're not using any interrupts. Why is this needed?

> +static int sun4i_spdif_startup(struct snd_pcm_substream *substream,
> +				struct snd_soc_dai *cpu_dai)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct sun4i_spdif_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
> +
> +	if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
> +		return -EINVAL;
> +
> +	sun4i_spdif_configure(host);
> +
> +	return clk_prepare_enable(host->clk);

You're still not using pm_runtime...

> +}
> +
> +static void sun4i_spdif_shutdown(struct snd_pcm_substream *substream,
> +				struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct sun4i_spdif_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
> +
> +	if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
> +		return;
> +
> +	clk_disable_unprepare(host->clk);
> +}
> +
> +static int sun4i_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;
> +	u32 reg_val;
> +	struct sun4i_spdif_dev *host = snd_soc_dai_get_drvdata(cpu_dai);
> +	struct platform_device *pdev = host->pdev;
> +
> +	/* 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 = SUN4I_SPDIF_TXCFG_NONAUDIO;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	switch (params_format(params)) {
> +	case SNDRV_PCM_FORMAT_S16_LE:
> +		fmt |= SUN4I_SPDIF_TXCFG_FMT16BIT;
> +		break;
> +	case SNDRV_PCM_FORMAT_S20_3LE:
> +		fmt |= SUN4I_SPDIF_TXCFG_FMT20BIT;
> +		break;
> +	case SNDRV_PCM_FORMAT_S24_LE:
> +		fmt |= SUN4I_SPDIF_TXCFG_FMT24BIT;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	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 = clk_set_rate(host->audio_clk, mclk);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev,
> +			"Setting pll2 clock rate for %d Hz failed!\n", mclk);
> +		return ret;
> +	}

You're still using the PLL2...

> +
> +	ret = clk_set_rate(host->clk, mclk);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev,
> +			"Setting SPDIF clock rate for %d Hz failed!\n", mclk);
> +		return ret;
> +	}
> +
> +	reg_val = 0;
> +	reg_val &= ~SUN4I_SPDIF_FCTL_FIFOSRC;
> +	reg_val |= SUN4I_SPDIF_FCTL_TXTL_MASK;
> +	reg_val |= SUN4I_SPDIF_FCTL_RXTL_MASK;
> +	reg_val |= SUN4I_SPDIF_FCTL_TXIM;
> +	reg_val |= SUN4I_SPDIF_FCTL_RXOM_MASK;
> +	regmap_write(host->regmap, SUN4I_SPDIF_FCTL, reg_val);

You're still not using regmap_update_bits...

IF you're really going to ignore all the comments we did, please tell
us upfront. That way, we will not waste our time doing a review of
your patches.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151001/a9646fd9/attachment.sig>

[PATCH v2 4/4] ASOC: sunxi: Add support for the spdif block

[Code Kipper]

>> +config SND_SOC_SUNXI_DAI_SPDIF
>> +        tristate
>> +     depends on OF
>> +        select SND_SOC_GENERIC_DMAENGINE_PCM
>> +        select REGMAP_MMIO
>> +
>> +config SND_SOC_SUNXI_MACHINE_SPDIF
>> +        tristate "APB on-chip sun4i/sun5i/sun7i SPDIF"
>> +     depends on OF
>> +        select SND_SOC_SUNXI_DAI_SPDIF
>> +        help
>> +          Say Y if you want to add support for SoC S/PDIF audio as simple audio card.
>
> You still haven't said why you can't use simple-card...

I mentioned in the covering letter that I thought that simple-card was
overkill. There is also a thread concerning issues with the ordering
of module bringup here
http://mailman.alsa-project.org/pipermail/alsa-devel/2013-December/070534.html.
I was initially trying to use the dummy spdif transmitter but couldn't
get it working, this set up works for me. I haven't got an audio guy
sitting next to me to ping and have reached out for some guidance. I
can do this using simple-card, it just with all the driver refactoring
it was the main place where I thought things would break.

>
>> +static void sun4i_spdif_configure(struct sun4i_spdif_dev *host)
>> +{
>> +     u32 reg_val;
>> +
>> +     /* soft reset SPDIF */
>> +     regmap_write(host->regmap, SUN4I_SPDIF_CTL, SUN4I_SPDIF_CTL_RESET);
>> +
>> +     /* MCLK OUTPUT enable */
>> +     regmap_update_bits(host->regmap, SUN4I_SPDIF_CTL,
>> +                     SUN4I_SPDIF_CTL_MCLKOUTEN, SUN4I_SPDIF_CTL_MCLKOUTEN);
>
> The alignment is still not right....

I'm not even sure if we need mclk output enabled. Let me see what
happens when I remove this.

>
>> +     /* flush TX FIFO */
>> +     regmap_update_bits(host->regmap, SUN4I_SPDIF_FCTL,
>> +                     SUN4I_SPDIF_FCTL_FTX, SUN4I_SPDIF_FCTL_FTX);
>> +
>> +     /* clear interrupt status */
>> +     regmap_read(host->regmap, SUN4I_SPDIF_ISTA, &reg_val);
>> +     regmap_write(host->regmap, SUN4I_SPDIF_ISTA, reg_val);
>
> You're not using any interrupts. Why is this needed?

ditto. This wasn't brought up in the previous reviews.

>
>> +static int sun4i_spdif_startup(struct snd_pcm_substream *substream,
>> +                             struct snd_soc_dai *cpu_dai)
>> +{
>> +     struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> +     struct sun4i_spdif_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
>> +
>> +     if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
>> +             return -EINVAL;
>> +
>> +     sun4i_spdif_configure(host);
>> +
>> +     return clk_prepare_enable(host->clk);
>
> You're still not using pm_runtime...

I've removed the pm stuff and this is the same as you have it in sun4i-codec.

>
>> +
>> +     ret = clk_set_rate(host->audio_clk, mclk);
>> +     if (ret < 0) {
>> +             dev_err(&pdev->dev,
>> +                     "Setting pll2 clock rate for %d Hz failed!\n", mclk);
>> +             return ret;
>> +     }
>
> You're still using the PLL2...

I commented this out and it stopped working...let me check again.

>
>> +
>> +     ret = clk_set_rate(host->clk, mclk);
>> +     if (ret < 0) {
>> +             dev_err(&pdev->dev,
>> +                     "Setting SPDIF clock rate for %d Hz failed!\n", mclk);
>> +             return ret;
>> +     }
>> +
>> +     reg_val = 0;
>> +     reg_val &= ~SUN4I_SPDIF_FCTL_FIFOSRC;
>> +     reg_val |= SUN4I_SPDIF_FCTL_TXTL_MASK;
>> +     reg_val |= SUN4I_SPDIF_FCTL_RXTL_MASK;
>> +     reg_val |= SUN4I_SPDIF_FCTL_TXIM;
>> +     reg_val |= SUN4I_SPDIF_FCTL_RXOM_MASK;
>> +     regmap_write(host->regmap, SUN4I_SPDIF_FCTL, reg_val);
>
> You're still not using regmap_update_bits...

Why would I need to?, this is the first write to the register before
playback and I'm not interested in keeping any of the previous fifo
settings. Will remove"reg_val &= ~SUN4I_SPDIF_FCTL_FIFOSRC;" as that's
not doing anything.

>
> IF you're really going to ignore all the comments we did, please tell
> us upfront. That way, we will not waste our time doing a review of
> your patches.

All is a strong word....did you even read my covering letter?....there
was a major refactoring of the code and I think I covered a majority
of the comments. Apologies if you feel that you'd wasted a lot of time
of this....it can't be any more that the EVB dts.
Thanks anyway,
CK
>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

[PATCH v2 4/4] ASOC: sunxi: Add support for the spdif block

[Maxime Ripard]

On Fri, Oct 02, 2015 at 08:44:03AM +0200, Code Kipper wrote:
> >> +config SND_SOC_SUNXI_DAI_SPDIF
> >> +        tristate
> >> +     depends on OF
> >> +        select SND_SOC_GENERIC_DMAENGINE_PCM
> >> +        select REGMAP_MMIO
> >> +
> >> +config SND_SOC_SUNXI_MACHINE_SPDIF
> >> +        tristate "APB on-chip sun4i/sun5i/sun7i SPDIF"
> >> +     depends on OF
> >> +        select SND_SOC_SUNXI_DAI_SPDIF
> >> +        help
> >> +          Say Y if you want to add support for SoC S/PDIF audio as simple audio card.
> >
> > You still haven't said why you can't use simple-card...
> 
> I mentioned in the covering letter that I thought that simple-card was
> overkill.

Overkill for what? It adds no code, that will put no new maintainance
burden, without any duplication. While your code also adds all that.

> There is also a thread concerning issues with the ordering
> of module bringup here
> http://mailman.alsa-project.org/pipermail/alsa-devel/2013-December/070534.html.
> I was initially trying to use the dummy spdif transmitter but couldn't
> get it working, this set up works for me. I haven't got an audio guy
> sitting next to me to ping and have reached out for some guidance. I
> can do this using simple-card, it just with all the driver refactoring
> it was the main place where I thought things would break.

We're all on IRC.


> >> +static void sun4i_spdif_configure(struct sun4i_spdif_dev *host)
> >> +{
> >> +     u32 reg_val;
> >> +
> >> +     /* soft reset SPDIF */
> >> +     regmap_write(host->regmap, SUN4I_SPDIF_CTL, SUN4I_SPDIF_CTL_RESET);
> >> +
> >> +     /* MCLK OUTPUT enable */
> >> +     regmap_update_bits(host->regmap, SUN4I_SPDIF_CTL,
> >> +                     SUN4I_SPDIF_CTL_MCLKOUTEN, SUN4I_SPDIF_CTL_MCLKOUTEN);
> >
> > The alignment is still not right....
> 
> I'm not even sure if we need mclk output enabled. Let me see what
> happens when I remove this.

It's not really the point. The alignment of all your wrapped lines is
wrong.

> >> +static int sun4i_spdif_startup(struct snd_pcm_substream *substream,
> >> +                             struct snd_soc_dai *cpu_dai)
> >> +{
> >> +     struct snd_soc_pcm_runtime *rtd = substream->private_data;
> >> +     struct sun4i_spdif_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
> >> +
> >> +     if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
> >> +             return -EINVAL;
> >> +
> >> +     sun4i_spdif_configure(host);
> >> +
> >> +     return clk_prepare_enable(host->clk);
> >
> > You're still not using pm_runtime...
> 
> I've removed the pm stuff and this is the same as you have it in
> sun4i-codec.

You've removed the suspend code, and both Mark and I asked you to use
runtime_pm to handle your bus clock.

And this has also been asked for the codec.

> >> +
> >> +     ret = clk_set_rate(host->audio_clk, mclk);
> >> +     if (ret < 0) {
> >> +             dev_err(&pdev->dev,
> >> +                     "Setting pll2 clock rate for %d Hz failed!\n", mclk);
> >> +             return ret;
> >> +     }
> >
> > You're still using the PLL2...
> 
> I commented this out and it stopped working...let me check again.

Then something is wrong somewhere else that needs to be fixed, either
in the clock driver or in this driver. Did you update all the other
references to PLL2 as well?

> 
> >
> >> +
> >> +     ret = clk_set_rate(host->clk, mclk);
> >> +     if (ret < 0) {
> >> +             dev_err(&pdev->dev,
> >> +                     "Setting SPDIF clock rate for %d Hz failed!\n", mclk);
> >> +             return ret;
> >> +     }
> >> +
> >> +     reg_val = 0;
> >> +     reg_val &= ~SUN4I_SPDIF_FCTL_FIFOSRC;
> >> +     reg_val |= SUN4I_SPDIF_FCTL_TXTL_MASK;
> >> +     reg_val |= SUN4I_SPDIF_FCTL_RXTL_MASK;
> >> +     reg_val |= SUN4I_SPDIF_FCTL_TXIM;
> >> +     reg_val |= SUN4I_SPDIF_FCTL_RXOM_MASK;
> >> +     regmap_write(host->regmap, SUN4I_SPDIF_FCTL, reg_val);
> >
> > You're still not using regmap_update_bits...
> 
> Why would I need to?, this is the first write to the register before
> playback and I'm not interested in keeping any of the previous fifo
> settings. Will remove"reg_val &= ~SUN4I_SPDIF_FCTL_FIFOSRC;" as that's
> not doing anything.

Dropping the masking is also an option.

> > IF you're really going to ignore all the comments we did, please tell
> > us upfront. That way, we will not waste our time doing a review of
> > your patches.
> 
> All is a strong word....did you even read my covering letter?....there
> was a major refactoring of the code and I think I covered a majority
> of the comments. Apologies if you feel that you'd wasted a lot of time
> of this....it can't be any more that the EVB dts.

The point of this is that this is a discussion. You simply ignored
most of the comments, without even mentionning why you wanted to
ignore them, and simply sent a new version.

If you feel like one comment is invalid, let's discuss this, like you
did here. But silently discarding them is not an option and actually
quite rude.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151006/95784d5c/attachment.sig>

[PATCH v2 4/4] ASOC: sunxi: Add support for the spdif block

[Code Kipper]

On 6 October 2015 at 11:00, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Fri, Oct 02, 2015 at 08:44:03AM +0200, Code Kipper wrote:
>> >> +config SND_SOC_SUNXI_DAI_SPDIF
>> >> +        tristate
>> >> +     depends on OF
>> >> +        select SND_SOC_GENERIC_DMAENGINE_PCM
>> >> +        select REGMAP_MMIO
>> >> +
>> >> +config SND_SOC_SUNXI_MACHINE_SPDIF
>> >> +        tristate "APB on-chip sun4i/sun5i/sun7i SPDIF"
>> >> +     depends on OF
>> >> +        select SND_SOC_SUNXI_DAI_SPDIF
>> >> +        help
>> >> +          Say Y if you want to add support for SoC S/PDIF audio as simple audio card.
>> >
>> > You still haven't said why you can't use simple-card...
>>
>> I mentioned in the covering letter that I thought that simple-card was
>> overkill.
>
> Overkill for what? It adds no code, that will put no new maintainance
> burden, without any duplication. While your code also adds all that.
>
>> There is also a thread concerning issues with the ordering
>> of module bringup here
>> http://mailman.alsa-project.org/pipermail/alsa-devel/2013-December/070534.html.
>> I was initially trying to use the dummy spdif transmitter but couldn't
>> get it working, this set up works for me. I haven't got an audio guy
>> sitting next to me to ping and have reached out for some guidance. I
>> can do this using simple-card, it just with all the driver refactoring
>> it was the main place where I thought things would break.
>
> We're all on IRC.

OK, let me sit on the next patch release until I've properly
investigated this. I'm not a big IRCer so I will need to change my
habits.

>
>
>> >> +static void sun4i_spdif_configure(struct sun4i_spdif_dev *host)
>> >> +{
>> >> +     u32 reg_val;
>> >> +
>> >> +     /* soft reset SPDIF */
>> >> +     regmap_write(host->regmap, SUN4I_SPDIF_CTL, SUN4I_SPDIF_CTL_RESET);
>> >> +
>> >> +     /* MCLK OUTPUT enable */
>> >> +     regmap_update_bits(host->regmap, SUN4I_SPDIF_CTL,
>> >> +                     SUN4I_SPDIF_CTL_MCLKOUTEN, SUN4I_SPDIF_CTL_MCLKOUTEN);
>> >
>> > The alignment is still not right....
>>
>> I'm not even sure if we need mclk output enabled. Let me see what
>> happens when I remove this.
>
> It's not really the point. The alignment of all your wrapped lines is
> wrong.

Ahhhh....I was brought up to not mix tabs and spaces and I now see
with a quick check that checkpatch doesn't barf...I'll fix this.

>
>> >> +static int sun4i_spdif_startup(struct snd_pcm_substream *substream,
>> >> +                             struct snd_soc_dai *cpu_dai)
>> >> +{
>> >> +     struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> >> +     struct sun4i_spdif_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
>> >> +
>> >> +     if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
>> >> +             return -EINVAL;
>> >> +
>> >> +     sun4i_spdif_configure(host);
>> >> +
>> >> +     return clk_prepare_enable(host->clk);
>> >
>> > You're still not using pm_runtime...
>>
>> I've removed the pm stuff and this is the same as you have it in
>> sun4i-codec.
>
> You've removed the suspend code, and both Mark and I asked you to use
> runtime_pm to handle your bus clock.
>
> And this has also been asked for the codec.

You asked if I had tested the pm operations which I hadn't so I
removed them after looking at your driver and searching for pm_runtime
usage elsewhere in sound/soc. I will add them back.

>
>> >> +
>> >> +     ret = clk_set_rate(host->audio_clk, mclk);
>> >> +     if (ret < 0) {
>> >> +             dev_err(&pdev->dev,
>> >> +                     "Setting pll2 clock rate for %d Hz failed!\n", mclk);
>> >> +             return ret;
>> >> +     }
>> >
>> > You're still using the PLL2...
>>
>> I commented this out and it stopped working...let me check again.
>
> Then something is wrong somewhere else that needs to be fixed, either
> in the clock driver or in this driver. Did you update all the other
> references to PLL2 as well?

To be honest...the underlying clock code that I used wasn't based on
your latest patches. I'll relook at this, maybe my dsti is at fault.
>
>>
>> >
>> >> +
>> >> +     ret = clk_set_rate(host->clk, mclk);
>> >> +     if (ret < 0) {
>> >> +             dev_err(&pdev->dev,
>> >> +                     "Setting SPDIF clock rate for %d Hz failed!\n", mclk);
>> >> +             return ret;
>> >> +     }
>> >> +
>> >> +     reg_val = 0;
>> >> +     reg_val &= ~SUN4I_SPDIF_FCTL_FIFOSRC;
>> >> +     reg_val |= SUN4I_SPDIF_FCTL_TXTL_MASK;
>> >> +     reg_val |= SUN4I_SPDIF_FCTL_RXTL_MASK;
>> >> +     reg_val |= SUN4I_SPDIF_FCTL_TXIM;
>> >> +     reg_val |= SUN4I_SPDIF_FCTL_RXOM_MASK;
>> >> +     regmap_write(host->regmap, SUN4I_SPDIF_FCTL, reg_val);
>> >
>> > You're still not using regmap_update_bits...
>>
>> Why would I need to?, this is the first write to the register before
>> playback and I'm not interested in keeping any of the previous fifo
>> settings. Will remove"reg_val &= ~SUN4I_SPDIF_FCTL_FIFOSRC;" as that's
>> not doing anything.
>
> Dropping the masking is also an option.

Yeah...that still doesn't look right. I'll investigate.

>
>> > IF you're really going to ignore all the comments we did, please tell
>> > us upfront. That way, we will not waste our time doing a review of
>> > your patches.
>>
>> All is a strong word....did you even read my covering letter?....there
>> was a major refactoring of the code and I think I covered a majority
>> of the comments. Apologies if you feel that you'd wasted a lot of time
>> of this....it can't be any more that the EVB dts.
>
> The point of this is that this is a discussion. You simply ignored
> most of the comments, without even mentionning why you wanted to
> ignore them, and simply sent a new version.
>
> If you feel like one comment is invalid, let's discuss this, like you
> did here. But silently discarding them is not an option and actually
> quite rude.

I won't be in such a rush to resend the next patch set. I'll clear up
everything and if I run into any difficulties then I'll push to my
github and ping you on IRC.
Thanks,
CK
>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

[PATCH v2 4/4] ASOC: sunxi: Add support for the spdif block

[Maxime Ripard]

Hi,

On Tue, Oct 06, 2015 at 12:38:57PM +0200, Code Kipper wrote:
> >> >> +static void sun4i_spdif_configure(struct sun4i_spdif_dev *host)
> >> >> +{
> >> >> +     u32 reg_val;
> >> >> +
> >> >> +     /* soft reset SPDIF */
> >> >> +     regmap_write(host->regmap, SUN4I_SPDIF_CTL, SUN4I_SPDIF_CTL_RESET);
> >> >> +
> >> >> +     /* MCLK OUTPUT enable */
> >> >> +     regmap_update_bits(host->regmap, SUN4I_SPDIF_CTL,
> >> >> +                     SUN4I_SPDIF_CTL_MCLKOUTEN, SUN4I_SPDIF_CTL_MCLKOUTEN);
> >> >
> >> > The alignment is still not right....
> >>
> >> I'm not even sure if we need mclk output enabled. Let me see what
> >> happens when I remove this.
> >
> > It's not really the point. The alignment of all your wrapped lines is
> > wrong.
> 
> Ahhhh....I was brought up to not mix tabs and spaces and I now see
> with a quick check that checkpatch doesn't barf...I'll fix this.

checkpatch --strict does

> >> >> +static int sun4i_spdif_startup(struct snd_pcm_substream *substream,
> >> >> +                             struct snd_soc_dai *cpu_dai)
> >> >> +{
> >> >> +     struct snd_soc_pcm_runtime *rtd = substream->private_data;
> >> >> +     struct sun4i_spdif_dev *host = snd_soc_dai_get_drvdata(rtd->cpu_dai);
> >> >> +
> >> >> +     if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
> >> >> +             return -EINVAL;
> >> >> +
> >> >> +     sun4i_spdif_configure(host);
> >> >> +
> >> >> +     return clk_prepare_enable(host->clk);
> >> >
> >> > You're still not using pm_runtime...
> >>
> >> I've removed the pm stuff and this is the same as you have it in
> >> sun4i-codec.
> >
> > You've removed the suspend code, and both Mark and I asked you to use
> > runtime_pm to handle your bus clock.
> >
> > And this has also been asked for the codec.
> 
> You asked if I had tested the pm operations which I hadn't so I
> removed them after looking at your driver and searching for pm_runtime
> usage elsewhere in sound/soc. I will add them back.

What we asked you to remove were the suspend / resume hooks. What we
want you to add are runtime_pm hooks. These are not the same hooks,
and they're not called at the same moment.

The suspend / resume hooks are called before entering suspend and
after coming back from it. We don't have no way to suspend at the
moment, so there's no way you've been able to test it.

The runtime_pm hooks are called whenever your device start to be used
(for example when you start playing back an audio file on your
system).

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151026/b76b2397/attachment.sig>