Arizona clean up driver removal and error paths

PATCH Applied Charles Keepax 2 participants 12 messages Jun 2, 2014 – Jun 17, 2014

Subsystems (2)

Multifunction devices (MFD) Wolfson microelectronics drivers

lore

Share & tools

lore

Charles Keepax

author

2014-06-02 08:58 UTC #0 lore raw

cover Arizona clean up driver removal and error paths

Several small fixes to the error and driver removal paths for the arizona devices. This series will likely need to be applied onto of my earlier "wm5110 Interrupt Updates" series to apply cleanly, please let me know if I need to resend that. It would also be possible to rebase them the other way round although that will obviously require me to resend the interrupt series.

Thanks,
Charles

Charles Keepax (5):
mfd: arizona: Disable PM runtime at start of driver removal
mfd: arizona: Disable DCVDD before we destroy the MFD
mfd: arizona: Don't use devres for DCVDD
mfd: arizona: Use num_core_supplies in arizona_dev_exit
mfd: arizona: Lower ARIZONA_MAX_CORE_SUPPLIES to 2

 drivers/mfd/arizona-core.c       |   18 ++++++++++++------
 include/linux/mfd/arizona/core.h |    2 +-
 2 files changed, 13 insertions(+), 7 deletions(-)

Signature · 1 line

1.7.2.5

Charles Keepax

author

2014-06-02 08:58 UTC #1 lore raw

PATCH 1/5 mfd: arizona: Disable PM runtime at start of driver removal

We don't want to trigger any PM runtime operations whilst we are tearing down the driver, as things the suspend and resume callbacks rely on might already have been destroyed. So disable PM runtime for the device as the first step arizona_dev_exit.

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

---
 drivers/mfd/arizona-core.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

drivers/mfd/arizona-core.c +2 -1

index cabe8f4..7bdc0f5 100644
@@ -1024,11 +1024,12 @@ EXPORT_SYMBOL_GPL(arizona_dev_init);
 
 int arizona_dev_exit(struct arizona *arizona)
 {
+	pm_runtime_disable(arizona->dev);
+
 	mfd_remove_devices(arizona->dev);
 	arizona_free_irq(arizona, ARIZONA_IRQ_UNDERCLOCKED, arizona);
 	arizona_free_irq(arizona, ARIZONA_IRQ_OVERCLOCKED, arizona);
 	arizona_free_irq(arizona, ARIZONA_IRQ_CLKGEN_ERR, arizona);
-	pm_runtime_disable(arizona->dev);
 	arizona_irq_exit(arizona);
 	if (arizona->pdata.reset)
 		gpio_set_value_cansleep(arizona->pdata.reset, 0);

Signature · 1 line

1.7.2.5

Lee Jones

Applied

2014-06-17 14:33 UTC #2 lore raw

Re: [PATCH 1/5] mfd: arizona: Disable PM runtime at start of driver removal

Quoted 9 lines

We don't want to trigger any PM runtime operations whilst we are tearing down the driver, as things the suspend and resume callbacks rely on might already have been destroyed. So disable PM runtime for the device as the first step arizona_dev_exit.
Signed-off-by Charles Keepax <[email protected]>
---
 drivers/mfd/arizona-core.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Applied, thanks.

Quoted 18 lines

drivers/mfd/arizona-core.c +2 -1

index cabe8f4..7bdc0f5 100644
@@ -1024,11 +1024,12 @@ EXPORT_SYMBOL_GPL(arizona_dev_init);
 
 int arizona_dev_exit(struct arizona *arizona)
 {
+	pm_runtime_disable(arizona->dev);
+
 	mfd_remove_devices(arizona->dev);
 	arizona_free_irq(arizona, ARIZONA_IRQ_UNDERCLOCKED, arizona);
 	arizona_free_irq(arizona, ARIZONA_IRQ_OVERCLOCKED, arizona);
 	arizona_free_irq(arizona, ARIZONA_IRQ_CLKGEN_ERR, arizona);
-	pm_runtime_disable(arizona->dev);
 	arizona_irq_exit(arizona);
 	if (arizona->pdata.reset)
 		gpio_set_value_cansleep(arizona->pdata.reset, 0);

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Charles Keepax

author

2014-06-02 08:58 UTC #3 lore raw

PATCH 2/5 mfd: arizona: Disable DCVDD before we destroy the MFD

In reply to Charles Keepax

As DCVDD is probably supplied by a child of the MFD device move its disable to before we destroy the MFD children as the regulator likely won't exist after that.

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 7bdc0f5..48acac6 100644
@@ -1026,6 +1026,8 @@ int arizona_dev_exit(struct arizona *arizona)
 {
 	pm_runtime_disable(arizona->dev);
 
+	regulator_disable(arizona->dcvdd);
+
 	mfd_remove_devices(arizona->dev);
 	arizona_free_irq(arizona, ARIZONA_IRQ_UNDERCLOCKED, arizona);
 	arizona_free_irq(arizona, ARIZONA_IRQ_OVERCLOCKED, arizona);

@@ -1033,7 +1035,7 @@ int arizona_dev_exit(struct arizona *arizona)
 	arizona_irq_exit(arizona);
 	if (arizona->pdata.reset)
 		gpio_set_value_cansleep(arizona->pdata.reset, 0);
-	regulator_disable(arizona->dcvdd);
+
 	regulator_bulk_disable(ARRAY_SIZE(arizona->core_supplies),
 			       arizona->core_supplies);
 	return 0;

Signature · 1 line

1.7.2.5

Lee Jones

Applied

2014-06-17 14:34 UTC #4 lore raw

Re: [PATCH 2/5] mfd: arizona: Disable DCVDD before we destroy the MFD

Quoted 8 lines · Charles Keepax

As DCVDD is probably supplied by a child of the MFD device move its disable to before we destroy the MFD children as the regulator likely won't exist after that.
Signed-off-by Charles Keepax <[email protected]>
---
 drivers/mfd/arizona-core.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

Applied, thanks.

Quoted 22 lines

drivers/mfd/arizona-core.c +3 -1

index 7bdc0f5..48acac6 100644
@@ -1026,6 +1026,8 @@ int arizona_dev_exit(struct arizona *arizona)
 {
 	pm_runtime_disable(arizona->dev);
 
+	regulator_disable(arizona->dcvdd);
+
 	mfd_remove_devices(arizona->dev);
 	arizona_free_irq(arizona, ARIZONA_IRQ_UNDERCLOCKED, arizona);
 	arizona_free_irq(arizona, ARIZONA_IRQ_OVERCLOCKED, arizona);

@@ -1033,7 +1035,7 @@ int arizona_dev_exit(struct arizona *arizona)
 	arizona_irq_exit(arizona);
 	if (arizona->pdata.reset)
 		gpio_set_value_cansleep(arizona->pdata.reset, 0);
-	regulator_disable(arizona->dcvdd);
+
 	regulator_bulk_disable(ARRAY_SIZE(arizona->core_supplies),
 			       arizona->core_supplies);
 	return 0;

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Charles Keepax

author

2014-06-02 08:59 UTC #5 lore raw

PATCH 3/5 mfd: arizona: Don't use devres for DCVDD

In reply to Charles Keepax

Currently the Arizona core uses a devm_regulator_get against its own device node to obtain DCVDD. The Arizona core is an MFD device and DCVDD is usually supplied by a child node (arizona-ldo1) of the core. As devres destruction for the MFD device will run after all its children have been destroyed, the regulator will be destroyed before devres calls regulator_put. This causes a warning from both the destruction of the child node, as the regulator is still open, and from the put of the regulator as the regulator device has already been destroyed.

This patch handles the regulator get and put without devres to avoid this issue.

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

---
 drivers/mfd/arizona-core.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

drivers/mfd/arizona-core.c +6 -3

index 48acac6..7678e84 100644
@@ -676,7 +676,7 @@ int arizona_dev_init(struct arizona *arizona)
 		goto err_early;
 	}
 
-	arizona->dcvdd = devm_regulator_get(arizona->dev, "DCVDD");
+	arizona->dcvdd = regulator_get(arizona->dev, "DCVDD");
 	if (IS_ERR(arizona->dcvdd)) {
 		ret = PTR_ERR(arizona->dcvdd);
 		dev_err(dev, "Failed to request DCVDD: %d\n", ret);

@@ -690,7 +690,7 @@ int arizona_dev_init(struct arizona *arizona)
 				       "arizona /RESET");
 		if (ret != 0) {
 			dev_err(dev, "Failed to request /RESET: %d\n", ret);
-			goto err_early;
+			goto err_dcvdd;
 		}
 	}

@@ -699,7 +699,7 @@ int arizona_dev_init(struct arizona *arizona)
 	if (ret != 0) {
 		dev_err(dev, "Failed to enable core supplies: %d\n",
 			ret);
-		goto err_early;
+		goto err_dcvdd;
 	}
 
 	ret = regulator_enable(arizona->dcvdd);

@@ -1016,6 +1016,8 @@ err_reset:
 err_enable:
 	regulator_bulk_disable(arizona->num_core_supplies,
 			       arizona->core_supplies);
+err_dcvdd:
+	regulator_put(arizona->dcvdd);
 err_early:
 	mfd_remove_devices(dev);
 	return ret;

@@ -1027,6 +1029,7 @@ int arizona_dev_exit(struct arizona *arizona)
 	pm_runtime_disable(arizona->dev);
 
 	regulator_disable(arizona->dcvdd);
+	regulator_put(arizona->dcvdd);
 
 	mfd_remove_devices(arizona->dev);
 	arizona_free_irq(arizona, ARIZONA_IRQ_UNDERCLOCKED, arizona);

Signature · 1 line

1.7.2.5

Lee Jones

2014-06-17 14:36 UTC #6 lore raw

Re: [PATCH 3/5] mfd: arizona: Don't use devres for DCVDD

Quoted 9 lines · Charles Keepax

Currently the Arizona core uses a devm_regulator_get against its own device node to obtain DCVDD. The Arizona core is an MFD device and DCVDD is usually supplied by a child node (arizona-ldo1) of the core. As devres destruction for the MFD device will run after all its children have been destroyed, the regulator will be destroyed before devres calls regulator_put. This causes a warning from both the destruction of the child node, as the regulator is still open, and from the put of the regulator as the regulator device has already been destroyed.

Quoted 7 lines

This patch handles the regulator get and put without devres to avoid this issue.
Signed-off-by Charles Keepax <[email protected]>
---
 drivers/mfd/arizona-core.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

For the fear of someone coming along and undoing this work, can you write a nice succinct comment above regulator_get() that describes why you're not using managed resources as a subsequent patch please?

Patch applied though.

Quoted 48 lines

drivers/mfd/arizona-core.c +6 -3

index 48acac6..7678e84 100644
@@ -676,7 +676,7 @@ int arizona_dev_init(struct arizona *arizona)
 		goto err_early;
 	}
 
-	arizona->dcvdd = devm_regulator_get(arizona->dev, "DCVDD");
+	arizona->dcvdd = regulator_get(arizona->dev, "DCVDD");
 	if (IS_ERR(arizona->dcvdd)) {
 		ret = PTR_ERR(arizona->dcvdd);
 		dev_err(dev, "Failed to request DCVDD: %d\n", ret);

@@ -690,7 +690,7 @@ int arizona_dev_init(struct arizona *arizona)
 				       "arizona /RESET");
 		if (ret != 0) {
 			dev_err(dev, "Failed to request /RESET: %d\n", ret);
-			goto err_early;
+			goto err_dcvdd;
 		}
 	}

@@ -699,7 +699,7 @@ int arizona_dev_init(struct arizona *arizona)
 	if (ret != 0) {
 		dev_err(dev, "Failed to enable core supplies: %d\n",
 			ret);
-		goto err_early;
+		goto err_dcvdd;
 	}
 
 	ret = regulator_enable(arizona->dcvdd);

@@ -1016,6 +1016,8 @@ err_reset:
 err_enable:
 	regulator_bulk_disable(arizona->num_core_supplies,
 			       arizona->core_supplies);
+err_dcvdd:
+	regulator_put(arizona->dcvdd);
 err_early:
 	mfd_remove_devices(dev);
 	return ret;

@@ -1027,6 +1029,7 @@ int arizona_dev_exit(struct arizona *arizona)
 	pm_runtime_disable(arizona->dev);
 
 	regulator_disable(arizona->dcvdd);
+	regulator_put(arizona->dcvdd);
 
 	mfd_remove_devices(arizona->dev);
 	arizona_free_irq(arizona, ARIZONA_IRQ_UNDERCLOCKED, arizona);

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Charles Keepax

author

2014-06-17 15:17 UTC #7 lore raw

Re: [PATCH 3/5] mfd: arizona: Don't use devres for DCVDD

Quoted 19 lines · Lee Jones

Quoted 9 lines · Charles Keepax
Currently the Arizona core uses a devm_regulator_get against its own device node to obtain DCVDD. The Arizona core is an MFD device and DCVDD is usually supplied by a child node (arizona-ldo1) of the core. As devres destruction for the MFD device will run after all its children have been destroyed, the regulator will be destroyed before devres calls regulator_put. This causes a warning from both the destruction of the child node, as the regulator is still open, and from the put of the regulator as the regulator device has already been destroyed.
Quoted 7 lines
This patch handles the regulator get and put without devres to avoid this issue.
Signed-off-by Charles Keepax <[email protected]>
---
 drivers/mfd/arizona-core.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

Quoted 5 lines

For the fear of someone coming along and undoing this work, can you write a nice succinct comment above regulator_get() that describes why you're not using managed resources as a subsequent patch please?
Patch applied though.

Good point I should have thought to do that myself, I will get a patch out for this shortly.

Thanks,
Charles

Charles Keepax

author

2014-06-02 08:58 UTC #8 lore raw

PATCH 4/5 mfd: arizona: Use num_core_supplies in arizona_dev_exit

In reply to Charles Keepax

Currently we call regulator_bulk_disable with ARRAY_SIZE(arizona->core_supplies), however this array may be larger than the number of supplies actually used by the chip we are dealing with. Use the provided num_core_supplies member instead, so that we only disable supplies which actually exist.

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

---
 drivers/mfd/arizona-core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/mfd/arizona-core.c +1 -1

index 7678e84..51c0110 100644
@@ -1039,7 +1039,7 @@ int arizona_dev_exit(struct arizona *arizona)
 	if (arizona->pdata.reset)
 		gpio_set_value_cansleep(arizona->pdata.reset, 0);
 
-	regulator_bulk_disable(ARRAY_SIZE(arizona->core_supplies),
+	regulator_bulk_disable(arizona->num_core_supplies,
 			       arizona->core_supplies);
 	return 0;
 }

Signature · 1 line

1.7.2.5

Lee Jones

Applied

2014-06-17 14:36 UTC #9 lore raw

Re: [PATCH 4/5] mfd: arizona: Use num_core_supplies in arizona_dev_exit

Quoted 10 lines · Charles Keepax

Currently we call regulator_bulk_disable with ARRAY_SIZE(arizona->core_supplies), however this array may be larger than the number of supplies actually used by the chip we are dealing with. Use the provided num_core_supplies member instead, so that we only disable supplies which actually exist.
Signed-off-by Charles Keepax <[email protected]>
---
 drivers/mfd/arizona-core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Applied, thanks.

Quoted 13 lines

drivers/mfd/arizona-core.c +1 -1

index 7678e84..51c0110 100644
@@ -1039,7 +1039,7 @@ int arizona_dev_exit(struct arizona *arizona)
 	if (arizona->pdata.reset)
 		gpio_set_value_cansleep(arizona->pdata.reset, 0);
 
-	regulator_bulk_disable(ARRAY_SIZE(arizona->core_supplies),
+	regulator_bulk_disable(arizona->num_core_supplies,
 			       arizona->core_supplies);
 	return 0;
 }

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Charles Keepax

author

2014-06-02 08:59 UTC #10 lore raw

PATCH 5/5 mfd: arizona: Lower ARIZONA_MAX_CORE_SUPPLIES to 2

In reply to Charles Keepax

There are no Arizona devices with 3 core supplies but we define a fix array with space for 3 core supplies. Lower the ARIZONA_MAX_CORE_SUPPLIES define to 2, to save a few bytes.

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

---
 include/linux/mfd/arizona/core.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

include/linux/mfd/arizona/core.h +1 -1

index 6acb058..2bf1d74 100644
@@ -18,7 +18,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/mfd/arizona/pdata.h>
 
-#define ARIZONA_MAX_CORE_SUPPLIES 3
+#define ARIZONA_MAX_CORE_SUPPLIES 2
 
 enum arizona_type {
 	WM5102 = 1,

Signature · 1 line

1.7.2.5

Lee Jones

Applied

2014-06-17 14:36 UTC #11 lore raw

Re: [PATCH 5/5] mfd: arizona: Lower ARIZONA_MAX_CORE_SUPPLIES to 2

Quoted 8 lines · Charles Keepax

There are no Arizona devices with 3 core supplies but we define a fix array with space for 3 core supplies. Lower the ARIZONA_MAX_CORE_SUPPLIES define to 2, to save a few bytes.
Signed-off-by Charles Keepax <[email protected]>
---
 include/linux/mfd/arizona/core.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Applied, thanks.

Quoted 13 lines

include/linux/mfd/arizona/core.h +1 -1

index 6acb058..2bf1d74 100644
@@ -18,7 +18,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/mfd/arizona/pdata.h>
 
-#define ARIZONA_MAX_CORE_SUPPLIES 3
+#define ARIZONA_MAX_CORE_SUPPLIES 2
 
 enum arizona_type {
 	WM5102 = 1,

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog