| Message ID | 1392818784-6248-2-git-send-email-liam.r.girdwood@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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 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 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! :)
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.
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).
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
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(+)