| Message ID | 1493281194-5200-3-git-send-email-jacopo+renesas@jmondi.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
On Thu, Apr 27, 2017 at 10:19 AM, Jacopo Mondi <jacopo+renesas@jmondi.org> 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 <jacopo+renesas@jmondi.org> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Thu, Apr 27, 2017 at 10:19 AM, Jacopo Mondi <jacopo+renesas@jmondi.org> 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 <jacopo+renesas@jmondi.org> (...) /* * 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. Yours, Linus Walleij
On Fri, Apr 28, 2017 at 10:16 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Thu, Apr 27, 2017 at 10:19 AM, Jacopo Mondi >> +#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); > } Cool, need...more...context...in...patches ;-) > 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. Sure. Thanks for noticing! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
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 > <jacopo+renesas@jmondi.org> 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 <jacopo+renesas@jmondi.org> > > (...) > > /* > * 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
On Fri, Apr 28, 2017 at 11:16 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Thu, Apr 27, 2017 at 10:19 AM, Jacopo Mondi > <jacopo+renesas@jmondi.org> wrote: > But why. > > I have these two static inlines just below your new macros: +1. > We generally prefer static inlines over macros because they are easier > to read. Not only. It adds type checking as well AFAIUC.
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h index 279e3c5..2cd2a03 100644 --- a/include/linux/pinctrl/pinconf-generic.h +++ b/include/linux/pinctrl/pinconf-generic.h @@ -118,7 +118,9 @@ enum pin_config_param { /* * Helpful configuration macro to be used in tables etc. */ -#define PIN_CONF_PACKED(p, a) ((a << 8) | ((unsigned long) p & 0xffUL)) +#define PIN_CONF_PACKED(p, a) (((a) << 8) | ((unsigned long) (p) & 0xffUL)) +#define PIN_CONF_UNPACK_PARAM(c) ((c) & 0xffUL) +#define PIN_CONF_UNPACK_ARGS(c) ((c) >> 8) /* * The following inlines stuffs a configuration parameter and data value
Add PIN_CONF_UNPACK_PARAM and PIN_CONF_UNPACK_ARGS macros useful to unpack generic properties and their arguments Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- include/linux/pinctrl/pinconf-generic.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)