[alsa-devel] [PATCH 1/2] ASoC: Refector DAPM event handler

[Takashi Iwai]

At Thu, 17 Jul 2008 11:59:35 +0100,
Mark Brown wrote:
> 
> The DAPM event callback code has many layers of indentation. Refector

Refactor?

> the code to give less indentation in order to facilitiate further
> refactoring.
> 
> Signed-off-by: Mark Brown <broonie at opensource.wolfsonmicro.com>
> ---
>  sound/soc/soc-dapm.c |   76 +++++++++++++++++++++++++-------------------------
>  1 files changed, 38 insertions(+), 38 deletions(-)
> 
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index 2c87061..9307b10 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -587,44 +587,44 @@ static int dapm_power_widgets(struct snd_soc_codec *codec, int event)
>  			w->power = power;
>  
>  			/* call any power change event handlers */
> -			if (power_change) {
> -				if (w->event) {
> -					pr_debug("power %s event for %s flags %x\n",
> -						 w->power ? "on" : "off", w->name, w->event_flags);
> -					if (power) {
> -						/* power up event */
> -						if (w->event_flags & SND_SOC_DAPM_PRE_PMU) {
> -							ret = w->event(w,
> -								NULL, SND_SOC_DAPM_PRE_PMU);
> -							if (ret < 0)
> -								return ret;
> -						}
> -						dapm_update_bits(w);
> -						if (w->event_flags & SND_SOC_DAPM_POST_PMU){
> -							ret = w->event(w,
> -								NULL, SND_SOC_DAPM_POST_PMU);
> -							if (ret < 0)
> -								return ret;
> -						}
> -					} else {
> -						/* power down event */
> -						if (w->event_flags & SND_SOC_DAPM_PRE_PMD) {
> -							ret = w->event(w,
> -								NULL, SND_SOC_DAPM_PRE_PMD);
> -							if (ret < 0)
> -								return ret;
> -						}
> -						dapm_update_bits(w);
> -						if (w->event_flags & SND_SOC_DAPM_POST_PMD) {
> -							ret = w->event(w,
> -								NULL, SND_SOC_DAPM_POST_PMD);
> -							if (ret < 0)
> -								return ret;
> -						}
> -					}
> -				} else
> -					/* no event handler */
> -					dapm_update_bits(w);
> +			if (power_change && w->event)
> +				pr_debug("power %s event for %s flags %x\n",
> +					 w->power ? "on" : "off",
> +					 w->name, w->event_flags);
> +
> +			/* power up pre event */
> +			if (power_change && power && w->event &&
> +			    w->event_flags & SND_SOC_DAPM_PRE_PMU) {
> +				ret = w->event(w, NULL, SND_SOC_DAPM_PRE_PMU);
> +				if (ret < 0)
> +					return ret;
> +			}
> +
> +			/* power down pre event */
> +			if (power_change && !power && w->event &&
> +			    w->event_flags & SND_SOC_DAPM_PRE_PMD) {
> +				ret = w->event(w, NULL, SND_SOC_DAPM_PRE_PMD);
> +				if (ret < 0)
> +					return ret;
> +			}
> +
> +			dapm_update_bits(w);
> +
> +			/* power up post event */
> +			if (power_change && power && w->event &&
> +			    w->event_flags & SND_SOC_DAPM_POST_PMU){
> +				ret = w->event(w,
> +					       NULL, SND_SOC_DAPM_POST_PMU);
> +				if (ret < 0)
> +					return ret;
> +			}
> +
> +			/* power down post event */
> +			if (power_change && !power && w->event &&
> +			    w->event_flags & SND_SOC_DAPM_POST_PMD) {
> +				ret = w->event(w, NULL, SND_SOC_DAPM_POST_PMD);
> +				if (ret < 0)
> +					return ret;

With this patch, dapm_update_bits() is called even when
power_change=0, so it changes the behavior.  You can check
power_change only once and skip the rest via continue if it's false.

Also, better to put parentheses around bit-and operation.

Anyway, if the indent level really matters, you should better to put
it as a function.


thanks,

Takashi