Skip to contentSkip to conversation

wm5110 Boot Fixups Part 2

PATCH In review Charles Keepax 1 participant 7 messages Mar 24, 2015 – Apr 3, 2015

Subsystems (2)
Multifunction devices (MFD) Wolfson microelectronics drivers
lore
Share & tools
lore

Charles Keepax

author
#0 lore raw

v2 cover wm5110 Boot Fixups Part 2

Hi,

This series replaces the reset patch Mark commented on in the early versions (see [1]) of the wm5110 boot series being reviewed at the moment.

Basically, this series allows the CODEC to completely power off if nothing is using it. This should cover the case where system suspend powers the regulators down causing a second cold boot.

NOTE: This series is dependent on the other series being reviewed starting with:

mfd: arizona: Factor out SYSCLK enable from wm5102 hardware patch

Thanks,
Charles

Changes since v1:
- Use devres for the reset GPIO

[1] https://lkml.org/lkml/2015/3/16/811

Charles Keepax (5):
mfd: arizona: Use devres to manage reset GPIO
mfd: wm5102: Move check for custom boot into arizona_wait_for_boot
mfd: arizona: Factor out hard reset into helper functions
mfd: arizona: Add better support for system suspend
mfd: wm5110: Add delay before releasing reset line

 drivers/mfd/arizona-core.c       |  143 +++++++++++++++++++++++++++-----------
 include/linux/mfd/arizona/core.h |    1 +
 2 files changed, 103 insertions(+), 41 deletions(-)
Signature · 1 line
1.7.2.5
author
#1 lore raw

PATCH v2 1/5 mfd: arizona: Use devres to manage reset GPIO

This also handily fixes a leak of the GPIO in arizona_dev_exit.

Signed-off-by Charles Keepax <[email protected]>
---
 drivers/mfd/arizona-core.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)
drivers/mfd/arizona-core.c +4 -6
index 01fb9eb..c490fe9 100644
@@ -853,9 +853,9 @@ int arizona_dev_init(struct arizona *arizona)
 
 	if (arizona->pdata.reset) {
 		/* Start out with /RESET low to put the chip into reset */
-		ret = gpio_request_one(arizona->pdata.reset,
-				       GPIOF_DIR_OUT | GPIOF_INIT_LOW,
-				       "arizona /RESET");
+		ret = devm_gpio_request_one(arizona->dev, arizona->pdata.reset,
+					    GPIOF_DIR_OUT | GPIOF_INIT_LOW,
+					    "arizona /RESET");
 		if (ret != 0) {
 			dev_err(dev, "Failed to request /RESET: %d\n", ret);
 			goto err_dcvdd;

@@ -1189,10 +1189,8 @@ int arizona_dev_init(struct arizona *arizona)
 err_irq:
 	arizona_irq_exit(arizona);
 err_reset:
-	if (arizona->pdata.reset) {
+	if (arizona->pdata.reset)
 		gpio_set_value_cansleep(arizona->pdata.reset, 0);
-		gpio_free(arizona->pdata.reset);
-	}
 	regulator_disable(arizona->dcvdd);
 err_enable:
 	regulator_bulk_disable(arizona->num_core_supplies,
Signature · 1 line
1.7.2.5
author
#2 lore raw

PATCH v2 2/5 mfd: wm5102: Move check for custom boot into arizona_wait_for_boot

In reply to Charles Keepax

It is better to have the arizona_wait_for_boot function behave sensibly for all devices rather than having to have special handling outside of it for wm5102. As such move the code which checks for wm5102's custom boot into arizona_wait_for_boot, essentially making it a no-op on wm5102 once the custom boot is activated.

Signed-off-by Charles Keepax <[email protected]>
---
 drivers/mfd/arizona-core.c |   54 +++++++++++++++++++++----------------------
 1 files changed, 26 insertions(+), 28 deletions(-)
drivers/mfd/arizona-core.c +26 -28
index c490fe9..710e9cc 100644
@@ -232,6 +232,23 @@ static int arizona_poll_reg(struct arizona *arizona,
 static int arizona_wait_for_boot(struct arizona *arizona)
 {
 	int ret;
+	unsigned int val;
+
+	/* Check for wm5102 custom boot */
+	switch (arizona->type) {
+	case WM5102:
+		ret = regmap_read(arizona->regmap,
+				  ARIZONA_WRITE_SEQUENCER_CTRL_3, &val);
+		if (ret)
+			dev_err(arizona->dev,
+				"Failed to check write sequencer state: %d\n",
+				ret);
+		else if (val & 0x01)
+			return 0;
+		break;
+	default:
+		break;
+	}
 
 	/*
 	 * We can't use an interrupt as we need to runtime resume to do so,

@@ -419,6 +436,10 @@ static int arizona_runtime_resume(struct device *dev)
 
 	regcache_cache_only(arizona->regmap, false);
 
+	ret = arizona_wait_for_boot(arizona);
+	if (ret)
+		goto err;
+
 	switch (arizona->type) {
 	case WM5102:
 		if (arizona->external_dcvdd) {

@@ -449,10 +470,6 @@ static int arizona_runtime_resume(struct device *dev)
 		break;
 	case WM5110:
 	case WM8280:
-		ret = arizona_wait_for_boot(arizona);
-		if (ret)
-			goto err;
-
 		if (arizona->external_dcvdd) {
 			ret = regmap_update_bits(arizona->regmap,
 						 ARIZONA_ISOLATION_CONTROL,

@@ -479,11 +496,6 @@ static int arizona_runtime_resume(struct device *dev)
 		}
 		break;
 	default:
-		ret = arizona_wait_for_boot(arizona);
-		if (ret != 0) {
-			goto err;
-		}
-
 		if (arizona->external_dcvdd) {
 			ret = regmap_update_bits(arizona->regmap,
 						 ARIZONA_ISOLATION_CONTROL,

@@ -920,25 +932,11 @@ int arizona_dev_init(struct arizona *arizona)
 	}
 
 	/* Ensure device startup is complete */
-	switch (arizona->type) {
-	case WM5102:
-		ret = regmap_read(arizona->regmap,
-				  ARIZONA_WRITE_SEQUENCER_CTRL_3, &val);
-		if (ret != 0)
-			dev_err(dev,
-				"Failed to check write sequencer state: %d\n",
-				ret);
-		else if (val & 0x01)
-			break;
-		/* Fall through */
-	default:
-		ret = arizona_wait_for_boot(arizona);
-		if (ret != 0) {
-			dev_err(arizona->dev,
-				"Device failed initial boot: %d\n", ret);
-			goto err_reset;
-		}
-		break;
+	ret = arizona_wait_for_boot(arizona);
+	if (ret) {
+		dev_err(arizona->dev,
+			"Device failed initial boot: %d\n", ret);
+		goto err_reset;
 	}
 
 	/* Read the device ID information & do device specific stuff */
Signature · 1 line
1.7.2.5
author
#3 lore raw

PATCH v2 3/5 mfd: arizona: Factor out hard reset into helper functions

In reply to Charles Keepax

This patch adds functions for enabling and disabling the physical reset line. This will be helpful in future refactoring.

Signed-off-by Charles Keepax <[email protected]>
---
 drivers/mfd/arizona-core.c |   25 +++++++++++++++++--------
 1 files changed, 17 insertions(+), 8 deletions(-)
drivers/mfd/arizona-core.c +17 -8
index 710e9cc..de4a5db 100644
@@ -267,6 +267,20 @@ static int arizona_wait_for_boot(struct arizona *arizona)
 	return ret;
 }
 
+static inline void arizona_enable_reset(struct arizona *arizona)
+{
+	if (arizona->pdata.reset)
+		gpio_set_value_cansleep(arizona->pdata.reset, 0);
+}
+
+static void arizona_disable_reset(struct arizona *arizona)
+{
+	if (arizona->pdata.reset) {
+		gpio_set_value_cansleep(arizona->pdata.reset, 1);
+		msleep(1);
+	}
+}
+
 struct arizona_sysclk_state {
 	unsigned int fll;
 	unsigned int sysclk;

@@ -888,10 +902,7 @@ int arizona_dev_init(struct arizona *arizona)
 		goto err_enable;
 	}
 
-	if (arizona->pdata.reset) {
-		gpio_set_value_cansleep(arizona->pdata.reset, 1);
-		msleep(1);
-	}
+	arizona_disable_reset(arizona);
 
 	regcache_cache_only(arizona->regmap, false);
 

@@ -1187,8 +1198,7 @@ int arizona_dev_init(struct arizona *arizona)
 err_irq:
 	arizona_irq_exit(arizona);
 err_reset:
-	if (arizona->pdata.reset)
-		gpio_set_value_cansleep(arizona->pdata.reset, 0);
+	arizona_enable_reset(arizona);
 	regulator_disable(arizona->dcvdd);
 err_enable:
 	regulator_bulk_disable(arizona->num_core_supplies,

@@ -1213,8 +1223,7 @@ int arizona_dev_exit(struct arizona *arizona)
 	arizona_free_irq(arizona, ARIZONA_IRQ_OVERCLOCKED, arizona);
 	arizona_free_irq(arizona, ARIZONA_IRQ_CLKGEN_ERR, arizona);
 	arizona_irq_exit(arizona);
-	if (arizona->pdata.reset)
-		gpio_set_value_cansleep(arizona->pdata.reset, 0);
+	arizona_enable_reset(arizona);
 
 	regulator_bulk_disable(arizona->num_core_supplies,
 			       arizona->core_supplies);
Signature · 1 line
1.7.2.5
author
#4 lore raw

PATCH v2 4/5 mfd: arizona: Add better support for system suspend

In reply to Charles Keepax

Allow the chip to completely power off if we enter runtime suspend and there is no jack detection active. This is helpful for systems where system suspend might remove the supplies to the CODEC, without informing us. Note the powering off is done in runtime suspend rather than system suspend, because we need to hold reset until the first time DCVDD is powered anyway (which would be in runtime resume), and we might as well save the extra power.

Signed-off-by Charles Keepax <[email protected]>
---
 drivers/mfd/arizona-core.c       |   47 ++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/arizona/core.h |    1 +
 2 files changed, 48 insertions(+), 0 deletions(-)
drivers/mfd/arizona-core.c +47 -0
index de4a5db..ba48e17 100644
@@ -442,12 +442,32 @@ static int arizona_runtime_resume(struct device *dev)
 
 	dev_dbg(arizona->dev, "Leaving AoD mode\n");
 
+	if (arizona->has_fully_powered_off) {
+		dev_dbg(arizona->dev, "Re-enabling core supplies\n");
+
+		ret = regulator_bulk_enable(arizona->num_core_supplies,
+					    arizona->core_supplies);
+		if (ret) {
+			dev_err(dev, "Failed to enable core supplies: %d\n",
+				ret);
+			return ret;
+		}
+	}
+
 	ret = regulator_enable(arizona->dcvdd);
 	if (ret != 0) {
 		dev_err(arizona->dev, "Failed to enable DCVDD: %d\n", ret);
+		if (arizona->has_fully_powered_off)
+			regulator_bulk_disable(arizona->num_core_supplies,
+					       arizona->core_supplies);
 		return ret;
 	}
 
+	if (arizona->has_fully_powered_off) {
+		arizona_disable_reset(arizona);
+		arizona->has_fully_powered_off = false;
+	}
+
 	regcache_cache_only(arizona->regmap, false);
 
 	ret = arizona_wait_for_boot(arizona);

@@ -508,6 +528,14 @@ static int arizona_runtime_resume(struct device *dev)
 				goto err;
 			}
 		}
+
+		ret = wm5110_apply_sleep_patch(arizona);
+		if (ret) {
+			dev_err(arizona->dev,
+				"Failed to re-apply sleep patch: %d\n",
+				ret);
+			goto err;
+		}
 		break;
 	default:
 		if (arizona->external_dcvdd) {

@@ -540,10 +568,17 @@ err:
 static int arizona_runtime_suspend(struct device *dev)
 {
 	struct arizona *arizona = dev_get_drvdata(dev);
+	unsigned int val;
 	int ret;
 
 	dev_dbg(arizona->dev, "Entering AoD mode\n");
 
+	ret = regmap_read(arizona->regmap, ARIZONA_JACK_DETECT_ANALOGUE, &val);
+	if (ret) {
+		dev_err(dev, "Failed to check jack det status: %d\n", ret);
+		return ret;
+	}
+
 	if (arizona->external_dcvdd) {
 		ret = regmap_update_bits(arizona->regmap,
 					 ARIZONA_ISOLATION_CONTROL,

@@ -581,6 +616,18 @@ static int arizona_runtime_suspend(struct device *dev)
 	regcache_mark_dirty(arizona->regmap);
 	regulator_disable(arizona->dcvdd);
 
+	/* Allow us to completely power down if no jack detection */
+	if (!(val & ARIZONA_JD1_ENA)) {
+		dev_dbg(arizona->dev, "Fully powering off\n");
+
+		arizona->has_fully_powered_off = true;
+
+		arizona_enable_reset(arizona);
+
+		regulator_bulk_disable(arizona->num_core_supplies,
+				       arizona->core_supplies);
+	}
+
 	return 0;
 }
 #endif
include/linux/mfd/arizona/core.h +1 -0
index f970105..7c210af 100644
@@ -117,6 +117,7 @@ struct arizona {
 	int num_core_supplies;
 	struct regulator_bulk_data core_supplies[ARIZONA_MAX_CORE_SUPPLIES];
 	struct regulator *dcvdd;
+	bool has_fully_powered_off;
 
 	struct arizona_pdata pdata;
Signature · 1 line
1.7.2.5
author
#5 lore raw

Re: [PATCH v2 4/5] mfd: arizona: Add better support for system suspend

Quoted 16 lines · Charles Keepax

Allow the chip to completely power off if we enter runtime suspend and there is no jack detection active. This is helpful for systems where system suspend might remove the supplies to the CODEC, without informing us. Note the powering off is done in runtime suspend rather than system suspend, because we need to hold reset until the first time DCVDD is powered anyway (which would be in runtime resume), and we might as well save the extra power.

Signed-off-by Charles Keepax <[email protected]>

---

drivers/mfd/arizona-core.c +2 -0
@@ -581,6 +616,18 @@ static int arizona_runtime_suspend(struct device *dev)
 	regcache_mark_dirty(arizona->regmap);
 	regulator_disable(arizona->dcvdd);
 
+	/* Allow us to completely power down if no jack detection */
+	if (!(val & ARIZONA_JD1_ENA)) {

drivers/mfd/arizona-core.c static int arizona_runtime_suspend(struct device *dev) · line 630

Quoted 10 lines
drivers/mfd/arizona-core.c +10 -0
+		dev_dbg(arizona->dev, "Fully powering off\n");
+
+		arizona->has_fully_powered_off = true;
+
+		arizona_enable_reset(arizona);
+
+		regulator_bulk_disable(arizona->num_core_supplies,
+				       arizona->core_supplies);
+	}
+

Turns out we will need to disable the IRQ across this as well, we lose control of it once we put the chip into reset. So depending on pulls etc. we might get pulled straight out of suspend again if we leave the IRQ enabled.

I will send a new series shortly.

Thanks,
Charles

author
#6 lore raw

PATCH v2 5/5 mfd: wm5110: Add delay before releasing reset line

In reply to Charles Keepax

On the wm5110 it is important the reset line is held for slightly longer to ensure the device starts up well. This patch adds a 5mS delay for this.

Signed-off-by Charles Keepax <[email protected]>
---
 drivers/mfd/arizona-core.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)
drivers/mfd/arizona-core.c +9 -0
index ba48e17..54da62d 100644
@@ -276,6 +276,15 @@ static inline void arizona_enable_reset(struct arizona *arizona)
 static void arizona_disable_reset(struct arizona *arizona)
 {
 	if (arizona->pdata.reset) {
+		switch (arizona->type) {
+		case WM5110:
+		case WM8280:
+			msleep(5);
+			break;
+		default:
+			break;
+		}
+
 		gpio_set_value_cansleep(arizona->pdata.reset, 1);
 		msleep(1);
 	}
Signature · 1 line
1.7.2.5