On Wed, 18 Dec 2019 16:21:22 +0100,
Pierre-Louis Bossart wrote:
>
> >>> Also, can we reduce even the ifdef around sof_dev_desc definitions by
> >>> __maybe_unused atttribute?
> >>
> >> Sorry, I am not following your suggestion. I would really like to keep
> >> the ifdefs for now, and while it can be seen as overkill to have
> >> descriptors that are identical in some cases the past experience shows
> >> it's useful when we have to add quirks for specific 'hardware
> >> recommended programming sequences'.
> >
> > What I suggested was simple, just dropping ifdef by something like
> >
> > diff --git a/sound/soc/sof/sof-pci-dev.c b/sound/soc/sof/sof-pci-dev.c
> > index bbeffd932de7..297632a54f1b 100644
> > --- a/sound/soc/sof/sof-pci-dev.c
> > +++ b/sound/soc/sof/sof-pci-dev.c
> > @@ -36,8 +36,7 @@ MODULE_PARM_DESC(sof_pci_debug, "SOF PCI debug options (0x0 all off)");
> > #define SOF_PCI_DISABLE_PM_RUNTIME BIT(0)
> > -#if IS_ENABLED(CONFIG_SND_SOC_SOF_APOLLOLAKE)
> > -static const struct sof_dev_desc bxt_desc = {
> > +static const struct sof_dev_desc __maybe_unused bxt_desc = {
> > .machines = snd_soc_acpi_intel_bxt_machines,
> > .resindex_lpe_base = 0,
> > .resindex_pcicfg_base = -1,
> > @@ -52,10 +51,8 @@ static const struct sof_dev_desc bxt_desc = {
> > .ops = &sof_apl_ops,
> > .arch_ops = &sof_xtensa_arch_ops
> > };
> > -#endif
> > -#if IS_ENABLED(CONFIG_SND_SOC_SOF_GEMINILAKE)
> > -static const struct sof_dev_desc glk_desc = {
> > +static const struct sof_dev_desc __maybe_unused glk_desc = {
> > .machines = snd_soc_acpi_intel_glk_machines,
> > .resindex_lpe_base = 0,
> > .resindex_pcicfg_base = -1,
> > @@ -70,10 +67,8 @@ static const struct sof_dev_desc glk_desc = {
> > .ops = &sof_apl_ops,
> > .arch_ops = &sof_xtensa_arch_ops
> > };
> > -#endif
> > .....
> >
> >
> > Then the issue I pointed above can be solved as well.
>
> The ifdefs are still needed in the PCI IDs tables
Yes, but it halves the messes :)
Takashi
>
> static const struct pci_device_id sof_pci_ids[] = {
> #if IS_ENABLED(CONFIG_SND_SOC_SOF_MERRIFIELD)
> { PCI_DEVICE(0x8086, 0x119a),
> .driver_data = (unsigned long)&tng_desc},
> #endif
> #if IS_ENABLED(CONFIG_SND_SOC_SOF_APOLLOLAKE)
> /* BXT-P & Apollolake */
> { PCI_DEVICE(0x8086, 0x5a98),
> .driver_data = (unsigned long)&bxt_desc},
> { PCI_DEVICE(0x8086, 0x1a98),
> .driver_data = (unsigned long)&bxt_desc},
> #endif
> #if IS_ENABLED(CONFIG_SND_SOC_SOF_GEMINILAKE)
> { PCI_DEVICE(0x8086, 0x3198),
> .driver_data = (unsigned long)&glk_desc},
> #endif
>
> so for consistency I personally prefer the matching ifdef for the
> descriptors.
>
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
