Date: Sat, 13 Dec 2008 13:08:16 -0700 (MST) From: Warner Losh <imp@bsdimp.com> To: hselasky@c2i.net Cc: sam@freebsd.org, perforce@freebsd.org Subject: Re: PERFORCE change 154573 for review Message-ID: <20081213.130816.74659290.imp@bsdimp.com> In-Reply-To: <200812131005.33499.hselasky@c2i.net> References: <200812122326.mBCNQX6w024511@repoman.freebsd.org> <200812131005.33499.hselasky@c2i.net>
next in thread | previous in thread | raw e-mail | index | archive | help
From: Hans Petter Selasky <hselasky@c2i.net> Subject: Re: PERFORCE change 154573 for review Date: Sat, 13 Dec 2008 10:05:32 +0100 > On Saturday 13 December 2008, Sam Leffler wrote: > > http://perforce.freebsd.org/chv.cgi?CH=154573 > > > > Change 154573 by sam@sam_ebb on 2008/12/12 23:25:40 > > > > Checkpoint cambria/ixp435 ehci support: add EHCI_SCFLG_BIGEDESC > > flag to force descriptor contents to be left in big-endian byte > > order intead of little-endian; this is required by the ixp435 > > builtin controllers. > > I would prefer if you could implement this using ifdefs. > > ehcireg.h: > > #ifdef HOST_ENDIAN_BUILD > #undef htole32 > #define htole32 htobe32 > ... > #define EXTERNAL(name) name##_be > #else > #define EXTERNAL(name) name##_le > #endif > > ehci_wrap.c: (new file) > > void > ehci_init(ehci_sc_t *sc) > { > if (force use bigendian) > ehci_init_be(sc); > else > ehci_init_le(sc); > return; > } > > And the same for all other globally exported functions from ehci.c which are > not so many! > > ehci.c: > > void > EXTERNAL(ehci_init)(ehci_sc_t *sc) > { > ... same like before ... > } > > ehci_be.c: > #define HOST_ENDIAN_BUILD > #include <sys/dev/usb/ehci.c> This is absoultely the wrong way to implement this. It is so wrong, I don't even know where to begin. Consider this an 'over my dead body' level of objection to this design. Warner
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20081213.130816.74659290.imp>