| Message ID | 20250304142620.582191-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | snd: hda: realtek: fix incorrect IS_REACHABLE() usage | expand |
On Tue, 04 Mar 2025 15:25:55 +0100, Arnd Bergmann wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > The alternative path leads to a build error after a recent change: > > sound/pci/hda/patch_realtek.c: In function 'alc233_fixup_lenovo_low_en_micmute_led': > include/linux/stddef.h:9:14: error: called object is not a function or function pointer > 9 | #define NULL ((void *)0) > | ^ > sound/pci/hda/patch_realtek.c:5041:49: note: in expansion of macro 'NULL' > 5041 | #define alc233_fixup_lenovo_line2_mic_hotkey NULL > | ^~~~ > sound/pci/hda/patch_realtek.c:5063:9: note: in expansion of macro 'alc233_fixup_lenovo_line2_mic_hotkey' > 5063 | alc233_fixup_lenovo_line2_mic_hotkey(codec, fix, action); > > Using IS_REACHABLE() is somewhat questionable here anyway since it > leads to the input code not working when the HDA driver is builtin > but input is in a loadable module. Replace this with a hard compile-time > dependency on CONFIG_INPUT. In practice this won't chance much > other than solve the compiler error because it is rare to require > sound output but no input support. > > Fixes: f603b159231b ("ALSA: hda/realtek - add supported Mic Mute LED for Lenovo platform") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Thanks for the patch. Indeed it's a very corner case, but I still hesitate to add a kconfig dependency. Can we take an alternative fix to define the proper dummy functions like below instead? Takashi -- 8< -- --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -5038,8 +5038,21 @@ static void alc233_fixup_lenovo_line2_mic_hotkey(struct hda_codec *codec, } } #else /* INPUT */ -#define alc280_fixup_hp_gpio2_mic_hotkey NULL -#define alc233_fixup_lenovo_line2_mic_hotkey NULL +static void alc280_fixup_hp_gpio2_mic_hotkey(struct hda_codec *codec, + const struct hda_fixup *fix, + int action) +{ + if (action == HDA_FIXUP_ACT_PRE_PROBE) + codec_info(codec, "Input is disabled, skipping quirk\n"); +} + +static void alc233_fixup_lenovo_line2_mic_hotkey(struct hda_codec *codec, + const struct hda_fixup *fix, + int action) +{ + if (action == HDA_FIXUP_ACT_PRE_PROBE) + codec_info(codec, "Input is disabled, skipping quirk\n"); +} #endif /* INPUT */ static void alc269_fixup_hp_line1_mic1_led(struct hda_codec *codec,
On Tue, Mar 4, 2025, at 15:43, Takashi Iwai wrote: > On Tue, 04 Mar 2025 15:25:55 +0100, > > Thanks for the patch. Indeed it's a very corner case, but I still > hesitate to add a kconfig dependency. Can we take an alternative fix > to define the proper dummy functions like below instead? I'm sure that works, and I had considered something like that, but I really dislike the IS_REACHABLE() use because I think it causes more problems than it solves. (I introduced the macro originally but regret that). Note that the only way to disable input is to have manually flip CONFIG_EXPERT and CONFIG_TTY as well as the major GPU drivers that select it: config INPUT tristate "Generic input layer (needed for keyboard, mouse, ...)" if EXPERT default y so in the end, there is very little difference between your patch and mine, as they both fix build testing on randconfig. Arnd
On Tue, 04 Mar 2025 15:54:33 +0100, Arnd Bergmann wrote: > > On Tue, Mar 4, 2025, at 15:43, Takashi Iwai wrote: > > On Tue, 04 Mar 2025 15:25:55 +0100, > > > > Thanks for the patch. Indeed it's a very corner case, but I still > > hesitate to add a kconfig dependency. Can we take an alternative fix > > to define the proper dummy functions like below instead? > > I'm sure that works, and I had considered something like that, > but I really dislike the IS_REACHABLE() use because I think it > causes more problems than it solves. (I introduced the macro > originally but regret that). > > Note that the only way to disable input is to have manually flip > CONFIG_EXPERT and CONFIG_TTY as well as the major GPU drivers > that select it: > > config INPUT > tristate "Generic input layer (needed for keyboard, mouse, ...)" if EXPERT > default y > > so in the end, there is very little difference between > your patch and mine, as they both fix build testing on > randconfig. OK, I don't mind much, so I take your patch now, as this actually reduces the code size. thanks, Takashi
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 9f68cb73b54f..fb955a205d50 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -222,6 +222,7 @@ comment "Set to Y if you want auto-loading the side codec driver" config SND_HDA_CODEC_REALTEK tristate "Build Realtek HD-audio codec support" + depends on INPUT select SND_HDA_GENERIC select SND_HDA_GENERIC_LEDS select SND_HDA_SCODEC_COMPONENT diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index ebf54ef5877a..697a38e41e16 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -4927,7 +4927,6 @@ static void alc298_fixup_samsung_amp_v2_4_amps(struct hda_codec *codec, alc298_samsung_v2_init_amps(codec, 4); } -#if IS_REACHABLE(CONFIG_INPUT) static void gpio2_mic_hotkey_event(struct hda_codec *codec, struct hda_jack_callback *event) { @@ -5036,10 +5035,6 @@ static void alc233_fixup_lenovo_line2_mic_hotkey(struct hda_codec *codec, spec->kb_dev = NULL; } } -#else /* INPUT */ -#define alc280_fixup_hp_gpio2_mic_hotkey NULL -#define alc233_fixup_lenovo_line2_mic_hotkey NULL -#endif /* INPUT */ static void alc269_fixup_hp_line1_mic1_led(struct hda_codec *codec, const struct hda_fixup *fix, int action)