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

[Takashi Iwai]

At Thu, 30 Oct 2014 21:44:28 +0530,
Vinod Koul wrote:
> 
> On Thu, Oct 30, 2014 at 05:49:39PM +0100, Takashi Iwai wrote:
> > At Thu, 30 Oct 2014 21:29:47 +0530,
> > Vinod Koul wrote:
> > > 
> > > On Thu, Oct 30, 2014 at 05:29:00PM +0100, Takashi Iwai wrote:
> > > > At Thu, 30 Oct 2014 21:07:19 +0530,
> > > > Vinod Koul wrote:
> > > > > 
> > > > > On Thu, Oct 30, 2014 at 04:27:09PM +0100, Takashi Iwai wrote:
> > > > > > > > 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.
> > > > > Hmmm, that makes me wonder why cant we do (based on your input above)
> > > > > 
> > > > > snd-intel-sst-y := sst.o sst_ipc.o sst_stream.o sst_drv_interface.o sst_loader.o sst_pvt.o
> > > > > 
> > > > > then either this:
> > > > > snd-intel-sst-$(CONFIG_SND_SST_IPC_PCI) += sst_pci.o
> > > > > snd-intel-sst-$(CONFIG_SND_SST_IPC_ACPI) += sst_acpi.o
> > > > 
> > > > This may compile both sst_pci.c and sst_acpi.c into snd-intel-sst
> > > > module, so it doesn't work unless both Kconfigs are really exclusive.
> > > > 
> > > > > Or:
> > > > > snd-intel-sst-pci-y += snd-intel-sst.o sst_pci.o
> > > > > snd-intel-sst-acpi-y += snd-intel-sst.o sst_acpi.o
> > > > 
> > > > This is wrong in two ways.  First off, you can't link a module against
> > > > another module.
> > > > 
> > > > Second, this causes the problem when user build one driver as a module
> > > > and another as built-in.  Then you'll get the doubly symbols.
> > > oh no, the core is not a module. the PCI and ACPI will be modules and
> > > allowed to be selected.
> > 
> > Then you should have stripped snd- prefix from snd-intel-sst.o.
> > snd-prefix is supposed to be used for the modules.
> Sure I can do that
> 
> so we can have
> 
> sst-intel-core-y := sst.o sst_ipc.o sst_stream.o sst_drv_interface.o sst_loader.o sst_pvt.o
> 
> and then
> 
> snd-intel-sst-pci-y += sst-intel-core.o sst_pci.o
> snd-intel-sst-acpi-y += st-intel-core.o sst_acpi.o

Kbuild doesn't allow this syntax, AFAIK.

> > > Can we prevent the module/built-in by making depends. If one is built-in
> > > other cant be module. That way it would be okay
> > 
> > As said, this isn't the best.  It disallows the compile testing and
> > other tests in a single shot.  Testing with various Kconfigs is more
> > difficult than catching with make allyesconfig (or such).
> well both can be compile tested and we will put in symbols about limitation
> of having bpth apci and pci as built-in or module.

No, the point is that the exclusiveness in Kconfig level gives more
demerits.  This makes impossible to build the both codes in a single
shot, which makes also impossible to cover wider build tests, etc.
My concern isn't about the actual operation but about testing.

That is: don't try to side step such a build issue.  I bet it'll
strike back later.  Better to keep rather the simple and common
approach other drivers take.


Takashi