Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1035877AbdD1MtW (ORCPT ); Fri, 28 Apr 2017 08:49:22 -0400 Received: from relay4-d.mail.gandi.net ([217.70.183.196]:55801 "EHLO relay4-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1031780AbdD1MtP (ORCPT ); Fri, 28 Apr 2017 08:49:15 -0400 Date: Fri, 28 Apr 2017 14:49:02 +0200 From: jmondi To: Linus Walleij Cc: Jacopo Mondi , Geert Uytterhoeven , Laurent Pinchart , Chris Brandt , Rob Herring , Mark Rutland , Russell King , Linux-Renesas , "linux-gpio@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v5 02/10] pinctrl: generic: Add macros to unpack properties Message-ID: <20170428124902.GA29046@w540> References: <1493281194-5200-1-git-send-email-jacopo+renesas@jmondi.org> <1493281194-5200-3-git-send-email-jacopo+renesas@jmondi.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1397 Lines: 52 Hi Linus, On Fri, Apr 28, 2017 at 10:16:22AM +0200, Linus Walleij wrote: > On Thu, Apr 27, 2017 at 10:19 AM, Jacopo Mondi > wrote: > > > Add PIN_CONF_UNPACK_PARAM and PIN_CONF_UNPACK_ARGS macros useful to > > unpack generic properties and their arguments > > > > Signed-off-by: Jacopo Mondi > > (...) > > /* > * Helpful configuration macro to be used in tables etc. > > Then this should say "macros" rather than "macro". > > > -#define PIN_CONF_PACKED(p, a) ((a << 8) | ((unsigned long) p & 0xffUL)) > > +#define PIN_CONF_PACKED(p, a) (((a) << 8) | ((unsigned long) (p) & 0xffUL)) > > Also adding some extra parantheses I see. > > > +#define PIN_CONF_UNPACK_PARAM(c) ((c) & 0xffUL) > > +#define PIN_CONF_UNPACK_ARGS(c) ((c) >> 8) > > But why. > > I have these two static inlines just below your new macros: > > static inline enum pin_config_param pinconf_to_config_param(unsigned > long config) > { > return (enum pin_config_param) (config & 0xffUL); > } > > static inline u32 pinconf_to_config_argument(unsigned long config) > { > return (u32) ((config >> 8) & 0xffffffUL); > } > > Why can't you use this in your code instead of macros? > > We generally prefer static inlines over macros because they are easier > to read. > Right. I haven't noticed them. I'll drop this patch, sorry for noise > Yours, > Linus Walleij