On Tue, 2016-28-06 at 04:40:56 UTC, Suraj Jitindar Singh wrote: > An opal_msg of type OPAL_MSG_ASYNC_COMP contains the return code in the > params[1] struct member. However this isn't intuitive or obvious when > reading the code and requires that a user look at the skiboot > documentation or opal-api.h to verify this. > > Add a #define to get the return code from an opal_msg and update call > sites accordingly. Thanks for cleaning this up. Two gripes though :) > arch/powerpc/include/asm/opal-api.h | 4 ++++ opal-api.h is supposed to be a subset of the skiboot version. So something like this should go in the kernel's opal.h, which has all the kernel prototypes etc. which aren't part of the OPAL API. I think this routine should fall under that. > diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h > index 9bb8ddf..7433cf0 100644 > --- a/arch/powerpc/include/asm/opal-api.h > +++ b/arch/powerpc/include/asm/opal-api.h > @@ -387,6 +387,10 @@ struct opal_msg { > __be64 params[8]; > }; > > +#define GET_OPAL_MSG_ASYNC_COMP_RC(msg) (msg.msg_type == OPAL_MSG_ASYNC_COMP ? \ > + be64_to_cpu(msg.params[1]) : \ > + OPAL_PARAMETER) > + You forgot the 7th commandment! "Never use a #define when a static inline would work" :) A few reasons: - it's less shouty. - you get type checking. - you don't have to wrap lines with \ etc. cheers -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html