[alsa-devel] [PATCHv2] ASoC: Intel: Fix sparse warnings for firmware loader.
Changes since V1 :- o Fixed sparse warning for block_module_remove o Fixed additional sparse warnings when building entire driver.
Sparse gives us the following warnings on sst-firmware.c
CHECK sound/soc/intel/sst-firmware.c sound/soc/intel/sst-firmware.c:39:34: warning: incorrect type in argument 1 (different address spaces) sound/soc/intel/sst-firmware.c:39:34: expected void volatile [noderef] asn:2*dst sound/soc/intel/sst-firmware.c:39:34: got void * sound/soc/intel/sst-firmware.c:417:36: warning: incorrect type in argument 1 (different address spaces) sound/soc/intel/sst-firmware.c:417:36: expected void *dest sound/soc/intel/sst-firmware.c:417:36: got void [noderef] asn:2* sound/soc/intel/sst-firmware.c:430:5: warning: symbol 'sst_block_module_remove' was not declared. Should it be static?
and
CC [M] sound/soc/intel/sst-dsp.o sound/soc/intel/sst-dsp-priv.h:271:53: warning: incorrect type in argument 3 (different address spaces) sound/soc/intel/sst-dsp-priv.h:271:53: expected void *src sound/soc/intel/sst-dsp-priv.h:271:53: got void [noderef] asn:2* sound/soc/intel/sst-dsp-priv.h:271:53: warning: incorrect type in argument 3 (different address spaces) sound/soc/intel/sst-dsp-priv.h:271:53: expected void *src sound/soc/intel/sst-dsp-priv.h:271:53: got void [noderef] asn:2* sound/soc/intel/sst-dsp-priv.h:271:53: warning: incorrect type in argument 3 (different address spaces) sound/soc/intel/sst-dsp-priv.h:271:53: expected void *src sound/soc/intel/sst-dsp-priv.h:271:53: got void [noderef] asn:2*
This patch removes these warnings
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com --- sound/soc/intel/sst-dsp-priv.h | 7 +++++-- sound/soc/intel/sst-firmware.c | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/sound/soc/intel/sst-dsp-priv.h b/sound/soc/intel/sst-dsp-priv.h index 6135564..fe8e81a 100644 --- a/sound/soc/intel/sst-dsp-priv.h +++ b/sound/soc/intel/sst-dsp-priv.h @@ -41,8 +41,10 @@ struct sst_ops { u64 (*read64)(void __iomem *addr, u32 offset);
/* DSP I/DRAM IO */ - void (*ram_read)(struct sst_dsp *sst, void *dest, void *src, size_t bytes); - void (*ram_write)(struct sst_dsp *sst, void *dest, void *src, size_t bytes); + void (*ram_read)(struct sst_dsp *sst, void *dest, void __iomem *src, + size_t bytes); + void (*ram_write)(struct sst_dsp *sst, void __iomem *dest, void *src, + size_t bytes);
void (*dump)(struct sst_dsp *);
@@ -296,6 +298,7 @@ struct sst_module *sst_module_get_from_id(struct sst_dsp *dsp, u32 id); struct sst_module *sst_mem_block_alloc_scratch(struct sst_dsp *dsp); void sst_mem_block_free_scratch(struct sst_dsp *dsp, struct sst_module *scratch); +int sst_block_module_remove(struct sst_module *module);
/* Register the DSPs memory blocks - would be nice to read from ACPI */ struct sst_mem_block *sst_mem_block_register(struct sst_dsp *dsp, u32 offset, diff --git a/sound/soc/intel/sst-firmware.c b/sound/soc/intel/sst-firmware.c index 31cd154..dee7eb5 100644 --- a/sound/soc/intel/sst-firmware.c +++ b/sound/soc/intel/sst-firmware.c @@ -30,7 +30,7 @@ #include "sst-dsp.h" #include "sst-dsp-priv.h"
-static void sst_memcpy32(void *dest, void *src, u32 bytes) +static void sst_memcpy32(volatile void __iomem *dest, void *src, u32 bytes) { u32 i;
Disable build on non X86 architectures. This fixes the following build errors on PPC :-
sound/soc/intel/sst-dsp.c: In function 'sst_dsp_outbox_write': sound/soc/intel/sst-dsp.c:218:2: error: implicit declaration of function 'memcpy_toio' [-Werror=implicit-function-declaration] memcpy_toio(sst->mailbox.out_base, message, bytes); ^ sound/soc/intel/sst-dsp.c: In function 'sst_dsp_outbox_read': sound/soc/intel/sst-dsp.c:231:2: error: implicit declaration of function 'memcpy_fromio' [-Werror=implicit-function-declaration] memcpy_fromio(message, sst->mailbox.out_base, bytes); ^
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com --- sound/soc/intel/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index c962432..e61bd89 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -15,6 +15,7 @@ config SND_SST_MFLD_PLATFORM config SND_SOC_INTEL_SST tristate "ASoC support for Intel(R) Smart Sound Technology" select SND_SOC_INTEL_SST_ACPI if ACPI + depends on X86 help This adds support for Intel(R) Smart Sound Technology (SST). Say Y if you have such a device
On Wed, Feb 19, 2014 at 02:06:24PM +0000, Liam Girdwood wrote:
Disable build on non X86 architectures. This fixes the following build errors on PPC :-
sound/soc/intel/sst-dsp.c: In function 'sst_dsp_outbox_write': sound/soc/intel/sst-dsp.c:218:2: error: implicit declaration of function 'memcpy_toio' [-Werror=implicit-function-declaration] memcpy_toio(sst->mailbox.out_base, message, bytes); ^
But lots of architectures do actually have these operations, I suspect looking at some of the existing users depending on PCI is enough if excessively strict (this will improve build coverage which tends to be useful even if the driver can't be run).
At Thu, 20 Feb 2014 00:00:40 +0900, Mark Brown wrote:
On Wed, Feb 19, 2014 at 02:06:24PM +0000, Liam Girdwood wrote:
Disable build on non X86 architectures. This fixes the following build errors on PPC :-
sound/soc/intel/sst-dsp.c: In function 'sst_dsp_outbox_write': sound/soc/intel/sst-dsp.c:218:2: error: implicit declaration of function 'memcpy_toio' [-Werror=implicit-function-declaration] memcpy_toio(sst->mailbox.out_base, message, bytes); ^
But lots of architectures do actually have these operations, I suspect looking at some of the existing users depending on PCI is enough if excessively strict (this will improve build coverage which tends to be useful even if the driver can't be run).
Yes. I guess including linux/io.h should fix the build issue.
Though, limiting the build to X86 isn't bad particularly for this driver.
Takashi
On Wed, Feb 19, 2014 at 04:21:05PM +0100, Takashi Iwai wrote:
Mark Brown wrote:
But lots of architectures do actually have these operations, I suspect looking at some of the existing users depending on PCI is enough if excessively strict (this will improve build coverage which tends to be useful even if the driver can't be run).
Yes. I guess including linux/io.h should fix the build issue.
It should for PowerPC (and ought to be done anyway since an implicit include is going to break eventually on x86 too) but it won't for architectures that don't have the function at all.
Though, limiting the build to X86 isn't bad particularly for this driver.
Most people working on ASoC don't build x86 that often, more build coverage of these minority platforms is going to help avoid issues for -next! :)
At Thu, 20 Feb 2014 00:57:58 +0900, Mark Brown wrote:
On Wed, Feb 19, 2014 at 04:21:05PM +0100, Takashi Iwai wrote:
Mark Brown wrote:
But lots of architectures do actually have these operations, I suspect looking at some of the existing users depending on PCI is enough if excessively strict (this will improve build coverage which tends to be useful even if the driver can't be run).
Yes. I guess including linux/io.h should fix the build issue.
It should for PowerPC (and ought to be done anyway since an implicit include is going to break eventually on x86 too) but it won't for architectures that don't have the function at all.
memcpy_toio() is supposed to be defined in all architecture. If the arch doesn't support it properly, it still should take from asm-generic/io.h. So, it's a good test coverage for such archs ;)
Takashi
On Wed, Feb 19, 2014 at 05:15:44PM +0100, Takashi Iwai wrote:
memcpy_toio() is supposed to be defined in all architecture. If the arch doesn't support it properly, it still should take from asm-generic/io.h. So, it's a good test coverage for such archs ;)
Ah, so it is - in that case it's just the include that needs to be added (and ideally the depends X86 || COMPILE_TEST).
On Thu, 2014-02-20 at 00:00 +0900, Mark Brown wrote:
On Wed, Feb 19, 2014 at 02:06:24PM +0000, Liam Girdwood wrote:
Disable build on non X86 architectures. This fixes the following build errors on PPC :-
sound/soc/intel/sst-dsp.c: In function 'sst_dsp_outbox_write': sound/soc/intel/sst-dsp.c:218:2: error: implicit declaration of function 'memcpy_toio' [-Werror=implicit-function-declaration] memcpy_toio(sst->mailbox.out_base, message, bytes); ^
But lots of architectures do actually have these operations, I suspect looking at some of the existing users depending on PCI is enough if excessively strict (this will improve build coverage which tends to be useful even if the driver can't be run).
I know that the other architectures will implement their own ops, but no other architecture other than X86 will have Intel SST.
I can either send a new patch that additionally includes linux/io.h or send a V2 ?
Liam
On Wed, Feb 19, 2014 at 03:24:21PM +0000, Liam Girdwood wrote:
On Thu, 2014-02-20 at 00:00 +0900, Mark Brown wrote:
But lots of architectures do actually have these operations, I suspect looking at some of the existing users depending on PCI is enough if excessively strict (this will improve build coverage which tends to be useful even if the driver can't be run).
I know that the other architectures will implement their own ops, but no other architecture other than X86 will have Intel SST.
Yes, it should be something like
depends on (X86 || COMPILE_TEST) && WHATEVER
if there's a WHATEVER needed to guarantee the operation is there (I think PCI does the job). That way only people actively looking to build test will see the option on !X86 but people doing wider scale work can get build coverage more easily.
I can either send a new patch that additionally includes linux/io.h or send a V2 ?
The include of linux/io.h is needed no matter what - if it's being implicitly included on x86 then someone could change the headers and break it later on. If you can send a new patch right now that'd be great.
On Wed, Feb 19, 2014 at 02:06:23PM +0000, Liam Girdwood wrote:
Applied, thanks.
Changes since V1 :- o Fixed sparse warning for block_module_remove o Fixed additional sparse warnings when building entire driver.
Don't put stuff like this in the changelog, if you want to include it put it after the ---. Nobody is going to care when they look at the git log since v1 won't be there.
participants (3)
-
Liam Girdwood -
Mark Brown -
Takashi Iwai