2012-02-07 19:49:45

by Janusz Krzysztofik

[permalink] [raw]
Subject: [RESUBMIT][PATCH v2 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.

I'm resubmitting v2 of this series, originally posted on 30 Dec 2011,
with Mark's Ack already collected, now rebased on top of current
linux-omap/omap1 tip, commit 967809bd7faf71ddc29c8081e0f21db8b201a0f4.

Since patch 2/4, "ASoC: cx20442: add bias control over a platform
provided regulator", has already been applied by Liam (thanks!) and is
already present in mainline, I've omitted it from this submission.

v2 changes against initial version:
* in both the codec and the modem callbacks, don't track the regulator
enable/disable state, compare new target bias level (the codec case)
or power state (the modem case) with the old value instead; thanks to
Mark Brown who suggested this solution,
* a few other minor changes, mostly stylistic.

Janusz Krzysztofik (3):
ARM: OMAP1: ams-delta: set up regulator over modem reset GPIO pin
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 | 94 +++++++++++++++++++--
arch/arm/plat-omap/include/plat/board-ams-delta.h | 1 -
sound/soc/omap/ams-delta.c | 32 -------
4 files changed, 87 insertions(+), 42 deletions(-)

--
1.7.3.4


2012-02-07 19:20:12

by Janusz Krzysztofik

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

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 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 consumer data needed for tracking the regulator state,
will be removed once the drivers are updated.

Depends on patch series "ARM: OMAP1: ams-delta: replace custom I/O with
GPIO".

Signed-off-by: Janusz Krzysztofik <[email protected]>
Cc: Tony Lindgren <[email protected]>
---
Changes against initial version:
* rename consumer setup elements to match their final, modem only
related purpose,
* initialize the regulator pointer and mutex before first use, then
omit testing that pointer against NULL value.

arch/arm/mach-omap1/Kconfig | 2 +
arch/arm/mach-omap1/board-ams-delta.c | 104 +++++++++++++++++++++++++++++++--
2 files changed, 100 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 87b1303..43dcbda 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,71 @@ 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 modem_private_data {
+ struct regulator *regulator;
+ struct {
+ struct mutex lock;
+ bool enabled;
+ } consumer;
+};
+
+static int regulator_toggle(struct modem_private_data *priv, bool enable)
+{
+ int err = 0;
+
+ mutex_lock(&priv->consumer.lock);
+ if (IS_ERR(priv->regulator)) {
+ err = PTR_ERR(priv->regulator);
+ } else if (enable) {
+ if (!priv->consumer.enabled) {
+ err = regulator_enable(priv->regulator);
+ priv->consumer.enabled = true;
+ }
+ } else {
+ if (priv->consumer.enabled) {
+ err = regulator_disable(priv->regulator);
+ priv->consumer.enabled = false;
+ }
+ }
+ mutex_unlock(&priv->consumer.lock);
+
+ return err;
+}
+
+static struct modem_private_data modem_priv;
+
void ams_delta_latch_write(int base, int ngpio, u16 mask, u16 value)
{
int bit = 0;
@@ -266,7 +330,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);
+ else if (base + bit == AMS_DELTA_GPIO_PIN_MODEM_NRESET)
+ regulator_toggle(&modem_priv, (value & bitpos) != 0);
+ else
+ gpio_set_value(base + bit, (value & bitpos) != 0);
}
}
EXPORT_SYMBOL(ams_delta_latch_write);
@@ -496,6 +563,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);
@@ -507,6 +580,10 @@ static int __init late_init(void)
}
gpio_direction_input(AMS_DELTA_GPIO_PIN_MODEM_IRQ);

+ /* Initialize the modem_nreset regulator consumer before use */
+ mutex_init(&modem_priv.consumer.lock);
+ modem_priv.regulator = ERR_PTR(-ENODEV);
+
ams_delta_latch2_write(
AMS_DELTA_LATCH2_MODEM_NRESET | AMS_DELTA_LATCH2_MODEM_CODEC,
AMS_DELTA_LATCH2_MODEM_NRESET | AMS_DELTA_LATCH2_MODEM_CODEC);
@@ -514,8 +591,23 @@ 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.
+ */
+ modem_priv.regulator = regulator_get(&ams_delta_modem_device.dev,
+ "RESET#");
+ if (IS_ERR(modem_priv.regulator)) {
+ err = PTR_ERR(modem_priv.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

2012-02-07 19:20:18

by Janusz Krzysztofik

[permalink] [raw]
Subject: [RESUBMIT][PATCH v2 3/4] ARM: OMAP1: ams-delta: update the modem to use regulator API

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 a regulator over the
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.

Signed-off-by: Janusz Krzysztofik <[email protected]>
Cc: Tony Lindgren <[email protected]>
---
No functional changes against initial version.

arch/arm/mach-omap1/board-ams-delta.c | 22 +++++++++++++++++-----
1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 43dcbda..cfcf871 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -526,6 +526,16 @@ 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 modem_private_data *priv = port->private_data;
+
+ if (state == old)
+ return;
+
+ regulator_toggle(priv, state == 0);
+}
+
static struct plat_serial8250_port ams_delta_modem_ports[] = {
{
.membase = IOMEM(MODEM_VIRT),
@@ -536,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_priv,
},
{ },
};
@@ -584,9 +596,8 @@ static int __init late_init(void)
mutex_init(&modem_priv.consumer.lock);
modem_priv.regulator = ERR_PTR(-ENODEV);

- 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)
@@ -595,8 +606,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.
*/
modem_priv.regulator = regulator_get(&ams_delta_modem_device.dev,
"RESET#");
--
1.7.3.4

2012-02-07 19:20:26

by Janusz Krzysztofik

[permalink] [raw]
Subject: [RESUBMIT][PATCH v2 4/4] ASoC: OMAP: ams-delta: drop .set_bias_level callback

This functionality has just been implemented in the cx20442 codec
driver, no need to keep it here duplicated.

Once done, remove the no longer used AMS_DELTA_LATCH2_MODEM_NRESET
symbol from the board 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, simplify the way the modem .pm callback handles the
regulator and drop that helper function and its related consumer setup
completely.

Signed-off-by: Janusz Krzysztofik <[email protected]>
Acked-by: Mark Brown <[email protected]>
Cc: Tony Lindgren <[email protected]>
---
Changes against initial version:
* don't move consumer setup elements, now named to indicated their
modem related purpose, down the file,
* don't track the regulator enavble/disable state, compare new target
power state with the old one instead; thanks to Mark Brown for
suggesting this simplification,
* actually drop all references to AMS_DELTA_LATCH2_MODEM_NRESET.

arch/arm/mach-omap1/board-ams-delta.c | 42 +++-----------------
arch/arm/plat-omap/include/plat/board-ams-delta.h | 1 -
sound/soc/omap/ams-delta.c | 32 ----------------
3 files changed, 7 insertions(+), 68 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index cfcf871..6ab5331 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -18,7 +18,6 @@
#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>
@@ -291,35 +290,8 @@ static struct platform_device modem_nreset_device = {

struct modem_private_data {
struct regulator *regulator;
- struct {
- struct mutex lock;
- bool enabled;
- } consumer;
};

-static int regulator_toggle(struct modem_private_data *priv, bool enable)
-{
- int err = 0;
-
- mutex_lock(&priv->consumer.lock);
- if (IS_ERR(priv->regulator)) {
- err = PTR_ERR(priv->regulator);
- } else if (enable) {
- if (!priv->consumer.enabled) {
- err = regulator_enable(priv->regulator);
- priv->consumer.enabled = true;
- }
- } else {
- if (priv->consumer.enabled) {
- err = regulator_disable(priv->regulator);
- priv->consumer.enabled = false;
- }
- }
- mutex_unlock(&priv->consumer.lock);
-
- return err;
-}
-
static struct modem_private_data modem_priv;

void ams_delta_latch_write(int base, int ngpio, u16 mask, u16 value)
@@ -330,8 +302,6 @@ void ams_delta_latch_write(int base, int ngpio, u16 mask, u16 value)
for (; bit < ngpio; bit++, bitpos = bitpos << 1) {
if (!(mask & bitpos))
continue;
- else if (base + bit == AMS_DELTA_GPIO_PIN_MODEM_NRESET)
- regulator_toggle(&modem_priv, (value & bitpos) != 0);
else
gpio_set_value(base + bit, (value & bitpos) != 0);
}
@@ -530,10 +500,16 @@ static void modem_pm(struct uart_port *port, unsigned int state, unsigned old)
{
struct modem_private_data *priv = port->private_data;

+ if (IS_ERR(priv->regulator))
+ return;
+
if (state == old)
return;

- regulator_toggle(priv, state == 0);
+ if (state == 0)
+ regulator_enable(priv->regulator);
+ else if (old == 0)
+ regulator_disable(priv->regulator);
}

static struct plat_serial8250_port ams_delta_modem_ports[] = {
@@ -593,7 +569,6 @@ static int __init late_init(void)
gpio_direction_input(AMS_DELTA_GPIO_PIN_MODEM_IRQ);

/* Initialize the modem_nreset regulator consumer before use */
- mutex_init(&modem_priv.consumer.lock);
modem_priv.regulator = ERR_PTR(-ENODEV);

ams_delta_latch2_write(AMS_DELTA_LATCH2_MODEM_CODEC,
@@ -606,9 +581,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.
*/
modem_priv.regulator = regulator_get(&ams_delta_modem_device.dev,
"RESET#");
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 da6e005..f610260 100644
--- a/sound/soc/omap/ams-delta.c
+++ b/sound/soc/omap/ams-delta.c
@@ -426,29 +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)
-{
- switch (level) {
- case SND_SOC_BIAS_ON:
- case SND_SOC_BIAS_PREPARE:
- case SND_SOC_BIAS_STANDBY:
- if (card->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 (card->dapm.bias_level != SND_SOC_BIAS_OFF)
- ams_delta_latch2_write(AMS_DELTA_LATCH2_MODEM_NRESET,
- 0);
- }
- card->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;
@@ -512,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",
@@ -598,7 +572,6 @@ static struct snd_soc_card ams_delta_audio_card = {
.owner = THIS_MODULE,
.dai_link = &ams_delta_dai_link,
.num_links = 1,
- .set_bias_level = ams_delta_set_bias_level,
};

/* Module init/exit */
@@ -647,11 +620,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