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

Sylwester Nawrocki

4 Aug 2016 4 Aug '16

3:38 p.m.

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

Show replies by date

Sylwester Nawrocki

4 Aug 4 Aug

3:38 p.m.

New subject: [alsa-devel] [PATCH 1/8] ASoC: L3 bus: Add default gpio ops

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

Sylwester Nawrocki

3:38 p.m.

New subject: [alsa-devel] [PATCH 2/8] ARM: S3C24XX: Specify audio codec platform_data for mini2440 board

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

Sylwester Nawrocki

3:38 p.m.

New subject: [alsa-devel] [PATCH 3/8] ASoC: uda134x: Optionally initialize L3 ops to default GPIO ops

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

Mark Brown

11:03 p.m.

New subject: [alsa-devel] [PATCH 3/8] ASoC: uda134x: Optionally initialize L3 ops to default GPIO ops

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?

Sylwester Nawrocki

5 Aug 5 Aug

10:48 a.m.

New subject: [alsa-devel] [PATCH 3/8] ASoC: uda134x: Optionally initialize L3 ops to default GPIO ops

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

Sylwester Nawrocki

4 Aug 4 Aug

3:38 p.m.

New subject: [alsa-devel] [PATCH 4/8] ASoC: s3c24xx_uda134x: Remove unused power() callback

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

Sylwester Nawrocki

3:38 p.m.

New subject: [alsa-devel] [PATCH 5/8] ASoC: s3c24xx_uda134x: Drop initialization of codec's platform data

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

Sylwester Nawrocki

3:38 p.m.

New subject: [alsa-devel] [PATCH 6/8] ASoC: samsung: Convert s3c24xx_uda134x to use devm_snd_soc_register_card()

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

Sylwester Nawrocki

3:38 p.m.

New subject: [alsa-devel] [PATCH 7/8] ASoC: samsung: s3c24xx_uda134x: debug/error trace cleanup

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

Mark Brown

11:05 p.m.

New subject: [alsa-devel] [PATCH 7/8] ASoC: samsung: s3c24xx_uda134x: debug/error trace cleanup

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.

Sylwester Nawrocki

5 Aug 5 Aug

11:52 a.m.

New subject: [alsa-devel] [PATCH 7/8] ASoC: samsung: s3c24xx_uda134x: debug/error trace cleanup

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.

Sylwester Nawrocki

4 Aug 4 Aug

3:38 p.m.

New subject: [alsa-devel] [PATCH 8/8] ASoC: s3c244_uda134x: Allocate private data dynamically

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

Mark Brown

25 Oct 25 Oct

9:24 p.m.

New subject: [alsa-devel] Applied "ASoC: s3c24xx_uda134x: Move global variables to driver's data structure" to the asoc tree

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"

- mutex_lock(&clk_lock); + mutex_lock(&priv->clk_lock);

ret = devm_snd_soc_register_card(&pdev->dev, card); if (ret)

-- 2.8.1

2980

Age (days ago)

3062

Last active (days ago)

List overview

Download

13 comments

2 participants

tags (0)

participants (2)

Mark Brown
Sylwester Nawrocki