Re: git: eee0f7aea425 - main - acpi: Put CPPC workaround behind i386/amd64 if def

From: Warner Losh <imp_at_bsdimp.com>
Date: Tue, 11 Oct 2022 13:30:10 UTC
On Tue, Oct 11, 2022 at 2:32 AM Tom Jones <thj@freebsd.org> wrote:

> The branch main has been updated by thj:
>
> URL:
> https://cgit.FreeBSD.org/src/commit/?id=eee0f7aea42564fe005c74f004d63f8cc170ef59
>
> commit eee0f7aea42564fe005c74f004d63f8cc170ef59
> Author:     Tom Jones <thj@FreeBSD.org>
> AuthorDate: 2022-10-11 08:30:34 +0000
> Commit:     Tom Jones <thj@FreeBSD.org>
> CommitDate: 2022-10-11 08:31:22 +0000
>
>     acpi: Put CPPC workaround behind i386/amd64 if def
>
>     While CPPC is available on arm64 platforms with ACPI we don't know if
> we
>     need to work around issues with firmware there.
> ---
>  sys/dev/acpica/acpi_cpu.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/sys/dev/acpica/acpi_cpu.c b/sys/dev/acpica/acpi_cpu.c
> index 49d2bd11fdaa..ea99cfdeb90f 100644
> --- a/sys/dev/acpica/acpi_cpu.c
> +++ b/sys/dev/acpica/acpi_cpu.c
> @@ -153,7 +153,9 @@ static struct sysctl_ctx_list cpu_sysctl_ctx;
>  static struct sysctl_oid *cpu_sysctl_tree;
>  static int              cpu_cx_generic;
>  static int              cpu_cx_lowest_lim;
> +#if defined(__i386__) || defined(__amd64__)
>  static bool             cppc_notify;
> +#endif
>
>  static struct acpi_cpu_softc **cpu_softc;
>  ACPI_SERIAL_DECL(cpu, "ACPI CPU");
> @@ -985,11 +987,13 @@ acpi_cpu_startup(void *arg)
>         NULL, 0, acpi_cpu_global_cx_lowest_sysctl, "A",
>         "Global lowest Cx sleep state to use");
>
> +#if defined(__i386__) || defined(__amd64__)
>      /* Add sysctl handler to control registering for CPPC notifications */
>      cppc_notify = 1;
>      SYSCTL_ADD_BOOL(&cpu_sysctl_ctx, SYSCTL_CHILDREN(cpu_sysctl_tree),
>         OID_AUTO, "cppc_notify", CTLFLAG_RDTUN | CTLFLAG_MPSAFE,
>         &cppc_notify, 0, "Register for CPPC Notifications");
> +#endif
>
>      /* Take over idling from cpu_idle_default(). */
>      cpu_cx_lowest_lim = 0;
>

I'd suggest changing the #ifdef to something more like:

#if defined(__i386__) || defined (__amd64__)
cppc_notify=1;
#else
cppc_notify=0;
#endif
SYSCTL_ADD...

instead. What do you think? I'd have done it as a one-liner, but you can't
say cppc_notify=defined(__i386__) || defined(__amd64__) :)

Warner