[PATCH] ASoC: SDCA: fix HID dependency

[PATCH] ASoC: SDCA: fix HID dependency

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

It is not possible to enable SND_SOC_SDCA_HID when SND_SOC_SDCA is built-in
but HID is in a loadable module, as that results in a link failure:

x86_64-linux-ld: sound/soc/sdca/sdca_functions.o: in function `find_sdca_entity_hide':
sdca_functions.c:(.text+0x25b): undefined reference to `sdca_add_hid_device'

Change SND_SOC_SDCA_HID into a 'bool' option that can only be enabled
if this results in a working build, and change the Makefile so this driver
is a loadable module if possible.

Fixes: ac558015dfd8 ("ASoC: SDCA: add a HID device for HIDE entity")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 sound/soc/sdca/Kconfig  | 3 ++-
 sound/soc/sdca/Makefile | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sdca/Kconfig b/sound/soc/sdca/Kconfig
index 53f6926255ae..36c2fed54dad 100644
--- a/sound/soc/sdca/Kconfig
+++ b/sound/soc/sdca/Kconfig
@@ -12,8 +12,9 @@ config SND_SOC_SDCA_OPTIONAL
 	def_tristate SND_SOC_SDCA || !SND_SOC_SDCA
 
 config SND_SOC_SDCA_HID
-	tristate "SDCA HID support"
+	bool "SDCA HID support"
 	depends on SND_SOC_SDCA && HID
+	depends on HID=y || HID=SND_SOC_SDCA
 
 config SND_SOC_SDCA_IRQ
 	tristate
diff --git a/sound/soc/sdca/Makefile b/sound/soc/sdca/Makefile
index 2a3938d11ca9..1efc869c6cbc 100644
--- a/sound/soc/sdca/Makefile
+++ b/sound/soc/sdca/Makefile
@@ -5,5 +5,7 @@ snd-soc-sdca-hid-y := sdca_hid.o
 snd-soc-sdca-irq-y := sdca_interrupts.o
 
 obj-$(CONFIG_SND_SOC_SDCA)	+= snd-soc-sdca.o
-obj-$(CONFIG_SND_SOC_SDCA_HID)	+= snd-soc-sdca-hid.o
+ifdef CONFIG_SND_SOC_SDCA_HID
+obj-$(CONFIG_SND_SOC_SDCA)	+= snd-soc-sdca-hid.o
+endif
 obj-$(CONFIG_SND_SOC_SDCA_IRQ)	+= snd-soc-sdca-irq.o
-- 
2.39.5

Re: [PATCH] ASoC: SDCA: fix HID dependency

[Charles Keepax]

On Tue, Jul 08, 2025 at 08:46:06PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> It is not possible to enable SND_SOC_SDCA_HID when SND_SOC_SDCA is built-in
> but HID is in a loadable module, as that results in a link failure:
> 
> x86_64-linux-ld: sound/soc/sdca/sdca_functions.o: in function `find_sdca_entity_hide':
> sdca_functions.c:(.text+0x25b): undefined reference to `sdca_add_hid_device'
> 
> Change SND_SOC_SDCA_HID into a 'bool' option that can only be enabled
> if this results in a working build, and change the Makefile so this driver
> is a loadable module if possible.
> 
> Fixes: ac558015dfd8 ("ASoC: SDCA: add a HID device for HIDE entity")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---

Thanks for spotting this.

>  sound/soc/sdca/Kconfig  | 3 ++-
>  sound/soc/sdca/Makefile | 4 +++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/sdca/Kconfig b/sound/soc/sdca/Kconfig
> index 53f6926255ae..36c2fed54dad 100644
> --- a/sound/soc/sdca/Kconfig
> +++ b/sound/soc/sdca/Kconfig
> @@ -12,8 +12,9 @@ config SND_SOC_SDCA_OPTIONAL
>  	def_tristate SND_SOC_SDCA || !SND_SOC_SDCA
>  
>  config SND_SOC_SDCA_HID
> -	tristate "SDCA HID support"
> +	bool "SDCA HID support"
>  	depends on SND_SOC_SDCA && HID
> +	depends on HID=y || HID=SND_SOC_SDCA

Does it perhaps make more sense to add a (HID || !HID) dependency
on SND_SOC_SDCA and leave the dependencies here alone? I feel
like that lets the Kconfig figure out the right settings for
SND_SOC_SDCA automatically, rather than just disappearing the SDCA
HID option when HID and SDCA are set incompatibly.

>  
>  config SND_SOC_SDCA_IRQ
>  	tristate
> diff --git a/sound/soc/sdca/Makefile b/sound/soc/sdca/Makefile
> index 2a3938d11ca9..1efc869c6cbc 100644
> --- a/sound/soc/sdca/Makefile
> +++ b/sound/soc/sdca/Makefile
> @@ -5,5 +5,7 @@ snd-soc-sdca-hid-y := sdca_hid.o
>  snd-soc-sdca-irq-y := sdca_interrupts.o
>  
>  obj-$(CONFIG_SND_SOC_SDCA)	+= snd-soc-sdca.o
> -obj-$(CONFIG_SND_SOC_SDCA_HID)	+= snd-soc-sdca-hid.o
> +ifdef CONFIG_SND_SOC_SDCA_HID
> +obj-$(CONFIG_SND_SOC_SDCA)	+= snd-soc-sdca-hid.o
> +endif

Yeah I think making HID more of an option on the module rather
than a module in its own right makes a lot of sense.

We should probably do something similar with the IRQ stuff I will
look at doing a patch for that once we have sorted this one.

Thanks,
Charles

Re: [PATCH] ASoC: SDCA: fix HID dependency

[Arnd Bergmann]

On Wed, Jul 9, 2025, at 14:38, Charles Keepax wrote:
> On Tue, Jul 08, 2025 at 08:46:06PM +0200, Arnd Bergmann wrote:
>>  
>>  config SND_SOC_SDCA_HID
>> -	tristate "SDCA HID support"
>> +	bool "SDCA HID support"
>>  	depends on SND_SOC_SDCA && HID
>> +	depends on HID=y || HID=SND_SOC_SDCA
>
> Does it perhaps make more sense to add a (HID || !HID) dependency
> on SND_SOC_SDCA and leave the dependencies here alone? I feel
> like that lets the Kconfig figure out the right settings for
> SND_SOC_SDCA automatically, rather than just disappearing the SDCA
> HID option when HID and SDCA are set incompatibly.

The problem is that SND_SOC_SDCA is not a user-visible option
but instead gets selected by SND_SOC_ACPI_INTEL_SDCA_QUIRKS,
which itself gets selected indirectly.

It could work, but then the 'depends on HID || !HID' would
need to be under SND_SST_ATOM_HIFI2_PLATFORM_ACPI and anything
else that might select SND_SOC_ACPI_INTEL_MATCH,
SND_SOC_ACPI_INTEL_SDCA_QUIRKS, or SND_SOC_SDCA in the future.

     Arnd

Re: [PATCH] ASoC: SDCA: fix HID dependency

[Charles Keepax]

On Wed, Jul 09, 2025 at 03:36:49PM +0200, Arnd Bergmann wrote:
> On Wed, Jul 9, 2025, at 14:38, Charles Keepax wrote:
> > On Tue, Jul 08, 2025 at 08:46:06PM +0200, Arnd Bergmann wrote:
> >>  
> >>  config SND_SOC_SDCA_HID
> >> -	tristate "SDCA HID support"
> >> +	bool "SDCA HID support"
> >>  	depends on SND_SOC_SDCA && HID
> >> +	depends on HID=y || HID=SND_SOC_SDCA
> >
> > Does it perhaps make more sense to add a (HID || !HID) dependency
> > on SND_SOC_SDCA and leave the dependencies here alone? I feel
> > like that lets the Kconfig figure out the right settings for
> > SND_SOC_SDCA automatically, rather than just disappearing the SDCA
> > HID option when HID and SDCA are set incompatibly.
> 
> The problem is that SND_SOC_SDCA is not a user-visible option
> but instead gets selected by SND_SOC_ACPI_INTEL_SDCA_QUIRKS,
> which itself gets selected indirectly.
> 
> It could work, but then the 'depends on HID || !HID' would
> need to be under SND_SST_ATOM_HIFI2_PLATFORM_ACPI and anything
> else that might select SND_SOC_ACPI_INTEL_MATCH,
> SND_SOC_ACPI_INTEL_SDCA_QUIRKS, or SND_SOC_SDCA in the future.

Ah yes I forgot things are selected by the Intel stuff, at least
the double depends on HID can be changed though? ie.

depends on SND_SOC_SDCA
depends on HID=y || HID=SND_SOC_SDCA

Thanks,
Charles

Re: [PATCH] ASoC: SDCA: fix HID dependency

[Arnd Bergmann]

On Wed, Jul 9, 2025, at 17:04, Charles Keepax wrote:
> On Wed, Jul 09, 2025 at 03:36:49PM +0200, Arnd Bergmann wrote:
>> On Wed, Jul 9, 2025, at 14:38, Charles Keepax wrote:
>> > On Tue, Jul 08, 2025 at 08:46:06PM +0200, Arnd Bergmann wrote:
>> It could work, but then the 'depends on HID || !HID' would
>> need to be under SND_SST_ATOM_HIFI2_PLATFORM_ACPI and anything
>> else that might select SND_SOC_ACPI_INTEL_MATCH,
>> SND_SOC_ACPI_INTEL_SDCA_QUIRKS, or SND_SOC_SDCA in the future.
>
> Ah yes I forgot things are selected by the Intel stuff, at least
> the double depends on HID can be changed though? ie.
>
> depends on SND_SOC_SDCA
> depends on HID=y || HID=SND_SOC_SDCA

Yes, that functionally the same, I wasn't sure which one
you'd prefer. Let me know if I should send a v2.

     Arnd

Re: [PATCH] ASoC: SDCA: fix HID dependency

[Charles Keepax]

On Wed, Jul 09, 2025 at 05:13:05PM +0200, Arnd Bergmann wrote:
> On Wed, Jul 9, 2025, at 17:04, Charles Keepax wrote:
> > On Wed, Jul 09, 2025 at 03:36:49PM +0200, Arnd Bergmann wrote:
> >> On Wed, Jul 9, 2025, at 14:38, Charles Keepax wrote:
> >> > On Tue, Jul 08, 2025 at 08:46:06PM +0200, Arnd Bergmann wrote:
> >> It could work, but then the 'depends on HID || !HID' would
> >> need to be under SND_SST_ATOM_HIFI2_PLATFORM_ACPI and anything
> >> else that might select SND_SOC_ACPI_INTEL_MATCH,
> >> SND_SOC_ACPI_INTEL_SDCA_QUIRKS, or SND_SOC_SDCA in the future.
> >
> > Ah yes I forgot things are selected by the Intel stuff, at least
> > the double depends on HID can be changed though? ie.
> >
> > depends on SND_SOC_SDCA
> > depends on HID=y || HID=SND_SOC_SDCA
> 
> Yes, that functionally the same, I wasn't sure which one
> you'd prefer. Let me know if I should send a v2.

Yeah lets do a v2 without the duplicated depends and feel free to
add:

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Also thanks for the patience with my questions. I will have a
look at moving the IRQ bits over to a similar scheme to keep
things consisent and hopefully avoid future issues.

Thanks,
Charles

Re: [PATCH] ASoC: SDCA: fix HID dependency

[Mark Brown]

On Tue, 08 Jul 2025 20:46:06 +0200, Arnd Bergmann wrote:
> It is not possible to enable SND_SOC_SDCA_HID when SND_SOC_SDCA is built-in
> but HID is in a loadable module, as that results in a link failure:
> 
> x86_64-linux-ld: sound/soc/sdca/sdca_functions.o: in function `find_sdca_entity_hide':
> sdca_functions.c:(.text+0x25b): undefined reference to `sdca_add_hid_device'
> 
> Change SND_SOC_SDCA_HID into a 'bool' option that can only be enabled
> if this results in a working build, and change the Makefile so this driver
> is a loadable module if possible.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: SDCA: fix HID dependency
      commit: ad4655653a6c463026ed3c300e5fb34f39abff48

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark