[alsa-devel] [PATCH 2/2] ASoC: pcm1681: Improve the logic for de-emphasis sampling rate selection

[Axel Lin]

2015-07-24 3:48 GMT+08:00 Marek Belisko <marek.belisko at streamunlimited.com>:
> Hi Axel,
>
> On 23.07.2015 17:23, Axel Lin wrote:
>>
>> Slightly improve the logic for de-emphasis sampling rate selection by
>> break
>> out the loop if the rate is matched.
>>
>> Signed-off-by: Axel Lin <axel.lin at ingics.com>
>> ---
>>   sound/soc/codecs/pcm1681.c | 13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/sound/soc/codecs/pcm1681.c b/sound/soc/codecs/pcm1681.c
>> index 1011142..5832523 100644
>> --- a/sound/soc/codecs/pcm1681.c
>> +++ b/sound/soc/codecs/pcm1681.c
>> @@ -95,17 +95,22 @@ static int pcm1681_set_deemph(struct snd_soc_codec
>> *codec)
>>         struct pcm1681_private *priv = snd_soc_codec_get_drvdata(codec);
>>         int i = 0, val = -1, enable = 0;
>>
>> -       if (priv->deemph)
>> -               for (i = 0; i < ARRAY_SIZE(pcm1681_deemph); i++)
>> -                       if (pcm1681_deemph[i] == priv->rate)
>> +       if (priv->deemph) {
>> +               for (i = 0; i < ARRAY_SIZE(pcm1681_deemph); i++) {
>> +                       if (pcm1681_deemph[i] == priv->rate) {
>>                                 val = i;
>> +                               break;
>> +                       }
>> +               }
>> +       }
>
> ^^^^^^^
> I think we don't need those brackets only for if statement (where you add
> break)

Because I think having the brackets here makes the code looks better.

>>
>>
>>         if (val != -1) {
>>                 regmap_update_bits(priv->regmap, PCM1681_DEEMPH_CONTROL,
>>                                    PCM1681_DEEMPH_RATE_MASK, val << 3);
>>                 enable = 1;
>> -       } else
>> +       } else {
>>                 enable = 0;
>> +       }
>
> ^^^ same here

This is also a common pattern in kernel coding style that if we have
brackets around the if statement, also add the brackets for the else
part.

Regards,
Axel