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@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
> ---
> 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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
