Date: Fri, 3 Sep 2010 13:50:36 -0400 From: John Baldwin <jhb@freebsd.org> To: "M. Warner Losh" <imp@bsdimp.com> Cc: avg@freebsd.org, freebsd-arch@freebsd.org Subject: Re: newbus: type (max value) for device order Message-ID: <201009031350.36931.jhb@freebsd.org> In-Reply-To: <20100903.105927.70320533242210716.imp@bsdimp.com> References: <4C80A728.6090002@freebsd.org> <201009031001.58036.jhb@freebsd.org> <20100903.105927.70320533242210716.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Friday, September 03, 2010 12:59:27 pm M. Warner Losh wrote: > In message: <201009031001.58036.jhb@freebsd.org> > John Baldwin <jhb@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201009031350.36931.jhb>