From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Date: Fri, 06 Mar 2015 11:05:24 +0000 Subject: Re: [PATCH 2/4] pinctrl: sh-pfc: Store register/field widths in u8 instead of unsigned long Message-Id: <2354666.zjhAAbqIil@avalon> List-Id: References: <1425058685-12956-1-git-send-email-geert+renesas@glider.be> <12083025.CFMfs0EISD@avalon> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Geert Uytterhoeven Cc: Geert Uytterhoeven , Linus Walleij , Magnus Damm , "linux-gpio@vger.kernel.org" , Linux-sh list Hi Geert, On Thursday 05 March 2015 10:19:33 Geert Uytterhoeven wrote: > On Thu, Mar 5, 2015 at 10:03 AM, Laurent Pinchart wrote: > >> --- a/drivers/pinctrl/sh-pfc/sh_pfc.h > >> +++ b/drivers/pinctrl/sh-pfc/sh_pfc.h > >> @@ -69,9 +69,10 @@ struct pinmux_func { > >> }; > >> > >> struct pinmux_cfg_reg { > >> - unsigned long reg, reg_width, field_width; > >> + unsigned long reg; > > > > How about making reg a u32 ? It won't make a difference in practice on > > 32-bit systems, but it would be more explicit. You might have missed this comment. > > We could also save space by making reg a u16 and storing the register > > offset only instead of the full address (assuming it can always fit in 16 > > bits, which should be checked). We'll also need to support 64-bit systems > > at some point, and making reg a u64 would increase space waste. > > That would be more intrusive (and definitely needs to be in a separate > patch), as reg is used here to store a physical register address, for > conversion between physical and virtual addresses. I didn't want to go that > far yet. > > u16 would indeed be nice, as it means reg, reg_width, and field_width > would fit in one 32-bit word, which I hadn't realized. That means we can > reduce each entry by 2 words instead of 1. > > For 64-bit that would still be suboptimal, as pointers are aligned to 8 > bytes, leading to gaps. > Perhaps we do want __packed here, too? I don't think the performance drop of > doing some unaligned accesses would be significant. This isn't a hot path. -- Regards, Laurent Pinchart