[PATCH 0/8] ASoC: s3c24xx_uda134x card fixes and cleanups

[PATCH 0/8] ASoC: s3c24xx_uda134x card fixes and cleanups

[Sylwester Nawrocki]

This series might have a bit of an archaeological aspect, nevertheless
my goal was to have sound working on one of the older S3C24XX SoC
based boards for testing any patches touching the older Samsung
SoC IP block related code.

Together with patches:
  ASoC: samsung: Fix clock handling in S3C24XX_UDA134X card
  ARM: S3C24XX: Add dma_mask assignments for DMA devices
  ARM: S3C24XX: Add missing DMA device for Mini2440 board

this series fixes sound on Micro2440SDK/mini2440 board broken since
the 3.12 kernel release.

I'm not sure how it worked before but currently the uda134x codec
always fails to probe due to uninitialized platform_data, which
is supposed to be set by the ASoC machine driver.

This series addresses the issue by moving all required codec
platform_data to the board file and exposing optional GPIO bitbang
L3 bus ops which are selected by the codec.

ASoC dependency patches:
  ASoC: samsung: Fix clock handling in S3C24XX_UDA134X card

The patch
"ARM: S3C24XX: Specify audio codec platform_data for mini2440 board"
is for arm-soc but it would be good to merge whole series through one
tree to avoid compilation breaks due to unmet dependencies.

Sylwester Nawrocki (8):
  ASoC: L3 bus: Add default gpio ops
  ARM: S3C24XX: Specify audio codec platform_data for mini2440 board
  ASoC: uda134x: Optionally initialize L3 ops to default GPIO ops
  ASoC: s3c24xx_uda134x: Remove unused power() callback
  ASoC: s3c24xx_uda134x: Drop initialization of codec's platform data
  ASoC: samsung: Convert s3c24xx_uda134x to use
    devm_snd_soc_register_card()
  ASoC: samsung: s3c24xx_uda134x: debug/error trace cleanup
  ASoC: s3c244_uda134x: Allocate private data dynamically

 arch/arm/mach-s3c24xx/mach-mini2440.c |  19 ++++
 include/sound/l3.h                    |  15 ++-
 include/sound/s3c24xx_uda134x.h       |   1 -
 sound/soc/codecs/l3.c                 |  71 ++++++++++--
 sound/soc/codecs/uda134x.c            |   7 ++
 sound/soc/samsung/s3c24xx_uda134x.c   | 205 ++++++++++------------------------
 6 files changed, 158 insertions(+), 160 deletions(-)

--
1.9.1

[PATCH 1/8] ASoC: L3 bus: Add default gpio ops

[Sylwester Nawrocki]

This adds aptional GPIO bit-bang based callback implementations
for setting CLK, DATA and MODE L3 bus lines.  It is added here
to avoid possible duplicate implementations across users of
the bus.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 include/sound/l3.h    | 15 ++++++++---
 sound/soc/codecs/l3.c | 71 ++++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 71 insertions(+), 15 deletions(-)

diff --git a/include/sound/l3.h b/include/sound/l3.h
index 423a08f..1471da2 100644
--- a/include/sound/l3.h
+++ b/include/sound/l3.h
@@ -2,9 +2,15 @@
 #define _L3_H_ 1
 
 struct l3_pins {
-	void (*setdat)(int);
-	void (*setclk)(int);
-	void (*setmode)(int);
+	void (*setdat)(struct l3_pins *, int);
+	void (*setclk)(struct l3_pins *, int);
+	void (*setmode)(struct l3_pins *, int);
+
+	int gpio_data;
+	int gpio_clk;
+	int gpio_mode;
+	int use_gpios;
+
 	int data_hold;
 	int data_setup;
 	int clock_high;
@@ -13,6 +19,9 @@ struct l3_pins {
 	int mode_setup;
 };
 
+struct device;
+
 int l3_write(struct l3_pins *adap, u8 addr, u8 *data, int len);
+int l3_set_gpio_ops(struct device *dev, struct l3_pins *adap);
 
 #endif
diff --git a/sound/soc/codecs/l3.c b/sound/soc/codecs/l3.c
index 5353af5..a10ea3c 100644
--- a/sound/soc/codecs/l3.c
+++ b/sound/soc/codecs/l3.c
@@ -20,6 +20,8 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio.h>
 
 #include <sound/l3.h>
 
@@ -32,11 +34,11 @@ static void sendbyte(struct l3_pins *adap, unsigned int byte)
 	int i;
 
 	for (i = 0; i < 8; i++) {
-		adap->setclk(0);
+		adap->setclk(adap, 0);
 		udelay(adap->data_hold);
-		adap->setdat(byte & 1);
+		adap->setdat(adap, byte & 1);
 		udelay(adap->data_setup);
-		adap->setclk(1);
+		adap->setclk(adap, 1);
 		udelay(adap->clock_high);
 		byte >>= 1;
 	}
@@ -55,10 +57,10 @@ static void sendbytes(struct l3_pins *adap, const u8 *buf,
 	for (i = 0; i < len; i++) {
 		if (i) {
 			udelay(adap->mode_hold);
-			adap->setmode(0);
+			adap->setmode(adap, 0);
 			udelay(adap->mode);
 		}
-		adap->setmode(1);
+		adap->setmode(adap, 1);
 		udelay(adap->mode_setup);
 		sendbyte(adap, buf[i]);
 	}
@@ -66,26 +68,71 @@ static void sendbytes(struct l3_pins *adap, const u8 *buf,
 
 int l3_write(struct l3_pins *adap, u8 addr, u8 *data, int len)
 {
-	adap->setclk(1);
-	adap->setdat(1);
-	adap->setmode(1);
+	adap->setclk(adap, 1);
+	adap->setdat(adap, 1);
+	adap->setmode(adap, 1);
 	udelay(adap->mode);
 
-	adap->setmode(0);
+	adap->setmode(adap, 0);
 	udelay(adap->mode_setup);
 	sendbyte(adap, addr);
 	udelay(adap->mode_hold);
 
 	sendbytes(adap, data, len);
 
-	adap->setclk(1);
-	adap->setdat(1);
-	adap->setmode(0);
+	adap->setclk(adap, 1);
+	adap->setdat(adap, 1);
+	adap->setmode(adap, 0);
 
 	return len;
 }
 EXPORT_SYMBOL_GPL(l3_write);
 
+
+static void l3_set_clk(struct l3_pins *adap, int val)
+{
+	gpio_set_value(adap->gpio_clk, val);
+}
+
+static void l3_set_data(struct l3_pins *adap, int val)
+{
+	gpio_set_value(adap->gpio_data, val);
+}
+
+static void l3_set_mode(struct l3_pins *adap, int val)
+{
+	gpio_set_value(adap->gpio_mode, val);
+}
+
+int l3_set_gpio_ops(struct device *dev, struct l3_pins *adap)
+{
+	int ret;
+
+	if (!adap->use_gpios)
+		return -EINVAL;
+
+	ret = devm_gpio_request_one(dev, adap->gpio_data,
+				GPIOF_OUT_INIT_LOW, "l3_data");
+	if (ret < 0)
+		return ret;
+	adap->setdat = l3_set_data;
+
+	ret = devm_gpio_request_one(dev, adap->gpio_clk,
+				GPIOF_OUT_INIT_LOW, "l3_clk");
+	if (ret < 0)
+		return ret;
+	adap->setclk = l3_set_clk;
+
+	ret = devm_gpio_request_one(dev, adap->gpio_mode,
+				GPIOF_OUT_INIT_LOW, "l3_mode");
+	if (ret < 0)
+		return ret;
+	adap->setmode = l3_set_mode;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(l3_set_gpio_ops);
+
 MODULE_DESCRIPTION("L3 bit-banging driver");
 MODULE_AUTHOR("Christian Pellegrin <chripell@evolware.org>");
 MODULE_LICENSE("GPL");
-- 
1.9.1

[PATCH 2/8] ARM: S3C24XX: Specify audio codec platform_data for mini2440 board

[Sylwester Nawrocki]

The L3 bus GPIOs are specified in the board file rather than through
the sound card's device platform_data. This allows to ensure the codec
driver doesn't get probed with uninitialized platform_data field of
its corresponding platform device.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 arch/arm/mach-s3c24xx/mach-mini2440.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/mach-s3c24xx/mach-mini2440.c b/arch/arm/mach-s3c24xx/mach-mini2440.c
index a852168..13999c1 100644
--- a/arch/arm/mach-s3c24xx/mach-mini2440.c
+++ b/arch/arm/mach-s3c24xx/mach-mini2440.c
@@ -497,9 +497,28 @@ static struct i2c_board_info mini2440_i2c_devs[] __initdata = {
 	},
 };
 
+static struct uda134x_platform_data s3c24xx_uda134x = {
+	.l3 = {
+		.gpio_clk = S3C2410_GPB(4),
+		.gpio_data = S3C2410_GPB(3),
+		.gpio_mode = S3C2410_GPB(2),
+		.use_gpios = 1,
+		.data_hold = 1,
+		.data_setup = 1,
+		.clock_high = 1,
+		.mode_hold = 1,
+		.mode = 1,
+		.mode_setup = 1,
+	},
+	.model = UDA134X_UDA1341,
+};
+
 static struct platform_device uda1340_codec = {
 		.name = "uda134x-codec",
 		.id = -1,
+		.dev = {
+			.platform_data	= &s3c24xx_uda134x,
+		},
 };
 
 static struct platform_device *mini2440_devices[] __initdata = {
-- 
1.9.1

[PATCH 3/8] ASoC: uda134x: Optionally initialize L3 ops to default GPIO ops

[Sylwester Nawrocki]

The GPIO ops can be selected by platform_data which allows the
codec platform device to probe without the sound card's driver
intervention.

The downside is that it will request GPIOs on behalf of the codec
device and thus allow only one user on the bus, but it desn't seem
to be a limitation with current code and usage of the GPIO ops is
optional anyway.
The proper approach would presumably be to create a proper Linux
bus driver for L3, should this rather ancient bus specification
suddenly gain more interest.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/codecs/uda134x.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sound/soc/codecs/uda134x.c b/sound/soc/codecs/uda134x.c
index e4c694c..b72c925 100644
--- a/sound/soc/codecs/uda134x.c
+++ b/sound/soc/codecs/uda134x.c
@@ -544,6 +544,7 @@ static int uda134x_codec_probe(struct platform_device *pdev)
 {
 	struct uda134x_platform_data *pd = pdev->dev.platform_data;
 	struct uda134x_priv *uda134x;
+	int ret;
 
 	if (!pd) {
 		dev_err(&pdev->dev, "Missing L3 bitbang function\n");
@@ -557,6 +558,12 @@ static int uda134x_codec_probe(struct platform_device *pdev)
 	uda134x->pd = pd;
 	platform_set_drvdata(pdev, uda134x);
 
+	if (pd->l3.use_gpios) {
+		ret = l3_set_gpio_ops(&pdev->dev, &uda134x->pd->l3);
+		if (ret < 0)
+			return ret;
+	}
+
 	uda134x->regmap = devm_regmap_init(&pdev->dev, NULL, pd,
 		&uda134x_regmap_config);
 	if (IS_ERR(uda134x->regmap))
-- 
1.9.1

[PATCH 3/8] ASoC: uda134x: Optionally initialize L3 ops to default GPIO ops

[Mark Brown]

On Thu, Aug 04, 2016 at 03:38:43PM +0200, Sylwester Nawrocki wrote:

> The downside is that it will request GPIOs on behalf of the codec
> device and thus allow only one user on the bus, but it desn't seem
> to be a limitation with current code and usage of the GPIO ops is
> optional anyway.

Does L3 actually support multiple devices?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160804/259c583f/attachment.sig>

[PATCH 3/8] ASoC: uda134x: Optionally initialize L3 ops to default GPIO ops

[Sylwester Nawrocki]

On 08/04/2016 11:03 PM, Mark Brown wrote:
> Does L3 actually support multiple devices?

>From the bus operation description in a few datasheets of the Philips'
UDA13XX codecs I have checked I would say it's possible to have multiple
devices working on same bus.  The slave devices are selectable by 6
most significant bits of the address byte and the device addresses
seem to be unique at first sight.

"In the event that the UDA1341TS receives a different address, it will
deselect its microcontroller interface logic." (ch. 7.19 [1])

However I can't answer yes for sure, I've never seen multiple device
operation mentioned explicitly in the datasheets nor such an hardware
configuration.

[1] http://www.nxp.com/documents/data_sheet/UDA1341TS.pdf

[PATCH 4/8] ASoC: s3c24xx_uda134x: Remove unused power() callback

[Sylwester Nawrocki]

The power() callback has always been empty so just remove it.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 include/sound/s3c24xx_uda134x.h     | 1 -
 sound/soc/samsung/s3c24xx_uda134x.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/include/sound/s3c24xx_uda134x.h b/include/sound/s3c24xx_uda134x.h
index 33df4cb..ffaf1f0 100644
--- a/include/sound/s3c24xx_uda134x.h
+++ b/include/sound/s3c24xx_uda134x.h
@@ -7,7 +7,6 @@ struct s3c24xx_uda134x_platform_data {
 	int l3_clk;
 	int l3_mode;
 	int l3_data;
-	void (*power) (int);
 	int model;
 };
 
diff --git a/sound/soc/samsung/s3c24xx_uda134x.c b/sound/soc/samsung/s3c24xx_uda134x.c
index 92e88bc..c145eb2 100644
--- a/sound/soc/samsung/s3c24xx_uda134x.c
+++ b/sound/soc/samsung/s3c24xx_uda134x.c
@@ -282,7 +282,6 @@ static int s3c24xx_uda134x_probe(struct platform_device *pdev)
 		       "unable to find platform data\n");
 		return -ENODEV;
 	}
-	s3c24xx_uda134x.power = s3c24xx_uda134x_l3_pins->power;
 	s3c24xx_uda134x.model = s3c24xx_uda134x_l3_pins->model;
 
 	if (s3c24xx_uda134x_setup_pin(s3c24xx_uda134x_l3_pins->l3_data,
-- 
1.9.1

[PATCH 5/8] ASoC: s3c24xx_uda134x: Drop initialization of codec's platform data

[Sylwester Nawrocki]

It's already taken care by the codec and its platform_data defined
in the arch/arm/mach-s3c24xx/mach-mini2440.c board file.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/samsung/s3c24xx_uda134x.c | 71 +------------------------------------
 1 file changed, 1 insertion(+), 70 deletions(-)

diff --git a/sound/soc/samsung/s3c24xx_uda134x.c b/sound/soc/samsung/s3c24xx_uda134x.c
index c145eb2..794e30d 100644
--- a/sound/soc/samsung/s3c24xx_uda134x.c
+++ b/sound/soc/samsung/s3c24xx_uda134x.c
@@ -227,78 +227,12 @@ static struct snd_soc_card snd_soc_s3c24xx_uda134x = {
 	.num_links = 1,
 };
 
-static struct s3c24xx_uda134x_platform_data *s3c24xx_uda134x_l3_pins;
-
-static void setdat(int v)
-{
-	gpio_set_value(s3c24xx_uda134x_l3_pins->l3_data, v > 0);
-}
-
-static void setclk(int v)
-{
-	gpio_set_value(s3c24xx_uda134x_l3_pins->l3_clk, v > 0);
-}
-
-static void setmode(int v)
-{
-	gpio_set_value(s3c24xx_uda134x_l3_pins->l3_mode, v > 0);
-}
-
-/* FIXME - This must be codec platform data but in which board file ?? */
-static struct uda134x_platform_data s3c24xx_uda134x = {
-	.l3 = {
-		.setdat = setdat,
-		.setclk = setclk,
-		.setmode = setmode,
-		.data_hold = 1,
-		.data_setup = 1,
-		.clock_high = 1,
-		.mode_hold = 1,
-		.mode = 1,
-		.mode_setup = 1,
-	},
-};
-
-static int s3c24xx_uda134x_setup_pin(int pin, char *fun)
-{
-	if (gpio_request(pin, "s3c24xx_uda134x") < 0) {
-		printk(KERN_ERR "S3C24XX_UDA134X SoC Audio: "
-		       "l3 %s pin already in use", fun);
-		return -EBUSY;
-	}
-	gpio_direction_output(pin, 0);
-	return 0;
-}
-
 static int s3c24xx_uda134x_probe(struct platform_device *pdev)
 {
 	int ret;
 
 	printk(KERN_INFO "S3C24XX_UDA134X SoC Audio driver\n");
 
-	s3c24xx_uda134x_l3_pins = pdev->dev.platform_data;
-	if (s3c24xx_uda134x_l3_pins == NULL) {
-		printk(KERN_ERR "S3C24XX_UDA134X SoC Audio: "
-		       "unable to find platform data\n");
-		return -ENODEV;
-	}
-	s3c24xx_uda134x.model = s3c24xx_uda134x_l3_pins->model;
-
-	if (s3c24xx_uda134x_setup_pin(s3c24xx_uda134x_l3_pins->l3_data,
-				      "data") < 0)
-		return -EBUSY;
-	if (s3c24xx_uda134x_setup_pin(s3c24xx_uda134x_l3_pins->l3_clk,
-				      "clk") < 0) {
-		gpio_free(s3c24xx_uda134x_l3_pins->l3_data);
-		return -EBUSY;
-	}
-	if (s3c24xx_uda134x_setup_pin(s3c24xx_uda134x_l3_pins->l3_mode,
-				      "mode") < 0) {
-		gpio_free(s3c24xx_uda134x_l3_pins->l3_data);
-		gpio_free(s3c24xx_uda134x_l3_pins->l3_clk);
-		return -EBUSY;
-	}
-
 	s3c24xx_uda134x_snd_device = platform_device_alloc("soc-audio", -1);
 	if (!s3c24xx_uda134x_snd_device) {
 		printk(KERN_ERR "S3C24XX_UDA134X SoC Audio: "
@@ -308,7 +242,7 @@ static int s3c24xx_uda134x_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(s3c24xx_uda134x_snd_device,
 			     &snd_soc_s3c24xx_uda134x);
-	platform_device_add_data(s3c24xx_uda134x_snd_device, &s3c24xx_uda134x, sizeof(s3c24xx_uda134x));
+
 	ret = platform_device_add(s3c24xx_uda134x_snd_device);
 	if (ret) {
 		printk(KERN_ERR "S3C24XX_UDA134X SoC Audio: Unable to add\n");
@@ -321,9 +255,6 @@ static int s3c24xx_uda134x_probe(struct platform_device *pdev)
 static int s3c24xx_uda134x_remove(struct platform_device *pdev)
 {
 	platform_device_unregister(s3c24xx_uda134x_snd_device);
-	gpio_free(s3c24xx_uda134x_l3_pins->l3_data);
-	gpio_free(s3c24xx_uda134x_l3_pins->l3_clk);
-	gpio_free(s3c24xx_uda134x_l3_pins->l3_mode);
 	return 0;
 }
 
-- 
1.9.1

[PATCH 6/8] ASoC: samsung: Convert s3c24xx_uda134x to use devm_snd_soc_register_card()

[Sylwester Nawrocki]

Suppreses a following kernel warning: "soc-audio soc-audio:
ASoC: machine S3C24XX_UDA134X should use snd_soc_register_card()".

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/samsung/s3c24xx_uda134x.c | 34 +++++++---------------------------
 1 file changed, 7 insertions(+), 27 deletions(-)

diff --git a/sound/soc/samsung/s3c24xx_uda134x.c b/sound/soc/samsung/s3c24xx_uda134x.c
index 794e30d..a8102a7 100644
--- a/sound/soc/samsung/s3c24xx_uda134x.c
+++ b/sound/soc/samsung/s3c24xx_uda134x.c
@@ -54,8 +54,6 @@ static struct snd_pcm_hw_constraint_list hw_constraints_rates = {
 };
 #endif
 
-static struct platform_device *s3c24xx_uda134x_snd_device;
-
 static int s3c24xx_uda134x_startup(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
@@ -68,7 +66,7 @@ static int s3c24xx_uda134x_startup(struct snd_pcm_substream *substream)
 	mutex_lock(&clk_lock);
 	pr_debug("%s %d\n", __func__, clk_users);
 	if (clk_users == 0) {
-		xtal = clk_get(&s3c24xx_uda134x_snd_device->dev, "xtal");
+		xtal = clk_get(rtd->dev, "xtal");
 		if (IS_ERR(xtal)) {
 			printk(KERN_ERR "%s cannot get xtal\n", __func__);
 			ret = PTR_ERR(xtal);
@@ -229,43 +227,25 @@ static struct snd_soc_card snd_soc_s3c24xx_uda134x = {
 
 static int s3c24xx_uda134x_probe(struct platform_device *pdev)
 {
+	struct snd_soc_card *card = &snd_soc_s3c24xx_uda134x;
 	int ret;
 
-	printk(KERN_INFO "S3C24XX_UDA134X SoC Audio driver\n");
-
-	s3c24xx_uda134x_snd_device = platform_device_alloc("soc-audio", -1);
-	if (!s3c24xx_uda134x_snd_device) {
-		printk(KERN_ERR "S3C24XX_UDA134X SoC Audio: "
-		       "Unable to register\n");
-		return -ENOMEM;
-	}
+	platform_set_drvdata(pdev, card);
+	card->dev = &pdev->dev;
 
-	platform_set_drvdata(s3c24xx_uda134x_snd_device,
-			     &snd_soc_s3c24xx_uda134x);
-
-	ret = platform_device_add(s3c24xx_uda134x_snd_device);
-	if (ret) {
-		printk(KERN_ERR "S3C24XX_UDA134X SoC Audio: Unable to add\n");
-		platform_device_put(s3c24xx_uda134x_snd_device);
-	}
+	ret = devm_snd_soc_register_card(&pdev->dev, card);
+	if (ret)
+		dev_err(&pdev->dev, "failed to register card: %d\n", ret);
 
 	return ret;
 }
 
-static int s3c24xx_uda134x_remove(struct platform_device *pdev)
-{
-	platform_device_unregister(s3c24xx_uda134x_snd_device);
-	return 0;
-}
-
 static struct platform_driver s3c24xx_uda134x_driver = {
 	.probe  = s3c24xx_uda134x_probe,
-	.remove = s3c24xx_uda134x_remove,
 	.driver = {
 		.name = "s3c24xx_uda134x",
 	},
 };
-
 module_platform_driver(s3c24xx_uda134x_driver);
 
 MODULE_AUTHOR("Zoltan Devai, Christian Pellegrin <chripell@evolware.org>");
-- 
1.9.1

[PATCH 7/8] ASoC: samsung: s3c24xx_uda134x: debug/error trace cleanup

[Sylwester Nawrocki]

Switch from pr_* to dev_* macros and drop some debug traces.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/samsung/s3c24xx_uda134x.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/sound/soc/samsung/s3c24xx_uda134x.c b/sound/soc/samsung/s3c24xx_uda134x.c
index a8102a7..7853fbe 100644
--- a/sound/soc/samsung/s3c24xx_uda134x.c
+++ b/sound/soc/samsung/s3c24xx_uda134x.c
@@ -64,17 +64,17 @@ static int s3c24xx_uda134x_startup(struct snd_pcm_substream *substream)
 	int ret = 0;
 
 	mutex_lock(&clk_lock);
-	pr_debug("%s %d\n", __func__, clk_users);
+
 	if (clk_users == 0) {
 		xtal = clk_get(rtd->dev, "xtal");
 		if (IS_ERR(xtal)) {
-			printk(KERN_ERR "%s cannot get xtal\n", __func__);
+			dev_err(rtd->dev, "%s cannot get xtal\n", __func__);
 			ret = PTR_ERR(xtal);
 		} else {
 			pclk = clk_get(cpu_dai->dev, "iis");
 			if (IS_ERR(pclk)) {
-				printk(KERN_ERR "%s cannot get pclk\n",
-				       __func__);
+				dev_err(rtd->dev, "%s cannot get pclk\n",
+					__func__);
 				clk_put(xtal);
 				ret = PTR_ERR(pclk);
 			}
@@ -100,8 +100,8 @@ static int s3c24xx_uda134x_startup(struct snd_pcm_substream *substream)
 						 SNDRV_PCM_HW_PARAM_RATE,
 						 &hw_constraints_rates);
 		if (ret < 0)
-			printk(KERN_ERR "%s cannot set constraints\n",
-			       __func__);
+			dev_err(rtd->dev, "%s cannot set constraints\n",
+				__func__);
 #endif
 	}
 	return ret;
@@ -110,7 +110,6 @@ static int s3c24xx_uda134x_startup(struct snd_pcm_substream *substream)
 static void s3c24xx_uda134x_shutdown(struct snd_pcm_substream *substream)
 {
 	mutex_lock(&clk_lock);
-	pr_debug("%s %d\n", __func__, clk_users);
 	clk_users -= 1;
 	if (clk_users == 0) {
 		clk_put(xtal);
@@ -157,18 +156,19 @@ static int s3c24xx_uda134x_hw_params(struct snd_pcm_substream *substream,
 		clk_source = S3C24XX_CLKSRC_PCLK;
 		div = bi % 33;
 	}
-	pr_debug("%s desired rate %lu, %d\n", __func__, rate, bi);
+
+	dev_dbg(rtd->dev, "%s desired rate %lu, %d\n", __func__, rate, bi);
 
 	clk = (fs_mode == S3C2410_IISMOD_384FS ? 384 : 256) * rate;
-	pr_debug("%s will use: %s %s %d sysclk %d err %ld\n", __func__,
-		 fs_mode == S3C2410_IISMOD_384FS ? "384FS" : "256FS",
-		 clk_source == S3C24XX_CLKSRC_MPLL ? "MPLLin" : "PCLK",
-		 div, clk, err);
+
+	dev_dbg(rtd->dev, "%s will use: %s %s %d sysclk %d err %ld\n", __func__,
+		fs_mode == S3C2410_IISMOD_384FS ? "384FS" : "256FS",
+		clk_source == S3C24XX_CLKSRC_MPLL ? "MPLLin" : "PCLK",
+		div, clk, err);
 
 	if ((err * 100 / rate) > 5) {
-		printk(KERN_ERR "S3C24XX_UDA134X: effective frequency "
-		       "too different from desired (%ld%%)\n",
-		       err * 100 / rate);
+		dev_err(rtd->dev, "effective frequency too different "
+				  "from desired (%ld%%)\n", err * 100 / rate);
 		return -EINVAL;
 	}
 
-- 
1.9.1

[PATCH 7/8] ASoC: samsung: s3c24xx_uda134x: debug/error trace cleanup

[Mark Brown]

On Thu, Aug 04, 2016 at 03:38:47PM +0200, Sylwester Nawrocki wrote:
> Switch from pr_* to dev_* macros and drop some debug traces.

This doesn't apply against current code and the previous patch had
unknown SHA1s suggesting that you've got some other patches in your
tree, please check and resend.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160804/4dbf09e6/attachment.sig>

[PATCH 7/8] ASoC: samsung: s3c24xx_uda134x: debug/error trace cleanup

[Sylwester Nawrocki]

On 08/04/2016 11:05 PM, Mark Brown wrote:
> On Thu, Aug 04, 2016 at 03:38:47PM +0200, Sylwester Nawrocki wrote:
>> Switch from pr_* to dev_* macros and drop some debug traces.
> 
> This doesn't apply against current code and the previous patch had
> unknown SHA1s suggesting that you've got some other patches in your
> tree, please check and resend.

I suspect the cause is this patch missing:
"ASoC: samsung: Fix clock handling in S3C24XX_UDA134X card"
which I really should have sent as a first one together with
this series. My apologies for the mess. 


I've resent $subject patch and will hold on for a moment with 
PATCH 8/8 as there are large conflicts when I first rebase it 
onto topic/samsung and then try to merge fix/samsung and 
topic/samsung to the for-next branch.

[PATCH 8/8] ASoC: s3c244_uda134x: Allocate private data dynamically

[Sylwester Nawrocki]

Gather all driver's private variable in common data structure and allocate
it dynamically.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/samsung/s3c24xx_uda134x.c | 79 ++++++++++++++++++++-----------------
 1 file changed, 43 insertions(+), 36 deletions(-)

diff --git a/sound/soc/samsung/s3c24xx_uda134x.c b/sound/soc/samsung/s3c24xx_uda134x.c
index 7853fbe..81a7894 100644
--- a/sound/soc/samsung/s3c24xx_uda134x.c
+++ b/sound/soc/samsung/s3c24xx_uda134x.c
@@ -19,9 +19,15 @@
 #include <sound/s3c24xx_uda134x.h>
 
 #include "regs-iis.h"
-
 #include "s3c24xx-i2s.h"
 
+struct s3c24xx_uda134x {
+	struct clk *xtal;
+	struct clk *pclk;
+	struct mutex clk_lock;
+	int clk_users;
+};
+
 /* #define ENFORCE_RATES 1 */
 /*
   Unfortunately the S3C24XX in master mode has a limited capacity of
@@ -36,15 +42,6 @@
   possible an error will be returned.
 */
 
-static struct clk *xtal;
-static struct clk *pclk;
-/* this is need because we don't have a place where to keep the
- * pointers to the clocks in each substream. We get the clocks only
- * when we are actually using them so we don't block stuff like
- * frequency change or oscillator power-off */
-static int clk_users;
-static DEFINE_MUTEX(clk_lock);
-
 static unsigned int rates[33 * 2];
 #ifdef ENFORCE_RATES
 static struct snd_pcm_hw_constraint_list hw_constraints_rates = {
@@ -57,26 +54,24 @@ static struct snd_pcm_hw_constraint_list hw_constraints_rates = {
 static int s3c24xx_uda134x_startup(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct s3c24xx_uda134x *priv = snd_soc_card_get_drvdata(rtd->card);
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-#ifdef ENFORCE_RATES
-	struct snd_pcm_runtime *runtime = substream->runtime;
-#endif
 	int ret = 0;
 
-	mutex_lock(&clk_lock);
+	mutex_lock(&priv->clk_lock);
 
-	if (clk_users == 0) {
-		xtal = clk_get(rtd->dev, "xtal");
-		if (IS_ERR(xtal)) {
+	if (priv->clk_users == 0) {
+		priv->xtal = clk_get(rtd->dev, "xtal");
+		if (IS_ERR(priv->xtal)) {
 			dev_err(rtd->dev, "%s cannot get xtal\n", __func__);
-			ret = PTR_ERR(xtal);
+			ret = PTR_ERR(priv->xtal);
 		} else {
-			pclk = clk_get(cpu_dai->dev, "iis");
-			if (IS_ERR(pclk)) {
+			priv->pclk = clk_get(cpu_dai->dev, "iis");
+			if (IS_ERR(priv->pclk)) {
 				dev_err(rtd->dev, "%s cannot get pclk\n",
 					__func__);
-				clk_put(xtal);
-				ret = PTR_ERR(pclk);
+				clk_put(priv->xtal);
+				ret = PTR_ERR(priv->pclk);
 			}
 		}
 		if (!ret) {
@@ -85,18 +80,19 @@ static int s3c24xx_uda134x_startup(struct snd_pcm_substream *substream)
 			for (i = 0; i < 2; i++) {
 				int fs = i ? 256 : 384;
 
-				rates[i*33] = clk_get_rate(xtal) / fs;
+				rates[i*33] = clk_get_rate(priv->xtal) / fs;
 				for (j = 1; j < 33; j++)
-					rates[i*33 + j] = clk_get_rate(pclk) /
+					rates[i*33 + j] = clk_get_rate(priv->pclk) /
 						(j * fs);
 			}
 		}
 	}
-	clk_users += 1;
-	mutex_unlock(&clk_lock);
+	priv->clk_users += 1;
+	mutex_unlock(&priv->clk_lock);
+
 	if (!ret) {
 #ifdef ENFORCE_RATES
-		ret = snd_pcm_hw_constraint_list(runtime, 0,
+		ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
 						 SNDRV_PCM_HW_PARAM_RATE,
 						 &hw_constraints_rates);
 		if (ret < 0)
@@ -109,15 +105,18 @@ static int s3c24xx_uda134x_startup(struct snd_pcm_substream *substream)
 
 static void s3c24xx_uda134x_shutdown(struct snd_pcm_substream *substream)
 {
-	mutex_lock(&clk_lock);
-	clk_users -= 1;
-	if (clk_users == 0) {
-		clk_put(xtal);
-		xtal = NULL;
-		clk_put(pclk);
-		pclk = NULL;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct s3c24xx_uda134x *priv = snd_soc_card_get_drvdata(rtd->card);
+
+	mutex_lock(&priv->clk_lock);
+	priv->clk_users -= 1;
+	if (priv->clk_users == 0) {
+		clk_put(priv->xtal);
+		priv->xtal = NULL;
+		clk_put(priv->pclk);
+		priv->pclk = NULL;
 	}
-	mutex_unlock(&clk_lock);
+	mutex_unlock(&priv->clk_lock);
 }
 
 static int s3c24xx_uda134x_hw_params(struct snd_pcm_substream *substream,
@@ -228,10 +227,18 @@ static struct snd_soc_card snd_soc_s3c24xx_uda134x = {
 static int s3c24xx_uda134x_probe(struct platform_device *pdev)
 {
 	struct snd_soc_card *card = &snd_soc_s3c24xx_uda134x;
+	struct s3c24xx_uda134x *priv;
 	int ret;
 
-	platform_set_drvdata(pdev, card);
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	mutex_init(&priv->clk_lock);
+
 	card->dev = &pdev->dev;
+	platform_set_drvdata(pdev, card);
+	snd_soc_card_set_drvdata(card, priv);
 
 	ret = devm_snd_soc_register_card(&pdev->dev, card);
 	if (ret)
-- 
1.9.1

Applied "ASoC: s3c24xx_uda134x: Move global variables to driver's data structure" to the asoc tree

[Mark Brown]

The patch

   ASoC: s3c24xx_uda134x: Move global variables to driver's data structure

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 892ccf0f2173a9ede5448411e9475616fb21fb51 Mon Sep 17 00:00:00 2001
From: Sylwester Nawrocki <s.nawrocki@samsung.com>
Date: Tue, 25 Oct 2016 12:57:57 +0200
Subject: [PATCH] ASoC: s3c24xx_uda134x: Move global variables to driver's data
 structure

Gather all driver's private variables in common data structure
and allocate the data structure dynamically.

Also unused ENFORCE_RATES symbol and local variable (leftovers
from an erroneous rebase) are removed.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/samsung/s3c24xx_uda134x.c | 79 ++++++++++++++++++++-----------------
 1 file changed, 43 insertions(+), 36 deletions(-)

diff --git a/sound/soc/samsung/s3c24xx_uda134x.c b/sound/soc/samsung/s3c24xx_uda134x.c
index 7853fbe6ccc9..81a78940967c 100644
--- a/sound/soc/samsung/s3c24xx_uda134x.c
+++ b/sound/soc/samsung/s3c24xx_uda134x.c
@@ -19,9 +19,15 @@
 #include <sound/s3c24xx_uda134x.h>
 
 #include "regs-iis.h"
-
 #include "s3c24xx-i2s.h"
 
+struct s3c24xx_uda134x {
+	struct clk *xtal;
+	struct clk *pclk;
+	struct mutex clk_lock;
+	int clk_users;
+};
+
 /* #define ENFORCE_RATES 1 */
 /*
   Unfortunately the S3C24XX in master mode has a limited capacity of
@@ -36,15 +42,6 @@
   possible an error will be returned.
 */
 
-static struct clk *xtal;
-static struct clk *pclk;
-/* this is need because we don't have a place where to keep the
- * pointers to the clocks in each substream. We get the clocks only
- * when we are actually using them so we don't block stuff like
- * frequency change or oscillator power-off */
-static int clk_users;
-static DEFINE_MUTEX(clk_lock);
-
 static unsigned int rates[33 * 2];
 #ifdef ENFORCE_RATES
 static struct snd_pcm_hw_constraint_list hw_constraints_rates = {
@@ -57,26 +54,24 @@ static struct snd_pcm_hw_constraint_list hw_constraints_rates = {
 static int s3c24xx_uda134x_startup(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct s3c24xx_uda134x *priv = snd_soc_card_get_drvdata(rtd->card);
 	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-#ifdef ENFORCE_RATES
-	struct snd_pcm_runtime *runtime = substream->runtime;
-#endif
 	int ret = 0;
 
-	mutex_lock(&clk_lock);
+	mutex_lock(&priv->clk_lock);
 
-	if (clk_users == 0) {
-		xtal = clk_get(rtd->dev, "xtal");
-		if (IS_ERR(xtal)) {
+	if (priv->clk_users == 0) {
+		priv->xtal = clk_get(rtd->dev, "xtal");
+		if (IS_ERR(priv->xtal)) {
 			dev_err(rtd->dev, "%s cannot get xtal\n", __func__);
-			ret = PTR_ERR(xtal);
+			ret = PTR_ERR(priv->xtal);
 		} else {
-			pclk = clk_get(cpu_dai->dev, "iis");
-			if (IS_ERR(pclk)) {
+			priv->pclk = clk_get(cpu_dai->dev, "iis");
+			if (IS_ERR(priv->pclk)) {
 				dev_err(rtd->dev, "%s cannot get pclk\n",
 					__func__);
-				clk_put(xtal);
-				ret = PTR_ERR(pclk);
+				clk_put(priv->xtal);
+				ret = PTR_ERR(priv->pclk);
 			}
 		}
 		if (!ret) {
@@ -85,18 +80,19 @@ static int s3c24xx_uda134x_startup(struct snd_pcm_substream *substream)
 			for (i = 0; i < 2; i++) {
 				int fs = i ? 256 : 384;
 
-				rates[i*33] = clk_get_rate(xtal) / fs;
+				rates[i*33] = clk_get_rate(priv->xtal) / fs;
 				for (j = 1; j < 33; j++)
-					rates[i*33 + j] = clk_get_rate(pclk) /
+					rates[i*33 + j] = clk_get_rate(priv->pclk) /
 						(j * fs);
 			}
 		}
 	}
-	clk_users += 1;
-	mutex_unlock(&clk_lock);
+	priv->clk_users += 1;
+	mutex_unlock(&priv->clk_lock);
+
 	if (!ret) {
 #ifdef ENFORCE_RATES
-		ret = snd_pcm_hw_constraint_list(runtime, 0,
+		ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
 						 SNDRV_PCM_HW_PARAM_RATE,
 						 &hw_constraints_rates);
 		if (ret < 0)
@@ -109,15 +105,18 @@ static int s3c24xx_uda134x_startup(struct snd_pcm_substream *substream)
 
 static void s3c24xx_uda134x_shutdown(struct snd_pcm_substream *substream)
 {
-	mutex_lock(&clk_lock);
-	clk_users -= 1;
-	if (clk_users == 0) {
-		clk_put(xtal);
-		xtal = NULL;
-		clk_put(pclk);
-		pclk = NULL;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct s3c24xx_uda134x *priv = snd_soc_card_get_drvdata(rtd->card);
+
+	mutex_lock(&priv->clk_lock);
+	priv->clk_users -= 1;
+	if (priv->clk_users == 0) {
+		clk_put(priv->xtal);
+		priv->xtal = NULL;
+		clk_put(priv->pclk);
+		priv->pclk = NULL;
 	}
-	mutex_unlock(&clk_lock);
+	mutex_unlock(&priv->clk_lock);
 }
 
 static int s3c24xx_uda134x_hw_params(struct snd_pcm_substream *substream,
@@ -228,10 +227,18 @@ static struct snd_soc_card snd_soc_s3c24xx_uda134x = {
 static int s3c24xx_uda134x_probe(struct platform_device *pdev)
 {
 	struct snd_soc_card *card = &snd_soc_s3c24xx_uda134x;
+	struct s3c24xx_uda134x *priv;
 	int ret;
 
-	platform_set_drvdata(pdev, card);
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	mutex_init(&priv->clk_lock);
+
 	card->dev = &pdev->dev;
+	platform_set_drvdata(pdev, card);
+	snd_soc_card_set_drvdata(card, priv);
 
 	ret = devm_snd_soc_register_card(&pdev->dev, card);
 	if (ret)
-- 
2.8.1