* [RFC PATCH 0/2] ASoC: remove cppchecks warnings on lm49453 and da732x
@ 2021-03-26 22:16 Pierre-Louis Bossart
2021-03-26 22:16 ` Pierre-Louis Bossart
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Pierre-Louis Bossart @ 2021-03-26 22:16 UTC (permalink / raw)
To: alsa-devel; +Cc: tiwai, broonie, linux-kernel, Pierre-Louis Bossart
There are the last two patches in the cleanups, this time I am not
sure what the code does and what the proper fix might be. Feedback
welcome.
Pierre-Louis Bossart (2):
ASoC: lm49453: fix useless assignment before return
ASoC: da732x: simplify code
sound/soc/codecs/da732x.c | 17 ++++++-----------
sound/soc/codecs/da732x.h | 12 ++++--------
sound/soc/codecs/lm49453.c | 2 --
3 files changed, 10 insertions(+), 21 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC PATCH 1/2] ASoC: lm49453: fix useless assignment before return
2021-03-26 22:16 [RFC PATCH 0/2] ASoC: remove cppchecks warnings on lm49453 and da732x Pierre-Louis Bossart
@ 2021-03-26 22:16 ` Pierre-Louis Bossart
2021-03-26 22:16 ` Pierre-Louis Bossart
2021-04-01 10:16 ` Mark Brown
2 siblings, 0 replies; 13+ messages in thread
From: Pierre-Louis Bossart @ 2021-03-26 22:16 UTC (permalink / raw)
To: alsa-devel
Cc: tiwai, broonie, linux-kernel, Pierre-Louis Bossart,
M R Swami Reddy, Vishwas A Deshpande, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cppcheck warning:
sound/soc/codecs/lm49453.c:1210:11: style: Variable 'pll_clk' is
assigned a value that is never used. [unreadVariable]
pll_clk = BIT(4);
^
FIXME: What is the correct fix?
/* fll clk slection */
pll_clk = BIT(4);
return 0;
is the assignment redundant or the 'return 0' a mistake?
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
sound/soc/codecs/lm49453.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/sound/soc/codecs/lm49453.c b/sound/soc/codecs/lm49453.c
index eb3dd0bd80d9..fb0fb23537e7 100644
--- a/sound/soc/codecs/lm49453.c
+++ b/sound/soc/codecs/lm49453.c
@@ -1206,8 +1206,6 @@ static int lm49453_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
break;
case 48000:
case 32576:
- /* fll clk slection */
- pll_clk = BIT(4);
return 0;
default:
return -EINVAL;
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [RFC PATCH 1/2] ASoC: lm49453: fix useless assignment before return
@ 2021-03-26 22:16 ` Pierre-Louis Bossart
0 siblings, 0 replies; 13+ messages in thread
From: Pierre-Louis Bossart @ 2021-03-26 22:16 UTC (permalink / raw)
To: alsa-devel
Cc: Liam Girdwood, tiwai, Takashi Iwai, linux-kernel,
Pierre-Louis Bossart, Vishwas A Deshpande, broonie,
M R Swami Reddy
Cppcheck warning:
sound/soc/codecs/lm49453.c:1210:11: style: Variable 'pll_clk' is
assigned a value that is never used. [unreadVariable]
pll_clk = BIT(4);
^
FIXME: What is the correct fix?
/* fll clk slection */
pll_clk = BIT(4);
return 0;
is the assignment redundant or the 'return 0' a mistake?
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
sound/soc/codecs/lm49453.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/sound/soc/codecs/lm49453.c b/sound/soc/codecs/lm49453.c
index eb3dd0bd80d9..fb0fb23537e7 100644
--- a/sound/soc/codecs/lm49453.c
+++ b/sound/soc/codecs/lm49453.c
@@ -1206,8 +1206,6 @@ static int lm49453_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
break;
case 48000:
case 32576:
- /* fll clk slection */
- pll_clk = BIT(4);
return 0;
default:
return -EINVAL;
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [RFC PATCH 2/2] ASoC: da732x: simplify code
2021-03-26 22:16 [RFC PATCH 0/2] ASoC: remove cppchecks warnings on lm49453 and da732x Pierre-Louis Bossart
@ 2021-03-26 22:16 ` Pierre-Louis Bossart
2021-03-26 22:16 ` Pierre-Louis Bossart
2021-04-01 10:16 ` Mark Brown
2 siblings, 0 replies; 13+ messages in thread
From: Pierre-Louis Bossart @ 2021-03-26 22:16 UTC (permalink / raw)
To: alsa-devel
Cc: tiwai, broonie, linux-kernel, Pierre-Louis Bossart,
Support Opensource, Liam Girdwood, Jaroslav Kysela, Takashi Iwai
cppcheck reports a false positive:
sound/soc/codecs/da732x.c:1161:25: warning: Either the condition
'indiv<0' is redundant or there is division by zero at line
1161. [zerodivcond]
fref = (da732x->sysclk / indiv);
^
sound/soc/codecs/da732x.c:1158:12: note: Assuming that condition
'indiv<0' is not redundant
if (indiv < 0)
^
sound/soc/codecs/da732x.c:1161:25: note: Division by zero
fref = (da732x->sysclk / indiv);
^
The code is awfully convoluted/confusing and can be simplified with a
single variable and the BIT macro.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
sound/soc/codecs/da732x.c | 17 ++++++-----------
sound/soc/codecs/da732x.h | 12 ++++--------
2 files changed, 10 insertions(+), 19 deletions(-)
diff --git a/sound/soc/codecs/da732x.c b/sound/soc/codecs/da732x.c
index d43ee7159ae0..42d6a3fc3af5 100644
--- a/sound/soc/codecs/da732x.c
+++ b/sound/soc/codecs/da732x.c
@@ -168,30 +168,25 @@ static const struct reg_default da732x_reg_cache[] = {
static inline int da732x_get_input_div(struct snd_soc_component *component, int sysclk)
{
int val;
- int ret;
if (sysclk < DA732X_MCLK_10MHZ) {
- val = DA732X_MCLK_RET_0_10MHZ;
- ret = DA732X_MCLK_VAL_0_10MHZ;
+ val = DA732X_MCLK_VAL_0_10MHZ;
} else if ((sysclk >= DA732X_MCLK_10MHZ) &&
(sysclk < DA732X_MCLK_20MHZ)) {
- val = DA732X_MCLK_RET_10_20MHZ;
- ret = DA732X_MCLK_VAL_10_20MHZ;
+ val = DA732X_MCLK_VAL_10_20MHZ;
} else if ((sysclk >= DA732X_MCLK_20MHZ) &&
(sysclk < DA732X_MCLK_40MHZ)) {
- val = DA732X_MCLK_RET_20_40MHZ;
- ret = DA732X_MCLK_VAL_20_40MHZ;
+ val = DA732X_MCLK_VAL_20_40MHZ;
} else if ((sysclk >= DA732X_MCLK_40MHZ) &&
(sysclk <= DA732X_MCLK_54MHZ)) {
- val = DA732X_MCLK_RET_40_54MHZ;
- ret = DA732X_MCLK_VAL_40_54MHZ;
+ val = DA732X_MCLK_VAL_40_54MHZ;
} else {
return -EINVAL;
}
snd_soc_component_write(component, DA732X_REG_PLL_CTRL, val);
- return ret;
+ return val;
}
static void da732x_set_charge_pump(struct snd_soc_component *component, int state)
@@ -1158,7 +1153,7 @@ static int da732x_set_dai_pll(struct snd_soc_component *component, int pll_id,
if (indiv < 0)
return indiv;
- fref = (da732x->sysclk / indiv);
+ fref = da732x->sysclk / BIT(indiv);
div_hi = freq_out / fref;
frac_div = (u64)(freq_out % fref) * 8192ULL;
do_div(frac_div, fref);
diff --git a/sound/soc/codecs/da732x.h b/sound/soc/codecs/da732x.h
index c5af17ee1516..c2f784c3f359 100644
--- a/sound/soc/codecs/da732x.h
+++ b/sound/soc/codecs/da732x.h
@@ -48,14 +48,10 @@
#define DA732X_MCLK_20MHZ 20000000
#define DA732X_MCLK_40MHZ 40000000
#define DA732X_MCLK_54MHZ 54000000
-#define DA732X_MCLK_RET_0_10MHZ 0
-#define DA732X_MCLK_VAL_0_10MHZ 1
-#define DA732X_MCLK_RET_10_20MHZ 1
-#define DA732X_MCLK_VAL_10_20MHZ 2
-#define DA732X_MCLK_RET_20_40MHZ 2
-#define DA732X_MCLK_VAL_20_40MHZ 4
-#define DA732X_MCLK_RET_40_54MHZ 3
-#define DA732X_MCLK_VAL_40_54MHZ 8
+#define DA732X_MCLK_VAL_0_10MHZ 0
+#define DA732X_MCLK_VAL_10_20MHZ 1
+#define DA732X_MCLK_VAL_20_40MHZ 2
+#define DA732X_MCLK_VAL_40_54MHZ 3
#define DA732X_DAI_ID1 0
#define DA732X_DAI_ID2 1
#define DA732X_SRCCLK_PLL 0
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [RFC PATCH 2/2] ASoC: da732x: simplify code
@ 2021-03-26 22:16 ` Pierre-Louis Bossart
0 siblings, 0 replies; 13+ messages in thread
From: Pierre-Louis Bossart @ 2021-03-26 22:16 UTC (permalink / raw)
To: alsa-devel
Cc: Pierre-Louis Bossart, Support Opensource, tiwai, Takashi Iwai,
Liam Girdwood, linux-kernel, broonie
cppcheck reports a false positive:
sound/soc/codecs/da732x.c:1161:25: warning: Either the condition
'indiv<0' is redundant or there is division by zero at line
1161. [zerodivcond]
fref = (da732x->sysclk / indiv);
^
sound/soc/codecs/da732x.c:1158:12: note: Assuming that condition
'indiv<0' is not redundant
if (indiv < 0)
^
sound/soc/codecs/da732x.c:1161:25: note: Division by zero
fref = (da732x->sysclk / indiv);
^
The code is awfully convoluted/confusing and can be simplified with a
single variable and the BIT macro.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
sound/soc/codecs/da732x.c | 17 ++++++-----------
sound/soc/codecs/da732x.h | 12 ++++--------
2 files changed, 10 insertions(+), 19 deletions(-)
diff --git a/sound/soc/codecs/da732x.c b/sound/soc/codecs/da732x.c
index d43ee7159ae0..42d6a3fc3af5 100644
--- a/sound/soc/codecs/da732x.c
+++ b/sound/soc/codecs/da732x.c
@@ -168,30 +168,25 @@ static const struct reg_default da732x_reg_cache[] = {
static inline int da732x_get_input_div(struct snd_soc_component *component, int sysclk)
{
int val;
- int ret;
if (sysclk < DA732X_MCLK_10MHZ) {
- val = DA732X_MCLK_RET_0_10MHZ;
- ret = DA732X_MCLK_VAL_0_10MHZ;
+ val = DA732X_MCLK_VAL_0_10MHZ;
} else if ((sysclk >= DA732X_MCLK_10MHZ) &&
(sysclk < DA732X_MCLK_20MHZ)) {
- val = DA732X_MCLK_RET_10_20MHZ;
- ret = DA732X_MCLK_VAL_10_20MHZ;
+ val = DA732X_MCLK_VAL_10_20MHZ;
} else if ((sysclk >= DA732X_MCLK_20MHZ) &&
(sysclk < DA732X_MCLK_40MHZ)) {
- val = DA732X_MCLK_RET_20_40MHZ;
- ret = DA732X_MCLK_VAL_20_40MHZ;
+ val = DA732X_MCLK_VAL_20_40MHZ;
} else if ((sysclk >= DA732X_MCLK_40MHZ) &&
(sysclk <= DA732X_MCLK_54MHZ)) {
- val = DA732X_MCLK_RET_40_54MHZ;
- ret = DA732X_MCLK_VAL_40_54MHZ;
+ val = DA732X_MCLK_VAL_40_54MHZ;
} else {
return -EINVAL;
}
snd_soc_component_write(component, DA732X_REG_PLL_CTRL, val);
- return ret;
+ return val;
}
static void da732x_set_charge_pump(struct snd_soc_component *component, int state)
@@ -1158,7 +1153,7 @@ static int da732x_set_dai_pll(struct snd_soc_component *component, int pll_id,
if (indiv < 0)
return indiv;
- fref = (da732x->sysclk / indiv);
+ fref = da732x->sysclk / BIT(indiv);
div_hi = freq_out / fref;
frac_div = (u64)(freq_out % fref) * 8192ULL;
do_div(frac_div, fref);
diff --git a/sound/soc/codecs/da732x.h b/sound/soc/codecs/da732x.h
index c5af17ee1516..c2f784c3f359 100644
--- a/sound/soc/codecs/da732x.h
+++ b/sound/soc/codecs/da732x.h
@@ -48,14 +48,10 @@
#define DA732X_MCLK_20MHZ 20000000
#define DA732X_MCLK_40MHZ 40000000
#define DA732X_MCLK_54MHZ 54000000
-#define DA732X_MCLK_RET_0_10MHZ 0
-#define DA732X_MCLK_VAL_0_10MHZ 1
-#define DA732X_MCLK_RET_10_20MHZ 1
-#define DA732X_MCLK_VAL_10_20MHZ 2
-#define DA732X_MCLK_RET_20_40MHZ 2
-#define DA732X_MCLK_VAL_20_40MHZ 4
-#define DA732X_MCLK_RET_40_54MHZ 3
-#define DA732X_MCLK_VAL_40_54MHZ 8
+#define DA732X_MCLK_VAL_0_10MHZ 0
+#define DA732X_MCLK_VAL_10_20MHZ 1
+#define DA732X_MCLK_VAL_20_40MHZ 2
+#define DA732X_MCLK_VAL_40_54MHZ 3
#define DA732X_DAI_ID1 0
#define DA732X_DAI_ID2 1
#define DA732X_SRCCLK_PLL 0
--
2.25.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 0/2] ASoC: remove cppchecks warnings on lm49453 and da732x
2021-03-26 22:16 [RFC PATCH 0/2] ASoC: remove cppchecks warnings on lm49453 and da732x Pierre-Louis Bossart
@ 2021-04-01 10:16 ` Mark Brown
2021-03-26 22:16 ` Pierre-Louis Bossart
2021-04-01 10:16 ` Mark Brown
2 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2021-04-01 10:16 UTC (permalink / raw)
To: Pierre-Louis Bossart, alsa-devel; +Cc: Mark Brown, tiwai, linux-kernel
On Fri, 26 Mar 2021 17:16:17 -0500, Pierre-Louis Bossart wrote:
> There are the last two patches in the cleanups, this time I am not
> sure what the code does and what the proper fix might be. Feedback
> welcome.
>
> Pierre-Louis Bossart (2):
> ASoC: lm49453: fix useless assignment before return
> ASoC: da732x: simplify code
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/2] ASoC: lm49453: fix useless assignment before return
commit: 458c23c509f66c5950da7e5496ea952ad15128f7
[2/2] ASoC: da732x: simplify code
commit: 945b0b58c5d7c6640f9aad2096e4675bc7f5371c
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 0/2] ASoC: remove cppchecks warnings on lm49453 and da732x
@ 2021-04-01 10:16 ` Mark Brown
0 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2021-04-01 10:16 UTC (permalink / raw)
To: Pierre-Louis Bossart, alsa-devel; +Cc: tiwai, Mark Brown, linux-kernel
On Fri, 26 Mar 2021 17:16:17 -0500, Pierre-Louis Bossart wrote:
> There are the last two patches in the cleanups, this time I am not
> sure what the code does and what the proper fix might be. Feedback
> welcome.
>
> Pierre-Louis Bossart (2):
> ASoC: lm49453: fix useless assignment before return
> ASoC: da732x: simplify code
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/2] ASoC: lm49453: fix useless assignment before return
commit: 458c23c509f66c5950da7e5496ea952ad15128f7
[2/2] ASoC: da732x: simplify code
commit: 945b0b58c5d7c6640f9aad2096e4675bc7f5371c
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [RFC PATCH 2/2] ASoC: da732x: simplify code
2021-03-26 22:16 ` Pierre-Louis Bossart
@ 2021-04-15 16:00 ` Adam Thomson
-1 siblings, 0 replies; 13+ messages in thread
From: Adam Thomson @ 2021-04-15 16:00 UTC (permalink / raw)
To: Pierre-Louis Bossart, alsa-devel
Cc: tiwai, broonie, linux-kernel, Support Opensource, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
On 26 March 2021 22:16, Pierre-Louis Bossart wrote:
> cppcheck reports a false positive:
>
> sound/soc/codecs/da732x.c:1161:25: warning: Either the condition
> 'indiv<0' is redundant or there is division by zero at line
> 1161. [zerodivcond]
> fref = (da732x->sysclk / indiv);
> ^
> sound/soc/codecs/da732x.c:1158:12: note: Assuming that condition
> 'indiv<0' is not redundant
> if (indiv < 0)
> ^
> sound/soc/codecs/da732x.c:1161:25: note: Division by zero
> fref = (da732x->sysclk / indiv);
> ^
>
> The code is awfully convoluted/confusing and can be simplified with a
> single variable and the BIT macro.
>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
Apologies for the delay in getting to this. The change looks fine to me,
although this part was EOL some time back, and I find it hard to believe anyone
out there has a board with this on. Wondering if it would make sense to remove
the driver permanently?
For the change at hand though:
Reviewed-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> sound/soc/codecs/da732x.c | 17 ++++++-----------
> sound/soc/codecs/da732x.h | 12 ++++--------
> 2 files changed, 10 insertions(+), 19 deletions(-)
>
> diff --git a/sound/soc/codecs/da732x.c b/sound/soc/codecs/da732x.c
> index d43ee7159ae0..42d6a3fc3af5 100644
> --- a/sound/soc/codecs/da732x.c
> +++ b/sound/soc/codecs/da732x.c
> @@ -168,30 +168,25 @@ static const struct reg_default da732x_reg_cache[] = {
> static inline int da732x_get_input_div(struct snd_soc_component *component,
> int sysclk)
> {
> int val;
> - int ret;
>
> if (sysclk < DA732X_MCLK_10MHZ) {
> - val = DA732X_MCLK_RET_0_10MHZ;
> - ret = DA732X_MCLK_VAL_0_10MHZ;
> + val = DA732X_MCLK_VAL_0_10MHZ;
> } else if ((sysclk >= DA732X_MCLK_10MHZ) &&
> (sysclk < DA732X_MCLK_20MHZ)) {
> - val = DA732X_MCLK_RET_10_20MHZ;
> - ret = DA732X_MCLK_VAL_10_20MHZ;
> + val = DA732X_MCLK_VAL_10_20MHZ;
> } else if ((sysclk >= DA732X_MCLK_20MHZ) &&
> (sysclk < DA732X_MCLK_40MHZ)) {
> - val = DA732X_MCLK_RET_20_40MHZ;
> - ret = DA732X_MCLK_VAL_20_40MHZ;
> + val = DA732X_MCLK_VAL_20_40MHZ;
> } else if ((sysclk >= DA732X_MCLK_40MHZ) &&
> (sysclk <= DA732X_MCLK_54MHZ)) {
> - val = DA732X_MCLK_RET_40_54MHZ;
> - ret = DA732X_MCLK_VAL_40_54MHZ;
> + val = DA732X_MCLK_VAL_40_54MHZ;
> } else {
> return -EINVAL;
> }
>
> snd_soc_component_write(component, DA732X_REG_PLL_CTRL, val);
>
> - return ret;
> + return val;
> }
>
> static void da732x_set_charge_pump(struct snd_soc_component *component,
> int state)
> @@ -1158,7 +1153,7 @@ static int da732x_set_dai_pll(struct
> snd_soc_component *component, int pll_id,
> if (indiv < 0)
> return indiv;
>
> - fref = (da732x->sysclk / indiv);
> + fref = da732x->sysclk / BIT(indiv);
> div_hi = freq_out / fref;
> frac_div = (u64)(freq_out % fref) * 8192ULL;
> do_div(frac_div, fref);
> diff --git a/sound/soc/codecs/da732x.h b/sound/soc/codecs/da732x.h
> index c5af17ee1516..c2f784c3f359 100644
> --- a/sound/soc/codecs/da732x.h
> +++ b/sound/soc/codecs/da732x.h
> @@ -48,14 +48,10 @@
> #define DA732X_MCLK_20MHZ 20000000
> #define DA732X_MCLK_40MHZ 40000000
> #define DA732X_MCLK_54MHZ 54000000
> -#define DA732X_MCLK_RET_0_10MHZ 0
> -#define DA732X_MCLK_VAL_0_10MHZ 1
> -#define DA732X_MCLK_RET_10_20MHZ 1
> -#define DA732X_MCLK_VAL_10_20MHZ 2
> -#define DA732X_MCLK_RET_20_40MHZ 2
> -#define DA732X_MCLK_VAL_20_40MHZ 4
> -#define DA732X_MCLK_RET_40_54MHZ 3
> -#define DA732X_MCLK_VAL_40_54MHZ 8
> +#define DA732X_MCLK_VAL_0_10MHZ 0
> +#define DA732X_MCLK_VAL_10_20MHZ 1
> +#define DA732X_MCLK_VAL_20_40MHZ 2
> +#define DA732X_MCLK_VAL_40_54MHZ 3
> #define DA732X_DAI_ID1 0
> #define DA732X_DAI_ID2 1
> #define DA732X_SRCCLK_PLL 0
> --
> 2.25.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [RFC PATCH 2/2] ASoC: da732x: simplify code
@ 2021-04-15 16:00 ` Adam Thomson
0 siblings, 0 replies; 13+ messages in thread
From: Adam Thomson @ 2021-04-15 16:00 UTC (permalink / raw)
To: Pierre-Louis Bossart, alsa-devel
Cc: Support Opensource, tiwai, linux-kernel, Takashi Iwai,
Liam Girdwood, broonie
On 26 March 2021 22:16, Pierre-Louis Bossart wrote:
> cppcheck reports a false positive:
>
> sound/soc/codecs/da732x.c:1161:25: warning: Either the condition
> 'indiv<0' is redundant or there is division by zero at line
> 1161. [zerodivcond]
> fref = (da732x->sysclk / indiv);
> ^
> sound/soc/codecs/da732x.c:1158:12: note: Assuming that condition
> 'indiv<0' is not redundant
> if (indiv < 0)
> ^
> sound/soc/codecs/da732x.c:1161:25: note: Division by zero
> fref = (da732x->sysclk / indiv);
> ^
>
> The code is awfully convoluted/confusing and can be simplified with a
> single variable and the BIT macro.
>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
Apologies for the delay in getting to this. The change looks fine to me,
although this part was EOL some time back, and I find it hard to believe anyone
out there has a board with this on. Wondering if it would make sense to remove
the driver permanently?
For the change at hand though:
Reviewed-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> sound/soc/codecs/da732x.c | 17 ++++++-----------
> sound/soc/codecs/da732x.h | 12 ++++--------
> 2 files changed, 10 insertions(+), 19 deletions(-)
>
> diff --git a/sound/soc/codecs/da732x.c b/sound/soc/codecs/da732x.c
> index d43ee7159ae0..42d6a3fc3af5 100644
> --- a/sound/soc/codecs/da732x.c
> +++ b/sound/soc/codecs/da732x.c
> @@ -168,30 +168,25 @@ static const struct reg_default da732x_reg_cache[] = {
> static inline int da732x_get_input_div(struct snd_soc_component *component,
> int sysclk)
> {
> int val;
> - int ret;
>
> if (sysclk < DA732X_MCLK_10MHZ) {
> - val = DA732X_MCLK_RET_0_10MHZ;
> - ret = DA732X_MCLK_VAL_0_10MHZ;
> + val = DA732X_MCLK_VAL_0_10MHZ;
> } else if ((sysclk >= DA732X_MCLK_10MHZ) &&
> (sysclk < DA732X_MCLK_20MHZ)) {
> - val = DA732X_MCLK_RET_10_20MHZ;
> - ret = DA732X_MCLK_VAL_10_20MHZ;
> + val = DA732X_MCLK_VAL_10_20MHZ;
> } else if ((sysclk >= DA732X_MCLK_20MHZ) &&
> (sysclk < DA732X_MCLK_40MHZ)) {
> - val = DA732X_MCLK_RET_20_40MHZ;
> - ret = DA732X_MCLK_VAL_20_40MHZ;
> + val = DA732X_MCLK_VAL_20_40MHZ;
> } else if ((sysclk >= DA732X_MCLK_40MHZ) &&
> (sysclk <= DA732X_MCLK_54MHZ)) {
> - val = DA732X_MCLK_RET_40_54MHZ;
> - ret = DA732X_MCLK_VAL_40_54MHZ;
> + val = DA732X_MCLK_VAL_40_54MHZ;
> } else {
> return -EINVAL;
> }
>
> snd_soc_component_write(component, DA732X_REG_PLL_CTRL, val);
>
> - return ret;
> + return val;
> }
>
> static void da732x_set_charge_pump(struct snd_soc_component *component,
> int state)
> @@ -1158,7 +1153,7 @@ static int da732x_set_dai_pll(struct
> snd_soc_component *component, int pll_id,
> if (indiv < 0)
> return indiv;
>
> - fref = (da732x->sysclk / indiv);
> + fref = da732x->sysclk / BIT(indiv);
> div_hi = freq_out / fref;
> frac_div = (u64)(freq_out % fref) * 8192ULL;
> do_div(frac_div, fref);
> diff --git a/sound/soc/codecs/da732x.h b/sound/soc/codecs/da732x.h
> index c5af17ee1516..c2f784c3f359 100644
> --- a/sound/soc/codecs/da732x.h
> +++ b/sound/soc/codecs/da732x.h
> @@ -48,14 +48,10 @@
> #define DA732X_MCLK_20MHZ 20000000
> #define DA732X_MCLK_40MHZ 40000000
> #define DA732X_MCLK_54MHZ 54000000
> -#define DA732X_MCLK_RET_0_10MHZ 0
> -#define DA732X_MCLK_VAL_0_10MHZ 1
> -#define DA732X_MCLK_RET_10_20MHZ 1
> -#define DA732X_MCLK_VAL_10_20MHZ 2
> -#define DA732X_MCLK_RET_20_40MHZ 2
> -#define DA732X_MCLK_VAL_20_40MHZ 4
> -#define DA732X_MCLK_RET_40_54MHZ 3
> -#define DA732X_MCLK_VAL_40_54MHZ 8
> +#define DA732X_MCLK_VAL_0_10MHZ 0
> +#define DA732X_MCLK_VAL_10_20MHZ 1
> +#define DA732X_MCLK_VAL_20_40MHZ 2
> +#define DA732X_MCLK_VAL_40_54MHZ 3
> #define DA732X_DAI_ID1 0
> #define DA732X_DAI_ID2 1
> #define DA732X_SRCCLK_PLL 0
> --
> 2.25.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 2/2] ASoC: da732x: simplify code
2021-04-15 16:00 ` Adam Thomson
@ 2021-04-15 16:04 ` Mark Brown
-1 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2021-04-15 16:04 UTC (permalink / raw)
To: Adam Thomson
Cc: Pierre-Louis Bossart, alsa-devel, tiwai, linux-kernel,
Support Opensource, Liam Girdwood, Jaroslav Kysela, Takashi Iwai
[-- Attachment #1: Type: text/plain, Size: 537 bytes --]
On Thu, Apr 15, 2021 at 04:00:48PM +0000, Adam Thomson wrote:
> On 26 March 2021 22:16, Pierre-Louis Bossart wrote:
> Apologies for the delay in getting to this. The change looks fine to me,
> although this part was EOL some time back, and I find it hard to believe anyone
> out there has a board with this on. Wondering if it would make sense to remove
> the driver permanently?
Unless it's actually getting in the way it's generally easier to just
leave the driver than try to figure out if anyone is updating a system
that uses it.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCH 2/2] ASoC: da732x: simplify code
@ 2021-04-15 16:04 ` Mark Brown
0 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2021-04-15 16:04 UTC (permalink / raw)
To: Adam Thomson
Cc: alsa-devel, Support Opensource, tiwai, linux-kernel,
Takashi Iwai, Liam Girdwood, Pierre-Louis Bossart
[-- Attachment #1: Type: text/plain, Size: 537 bytes --]
On Thu, Apr 15, 2021 at 04:00:48PM +0000, Adam Thomson wrote:
> On 26 March 2021 22:16, Pierre-Louis Bossart wrote:
> Apologies for the delay in getting to this. The change looks fine to me,
> although this part was EOL some time back, and I find it hard to believe anyone
> out there has a board with this on. Wondering if it would make sense to remove
> the driver permanently?
Unless it's actually getting in the way it's generally easier to just
leave the driver than try to figure out if anyone is updating a system
that uses it.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [RFC PATCH 2/2] ASoC: da732x: simplify code
2021-04-15 16:04 ` Mark Brown
@ 2021-04-15 16:09 ` Adam Thomson
-1 siblings, 0 replies; 13+ messages in thread
From: Adam Thomson @ 2021-04-15 16:09 UTC (permalink / raw)
To: Mark Brown, Adam Thomson
Cc: Pierre-Louis Bossart, alsa-devel, tiwai, linux-kernel,
Support Opensource, Liam Girdwood, Jaroslav Kysela, Takashi Iwai
On 15 April 2021 17:04, Mark Brown wrote:
> On Thu, Apr 15, 2021 at 04:00:48PM +0000, Adam Thomson wrote:
> > On 26 March 2021 22:16, Pierre-Louis Bossart wrote:
>
> > Apologies for the delay in getting to this. The change looks fine to me,
> > although this part was EOL some time back, and I find it hard to believe anyone
> > out there has a board with this on. Wondering if it would make sense to
> remove
> > the driver permanently?
>
> Unless it's actually getting in the way it's generally easier to just
> leave the driver than try to figure out if anyone is updating a system
> that uses it.
Fair enough. Just don't want to waste people's time with unnecessary updates :)
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [RFC PATCH 2/2] ASoC: da732x: simplify code
@ 2021-04-15 16:09 ` Adam Thomson
0 siblings, 0 replies; 13+ messages in thread
From: Adam Thomson @ 2021-04-15 16:09 UTC (permalink / raw)
To: Mark Brown, Adam Thomson
Cc: alsa-devel, Support Opensource, tiwai, linux-kernel,
Takashi Iwai, Liam Girdwood, Pierre-Louis Bossart
On 15 April 2021 17:04, Mark Brown wrote:
> On Thu, Apr 15, 2021 at 04:00:48PM +0000, Adam Thomson wrote:
> > On 26 March 2021 22:16, Pierre-Louis Bossart wrote:
>
> > Apologies for the delay in getting to this. The change looks fine to me,
> > although this part was EOL some time back, and I find it hard to believe anyone
> > out there has a board with this on. Wondering if it would make sense to
> remove
> > the driver permanently?
>
> Unless it's actually getting in the way it's generally easier to just
> leave the driver than try to figure out if anyone is updating a system
> that uses it.
Fair enough. Just don't want to waste people's time with unnecessary updates :)
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-04-15 16:10 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-26 22:16 [RFC PATCH 0/2] ASoC: remove cppchecks warnings on lm49453 and da732x Pierre-Louis Bossart
2021-03-26 22:16 ` [RFC PATCH 1/2] ASoC: lm49453: fix useless assignment before return Pierre-Louis Bossart
2021-03-26 22:16 ` Pierre-Louis Bossart
2021-03-26 22:16 ` [RFC PATCH 2/2] ASoC: da732x: simplify code Pierre-Louis Bossart
2021-03-26 22:16 ` Pierre-Louis Bossart
2021-04-15 16:00 ` Adam Thomson
2021-04-15 16:00 ` Adam Thomson
2021-04-15 16:04 ` Mark Brown
2021-04-15 16:04 ` Mark Brown
2021-04-15 16:09 ` Adam Thomson
2021-04-15 16:09 ` Adam Thomson
2021-04-01 10:16 ` [RFC PATCH 0/2] ASoC: remove cppchecks warnings on lm49453 and da732x Mark Brown
2021-04-01 10:16 ` Mark Brown
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.