Date: Tue, 11 Oct 2022 07:30:10 -0600 From: Warner Losh <imp@bsdimp.com> To: Tom Jones <thj@freebsd.org> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: eee0f7aea425 - main - acpi: Put CPPC workaround behind i386/amd64 if def Message-ID: <CANCZdfpXQgEZu6XDDJPvT8ctfScY79ZeZfiLBhektmKYp_JzGw@mail.gmail.com> In-Reply-To: <202210110832.29B8WnnR003119@gitrepo.freebsd.org> References: <202210110832.29B8WnnR003119@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] 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 [-- Attachment #2 --] <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Oct 11, 2022 at 2:32 AM Tom Jones <<a href="mailto:thj@freebsd.org">thj@freebsd.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The branch main has been updated by thj:<br> <br> URL: <a href="https://cgit.FreeBSD.org/src/commit/?id=eee0f7aea42564fe005c74f004d63f8cc170ef59" rel="noreferrer" target="_blank">https://cgit.FreeBSD.org/src/commit/?id=eee0f7aea42564fe005c74f004d63f8cc170ef59</a><br>; <br> commit eee0f7aea42564fe005c74f004d63f8cc170ef59<br> Author: Tom Jones <thj@FreeBSD.org><br> AuthorDate: 2022-10-11 08:30:34 +0000<br> Commit: Tom Jones <thj@FreeBSD.org><br> CommitDate: 2022-10-11 08:31:22 +0000<br> <br> acpi: Put CPPC workaround behind i386/amd64 if def<br> <br> While CPPC is available on arm64 platforms with ACPI we don't know if we<br> need to work around issues with firmware there.<br> ---<br> sys/dev/acpica/acpi_cpu.c | 4 ++++<br> 1 file changed, 4 insertions(+)<br> <br> diff --git a/sys/dev/acpica/acpi_cpu.c b/sys/dev/acpica/acpi_cpu.c<br> index 49d2bd11fdaa..ea99cfdeb90f 100644<br> --- a/sys/dev/acpica/acpi_cpu.c<br> +++ b/sys/dev/acpica/acpi_cpu.c<br> @@ -153,7 +153,9 @@ static struct sysctl_ctx_list cpu_sysctl_ctx;<br> static struct sysctl_oid *cpu_sysctl_tree;<br> static int cpu_cx_generic;<br> static int cpu_cx_lowest_lim;<br> +#if defined(__i386__) || defined(__amd64__)<br> static bool cppc_notify;<br> +#endif<br> <br> static struct acpi_cpu_softc **cpu_softc;<br> ACPI_SERIAL_DECL(cpu, "ACPI CPU");<br> @@ -985,11 +987,13 @@ acpi_cpu_startup(void *arg)<br> NULL, 0, acpi_cpu_global_cx_lowest_sysctl, "A",<br> "Global lowest Cx sleep state to use");<br> <br> +#if defined(__i386__) || defined(__amd64__)<br> /* Add sysctl handler to control registering for CPPC notifications */<br> cppc_notify = 1;<br> SYSCTL_ADD_BOOL(&cpu_sysctl_ctx, SYSCTL_CHILDREN(cpu_sysctl_tree),<br> OID_AUTO, "cppc_notify", CTLFLAG_RDTUN | CTLFLAG_MPSAFE,<br> &cppc_notify, 0, "Register for CPPC Notifications");<br> +#endif<br> <br> /* Take over idling from cpu_idle_default(). */<br> cpu_cx_lowest_lim = 0;<br></blockquote><div><br></div><div>I'd suggest changing the #ifdef to something more like:</div><div><br></div><div>#if defined(__i386__) || defined (__amd64__)</div><div>cppc_notify=1;</div><div>#else</div><div>cppc_notify=0;</div><div>#endif </div><div>SYSCTL_ADD...</div><div><br></div><div>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__) :)</div><div><br></div><div>Warner</div></div></div>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfpXQgEZu6XDDJPvT8ctfScY79ZeZfiLBhektmKYp_JzGw>