Re: [PATCH v4 1/8] ASoC: SDCA: allow building without ACPI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ASoC: SDCA: ACPI parser is unconditionally built on non-ACPI platforms
--

commit b342382e9b5b09b26bd417be9cd5ee4c2d90de61
Author: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>

ASoC: SDCA: allow building without ACPI

This commit drops the ACPI dependency from SND_SOC_SDCA to allow the SDCA
class driver to be built on non-ACPI platforms. It wraps ACPI-parsing
implementations in sdca_device.c and sdca_functions.c with
IS_ENABLED(CONFIG_ACPI) and provides empty stubs when ACPI is off.

> diff --git a/sound/soc/sdca/Kconfig b/sound/soc/sdca/Kconfig
> index 4c0dcb9ff3b94..72c02943c49ed 100644
> --- a/sound/soc/sdca/Kconfig
> +++ b/sound/soc/sdca/Kconfig
> @@ -3,11 +3,13 @@ menu "SoundWire (SDCA)"
>  
>  config SND_SOC_SDCA
>  	tristate "SDCA core support"
> -	depends on ACPI
>  	select AUXILIARY_BUS
>  	help
>  	  This option enables support for the MIPI SoundWire Device
> -	  Class for Audio (SDCA).
> +	  Class for Audio (SDCA).  The ACPI/DisCo topology parser is
> +	  only built when CONFIG_ACPI is enabled; on non-ACPI platforms
> +	  (e.g. ARM/DT) codec drivers supply the SDCA function data
> +	  via sdca_class_hw_ops.populate_function.

[Severity: Medium]
Is this Kconfig help text accurate for SND_SOC_SDCA?

The text claims the ACPI/DisCo topology parser is only built when CONFIG_ACPI
is enabled. However, the CONFIG_ACPI guard in sdca_functions.c is closed
early. When building the kernel with CONFIG_SND_SOC_SDCA=y and CONFIG_ACPI=n,
the rest of the file's DisCo parsing logic is unconditionally compiled.

Note that the related probe failure where sdca_parse_function() was
unconditionally called on non-ACPI platforms is resolved in Patch 4 of this
series, but the dead code and Kconfig discrepancy remains unfixed here.

[ ... ]

> diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c
> index f39008ac0e7d3..1eb6418e132dc 100644
> --- a/sound/soc/sdca/sdca_functions.c
> +++ b/sound/soc/sdca/sdca_functions.c
> @@ -25,6 +25,7 @@
>   */
>  #define SDCA_PROPERTY_LENGTH 64
>  
> +#if IS_ENABLED(CONFIG_ACPI)
>  static int patch_sdca_function_type(u32 interface_revision, u32 *function_type)
>  {
>  	/*
> @@ -201,6 +202,7 @@ void sdca_lookup_functions(struct sdw_slave *slave)
>  	acpi_dev_for_each_child(adev, find_sdca_function, &slave->sdca_data);
>  }
>  EXPORT_SYMBOL_NS(sdca_lookup_functions, "SND_SOC_SDCA");
> +#endif

[Severity: Medium]
Does this #endif prematurely close the CONFIG_ACPI guard?

By closing the guard here after sdca_lookup_functions(), the remaining
topology parser logic in this file, including the entry point
sdca_parse_function() and its helpers, remains unconditionally compiled
on non-ACPI builds.

>  
>  struct raw_init_write {
>  	__le32 addr;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918131914.681588-1-srinivas.kandagatla@oss.qualcomm.com?part=1