[alsa-devel] [PATCH 0/5] ASoC: Fix trival sparse warnings
Hi, This series fixes a couple of trivial issues that are reported when running the sparse codechecker on the ASoC tree. - Lars Lars-Peter Clausen (5): ASoC: tlv320aic31xx: Remove duplicate const ASoC: cs42xx8: Make of match table static ASoC: omap-pcm: Include omap-pcm.h ASoC: rcar: Fix dma direction type ASoC: sh/fsi: Make one-bit bitfields unsigned sound/soc/codecs/cs42xx8.c | 2 +- sound/soc/codecs/tlv320aic31xx.c | 21 ++++++++++----------- sound/soc/omap/omap-pcm.c | 1 + sound/soc/sh/fsi.c | 12 ++++++------ sound/soc/sh/rcar/core.c | 2 +- sound/soc/sh/rcar/rsnd.h | 2 +- 6 files changed, 20 insertions(+), 20 deletions(-) -- 1.8.0
SOC_ENUM_SINGLE_DECL() already adds the const qualifier, so there is no need to manually specify it. Fixes the following warnings from sparse: sound/soc/codecs/tlv320aic31xx.c:253:1: warning: duplicate const sound/soc/codecs/tlv320aic31xx.c:255:1: warning: duplicate const sound/soc/codecs/tlv320aic31xx.c:257:1: warning: duplicate const sound/soc/codecs/tlv320aic31xx.c:260:1: warning: duplicate const sound/soc/codecs/tlv320aic31xx.c:262:1: warning: duplicate const Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- sound/soc/codecs/tlv320aic31xx.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/sound/soc/codecs/tlv320aic31xx.c b/sound/soc/codecs/tlv320aic31xx.c index 2341910..5128e66 100644 --- a/sound/soc/codecs/tlv320aic31xx.c +++ b/sound/soc/codecs/tlv320aic31xx.c @@ -249,17 +249,16 @@ static const char * const mic_select_text[] = { "Off", "FFR 10 Ohm", "FFR 20 Ohm", "FFR 40 Ohm" }; -static const -SOC_ENUM_SINGLE_DECL(mic1lp_p_enum, AIC31XX_MICPGAPI, 6, mic_select_text); -static const -SOC_ENUM_SINGLE_DECL(mic1rp_p_enum, AIC31XX_MICPGAPI, 4, mic_select_text); -static const -SOC_ENUM_SINGLE_DECL(mic1lm_p_enum, AIC31XX_MICPGAPI, 2, mic_select_text); - -static const -SOC_ENUM_SINGLE_DECL(cm_m_enum, AIC31XX_MICPGAMI, 6, mic_select_text); -static const -SOC_ENUM_SINGLE_DECL(mic1lm_m_enum, AIC31XX_MICPGAMI, 4, mic_select_text); +static SOC_ENUM_SINGLE_DECL(mic1lp_p_enum, AIC31XX_MICPGAPI, 6, + mic_select_text); +static SOC_ENUM_SINGLE_DECL(mic1rp_p_enum, AIC31XX_MICPGAPI, 4, + mic_select_text); +static SOC_ENUM_SINGLE_DECL(mic1lm_p_enum, AIC31XX_MICPGAPI, 2, + mic_select_text); + +static SOC_ENUM_SINGLE_DECL(cm_m_enum, AIC31XX_MICPGAMI, 6, mic_select_text); +static SOC_ENUM_SINGLE_DECL(mic1lm_m_enum, AIC31XX_MICPGAMI, 4, + mic_select_text); static const DECLARE_TLV_DB_SCALE(dac_vol_tlv, -6350, 50, 0); static const DECLARE_TLV_DB_SCALE(adc_fgain_tlv, 0, 10, 0); -- 1.8.0
On Thu, Jun 19, 2014 at 09:40:27AM +0200, Lars-Peter Clausen wrote:
SOC_ENUM_SINGLE_DECL() already adds the const qualifier, so there is no need to manually specify it.
Applied, thanks.
The cs42xx8_of_match table is not used outside of the driver, hence it can and should be made static. Fixes the following warning from sparse: sound/soc/codecs/cs42xx8.c:425:27: warning: symbol 'cs42xx8_of_match' was not declared. Should it be static? Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- sound/soc/codecs/cs42xx8.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/codecs/cs42xx8.c b/sound/soc/codecs/cs42xx8.c index a25bc60..ec53ffc 100644 --- a/sound/soc/codecs/cs42xx8.c +++ b/sound/soc/codecs/cs42xx8.c @@ -422,7 +422,7 @@ const struct cs42xx8_driver_data cs42888_data = { }; EXPORT_SYMBOL_GPL(cs42888_data); -const struct of_device_id cs42xx8_of_match[] = { +static const struct of_device_id cs42xx8_of_match[] = { { .compatible = "cirrus,cs42448", .data = &cs42448_data, }, { .compatible = "cirrus,cs42888", .data = &cs42888_data, }, { /* sentinel */ } -- 1.8.0
On Thu, 19 Jun 2014, Lars-Peter Clausen wrote:
The cs42xx8_of_match table is not used outside of the driver, hence it can and should be made static.
Fixes the following warning from sparse: sound/soc/codecs/cs42xx8.c:425:27: warning: symbol 'cs42xx8_of_match' was not declared. Should it be static?
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> Acked-by: Brian Austin <brian.austin@cirrus.com>
Thanks,
On Thu, Jun 19, 2014 at 09:40:28AM +0200, Lars-Peter Clausen wrote:
The cs42xx8_of_match table is not used outside of the driver, hence it can and should be made static.
Applied, thanks.
omap_pcm_platform_register() is declared in omap-pcm.h and defined in omap-pcm.c. To make sure that the function signature matches for both omap-pcm.c should include omap-pcm.h Fixes the following warning from sparse: sound/soc/omap/omap-pcm.c:235:5: warning: symbol 'omap_pcm_platform_register' was not declared. Should it be static? Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- sound/soc/omap/omap-pcm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c index 8d809f8..f4b05bc 100644 --- a/sound/soc/omap/omap-pcm.c +++ b/sound/soc/omap/omap-pcm.c @@ -31,6 +31,7 @@ #include <sound/pcm_params.h> #include <sound/dmaengine_pcm.h> #include <sound/soc.h> +#include <sound/omap-pcm.h> #ifdef CONFIG_ARCH_OMAP1 #define pcm_omap1510() cpu_is_omap1510() -- 1.8.0
On Thu, Jun 19, 2014 at 09:40:29AM +0200, Lars-Peter Clausen wrote:
omap_pcm_platform_register() is declared in omap-pcm.h and defined in omap-pcm.c. To make sure that the function signature matches for both omap-pcm.c should include omap-pcm.h
Applied, thanks.
On 06/21/2014 10:00 PM, Mark Brown wrote:
On Thu, Jun 19, 2014 at 09:40:29AM +0200, Lars-Peter Clausen wrote:
omap_pcm_platform_register() is declared in omap-pcm.h and defined in omap-pcm.c. To make sure that the function signature matches for both omap-pcm.c should include omap-pcm.h
Applied, thanks.
This patch somehow did not make it, the patch is not in asoc/next, nor is there a asoc/topic/omap or similar. Shall I resend? Thanks, - Lars
On Mon, Jun 30, 2014 at 06:48:33PM +0200, Lars-Peter Clausen wrote:
On 06/21/2014 10:00 PM, Mark Brown wrote:
Applied, thanks.
This patch somehow did not make it, the patch is not in asoc/next, nor is there a asoc/topic/omap or similar. Shall I resend?
It seems to be there?
On 06/30/2014 11:54 PM, Mark Brown wrote:
On Mon, Jun 30, 2014 at 06:48:33PM +0200, Lars-Peter Clausen wrote:
On 06/21/2014 10:00 PM, Mark Brown wrote:
Applied, thanks.
This patch somehow did not make it, the patch is not in asoc/next, nor is there a asoc/topic/omap or similar. Shall I resend?
It seems to be there?
Now it is. Thanks, - Lars
dmaengine_prep_slave_single() expects a enum dma_transfer_direction and not a enum dma_data_direction. Since the integer representations of both DMA_TO_DEVICE and DMA_MEM_TO_DEV aswell as DMA_FROM_DEVICE and DMA_DEV_TO_MEM have the same value the code worked fine even though it was using the wrong type. Fixes the following warning from sparse: sound/soc/sh/rcar/core.c:227:49: warning: mixing different enum types sound/soc/sh/rcar/core.c:227:49: int enum dma_data_direction versus sound/soc/sh/rcar/core.c:227:49: int enum dma_transfer_direction Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- sound/soc/sh/rcar/core.c | 2 +- sound/soc/sh/rcar/rsnd.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c index 4e86265..518a78c 100644 --- a/sound/soc/sh/rcar/core.c +++ b/sound/soc/sh/rcar/core.c @@ -363,7 +363,7 @@ int rsnd_dma_init(struct rsnd_priv *priv, struct rsnd_dma *dma, if (ret < 0) goto rsnd_dma_init_err; - dma->dir = is_play ? DMA_TO_DEVICE : DMA_FROM_DEVICE; + dma->dir = is_play ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM; INIT_WORK(&dma->work, rsnd_dma_do_work); return 0; diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h index 39d98af..067a89e 100644 --- a/sound/soc/sh/rcar/rsnd.h +++ b/sound/soc/sh/rcar/rsnd.h @@ -158,7 +158,7 @@ struct rsnd_dma { struct sh_dmae_slave slave; struct work_struct work; struct dma_chan *chan; - enum dma_data_direction dir; + enum dma_transfer_direction dir; int submit_loop; int offset; /* it cares A/B plane */ -- 1.8.0
Hi Lars
dmaengine_prep_slave_single() expects a enum dma_transfer_direction and not a enum dma_data_direction. Since the integer representations of both DMA_TO_DEVICE and DMA_MEM_TO_DEV aswell as DMA_FROM_DEVICE and DMA_DEV_TO_MEM have the same value the code worked fine even though it was using the wrong type.
Fixes the following warning from sparse: sound/soc/sh/rcar/core.c:227:49: warning: mixing different enum types sound/soc/sh/rcar/core.c:227:49: int enum dma_data_direction versus sound/soc/sh/rcar/core.c:227:49: int enum dma_transfer_direction
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> ---
Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Actually, ${LINUX}/sound/soc/sh/fsi.c :: fsi_dma_transfer() is using enum dma_data_direction to dmaengine_prep_dma_cyclic() Does it have same issue ?
sound/soc/sh/rcar/core.c | 2 +- sound/soc/sh/rcar/rsnd.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c index 4e86265..518a78c 100644 --- a/sound/soc/sh/rcar/core.c +++ b/sound/soc/sh/rcar/core.c @@ -363,7 +363,7 @@ int rsnd_dma_init(struct rsnd_priv *priv, struct rsnd_dma *dma, if (ret < 0) goto rsnd_dma_init_err;
- dma->dir = is_play ? DMA_TO_DEVICE : DMA_FROM_DEVICE; + dma->dir = is_play ? DMA_MEM_TO_DEV : DMA_DEV_TO_MEM; INIT_WORK(&dma->work, rsnd_dma_do_work);
return 0; diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h index 39d98af..067a89e 100644 --- a/sound/soc/sh/rcar/rsnd.h +++ b/sound/soc/sh/rcar/rsnd.h @@ -158,7 +158,7 @@ struct rsnd_dma { struct sh_dmae_slave slave; struct work_struct work; struct dma_chan *chan; - enum dma_data_direction dir; + enum dma_transfer_direction dir;
int submit_loop; int offset; /* it cares A/B plane */ -- 1.8.0
On 06/19/2014 11:12 AM, Kuninori Morimoto wrote:
Hi Lars
dmaengine_prep_slave_single() expects a enum dma_transfer_direction and not a enum dma_data_direction. Since the integer representations of both DMA_TO_DEVICE and DMA_MEM_TO_DEV aswell as DMA_FROM_DEVICE and DMA_DEV_TO_MEM have the same value the code worked fine even though it was using the wrong type.
Fixes the following warning from sparse: sound/soc/sh/rcar/core.c:227:49: warning: mixing different enum types sound/soc/sh/rcar/core.c:227:49: int enum dma_data_direction versus sound/soc/sh/rcar/core.c:227:49: int enum dma_transfer_direction
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> ---
Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Actually, ${LINUX}/sound/soc/sh/fsi.c :: fsi_dma_transfer() is using enum dma_data_direction to dmaengine_prep_dma_cyclic() Does it have same issue ?
Yes, but the fix for that should go in after your "ASoC: fsi: use SNDRV_DMA_TYPE_DEV for sound buffer" patch, because otherwise the fix just introduces a new warning because before that patch dir is also passed to dma_sync_single_xxx() which wants a enum dma_data_direction. - Lars
Hi Lars
dmaengine_prep_slave_single() expects a enum dma_transfer_direction and not a enum dma_data_direction. Since the integer representations of both DMA_TO_DEVICE and DMA_MEM_TO_DEV aswell as DMA_FROM_DEVICE and DMA_DEV_TO_MEM have the same value the code worked fine even though it was using the wrong type.
Fixes the following warning from sparse: sound/soc/sh/rcar/core.c:227:49: warning: mixing different enum types sound/soc/sh/rcar/core.c:227:49: int enum dma_data_direction versus sound/soc/sh/rcar/core.c:227:49: int enum dma_transfer_direction
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> ---
Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Actually, ${LINUX}/sound/soc/sh/fsi.c :: fsi_dma_transfer() is using enum dma_data_direction to dmaengine_prep_dma_cyclic() Does it have same issue ?
Yes, but the fix for that should go in after your "ASoC: fsi: use SNDRV_DMA_TYPE_DEV for sound buffer" patch, because otherwise the fix just introduces a new warning because before that patch dir is also passed to dma_sync_single_xxx() which wants a enum dma_data_direction.
Ahh, yes, indeed Thank you for your explain
On Thu, Jun 19, 2014 at 09:40:30AM +0200, Lars-Peter Clausen wrote:
dmaengine_prep_slave_single() expects a enum dma_transfer_direction and not a enum dma_data_direction. Since the integer representations of both DMA_TO_DEVICE and DMA_MEM_TO_DEV aswell as DMA_FROM_DEVICE and DMA_DEV_TO_MEM have the same value the code worked fine even though it was using the wrong type.
Applied, thanks.
One-bit signed bitfields have two possible values: 0 and -1. This sometimes leads to unexpected results (e.g. foo.bar = 1; foo.bar == 1 => false) which is why it is recommended to make one-bit bitfields unsigned. This fixes the following sparse warnings: sound/soc/sh/fsi.c:267:25: error: dubious one-bit signed bitfield sound/soc/sh/fsi.c:268:22: error: dubious one-bit signed bitfield sound/soc/sh/fsi.c:269:20: error: dubious one-bit signed bitfield sound/soc/sh/fsi.c:270:28: error: dubious one-bit signed bitfield sound/soc/sh/fsi.c:271:26: error: dubious one-bit signed bitfield sound/soc/sh/fsi.c:272:25: error: dubious one-bit signed bitfield Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- sound/soc/sh/fsi.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 710a079..2c95d85 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -264,12 +264,12 @@ struct fsi_priv { u32 fmt; int chan_num:16; - int clk_master:1; - int clk_cpg:1; - int spdif:1; - int enable_stream:1; - int bit_clk_inv:1; - int lr_clk_inv:1; + unsigned int clk_master:1; + unsigned int clk_cpg:1; + unsigned int spdif:1; + unsigned int enable_stream:1; + unsigned int bit_clk_inv:1; + unsigned int lr_clk_inv:1; }; struct fsi_stream_handler { -- 1.8.0
Hi
One-bit signed bitfields have two possible values: 0 and -1. This sometimes leads to unexpected results (e.g. foo.bar = 1; foo.bar == 1 => false) which is why it is recommended to make one-bit bitfields unsigned.
This fixes the following sparse warnings: sound/soc/sh/fsi.c:267:25: error: dubious one-bit signed bitfield sound/soc/sh/fsi.c:268:22: error: dubious one-bit signed bitfield sound/soc/sh/fsi.c:269:20: error: dubious one-bit signed bitfield sound/soc/sh/fsi.c:270:28: error: dubious one-bit signed bitfield sound/soc/sh/fsi.c:271:26: error: dubious one-bit signed bitfield sound/soc/sh/fsi.c:272:25: error: dubious one-bit signed bitfield
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> ---
Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
sound/soc/sh/fsi.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 710a079..2c95d85 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -264,12 +264,12 @@ struct fsi_priv { u32 fmt;
int chan_num:16; - int clk_master:1; - int clk_cpg:1; - int spdif:1; - int enable_stream:1; - int bit_clk_inv:1; - int lr_clk_inv:1; + unsigned int clk_master:1; + unsigned int clk_cpg:1; + unsigned int spdif:1; + unsigned int enable_stream:1; + unsigned int bit_clk_inv:1; + unsigned int lr_clk_inv:1; };
struct fsi_stream_handler { -- 1.8.0
On Thu, Jun 19, 2014 at 09:40:31AM +0200, Lars-Peter Clausen wrote:
One-bit signed bitfields have two possible values: 0 and -1. This sometimes leads to unexpected results (e.g. foo.bar = 1; foo.bar == 1 => false) which is why it is recommended to make one-bit bitfields unsigned.
Applied, thanks.
participants (4)
-
Brian Austin -
Kuninori Morimoto -
Lars-Peter Clausen -
Mark Brown