[alsa-devel] [PATCH 7/9] ASoC: Intel: move PCI probe to a seprate file

[Takashi Iwai]

At Thu, 30 Oct 2014 20:04:44 +0530,
Vinod Koul wrote:
> 
> On Thu, Oct 30, 2014 at 04:03:59PM +0100, Takashi Iwai wrote:
> > At Thu, 30 Oct 2014 16:21:50 +0530,
> > Vinod Koul wrote:
> > > 
> > > From: Subhransu S. Prusty <subhransu.s.prusty at intel.com>
> > > 
> > > This allow the sst.c to be common across PCI and APCI usages
> > > 
> > > Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty at intel.com>
> > > Signed-off-by: Vinod Koul <vinod.koul at intel.com>
> > > 
> > > Author:    Subhransu S. Prusty <subhransu.s.prusty at intel.com>
> > > ---
> > >  sound/soc/intel/Kconfig       |    4 +
> > >  sound/soc/intel/sst/Makefile  |    4 +
> > >  sound/soc/intel/sst/sst.c     |  200 +------------------------------------
> > >  sound/soc/intel/sst/sst.h     |    6 +
> > >  sound/soc/intel/sst/sst_pci.c |  225 +++++++++++++++++++++++++++++++++++++++++
> > >  5 files changed, 240 insertions(+), 199 deletions(-)
> > >  create mode 100644 sound/soc/intel/sst/sst_pci.c
> > > 
> > > diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
> > > index 2a3af88..cbc987e 100644
> > > --- a/sound/soc/intel/Kconfig
> > > +++ b/sound/soc/intel/Kconfig
> > > @@ -4,6 +4,7 @@ config SND_MFLD_MACHINE
> > >  	select SND_SOC_SN95031
> > >  	select SND_SST_MFLD_PLATFORM
> > >  	select SND_SST_IPC
> > > +	select SND_SST_IPC_PCI
> > >  	help
> > >            This adds support for ASoC machine driver for Intel(R) MID Medfield platform
> > >            used as alsa device in audio substem in Intel(R) MID devices
> > > @@ -16,6 +17,9 @@ config SND_SST_MFLD_PLATFORM
> > >  config SND_SST_IPC
> > >  	tristate
> > >  
> > > +config SND_SST_IPC_PCI
> > > +	bool
> > > +
> > >  config SND_SOC_INTEL_SST
> > >  	tristate "ASoC support for Intel(R) Smart Sound Technology"
> > >  	select SND_SOC_INTEL_SST_ACPI if ACPI
> > > diff --git a/sound/soc/intel/sst/Makefile b/sound/soc/intel/sst/Makefile
> > > index 4d0e79b..b3fbccd 100644
> > > --- a/sound/soc/intel/sst/Makefile
> > > +++ b/sound/soc/intel/sst/Makefile
> > > @@ -1,3 +1,7 @@
> > >  snd-intel-sst-objs := sst.o sst_ipc.o sst_stream.o sst_drv_interface.o sst_loader.o sst_pvt.o
> > >  
> > > +ifneq ($(CONFIG_SND_SST_IPC_PCI),)
> > > +snd-intel-sst-objs += sst_pci.o
> > > +endif
> > 
> > The standard way is something like
> > 
> > snd-intel-sst-y := sst.o sst_ipc.o sst_stream.o sst_drv_interface.o sst_loader.o sst_pvt.o
> > snd-intel-sst-$(CONFIG_SND_SST_IPC_PCI) += sst_pci.o
> > 
> > But, when looking at the later patch, you try to build ACPI stuff into
> > snd-intel-sst, too, and both are implemented as exclusive.  This
> > doesn't work well in general.
> The hardware, firmware so the driver is pretty same. So either it gets probed
> as PCI device is SFI platforms and as APCI device on ACPI ones.
> Since the probe method is the only one differing, the machine will select
> either PCI or ACPI. That one would get compiled in.

... and this exclusion mechanism is missing in your patches.
Your current code doesn't allow to mix them.

> Am okay to change if we have better method which works for both

In general, it's better to provide individual modules for different
interfaces and a common module that is referred by them.  The
selective build makes the build tests more difficult and is rather
error-prone.


Takashi