[PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[Tushar Behera]

Added machine driver to instantiate I2S based sound card on Snow
board. It has MAX98095 audio codec on board.

There are some other variants for Snow board which have MAX98090
audio codec. Hence support for MAX98090 is also added to this
driver.

Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
Changes for V2:
Addressed various comments from Mark Brown.
1. Moved DAIFMT flags to dai_link. Removed snd_soc_dai_set_fmt calls.
2. Removed BCLK settings.
3. Moved sysclk settings to late_probe.
4. Removed now empty snow_hw_params.
5. Removed snow_init
6. Used devm_snd_soc_register_card. Also removed empty remove function.
7. Removed non-DT handling as the driver is DT only.

Additional nits:
1. Renamed snow_driver_probe to snow_probe
2. Removed unused pm_ops.
3. Updated clock settings for I2S block (Earlier it was I2S_CDCLK,
   whereas it should be I2S_RCLKSRC_0).
4. Removed 'card-name' DT property as it was not used in earlier
   sound-card driver DT bindings. Will be added later.

 Documentation/devicetree/bindings/sound/snow.txt |   17 +++
 sound/soc/samsung/Kconfig                        |   10 ++
 sound/soc/samsung/Makefile                       |    2 +
 sound/soc/samsung/snow.c                         |  129 ++++++++++++++++++++++
 4 files changed, 158 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/snow.txt
 create mode 100644 sound/soc/samsung/snow.c

diff --git a/Documentation/devicetree/bindings/sound/snow.txt b/Documentation/devicetree/bindings/sound/snow.txt
new file mode 100644
index 0000000..6f3fd02
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/snow.txt
@@ -0,0 +1,17 @@
+Audio Binding for Snow boards
+
+Required properties:
+- compatible : Can be one of the following,
+			"google,snow-audio-max98090" or
+			"google,snow-audio-max98095"
+- samsung,i2s-controller: The phandle of the Samsung I2S0 controller
+- samsung,audio-codec: The phandle of the audio codec
+
+Example:
+
+sound {
+		compatible = "google,snow-audio-max98095";
+
+		samsung,i2s-controller = <&i2s0>;
+		samsung,audio-codec = <&max98095>;
+};
diff --git a/sound/soc/samsung/Kconfig b/sound/soc/samsung/Kconfig
index f2e2891..50aa289 100644
--- a/sound/soc/samsung/Kconfig
+++ b/sound/soc/samsung/Kconfig
@@ -231,3 +231,13 @@ config SND_SOC_LITTLEMILL
 	select SND_SAMSUNG_I2S
 	select MFD_WM8994
 	select SND_SOC_WM8994
+
+config SND_SOC_SNOW
+	tristate "Audio support for Google Snow boards"
+	depends on SND_SOC_SAMSUNG
+	select SND_SOC_MAX98090
+	select SND_SOC_MAX98095
+	select SND_SAMSUNG_I2S
+	help
+	  Say Y if you want to add audio support for various Snow
+	  boards based on Exynos5 series of SoCs.
diff --git a/sound/soc/samsung/Makefile b/sound/soc/samsung/Makefile
index 86715d8..6d0212b 100644
--- a/sound/soc/samsung/Makefile
+++ b/sound/soc/samsung/Makefile
@@ -34,6 +34,7 @@ snd-soc-h1940-uda1380-objs := h1940_uda1380.o
 snd-soc-rx1950-uda1380-objs := rx1950_uda1380.o
 snd-soc-smdk-wm8580-objs := smdk_wm8580.o
 snd-soc-smdk-wm8994-objs := smdk_wm8994.o
+snd-soc-snow-objs := snow.o
 snd-soc-smdk-wm9713-objs := smdk_wm9713.o
 snd-soc-s3c64xx-smartq-wm8987-objs := smartq_wm8987.o
 snd-soc-goni-wm8994-objs := goni_wm8994.o
@@ -58,6 +59,7 @@ obj-$(CONFIG_SND_SOC_SAMSUNG_H1940_UDA1380) += snd-soc-h1940-uda1380.o
 obj-$(CONFIG_SND_SOC_SAMSUNG_RX1950_UDA1380) += snd-soc-rx1950-uda1380.o
 obj-$(CONFIG_SND_SOC_SAMSUNG_SMDK_WM8580) += snd-soc-smdk-wm8580.o
 obj-$(CONFIG_SND_SOC_SAMSUNG_SMDK_WM8994) += snd-soc-smdk-wm8994.o
+obj-$(CONFIG_SND_SOC_SNOW) += snd-soc-snow.o
 obj-$(CONFIG_SND_SOC_SAMSUNG_SMDK_WM9713) += snd-soc-smdk-wm9713.o
 obj-$(CONFIG_SND_SOC_SMARTQ) += snd-soc-s3c64xx-smartq-wm8987.o
 obj-$(CONFIG_SND_SOC_SAMSUNG_SMDK_SPDIF) += snd-soc-smdk-spdif.o
diff --git a/sound/soc/samsung/snow.c b/sound/soc/samsung/snow.c
new file mode 100644
index 0000000..e1a4b45
--- /dev/null
+++ b/sound/soc/samsung/snow.c
@@ -0,0 +1,129 @@
+/*
+ * ASoC machine driver for Snow boards
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+
+#include <sound/soc.h>
+
+#include "i2s.h"
+
+#define FIN_PLL_RATE		24000000
+
+static struct snd_soc_dai_link snow_dai[] = {
+	{ /* Playback DAI i/f */
+		.name = "Playback",
+		.stream_name = "Playback",
+		.codec_dai_name = "HiFi",
+		.dai_fmt = SND_SOC_DAIFMT_I2S |
+				SND_SOC_DAIFMT_NB_NF |
+				SND_SOC_DAIFMT_CBS_CFS,
+	}, { /* Capture DAI i/f */
+		.name = "Capture",
+		.stream_name = "Capture",
+		.codec_dai_name = "HiFi",
+		.dai_fmt = SND_SOC_DAIFMT_I2S |
+				SND_SOC_DAIFMT_NB_NF |
+				SND_SOC_DAIFMT_CBS_CFS,
+	},
+};
+
+static int snow_late_probe(struct snd_soc_card *card)
+{
+	struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai;
+	struct snd_soc_dai *cpu_dai = card->rtd[0].cpu_dai;
+	int ret;
+
+	/* Set the MCLK rate for the codec */
+	ret = snd_soc_dai_set_sysclk(codec_dai, 0,
+					FIN_PLL_RATE, SND_SOC_CLOCK_IN);
+	if (ret < 0)
+		return ret;
+
+	/* Select I2S Bus clock to set RCLK and BCLK */
+	ret = snd_soc_dai_set_sysclk(cpu_dai, SAMSUNG_I2S_RCLKSRC_0,
+					0, SND_SOC_CLOCK_IN);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static struct snd_soc_card snow_snd = {
+	.name = "Snow-I2S",
+	.dai_link = snow_dai,
+	.num_links = ARRAY_SIZE(snow_dai),
+
+	.late_probe = snow_late_probe,
+};
+
+static int snow_probe(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = &snow_snd;
+	struct device_node *i2s_node, *codec_node;
+	int i, ret;
+
+	i2s_node = of_parse_phandle(pdev->dev.of_node,
+				    "samsung,i2s-controller", 0);
+	if (!i2s_node) {
+		dev_err(&pdev->dev,
+			"Property 'i2s-controller' missing or invalid\n");
+		return -EINVAL;
+	}
+
+	codec_node = of_parse_phandle(pdev->dev.of_node,
+				      "samsung,audio-codec", 0);
+	if (!codec_node) {
+		dev_err(&pdev->dev,
+			"Property 'audio-codec' missing or invalid\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(snow_dai); i++) {
+		snow_dai[i].codec_of_node = codec_node;
+		snow_dai[i].cpu_of_node = i2s_node;
+		snow_dai[i].platform_of_node = i2s_node;
+	}
+
+	card->dev = &pdev->dev;
+
+	ret = devm_snd_soc_register_card(&pdev->dev, card);
+	if (ret) {
+		dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static const struct of_device_id snow_of_match[] = {
+	{ .compatible = "google,snow-audio-max98090", },
+	{ .compatible = "google,snow-audio-max98095", },
+	{},
+};
+
+static struct platform_driver snow_driver = {
+	.driver = {
+		.name = "snow-audio",
+		.owner = THIS_MODULE,
+		.of_match_table = snow_of_match,
+	},
+	.probe = snow_probe,
+};
+
+module_platform_driver(snow_driver);
+
+MODULE_DESCRIPTION("ALSA SoC Audio machine driver for Snow");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

[PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[Tushar Behera]

Added machine driver to instantiate I2S based sound card on Snow
board. It has MAX98095 audio codec on board.

There are some other variants for Snow board which have MAX98090
audio codec. Hence support for MAX98090 is also added to this
driver.

Signed-off-by: Tushar Behera <tushar.behera-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
Changes for V2:
Addressed various comments from Mark Brown.
1. Moved DAIFMT flags to dai_link. Removed snd_soc_dai_set_fmt calls.
2. Removed BCLK settings.
3. Moved sysclk settings to late_probe.
4. Removed now empty snow_hw_params.
5. Removed snow_init
6. Used devm_snd_soc_register_card. Also removed empty remove function.
7. Removed non-DT handling as the driver is DT only.

Additional nits:
1. Renamed snow_driver_probe to snow_probe
2. Removed unused pm_ops.
3. Updated clock settings for I2S block (Earlier it was I2S_CDCLK,
   whereas it should be I2S_RCLKSRC_0).
4. Removed 'card-name' DT property as it was not used in earlier
   sound-card driver DT bindings. Will be added later.

 Documentation/devicetree/bindings/sound/snow.txt |   17 +++
 sound/soc/samsung/Kconfig                        |   10 ++
 sound/soc/samsung/Makefile                       |    2 +
 sound/soc/samsung/snow.c                         |  129 ++++++++++++++++++++++
 4 files changed, 158 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/snow.txt
 create mode 100644 sound/soc/samsung/snow.c

diff --git a/Documentation/devicetree/bindings/sound/snow.txt b/Documentation/devicetree/bindings/sound/snow.txt
new file mode 100644
index 0000000..6f3fd02
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/snow.txt
@@ -0,0 +1,17 @@
+Audio Binding for Snow boards
+
+Required properties:
+- compatible : Can be one of the following,
+			"google,snow-audio-max98090" or
+			"google,snow-audio-max98095"
+- samsung,i2s-controller: The phandle of the Samsung I2S0 controller
+- samsung,audio-codec: The phandle of the audio codec
+
+Example:
+
+sound {
+		compatible = "google,snow-audio-max98095";
+
+		samsung,i2s-controller = <&i2s0>;
+		samsung,audio-codec = <&max98095>;
+};
diff --git a/sound/soc/samsung/Kconfig b/sound/soc/samsung/Kconfig
index f2e2891..50aa289 100644
--- a/sound/soc/samsung/Kconfig
+++ b/sound/soc/samsung/Kconfig
@@ -231,3 +231,13 @@ config SND_SOC_LITTLEMILL
 	select SND_SAMSUNG_I2S
 	select MFD_WM8994
 	select SND_SOC_WM8994
+
+config SND_SOC_SNOW
+	tristate "Audio support for Google Snow boards"
+	depends on SND_SOC_SAMSUNG
+	select SND_SOC_MAX98090
+	select SND_SOC_MAX98095
+	select SND_SAMSUNG_I2S
+	help
+	  Say Y if you want to add audio support for various Snow
+	  boards based on Exynos5 series of SoCs.
diff --git a/sound/soc/samsung/Makefile b/sound/soc/samsung/Makefile
index 86715d8..6d0212b 100644
--- a/sound/soc/samsung/Makefile
+++ b/sound/soc/samsung/Makefile
@@ -34,6 +34,7 @@ snd-soc-h1940-uda1380-objs := h1940_uda1380.o
 snd-soc-rx1950-uda1380-objs := rx1950_uda1380.o
 snd-soc-smdk-wm8580-objs := smdk_wm8580.o
 snd-soc-smdk-wm8994-objs := smdk_wm8994.o
+snd-soc-snow-objs := snow.o
 snd-soc-smdk-wm9713-objs := smdk_wm9713.o
 snd-soc-s3c64xx-smartq-wm8987-objs := smartq_wm8987.o
 snd-soc-goni-wm8994-objs := goni_wm8994.o
@@ -58,6 +59,7 @@ obj-$(CONFIG_SND_SOC_SAMSUNG_H1940_UDA1380) += snd-soc-h1940-uda1380.o
 obj-$(CONFIG_SND_SOC_SAMSUNG_RX1950_UDA1380) += snd-soc-rx1950-uda1380.o
 obj-$(CONFIG_SND_SOC_SAMSUNG_SMDK_WM8580) += snd-soc-smdk-wm8580.o
 obj-$(CONFIG_SND_SOC_SAMSUNG_SMDK_WM8994) += snd-soc-smdk-wm8994.o
+obj-$(CONFIG_SND_SOC_SNOW) += snd-soc-snow.o
 obj-$(CONFIG_SND_SOC_SAMSUNG_SMDK_WM9713) += snd-soc-smdk-wm9713.o
 obj-$(CONFIG_SND_SOC_SMARTQ) += snd-soc-s3c64xx-smartq-wm8987.o
 obj-$(CONFIG_SND_SOC_SAMSUNG_SMDK_SPDIF) += snd-soc-smdk-spdif.o
diff --git a/sound/soc/samsung/snow.c b/sound/soc/samsung/snow.c
new file mode 100644
index 0000000..e1a4b45
--- /dev/null
+++ b/sound/soc/samsung/snow.c
@@ -0,0 +1,129 @@
+/*
+ * ASoC machine driver for Snow boards
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+
+#include <sound/soc.h>
+
+#include "i2s.h"
+
+#define FIN_PLL_RATE		24000000
+
+static struct snd_soc_dai_link snow_dai[] = {
+	{ /* Playback DAI i/f */
+		.name = "Playback",
+		.stream_name = "Playback",
+		.codec_dai_name = "HiFi",
+		.dai_fmt = SND_SOC_DAIFMT_I2S |
+				SND_SOC_DAIFMT_NB_NF |
+				SND_SOC_DAIFMT_CBS_CFS,
+	}, { /* Capture DAI i/f */
+		.name = "Capture",
+		.stream_name = "Capture",
+		.codec_dai_name = "HiFi",
+		.dai_fmt = SND_SOC_DAIFMT_I2S |
+				SND_SOC_DAIFMT_NB_NF |
+				SND_SOC_DAIFMT_CBS_CFS,
+	},
+};
+
+static int snow_late_probe(struct snd_soc_card *card)
+{
+	struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai;
+	struct snd_soc_dai *cpu_dai = card->rtd[0].cpu_dai;
+	int ret;
+
+	/* Set the MCLK rate for the codec */
+	ret = snd_soc_dai_set_sysclk(codec_dai, 0,
+					FIN_PLL_RATE, SND_SOC_CLOCK_IN);
+	if (ret < 0)
+		return ret;
+
+	/* Select I2S Bus clock to set RCLK and BCLK */
+	ret = snd_soc_dai_set_sysclk(cpu_dai, SAMSUNG_I2S_RCLKSRC_0,
+					0, SND_SOC_CLOCK_IN);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static struct snd_soc_card snow_snd = {
+	.name = "Snow-I2S",
+	.dai_link = snow_dai,
+	.num_links = ARRAY_SIZE(snow_dai),
+
+	.late_probe = snow_late_probe,
+};
+
+static int snow_probe(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = &snow_snd;
+	struct device_node *i2s_node, *codec_node;
+	int i, ret;
+
+	i2s_node = of_parse_phandle(pdev->dev.of_node,
+				    "samsung,i2s-controller", 0);
+	if (!i2s_node) {
+		dev_err(&pdev->dev,
+			"Property 'i2s-controller' missing or invalid\n");
+		return -EINVAL;
+	}
+
+	codec_node = of_parse_phandle(pdev->dev.of_node,
+				      "samsung,audio-codec", 0);
+	if (!codec_node) {
+		dev_err(&pdev->dev,
+			"Property 'audio-codec' missing or invalid\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(snow_dai); i++) {
+		snow_dai[i].codec_of_node = codec_node;
+		snow_dai[i].cpu_of_node = i2s_node;
+		snow_dai[i].platform_of_node = i2s_node;
+	}
+
+	card->dev = &pdev->dev;
+
+	ret = devm_snd_soc_register_card(&pdev->dev, card);
+	if (ret) {
+		dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static const struct of_device_id snow_of_match[] = {
+	{ .compatible = "google,snow-audio-max98090", },
+	{ .compatible = "google,snow-audio-max98095", },
+	{},
+};
+
+static struct platform_driver snow_driver = {
+	.driver = {
+		.name = "snow-audio",
+		.owner = THIS_MODULE,
+		.of_match_table = snow_of_match,
+	},
+	.probe = snow_probe,
+};
+
+module_platform_driver(snow_driver);
+
+MODULE_DESCRIPTION("ALSA SoC Audio machine driver for Snow");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1247 bytes --]

On Wed, Apr 23, 2014 at 02:31:45PM +0530, Tushar Behera wrote:

> +Required properties:
> +- compatible : Can be one of the following,
> +			"google,snow-audio-max98090" or
> +			"google,snow-audio-max98095"
> +- samsung,i2s-controller: The phandle of the Samsung I2S0 controller

This should be any I2S controller, not just I2S0?

> +- samsung,audio-codec: The phandle of the audio codec

This binding only has one I2S controller and CODEC.  However...

> +static struct snd_soc_dai_link snow_dai[] = {
> +	{ /* Playback DAI i/f */
> +		.name = "Playback",
> +		.stream_name = "Playback",
> +		.codec_dai_name = "HiFi",
> +		.dai_fmt = SND_SOC_DAIFMT_I2S |
> +				SND_SOC_DAIFMT_NB_NF |
> +				SND_SOC_DAIFMT_CBS_CFS,
> +	}, { /* Capture DAI i/f */
> +		.name = "Capture",
> +		.stream_name = "Capture",
> +		.codec_dai_name = "HiFi",
> +		.dai_fmt = SND_SOC_DAIFMT_I2S |
> +				SND_SOC_DAIFMT_NB_NF |
> +				SND_SOC_DAIFMT_CBS_CFS,
> +	},
> +};

...for some reason we have separate capture and playback DAI links
defined.  Why is that?  Also, why is the secondary I2S playback stream
not supported (this may be a reason to restrict to only the one I2S
interface)?

Please also use subject lines consistent with the subsystem - NO NEED TO
SHOUT.

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

Re: [PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[Tushar Behera]

On 04/24/2014 07:09 PM, Mark Brown wrote:
> On Wed, Apr 23, 2014 at 02:31:45PM +0530, Tushar Behera wrote:
> 
>> +Required properties:
>> +- compatible : Can be one of the following,
>> +			"google,snow-audio-max98090" or
>> +			"google,snow-audio-max98095"
>> +- samsung,i2s-controller: The phandle of the Samsung I2S0 controller
> 
> This should be any I2S controller, not just I2S0?
> 

Yes, right. It can be any I2S controller. Just that, right now it is
wired to I2S0.

>> +- samsung,audio-codec: The phandle of the audio codec
> 
> This binding only has one I2S controller and CODEC.  However...
> 
>> +static struct snd_soc_dai_link snow_dai[] = {
>> +	{ /* Playback DAI i/f */
>> +		.name = "Playback",
>> +		.stream_name = "Playback",
>> +		.codec_dai_name = "HiFi",
>> +		.dai_fmt = SND_SOC_DAIFMT_I2S |
>> +				SND_SOC_DAIFMT_NB_NF |
>> +				SND_SOC_DAIFMT_CBS_CFS,
>> +	}, { /* Capture DAI i/f */
>> +		.name = "Capture",
>> +		.stream_name = "Capture",
>> +		.codec_dai_name = "HiFi",
>> +		.dai_fmt = SND_SOC_DAIFMT_I2S |
>> +				SND_SOC_DAIFMT_NB_NF |
>> +				SND_SOC_DAIFMT_CBS_CFS,
>> +	},
>> +};
> 
> ...for some reason we have separate capture and playback DAI links

That was lack of understanding from my side. I was of the impression
that the back-end uses different DAI interfaces for aplay and arecord,
which certainly is not the case. I will remove the 'Capture' dai and
make 'Playback' dai as the primary DAI.

> defined.  Why is that?  Also, why is the secondary I2S playback stream
> not supported (this may be a reason to restrict to only the one I2S
> interface)?

AFAICS, I2S driver doesn't support secondary DAI with DT (dai type is
always TYPE_PRI in case of DT). Hence I could not find a setup to test
secondary dai with this board.

> 
> Please also use subject lines consistent with the subsystem - NO NEED TO
> SHOUT.
> 

Noted, will take care.

Thanks.
-- 
Tushar Behera

Re: [PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 623 bytes --]

On Fri, Apr 25, 2014 at 09:46:11AM +0530, Tushar Behera wrote:
> On 04/24/2014 07:09 PM, Mark Brown wrote:

> > defined.  Why is that?  Also, why is the secondary I2S playback stream
> > not supported (this may be a reason to restrict to only the one I2S
> > interface)?

> AFAICS, I2S driver doesn't support secondary DAI with DT (dai type is
> always TYPE_PRI in case of DT). Hence I could not find a setup to test
> secondary dai with this board.

OK, that should be fixed at some point.  I will hopefully be able to
look at this myself on the Arndale Octa though (or Chromebook or Arndale
once XCLKOUT is implemented).

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

Re: [PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[Tushar Behera]

On 04/30/2014 03:59 AM, Mark Brown wrote:
> On Fri, Apr 25, 2014 at 09:46:11AM +0530, Tushar Behera wrote:
>> On 04/24/2014 07:09 PM, Mark Brown wrote:
> 
>>> defined.  Why is that?  Also, why is the secondary I2S playback stream
>>> not supported (this may be a reason to restrict to only the one I2S
>>> interface)?
> 
>> AFAICS, I2S driver doesn't support secondary DAI with DT (dai type is
>> always TYPE_PRI in case of DT). Hence I could not find a setup to test
>> secondary dai with this board.
> 
> OK, that should be fixed at some point.  I will hopefully be able to
> look at this myself on the Arndale Octa though (or Chromebook or Arndale
> once XCLKOUT is implemented).
> 

XCLKOUT mux register (0x10040a00) is not part of core clock SFR range,
rather it is part of pmu-system-controller node.

One option would be to add a clock provider for XCLKOUT. That would
require me to extend current clock driver to get the
pmu-system-controller phandle which in turn is dependent on the patch to
early initialize syscon driver.[1]

Will it be okay to configure XCLKOUT in snow sound-card driver by taking
the phandle of pmu-system-controller from device tree? The
implementation is provided below.

[1] article.gmane.org/gmane.linux.kernel/1679446

diff --git a/sound/soc/samsung/snow.c b/sound/soc/samsung/snow.c
index 0fa89a4..d7fe7a0 100644
--- a/sound/soc/samsung/snow.c
+++ b/sound/soc/samsung/snow.c
@@ -15,6 +15,7 @@
 #include <linux/platform_device.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/mfd/syscon.h>

 #include <sound/soc.h>

@@ -22,6 +23,10 @@

 #define FIN_PLL_RATE           24000000

+#define XCLKOUT_OFFSET                 0x0a00
+#define XCLKOUT_PARENT_MASK            0x1f00
+#define XCLKOUT_PARENT_XXTI            0x1000
+
 static struct snd_soc_dai_link snow_dai[] = {
        {
                .name = "Primary",
@@ -66,6 +71,7 @@ static int snow_probe(struct platform_device *pdev)
 {
        struct snd_soc_card *card = &snow_snd;
        struct device_node *i2s_node, *codec_node;
+       struct regmap *pmureg;
        int i, ret;

        i2s_node = of_parse_phandle(pdev->dev.of_node,
@@ -84,6 +90,19 @@ static int snow_probe(struct platform_device *pdev)
                return -EINVAL;
        }

+       /*
+        * On Snow board, SoC XCLKOUT line is connected to codec MCLK.
+        * One option would be to add another clock provider to configure
+        * XCLKOUT mux register. Else we can get pmu-syscon phandle from
+        * device tree and update the register here.
+        */
+       pmureg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+                       "samsung,pmu-syscon");
+       if (!IS_ERR(pmureg)) {
+               regmap_update_bits(pmureg, XCLKOUT_OFFSET,
+                               XCLKOUT_PARENT_MASK, XCLKOUT_PARENT_XXTI);
+       }
+
        for (i = 0; i < ARRAY_SIZE(snow_dai); i++) {
                snow_dai[i].codec_of_node = codec_node;
                snow_dai[i].cpu_of_node = i2s_node;

-- 
Tushar Behera

Re: [PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

On Wed, Apr 30, 2014 at 05:30:39PM +0530, Tushar Behera wrote:

> XCLKOUT mux register (0x10040a00) is not part of core clock SFR range,
> rather it is part of pmu-system-controller node.

> One option would be to add a clock provider for XCLKOUT. That would
> require me to extend current clock driver to get the
> pmu-system-controller phandle which in turn is dependent on the patch to
> early initialize syscon driver.[1]

> Will it be okay to configure XCLKOUT in snow sound-card driver by taking
> the phandle of pmu-system-controller from device tree? The
> implementation is provided below.

I can't think of any reason why we'd not want to have this visible as a
clock?  Ideally we don't want to have any ASoC specific clock APIs when
we're done, the reason we do at the minute is that the clock API isn't
available as standard.

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

Re: [PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[Tushar Behera]

On 04/30/2014 11:33 PM, Mark Brown wrote:
> On Wed, Apr 30, 2014 at 05:30:39PM +0530, Tushar Behera wrote:
> 
>> XCLKOUT mux register (0x10040a00) is not part of core clock SFR range,
>> rather it is part of pmu-system-controller node.
> 
>> One option would be to add a clock provider for XCLKOUT. That would
>> require me to extend current clock driver to get the
>> pmu-system-controller phandle which in turn is dependent on the patch to
>> early initialize syscon driver.[1]
> 
>> Will it be okay to configure XCLKOUT in snow sound-card driver by taking
>> the phandle of pmu-system-controller from device tree? The
>> implementation is provided below.
> 
> I can't think of any reason why we'd not want to have this visible as a
> clock?  Ideally we don't want to have any ASoC specific clock APIs when
> we're done, the reason we do at the minute is that the clock API isn't
> available as standard.
> 

Okay, I will extend the existing clock driver to support XCLKOUT.

Of the many parents of XCLKOUT, we need to set XXTI clock as the parent.
Is it okay if we pass two clocks "mclk" (XCLKOUT) and "mclk_parent"
(XXTI) to sound-card driver via DT and do the necessary reparenting
during the sound-card driver probe call?

Else, we can push that change to bootloader (to set the XCLKOUT mux
register) and only enable/disable the clock in sound-card driver.

Let me know your opinion on this.

-- 
Tushar Behera

Re: [PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 979 bytes --]

On Thu, May 01, 2014 at 04:59:08PM +0530, Tushar Behera wrote:

> Okay, I will extend the existing clock driver to support XCLKOUT.

It may make more sense to add another clock driver for this clock
depending on how things are done, I don't know.

> Of the many parents of XCLKOUT, we need to set XXTI clock as the parent.
> Is it okay if we pass two clocks "mclk" (XCLKOUT) and "mclk_parent"
> (XXTI) to sound-card driver via DT and do the necessary reparenting
> during the sound-card driver probe call?

No, that's not OK at all, it won't allow for configuration of the
system.  This is what I was talking about when I was talking the clock
framework extensions to allow the clock tree to be configured using DT,
that would allow the settings to be put in DT.

> Else, we can push that change to bootloader (to set the XCLKOUT mux
> register) and only enable/disable the clock in sound-card driver.

That's not going to work given that the existing bootloaders don't do
this.

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

Re: [PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[Doug Anderson]

Hi,

On Thu, May 1, 2014 at 7:15 AM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, May 01, 2014 at 04:59:08PM +0530, Tushar Behera wrote:
>
>> Okay, I will extend the existing clock driver to support XCLKOUT.
>
> It may make more sense to add another clock driver for this clock
> depending on how things are done, I don't know.
>
>> Of the many parents of XCLKOUT, we need to set XXTI clock as the parent.
>> Is it okay if we pass two clocks "mclk" (XCLKOUT) and "mclk_parent"
>> (XXTI) to sound-card driver via DT and do the necessary reparenting
>> during the sound-card driver probe call?
>
> No, that's not OK at all, it won't allow for configuration of the
> system.  This is what I was talking about when I was talking the clock
> framework extensions to allow the clock tree to be configured using DT,
> that would allow the settings to be put in DT.
>
>> Else, we can push that change to bootloader (to set the XCLKOUT mux
>> register) and only enable/disable the clock in sound-card driver.
>
> That's not going to work given that the existing bootloaders don't do
> this.

This is exactly the thing (expected clock parenting) we agreed could
be put in the device tree I think.  ...but I don't know that anyone
proposed exactly how that would work.

NOTE: in existing ChromeOS this type of thing is done a tiny amount in
<https://chromium.googlesource.com/chromiumos/third_party/kernel-next/+/chromeos-3.8/arch/arm/mach-exynos/mach-exynos5-dt.c>.
 See apply_clock_muxing().

-Doug

Re: [PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 445 bytes --]

On Thu, May 01, 2014 at 08:54:14AM -0700, Doug Anderson wrote:

> This is exactly the thing (expected clock parenting) we agreed could
> be put in the device tree I think.  ...but I don't know that anyone
> proposed exactly how that would work.

There's patches been posted by (IIRC) Sylvester Nawrocki for this which
I think Mike was basically happy with - I don't immediately see them in
-next though, but I may be looking in the wrong place.

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

Re: [PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[Tushar Behera]

On 05/01/2014 10:10 PM, Mark Brown wrote:
> On Thu, May 01, 2014 at 08:54:14AM -0700, Doug Anderson wrote:
> 
>> This is exactly the thing (expected clock parenting) we agreed could
>> be put in the device tree I think.  ...but I don't know that anyone
>> proposed exactly how that would work.
> 
> There's patches been posted by (IIRC) Sylvester Nawrocki for this which
> I think Mike was basically happy with - I don't immediately see them in
> -next though, but I may be looking in the wrong place.
> 

Right, there are some patches from Sylwester regarding this. I dont'
think the patches are accepted yet as Rob has some reservations against
the clock parents.

https://lkml.org/lkml/2014/4/9/173

-- 
Tushar Behera

Re: [PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 656 bytes --]

On Fri, May 02, 2014 at 10:26:08AM +0530, Tushar Behera wrote:
> On 05/01/2014 10:10 PM, Mark Brown wrote:

> > There's patches been posted by (IIRC) Sylvester Nawrocki for this which
> > I think Mike was basically happy with - I don't immediately see them in
> > -next though, but I may be looking in the wrong place.

> Right, there are some patches from Sylwester regarding this. I dont'
> think the patches are accepted yet as Rob has some reservations against
> the clock parents.

> https://lkml.org/lkml/2014/4/9/173

Well, OK - that's the bit to fix.  Speaking to Mike the other day he
said there was also another competing implementation of this.

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

Re: [PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[Tushar Behera]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/02/2014 10:26 PM, Mark Brown wrote:
> On Fri, May 02, 2014 at 10:26:08AM +0530, Tushar Behera wrote:
>> On 05/01/2014 10:10 PM, Mark Brown wrote:
> 
>>> There's patches been posted by (IIRC) Sylvester Nawrocki for
>>> this which I think Mike was basically happy with - I don't
>>> immediately see them in -next though, but I may be looking in
>>> the wrong place.
> 
>> Right, there are some patches from Sylwester regarding this. I
>> dont' think the patches are accepted yet as Rob has some
>> reservations against the clock parents.
> 
>> https://lkml.org/lkml/2014/4/9/173
> 
> Well, OK - that's the bit to fix.  Speaking to Mike the other day
> he said there was also another competing implementation of this.
> 

Instead of setting the parent clock, setting the desired rate on
XCLKOUT works for audio playback.

Will it be okay to do a 'clk_set_rate(mclk, 24000000)' in sound card
driver or should we wait for Sylwester's patch to get merged?

- -- 
Tushar Behera
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTefPOAAoJELqclMPPkq4N6UIH/i5W7KYjyKTQtX+AGdUmJwIx
3TTwyWVz5YQ6gFpLHwwy5AwIkj4mygox2e7l0qWU6yOXCMceaXrvzxOK+B/LRXlp
+s8xjd+C+Idxhs0T6V4GkJRA9jtari3gcIGfz4tFtWY5O0l0eNTUduILhAvmE/Sw
c4m8aOwK33P0kvkTIp/ShSujYa8v82XosMRuf4P0iEz9I3qw3syJkWWvqNZFrnPs
B7i+9ZiZkyttsUVBOAWMBfOdLP5sMtxKDdk1GTkN1plxAE8lKdNVATR5htQL4bxo
ZFcWJxE14pWQ770gEJVXkDT9o8dhmVNrDVp4eGe3m+43gN+Bssh+9kgdxb3vmIY=
=gXaJ
-----END PGP SIGNATURE-----

Re: [PATCH V2] ASoC: SAMSUNG: Add sound card driver for Snow board

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]

On Mon, May 19, 2014 at 05:36:38PM +0530, Tushar Behera wrote:

> Instead of setting the parent clock, setting the desired rate on
> XCLKOUT works for audio playback.

> Will it be okay to do a 'clk_set_rate(mclk, 24000000)' in sound card
> driver or should we wait for Sylwester's patch to get merged?

Yes, that's fine - going further back into the clock tree isn't.

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