From owner-freebsd-arch@freebsd.org Sun Aug 30 07:24:31 2015 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 09ED99C490D for ; Sun, 30 Aug 2015 07:24:31 +0000 (UTC) (envelope-from chmeeedalf@gmail.com) Received: from mail-ig0-x22b.google.com (mail-ig0-x22b.google.com [IPv6:2607:f8b0:4001:c05::22b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id C81F77F2 for ; Sun, 30 Aug 2015 07:24:30 +0000 (UTC) (envelope-from chmeeedalf@gmail.com) Received: by igbuu8 with SMTP id uu8so30370115igb.0 for ; Sun, 30 Aug 2015 00:24:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=tNFZ9whj+Ks/z+meJb9JOXwXUbq6LBGTj+lDxXebVo4=; b=GAzGHO+m8L0lxt1bFRBn/qpVqUkwvtuPg4HUHZJcQ9Mb6ATTe9a564leBpBfuGFxHy F2akOtIUs6GRTcY7w/ihOTFtzAYOlFXXHRwNRaawrmeb8yR3mfMkHiTfKqqwSQSBOIUC G+yjwxjQ8ny1vbKIoFhxogJLRJakA/Dn6vg2OEseiIjAA3h/UPwoRNFHQdGBuGrjJYoH v4R6YWI8gk733mb6FAd08lWByd3MOksUHiQClt2WJep92WUHc/AO03fIwtCWIM0F5zE6 GjhZzcu80FMgN5rCyiQk8IwdYOWIPivbDRDM8UVBTTLgtp6Ho2+4DXQwr+5Qt1WqgGqN 2oLg== MIME-Version: 1.0 X-Received: by 10.50.62.148 with SMTP id y20mr9761242igr.17.1440919470181; Sun, 30 Aug 2015 00:24:30 -0700 (PDT) Sender: chmeeedalf@gmail.com Received: by 10.36.58.149 with HTTP; Sun, 30 Aug 2015 00:24:29 -0700 (PDT) In-Reply-To: <20150830153859.S1263@besplex.bde.org> References: <1568331.OrSoeYfXsf@ralph.baldwin.cx> <20150830130612.L890@besplex.bde.org> <20150830153859.S1263@besplex.bde.org> Date: Sun, 30 Aug 2015 00:24:29 -0700 X-Google-Sender-Auth: 60wbfbO6IaSvthlPfoYGGsknACw Message-ID: Subject: Re: Devices with 36-bit paddr on 32-bit system From: Justin Hibbits To: Bruce Evans Cc: Adrian Chadd , "freebsd-arch@freebsd.org" , Marcel Moolenaar Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 30 Aug 2015 07:24:31 -0000 Hi Bruce, On Sat, Aug 29, 2015 at 10:49 PM, Bruce Evans 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 >> % #include >> % #include >> % #include >> % @@ -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