[alsa-devel] [PATCH 1/2] ASoC: pcm1681: Fix setting de-emphasis sampling rate selection
The de-emphasis sampling rate selection is controlled by BIT[3:4] of PCM1681_DEEMPH_CONTROL register. Do proper left shift to set it.
Signed-off-by: Axel Lin axel.lin@ingics.com --- sound/soc/codecs/pcm1681.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/pcm1681.c b/sound/soc/codecs/pcm1681.c index b2c990f..1011142 100644 --- a/sound/soc/codecs/pcm1681.c +++ b/sound/soc/codecs/pcm1681.c @@ -102,7 +102,7 @@ static int pcm1681_set_deemph(struct snd_soc_codec *codec)
if (val != -1) { regmap_update_bits(priv->regmap, PCM1681_DEEMPH_CONTROL, - PCM1681_DEEMPH_RATE_MASK, val); + PCM1681_DEEMPH_RATE_MASK, val << 3); enable = 1; } else enable = 0;
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@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; + } + } + }
if (val != -1) { regmap_update_bits(priv->regmap, PCM1681_DEEMPH_CONTROL, PCM1681_DEEMPH_RATE_MASK, val << 3); enable = 1; - } else + } else { enable = 0; + }
/* enable/disable deemphasis functionality */ return regmap_update_bits(priv->regmap, PCM1681_DEEMPH_CONTROL,
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@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)
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
/* enable/disable deemphasis functionality */ return regmap_update_bits(priv->regmap, PCM1681_DEEMPH_CONTROL,
BR,
marek
2015-07-24 3:48 GMT+08:00 Marek Belisko marek.belisko@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@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
On 07/24/2015 03:17 AM, Axel Lin wrote:
2015-07-24 3:48 GMT+08:00 Marek Belisko marek.belisko@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@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 we follow CodingStyle then it says no brackets around single statement.
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.
OK agree. Sorry about that. I have to read again CodingStyle ;)
Regards, Axel
BR,
marek
2015-07-24 13:22 GMT+08:00 Marek Belisko marek.belisko@streamunlimited.com:
On 07/24/2015 03:17 AM, Axel Lin wrote:
2015-07-24 3:48 GMT+08:00 Marek Belisko marek.belisko@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@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 we follow CodingStyle then it says no brackets around single statement.
Yes, but it's not always good if the "single statement" is actually cross multiple lines. And checkpatch does not complain this patch.
Well, this is probably a bit personal preference because I feel it has better readability with the brackets here.
If you insist on removing the brackets, I can send v2 to do that.
Regards, Axel
On 07/24/2015 07:48 AM, Axel Lin wrote:
2015-07-24 13:22 GMT+08:00 Marek Belisko marek.belisko@streamunlimited.com:
On 07/24/2015 03:17 AM, Axel Lin wrote:
2015-07-24 3:48 GMT+08:00 Marek Belisko marek.belisko@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@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 we follow CodingStyle then it says no brackets around single statement.
Yes, but it's not always good if the "single statement" is actually cross multiple lines. And checkpatch does not complain this patch.
Well, this is probably a bit personal preference because I feel it has better readability with the brackets here.
If you insist on removing the brackets, I can send v2 to do that.
I have no strong preference about it. I'll keep it on maintainers. You can add my Acked-by: Marek Belisko marek.belisko@streamunlimited.com
Regards, Axel
BR,
marek
The patch
ASoC: pcm1681: Improve the logic for de-emphasis sampling rate selection
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 48f403be3eb9b603cfaf946ca7a0c76272750469 Mon Sep 17 00:00:00 2001
From: Axel Lin axel.lin@ingics.com Date: Thu, 23 Jul 2015 23:23:35 +0800 Subject: [PATCH] ASoC: pcm1681: Improve the logic for de-emphasis sampling rate selection
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@ingics.com Signed-off-by: Mark Brown broonie@kernel.org --- 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 e7ba557979cb..490970e5ab8c 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; + } + } + }
if (val != -1) { regmap_update_bits(priv->regmap, PCM1681_DEEMPH_CONTROL, PCM1681_DEEMPH_RATE_MASK, val << 3); enable = 1; - } else + } else { enable = 0; + }
/* enable/disable deemphasis functionality */ return regmap_update_bits(priv->regmap, PCM1681_DEEMPH_CONTROL,
On 23.07.2015 17:22, Axel Lin wrote:
The de-emphasis sampling rate selection is controlled by BIT[3:4] of PCM1681_DEEMPH_CONTROL register. Do proper left shift to set it.
Signed-off-by: Axel Lin axel.lin@ingics.com
sound/soc/codecs/pcm1681.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/pcm1681.c b/sound/soc/codecs/pcm1681.c index b2c990f..1011142 100644 --- a/sound/soc/codecs/pcm1681.c +++ b/sound/soc/codecs/pcm1681.c @@ -102,7 +102,7 @@ static int pcm1681_set_deemph(struct snd_soc_codec *codec)
if (val != -1) { regmap_update_bits(priv->regmap, PCM1681_DEEMPH_CONTROL,
PCM1681_DEEMPH_RATE_MASK, val);
PCM1681_DEEMPH_RATE_MASK, val << 3);
Good catch. Acked-by: Marek Belisko marek.belisko@streamunlimited.com
enable = 1;} else enable = 0;
The patch
ASoC: pcm1681: Fix setting de-emphasis sampling rate selection
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From fa8173a3ef0570affde7da352de202190b3786c2 Mon Sep 17 00:00:00 2001
From: Axel Lin axel.lin@ingics.com Date: Thu, 23 Jul 2015 23:22:26 +0800 Subject: [PATCH] ASoC: pcm1681: Fix setting de-emphasis sampling rate selection
The de-emphasis sampling rate selection is controlled by BIT[3:4] of PCM1681_DEEMPH_CONTROL register. Do proper left shift to set it.
Signed-off-by: Axel Lin axel.lin@ingics.com Acked-by: Marek Belisko marek.belisko@streamunlimited.com Signed-off-by: Mark Brown broonie@kernel.org Cc: stable@vger.kernel.org --- sound/soc/codecs/pcm1681.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/pcm1681.c b/sound/soc/codecs/pcm1681.c index 477e13d30971..e7ba557979cb 100644 --- a/sound/soc/codecs/pcm1681.c +++ b/sound/soc/codecs/pcm1681.c @@ -102,7 +102,7 @@ static int pcm1681_set_deemph(struct snd_soc_codec *codec)
if (val != -1) { regmap_update_bits(priv->regmap, PCM1681_DEEMPH_CONTROL, - PCM1681_DEEMPH_RATE_MASK, val); + PCM1681_DEEMPH_RATE_MASK, val << 3); enable = 1; } else enable = 0;
participants (3)
-
Axel Lin -
Marek Belisko -
Mark Brown