[PATCH 0/2] ASoC: sirf: Add sirf audio hub driver and use it.

[PATCH 0/2] ASoC: sirf: Add sirf audio hub driver and use it.

[RongJun Ying]

From: Rongjun Ying <rongjun.ying@csr.com>

This patchset adds including:
1. SiRF audio hub driver
2. The internal codec and port get regmap form audio hub driver
3. Add SiRF audio hub binding document

Rongjun Ying (2):
  ASoC: sirf: Add sirf audio hub driver for sharing same register
    address space
  Documentation: Describe the SiRF audio hub Device Tree bindings

 .../devicetree/bindings/sound/sirf-audio-hub.txt   |   52 +++++++++++++++
 sound/soc/codecs/sirf-audio-codec.c                |   32 ++--------
 sound/soc/sirf/Kconfig                             |    5 ++
 sound/soc/sirf/Makefile                            |    2 +
 sound/soc/sirf/sirf-audio-hub.c                    |   66 ++++++++++++++++++++
 sound/soc/sirf/sirf-audio-port.c                   |   26 +--------
 6 files changed, 131 insertions(+), 52 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/sirf-audio-hub.txt
 create mode 100644 sound/soc/sirf/sirf-audio-hub.c

-- 
1.7.5.4

[PATCH 1/2] ASoC: sirf: Add sirf audio hub driver for sharing same register address space

[RongJun Ying]

From: Rongjun Ying <rongjun.ying@csr.com>

The SiRF audio modules such as I2S, internal audio codec and audio port are sharing
same register address space. This driver create regmap and register all child audio device.
The child audio drivers can get regmap form its' parent driver data.

Signed-off-by: Rongjun Ying <rongjun.ying@csr.com>
---
 sound/soc/codecs/sirf-audio-codec.c |   32 +++--------------
 sound/soc/sirf/Kconfig              |    5 +++
 sound/soc/sirf/Makefile             |    2 +
 sound/soc/sirf/sirf-audio-hub.c     |   66 +++++++++++++++++++++++++++++++++++
 sound/soc/sirf/sirf-audio-port.c    |   26 +-------------
 5 files changed, 79 insertions(+), 52 deletions(-)
 create mode 100644 sound/soc/sirf/sirf-audio-hub.c

diff --git a/sound/soc/codecs/sirf-audio-codec.c b/sound/soc/codecs/sirf-audio-codec.c
index 90e3a22..d04e202 100644
--- a/sound/soc/codecs/sirf-audio-codec.c
+++ b/sound/soc/codecs/sirf-audio-codec.c
@@ -283,9 +283,10 @@ static int sirf_audio_codec_trigger(struct snd_pcm_substream *substream,
 		int cmd,
 		struct snd_soc_dai *dai)
 {
-	int playback = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
 	struct snd_soc_codec *codec = dai->codec;
 	u32 val = 0;
+	if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
+		return 0;
 
 	/*
 	 * This is a workaround, When stop playback,
@@ -299,15 +300,13 @@ static int sirf_audio_codec_trigger(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		if (playback)
-			val = IC_HSLEN | IC_HSREN;
+		val = IC_HSLEN | IC_HSREN;
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	if (playback)
-		snd_soc_update_bits(codec, AUDIO_IC_CODEC_CTRL0,
+	snd_soc_update_bits(codec, AUDIO_IC_CODEC_CTRL0,
 			IC_HSLEN | IC_HSREN, val);
 	return 0;
 }
@@ -397,23 +396,10 @@ static const struct of_device_id sirf_audio_codec_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, sirf_audio_codec_of_match);
 
-static const struct regmap_config sirf_audio_codec_regmap_config = {
-	.reg_bits = 32,
-	.reg_stride = 4,
-	.val_bits = 32,
-	.max_register = AUDIO_IC_CODEC_CTRL3,
-	.cache_type = REGCACHE_NONE,
-};
-
 static int sirf_audio_codec_driver_probe(struct platform_device *pdev)
 {
 	int ret;
 	struct sirf_audio_codec *sirf_audio_codec;
-	void __iomem *base;
-	struct resource *mem_res;
-	const struct of_device_id *match;
-
-	match = of_match_node(sirf_audio_codec_of_match, pdev->dev.of_node);
 
 	sirf_audio_codec = devm_kzalloc(&pdev->dev,
 		sizeof(struct sirf_audio_codec), GFP_KERNEL);
@@ -422,15 +408,7 @@ static int sirf_audio_codec_driver_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, sirf_audio_codec);
 
-	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(&pdev->dev, mem_res);
-	if (base == NULL)
-		return -ENOMEM;
-
-	sirf_audio_codec->regmap = devm_regmap_init_mmio(&pdev->dev, base,
-					    &sirf_audio_codec_regmap_config);
-	if (IS_ERR(sirf_audio_codec->regmap))
-		return PTR_ERR(sirf_audio_codec->regmap);
+	sirf_audio_codec->regmap = dev_get_drvdata(pdev->dev.parent);
 
 	sirf_audio_codec->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(sirf_audio_codec->clk)) {
diff --git a/sound/soc/sirf/Kconfig b/sound/soc/sirf/Kconfig
index 89e8942..723641c 100644
--- a/sound/soc/sirf/Kconfig
+++ b/sound/soc/sirf/Kconfig
@@ -8,6 +8,11 @@ config SND_SOC_SIRF_AUDIO
 	depends on SND_SOC_SIRF
 	select SND_SOC_SIRF_AUDIO_CODEC
 	select SND_SOC_SIRF_AUDIO_PORT
+	select SND_SOC_SIRF_AUDIO_HUB
+
+config SND_SOC_SIRF_AUDIO_HUB
+	depends on SND_SOC_SIRF
+	tristate
 
 config SND_SOC_SIRF_AUDIO_PORT
 	select REGMAP_MMIO
diff --git a/sound/soc/sirf/Makefile b/sound/soc/sirf/Makefile
index 913b932..74927e8 100644
--- a/sound/soc/sirf/Makefile
+++ b/sound/soc/sirf/Makefile
@@ -1,5 +1,7 @@
 snd-soc-sirf-audio-objs := sirf-audio.o
+snd-soc-sirf-audio-hub-objs := sirf-audio-hub.o
 snd-soc-sirf-audio-port-objs := sirf-audio-port.o
 
 obj-$(CONFIG_SND_SOC_SIRF_AUDIO) += snd-soc-sirf-audio.o
+obj-$(CONFIG_SND_SOC_SIRF_AUDIO_HUB) += snd-soc-sirf-audio-hub.o
 obj-$(CONFIG_SND_SOC_SIRF_AUDIO_PORT) += snd-soc-sirf-audio-port.o
diff --git a/sound/soc/sirf/sirf-audio-hub.c b/sound/soc/sirf/sirf-audio-hub.c
new file mode 100644
index 0000000..d30ef78
--- /dev/null
+++ b/sound/soc/sirf/sirf-audio-hub.c
@@ -0,0 +1,66 @@
+/*
+ * sirf-audio-hub.c - SiRF Audio hub driver purpose is sharing regmap.
+ *
+ * Copyright (c) 2011 Cambridge Silicon Radio Limited, a CSR plc group company.
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+/* This regmap is shared all child audio drivers */
+struct regmap *regmap;
+
+static const struct regmap_config sirf_audio_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.cache_type = REGCACHE_NONE,
+};
+
+static int sirf_audio_probe(struct platform_device *pdev)
+{
+	struct resource *mem_res;
+	void __iomem *base;
+
+	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, mem_res);
+	if (base == NULL)
+		return -ENOMEM;
+
+	regmap = devm_regmap_init_mmio(&pdev->dev, base,
+			&sirf_audio_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	platform_set_drvdata(pdev, regmap);
+
+	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+}
+
+static const struct of_device_id sirf_audio_of_match[] = {
+	{ .compatible = "sirf,audio-hub", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, sirf_audio_of_match);
+
+static struct platform_driver sirf_audio_driver = {
+	.driver = {
+		.name = "sirf-audio-hub",
+		.owner = THIS_MODULE,
+		.of_match_table = sirf_audio_of_match,
+	},
+	.probe = sirf_audio_probe,
+};
+
+module_platform_driver(sirf_audio_driver);
+
+MODULE_DESCRIPTION("SiRF Audio hub driver");
+MODULE_AUTHOR("RongJun Ying <Rongjun.Ying@csr.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/sirf/sirf-audio-port.c b/sound/soc/sirf/sirf-audio-port.c
index b04a53f..cda4cb4 100644
--- a/sound/soc/sirf/sirf-audio-port.c
+++ b/sound/soc/sirf/sirf-audio-port.c
@@ -127,41 +127,17 @@ static const struct snd_soc_component_driver sirf_audio_port_component = {
 	.name       = "sirf-audio-port",
 };
 
-static const struct regmap_config sirf_audio_port_regmap_config = {
-	.reg_bits = 32,
-	.reg_stride = 4,
-	.val_bits = 32,
-	.max_register = AUDIO_PORT_IC_RXFIFO_INT_MSK,
-	.cache_type = REGCACHE_NONE,
-};
-
 static int sirf_audio_port_probe(struct platform_device *pdev)
 {
 	int ret;
 	struct sirf_audio_port *port;
-	void __iomem *base;
-	struct resource *mem_res;
 
 	port = devm_kzalloc(&pdev->dev,
 			sizeof(struct sirf_audio_port), GFP_KERNEL);
 	if (!port)
 		return -ENOMEM;
 
-	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!mem_res) {
-		dev_err(&pdev->dev, "no mem resource?\n");
-		return -ENODEV;
-	}
-
-	base = devm_ioremap(&pdev->dev, mem_res->start,
-			resource_size(mem_res));
-	if (base == NULL)
-		return -ENOMEM;
-
-	port->regmap = devm_regmap_init_mmio(&pdev->dev, base,
-					    &sirf_audio_port_regmap_config);
-	if (IS_ERR(port->regmap))
-		return PTR_ERR(port->regmap);
+	port->regmap = dev_get_drvdata(pdev->dev.parent);
 
 	ret = devm_snd_soc_register_component(&pdev->dev,
 			&sirf_audio_port_component, &sirf_audio_port_dai, 1);
-- 
1.7.5.4

[PATCH 2/2] Documentation: Describe the SiRF audio hub Device Tree bindings

[RongJun Ying]

From: Rongjun Ying <rongjun.ying@csr.com>

Signed-off-by: Rongjun Ying <rongjun.ying@csr.com>
---
 .../devicetree/bindings/sound/sirf-audio-hub.txt   |   52 ++++++++++++++++++++
 1 files changed, 52 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/sirf-audio-hub.txt

diff --git a/Documentation/devicetree/bindings/sound/sirf-audio-hub.txt b/Documentation/devicetree/bindings/sound/sirf-audio-hub.txt
new file mode 100644
index 0000000..4f148a1
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/sirf-audio-hub.txt
@@ -0,0 +1,52 @@
+SiRF audio hub
+
+The audio hub is the consist of an internal audio codec, internal
+audio port and i2s controller. There are sharing same register
+address space.
+
+Required properties:
+- compatible: Must be "sirf,audio-hub"
+- reg : The register address of the device
+- #address-cells: Must be 1
+- #size-cells: Must be 1
+
+Nodes:
+Internal audio codec:
+- compatible : "sirf,atlas6-audio-codec" or "sirf,prima2-audio-codec"
+- clocks: A phandle reference to the SiRF internal audio codec clock
+
+Internal audio port:
+- compatible: "sirf,audio-port"
+
+I2S controller:
+- compatible: "sirf,prima2-i2s"
+- clocks: A phandle reference to the SiRF I2S controller clock
+
+Example:
+sirf_audio_hub: audio_hub@b0040000 {
+	compatible = "sirf,audio-hub";
+	reg = <0xb0040000 0x10000>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	audiocodec: audiocodec {
+		compatible = "sirf,atlas6-audio-codec";
+		interrupts = <35>;
+		clocks = <&clks 27>;
+	};
+
+	audioport: audioport {
+		compatible = "sirf,audio-port";
+		dmas = <&dmac1 3>, <&dmac1 8>;
+		dma-names = "rx", "tx";
+	};
+
+	i2s: i2s {
+		compatible = "sirf,prima2-i2s";
+		dmas = <&dmac1 6>, <&dmac1 12>;
+		dma-names = "rx", "tx";
+		clocks = <&clks 27>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&i2s_pins_a>;
+	};
+};
-- 
1.7.5.4

Re: [PATCH 1/2] ASoC: sirf: Add sirf audio hub driver for sharing same register address space

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1717 bytes --]

On Thu, Mar 13, 2014 at 06:38:30PM +0800, RongJun Ying wrote:
> @@ -283,9 +jc83,10 @@ static int sirf_audio_codec_trigger(struct snd_pcm_substream *substream,
>  		int cmd,
>  		struct snd_soc_dai *dai)
>  {
> -	int playback = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
>  	struct snd_soc_codec *codec = dai->codec;
>  	u32 val = 0;
> +	if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
> +		return 0;
>  
>  	/*
>  	 * This is a workaround, When stop playback,
> @@ -299,15 +300,13 @@ static int sirf_audio_codec_trigger(struct snd_pcm_substream *substream,
>  	case SNDRV_PCM_TRIGGER_START:
>  	case SNDRV_PCM_TRIGGER_RESUME:
>  	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> -		if (playback)
> -			val = IC_HSLEN | IC_HSREN;
> +		val = IC_HSLEN | IC_HSREN;
>  		break;
>  	default:
>  		return -EINVAL;
>  	}
>  
> -	if (playback)
> -		snd_soc_update_bits(codec, AUDIO_IC_CODEC_CTRL0,
> +	snd_soc_update_bits(codec, AUDIO_IC_CODEC_CTRL0,
>  			IC_HSLEN | IC_HSREN, val);
>  	return 0;
>  }

All the changes in this file appear to be unrelated stylistic changes.
They're fine but you should do such things as separate commits, this
makes things harder to review.

> +/* This regmap is shared all child audio drivers */
> +struct regmap *regmap;

Why is this a global variable?  I'd expect it to be in the driver data
for the device.  This is also a global symbol so would at the very least
would need better namespacing.

> +
> +static const struct regmap_config sirf_audio_regmap_config = 
> +	.reg_bits =
> +	.val_bits = 32,
> +	.cache_type = REGCACHE_NONE,

None is the default.

Looking at this code I'm not clear how the function devices get
instantiated?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] Documentation: Describe the SiRF audio hub Device Tree bindings

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 664 bytes --]

On Thu, Mar 13, 2014 at 06:38:31PM +0800, RongJun Ying wrote:

> +	audiocodec: audiocodec {
> +		compatible = "sirf,atlas6-audio-codec";
> +		interrupts = <35>;
> +		clocks = <&clks 27>;
> +	};
> +
> +	audioport: audioport {
> +		compatible = "sirf,audio-port";

> +	i2s: i2s {
> +		compatible = "sirf,prima2-i2s";

Given that this is all one single IP block which doesn't really exist
independently at the hardware level it shouldn't have separate nodes or
compatible strings for the functions at the DT level - that's all to do
with how Linux structures things, not about the hardware.  As ever DT
should represent the hardware, not Linux's abstractions for it.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]