Re: SC520 and reboot

In message <20031214082913.A2027_at_freebsd3.cimlogic.com.au>, John Birrell writes
:

Sorry about the delay in answering.

>In sys/i386/i386/machdep.c:cpu_reset_real() I've added:
>
>#ifdef  CPU_ELAN
>    if (elan_mmcr != NULL)
>        /* SYS_RST */
>        elan_mmcr->RESCFG = 1;
>#endif

This looks like it should just be committed.

>For convenience, I use the following structure to access the Elan SC520
>memory mapped configuration region instead of the opaque uint16_t array
>that phk uses. This changes lines like:
>
>elan_mmcr[0xc82 / 2] = 0xc001;
>
>to
>
>elan_mmcr->GPTMR2CTL = 0xc001;
>
>which is more intuitive to me when reading the AMD register set manual.

I'm not against it, but I have had so many bad experiences with
trying to make compilers understand a particular layout of bytes
which it is not allowed to change, that I will only agree to a
change to a struct on four conditions:

1.  All bytes in the data structure must be accounted for.  In
    other words, no reliance on the compiler adding padding inside
    the structure.

2.  No bitfields.  They just don't work the way people expect most
    of the time.

3.  A CTASSERT which protects against compilation if the compiler
    found some way to screw it up anyway.

4.  No cute tricks.  By this I mean don't do things like this:
	>struct elan_mmcr {
	>	[...]
	>	u_int16_t	DRCBENDADR;
	>	u_int8_t	pad_0x01a[0x6];

    This should have been either
	 	u_int16_t	DRCBENDADR0;
	 	u_int16_t	DRCBENDADR1;
	 	u_int16_t	DRCBENDADR2;
	 	u_int16_t	DRCBENDADR3;
    or
	 	u_int16_t	DRCBENDADR[4];

    Pick your poison...


-- 
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
phk_at_FreeBSD.ORG         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe    
Never attribute to malice what can adequately be explained by incompetence.