Date: Mon, 27 Feb 2023 07:42:20 -0700 From: Warner Losh <imp@bsdimp.com> To: Mark Millard <marklmi@yahoo.com> Cc: imp@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: c51978f4b2f0 - main - kbd: add KBD_DELAY1 and KBD_DELAY2 Message-ID: <CANCZdfp0UnZ_nZN5%2B_1qqFewxWGK%2BFCa0GfTJ91egs9BJi8yHw@mail.gmail.com> In-Reply-To: <5BEC0410-DBAA-4CBF-8BDB-5C317AD2A094@yahoo.com> References: <66452C86-02AA-4604-B65C-5E32EEBAFCC3.ref@yahoo.com> <66452C86-02AA-4604-B65C-5E32EEBAFCC3@yahoo.com> <CANCZdfqKfAT84iKXctXbMsg6W75-95G=em0qR-0wLSWAHyMPgA@mail.gmail.com> <5BEC0410-DBAA-4CBF-8BDB-5C317AD2A094@yahoo.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] On Sun, Feb 26, 2023 at 9:12 PM Mark Millard <marklmi@yahoo.com> wrote: > On Feb 26, 2023, at 19:33, Warner Losh <imp@bsdimp.com> wrote: > > > On Sat, Feb 25, 2023 at 10:03 AM 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: > https://cgit.FreeBSD.org/src/commit/?id=c51978f4b2f080c80ddc891c24b151d35acb8ba4 > > > > > > commit c51978f4b2f080c80ddc891c24b151d35acb8ba4 > > > Author: Michael Paepcke <git@paepcke.de> > > > AuthorDate: 2023-02-18 09:11:37 +0000 > > > Commit: Warner Losh <imp@FreeBSD.org> > > > 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 = NULL; > > > kbd->kb_fkeytab = NULL; > > > kbd->kb_fkeytab_size = 0; > > > - kbd->kb_delay1 = KB_DELAY1; /* these values are advisory > only */ > > > - kbd->kb_delay2 = KB_DELAY2; > > > + kbd->kb_delay1 = KBD_DELAY1; /* these values are advisory > only */ > > > + kbd->kb_delay2 = KBD_DELAY2; > > > kbd->kb_count = 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 <doh>. Warner > > Warner > > > unsigned long kb_count; /* # of processed key strokes */ > > > u_char kb_lastact[NUM_KEYS/2]; > > > struct cdev *kb_dev; > > > > === > Mark Millard > marklmi at yahoo.com > > [-- Attachment #2 --] <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Feb 26, 2023 at 9:12 PM Mark Millard <<a href="mailto:marklmi@yahoo.com">marklmi@yahoo.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">On Feb 26, 2023, at 19:33, Warner Losh <<a href="mailto:imp@bsdimp.com" target="_blank">imp@bsdimp.com</a>> wrote:<br> <br> > On Sat, Feb 25, 2023 at 10:03 AM Mark Millard <<a href="mailto:marklmi@yahoo.com" target="_blank">marklmi@yahoo.com</a>> wrote:<br> > Warner Losh <imp_at_FreeBSD.org> wrote on<br> > Date: Sat, 25 Feb 2023 06:26:00 UTC :<br> > <br> > > The branch main has been updated by imp:<br> > > <br> > > URL: <a href="https://cgit.FreeBSD.org/src/commit/?id=c51978f4b2f080c80ddc891c24b151d35acb8ba4" rel="noreferrer" target="_blank">https://cgit.FreeBSD.org/src/commit/?id=c51978f4b2f080c80ddc891c24b151d35acb8ba4</a><br>; > > <br> > > commit c51978f4b2f080c80ddc891c24b151d35acb8ba4<br> > > Author: Michael Paepcke <<a href="mailto:git@paepcke.de" target="_blank">git@paepcke.de</a>><br> > > AuthorDate: 2023-02-18 09:11:37 +0000<br> > > Commit: Warner Losh <imp@FreeBSD.org><br> > > CommitDate: 2023-02-25 06:19:05 +0000<br> > > <br> > > kbd: add KBD_DELAY1 and KBD_DELAY2<br> > > <br> > > Allow to configure KBD_DELAY* via KERNCONF for user-land less embedded<br> > > and security appliances<br> > > <br> > > Reviewed by: imp (folded)<br> > > Pull Request: <a href="https://github.com/freebsd/freebsd-src/pull/649" rel="noreferrer" target="_blank">https://github.com/freebsd/freebsd-src/pull/649</a><br>; > > ---<br> > > sys/conf/options | 3 ++-<br> > > sys/dev/kbd/kbd.c | 4 ++--<br> > > sys/dev/kbd/kbdreg.h | 8 ++++++--<br> > > 3 files changed, 10 insertions(+), 5 deletions(-)<br> > > <br> > > diff --git a/sys/conf/options b/sys/conf/options<br> > > index 7855a2f3f20e..42529a90a54e 100644<br> > > --- a/sys/conf/options<br> > > +++ b/sys/conf/options<br> > > @@ -803,8 +803,9 @@ KBD_INSTALL_CDEV opt_kbd.h<br> > > KBD_MAXRETRY opt_kbd.h<br> > > KBD_MAXWAIT opt_kbd.h<br> > > KBD_RESETDELAY opt_kbd.h<br> > > +KBD_DELAY1 opt_kbd.h<br> > > +KBD_DELAY2 opt_kbd.h<br> > > KBDIO_DEBUG opt_kbd.h<br> > > -<br> > > KBDMUX_DFLT_KEYMAP opt_kbdmux.h<br> > > <br> > > # options for the Atheros driver<br> > > diff --git a/sys/dev/kbd/kbd.c b/sys/dev/kbd/kbd.c<br> > > index 205d76639e0f..ebc779de4073 100644<br> > > --- a/sys/dev/kbd/kbd.c<br> > > +++ b/sys/dev/kbd/kbd.c<br> > > @@ -143,8 +143,8 @@ kbd_init_struct(keyboard_t *kbd, char *name, int type, int unit, int config,<br> > > kbd->kb_accentmap = NULL;<br> > > kbd->kb_fkeytab = NULL;<br> > > kbd->kb_fkeytab_size = 0;<br> > > - kbd->kb_delay1 = KB_DELAY1; /* these values are advisory only */<br> > > - kbd->kb_delay2 = KB_DELAY2;<br> > > + kbd->kb_delay1 = KBD_DELAY1; /* these values are advisory only */<br> > > + kbd->kb_delay2 = KBD_DELAY2;<br> > > kbd->kb_count = 0L;<br> > > bzero(kbd->kb_lastact, sizeof(kbd->kb_lastact));<br> > > }<br> > > diff --git a/sys/dev/kbd/kbdreg.h b/sys/dev/kbd/kbdreg.h<br> > > index 15b7e5183f35..2839e259420d 100644<br> > > --- a/sys/dev/kbd/kbdreg.h<br> > > +++ b/sys/dev/kbd/kbdreg.h<br> > > @@ -151,8 +151,12 @@ struct keyboard {<br> > > void *kb_data; /* the driver's private data */<br> > > int kb_delay1;<br> > > int kb_delay2;<br> > > -#define KB_DELAY1 500<br> > > -#define KB_DELAY2 100<br> > > +#ifndef KBD_DELAY1<br> > > +#define KBD_DELAY1 500<br> > > +#endif<br> > > +#ifndef KBD_DELAY2<br> > > +#define KBD_DELAY2 100<br> > > +#endif<br> > <br> > [Just reporting Ximalas's Discord note.]<br> > <br> > So opt_kbd.h must be included before kbdreg.h in<br> > order to avoid: macro redefined in opt_kbd.h ?<br> > <br> > Should something force the right order?<br> > <br> > If we include them in the wrong order, then the compiler will complain,<br> > and that's likely sufficient. I'll double check NOTES to make sure that the<br> > values there are different than the defaults.<br> <br> The report on Discord (Kernel) was that the attempted use<br> resulted in:<br> <br> --- psm.o ---<br> In file included from /usr/src/sys/dev/atkbdc/psm.c:100:<br> In file included from /usr/src/sys/dev/atkbdc/atkbdcreg.h:39:<br> ./opt_kbd.h:1:9: error: 'KBD_DELAY1' macro redefined [-Werror,-Wmacro-redefined]<br> #define KBD_DELAY1 200<br> ^<br> /usr/src/sys/dev/kbd/kbdreg.h:155:9: note: previous definition is here<br> #define KBD_DELAY1 500<br> ^<br> In file included from /usr/src/sys/dev/atkbdc/psm.c:100:<br> In file included from /usr/src/sys/dev/atkbdc/atkbdcreg.h:39:<br> ./opt_kbd.h:3:9: error: 'KBD_DELAY2' macro redefined [-Werror,-Wmacro-redefined]<br> #define KBD_DELAY2 15<br> ^<br> /usr/src/sys/dev/kbd/kbdreg.h:158:9: note: previous definition is here<br> #define KBD_DELAY2 100<br> ^<br> 2 errors generated.<br> *** [psm.o] Error code 1<br> <br> <br> The reported workaround was:<br> <br> diff --git a/sys/dev/atkbdc/psm.c b/sys/dev/atkbdc/psm.c<br> index a308cc81cd3a..86560f7bc2dd 100644<br> --- a/sys/dev/atkbdc/psm.c<br> +++ b/sys/dev/atkbdc/psm.c<br> @@ -62,6 +62,7 @@<br> __FBSDID("$FreeBSD$");<br> <br> #include "opt_isa.h"<br> +#include "opt_kbd.h"<br> #include "opt_psm.h"<br> #include "opt_evdev.h"<br></blockquote><div><br></div><div>That works around it. We include opt_kbd.h from atkbdcreg.h, which is a bit odd (but it's a kernel-only,</div><div>so I moved it). And kbdreg.h is also kernel only, so I added it there instead. That's likely safer and</div><div>I should likely have listened a bit better when you asked about it before... And not committed to LINT</div><div>from machine A when I test compiled it on machine B by mistake <doh>.<br></div><div><br></div><div>Warner<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > Warner<br> > > unsigned long kb_count; /* # of processed key strokes */<br> > > u_char kb_lastact[NUM_KEYS/2];<br> > > struct cdev *kb_dev;<br> <br> <br> <br> ===<br> Mark Millard<br> marklmi at <a href="http://yahoo.com" rel="noreferrer" target="_blank">yahoo.com</a><br> <br> </blockquote></div></div>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfp0UnZ_nZN5%2B_1qqFewxWGK%2BFCa0GfTJ91egs9BJi8yHw>