[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <da5bf77b-5bdb-440f-92b5-db35d8687987@intel.com>
Date: Mon, 16 Jun 2025 10:23:25 +0200
From: Cezary Rojewski <cezary.rojewski@...el.com>
To: "Ahmed S. Darwish" <darwi@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>
CC: Thomas Gleixner <tglx@...utronix.de>, Andrew Cooper
<andrew.cooper3@...rix.com>, "H. Peter Anvin" <hpa@...or.com>, Peter Zijlstra
<peterz@...radead.org>, Sean Christopherson <seanjc@...gle.com>, Sohil Mehta
<sohil.mehta@...el.com>, Ard Biesheuvel <ardb@...nel.org>, John Ogness
<john.ogness@...utronix.de>, <x86@...nel.org>, <x86-cpuid@...ts.linux.dev>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 02/44] ASoC: Intel: avs: Include CPUID header at file
scope
On 2025-06-13 1:39 AM, Ahmed S. Darwish wrote:
> Commit
>
> cbe37a4d2b3c ("ASoC: Intel: avs: Configure basefw on TGL-based platforms")
>
> includes the main CPUID header from within a C function. This obviously
> works by luck and forbids valid refactorings 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.
The existing location of #include isn't my best work, clearly. Thank you
for addressing that, Ahmed. For the avs-driver bits:
Acked-by: Cezary Rojewski <cezary.rojewski@...el.com>
> Signed-off-by: Ahmed S. Darwish <darwi@...utronix.de>
> ---
> sound/soc/intel/avs/tgl.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/sound/soc/intel/avs/tgl.c b/sound/soc/intel/avs/tgl.c
> index 9dbb3ad0954a..cf19d3a7ced2 100644
> --- a/sound/soc/intel/avs/tgl.c
> +++ b/sound/soc/intel/avs/tgl.c
> @@ -10,8 +10,6 @@
> #include "avs.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;
> @@ -39,22 +37,31 @@ static int avs_tgl_dsp_core_stall(struct avs_dev *adev, u32 core_mask, bool stal
> return avs_dsp_core_stall(adev, core_mask, stall);
> }
>
> +#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 */
> +
> static int avs_tgl_config_basefw(struct avs_dev *adev)
> {
> + unsigned int freq = intel_crystal_freq_hz();
> 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 (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);
Powered by blists - more mailing lists