Re: git: c51978f4b2f0 - main - kbd: add KBD_DELAY1 and KBD_DELAY2

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"

> 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