[alsa-devel] [PATCH 0/4] Amstrad Delta: access MODEM_RESET GPIO pin over a regulator
The Amstrad Delta on-board latch2 bit named MODEM_NRESET, now available as a GPIO pin AMS_DELTA_GPIO_PIN_NMODEM_RESET, is used to power up/down (bring into/out of a reset state) two distinct on-board devices simultaneously: the modem, and the voice codec. As a consequence, that bit is, or can be, manipulated concurrently by two drivers, or their platform provided hooks.
Instead of updating those drivers to use the gpiolib API as a new method of controlling the MODEM_NRESET pin state, like it was done to other drivers accessing latch2 pins, and still being vulnerable to potential concurrency conflicts, or trying to solve that sharing issue with a custom piece of code, set up a fixed regulator device on top of that GPIO pin and update both drivers to manipulate that regulator, not the GPIO pin directly.
Janusz Krzysztofik (4): ARM: OMAP1: ams-delta: set up regulator over modem reset GPIO pin ASoC: cx20442: add bias control over a platform provided regulator ARM: OMAP1: ams-delta: update the modem to use regulator API ASoC: OMAP: ams-delta: drop .set_bias_level callback
arch/arm/mach-omap1/Kconfig | 2 + arch/arm/mach-omap1/board-ams-delta.c | 116 +++++++++++++++++++-- arch/arm/plat-omap/include/plat/board-ams-delta.h | 1 - sound/soc/codecs/cx20442.c | 58 ++++++++++- sound/soc/omap/ams-delta.c | 34 ------ 5 files changed, 165 insertions(+), 46 deletions(-)
The Amstrad Delta on-board latch2 bit named MODEM_NRESET, now available as a GPIO pin AMS_DELTA_GPIO_PIN_NMODEM_RESET, is used to power up/down (bring into/out of a reset state) two distinct on-board devices simultaneously: the modem, and the voice codec. As a consequence, that bit is, or can be, manipulated concurrently by two drivers, or their platform provided hooks.
Instead of updating those drivers to use the gpiolib API as a new method of controlling the MODEM_NRESET pin state, like it was done to other drivers accessing latch2 pins, and still being vulnerable to potential concurrency conflicts, or trying to solve that sharing issue with a custom piece of code, set up a fixed regulator device on top of that GPIO pin, with the intention of updating both drivers to manipulate that regulator, not the GPIO pin directly.
Before the ASoC driver is updated to use that regulator, and the modem platform data expanded with a power management callback for switching its power, the ams_delta_latch_write() function, which still provides the old API for accessing latch2 functionality from not updated drivers, is modified to toggle the regulator instead of the MODEM_NRESET GPIO pin. A helper function provided for balancing the regulator enable/disable operations, together with the regulator consumer data needed for maintaining its state, will be reused by the above mentioned modem power management callback once implemented.
Depends on patch series "ARM: OMAP1: ams-delta: replace custom I/O with GPIO".
Signed-off-by: Janusz Krzysztofik jkrzyszt@tis.icnet.pl --- arch/arm/mach-omap1/Kconfig | 2 + arch/arm/mach-omap1/board-ams-delta.c | 103 +++++++++++++++++++++++++++++++-- 2 files changed, 99 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-omap1/Kconfig b/arch/arm/mach-omap1/Kconfig index 5b1edba..4b6a774 100644 --- a/arch/arm/mach-omap1/Kconfig +++ b/arch/arm/mach-omap1/Kconfig @@ -157,6 +157,8 @@ config MACH_AMS_DELTA select FIQ select GPIO_GENERIC_PLATFORM select LEDS_GPIO_REGISTER + select REGULATOR + select REGULATOR_FIXED_VOLTAGE help Support for the Amstrad E3 (codename Delta) videophone. Say Y here if you have such a device. diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c index 673cf21..b5a1a3b 100644 --- a/arch/arm/mach-omap1/board-ams-delta.c +++ b/arch/arm/mach-omap1/board-ams-delta.c @@ -18,7 +18,11 @@ #include <linux/input.h> #include <linux/interrupt.h> #include <linux/leds.h> +#include <linux/mutex.h> #include <linux/platform_device.h> +#include <linux/regulator/consumer.h> +#include <linux/regulator/fixed.h> +#include <linux/regulator/machine.h> #include <linux/serial_8250.h> #include <linux/export.h>
@@ -237,11 +241,6 @@ static struct gpio latch_gpios[] __initconst = { .label = "scard_cmdvcc", }, { - .gpio = AMS_DELTA_GPIO_PIN_MODEM_NRESET, - .flags = GPIOF_OUT_INIT_LOW, - .label = "modem_nreset", - }, - { .gpio = AMS_DELTA_GPIO_PIN_MODEM_CODEC, .flags = GPIOF_OUT_INIT_LOW, .label = "modem_codec", @@ -258,6 +257,73 @@ static struct gpio latch_gpios[] __initconst = { }, };
+static struct regulator_consumer_supply modem_nreset_consumers[] __initconst = { + REGULATOR_SUPPLY("RESET#", "serial8250.1"), + REGULATOR_SUPPLY("POR", "cx20442-codec"), +}; + +static struct regulator_init_data modem_nreset_data __initconst = { + .constraints = { + .valid_ops_mask = REGULATOR_CHANGE_STATUS, + .boot_on = 1, + }, + .num_consumer_supplies = ARRAY_SIZE(modem_nreset_consumers), + .consumer_supplies = modem_nreset_consumers, +}; + +static struct fixed_voltage_config modem_nreset_config __initconst = { + .supply_name = "modem_nreset", + .microvolts = 3300000, + .gpio = AMS_DELTA_GPIO_PIN_MODEM_NRESET, + .startup_delay = 25000, + .enable_high = 1, + .enabled_at_boot = 1, + .init_data = &modem_nreset_data, +}; + +static struct platform_device modem_nreset_device = { + .name = "reg-fixed-voltage", + .id = -1, + .dev = { + .platform_data = &modem_nreset_config, + }, +}; + +struct regulator_consumer_data { + struct mutex lock; + struct regulator *regulator; + bool enabled; +}; + +static int regulator_toggle(struct regulator_consumer_data *consumer, + bool enable) +{ + int err = 0; + + if (!consumer->regulator) + return -ENODEV; + + mutex_lock(&consumer->lock); + if (IS_ERR(consumer->regulator)) { + err = PTR_ERR(consumer->regulator); + } else if (enable) { + if (!consumer->enabled) { + err = regulator_enable(consumer->regulator); + consumer->enabled = true; + } + } else { + if (consumer->enabled) { + err = regulator_disable(consumer->regulator); + consumer->enabled = false; + } + } + mutex_unlock(&consumer->lock); + + return err; +} + +static struct regulator_consumer_data modem_nreset; + void ams_delta_latch_write(int base, int ngpio, u16 mask, u16 value) { int bit = 0; @@ -266,7 +332,10 @@ void ams_delta_latch_write(int base, int ngpio, u16 mask, u16 value) for (; bit < ngpio; bit++, bitpos = bitpos << 1) { if (!(mask & bitpos)) continue; - gpio_set_value(base + bit, (value & bitpos) != 0); + if (base + bit == AMS_DELTA_GPIO_PIN_MODEM_NRESET) + regulator_toggle(&modem_nreset, (value & bitpos) != 0); + else + gpio_set_value(base + bit, (value & bitpos) != 0); } } EXPORT_SYMBOL(ams_delta_latch_write); @@ -496,6 +565,12 @@ static int __init late_init(void)
platform_add_devices(late_devices, ARRAY_SIZE(late_devices));
+ err = platform_device_register(&modem_nreset_device); + if (err) { + pr_err("Couldn't register the modem regulator device\n"); + return err; + } + omap_cfg_reg(M14_1510_GPIO2); ams_delta_modem_ports[0].irq = gpio_to_irq(AMS_DELTA_GPIO_PIN_MODEM_IRQ); @@ -514,8 +589,24 @@ static int __init late_init(void) err = platform_device_register(&ams_delta_modem_device); if (err) goto gpio_free; + + /* + * Once the modem device is registered, the modem_nreset + * regulator can be requested on behalf of that device. + * The regulator is used via ams_delta_latch_write() + * by the modem and ASoC drivers until updated. + */ + mutex_init(&modem_nreset.lock); + modem_nreset.regulator = regulator_get(&ams_delta_modem_device.dev, + "RESET#"); + if (IS_ERR(modem_nreset.regulator)) { + err = PTR_ERR(modem_nreset.regulator); + goto unregister; + } return 0;
+unregister: + platform_device_unregister(&ams_delta_modem_device); gpio_free: gpio_free(AMS_DELTA_GPIO_PIN_MODEM_IRQ); return err;
Now that a regulator device for controlling the codec chip reset state over a platform agnostic regulator API is available on the only board using this driver so far, extend the driver with a bias control function which will request virtual power to the codec chip from that virtual regulator, and will supersede the present implementation existing at the sound card level.
Thanks to the regulator sharing mechanism, both the old (the sound card) and the new (the codec) implementations will coexist smoothly until the sound card file is updated.
While extending the cx20442 structure, drop unused control_type member.
Created against linxu-3.2-rc6, tested on top of patch 1/4 "ARM: OMAP1: ams-delta: set up regulator over modem reset GPIO pin".
Signed-off-by: Janusz Krzysztofik jkrzyszt@tis.icnet.pl --- sound/soc/codecs/cx20442.c | 58 ++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/cx20442.c b/sound/soc/codecs/cx20442.c index bc7067d..d159c23 100644 --- a/sound/soc/codecs/cx20442.c +++ b/sound/soc/codecs/cx20442.c @@ -16,6 +16,7 @@ #include <linux/tty.h> #include <linux/slab.h> #include <linux/module.h> +#include <linux/regulator/consumer.h>
#include <sound/core.h> #include <sound/initval.h> @@ -25,8 +26,12 @@
struct cx20442_priv { - enum snd_soc_control_type control_type; void *control_data; + struct { + struct mutex lock; + struct regulator *regulator; + bool enabled; + } por; };
#define CX20442_PM 0x0 @@ -324,6 +329,41 @@ static struct snd_soc_dai_driver cx20442_dai = { }, };
+static int cx20442_set_bias_level(struct snd_soc_codec *codec, + enum snd_soc_bias_level level) +{ + struct cx20442_priv *cx20442 = snd_soc_codec_get_drvdata(codec); + int err = 0; + + mutex_lock(&cx20442->por.lock); + switch (level) { + case SND_SOC_BIAS_ON: + case SND_SOC_BIAS_PREPARE: + if (IS_ERR(cx20442->por.regulator)) { + err = PTR_ERR(cx20442->por.regulator); + } else if (!cx20442->por.enabled) { + err = regulator_enable(cx20442->por.regulator); + if (!err) + cx20442->por.enabled = true; + } + break; + case SND_SOC_BIAS_STANDBY: + case SND_SOC_BIAS_OFF: + if (IS_ERR(cx20442->por.regulator)) { + err = PTR_ERR(cx20442->por.regulator); + } else if (cx20442->por.enabled) { + err = regulator_disable(cx20442->por.regulator); + if (!err) + cx20442->por.enabled = false; + } + } + mutex_unlock(&cx20442->por.lock); + if (!err) + codec->dapm.bias_level = level; + + return err; +} + static int cx20442_codec_probe(struct snd_soc_codec *codec) { struct cx20442_priv *cx20442; @@ -331,9 +371,12 @@ static int cx20442_codec_probe(struct snd_soc_codec *codec) cx20442 = kzalloc(sizeof(struct cx20442_priv), GFP_KERNEL); if (cx20442 == NULL) return -ENOMEM; - snd_soc_codec_set_drvdata(codec, cx20442);
+ mutex_init(&cx20442->por.lock); + cx20442->por.regulator = regulator_get(codec->dev, "POR"); cx20442->control_data = NULL; + + snd_soc_codec_set_drvdata(codec, cx20442); codec->hw_write = NULL; codec->card->pop_time = 0;
@@ -350,6 +393,16 @@ static int cx20442_codec_remove(struct snd_soc_codec *codec) tty_hangup(tty); }
+ mutex_lock(&cx20442->por.lock); + if (!IS_ERR(cx20442->por.regulator)) { + if (cx20442->por.enabled) + regulator_disable(cx20442->por.regulator); + regulator_put(cx20442->por.regulator); + cx20442->por.regulator = ERR_PTR(-ENODEV); + } + mutex_unlock(&cx20442->por.lock); + + snd_soc_codec_set_drvdata(codec, NULL); kfree(cx20442); return 0; } @@ -359,6 +412,7 @@ static const u8 cx20442_reg; static struct snd_soc_codec_driver cx20442_codec_dev = { .probe = cx20442_codec_probe, .remove = cx20442_codec_remove, + .set_bias_level = cx20442_set_bias_level, .reg_cache_default = &cx20442_reg, .reg_cache_size = 1, .reg_word_size = sizeof(u8),
On Sat, Dec 24, 2011 at 12:12:22AM +0100, Janusz Krzysztofik wrote:
- case SND_SOC_BIAS_ON:
- case SND_SOC_BIAS_PREPARE:
if (IS_ERR(cx20442->por.regulator)) {err = PTR_ERR(cx20442->por.regulator);} else if (!cx20442->por.enabled) {err = regulator_enable(cx20442->por.regulator);if (!err)cx20442->por.enabled = true;}break;- case SND_SOC_BIAS_STANDBY:
- case SND_SOC_BIAS_OFF:
if (IS_ERR(cx20442->por.regulator)) {err = PTR_ERR(cx20442->por.regulator);} else if (cx20442->por.enabled) {err = regulator_disable(cx20442->por.regulator);if (!err)cx20442->por.enabled = false;}- }
- mutex_unlock(&cx20442->por.lock);
You can avoid the mutex and simplify the code by relying on the fact that the only possible transitions are:
OFF <-> STANDBY <-> PREPARE <-> ON
which would look a lot more natural - you shouldn't need to remember if the regulator is enabled, you should just turn it on in the STANDBY to PREPARE transition and turn it off in the ON to PREPARE or PREPARE to STANDBY transitions.
On Monday 26 of December 2011 at 12:02:01, Mark Brown wrote:
On Sat, Dec 24, 2011 at 12:12:22AM +0100, Janusz Krzysztofik wrote:
- case SND_SOC_BIAS_ON:
- case SND_SOC_BIAS_PREPARE:
if (IS_ERR(cx20442->por.regulator)) {err = PTR_ERR(cx20442->por.regulator);} else if (!cx20442->por.enabled) {err = regulator_enable(cx20442->por.regulator);if (!err)cx20442->por.enabled = true;}break;- case SND_SOC_BIAS_STANDBY:
- case SND_SOC_BIAS_OFF:
if (IS_ERR(cx20442->por.regulator)) {err = PTR_ERR(cx20442->por.regulator);} else if (cx20442->por.enabled) {err = regulator_disable(cx20442->por.regulator);if (!err)cx20442->por.enabled = false;}- }
- mutex_unlock(&cx20442->por.lock);
You can avoid the mutex and simplify the code by relying on the fact that the only possible transitions are:
OFF <-> STANDBY <-> PREPARE <-> ON
which would look a lot more natural - you shouldn't need to remember if the regulator is enabled, you should just turn it on in the STANDBY to PREPARE transition and turn it off in the ON to PREPARE or PREPARE to STANDBY transitions.
OK, will do, thanks for the hint. Janusz
On Monday 26 of December 2011 at 12:02:01, Mark Brown wrote:
On Sat, Dec 24, 2011 at 12:12:22AM +0100, Janusz Krzysztofik wrote:
- case SND_SOC_BIAS_ON:
- case SND_SOC_BIAS_PREPARE:
if (IS_ERR(cx20442->por.regulator)) {err = PTR_ERR(cx20442->por.regulator);} else if (!cx20442->por.enabled) {err = regulator_enable(cx20442->por.regulator);if (!err)cx20442->por.enabled = true;}break;- case SND_SOC_BIAS_STANDBY:
- case SND_SOC_BIAS_OFF:
if (IS_ERR(cx20442->por.regulator)) {err = PTR_ERR(cx20442->por.regulator);} else if (cx20442->por.enabled) {err = regulator_disable(cx20442->por.regulator);if (!err)cx20442->por.enabled = false;}- }
- mutex_unlock(&cx20442->por.lock);
You can avoid the mutex and simplify the code by relying on the fact that the only possible transitions are:
OFF <-> STANDBY <-> PREPARE <-> ON
which would look a lot more natural - you shouldn't need to remember if the regulator is enabled, you should just turn it on in the STANDBY to PREPARE transition and turn it off in the ON to PREPARE or PREPARE to STANDBY transitions.
Can I assume STANDBY or OFF at the time the codec .remove method is called? If not, is there a helper function available which can be called in order to perform all those ON -> PREPARE -> STANDBY [-> OFF] transitions before calling regulator_put()?
Thanks, Janusz
On Tue, Dec 27, 2011 at 03:16:08PM +0100, Janusz Krzysztofik wrote:
On Monday 26 of December 2011 at 12:02:01, Mark Brown wrote:
which would look a lot more natural - you shouldn't need to remember if the regulator is enabled, you should just turn it on in the STANDBY to PREPARE transition and turn it off in the ON to PREPARE or PREPARE to STANDBY transitions.
Can I assume STANDBY or OFF at the time the codec .remove method is called? If not, is there a helper function available which can be called in order to perform all those ON -> PREPARE -> STANDBY [-> OFF] transitions before calling regulator_put()?
It'll be in STANDBY or OFF depending on if the device sets idle_bias_off.
After the CX20442 codec driver already takes care of enabling the codec power for itself, but before dropping the old bias control method from the Amstrad Delta ASoC sound card file, which in fact keeps the modem power always on, even after the ASoC device close for now, extend the modem setup with a power management callback, which toggles the regulator up to the modem's needs, reusing the previously set up regulator consumer for this. Also, drop the MODEM_NRESET pin setup from the modem initialization procedure, as this operation was already ineffective since patch 1/4, and not needed because the regulator is set up as initially enabled.
Depends on patch 1/4 "ARM: OMAP1: ams-delta: set up regulator over modem reset GPIO pin" to apply cleanly, and requires patch 2/4 "ASoC: cx20442: add bias control over a platform provided regulator" for the sound card / codec bundle to still work correctly in coexistence with the modem.
Signed-off-by: Janusz Krzysztofik jkrzyszt@tis.icnet.pl --- arch/arm/mach-omap1/board-ams-delta.c | 20 +++++++++++++++----- 1 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c index b5a1a3b..9b52aeb 100644 --- a/arch/arm/mach-omap1/board-ams-delta.c +++ b/arch/arm/mach-omap1/board-ams-delta.c @@ -528,6 +528,14 @@ static void __init ams_delta_init(void) omap_writew(omap_readw(ARM_RSTCT1) | 0x0004, ARM_RSTCT1); }
+static void modem_pm(struct uart_port *port, unsigned int state, unsigned old) +{ + struct regulator_consumer_data *consumer = port->private_data; + + if (state != old) + regulator_toggle(consumer, state == 0); +} + static struct plat_serial8250_port ams_delta_modem_ports[] = { { .membase = IOMEM(MODEM_VIRT), @@ -538,6 +546,8 @@ static struct plat_serial8250_port ams_delta_modem_ports[] = { .iotype = UPIO_MEM, .regshift = 1, .uartclk = BASE_BAUD * 16, + .pm = modem_pm, + .private_data = &modem_nreset, }, { }, }; @@ -582,9 +592,8 @@ static int __init late_init(void) } gpio_direction_input(AMS_DELTA_GPIO_PIN_MODEM_IRQ);
- ams_delta_latch2_write( - AMS_DELTA_LATCH2_MODEM_NRESET | AMS_DELTA_LATCH2_MODEM_CODEC, - AMS_DELTA_LATCH2_MODEM_NRESET | AMS_DELTA_LATCH2_MODEM_CODEC); + ams_delta_latch2_write(AMS_DELTA_LATCH2_MODEM_CODEC, + AMS_DELTA_LATCH2_MODEM_CODEC);
err = platform_device_register(&ams_delta_modem_device); if (err) @@ -593,8 +602,9 @@ static int __init late_init(void) /* * Once the modem device is registered, the modem_nreset * regulator can be requested on behalf of that device. - * The regulator is used via ams_delta_latch_write() - * by the modem and ASoC drivers until updated. + * In addition to the modem .pm callback, that regulator + * is still used via the ams_delta_latch_write() wrapper + * by the ASoC driver until updated. */ mutex_init(&modem_nreset.lock); modem_nreset.regulator = regulator_get(&ams_delta_modem_device.dev,
This functionality has just been implemented in the cx20442 codec driver, no need to keep it here duplicated.
Once done, remove the no longer needed AMS_DELTA_LATCH2_MODEM_NRESET symbol from the Amstrad Delta header file, and a call to the regulator_toggle() helper function from the old API wrapper found in the board file. While being at it, move that function definition, together with the regulator consumer related data structure, down to the modem related section for better readability.
Depends on patch 3/4 "ARM: OMAP1: ams-delta: update the modem to use regulator API"
Signed-off-by: Janusz Krzysztofik jkrzyszt@tis.icnet.pl --- arch/arm/mach-omap1/board-ams-delta.c | 75 ++++++++++----------- arch/arm/plat-omap/include/plat/board-ams-delta.h | 1 - sound/soc/omap/ams-delta.c | 34 --------- 3 files changed, 36 insertions(+), 74 deletions(-)
diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c index 9b52aeb..945c4db 100644 --- a/arch/arm/mach-omap1/board-ams-delta.c +++ b/arch/arm/mach-omap1/board-ams-delta.c @@ -289,41 +289,6 @@ static struct platform_device modem_nreset_device = { }, };
-struct regulator_consumer_data { - struct mutex lock; - struct regulator *regulator; - bool enabled; -}; - -static int regulator_toggle(struct regulator_consumer_data *consumer, - bool enable) -{ - int err = 0; - - if (!consumer->regulator) - return -ENODEV; - - mutex_lock(&consumer->lock); - if (IS_ERR(consumer->regulator)) { - err = PTR_ERR(consumer->regulator); - } else if (enable) { - if (!consumer->enabled) { - err = regulator_enable(consumer->regulator); - consumer->enabled = true; - } - } else { - if (consumer->enabled) { - err = regulator_disable(consumer->regulator); - consumer->enabled = false; - } - } - mutex_unlock(&consumer->lock); - - return err; -} - -static struct regulator_consumer_data modem_nreset; - void ams_delta_latch_write(int base, int ngpio, u16 mask, u16 value) { int bit = 0; @@ -333,7 +298,7 @@ void ams_delta_latch_write(int base, int ngpio, u16 mask, u16 value) if (!(mask & bitpos)) continue; if (base + bit == AMS_DELTA_GPIO_PIN_MODEM_NRESET) - regulator_toggle(&modem_nreset, (value & bitpos) != 0); + continue; else gpio_set_value(base + bit, (value & bitpos) != 0); } @@ -528,6 +493,39 @@ static void __init ams_delta_init(void) omap_writew(omap_readw(ARM_RSTCT1) | 0x0004, ARM_RSTCT1); }
+struct regulator_consumer_data { + struct mutex lock; + struct regulator *regulator; + bool enabled; +}; + +static int regulator_toggle(struct regulator_consumer_data *consumer, + bool enable) +{ + int err = 0; + + if (!consumer->regulator) + return -ENODEV; + + mutex_lock(&consumer->lock); + if (IS_ERR(consumer->regulator)) { + err = PTR_ERR(consumer->regulator); + } else if (enable) { + if (!consumer->enabled) { + err = regulator_enable(consumer->regulator); + consumer->enabled = true; + } + } else { + if (consumer->enabled) { + err = regulator_disable(consumer->regulator); + consumer->enabled = false; + } + } + mutex_unlock(&consumer->lock); + + return err; +} + static void modem_pm(struct uart_port *port, unsigned int state, unsigned old) { struct regulator_consumer_data *consumer = port->private_data; @@ -536,6 +534,8 @@ static void modem_pm(struct uart_port *port, unsigned int state, unsigned old) regulator_toggle(consumer, state == 0); }
+static struct regulator_consumer_data modem_nreset; + static struct plat_serial8250_port ams_delta_modem_ports[] = { { .membase = IOMEM(MODEM_VIRT), @@ -602,9 +602,6 @@ static int __init late_init(void) /* * Once the modem device is registered, the modem_nreset * regulator can be requested on behalf of that device. - * In addition to the modem .pm callback, that regulator - * is still used via the ams_delta_latch_write() wrapper - * by the ASoC driver until updated. */ mutex_init(&modem_nreset.lock); modem_nreset.regulator = regulator_get(&ams_delta_modem_device.dev, diff --git a/arch/arm/plat-omap/include/plat/board-ams-delta.h b/arch/arm/plat-omap/include/plat/board-ams-delta.h index 027e79e..ad6f865 100644 --- a/arch/arm/plat-omap/include/plat/board-ams-delta.h +++ b/arch/arm/plat-omap/include/plat/board-ams-delta.h @@ -30,7 +30,6 @@
#define AMD_DELTA_LATCH2_SCARD_RSTIN 0x0400 #define AMD_DELTA_LATCH2_SCARD_CMDVCC 0x0800 -#define AMS_DELTA_LATCH2_MODEM_NRESET 0x1000 #define AMS_DELTA_LATCH2_MODEM_CODEC 0x2000
#define AMS_DELTA_GPIO_PIN_KEYBRD_DATA 0 diff --git a/sound/soc/omap/ams-delta.c b/sound/soc/omap/ams-delta.c index be81bc7..d213d04 100644 --- a/sound/soc/omap/ams-delta.c +++ b/sound/soc/omap/ams-delta.c @@ -426,31 +426,6 @@ static struct snd_soc_ops ams_delta_ops = { };
-/* Board specific codec bias level control */ -static int ams_delta_set_bias_level(struct snd_soc_card *card, - struct snd_soc_dapm_context *dapm, - enum snd_soc_bias_level level) -{ - struct snd_soc_codec *codec = card->rtd->codec; - - switch (level) { - case SND_SOC_BIAS_ON: - case SND_SOC_BIAS_PREPARE: - case SND_SOC_BIAS_STANDBY: - if (codec->dapm.bias_level == SND_SOC_BIAS_OFF) - ams_delta_latch2_write(AMS_DELTA_LATCH2_MODEM_NRESET, - AMS_DELTA_LATCH2_MODEM_NRESET); - break; - case SND_SOC_BIAS_OFF: - if (codec->dapm.bias_level != SND_SOC_BIAS_OFF) - ams_delta_latch2_write(AMS_DELTA_LATCH2_MODEM_NRESET, - 0); - } - codec->dapm.bias_level = level; - - return 0; -} - /* Digital mute implemented using modem/CPU multiplexer. * Shares hardware with codec config pulse generation */ static bool ams_delta_muted = 1; @@ -514,9 +489,6 @@ static int ams_delta_cx20442_init(struct snd_soc_pcm_runtime *rtd) ams_delta_ops.shutdown = ams_delta_shutdown; }
- /* Set codec bias level */ - ams_delta_set_bias_level(card, dapm, SND_SOC_BIAS_STANDBY); - /* Add hook switch - can be used to control the codec from userspace * even if line discipline fails */ ret = snd_soc_jack_new(rtd->codec, "hook_switch", @@ -599,7 +571,6 @@ static struct snd_soc_card ams_delta_audio_card = { .name = "AMS_DELTA", .dai_link = &ams_delta_dai_link, .num_links = 1, - .set_bias_level = ams_delta_set_bias_level, };
/* Module init/exit */ @@ -648,11 +619,6 @@ static void __exit ams_delta_module_exit(void) ARRAY_SIZE(ams_delta_hook_switch_gpios), ams_delta_hook_switch_gpios);
- /* Keep modem power on */ - ams_delta_set_bias_level(&ams_delta_audio_card, - &ams_delta_audio_card.rtd[0].codec->dapm, - SND_SOC_BIAS_STANDBY); - platform_device_unregister(cx20442_platform_device); platform_device_unregister(ams_delta_audio_platform_device); }
On Sat, Dec 24, 2011 at 12:12:24AM +0100, Janusz Krzysztofik wrote:
+struct regulator_consumer_data {
- struct mutex lock;
- struct regulator *regulator;
- bool enabled;
+};
+static int regulator_toggle(struct regulator_consumer_data *consumer,
bool enable)+{
- int err = 0;
- if (!consumer->regulator)
return -ENODEV;- mutex_lock(&consumer->lock);
- if (IS_ERR(consumer->regulator)) {
err = PTR_ERR(consumer->regulator);- } else if (enable) {
if (!consumer->enabled) {err = regulator_enable(consumer->regulator);consumer->enabled = true;}- } else {
if (consumer->enabled) {err = regulator_disable(consumer->regulator);consumer->enabled = false;}- }
- mutex_unlock(&consumer->lock);
- return err;
+}
Why's this code not been dropped and what is it for?
On Monday 26 of December 2011 at 12:03:49, Mark Brown wrote:
On Sat, Dec 24, 2011 at 12:12:24AM +0100, Janusz Krzysztofik wrote:
+struct regulator_consumer_data {
- struct mutex lock;
- struct regulator *regulator;
- bool enabled;
+};
+static int regulator_toggle(struct regulator_consumer_data *consumer,
bool enable)+{
- int err = 0;
- if (!consumer->regulator)
return -ENODEV;- mutex_lock(&consumer->lock);
- if (IS_ERR(consumer->regulator)) {
err = PTR_ERR(consumer->regulator);- } else if (enable) {
if (!consumer->enabled) {err = regulator_enable(consumer->regulator);consumer->enabled = true;}- } else {
if (consumer->enabled) {err = regulator_disable(consumer->regulator);consumer->enabled = false;}- }
- mutex_unlock(&consumer->lock);
- return err;
+}
Why's this code not been dropped and what is it for?
It is, and will be, used by another regulator consumer, the modem. The device is controlled by the generic serial8250 driver, extended with a custom .pm hook which enables/disables the regulator. The above code, temporarily shared with the ASoC device before updated, is now just moved down the board file where the modem related bits are located (see patch 3/4).
Any hints like that for the codec driver?
Thanks, Janusz
On Mon, Dec 26, 2011 at 01:30:22PM +0100, Janusz Krzysztofik wrote:
Why's this code not been dropped and what is it for?
It is, and will be, used by another regulator consumer, the modem. The device is controlled by the generic serial8250 driver, extended with a custom .pm hook which enables/disables the regulator. The above code, temporarily shared with the ASoC device before updated, is now just moved down the board file where the modem related bits are located (see patch 3/4).
Any hints like that for the codec driver?
The main thing that looks bad here is that you're keeping track of the state of the regulator by hand. It may be that the upper layers aren't reference counting in which case you might need this but it certainly looks very suspicious as-is.
On Monday 26 of December 2011 at 18:23:50, Mark Brown wrote:
On Mon, Dec 26, 2011 at 01:30:22PM +0100, Janusz Krzysztofik wrote:
Why's this code not been dropped and what is it for?
It is, and will be, used by another regulator consumer, the modem. The device is controlled by the generic serial8250 driver, extended with a custom .pm hook which enables/disables the regulator. The above code, temporarily shared with the ASoC device before updated, is now just moved down the board file where the modem related bits are located (see patch 3/4).
Any hints like that for the codec driver?
The main thing that looks bad here is that you're keeping track of the state of the regulator by hand. It may be that the upper layers aren't reference counting in which case you might need this but it certainly looks very suspicious as-is.
OK, I can try to examine how the serial8250 driver tracks the device power state when calling the .pm hook and if it looks safe to drop that regulator state manual tracking.
Thanks, Janusz
participants (2)
-
Janusz Krzysztofik -
Mark Brown