Re: [PATCH] sound/pci/hda: depends on instead of select for INPUT

[Takashi Iwai]

On Mon, 15 Jan 2018 19:47:26 +0100,
Randy Dunlap wrote:
> 
> On 01/15/18 01:50, Takashi Iwai wrote:
> > On Mon, 15 Jan 2018 08:50:17 +0100,
> > Takashi Iwai wrote:
> >>
> >> On Mon, 15 Jan 2018 06:11:56 +0100,
> >> Randy Dunlap wrote:
> >>>
> >>> From: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
> >>>
> >>> Drivers should not 'select' a subsystem. Instead they should depend
> >>> on it. If the subsystem is disabled, the user probably did that for
> >>> a purpose and one driver shouldn't be changing that.
> >>>
> >>> This also makes all sound/ drivers consistent w.r.t depending on INPUT
> >>> instead of selecting it.
> >>>
> >>> Signed-off-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
> >>> Cc: Jaroslav Kysela <perex@xxxxxxxx>
> >>> Cc: Takashi Iwai <tiwai@xxxxxxxx>
> >>> Cc: alsa-devel@xxxxxxxxxxxxxxxx (moderated for non-subscribers)
> >>> ---
> >>>  sound/pci/hda/Kconfig |    2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> --- lnx-415-rc8.orig/sound/pci/hda/Kconfig
> >>> +++ lnx-415-rc8/sound/pci/hda/Kconfig
> >>> @@ -87,8 +87,8 @@ config SND_HDA_PATCH_LOADER
> >>>  
> >>>  config SND_HDA_CODEC_REALTEK
> >>>  	tristate "Build Realtek HD-audio codec support"
> >>> +	depends on INPUT
> >>>  	select SND_HDA_GENERIC
> >>> -	select INPUT
> >>
> >> This would break if INPUT=m and SND_HDA_CODEC_REALTEK=y.
> >> Usually, we take a trick like
> >>
> >> 	depends on INPUT=y || INPUT=SND_HDA_CODEC_REALTEK
> >>
> >> But, looking at the change that introduced the dependency (commit
> >> 33f4acd3b214), the code doesn't necessarily depend on INPUT at all.
> >> The select above was put there just because the random build with
> >> INPUT=m and SND_HDA_CODEC_REALTEK=y would break otherwise.
> >>
> >> The right fix in this case would be to replace IS_ENABLE(INPUT) with
> >> IS_REACHABLE(INPUT) instead.
> > 
> > ... that is, a patch like below.
> > 
> > 
> > Takashi
> > 
> > -- 8< --
> > From: Takashi Iwai <tiwai@xxxxxxx>
> > Subject: [PATCH] ALSA: hda - Use IS_REACHABLE() for dependency on input
> > 
> > The commit ffcd28d88e4f ("ALSA: hda - Select INPUT for Realtek
> > HD-audio codec") introduced the reverse-selection of CONFIG_INPUT for
> > Realtek codec in order to avoid the mess with dependency between
> > built-in and modules.  Later on, we obtained IS_REACHABLE() macro
> > exactly for this kind of problems, and now we can remove th INPUT
> > selection in Kconfig and put IS_REACHABLE(INPUT) to the appropriate
> > places in the code, so that the driver doesn't need to select other
> > subsystem forcibly.
> > 
> > Fixes: ffcd28d88e4f ("ALSA: hda - Select INPUT for Realtek HD-audio codec")
> > Reported-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> 
> Acked-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx> # and build-tested

Thanks, the patch is queued now.


Takashi