Date: Mon, 24 Oct 2022 10:37:19 -0600 From: Warner Losh <imp@bsdimp.com> To: Kyle Evans <kevans@freebsd.org> Cc: Jessica Clarke <jrtc27@freebsd.org>, Takanori Watanabe <takawata@freebsd.org>, src-committers <src-committers@freebsd.org>, "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>, dev-commits-src-main@freebsd.org Subject: Re: git: 9cf5db63698b - main - acpi_ged: fix build, as module and non INTRNG case. Message-ID: <CANCZdforiFcPdMVatmuCPSsDpAdxaaj_ksj-K5D3CyO%2B8N4xTg@mail.gmail.com> In-Reply-To: <CANCZdfpDeF5LGs4oVuupLHrNrHGb2sXJTKpvc_BxsnYxvbDgNQ@mail.gmail.com> References: <202210241439.29OEdKOI013861@gitrepo.freebsd.org> <170CEFAF-A78C-4DD7-A639-3B7090C5D347@freebsd.org> <CACNAnaFNPtAUS1dYEYhzpnpuzEAYmNnTuasxgYN7JzsdDEEzJA@mail.gmail.com> <CANCZdfpDeF5LGs4oVuupLHrNrHGb2sXJTKpvc_BxsnYxvbDgNQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] On Mon, Oct 24, 2022 at 9:28 AM Warner Losh <imp@bsdimp.com> wrote: > > > On Mon, Oct 24, 2022, 9:22 AM Kyle Evans <kevans@freebsd.org> wrote: > >> On Mon, Oct 24, 2022 at 10:16 AM Jessica Clarke <jrtc27@freebsd.org> >> wrote: >> > >> > On 24 Oct 2022, at 15:39, Takanori Watanabe <takawata@FreeBSD.org> >> wrote: >> > > >> > > The branch main has been updated by takawata: >> > > >> > > URL: >> https://cgit.FreeBSD.org/src/commit/?id=9cf5db63698b3c73edd632412bf68735d3c20d37 >> > > >> > > commit 9cf5db63698b3c73edd632412bf68735d3c20d37 >> > > Author: Takanori Watanabe <takawata@FreeBSD.org> >> > > AuthorDate: 2022-10-24 14:19:12 +0000 >> > > Commit: Takanori Watanabe <takawata@FreeBSD.org> >> > > CommitDate: 2022-10-24 14:37:28 +0000 >> > > >> > > acpi_ged: fix build, as module and non INTRNG case. >> > > >> > > Reviewed-by: cy >> > > >> > > Differential Revision: https://reviews.freebsd.org/D37104 >> > > --- >> > > sys/dev/acpica/acpi_ged.c | 2 +- >> > > sys/modules/acpi/acpi_ged/Makefile | 2 +- >> > > 2 files changed, 2 insertions(+), 2 deletions(-) >> > > >> > > diff --git a/sys/dev/acpica/acpi_ged.c b/sys/dev/acpica/acpi_ged.c >> > > index 9459ccc3525b..8ee56c8b0335 100644 >> > > --- a/sys/dev/acpica/acpi_ged.c >> > > +++ b/sys/dev/acpica/acpi_ged.c >> > > @@ -198,7 +198,7 @@ acpi_ged_attach(device_t dev) >> > > } >> > > } >> > > #else >> > > - rawirq = rman_get_start(sc->evt[i].r); >> > > + rawirq = rman_get_start(sc->evts[i].r); >> > > trig = INTR_TRIGGER_LEVEL; >> > > if (ACPI_SUCCESS(acpi_lookup_irq_resource >> > > (dev, sc->evts[i].rid, >> > > diff --git a/sys/modules/acpi/acpi_ged/Makefile >> b/sys/modules/acpi/acpi_ged/Makefile >> > > index a937249357f4..87dd53b88b2b 100644 >> > > --- a/sys/modules/acpi/acpi_ged/Makefile >> > > +++ b/sys/modules/acpi/acpi_ged/Makefile >> > > @@ -1,7 +1,7 @@ >> > > # $FreeBSD$ >> > > >> > > .PATH: ${SRCTOP}/sys/dev/acpica >> > > -.if ${TARGET_ARCH} == aarch64 >> > > +.if ${MACHINE_CPUARCH} == "aarch64" >> > > CFLAGS += -DINTRNG >> > > .endif >> > > KMOD= acpi_ged >> > >> > Why isn’t acpi_ged.c just #include’ing opt_global.h? >> > >> >> I suspect this is trying to cope with untied builds, though I agree >> it's using the wrong approach. We should push this into >> sys/conf/config.mk instead (+ arm, riscv), which should do the right >> thing and actually #define it in opt_global.h (which is included via >> CFLAGS). >> > > I was about to make similar comments. I'll take a look at fixing while > I'm on the plane today... if some else doesn't take a stab I. The mean time. > Consider https://reviews.freebsd.org/D37107 and https://reviews.freebsd.org/D37108 (which don't yet adjust this file). 37107 moves all the options that we want in a opt_global.h file into DEFAULTS and 37108 adjusts config.mk to generate opt_global.h with them in it. Comments? Warner [-- Attachment #2 --] <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Oct 24, 2022 at 9:28 AM Warner Losh <<a href="mailto:imp@bsdimp.com">imp@bsdimp.com</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"><div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Oct 24, 2022, 9:22 AM Kyle Evans <<a href="mailto:kevans@freebsd.org" target="_blank">kevans@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">On Mon, Oct 24, 2022 at 10:16 AM Jessica Clarke <<a href="mailto:jrtc27@freebsd.org" rel="noreferrer" target="_blank">jrtc27@freebsd.org</a>> wrote:<br> ><br> > On 24 Oct 2022, at 15:39, Takanori Watanabe <takawata@FreeBSD.org> wrote:<br> > ><br> > > The branch main has been updated by takawata:<br> > ><br> > > URL: <a href="https://cgit.FreeBSD.org/src/commit/?id=9cf5db63698b3c73edd632412bf68735d3c20d37" rel="noreferrer noreferrer" target="_blank">https://cgit.FreeBSD.org/src/commit/?id=9cf5db63698b3c73edd632412bf68735d3c20d37</a><br>; > ><br> > > commit 9cf5db63698b3c73edd632412bf68735d3c20d37<br> > > Author: Takanori Watanabe <takawata@FreeBSD.org><br> > > AuthorDate: 2022-10-24 14:19:12 +0000<br> > > Commit: Takanori Watanabe <takawata@FreeBSD.org><br> > > CommitDate: 2022-10-24 14:37:28 +0000<br> > ><br> > > acpi_ged: fix build, as module and non INTRNG case.<br> > ><br> > > Reviewed-by: cy<br> > ><br> > > Differential Revision: <a href="https://reviews.freebsd.org/D37104" rel="noreferrer noreferrer" target="_blank">https://reviews.freebsd.org/D37104</a><br>; > > ---<br> > > sys/dev/acpica/acpi_ged.c | 2 +-<br> > > sys/modules/acpi/acpi_ged/Makefile | 2 +-<br> > > 2 files changed, 2 insertions(+), 2 deletions(-)<br> > ><br> > > diff --git a/sys/dev/acpica/acpi_ged.c b/sys/dev/acpica/acpi_ged.c<br> > > index 9459ccc3525b..8ee56c8b0335 100644<br> > > --- a/sys/dev/acpica/acpi_ged.c<br> > > +++ b/sys/dev/acpica/acpi_ged.c<br> > > @@ -198,7 +198,7 @@ acpi_ged_attach(device_t dev)<br> > > }<br> > > }<br> > > #else<br> > > - rawirq = rman_get_start(sc->evt[i].r);<br> > > + rawirq = rman_get_start(sc->evts[i].r);<br> > > trig = INTR_TRIGGER_LEVEL;<br> > > if (ACPI_SUCCESS(acpi_lookup_irq_resource<br> > > (dev, sc->evts[i].rid,<br> > > diff --git a/sys/modules/acpi/acpi_ged/Makefile b/sys/modules/acpi/acpi_ged/Makefile<br> > > index a937249357f4..87dd53b88b2b 100644<br> > > --- a/sys/modules/acpi/acpi_ged/Makefile<br> > > +++ b/sys/modules/acpi/acpi_ged/Makefile<br> > > @@ -1,7 +1,7 @@<br> > > # $FreeBSD$<br> > ><br> > > .PATH: ${SRCTOP}/sys/dev/acpica<br> > > -.if ${TARGET_ARCH} == aarch64<br> > > +.if ${MACHINE_CPUARCH} == "aarch64"<br> > > CFLAGS += -DINTRNG<br> > > .endif<br> > > KMOD= acpi_ged<br> ><br> > Why isn’t acpi_ged.c just #include’ing opt_global.h?<br> ><br> <br> I suspect this is trying to cope with untied builds, though I agree<br> it's using the wrong approach. We should push this into<br> sys/conf/<a href="http://config.mk" rel="noreferrer noreferrer" target="_blank">config.mk</a> instead (+ arm, riscv), which should do the right<br> thing and actually #define it in opt_global.h (which is included via<br> CFLAGS).<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">I was about to make similar comments. I'll take a look at fixing while I'm on the plane today... if some else doesn't take a stab I. The mean time.</div></div></blockquote><div><br></div><div>Consider <a href="https://reviews.freebsd.org/D37107">https://reviews.freebsd.org/D37107</a>; and <a href="https://reviews.freebsd.org/D37108">https://reviews.freebsd.org/D37108</a>; (which don't yet adjust this file). 37107 moves all the options that we want in a opt_global.h file into DEFAULTS and 37108 adjusts <a href="http://config.mk">config.mk</a>; to generate opt_global.h with them in it.</div><div><br></div><div>Comments?</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?CANCZdforiFcPdMVatmuCPSsDpAdxaaj_ksj-K5D3CyO%2B8N4xTg>