[PATCH 0/4] Amstrad Delta: access MODEM_RESET GPIO pin over a regulator

[PATCH 0/4] Amstrad Delta: access MODEM_RESET GPIO pin over a regulator

[Janusz Krzysztofik]

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(-)

-- 
1.7.3.4

[PATCH 1/4] ARM: OMAP1: ams-delta: set up regulator over modem reset GPIO pin

[Janusz Krzysztofik]

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;
-- 
1.7.3.4

[PATCH 2/4] ASoC: cx20442: add bias control over a platform provided regulator

[Janusz Krzysztofik]

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),
-- 
1.7.3.4

[PATCH 2/4] ASoC: cx20442: add bias control over a platform provided regulator

[Mark Brown]

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.

[alsa-devel] [PATCH 2/4] ASoC: cx20442: add bias control over a platform provided regulator

[Janusz Krzysztofik]

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

[alsa-devel] [PATCH 2/4] ASoC: cx20442: add bias control over a platform provided regulator

[Janusz Krzysztofik]

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

[alsa-devel] [PATCH 2/4] ASoC: cx20442: add bias control over a platform provided regulator

[Mark Brown]

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.

[PATCH 3/4] ARM: OMAP1: ams-delta: update the modem to use regulator API

[Janusz Krzysztofik]

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,
-- 
1.7.3.4

[PATCH 4/4] ASoC: OMAP: ams-delta: drop .set_bias_level callback

[Janusz Krzysztofik]

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);
 }
-- 
1.7.3.4

[PATCH 4/4] ASoC: OMAP: ams-delta: drop .set_bias_level callback

[Mark Brown]

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?

[PATCH 4/4] ASoC: OMAP: ams-delta: drop .set_bias_level callback

[Janusz Krzysztofik]

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

[PATCH 4/4] ASoC: OMAP: ams-delta: drop .set_bias_level callback

[Mark Brown]

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.

[PATCH 4/4] ASoC: OMAP: ams-delta: drop .set_bias_level callback

[Janusz Krzysztofik]

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