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.