Re: [PATCH 1/9] ASoC: Intel: Fix Kconfig with top-level selector

[Andy Shevchenko]

On Thu, 2017-12-14 at 18:44 -0600, Pierre-Louis Bossart wrote:
> Follow network example suggested by Linus, move Intel definitions
> in if/endif block and clarify in help text which options distro
> configurations should enable - everything except legacy Baytrail stuff
> and
> NOCODEC (test only)
> 
> To avoid user confusion, machine drivers are handled with a submenu
> made
> dependent on this top-level selector.
> 
> There should be no functionality change - except that sound
> capabilities
> are restored when using older configs without any user selection.
> 
> Note that the SND_SOC_ACPI_INTEL_MATCH config is not filtered out by
> the top-level selector since it will also be selected with the
> upcoming
> SOF drivers. Likewise the machine drivers are filtered by a top-level
> selector which will allow for selection/reuse of the same machine
> driver
> with existing SST or SOF-based platform drivers.
> 
> (simplification with submenu for machine drivers by Vinod Koul)

My comments below.

> +if SND_SOC_INTEL_SST_TOPLEVEL

...

> +
> +endif ## SND_SOC_INTEL_SST_TOPLEVEL

> +# configs common to SST and SOF to use matching tables
> +
> +config SND_SOC_ACPI_INTEL_MATCH
> +	tristate
> +	depends on X86 && ACPI
> +	select SND_SOC_ACPI

> +	# this option controls the compilation of ACPI matching
> tables and

this -> This ?

> +	# helpers and is not meant to be selected by the user. It is
> not
> +	# filtered out on purpose by the top-level selector since it
> will
> +	# be selected by SST or SOF platform driver options

>  if SND_SOC_INTEL_MACH
 
> +if SND_SOC_INTEL_HASWELL

...

> +endif
> +
> +if SND_SOC_INTEL_BAYTRAIL
>  

>  config SND_SOC_INTEL_BYT_MAX98090_MACH
>  	tristate "ASoC Audio driver for Intel Baytrail with MAX98090
> codec"
>  	depends on X86_INTEL_LPSS && I2C
> -	depends on SND_SST_IPC_ACPI = n
> -	depends on SND_SOC_INTEL_BAYTRAIL
>  	select SND_SOC_MAX98090
>  	help
>  	  This adds audio driver for Intel Baytrail platform based
> boards
> -	  with the MAX98090 audio codec.
> +	  with the MAX98090 audio codec. This driver is deprecated,
> use
> +	  SND_SOC_INTEL_CHT_BSW_MAX98090_TI_MACH instead for better
> +	  functionality.

Looking to somehow established practice (few other Kconfigs in the
kernel) I would suggest to add
" (DEPRECATED)" to the tristate help string. 

> +endif
> +
> +if SND_SST_ATOM_HIFI2_PLATFORM

...
 
> +endif
> +
> +if SND_SOC_INTEL_SKYLAKE

...

>            If unsure select "N".

> -

I would rather not remove this empty line.

>  endif
> +
> +endif ## SND_SOC_INTEL_MACH

-- 
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy