In article <44143C93.2090007@netbsd.org>
shige@NetBSD.org wrote:

> I think, I'm going to commit its fix.
> I'm sorry to trouble tou, but please confirm again now whether is it ok?

IMHO it's still better to post the latest patch to be committed,
but I'd like some cosmetics in your previous patch:
(sorry maybe I'm a paranoid again ;-)

> RCS file: /cvsroot/src/sys/arch/evbppc/conf/files.obs405,v
> retrieving revision 1.12
> diff -u -u -r1.12 files.obs405
> --- sys/arch/evbppc/conf/files.obs405	10 Sep 2005 04:34:39 -0000	1.12
> +++ sys/arch/evbppc/conf/files.obs405	1 Oct 2005 16:12:52 -0000
> @@ -14,9 +14,9 @@
>  file	arch/evbppc/obs405/obs405_autoconf.c
>  file	arch/evbppc/obs405/obs405_machdep.c
>  
> -device obsled
> -attach  obsled at gpio
> -file	arch/evbppc/obs405/dev/obsled.c		obsled
> +#device obsled
> +#attach  obsled at ibmgpio
> +#file	arch/evbppc/obs405/dev/obsled.c		obsled

Why is obsled commented out?
Will it be rewritten to use MI gpio later?

> RCS file: /cvsroot/src/sys/arch/evbppc/obs405/obs266_machdep.c,v
> retrieving revision 1.1
> diff -u -u -r1.1 obs266_machdep.c
> --- sys/arch/evbppc/obs405/obs266_machdep.c	18 Mar 2005 14:12:34 -0000	1.1
> +++ sys/arch/evbppc/obs405/obs266_machdep.c	1 Oct 2005 16:12:52 -0000
> @@ -214,7 +214,7 @@
>  	static char str[256];
>  	char *ap = str, *ap1 = ap;
>  
> -	obs266_led_set(OBS266_LED_ON);
> +	/* obs266_led_set(OBS266_LED_ON); */

I'm not sure how the leds should be handled by MI gpio,
but please use #if 0/#endif pair to disable code.

> RCS file: /cvsroot/src/sys/arch/powerpc/conf/files.ibm4xx,v
> retrieving revision 1.7
> diff -u -u -r1.7 files.ibm4xx
> --- sys/arch/powerpc/conf/files.ibm4xx	23 Jan 2005 19:24:31 -0000	1.7
> +++ sys/arch/powerpc/conf/files.ibm4xx	1 Oct 2005 16:12:52 -0000
> @@ -30,9 +30,9 @@
>  file	arch/powerpc/ibm4xx/dev/com_opb.c	com_opb
>  
>  # On-chip GPIO controller
> -device	gpio { addr, [size = -1] }
> -attach	gpio at opb
> -file	arch/powerpc/ibm4xx/dev/gpio_opb.c	gpio
> +device	gpio_opb: gpiobus
> +attach	gpio_opb at opb
> +file	arch/powerpc/ibm4xx/dev/gpio_opb.c	gpio_opb

Underscore in a device name (i.e. "giop_opb0 at opb?" in config files)
looks weird a bit to me, and I'm afraid it causes some confusion.
Isn't "opbgpio" like epgpio for arm/ep93xx better?

> RCS file: /cvsroot/src/sys/arch/powerpc/ibm4xx/dev/opb.c,v
> retrieving revision 1.20
> diff -u -u -r1.20 opb.c
> --- sys/arch/powerpc/ibm4xx/dev/opb.c	26 Aug 2005 13:19:37 -0000	1.20
> +++ sys/arch/powerpc/ibm4xx/dev/opb.c	1 Oct 2005 16:12:52 -0000
 :
>  static struct powerpc_bus_space opb_tag = {
> -	_BUS_SPACE_LITTLE_ENDIAN|_BUS_SPACE_MEM_TYPE,
> +	_BUS_SPACE_BIG_ENDIAN|_BUS_SPACE_MEM_TYPE,
>  	0x0, IBM405GP_UART0_BASE, 0x1000

if_emac.c uses bus_space_{read,write}_stream_4() to access
registers, but I wonder why these stream ops are needed...
(powerpc/bus_space.c doesn't handle stream ops correctly anyway)

Lastly, random KNF stuff:
- uint32_t rather than u_int32_t
- no parenthesis around return values
- "Insert an empty line if the function has no local variables."
etc.

---
Izumi Tsutsui