Re: [PATCH v5 01/35] ASoC: Intel: avs: Include CPUID header at file scope

Re: [PATCH v5 01/35] ASoC: Intel: avs: Include CPUID header at file scope

[Borislav Petkov]

On Fri, Sep 05, 2025 at 02:14:41PM +0200, Ahmed S. Darwish wrote:
> +#ifdef CONFIG_X86
> +#include <asm/cpuid/api.h>
> +static unsigned int intel_crystal_freq_hz(void)
> +{
> +	return cpuid_ecx(CPUID_LEAF_TSC);
> +}
> +#else
> +static unsigned int intel_crystal_freq_hz(void)
> +{
> +	return 0;
> +}
> +#endif /* !CONFIG_X86 */

Why are we even bothering with !CONFIG_X86?

Because this thing has || COMPILE_TEST in Kconfig.

But this thing gets enough compile testing on x86 already so why not simply
drop the whole unnecessary gunk?

---

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index 412555e626b8..63367364916a 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -95,7 +95,7 @@ config SND_SOC_INTEL_KEEMBAY
 
 config SND_SOC_INTEL_AVS
 	tristate "Intel AVS driver"
-	depends on X86 || COMPILE_TEST
+	depends on X86
 	depends on PCI
 	depends on COMMON_CLK
 	select ACPI_NHLT if ACPI
diff --git a/sound/soc/intel/avs/tgl.c b/sound/soc/intel/avs/tgl.c
index afb066516101..d920488d24b1 100644
--- a/sound/soc/intel/avs/tgl.c
+++ b/sound/soc/intel/avs/tgl.c
@@ -11,7 +11,7 @@
 #include "debug.h"
 #include "messages.h"
 
-#define CPUID_TSC_LEAF 0x15
+#include <asm/cpuid/api.h>
 
 static int avs_tgl_dsp_core_power(struct avs_dev *adev, u32 core_mask, bool power)
 {
@@ -44,18 +44,15 @@ static int avs_tgl_config_basefw(struct avs_dev *adev)
 {
 	struct pci_dev *pci = adev->base.pci;
 	struct avs_bus_hwid hwid;
+	unsigned int freq;
 	int ret;
-#ifdef CONFIG_X86
-	unsigned int ecx;
 
-#include <asm/cpuid/api.h>
-	ecx = cpuid_ecx(CPUID_TSC_LEAF);
-	if (ecx) {
-		ret = avs_ipc_set_fw_config(adev, 1, AVS_FW_CFG_XTAL_FREQ_HZ, sizeof(ecx), &ecx);
+	freq = cpuid_ecx(CPUID_LEAF_TSC);
+	if (freq) {
+		ret = avs_ipc_set_fw_config(adev, 1, AVS_FW_CFG_XTAL_FREQ_HZ, sizeof(freq), &freq);
 		if (ret)
 			return AVS_IPC_RET(ret);
 	}
-#endif
 
 	hwid.device = pci->device;
 	hwid.subsystem = pci->subsystem_vendor | (pci->subsystem_device << 16);

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v5 01/35] ASoC: Intel: avs: Include CPUID header at file scope

[Ahmed S. Darwish]

On Thu, 16 Oct 2025, Borislav Petkov wrote:
>
> Why are we even bothering with !CONFIG_X86?
>
> Because this thing has || COMPILE_TEST in Kconfig.
>
> But this thing gets enough compile testing on x86 already so why not
> simply drop the whole unnecessary gunk?
>

Makes sense; will do.

Thanks!
Ahmed

Re: [PATCH v5 01/35] ASoC: Intel: avs: Include CPUID header at file scope

[Borislav Petkov]

On Thu, Oct 16, 2025 at 09:21:26PM +0200, Ahmed S. Darwish wrote:
> On Thu, 16 Oct 2025, Borislav Petkov wrote:
> >
> > Why are we even bothering with !CONFIG_X86?
> >
> > Because this thing has || COMPILE_TEST in Kconfig.
> >
> > But this thing gets enough compile testing on x86 already so why not
> > simply drop the whole unnecessary gunk?
> >
> 
> Makes sense; will do.

I'll queue the below soon, if no one complains:

---

From 65453ddafe5d7cf715266f7d32356c42965f2f86 Mon Sep 17 00:00:00 2001
From: "Ahmed S. Darwish" <darwi@linutronix.de>
Date: Fri, 5 Sep 2025 14:14:41 +0200
Subject: [PATCH] ASoC: Intel: avs: Include CPUID header at file scope

Commit

    cbe37a4d2b3c ("ASoC: Intel: avs: Configure basefw on TGL-based platforms")

includes the main CPUID header from within a C function.  This works by
sheer luck and forbids further refactoring inside the CPUID header.

Include the CPUID header at file scope instead.

Note, for the CPUID(0x15) leaf number, use CPUID_LEAF_TSC instead of
defining a custom local macro for it.

  [ bp: Massage, zap the CONFIG_X86 ifdeffery and dependency on
    COMPILE_TEST because this driver gets enough compile testing on x86
    alone already. ]

Signed-off-by: Ahmed S. Darwish <darwi@linutronix.de>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Link: https://patch.msgid.link/20250905121515.192792-2-darwi@linutronix.de
---
 sound/soc/intel/Kconfig   |  2 +-
 sound/soc/intel/avs/tgl.c | 18 ++++++------------
 2 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index 412555e626b8..63367364916a 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -95,7 +95,7 @@ config SND_SOC_INTEL_KEEMBAY
 
 config SND_SOC_INTEL_AVS
 	tristate "Intel AVS driver"
-	depends on X86 || COMPILE_TEST
+	depends on X86
 	depends on PCI
 	depends on COMMON_CLK
 	select ACPI_NHLT if ACPI
diff --git a/sound/soc/intel/avs/tgl.c b/sound/soc/intel/avs/tgl.c
index afb066516101..ba4831f4de5a 100644
--- a/sound/soc/intel/avs/tgl.c
+++ b/sound/soc/intel/avs/tgl.c
@@ -7,12 +7,12 @@
 //
 
 #include <linux/pci.h>
+#include <asm/cpuid/api.h>
+
 #include "avs.h"
 #include "debug.h"
 #include "messages.h"
 
-#define CPUID_TSC_LEAF 0x15
-
 static int avs_tgl_dsp_core_power(struct avs_dev *adev, u32 core_mask, bool power)
 {
 	core_mask &= AVS_MAIN_CORE_MASK;
@@ -42,20 +42,14 @@ static int avs_tgl_dsp_core_stall(struct avs_dev *adev, u32 core_mask, bool stal
 
 static int avs_tgl_config_basefw(struct avs_dev *adev)
 {
+	unsigned int freq = cpuid_ecx(CPUID_LEAF_TSC);
 	struct pci_dev *pci = adev->base.pci;
 	struct avs_bus_hwid hwid;
 	int ret;
-#ifdef CONFIG_X86
-	unsigned int ecx;
 
-#include <asm/cpuid/api.h>
-	ecx = cpuid_ecx(CPUID_TSC_LEAF);
-	if (ecx) {
-		ret = avs_ipc_set_fw_config(adev, 1, AVS_FW_CFG_XTAL_FREQ_HZ, sizeof(ecx), &ecx);
-		if (ret)
-			return AVS_IPC_RET(ret);
-	}
-#endif
+	ret = avs_ipc_set_fw_config(adev, 1, AVS_FW_CFG_XTAL_FREQ_HZ, sizeof(freq), &freq);
+	if (ret)
+		return AVS_IPC_RET(ret);
 
 	hwid.device = pci->device;
 	hwid.subsystem = pci->subsystem_vendor | (pci->subsystem_device << 16);
-- 
2.51.0

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette