On Wed, May 27, 2015 at 11:25:59PM +0000, Winkler, Tomas wrote:

quoted

-----Original Message-----
From: Greg KH [mailto:gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org]
Sent: Thursday, May 28, 2015 02:23
To: Winkler, Tomas
Cc: arnd-r2nGTMty4D4@public.gmane.org; Stephen Rothwell; linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
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:

quoted

On Wed, May 27, 2015 at 05:42:22PM +0000, Winkler, Tomas wrote:

quoted

quoted

-----Original Message-----
From: Greg KH [mailto:gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org]
Sent: Wednesday, May 27, 2015 20:29
To: Winkler, Tomas
Cc: arnd-r2nGTMty4D4@public.gmane.org; Stephen Rothwell; linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning

raw

quoted

quoted

quoted

arrays

On Wed, May 27, 2015 at 05:24:01PM +0000, Winkler, Tomas wrote:

quoted

quoted

On Wed, May 27, 2015 at 06:42:13PM +0300, Tomas Winkler wrote:

quoted

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

quoted

quoted

quoted

quoted

quoted

quoted

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

quoted

quoted

quoted

all the other flows.

quoted

 But I can't see why this is needed, what

quoted

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"

quoted

quoted

quoted

quoted

quoted

level macro than every other subsystem?

It's not special there is actually a lot of code duplication around uuid

handling

quoted

quoted

quoted

quoted

every subsystem is using their own macros but it can be consolidated

around

quoted

quoted

quoted

uuid.h

quoted

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:

quoted

quoted

quoted

quoted

#define HV_NIC_GUID __UUID_LE(f8615163-df3e-46c5-913f-

f2d2f965ed0e)

quoted

quoted

quoted

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?

quoted

quoted

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 :)

quoted

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