[PATCH 0/5] Arizona clean up driver removal and error paths

[PATCH 0/5] Arizona clean up driver removal and error paths

[Charles Keepax]

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

-- 
1.7.2.5

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

[Charles Keepax]

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 <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/mfd/arizona-core.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index cabe8f4..7bdc0f5 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -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);
-- 
1.7.2.5

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

[Lee Jones]

> 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 <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/mfd/arizona-core.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
> index cabe8f4..7bdc0f5 100644
> --- a/drivers/mfd/arizona-core.c
> +++ b/drivers/mfd/arizona-core.c
> @@ -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);

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

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

[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 <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/mfd/arizona-core.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index 7bdc0f5..48acac6 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -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;
-- 
1.7.2.5

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

[Lee Jones]

On Mon, 02 Jun 2014, Charles Keepax wrote:

> 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 <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/mfd/arizona-core.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
> index 7bdc0f5..48acac6 100644
> --- a/drivers/mfd/arizona-core.c
> +++ b/drivers/mfd/arizona-core.c
> @@ -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;

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

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

[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 <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/mfd/arizona-core.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index 48acac6..7678e84 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -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);
-- 
1.7.2.5

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

[Lee Jones]

On Mon, 02 Jun 2014, Charles Keepax wrote:

> 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 <ckeepax@opensource.wolfsonmicro.com>
> ---
>  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.

> diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
> index 48acac6..7678e84 100644
> --- a/drivers/mfd/arizona-core.c
> +++ b/drivers/mfd/arizona-core.c
> @@ -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);

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

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

[Charles Keepax]

On Tue, Jun 17, 2014 at 03:36:10PM +0100, Lee Jones wrote:
> On Mon, 02 Jun 2014, Charles Keepax wrote:
> 
> > 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 <ckeepax@opensource.wolfsonmicro.com>
> > ---
> >  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.

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

Thanks,
Charles

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

[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 <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/mfd/arizona-core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index 7678e84..51c0110 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -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;
 }
-- 
1.7.2.5

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

[Lee Jones]

On Mon, 02 Jun 2014, Charles Keepax wrote:

> 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 <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/mfd/arizona-core.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
> index 7678e84..51c0110 100644
> --- a/drivers/mfd/arizona-core.c
> +++ b/drivers/mfd/arizona-core.c
> @@ -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;
>  }

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

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

[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 <ckeepax@opensource.wolfsonmicro.com>
---
 include/linux/mfd/arizona/core.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/mfd/arizona/core.h b/include/linux/mfd/arizona/core.h
index 6acb058..2bf1d74 100644
--- a/include/linux/mfd/arizona/core.h
+++ b/include/linux/mfd/arizona/core.h
@@ -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,
-- 
1.7.2.5

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

[Lee Jones]

On Mon, 02 Jun 2014, Charles Keepax wrote:

> 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 <ckeepax@opensource.wolfsonmicro.com>
> ---
>  include/linux/mfd/arizona/core.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Applied, thanks.

> diff --git a/include/linux/mfd/arizona/core.h b/include/linux/mfd/arizona/core.h
> index 6acb058..2bf1d74 100644
> --- a/include/linux/mfd/arizona/core.h
> +++ b/include/linux/mfd/arizona/core.h
> @@ -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,

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