Re: [PATCH v2 8/9] mfd: arizona: Add better support for system suspend

[Lee Jones <lee.jones@linaro.org>]

On Fri, 17 Apr 2015, Charles Keepax wrote:

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

It's ugly, but I can't think of a better way ...

Acked-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
> index 1b2fec0..def3ad3 100644
> --- a/drivers/mfd/arizona-core.c
> +++ b/drivers/mfd/arizona-core.c
> @@ -442,12 +442,33 @@ 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);
> +		enable_irq(arizona->irq);
> +		arizona->has_fully_powered_off = false;
> +	}
> +
>  	regcache_cache_only(arizona->regmap, false);
>  
>  	switch (arizona->type) {
> @@ -508,6 +529,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:
>  		ret = arizona_wait_for_boot(arizona);
> @@ -545,10 +574,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,
> @@ -559,33 +595,57 @@ static int arizona_runtime_suspend(struct device *dev)
>  				ret);
>  			return ret;
>  		}
> -	} else {
> -		switch (arizona->type) {
> -		case WM5110:
> -		case WM8280:
> -			/*
> -			 * As this is only called for the internal regulator
> -			 * (where we know voltage ranges available) it is ok
> -			 * to request an exact range.
> -			 */
> -			ret = regulator_set_voltage(arizona->dcvdd,
> -						    1175000, 1175000);
> -			if (ret < 0) {
> +	}
> +
> +	switch (arizona->type) {
> +	case WM5110:
> +	case WM8280:
> +		if (arizona->external_dcvdd)
> +			break;
> +
> +		/*
> +		 * As this is only called for the internal regulator
> +		 * (where we know voltage ranges available) it is ok
> +		 * to request an exact range.
> +		 */
> +		ret = regulator_set_voltage(arizona->dcvdd, 1175000, 1175000);
> +		if (ret < 0) {
> +			dev_err(arizona->dev,
> +				"Failed to set suspend voltage: %d\n", ret);
> +			return ret;
> +		}
> +		break;
> +	case WM5102:
> +		if (!(val & ARIZONA_JD1_ENA))
> +			ret = regmap_write(arizona->regmap,
> +					   ARIZONA_WRITE_SEQUENCER_CTRL_3, 0x0);
> +			if (ret) {
>  				dev_err(arizona->dev,
> -					"Failed to set suspend voltage: %d\n",
> +					"Failed to clear write sequencer: %d\n",
>  					ret);
>  				return ret;
>  			}
> -			break;
> -		default:
> -			break;
> -		}
> +		break;
> +	default:
> +		break;
>  	}
>  
>  	regcache_cache_only(arizona->regmap, true);
>  	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;
> +
> +		disable_irq(arizona->irq);
> +		arizona_enable_reset(arizona);
> +		regulator_bulk_disable(arizona->num_core_supplies,
> +				       arizona->core_supplies);
> +	}
> +
>  	return 0;
>  }
>  #endif
> diff --git a/include/linux/mfd/arizona/core.h b/include/linux/mfd/arizona/core.h
> index f970105..7c210af 100644
> --- a/include/linux/mfd/arizona/core.h
> +++ b/include/linux/mfd/arizona/core.h
> @@ -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;
>  

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