newbus: type (max value) for device order

[John Baldwin]

On Friday, September 03, 2010 12:59:27 pm M. Warner Losh wrote:
> In message: <201009031001.58036.jhb at freebsd.org>
>             John Baldwin <jhb at FreeBSD.org> writes:
> : On Friday, September 03, 2010 3:43:36 am Andriy Gapon wrote:
> : > 
> : > device_add_child_ordered() takes order as a parameter of int type.
> : > struct device stores it as u_char.
> : > 
> : > This can be confusing, can't it?
> : > In fact, up to r203776 we used to use order value of 100000 in acpi.c (which
> : > effectively was 160 according to my calculations).
> : > 
> : > Not sure what I want to suggest, perhaps defining DEVICE_MAX_ORDER or something.
> : > Or changing the type in struct device to int.
> : 
> : Just fix device_t to store an int I think.  Also, it should probably be a
> : u_int as negative values don't really make sense.
> 
> One caution:
> 	u_short		flags;		/**< internal device flags  */
> #define	DF_ENABLED	1		/* device should be probed/attached */
> #define	DF_FIXEDCLASS	2		/* devclass specified at create time */
> #define	DF_WILDCARD	4		/* unit was originally wildcard */
> #define	DF_DESCMALLOCED	8		/* description was malloced */
> #define	DF_QUIET	16		/* don't print verbose attach message */
> #define	DF_DONENOMATCH	32		/* don't execute DEVICE_NOMATCH again */
> #define	DF_EXTERNALSOFTC 64		/* softc not allocated by us */
> #define	DF_REBID	128		/* Can rebid after attach */
> #define DF_REMAPPED	256		/* all remapping completed */
> 	u_char	order;			/**< order from device_add_child_ordered() */
> 	u_char	pad;
> 
> I'd be inclined to change this to:
> 
> 	u_int		flags;		/**< internal device flags  */
> #define	DF_ENABLED	1		/* device should be probed/attached */
> #define	DF_FIXEDCLASS	2		/* devclass specified at create time */
> #define	DF_WILDCARD	4		/* unit was originally wildcard */
> #define	DF_DESCMALLOCED	8		/* description was malloced */
> #define	DF_QUIET	16		/* don't print verbose attach message */
> #define	DF_DONENOMATCH	32		/* don't execute DEVICE_NOMATCH again */
> #define	DF_EXTERNALSOFTC 64		/* softc not allocated by us */
> #define	DF_REBID	128		/* Can rebid after attach */
> #define DF_REMAPPED	256		/* all remapping completed */
> 	u_int	order;			/**< order from device_add_child_ordered() */
> 
> so that the padding and such remains consistent.

Agreed.

> I think this is even an MFCable change, since device_t's size isn't
> exposed outside of subr_bus.c...

Yes.

-- 
John Baldwin