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
--00000000000063fae405ebca68a0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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=3D9cf5db63698b3c73edd632412bf687= 35d3c20d37 >> > > >> > > 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 =3D rman_get_start(sc->evt[i].r); >> > > + rawirq =3D rman_get_start(sc->evts[i].r); >> > > trig =3D 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} =3D=3D aarch64 >> > > +.if ${MACHINE_CPUARCH} =3D=3D "aarch64" >> > > CFLAGS +=3D -DINTRNG >> > > .endif >> > > KMOD=3D acpi_ged >> > >> > Why isn=E2=80=99t acpi_ged.c just #include=E2=80=99ing 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 ti= me. > 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 --00000000000063fae405ebca68a0 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">= <div dir=3D"ltr" class=3D"gmail_attr">On Mon, Oct 24, 2022 at 9:28 AM Warne= r Losh <<a href=3D"mailto:imp@bsdimp.com">imp@bsdimp.com</a>> wrote:<= br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8e= x;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir=3D"auto= "><div><br><br><div class=3D"gmail_quote"><div dir=3D"ltr" class=3D"gmail_a= ttr">On Mon, Oct 24, 2022, 9:22 AM Kyle Evans <<a href=3D"mailto:kevans@= freebsd.org" target=3D"_blank">kevans@freebsd.org</a>> wrote:<br></div><= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Oct 24, 2022 at 10= :16 AM Jessica Clarke <<a href=3D"mailto:jrtc27@freebsd.org" rel=3D"nore= ferrer" target=3D"_blank">jrtc27@freebsd.org</a>> wrote:<br> ><br> > On 24 Oct 2022, at 15:39, Takanori Watanabe <takawata@FreeBSD.org&g= t; wrote:<br> > ><br> > > The branch main has been updated by takawata:<br> > ><br> > > URL: <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3D9cf5db6= 3698b3c73edd632412bf68735d3c20d37" rel=3D"noreferrer noreferrer" target=3D"= _blank">https://cgit.FreeBSD.org/src/commit/?id=3D9cf5db63698b3c73edd632412= bf68735d3c20d37</a><br> > ><br> > > commit 9cf5db63698b3c73edd632412bf68735d3c20d37<br> > > Author:=C2=A0 =C2=A0 =C2=A0Takanori Watanabe <takawata@FreeBSD= .org><br> > > AuthorDate: 2022-10-24 14:19:12 +0000<br> > > Commit:=C2=A0 =C2=A0 =C2=A0Takanori Watanabe <takawata@FreeBSD= .org><br> > > CommitDate: 2022-10-24 14:37:28 +0000<br> > ><br> > >=C2=A0 =C2=A0 acpi_ged: fix build, as module and non INTRNG case.<= br> > ><br> > >=C2=A0 =C2=A0 Reviewed-by: cy<br> > ><br> > >=C2=A0 =C2=A0 Differential Revision: <a href=3D"https://reviews.fr= eebsd.org/D37104" rel=3D"noreferrer noreferrer" target=3D"_blank">https://r= eviews.freebsd.org/D37104</a><br> > > ---<br> > > sys/dev/acpica/acpi_ged.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | 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> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0}<br> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}<br> > > #else<br> > > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0rawirq =3D rman_= get_start(sc->evt[i].r);<br> > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0rawirq =3D rman_= get_start(sc->evts[i].r);<br> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0trig =3D IN= TR_TRIGGER_LEVEL;<br> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (ACPI_SU= CCESS(acpi_lookup_irq_resource<br> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(dev, sc->evts[i].rid,<br> > > diff --git a/sys/modules/acpi/acpi_ged/Makefile b/sys/modules/acp= i/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> > > #=C2=A0 =C2=A0 =C2=A0$FreeBSD$<br> > ><br> > > .PATH:=C2=A0 =C2=A0 =C2=A0 =C2=A0 ${SRCTOP}/sys/dev/acpica<br> > > -.if ${TARGET_ARCH} =3D=3D aarch64<br> > > +.if ${MACHINE_CPUARCH} =3D=3D "aarch64"<br> > > CFLAGS +=3D -DINTRNG<br> > > .endif<br> > > KMOD=3D acpi_ged<br> ><br> > Why isn=E2=80=99t acpi_ged.c just #include=E2=80=99ing opt_global.h?<b= r> ><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=3D"http://config.mk" rel=3D"noreferrer noreferrer" target= =3D"_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=3D"auto"><br></div><div dir= =3D"auto">I was about to make similar comments.=C2=A0 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>Consi= der <a href=3D"https://reviews.freebsd.org/D37107">https://reviews.freebsd.= org/D37107</a> and=C2=A0<a href=3D"https://reviews.freebsd.org/D37108">http= s://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 DEFAUL= TS and 37108 adjusts <a href=3D"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> --00000000000063fae405ebca68a0--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdforiFcPdMVatmuCPSsDpAdxaaj_ksj-K5D3CyO%2B8N4xTg>