Re: [PATCH v15 3/6] ASoC: es9356-sdca: Add ES9356 SDCA driver

[Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>]

On 5/19/26 00:46, Nathan Chancellor wrote:
> Hi,
> 
> On Thu, May 14, 2026 at 03:52:03PM +0800, Zhang Yi wrote:
>> This is the codec driver for es9356-sdca.
>>
>> Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
> ...
>> diff --git a/sound/soc/codecs/es9356.c b/sound/soc/codecs/es9356.c
>> new file mode 100644
>> index 000000000..78fddd9d0
>> --- /dev/null
>> +++ b/sound/soc/codecs/es9356.c
> ...
>> +static const struct snd_kcontrol_new es9356_sdca_controls[] = {
>> +	SDCA_SINGLE_Q78_TLV("FU41 Left Playback Volume",
>> +		SDW_SDCA_CTL(FUNC_NUM_UAJ, ES9356_SDCA_ENT_FU41, ES9356_SDCA_CTL_FU_VOLUME, CH_L),
>> +		ES9356_VOLUME_MIN, ES9356_VOLUME_MAX, ES9356_VOLUME_STEP, out_vol_tlv),
>> +	SDCA_SINGLE_Q78_TLV("FU41 Right Playback Volume",
>> +		SDW_SDCA_CTL(FUNC_NUM_UAJ, ES9356_SDCA_ENT_FU41, ES9356_SDCA_CTL_FU_VOLUME, CH_R),
>> +		ES9356_VOLUME_MIN, ES9356_VOLUME_MAX, ES9356_VOLUME_STEP, out_vol_tlv),
>> +	SDCA_SINGLE_Q78_TLV("FU36 Left Capture Volume",
>> +		SDW_SDCA_CTL(FUNC_NUM_UAJ, ES9356_SDCA_ENT_FU36, ES9356_SDCA_CTL_FU_VOLUME, CH_L),
>> +		ES9356_VOLUME_MIN, ES9356_VOLUME_MAX, ES9356_VOLUME_STEP, out_vol_tlv),
>> +	SDCA_SINGLE_Q78_TLV("FU36 Right Capture Volume",
>> +		SDW_SDCA_CTL(FUNC_NUM_UAJ, ES9356_SDCA_ENT_FU36, ES9356_SDCA_CTL_FU_VOLUME, CH_R),
>> +		ES9356_VOLUME_MIN, ES9356_VOLUME_MAX, ES9356_VOLUME_STEP, out_vol_tlv),
>> +	SDCA_SINGLE_Q78_TLV("FU33 Capture Volume",
>> +		SDW_SDCA_CTL(FUNC_NUM_UAJ, ES9356_SDCA_ENT_FU33, ES9356_SDCA_CTL_FU_CH_GAIN, 0),
>> +		ES9356_GAIN_MIN, ES9356_AMIC_GAIN_MAX, ES9356_AMIC_GAIN_STEP, amic_gain_tlv),
>> +	SDCA_SINGLE_Q78_TLV("FU21 Left Playback Volume",
>> +		SDW_SDCA_CTL(FUNC_NUM_AMP, ES9356_SDCA_ENT_FU21, ES9356_SDCA_CTL_FU_VOLUME, CH_L),
>> +		ES9356_VOLUME_MIN, ES9356_VOLUME_MAX, ES9356_VOLUME_STEP, out_vol_tlv),
>> +	SDCA_SINGLE_Q78_TLV("FU21 Right Playback Volume",
>> +		SDW_SDCA_CTL(FUNC_NUM_AMP, ES9356_SDCA_ENT_FU21, ES9356_SDCA_CTL_FU_VOLUME, CH_R),
>> +		ES9356_VOLUME_MIN, ES9356_VOLUME_MAX, ES9356_VOLUME_STEP, out_vol_tlv),
>> +	SDCA_SINGLE_Q78_TLV("FU113 Left Capture Volume",
>> +		SDW_SDCA_CTL(FUNC_NUM_MIC, ES9356_SDCA_ENT_FU113, ES9356_SDCA_CTL_FU_VOLUME, CH_L),
>> +		ES9356_VOLUME_MIN, ES9356_VOLUME_MAX, ES9356_VOLUME_STEP, out_vol_tlv),
>> +	SDCA_SINGLE_Q78_TLV("FU113 Right Capture Volume",
>> +		SDW_SDCA_CTL(FUNC_NUM_MIC, ES9356_SDCA_ENT_FU113, ES9356_SDCA_CTL_FU_VOLUME, CH_R),
>> +		ES9356_VOLUME_MIN, ES9356_VOLUME_MAX, ES9356_VOLUME_STEP, out_vol_tlv),
>> +	SDCA_SINGLE_Q78_TLV("FU11 Capture Volume",
>> +		SDW_SDCA_CTL(FUNC_NUM_MIC, ES9356_SDCA_ENT_FU11, ES9356_SDCA_CTL_FU_CH_GAIN, 0),
>> +		ES9356_GAIN_MIN, ES9356_DMIC_GAIN_MAX, ES9356_DMIC_GAIN_STEP, dmic_gain_tlv),
>> +};
> 
> The use of SDCA_SINGLE_Q78_TLV() can break the build for architectures
> that do not support ACPI, as CONFIG_SND_SOC_SDCA depends on ACPI and
> the driver does not actually select it, so it might not be enabled if
> another driver that does select it is not enabled:
> 
>   $ printf "CONFIG_%s=m\n" SOUNDWIRE SND_SOC_ES9356 >arch/arm/configs/repro.config
> 
>   $ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- mrproper defconfig repro.config all
>   ERROR: modpost: "sdca_asoc_q78_get_volsw" [sound/soc/codecs/snd-soc-es9356.ko] undefined!
>   ERROR: modpost: "sdca_asoc_q78_put_volsw" [sound/soc/codecs/snd-soc-es9356.ko] undefined!
>   ...
> 
> Maybe something like this is needed?
> 
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index 5fdd0334c355..b9e8df5ddfd5 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -1302,7 +1302,9 @@ config SND_SOC_ES8389
>  
>  config SND_SOC_ES9356
>          tristate "Everest Semi ES9356 CODEC SDW"
> +	depends on ACPI
>          depends on SOUNDWIRE
> +	select SND_SOC_SDCA
>          select REGMAP_SOUNDWIRE
>          select REGMAP_SOUNDWIRE_MBQ

Maybe this instead?

depends on SOUNDWIRE
depends on SND_SOC_SDCA