On Wed, May 27, 2015 at 11:25:59PM +0000, Winkler, Tomas wrote: > > > > -----Original Message----- > > From: Greg KH [mailto:gregkh@xxxxxxxxxxxxxxxxxxx] > > Sent: Thursday, May 28, 2015 02:23 > > To: Winkler, Tomas > > Cc: arnd@xxxxxxxx; Stephen Rothwell; linux-api@xxxxxxxxxxxxxxx > > Subject: Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw > > arrays > > > > On Wed, May 27, 2015 at 04:14:20PM -0700, Greg KH wrote: > > > On Wed, May 27, 2015 at 05:42:22PM +0000, Winkler, Tomas wrote: > > > > > > > > > > > > > -----Original Message----- > > > > > From: Greg KH [mailto:gregkh@xxxxxxxxxxxxxxxxxxx] > > > > > Sent: Wednesday, May 27, 2015 20:29 > > > > > To: Winkler, Tomas > > > > > Cc: arnd@xxxxxxxx; Stephen Rothwell; linux-api@xxxxxxxxxxxxxxx > > > > > Subject: Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning > > raw > > > > > arrays > > > > > > > > > > On Wed, May 27, 2015 at 05:24:01PM +0000, Winkler, Tomas wrote: > > > > > > > > > > > > > > > > > > > > On Wed, May 27, 2015 at 06:42:13PM +0300, Tomas Winkler wrote: > > > > > > > > In order for mei client devices to use device id based on uuid we > > > > > > > > have to use common types between user space (file2alias.c). > > > > > > > > Similarly to vmbus, mei uses raw 16 byte array for that. > > > > > > > > To leverage on existing infrastructure around uuid_le type > > > > > > > > defined in uuid.h we add helper macros to handle conversions > > between > > > > > > > > raw 16 byte array and uuid_{le,be} types. > > > > > > > > > > > > > > You aren't adding a helper macro, you are just redefining the existing > > > > > > > macros using a different one. > > > > > > > > > > > > Not exactly I'm using both the one I've added for device ids and the old > > one for > > > > > all the other flows. > > > > > > > > > > > > But I can't see why this is needed, what > > > > > > > does this solve that vmbus and other uses of the existing macros don't > > > > > > > need? In other words, what makes mei so special that it needs a > > "lower" > > > > > > > level macro than every other subsystem? > > > > > > > > > > > > It's not special there is actually a lot of code duplication around uuid > > handling > > > > > > every subsystem is using their own macros but it can be consolidated > > around > > > > > uuid.h > > > > > > > > > > > > So vmbus can use that > > > > > > Instead of > > > > > > /* > > > > > > * Network GUID > > > > > > * {f8615163-df3e-46c5-913f-f2d2f965ed0e} > > > > > > */ > > > > > > #define HV_NIC_GUID \ > > > > > > .guid = { \ > > > > > > 0x63, 0x51, 0x61, 0xf8, 0x3e, 0xdf, 0xc5, 0x46, \ > > > > > > 0x91, 0x3f, 0xf2, 0xd2, 0xf9, 0x65, 0xed, 0x0e \ > > > > > > } > > > > > > > > > > > > The can use the new macro to make it more readable, something in spirit > > of: > > > > > > > > > > > > #define HV_NIC_GUID __UUID_LE(f8615163-df3e-46c5-913f- > > f2d2f965ed0e) > > > > > > > > > > Why the "__" usage here? That signifies a "private" namespace, why add > > > > > that to the user visible header files? > > > > > > > > I take any other suggestion for macro names. > > > > Not sure how I would reuse the macros if I don't export them both, > > > > second this can be used also by user space. > > > > > > But it's not needed at all. > > > > > > Below is a patch on top of my current tree that makes this patch not > > > needed. Any objection to me just applying it instead? > > > > > > > > And are you going to send patches for vmbus and other drivers to fix > > > > > everything up to use these new macros? Someone has to... > > > > > > > > Can be done but I cannot test their code and now I'm busy with splitting the > > big bus patch :) > > > > > > Ok, that means no one is ever going to do that work, so it's not a valid > > > reason to accept such a change. I prefer the patch below. > > > > Oops, that didn't build, I forgot one change in the mei core (getting > > rid of two casts.) Try this one instead. > > > > thanks, > > > > greg k-h > > > > > > diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c > > index de8fd089a8a4..dd52f224027e 100644 > > --- a/drivers/misc/mei/bus.c > > +++ b/drivers/misc/mei/bus.c > > @@ -54,9 +54,9 @@ static int mei_cl_device_match(struct device *dev, struct > > device_driver *drv) > > > > id = driver->id_table; > > > > - while (uuid_le_cmp(NULL_UUID_LE, uuid_le_cast(id->uuid))) { > > + while (uuid_le_cmp(NULL_UUID_LE, id->uuid)) { > > > > - if (!uuid_le_cmp(*uuid, uuid_le_cast(id->uuid))) { > > + if (!uuid_le_cmp(*uuid, id->uuid)) { > > if (id->name[0]) { > > if (!strncmp(name, id->name, sizeof(id->name))) > > return 1; > > You can also drop the uuid_le_cast function Odd that gcc didn't complain about that. Now removed, I'll resend it in "official" form. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
