--Hf61M2y+wYpnELGG
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sun, May 21, 2006 at 10:57:17PM +0200, Arnaud Lacombe wrote:
> On Sun, May 21, 2006 at 06:19:09PM +0200, M=E1ty=E1s J=E1nos wrote:
> > Hi,
> >=20
> > I attached a newer version. This little patch provides the missing
> > functionality and hope it's already committable.  I leave the sysctl
> > work to somebody else who is more familiar with NetBSD kernel
> > programming. For me it's good enough to set it at compilation time.
>=20
> I'm not really more familiar with the NetBSD kernel than you are, but I
> think the attached patch is enough to add the sysctl to set number
> of beep before halt (tested on i386, not on xen).

It is wrong in a few aspects, though.

> The only one problem I see with this patch is that with infinite beep
> (beep_onhalt_count =3D=3D -1), we are no longer able to reboot the machine
> by pressing a key (and thus, the message printed just before is wrong).

Yes, this is a problem.  However, it might be acceptable;  it is an
optional feature after all.

> ps: was there any reason to define function as 'static void' and not
> just 'void' as it is done by other function in machdep.c ?

No.  I don't think there is a compelling reason to extract the code
from cpu_reboot() if it is to leave it in machdep.c, though.  Now,
making the whole thing MI, that would be slightly more interesting.

There are a handful of archs that define a sysbeep(), all with the same
prototype and mostly the same semantics (mostly, not exactly as far as
I can tell).

I think sysbeep(4) [i.e., the device] could be made MI through some
config(5) trickery, and e.g., pcppi(4) would attach a sysbeep(4) device
with the relevant attach_args to have a pointer to the actual function
that manipulates the speaker.  Then, we'd have sysbeep(9) [i.e., a
kernel API] that would make all sysbeep(4) devices of the system produce
the beep.

I say "all", because at some point we can imagine having a sysbeep* at
audio? that would produce whatever sound we'd like.  It is also
reasonable to map a sysbeep(4) device on a LED of the system, because
the semantics are about the same.

> Index: arch/i386/include/cpu.h
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> RCS file: /cvsroot/src/sys/arch/i386/include/cpu.h,v
> retrieving revision 1.123
> diff -u -r1.123 cpu.h
> --- arch/i386/include/cpu.h	16 Feb 2006 20:17:13 -0000	1.123
> +++ arch/i386/include/cpu.h	21 May 2006 20:40:03 -0000
[...]

Forget about that.  We don't hard-code sysctl OID numbers anymore.

> Index: arch/i386/i386/machdep.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> RCS file: /cvsroot/src/sys/arch/i386/i386/machdep.c,v
> retrieving revision 1.572
> diff -u -r1.572 machdep.c
> --- arch/i386/i386/machdep.c	14 Apr 2006 09:50:16 -0000	1.572
> +++ arch/i386/i386/machdep.c	21 May 2006 20:40:04 -0000
> @@ -190,6 +190,7 @@
>  #include <machine/mpbiosvar.h>	/* XXX */
>  #endif				/* XXX */
> =20
> +#ifdef BEEP_ONHALT
>  #ifndef BEEP_ONHALT_COUNT
>  #define BEEP_ONHALT_COUNT 3
>  #endif
> @@ -200,6 +201,13 @@
>  #define BEEP_ONHALT_PERIOD 250
>  #endif
> =20
> +int beep_onhalt_count =3D BEEP_ONHALT_COUNT;

If you define one of the settings as a variable (which I support),
define them all.  E.g.,

machdep.beeponhalt.enabled
machdep.beeponhalt.count
machdep.beeponhalt.pitch
machdep.beeponhalt.period

[...]
> +#ifdef BEEP_ONHALT
> +	sysctl_createv(clog, 0, NULL, NULL,
> +		       CTLFLAG_PERMANENT | CTLFLAG_READWRITE,
> +		       CTLTYPE_INT, "beep_onhalt_count", NULL,
> +		       NULL, 0, &beep_onhalt_count, 0,
> +		       CTL_MACHDEP, CPU_BEEP_ONHALT_COUNT, CTL_EOL);
> +#endif

s/CPU_BEEP_ONHALT_COUNT/CTL_CREATE/

>  #ifdef BEEP_ONHALT
> -		{
> -			int c;
> -			for (c =3D BEEP_ONHALT_COUNT; c > 0; c--) {
> -				sysbeep(BEEP_ONHALT_PITCH,
> -					BEEP_ONHALT_PERIOD * hz / 1000);
> -				delay(BEEP_ONHALT_PERIOD * 1000);
> -				sysbeep(0, BEEP_ONHALT_PERIOD * hz / 1000);
> -				delay(BEEP_ONHALT_PERIOD * 1000);
> -			}
> -		}
> +		beep_onhalt();
>  #endif
> =20
>  		cnpollc(1);	/* for proper keyboard command handling */
> @@ -929,6 +935,31 @@
>  	/*NOTREACHED*/

To solve the "beeping forever prevents rebooting", you'd just have to
mix the cnpollc() calls with the beeping (that means not having a
beep_onhalt() function, but as I said I don't see the point in having
one).

>  }
> =20
> +#ifdef BEEP_ONHALT
> +void
> +beep_onhalt()
> +{
> +	if (beep_onhalt_count =3D=3D -1) {
> +		for (;;)
> +			beep_onhalt_beep_once();
> +		/*NOTREACHED*/
> +	} else {
> +		int c;
> +		for (c =3D beep_onhalt_count; c > 0; c--)
> +			beep_onhalt_beep_once();
> +	}
> +}

for (c =3D beep_onhalt_count; beep_onhalt_count =3D=3D -1 || c > 0; c--)

> +
> +void
> +beep_onhalt_beep_once()
> +{
> +	sysbeep(BEEP_ONHALT_PITCH, BEEP_ONHALT_PERIOD * hz / 1000);
> +	delay(BEEP_ONHALT_PERIOD * 1000);
> +	sysbeep(0, BEEP_ONHALT_PERIOD * hz / 1000);
> +	delay(BEEP_ONHALT_PERIOD * 1000);
> +}
> +#endif
> +
>  /*
>   * These variables are needed by /sbin/savecore
>   */

Have a nice hacking time :)

--=20
Quentin Garnier - cube@cubidou.net - cube@NetBSD.org
"When I find the controls, I'll go where I like, I'll know where I want
to be, but maybe for now I'll stay right here on a silent sea."
KT Tunstall, Silent Sea, Eye to the Telescope, 2004.

--Hf61M2y+wYpnELGG
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (NetBSD)

iQEVAwUBRHDbXtgoQloHrPnoAQJHvAgAjTGFxLpfs5dZ8IjGeaL/Q037Hspnwbha
5sqPmXx5XmVASbTPsdNcGBYqd1EVoXb+eaqdv+PU+jm0xlqP1jrpwVqrMV7Nu83Z
uX3bBtU1IiJB3/R27etax0hwQgeCYRvOWJs4HCFdC4lyE5O9fbT+xZ0922pnpheG
VJGIYwTJgWnNXW14Xc9EBpxsTbBPPDcJw2+MARo7mtLEIpMhLwmoXSP4cz3ANOz4
s7WtoDYMdbw5SuajhFPY65Qqf6jytlN4pN6B1Mrw2pgiQQDYjecd64LxnDrf3TwQ
QrHJP00+2tpOjs5x5wtsiDaQhJwAotqb6rxOxkBXpDUMAnkueun+4w==
=rHp8
-----END PGP SIGNATURE-----

--Hf61M2y+wYpnELGG--