Skip to contentSkip to conversation

wm5110 Boot Fixups Part 2

PATCH Superseded Charles Keepax 2 participants 8 messages Mar 24, 2015

Subsystems (2)
Multifunction devices (MFD) Wolfson microelectronics drivers

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Charles Keepax

author
#0 lore raw

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

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

Charles Keepax (5):
mfd: arizona: Add missing gpio_free in arizona_dev_exit
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       |  137 ++++++++++++++++++++++++++++----------
 include/linux/mfd/arizona/core.h |    1 +
 2 files changed, 102 insertions(+), 36 deletions(-)
Signature · 1 line
1.7.2.5
author
#1 lore raw

PATCH 1/5 mfd: arizona: Add missing gpio_free in arizona_dev_exit

Signed-off-by Charles Keepax <[email protected]>
---
 drivers/mfd/arizona-core.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
drivers/mfd/arizona-core.c +3 -1
index 01fb9eb..f0dbaa6 100644
@@ -1217,8 +1217,10 @@ 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)
+	if (arizona->pdata.reset) {
 		gpio_set_value_cansleep(arizona->pdata.reset, 0);
+		gpio_free(arizona->pdata.reset);
+	}
 
 	regulator_bulk_disable(arizona->num_core_supplies,
 			       arizona->core_supplies);
Signature · 1 line
1.7.2.5
#2 lore raw

Re: [PATCH 1/5] mfd: arizona: Add missing gpio_free in arizona_dev_exit

drivers/mfd/arizona-core.c int arizona_dev_exit(struct arizona *arizona) · line 1223

Quoted 5 lines · Charles Keepax
drivers/mfd/arizona-core.c +3 -1
-	if (arizona->pdata.reset)
+	if (arizona->pdata.reset) {
 		gpio_set_value_cansleep(arizona->pdata.reset, 0);
+		gpio_free(arizona->pdata.reset);
+	}

Or acquire the GPIO with devm_ and avoid having to explicitly free it at all?

author
#3 lore raw

Re: [PATCH 1/5] mfd: arizona: Add missing gpio_free in arizona_dev_exit

Quoted 10 lines · Mark Brown

drivers/mfd/arizona-core.c int arizona_dev_exit(struct arizona *arizona) · line 1223

Quoted 5 lines · Charles Keepax
drivers/mfd/arizona-core.c +3 -1
-	if (arizona->pdata.reset)
+	if (arizona->pdata.reset) {
 		gpio_set_value_cansleep(arizona->pdata.reset, 0);
+		gpio_free(arizona->pdata.reset);
+	}

Or acquire the GPIO with devm_ and avoid having to explicitly free it at all?

Ooops... yeah will respin for that.

Thanks,
Charles

author
#4 lore raw

PATCH 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 f0dbaa6..aaaecaa 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
#5 lore raw

PATCH 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 |   29 +++++++++++++++++++----------
 1 files changed, 19 insertions(+), 10 deletions(-)
drivers/mfd/arizona-core.c +19 -10
index aaaecaa..181da79 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,10 +1198,9 @@ 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);
+	if (arizona->pdata.reset)
 		gpio_free(arizona->pdata.reset);
-	}
 	regulator_disable(arizona->dcvdd);
 err_enable:
 	regulator_bulk_disable(arizona->num_core_supplies,

@@ -1215,10 +1225,9 @@ 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);
+	if (arizona->pdata.reset)
 		gpio_free(arizona->pdata.reset);
-	}
 
 	regulator_bulk_disable(arizona->num_core_supplies,
 			       arizona->core_supplies);
Signature · 1 line
1.7.2.5
author
#6 lore raw

PATCH 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 181da79..c3973a0 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
#7 lore raw

PATCH 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 c3973a0..4de5bf4 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