Hi, On 2/8/21 11:04 AM, Takashi Iwai wrote: > On Mon, 08 Feb 2021 10:37:59 +0100, > Hans de Goede wrote: >> >> Instead of hardcording the SST driver having the highest prio, add >> FLAG_BYT_FIRST and FLAG_BYT_SECOND defines, which get set like this >> when both drivers are enabled: >> >> #define FLAG_BYT_FIRST FLAG_SST >> #define FLAG_BYT_SECOND FLAG_SOF >> >> And when only 1 driver is enabled then FLAG_BYT_FIRST gets set to >> the flag for that driver. >> >> This is a preparation patch for making which driver is preferred >> configurable through Kconfig. >> >> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> > > I find the idea is fine, but the ifdef conditions become too complex > after this change. It took minutes to check whether the ifdef changes > are really correct for me :) I understand but... > So, it'd be appreciated if this can be re-designed and simplified... This was actually the cleanest which I could come up with, well maybe not the cleanest, but the most "do not repeat yourself" option. The alternative would be something like this: static const struct config_entry acpi_config_table[] = { /* BayTrail */ #ifdef CONFIG_SND_INTEL_BYT_PREFER_SOF /* implies both drivers are enabled */ { .flags = FLAG_SOF, .acpi_hid = "80860F28", }, { .flags = FLAG_SST, .acpi_hid = "80860F28", }, #else #if IS_ENABLED(CONFIG_SND_SST_ATOM_HIFI2_PLATFORM_ACPI) { .flags = FLAG_SST, .acpi_hid = "80860F28", }, #endif #if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL { .flags = FLAG_SOF, .acpi_hid = "80860F28", }, #endif #endif With the same thing repeating for the Cherry Trail case, now that I actually have written this out I guess it is not too bad, but it does mean repeating all the BYT/CHT entries once, visually leading to 4 extra entries (but the #ifdef #else #endif will always include only 2/4 for each of BYT and CHT. If you like this better I can do a v2 with this approach, that would also reduce the set to a single patch. Regards, Hans