[PATCHv2] ASoC: Intel: Fix sparse warnings for firmware loader.

[PATCHv2] ASoC: Intel: Fix sparse warnings for firmware loader.

[Liam Girdwood]

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;
 
-- 
1.8.3.2

[PATCH] ASoC: Intel: Make sure we only build SST drivers on X86

[Liam Girdwood]

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
-- 
1.8.3.2

Re: [PATCH] ASoC: Intel: Make sure we only build SST drivers on X86

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 664 bytes --]

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).

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCHv2] ASoC: Intel: Fix sparse warnings for firmware loader.

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 406 bytes --]

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.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: Intel: Make sure we only build SST drivers on X86

[Takashi Iwai]

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

Re: [PATCH] ASoC: Intel: Make sure we only build SST drivers on X86

[Liam Girdwood]

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 

Re: [PATCH] ASoC: Intel: Make sure we only build SST drivers on X86

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 831 bytes --]

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! :)

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: Intel: Make sure we only build SST drivers on X86

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1173 bytes --]

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.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: Intel: Make sure we only build SST drivers on X86

[Takashi Iwai]

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

Re: [PATCH] ASoC: Intel: Make sure we only build SST drivers on X86

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 386 bytes --]

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).

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]