linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 00/10] uuid: convert users to generic UUID API
@ 2016-02-17 12:17 Andy Shevchenko
  2016-02-17 12:17 ` [PATCH v1 01/10] lib/vsprintf: simplify UUID printing Andy Shevchenko
                   ` (9 more replies)
  0 siblings, 10 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-02-17 12:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, linux-kernel, dri-devel, linux-efi,
	linux-api, linux-nvdimm
  Cc: Andy Shevchenko

There are few fumctions here and there along with type definitions that provide
UUID API. This series consolidates everything under one hood and converts
current users.

This has been tested for a while internally, however it doesn't mean we covered
all possible cases (especially accuracy of UUID constants after conversion).
So, please test this as much as you can and provide your tag. We appreciate the
effort.

Andy Shevchenko (10):
  lib/vsprintf: simplify UUID printing
  lib/uuid: move generate_random_uuid() to uuid.c
  lib/uuid: introduce few more generic helpers for UUID
  lib/uuid: remove FSF address
  ACPI: switch to use generic UUID API
  device property: switch to use UUID API
  sysctl: drop away useless label
  sysctl: use generic UUID library
  efi: redefine type, constant, macro from generic code
  efivars: use generic UUID library

 drivers/acpi/acpi_extlog.c                        |  8 +-
 drivers/acpi/bus.c                                | 29 +------
 drivers/acpi/nfit.c                               | 34 ++++----
 drivers/acpi/nfit.h                               |  3 +-
 drivers/acpi/property.c                           | 18 ++---
 drivers/acpi/utils.c                              |  4 +-
 drivers/char/random.c                             | 21 +----
 drivers/char/tpm/tpm_crb.c                        |  9 +--
 drivers/char/tpm/tpm_ppi.c                        | 20 ++---
 drivers/gpu/drm/i915/intel_acpi.c                 | 14 ++--
 drivers/gpu/drm/nouveau/nouveau_acpi.c            | 20 +++--
 drivers/gpu/drm/nouveau/nvkm/subdev/mxm/base.c    |  9 +--
 drivers/hid/i2c-hid/i2c-hid.c                     |  9 +--
 drivers/iommu/dmar.c                              | 11 ++-
 drivers/pci/pci-acpi.c                            | 11 ++-
 drivers/pci/pci-label.c                           |  4 +-
 drivers/thermal/int340x_thermal/int3400_thermal.c |  6 +-
 drivers/usb/host/xhci-pci.c                       |  9 +--
 fs/btrfs/volumes.c                                |  2 +-
 fs/efivarfs/inode.c                               | 40 +---------
 fs/ext4/ioctl.c                                   |  1 +
 fs/f2fs/file.c                                    |  2 +-
 fs/reiserfs/objectid.c                            |  2 +-
 fs/ubifs/sb.c                                     |  2 +-
 include/acpi/acpi_bus.h                           | 10 ++-
 include/linux/acpi.h                              |  2 +-
 include/linux/efi.h                               | 14 +---
 include/linux/pci-acpi.h                          |  2 +-
 include/linux/random.h                            |  1 -
 include/linux/uuid.h                              | 21 +++--
 include/uapi/linux/uuid.h                         |  4 -
 kernel/sysctl_binary.c                            | 30 +++----
 lib/uuid.c                                        | 96 +++++++++++++++++++++--
 lib/vsprintf.c                                    | 21 ++---
 sound/soc/intel/skylake/skl-nhlt.c                |  7 +-
 35 files changed, 237 insertions(+), 259 deletions(-)

-- 
2.7.0

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v1 01/10] lib/vsprintf: simplify UUID printing
  2016-02-17 12:17 [PATCH v1 00/10] uuid: convert users to generic UUID API Andy Shevchenko
@ 2016-02-17 12:17 ` Andy Shevchenko
  2016-02-17 12:17 ` [PATCH v1 02/10] lib/uuid: move generate_random_uuid() to uuid.c Andy Shevchenko
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-02-17 12:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, linux-kernel, dri-devel, linux-efi,
	linux-api, linux-nvdimm
  Cc: Andy Shevchenko

Since we have hex_byte_pack_upper() we may use it directly and avoid second
loop.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 lib/vsprintf.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 525c8e1..17d976b 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1324,7 +1324,10 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
 	}
 
 	for (i = 0; i < 16; i++) {
-		p = hex_byte_pack(p, addr[index[i]]);
+		if (uc)
+			p = hex_byte_pack_upper(p, addr[index[i]]);
+		else
+			p = hex_byte_pack(p, addr[index[i]]);
 		switch (i) {
 		case 3:
 		case 5:
@@ -1337,13 +1340,6 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
 
 	*p = 0;
 
-	if (uc) {
-		p = uuid;
-		do {
-			*p = toupper(*p);
-		} while (*(++p));
-	}
-
 	return string(buf, end, uuid, spec);
 }
 
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v1 02/10] lib/uuid: move generate_random_uuid() to uuid.c
  2016-02-17 12:17 [PATCH v1 00/10] uuid: convert users to generic UUID API Andy Shevchenko
  2016-02-17 12:17 ` [PATCH v1 01/10] lib/vsprintf: simplify UUID printing Andy Shevchenko
@ 2016-02-17 12:17 ` Andy Shevchenko
  2016-02-17 12:17 ` [PATCH v1 03/10] lib/uuid: introduce few more generic helpers for UUID Andy Shevchenko
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-02-17 12:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, linux-kernel, dri-devel, linux-efi,
	linux-api, linux-nvdimm
  Cc: Andy Shevchenko

Let's gather the UUID related functions under one hood.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/char/random.c  | 21 +--------------------
 fs/btrfs/volumes.c     |  2 +-
 fs/ext4/ioctl.c        |  1 +
 fs/f2fs/file.c         |  2 +-
 fs/reiserfs/objectid.c |  2 +-
 fs/ubifs/sb.c          |  2 +-
 include/linux/random.h |  1 -
 include/linux/uuid.h   |  2 ++
 lib/uuid.c             | 20 ++++++++++++++++++++
 9 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index b583e53..0158d3b 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -260,6 +260,7 @@
 #include <linux/irq.h>
 #include <linux/syscalls.h>
 #include <linux/completion.h>
+#include <linux/uuid.h>
 
 #include <asm/processor.h>
 #include <asm/uaccess.h>
@@ -1621,26 +1622,6 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
 	return urandom_read(NULL, buf, count, NULL);
 }
 
-/***************************************************************
- * Random UUID interface
- *
- * Used here for a Boot ID, but can be useful for other kernel
- * drivers.
- ***************************************************************/
-
-/*
- * Generate random UUID
- */
-void generate_random_uuid(unsigned char uuid_out[16])
-{
-	get_random_bytes(uuid_out, 16);
-	/* Set UUID version to 4 --- truly random generation */
-	uuid_out[6] = (uuid_out[6] & 0x0F) | 0x40;
-	/* Set the UUID variant to DCE */
-	uuid_out[8] = (uuid_out[8] & 0x3F) | 0x80;
-}
-EXPORT_SYMBOL(generate_random_uuid);
-
 /********************************************************************
  *
  * Sysctl interface
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 2555e37..548c525 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -20,13 +20,13 @@
 #include <linux/slab.h>
 #include <linux/buffer_head.h>
 #include <linux/blkdev.h>
-#include <linux/random.h>
 #include <linux/iocontext.h>
 #include <linux/capability.h>
 #include <linux/ratelimit.h>
 #include <linux/kthread.h>
 #include <linux/raid/pq.h>
 #include <linux/semaphore.h>
+#include <linux/uuid.h>
 #include <asm/div64.h>
 #include "ctree.h"
 #include "extent_map.h"
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index a99b010..9e100f6 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -15,6 +15,7 @@
 #include <linux/file.h>
 #include <linux/random.h>
 #include <linux/quotaops.h>
+#include <linux/uuid.h>
 #include <asm/uaccess.h>
 #include "ext4_jbd2.h"
 #include "ext4.h"
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index a4362d4..c7fec78 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -20,7 +20,7 @@
 #include <linux/uaccess.h>
 #include <linux/mount.h>
 #include <linux/pagevec.h>
-#include <linux/random.h>
+#include <linux/uuid.h>
 
 #include "f2fs.h"
 #include "node.h"
diff --git a/fs/reiserfs/objectid.c b/fs/reiserfs/objectid.c
index 99a5d5d..415d66c 100644
--- a/fs/reiserfs/objectid.c
+++ b/fs/reiserfs/objectid.c
@@ -3,8 +3,8 @@
  */
 
 #include <linux/string.h>
-#include <linux/random.h>
 #include <linux/time.h>
+#include <linux/uuid.h>
 #include "reiserfs.h"
 
 /* find where objectid map starts */
diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
index f4fbc7b..3cbb904 100644
--- a/fs/ubifs/sb.c
+++ b/fs/ubifs/sb.c
@@ -28,8 +28,8 @@
 
 #include "ubifs.h"
 #include <linux/slab.h>
-#include <linux/random.h>
 #include <linux/math64.h>
+#include <linux/uuid.h>
 
 /*
  * Default journal size in logical eraseblocks as a percent of total
diff --git a/include/linux/random.h b/include/linux/random.h
index 9c29122..e47e533 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -26,7 +26,6 @@ extern void get_random_bytes(void *buf, int nbytes);
 extern int add_random_ready_callback(struct random_ready_callback *rdy);
 extern void del_random_ready_callback(struct random_ready_callback *rdy);
 extern void get_random_bytes_arch(void *buf, int nbytes);
-void generate_random_uuid(unsigned char uuid_out[16]);
 extern int random_int_secret_init(void);
 
 #ifndef MODULE
diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 6df2509..91c2b6d 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -33,6 +33,8 @@ static inline int uuid_be_cmp(const uuid_be u1, const uuid_be u2)
 	return memcmp(&u1, &u2, sizeof(uuid_be));
 }
 
+void generate_random_uuid(unsigned char uuid[16]);
+
 extern void uuid_le_gen(uuid_le *u);
 extern void uuid_be_gen(uuid_be *u);
 
diff --git a/lib/uuid.c b/lib/uuid.c
index 398821e..6c81c0b 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -23,6 +23,26 @@
 #include <linux/uuid.h>
 #include <linux/random.h>
 
+/***************************************************************
+ * Random UUID interface
+ *
+ * Used here for a Boot ID, but can be useful for other kernel
+ * drivers.
+ ***************************************************************/
+
+/*
+ * Generate random UUID
+ */
+void generate_random_uuid(unsigned char uuid[16])
+{
+	get_random_bytes(uuid, 16);
+	/* Set UUID version to 4 --- truly random generation */
+	uuid[6] = (uuid[6] & 0x0F) | 0x40;
+	/* Set the UUID variant to DCE */
+	uuid[8] = (uuid[8] & 0x3F) | 0x80;
+}
+EXPORT_SYMBOL(generate_random_uuid);
+
 static void __uuid_gen_common(__u8 b[16])
 {
 	prandom_bytes(b, 16);
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v1 03/10] lib/uuid: introduce few more generic helpers for UUID
  2016-02-17 12:17 [PATCH v1 00/10] uuid: convert users to generic UUID API Andy Shevchenko
  2016-02-17 12:17 ` [PATCH v1 01/10] lib/vsprintf: simplify UUID printing Andy Shevchenko
  2016-02-17 12:17 ` [PATCH v1 02/10] lib/uuid: move generate_random_uuid() to uuid.c Andy Shevchenko
@ 2016-02-17 12:17 ` Andy Shevchenko
  2016-02-17 12:17 ` [PATCH v1 04/10] lib/uuid: remove FSF address Andy Shevchenko
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-02-17 12:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, linux-kernel, dri-devel, linux-efi,
	linux-api, linux-nvdimm
  Cc: Andy Shevchenko

There are new helpers in this patch:

uuid_is_valid		checks if a UUID is valid
uuid_be_to_bin		converts from string to binary (big endian)
uuid_le_to_bin		converts from string to binary (little endian)

They will be used in future, i.e. in the following patches in the series.

This also moves indices arrays to lib/uuid.c to be shared accross modules.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/uuid.h | 13 ++++++++++
 lib/uuid.c           | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/vsprintf.c       |  9 +++----
 3 files changed, 87 insertions(+), 5 deletions(-)

diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 91c2b6d..616347f 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -22,6 +22,11 @@
 
 #include <uapi/linux/uuid.h>
 
+/*
+ * The length of a UUID string ("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
+ * not including trailing NUL.
+ */
+#define	UUID_STRING_LEN		36
 
 static inline int uuid_le_cmp(const uuid_le u1, const uuid_le u2)
 {
@@ -38,4 +43,12 @@ void generate_random_uuid(unsigned char uuid[16]);
 extern void uuid_le_gen(uuid_le *u);
 extern void uuid_be_gen(uuid_be *u);
 
+int __must_check uuid_is_valid(const char *uuid);
+
+extern const u8 uuid_le_index[16];
+extern const u8 uuid_be_index[16];
+
+int uuid_le_to_bin(const char *uuid, uuid_le *u);
+int uuid_be_to_bin(const char *uuid, uuid_be *u);
+
 #endif
diff --git a/lib/uuid.c b/lib/uuid.c
index 6c81c0b..995865c 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -19,10 +19,17 @@
  */
 
 #include <linux/kernel.h>
+#include <linux/ctype.h>
+#include <linux/errno.h>
 #include <linux/export.h>
 #include <linux/uuid.h>
 #include <linux/random.h>
 
+const u8 uuid_le_index[16] = {3,2,1,0,5,4,7,6,8,9,10,11,12,13,14,15};
+EXPORT_SYMBOL(uuid_le_index);
+const u8 uuid_be_index[16] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
+EXPORT_SYMBOL(uuid_be_index);
+
 /***************************************************************
  * Random UUID interface
  *
@@ -65,3 +72,66 @@ void uuid_be_gen(uuid_be *bu)
 	bu->b[6] = (bu->b[6] & 0x0F) | 0x40;
 }
 EXPORT_SYMBOL_GPL(uuid_be_gen);
+
+/**
+  * uuid_is_valid - checks if UUID string valid
+  * @uuid:	UUID string to check
+  *
+  * Description:
+  * It checks if the UUID string is following the format:
+  *	xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+  * where x is a hex digit.
+  *
+  * Return: 0 on success, %-EINVAL otherwise.
+  */
+int uuid_is_valid(const char *uuid)
+{
+	unsigned int i;
+
+	if (strnlen(uuid, UUID_STRING_LEN) < UUID_STRING_LEN)
+		return -EINVAL;
+
+	for (i = 0; i < UUID_STRING_LEN; i++) {
+		if (i == 8 || i == 13 || i == 18 || i == 23) {
+			if (uuid[i] != '-')
+				return -EINVAL;
+		} else if (!isxdigit(uuid[i])) {
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(uuid_is_valid);
+
+static int __uuid_to_bin(const char *uuid, __u8 b[16], const u8 ei[16])
+{
+	static const u8 si[16] = {0,2,4,6,9,11,14,16,19,21,24,26,28,30,32,34};
+	unsigned int i;
+	int ret;
+
+	ret = uuid_is_valid(uuid);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < 16; i++) {
+		int hi = hex_to_bin(uuid[si[i]] + 0);
+		int lo = hex_to_bin(uuid[si[i]] + 1);
+
+		b[ei[i]] = (hi << 4) | lo;
+	}
+
+	return 0;
+}
+
+int uuid_le_to_bin(const char *uuid, uuid_le *u)
+{
+	return __uuid_to_bin(uuid, u->b, uuid_le_index);
+}
+EXPORT_SYMBOL(uuid_le_to_bin);
+
+int uuid_be_to_bin(const char *uuid, uuid_be *u)
+{
+	return __uuid_to_bin(uuid, u->b, uuid_be_index);
+}
+EXPORT_SYMBOL(uuid_be_to_bin);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 17d976b..752e78d 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -30,6 +30,7 @@
 #include <linux/ioport.h>
 #include <linux/dcache.h>
 #include <linux/cred.h>
+#include <linux/uuid.h>
 #include <net/addrconf.h>
 #ifdef CONFIG_BLOCK
 #include <linux/blkdev.h>
@@ -1304,19 +1305,17 @@ static noinline_for_stack
 char *uuid_string(char *buf, char *end, const u8 *addr,
 		  struct printf_spec spec, const char *fmt)
 {
-	char uuid[sizeof("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx")];
+	char uuid[UUID_STRING_LEN + 1];
 	char *p = uuid;
 	int i;
-	static const u8 be[16] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
-	static const u8 le[16] = {3,2,1,0,5,4,7,6,8,9,10,11,12,13,14,15};
-	const u8 *index = be;
+	const u8 *index = uuid_be_index;
 	bool uc = false;
 
 	switch (*(++fmt)) {
 	case 'L':
 		uc = true;		/* fall-through */
 	case 'l':
-		index = le;
+		index = uuid_le_index;
 		break;
 	case 'B':
 		uc = true;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v1 04/10] lib/uuid: remove FSF address
  2016-02-17 12:17 [PATCH v1 00/10] uuid: convert users to generic UUID API Andy Shevchenko
                   ` (2 preceding siblings ...)
  2016-02-17 12:17 ` [PATCH v1 03/10] lib/uuid: introduce few more generic helpers for UUID Andy Shevchenko
@ 2016-02-17 12:17 ` Andy Shevchenko
  2016-02-17 12:17 ` [PATCH v1 05/10] ACPI: switch to use generic UUID API Andy Shevchenko
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-02-17 12:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, linux-kernel, dri-devel, linux-efi,
	linux-api, linux-nvdimm
  Cc: Andy Shevchenko

There is no point to keep an address in the file since it's a subject to
change.

While here, update Intel Copyright years.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/uuid.h      | 6 +-----
 include/uapi/linux/uuid.h | 4 ----
 lib/uuid.c                | 6 +-----
 3 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 616347f..6bd40d0 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -1,7 +1,7 @@
 /*
  * UUID/GUID definition
  *
- * Copyright (C) 2010, Intel Corp.
+ * Copyright (C) 2010, 2015 Intel Corp.
  *	Huang Ying <ying.huang@intel.com>
  *
  * This program is free software; you can redistribute it and/or
@@ -12,10 +12,6 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #ifndef _LINUX_UUID_H_
 #define _LINUX_UUID_H_
diff --git a/include/uapi/linux/uuid.h b/include/uapi/linux/uuid.h
index 786f077..3738e5f 100644
--- a/include/uapi/linux/uuid.h
+++ b/include/uapi/linux/uuid.h
@@ -12,10 +12,6 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
 #ifndef _UAPI_LINUX_UUID_H_
diff --git a/lib/uuid.c b/lib/uuid.c
index 995865c..0918232 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -1,7 +1,7 @@
 /*
  * Unified UUID/GUID definition
  *
- * Copyright (C) 2009, Intel Corp.
+ * Copyright (C) 2009, 2015 Intel Corp.
  *	Huang Ying <ying.huang@intel.com>
  *
  * This program is free software; you can redistribute it and/or
@@ -12,10 +12,6 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
 #include <linux/kernel.h>
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v1 05/10] ACPI: switch to use generic UUID API
  2016-02-17 12:17 [PATCH v1 00/10] uuid: convert users to generic UUID API Andy Shevchenko
                   ` (3 preceding siblings ...)
  2016-02-17 12:17 ` [PATCH v1 04/10] lib/uuid: remove FSF address Andy Shevchenko
@ 2016-02-17 12:17 ` Andy Shevchenko
  2016-02-17 17:49   ` Dan Williams
  2016-02-17 12:17 ` [PATCH v1 06/10] device property: switch to use " Andy Shevchenko
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2016-02-17 12:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, linux-kernel, dri-devel, linux-efi,
	linux-api, linux-nvdimm
  Cc: Andy Shevchenko

Instead of opencoding the existing library functions let's use them directly.

The conversion fixes a potential bug in int340x_thermal as well since we have
to use memcmp() on binary data.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/acpi_extlog.c                        |  8 +++---
 drivers/acpi/bus.c                                | 29 ++-----------------
 drivers/acpi/nfit.c                               | 34 +++++++++++------------
 drivers/acpi/nfit.h                               |  3 +-
 drivers/acpi/utils.c                              |  4 +--
 drivers/char/tpm/tpm_crb.c                        |  9 +++---
 drivers/char/tpm/tpm_ppi.c                        | 20 ++++++-------
 drivers/gpu/drm/i915/intel_acpi.c                 | 14 ++++------
 drivers/gpu/drm/nouveau/nouveau_acpi.c            | 20 ++++++-------
 drivers/gpu/drm/nouveau/nvkm/subdev/mxm/base.c    |  9 +++---
 drivers/hid/i2c-hid/i2c-hid.c                     |  9 +++---
 drivers/iommu/dmar.c                              | 11 ++++----
 drivers/pci/pci-acpi.c                            | 11 ++++----
 drivers/pci/pci-label.c                           |  4 +--
 drivers/thermal/int340x_thermal/int3400_thermal.c |  6 ++--
 drivers/usb/host/xhci-pci.c                       |  9 +++---
 include/acpi/acpi_bus.h                           | 10 ++++---
 include/linux/acpi.h                              |  2 +-
 include/linux/pci-acpi.h                          |  2 +-
 sound/soc/intel/skylake/skl-nhlt.c                |  7 +++--
 20 files changed, 92 insertions(+), 129 deletions(-)

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index b3842ff..a3a25ec 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -45,7 +45,9 @@ struct extlog_l1_head {
 
 static int old_edac_report_status;
 
-static u8 extlog_dsm_uuid[] __initdata = "663E35AF-CC10-41A4-88EA-5470AF055295";
+static uuid_le extlog_dsm_uuid __initdata =
+	UUID_LE(0x663E35AF, 0xCC10, 0x41A4,
+		0x88, 0xEA, 0x54, 0x70, 0xAF, 0x05, 0x52, 0x95);
 
 /* L1 table related physical address */
 static u64 elog_base;
@@ -182,12 +184,10 @@ out:
 
 static bool __init extlog_get_l1addr(void)
 {
-	u8 uuid[16];
+	uuid_le *uuid = &extlog_dsm_uuid;
 	acpi_handle handle;
 	union acpi_object *obj;
 
-	acpi_str_to_uuid(extlog_dsm_uuid, uuid);
-
 	if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
 		return false;
 	if (!acpi_check_dsm(handle, uuid, EXTLOG_DSM_REV, 1 << EXTLOG_FN_ADDR))
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 891c42d..80db48b 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -192,42 +192,19 @@ static void acpi_print_osc_error(acpi_handle handle,
 	printk("\n");
 }
 
-acpi_status acpi_str_to_uuid(char *str, u8 *uuid)
-{
-	int i;
-	static int opc_map_to_uuid[16] = {6, 4, 2, 0, 11, 9, 16, 14, 19, 21,
-		24, 26, 28, 30, 32, 34};
-
-	if (strlen(str) != 36)
-		return AE_BAD_PARAMETER;
-	for (i = 0; i < 36; i++) {
-		if (i == 8 || i == 13 || i == 18 || i == 23) {
-			if (str[i] != '-')
-				return AE_BAD_PARAMETER;
-		} else if (!isxdigit(str[i]))
-			return AE_BAD_PARAMETER;
-	}
-	for (i = 0; i < 16; i++) {
-		uuid[i] = hex_to_bin(str[opc_map_to_uuid[i]]) << 4;
-		uuid[i] |= hex_to_bin(str[opc_map_to_uuid[i] + 1]);
-	}
-	return AE_OK;
-}
-EXPORT_SYMBOL_GPL(acpi_str_to_uuid);
-
 acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
 {
 	acpi_status status;
 	struct acpi_object_list input;
 	union acpi_object in_params[4];
 	union acpi_object *out_obj;
-	u8 uuid[16];
+	uuid_le uuid;
 	u32 errors;
 	struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL};
 
 	if (!context)
 		return AE_ERROR;
-	if (ACPI_FAILURE(acpi_str_to_uuid(context->uuid_str, uuid)))
+	if (uuid_le_to_bin(context->uuid_str, &uuid))
 		return AE_ERROR;
 	context->ret.length = ACPI_ALLOCATE_BUFFER;
 	context->ret.pointer = NULL;
@@ -237,7 +214,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
 	input.pointer = in_params;
 	in_params[0].type 		= ACPI_TYPE_BUFFER;
 	in_params[0].buffer.length 	= 16;
-	in_params[0].buffer.pointer	= uuid;
+	in_params[0].buffer.pointer	= (char *)&uuid;
 	in_params[1].type 		= ACPI_TYPE_INTEGER;
 	in_params[1].integer.value 	= context->rev;
 	in_params[2].type 		= ACPI_TYPE_INTEGER;
diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index ad6d8c6..3beb99b 100644
--- a/drivers/acpi/nfit.c
+++ b/drivers/acpi/nfit.c
@@ -43,11 +43,11 @@ struct nfit_table_prev {
 	struct list_head flushes;
 };
 
-static u8 nfit_uuid[NFIT_UUID_MAX][16];
+static uuid_le nfit_uuid[NFIT_UUID_MAX];
 
-const u8 *to_nfit_uuid(enum nfit_uuids id)
+const uuid_le *to_nfit_uuid(enum nfit_uuids id)
 {
-	return nfit_uuid[id];
+	return &nfit_uuid[id];
 }
 EXPORT_SYMBOL(to_nfit_uuid);
 
@@ -83,7 +83,7 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
 	const char *cmd_name, *dimm_name;
 	unsigned long dsm_mask;
 	acpi_handle handle;
-	const u8 *uuid;
+	const uuid_le *uuid;
 	u32 offset;
 	int rc, i;
 
@@ -225,7 +225,7 @@ static int nfit_spa_type(struct acpi_nfit_system_address *spa)
 	int i;
 
 	for (i = 0; i < NFIT_UUID_MAX; i++)
-		if (memcmp(to_nfit_uuid(i), spa->range_guid, 16) == 0)
+		if (!uuid_le_cmp(*to_nfit_uuid(i), *(uuid_le *)spa->range_guid))
 			return i;
 	return -1;
 }
@@ -829,7 +829,7 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc,
 {
 	struct acpi_device *adev, *adev_dimm;
 	struct device *dev = acpi_desc->dev;
-	const u8 *uuid = to_nfit_uuid(NFIT_DEV_DIMM);
+	const uuid_le *uuid = to_nfit_uuid(NFIT_DEV_DIMM);
 	int i;
 
 	nfit_mem->dsm_mask = acpi_desc->dimm_dsm_force_en;
@@ -909,7 +909,7 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
 static void acpi_nfit_init_dsms(struct acpi_nfit_desc *acpi_desc)
 {
 	struct nvdimm_bus_descriptor *nd_desc = &acpi_desc->nd_desc;
-	const u8 *uuid = to_nfit_uuid(NFIT_DEV_BUS);
+	const uuid_le *uuid = to_nfit_uuid(NFIT_DEV_BUS);
 	struct acpi_device *adev;
 	int i;
 
@@ -2079,16 +2079,16 @@ static __init int nfit_init(void)
 	BUILD_BUG_ON(sizeof(struct acpi_nfit_control_region) != 80);
 	BUILD_BUG_ON(sizeof(struct acpi_nfit_data_region) != 40);
 
-	acpi_str_to_uuid(UUID_VOLATILE_MEMORY, nfit_uuid[NFIT_SPA_VOLATILE]);
-	acpi_str_to_uuid(UUID_PERSISTENT_MEMORY, nfit_uuid[NFIT_SPA_PM]);
-	acpi_str_to_uuid(UUID_CONTROL_REGION, nfit_uuid[NFIT_SPA_DCR]);
-	acpi_str_to_uuid(UUID_DATA_REGION, nfit_uuid[NFIT_SPA_BDW]);
-	acpi_str_to_uuid(UUID_VOLATILE_VIRTUAL_DISK, nfit_uuid[NFIT_SPA_VDISK]);
-	acpi_str_to_uuid(UUID_VOLATILE_VIRTUAL_CD, nfit_uuid[NFIT_SPA_VCD]);
-	acpi_str_to_uuid(UUID_PERSISTENT_VIRTUAL_DISK, nfit_uuid[NFIT_SPA_PDISK]);
-	acpi_str_to_uuid(UUID_PERSISTENT_VIRTUAL_CD, nfit_uuid[NFIT_SPA_PCD]);
-	acpi_str_to_uuid(UUID_NFIT_BUS, nfit_uuid[NFIT_DEV_BUS]);
-	acpi_str_to_uuid(UUID_NFIT_DIMM, nfit_uuid[NFIT_DEV_DIMM]);
+	uuid_le_to_bin(UUID_VOLATILE_MEMORY, &nfit_uuid[NFIT_SPA_VOLATILE]);
+	uuid_le_to_bin(UUID_PERSISTENT_MEMORY, &nfit_uuid[NFIT_SPA_PM]);
+	uuid_le_to_bin(UUID_CONTROL_REGION, &nfit_uuid[NFIT_SPA_DCR]);
+	uuid_le_to_bin(UUID_DATA_REGION, &nfit_uuid[NFIT_SPA_BDW]);
+	uuid_le_to_bin(UUID_VOLATILE_VIRTUAL_DISK, &nfit_uuid[NFIT_SPA_VDISK]);
+	uuid_le_to_bin(UUID_VOLATILE_VIRTUAL_CD, &nfit_uuid[NFIT_SPA_VCD]);
+	uuid_le_to_bin(UUID_PERSISTENT_VIRTUAL_DISK, &nfit_uuid[NFIT_SPA_PDISK]);
+	uuid_le_to_bin(UUID_PERSISTENT_VIRTUAL_CD, &nfit_uuid[NFIT_SPA_PCD]);
+	uuid_le_to_bin(UUID_NFIT_BUS, &nfit_uuid[NFIT_DEV_BUS]);
+	uuid_le_to_bin(UUID_NFIT_DIMM, &nfit_uuid[NFIT_DEV_DIMM]);
 
 	return acpi_bus_register_driver(&acpi_nfit_driver);
 }
diff --git a/drivers/acpi/nfit.h b/drivers/acpi/nfit.h
index 3d549a3..88b73e8 100644
--- a/drivers/acpi/nfit.h
+++ b/drivers/acpi/nfit.h
@@ -16,7 +16,6 @@
 #define __NFIT_H__
 #include <linux/libnvdimm.h>
 #include <linux/types.h>
-#include <linux/uuid.h>
 #include <linux/acpi.h>
 #include <acpi/acuuid.h>
 
@@ -180,7 +179,7 @@ static inline struct acpi_nfit_desc *to_acpi_desc(
 	return container_of(nd_desc, struct acpi_nfit_desc, nd_desc);
 }
 
-const u8 *to_nfit_uuid(enum nfit_uuids id);
+const uuid_le *to_nfit_uuid(enum nfit_uuids id);
 int acpi_nfit_init(struct acpi_nfit_desc *nfit, acpi_size sz);
 extern const struct attribute_group *acpi_nfit_attribute_groups[];
 #endif /* __NFIT_H__ */
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index f2f9873..a56a995 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -629,7 +629,7 @@ acpi_status acpi_evaluate_lck(acpi_handle handle, int lock)
  * some old BIOSes do expect a buffer or an integer etc.
  */
 union acpi_object *
-acpi_evaluate_dsm(acpi_handle handle, const u8 *uuid, int rev, int func,
+acpi_evaluate_dsm(acpi_handle handle, const uuid_le *uuid, int rev, int func,
 		  union acpi_object *argv4)
 {
 	acpi_status ret;
@@ -678,7 +678,7 @@ EXPORT_SYMBOL(acpi_evaluate_dsm);
  * functions. Currently only support 64 functions at maximum, should be
  * enough for now.
  */
-bool acpi_check_dsm(acpi_handle handle, const u8 *uuid, int rev, u64 funcs)
+bool acpi_check_dsm(acpi_handle handle, const uuid_le *uuid, int rev, u64 funcs)
 {
 	int i;
 	u64 mask = 0;
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 916332c..57e6e82 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -24,10 +24,9 @@
 
 #define ACPI_SIG_TPM2 "TPM2"
 
-static const u8 CRB_ACPI_START_UUID[] = {
-	/* 0000 */ 0xAB, 0x6C, 0xBF, 0x6B, 0x63, 0x54, 0x14, 0x47,
-	/* 0008 */ 0xB7, 0xCD, 0xF0, 0x20, 0x3C, 0x03, 0x68, 0xD4
-};
+static const uuid_le crb_acpi_start_uuid =
+	UUID_LE(0x6BBF6CAB, 0x5463, 0x4714,
+		0xB7, 0xCD, 0xF0, 0x20, 0x3C, 0x03, 0x68, 0xD4);
 
 enum crb_defaults {
 	CRB_ACPI_START_REVISION_ID = 1,
@@ -127,7 +126,7 @@ static int crb_do_acpi_start(struct tpm_chip *chip)
 	int rc;
 
 	obj = acpi_evaluate_dsm(chip->acpi_dev_handle,
-				CRB_ACPI_START_UUID,
+				&crb_acpi_start_uuid,
 				CRB_ACPI_START_REVISION_ID,
 				CRB_ACPI_START_INDEX,
 				NULL);
diff --git a/drivers/char/tpm/tpm_ppi.c b/drivers/char/tpm/tpm_ppi.c
index 692a2c6..7cf6824 100644
--- a/drivers/char/tpm/tpm_ppi.c
+++ b/drivers/char/tpm/tpm_ppi.c
@@ -32,20 +32,16 @@
 #define PPI_VS_REQ_START	128
 #define PPI_VS_REQ_END		255
 
-static const u8 tpm_ppi_uuid[] = {
-	0xA6, 0xFA, 0xDD, 0x3D,
-	0x1B, 0x36,
-	0xB4, 0x4E,
-	0xA4, 0x24,
-	0x8D, 0x10, 0x08, 0x9D, 0x16, 0x53
-};
+static const uuid_le tpm_ppi_uuid =
+	UUID_LE(0x3DDDFAA6, 0x361B, 0x4EB4,
+		0xA4, 0x24, 0x8D, 0x10, 0x08, 0x9D, 0x16, 0x53);
 
 static inline union acpi_object *
 tpm_eval_dsm(acpi_handle ppi_handle, int func, acpi_object_type type,
 	     union acpi_object *argv4)
 {
 	BUG_ON(!ppi_handle);
-	return acpi_evaluate_dsm_typed(ppi_handle, tpm_ppi_uuid,
+	return acpi_evaluate_dsm_typed(ppi_handle, &tpm_ppi_uuid,
 				       TPM_PPI_REVISION_ID,
 				       func, argv4, type);
 }
@@ -107,7 +103,7 @@ static ssize_t tpm_store_ppi_request(struct device *dev,
 	 * is updated with function index from SUBREQ to SUBREQ2 since PPI
 	 * version 1.1
 	 */
-	if (acpi_check_dsm(chip->acpi_dev_handle, tpm_ppi_uuid,
+	if (acpi_check_dsm(chip->acpi_dev_handle, &tpm_ppi_uuid,
 			   TPM_PPI_REVISION_ID, 1 << TPM_PPI_FN_SUBREQ2))
 		func = TPM_PPI_FN_SUBREQ2;
 
@@ -268,7 +264,7 @@ static ssize_t show_ppi_operations(acpi_handle dev_handle, char *buf, u32 start,
 		"User not required",
 	};
 
-	if (!acpi_check_dsm(dev_handle, tpm_ppi_uuid, TPM_PPI_REVISION_ID,
+	if (!acpi_check_dsm(dev_handle, &tpm_ppi_uuid, TPM_PPI_REVISION_ID,
 			    1 << TPM_PPI_FN_GETOPR))
 		return -EPERM;
 
@@ -341,12 +337,12 @@ void tpm_add_ppi(struct tpm_chip *chip)
 	if (!chip->acpi_dev_handle)
 		return;
 
-	if (!acpi_check_dsm(chip->acpi_dev_handle, tpm_ppi_uuid,
+	if (!acpi_check_dsm(chip->acpi_dev_handle, &tpm_ppi_uuid,
 			    TPM_PPI_REVISION_ID, 1 << TPM_PPI_FN_VERSION))
 		return;
 
 	/* Cache PPI version string. */
-	obj = acpi_evaluate_dsm_typed(chip->acpi_dev_handle, tpm_ppi_uuid,
+	obj = acpi_evaluate_dsm_typed(chip->acpi_dev_handle, &tpm_ppi_uuid,
 				      TPM_PPI_REVISION_ID, TPM_PPI_FN_VERSION,
 				      NULL, ACPI_TYPE_STRING);
 	if (obj) {
diff --git a/drivers/gpu/drm/i915/intel_acpi.c b/drivers/gpu/drm/i915/intel_acpi.c
index eb638a1..72bfe6c 100644
--- a/drivers/gpu/drm/i915/intel_acpi.c
+++ b/drivers/gpu/drm/i915/intel_acpi.c
@@ -15,13 +15,9 @@ static struct intel_dsm_priv {
 	acpi_handle dhandle;
 } intel_dsm_priv;
 
-static const u8 intel_dsm_guid[] = {
-	0xd3, 0x73, 0xd8, 0x7e,
-	0xd0, 0xc2,
-	0x4f, 0x4e,
-	0xa8, 0x54,
-	0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c
-};
+static const uuid_le intel_dsm_guid =
+	UUID_LE(0x7ed873d3, 0xc2d0, 0x4e4f,
+		0xa8, 0x54, 0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c);
 
 static char *intel_dsm_port_name(u8 id)
 {
@@ -80,7 +76,7 @@ static void intel_dsm_platform_mux_info(void)
 	int i;
 	union acpi_object *pkg, *connector_count;
 
-	pkg = acpi_evaluate_dsm_typed(intel_dsm_priv.dhandle, intel_dsm_guid,
+	pkg = acpi_evaluate_dsm_typed(intel_dsm_priv.dhandle, &intel_dsm_guid,
 			INTEL_DSM_REVISION_ID, INTEL_DSM_FN_PLATFORM_MUX_INFO,
 			NULL, ACPI_TYPE_PACKAGE);
 	if (!pkg) {
@@ -118,7 +114,7 @@ static bool intel_dsm_pci_probe(struct pci_dev *pdev)
 	if (!dhandle)
 		return false;
 
-	if (!acpi_check_dsm(dhandle, intel_dsm_guid, INTEL_DSM_REVISION_ID,
+	if (!acpi_check_dsm(dhandle, &intel_dsm_guid, INTEL_DSM_REVISION_ID,
 			    1 << INTEL_DSM_FN_PLATFORM_MUX_INFO)) {
 		DRM_DEBUG_KMS("no _DSM method for intel device\n");
 		return false;
diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index cdf5227..52ee9a3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
@@ -61,15 +61,13 @@ bool nouveau_is_v1_dsm(void) {
 #define NOUVEAU_DSM_HAS_OPT 0x2
 
 #ifdef CONFIG_VGA_SWITCHEROO
-static const char nouveau_dsm_muid[] = {
-	0xA0, 0xA0, 0x95, 0x9D, 0x60, 0x00, 0x48, 0x4D,
-	0xB3, 0x4D, 0x7E, 0x5F, 0xEA, 0x12, 0x9F, 0xD4,
-};
+static const uuid_le nouveau_dsm_muid =
+	UUID_LE(0x9D95A0A0, 0x0060, 0x4D48,
+		0xB3, 0x4D, 0x7E, 0x5F, 0xEA, 0x12, 0x9F, 0xD4);
 
-static const char nouveau_op_dsm_muid[] = {
-	0xF8, 0xD8, 0x86, 0xA4, 0xDA, 0x0B, 0x1B, 0x47,
-	0xA7, 0x2B, 0x60, 0x42, 0xA6, 0xB5, 0xBE, 0xE0,
-};
+static const uuid_le nouveau_op_dsm_muid =
+	UUID_LE(0xA486D8F8, 0x0BDA, 0x471B,
+		0xA7, 0x2B, 0x60, 0x42, 0xA6, 0xB5, 0xBE, 0xE0);
 
 static int nouveau_optimus_dsm(acpi_handle handle, int func, int arg, uint32_t *result)
 {
@@ -87,7 +85,7 @@ static int nouveau_optimus_dsm(acpi_handle handle, int func, int arg, uint32_t *
 		args_buff[i] = (arg >> i * 8) & 0xFF;
 
 	*result = 0;
-	obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, 0x00000100,
+	obj = acpi_evaluate_dsm_typed(handle, &nouveau_op_dsm_muid, 0x00000100,
 				      func, &argv4, ACPI_TYPE_BUFFER);
 	if (!obj) {
 		acpi_handle_info(handle, "failed to evaluate _DSM\n");
@@ -137,7 +135,7 @@ static int nouveau_dsm(acpi_handle handle, int func, int arg)
 		.integer.value = arg,
 	};
 
-	obj = acpi_evaluate_dsm_typed(handle, nouveau_dsm_muid, 0x00000102,
+	obj = acpi_evaluate_dsm_typed(handle, &nouveau_dsm_muid, 0x00000102,
 				      func, &argv4, ACPI_TYPE_INTEGER);
 	if (!obj) {
 		acpi_handle_info(handle, "failed to evaluate _DSM\n");
@@ -224,7 +222,7 @@ static int nouveau_dsm_pci_probe(struct pci_dev *pdev)
 	if (!acpi_has_method(dhandle, "_DSM"))
 		return false;
 
-	if (acpi_check_dsm(dhandle, nouveau_dsm_muid, 0x00000102,
+	if (acpi_check_dsm(dhandle, &nouveau_dsm_muid, 0x00000102,
 			   1 << NOUVEAU_DSM_POWER))
 		retval |= NOUVEAU_DSM_HAS_MUX;
 
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/base.c
index 9700a76..5fe45e9 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/base.c
@@ -81,10 +81,9 @@ mxm_shadow_dsm(struct nvkm_mxm *mxm, u8 version)
 {
 	struct nvkm_subdev *subdev = &mxm->subdev;
 	struct nvkm_device *device = subdev->device;
-	static char muid[] = {
-		0x00, 0xA4, 0x04, 0x40, 0x7D, 0x91, 0xF2, 0x4C,
-		0xB8, 0x9C, 0x79, 0xB6, 0x2F, 0xD5, 0x56, 0x65
-	};
+	static uuid_le muid =
+		UUID_LE(0x4004A400, 0x917D, 0x4CF2,
+			0xB8, 0x9C, 0x79, 0xB6, 0x2F, 0xD5, 0x56, 0x65);
 	u32 mxms_args[] = { 0x00000000 };
 	union acpi_object argv4 = {
 		.buffer.type = ACPI_TYPE_BUFFER,
@@ -105,7 +104,7 @@ mxm_shadow_dsm(struct nvkm_mxm *mxm, u8 version)
 	 * unless you pass in exactly the version it supports..
 	 */
 	rev = (version & 0xf0) << 4 | (version & 0x0f);
-	obj = acpi_evaluate_dsm(handle, muid, rev, 0x00000010, &argv4);
+	obj = acpi_evaluate_dsm(handle, &muid, rev, 0x00000010, &argv4);
 	if (!obj) {
 		nvkm_debug(subdev, "DSM MXMS failed\n");
 		return false;
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index b921693..291bacf 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -871,10 +871,9 @@ static const struct acpi_gpio_mapping i2c_hid_acpi_gpios[] = {
 static int i2c_hid_acpi_pdata(struct i2c_client *client,
 		struct i2c_hid_platform_data *pdata)
 {
-	static u8 i2c_hid_guid[] = {
-		0xF7, 0xF6, 0xDF, 0x3C, 0x67, 0x42, 0x55, 0x45,
-		0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE,
-	};
+	static uuid_le i2c_hid_guid =
+		UUID_LE(0x3CDFF6F7, 0x4267, 0x4555,
+			0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
 	union acpi_object *obj;
 	struct acpi_device *adev;
 	acpi_handle handle;
@@ -884,7 +883,7 @@ static int i2c_hid_acpi_pdata(struct i2c_client *client,
 	if (!handle || acpi_bus_get_device(handle, &adev))
 		return -ENODEV;
 
-	obj = acpi_evaluate_dsm_typed(handle, i2c_hid_guid, 1, 1, NULL,
+	obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
 				      ACPI_TYPE_INTEGER);
 	if (!obj) {
 		dev_err(&client->dev, "device _DSM execution failed\n");
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index fb092f3..1b4509e 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1783,10 +1783,9 @@ IOMMU_INIT_POST(detect_intel_iommu);
  * for Directed-IO Architecture Specifiction, Rev 2.2, Section 8.8
  * "Remapping Hardware Unit Hot Plug".
  */
-static u8 dmar_hp_uuid[] = {
-	/* 0000 */    0xA6, 0xA3, 0xC1, 0xD8, 0x9B, 0xBE, 0x9B, 0x4C,
-	/* 0008 */    0x91, 0xBF, 0xC3, 0xCB, 0x81, 0xFC, 0x5D, 0xAF
-};
+static uuid_le dmar_hp_uuid =
+	UUID_LE(0xD8C1A3A6, 0xBE9B, 0x4C9B,
+		0x91, 0xBF, 0xC3, 0xCB, 0x81, 0xFC, 0x5D, 0xAF);
 
 /*
  * Currently there's only one revision and BIOS will not check the revision id,
@@ -1799,7 +1798,7 @@ static u8 dmar_hp_uuid[] = {
 
 static inline bool dmar_detect_dsm(acpi_handle handle, int func)
 {
-	return acpi_check_dsm(handle, dmar_hp_uuid, DMAR_DSM_REV_ID, 1 << func);
+	return acpi_check_dsm(handle, &dmar_hp_uuid, DMAR_DSM_REV_ID, 1 << func);
 }
 
 static int dmar_walk_dsm_resource(acpi_handle handle, int func,
@@ -1818,7 +1817,7 @@ static int dmar_walk_dsm_resource(acpi_handle handle, int func,
 	if (!dmar_detect_dsm(handle, func))
 		return 0;
 
-	obj = acpi_evaluate_dsm_typed(handle, dmar_hp_uuid, DMAR_DSM_REV_ID,
+	obj = acpi_evaluate_dsm_typed(handle, &dmar_hp_uuid, DMAR_DSM_REV_ID,
 				      func, NULL, ACPI_TYPE_BUFFER);
 	if (!obj)
 		return -ENODEV;
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 9a033e8..c129da3 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -24,10 +24,9 @@
  * The UUID is defined in the PCI Firmware Specification available here:
  * https://www.pcisig.com/members/downloads/pcifw_r3_1_13Dec10.pdf
  */
-const u8 pci_acpi_dsm_uuid[] = {
-	0xd0, 0x37, 0xc9, 0xe5, 0x53, 0x35, 0x7a, 0x4d,
-	0x91, 0x17, 0xea, 0x4d, 0x19, 0xc3, 0x43, 0x4d
-};
+const uuid_le pci_acpi_dsm_uuid =
+	UUID_LE(0xe5c937d0, 0x3553, 0x4d7a,
+		0x91, 0x17, 0xea, 0x4d, 0x19, 0xc3, 0x43, 0x4d);
 
 phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle)
 {
@@ -558,7 +557,7 @@ void acpi_pci_add_bus(struct pci_bus *bus)
 	if (!pci_is_root_bus(bus))
 		return;
 
-	obj = acpi_evaluate_dsm(ACPI_HANDLE(bus->bridge), pci_acpi_dsm_uuid, 3,
+	obj = acpi_evaluate_dsm(ACPI_HANDLE(bus->bridge), &pci_acpi_dsm_uuid, 3,
 				RESET_DELAY_DSM, NULL);
 	if (!obj)
 		return;
@@ -623,7 +622,7 @@ static void pci_acpi_optimize_delay(struct pci_dev *pdev,
 	if (bridge->ignore_reset_delay)
 		pdev->d3cold_delay = 0;
 
-	obj = acpi_evaluate_dsm(handle, pci_acpi_dsm_uuid, 3,
+	obj = acpi_evaluate_dsm(handle, &pci_acpi_dsm_uuid, 3,
 				FUNCTION_DELAY_DSM, NULL);
 	if (!obj)
 		return;
diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
index 0ae74d9..05d94d7 100644
--- a/drivers/pci/pci-label.c
+++ b/drivers/pci/pci-label.c
@@ -172,7 +172,7 @@ static int dsm_get_label(struct device *dev, char *buf,
 	if (!handle)
 		return -1;
 
-	obj = acpi_evaluate_dsm(handle, pci_acpi_dsm_uuid, 0x2,
+	obj = acpi_evaluate_dsm(handle, &pci_acpi_dsm_uuid, 0x2,
 				DEVICE_LABEL_DSM, NULL);
 	if (!obj)
 		return -1;
@@ -212,7 +212,7 @@ static bool device_has_dsm(struct device *dev)
 	if (!handle)
 		return false;
 
-	return !!acpi_check_dsm(handle, pci_acpi_dsm_uuid, 0x2,
+	return !!acpi_check_dsm(handle, &pci_acpi_dsm_uuid, 0x2,
 				1 << DEVICE_LABEL_DSM);
 }
 
diff --git a/drivers/thermal/int340x_thermal/int3400_thermal.c b/drivers/thermal/int340x_thermal/int3400_thermal.c
index 5836e55..47f54bc 100644
--- a/drivers/thermal/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/int340x_thermal/int3400_thermal.c
@@ -141,10 +141,10 @@ static int int3400_thermal_get_uuids(struct int3400_thermal_priv *priv)
 		}
 
 		for (j = 0; j < INT3400_THERMAL_MAXIMUM_UUID; j++) {
-			u8 uuid[16];
+			uuid_le u;
 
-			acpi_str_to_uuid(int3400_thermal_uuids[j], uuid);
-			if (!strncmp(uuid, objb->buffer.pointer, 16)) {
+			uuid_le_to_bin(int3400_thermal_uuids[j], &u);
+			if (!uuid_le_cmp(u, *(uuid_le *)objb->buffer.pointer)) {
 				priv->uuid_bitmap |= (1 << j);
 				break;
 			}
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index f0640b7..4290e94 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -191,13 +191,12 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 #ifdef CONFIG_ACPI
 static void xhci_pme_acpi_rtd3_enable(struct pci_dev *dev)
 {
-	static const u8 intel_dsm_uuid[] = {
-		0xb7, 0x0c, 0x34, 0xac,	0x01, 0xe9, 0xbf, 0x45,
-		0xb7, 0xe6, 0x2b, 0x34, 0xec, 0x93, 0x1e, 0x23,
-	};
+	static const uuid_le intel_dsm_uuid =
+		UUID_LE(0xac340cb7, 0xe901, 0x45bf,
+			0xb7, 0xe6, 0x2b, 0x34, 0xec, 0x93, 0x1e, 0x23);
 	union acpi_object *obj;
 
-	obj = acpi_evaluate_dsm(ACPI_HANDLE(&dev->dev), intel_dsm_uuid, 3, 1,
+	obj = acpi_evaluate_dsm(ACPI_HANDLE(&dev->dev), &intel_dsm_uuid, 3, 1,
 				NULL);
 	ACPI_FREE(obj);
 }
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 14362a8..b4b6fa2 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -61,13 +61,15 @@ bool acpi_ata_match(acpi_handle handle);
 bool acpi_bay_match(acpi_handle handle);
 bool acpi_dock_match(acpi_handle handle);
 
-bool acpi_check_dsm(acpi_handle handle, const u8 *uuid, int rev, u64 funcs);
-union acpi_object *acpi_evaluate_dsm(acpi_handle handle, const u8 *uuid,
+bool acpi_check_dsm(acpi_handle handle, const uuid_le *uuid, int rev,
+		    u64 funcs);
+union acpi_object *acpi_evaluate_dsm(acpi_handle handle, const uuid_le *uuid,
 			int rev, int func, union acpi_object *argv4);
 
 static inline union acpi_object *
-acpi_evaluate_dsm_typed(acpi_handle handle, const u8 *uuid, int rev, int func,
-			union acpi_object *argv4, acpi_object_type type)
+acpi_evaluate_dsm_typed(acpi_handle handle, const uuid_le *uuid, int rev,
+			int func, union acpi_object *argv4,
+			acpi_object_type type)
 {
 	union acpi_object *obj;
 
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 06ed7e5..7d1939d 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -39,6 +39,7 @@
 #include <linux/dynamic_debug.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/uuid.h>
 
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
@@ -430,7 +431,6 @@ struct acpi_osc_context {
 	struct acpi_buffer ret;		/* free by caller if success */
 };
 
-acpi_status acpi_str_to_uuid(char *str, u8 *uuid);
 acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context);
 
 /* Indexes into _OSC Capabilities Buffer (DWORDs 2 & 3 are device-specific) */
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 89ab057..861a779 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -101,7 +101,7 @@ static inline void acpiphp_remove_slots(struct pci_bus *bus) { }
 static inline void acpiphp_check_host_bridge(struct acpi_device *adev) { }
 #endif
 
-extern const u8 pci_acpi_dsm_uuid[];
+extern const uuid_le pci_acpi_dsm_uuid;
 #define DEVICE_LABEL_DSM	0x07
 #define RESET_DELAY_DSM		0x08
 #define FUNCTION_DELAY_DSM	0x09
diff --git a/sound/soc/intel/skylake/skl-nhlt.c b/sound/soc/intel/skylake/skl-nhlt.c
index 6e4b21c..ac1b641 100644
--- a/sound/soc/intel/skylake/skl-nhlt.c
+++ b/sound/soc/intel/skylake/skl-nhlt.c
@@ -20,8 +20,9 @@
 #include "skl.h"
 
 /* Unique identification for getting NHLT blobs */
-static u8 OSC_UUID[16] = {0x6E, 0x88, 0x9F, 0xA6, 0xEB, 0x6C, 0x94, 0x45,
-				0xA4, 0x1F, 0x7B, 0x5D, 0xCE, 0x24, 0xC5, 0x53};
+static uuid_le osc_uuid =
+	UUID_LE(0xA69F886E, 0x6CEB, 0x4594,
+		0xA4, 0x1F, 0x7B, 0x5D, 0xCE, 0x24, 0xC5, 0x53);
 
 #define DSDT_NHLT_PATH "\\_SB.PCI0.HDAS"
 
@@ -36,7 +37,7 @@ void *skl_nhlt_init(struct device *dev)
 		return NULL;
 	}
 
-	obj = acpi_evaluate_dsm(handle, OSC_UUID, 1, 1, NULL);
+	obj = acpi_evaluate_dsm(handle, &osc_uuid, 1, 1, NULL);
 	if (obj && obj->type == ACPI_TYPE_BUFFER) {
 		nhlt_ptr = (struct nhlt_resource_desc  *)obj->buffer.pointer;
 
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v1 06/10] device property: switch to use UUID API
  2016-02-17 12:17 [PATCH v1 00/10] uuid: convert users to generic UUID API Andy Shevchenko
                   ` (4 preceding siblings ...)
  2016-02-17 12:17 ` [PATCH v1 05/10] ACPI: switch to use generic UUID API Andy Shevchenko
@ 2016-02-17 12:17 ` Andy Shevchenko
  2016-02-18  0:03   ` Rafael J. Wysocki
  2016-02-18 11:07   ` Mika Westerberg
  2016-02-17 12:17 ` [PATCH v1 07/10] sysctl: drop away useless label Andy Shevchenko
                   ` (3 subsequent siblings)
  9 siblings, 2 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-02-17 12:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, linux-kernel, dri-devel, linux-efi,
	linux-api, linux-nvdimm
  Cc: Andy Shevchenko

Switch to use a generic UUID API instead of custom approach. It allows to
define UUIDs, compare them, and validate.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/property.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 2aee416..1e75305 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -25,15 +25,13 @@ static int acpi_data_get_property_array(struct acpi_device_data *data,
 					const union acpi_object **obj);
 
 /* ACPI _DSD device properties UUID: daffd814-6eba-4d8c-8a91-bc9bbf4aa301 */
-static const u8 prp_uuid[16] = {
-	0x14, 0xd8, 0xff, 0xda, 0xba, 0x6e, 0x8c, 0x4d,
-	0x8a, 0x91, 0xbc, 0x9b, 0xbf, 0x4a, 0xa3, 0x01
-};
+static const uuid_le prp_uuid =
+	UUID_LE(0xdaffd814, 0x6eba, 0x4d8c,
+		0x8a, 0x91, 0xbc, 0x9b, 0xbf, 0x4a, 0xa3, 0x01);
 /* ACPI _DSD data subnodes UUID: dbb8e3e6-5886-4ba6-8795-1319f52a966b */
-static const u8 ads_uuid[16] = {
-	0xe6, 0xe3, 0xb8, 0xdb, 0x86, 0x58, 0xa6, 0x4b,
-	0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b
-};
+static const uuid_le ads_uuid =
+	UUID_LE(0xdbb8e3e6, 0x5886, 0x4ba6,
+		0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b);
 
 static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
 					   const union acpi_object *desc,
@@ -138,7 +136,7 @@ static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
 		    || links->type != ACPI_TYPE_PACKAGE)
 			break;
 
-		if (memcmp(uuid->buffer.pointer, ads_uuid, sizeof(ads_uuid)))
+		if (uuid_le_cmp(*(uuid_le *)uuid->buffer.pointer, ads_uuid))
 			continue;
 
 		return acpi_add_nondev_subnodes(scope, links, &data->subnodes);
@@ -245,7 +243,7 @@ static bool acpi_extract_properties(const union acpi_object *desc,
 		    || properties->type != ACPI_TYPE_PACKAGE)
 			break;
 
-		if (memcmp(uuid->buffer.pointer, prp_uuid, sizeof(prp_uuid)))
+		if (uuid_le_cmp(*(uuid_le *)uuid->buffer.pointer, prp_uuid))
 			continue;
 
 		/*
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v1 07/10] sysctl: drop away useless label
  2016-02-17 12:17 [PATCH v1 00/10] uuid: convert users to generic UUID API Andy Shevchenko
                   ` (5 preceding siblings ...)
  2016-02-17 12:17 ` [PATCH v1 06/10] device property: switch to use " Andy Shevchenko
@ 2016-02-17 12:17 ` Andy Shevchenko
  2016-02-17 12:17 ` [PATCH v1 08/10] sysctl: use generic UUID library Andy Shevchenko
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-02-17 12:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, linux-kernel, dri-devel, linux-efi,
	linux-api, linux-nvdimm
  Cc: Andy Shevchenko

We have no locking in bin_uuid(). Thus, we may remove the out label and use
return statements directly.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 kernel/sysctl_binary.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
index 7e7746a..509abe4 100644
--- a/kernel/sysctl_binary.c
+++ b/kernel/sysctl_binary.c
@@ -1123,15 +1123,14 @@ static ssize_t bin_uuid(struct file *file,
 
 		result = kernel_read(file, 0, buf, sizeof(buf) - 1);
 		if (result < 0)
-			goto out;
+			return result;
 
 		buf[result] = '\0';
 
 		/* Convert the uuid to from a string to binary */
 		for (i = 0; i < 16; i++) {
-			result = -EIO;
 			if (!isxdigit(str[0]) || !isxdigit(str[1]))
-				goto out;
+				return -EIO;
 
 			uuid[i] = (hex_to_bin(str[0]) << 4) |
 					hex_to_bin(str[1]);
@@ -1143,15 +1142,12 @@ static ssize_t bin_uuid(struct file *file,
 		if (oldlen > 16)
 			oldlen = 16;
 
-		result = -EFAULT;
 		if (copy_to_user(oldval, uuid, oldlen))
-			goto out;
+			return -EFAULT;
 
 		copied = oldlen;
 	}
-	result = copied;
-out:
-	return result;
+	return copied;
 }
 
 static ssize_t bin_dn_node_address(struct file *file,
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v1 08/10] sysctl: use generic UUID library
  2016-02-17 12:17 [PATCH v1 00/10] uuid: convert users to generic UUID API Andy Shevchenko
                   ` (6 preceding siblings ...)
  2016-02-17 12:17 ` [PATCH v1 07/10] sysctl: drop away useless label Andy Shevchenko
@ 2016-02-17 12:17 ` Andy Shevchenko
  2016-02-17 12:17 ` [PATCH v1 09/10] efi: redefine type, constant, macro from generic code Andy Shevchenko
  2016-02-17 12:17 ` [PATCH v1 10/10] efivars: use generic UUID library Andy Shevchenko
  9 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-02-17 12:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, linux-kernel, dri-devel, linux-efi,
	linux-api, linux-nvdimm
  Cc: Andy Shevchenko

UUID library provides uuid_be type and uuid_be_to_bin() function. This
substitutes open coded variant by generic library calls.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 kernel/sysctl_binary.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
index 509abe4..4e5ca94 100644
--- a/kernel/sysctl_binary.c
+++ b/kernel/sysctl_binary.c
@@ -13,6 +13,7 @@
 #include <linux/ctype.h>
 #include <linux/netdevice.h>
 #include <linux/kernel.h>
+#include <linux/uuid.h>
 #include <linux/slab.h>
 #include <linux/compat.h>
 
@@ -1117,9 +1118,8 @@ static ssize_t bin_uuid(struct file *file,
 
 	/* Only supports reads */
 	if (oldval && oldlen) {
-		char buf[40], *str = buf;
-		unsigned char uuid[16];
-		int i;
+		char buf[UUID_STRING_LEN + 1];
+		uuid_be uuid;
 
 		result = kernel_read(file, 0, buf, sizeof(buf) - 1);
 		if (result < 0)
@@ -1128,21 +1128,13 @@ static ssize_t bin_uuid(struct file *file,
 		buf[result] = '\0';
 
 		/* Convert the uuid to from a string to binary */
-		for (i = 0; i < 16; i++) {
-			if (!isxdigit(str[0]) || !isxdigit(str[1]))
-				return -EIO;
-
-			uuid[i] = (hex_to_bin(str[0]) << 4) |
-					hex_to_bin(str[1]);
-			str += 2;
-			if (*str == '-')
-				str++;
-		}
+		if (uuid_be_to_bin(buf, &uuid))
+			return -EIO;
 
 		if (oldlen > 16)
 			oldlen = 16;
 
-		if (copy_to_user(oldval, uuid, oldlen))
+		if (copy_to_user(oldval, &uuid, oldlen))
 			return -EFAULT;
 
 		copied = oldlen;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v1 09/10] efi: redefine type, constant, macro from generic code
  2016-02-17 12:17 [PATCH v1 00/10] uuid: convert users to generic UUID API Andy Shevchenko
                   ` (7 preceding siblings ...)
  2016-02-17 12:17 ` [PATCH v1 08/10] sysctl: use generic UUID library Andy Shevchenko
@ 2016-02-17 12:17 ` Andy Shevchenko
  2016-02-17 12:17 ` [PATCH v1 10/10] efivars: use generic UUID library Andy Shevchenko
  9 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-02-17 12:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, linux-kernel, dri-devel, linux-efi,
	linux-api, linux-nvdimm
  Cc: Andy Shevchenko

Generic UUID library defines structure type, macro to define UUID, and the
length of the UUID string. This patch removes duplicate data structure
definition, UUID string length constant  as well as macro for UUID handling.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/efi.h  | 14 ++++----------
 include/linux/uuid.h |  2 +-
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 3c6cbbd..1a0dad5 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -21,6 +21,7 @@
 #include <linux/pfn.h>
 #include <linux/pstore.h>
 #include <linux/reboot.h>
+#include <linux/uuid.h>
 
 #include <asm/page.h>
 
@@ -43,17 +44,10 @@ typedef u16 efi_char16_t;		/* UNICODE character */
 typedef u64 efi_physical_addr_t;
 typedef void *efi_handle_t;
 
-
-typedef struct {
-	u8 b[16];
-} efi_guid_t;
+typedef uuid_le efi_guid_t;
 
 #define EFI_GUID(a,b,c,d0,d1,d2,d3,d4,d5,d6,d7) \
-((efi_guid_t) \
-{{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \
-  (b) & 0xff, ((b) >> 8) & 0xff, \
-  (c) & 0xff, ((c) >> 8) & 0xff, \
-  (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
+	UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)
 
 /*
  * Generic EFI table header
@@ -1025,7 +1019,7 @@ efi_reboot(enum reboot_mode reboot_mode, const char *__unused) {}
  * Length of a GUID string (strlen("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"))
  * not including trailing NUL
  */
-#define EFI_VARIABLE_GUID_LEN 36
+#define EFI_VARIABLE_GUID_LEN	UUID_STRING_LEN
 
 /*
  * The type of search to perform when calling boottime->locate_handle
diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 6bd40d0..bb778a4 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -20,7 +20,7 @@
 
 /*
  * The length of a UUID string ("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
- * not including trailing NUL.
+ * excluding trailing NUL.
  */
 #define	UUID_STRING_LEN		36
 
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v1 10/10] efivars: use generic UUID library
  2016-02-17 12:17 [PATCH v1 00/10] uuid: convert users to generic UUID API Andy Shevchenko
                   ` (8 preceding siblings ...)
  2016-02-17 12:17 ` [PATCH v1 09/10] efi: redefine type, constant, macro from generic code Andy Shevchenko
@ 2016-02-17 12:17 ` Andy Shevchenko
  2016-02-18 15:07   ` Matt Fleming
  9 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2016-02-17 12:17 UTC (permalink / raw)
  To: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, linux-kernel, dri-devel, linux-efi,
	linux-api, linux-nvdimm
  Cc: Andy Shevchenko

Instead of opencoding let's use generic UUID library functions here.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 fs/efivarfs/inode.c | 40 +++-------------------------------------
 1 file changed, 3 insertions(+), 37 deletions(-)

diff --git a/fs/efivarfs/inode.c b/fs/efivarfs/inode.c
index 3381b9d..b579e3a 100644
--- a/fs/efivarfs/inode.c
+++ b/fs/efivarfs/inode.c
@@ -11,6 +11,7 @@
 #include <linux/fs.h>
 #include <linux/ctype.h>
 #include <linux/slab.h>
+#include <linux/uuid.h>
 
 #include "internal.h"
 
@@ -44,11 +45,7 @@ struct inode *efivarfs_get_inode(struct super_block *sb,
  */
 bool efivarfs_valid_name(const char *str, int len)
 {
-	static const char dashes[EFI_VARIABLE_GUID_LEN] = {
-		[8] = 1, [13] = 1, [18] = 1, [23] = 1
-	};
 	const char *s = str + len - EFI_VARIABLE_GUID_LEN;
-	int i;
 
 	/*
 	 * We need a GUID, plus at least one letter for the variable name,
@@ -66,37 +63,7 @@ bool efivarfs_valid_name(const char *str, int len)
 	 *
 	 *	12345678-1234-1234-1234-123456789abc
 	 */
-	for (i = 0; i < EFI_VARIABLE_GUID_LEN; i++) {
-		if (dashes[i]) {
-			if (*s++ != '-')
-				return false;
-		} else {
-			if (!isxdigit(*s++))
-				return false;
-		}
-	}
-
-	return true;
-}
-
-static void efivarfs_hex_to_guid(const char *str, efi_guid_t *guid)
-{
-	guid->b[0] = hex_to_bin(str[6]) << 4 | hex_to_bin(str[7]);
-	guid->b[1] = hex_to_bin(str[4]) << 4 | hex_to_bin(str[5]);
-	guid->b[2] = hex_to_bin(str[2]) << 4 | hex_to_bin(str[3]);
-	guid->b[3] = hex_to_bin(str[0]) << 4 | hex_to_bin(str[1]);
-	guid->b[4] = hex_to_bin(str[11]) << 4 | hex_to_bin(str[12]);
-	guid->b[5] = hex_to_bin(str[9]) << 4 | hex_to_bin(str[10]);
-	guid->b[6] = hex_to_bin(str[16]) << 4 | hex_to_bin(str[17]);
-	guid->b[7] = hex_to_bin(str[14]) << 4 | hex_to_bin(str[15]);
-	guid->b[8] = hex_to_bin(str[19]) << 4 | hex_to_bin(str[20]);
-	guid->b[9] = hex_to_bin(str[21]) << 4 | hex_to_bin(str[22]);
-	guid->b[10] = hex_to_bin(str[24]) << 4 | hex_to_bin(str[25]);
-	guid->b[11] = hex_to_bin(str[26]) << 4 | hex_to_bin(str[27]);
-	guid->b[12] = hex_to_bin(str[28]) << 4 | hex_to_bin(str[29]);
-	guid->b[13] = hex_to_bin(str[30]) << 4 | hex_to_bin(str[31]);
-	guid->b[14] = hex_to_bin(str[32]) << 4 | hex_to_bin(str[33]);
-	guid->b[15] = hex_to_bin(str[34]) << 4 | hex_to_bin(str[35]);
+	return uuid_is_valid(s);
 }
 
 static int efivarfs_create(struct inode *dir, struct dentry *dentry,
@@ -122,8 +89,7 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry,
 	/* length of the variable name itself: remove GUID and separator */
 	namelen = dentry->d_name.len - EFI_VARIABLE_GUID_LEN - 1;
 
-	efivarfs_hex_to_guid(dentry->d_name.name + namelen + 1,
-			&var->var.VendorGuid);
+	uuid_le_to_bin(dentry->d_name.name + namelen + 1, &var->var.VendorGuid);
 
 	for (i = 0; i < namelen; i++)
 		var->var.VariableName[i] = dentry->d_name.name[i];
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 05/10] ACPI: switch to use generic UUID API
  2016-02-17 12:17 ` [PATCH v1 05/10] ACPI: switch to use generic UUID API Andy Shevchenko
@ 2016-02-17 17:49   ` Dan Williams
  2016-02-26 13:46     ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Dan Williams @ 2016-02-17 17:49 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, Linux ACPI, linux-kernel,
	Maling list - DRI developers, linux-efi, linux-api,
	linux-nvdimm@lists.01.org

On Wed, Feb 17, 2016 at 4:17 AM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> Instead of opencoding the existing library functions let's use them directly.
>
> The conversion fixes a potential bug in int340x_thermal as well since we have
> to use memcmp() on binary data.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/acpi/acpi_extlog.c                        |  8 +++---
>  drivers/acpi/bus.c                                | 29 ++-----------------
>  drivers/acpi/nfit.c                               | 34 +++++++++++------------
>  drivers/acpi/nfit.h                               |  3 +-
>  drivers/acpi/utils.c                              |  4 +--
>  drivers/char/tpm/tpm_crb.c                        |  9 +++---
>  drivers/char/tpm/tpm_ppi.c                        | 20 ++++++-------
>  drivers/gpu/drm/i915/intel_acpi.c                 | 14 ++++------
>  drivers/gpu/drm/nouveau/nouveau_acpi.c            | 20 ++++++-------
>  drivers/gpu/drm/nouveau/nvkm/subdev/mxm/base.c    |  9 +++---
>  drivers/hid/i2c-hid/i2c-hid.c                     |  9 +++---
>  drivers/iommu/dmar.c                              | 11 ++++----
>  drivers/pci/pci-acpi.c                            | 11 ++++----
>  drivers/pci/pci-label.c                           |  4 +--
>  drivers/thermal/int340x_thermal/int3400_thermal.c |  6 ++--
>  drivers/usb/host/xhci-pci.c                       |  9 +++---
>  include/acpi/acpi_bus.h                           | 10 ++++---
>  include/linux/acpi.h                              |  2 +-
>  include/linux/pci-acpi.h                          |  2 +-
>  sound/soc/intel/skylake/skl-nhlt.c                |  7 +++--
>  20 files changed, 92 insertions(+), 129 deletions(-)
>
[..]
> diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
> index ad6d8c6..3beb99b 100644
> --- a/drivers/acpi/nfit.c
> +++ b/drivers/acpi/nfit.c
> @@ -43,11 +43,11 @@ struct nfit_table_prev {
>         struct list_head flushes;
>  };
>
> -static u8 nfit_uuid[NFIT_UUID_MAX][16];
> +static uuid_le nfit_uuid[NFIT_UUID_MAX];
>
> -const u8 *to_nfit_uuid(enum nfit_uuids id)
> +const uuid_le *to_nfit_uuid(enum nfit_uuids id)
>  {
> -       return nfit_uuid[id];
> +       return &nfit_uuid[id];
>  }
>  EXPORT_SYMBOL(to_nfit_uuid);
>
> @@ -83,7 +83,7 @@ static int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
>         const char *cmd_name, * dimm_name;
>         unsigned long dsm_mask;
>         acpi_handle handle;
> -       const u8 *uuid;
> +       const uuid_le *uuid;
>         u32 offset;
>         int rc, i;
>
> @@ -225,7 +225,7 @@ static int nfit_spa_type(struct acpi_nfit_system_address *spa)
>         int i;
>
>         for (i = 0; i < NFIT_UUID_MAX; i++)
> -               if (memcmp(to_nfit_uuid(i), spa->range_guid, 16) == 0)
> +               if (!uuid_le_cmp(*to_nfit_uuid(i), *(uuid_le *)spa->range_guid))
>                         return i;
>         return -1;
>  }
> @@ -829,7 +829,7 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc,
>  {
>         struct acpi_device *adev, *adev_dimm;
>         struct device *dev = acpi_desc->dev;
> -       const u8 *uuid = to_nfit_uuid(NFIT_DEV_DIMM);
> +       const uuid_le *uuid = to_nfit_uuid(NFIT_DEV_DIMM);
>         int i;
>
>         nfit_mem->dsm_mask = acpi_desc->dimm_dsm_force_en;
> @@ -909,7 +909,7 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
>  static void acpi_nfit_init_dsms(struct acpi_nfit_desc *acpi_desc)
>  {
>         struct nvdimm_bus_descriptor *nd_desc = &acpi_desc->nd_desc;
> -       const u8 *uuid = to_nfit_uuid(NFIT_DEV_BUS);
> +       const uuid_le *uuid = to_nfit_uuid(NFIT_DEV_BUS);
>         struct acpi_device *adev;
>         int i;
>
> @@ -2079,16 +2079,16 @@ static __init int nfit_init(void)
>         BUILD_BUG_ON(sizeof(struct acpi_nfit_control_region) != 80);
>         BUILD_BUG_ON(sizeof(struct acpi_nfit_data_region) != 40);
>
> -       acpi_str_to_uuid(UUID_VOLATILE_MEMORY, nfit_uuid[NFIT_SPA_VOLATILE]);
> -       acpi_str_to_uuid(UUID_PERSISTENT_MEMORY, nfit_uuid[NFIT_SPA_PM]);
> -       acpi_str_to_uuid(UUID_CONTROL_REGION, nfit_uuid[NFIT_SPA_DCR]);
> -       acpi_str_to_uuid(UUID_DATA_REGION, nfit_uuid[NFIT_SPA_BDW]);
> -       acpi_str_to_uuid(UUID_VOLATILE_VIRTUAL_DISK, nfit_uuid[NFIT_SPA_VDISK]);
> -       acpi_str_to_uuid(UUID_VOLATILE_VIRTUAL_CD, nfit_uuid[NFIT_SPA_VCD]);
> -       acpi_str_to_uuid(UUID_PERSISTENT_VIRTUAL_DISK, nfit_uuid[NFIT_SPA_PDISK]);
> -       acpi_str_to_uuid(UUID_PERSISTENT_VIRTUAL_CD, nfit_uuid[NFIT_SPA_PCD]);
> -       acpi_str_to_uuid(UUID_NFIT_BUS, nfit_uuid[NFIT_DEV_BUS]);
> -       acpi_str_to_uuid(UUID_NFIT_DIMM, nfit_uuid[NFIT_DEV_DIMM]);
> +       uuid_le_to_bin(UUID_VOLATILE_MEMORY, &nfit_uuid[NFIT_SPA_VOLATILE]);
> +       uuid_le_to_bin(UUID_PERSISTENT_MEMORY, &nfit_uuid[NFIT_SPA_PM]);
> +       uuid_le_to_bin(UUID_CONTROL_REGION, &nfit_uuid[NFIT_SPA_DCR]);
> +       uuid_le_to_bin(UUID_DATA_REGION, &nfit_uuid[NFIT_SPA_BDW]);
> +       uuid_le_to_bin(UUID_VOLATILE_VIRTUAL_DISK, &nfit_uuid[NFIT_SPA_VDISK]);
> +       uuid_le_to_bin(UUID_VOLATILE_VIRTUAL_CD, &nfit_uuid[NFIT_SPA_VCD]);
> +       uuid_le_to_bin(UUID_PERSISTENT_VIRTUAL_DISK, &nfit_uuid[NFIT_SPA_PDISK]);
> +       uuid_le_to_bin(UUID_PERSISTENT_VIRTUAL_CD, &nfit_uuid[NFIT_SPA_PCD]);
> +       uuid_le_to_bin(UUID_NFIT_BUS, &nfit_uuid[NFIT_DEV_BUS]);
> +       uuid_le_to_bin(UUID_NFIT_DIMM, &nfit_uuid[NFIT_DEV_DIMM]);

I don't see the benefit of this change.  For the NFIT driver we went
through a fire drill trying to make sure the ACPI spec format of a
UUID matched the Linux interpretation, and this change makes that
harder to determine.  ACPI drivers should use ACPICA uuid helper
routines in my opinion.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 06/10] device property: switch to use UUID API
  2016-02-17 12:17 ` [PATCH v1 06/10] device property: switch to use " Andy Shevchenko
@ 2016-02-18  0:03   ` Rafael J. Wysocki
  2016-02-26 14:11     ` Andy Shevchenko
  2016-02-18 11:07   ` Mika Westerberg
  1 sibling, 1 reply; 23+ messages in thread
From: Rafael J. Wysocki @ 2016-02-18  0:03 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Theodore Ts'o, Arnd Bergmann, Greg Kroah-Hartman,
	Jarkko Sakkinen, Jani Nikula, David Airlie, Benjamin Tissoires,
	Bjorn Helgaas, Mathias Nyman, Matt Fleming, Lv Zheng, Mark Brown,
	Zhang Rui, Mika Westerberg, Andrew Morton, Rasmus Villemoes,
	linux-acpi, linux-kernel, dri-devel, linux-efi, linux-api,
	linux-nvdimm

On Wednesday, February 17, 2016 02:17:24 PM Andy Shevchenko wrote:
> Switch to use a generic UUID API instead of custom approach. It allows to
> define UUIDs, compare them, and validate.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/acpi/property.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 2aee416..1e75305 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -25,15 +25,13 @@ static int acpi_data_get_property_array(struct acpi_device_data *data,
>  					const union acpi_object **obj);
>  
>  /* ACPI _DSD device properties UUID: daffd814-6eba-4d8c-8a91-bc9bbf4aa301 */
> -static const u8 prp_uuid[16] = {
> -	0x14, 0xd8, 0xff, 0xda, 0xba, 0x6e, 0x8c, 0x4d,
> -	0x8a, 0x91, 0xbc, 0x9b, 0xbf, 0x4a, 0xa3, 0x01
> -};
> +static const uuid_le prp_uuid =
> +	UUID_LE(0xdaffd814, 0x6eba, 0x4d8c,
> +		0x8a, 0x91, 0xbc, 0x9b, 0xbf, 0x4a, 0xa3, 0x01);
>  /* ACPI _DSD data subnodes UUID: dbb8e3e6-5886-4ba6-8795-1319f52a966b */
> -static const u8 ads_uuid[16] = {
> -	0xe6, 0xe3, 0xb8, 0xdb, 0x86, 0x58, 0xa6, 0x4b,
> -	0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b
> -};
> +static const uuid_le ads_uuid =
> +	UUID_LE(0xdbb8e3e6, 0x5886, 0x4ba6,
> +		0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b);
>  
>  static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
>  					   const union acpi_object *desc,
> @@ -138,7 +136,7 @@ static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
>  		    || links->type != ACPI_TYPE_PACKAGE)
>  			break;
>  
> -		if (memcmp(uuid->buffer.pointer, ads_uuid, sizeof(ads_uuid)))
> +		if (uuid_le_cmp(*(uuid_le *)uuid->buffer.pointer, ads_uuid))

Maybe it's too late, but I don't quite understand the pointer manipulations here.

I can see why you need a type conversion (although it looks ugly), but why do you
need to dereference it too?

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 06/10] device property: switch to use UUID API
  2016-02-17 12:17 ` [PATCH v1 06/10] device property: switch to use " Andy Shevchenko
  2016-02-18  0:03   ` Rafael J. Wysocki
@ 2016-02-18 11:07   ` Mika Westerberg
  2016-02-26 14:28     ` Andy Shevchenko
  1 sibling, 1 reply; 23+ messages in thread
From: Mika Westerberg @ 2016-02-18 11:07 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Andrew Morton, Rasmus Villemoes,
	linux-acpi, linux-kernel, dri-devel, linux-efi, linux-api,
	linux-nvdimm

On Wed, Feb 17, 2016 at 02:17:24PM +0200, Andy Shevchenko wrote:
> Switch to use a generic UUID API instead of custom approach. It allows to
> define UUIDs, compare them, and validate.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/acpi/property.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 2aee416..1e75305 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -25,15 +25,13 @@ static int acpi_data_get_property_array(struct acpi_device_data *data,
>  					const union acpi_object **obj);
>  
>  /* ACPI _DSD device properties UUID: daffd814-6eba-4d8c-8a91-bc9bbf4aa301 */
> -static const u8 prp_uuid[16] = {
> -	0x14, 0xd8, 0xff, 0xda, 0xba, 0x6e, 0x8c, 0x4d,
> -	0x8a, 0x91, 0xbc, 0x9b, 0xbf, 0x4a, 0xa3, 0x01
> -};
> +static const uuid_le prp_uuid =
> +	UUID_LE(0xdaffd814, 0x6eba, 0x4d8c,
> +		0x8a, 0x91, 0xbc, 0x9b, 0xbf, 0x4a, 0xa3, 0x01);
>  /* ACPI _DSD data subnodes UUID: dbb8e3e6-5886-4ba6-8795-1319f52a966b */
> -static const u8 ads_uuid[16] = {
> -	0xe6, 0xe3, 0xb8, 0xdb, 0x86, 0x58, 0xa6, 0x4b,
> -	0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b
> -};
> +static const uuid_le ads_uuid =
> +	UUID_LE(0xdbb8e3e6, 0x5886, 0x4ba6,
> +		0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b);

This looks better than the original but it is still not too readable.

I think it would be better to have uuid as string like:

static const char *ads_uuid = "dbb8e3e6-5886-4ba6-8795-1319f52a966b";

and then convert that to the right byte presentation when first time
used. That would match the uuid format in the ACPI (and device
properties) spec. and thus make it easier to read and understand.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 10/10] efivars: use generic UUID library
  2016-02-17 12:17 ` [PATCH v1 10/10] efivars: use generic UUID library Andy Shevchenko
@ 2016-02-18 15:07   ` Matt Fleming
  2016-02-26 14:29     ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Matt Fleming @ 2016-02-18 15:07 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Lv Zheng,
	Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, linux-kernel, dri-devel, linux-efi,
	linux-api, linux-nvdimm

On Wed, 17 Feb, at 02:17:28PM, Andy Shevchenko wrote:
> Instead of opencoding let's use generic UUID library functions here.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  fs/efivarfs/inode.c | 40 +++-------------------------------------
>  1 file changed, 3 insertions(+), 37 deletions(-)
> 
> diff --git a/fs/efivarfs/inode.c b/fs/efivarfs/inode.c
> index 3381b9d..b579e3a 100644
> --- a/fs/efivarfs/inode.c
> +++ b/fs/efivarfs/inode.c
> @@ -11,6 +11,7 @@
>  #include <linux/fs.h>
>  #include <linux/ctype.h>
>  #include <linux/slab.h>
> +#include <linux/uuid.h>
>  
>  #include "internal.h"
>  
> @@ -44,11 +45,7 @@ struct inode *efivarfs_get_inode(struct super_block *sb,
>   */
>  bool efivarfs_valid_name(const char *str, int len)
>  {
> -	static const char dashes[EFI_VARIABLE_GUID_LEN] = {
> -		[8] = 1, [13] = 1, [18] = 1, [23] = 1
> -	};
>  	const char *s = str + len - EFI_VARIABLE_GUID_LEN;
> -	int i;
>  
>  	/*
>  	 * We need a GUID, plus at least one letter for the variable name,
> @@ -66,37 +63,7 @@ bool efivarfs_valid_name(const char *str, int len)
>  	 *
>  	 *	12345678-1234-1234-1234-123456789abc
>  	 */
> -	for (i = 0; i < EFI_VARIABLE_GUID_LEN; i++) {
> -		if (dashes[i]) {
> -			if (*s++ != '-')
> -				return false;
> -		} else {
> -			if (!isxdigit(*s++))
> -				return false;
> -		}
> -	}
> -
> -	return true;
> -}
> -
> -static void efivarfs_hex_to_guid(const char *str, efi_guid_t *guid)
> -{
> -	guid->b[0] = hex_to_bin(str[6]) << 4 | hex_to_bin(str[7]);
> -	guid->b[1] = hex_to_bin(str[4]) << 4 | hex_to_bin(str[5]);
> -	guid->b[2] = hex_to_bin(str[2]) << 4 | hex_to_bin(str[3]);
> -	guid->b[3] = hex_to_bin(str[0]) << 4 | hex_to_bin(str[1]);
> -	guid->b[4] = hex_to_bin(str[11]) << 4 | hex_to_bin(str[12]);
> -	guid->b[5] = hex_to_bin(str[9]) << 4 | hex_to_bin(str[10]);
> -	guid->b[6] = hex_to_bin(str[16]) << 4 | hex_to_bin(str[17]);
> -	guid->b[7] = hex_to_bin(str[14]) << 4 | hex_to_bin(str[15]);
> -	guid->b[8] = hex_to_bin(str[19]) << 4 | hex_to_bin(str[20]);
> -	guid->b[9] = hex_to_bin(str[21]) << 4 | hex_to_bin(str[22]);
> -	guid->b[10] = hex_to_bin(str[24]) << 4 | hex_to_bin(str[25]);
> -	guid->b[11] = hex_to_bin(str[26]) << 4 | hex_to_bin(str[27]);
> -	guid->b[12] = hex_to_bin(str[28]) << 4 | hex_to_bin(str[29]);
> -	guid->b[13] = hex_to_bin(str[30]) << 4 | hex_to_bin(str[31]);
> -	guid->b[14] = hex_to_bin(str[32]) << 4 | hex_to_bin(str[33]);
> -	guid->b[15] = hex_to_bin(str[34]) << 4 | hex_to_bin(str[35]);
> +	return uuid_is_valid(s);
>  }

I think you've confused yourself here. You've inverted the return
value meaning for efivarfs_valid_name().

Normally I would expect this change to be correct but uuid_is_valid()
returns 0 for success, -EINVAL for failure. Either the function is
misnamed or the return value semantics are wrong.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 05/10] ACPI: switch to use generic UUID API
  2016-02-17 17:49   ` Dan Williams
@ 2016-02-26 13:46     ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-02-26 13:46 UTC (permalink / raw)
  To: Dan Williams
  Cc: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, Linux ACPI, linux-kernel,
	Maling list - DRI developers, linux-efi, linux-api,
	linux-nvdimm@lists.01.org

On Wed, 2016-02-17 at 09:49 -0800, Dan Williams wrote:
> On Wed, Feb 17, 2016 at 4:17 AM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > Instead of opencoding the existing library functions let's use them
> > directly.
> > 
> > The conversion fixes a potential bug in int340x_thermal as well
> > since we have
> > to use memcmp() on binary data.

[..]

> > +       uuid_le_to_bin(UUID_NFIT_BUS, &nfit_uuid[NFIT_DEV_BUS]);
> > +       uuid_le_to_bin(UUID_NFIT_DIMM, &nfit_uuid[NFIT_DEV_DIMM]);
> 
> I don't see the benefit of this change.

The benefit is to have less places (ideally one) which provides UUID
helper routines for kernel.

>   For the NFIT driver we went
> through a fire drill trying to make sure the ACPI spec format of a
> UUID matched the Linux interpretation, and this change makes that
> harder to determine.  

How harder? At least you look into one place and see exactly all
functions related. Besides this series actually uses the same approach
that was used in ACPI.

> ACPI drivers should use ACPICA uuid helper
> routines in my opinion.

As far as I can see ACPICA lacks of needed API.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 06/10] device property: switch to use UUID API
  2016-02-18  0:03   ` Rafael J. Wysocki
@ 2016-02-26 14:11     ` Andy Shevchenko
  2016-04-07 16:41       ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2016-02-26 14:11 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Theodore Ts'o, Arnd Bergmann, Greg Kroah-Hartman,
	Jarkko Sakkinen, Jani Nikula, David Airlie, Benjamin Tissoires,
	Bjorn Helgaas, Mathias Nyman, Matt Fleming, Lv Zheng, Mark Brown,
	Zhang Rui, Mika Westerberg, Andrew Morton, Rasmus Villemoes,
	linux-acpi, linux-kernel, dri-devel, linux-efi, linux-api,
	linux-nvdimm

On Thu, 2016-02-18 at 01:03 +0100, Rafael J. Wysocki wrote:
> On Wednesday, February 17, 2016 02:17:24 PM Andy Shevchenko wrote:
> > Switch to use a generic UUID API instead of custom approach. It
> > allows to
> > define UUIDs, compare them, and validate.

[]

> > +static const uuid_le ads_uuid =
> > +	UUID_LE(0xdbb8e3e6, 0x5886, 0x4ba6,
> > +		0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b);
> >  
> >  static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
> >  					   const union acpi_object
> > *desc,
> > @@ -138,7 +136,7 @@ static bool
> > acpi_enumerate_nondev_subnodes(acpi_handle scope,
> >  		    || links->type != ACPI_TYPE_PACKAGE)
> >  			break;
> >  
> > -		if (memcmp(uuid->buffer.pointer, ads_uuid,
> > sizeof(ads_uuid)))
> > +		if (uuid_le_cmp(*(uuid_le *)uuid->buffer.pointer,
> > ads_uuid))
> 
> Maybe it's too late, but I don't quite understand the pointer
> manipulations here.
> 
> I can see why you need a type conversion (although it looks ugly),
> but why do you
> need to dereference it too?

The function takes that kind of type on input. The other variants are
not compiled.
Perhaps we better change uuid_{lb}e_cmp() first to take normal
pointers, though I think the initial idea was to get type checking at
compile time.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 06/10] device property: switch to use UUID API
  2016-02-18 11:07   ` Mika Westerberg
@ 2016-02-26 14:28     ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-02-26 14:28 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Andrew Morton, Rasmus Villemoes,
	linux-acpi, linux-kernel, dri-devel, linux-efi, linux-api,
	linux-nvdimm

On Thu, 2016-02-18 at 13:07 +0200, Mika Westerberg wrote:
> On Wed, Feb 17, 2016 at 02:17:24PM +0200, Andy Shevchenko wrote:
> > Switch to use a generic UUID API instead of custom approach. It
> > allows to
> > define UUIDs, compare them, and validate.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/acpi/property.c | 18 ++++++++----------
> >  1 file changed, 8 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> > index 2aee416..1e75305 100644
> > --- a/drivers/acpi/property.c
> > +++ b/drivers/acpi/property.c
> > @@ -25,15 +25,13 @@ static int acpi_data_get_property_array(struct
> > acpi_device_data *data,
> >  					const union acpi_object
> > **obj);
> >  
> >  /* ACPI _DSD device properties UUID: daffd814-6eba-4d8c-8a91-
> > bc9bbf4aa301 */
> > -static const u8 prp_uuid[16] = {
> > -	0x14, 0xd8, 0xff, 0xda, 0xba, 0x6e, 0x8c, 0x4d,
> > -	0x8a, 0x91, 0xbc, 0x9b, 0xbf, 0x4a, 0xa3, 0x01
> > -};
> > +static const uuid_le prp_uuid =
> > +	UUID_LE(0xdaffd814, 0x6eba, 0x4d8c,
> > +		0x8a, 0x91, 0xbc, 0x9b, 0xbf, 0x4a, 0xa3, 0x01);
> >  /* ACPI _DSD data subnodes UUID: dbb8e3e6-5886-4ba6-8795-
> > 1319f52a966b */
> > -static const u8 ads_uuid[16] = {
> > -	0xe6, 0xe3, 0xb8, 0xdb, 0x86, 0x58, 0xa6, 0x4b,
> > -	0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b
> > -};
> > +static const uuid_le ads_uuid =
> > +	UUID_LE(0xdbb8e3e6, 0x5886, 0x4ba6,
> > +		0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b);
> 
> This looks better than the original but it is still not too readable.
> 
> I think it would be better to have uuid as string like:
> 
> static const char *ads_uuid = "dbb8e3e6-5886-4ba6-8795-1319f52a966b";
> 
> and then convert that to the right byte presentation when first time
> used. That would match the uuid format in the ACPI (and device
> properties) spec. and thus make it easier to read and understand.

Yes we can do this on slow paths. Or in cases where we know that
conversion happens only once.


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 10/10] efivars: use generic UUID library
  2016-02-18 15:07   ` Matt Fleming
@ 2016-02-26 14:29     ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2016-02-26 14:29 UTC (permalink / raw)
  To: Matt Fleming
  Cc: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Lv Zheng,
	Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, linux-kernel, dri-devel, linux-efi,
	linux-api, linux-nvdimm

On Thu, 2016-02-18 at 15:07 +0000, Matt Fleming wrote:
> On Wed, 17 Feb, at 02:17:28PM, Andy Shevchenko wrote:
> > Instead of opencoding let's use generic UUID library functions
> > here.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  fs/efivarfs/inode.c | 40 +++-------------------------------------
> >  1 file changed, 3 insertions(+), 37 deletions(-)
> > 
> > diff --git a/fs/efivarfs/inode.c b/fs/efivarfs/inode.c
> > index 3381b9d..b579e3a 100644
> > --- a/fs/efivarfs/inode.c
> > +++ b/fs/efivarfs/inode.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/fs.h>
> >  #include <linux/ctype.h>
> >  #include <linux/slab.h>
> > +#include <linux/uuid.h>
> >  
> >  #include "internal.h"
> >  
> > @@ -44,11 +45,7 @@ struct inode *efivarfs_get_inode(struct
> > super_block *sb,
> >   */
> >  bool efivarfs_valid_name(const char *str, int len)
> >  {
> > -	static const char dashes[EFI_VARIABLE_GUID_LEN] = {
> > -		[8] = 1, [13] = 1, [18] = 1, [23] = 1
> > -	};
> >  	const char *s = str + len - EFI_VARIABLE_GUID_LEN;
> > -	int i;
> >  
> >  	/*
> >  	 * We need a GUID, plus at least one letter for the
> > variable name,
> > @@ -66,37 +63,7 @@ bool efivarfs_valid_name(const char *str, int
> > len)
> >  	 *
> >  	 *	12345678-1234-1234-1234-123456789abc
> >  	 */
> > -	for (i = 0; i < EFI_VARIABLE_GUID_LEN; i++) {
> > -		if (dashes[i]) {
> > -			if (*s++ != '-')
> > -				return false;
> > -		} else {
> > -			if (!isxdigit(*s++))
> > -				return false;
> > -		}
> > -	}
> > -
> > -	return true;
> > -}
> > -
> > -static void efivarfs_hex_to_guid(const char *str, efi_guid_t
> > *guid)
> > -{
> > -	guid->b[0] = hex_to_bin(str[6]) << 4 | hex_to_bin(str[7]);
> > -	guid->b[1] = hex_to_bin(str[4]) << 4 | hex_to_bin(str[5]);
> > -	guid->b[2] = hex_to_bin(str[2]) << 4 | hex_to_bin(str[3]);
> > -	guid->b[3] = hex_to_bin(str[0]) << 4 | hex_to_bin(str[1]);
> > -	guid->b[4] = hex_to_bin(str[11]) << 4 |
> > hex_to_bin(str[12]);
> > -	guid->b[5] = hex_to_bin(str[9]) << 4 |
> > hex_to_bin(str[10]);
> > -	guid->b[6] = hex_to_bin(str[16]) << 4 |
> > hex_to_bin(str[17]);
> > -	guid->b[7] = hex_to_bin(str[14]) << 4 |
> > hex_to_bin(str[15]);
> > -	guid->b[8] = hex_to_bin(str[19]) << 4 |
> > hex_to_bin(str[20]);
> > -	guid->b[9] = hex_to_bin(str[21]) << 4 |
> > hex_to_bin(str[22]);
> > -	guid->b[10] = hex_to_bin(str[24]) << 4 |
> > hex_to_bin(str[25]);
> > -	guid->b[11] = hex_to_bin(str[26]) << 4 |
> > hex_to_bin(str[27]);
> > -	guid->b[12] = hex_to_bin(str[28]) << 4 |
> > hex_to_bin(str[29]);
> > -	guid->b[13] = hex_to_bin(str[30]) << 4 |
> > hex_to_bin(str[31]);
> > -	guid->b[14] = hex_to_bin(str[32]) << 4 |
> > hex_to_bin(str[33]);
> > -	guid->b[15] = hex_to_bin(str[34]) << 4 |
> > hex_to_bin(str[35]);
> > +	return uuid_is_valid(s);
> >  }
> 
> I think you've confused yourself here. You've inverted the return
> value meaning for efivarfs_valid_name().
> 
> Normally I would expect this change to be correct but uuid_is_valid()
> returns 0 for success, -EINVAL for failure. Either the function is
> misnamed or the return value semantics are wrong.

Oops, thanks for noticing this. Right the return value should be
aligned.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 06/10] device property: switch to use UUID API
  2016-02-26 14:11     ` Andy Shevchenko
@ 2016-04-07 16:41       ` Andy Shevchenko
  2016-04-08  1:27         ` Huang, Ying
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-07 16:41 UTC (permalink / raw)
  To: Rafael J. Wysocki, Huang Ying
  Cc: Theodore Ts'o, Arnd Bergmann, Greg Kroah-Hartman,
	Jarkko Sakkinen, Jani Nikula, David Airlie, Benjamin Tissoires,
	Bjorn Helgaas, Mathias Nyman, Matt Fleming, Lv Zheng, Mark Brown,
	Zhang Rui, Mika Westerberg, Andrew Morton, Rasmus Villemoes,
	linux-acpi, linux-kernel, dri-devel, linux-efi, linux-api,
	linux-nvdimm

On Fri, 2016-02-26 at 16:11 +0200, Andy Shevchenko wrote:
> On Thu, 2016-02-18 at 01:03 +0100, Rafael J. Wysocki wrote:
> > 
> > On Wednesday, February 17, 2016 02:17:24 PM Andy Shevchenko wrote:
> > > 
> > > Switch to use a generic UUID API instead of custom approach. It
> > > allows to
> > > define UUIDs, compare them, and validate.
> []
> 

Summon initial author of the UUID library.

Summary: the API of comparison functions is rather strange. What the
point to not take pointers directly? (Moreover I hope compiler too
clever not to make a copy of constant arguments there)

I could only imagine the case you are trying to avoid temporary
variables for constants like NULL_UUID.

Issue with this is the ugliness in the users of that, in particularly
present in ACPI (drivers/acpi/apei/ghes.c).

I would like to have more clear interface for that. Perhaps we may add
something like

cmp_p(pointer, non-pointer);
cmp_pp(pointer, pointer);

to not break existing API for now.

It would be useful for many cases in the kernel.

> > 
> > > 
> > > +static const uuid_le ads_uuid =
> > > +	UUID_LE(0xdbb8e3e6, 0x5886, 0x4ba6,
> > > +		0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b);
> > >  
> > >  static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
> > >  					   const union
> > > acpi_object
> > > *desc,
> > > @@ -138,7 +136,7 @@ static bool
> > > acpi_enumerate_nondev_subnodes(acpi_handle scope,
> > >  		    || links->type != ACPI_TYPE_PACKAGE)
> > >  			break;
> > >  
> > > -		if (memcmp(uuid->buffer.pointer, ads_uuid,
> > > sizeof(ads_uuid)))
> > > +		if (uuid_le_cmp(*(uuid_le *)uuid->buffer.pointer,
> > > ads_uuid))
> > Maybe it's too late, but I don't quite understand the pointer
> > manipulations here.
> > 
> > I can see why you need a type conversion (although it looks ugly),
> > but why do you
> > need to dereference it too?
> The function takes that kind of type on input. The other variants are
> not compiled.
> Perhaps we better change uuid_{lb}e_cmp() first to take normal
> pointers, though I think the initial idea was to get type checking at
> compile time.
> 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 06/10] device property: switch to use UUID API
  2016-04-07 16:41       ` Andy Shevchenko
@ 2016-04-08  1:27         ` Huang, Ying
  2016-04-08 10:00           ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Huang, Ying @ 2016-04-08  1:27 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J. Wysocki, Huang Ying, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, linux-kernel, dri-devel, linux-efi,
	linux-api, linux-nvdimm

Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:

> On Fri, 2016-02-26 at 16:11 +0200, Andy Shevchenko wrote:
>> On Thu, 2016-02-18 at 01:03 +0100, Rafael J. Wysocki wrote:
>> > 
>> > On Wednesday, February 17, 2016 02:17:24 PM Andy Shevchenko wrote:
>> > > 
>> > > Switch to use a generic UUID API instead of custom approach. It
>> > > allows to
>> > > define UUIDs, compare them, and validate.
>> []
>> 
>
> Summon initial author of the UUID library.
>
> Summary: the API of comparison functions is rather strange. What the
> point to not take pointers directly? (Moreover I hope compiler too
> clever not to make a copy of constant arguments there)
>
> I could only imagine the case you are trying to avoid temporary
> variables for constants like NULL_UUID.
>
> Issue with this is the ugliness in the users of that, in particularly
> present in ACPI (drivers/acpi/apei/ghes.c).
>
> I would like to have more clear interface for that. Perhaps we may add
> something like
>
> cmp_p(pointer, non-pointer);
> cmp_pp(pointer, pointer);
>
> to not break existing API for now.
>
> It would be useful for many cases in the kernel.

You can take a look at the drivers/acpi/apei/erst.c for uuid_le_cmp
usage.

#define CPER_CREATOR_PSTORE                                             \
        UUID_LE(0x75a574e3, 0x5052, 0x4b29, 0x8a, 0x8e, 0xbe, 0x2c,     \
                0x64, 0x90, 0xb8, 0x9d)

        if (uuid_le_cmp(rcd->hdr.creator_id, CPER_CREATOR_PSTORE) != 0)
                goto skip;

Looks better?

This is the typical use case in mind when I write the uuid.h.

As for uuid_le_cmp usage in drivers/acpi/apei/ghes.c,

		if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
				 CPER_SEC_PLATFORM_MEM)) {

The code looks not good mainly because acpi_hest_generic_data is not
defined with uuid_le in mind.

struct acpi_hest_generic_data {
	u8 section_type[16];
	u32 error_severity;
	u16 revision;
	u8 validation_bits;
	u8 flags;
	u32 error_data_length;
	u8 fru_id[16];
	u8 fru_text[20];
};

If section_type was defined as uuid_le instead of u8[16], the
uuid_le_cmp usage would look better.  So I suggest to use uuid_le/be in
data structure definition in new code if possible.

Best Regards,
Huang, Ying

>> > 
>> > > 
>> > > +static const uuid_le ads_uuid =
>> > > +	UUID_LE(0xdbb8e3e6, 0x5886, 0x4ba6,
>> > > +		0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96, 0x6b);
>> > >  
>> > >  static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
>> > >  					   const union
>> > > acpi_object
>> > > *desc,
>> > > @@ -138,7 +136,7 @@ static bool
>> > > acpi_enumerate_nondev_subnodes(acpi_handle scope,
>> > >  		    || links->type != ACPI_TYPE_PACKAGE)
>> > >  			break;
>> > >  
>> > > -		if (memcmp(uuid->buffer.pointer, ads_uuid,
>> > > sizeof(ads_uuid)))
>> > > +		if (uuid_le_cmp(*(uuid_le *)uuid->buffer.pointer,
>> > > ads_uuid))
>> > Maybe it's too late, but I don't quite understand the pointer
>> > manipulations here.
>> > 
>> > I can see why you need a type conversion (although it looks ugly),
>> > but why do you
>> > need to dereference it too?
>> The function takes that kind of type on input. The other variants are
>> not compiled.
>> Perhaps we better change uuid_{lb}e_cmp() first to take normal
>> pointers, though I think the initial idea was to get type checking at
>> compile time.
>> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 06/10] device property: switch to use UUID API
  2016-04-08  1:27         ` Huang, Ying
@ 2016-04-08 10:00           ` Andy Shevchenko
  2016-04-08 23:46             ` huang ying
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2016-04-08 10:00 UTC (permalink / raw)
  To: Huang, Ying
  Cc: Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, linux-kernel, dri-devel, linux-efi,
	linux-api, linux-nvdimm

On Fri, 2016-04-08 at 09:27 +0800, Huang, Ying wrote:
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:
> 
> > 
> > On Fri, 2016-02-26 at 16:11 +0200, Andy Shevchenko wrote:
> > > 
> > > On Thu, 2016-02-18 at 01:03 +0100, Rafael J. Wysocki wrote:
> > > > 
> > > > 
> > > > On Wednesday, February 17, 2016 02:17:24 PM Andy Shevchenko
> > > > wrote:
> > > > > 
> > > > > 
> > > > > Switch to use a generic UUID API instead of custom approach.
> > > > > It
> > > > > allows to
> > > > > define UUIDs, compare them, and validate.
> > > []
> > > 
> > Summon initial author of the UUID library.
> > 
> > Summary: the API of comparison functions is rather strange. What the
> > point to not take pointers directly? (Moreover I hope compiler too
> > clever not to make a copy of constant arguments there)
> > 
> > I could only imagine the case you are trying to avoid temporary
> > variables for constants like NULL_UUID.
> > 
> > Issue with this is the ugliness in the users of that, in
> > particularly
> > present in ACPI (drivers/acpi/apei/ghes.c).
> > 
> > I would like to have more clear interface for that. Perhaps we may
> > add
> > something like
> > 
> > cmp_p(pointer, non-pointer);
> > cmp_pp(pointer, pointer);
> > 
> > to not break existing API for now.
> > 
> > It would be useful for many cases in the kernel.
> You can take a look at the drivers/acpi/apei/erst.c for uuid_le_cmp
> usage.
> 
> #define
> CPER_CREATOR_PSTORE                                             \
>         UUID_LE(0x75a574e3, 0x5052, 0x4b29, 0x8a, 0x8e, 0xbe,
> 0x2c,     \
>                 0x64, 0x90, 0xb8, 0x9d)
> 
>         if (uuid_le_cmp(rcd->hdr.creator_id, CPER_CREATOR_PSTORE) !=
> 0)
>                 goto skip;
> 
> Looks better?

I don't quite understand the issues with 

if (uuid_le_cmp(&rcd->hdr.creator_id, &CPER_CREATOR_PSTORE) != 0)

or, like I mentioned previously, we may introduce _cmp_p() and use like

if (uuid_le_cmp_p(&rcd->hdr.creator_id, CPER_CREATOR_PSTORE) != 0)

if it looks better (again, I don't know if compiler is going to copy the last argument).

> 
> This is the typical use case in mind when I write the uuid.h.
> 
> As for uuid_le_cmp usage in drivers/acpi/apei/ghes.c,
> 
> 		if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
> 				 CPER_SEC_PLATFORM_MEM)) {

Ditto

if (!uuid_le_cmp_p((uuid_le *)gdata->section_type,
CPER_SEC_PLATFORM_MEM)) {

> 
> The code looks not good mainly because acpi_hest_generic_data is not
> defined with uuid_le in mind.
> 
> struct acpi_hest_generic_data {
> 	u8 section_type[16];
> 	u32 error_severity;
> 	u16 revision;
> 	u8 validation_bits;
> 	u8 flags;
> 	u32 error_data_length;
> 	u8 fru_id[16];
> 	u8 fru_text[20];
> };
> 
> If section_type was defined as uuid_le instead of u8[16], the
> uuid_le_cmp usage would look better.  So I suggest to use uuid_le/be
> in
> data structure definition in new code if possible.

This is understandable for such structures, but we might get a UUID from
a buffer which is pointer to u8. It's not possible to convert to uuid_*
since it's too generic stuff and might require to introduce
ACPI_TYPE_UUID with standardization and all necessary work. Apparently
not the shortest way.

> 
> Best Regards,
> Huang, Ying
> 
> > 
> > > 
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > +static const uuid_le ads_uuid =
> > > > > +	UUID_LE(0xdbb8e3e6, 0x5886, 0x4ba6,
> > > > > +		0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96,
> > > > > 0x6b);
> > > > >  
> > > > >  static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
> > > > >  					   const union
> > > > > acpi_object
> > > > > *desc,
> > > > > @@ -138,7 +136,7 @@ static bool
> > > > > acpi_enumerate_nondev_subnodes(acpi_handle scope,
> > > > >  		    || links->type != ACPI_TYPE_PACKAGE)
> > > > >  			break;
> > > > >  
> > > > > -		if (memcmp(uuid->buffer.pointer, ads_uuid,
> > > > > sizeof(ads_uuid)))
> > > > > +		if (uuid_le_cmp(*(uuid_le *)uuid-
> > > > > >buffer.pointer,
> > > > > ads_uuid))
> > > > Maybe it's too late, but I don't quite understand the pointer
> > > > manipulations here.
> > > > 
> > > > I can see why you need a type conversion (although it looks
> > > > ugly),
> > > > but why do you
> > > > need to dereference it too?
> > > The function takes that kind of type on input. The other variants
> > > are
> > > not compiled.
> > > Perhaps we better change uuid_{lb}e_cmp() first to take normal
> > > pointers, though I think the initial idea was to get type checking
> > > at
> > > compile time.
> > > 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v1 06/10] device property: switch to use UUID API
  2016-04-08 10:00           ` Andy Shevchenko
@ 2016-04-08 23:46             ` huang ying
  0 siblings, 0 replies; 23+ messages in thread
From: huang ying @ 2016-04-08 23:46 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Huang, Ying, Rafael J. Wysocki, Theodore Ts'o, Arnd Bergmann,
	Greg Kroah-Hartman, Jarkko Sakkinen, Jani Nikula, David Airlie,
	Benjamin Tissoires, Bjorn Helgaas, Mathias Nyman, Matt Fleming,
	Lv Zheng, Mark Brown, Zhang Rui, Mika Westerberg, Andrew Morton,
	Rasmus Villemoes, linux-acpi, LKML, dri-devel, linux-efi,
	linux-api, linux-nvdimm

On Fri, Apr 8, 2016 at 6:00 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Fri, 2016-04-08 at 09:27 +0800, Huang, Ying wrote:
>> Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:
>>
>> >
>> > On Fri, 2016-02-26 at 16:11 +0200, Andy Shevchenko wrote:
>> > >
>> > > On Thu, 2016-02-18 at 01:03 +0100, Rafael J. Wysocki wrote:
>> > > >
>> > > >
>> > > > On Wednesday, February 17, 2016 02:17:24 PM Andy Shevchenko
>> > > > wrote:
>> > > > >
>> > > > >
>> > > > > Switch to use a generic UUID API instead of custom approach.
>> > > > > It
>> > > > > allows to
>> > > > > define UUIDs, compare them, and validate.
>> > > []
>> > >
>> > Summon initial author of the UUID library.
>> >
>> > Summary: the API of comparison functions is rather strange. What the
>> > point to not take pointers directly? (Moreover I hope compiler too
>> > clever not to make a copy of constant arguments there)
>> >
>> > I could only imagine the case you are trying to avoid temporary
>> > variables for constants like NULL_UUID.
>> >
>> > Issue with this is the ugliness in the users of that, in
>> > particularly
>> > present in ACPI (drivers/acpi/apei/ghes.c).
>> >
>> > I would like to have more clear interface for that. Perhaps we may
>> > add
>> > something like
>> >
>> > cmp_p(pointer, non-pointer);
>> > cmp_pp(pointer, pointer);
>> >
>> > to not break existing API for now.
>> >
>> > It would be useful for many cases in the kernel.
>> You can take a look at the drivers/acpi/apei/erst.c for uuid_le_cmp
>> usage.
>>
>> #define
>> CPER_CREATOR_PSTORE                                             \
>>         UUID_LE(0x75a574e3, 0x5052, 0x4b29, 0x8a, 0x8e, 0xbe,
>> 0x2c,     \
>>                 0x64, 0x90, 0xb8, 0x9d)
>>
>>         if (uuid_le_cmp(rcd->hdr.creator_id, CPER_CREATOR_PSTORE) !=
>> 0)
>>                 goto skip;
>>
>> Looks better?
>
> I don't quite understand the issues with
>
> if (uuid_le_cmp(&rcd->hdr.creator_id, &CPER_CREATOR_PSTORE) != 0)

I tried to make uuid_le looks like a primitive data type and UUID
constant looks like primitive type constants if possible.  If we can
define data as uuid_le/be, then it will look just like that.  But if
there are too many places we cannot use uuid_le/be directly, I am OK
to convert the interface to use pointer instead.

> or, like I mentioned previously, we may introduce _cmp_p() and use like
>
> if (uuid_le_cmp_p(&rcd->hdr.creator_id, CPER_CREATOR_PSTORE) != 0)

Personally, I don't like this interface. It is better for two
parameters to have same data type.

> if it looks better (again, I don't know if compiler is going to copy the last argument).
>
>>
>> This is the typical use case in mind when I write the uuid.h.
>>
>> As for uuid_le_cmp usage in drivers/acpi/apei/ghes.c,
>>
>>               if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
>>                                CPER_SEC_PLATFORM_MEM)) {
>
> Ditto
>
> if (!uuid_le_cmp_p((uuid_le *)gdata->section_type,
> CPER_SEC_PLATFORM_MEM)) {
>
>>
>> The code looks not good mainly because acpi_hest_generic_data is not
>> defined with uuid_le in mind.
>>
>> struct acpi_hest_generic_data {
>>       u8 section_type[16];
>>       u32 error_severity;
>>       u16 revision;
>>       u8 validation_bits;
>>       u8 flags;
>>       u32 error_data_length;
>>       u8 fru_id[16];
>>       u8 fru_text[20];
>> };
>>
>> If section_type was defined as uuid_le instead of u8[16], the
>> uuid_le_cmp usage would look better.  So I suggest to use uuid_le/be
>> in
>> data structure definition in new code if possible.
>
> This is understandable for such structures, but we might get a UUID from
> a buffer which is pointer to u8. It's not possible to convert to uuid_*
> since it's too generic stuff and might require to introduce
> ACPI_TYPE_UUID with standardization and all necessary work. Apparently
> not the shortest way.

If this is just a special case that happens seldom, we can just work
around it with *(uuid_le/be *)buf.  If it is common, we can change the
interface or add a new interface.

Best Regards,
Huang, YIng

>> >
>> > >
>> > > >
>> > > >
>> > > > >
>> > > > >
>> > > > > +static const uuid_le ads_uuid =
>> > > > > +     UUID_LE(0xdbb8e3e6, 0x5886, 0x4ba6,
>> > > > > +             0x87, 0x95, 0x13, 0x19, 0xf5, 0x2a, 0x96,
>> > > > > 0x6b);
>> > > > >
>> > > > >  static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
>> > > > >                                          const union
>> > > > > acpi_object
>> > > > > *desc,
>> > > > > @@ -138,7 +136,7 @@ static bool
>> > > > > acpi_enumerate_nondev_subnodes(acpi_handle scope,
>> > > > >                   || links->type != ACPI_TYPE_PACKAGE)
>> > > > >                       break;
>> > > > >
>> > > > > -             if (memcmp(uuid->buffer.pointer, ads_uuid,
>> > > > > sizeof(ads_uuid)))
>> > > > > +             if (uuid_le_cmp(*(uuid_le *)uuid-
>> > > > > >buffer.pointer,
>> > > > > ads_uuid))
>> > > > Maybe it's too late, but I don't quite understand the pointer
>> > > > manipulations here.
>> > > >
>> > > > I can see why you need a type conversion (although it looks
>> > > > ugly),
>> > > > but why do you
>> > > > need to dereference it too?
>> > > The function takes that kind of type on input. The other variants
>> > > are
>> > > not compiled.
>> > > Perhaps we better change uuid_{lb}e_cmp() first to take normal
>> > > pointers, though I think the initial idea was to get type checking
>> > > at
>> > > compile time.
>> > >
>
> --
> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Intel Finland Oy
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2016-04-08 23:46 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-17 12:17 [PATCH v1 00/10] uuid: convert users to generic UUID API Andy Shevchenko
2016-02-17 12:17 ` [PATCH v1 01/10] lib/vsprintf: simplify UUID printing Andy Shevchenko
2016-02-17 12:17 ` [PATCH v1 02/10] lib/uuid: move generate_random_uuid() to uuid.c Andy Shevchenko
2016-02-17 12:17 ` [PATCH v1 03/10] lib/uuid: introduce few more generic helpers for UUID Andy Shevchenko
2016-02-17 12:17 ` [PATCH v1 04/10] lib/uuid: remove FSF address Andy Shevchenko
2016-02-17 12:17 ` [PATCH v1 05/10] ACPI: switch to use generic UUID API Andy Shevchenko
2016-02-17 17:49   ` Dan Williams
2016-02-26 13:46     ` Andy Shevchenko
2016-02-17 12:17 ` [PATCH v1 06/10] device property: switch to use " Andy Shevchenko
2016-02-18  0:03   ` Rafael J. Wysocki
2016-02-26 14:11     ` Andy Shevchenko
2016-04-07 16:41       ` Andy Shevchenko
2016-04-08  1:27         ` Huang, Ying
2016-04-08 10:00           ` Andy Shevchenko
2016-04-08 23:46             ` huang ying
2016-02-18 11:07   ` Mika Westerberg
2016-02-26 14:28     ` Andy Shevchenko
2016-02-17 12:17 ` [PATCH v1 07/10] sysctl: drop away useless label Andy Shevchenko
2016-02-17 12:17 ` [PATCH v1 08/10] sysctl: use generic UUID library Andy Shevchenko
2016-02-17 12:17 ` [PATCH v1 09/10] efi: redefine type, constant, macro from generic code Andy Shevchenko
2016-02-17 12:17 ` [PATCH v1 10/10] efivars: use generic UUID library Andy Shevchenko
2016-02-18 15:07   ` Matt Fleming
2016-02-26 14:29     ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).