Re: [PATCH 3/3] ALSA: hda: register selectively for SPT-LP

[Takashi Iwai <tiwai@suse.de>]

At Thu, 30 Apr 2015 21:33:55 +0100,
Mark Brown wrote:
> 
> On Thu, Apr 30, 2015 at 08:22:36PM +0530, Vinod Koul wrote:
> 
> > -	/* Sunrise Point-LP */
> > -	{ PCI_DEVICE(0x8086, 0x9d70),
> > -	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE },
> 
> > +static const struct pci_device_id azx_intel_adsp_ids[] = {
> > +	/* Sunrise Point-LP */
> > +	{ PCI_DEVICE(0x8086, 0x9d70),
> > +	  .driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_SKYLAKE },
> > +	{ 0, }
> > +};
> > +MODULE_DEVICE_TABLE(pci, azx_intel_adsp_ids);
> > +
> 
> >  static int __init azx_module_init(void)
> >  {
> >  	int ret;
> >  
> >  	ret = pci_register_driver(&azx_driver);
> >  
> > +	if (!hdac_adsp_enable)
> > +		ret = pci_register_driver(&azx_intel_adsp_driver);
> 
> This feels like the wrong idiom here.  I'd expect us to do this by
> binding to the device but ignoring the DSP functionality, or if the
> device has only DSP functionality (it's not clear with the context I
> have but I think that's the case) just printing a message and not
> telling the sound core about it.  Having the driver just fail to load
> seems like it might be a bit obscure.
> 
> Though to be honest if it *is* going to register as a separate HDA
> device then it seems like userspace quirking for this is just as viable.

Ideally, it'd be best if the system can do everything automatically,
e.g. loads the DSP driver at first, then falls back to the legacy
driver if no DSP is found.  But there seems no clean way to do this,
so far.  Thus the flag was introduced.

BTW, the patch won't work as expected.  The device list is created at
the module build time (modpost), thus it must expose all devices,
including SKL.  The patch essentially excludes SKL PCI ID from the
list.

Another way to skip the unwanted driver is just to return -ENODEV from
its probe callback when the given PCI is SKL and the flag is set.
Then the driver core ignores this and continues to the next possible
driver (i.e. the dsp one).  The same should be applied to hda-soc-skl
driver in vice versa.


Takashi