From nobody Mon Feb 27 14:42:20 2023 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4PQNWR3rtgz3vByg for ; Mon, 27 Feb 2023 14:42:27 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-ed1-x530.google.com (mail-ed1-x530.google.com [IPv6:2a00:1450:4864:20::530]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4PQNWR25jvz3JmY for ; Mon, 27 Feb 2023 14:42:27 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-ed1-x530.google.com with SMTP id cy6so26800520edb.5 for ; Mon, 27 Feb 2023 06:42:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20210112.gappssmtp.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=7HI4jaWh5b/aoZjhnZn4zU/oTRjO1gfasaiTRhnQdPc=; b=uDOCjpv5c70eFetMiD+FXGn76eHY+SJgj5WjUClIv1+0swUOuVsQaPwT0fUKYHihSi rGwe0XHSuAuefcUE5XyQqyTjj+aCEOSnLMJIMNmgk5F/4YqyfeJPOYCljdygmlDZOZ9+ wF0L7HTzAvpQTsh7+0boJubGdx/lG0nMFmIc8wF4M5DynU8oOU/+e+h1f/VN4KX3ObFa uXnpFtUBrErirTGiclynVXiwuXk3iCiwMLEV4RsYBUrHeRKpkpXIZvuc+4HKkhpjuxXn HmuVguSSjDwIaeXTc90gDvjBBsGAREmFE6eyvkNG37wfJD/6qbhmfysrKG6mzZQqV8Rx sCYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=7HI4jaWh5b/aoZjhnZn4zU/oTRjO1gfasaiTRhnQdPc=; b=rC6+Rm4zg83AKSewwByetu+ylEEhbZQRwjPlfGQ4SZKfvQpLbWJwi9cETYZI7t5vqt 1I/EYiRPbcmTIGiLG255t3H0SM+82QrKIntaDvgGmnVCIRpQhjpnfwLjApvto0+khBeu +EnVeCgpTVFHhlPuetvWL9CWT37KAWJg5+5GWc7wfJ9KnY9qAIbOm92pkrbXZ+vHjK5c rgyrOmz6qli3Ja1PffSYmAllJnFus9okejzrLUXkMOauxnHYbmb2p9wWFGLX9wtaQFFB +cILegFIic1f80+g24zU6WEyT4d0ZZyK7AMNICTATJ0hGTYgc7Q0f0qQjDrcZLjnS1B0 aEZg== X-Gm-Message-State: AO0yUKXtSRnnreZPGseNz5l/A5I3+jdUHZz2WkOw2udK5e1Rvn1D+32B qPkubLLeJGqXFkEvuxYQpB6hQILVzhbmxcyM0QZOD7871X8mNA== X-Google-Smtp-Source: AK7set9lY/RoZyW9MFSOL2bn616ZyzBu8y3cNms5ndMnVM8fLtY9P8XMiqXb3s/s2oYAOZNUzWnCtn8d3TN5WISQUkk= X-Received: by 2002:a05:6402:2811:b0:4af:70a5:5674 with SMTP id h17-20020a056402281100b004af70a55674mr6607383ede.0.1677508945931; Mon, 27 Feb 2023 06:42:25 -0800 (PST) List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 References: <66452C86-02AA-4604-B65C-5E32EEBAFCC3.ref@yahoo.com> <66452C86-02AA-4604-B65C-5E32EEBAFCC3@yahoo.com> <5BEC0410-DBAA-4CBF-8BDB-5C317AD2A094@yahoo.com> In-Reply-To: <5BEC0410-DBAA-4CBF-8BDB-5C317AD2A094@yahoo.com> From: Warner Losh Date: Mon, 27 Feb 2023 07:42:20 -0700 Message-ID: Subject: Re: git: c51978f4b2f0 - main - kbd: add KBD_DELAY1 and KBD_DELAY2 To: Mark Millard Cc: imp@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: multipart/alternative; boundary="000000000000d47e1905f5af7c88" X-Rspamd-Queue-Id: 4PQNWR25jvz3JmY X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N --000000000000d47e1905f5af7c88 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sun, Feb 26, 2023 at 9:12=E2=80=AFPM Mark Millard wr= ote: > On Feb 26, 2023, at 19:33, Warner Losh wrote: > > > On Sat, Feb 25, 2023 at 10:03=E2=80=AFAM Mark Millard wrote: > > Warner Losh wrote on > > Date: Sat, 25 Feb 2023 06:26:00 UTC : > > > > > The branch main has been updated by imp: > > > > > > URL: > https://cgit.FreeBSD.org/src/commit/?id=3Dc51978f4b2f080c80ddc891c24b151d= 35acb8ba4 > > > > > > commit c51978f4b2f080c80ddc891c24b151d35acb8ba4 > > > Author: Michael Paepcke > > > AuthorDate: 2023-02-18 09:11:37 +0000 > > > Commit: Warner Losh > > > CommitDate: 2023-02-25 06:19:05 +0000 > > > > > > kbd: add KBD_DELAY1 and KBD_DELAY2 > > > > > > Allow to configure KBD_DELAY* via KERNCONF for user-land less > embedded > > > and security appliances > > > > > > Reviewed by: imp (folded) > > > Pull Request: https://github.com/freebsd/freebsd-src/pull/649 > > > --- > > > sys/conf/options | 3 ++- > > > sys/dev/kbd/kbd.c | 4 ++-- > > > sys/dev/kbd/kbdreg.h | 8 ++++++-- > > > 3 files changed, 10 insertions(+), 5 deletions(-) > > > > > > diff --git a/sys/conf/options b/sys/conf/options > > > index 7855a2f3f20e..42529a90a54e 100644 > > > --- a/sys/conf/options > > > +++ b/sys/conf/options > > > @@ -803,8 +803,9 @@ KBD_INSTALL_CDEV opt_kbd.h > > > KBD_MAXRETRY opt_kbd.h > > > KBD_MAXWAIT opt_kbd.h > > > KBD_RESETDELAY opt_kbd.h > > > +KBD_DELAY1 opt_kbd.h > > > +KBD_DELAY2 opt_kbd.h > > > KBDIO_DEBUG opt_kbd.h > > > - > > > KBDMUX_DFLT_KEYMAP opt_kbdmux.h > > > > > > # options for the Atheros driver > > > diff --git a/sys/dev/kbd/kbd.c b/sys/dev/kbd/kbd.c > > > index 205d76639e0f..ebc779de4073 100644 > > > --- a/sys/dev/kbd/kbd.c > > > +++ b/sys/dev/kbd/kbd.c > > > @@ -143,8 +143,8 @@ kbd_init_struct(keyboard_t *kbd, char *name, int > type, int unit, int config, > > > kbd->kb_accentmap =3D NULL; > > > kbd->kb_fkeytab =3D NULL; > > > kbd->kb_fkeytab_size =3D 0; > > > - kbd->kb_delay1 =3D KB_DELAY1; /* these values are advisory > only */ > > > - kbd->kb_delay2 =3D KB_DELAY2; > > > + kbd->kb_delay1 =3D KBD_DELAY1; /* these values are advisory > only */ > > > + kbd->kb_delay2 =3D KBD_DELAY2; > > > kbd->kb_count =3D 0L; > > > bzero(kbd->kb_lastact, sizeof(kbd->kb_lastact)); > > > } > > > diff --git a/sys/dev/kbd/kbdreg.h b/sys/dev/kbd/kbdreg.h > > > index 15b7e5183f35..2839e259420d 100644 > > > --- a/sys/dev/kbd/kbdreg.h > > > +++ b/sys/dev/kbd/kbdreg.h > > > @@ -151,8 +151,12 @@ struct keyboard { > > > void *kb_data; /* the driver's private data */ > > > int kb_delay1; > > > int kb_delay2; > > > -#define KB_DELAY1 500 > > > -#define KB_DELAY2 100 > > > +#ifndef KBD_DELAY1 > > > +#define KBD_DELAY1 500 > > > +#endif > > > +#ifndef KBD_DELAY2 > > > +#define KBD_DELAY2 100 > > > +#endif > > > > [Just reporting Ximalas's Discord note.] > > > > So opt_kbd.h must be included before kbdreg.h in > > order to avoid: macro redefined in opt_kbd.h ? > > > > Should something force the right order? > > > > If we include them in the wrong order, then the compiler will complain, > > and that's likely sufficient. I'll double check NOTES to make sure that > the > > values there are different than the defaults. > > The report on Discord (Kernel) was that the attempted use > resulted in: > > --- psm.o --- > In file included from /usr/src/sys/dev/atkbdc/psm.c:100: > In file included from /usr/src/sys/dev/atkbdc/atkbdcreg.h:39: > ./opt_kbd.h:1:9: error: 'KBD_DELAY1' macro redefined > [-Werror,-Wmacro-redefined] > #define KBD_DELAY1 200 > ^ > /usr/src/sys/dev/kbd/kbdreg.h:155:9: note: previous definition is here > #define KBD_DELAY1 500 > ^ > In file included from /usr/src/sys/dev/atkbdc/psm.c:100: > In file included from /usr/src/sys/dev/atkbdc/atkbdcreg.h:39: > ./opt_kbd.h:3:9: error: 'KBD_DELAY2' macro redefined > [-Werror,-Wmacro-redefined] > #define KBD_DELAY2 15 > ^ > /usr/src/sys/dev/kbd/kbdreg.h:158:9: note: previous definition is here > #define KBD_DELAY2 100 > ^ > 2 errors generated. > *** [psm.o] Error code 1 > > > The reported workaround was: > > diff --git a/sys/dev/atkbdc/psm.c b/sys/dev/atkbdc/psm.c > index a308cc81cd3a..86560f7bc2dd 100644 > --- a/sys/dev/atkbdc/psm.c > +++ b/sys/dev/atkbdc/psm.c > @@ -62,6 +62,7 @@ > __FBSDID("$FreeBSD$"); > > #include "opt_isa.h" > +#include "opt_kbd.h" > #include "opt_psm.h" > #include "opt_evdev.h" > That works around it. We include opt_kbd.h from atkbdcreg.h, which is a bit odd (but it's a kernel-only, so I moved it). And kbdreg.h is also kernel only, so I added it there instead. That's likely safer and I should likely have listened a bit better when you asked about it before... And not committed to LINT from machine A when I test compiled it on machine B by mistake . Warner > > Warner > > > unsigned long kb_count; /* # of processed key strokes = */ > > > u_char kb_lastact[NUM_KEYS/2]; > > > struct cdev *kb_dev; > > > > =3D=3D=3D > Mark Millard > marklmi at yahoo.com > > --000000000000d47e1905f5af7c88 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Sun, Feb 26, 2023 at 9:12=E2=80=AF= PM Mark Millard <marklmi@yahoo.com<= /a>> wrote:
O= n Feb 26, 2023, at 19:33, Warner Losh <imp@bsdimp.com> wrote:

> On Sat, Feb 25, 2023 at 10:03=E2=80=AFAM Mark Millard <marklmi@yahoo.com> wrote:=
> Warner Losh <imp_at_FreeBSD.org> wrote on
> Date: Sat, 25 Feb 2023 06:26:00 UTC :
>
> > The branch main has been updated by imp:
> >
> > URL: htt= ps://cgit.FreeBSD.org/src/commit/?id=3Dc51978f4b2f080c80ddc891c24b151d35acb= 8ba4
> >
> > commit c51978f4b2f080c80ddc891c24b151d35acb8ba4
> > Author:=C2=A0 =C2=A0 =C2=A0Michael Paepcke <git@paepcke.de>
> > AuthorDate: 2023-02-18 09:11:37 +0000
> > Commit:=C2=A0 =C2=A0 =C2=A0Warner Losh <imp@FreeBSD.org> > > CommitDate: 2023-02-25 06:19:05 +0000
> >
> >=C2=A0 =C2=A0 =C2=A0kbd: add KBD_DELAY1 and KBD_DELAY2
> >=C2=A0 =C2=A0 =C2=A0
> >=C2=A0 =C2=A0 =C2=A0Allow to configure KBD_DELAY* via KERNCONF for= user-land less embedded
> >=C2=A0 =C2=A0 =C2=A0and security appliances
> >=C2=A0 =C2=A0 =C2=A0
> >=C2=A0 =C2=A0 =C2=A0Reviewed by: imp (folded)
> >=C2=A0 =C2=A0 =C2=A0Pull Request: https://gi= thub.com/freebsd/freebsd-src/pull/649
> > ---
> >=C2=A0 sys/conf/options=C2=A0 =C2=A0 =C2=A0| 3 ++-
> >=C2=A0 sys/dev/kbd/kbd.c=C2=A0 =C2=A0 | 4 ++--
> >=C2=A0 sys/dev/kbd/kbdreg.h | 8 ++++++--
> >=C2=A0 3 files changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/sys/conf/options b/sys/conf/options
> > index 7855a2f3f20e..42529a90a54e 100644
> > --- a/sys/conf/options
> > +++ b/sys/conf/options
> > @@ -803,8 +803,9 @@ KBD_INSTALL_CDEV=C2=A0 opt_kbd.h
> >=C2=A0 KBD_MAXRETRY=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0opt_kbd.h
> >=C2=A0 KBD_MAXWAIT=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 opt_kbd.h
> >=C2=A0 KBD_RESETDELAY=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0opt_kbd.h
> > +KBD_DELAY1=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0opt_kbd.h
> > +KBD_DELAY2=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0opt_kbd.h
> >=C2=A0 KBDIO_DEBUG=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 opt_kbd.h
> > -
> >=C2=A0 KBDMUX_DFLT_KEYMAP=C2=A0 =C2=A0opt_kbdmux.h
> >=C2=A0
> >=C2=A0 # options for the Atheros driver
> > diff --git a/sys/dev/kbd/kbd.c b/sys/dev/kbd/kbd.c
> > index 205d76639e0f..ebc779de4073 100644
> > --- a/sys/dev/kbd/kbd.c
> > +++ b/sys/dev/kbd/kbd.c
> > @@ -143,8 +143,8 @@ kbd_init_struct(keyboard_t *kbd, char *name, = int type, int unit, int config,
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0kbd->kb_accentmap =3D NULL;
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0kbd->kb_fkeytab =3D NULL;
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0kbd->kb_fkeytab_size =3D 0;
> > -=C2=A0 =C2=A0 =C2=A0kbd->kb_delay1 =3D KB_DELAY1;=C2=A0 =C2= =A0 =C2=A0/* these values are advisory only */
> > -=C2=A0 =C2=A0 =C2=A0kbd->kb_delay2 =3D KB_DELAY2;
> > +=C2=A0 =C2=A0 =C2=A0kbd->kb_delay1 =3D KBD_DELAY1;=C2=A0 =C2= =A0 /* these values are advisory only */
> > +=C2=A0 =C2=A0 =C2=A0kbd->kb_delay2 =3D KBD_DELAY2;
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0kbd->kb_count =3D 0L;
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0bzero(kbd->kb_lastact, sizeof(kbd-&g= t;kb_lastact));
> >=C2=A0 }
> > diff --git a/sys/dev/kbd/kbdreg.h b/sys/dev/kbd/kbdreg.h
> > index 15b7e5183f35..2839e259420d 100644
> > --- a/sys/dev/kbd/kbdreg.h
> > +++ b/sys/dev/kbd/kbdreg.h
> > @@ -151,8 +151,12 @@ struct keyboard {
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0void=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 *kb_data;=C2=A0 =C2=A0 =C2=A0 =C2=A0/* the driver's private data= */
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0int=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0kb_delay1;
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0int=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0kb_delay2;
> > -#define KB_DELAY1=C2=A0 =C2=A0 500
> > -#define KB_DELAY2=C2=A0 =C2=A0 100
> > +#ifndef KBD_DELAY1
> > +#define KBD_DELAY1=C2=A0 =C2=A0500
> > +#endif
> > +#ifndef KBD_DELAY2
> > +#define KBD_DELAY2=C2=A0 =C2=A0100
> > +#endif
>
> [Just reporting Ximalas's Discord note.]
>
> So opt_kbd.h must be included before kbdreg.h in
> order to avoid: macro redefined in opt_kbd.h ?
>
> Should something force the right order?
>
> If we include them in the wrong order, then the compiler will complain= ,
> and that's likely sufficient. I'll double check NOTES to make = sure that the
> values there are different than the defaults.

The report on Discord (Kernel) was that the attempted use
resulted in:

--- psm.o ---
In file included from /usr/src/sys/dev/atkbdc/psm.c:100:
In file included from /usr/src/sys/dev/atkbdc/atkbdcreg.h:39:
./opt_kbd.h:1:9: error: 'KBD_DELAY1' macro redefined [-Werror,-Wmac= ro-redefined]
#define KBD_DELAY1 200
=C2=A0 =C2=A0 =C2=A0 =C2=A0 ^
/usr/src/sys/dev/kbd/kbdreg.h:155:9: note: previous definition is here
#define KBD_DELAY1=C2=A0 =C2=A0 =C2=A0 500
=C2=A0 =C2=A0 =C2=A0 =C2=A0 ^
In file included from /usr/src/sys/dev/atkbdc/psm.c:100:
In file included from /usr/src/sys/dev/atkbdc/atkbdcreg.h:39:
./opt_kbd.h:3:9: error: 'KBD_DELAY2' macro redefined [-Werror,-Wmac= ro-redefined]
#define KBD_DELAY2 15
=C2=A0 =C2=A0 =C2=A0 =C2=A0 ^
/usr/src/sys/dev/kbd/kbdreg.h:158:9: note: previous definition is here
#define KBD_DELAY2=C2=A0 =C2=A0 =C2=A0 100
=C2=A0 =C2=A0 =C2=A0 =C2=A0 ^
2 errors generated.
*** [psm.o] Error code 1


The reported workaround was:

diff --git a/sys/dev/atkbdc/psm.c b/sys/dev/atkbdc/psm.c
index a308cc81cd3a..86560f7bc2dd 100644
--- a/sys/dev/atkbdc/psm.c
+++ b/sys/dev/atkbdc/psm.c
@@ -62,6 +62,7 @@
__FBSDID("$FreeBSD$");

#include "opt_isa.h"
+#include "opt_kbd.h"
#include "opt_psm.h"
#include "opt_evdev.h"

That w= orks around it. We include opt_kbd.h from atkbdcreg.h, which is a bit odd (= but it's a kernel-only,
so I moved it). And kbdreg.h is also = kernel only, so I added it there instead. That's likely safer and
=
I should likely have listened a bit better when you asked about it bef= ore...=C2=A0 And not committed to LINT
from machine A when I test= compiled it on machine B by mistake <doh>.

<= div>Warner
=C2=A0
> Warner
>=C2=A0 >=C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned long=C2=A0 =C2=A0kb_coun= t;=C2=A0 =C2=A0 =C2=A0 =C2=A0/* # of processed key strokes */
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0u_char=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 kb_lastact[NUM_KEYS/2];
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0struct cdev *kb_dev;



=3D=3D=3D
Mark Millard
marklmi at yahoo.com

--000000000000d47e1905f5af7c88--