Date: Sun, 30 Aug 2015 00:24:29 -0700 From: Justin Hibbits <jrh29@alumni.cwru.edu> To: Bruce Evans <brde@optusnet.com.au> Cc: Adrian Chadd <adrian.chadd@gmail.com>, "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org>, Marcel Moolenaar <marcel@xcllnt.net> Subject: Re: Devices with 36-bit paddr on 32-bit system Message-ID: <CAHSQbTDtEagLJ%2BoTke=bCHLeBEryyVpx5fh1rk-oR7TUKTQkoQ@mail.gmail.com> In-Reply-To: <20150830153859.S1263@besplex.bde.org> References: <CAHSQbTDsvB32%2BLyzHJO78VwUwAfUTMOUQp13BMCUpapSMT0fbg@mail.gmail.com> <ED4B5B25-D7A7-440C-9452-4C79B0800D2E@xcllnt.net> <1568331.OrSoeYfXsf@ralph.baldwin.cx> <CAJ-VmomduZBYT6=e7HUm2V1m0taM0MAMXxMojYV8wvgEKyUEyA@mail.gmail.com> <CAHSQbTAGD=4A20XZL09YXbEm9hdf5K2_QCRPFOAjrXHF4eg9sQ@mail.gmail.com> <20150830130612.L890@besplex.bde.org> <20150830153859.S1263@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Bruce, On Sat, Aug 29, 2015 at 10:49 PM, Bruce Evans <brde@optusnet.com.au> wrote: > On Sun, 30 Aug 2015, Bruce Evans wrote: > >> % ... >> % Index: sys/sys/bus.h >> % =================================================================== >> % --- sys/sys/bus.h (revision 287189) >> % +++ sys/sys/bus.h (working copy) >> % @@ -29,6 +29,7 @@ >> % #ifndef _SYS_BUS_H_ >> % #define _SYS_BUS_H_ >> % % +#include <machine/_bus.h> >> % #include <machine/_limits.h> >> % #include <sys/_bus_dma.h> >> % #include <sys/ioccom.h> >> % @@ -292,8 +293,8 @@ >> % int rid; /**< @brief resource identifier */ >> % int flags; /**< @brief resource flags */ >> % struct resource *res; /**< @brief the real resource when >> allocated */ >> % - u_long start; /**< @brief start of resource >> range */ >> % - u_long end; /**< @brief end of resource range >> */ >> % + bus_addr_t start; /**< @brief start of resource >> range */ >> % + bus_addr_t end; /**< @brief end of resource range >> */ > > > Mail programs (mostly mine) corrupted the formatting more competely. > >> I think rman functions should use an rman type and not hard-code >> bus_addr_t. >> Related bus functions should then use this type. Style bugs from blind >> substitution can be reduced by using a less verbose name. >> >> % u_long count; /**< @brief count within range */ >> % }; > > > Or just use uintmax_t for everything in rman. rman was written before > C99 broke C by making u_long no longer the largest integer type. It > used u_long because it was the largest integer type (though it actually > wasn't, since FreeBSD used nonstandard extensions in Gnu C) and it is > easiest to use a single non-typedefed type that is large enough for all > cases. uintmax_t is C99's replacement of u_long. > > I don't like the bloat from using uintmax_t for everything, but rman > should only used for initialization so uintmax_t for rman should only > give space bloat, only on 32-bit arches. > > An rman typedef for this type allows re-optimizing the 32-bit arches, > but brings back the problem of typedefed types being hard to use. > > Bruce Thanks for the lengthy review. I am well aware of (most of) the style issues and intend to address them all before releasing a final patch. My goal with this rough early release was to solicit comments on the approach taken, and if there should be a better way to address the overall problem. I did consider your suggestion of uintmax_t across the board early on, but felt there must be a better type name to identify purpose. A 'rman_addr_t' is better than bus_addr_t, so I can easily make that mechanical change (simple sed on the diff would do it). Also, good eye on the qmax()/qmin() needing to have unsigned counterparts, like I said, I haven't yet tested on a full 64-bit platform :) Regarding casts and macros, since this was an evolutionary process, I haven't yet cleaned things up (why clean it up, if people tell me the basic approach sucks anyway?), so that'll come in round 2, but thanks for identifying them, they may have slipped through later patches. I'm not tied one way or the other to RM_MIN_START vs 0. Keeping it 0 would definitely be easier on the fingers, less rewrites, and I do have more to change for the RM_MAX_END (choose a better name?) As for the 'long long abomination', do you have anything better? I can't use PRIxPTR, because if that was possible I wouldn't need to make this change in the first place. I guess I could use %jx, or PRIxMAX, and cast to uintmax_t. So, that all being said, any suggestion on what the naming should be? I'll pose: rman_addr_t and rman_size_t, as counterparts to bus_*_t. Yes, it's more verbose, but it also shows usage intent. However, I would also yield to uintmax_t if others think that's the best option as well. - Justin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAHSQbTDtEagLJ%2BoTke=bCHLeBEryyVpx5fh1rk-oR7TUKTQkoQ>