On Sun, Mar 04, 2018 at 08:44:26PM +0100, Michał Kępień wrote: > > On Wed, Feb 28, 2018 at 06:08:52PM +0200, Andy Shevchenko wrote: > > > On Tue, Feb 27, 2018 at 11:15 PM, Micha?? K??pie?? <kernel@xxxxxxxxxx> wrote: > > > > Various functions exposed by the firmware through the FUNC interface > > > > tend to use a consistent set of integers for denoting the type of > > > > operation to be performed for a specified feature. Use named constants > > > > instead of integers in each call_fext_func() invocation in order to more > > > > clearly convey the intent of each call. > > > > > > > > Note that FUNC_FLAGS is a bit peculiar: > > > > > > > +/* FUNC interface - operations */ > > > > +#define OP_GET BIT(1) > > > > +#define OP_GET_CAPS 0 > > > > +#define OP_GET_EVENTS BIT(0) > > > > +#define OP_GET_EXT BIT(2) > > > > +#define OP_SET BIT(0) > > > > +#define OP_SET_EXT (BIT(2) | BIT(0)) > > > > > > Hmm... this looks unordered a bit. > > > > It seems to be ordered alphabetically on the identifier. Andy, is it > > preferred to order defines like this based on resolved numeric order? > > Just to expand on what Jonathan wrote above: if you take a peek at the > end result of the patch series, you will notice a pattern: constants in > each section are ordered alphabetically by their name. I wanted all > sections to be consistently ordered. If you would rather have me order > things by the bit index, sure, no problem, just please note that the > order above is not accidental. Hrm. In my experience it is more typical to order by value (bit), that's a little less obvious when using the BIT()|BIT() macros though. So long as it's consistent, I think that's what matters most. -- Darren Hart VMware Open Source Technology Center