linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 00/12] Add ACPI _DSD and unified device properties support
@ 2014-10-21 21:08 Rafael J. Wysocki
  2014-10-21 21:09 ` [PATCH v6 01/12] ACPI: Add support for device specific properties Rafael J. Wysocki
                   ` (12 more replies)
  0 siblings, 13 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-10-21 21:08 UTC (permalink / raw)
  To: Linux Kernel Mailing List
  Cc: ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Mika Westerberg, Aaron Lu, Grant Likely, Arnd Bergmann,
	devicetree, Linus Walleij, Alexandre Courbot, Dmitry Torokhov,
	Bryan Wu

Hi Everyone,

This is version 6 of the unified device properties interface patchset.

The original cover letter from Mika is here:

http://marc.info/?l=devicetree&m=141087052200600&w=4

and my cover letters for previous iterations are at:

http://marc.info/?l=linux-acpi&m=141212903816560&w=4
http://marc.info/?l=linux-kernel&m=141354745011569&w=4

There are a few changes with respect to v5 and the affected patches are
[02-03/12] and [09-12/12].  The remaining ones have not been modified.

Most importantly, requesting the first element of a list (package) property
from _DSD is now equivalent to accessing a single-value property of the
same type, so device_property_read_u8(dev, pname, val) will now be equivalent
to device_property_read_u8_array(dev, pname, val, 1), for example.
Consequently, this _DSD definition:

Name (_DSD, Package () {
    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
    Package () {
        Package () {"blah", "A string"},
    }
})

can be used instead of

Name (_DSD, Package () {
    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
    Package () {
        Package () {"blah", Package () {"A string"}},
    }
})

and the code will be able to retrieve the property value from the both of
them just fine.

This means, among other things, that accessors for single-value properties
can be implemented in terms of the analogous "array" property accessors
which allows the code size to be reduced somewhat.

Patches [02/12] and [09/12] have been modified to achieve that and patch
[03/12] have been modified accordingly for the "compatible" property in
_DSD to behave in an analogous way.  Additionally, the bodies of the
numerical property accessors in patches [02/12] and [09/12] are now
generated using macros (string property accessors have slightly different
rules and are simply open coded for that reason).

Patch [10/12] has been modified to drop function arguments that happened to
have the same values for both of the current users of those functions and
patches [11-12/12] have been modified to take that change into account.  If
the code in question needs to be made more complex in the future, there
should not be any problems with that.

Due to the nature of the changes I have retained all ACKs except for the
Grant's Reviewed-by on patch [03/12] (if that had been Acked-by, I would have
retained it too, but that didn't feel appropriate for the "reviewed by" thing
to me).  If any of you think that the ACKs are not applicable any more, please
let me know and I'll drop them.

Finally, many thanks to Mika for testing the series on MinnowBoard 1 and
MinnowBoard Max.  In case anybody else would like to test it, it is available
from the device-properties branch of the linux-pm.git tree:

git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git device-properties

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* [PATCH v6 01/12] ACPI: Add support for device specific properties
  2014-10-21 21:08 [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
@ 2014-10-21 21:09 ` Rafael J. Wysocki
  2014-10-21 21:15 ` [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware Rafael J. Wysocki
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-10-21 21:09 UTC (permalink / raw)
  To: Linux Kernel Mailing List
  Cc: ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Mika Westerberg, Aaron Lu, Grant Likely, Arnd Bergmann,
	devicetree, Linus Walleij, Alexandre Courbot, Dmitry Torokhov,
	Bryan Wu

From: Mika Westerberg <mika.westerberg@linux.intel.com>

Device Tree is used in many embedded systems to describe the system
configuration to the OS. It supports attaching properties or name-value
pairs to the devices it describe. With these properties one can pass
additional information to the drivers that would not be available
otherwise.

ACPI is another configuration mechanism (among other things) typically
seen, but not limited to, x86 machines. ACPI allows passing arbitrary
data from methods but there has not been mechanism equivalent to Device
Tree until the introduction of _DSD in the recent publication of the
ACPI 5.1 specification.

In order to facilitate ACPI usage in systems where Device Tree is
typically used, it would be beneficial to standardize a way to retrieve
Device Tree style properties from ACPI devices, which is what we do in
this patch.

If a given device described in ACPI namespace wants to export properties it
must implement _DSD method (Device Specific Data, introduced with ACPI 5.1)
that returns the properties in a package of packages. For example:

	Name (_DSD, Package () {
		ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
		Package () {
			Package () {"name1", <VALUE1>},
			Package () {"name2", <VALUE2>},
			...
		}
	})

The UUID reserved for properties is daffd814-6eba-4d8c-8a91-bc9bbf4aa301
and is documented in the ACPI 5.1 companion document called "_DSD
Implementation Guide" [1], [2].

We add several helper functions that can be used to extract these
properties and convert them to different Linux data types.

The ultimate goal is that we only have one device property API that
retrieves the requested properties from Device Tree or from ACPI
transparent to the caller.

[1] http://www.uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel.htm
[2] http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
Reviewed-by: Grant Likely <grant.likely@linaro.org>
Signed-off-by: Darren Hart <dvhart@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/Makefile   |    1 
 drivers/acpi/internal.h |    6 
 drivers/acpi/property.c |  364 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/scan.c     |    2 
 include/acpi/acpi_bus.h |    7 
 include/linux/acpi.h    |   40 +++++
 6 files changed, 420 insertions(+)
 create mode 100644 drivers/acpi/property.c

Index: linux-pm/drivers/acpi/Makefile
===================================================================
--- linux-pm.orig/drivers/acpi/Makefile
+++ linux-pm/drivers/acpi/Makefile
@@ -46,6 +46,7 @@ acpi-y				+= acpi_pnp.o
 acpi-y				+= power.o
 acpi-y				+= event.o
 acpi-y				+= sysfs.o
+acpi-y				+= property.o
 acpi-$(CONFIG_X86)		+= acpi_cmos_rtc.o
 acpi-$(CONFIG_DEBUG_FS)		+= debugfs.o
 acpi-$(CONFIG_ACPI_NUMA)	+= numa.o
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -181,4 +181,10 @@ struct platform_device *acpi_create_plat
 bool acpi_osi_is_win8(void);
 #endif
 
+/*--------------------------------------------------------------------------
+				Device properties
+  -------------------------------------------------------------------------- */
+void acpi_init_properties(struct acpi_device *adev);
+void acpi_free_properties(struct acpi_device *adev);
+
 #endif /* _ACPI_INTERNAL_H_ */
Index: linux-pm/drivers/acpi/property.c
===================================================================
--- /dev/null
+++ linux-pm/drivers/acpi/property.c
@@ -0,0 +1,364 @@
+/*
+ * ACPI device specific properties support.
+ *
+ * Copyright (C) 2014, Intel Corporation
+ * All rights reserved.
+ *
+ * Authors: Mika Westerberg <mika.westerberg@linux.intel.com>
+ *          Darren Hart <dvhart@linux.intel.com>
+ *          Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/export.h>
+
+#include "internal.h"
+
+/* 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 bool acpi_property_value_ok(const union acpi_object *value)
+{
+	int j;
+
+	/*
+	 * The value must be an integer, a string, a reference, or a package
+	 * whose every element must be an integer, a string, or a reference.
+	 */
+	switch (value->type) {
+	case ACPI_TYPE_INTEGER:
+	case ACPI_TYPE_STRING:
+	case ACPI_TYPE_LOCAL_REFERENCE:
+		return true;
+
+	case ACPI_TYPE_PACKAGE:
+		for (j = 0; j < value->package.count; j++)
+			switch (value->package.elements[j].type) {
+			case ACPI_TYPE_INTEGER:
+			case ACPI_TYPE_STRING:
+			case ACPI_TYPE_LOCAL_REFERENCE:
+				continue;
+
+			default:
+				return false;
+			}
+
+		return true;
+	}
+	return false;
+}
+
+static bool acpi_properties_format_valid(const union acpi_object *properties)
+{
+	int i;
+
+	for (i = 0; i < properties->package.count; i++) {
+		const union acpi_object *property;
+
+		property = &properties->package.elements[i];
+		/*
+		 * Only two elements allowed, the first one must be a string and
+		 * the second one has to satisfy certain conditions.
+		 */
+		if (property->package.count != 2
+		    || property->package.elements[0].type != ACPI_TYPE_STRING
+		    || !acpi_property_value_ok(&property->package.elements[1]))
+			return false;
+	}
+	return true;
+}
+
+void acpi_init_properties(struct acpi_device *adev)
+{
+	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
+	const union acpi_object *desc;
+	acpi_status status;
+	int i;
+
+	status = acpi_evaluate_object_typed(adev->handle, "_DSD", NULL, &buf,
+					    ACPI_TYPE_PACKAGE);
+	if (ACPI_FAILURE(status))
+		return;
+
+	desc = buf.pointer;
+	if (desc->package.count % 2)
+		goto fail;
+
+	/* Look for the device properties UUID. */
+	for (i = 0; i < desc->package.count; i += 2) {
+		const union acpi_object *uuid, *properties;
+
+		uuid = &desc->package.elements[i];
+		properties = &desc->package.elements[i + 1];
+
+		/*
+		 * The first element must be a UUID and the second one must be
+		 * a package.
+		 */
+		if (uuid->type != ACPI_TYPE_BUFFER || uuid->buffer.length != 16
+		    || properties->type != ACPI_TYPE_PACKAGE)
+			break;
+
+		if (memcmp(uuid->buffer.pointer, prp_uuid, sizeof(prp_uuid)))
+			continue;
+
+		/*
+		 * We found the matching UUID. Now validate the format of the
+		 * package immediately following it.
+		 */
+		if (!acpi_properties_format_valid(properties))
+			break;
+
+		adev->data.pointer = buf.pointer;
+		adev->data.properties = properties;
+		return;
+	}
+
+ fail:
+	dev_warn(&adev->dev, "Returned _DSD data is not valid, skipping\n");
+	ACPI_FREE(buf.pointer);
+}
+
+void acpi_free_properties(struct acpi_device *adev)
+{
+	ACPI_FREE((void *)adev->data.pointer);
+	adev->data.pointer = NULL;
+	adev->data.properties = NULL;
+}
+
+/**
+ * acpi_dev_get_property - return an ACPI property with given name
+ * @adev: ACPI device to get property
+ * @name: Name of the property
+ * @type: Expected property type
+ * @obj: Location to store the property value (if not %NULL)
+ *
+ * Look up a property with @name and store a pointer to the resulting ACPI
+ * object at the location pointed to by @obj if found.
+ *
+ * Callers must not attempt to free the returned objects.  These objects will be
+ * freed by the ACPI core automatically during the removal of @adev.
+ *
+ * Return: %0 if property with @name has been found (success),
+ *         %-EINVAL if the arguments are invalid,
+ *         %-ENODATA if the property doesn't exist,
+ *         %-EPROTO if the property value type doesn't match @type.
+ */
+int acpi_dev_get_property(struct acpi_device *adev, const char *name,
+			  acpi_object_type type, const union acpi_object **obj)
+{
+	const union acpi_object *properties;
+	int i;
+
+	if (!adev || !name)
+		return -EINVAL;
+
+	if (!adev->data.pointer || !adev->data.properties)
+		return -ENODATA;
+
+	properties = adev->data.properties;
+	for (i = 0; i < properties->package.count; i++) {
+		const union acpi_object *propname, *propvalue;
+		const union acpi_object *property;
+
+		property = &properties->package.elements[i];
+
+		propname = &property->package.elements[0];
+		propvalue = &property->package.elements[1];
+
+		if (!strcmp(name, propname->string.pointer)) {
+			if (type != ACPI_TYPE_ANY && propvalue->type != type)
+				return -EPROTO;
+			else if (obj)
+				*obj = propvalue;
+
+			return 0;
+		}
+	}
+	return -ENODATA;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_get_property);
+
+/**
+ * acpi_dev_get_property_array - return an ACPI array property with given name
+ * @adev: ACPI device to get property
+ * @name: Name of the property
+ * @type: Expected type of array elements
+ * @obj: Location to store a pointer to the property value (if not NULL)
+ *
+ * Look up an array property with @name and store a pointer to the resulting
+ * ACPI object at the location pointed to by @obj if found.
+ *
+ * Callers must not attempt to free the returned objects.  Those objects will be
+ * freed by the ACPI core automatically during the removal of @adev.
+ *
+ * Return: %0 if array property (package) with @name has been found (success),
+ *         %-EINVAL if the arguments are invalid,
+ *         %-ENODATA if the property doesn't exist,
+ *         %-EPROTO if the property is not a package or the type of its elements
+ *           doesn't match @type.
+ */
+int acpi_dev_get_property_array(struct acpi_device *adev, const char *name,
+				acpi_object_type type,
+				const union acpi_object **obj)
+{
+	const union acpi_object *prop;
+	int ret, i;
+
+	ret = acpi_dev_get_property(adev, name, ACPI_TYPE_PACKAGE, &prop);
+	if (ret)
+		return ret;
+
+	if (type != ACPI_TYPE_ANY) {
+		/* Check that all elements are of correct type. */
+		for (i = 0; i < prop->package.count; i++)
+			if (prop->package.elements[i].type != type)
+				return -EPROTO;
+	}
+	if (obj)
+		*obj = prop;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_get_property_array);
+
+/**
+ * acpi_dev_get_property_reference - returns handle to the referenced object
+ * @adev: ACPI device to get property
+ * @name: Name of the property
+ * @size_prop: Name of the "size" property in referenced object
+ * @index: Index of the reference to return
+ * @args: Location to store the returned reference with optional arguments
+ *
+ * Find property with @name, verifify that it is a package containing at least
+ * one object reference and if so, store the ACPI device object pointer to the
+ * target object in @args->adev.
+ *
+ * If the reference includes arguments (@size_prop is not %NULL) follow the
+ * reference and check whether or not there is an integer property @size_prop
+ * under the target object and if so, whether or not its value matches the
+ * number of arguments that follow the reference.  If there's more than one
+ * reference in the property value package, @index is used to select the one to
+ * return.
+ *
+ * Return: %0 on success, negative error code on failure.
+ */
+int acpi_dev_get_property_reference(struct acpi_device *adev, const char *name,
+				    const char *size_prop, size_t index,
+				    struct acpi_reference_args *args)
+{
+	const union acpi_object *element, *end;
+	const union acpi_object *obj;
+	struct acpi_device *device;
+	int ret, idx = 0;
+
+	ret = acpi_dev_get_property(adev, name, ACPI_TYPE_ANY, &obj);
+	if (ret)
+		return ret;
+
+	/*
+	 * The simplest case is when the value is a single reference.  Just
+	 * return that reference then.
+	 */
+	if (obj->type == ACPI_TYPE_LOCAL_REFERENCE) {
+		if (size_prop || index)
+			return -EINVAL;
+
+		ret = acpi_bus_get_device(obj->reference.handle, &device);
+		if (ret)
+			return ret;
+
+		args->adev = device;
+		args->nargs = 0;
+		return 0;
+	}
+
+	/*
+	 * If it is not a single reference, then it is a package of
+	 * references followed by number of ints as follows:
+	 *
+	 *  Package () { REF, INT, REF, INT, INT }
+	 *
+	 * The index argument is then used to determine which reference
+	 * the caller wants (along with the arguments).
+	 */
+	if (obj->type != ACPI_TYPE_PACKAGE || index >= obj->package.count)
+		return -EPROTO;
+
+	element = obj->package.elements;
+	end = element + obj->package.count;
+
+	while (element < end) {
+		u32 nargs, i;
+
+		if (element->type != ACPI_TYPE_LOCAL_REFERENCE)
+			return -EPROTO;
+
+		ret = acpi_bus_get_device(element->reference.handle, &device);
+		if (ret)
+			return -ENODEV;
+
+		element++;
+		nargs = 0;
+
+		if (size_prop) {
+			const union acpi_object *prop;
+
+			/*
+			 * Find out how many arguments the refenced object
+			 * expects by reading its size_prop property.
+			 */
+			ret = acpi_dev_get_property(device, size_prop,
+						    ACPI_TYPE_INTEGER, &prop);
+			if (ret)
+				return ret;
+
+			nargs = prop->integer.value;
+			if (nargs > MAX_ACPI_REFERENCE_ARGS
+			    || element + nargs > end)
+				return -EPROTO;
+
+			/*
+			 * Skip to the start of the arguments and verify
+			 * that they all are in fact integers.
+			 */
+			for (i = 0; i < nargs; i++)
+				if (element[i].type != ACPI_TYPE_INTEGER)
+					return -EPROTO;
+		} else {
+			/* assume following integer elements are all args */
+			for (i = 0; element + i < end; i++) {
+				int type = element[i].type;
+
+				if (type == ACPI_TYPE_INTEGER)
+					nargs++;
+				else if (type == ACPI_TYPE_LOCAL_REFERENCE)
+					break;
+				else
+					return -EPROTO;
+			}
+		}
+
+		if (idx++ == index) {
+			args->adev = device;
+			args->nargs = nargs;
+			for (i = 0; i < nargs; i++)
+				args->args[i] = element[i].integer.value;
+
+			return 0;
+		}
+
+		element += nargs;
+	}
+
+	return -EPROTO;
+}
+EXPORT_SYMBOL_GPL(acpi_dev_get_property_reference);
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -884,6 +884,7 @@ static void acpi_device_release(struct d
 {
 	struct acpi_device *acpi_dev = to_acpi_device(dev);
 
+	acpi_free_properties(acpi_dev);
 	acpi_free_pnp_ids(&acpi_dev->pnp);
 	acpi_free_power_resources_lists(acpi_dev);
 	kfree(acpi_dev);
@@ -1888,6 +1889,7 @@ void acpi_init_device_object(struct acpi
 	acpi_set_device_status(device, sta);
 	acpi_device_get_busid(device);
 	acpi_set_pnp_ids(handle, &device->pnp, type);
+	acpi_init_properties(device);
 	acpi_bus_get_flags(device);
 	device->flags.match_driver = false;
 	device->flags.initialized = true;
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -337,6 +337,12 @@ struct acpi_device_physical_node {
 	bool put_online:1;
 };
 
+/* ACPI Device Specific Data (_DSD) */
+struct acpi_device_data {
+	const union acpi_object *pointer;
+	const union acpi_object *properties;
+};
+
 /* Device */
 struct acpi_device {
 	int device_type;
@@ -353,6 +359,7 @@ struct acpi_device {
 	struct acpi_device_wakeup wakeup;
 	struct acpi_device_perf performance;
 	struct acpi_device_dir dir;
+	struct acpi_device_data data;
 	struct acpi_scan_handler *handler;
 	struct acpi_hotplug_context *hp;
 	struct acpi_driver *driver;
Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -658,4 +658,44 @@ do {									\
 #endif
 #endif
 
+/* Device properties */
+
+#define MAX_ACPI_REFERENCE_ARGS	8
+struct acpi_reference_args {
+	struct acpi_device *adev;
+	size_t nargs;
+	u64 args[MAX_ACPI_REFERENCE_ARGS];
+};
+
+#ifdef CONFIG_ACPI
+int acpi_dev_get_property(struct acpi_device *adev, const char *name,
+			  acpi_object_type type, const union acpi_object **obj);
+int acpi_dev_get_property_array(struct acpi_device *adev, const char *name,
+				acpi_object_type type,
+				const union acpi_object **obj);
+int acpi_dev_get_property_reference(struct acpi_device *adev, const char *name,
+				    const char *cells_name, size_t index,
+				    struct acpi_reference_args *args);
+#else
+static inline int acpi_dev_get_property(struct acpi_device *adev,
+					const char *name, acpi_object_type type,
+					const union acpi_object **obj)
+{
+	return -ENXIO;
+}
+static inline int acpi_dev_get_property_array(struct acpi_device *adev,
+					      const char *name,
+					      acpi_object_type type,
+					      const union acpi_object **obj)
+{
+	return -ENXIO;
+}
+static inline int acpi_dev_get_property_reference(struct acpi_device *adev,
+				const char *name, const char *cells_name,
+				size_t index, struct acpi_reference_args *args)
+{
+	return -ENXIO;
+}
+#endif
+
 #endif	/*_LINUX_ACPI_H*/


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

* [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware
  2014-10-21 21:08 [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
  2014-10-21 21:09 ` [PATCH v6 01/12] ACPI: Add support for device specific properties Rafael J. Wysocki
@ 2014-10-21 21:15 ` Rafael J. Wysocki
  2014-11-03 15:40   ` Grant Likely
                     ` (2 more replies)
  2014-10-21 21:19 ` [PATCH v6 03/12] ACPI: Allow drivers to match using Device Tree compatible property Rafael J. Wysocki
                   ` (10 subsequent siblings)
  12 siblings, 3 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-10-21 21:15 UTC (permalink / raw)
  To: Linux Kernel Mailing List, Grant Likely, Arnd Bergmann
  Cc: ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Mika Westerberg, Aaron Lu, devicetree, Linus Walleij,
	Alexandre Courbot, Dmitry Torokhov, Bryan Wu

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Add a uniform interface by which device drivers can request device
properties from the platform firmware by providing a property name
and the corresponding data type.  The purpose of it is to help to
write portable code that won't depend on any particular platform
firmware interface.

The following general helper functions are added:

device_property_present()
device_property_read_u8()
device_property_read_u16()
device_property_read_u32()
device_property_read_u64()
device_property_read_string()
device_property_read_u8_array()
device_property_read_u16_array()
device_property_read_u32_array()
device_property_read_u64_array()
device_property_read_string_array()

The first one allows the caller to check if the given property is
present.  The next 5 of them allow single-valued properties of
various types to be retrieved in a uniform way.  The remaining 5 are
for reading properties with multiple values (arrays of either numbers
or strings).

The interface covers both ACPI and Device Trees.

This change set includes material from Mika Westerberg and Aaron Lu.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Changes from v5:
- acpi_dev_prop_read() can now handle both list (package) and single-value
  properties (the latter are tried first for the last argument equal to 1).
- There is a new macro to generate the bodies of device_property_read_u*_array().
- device_property_read_u*() are implemented using device_property_read_u*_array().
- device_property_read_bool() is a new static inline wrapper around
  device_property_present().

---
 drivers/acpi/property.c  |  178 +++++++++++++++++++++++++++++++++++
 drivers/base/Makefile    |    2 
 drivers/base/property.c  |  233 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/of/base.c        |  106 ++++++++++++++++++---
 include/linux/acpi.h     |   32 ++++++
 include/linux/of.h       |   22 ++++
 include/linux/property.h |   53 ++++++++++
 7 files changed, 609 insertions(+), 17 deletions(-)
 create mode 100644 drivers/base/property.c
 create mode 100644 include/linux/property.h

Index: linux-pm/include/linux/property.h
===================================================================
--- /dev/null
+++ linux-pm/include/linux/property.h
@@ -0,0 +1,53 @@
+/*
+ * property.h - Unified device property interface.
+ *
+ * Copyright (C) 2014, Intel Corporation
+ * Authors: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+ *          Mika Westerberg <mika.westerberg@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _LINUX_PROPERTY_H_
+#define _LINUX_PROPERTY_H_
+
+#include <linux/types.h>
+
+struct device;
+
+enum dev_prop_type {
+	DEV_PROP_U8,
+	DEV_PROP_U16,
+	DEV_PROP_U32,
+	DEV_PROP_U64,
+	DEV_PROP_STRING,
+	DEV_PROP_MAX,
+};
+
+bool device_property_present(struct device *dev, const char *propname);
+int device_property_read_u8_array(struct device *dev, const char *propname,
+				  u8 *val, size_t nval);
+int device_property_read_u16_array(struct device *dev, const char *propname,
+				   u16 *val, size_t nval);
+int device_property_read_u32_array(struct device *dev, const char *propname,
+				   u32 *val, size_t nval);
+int device_property_read_u64_array(struct device *dev, const char *propname,
+				   u64 *val, size_t nval);
+int device_property_read_string_array(struct device *dev, const char *propname,
+				      char **val, size_t nval);
+int device_property_read_u8(struct device *dev, const char *propname, u8 *val);
+int device_property_read_u16(struct device *dev, const char *propname, u16 *val);
+int device_property_read_u32(struct device *dev, const char *propname, u32 *val);
+int device_property_read_u64(struct device *dev, const char *propname, u64 *val);
+int device_property_read_string(struct device *dev, const char *propname,
+				const char **val);
+
+static inline bool device_property_read_bool(struct device *dev,
+					     const char *propname)
+{
+	return device_property_present(dev, propname);
+}
+
+#endif /* _LINUX_PROPERTY_H_ */
Index: linux-pm/include/linux/of.h
===================================================================
--- linux-pm.orig/include/linux/of.h
+++ linux-pm/include/linux/of.h
@@ -23,6 +23,7 @@
 #include <linux/spinlock.h>
 #include <linux/topology.h>
 #include <linux/notifier.h>
+#include <linux/property.h>
 
 #include <asm/byteorder.h>
 #include <asm/errno.h>
@@ -263,6 +264,10 @@ extern int of_property_read_u32_array(co
 				      size_t sz);
 extern int of_property_read_u64(const struct device_node *np,
 				const char *propname, u64 *out_value);
+extern int of_property_read_u64_array(const struct device_node *np,
+				      const char *propname,
+				      u64 *out_values,
+				      size_t sz);
 
 extern int of_property_read_string(struct device_node *np,
 				   const char *propname,
@@ -275,6 +280,9 @@ extern int of_property_match_string(stru
 				    const char *string);
 extern int of_property_count_strings(struct device_node *np,
 				     const char *propname);
+extern int of_property_read_string_array(struct device_node *np,
+					      const char *propname,
+					      char **out_strs, size_t sz);
 extern int of_device_is_compatible(const struct device_node *device,
 				   const char *);
 extern int of_device_is_available(const struct device_node *device);
@@ -479,6 +487,13 @@ static inline int of_property_read_u32_a
 	return -ENOSYS;
 }
 
+static inline int of_property_read_u64_array(const struct device_node *np,
+					     const char *propname,
+					     u64 *out_values, size_t sz)
+{
+	return -ENOSYS;
+}
+
 static inline int of_property_read_string(struct device_node *np,
 					  const char *propname,
 					  const char **out_string)
@@ -498,6 +513,13 @@ static inline int of_property_count_stri
 {
 	return -ENOSYS;
 }
+
+static inline int of_property_read_string_array(struct device_node *np,
+						 const char *propname,
+						 char **out_strs, size_t sz)
+{
+	return -ENOSYS;
+}
 
 static inline const void *of_get_property(const struct device_node *node,
 				const char *name,
Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -28,6 +28,7 @@
 #include <linux/errno.h>
 #include <linux/ioport.h>	/* for struct resource */
 #include <linux/device.h>
+#include <linux/property.h>
 
 #ifndef _LINUX
 #define _LINUX
@@ -676,6 +677,13 @@ int acpi_dev_get_property_array(struct a
 int acpi_dev_get_property_reference(struct acpi_device *adev, const char *name,
 				    const char *cells_name, size_t index,
 				    struct acpi_reference_args *args);
+
+int acpi_dev_prop_get(struct acpi_device *adev, const char *propname,
+		      void **valptr);
+int acpi_dev_prop_read_single(struct acpi_device *adev, const char *propname,
+			      enum dev_prop_type proptype, void *val);
+int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
+		       enum dev_prop_type proptype, void *val, size_t nval);
 #else
 static inline int acpi_dev_get_property(struct acpi_device *adev,
 					const char *name, acpi_object_type type,
@@ -696,6 +704,30 @@ static inline int acpi_dev_get_property_
 {
 	return -ENXIO;
 }
+
+static inline int acpi_dev_prop_get(struct acpi_device *adev,
+				    const char *propname,
+				    void **valptr)
+{
+	return -ENXIO;
+}
+
+static inline int acpi_dev_prop_read_single(struct acpi_device *adev,
+					    const char *propname,
+					    enum dev_prop_type proptype,
+					    void *val)
+{
+	return -ENXIO;
+}
+
+static inline int acpi_dev_prop_read(struct acpi_device *adev,
+				     const char *propname,
+				     enum dev_prop_type proptype,
+				     void *val, size_t nval)
+{
+	return -ENXIO;
+}
+
 #endif
 
 #endif	/*_LINUX_ACPI_H*/
Index: linux-pm/drivers/base/Makefile
===================================================================
--- linux-pm.orig/drivers/base/Makefile
+++ linux-pm/drivers/base/Makefile
@@ -4,7 +4,7 @@ obj-y			:= component.o core.o bus.o dd.o
 			   driver.o class.o platform.o \
 			   cpu.o firmware.o init.o map.o devres.o \
 			   attribute_container.o transport_class.o \
-			   topology.o container.o
+			   topology.o container.o property.o
 obj-$(CONFIG_DEVTMPFS)	+= devtmpfs.o
 obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
 obj-y			+= power/
Index: linux-pm/drivers/base/property.c
===================================================================
--- /dev/null
+++ linux-pm/drivers/base/property.c
@@ -0,0 +1,233 @@
+/*
+ * property.c - Unified device property interface.
+ *
+ * Copyright (C) 2014, Intel Corporation
+ * Authors: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+ *          Mika Westerberg <mika.westerberg@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/property.h>
+#include <linux/export.h>
+#include <linux/acpi.h>
+#include <linux/of.h>
+
+/**
+ * device_property_present - check if a property of a device is present
+ * @dev: Device whose property is being checked
+ * @propname: Name of the property
+ *
+ * Check if property @propname is present in the device firmware description.
+ */
+bool device_property_present(struct device *dev, const char *propname)
+{
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+		return of_property_read_bool(dev->of_node, propname);
+
+	return !acpi_dev_prop_get(ACPI_COMPANION(dev), propname, NULL);
+}
+EXPORT_SYMBOL_GPL(device_property_present);
+
+#define OF_DEV_PROP_READ_ARRAY(node, propname, type, val, nval) \
+	(val) ? of_property_read_##type##_array((node), (propname), (val), (nval)) \
+	      : of_property_count_elems_of_size((node), (propname), sizeof(type))
+
+#define DEV_PROP_READ_ARRAY(_dev_, _propname_, _type_, _proptype_, _val_, _nval_) \
+	IS_ENABLED(CONFIG_OF) && _dev_->of_node ? \
+		(OF_DEV_PROP_READ_ARRAY(_dev_->of_node, _propname_, _type_, \
+					_val_, _nval_)) : \
+		acpi_dev_prop_read(ACPI_COMPANION(_dev_), _propname_, \
+				   _proptype_, _val_, _nval_)
+
+/**
+ * device_property_read_u8_array - return a u8 array property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of u8 properties with @propname from the device
+ * firmware description and stores them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected.
+ */
+int device_property_read_u8_array(struct device *dev, const char *propname,
+				  u8 *val, size_t nval)
+{
+	return DEV_PROP_READ_ARRAY(dev, propname, u8, DEV_PROP_U8, val, nval);
+}
+EXPORT_SYMBOL_GPL(device_property_read_u8_array);
+
+/**
+ * device_property_read_u16_array - return a u16 array property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of u16 properties with @propname from the device
+ * firmware description and stores them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected.
+ */
+int device_property_read_u16_array(struct device *dev, const char *propname,
+				   u16 *val, size_t nval)
+{
+	return DEV_PROP_READ_ARRAY(dev, propname, u16, DEV_PROP_U16, val, nval);
+}
+EXPORT_SYMBOL_GPL(device_property_read_u16_array);
+
+/**
+ * device_property_read_u32_array - return a u32 array property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of u32 properties with @propname from the device
+ * firmware description and stores them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected.
+ */
+int device_property_read_u32_array(struct device *dev, const char *propname,
+				   u32 *val, size_t nval)
+{
+	return DEV_PROP_READ_ARRAY(dev, propname, u32, DEV_PROP_U32, val, nval);
+}
+EXPORT_SYMBOL_GPL(device_property_read_u32_array);
+
+/**
+ * device_property_read_u64_array - return a u64 array property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of u64 properties with @propname from the device
+ * firmware description and stores them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected.
+ */
+int device_property_read_u64_array(struct device *dev, const char *propname,
+				   u64 *val, size_t nval)
+{
+	return DEV_PROP_READ_ARRAY(dev, propname, u64, DEV_PROP_U64, val, nval);
+}
+EXPORT_SYMBOL_GPL(device_property_read_u64_array);
+
+/**
+ * device_property_read_string_array - return a string array property of device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of string properties with @propname from the device
+ * firmware description and stores them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO or %-EILSEQ if the property is not an array of strings,
+ *	   %-EOVERFLOW if the size of the property is not as expected.
+ */
+int device_property_read_string_array(struct device *dev, const char *propname,
+				      char **val, size_t nval)
+{
+	return IS_ENABLED(CONFIG_OF) && dev->of_node ?
+		of_property_read_string_array(dev->of_node, propname, val, nval) :
+		acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
+				   DEV_PROP_STRING, val, nval);
+}
+EXPORT_SYMBOL_GPL(device_property_read_string_array);
+
+/**
+ * device_property_read_u8 - return a u8 property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The value is stored here
+ */
+int device_property_read_u8(struct device *dev, const char *propname, u8 *val)
+{
+	return device_property_read_u8_array(dev, propname, val, 1);
+}
+EXPORT_SYMBOL_GPL(device_property_read_u8);
+
+/**
+ * device_property_read_u16 - return a u16 property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The value is stored here
+ */
+int device_property_read_u16(struct device *dev, const char *propname, u16 *val)
+{
+	return device_property_read_u16_array(dev, propname, val, 1);
+}
+EXPORT_SYMBOL_GPL(device_property_read_u16);
+
+/**
+ * device_property_read_u32 - return a u32 property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The value is stored here
+ */
+int device_property_read_u32(struct device *dev, const char *propname, u32 *val)
+{
+	return device_property_read_u32_array(dev, propname, val, 1);
+}
+EXPORT_SYMBOL_GPL(device_property_read_u32);
+
+/**
+ * device_property_read_u64 - return a u64 property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The value is stored here
+ */
+int device_property_read_u64(struct device *dev, const char *propname, u64 *val)
+{
+	return device_property_read_u64_array(dev, propname, val, 1);
+}
+EXPORT_SYMBOL_GPL(device_property_read_u64);
+
+/**
+ * device_property_read_string - return a string property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The value is stored here
+ *
+ * Function reads property @propname from the device firmware description and
+ * stores the value into @val if found. The value is checked to be a string.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO or %-EILSEQ if the property type is not a string.
+ */
+int device_property_read_string(struct device *dev, const char *propname,
+				const char **val)
+{
+	return IS_ENABLED(CONFIG_OF) && dev->of_node ?
+		of_property_read_string(dev->of_node, propname, val) :
+		acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
+				   DEV_PROP_STRING, val, 1);
+}
+EXPORT_SYMBOL_GPL(device_property_read_string);
Index: linux-pm/drivers/of/base.c
===================================================================
--- linux-pm.orig/drivers/of/base.c
+++ linux-pm/drivers/of/base.c
@@ -1250,6 +1250,39 @@ int of_property_read_u64(const struct de
 EXPORT_SYMBOL_GPL(of_property_read_u64);
 
 /**
+ * of_property_read_u64_array - Find and read an array of 64 bit integers
+ * from a property.
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ * @out_values:	pointer to return value, modified only if return value is 0.
+ * @sz:		number of array elements to read
+ *
+ * Search for a property in a device node and read 64-bit value(s) from
+ * it. Returns 0 on success, -EINVAL if the property does not exist,
+ * -ENODATA if property does not have a value, and -EOVERFLOW if the
+ * property data isn't large enough.
+ *
+ * The out_values is modified only if a valid u64 value can be decoded.
+ */
+int of_property_read_u64_array(const struct device_node *np,
+			       const char *propname, u64 *out_values,
+			       size_t sz)
+{
+	const __be32 *val = of_find_property_value_of_size(np, propname,
+						(sz * sizeof(*out_values)));
+
+	if (IS_ERR(val))
+		return PTR_ERR(val);
+
+	while (sz--) {
+		*out_values++ = of_read_number(val, 2);
+		val += 2;
+	}
+	return 0;
+}
+
+/**
  * of_property_read_string - Find and read a string from a property
  * @np:		device node from which the property value is to be read.
  * @propname:	name of the property to be searched.
@@ -1362,24 +1395,11 @@ int of_property_match_string(struct devi
 }
 EXPORT_SYMBOL_GPL(of_property_match_string);
 
-/**
- * of_property_count_strings - Find and return the number of strings from a
- * multiple strings property.
- * @np:		device node from which the property value is to be read.
- * @propname:	name of the property to be searched.
- *
- * Search for a property in a device tree node and retrieve the number of null
- * terminated string contain in it. Returns the number of strings on
- * success, -EINVAL if the property does not exist, -ENODATA if property
- * does not have a value, and -EILSEQ if the string is not null-terminated
- * within the length of the property data.
- */
-int of_property_count_strings(struct device_node *np, const char *propname)
+static int property_count_strings(struct property *prop)
 {
-	struct property *prop = of_find_property(np, propname, NULL);
-	int i = 0;
-	size_t l = 0, total = 0;
 	const char *p;
+	size_t l = 0, total = 0;
+	int i = 0;
 
 	if (!prop)
 		return -EINVAL;
@@ -1395,8 +1415,62 @@ int of_property_count_strings(struct dev
 
 	return i;
 }
+
+/**
+ * of_property_count_strings - Find and return the number of strings from a
+ * multiple strings property.
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ *
+ * Search for a property in a device tree node and retrieve the number of null
+ * terminated string contain in it. Returns the number of strings on
+ * success, -EINVAL if the property does not exist, -ENODATA if property
+ * does not have a value, and -EILSEQ if the string is not null-terminated
+ * within the length of the property data.
+ */
+int of_property_count_strings(struct device_node *np, const char *propname)
+{
+	return property_count_strings(of_find_property(np, propname, NULL));
+}
 EXPORT_SYMBOL_GPL(of_property_count_strings);
 
+/**
+ * of_property_read_string_array - Read an array of strings from a multiple
+ * strings property.
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ * @out_strs:	output array of string pointers.
+ * @sz:		number of array elements to read.
+ *
+ * Search for a property in a device tree node and retrieve a list of
+ * terminated string values (pointer to data, not a copy) in that property.
+ *
+ * If @out_strs is NULL, the number of strings in the property is returned.
+ */
+int of_property_read_string_array(struct device_node *np, const char *propname,
+				  char **out_strs, size_t sz)
+{
+	struct property *prop = of_find_property(np, propname, NULL);
+	char *p = prop->value;
+	size_t total = 0;
+	int i;
+
+	i = property_count_strings(prop);
+	if (!out_strs || i < 0)
+		return i;
+
+	if (i < sz)
+		return -EOVERFLOW;
+
+	while (total < prop->length && i < sz) {
+		size_t l = strlen(p) + 1;
+
+		out_strs[i++] = p;
+		total += l;
+		p += l;
+	}
+}
+
 void of_print_phandle_args(const char *msg, const struct of_phandle_args *args)
 {
 	int i;
Index: linux-pm/drivers/acpi/property.c
===================================================================
--- linux-pm.orig/drivers/acpi/property.c
+++ linux-pm/drivers/acpi/property.c
@@ -362,3 +362,181 @@ int acpi_dev_get_property_reference(stru
 	return -EPROTO;
 }
 EXPORT_SYMBOL_GPL(acpi_dev_get_property_reference);
+
+int acpi_dev_prop_get(struct acpi_device *adev, const char *propname,
+		      void **valptr)
+{
+	return acpi_dev_get_property(adev, propname, ACPI_TYPE_ANY,
+				     (const union acpi_object **)valptr);
+}
+
+int acpi_dev_prop_read_single(struct acpi_device *adev, const char *propname,
+			      enum dev_prop_type proptype, void *val)
+{
+	const union acpi_object *obj;
+	int ret;
+
+	if (!val)
+		return -EINVAL;
+
+	if (proptype >= DEV_PROP_U8 && proptype <= DEV_PROP_U64) {
+		ret = acpi_dev_get_property(adev, propname, ACPI_TYPE_INTEGER, &obj);
+		if (ret)
+			return ret;
+
+		switch (proptype) {
+		case DEV_PROP_U8:
+			if (obj->integer.value > U8_MAX)
+				return -EOVERFLOW;
+			*(u8 *)val = obj->integer.value;
+			break;
+		case DEV_PROP_U16:
+			if (obj->integer.value > U16_MAX)
+				return -EOVERFLOW;
+			*(u16 *)val = obj->integer.value;
+			break;
+		case DEV_PROP_U32:
+			if (obj->integer.value > U32_MAX)
+				return -EOVERFLOW;
+			*(u32 *)val = obj->integer.value;
+			break;
+		default:
+			*(u64 *)val = obj->integer.value;
+			break;
+		}
+	} else if (proptype == DEV_PROP_STRING) {
+		ret = acpi_dev_get_property(adev, propname, ACPI_TYPE_STRING, &obj);
+		if (ret)
+			return ret;
+
+		*(char **)val = obj->string.pointer;
+	} else {
+		ret = -EINVAL;
+	}
+	return ret;
+}
+
+static int acpi_copy_property_array_u8(const union acpi_object *items, u8 *val,
+				       size_t nval)
+{
+	int i;
+
+	for (i = 0; i < nval; i++) {
+		if (items[i].type != ACPI_TYPE_INTEGER)
+			return -EPROTO;
+		if (items[i].integer.value > U8_MAX)
+			return -EOVERFLOW;
+
+		val[i] = items[i].integer.value;
+	}
+	return 0;
+}
+
+static int acpi_copy_property_array_u16(const union acpi_object *items,
+					u16 *val, size_t nval)
+{
+	int i;
+
+	for (i = 0; i < nval; i++) {
+		if (items[i].type != ACPI_TYPE_INTEGER)
+			return -EPROTO;
+		if (items[i].integer.value > U16_MAX)
+			return -EOVERFLOW;
+
+		val[i] = items[i].integer.value;
+	}
+	return 0;
+}
+
+static int acpi_copy_property_array_u32(const union acpi_object *items,
+					u32 *val, size_t nval)
+{
+	int i;
+
+	for (i = 0; i < nval; i++) {
+		if (items[i].type != ACPI_TYPE_INTEGER)
+			return -EPROTO;
+		if (items[i].integer.value > U32_MAX)
+			return -EOVERFLOW;
+
+		val[i] = items[i].integer.value;
+	}
+	return 0;
+}
+
+static int acpi_copy_property_array_u64(const union acpi_object *items,
+					u64 *val, size_t nval)
+{
+	int i;
+
+	for (i = 0; i < nval; i++) {
+		if (items[i].type != ACPI_TYPE_INTEGER)
+			return -EPROTO;
+
+		val[i] = items[i].integer.value;
+	}
+	return 0;
+}
+
+static int acpi_copy_property_array_string(const union acpi_object *items,
+					   char **val, size_t nval)
+{
+	int i;
+
+	for (i = 0; i < nval; i++) {
+		if (items[i].type != ACPI_TYPE_STRING)
+			return -EPROTO;
+
+		val[i] = items[i].string.pointer;
+	}
+	return 0;
+}
+
+int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
+		       enum dev_prop_type proptype, void *val, size_t nval)
+{
+	const union acpi_object *obj;
+	const union acpi_object *items;
+	int ret;
+
+	if (val && nval == 1) {
+		ret = acpi_dev_prop_read_single(adev, propname, proptype, val);
+		if (!ret)
+			return ret;
+	}
+
+	ret = acpi_dev_get_property_array(adev, propname, ACPI_TYPE_ANY, &obj);
+	if (ret)
+		return ret;
+
+	if (!val)
+		return obj->package.count;
+	else if (nval <= 0)
+		return -EINVAL;
+
+	if (nval > obj->package.count)
+		return -EOVERFLOW;
+
+	items = obj->package.elements;
+	switch (proptype) {
+	case DEV_PROP_U8:
+		ret = acpi_copy_property_array_u8(items, (u8 *)val, nval);
+		break;
+	case DEV_PROP_U16:
+		ret = acpi_copy_property_array_u16(items, (u16 *)val, nval);
+		break;
+	case DEV_PROP_U32:
+		ret = acpi_copy_property_array_u32(items, (u32 *)val, nval);
+		break;
+	case DEV_PROP_U64:
+		ret = acpi_copy_property_array_u64(items, (u64 *)val, nval);
+		break;
+	case DEV_PROP_STRING:
+		ret = acpi_copy_property_array_string(items, (char **)val, nval);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	return ret;
+}


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

* [PATCH v6 03/12] ACPI: Allow drivers to match using Device Tree compatible property
  2014-10-21 21:08 [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
  2014-10-21 21:09 ` [PATCH v6 01/12] ACPI: Add support for device specific properties Rafael J. Wysocki
  2014-10-21 21:15 ` [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware Rafael J. Wysocki
@ 2014-10-21 21:19 ` Rafael J. Wysocki
  2014-10-21 21:19 ` [PATCH v6 04/12] misc: at25: Make use of device property API Rafael J. Wysocki
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-10-21 21:19 UTC (permalink / raw)
  To: Linux Kernel Mailing List, Grant Likely
  Cc: ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Mika Westerberg, Aaron Lu, Arnd Bergmann, devicetree,
	Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu

From: Mika Westerberg <mika.westerberg@linux.intel.com>

We have lots of existing Device Tree enabled drivers and allocating
separate _HID for each is not feasible. Instead we allocate special _HID
"PRP0001" that means that the match should be done using Device Tree
compatible property using driver's .of_match_table instead if the driver
is missing .acpi_match_table.

If there is a need to distinguish from where the device is enumerated
(DT/ACPI) driver can check dev->of_node or ACPI_COMPATION(dev).

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Changes from v5:
- Package () { "compatible", "blah" } is now regarded as a valid compatible
  property definition and is equivalent to
  Package () { "compatible", Package () { "blah" } }

Grant, I've dropped the Reviewed-by from you due to the modifications, please
have another look at the code.

---
 drivers/acpi/property.c |   39 ++++++++++++++++++
 drivers/acpi/scan.c     |  104 +++++++++++++++++++++++++++++++++++++++++++-----
 include/acpi/acpi_bus.h |    1 
 include/linux/acpi.h    |    8 ---
 4 files changed, 136 insertions(+), 16 deletions(-)

Index: linux-pm/drivers/acpi/property.c
===================================================================
--- linux-pm.orig/drivers/acpi/property.c
+++ linux-pm/drivers/acpi/property.c
@@ -76,6 +76,42 @@ static bool acpi_properties_format_valid
 	return true;
 }
 
+static void acpi_init_of_compatible(struct acpi_device *adev)
+{
+	const union acpi_object *of_compatible;
+	struct acpi_hardware_id *hwid;
+	bool acpi_of = false;
+	int ret;
+
+	/*
+	 * Check if the special PRP0001 ACPI ID is present and in that
+	 * case we fill in Device Tree compatible properties for this
+	 * device.
+	 */
+	list_for_each_entry(hwid, &adev->pnp.ids, list) {
+		if (!strcmp(hwid->id, "PRP0001")) {
+			acpi_of = true;
+			break;
+		}
+	}
+
+	if (!acpi_of)
+		return;
+
+	ret = acpi_dev_get_property_array(adev, "compatible", ACPI_TYPE_STRING,
+					  &of_compatible);
+	if (ret) {
+		ret = acpi_dev_get_property(adev, "compatible",
+					    ACPI_TYPE_STRING, &of_compatible);
+		if (ret) {
+			acpi_handle_warn(adev->handle,
+					 "PRP0001 requires compatible property\n");
+			return;
+		}
+	}
+	adev->data.of_compatible = of_compatible;
+}
+
 void acpi_init_properties(struct acpi_device *adev)
 {
 	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
@@ -119,6 +155,8 @@ void acpi_init_properties(struct acpi_de
 
 		adev->data.pointer = buf.pointer;
 		adev->data.properties = properties;
+
+		acpi_init_of_compatible(adev);
 		return;
 	}
 
@@ -130,6 +168,7 @@ void acpi_init_properties(struct acpi_de
 void acpi_free_properties(struct acpi_device *adev)
 {
 	ACPI_FREE((void *)adev->data.pointer);
+	adev->data.of_compatible = NULL;
 	adev->data.pointer = NULL;
 	adev->data.properties = NULL;
 }
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -124,17 +124,56 @@ static int create_modalias(struct acpi_d
 	if (list_empty(&acpi_dev->pnp.ids))
 		return 0;
 
-	len = snprintf(modalias, size, "acpi:");
-	size -= len;
+	/*
+	 * If the device has PRP0001 we expose DT compatible modalias
+	 * instead in form of of:NnameTCcompatible.
+	 */
+	if (acpi_dev->data.of_compatible) {
+		struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
+		const union acpi_object *of_compatible, *obj;
+		int i, nval;
+		char *c;
+
+		acpi_get_name(acpi_dev->handle, ACPI_SINGLE_NAME, &buf);
+		/* DT strings are all in lower case */
+		for (c = buf.pointer; *c != '\0'; c++)
+			*c = tolower(*c);
+
+		len = snprintf(modalias, size, "of:N%sT", (char *)buf.pointer);
+		ACPI_FREE(buf.pointer);
+
+		of_compatible = acpi_dev->data.of_compatible;
+		if (of_compatible->type == ACPI_TYPE_PACKAGE) {
+			nval = of_compatible->package.count;
+			obj = of_compatible->package.elements;
+		} else { /* Must be ACPI_TYPE_STRING. */
+			nval = 1;
+			obj = of_compatible;
+		}
+		for (i = 0; i < nval; i++, obj++) {
+			count = snprintf(&modalias[len], size, "C%s",
+					 obj->string.pointer);
+			if (count < 0)
+				return -EINVAL;
+			if (count >= size)
+				return -ENOMEM;
+
+			len += count;
+			size -= count;
+		}
+	} else {
+		len = snprintf(modalias, size, "acpi:");
+		size -= len;
 
-	list_for_each_entry(id, &acpi_dev->pnp.ids, list) {
-		count = snprintf(&modalias[len], size, "%s:", id->id);
-		if (count < 0)
-			return -EINVAL;
-		if (count >= size)
-			return -ENOMEM;
-		len += count;
-		size -= count;
+		list_for_each_entry(id, &acpi_dev->pnp.ids, list) {
+			count = snprintf(&modalias[len], size, "%s:", id->id);
+			if (count < 0)
+				return -EINVAL;
+			if (count >= size)
+				return -ENOMEM;
+			len += count;
+			size -= count;
+		}
 	}
 
 	modalias[len] = '\0';
@@ -864,6 +903,51 @@ int acpi_match_device_ids(struct acpi_de
 }
 EXPORT_SYMBOL(acpi_match_device_ids);
 
+/* Performs match against special "PRP0001" shoehorn ACPI ID */
+static bool acpi_of_driver_match_device(struct device *dev,
+					const struct device_driver *drv)
+{
+	const union acpi_object *of_compatible, *obj;
+	struct acpi_device *adev;
+	int i, nval;
+
+	adev = ACPI_COMPANION(dev);
+	if (!adev)
+		return false;
+
+	of_compatible = adev->data.of_compatible;
+	if (!drv->of_match_table || !of_compatible)
+		return false;
+
+	if (of_compatible->type == ACPI_TYPE_PACKAGE) {
+		nval = of_compatible->package.count;
+		obj = of_compatible->package.elements;
+	} else { /* Must be ACPI_TYPE_STRING. */
+		nval = 1;
+		obj = of_compatible;
+	}
+	/* Now we can look for the driver DT compatible strings */
+	for (i = 0; i < nval; i++, obj++) {
+		const struct of_device_id *id;
+
+		for (id = drv->of_match_table; id->compatible[0]; id++)
+			if (!strcasecmp(obj->string.pointer, id->compatible))
+				return true;
+	}
+
+	return false;
+}
+
+bool acpi_driver_match_device(struct device *dev,
+			      const struct device_driver *drv)
+{
+	if (!drv->acpi_match_table)
+		return acpi_of_driver_match_device(dev, drv);
+
+	return !!acpi_match_device(drv->acpi_match_table, dev);
+}
+EXPORT_SYMBOL_GPL(acpi_driver_match_device);
+
 static void acpi_free_power_resources_lists(struct acpi_device *device)
 {
 	int i;
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -341,6 +341,7 @@ struct acpi_device_physical_node {
 struct acpi_device_data {
 	const union acpi_object *pointer;
 	const union acpi_object *properties;
+	const union acpi_object *of_compatible;
 };
 
 /* Device */
Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -424,12 +424,8 @@ extern int acpi_nvs_for_each_region(int
 const struct acpi_device_id *acpi_match_device(const struct acpi_device_id *ids,
 					       const struct device *dev);
 
-static inline bool acpi_driver_match_device(struct device *dev,
-					    const struct device_driver *drv)
-{
-	return !!acpi_match_device(drv->acpi_match_table, dev);
-}
-
+extern bool acpi_driver_match_device(struct device *dev,
+				     const struct device_driver *drv);
 int acpi_device_uevent_modalias(struct device *, struct kobj_uevent_env *);
 int acpi_device_modalias(struct device *, char *, int);
 


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

* [PATCH v6 04/12] misc: at25: Make use of device property API
  2014-10-21 21:08 [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2014-10-21 21:19 ` [PATCH v6 03/12] ACPI: Allow drivers to match using Device Tree compatible property Rafael J. Wysocki
@ 2014-10-21 21:19 ` Rafael J. Wysocki
  2014-11-04 14:18   ` Grant Likely
  2014-10-21 21:20 ` [PATCH v6 05/12] gpio / ACPI: Add support for _DSD device properties Rafael J. Wysocki
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-10-21 21:19 UTC (permalink / raw)
  To: Linux Kernel Mailing List
  Cc: ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Mika Westerberg, Aaron Lu, Grant Likely, Arnd Bergmann,
	devicetree, Linus Walleij, Alexandre Courbot, Dmitry Torokhov,
	Bryan Wu

From: Mika Westerberg <mika.westerberg@linux.intel.com>

Make use of device property API in this driver so that both DT and ACPI
based systems can use this driver.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/misc/eeprom/at25.c |   34 +++++++++++++---------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

Index: linux-pm/drivers/misc/eeprom/at25.c
===================================================================
--- linux-pm.orig/drivers/misc/eeprom/at25.c
+++ linux-pm/drivers/misc/eeprom/at25.c
@@ -18,7 +18,7 @@
 
 #include <linux/spi/spi.h>
 #include <linux/spi/eeprom.h>
-#include <linux/of.h>
+#include <linux/property.h>
 
 /*
  * NOTE: this is an *EEPROM* driver.  The vagaries of product naming
@@ -301,35 +301,33 @@ static ssize_t at25_mem_write(struct mem
 
 /*-------------------------------------------------------------------------*/
 
-static int at25_np_to_chip(struct device *dev,
-			   struct device_node *np,
-			   struct spi_eeprom *chip)
+static int at25_fw_to_chip(struct device *dev, struct spi_eeprom *chip)
 {
 	u32 val;
 
 	memset(chip, 0, sizeof(*chip));
-	strncpy(chip->name, np->name, sizeof(chip->name));
+	strncpy(chip->name, "at25", sizeof(chip->name));
 
-	if (of_property_read_u32(np, "size", &val) == 0 ||
-	    of_property_read_u32(np, "at25,byte-len", &val) == 0) {
+	if (device_property_read_u32(dev, "size", &val) == 0 ||
+	    device_property_read_u32(dev, "at25,byte-len", &val) == 0) {
 		chip->byte_len = val;
 	} else {
 		dev_err(dev, "Error: missing \"size\" property\n");
 		return -ENODEV;
 	}
 
-	if (of_property_read_u32(np, "pagesize", &val) == 0 ||
-	    of_property_read_u32(np, "at25,page-size", &val) == 0) {
+	if (device_property_read_u32(dev, "pagesize", &val) == 0 ||
+	    device_property_read_u32(dev, "at25,page-size", &val) == 0) {
 		chip->page_size = (u16)val;
 	} else {
 		dev_err(dev, "Error: missing \"pagesize\" property\n");
 		return -ENODEV;
 	}
 
-	if (of_property_read_u32(np, "at25,addr-mode", &val) == 0) {
+	if (device_property_read_u32(dev, "at25,addr-mode", &val) == 0) {
 		chip->flags = (u16)val;
 	} else {
-		if (of_property_read_u32(np, "address-width", &val)) {
+		if (device_property_read_u32(dev, "address-width", &val)) {
 			dev_err(dev,
 				"Error: missing \"address-width\" property\n");
 			return -ENODEV;
@@ -350,7 +348,7 @@ static int at25_np_to_chip(struct device
 				val);
 			return -ENODEV;
 		}
-		if (of_find_property(np, "read-only", NULL))
+		if (device_property_present(dev, "read-only"))
 			chip->flags |= EE_READONLY;
 	}
 	return 0;
@@ -360,21 +358,15 @@ static int at25_probe(struct spi_device
 {
 	struct at25_data	*at25 = NULL;
 	struct spi_eeprom	chip;
-	struct device_node	*np = spi->dev.of_node;
 	int			err;
 	int			sr;
 	int			addrlen;
 
 	/* Chip description */
 	if (!spi->dev.platform_data) {
-		if (np) {
-			err = at25_np_to_chip(&spi->dev, np, &chip);
-			if (err)
-				return err;
-		} else {
-			dev_err(&spi->dev, "Error: no chip description\n");
-			return -ENODEV;
-		}
+		err = at25_fw_to_chip(&spi->dev, &chip);
+		if (err)
+			return err;
 	} else
 		chip = *(struct spi_eeprom *)spi->dev.platform_data;
 


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

* [PATCH v6 05/12] gpio / ACPI: Add support for _DSD device properties
  2014-10-21 21:08 [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2014-10-21 21:19 ` [PATCH v6 04/12] misc: at25: Make use of device property API Rafael J. Wysocki
@ 2014-10-21 21:20 ` Rafael J. Wysocki
  2014-10-21 21:21 ` [PATCH v6 06/12] gpio: sch: Consolidate core and resume banks Rafael J. Wysocki
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-10-21 21:20 UTC (permalink / raw)
  To: Linux Kernel Mailing List
  Cc: ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Mika Westerberg, Aaron Lu, Grant Likely, Arnd Bergmann,
	devicetree, Linus Walleij, Alexandre Courbot, Dmitry Torokhov,
	Bryan Wu

From: Mika Westerberg <mika.westerberg@linux.intel.com>

With release of ACPI 5.1 and _DSD method we can finally name GPIOs (and
other things as well) returned by _CRS. Previously we were only able to
use integer index to find the corresponding GPIO, which is pretty error
prone if the order changes.

With _DSD we can now query GPIOs using name instead of an integer index,
like the below example shows:

  // Bluetooth device with reset and shutdown GPIOs
  Device (BTH)
  {
      Name (_HID, ...)

      Name (_CRS, ResourceTemplate ()
      {
          GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
                  "\\_SB.GPO0", 0, ResourceConsumer) {15}
          GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
                  "\\_SB.GPO0", 0, ResourceConsumer) {27, 31}
      })

      Name (_DSD, Package ()
      {
          ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
          Package ()
	  {
              Package () {"reset-gpio", Package() {^BTH, 1, 1, 0 }},
              Package () {"shutdown-gpio", Package() {^BTH, 0, 0, 0 }},
          }
      })
  }

The format of the supported GPIO property is:

  Package () { "name", Package () { ref, index, pin, active_low }}

  ref - The device that has _CRS containing GpioIo()/GpioInt() resources,
        typically this is the device itself (BTH in our case).
  index - Index of the GpioIo()/GpioInt() resource in _CRS starting from zero.
  pin - Pin in the GpioIo()/GpioInt() resource. Typically this is zero.
  active_low - If 1 the GPIO is marked as active_low.

Since ACPI GpioIo() resource does not have field saying whether it is
active low or high, the "active_low" argument can be used here. Setting
it to 1 marks the GPIO as active low.

In our Bluetooth example the "reset-gpio" refers to the second GpioIo()
resource, second pin in that resource with the GPIO number of 31.

This patch implements necessary support to gpiolib for extracting GPIOs
using _DSD device properties.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/acpi/gpio-properties.txt |   52 ++++++++++++++++++++++
 drivers/gpio/gpiolib-acpi.c            |   78 +++++++++++++++++++++++++++------
 drivers/gpio/gpiolib.c                 |   30 +++++++++++-
 drivers/gpio/gpiolib.h                 |    7 +-
 4 files changed, 146 insertions(+), 21 deletions(-)

Index: linux-pm/drivers/gpio/gpiolib-acpi.c
===================================================================
--- linux-pm.orig/drivers/gpio/gpiolib-acpi.c
+++ linux-pm/drivers/gpio/gpiolib-acpi.c
@@ -290,6 +290,7 @@ void acpi_gpiochip_free_interrupts(struc
 struct acpi_gpio_lookup {
 	struct acpi_gpio_info info;
 	int index;
+	int pin_index;
 	struct gpio_desc *desc;
 	int n;
 };
@@ -303,13 +304,24 @@ static int acpi_find_gpio(struct acpi_re
 
 	if (lookup->n++ == lookup->index && !lookup->desc) {
 		const struct acpi_resource_gpio *agpio = &ares->data.gpio;
+		int pin_index = lookup->pin_index;
+
+		if (pin_index >= agpio->pin_table_length)
+			return 1;
 
 		lookup->desc = acpi_get_gpiod(agpio->resource_source.string_ptr,
-					      agpio->pin_table[0]);
+					      agpio->pin_table[pin_index]);
 		lookup->info.gpioint =
 			agpio->connection_type == ACPI_RESOURCE_GPIO_TYPE_INT;
-		lookup->info.active_low =
-			agpio->polarity == ACPI_ACTIVE_LOW;
+
+		/*
+		 * ActiveLow is only specified for GpioInt resource. If
+		 * GpioIo is used then the only way to set the flag is
+		 * to use _DSD "gpios" property.
+		 */
+		if (lookup->info.gpioint)
+			lookup->info.active_low =
+				agpio->polarity == ACPI_ACTIVE_LOW;
 	}
 
 	return 1;
@@ -317,40 +329,75 @@ static int acpi_find_gpio(struct acpi_re
 
 /**
  * acpi_get_gpiod_by_index() - get a GPIO descriptor from device resources
- * @dev: pointer to a device to get GPIO from
+ * @adev: pointer to a ACPI device to get GPIO from
+ * @propname: Property name of the GPIO (optional)
  * @index: index of GpioIo/GpioInt resource (starting from %0)
  * @info: info pointer to fill in (optional)
  *
- * Function goes through ACPI resources for @dev and based on @index looks
+ * Function goes through ACPI resources for @adev and based on @index looks
  * up a GpioIo/GpioInt resource, translates it to the Linux GPIO descriptor,
  * and returns it. @index matches GpioIo/GpioInt resources only so if there
  * are total %3 GPIO resources, the index goes from %0 to %2.
  *
+ * If @propname is specified the GPIO is looked using device property. In
+ * that case @index is used to select the GPIO entry in the property value
+ * (in case of multiple).
+ *
  * If the GPIO cannot be translated or there is an error an ERR_PTR is
  * returned.
  *
  * Note: if the GPIO resource has multiple entries in the pin list, this
  * function only returns the first.
  */
-struct gpio_desc *acpi_get_gpiod_by_index(struct device *dev, int index,
+struct gpio_desc *acpi_get_gpiod_by_index(struct acpi_device *adev,
+					  const char *propname, int index,
 					  struct acpi_gpio_info *info)
 {
 	struct acpi_gpio_lookup lookup;
 	struct list_head resource_list;
-	struct acpi_device *adev;
-	acpi_handle handle;
+	bool active_low = false;
 	int ret;
 
-	if (!dev)
-		return ERR_PTR(-EINVAL);
-
-	handle = ACPI_HANDLE(dev);
-	if (!handle || acpi_bus_get_device(handle, &adev))
+	if (!adev)
 		return ERR_PTR(-ENODEV);
 
 	memset(&lookup, 0, sizeof(lookup));
 	lookup.index = index;
 
+	if (propname) {
+		struct acpi_reference_args args;
+
+		dev_dbg(&adev->dev, "GPIO: looking up %s\n", propname);
+
+		memset(&args, 0, sizeof(args));
+		ret = acpi_dev_get_property_reference(adev, propname, NULL,
+						      index, &args);
+		if (ret)
+			return ERR_PTR(ret);
+
+		/*
+		 * The property was found and resolved so need to
+		 * lookup the GPIO based on returned args instead.
+		 */
+		adev = args.adev;
+		if (args.nargs >= 2) {
+			lookup.index = args.args[0];
+			lookup.pin_index = args.args[1];
+			/*
+			 * 3rd argument, if present is used to
+			 * specify active_low.
+			 */
+			if (args.nargs >= 3)
+				active_low = !!args.args[2];
+		}
+
+		dev_dbg(&adev->dev, "GPIO: _DSD returned %s %zd %llu %llu %llu\n",
+			dev_name(&adev->dev), args.nargs,
+			args.args[0], args.args[1], args.args[2]);
+	} else {
+		dev_dbg(&adev->dev, "GPIO: looking up %d in _CRS\n", index);
+	}
+
 	INIT_LIST_HEAD(&resource_list);
 	ret = acpi_dev_get_resources(adev, &resource_list, acpi_find_gpio,
 				     &lookup);
@@ -359,8 +406,11 @@ struct gpio_desc *acpi_get_gpiod_by_inde
 
 	acpi_dev_free_resource_list(&resource_list);
 
-	if (lookup.desc && info)
+	if (lookup.desc && info) {
 		*info = lookup.info;
+		if (active_low)
+			info->active_low = active_low;
+	}
 
 	return lookup.desc ? lookup.desc : ERR_PTR(-ENOENT);
 }
Index: linux-pm/drivers/gpio/gpiolib.c
===================================================================
--- linux-pm.orig/drivers/gpio/gpiolib.c
+++ linux-pm/drivers/gpio/gpiolib.c
@@ -1505,14 +1505,36 @@ static struct gpio_desc *acpi_find_gpio(
 					unsigned int idx,
 					enum gpio_lookup_flags *flags)
 {
+	static const char * const suffixes[] = { "gpios", "gpio" };
+	struct acpi_device *adev = ACPI_COMPANION(dev);
 	struct acpi_gpio_info info;
 	struct gpio_desc *desc;
+	char propname[32];
+	int i;
 
-	desc = acpi_get_gpiod_by_index(dev, idx, &info);
-	if (IS_ERR(desc))
-		return desc;
+	/* Try first from _DSD */
+	for (i = 0; i < ARRAY_SIZE(suffixes); i++) {
+		if (con_id && strcmp(con_id, "gpios")) {
+			snprintf(propname, sizeof(propname), "%s-%s",
+				 con_id, suffixes[i]);
+		} else {
+			snprintf(propname, sizeof(propname), "%s",
+				 suffixes[i]);
+		}
 
-	if (info.gpioint && info.active_low)
+		desc = acpi_get_gpiod_by_index(adev, propname, 0, &info);
+		if (!IS_ERR(desc) || (PTR_ERR(desc) == -EPROBE_DEFER))
+			break;
+	}
+
+	/* Then from plain _CRS GPIOs */
+	if (IS_ERR(desc)) {
+		desc = acpi_get_gpiod_by_index(adev, NULL, idx, &info);
+		if (IS_ERR(desc))
+			return desc;
+	}
+
+	if (info.active_low)
 		*flags |= GPIO_ACTIVE_LOW;
 
 	return desc;
Index: linux-pm/drivers/gpio/gpiolib.h
===================================================================
--- linux-pm.orig/drivers/gpio/gpiolib.h
+++ linux-pm/drivers/gpio/gpiolib.h
@@ -34,7 +34,8 @@ void acpi_gpiochip_remove(struct gpio_ch
 void acpi_gpiochip_request_interrupts(struct gpio_chip *chip);
 void acpi_gpiochip_free_interrupts(struct gpio_chip *chip);
 
-struct gpio_desc *acpi_get_gpiod_by_index(struct device *dev, int index,
+struct gpio_desc *acpi_get_gpiod_by_index(struct acpi_device *adev,
+					  const char *propname, int index,
 					  struct acpi_gpio_info *info);
 #else
 static inline void acpi_gpiochip_add(struct gpio_chip *chip) { }
@@ -47,8 +48,8 @@ static inline void
 acpi_gpiochip_free_interrupts(struct gpio_chip *chip) { }
 
 static inline struct gpio_desc *
-acpi_get_gpiod_by_index(struct device *dev, int index,
-			struct acpi_gpio_info *info)
+acpi_get_gpiod_by_index(struct acpi_device *adev, const char *propname,
+			int index, struct acpi_gpio_info *info)
 {
 	return ERR_PTR(-ENOSYS);
 }
Index: linux-pm/Documentation/acpi/gpio-properties.txt
===================================================================
--- /dev/null
+++ linux-pm/Documentation/acpi/gpio-properties.txt
@@ -0,0 +1,52 @@
+_DSD Device Properties Related to GPIO
+--------------------------------------
+
+With the release of ACPI 5.1 and the _DSD configuration objecte names
+can finally be given to GPIOs (and other things as well) returned by
+_CRS.  Previously, we were only able to use an integer index to find
+the corresponding GPIO, which is pretty error prone (it depends on
+the _CRS output ordering, for example).
+
+With _DSD we can now query GPIOs using a name instead of an integer
+index, like the ASL example below shows:
+
+  // Bluetooth device with reset and shutdown GPIOs
+  Device (BTH)
+  {
+      Name (_HID, ...)
+
+      Name (_CRS, ResourceTemplate ()
+      {
+          GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
+                  "\\_SB.GPO0", 0, ResourceConsumer) {15}
+          GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
+                  "\\_SB.GPO0", 0, ResourceConsumer) {27, 31}
+      })
+
+      Name (_DSD, Package ()
+      {
+          ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+          Package ()
+	  {
+              Package () {"reset-gpio", Package() {^BTH, 1, 1, 0 }},
+              Package () {"shutdown-gpio", Package() {^BTH, 0, 0, 0 }},
+          }
+      })
+  }
+
+The format of the supported GPIO property is:
+
+  Package () { "name", Package () { ref, index, pin, active_low }}
+
+  ref - The device that has _CRS containing GpioIo()/GpioInt() resources,
+        typically this is the device itself (BTH in our case).
+  index - Index of the GpioIo()/GpioInt() resource in _CRS starting from zero.
+  pin - Pin in the GpioIo()/GpioInt() resource. Typically this is zero.
+  active_low - If 1 the GPIO is marked as active_low.
+
+Since ACPI GpioIo() resource does not have a field saying whether it is
+active low or high, the "active_low" argument can be used here.  Setting
+it to 1 marks the GPIO as active low.
+
+In our Bluetooth example the "reset-gpio" refers to the second GpioIo()
+resource, second pin in that resource with the GPIO number of 31.


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

* [PATCH v6 06/12] gpio: sch: Consolidate core and resume banks
  2014-10-21 21:08 [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
                   ` (4 preceding siblings ...)
  2014-10-21 21:20 ` [PATCH v6 05/12] gpio / ACPI: Add support for _DSD device properties Rafael J. Wysocki
@ 2014-10-21 21:21 ` Rafael J. Wysocki
  2014-10-21 21:21 ` [PATCH v6 07/12] leds: leds-gpio: Add support for GPIO descriptors Rafael J. Wysocki
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-10-21 21:21 UTC (permalink / raw)
  To: Linux Kernel Mailing List
  Cc: ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Mika Westerberg, Aaron Lu, Grant Likely, Arnd Bergmann,
	devicetree, Linus Walleij, Alexandre Courbot, Dmitry Torokhov,
	Bryan Wu

From: Mika Westerberg <mika.westerberg@linux.intel.com>

This is actually a single device with two sets of identical registers,
which just happen to start from a different offset. Instead of having
separate GPIO chips created we consolidate them to be single GPIO chip.

In addition having a single GPIO chip allows us to handle ACPI GPIO
translation in the core in a more generic way, since the two GPIO chips
share the same parent ACPI device.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/gpio/gpio-sch.c | 293 ++++++++++++++++++------------------------------
 1 file changed, 112 insertions(+), 181 deletions(-)

diff --git a/drivers/gpio/gpio-sch.c b/drivers/gpio/gpio-sch.c
index 41e91d7..99720c8 100644
--- a/drivers/gpio/gpio-sch.c
+++ b/drivers/gpio/gpio-sch.c
@@ -29,290 +29,221 @@
 
 #include <linux/gpio.h>
 
-static DEFINE_SPINLOCK(gpio_lock);
-
-#define CGEN	(0x00)
-#define CGIO	(0x04)
-#define CGLV	(0x08)
-
-#define RGEN	(0x20)
-#define RGIO	(0x24)
-#define RGLV	(0x28)
-
-static unsigned short gpio_ba;
-
-static int sch_gpio_core_direction_in(struct gpio_chip *gc, unsigned  gpio_num)
-{
-	u8 curr_dirs;
-	unsigned short offset, bit;
-
-	spin_lock(&gpio_lock);
-
-	offset = CGIO + gpio_num / 8;
-	bit = gpio_num % 8;
-
-	curr_dirs = inb(gpio_ba + offset);
-
-	if (!(curr_dirs & (1 << bit)))
-		outb(curr_dirs | (1 << bit), gpio_ba + offset);
+#define GEN	0x00
+#define GIO	0x04
+#define GLV	0x08
+
+struct sch_gpio {
+	struct gpio_chip chip;
+	spinlock_t lock;
+	unsigned short iobase;
+	unsigned short core_base;
+	unsigned short resume_base;
+};
 
-	spin_unlock(&gpio_lock);
-	return 0;
-}
+#define to_sch_gpio(c)	container_of(c, struct sch_gpio, chip)
 
-static int sch_gpio_core_get(struct gpio_chip *gc, unsigned gpio_num)
+static unsigned sch_gpio_offset(struct sch_gpio *sch, unsigned gpio,
+				unsigned reg)
 {
-	int res;
-	unsigned short offset, bit;
+	unsigned base = 0;
 
-	offset = CGLV + gpio_num / 8;
-	bit = gpio_num % 8;
+	if (gpio >= sch->resume_base) {
+		gpio -= sch->resume_base;
+		base += 0x20;
+	}
 
-	res = !!(inb(gpio_ba + offset) & (1 << bit));
-	return res;
+	return base + reg + gpio / 8;
 }
 
-static void sch_gpio_core_set(struct gpio_chip *gc, unsigned gpio_num, int val)
+static unsigned sch_gpio_bit(struct sch_gpio *sch, unsigned gpio)
 {
-	u8 curr_vals;
-	unsigned short offset, bit;
-
-	spin_lock(&gpio_lock);
-
-	offset = CGLV + gpio_num / 8;
-	bit = gpio_num % 8;
-
-	curr_vals = inb(gpio_ba + offset);
-
-	if (val)
-		outb(curr_vals | (1 << bit), gpio_ba + offset);
-	else
-		outb((curr_vals & ~(1 << bit)), gpio_ba + offset);
-	spin_unlock(&gpio_lock);
+	if (gpio >= sch->resume_base)
+		gpio -= sch->resume_base;
+	return gpio % 8;
 }
 
-static int sch_gpio_core_direction_out(struct gpio_chip *gc,
-					unsigned gpio_num, int val)
+static void sch_gpio_enable(struct sch_gpio *sch, unsigned gpio)
 {
-	u8 curr_dirs;
 	unsigned short offset, bit;
+	u8 enable;
 
-	spin_lock(&gpio_lock);
+	spin_lock(&sch->lock);
 
-	offset = CGIO + gpio_num / 8;
-	bit = gpio_num % 8;
-
-	curr_dirs = inb(gpio_ba + offset);
-	if (curr_dirs & (1 << bit))
-		outb(curr_dirs & ~(1 << bit), gpio_ba + offset);
+	offset = sch_gpio_offset(sch, gpio, GEN);
+	bit = sch_gpio_bit(sch, gpio);
 
-	spin_unlock(&gpio_lock);
+	enable = inb(sch->iobase + offset);
+	if (!(enable & (1 << bit)))
+		outb(enable | (1 << bit), sch->iobase + offset);
 
-	/*
-	 * according to the datasheet, writing to the level register has no
-	 * effect when GPIO is programmed as input.
-	 * Actually the the level register is read-only when configured as input.
-	 * Thus presetting the output level before switching to output is _NOT_ possible.
-	 * Hence we set the level after configuring the GPIO as output.
-	 * But we cannot prevent a short low pulse if direction is set to high
-	 * and an external pull-up is connected.
-	 */
-	sch_gpio_core_set(gc, gpio_num, val);
-	return 0;
+	spin_unlock(&sch->lock);
 }
 
-static struct gpio_chip sch_gpio_core = {
-	.label			= "sch_gpio_core",
-	.owner			= THIS_MODULE,
-	.direction_input	= sch_gpio_core_direction_in,
-	.get			= sch_gpio_core_get,
-	.direction_output	= sch_gpio_core_direction_out,
-	.set			= sch_gpio_core_set,
-};
-
-static int sch_gpio_resume_direction_in(struct gpio_chip *gc,
-					unsigned gpio_num)
+static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned  gpio_num)
 {
+	struct sch_gpio *sch = to_sch_gpio(gc);
 	u8 curr_dirs;
 	unsigned short offset, bit;
 
-	spin_lock(&gpio_lock);
+	spin_lock(&sch->lock);
 
-	offset = RGIO + gpio_num / 8;
-	bit = gpio_num % 8;
+	offset = sch_gpio_offset(sch, gpio_num, GIO);
+	bit = sch_gpio_bit(sch, gpio_num);
 
-	curr_dirs = inb(gpio_ba + offset);
+	curr_dirs = inb(sch->iobase + offset);
 
 	if (!(curr_dirs & (1 << bit)))
-		outb(curr_dirs | (1 << bit), gpio_ba + offset);
+		outb(curr_dirs | (1 << bit), sch->iobase + offset);
 
-	spin_unlock(&gpio_lock);
+	spin_unlock(&sch->lock);
 	return 0;
 }
 
-static int sch_gpio_resume_get(struct gpio_chip *gc, unsigned gpio_num)
+static int sch_gpio_get(struct gpio_chip *gc, unsigned gpio_num)
 {
+	struct sch_gpio *sch = to_sch_gpio(gc);
+	int res;
 	unsigned short offset, bit;
 
-	offset = RGLV + gpio_num / 8;
-	bit = gpio_num % 8;
+	offset = sch_gpio_offset(sch, gpio_num, GLV);
+	bit = sch_gpio_bit(sch, gpio_num);
+
+	res = !!(inb(sch->iobase + offset) & (1 << bit));
 
-	return !!(inb(gpio_ba + offset) & (1 << bit));
+	return res;
 }
 
-static void sch_gpio_resume_set(struct gpio_chip *gc,
-				unsigned gpio_num, int val)
+static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
 {
+	struct sch_gpio *sch = to_sch_gpio(gc);
 	u8 curr_vals;
 	unsigned short offset, bit;
 
-	spin_lock(&gpio_lock);
+	spin_lock(&sch->lock);
 
-	offset = RGLV + gpio_num / 8;
-	bit = gpio_num % 8;
+	offset = sch_gpio_offset(sch, gpio_num, GLV);
+	bit = sch_gpio_bit(sch, gpio_num);
 
-	curr_vals = inb(gpio_ba + offset);
+	curr_vals = inb(sch->iobase + offset);
 
 	if (val)
-		outb(curr_vals | (1 << bit), gpio_ba + offset);
+		outb(curr_vals | (1 << bit), sch->iobase + offset);
 	else
-		outb((curr_vals & ~(1 << bit)), gpio_ba + offset);
+		outb((curr_vals & ~(1 << bit)), sch->iobase + offset);
 
-	spin_unlock(&gpio_lock);
+	spin_unlock(&sch->lock);
 }
 
-static int sch_gpio_resume_direction_out(struct gpio_chip *gc,
-					unsigned gpio_num, int val)
+static int sch_gpio_direction_out(struct gpio_chip *gc, unsigned gpio_num,
+				  int val)
 {
+	struct sch_gpio *sch = to_sch_gpio(gc);
 	u8 curr_dirs;
 	unsigned short offset, bit;
 
-	offset = RGIO + gpio_num / 8;
-	bit = gpio_num % 8;
+	spin_lock(&sch->lock);
 
-	spin_lock(&gpio_lock);
+	offset = sch_gpio_offset(sch, gpio_num, GIO);
+	bit = sch_gpio_bit(sch, gpio_num);
 
-	curr_dirs = inb(gpio_ba + offset);
+	curr_dirs = inb(sch->iobase + offset);
 	if (curr_dirs & (1 << bit))
-		outb(curr_dirs & ~(1 << bit), gpio_ba + offset);
+		outb(curr_dirs & ~(1 << bit), sch->iobase + offset);
 
-	spin_unlock(&gpio_lock);
+	spin_unlock(&sch->lock);
 
 	/*
-	* according to the datasheet, writing to the level register has no
-	* effect when GPIO is programmed as input.
-	* Actually the the level register is read-only when configured as input.
-	* Thus presetting the output level before switching to output is _NOT_ possible.
-	* Hence we set the level after configuring the GPIO as output.
-	* But we cannot prevent a short low pulse if direction is set to high
-	* and an external pull-up is connected.
-	*/
-	sch_gpio_resume_set(gc, gpio_num, val);
+	 * according to the datasheet, writing to the level register has no
+	 * effect when GPIO is programmed as input.
+	 * Actually the the level register is read-only when configured as input.
+	 * Thus presetting the output level before switching to output is _NOT_ possible.
+	 * Hence we set the level after configuring the GPIO as output.
+	 * But we cannot prevent a short low pulse if direction is set to high
+	 * and an external pull-up is connected.
+	 */
+	sch_gpio_set(gc, gpio_num, val);
 	return 0;
 }
 
-static struct gpio_chip sch_gpio_resume = {
-	.label			= "sch_gpio_resume",
+static struct gpio_chip sch_gpio_chip = {
+	.label			= "sch_gpio",
 	.owner			= THIS_MODULE,
-	.direction_input	= sch_gpio_resume_direction_in,
-	.get			= sch_gpio_resume_get,
-	.direction_output	= sch_gpio_resume_direction_out,
-	.set			= sch_gpio_resume_set,
+	.direction_input	= sch_gpio_direction_in,
+	.get			= sch_gpio_get,
+	.direction_output	= sch_gpio_direction_out,
+	.set			= sch_gpio_set,
 };
 
 static int sch_gpio_probe(struct platform_device *pdev)
 {
+	struct sch_gpio *sch;
 	struct resource *res;
-	int err, id;
 
-	id = pdev->id;
-	if (!id)
-		return -ENODEV;
+	sch = devm_kzalloc(&pdev->dev, sizeof(*sch), GFP_KERNEL);
+	if (!sch)
+		return -ENOMEM;
 
 	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
 	if (!res)
 		return -EBUSY;
 
-	if (!request_region(res->start, resource_size(res), pdev->name))
+	if (!devm_request_region(&pdev->dev, res->start, resource_size(res),
+				 pdev->name))
 		return -EBUSY;
 
-	gpio_ba = res->start;
+	spin_lock_init(&sch->lock);
+	sch->iobase = res->start;
+	sch->chip = sch_gpio_chip;
+	sch->chip.label = dev_name(&pdev->dev);
+	sch->chip.dev = &pdev->dev;
 
-	switch (id) {
+	switch (pdev->id) {
 	case PCI_DEVICE_ID_INTEL_SCH_LPC:
-		sch_gpio_core.base = 0;
-		sch_gpio_core.ngpio = 10;
-		sch_gpio_resume.base = 10;
-		sch_gpio_resume.ngpio = 4;
+		sch->core_base = 0;
+		sch->resume_base = 10;
+		sch->chip.ngpio = 14;
+
 		/*
 		 * GPIO[6:0] enabled by default
 		 * GPIO7 is configured by the CMC as SLPIOVR
 		 * Enable GPIO[9:8] core powered gpios explicitly
 		 */
-		outb(0x3, gpio_ba + CGEN + 1);
+		sch_gpio_enable(sch, 8);
+		sch_gpio_enable(sch, 9);
 		/*
 		 * SUS_GPIO[2:0] enabled by default
 		 * Enable SUS_GPIO3 resume powered gpio explicitly
 		 */
-		outb(0x8, gpio_ba + RGEN);
+		sch_gpio_enable(sch, 13);
 		break;
 
 	case PCI_DEVICE_ID_INTEL_ITC_LPC:
-		sch_gpio_core.base = 0;
-		sch_gpio_core.ngpio = 5;
-		sch_gpio_resume.base = 5;
-		sch_gpio_resume.ngpio = 9;
+		sch->core_base = 0;
+		sch->resume_base = 5;
+		sch->chip.ngpio = 14;
 		break;
 
 	case PCI_DEVICE_ID_INTEL_CENTERTON_ILB:
-		sch_gpio_core.base = 0;
-		sch_gpio_core.ngpio = 21;
-		sch_gpio_resume.base = 21;
-		sch_gpio_resume.ngpio = 9;
+		sch->core_base = 0;
+		sch->resume_base = 21;
+		sch->chip.ngpio = 30;
 		break;
 
 	default:
-		err = -ENODEV;
-		goto err_sch_gpio_core;
+		return -ENODEV;
 	}
 
-	sch_gpio_core.dev = &pdev->dev;
-	sch_gpio_resume.dev = &pdev->dev;
-
-	err = gpiochip_add(&sch_gpio_core);
-	if (err < 0)
-		goto err_sch_gpio_core;
+	platform_set_drvdata(pdev, sch);
 
-	err = gpiochip_add(&sch_gpio_resume);
-	if (err < 0)
-		goto err_sch_gpio_resume;
-
-	return 0;
-
-err_sch_gpio_resume:
-	gpiochip_remove(&sch_gpio_core);
-
-err_sch_gpio_core:
-	release_region(res->start, resource_size(res));
-	gpio_ba = 0;
-
-	return err;
+	return gpiochip_add(&sch->chip);
 }
 
 static int sch_gpio_remove(struct platform_device *pdev)
 {
-	struct resource *res;
-	if (gpio_ba) {
-
-		gpiochip_remove(&sch_gpio_core);
-		gpiochip_remove(&sch_gpio_resume);
-
-		res = platform_get_resource(pdev, IORESOURCE_IO, 0);
-
-		release_region(res->start, resource_size(res));
-		gpio_ba = 0;
-	}
+	struct sch_gpio *sch = platform_get_drvdata(pdev);
 
+	gpiochip_remove(&sch->chip);
 	return 0;
 }
 
-- 
1.9.3



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

* [PATCH v6 07/12] leds: leds-gpio: Add support for GPIO descriptors
  2014-10-21 21:08 [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
                   ` (5 preceding siblings ...)
  2014-10-21 21:21 ` [PATCH v6 06/12] gpio: sch: Consolidate core and resume banks Rafael J. Wysocki
@ 2014-10-21 21:21 ` Rafael J. Wysocki
  2014-10-21 21:22 ` [PATCH v6 08/12] input: gpio_keys_polled: " Rafael J. Wysocki
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-10-21 21:21 UTC (permalink / raw)
  To: Linux Kernel Mailing List
  Cc: ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Mika Westerberg, Aaron Lu, Grant Likely, Arnd Bergmann,
	devicetree, Linus Walleij, Alexandre Courbot, Dmitry Torokhov,
	Bryan Wu

From: Mika Westerberg <mika.westerberg@linux.intel.com>

GPIO descriptors are the preferred way over legacy GPIO numbers
nowadays. Convert the driver to use GPIO descriptors internally but
still allow passing legacy GPIO numbers from platform data to support
existing platforms.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Bryan Wu <cooloney@gmail.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/leds/leds-gpio.c |   80 ++++++++++++++++++++++++++---------------------
 include/linux/leds.h     |    1 
 2 files changed, 46 insertions(+), 35 deletions(-)

Index: linux-pm/drivers/leds/leds-gpio.c
===================================================================
--- linux-pm.orig/drivers/leds/leds-gpio.c
+++ linux-pm/drivers/leds/leds-gpio.c
@@ -13,6 +13,7 @@
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/leds.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
@@ -24,11 +25,10 @@
 
 struct gpio_led_data {
 	struct led_classdev cdev;
-	unsigned gpio;
+	struct gpio_desc *gpiod;
 	struct work_struct work;
 	u8 new_level;
 	u8 can_sleep;
-	u8 active_low;
 	u8 blinking;
 	int (*platform_gpio_blink_set)(unsigned gpio, int state,
 			unsigned long *delay_on, unsigned long *delay_off);
@@ -40,12 +40,16 @@ static void gpio_led_work(struct work_st
 		container_of(work, struct gpio_led_data, work);
 
 	if (led_dat->blinking) {
-		led_dat->platform_gpio_blink_set(led_dat->gpio,
-						 led_dat->new_level,
-						 NULL, NULL);
+		int gpio = desc_to_gpio(led_dat->gpiod);
+		int level = led_dat->new_level;
+
+		if (gpiod_is_active_low(led_dat->gpiod))
+			level = !level;
+
+		led_dat->platform_gpio_blink_set(gpio, level, NULL, NULL);
 		led_dat->blinking = 0;
 	} else
-		gpio_set_value_cansleep(led_dat->gpio, led_dat->new_level);
+		gpiod_set_value_cansleep(led_dat->gpiod, led_dat->new_level);
 }
 
 static void gpio_led_set(struct led_classdev *led_cdev,
@@ -60,9 +64,6 @@ static void gpio_led_set(struct led_clas
 	else
 		level = 1;
 
-	if (led_dat->active_low)
-		level = !level;
-
 	/* Setting GPIOs with I2C/etc requires a task context, and we don't
 	 * seem to have a reliable way to know if we're already in one; so
 	 * let's just assume the worst.
@@ -72,11 +73,16 @@ static void gpio_led_set(struct led_clas
 		schedule_work(&led_dat->work);
 	} else {
 		if (led_dat->blinking) {
-			led_dat->platform_gpio_blink_set(led_dat->gpio, level,
-							 NULL, NULL);
+			int gpio = desc_to_gpio(led_dat->gpiod);
+
+			if (gpiod_is_active_low(led_dat->gpiod))
+				level = !level;
+
+			led_dat->platform_gpio_blink_set(gpio, level, NULL,
+							 NULL);
 			led_dat->blinking = 0;
 		} else
-			gpio_set_value(led_dat->gpio, level);
+			gpiod_set_value(led_dat->gpiod, level);
 	}
 }
 
@@ -85,9 +91,10 @@ static int gpio_blink_set(struct led_cla
 {
 	struct gpio_led_data *led_dat =
 		container_of(led_cdev, struct gpio_led_data, cdev);
+	int gpio = desc_to_gpio(led_dat->gpiod);
 
 	led_dat->blinking = 1;
-	return led_dat->platform_gpio_blink_set(led_dat->gpio, GPIO_LED_BLINK,
+	return led_dat->platform_gpio_blink_set(gpio, GPIO_LED_BLINK,
 						delay_on, delay_off);
 }
 
@@ -97,24 +104,33 @@ static int create_gpio_led(const struct
 {
 	int ret, state;
 
-	led_dat->gpio = -1;
+	if (!template->gpiod) {
+		unsigned long flags = 0;
 
-	/* skip leds that aren't available */
-	if (!gpio_is_valid(template->gpio)) {
-		dev_info(parent, "Skipping unavailable LED gpio %d (%s)\n",
-				template->gpio, template->name);
-		return 0;
+		/* skip leds that aren't available */
+		if (!gpio_is_valid(template->gpio)) {
+			dev_info(parent, "Skipping unavailable LED gpio %d (%s)\n",
+					template->gpio, template->name);
+			return 0;
+		}
+
+		if (template->active_low)
+			flags |= GPIOF_ACTIVE_LOW;
+
+		ret = devm_gpio_request_one(parent, template->gpio, flags,
+					    template->name);
+		if (ret < 0)
+			return ret;
+
+		led_dat->gpiod = gpio_to_desc(template->gpio);
+		if (IS_ERR(led_dat->gpiod))
+			return PTR_ERR(led_dat->gpiod);
 	}
 
-	ret = devm_gpio_request(parent, template->gpio, template->name);
-	if (ret < 0)
-		return ret;
-
 	led_dat->cdev.name = template->name;
 	led_dat->cdev.default_trigger = template->default_trigger;
-	led_dat->gpio = template->gpio;
-	led_dat->can_sleep = gpio_cansleep(template->gpio);
-	led_dat->active_low = template->active_low;
+	led_dat->gpiod = template->gpiod;
+	led_dat->can_sleep = gpiod_cansleep(template->gpiod);
 	led_dat->blinking = 0;
 	if (blink_set) {
 		led_dat->platform_gpio_blink_set = blink_set;
@@ -122,30 +138,24 @@ static int create_gpio_led(const struct
 	}
 	led_dat->cdev.brightness_set = gpio_led_set;
 	if (template->default_state == LEDS_GPIO_DEFSTATE_KEEP)
-		state = !!gpio_get_value_cansleep(led_dat->gpio) ^ led_dat->active_low;
+		state = !!gpiod_get_value_cansleep(led_dat->gpiod);
 	else
 		state = (template->default_state == LEDS_GPIO_DEFSTATE_ON);
 	led_dat->cdev.brightness = state ? LED_FULL : LED_OFF;
 	if (!template->retain_state_suspended)
 		led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
 
-	ret = gpio_direction_output(led_dat->gpio, led_dat->active_low ^ state);
+	ret = gpiod_direction_output(led_dat->gpiod, state);
 	if (ret < 0)
 		return ret;
 
 	INIT_WORK(&led_dat->work, gpio_led_work);
 
-	ret = led_classdev_register(parent, &led_dat->cdev);
-	if (ret < 0)
-		return ret;
-
-	return 0;
+	return led_classdev_register(parent, &led_dat->cdev);
 }
 
 static void delete_gpio_led(struct gpio_led_data *led)
 {
-	if (!gpio_is_valid(led->gpio))
-		return;
 	led_classdev_unregister(&led->cdev);
 	cancel_work_sync(&led->work);
 }
Index: linux-pm/include/linux/leds.h
===================================================================
--- linux-pm.orig/include/linux/leds.h
+++ linux-pm/include/linux/leds.h
@@ -251,6 +251,7 @@ struct gpio_led {
 	unsigned	retain_state_suspended : 1;
 	unsigned	default_state : 2;
 	/* default_state should be one of LEDS_GPIO_DEFSTATE_(ON|OFF|KEEP) */
+	struct gpio_desc *gpiod;
 };
 #define LEDS_GPIO_DEFSTATE_OFF		0
 #define LEDS_GPIO_DEFSTATE_ON		1


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

* [PATCH v6 08/12] input: gpio_keys_polled: Add support for GPIO descriptors
  2014-10-21 21:08 [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
                   ` (6 preceding siblings ...)
  2014-10-21 21:21 ` [PATCH v6 07/12] leds: leds-gpio: Add support for GPIO descriptors Rafael J. Wysocki
@ 2014-10-21 21:22 ` Rafael J. Wysocki
  2014-10-21 21:29 ` [PATCH v6 09/12] Driver core: Unified interface for firmware node properties Rafael J. Wysocki
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-10-21 21:22 UTC (permalink / raw)
  To: Linux Kernel Mailing List
  Cc: ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Mika Westerberg, Aaron Lu, Grant Likely, Arnd Bergmann,
	devicetree, Linus Walleij, Alexandre Courbot, Dmitry Torokhov,
	Bryan Wu

From: Aaron Lu <aaron.lu@intel.com>

GPIO descriptors are the preferred way over legacy GPIO numbers
nowadays. Convert the driver to use GPIO descriptors internally but
still allow passing legacy GPIO numbers from platform data to support
existing platforms.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/input/keyboard/gpio_keys_polled.c | 39 +++++++++++++++++++++----------
 include/linux/gpio_keys.h                 |  3 +++
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 432d363..b7a514c 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -23,6 +23,7 @@
 #include <linux/ioport.h>
 #include <linux/platform_device.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/gpio_keys.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
@@ -51,15 +52,14 @@ static void gpio_keys_polled_check_state(struct input_dev *input,
 	int state;
 
 	if (bdata->can_sleep)
-		state = !!gpio_get_value_cansleep(button->gpio);
+		state = !!gpiod_get_value_cansleep(button->gpiod);
 	else
-		state = !!gpio_get_value(button->gpio);
+		state = !!gpiod_get_value(button->gpiod);
 
 	if (state != bdata->last_state) {
 		unsigned int type = button->type ?: EV_KEY;
 
-		input_event(input, type, button->code,
-			    !!(state ^ button->active_low));
+		input_event(input, type, button->code, state);
 		input_sync(input);
 		bdata->count = 0;
 		bdata->last_state = state;
@@ -259,7 +259,6 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 	for (i = 0; i < pdata->nbuttons; i++) {
 		struct gpio_keys_button *button = &pdata->buttons[i];
 		struct gpio_keys_button_data *bdata = &bdev->data[i];
-		unsigned int gpio = button->gpio;
 		unsigned int type = button->type ?: EV_KEY;
 
 		if (button->wakeup) {
@@ -267,15 +266,31 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 			return -EINVAL;
 		}
 
-		error = devm_gpio_request_one(&pdev->dev, gpio, GPIOF_IN,
-					      button->desc ? : DRV_NAME);
-		if (error) {
-			dev_err(dev, "unable to claim gpio %u, err=%d\n",
-				gpio, error);
-			return error;
+		/*
+		 * Legacy GPIO number so request the GPIO here and
+		 * convert it to descriptor.
+		 */
+		if (!button->gpiod && gpio_is_valid(button->gpio)) {
+			unsigned flags = 0;
+
+			if (button->active_low)
+				flags |= GPIOF_ACTIVE_LOW;
+
+			error = devm_gpio_request_one(&pdev->dev, button->gpio,
+					flags, button->desc ? : DRV_NAME);
+			if (error) {
+				dev_err(dev, "unable to claim gpio %u, err=%d\n",
+					button->gpio, error);
+				return error;
+			}
+
+			button->gpiod = gpio_to_desc(button->gpio);
 		}
 
-		bdata->can_sleep = gpio_cansleep(gpio);
+		if (IS_ERR(button->gpiod))
+			return PTR_ERR(button->gpiod);
+
+		bdata->can_sleep = gpiod_cansleep(button->gpiod);
 		bdata->last_state = -1;
 		bdata->threshold = DIV_ROUND_UP(button->debounce_interval,
 						pdata->poll_interval);
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index 8b62246..ee2d8c6 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -2,6 +2,7 @@
 #define _GPIO_KEYS_H
 
 struct device;
+struct gpio_desc;
 
 /**
  * struct gpio_keys_button - configuration parameters
@@ -17,6 +18,7 @@ struct device;
  *			disable button via sysfs
  * @value:		axis value for %EV_ABS
  * @irq:		Irq number in case of interrupt keys
+ * @gpiod:		GPIO descriptor
  */
 struct gpio_keys_button {
 	unsigned int code;
@@ -29,6 +31,7 @@ struct gpio_keys_button {
 	bool can_disable;
 	int value;
 	unsigned int irq;
+	struct gpio_desc *gpiod;
 };
 
 /**
-- 
1.9.3



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

* [PATCH v6 09/12] Driver core: Unified interface for firmware node properties
  2014-10-21 21:08 [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
                   ` (7 preceding siblings ...)
  2014-10-21 21:22 ` [PATCH v6 08/12] input: gpio_keys_polled: " Rafael J. Wysocki
@ 2014-10-21 21:29 ` Rafael J. Wysocki
  2014-11-04 16:43   ` [Update][PATCH " Rafael J. Wysocki
  2014-10-21 21:33 ` [PATCH v6 10/12] gpio: Support for unified device properties interface Rafael J. Wysocki
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-10-21 21:29 UTC (permalink / raw)
  To: Linux Kernel Mailing List, Grant Likely, Arnd Bergmann
  Cc: ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Mika Westerberg, Aaron Lu, devicetree, Linus Walleij,
	Alexandre Courbot, Dmitry Torokhov, Bryan Wu

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Add new generic routines are provided for retrieving properties from
device description objects in the platform firmware in case there are
no struct device objects for them (either those objects have not been
created yet or they do not exist at all).

The following functions are provided:

fwnode_property_present()
fwnode_property_read_u8()
fwnode_property_read_u16()
fwnode_property_read_u32()
fwnode_property_read_u64()
fwnode_property_read_string()
fwnode_property_read_u8_array()
fwnode_property_read_u16_array()
fwnode_property_read_u32_array()
fwnode_property_read_u64_array()
fwnode_property_read_string_array()

in analogy with the corresponding functions for struct device added
previously.  For all of them, the first argument is a pointer to struct
fwnode_handle (new type) that allows a device description object
(depending on what platform firmware interface is in use) to be
obtained.

Add a new macro device_for_each_child_node() for iterating over the
children of the device description object associated with a given
device and a new function device_get_child_node_count() returning the
number of a given device's child nodes.

The interface covers both ACPI and Device Trees.

Suggested-by: Grant Likely <grant.likely@linaro.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Changes from v5:
- There's a new macro for generating the bodies of fwnode_property_read_u*_array().
- fwnode_property_read_u*() are implemented using fwnode_property_read_u*_array().
- fwnode_property_read_bool() is a new static inline wrapper around
  fwnode_property_present().
- IS_ENABLED(CONFIG_ACPI) is used in device_get_next_child_node() instead of
  ACPI_COMPANION(dev), because acpi_get_next_child() returns NULL if
  ACPI_COMPANION(dev) is NULL.

---
 drivers/acpi/scan.c      |   21 +++
 drivers/base/property.c  |  298 +++++++++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h  |   17 ++
 include/linux/acpi.h     |   26 ++++
 include/linux/of.h       |   22 +++
 include/linux/property.h |   54 ++++++++
 6 files changed, 438 insertions(+)

Index: linux-pm/include/linux/property.h
===================================================================
--- linux-pm.orig/include/linux/property.h
+++ linux-pm/include/linux/property.h
@@ -44,10 +44,64 @@ int device_property_read_u64(struct devi
 int device_property_read_string(struct device *dev, const char *propname,
 				const char **val);
 
+enum fwnode_type {
+	FWNODE_INVALID = 0,
+	FWNODE_OF,
+	FWNODE_ACPI,
+};
+
+struct fwnode_handle {
+	enum fwnode_type type;
+};
+
+bool fwnode_property_present(struct fwnode_handle *fwnode, const char *propname);
+int fwnode_property_read_u8_array(struct fwnode_handle *fwnode,
+				  const char *propname, u8 *val,
+				  size_t nval);
+int fwnode_property_read_u16_array(struct fwnode_handle *fwnode,
+				   const char *propname, u16 *val,
+				   size_t nval);
+int fwnode_property_read_u32_array(struct fwnode_handle *fwnode,
+				   const char *propname, u32 *val,
+				   size_t nval);
+int fwnode_property_read_u64_array(struct fwnode_handle *fwnode,
+				   const char *propname, u64 *val,
+				   size_t nval);
+int fwnode_property_read_string_array(struct fwnode_handle *fwnode,
+				      const char *propname, char **val,
+				      size_t nval);
+int fwnode_property_read_u8(struct fwnode_handle *fwnode, const char *propname,
+			    u8 *val);
+int fwnode_property_read_u16(struct fwnode_handle *fwnode, const char *propname,
+			     u16 *val);
+int fwnode_property_read_u32(struct fwnode_handle *fwnode, const char *propname,
+			     u32 *val);
+int fwnode_property_read_u64(struct fwnode_handle *fwnode, const char *propname,
+			     u64 *val);
+int fwnode_property_read_string(struct fwnode_handle *fwnode,
+				const char *propname, const char **val);
+
+struct fwnode_handle *device_get_next_child_node(struct device *dev,
+						 struct fwnode_handle *child);
+
+#define device_for_each_child_node(dev, child) \
+	for (child = device_get_next_child_node(dev, NULL); child; \
+	     child = device_get_next_child_node(dev, child))
+
+void fwnode_handle_put(struct fwnode_handle *fwnode);
+
+unsigned int device_get_child_node_count(struct device *dev);
+
 static inline bool device_property_read_bool(struct device *dev,
 					     const char *propname)
 {
 	return device_property_present(dev, propname);
 }
 
+static inline bool fwnode_property_read_bool(struct fwnode_handle *fwnode,
+					     const char *propname)
+{
+	return fwnode_property_present(fwnode, propname);
+}
+
 #endif /* _LINUX_PROPERTY_H_ */
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -27,6 +27,7 @@
 #define __ACPI_BUS_H__
 
 #include <linux/device.h>
+#include <linux/property.h>
 
 /* TBD: Make dynamic */
 #define ACPI_MAX_HANDLES	10
@@ -348,6 +349,7 @@ struct acpi_device_data {
 struct acpi_device {
 	int device_type;
 	acpi_handle handle;		/* no handle for fixed hardware */
+	struct fwnode_handle fwnode;
 	struct acpi_device *parent;
 	struct list_head children;
 	struct list_head node;
@@ -372,6 +374,21 @@ struct acpi_device {
 	void (*remove)(struct acpi_device *);
 };
 
+static inline bool is_acpi_node(struct fwnode_handle *fwnode)
+{
+	return fwnode && fwnode->type == FWNODE_ACPI;
+}
+
+static inline struct acpi_device *acpi_node(struct fwnode_handle *fwnode)
+{
+	return fwnode ? container_of(fwnode, struct acpi_device, fwnode) : NULL;
+}
+
+static inline struct fwnode_handle *acpi_fwnode_handle(struct acpi_device *adev)
+{
+	return &adev->fwnode;
+}
+
 static inline void *acpi_driver_data(struct acpi_device *d)
 {
 	return d->driver_data;
Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -439,6 +439,23 @@ int acpi_device_modalias(struct device *
 #define ACPI_COMPANION_SET(dev, adev)	do { } while (0)
 #define ACPI_HANDLE(dev)		(NULL)
 
+struct fwnode_handle;
+
+static inline bool is_acpi_node(struct fwnode_handle *fwnode)
+{
+	return false;
+}
+
+static inline struct acpi_device *acpi_node(struct fwnode_handle *fwnode)
+{
+	return NULL;
+}
+
+static inline struct fwnode_handle *acpi_fwnode_handle(struct acpi_device *adev)
+{
+	return NULL;
+}
+
 static inline const char *acpi_dev_name(struct acpi_device *adev)
 {
 	return NULL;
@@ -680,6 +697,9 @@ int acpi_dev_prop_read_single(struct acp
 			      enum dev_prop_type proptype, void *val);
 int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
 		       enum dev_prop_type proptype, void *val, size_t nval);
+
+struct acpi_device *acpi_get_next_child(struct device *dev,
+					struct acpi_device *child);
 #else
 static inline int acpi_dev_get_property(struct acpi_device *adev,
 					const char *name, acpi_object_type type,
@@ -724,6 +744,12 @@ static inline int acpi_dev_prop_read(str
 	return -ENXIO;
 }
 
+static inline struct acpi_device *acpi_get_next_child(struct device *dev,
+						      struct acpi_device *child)
+{
+	return NULL;
+}
+
 #endif
 
 #endif	/*_LINUX_ACPI_H*/
Index: linux-pm/include/linux/of.h
===================================================================
--- linux-pm.orig/include/linux/of.h
+++ linux-pm/include/linux/of.h
@@ -50,6 +50,7 @@ struct device_node {
 	const char *type;
 	phandle phandle;
 	const char *full_name;
+	struct fwnode_handle fwnode;
 
 	struct	property *properties;
 	struct	property *deadprops;	/* removed properties */
@@ -80,6 +81,7 @@ extern struct kobj_type of_node_ktype;
 static inline void of_node_init(struct device_node *node)
 {
 	kobject_init(&node->kobj, &of_node_ktype);
+	node->fwnode.type = FWNODE_OF;
 }
 
 /* true when node is initialized */
@@ -115,6 +117,16 @@ extern struct device_node *of_aliases;
 extern struct device_node *of_stdout;
 extern raw_spinlock_t devtree_lock;
 
+static inline bool is_of_node(struct fwnode_handle *fwnode)
+{
+	return fwnode && fwnode->type == FWNODE_OF;
+}
+
+static inline struct device_node *of_node(struct fwnode_handle *fwnode)
+{
+	return fwnode ? container_of(fwnode, struct device_node, fwnode) : NULL;
+}
+
 static inline bool of_have_populated_dt(void)
 {
 	return of_allnodes != NULL;
@@ -365,6 +377,16 @@ bool of_console_check(struct device_node
 
 #else /* CONFIG_OF */
 
+static inline bool is_of_node(struct fwnode_handle *fwnode)
+{
+	return false;
+}
+
+static inline struct device_node *of_node(struct fwnode_handle *fwnode)
+{
+	return NULL;
+}
+
 static inline const char* of_node_full_name(const struct device_node *np)
 {
 	return "<no-node>";
Index: linux-pm/drivers/base/property.c
===================================================================
--- linux-pm.orig/drivers/base/property.c
+++ linux-pm/drivers/base/property.c
@@ -31,6 +31,22 @@ bool device_property_present(struct devi
 }
 EXPORT_SYMBOL_GPL(device_property_present);
 
+/**
+ * fwnode_property_present - check if a property of a firmware node is present
+ * @fwnode: Firmware node whose property to check
+ * @propname: Name of the property
+ */
+bool fwnode_property_present(struct fwnode_handle *fwnode, const char *propname)
+{
+	if (is_of_node(fwnode))
+		return of_property_read_bool(of_node(fwnode), propname);
+	else if (is_acpi_node(fwnode))
+		return !acpi_dev_prop_get(acpi_node(fwnode), propname, NULL);
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(fwnode_property_present);
+
 #define OF_DEV_PROP_READ_ARRAY(node, propname, type, val, nval) \
 	(val) ? of_property_read_##type##_array((node), (propname), (val), (nval)) \
 	      : of_property_count_elems_of_size((node), (propname), sizeof(type))
@@ -231,3 +247,285 @@ int device_property_read_string(struct d
 				   DEV_PROP_STRING, val, 1);
 }
 EXPORT_SYMBOL_GPL(device_property_read_string);
+
+#define FWNODE_PROP_READ_ARRAY(_fwnode_, _propname_, _type_, _proptype_, _val_, _nval_) \
+({ \
+	int _ret_; \
+	if (is_of_node(_fwnode_)) \
+		_ret_ = OF_DEV_PROP_READ_ARRAY(of_node(_fwnode_), _propname_, \
+					       _type_, _val_, _nval_); \
+	else if (is_acpi_node(_fwnode_)) \
+		_ret_ = acpi_dev_prop_read(acpi_node(_fwnode_), _propname_, \
+					   _proptype_, _val_, _nval_); \
+	else \
+		_ret_ = -ENXIO; \
+	_ret_; \
+})
+
+/**
+ * fwnode_property_read_u8_array - return a u8 array property of firmware node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Read an array of u8 properties with @propname from @fwnode and stores them to
+ * @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected,
+ *	   %-ENXIO if no suitable firmware interface is present.
+ */
+int fwnode_property_read_u8_array(struct fwnode_handle *fwnode,
+				  const char *propname, u8 *val, size_t nval)
+{
+	return FWNODE_PROP_READ_ARRAY(fwnode, propname, u8, DEV_PROP_U8,
+				      val, nval);
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_u8_array);
+
+/**
+ * fwnode_property_read_u16_array - return a u16 array property of firmware node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Read an array of u16 properties with @propname from @fwnode and store them to
+ * @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected,
+ *	   %-ENXIO if no suitable firmware interface is present.
+ */
+int fwnode_property_read_u16_array(struct fwnode_handle *fwnode,
+				   const char *propname, u16 *val, size_t nval)
+{
+	return FWNODE_PROP_READ_ARRAY(fwnode, propname, u16, DEV_PROP_U16,
+				      val, nval);
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_u16_array);
+
+/**
+ * fwnode_property_read_u32_array - return a u32 array property of firmware node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Read an array of u32 properties with @propname from @fwnode store them to
+ * @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected,
+ *	   %-ENXIO if no suitable firmware interface is present.
+ */
+int fwnode_property_read_u32_array(struct fwnode_handle *fwnode,
+				   const char *propname, u32 *val, size_t nval)
+{
+	return FWNODE_PROP_READ_ARRAY(fwnode, propname, u32, DEV_PROP_U32,
+				      val, nval);
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_u32_array);
+
+/**
+ * fwnode_property_read_u64_array - return a u64 array property firmware node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Read an array of u64 properties with @propname from @fwnode and store them to
+ * @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected,
+ *	   %-ENXIO if no suitable firmware interface is present.
+ */
+int fwnode_property_read_u64_array(struct fwnode_handle *fwnode,
+				   const char *propname, u64 *val, size_t nval)
+{
+	return FWNODE_PROP_READ_ARRAY(fwnode, propname, u64, DEV_PROP_U64,
+				      val, nval);
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_u64_array);
+
+/**
+ * fwnode_property_read_string_array - return string array property of a node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Read an string list property @propname from the given firmware node and store
+ * them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of strings,
+ *	   %-EOVERFLOW if the size of the property is not as expected,
+ *	   %-ENXIO if no suitable firmware interface is present.
+ */
+int fwnode_property_read_string_array(struct fwnode_handle *fwnode,
+				      const char *propname, char **val,
+				      size_t nval)
+{
+	if (is_of_node(fwnode))
+		return of_property_read_string_array(of_node(fwnode), propname,
+						     val, nval);
+	else if (is_acpi_node(fwnode))
+		return acpi_dev_prop_read(acpi_node(fwnode), propname,
+					  DEV_PROP_STRING, val, nval);
+
+	return -ENXIO;
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_string_array);
+
+/**
+ * fwnode_property_read_u8 - return a u8 property of a firmware node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The value is stored here
+ */
+int fwnode_property_read_u8(struct fwnode_handle *fwnode, const char *propname,
+			    u8 *val)
+{
+	return fwnode_property_read_u8_array(fwnode, propname, val, 1);
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_u8);
+
+/**
+ * fwnode_property_read_u16 - return a u16 property of a firmware node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The value is stored here
+ */
+int fwnode_property_read_u16(struct fwnode_handle *fwnode, const char *propname,
+			     u16 *val)
+{
+	return fwnode_property_read_u16_array(fwnode, propname, val, 1);
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_u16);
+
+/**
+ * fwnode_property_read_u32 - return a u32 property of a firmware node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The value is stored here
+ */
+int fwnode_property_read_u32(struct fwnode_handle *fwnode, const char *propname,
+			     u32 *val)
+{
+	return fwnode_property_read_u32_array(fwnode, propname, val, 1);
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_u32);
+
+/**
+ * fwnode_property_read_u64 - return a u64 property of a firmware node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The value is stored here
+ */
+int fwnode_property_read_u64(struct fwnode_handle *fwnode, const char *propname,
+			     u64 *val)
+{
+	return fwnode_property_read_u64_array(fwnode, propname, val, 1);
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_u64);
+
+/**
+ * fwnode_property_read_string - return a string property of a firmware node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The value is stored here
+ *
+ * Read property @propname from the given firmware node and store the value into
+ * @val if found.  The value is checked to be a string.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO or %-EILSEQ if the property is not a string,
+ *	   %-ENXIO if no suitable firmware interface is present.
+ */
+int fwnode_property_read_string(struct fwnode_handle *fwnode,
+				const char *propname, const char **val)
+{
+	if (is_of_node(fwnode))
+		return of_property_read_string(of_node(fwnode),propname, val);
+	else if (is_acpi_node(fwnode))
+		return acpi_dev_prop_read(acpi_node(fwnode), propname,
+					  DEV_PROP_STRING, val, 1);
+
+	return -ENXIO;
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_string);
+
+/**
+ * device_get_next_child_node - Return the next child node handle for a device
+ * @dev: Device to find the next child node for.
+ * @child: Handle to one of the device's child nodes or a null handle.
+ */
+struct fwnode_handle *device_get_next_child_node(struct device *dev,
+						 struct fwnode_handle *child)
+{
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
+		struct device_node *node;
+
+		node = of_get_next_available_child(dev->of_node, of_node(child));
+		if (node)
+			return &node->fwnode;
+	} else if (IS_ENABLED(CONFIG_ACPI)) {
+		struct acpi_device *node;
+
+		node = acpi_get_next_child(dev, acpi_node(child));
+		if (node)
+			return acpi_fwnode_handle(node);
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(device_get_next_child_node);
+
+/**
+ * fwnode_handle_put - Drop reference to a device node
+ * @fwnode: Pointer to the device node to drop the reference to.
+ *
+ * This has to be used when terminating device_for_each_child_node() iteration
+ * with break or return to prevent stale device node references from being left
+ * behind.
+ */
+void fwnode_handle_put(struct fwnode_handle *fwnode)
+{
+	if (is_of_node(fwnode))
+		of_node_put(of_node(fwnode));
+}
+EXPORT_SYMBOL_GPL(fwnode_handle_put);
+
+/**
+ * device_get_child_node_count - return the number of child nodes for device
+ * @dev: Device to cound the child nodes for
+ */
+unsigned int device_get_child_node_count(struct device *dev)
+{
+	struct fwnode_handle *child;
+	unsigned int count = 0;
+
+	device_for_each_child_node(dev, child)
+		count++;
+
+	return count;
+}
+EXPORT_SYMBOL_GPL(device_get_child_node_count);
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1342,6 +1342,26 @@ int acpi_device_add(struct acpi_device *
 	return result;
 }
 
+struct acpi_device *acpi_get_next_child(struct device *dev,
+					struct acpi_device *child)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+	struct list_head *head, *next;
+
+	if (!adev)
+		return NULL;
+
+	head = &adev->children;
+	if (list_empty(head))
+		return NULL;
+
+	if (!child)
+		return list_first_entry(head, struct acpi_device, node);
+
+	next = child->node.next;
+	return next == head ? NULL : list_entry(next, struct acpi_device, node);
+}
+
 /* --------------------------------------------------------------------------
                                  Driver Management
    -------------------------------------------------------------------------- */
@@ -1961,6 +1981,7 @@ void acpi_init_device_object(struct acpi
 	device->device_type = type;
 	device->handle = handle;
 	device->parent = acpi_bus_get_parent(handle);
+	device->fwnode.type = FWNODE_ACPI;
 	acpi_set_device_status(device, sta);
 	acpi_device_get_busid(device);
 	acpi_set_pnp_ids(handle, &device->pnp, type);


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

* [PATCH v6 10/12] gpio: Support for unified device properties interface
  2014-10-21 21:08 [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
                   ` (8 preceding siblings ...)
  2014-10-21 21:29 ` [PATCH v6 09/12] Driver core: Unified interface for firmware node properties Rafael J. Wysocki
@ 2014-10-21 21:33 ` Rafael J. Wysocki
  2014-10-21 21:35 ` [PATCH v6 11/12] leds: leds-gpio: Make use of device property API Rafael J. Wysocki
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-10-21 21:33 UTC (permalink / raw)
  To: Linux Kernel Mailing List, Arnd Bergmann
  Cc: ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Mika Westerberg, Aaron Lu, Grant Likely, devicetree,
	Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu

From: Mika Westerberg <mika.westerberg@linux.intel.com>

Some drivers need to deal with only firmware representation of its
GPIOs. An example would be a GPIO button array driver where each button
is described as a separate firmware node in device tree. Typically these
child nodes do not have physical representation in the Linux device
model.

In order to help device drivers to handle such firmware child nodes we
add dev[m]_get_named_gpiod_from_child() that takes a child firmware
node pointer as its second argument (the first one is the parent device
itself), finds the GPIO using whatever is the underlying firmware
method, and requests the GPIO properly.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Changes from v5:
- fwnode_get_named_gpiod() takes 2 arguments (instead of 3; index has been
  dropped).
- devm_get_gpiod_from_child() (renamed from devm_get_named_gpiod_from_child())
  takes 2 arguments (instead of 4; the name and index have been dropped).  It
  passes "gpios" to fwnode_get_named_gpiod() as the name, because both its
  callers in the subsequent patches would do that anyway.

---
 drivers/gpio/devres.c         |   32 ++++++++++++++++++++++++
 drivers/gpio/gpiolib.c        |   55 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/gpio/consumer.h |    7 +++++
 3 files changed, 94 insertions(+)

Index: linux-pm/drivers/gpio/devres.c
===================================================================
--- linux-pm.orig/drivers/gpio/devres.c
+++ linux-pm/drivers/gpio/devres.c
@@ -109,6 +109,38 @@ struct gpio_desc *__must_check __devm_gp
 EXPORT_SYMBOL(__devm_gpiod_get_index);
 
 /**
+ * devm_get_gpiod_from_child - get a GPIO descriptor from a device's child node
+ * @dev:	GPIO consumer
+ * @child:	firmware node (child of @dev)
+ *
+ * GPIO descriptors returned from this function are automatically disposed on
+ * driver detach.
+ */
+struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
+					    struct fwnode_handle *child)
+{
+	struct gpio_desc **dr;
+	struct gpio_desc *desc;
+
+	dr = devres_alloc(devm_gpiod_release, sizeof(struct gpio_desc *),
+			  GFP_KERNEL);
+	if (!dr)
+		return ERR_PTR(-ENOMEM);
+
+	desc = fwnode_get_named_gpiod(child, "gpios");
+	if (IS_ERR(desc)) {
+		devres_free(dr);
+		return desc;
+	}
+
+	*dr = desc;
+	devres_add(dev, dr);
+
+	return desc;
+}
+EXPORT_SYMBOL(devm_get_gpiod_from_child);
+
+/**
  * devm_gpiod_get_index_optional - Resource-managed gpiod_get_index_optional()
  * @dev: GPIO consumer
  * @con_id: function within the GPIO consumer
Index: linux-pm/drivers/gpio/gpiolib.c
===================================================================
--- linux-pm.orig/drivers/gpio/gpiolib.c
+++ linux-pm/drivers/gpio/gpiolib.c
@@ -1735,6 +1735,61 @@ struct gpio_desc *__must_check __gpiod_g
 EXPORT_SYMBOL_GPL(__gpiod_get_index);
 
 /**
+ * fwnode_get_named_gpiod - obtain a GPIO from firmware node
+ * @fwnode:	handle of the firmware node
+ * @propname:	name of the firmware property representing the GPIO
+ *
+ * This function can be used for drivers that get their configuration
+ * from firmware.
+ *
+ * Function properly finds the corresponding GPIO using whatever is the
+ * underlying firmware interface and then makes sure that the GPIO
+ * descriptor is requested before it is returned to the caller.
+ *
+ * In case of error an ERR_PTR() is returned.
+ */
+struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
+					 const char *propname)
+{
+	struct gpio_desc *desc = ERR_PTR(-ENODEV);
+	bool active_low = false;
+	int ret;
+
+	if (!fwnode)
+		return ERR_PTR(-EINVAL);
+
+	if (is_of_node(fwnode)) {
+		enum of_gpio_flags flags;
+
+		desc = of_get_named_gpiod_flags(of_node(fwnode), propname, 0,
+						&flags);
+		if (!IS_ERR(desc))
+			active_low = flags & OF_GPIO_ACTIVE_LOW;
+	} else if (is_acpi_node(fwnode)) {
+		struct acpi_gpio_info info;
+
+		desc = acpi_get_gpiod_by_index(acpi_node(fwnode), propname, 0,
+					       &info);
+		if (!IS_ERR(desc))
+			active_low = info.active_low;
+	}
+
+	if (IS_ERR(desc))
+		return desc;
+
+	ret = gpiod_request(desc, NULL);
+	if (ret)
+		return ERR_PTR(ret);
+
+	/* Only value flag can be set from both DT and ACPI is active_low */
+	if (active_low)
+		set_bit(FLAG_ACTIVE_LOW, &desc->flags);
+
+	return desc;
+}
+EXPORT_SYMBOL_GPL(fwnode_get_named_gpiod);
+
+/**
  * gpiod_get_index_optional - obtain an optional GPIO from a multi-index GPIO
  *                            function
  * @dev: GPIO consumer, can be NULL for system-global GPIOs
Index: linux-pm/include/linux/gpio/consumer.h
===================================================================
--- linux-pm.orig/include/linux/gpio/consumer.h
+++ linux-pm/include/linux/gpio/consumer.h
@@ -94,6 +94,13 @@ int gpiod_to_irq(const struct gpio_desc
 struct gpio_desc *gpio_to_desc(unsigned gpio);
 int desc_to_gpio(const struct gpio_desc *desc);
 
+/* Child properties interface */
+struct fwnode_handle;
+
+struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
+					 const char *propname);
+struct gpio_desc *devm_get_gpiod_from_child(struct device *dev,
+					    struct fwnode_handle *child);
 #else /* CONFIG_GPIOLIB */
 
 static inline struct gpio_desc *__must_check __gpiod_get(struct device *dev,


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

* [PATCH v6 11/12] leds: leds-gpio: Make use of device property API
  2014-10-21 21:08 [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
                   ` (9 preceding siblings ...)
  2014-10-21 21:33 ` [PATCH v6 10/12] gpio: Support for unified device properties interface Rafael J. Wysocki
@ 2014-10-21 21:35 ` Rafael J. Wysocki
  2014-10-21 21:37 ` [PATCH v6 12/12] input: gpio_keys_polled: " Rafael J. Wysocki
  2014-10-24 22:10 ` [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
  12 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-10-21 21:35 UTC (permalink / raw)
  To: Linux Kernel Mailing List
  Cc: ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Mika Westerberg, Aaron Lu, Grant Likely, Arnd Bergmann,
	devicetree, Linus Walleij, Alexandre Courbot, Dmitry Torokhov,
	Bryan Wu

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Make use of device property API in this driver so that both OF and ACPI
based system can use the same driver.

This change contains material from Max Eliaser and Mika Westerberg.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Bryan Wu <cooloney@gmail.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Changes from v5:
- Use devm_get_gpiod_from_child() with 2 arguments (dev, child).

---
 drivers/leds/leds-gpio.c |   62 ++++++++++++++++++-----------------------------
 1 file changed, 25 insertions(+), 37 deletions(-)

Index: linux-pm/drivers/leds/leds-gpio.c
===================================================================
--- linux-pm.orig/drivers/leds/leds-gpio.c
+++ linux-pm/drivers/leds/leds-gpio.c
@@ -15,13 +15,11 @@
 #include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
 #include <linux/leds.h>
-#include <linux/of.h>
-#include <linux/of_platform.h>
-#include <linux/of_gpio.h>
 #include <linux/slab.h>
 #include <linux/workqueue.h>
 #include <linux/module.h>
 #include <linux/err.h>
+#include <linux/property.h>
 
 struct gpio_led_data {
 	struct led_classdev cdev;
@@ -171,40 +169,37 @@ static inline int sizeof_gpio_leds_priv(
 		(sizeof(struct gpio_led_data) * num_leds);
 }
 
-/* Code to create from OpenFirmware platform devices */
-#ifdef CONFIG_OF_GPIO
-static struct gpio_leds_priv *gpio_leds_create_of(struct platform_device *pdev)
+static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
 {
-	struct device_node *np = pdev->dev.of_node, *child;
+	struct device *dev = &pdev->dev;
+	struct fwnode_handle *child;
 	struct gpio_leds_priv *priv;
 	int count, ret;
 
-	/* count LEDs in this device, so we know how much to allocate */
-	count = of_get_available_child_count(np);
+	count = device_get_child_node_count(dev);
 	if (!count)
 		return ERR_PTR(-ENODEV);
 
-	for_each_available_child_of_node(np, child)
-		if (of_get_gpio(child, 0) == -EPROBE_DEFER)
-			return ERR_PTR(-EPROBE_DEFER);
-
-	priv = devm_kzalloc(&pdev->dev, sizeof_gpio_leds_priv(count),
-			GFP_KERNEL);
+	priv = devm_kzalloc(dev, sizeof_gpio_leds_priv(count), GFP_KERNEL);
 	if (!priv)
 		return ERR_PTR(-ENOMEM);
 
-	for_each_available_child_of_node(np, child) {
+	device_for_each_child_node(dev, child) {
 		struct gpio_led led = {};
-		enum of_gpio_flags flags;
-		const char *state;
+		const char *state = NULL;
+
+		led.gpiod = devm_get_gpiod_from_child(dev, child);
+		if (IS_ERR(led.gpiod)) {
+			fwnode_handle_put(child);
+			goto err;
+		}
+
+		fwnode_property_read_string(child, "label", &led.name);
+		fwnode_property_read_string(child, "linux,default-trigger",
+					    &led.default_trigger);
 
-		led.gpio = of_get_gpio_flags(child, 0, &flags);
-		led.active_low = flags & OF_GPIO_ACTIVE_LOW;
-		led.name = of_get_property(child, "label", NULL) ? : child->name;
-		led.default_trigger =
-			of_get_property(child, "linux,default-trigger", NULL);
-		state = of_get_property(child, "default-state", NULL);
-		if (state) {
+		if (!fwnode_property_read_string(child, "linux,default_state",
+						 &state)) {
 			if (!strcmp(state, "keep"))
 				led.default_state = LEDS_GPIO_DEFSTATE_KEEP;
 			else if (!strcmp(state, "on"))
@@ -213,13 +208,13 @@ static struct gpio_leds_priv *gpio_leds_
 				led.default_state = LEDS_GPIO_DEFSTATE_OFF;
 		}
 
-		if (of_get_property(child, "retain-state-suspended", NULL))
+		if (fwnode_property_present(child, "retain-state-suspended"))
 			led.retain_state_suspended = 1;
 
 		ret = create_gpio_led(&led, &priv->leds[priv->num_leds++],
-				      &pdev->dev, NULL);
+				      dev, NULL);
 		if (ret < 0) {
-			of_node_put(child);
+			fwnode_handle_put(child);
 			goto err;
 		}
 	}
@@ -238,13 +233,6 @@ static const struct of_device_id of_gpio
 };
 
 MODULE_DEVICE_TABLE(of, of_gpio_leds_match);
-#else /* CONFIG_OF_GPIO */
-static struct gpio_leds_priv *gpio_leds_create_of(struct platform_device *pdev)
-{
-	return ERR_PTR(-ENODEV);
-}
-#endif /* CONFIG_OF_GPIO */
-
 
 static int gpio_led_probe(struct platform_device *pdev)
 {
@@ -273,7 +261,7 @@ static int gpio_led_probe(struct platfor
 			}
 		}
 	} else {
-		priv = gpio_leds_create_of(pdev);
+		priv = gpio_leds_create(pdev);
 		if (IS_ERR(priv))
 			return PTR_ERR(priv);
 	}
@@ -300,7 +288,7 @@ static struct platform_driver gpio_led_d
 	.driver		= {
 		.name	= "leds-gpio",
 		.owner	= THIS_MODULE,
-		.of_match_table = of_match_ptr(of_gpio_leds_match),
+		.of_match_table = of_gpio_leds_match,
 	},
 };
 


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

* [PATCH v6 12/12] input: gpio_keys_polled: Make use of device property API
  2014-10-21 21:08 [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
                   ` (10 preceding siblings ...)
  2014-10-21 21:35 ` [PATCH v6 11/12] leds: leds-gpio: Make use of device property API Rafael J. Wysocki
@ 2014-10-21 21:37 ` Rafael J. Wysocki
  2014-10-24 22:10 ` [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
  12 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-10-21 21:37 UTC (permalink / raw)
  To: Linux Kernel Mailing List
  Cc: ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Mika Westerberg, Aaron Lu, Grant Likely, Arnd Bergmann,
	devicetree, Linus Walleij, Alexandre Courbot, Dmitry Torokhov,
	Bryan Wu

From: Aaron Lu <aaron.lu@intel.com>

Make use of device property API in this driver so that both OF based
system and ACPI based system can use this driver.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Changes from v5:
- Use devm_get_gpiod_from_child() with 2 arguments (dev, child).

---
 drivers/input/keyboard/gpio_keys_polled.c |   73 +++++++++---------------------
 1 file changed, 24 insertions(+), 49 deletions(-)

Index: linux-pm/drivers/input/keyboard/gpio_keys_polled.c
===================================================================
--- linux-pm.orig/drivers/input/keyboard/gpio_keys_polled.c
+++ linux-pm/drivers/input/keyboard/gpio_keys_polled.c
@@ -25,9 +25,7 @@
 #include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
 #include <linux/gpio_keys.h>
-#include <linux/of.h>
-#include <linux/of_platform.h>
-#include <linux/of_gpio.h>
+#include <linux/property.h>
 
 #define DRV_NAME	"gpio-keys-polled"
 
@@ -102,21 +100,15 @@ static void gpio_keys_polled_close(struc
 		pdata->disable(bdev->dev);
 }
 
-#ifdef CONFIG_OF
 static struct gpio_keys_platform_data *gpio_keys_polled_get_devtree_pdata(struct device *dev)
 {
-	struct device_node *node, *pp;
 	struct gpio_keys_platform_data *pdata;
 	struct gpio_keys_button *button;
+	struct fwnode_handle *child;
 	int error;
 	int nbuttons;
-	int i;
-
-	node = dev->of_node;
-	if (!node)
-		return NULL;
 
-	nbuttons = of_get_child_count(node);
+	nbuttons = device_get_child_node_count(dev);
 	if (nbuttons == 0)
 		return NULL;
 
@@ -126,52 +118,44 @@ static struct gpio_keys_platform_data *g
 		return ERR_PTR(-ENOMEM);
 
 	pdata->buttons = (struct gpio_keys_button *)(pdata + 1);
-	pdata->nbuttons = nbuttons;
 
-	pdata->rep = !!of_get_property(node, "autorepeat", NULL);
-	of_property_read_u32(node, "poll-interval", &pdata->poll_interval);
+	pdata->rep = device_property_present(dev, "autorepeat");
+	device_property_read_u32(dev, "poll-interval", &pdata->poll_interval);
 
-	i = 0;
-	for_each_child_of_node(node, pp) {
-		int gpio;
-		enum of_gpio_flags flags;
-
-		if (!of_find_property(pp, "gpios", NULL)) {
-			pdata->nbuttons--;
-			dev_warn(dev, "Found button without gpios\n");
-			continue;
-		}
+	device_for_each_child_node(dev, child) {
+		struct gpio_desc *desc;
 
-		gpio = of_get_gpio_flags(pp, 0, &flags);
-		if (gpio < 0) {
-			error = gpio;
+		desc = devm_get_gpiod_from_child(dev, child);
+		if (IS_ERR(desc)) {
+			error = PTR_ERR(desc);
 			if (error != -EPROBE_DEFER)
 				dev_err(dev,
 					"Failed to get gpio flags, error: %d\n",
 					error);
+			fwnode_handle_put(child);
 			return ERR_PTR(error);
 		}
 
-		button = &pdata->buttons[i++];
-
-		button->gpio = gpio;
-		button->active_low = flags & OF_GPIO_ACTIVE_LOW;
+		button = &pdata->buttons[pdata->nbuttons++];
+		button->gpiod = desc;
 
-		if (of_property_read_u32(pp, "linux,code", &button->code)) {
-			dev_err(dev, "Button without keycode: 0x%x\n",
-				button->gpio);
+		if (fwnode_property_read_u32(child, "linux,code", &button->code)) {
+			dev_err(dev, "Button without keycode: %d\n",
+				pdata->nbuttons - 1);
+			fwnode_handle_put(child);
 			return ERR_PTR(-EINVAL);
 		}
 
-		button->desc = of_get_property(pp, "label", NULL);
+		fwnode_property_read_string(child, "label", &button->desc);
 
-		if (of_property_read_u32(pp, "linux,input-type", &button->type))
+		if (fwnode_property_read_u32(child, "linux,input-type",
+					     &button->type))
 			button->type = EV_KEY;
 
-		button->wakeup = !!of_get_property(pp, "gpio-key,wakeup", NULL);
+		button->wakeup = fwnode_property_present(child, "gpio-key,wakeup");
 
-		if (of_property_read_u32(pp, "debounce-interval",
-					 &button->debounce_interval))
+		if (fwnode_property_read_u32(child, "debounce-interval",
+					     &button->debounce_interval))
 			button->debounce_interval = 5;
 	}
 
@@ -187,15 +171,6 @@ static const struct of_device_id gpio_ke
 };
 MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);
 
-#else
-
-static inline struct gpio_keys_platform_data *
-gpio_keys_polled_get_devtree_pdata(struct device *dev)
-{
-	return NULL;
-}
-#endif
-
 static int gpio_keys_polled_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -323,7 +298,7 @@ static struct platform_driver gpio_keys_
 	.driver	= {
 		.name	= DRV_NAME,
 		.owner	= THIS_MODULE,
-		.of_match_table = of_match_ptr(gpio_keys_polled_of_match),
+		.of_match_table = gpio_keys_polled_of_match,
 	},
 };
 module_platform_driver(gpio_keys_polled_driver);


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

* Re: [PATCH v6 00/12] Add ACPI _DSD and unified device properties support
  2014-10-21 21:08 [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
                   ` (11 preceding siblings ...)
  2014-10-21 21:37 ` [PATCH v6 12/12] input: gpio_keys_polled: " Rafael J. Wysocki
@ 2014-10-24 22:10 ` Rafael J. Wysocki
  2014-11-04 15:49   ` Grant Likely
  12 siblings, 1 reply; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-10-24 22:10 UTC (permalink / raw)
  To: Linux Kernel Mailing List, ACPI Devel Maling List
  Cc: Greg Kroah-Hartman, Darren Hart, Mika Westerberg, Aaron Lu,
	Grant Likely, Arnd Bergmann, devicetree, Linus Walleij,
	Alexandre Courbot, Dmitry Torokhov, Bryan Wu

On Tuesday, October 21, 2014 11:08:59 PM Rafael J. Wysocki wrote:
> Hi Everyone,
> 
> This is version 6 of the unified device properties interface patchset.
> 
> The original cover letter from Mika is here:
> 
> http://marc.info/?l=devicetree&m=141087052200600&w=4
> 
> and my cover letters for previous iterations are at:
> 
> http://marc.info/?l=linux-acpi&m=141212903816560&w=4
> http://marc.info/?l=linux-kernel&m=141354745011569&w=4
> 
> There are a few changes with respect to v5 and the affected patches are
> [02-03/12] and [09-12/12].  The remaining ones have not been modified.
> 
> Most importantly, requesting the first element of a list (package) property
> from _DSD is now equivalent to accessing a single-value property of the
> same type, so device_property_read_u8(dev, pname, val) will now be equivalent
> to device_property_read_u8_array(dev, pname, val, 1), for example.
> Consequently, this _DSD definition:
> 
> Name (_DSD, Package () {
>     ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>     Package () {
>         Package () {"blah", "A string"},
>     }
> })
> 
> can be used instead of
> 
> Name (_DSD, Package () {
>     ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>     Package () {
>         Package () {"blah", Package () {"A string"}},
>     }
> })
> 
> and the code will be able to retrieve the property value from the both of
> them just fine.
> 
> This means, among other things, that accessors for single-value properties
> can be implemented in terms of the analogous "array" property accessors
> which allows the code size to be reduced somewhat.
> 
> Patches [02/12] and [09/12] have been modified to achieve that and patch
> [03/12] have been modified accordingly for the "compatible" property in
> _DSD to behave in an analogous way.  Additionally, the bodies of the
> numerical property accessors in patches [02/12] and [09/12] are now
> generated using macros (string property accessors have slightly different
> rules and are simply open coded for that reason).
> 
> Patch [10/12] has been modified to drop function arguments that happened to
> have the same values for both of the current users of those functions and
> patches [11-12/12] have been modified to take that change into account.  If
> the code in question needs to be made more complex in the future, there
> should not be any problems with that.
> 
> Due to the nature of the changes I have retained all ACKs except for the
> Grant's Reviewed-by on patch [03/12] (if that had been Acked-by, I would have
> retained it too, but that didn't feel appropriate for the "reviewed by" thing
> to me).  If any of you think that the ACKs are not applicable any more, please
> let me know and I'll drop them.
> 
> Finally, many thanks to Mika for testing the series on MinnowBoard 1 and
> MinnowBoard Max.  In case anybody else would like to test it, it is available
> from the device-properties branch of the linux-pm.git tree:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git device-properties
> 
> Thanks!

Crickets ...

OK, so I'm taking the lack of comments as the lack of objections and I'm already
getting merge conflicts for this series.  Moreover, we already have done some
work on top of it.

So, if there still are no comments by Sunday evening, I'll add this series to
my linux-next branch with 3.19-rc1 as the target.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* Re: [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware
  2014-10-21 21:15 ` [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware Rafael J. Wysocki
@ 2014-11-03 15:40   ` Grant Likely
  2014-11-03 22:04     ` Rafael J. Wysocki
  2014-11-04 15:51   ` Grant Likely
  2014-11-04 16:38   ` [Update][PATCH " Rafael J. Wysocki
  2 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2014-11-03 15:40 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux Kernel Mailing List, Arnd Bergmann, ACPI Devel Maling List,
	Greg Kroah-Hartman, Darren Hart, Mika Westerberg, Aaron Lu,
	devicetree, Linus Walleij, Alexandre Courbot, Dmitry Torokhov,
	Bryan Wu

Hi Rafael,

While reviewing and testing these patches I ran into serious bugs in
the string parsers (in both the existing code and the new functions
here). It took me a number of days, but I've got a fix now which I'll
be posting shortly and I want to get into mainline right away. I'll cc
you when I post it.

The fix will conflict with this patch. Mostly as a side effect of the
fix, the of_property_*string* function changes will no longer be
needed in this patch. It will either need to be respun, or we'll need
to give Linus some guidance on resolving the conflicts when merging.

Other comments below...

On Tue, Oct 21, 2014 at 10:15 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> Add a uniform interface by which device drivers can request device
> properties from the platform firmware by providing a property name
> and the corresponding data type.  The purpose of it is to help to
> write portable code that won't depend on any particular platform
> firmware interface.
>
> The following general helper functions are added:
[...]
> +/**
> + * device_property_read_u8_array - return a u8 array property of a device
> + * @dev: Device to get the property of
> + * @propname: Name of the property
> + * @val: The values are stored here
> + * @nval: Size of the @val array
> + *
> + * Function reads an array of u8 properties with @propname from the device
> + * firmware description and stores them to @val if found.
> + *
> + * Return: %0 if the property was found (success),
> + *        %-EINVAL if given arguments are not valid,
> + *        %-ENODATA if the property does not have a value,
> + *        %-EPROTO if the property is not an array of numbers,
> + *        %-EOVERFLOW if the size of the property is not as expected.
> + */
> +int device_property_read_u8_array(struct device *dev, const char *propname,
> +                                 u8 *val, size_t nval)
> +{
> +       return DEV_PROP_READ_ARRAY(dev, propname, u8, DEV_PROP_U8, val, nval);
> +}
> +EXPORT_SYMBOL_GPL(device_property_read_u8_array);

Yup, I'm a lot happier with this approach. :-)

> +
> +/**
> + * device_property_read_u16_array - return a u16 array property of a device
> + * @dev: Device to get the property of
> + * @propname: Name of the property
> + * @val: The values are stored here
> + * @nval: Size of the @val array
> + *
> + * Function reads an array of u16 properties with @propname from the device
> + * firmware description and stores them to @val if found.
> + *
> + * Return: %0 if the property was found (success),
> + *        %-EINVAL if given arguments are not valid,
> + *        %-ENODATA if the property does not have a value,
> + *        %-EPROTO if the property is not an array of numbers,
> + *        %-EOVERFLOW if the size of the property is not as expected.
> + */
> +int device_property_read_u16_array(struct device *dev, const char *propname,
> +                                  u16 *val, size_t nval)
> +{
> +       return DEV_PROP_READ_ARRAY(dev, propname, u16, DEV_PROP_U16, val, nval);
> +}
> +EXPORT_SYMBOL_GPL(device_property_read_u16_array);
> +
> +/**
> + * device_property_read_u32_array - return a u32 array property of a device
> + * @dev: Device to get the property of
> + * @propname: Name of the property
> + * @val: The values are stored here
> + * @nval: Size of the @val array
> + *
> + * Function reads an array of u32 properties with @propname from the device
> + * firmware description and stores them to @val if found.
> + *
> + * Return: %0 if the property was found (success),
> + *        %-EINVAL if given arguments are not valid,
> + *        %-ENODATA if the property does not have a value,
> + *        %-EPROTO if the property is not an array of numbers,
> + *        %-EOVERFLOW if the size of the property is not as expected.
> + */
> +int device_property_read_u32_array(struct device *dev, const char *propname,
> +                                  u32 *val, size_t nval)
> +{
> +       return DEV_PROP_READ_ARRAY(dev, propname, u32, DEV_PROP_U32, val, nval);
> +}
> +EXPORT_SYMBOL_GPL(device_property_read_u32_array);
> +
> +/**
> + * device_property_read_u64_array - return a u64 array property of a device
> + * @dev: Device to get the property of
> + * @propname: Name of the property
> + * @val: The values are stored here
> + * @nval: Size of the @val array
> + *
> + * Function reads an array of u64 properties with @propname from the device
> + * firmware description and stores them to @val if found.
> + *
> + * Return: %0 if the property was found (success),
> + *        %-EINVAL if given arguments are not valid,
> + *        %-ENODATA if the property does not have a value,
> + *        %-EPROTO if the property is not an array of numbers,
> + *        %-EOVERFLOW if the size of the property is not as expected.
> + */
> +int device_property_read_u64_array(struct device *dev, const char *propname,
> +                                  u64 *val, size_t nval)
> +{
> +       return DEV_PROP_READ_ARRAY(dev, propname, u64, DEV_PROP_U64, val, nval);
> +}
> +EXPORT_SYMBOL_GPL(device_property_read_u64_array);
> +
> +/**
> + * device_property_read_string_array - return a string array property of device
> + * @dev: Device to get the property of
> + * @propname: Name of the property
> + * @val: The values are stored here
> + * @nval: Size of the @val array
> + *
> + * Function reads an array of string properties with @propname from the device
> + * firmware description and stores them to @val if found.
> + *
> + * Return: %0 if the property was found (success),
> + *        %-EINVAL if given arguments are not valid,
> + *        %-ENODATA if the property does not have a value,
> + *        %-EPROTO or %-EILSEQ if the property is not an array of strings,
> + *        %-EOVERFLOW if the size of the property is not as expected.
> + */
> +int device_property_read_string_array(struct device *dev, const char *propname,
> +                                     char **val, size_t nval)
> +{
> +       return IS_ENABLED(CONFIG_OF) && dev->of_node ?
> +               of_property_read_string_array(dev->of_node, propname, val, nval) :
> +               acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
> +                                  DEV_PROP_STRING, val, nval);
> +}
> +EXPORT_SYMBOL_GPL(device_property_read_string_array);
> +
> +/**
> + * device_property_read_u8 - return a u8 property of a device
> + * @dev: Device to get the property of
> + * @propname: Name of the property
> + * @val: The value is stored here
> + */
> +int device_property_read_u8(struct device *dev, const char *propname, u8 *val)
> +{
> +       return device_property_read_u8_array(dev, propname, val, 1);
> +}
> +EXPORT_SYMBOL_GPL(device_property_read_u8);
> +
> +/**
> + * device_property_read_u16 - return a u16 property of a device
> + * @dev: Device to get the property of
> + * @propname: Name of the property
> + * @val: The value is stored here
> + */
> +int device_property_read_u16(struct device *dev, const char *propname, u16 *val)
> +{
> +       return device_property_read_u16_array(dev, propname, val, 1);
> +}
> +EXPORT_SYMBOL_GPL(device_property_read_u16);
> +
> +/**
> + * device_property_read_u32 - return a u32 property of a device
> + * @dev: Device to get the property of
> + * @propname: Name of the property
> + * @val: The value is stored here
> + */
> +int device_property_read_u32(struct device *dev, const char *propname, u32 *val)
> +{
> +       return device_property_read_u32_array(dev, propname, val, 1);
> +}
> +EXPORT_SYMBOL_GPL(device_property_read_u32);
> +
> +/**
> + * device_property_read_u64 - return a u64 property of a device
> + * @dev: Device to get the property of
> + * @propname: Name of the property
> + * @val: The value is stored here
> + */
> +int device_property_read_u64(struct device *dev, const char *propname, u64 *val)
> +{
> +       return device_property_read_u64_array(dev, propname, val, 1);
> +}
> +EXPORT_SYMBOL_GPL(device_property_read_u64);

Instead of exporting, can these be static inline wrappers in the header file?

> ===================================================================
> --- linux-pm.orig/drivers/of/base.c
> +++ linux-pm/drivers/of/base.c
> @@ -1250,6 +1250,39 @@ int of_property_read_u64(const struct de
>  EXPORT_SYMBOL_GPL(of_property_read_u64);
>
>  /**
> + * of_property_read_u64_array - Find and read an array of 64 bit integers
> + * from a property.
> + *
> + * @np:                device node from which the property value is to be read.
> + * @propname:  name of the property to be searched.
> + * @out_values:        pointer to return value, modified only if return value is 0.
> + * @sz:                number of array elements to read
> + *
> + * Search for a property in a device node and read 64-bit value(s) from
> + * it. Returns 0 on success, -EINVAL if the property does not exist,
> + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> + * property data isn't large enough.
> + *
> + * The out_values is modified only if a valid u64 value can be decoded.
> + */
> +int of_property_read_u64_array(const struct device_node *np,
> +                              const char *propname, u64 *out_values,
> +                              size_t sz)
> +{
> +       const __be32 *val = of_find_property_value_of_size(np, propname,
> +                                               (sz * sizeof(*out_values)));
> +
> +       if (IS_ERR(val))
> +               return PTR_ERR(val);
> +
> +       while (sz--) {
> +               *out_values++ = of_read_number(val, 2);
> +               val += 2;
> +       }
> +       return 0;
> +}
> +
> +/**
>   * of_property_read_string - Find and read a string from a property
>   * @np:                device node from which the property value is to be read.
>   * @propname:  name of the property to be searched.
> @@ -1362,24 +1395,11 @@ int of_property_match_string(struct devi
>  }
>  EXPORT_SYMBOL_GPL(of_property_match_string);
>
> -/**
> - * of_property_count_strings - Find and return the number of strings from a
> - * multiple strings property.
> - * @np:                device node from which the property value is to be read.
> - * @propname:  name of the property to be searched.
> - *
> - * Search for a property in a device tree node and retrieve the number of null
> - * terminated string contain in it. Returns the number of strings on
> - * success, -EINVAL if the property does not exist, -ENODATA if property
> - * does not have a value, and -EILSEQ if the string is not null-terminated
> - * within the length of the property data.
> - */
> -int of_property_count_strings(struct device_node *np, const char *propname)
> +static int property_count_strings(struct property *prop)
>  {
> -       struct property *prop = of_find_property(np, propname, NULL);
> -       int i = 0;
> -       size_t l = 0, total = 0;
>         const char *p;
> +       size_t l = 0, total = 0;
> +       int i = 0;
>
>         if (!prop)
>                 return -EINVAL;
> @@ -1395,8 +1415,62 @@ int of_property_count_strings(struct dev
>
>         return i;
>  }
> +
> +/**
> + * of_property_count_strings - Find and return the number of strings from a
> + * multiple strings property.
> + * @np:                device node from which the property value is to be read.
> + * @propname:  name of the property to be searched.
> + *
> + * Search for a property in a device tree node and retrieve the number of null
> + * terminated string contain in it. Returns the number of strings on
> + * success, -EINVAL if the property does not exist, -ENODATA if property
> + * does not have a value, and -EILSEQ if the string is not null-terminated
> + * within the length of the property data.
> + */
> +int of_property_count_strings(struct device_node *np, const char *propname)
> +{
> +       return property_count_strings(of_find_property(np, propname, NULL));
> +}
>  EXPORT_SYMBOL_GPL(of_property_count_strings);
>
> +/**
> + * of_property_read_string_array - Read an array of strings from a multiple
> + * strings property.
> + * @np:                device node from which the property value is to be read.
> + * @propname:  name of the property to be searched.
> + * @out_strs:  output array of string pointers.
> + * @sz:                number of array elements to read.
> + *
> + * Search for a property in a device tree node and retrieve a list of
> + * terminated string values (pointer to data, not a copy) in that property.
> + *
> + * If @out_strs is NULL, the number of strings in the property is returned.
> + */
> +int of_property_read_string_array(struct device_node *np, const char *propname,
> +                                 char **out_strs, size_t sz)

should be const char **out_strs. All the other string parsers use const char

> +{
> +       struct property *prop = of_find_property(np, propname, NULL);
> +       char *p = prop->value;
> +       size_t total = 0;
> +       int i;
> +
> +       i = property_count_strings(prop);
> +       if (!out_strs || i < 0)
> +               return i;
> +
> +       if (i < sz)
> +               return -EOVERFLOW;
> +
> +       while (total < prop->length && i < sz) {
> +               size_t l = strlen(p) + 1;
> +
> +               out_strs[i++] = p;
> +               total += l;
> +               p += l;
> +       }
> +}
> +

drivers/of/base.c: In function 'of_property_read_string_array':
drivers/of/base.c:1481:1: warning: control reaches end of non-void
function [-Wreturn-type]

I also found that this parser code doesn't correctly handle malformed
(unterminated) string properties. It will overflow. The existing
functions have the same problem, so it isn't something that you've
added. I've got a fix, and as a side effect the fix creates the _array
version basically for free as part of reworking
of_property_count_strings() and of_property_read_string_index()

g.

>  void of_print_phandle_args(const char *msg, const struct of_phandle_args *args)
>  {
>         int i;
> Index: linux-pm/drivers/acpi/property.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/property.c
> +++ linux-pm/drivers/acpi/property.c
> @@ -362,3 +362,181 @@ int acpi_dev_get_property_reference(stru
>         return -EPROTO;
>  }
>  EXPORT_SYMBOL_GPL(acpi_dev_get_property_reference);
> +
> +int acpi_dev_prop_get(struct acpi_device *adev, const char *propname,
> +                     void **valptr)
> +{
> +       return acpi_dev_get_property(adev, propname, ACPI_TYPE_ANY,
> +                                    (const union acpi_object **)valptr);
> +}
> +
> +int acpi_dev_prop_read_single(struct acpi_device *adev, const char *propname,
> +                             enum dev_prop_type proptype, void *val)
> +{
> +       const union acpi_object *obj;
> +       int ret;
> +
> +       if (!val)
> +               return -EINVAL;
> +
> +       if (proptype >= DEV_PROP_U8 && proptype <= DEV_PROP_U64) {
> +               ret = acpi_dev_get_property(adev, propname, ACPI_TYPE_INTEGER, &obj);
> +               if (ret)
> +                       return ret;
> +
> +               switch (proptype) {
> +               case DEV_PROP_U8:
> +                       if (obj->integer.value > U8_MAX)
> +                               return -EOVERFLOW;
> +                       *(u8 *)val = obj->integer.value;
> +                       break;
> +               case DEV_PROP_U16:
> +                       if (obj->integer.value > U16_MAX)
> +                               return -EOVERFLOW;
> +                       *(u16 *)val = obj->integer.value;
> +                       break;
> +               case DEV_PROP_U32:
> +                       if (obj->integer.value > U32_MAX)
> +                               return -EOVERFLOW;
> +                       *(u32 *)val = obj->integer.value;
> +                       break;
> +               default:
> +                       *(u64 *)val = obj->integer.value;
> +                       break;
> +               }
> +       } else if (proptype == DEV_PROP_STRING) {
> +               ret = acpi_dev_get_property(adev, propname, ACPI_TYPE_STRING, &obj);
> +               if (ret)
> +                       return ret;
> +
> +               *(char **)val = obj->string.pointer;
> +       } else {
> +               ret = -EINVAL;
> +       }
> +       return ret;
> +}
> +
> +static int acpi_copy_property_array_u8(const union acpi_object *items, u8 *val,
> +                                      size_t nval)
> +{
> +       int i;
> +
> +       for (i = 0; i < nval; i++) {
> +               if (items[i].type != ACPI_TYPE_INTEGER)
> +                       return -EPROTO;
> +               if (items[i].integer.value > U8_MAX)
> +                       return -EOVERFLOW;
> +
> +               val[i] = items[i].integer.value;
> +       }
> +       return 0;
> +}
> +
> +static int acpi_copy_property_array_u16(const union acpi_object *items,
> +                                       u16 *val, size_t nval)
> +{
> +       int i;
> +
> +       for (i = 0; i < nval; i++) {
> +               if (items[i].type != ACPI_TYPE_INTEGER)
> +                       return -EPROTO;
> +               if (items[i].integer.value > U16_MAX)
> +                       return -EOVERFLOW;
> +
> +               val[i] = items[i].integer.value;
> +       }
> +       return 0;
> +}
> +
> +static int acpi_copy_property_array_u32(const union acpi_object *items,
> +                                       u32 *val, size_t nval)
> +{
> +       int i;
> +
> +       for (i = 0; i < nval; i++) {
> +               if (items[i].type != ACPI_TYPE_INTEGER)
> +                       return -EPROTO;
> +               if (items[i].integer.value > U32_MAX)
> +                       return -EOVERFLOW;
> +
> +               val[i] = items[i].integer.value;
> +       }
> +       return 0;
> +}
> +
> +static int acpi_copy_property_array_u64(const union acpi_object *items,
> +                                       u64 *val, size_t nval)
> +{
> +       int i;
> +
> +       for (i = 0; i < nval; i++) {
> +               if (items[i].type != ACPI_TYPE_INTEGER)
> +                       return -EPROTO;
> +
> +               val[i] = items[i].integer.value;
> +       }
> +       return 0;
> +}
> +
> +static int acpi_copy_property_array_string(const union acpi_object *items,
> +                                          char **val, size_t nval)
> +{
> +       int i;
> +
> +       for (i = 0; i < nval; i++) {
> +               if (items[i].type != ACPI_TYPE_STRING)
> +                       return -EPROTO;
> +
> +               val[i] = items[i].string.pointer;
> +       }
> +       return 0;
> +}
> +
> +int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
> +                      enum dev_prop_type proptype, void *val, size_t nval)
> +{
> +       const union acpi_object *obj;
> +       const union acpi_object *items;
> +       int ret;
> +
> +       if (val && nval == 1) {
> +               ret = acpi_dev_prop_read_single(adev, propname, proptype, val);
> +               if (!ret)
> +                       return ret;
> +       }
> +
> +       ret = acpi_dev_get_property_array(adev, propname, ACPI_TYPE_ANY, &obj);
> +       if (ret)
> +               return ret;
> +
> +       if (!val)
> +               return obj->package.count;
> +       else if (nval <= 0)
> +               return -EINVAL;
> +
> +       if (nval > obj->package.count)
> +               return -EOVERFLOW;
> +
> +       items = obj->package.elements;
> +       switch (proptype) {
> +       case DEV_PROP_U8:
> +               ret = acpi_copy_property_array_u8(items, (u8 *)val, nval);
> +               break;
> +       case DEV_PROP_U16:
> +               ret = acpi_copy_property_array_u16(items, (u16 *)val, nval);
> +               break;
> +       case DEV_PROP_U32:
> +               ret = acpi_copy_property_array_u32(items, (u32 *)val, nval);
> +               break;
> +       case DEV_PROP_U64:
> +               ret = acpi_copy_property_array_u64(items, (u64 *)val, nval);
> +               break;
> +       case DEV_PROP_STRING:
> +               ret = acpi_copy_property_array_string(items, (char **)val, nval);
> +               break;
> +       default:
> +               ret = -EINVAL;
> +               break;
> +       }
> +       return ret;
> +}
>

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

* Re: [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware
  2014-11-03 15:40   ` Grant Likely
@ 2014-11-03 22:04     ` Rafael J. Wysocki
  2014-11-04 17:01       ` Grant Likely
  0 siblings, 1 reply; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-11-03 22:04 UTC (permalink / raw)
  To: Grant Likely
  Cc: Linux Kernel Mailing List, Arnd Bergmann, ACPI Devel Maling List,
	Greg Kroah-Hartman, Darren Hart, Mika Westerberg, Aaron Lu,
	devicetree, Linus Walleij, Alexandre Courbot, Dmitry Torokhov,
	Bryan Wu

On Monday, November 03, 2014 03:40:08 PM Grant Likely wrote:
> Hi Rafael,

Hi,

> While reviewing and testing these patches I ran into serious bugs in
> the string parsers (in both the existing code and the new functions
> here). It took me a number of days, but I've got a fix now which I'll
> be posting shortly and I want to get into mainline right away. I'll cc
> you when I post it.

OK

> The fix will conflict with this patch. Mostly as a side effect of the
> fix, the of_property_*string* function changes will no longer be
> needed in this patch. It will either need to be respun, or we'll need
> to give Linus some guidance on resolving the conflicts when merging.

Respin might be better, we still have a number of -rcs before final 3.18.

> Other comments below...
> 
> On Tue, Oct 21, 2014 at 10:15 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> >
> > Add a uniform interface by which device drivers can request device
> > properties from the platform firmware by providing a property name
> > and the corresponding data type.  The purpose of it is to help to
> > write portable code that won't depend on any particular platform
> > firmware interface.
> >
> > The following general helper functions are added:
> [...]
> > +/**
> > + * device_property_read_u8_array - return a u8 array property of a device
> > + * @dev: Device to get the property of
> > + * @propname: Name of the property
> > + * @val: The values are stored here
> > + * @nval: Size of the @val array
> > + *
> > + * Function reads an array of u8 properties with @propname from the device
> > + * firmware description and stores them to @val if found.
> > + *
> > + * Return: %0 if the property was found (success),
> > + *        %-EINVAL if given arguments are not valid,
> > + *        %-ENODATA if the property does not have a value,
> > + *        %-EPROTO if the property is not an array of numbers,
> > + *        %-EOVERFLOW if the size of the property is not as expected.
> > + */
> > +int device_property_read_u8_array(struct device *dev, const char *propname,
> > +                                 u8 *val, size_t nval)
> > +{
> > +       return DEV_PROP_READ_ARRAY(dev, propname, u8, DEV_PROP_U8, val, nval);
> > +}
> > +EXPORT_SYMBOL_GPL(device_property_read_u8_array);
> 
> Yup, I'm a lot happier with this approach. :-)
> 
> > +
> > +/**
> > + * device_property_read_u16_array - return a u16 array property of a device
> > + * @dev: Device to get the property of
> > + * @propname: Name of the property
> > + * @val: The values are stored here
> > + * @nval: Size of the @val array
> > + *
> > + * Function reads an array of u16 properties with @propname from the device
> > + * firmware description and stores them to @val if found.
> > + *
> > + * Return: %0 if the property was found (success),
> > + *        %-EINVAL if given arguments are not valid,
> > + *        %-ENODATA if the property does not have a value,
> > + *        %-EPROTO if the property is not an array of numbers,
> > + *        %-EOVERFLOW if the size of the property is not as expected.
> > + */
> > +int device_property_read_u16_array(struct device *dev, const char *propname,
> > +                                  u16 *val, size_t nval)
> > +{
> > +       return DEV_PROP_READ_ARRAY(dev, propname, u16, DEV_PROP_U16, val, nval);
> > +}
> > +EXPORT_SYMBOL_GPL(device_property_read_u16_array);
> > +
> > +/**
> > + * device_property_read_u32_array - return a u32 array property of a device
> > + * @dev: Device to get the property of
> > + * @propname: Name of the property
> > + * @val: The values are stored here
> > + * @nval: Size of the @val array
> > + *
> > + * Function reads an array of u32 properties with @propname from the device
> > + * firmware description and stores them to @val if found.
> > + *
> > + * Return: %0 if the property was found (success),
> > + *        %-EINVAL if given arguments are not valid,
> > + *        %-ENODATA if the property does not have a value,
> > + *        %-EPROTO if the property is not an array of numbers,
> > + *        %-EOVERFLOW if the size of the property is not as expected.
> > + */
> > +int device_property_read_u32_array(struct device *dev, const char *propname,
> > +                                  u32 *val, size_t nval)
> > +{
> > +       return DEV_PROP_READ_ARRAY(dev, propname, u32, DEV_PROP_U32, val, nval);
> > +}
> > +EXPORT_SYMBOL_GPL(device_property_read_u32_array);
> > +
> > +/**
> > + * device_property_read_u64_array - return a u64 array property of a device
> > + * @dev: Device to get the property of
> > + * @propname: Name of the property
> > + * @val: The values are stored here
> > + * @nval: Size of the @val array
> > + *
> > + * Function reads an array of u64 properties with @propname from the device
> > + * firmware description and stores them to @val if found.
> > + *
> > + * Return: %0 if the property was found (success),
> > + *        %-EINVAL if given arguments are not valid,
> > + *        %-ENODATA if the property does not have a value,
> > + *        %-EPROTO if the property is not an array of numbers,
> > + *        %-EOVERFLOW if the size of the property is not as expected.
> > + */
> > +int device_property_read_u64_array(struct device *dev, const char *propname,
> > +                                  u64 *val, size_t nval)
> > +{
> > +       return DEV_PROP_READ_ARRAY(dev, propname, u64, DEV_PROP_U64, val, nval);
> > +}
> > +EXPORT_SYMBOL_GPL(device_property_read_u64_array);
> > +
> > +/**
> > + * device_property_read_string_array - return a string array property of device
> > + * @dev: Device to get the property of
> > + * @propname: Name of the property
> > + * @val: The values are stored here
> > + * @nval: Size of the @val array
> > + *
> > + * Function reads an array of string properties with @propname from the device
> > + * firmware description and stores them to @val if found.
> > + *
> > + * Return: %0 if the property was found (success),
> > + *        %-EINVAL if given arguments are not valid,
> > + *        %-ENODATA if the property does not have a value,
> > + *        %-EPROTO or %-EILSEQ if the property is not an array of strings,
> > + *        %-EOVERFLOW if the size of the property is not as expected.
> > + */
> > +int device_property_read_string_array(struct device *dev, const char *propname,
> > +                                     char **val, size_t nval)
> > +{
> > +       return IS_ENABLED(CONFIG_OF) && dev->of_node ?
> > +               of_property_read_string_array(dev->of_node, propname, val, nval) :
> > +               acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
> > +                                  DEV_PROP_STRING, val, nval);
> > +}
> > +EXPORT_SYMBOL_GPL(device_property_read_string_array);
> > +
> > +/**
> > + * device_property_read_u8 - return a u8 property of a device
> > + * @dev: Device to get the property of
> > + * @propname: Name of the property
> > + * @val: The value is stored here
> > + */
> > +int device_property_read_u8(struct device *dev, const char *propname, u8 *val)
> > +{
> > +       return device_property_read_u8_array(dev, propname, val, 1);
> > +}
> > +EXPORT_SYMBOL_GPL(device_property_read_u8);
> > +
> > +/**
> > + * device_property_read_u16 - return a u16 property of a device
> > + * @dev: Device to get the property of
> > + * @propname: Name of the property
> > + * @val: The value is stored here
> > + */
> > +int device_property_read_u16(struct device *dev, const char *propname, u16 *val)
> > +{
> > +       return device_property_read_u16_array(dev, propname, val, 1);
> > +}
> > +EXPORT_SYMBOL_GPL(device_property_read_u16);
> > +
> > +/**
> > + * device_property_read_u32 - return a u32 property of a device
> > + * @dev: Device to get the property of
> > + * @propname: Name of the property
> > + * @val: The value is stored here
> > + */
> > +int device_property_read_u32(struct device *dev, const char *propname, u32 *val)
> > +{
> > +       return device_property_read_u32_array(dev, propname, val, 1);
> > +}
> > +EXPORT_SYMBOL_GPL(device_property_read_u32);
> > +
> > +/**
> > + * device_property_read_u64 - return a u64 property of a device
> > + * @dev: Device to get the property of
> > + * @propname: Name of the property
> > + * @val: The value is stored here
> > + */
> > +int device_property_read_u64(struct device *dev, const char *propname, u64 *val)
> > +{
> > +       return device_property_read_u64_array(dev, propname, val, 1);
> > +}
> > +EXPORT_SYMBOL_GPL(device_property_read_u64);
> 
> Instead of exporting, can these be static inline wrappers in the header file?

Yes, they can.

The reason why I've done it this way is because I'm expecting the single value
reads to be the most common case and each one costs us a constant if that's
static inline.

> > ===================================================================
> > --- linux-pm.orig/drivers/of/base.c
> > +++ linux-pm/drivers/of/base.c
> > @@ -1250,6 +1250,39 @@ int of_property_read_u64(const struct de
> >  EXPORT_SYMBOL_GPL(of_property_read_u64);
> >
> >  /**
> > + * of_property_read_u64_array - Find and read an array of 64 bit integers
> > + * from a property.
> > + *
> > + * @np:                device node from which the property value is to be read.
> > + * @propname:  name of the property to be searched.
> > + * @out_values:        pointer to return value, modified only if return value is 0.
> > + * @sz:                number of array elements to read
> > + *
> > + * Search for a property in a device node and read 64-bit value(s) from
> > + * it. Returns 0 on success, -EINVAL if the property does not exist,
> > + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> > + * property data isn't large enough.
> > + *
> > + * The out_values is modified only if a valid u64 value can be decoded.
> > + */
> > +int of_property_read_u64_array(const struct device_node *np,
> > +                              const char *propname, u64 *out_values,
> > +                              size_t sz)
> > +{
> > +       const __be32 *val = of_find_property_value_of_size(np, propname,
> > +                                               (sz * sizeof(*out_values)));
> > +
> > +       if (IS_ERR(val))
> > +               return PTR_ERR(val);
> > +
> > +       while (sz--) {
> > +               *out_values++ = of_read_number(val, 2);
> > +               val += 2;
> > +       }
> > +       return 0;
> > +}
> > +
> > +/**
> >   * of_property_read_string - Find and read a string from a property
> >   * @np:                device node from which the property value is to be read.
> >   * @propname:  name of the property to be searched.
> > @@ -1362,24 +1395,11 @@ int of_property_match_string(struct devi
> >  }
> >  EXPORT_SYMBOL_GPL(of_property_match_string);
> >
> > -/**
> > - * of_property_count_strings - Find and return the number of strings from a
> > - * multiple strings property.
> > - * @np:                device node from which the property value is to be read.
> > - * @propname:  name of the property to be searched.
> > - *
> > - * Search for a property in a device tree node and retrieve the number of null
> > - * terminated string contain in it. Returns the number of strings on
> > - * success, -EINVAL if the property does not exist, -ENODATA if property
> > - * does not have a value, and -EILSEQ if the string is not null-terminated
> > - * within the length of the property data.
> > - */
> > -int of_property_count_strings(struct device_node *np, const char *propname)
> > +static int property_count_strings(struct property *prop)
> >  {
> > -       struct property *prop = of_find_property(np, propname, NULL);
> > -       int i = 0;
> > -       size_t l = 0, total = 0;
> >         const char *p;
> > +       size_t l = 0, total = 0;
> > +       int i = 0;
> >
> >         if (!prop)
> >                 return -EINVAL;
> > @@ -1395,8 +1415,62 @@ int of_property_count_strings(struct dev
> >
> >         return i;
> >  }
> > +
> > +/**
> > + * of_property_count_strings - Find and return the number of strings from a
> > + * multiple strings property.
> > + * @np:                device node from which the property value is to be read.
> > + * @propname:  name of the property to be searched.
> > + *
> > + * Search for a property in a device tree node and retrieve the number of null
> > + * terminated string contain in it. Returns the number of strings on
> > + * success, -EINVAL if the property does not exist, -ENODATA if property
> > + * does not have a value, and -EILSEQ if the string is not null-terminated
> > + * within the length of the property data.
> > + */
> > +int of_property_count_strings(struct device_node *np, const char *propname)
> > +{
> > +       return property_count_strings(of_find_property(np, propname, NULL));
> > +}
> >  EXPORT_SYMBOL_GPL(of_property_count_strings);
> >
> > +/**
> > + * of_property_read_string_array - Read an array of strings from a multiple
> > + * strings property.
> > + * @np:                device node from which the property value is to be read.
> > + * @propname:  name of the property to be searched.
> > + * @out_strs:  output array of string pointers.
> > + * @sz:                number of array elements to read.
> > + *
> > + * Search for a property in a device tree node and retrieve a list of
> > + * terminated string values (pointer to data, not a copy) in that property.
> > + *
> > + * If @out_strs is NULL, the number of strings in the property is returned.
> > + */
> > +int of_property_read_string_array(struct device_node *np, const char *propname,
> > +                                 char **out_strs, size_t sz)
> 
> should be const char **out_strs. All the other string parsers use const char

OK

> > +{
> > +       struct property *prop = of_find_property(np, propname, NULL);
> > +       char *p = prop->value;
> > +       size_t total = 0;
> > +       int i;
> > +
> > +       i = property_count_strings(prop);
> > +       if (!out_strs || i < 0)
> > +               return i;
> > +
> > +       if (i < sz)
> > +               return -EOVERFLOW;
> > +
> > +       while (total < prop->length && i < sz) {
> > +               size_t l = strlen(p) + 1;
> > +
> > +               out_strs[i++] = p;
> > +               total += l;
> > +               p += l;
> > +       }
> > +}
> > +
> 
> drivers/of/base.c: In function 'of_property_read_string_array':
> drivers/of/base.c:1481:1: warning: control reaches end of non-void
> function [-Wreturn-type]

That's already fixed in linux-next.

> I also found that this parser code doesn't correctly handle malformed
> (unterminated) string properties. It will overflow. The existing
> functions have the same problem, so it isn't something that you've
> added. I've got a fix, and as a side effect the fix creates the _array
> version basically for free as part of reworking
> of_property_count_strings() and of_property_read_string_index()

OK

So can you please point me to a git branch containing the fix?  I'll rebase the
patch on top of that then and everything should merge just fine.

Rafael


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

* Re: [PATCH v6 04/12] misc: at25: Make use of device property API
  2014-10-21 21:19 ` [PATCH v6 04/12] misc: at25: Make use of device property API Rafael J. Wysocki
@ 2014-11-04 14:18   ` Grant Likely
  2014-11-04 14:38     ` Mika Westerberg
  0 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2014-11-04 14:18 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux Kernel Mailing List
  Cc: ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Mika Westerberg, Aaron Lu, Arnd Bergmann, devicetree,
	Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu

On Tue, 21 Oct 2014 23:19:56 +0200
, "Rafael J. Wysocki" <rjw@rjwysocki.net>
 wrote:
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> Make use of device property API in this driver so that both DT and ACPI
> based systems can use this driver.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/misc/eeprom/at25.c |   34 +++++++++++++---------------------
>  1 file changed, 13 insertions(+), 21 deletions(-)
> 
> Index: linux-pm/drivers/misc/eeprom/at25.c
> ===================================================================
> --- linux-pm.orig/drivers/misc/eeprom/at25.c
> +++ linux-pm/drivers/misc/eeprom/at25.c
> @@ -18,7 +18,7 @@
>  
>  #include <linux/spi/spi.h>
>  #include <linux/spi/eeprom.h>
> -#include <linux/of.h>
> +#include <linux/property.h>
>  
>  /*
>   * NOTE: this is an *EEPROM* driver.  The vagaries of product naming
> @@ -301,35 +301,33 @@ static ssize_t at25_mem_write(struct mem
>  
>  /*-------------------------------------------------------------------------*/
>  
> -static int at25_np_to_chip(struct device *dev,
> -			   struct device_node *np,
> -			   struct spi_eeprom *chip)
> +static int at25_fw_to_chip(struct device *dev, struct spi_eeprom *chip)
>  {
>  	u32 val;
>  
>  	memset(chip, 0, sizeof(*chip));
> -	strncpy(chip->name, np->name, sizeof(chip->name));
> +	strncpy(chip->name, "at25", sizeof(chip->name));

This line changes behaviour of the driver. It's possibly not a problem,
but it should be commented on and whether any due diligance has been
done to make sure it there isn't anything that depends on it.

Otherwise:
Acked-by: Grant Likely <grant.likely@linaro.org>

>  
> -	if (of_property_read_u32(np, "size", &val) == 0 ||
> -	    of_property_read_u32(np, "at25,byte-len", &val) == 0) {
> +	if (device_property_read_u32(dev, "size", &val) == 0 ||
> +	    device_property_read_u32(dev, "at25,byte-len", &val) == 0) {
>  		chip->byte_len = val;
>  	} else {
>  		dev_err(dev, "Error: missing \"size\" property\n");
>  		return -ENODEV;
>  	}
>  
> -	if (of_property_read_u32(np, "pagesize", &val) == 0 ||
> -	    of_property_read_u32(np, "at25,page-size", &val) == 0) {
> +	if (device_property_read_u32(dev, "pagesize", &val) == 0 ||
> +	    device_property_read_u32(dev, "at25,page-size", &val) == 0) {
>  		chip->page_size = (u16)val;
>  	} else {
>  		dev_err(dev, "Error: missing \"pagesize\" property\n");
>  		return -ENODEV;
>  	}
>  
> -	if (of_property_read_u32(np, "at25,addr-mode", &val) == 0) {
> +	if (device_property_read_u32(dev, "at25,addr-mode", &val) == 0) {
>  		chip->flags = (u16)val;
>  	} else {
> -		if (of_property_read_u32(np, "address-width", &val)) {
> +		if (device_property_read_u32(dev, "address-width", &val)) {
>  			dev_err(dev,
>  				"Error: missing \"address-width\" property\n");
>  			return -ENODEV;
> @@ -350,7 +348,7 @@ static int at25_np_to_chip(struct device
>  				val);
>  			return -ENODEV;
>  		}
> -		if (of_find_property(np, "read-only", NULL))
> +		if (device_property_present(dev, "read-only"))
>  			chip->flags |= EE_READONLY;
>  	}
>  	return 0;
> @@ -360,21 +358,15 @@ static int at25_probe(struct spi_device
>  {
>  	struct at25_data	*at25 = NULL;
>  	struct spi_eeprom	chip;
> -	struct device_node	*np = spi->dev.of_node;
>  	int			err;
>  	int			sr;
>  	int			addrlen;
>  
>  	/* Chip description */
>  	if (!spi->dev.platform_data) {
> -		if (np) {
> -			err = at25_np_to_chip(&spi->dev, np, &chip);
> -			if (err)
> -				return err;
> -		} else {
> -			dev_err(&spi->dev, "Error: no chip description\n");
> -			return -ENODEV;
> -		}
> +		err = at25_fw_to_chip(&spi->dev, &chip);
> +		if (err)
> +			return err;
>  	} else
>  		chip = *(struct spi_eeprom *)spi->dev.platform_data;
>  
> 


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

* Re: [PATCH v6 04/12] misc: at25: Make use of device property API
  2014-11-04 14:18   ` Grant Likely
@ 2014-11-04 14:38     ` Mika Westerberg
  2014-11-04 15:04       ` Grant Likely
  0 siblings, 1 reply; 29+ messages in thread
From: Mika Westerberg @ 2014-11-04 14:38 UTC (permalink / raw)
  To: Grant Likely
  Cc: Rafael J. Wysocki, Linux Kernel Mailing List,
	ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Aaron Lu, Arnd Bergmann, devicetree, Linus Walleij,
	Alexandre Courbot, Dmitry Torokhov, Bryan Wu

On Tue, Nov 04, 2014 at 02:18:26PM +0000, Grant Likely wrote:
> > -	strncpy(chip->name, np->name, sizeof(chip->name));
> > +	strncpy(chip->name, "at25", sizeof(chip->name));
> 
> This line changes behaviour of the driver. It's possibly not a problem,
> but it should be commented on and whether any due diligance has been
> done to make sure it there isn't anything that depends on it.

I found only one user for "atmel,at25" in any of the DT sample files.

arch/arm/boot/dts/phy3250.dts:

	eeprom: at25@0 {
		...
		at25,byte-len = <0x8000>;
		at25,addr-mode = <2>;
		at25,page-size = <64>;

		compatible = "atmel,at25";
		reg = <0>;
		spi-max-frequency = <5000000>;
	};

I think np->name is "at25" in this case? The binding file
Documentation/devicetree/bindings/misc/at25.txt also has the same name.

Are you OK, if we add something like below to the changelog?

 In addition we hard-code the name of the chip to be "at25" for the
 reason that there is no common mechanism to fetch name of the firmware
 node. The only existing user (arch/arm/boot/dts/phy3250.dts) uses the
 same name so it should continue to work.

> Otherwise:
> Acked-by: Grant Likely <grant.likely@linaro.org>

Thanks.

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

* Re: [PATCH v6 04/12] misc: at25: Make use of device property API
  2014-11-04 14:38     ` Mika Westerberg
@ 2014-11-04 15:04       ` Grant Likely
  2014-11-04 16:19         ` Rafael J. Wysocki
  0 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2014-11-04 15:04 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Rafael J. Wysocki, Linux Kernel Mailing List,
	ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Aaron Lu, Arnd Bergmann, devicetree, Linus Walleij,
	Alexandre Courbot, Dmitry Torokhov, Bryan Wu

On Tue, Nov 4, 2014 at 2:38 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Tue, Nov 04, 2014 at 02:18:26PM +0000, Grant Likely wrote:
>> > -   strncpy(chip->name, np->name, sizeof(chip->name));
>> > +   strncpy(chip->name, "at25", sizeof(chip->name));
>>
>> This line changes behaviour of the driver. It's possibly not a problem,
>> but it should be commented on and whether any due diligance has been
>> done to make sure it there isn't anything that depends on it.
>
> I found only one user for "atmel,at25" in any of the DT sample files.
>
> arch/arm/boot/dts/phy3250.dts:
>
>         eeprom: at25@0 {
>                 ...
>                 at25,byte-len = <0x8000>;
>                 at25,addr-mode = <2>;
>                 at25,page-size = <64>;
>
>                 compatible = "atmel,at25";
>                 reg = <0>;
>                 spi-max-frequency = <5000000>;
>         };
>
> I think np->name is "at25" in this case? The binding file
> Documentation/devicetree/bindings/misc/at25.txt also has the same name.
>
> Are you OK, if we add something like below to the changelog?
>
>  In addition we hard-code the name of the chip to be "at25" for the
>  reason that there is no common mechanism to fetch name of the firmware
>  node. The only existing user (arch/arm/boot/dts/phy3250.dts) uses the
>  same name so it should continue to work.

Yes. If somebody complains, then we can reinstate the previous
behaviour, but assume it isn't necessary for now.

g.

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

* Re: [PATCH v6 00/12] Add ACPI _DSD and unified device properties support
  2014-10-24 22:10 ` [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
@ 2014-11-04 15:49   ` Grant Likely
  2014-11-04 16:20     ` Rafael J. Wysocki
  0 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2014-11-04 15:49 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux Kernel Mailing List, ACPI Devel Maling List
  Cc: Greg Kroah-Hartman, Darren Hart, Mika Westerberg, Aaron Lu,
	Arnd Bergmann, devicetree, Linus Walleij, Alexandre Courbot,
	Dmitry Torokhov, Bryan Wu

On Sat, 25 Oct 2014 00:10:20 +0200
, "Rafael J. Wysocki" <rjw@rjwysocki.net>
 wrote:
> On Tuesday, October 21, 2014 11:08:59 PM Rafael J. Wysocki wrote:
> > Hi Everyone,
> > 
> > This is version 6 of the unified device properties interface patchset.
> > 
> > The original cover letter from Mika is here:
> > 
> > http://marc.info/?l=devicetree&m=141087052200600&w=4
> > 
> > and my cover letters for previous iterations are at:
> > 
> > http://marc.info/?l=linux-acpi&m=141212903816560&w=4
> > http://marc.info/?l=linux-kernel&m=141354745011569&w=4
> > 
> > There are a few changes with respect to v5 and the affected patches are
> > [02-03/12] and [09-12/12].  The remaining ones have not been modified.
> > 
> > Most importantly, requesting the first element of a list (package) property
> > from _DSD is now equivalent to accessing a single-value property of the
> > same type, so device_property_read_u8(dev, pname, val) will now be equivalent
> > to device_property_read_u8_array(dev, pname, val, 1), for example.
> > Consequently, this _DSD definition:
> > 
> > Name (_DSD, Package () {
> >     ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> >     Package () {
> >         Package () {"blah", "A string"},
> >     }
> > })
> > 
> > can be used instead of
> > 
> > Name (_DSD, Package () {
> >     ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> >     Package () {
> >         Package () {"blah", Package () {"A string"}},
> >     }
> > })
> > 
> > and the code will be able to retrieve the property value from the both of
> > them just fine.
> > 
> > This means, among other things, that accessors for single-value properties
> > can be implemented in terms of the analogous "array" property accessors
> > which allows the code size to be reduced somewhat.
> > 
> > Patches [02/12] and [09/12] have been modified to achieve that and patch
> > [03/12] have been modified accordingly for the "compatible" property in
> > _DSD to behave in an analogous way.  Additionally, the bodies of the
> > numerical property accessors in patches [02/12] and [09/12] are now
> > generated using macros (string property accessors have slightly different
> > rules and are simply open coded for that reason).
> > 
> > Patch [10/12] has been modified to drop function arguments that happened to
> > have the same values for both of the current users of those functions and
> > patches [11-12/12] have been modified to take that change into account.  If
> > the code in question needs to be made more complex in the future, there
> > should not be any problems with that.
> > 
> > Due to the nature of the changes I have retained all ACKs except for the
> > Grant's Reviewed-by on patch [03/12] (if that had been Acked-by, I would have
> > retained it too, but that didn't feel appropriate for the "reviewed by" thing
> > to me).  If any of you think that the ACKs are not applicable any more, please
> > let me know and I'll drop them.
> > 
> > Finally, many thanks to Mika for testing the series on MinnowBoard 1 and
> > MinnowBoard Max.  In case anybody else would like to test it, it is available
> > from the device-properties branch of the linux-pm.git tree:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git device-properties
> > 
> > Thanks!
> 
> Crickets ...
> 
> OK, so I'm taking the lack of comments as the lack of objections and I'm already
> getting merge conflicts for this series.  Moreover, we already have done some
> work on top of it.
> 
> So, if there still are no comments by Sunday evening, I'll add this series to
> my linux-next branch with 3.19-rc1 as the target.

Aside from the comments I've made elsewhere, you can add my acked by for the whole series.

Acked-by: Grant Likely <grant.likely@linaro.org>

g.

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

* Re: [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware
  2014-10-21 21:15 ` [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware Rafael J. Wysocki
  2014-11-03 15:40   ` Grant Likely
@ 2014-11-04 15:51   ` Grant Likely
  2014-11-04 16:20     ` Rafael J. Wysocki
  2014-11-04 16:38   ` [Update][PATCH " Rafael J. Wysocki
  2 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2014-11-04 15:51 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux Kernel Mailing List, Arnd Bergmann
  Cc: ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Mika Westerberg, Aaron Lu, devicetree, Linus Walleij,
	Alexandre Courbot, Dmitry Torokhov, Bryan Wu

On Tue, 21 Oct 2014 23:15:50 +0200
, "Rafael J. Wysocki" <rjw@rjwysocki.net>
 wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> Add a uniform interface by which device drivers can request device
> properties from the platform firmware by providing a property name
> and the corresponding data type.  The purpose of it is to help to
> write portable code that won't depend on any particular platform
> firmware interface.
> 
> The following general helper functions are added:
> 
> device_property_present()
> device_property_read_u8()
> device_property_read_u16()
> device_property_read_u32()
> device_property_read_u64()
> device_property_read_string()
> device_property_read_u8_array()
> device_property_read_u16_array()
> device_property_read_u32_array()
> device_property_read_u64_array()
> device_property_read_string_array()
> 
> The first one allows the caller to check if the given property is
> present.  The next 5 of them allow single-valued properties of
> various types to be retrieved in a uniform way.  The remaining 5 are
> for reading properties with multiple values (arrays of either numbers
> or strings).
> 
> The interface covers both ACPI and Device Trees.
> 
> This change set includes material from Mika Westerberg and Aaron Lu.
> 
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> Changes from v5:
> - acpi_dev_prop_read() can now handle both list (package) and single-value
>   properties (the latter are tried first for the last argument equal to 1).
> - There is a new macro to generate the bodies of device_property_read_u*_array().
> - device_property_read_u*() are implemented using device_property_read_u*_array().
> - device_property_read_bool() is a new static inline wrapper around
>   device_property_present().
> 
> ---
>  drivers/acpi/property.c  |  178 +++++++++++++++++++++++++++++++++++
>  drivers/base/Makefile    |    2 
>  drivers/base/property.c  |  233 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/of/base.c        |  106 ++++++++++++++++++---
>  include/linux/acpi.h     |   32 ++++++
>  include/linux/of.h       |   22 ++++
>  include/linux/property.h |   53 ++++++++++
>  7 files changed, 609 insertions(+), 17 deletions(-)
>  create mode 100644 drivers/base/property.c
>  create mode 100644 include/linux/property.h
> 
> Index: linux-pm/include/linux/property.h
> ===================================================================
> --- /dev/null
> +++ linux-pm/include/linux/property.h
> @@ -0,0 +1,53 @@
> +/*
> + * property.h - Unified device property interface.
> + *
> + * Copyright (C) 2014, Intel Corporation
> + * Authors: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> + *          Mika Westerberg <mika.westerberg@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _LINUX_PROPERTY_H_
> +#define _LINUX_PROPERTY_H_
> +
> +#include <linux/types.h>
> +
> +struct device;
> +
> +enum dev_prop_type {
> +	DEV_PROP_U8,
> +	DEV_PROP_U16,
> +	DEV_PROP_U32,
> +	DEV_PROP_U64,
> +	DEV_PROP_STRING,
> +	DEV_PROP_MAX,
> +};
> +
> +bool device_property_present(struct device *dev, const char *propname);
> +int device_property_read_u8_array(struct device *dev, const char *propname,
> +				  u8 *val, size_t nval);
> +int device_property_read_u16_array(struct device *dev, const char *propname,
> +				   u16 *val, size_t nval);
> +int device_property_read_u32_array(struct device *dev, const char *propname,
> +				   u32 *val, size_t nval);
> +int device_property_read_u64_array(struct device *dev, const char *propname,
> +				   u64 *val, size_t nval);
> +int device_property_read_string_array(struct device *dev, const char *propname,
> +				      char **val, size_t nval);

I'm not sure if I asked this elsewhere. Can 'val' be made a const char **?

g.

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

* Re: [PATCH v6 04/12] misc: at25: Make use of device property API
  2014-11-04 16:19         ` Rafael J. Wysocki
@ 2014-11-04 16:07           ` Mika Westerberg
  0 siblings, 0 replies; 29+ messages in thread
From: Mika Westerberg @ 2014-11-04 16:07 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Grant Likely, Linux Kernel Mailing List, ACPI Devel Maling List,
	Greg Kroah-Hartman, Darren Hart, Aaron Lu, Arnd Bergmann,
	devicetree, Linus Walleij, Alexandre Courbot, Dmitry Torokhov,
	Bryan Wu

On Tue, Nov 04, 2014 at 05:19:02PM +0100, Rafael J. Wysocki wrote:
> > > I think np->name is "at25" in this case? The binding file
> > > Documentation/devicetree/bindings/misc/at25.txt also has the same name.
> > >
> > > Are you OK, if we add something like below to the changelog?
> > >
> > >  In addition we hard-code the name of the chip to be "at25" for the
> > >  reason that there is no common mechanism to fetch name of the firmware
> > >  node. The only existing user (arch/arm/boot/dts/phy3250.dts) uses the
> > >  same name so it should continue to work.
> > 
> > Yes. If somebody complains, then we can reinstate the previous
> > behaviour, but assume it isn't necessary for now.
> 
> OK, done in my tree. 

Thanks Rafael!

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

* Re: [PATCH v6 04/12] misc: at25: Make use of device property API
  2014-11-04 15:04       ` Grant Likely
@ 2014-11-04 16:19         ` Rafael J. Wysocki
  2014-11-04 16:07           ` Mika Westerberg
  0 siblings, 1 reply; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-11-04 16:19 UTC (permalink / raw)
  To: Grant Likely
  Cc: Mika Westerberg, Linux Kernel Mailing List,
	ACPI Devel Maling List, Greg Kroah-Hartman, Darren Hart,
	Aaron Lu, Arnd Bergmann, devicetree, Linus Walleij,
	Alexandre Courbot, Dmitry Torokhov, Bryan Wu

On Tuesday, November 04, 2014 03:04:54 PM Grant Likely wrote:
> On Tue, Nov 4, 2014 at 2:38 PM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > On Tue, Nov 04, 2014 at 02:18:26PM +0000, Grant Likely wrote:
> >> > -   strncpy(chip->name, np->name, sizeof(chip->name));
> >> > +   strncpy(chip->name, "at25", sizeof(chip->name));
> >>
> >> This line changes behaviour of the driver. It's possibly not a problem,
> >> but it should be commented on and whether any due diligance has been
> >> done to make sure it there isn't anything that depends on it.
> >
> > I found only one user for "atmel,at25" in any of the DT sample files.
> >
> > arch/arm/boot/dts/phy3250.dts:
> >
> >         eeprom: at25@0 {
> >                 ...
> >                 at25,byte-len = <0x8000>;
> >                 at25,addr-mode = <2>;
> >                 at25,page-size = <64>;
> >
> >                 compatible = "atmel,at25";
> >                 reg = <0>;
> >                 spi-max-frequency = <5000000>;
> >         };
> >
> > I think np->name is "at25" in this case? The binding file
> > Documentation/devicetree/bindings/misc/at25.txt also has the same name.
> >
> > Are you OK, if we add something like below to the changelog?
> >
> >  In addition we hard-code the name of the chip to be "at25" for the
> >  reason that there is no common mechanism to fetch name of the firmware
> >  node. The only existing user (arch/arm/boot/dts/phy3250.dts) uses the
> >  same name so it should continue to work.
> 
> Yes. If somebody complains, then we can reinstate the previous
> behaviour, but assume it isn't necessary for now.

OK, done in my tree.  I've added the ACK too, but that still is in my
bleeding-edge branch for now until it gets build tested and I can
pull http://marc.info/?l=linux-kernel&m=141505792126265&w=4 from
somewhere. :-)

Rafael


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

* Re: [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware
  2014-11-04 15:51   ` Grant Likely
@ 2014-11-04 16:20     ` Rafael J. Wysocki
  0 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-11-04 16:20 UTC (permalink / raw)
  To: Grant Likely
  Cc: Linux Kernel Mailing List, Arnd Bergmann, ACPI Devel Maling List,
	Greg Kroah-Hartman, Darren Hart, Mika Westerberg, Aaron Lu,
	devicetree, Linus Walleij, Alexandre Courbot, Dmitry Torokhov,
	Bryan Wu

On Tuesday, November 04, 2014 03:51:12 PM Grant Likely wrote:
> On Tue, 21 Oct 2014 23:15:50 +0200
> , "Rafael J. Wysocki" <rjw@rjwysocki.net>
>  wrote:
> > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > 
> > Add a uniform interface by which device drivers can request device
> > properties from the platform firmware by providing a property name
> > and the corresponding data type.  The purpose of it is to help to
> > write portable code that won't depend on any particular platform
> > firmware interface.
> > 
> > The following general helper functions are added:
> > 
> > device_property_present()
> > device_property_read_u8()
> > device_property_read_u16()
> > device_property_read_u32()
> > device_property_read_u64()
> > device_property_read_string()
> > device_property_read_u8_array()
> > device_property_read_u16_array()
> > device_property_read_u32_array()
> > device_property_read_u64_array()
> > device_property_read_string_array()
> > 
> > The first one allows the caller to check if the given property is
> > present.  The next 5 of them allow single-valued properties of
> > various types to be retrieved in a uniform way.  The remaining 5 are
> > for reading properties with multiple values (arrays of either numbers
> > or strings).
> > 
> > The interface covers both ACPI and Device Trees.
> > 
> > This change set includes material from Mika Westerberg and Aaron Lu.
> > 
> > Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> > 
> > Changes from v5:
> > - acpi_dev_prop_read() can now handle both list (package) and single-value
> >   properties (the latter are tried first for the last argument equal to 1).
> > - There is a new macro to generate the bodies of device_property_read_u*_array().
> > - device_property_read_u*() are implemented using device_property_read_u*_array().
> > - device_property_read_bool() is a new static inline wrapper around
> >   device_property_present().
> > 
> > ---
> >  drivers/acpi/property.c  |  178 +++++++++++++++++++++++++++++++++++
> >  drivers/base/Makefile    |    2 
> >  drivers/base/property.c  |  233 +++++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/of/base.c        |  106 ++++++++++++++++++---
> >  include/linux/acpi.h     |   32 ++++++
> >  include/linux/of.h       |   22 ++++
> >  include/linux/property.h |   53 ++++++++++
> >  7 files changed, 609 insertions(+), 17 deletions(-)
> >  create mode 100644 drivers/base/property.c
> >  create mode 100644 include/linux/property.h
> > 
> > Index: linux-pm/include/linux/property.h
> > ===================================================================
> > --- /dev/null
> > +++ linux-pm/include/linux/property.h
> > @@ -0,0 +1,53 @@
> > +/*
> > + * property.h - Unified device property interface.
> > + *
> > + * Copyright (C) 2014, Intel Corporation
> > + * Authors: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > + *          Mika Westerberg <mika.westerberg@linux.intel.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#ifndef _LINUX_PROPERTY_H_
> > +#define _LINUX_PROPERTY_H_
> > +
> > +#include <linux/types.h>
> > +
> > +struct device;
> > +
> > +enum dev_prop_type {
> > +	DEV_PROP_U8,
> > +	DEV_PROP_U16,
> > +	DEV_PROP_U32,
> > +	DEV_PROP_U64,
> > +	DEV_PROP_STRING,
> > +	DEV_PROP_MAX,
> > +};
> > +
> > +bool device_property_present(struct device *dev, const char *propname);
> > +int device_property_read_u8_array(struct device *dev, const char *propname,
> > +				  u8 *val, size_t nval);
> > +int device_property_read_u16_array(struct device *dev, const char *propname,
> > +				   u16 *val, size_t nval);
> > +int device_property_read_u32_array(struct device *dev, const char *propname,
> > +				   u32 *val, size_t nval);
> > +int device_property_read_u64_array(struct device *dev, const char *propname,
> > +				   u64 *val, size_t nval);
> > +int device_property_read_string_array(struct device *dev, const char *propname,
> > +				      char **val, size_t nval);
> 
> I'm not sure if I asked this elsewhere. Can 'val' be made a const char **?

You did. :-)

I have a new version of the patch with that change.  I'll send it in a while.

Rafael


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

* Re: [PATCH v6 00/12] Add ACPI _DSD and unified device properties support
  2014-11-04 15:49   ` Grant Likely
@ 2014-11-04 16:20     ` Rafael J. Wysocki
  0 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-11-04 16:20 UTC (permalink / raw)
  To: Grant Likely
  Cc: Linux Kernel Mailing List, ACPI Devel Maling List,
	Greg Kroah-Hartman, Darren Hart, Mika Westerberg, Aaron Lu,
	Arnd Bergmann, devicetree, Linus Walleij, Alexandre Courbot,
	Dmitry Torokhov, Bryan Wu

On Tuesday, November 04, 2014 03:49:50 PM Grant Likely wrote:
> On Sat, 25 Oct 2014 00:10:20 +0200
> , "Rafael J. Wysocki" <rjw@rjwysocki.net>
>  wrote:
> > On Tuesday, October 21, 2014 11:08:59 PM Rafael J. Wysocki wrote:
> > > Hi Everyone,
> > > 
> > > This is version 6 of the unified device properties interface patchset.
> > > 
> > > The original cover letter from Mika is here:
> > > 
> > > http://marc.info/?l=devicetree&m=141087052200600&w=4
> > > 
> > > and my cover letters for previous iterations are at:
> > > 
> > > http://marc.info/?l=linux-acpi&m=141212903816560&w=4
> > > http://marc.info/?l=linux-kernel&m=141354745011569&w=4
> > > 
> > > There are a few changes with respect to v5 and the affected patches are
> > > [02-03/12] and [09-12/12].  The remaining ones have not been modified.
> > > 
> > > Most importantly, requesting the first element of a list (package) property
> > > from _DSD is now equivalent to accessing a single-value property of the
> > > same type, so device_property_read_u8(dev, pname, val) will now be equivalent
> > > to device_property_read_u8_array(dev, pname, val, 1), for example.
> > > Consequently, this _DSD definition:
> > > 
> > > Name (_DSD, Package () {
> > >     ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > >     Package () {
> > >         Package () {"blah", "A string"},
> > >     }
> > > })
> > > 
> > > can be used instead of
> > > 
> > > Name (_DSD, Package () {
> > >     ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > >     Package () {
> > >         Package () {"blah", Package () {"A string"}},
> > >     }
> > > })
> > > 
> > > and the code will be able to retrieve the property value from the both of
> > > them just fine.
> > > 
> > > This means, among other things, that accessors for single-value properties
> > > can be implemented in terms of the analogous "array" property accessors
> > > which allows the code size to be reduced somewhat.
> > > 
> > > Patches [02/12] and [09/12] have been modified to achieve that and patch
> > > [03/12] have been modified accordingly for the "compatible" property in
> > > _DSD to behave in an analogous way.  Additionally, the bodies of the
> > > numerical property accessors in patches [02/12] and [09/12] are now
> > > generated using macros (string property accessors have slightly different
> > > rules and are simply open coded for that reason).
> > > 
> > > Patch [10/12] has been modified to drop function arguments that happened to
> > > have the same values for both of the current users of those functions and
> > > patches [11-12/12] have been modified to take that change into account.  If
> > > the code in question needs to be made more complex in the future, there
> > > should not be any problems with that.
> > > 
> > > Due to the nature of the changes I have retained all ACKs except for the
> > > Grant's Reviewed-by on patch [03/12] (if that had been Acked-by, I would have
> > > retained it too, but that didn't feel appropriate for the "reviewed by" thing
> > > to me).  If any of you think that the ACKs are not applicable any more, please
> > > let me know and I'll drop them.
> > > 
> > > Finally, many thanks to Mika for testing the series on MinnowBoard 1 and
> > > MinnowBoard Max.  In case anybody else would like to test it, it is available
> > > from the device-properties branch of the linux-pm.git tree:
> > > 
> > > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git device-properties
> > > 
> > > Thanks!
> > 
> > Crickets ...
> > 
> > OK, so I'm taking the lack of comments as the lack of objections and I'm already
> > getting merge conflicts for this series.  Moreover, we already have done some
> > work on top of it.
> > 
> > So, if there still are no comments by Sunday evening, I'll add this series to
> > my linux-next branch with 3.19-rc1 as the target.
> 
> Aside from the comments I've made elsewhere, you can add my acked by for the whole series.
> 
> Acked-by: Grant Likely <grant.likely@linaro.org>

Thanks!


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

* [Update][PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware
  2014-10-21 21:15 ` [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware Rafael J. Wysocki
  2014-11-03 15:40   ` Grant Likely
  2014-11-04 15:51   ` Grant Likely
@ 2014-11-04 16:38   ` Rafael J. Wysocki
  2 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-11-04 16:38 UTC (permalink / raw)
  To: Linux Kernel Mailing List, Grant Likely
  Cc: Arnd Bergmann, ACPI Devel Maling List, Greg Kroah-Hartman,
	Darren Hart, Mika Westerberg, Aaron Lu, devicetree,
	Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Add a uniform interface by which device drivers can request device
properties from the platform firmware by providing a property name
and the corresponding data type.  The purpose of it is to help to
write portable code that won't depend on any particular platform
firmware interface.

The following general helper functions are added:

device_property_present()
device_property_read_u8()
device_property_read_u16()
device_property_read_u32()
device_property_read_u64()
device_property_read_string()
device_property_read_u8_array()
device_property_read_u16_array()
device_property_read_u32_array()
device_property_read_u64_array()
device_property_read_string_array()

The first one allows the caller to check if the given property is
present.  The next 5 of them allow single-valued properties of
various types to be retrieved in a uniform way.  The remaining 5 are
for reading properties with multiple values (arrays of either numbers
or strings).

The interface covers both ACPI and Device Trees.

This change set includes material from Mika Westerberg and Aaron Lu.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This has been rebased on top of

	http://marc.info/?l=linux-kernel&m=141505792126265&w=4

(with the const bug fixed).

Grant, I've also made the other changes suggested by you.

Now, this *really* needs to stabilize, so I'd very much prefer to do any
subsequent fix-ups on top of this series rather than by modifying the
patches in it.

Rafael

---
 drivers/acpi/property.c  |  178 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/base/Makefile    |    2 
 drivers/base/property.c  |  185 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/of/base.c        |   33 ++++++++
 include/linux/acpi.h     |   32 ++++++++
 include/linux/of.h       |   12 +++
 include/linux/property.h |   73 ++++++++++++++++++
 7 files changed, 514 insertions(+), 1 deletion(-)
 create mode 100644 drivers/base/property.c
 create mode 100644 include/linux/property.h

Index: linux-pm/include/linux/property.h
===================================================================
--- /dev/null
+++ linux-pm/include/linux/property.h
@@ -0,0 +1,73 @@
+/*
+ * property.h - Unified device property interface.
+ *
+ * Copyright (C) 2014, Intel Corporation
+ * Authors: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+ *          Mika Westerberg <mika.westerberg@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _LINUX_PROPERTY_H_
+#define _LINUX_PROPERTY_H_
+
+#include <linux/types.h>
+
+struct device;
+
+enum dev_prop_type {
+	DEV_PROP_U8,
+	DEV_PROP_U16,
+	DEV_PROP_U32,
+	DEV_PROP_U64,
+	DEV_PROP_STRING,
+	DEV_PROP_MAX,
+};
+
+bool device_property_present(struct device *dev, const char *propname);
+int device_property_read_u8_array(struct device *dev, const char *propname,
+				  u8 *val, size_t nval);
+int device_property_read_u16_array(struct device *dev, const char *propname,
+				   u16 *val, size_t nval);
+int device_property_read_u32_array(struct device *dev, const char *propname,
+				   u32 *val, size_t nval);
+int device_property_read_u64_array(struct device *dev, const char *propname,
+				   u64 *val, size_t nval);
+int device_property_read_string_array(struct device *dev, const char *propname,
+				      const char **val, size_t nval);
+int device_property_read_string(struct device *dev, const char *propname,
+				const char **val);
+
+static inline bool device_property_read_bool(struct device *dev,
+					     const char *propname)
+{
+	return device_property_present(dev, propname);
+}
+
+static inline int device_property_read_u8(struct device *dev,
+					  const char *propname, u8 *val)
+{
+	return device_property_read_u8_array(dev, propname, val, 1);
+}
+
+static inline int device_property_read_u16(struct device *dev,
+					   const char *propname, u16 *val)
+{
+	return device_property_read_u16_array(dev, propname, val, 1);
+}
+
+static inline int device_property_read_u32(struct device *dev,
+					   const char *propname, u32 *val)
+{
+	return device_property_read_u32_array(dev, propname, val, 1);
+}
+
+static inline int device_property_read_u64(struct device *dev,
+					   const char *propname, u64 *val)
+{
+	return device_property_read_u64_array(dev, propname, val, 1);
+}
+
+#endif /* _LINUX_PROPERTY_H_ */
Index: linux-pm/include/linux/of.h
===================================================================
--- linux-pm.orig/include/linux/of.h
+++ linux-pm/include/linux/of.h
@@ -23,6 +23,7 @@
 #include <linux/spinlock.h>
 #include <linux/topology.h>
 #include <linux/notifier.h>
+#include <linux/property.h>
 
 #include <asm/byteorder.h>
 #include <asm/errno.h>
@@ -263,6 +264,10 @@ extern int of_property_read_u32_array(co
 				      size_t sz);
 extern int of_property_read_u64(const struct device_node *np,
 				const char *propname, u64 *out_value);
+extern int of_property_read_u64_array(const struct device_node *np,
+				      const char *propname,
+				      u64 *out_values,
+				      size_t sz);
 
 extern int of_property_read_string(struct device_node *np,
 				   const char *propname,
@@ -476,6 +481,13 @@ static inline int of_property_read_u32_a
 {
 	return -ENOSYS;
 }
+
+static inline int of_property_read_u64_array(const struct device_node *np,
+					     const char *propname,
+					     u64 *out_values, size_t sz)
+{
+	return -ENOSYS;
+}
 
 static inline int of_property_read_string(struct device_node *np,
 					  const char *propname,
Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -28,6 +28,7 @@
 #include <linux/errno.h>
 #include <linux/ioport.h>	/* for struct resource */
 #include <linux/device.h>
+#include <linux/property.h>
 
 #ifndef _LINUX
 #define _LINUX
@@ -677,6 +678,13 @@ int acpi_dev_get_property_array(struct a
 int acpi_dev_get_property_reference(struct acpi_device *adev, const char *name,
 				    const char *cells_name, size_t index,
 				    struct acpi_reference_args *args);
+
+int acpi_dev_prop_get(struct acpi_device *adev, const char *propname,
+		      void **valptr);
+int acpi_dev_prop_read_single(struct acpi_device *adev, const char *propname,
+			      enum dev_prop_type proptype, void *val);
+int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
+		       enum dev_prop_type proptype, void *val, size_t nval);
 #else
 static inline int acpi_dev_get_property(struct acpi_device *adev,
 					const char *name, acpi_object_type type,
@@ -697,6 +705,30 @@ static inline int acpi_dev_get_property_
 {
 	return -ENXIO;
 }
+
+static inline int acpi_dev_prop_get(struct acpi_device *adev,
+				    const char *propname,
+				    void **valptr)
+{
+	return -ENXIO;
+}
+
+static inline int acpi_dev_prop_read_single(struct acpi_device *adev,
+					    const char *propname,
+					    enum dev_prop_type proptype,
+					    void *val)
+{
+	return -ENXIO;
+}
+
+static inline int acpi_dev_prop_read(struct acpi_device *adev,
+				     const char *propname,
+				     enum dev_prop_type proptype,
+				     void *val, size_t nval)
+{
+	return -ENXIO;
+}
+
 #endif
 
 #endif	/*_LINUX_ACPI_H*/
Index: linux-pm/drivers/base/Makefile
===================================================================
--- linux-pm.orig/drivers/base/Makefile
+++ linux-pm/drivers/base/Makefile
@@ -4,7 +4,7 @@ obj-y			:= component.o core.o bus.o dd.o
 			   driver.o class.o platform.o \
 			   cpu.o firmware.o init.o map.o devres.o \
 			   attribute_container.o transport_class.o \
-			   topology.o container.o
+			   topology.o container.o property.o
 obj-$(CONFIG_DEVTMPFS)	+= devtmpfs.o
 obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
 obj-y			+= power/
Index: linux-pm/drivers/base/property.c
===================================================================
--- /dev/null
+++ linux-pm/drivers/base/property.c
@@ -0,0 +1,185 @@
+/*
+ * property.c - Unified device property interface.
+ *
+ * Copyright (C) 2014, Intel Corporation
+ * Authors: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+ *          Mika Westerberg <mika.westerberg@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/property.h>
+#include <linux/export.h>
+#include <linux/acpi.h>
+#include <linux/of.h>
+
+/**
+ * device_property_present - check if a property of a device is present
+ * @dev: Device whose property is being checked
+ * @propname: Name of the property
+ *
+ * Check if property @propname is present in the device firmware description.
+ */
+bool device_property_present(struct device *dev, const char *propname)
+{
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+		return of_property_read_bool(dev->of_node, propname);
+
+	return !acpi_dev_prop_get(ACPI_COMPANION(dev), propname, NULL);
+}
+EXPORT_SYMBOL_GPL(device_property_present);
+
+#define OF_DEV_PROP_READ_ARRAY(node, propname, type, val, nval) \
+	(val) ? of_property_read_##type##_array((node), (propname), (val), (nval)) \
+	      : of_property_count_elems_of_size((node), (propname), sizeof(type))
+
+#define DEV_PROP_READ_ARRAY(_dev_, _propname_, _type_, _proptype_, _val_, _nval_) \
+	IS_ENABLED(CONFIG_OF) && _dev_->of_node ? \
+		(OF_DEV_PROP_READ_ARRAY(_dev_->of_node, _propname_, _type_, \
+					_val_, _nval_)) : \
+		acpi_dev_prop_read(ACPI_COMPANION(_dev_), _propname_, \
+				   _proptype_, _val_, _nval_)
+
+/**
+ * device_property_read_u8_array - return a u8 array property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of u8 properties with @propname from the device
+ * firmware description and stores them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected.
+ */
+int device_property_read_u8_array(struct device *dev, const char *propname,
+				  u8 *val, size_t nval)
+{
+	return DEV_PROP_READ_ARRAY(dev, propname, u8, DEV_PROP_U8, val, nval);
+}
+EXPORT_SYMBOL_GPL(device_property_read_u8_array);
+
+/**
+ * device_property_read_u16_array - return a u16 array property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of u16 properties with @propname from the device
+ * firmware description and stores them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected.
+ */
+int device_property_read_u16_array(struct device *dev, const char *propname,
+				   u16 *val, size_t nval)
+{
+	return DEV_PROP_READ_ARRAY(dev, propname, u16, DEV_PROP_U16, val, nval);
+}
+EXPORT_SYMBOL_GPL(device_property_read_u16_array);
+
+/**
+ * device_property_read_u32_array - return a u32 array property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of u32 properties with @propname from the device
+ * firmware description and stores them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected.
+ */
+int device_property_read_u32_array(struct device *dev, const char *propname,
+				   u32 *val, size_t nval)
+{
+	return DEV_PROP_READ_ARRAY(dev, propname, u32, DEV_PROP_U32, val, nval);
+}
+EXPORT_SYMBOL_GPL(device_property_read_u32_array);
+
+/**
+ * device_property_read_u64_array - return a u64 array property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of u64 properties with @propname from the device
+ * firmware description and stores them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected.
+ */
+int device_property_read_u64_array(struct device *dev, const char *propname,
+				   u64 *val, size_t nval)
+{
+	return DEV_PROP_READ_ARRAY(dev, propname, u64, DEV_PROP_U64, val, nval);
+}
+EXPORT_SYMBOL_GPL(device_property_read_u64_array);
+
+/**
+ * device_property_read_string_array - return a string array property of device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of string properties with @propname from the device
+ * firmware description and stores them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO or %-EILSEQ if the property is not an array of strings,
+ *	   %-EOVERFLOW if the size of the property is not as expected.
+ */
+int device_property_read_string_array(struct device *dev, const char *propname,
+				      const char **val, size_t nval)
+{
+	return IS_ENABLED(CONFIG_OF) && dev->of_node ?
+		of_property_read_string_array(dev->of_node, propname, val, nval) :
+		acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
+				   DEV_PROP_STRING, val, nval);
+}
+EXPORT_SYMBOL_GPL(device_property_read_string_array);
+
+/**
+ * device_property_read_string - return a string property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The value is stored here
+ *
+ * Function reads property @propname from the device firmware description and
+ * stores the value into @val if found. The value is checked to be a string.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO or %-EILSEQ if the property type is not a string.
+ */
+int device_property_read_string(struct device *dev, const char *propname,
+				const char **val)
+{
+	return IS_ENABLED(CONFIG_OF) && dev->of_node ?
+		of_property_read_string(dev->of_node, propname, val) :
+		acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
+				   DEV_PROP_STRING, val, 1);
+}
+EXPORT_SYMBOL_GPL(device_property_read_string);
Index: linux-pm/drivers/of/base.c
===================================================================
--- linux-pm.orig/drivers/of/base.c
+++ linux-pm/drivers/of/base.c
@@ -1250,6 +1250,39 @@ int of_property_read_u64(const struct de
 EXPORT_SYMBOL_GPL(of_property_read_u64);
 
 /**
+ * of_property_read_u64_array - Find and read an array of 64 bit integers
+ * from a property.
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ * @out_values:	pointer to return value, modified only if return value is 0.
+ * @sz:		number of array elements to read
+ *
+ * Search for a property in a device node and read 64-bit value(s) from
+ * it. Returns 0 on success, -EINVAL if the property does not exist,
+ * -ENODATA if property does not have a value, and -EOVERFLOW if the
+ * property data isn't large enough.
+ *
+ * The out_values is modified only if a valid u64 value can be decoded.
+ */
+int of_property_read_u64_array(const struct device_node *np,
+			       const char *propname, u64 *out_values,
+			       size_t sz)
+{
+	const __be32 *val = of_find_property_value_of_size(np, propname,
+						(sz * sizeof(*out_values)));
+
+	if (IS_ERR(val))
+		return PTR_ERR(val);
+
+	while (sz--) {
+		*out_values++ = of_read_number(val, 2);
+		val += 2;
+	}
+	return 0;
+}
+
+/**
  * of_property_read_string - Find and read a string from a property
  * @np:		device node from which the property value is to be read.
  * @propname:	name of the property to be searched.
Index: linux-pm/drivers/acpi/property.c
===================================================================
--- linux-pm.orig/drivers/acpi/property.c
+++ linux-pm/drivers/acpi/property.c
@@ -362,3 +362,181 @@ int acpi_dev_get_property_reference(stru
 	return -EPROTO;
 }
 EXPORT_SYMBOL_GPL(acpi_dev_get_property_reference);
+
+int acpi_dev_prop_get(struct acpi_device *adev, const char *propname,
+		      void **valptr)
+{
+	return acpi_dev_get_property(adev, propname, ACPI_TYPE_ANY,
+				     (const union acpi_object **)valptr);
+}
+
+int acpi_dev_prop_read_single(struct acpi_device *adev, const char *propname,
+			      enum dev_prop_type proptype, void *val)
+{
+	const union acpi_object *obj;
+	int ret;
+
+	if (!val)
+		return -EINVAL;
+
+	if (proptype >= DEV_PROP_U8 && proptype <= DEV_PROP_U64) {
+		ret = acpi_dev_get_property(adev, propname, ACPI_TYPE_INTEGER, &obj);
+		if (ret)
+			return ret;
+
+		switch (proptype) {
+		case DEV_PROP_U8:
+			if (obj->integer.value > U8_MAX)
+				return -EOVERFLOW;
+			*(u8 *)val = obj->integer.value;
+			break;
+		case DEV_PROP_U16:
+			if (obj->integer.value > U16_MAX)
+				return -EOVERFLOW;
+			*(u16 *)val = obj->integer.value;
+			break;
+		case DEV_PROP_U32:
+			if (obj->integer.value > U32_MAX)
+				return -EOVERFLOW;
+			*(u32 *)val = obj->integer.value;
+			break;
+		default:
+			*(u64 *)val = obj->integer.value;
+			break;
+		}
+	} else if (proptype == DEV_PROP_STRING) {
+		ret = acpi_dev_get_property(adev, propname, ACPI_TYPE_STRING, &obj);
+		if (ret)
+			return ret;
+
+		*(char **)val = obj->string.pointer;
+	} else {
+		ret = -EINVAL;
+	}
+	return ret;
+}
+
+static int acpi_copy_property_array_u8(const union acpi_object *items, u8 *val,
+				       size_t nval)
+{
+	int i;
+
+	for (i = 0; i < nval; i++) {
+		if (items[i].type != ACPI_TYPE_INTEGER)
+			return -EPROTO;
+		if (items[i].integer.value > U8_MAX)
+			return -EOVERFLOW;
+
+		val[i] = items[i].integer.value;
+	}
+	return 0;
+}
+
+static int acpi_copy_property_array_u16(const union acpi_object *items,
+					u16 *val, size_t nval)
+{
+	int i;
+
+	for (i = 0; i < nval; i++) {
+		if (items[i].type != ACPI_TYPE_INTEGER)
+			return -EPROTO;
+		if (items[i].integer.value > U16_MAX)
+			return -EOVERFLOW;
+
+		val[i] = items[i].integer.value;
+	}
+	return 0;
+}
+
+static int acpi_copy_property_array_u32(const union acpi_object *items,
+					u32 *val, size_t nval)
+{
+	int i;
+
+	for (i = 0; i < nval; i++) {
+		if (items[i].type != ACPI_TYPE_INTEGER)
+			return -EPROTO;
+		if (items[i].integer.value > U32_MAX)
+			return -EOVERFLOW;
+
+		val[i] = items[i].integer.value;
+	}
+	return 0;
+}
+
+static int acpi_copy_property_array_u64(const union acpi_object *items,
+					u64 *val, size_t nval)
+{
+	int i;
+
+	for (i = 0; i < nval; i++) {
+		if (items[i].type != ACPI_TYPE_INTEGER)
+			return -EPROTO;
+
+		val[i] = items[i].integer.value;
+	}
+	return 0;
+}
+
+static int acpi_copy_property_array_string(const union acpi_object *items,
+					   char **val, size_t nval)
+{
+	int i;
+
+	for (i = 0; i < nval; i++) {
+		if (items[i].type != ACPI_TYPE_STRING)
+			return -EPROTO;
+
+		val[i] = items[i].string.pointer;
+	}
+	return 0;
+}
+
+int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
+		       enum dev_prop_type proptype, void *val, size_t nval)
+{
+	const union acpi_object *obj;
+	const union acpi_object *items;
+	int ret;
+
+	if (val && nval == 1) {
+		ret = acpi_dev_prop_read_single(adev, propname, proptype, val);
+		if (!ret)
+			return ret;
+	}
+
+	ret = acpi_dev_get_property_array(adev, propname, ACPI_TYPE_ANY, &obj);
+	if (ret)
+		return ret;
+
+	if (!val)
+		return obj->package.count;
+	else if (nval <= 0)
+		return -EINVAL;
+
+	if (nval > obj->package.count)
+		return -EOVERFLOW;
+
+	items = obj->package.elements;
+	switch (proptype) {
+	case DEV_PROP_U8:
+		ret = acpi_copy_property_array_u8(items, (u8 *)val, nval);
+		break;
+	case DEV_PROP_U16:
+		ret = acpi_copy_property_array_u16(items, (u16 *)val, nval);
+		break;
+	case DEV_PROP_U32:
+		ret = acpi_copy_property_array_u32(items, (u32 *)val, nval);
+		break;
+	case DEV_PROP_U64:
+		ret = acpi_copy_property_array_u64(items, (u64 *)val, nval);
+		break;
+	case DEV_PROP_STRING:
+		ret = acpi_copy_property_array_string(items, (char **)val, nval);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	return ret;
+}


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

* [Update][PATCH v6 09/12] Driver core: Unified interface for firmware node properties
  2014-10-21 21:29 ` [PATCH v6 09/12] Driver core: Unified interface for firmware node properties Rafael J. Wysocki
@ 2014-11-04 16:43   ` Rafael J. Wysocki
  0 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-11-04 16:43 UTC (permalink / raw)
  To: Linux Kernel Mailing List, Grant Likely
  Cc: Arnd Bergmann, ACPI Devel Maling List, Greg Kroah-Hartman,
	Darren Hart, Mika Westerberg, Aaron Lu, devicetree,
	Linus Walleij, Alexandre Courbot, Dmitry Torokhov, Bryan Wu

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Add new generic routines are provided for retrieving properties from
device description objects in the platform firmware in case there are
no struct device objects for them (either those objects have not been
created yet or they do not exist at all).

The following functions are provided:

fwnode_property_present()
fwnode_property_read_u8()
fwnode_property_read_u16()
fwnode_property_read_u32()
fwnode_property_read_u64()
fwnode_property_read_string()
fwnode_property_read_u8_array()
fwnode_property_read_u16_array()
fwnode_property_read_u32_array()
fwnode_property_read_u64_array()
fwnode_property_read_string_array()

in analogy with the corresponding functions for struct device added
previously.  For all of them, the first argument is a pointer to struct
fwnode_handle (new type) that allows a device description object
(depending on what platform firmware interface is in use) to be
obtained.

Add a new macro device_for_each_child_node() for iterating over the
children of the device description object associated with a given
device and a new function device_get_child_node_count() returning the
number of a given device's child nodes.

The interface covers both ACPI and Device Trees.

Suggested-by: Grant Likely <grant.likely@linaro.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Rebased on top of http://marc.info/?l=linux-kernel&m=141511786113206&w=4

Grant, I've also made changes following your suggestions for patch [2/12].

Rafael

---
 drivers/acpi/scan.c      |   21 ++++
 drivers/base/property.c  |  246 +++++++++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h  |   17 +++
 include/linux/acpi.h     |   26 ++++
 include/linux/of.h       |   22 ++++
 include/linux/property.h |   70 +++++++++++++
 6 files changed, 402 insertions(+)

Index: linux-pm/include/linux/property.h
===================================================================
--- linux-pm.orig/include/linux/property.h
+++ linux-pm/include/linux/property.h
@@ -40,6 +40,46 @@ int device_property_read_string_array(st
 int device_property_read_string(struct device *dev, const char *propname,
 				const char **val);
 
+enum fwnode_type {
+	FWNODE_INVALID = 0,
+	FWNODE_OF,
+	FWNODE_ACPI,
+};
+
+struct fwnode_handle {
+	enum fwnode_type type;
+};
+
+bool fwnode_property_present(struct fwnode_handle *fwnode, const char *propname);
+int fwnode_property_read_u8_array(struct fwnode_handle *fwnode,
+				  const char *propname, u8 *val,
+				  size_t nval);
+int fwnode_property_read_u16_array(struct fwnode_handle *fwnode,
+				   const char *propname, u16 *val,
+				   size_t nval);
+int fwnode_property_read_u32_array(struct fwnode_handle *fwnode,
+				   const char *propname, u32 *val,
+				   size_t nval);
+int fwnode_property_read_u64_array(struct fwnode_handle *fwnode,
+				   const char *propname, u64 *val,
+				   size_t nval);
+int fwnode_property_read_string_array(struct fwnode_handle *fwnode,
+				      const char *propname, const char **val,
+				      size_t nval);
+int fwnode_property_read_string(struct fwnode_handle *fwnode,
+				const char *propname, const char **val);
+
+struct fwnode_handle *device_get_next_child_node(struct device *dev,
+						 struct fwnode_handle *child);
+
+#define device_for_each_child_node(dev, child) \
+	for (child = device_get_next_child_node(dev, NULL); child; \
+	     child = device_get_next_child_node(dev, child))
+
+void fwnode_handle_put(struct fwnode_handle *fwnode);
+
+unsigned int device_get_child_node_count(struct device *dev);
+
 static inline bool device_property_read_bool(struct device *dev,
 					     const char *propname)
 {
@@ -70,4 +110,34 @@ static inline int device_property_read_u
 	return device_property_read_u64_array(dev, propname, val, 1);
 }
 
+static inline bool fwnode_property_read_bool(struct fwnode_handle *fwnode,
+					     const char *propname)
+{
+	return fwnode_property_present(fwnode, propname);
+}
+
+static inline int fwnode_property_read_u8(struct fwnode_handle *fwnode,
+					  const char *propname, u8 *val)
+{
+	return fwnode_property_read_u8_array(fwnode, propname, val, 1);
+}
+
+static inline int fwnode_property_read_u16(struct fwnode_handle *fwnode,
+					   const char *propname, u16 *val)
+{
+	return fwnode_property_read_u16_array(fwnode, propname, val, 1);
+}
+
+static inline int fwnode_property_read_u32(struct fwnode_handle *fwnode,
+					   const char *propname, u32 *val)
+{
+	return fwnode_property_read_u32_array(fwnode, propname, val, 1);
+}
+
+static inline int fwnode_property_read_u64(struct fwnode_handle *fwnode,
+					   const char *propname, u64 *val)
+{
+	return fwnode_property_read_u64_array(fwnode, propname, val, 1);
+}
+
 #endif /* _LINUX_PROPERTY_H_ */
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -27,6 +27,7 @@
 #define __ACPI_BUS_H__
 
 #include <linux/device.h>
+#include <linux/property.h>
 
 /* TBD: Make dynamic */
 #define ACPI_MAX_HANDLES	10
@@ -348,6 +349,7 @@ struct acpi_device_data {
 struct acpi_device {
 	int device_type;
 	acpi_handle handle;		/* no handle for fixed hardware */
+	struct fwnode_handle fwnode;
 	struct acpi_device *parent;
 	struct list_head children;
 	struct list_head node;
@@ -372,6 +374,21 @@ struct acpi_device {
 	void (*remove)(struct acpi_device *);
 };
 
+static inline bool is_acpi_node(struct fwnode_handle *fwnode)
+{
+	return fwnode && fwnode->type == FWNODE_ACPI;
+}
+
+static inline struct acpi_device *acpi_node(struct fwnode_handle *fwnode)
+{
+	return fwnode ? container_of(fwnode, struct acpi_device, fwnode) : NULL;
+}
+
+static inline struct fwnode_handle *acpi_fwnode_handle(struct acpi_device *adev)
+{
+	return &adev->fwnode;
+}
+
 static inline void *acpi_driver_data(struct acpi_device *d)
 {
 	return d->driver_data;
Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -440,6 +440,23 @@ struct platform_device *acpi_create_plat
 #define ACPI_COMPANION_SET(dev, adev)	do { } while (0)
 #define ACPI_HANDLE(dev)		(NULL)
 
+struct fwnode_handle;
+
+static inline bool is_acpi_node(struct fwnode_handle *fwnode)
+{
+	return false;
+}
+
+static inline struct acpi_device *acpi_node(struct fwnode_handle *fwnode)
+{
+	return NULL;
+}
+
+static inline struct fwnode_handle *acpi_fwnode_handle(struct acpi_device *adev)
+{
+	return NULL;
+}
+
 static inline const char *acpi_dev_name(struct acpi_device *adev)
 {
 	return NULL;
@@ -681,6 +698,9 @@ int acpi_dev_prop_read_single(struct acp
 			      enum dev_prop_type proptype, void *val);
 int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
 		       enum dev_prop_type proptype, void *val, size_t nval);
+
+struct acpi_device *acpi_get_next_child(struct device *dev,
+					struct acpi_device *child);
 #else
 static inline int acpi_dev_get_property(struct acpi_device *adev,
 					const char *name, acpi_object_type type,
@@ -725,6 +745,12 @@ static inline int acpi_dev_prop_read(str
 	return -ENXIO;
 }
 
+static inline struct acpi_device *acpi_get_next_child(struct device *dev,
+						      struct acpi_device *child)
+{
+	return NULL;
+}
+
 #endif
 
 #endif	/*_LINUX_ACPI_H*/
Index: linux-pm/include/linux/of.h
===================================================================
--- linux-pm.orig/include/linux/of.h
+++ linux-pm/include/linux/of.h
@@ -50,6 +50,7 @@ struct device_node {
 	const char *type;
 	phandle phandle;
 	const char *full_name;
+	struct fwnode_handle fwnode;
 
 	struct	property *properties;
 	struct	property *deadprops;	/* removed properties */
@@ -80,6 +81,7 @@ extern struct kobj_type of_node_ktype;
 static inline void of_node_init(struct device_node *node)
 {
 	kobject_init(&node->kobj, &of_node_ktype);
+	node->fwnode.type = FWNODE_OF;
 }
 
 /* true when node is initialized */
@@ -115,6 +117,16 @@ extern struct device_node *of_aliases;
 extern struct device_node *of_stdout;
 extern raw_spinlock_t devtree_lock;
 
+static inline bool is_of_node(struct fwnode_handle *fwnode)
+{
+	return fwnode && fwnode->type == FWNODE_OF;
+}
+
+static inline struct device_node *of_node(struct fwnode_handle *fwnode)
+{
+	return fwnode ? container_of(fwnode, struct device_node, fwnode) : NULL;
+}
+
 static inline bool of_have_populated_dt(void)
 {
 	return of_allnodes != NULL;
@@ -360,6 +372,16 @@ bool of_console_check(struct device_node
 
 #else /* CONFIG_OF */
 
+static inline bool is_of_node(struct fwnode_handle *fwnode)
+{
+	return false;
+}
+
+static inline struct device_node *of_node(struct fwnode_handle *fwnode)
+{
+	return NULL;
+}
+
 static inline const char* of_node_full_name(const struct device_node *np)
 {
 	return "<no-node>";
Index: linux-pm/drivers/base/property.c
===================================================================
--- linux-pm.orig/drivers/base/property.c
+++ linux-pm/drivers/base/property.c
@@ -31,6 +31,22 @@ bool device_property_present(struct devi
 }
 EXPORT_SYMBOL_GPL(device_property_present);
 
+/**
+ * fwnode_property_present - check if a property of a firmware node is present
+ * @fwnode: Firmware node whose property to check
+ * @propname: Name of the property
+ */
+bool fwnode_property_present(struct fwnode_handle *fwnode, const char *propname)
+{
+	if (is_of_node(fwnode))
+		return of_property_read_bool(of_node(fwnode), propname);
+	else if (is_acpi_node(fwnode))
+		return !acpi_dev_prop_get(acpi_node(fwnode), propname, NULL);
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(fwnode_property_present);
+
 #define OF_DEV_PROP_READ_ARRAY(node, propname, type, val, nval) \
 	(val) ? of_property_read_##type##_array((node), (propname), (val), (nval)) \
 	      : of_property_count_elems_of_size((node), (propname), sizeof(type))
@@ -183,3 +199,233 @@ int device_property_read_string(struct d
 				   DEV_PROP_STRING, val, 1);
 }
 EXPORT_SYMBOL_GPL(device_property_read_string);
+
+#define FWNODE_PROP_READ_ARRAY(_fwnode_, _propname_, _type_, _proptype_, _val_, _nval_) \
+({ \
+	int _ret_; \
+	if (is_of_node(_fwnode_)) \
+		_ret_ = OF_DEV_PROP_READ_ARRAY(of_node(_fwnode_), _propname_, \
+					       _type_, _val_, _nval_); \
+	else if (is_acpi_node(_fwnode_)) \
+		_ret_ = acpi_dev_prop_read(acpi_node(_fwnode_), _propname_, \
+					   _proptype_, _val_, _nval_); \
+	else \
+		_ret_ = -ENXIO; \
+	_ret_; \
+})
+
+/**
+ * fwnode_property_read_u8_array - return a u8 array property of firmware node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Read an array of u8 properties with @propname from @fwnode and stores them to
+ * @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected,
+ *	   %-ENXIO if no suitable firmware interface is present.
+ */
+int fwnode_property_read_u8_array(struct fwnode_handle *fwnode,
+				  const char *propname, u8 *val, size_t nval)
+{
+	return FWNODE_PROP_READ_ARRAY(fwnode, propname, u8, DEV_PROP_U8,
+				      val, nval);
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_u8_array);
+
+/**
+ * fwnode_property_read_u16_array - return a u16 array property of firmware node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Read an array of u16 properties with @propname from @fwnode and store them to
+ * @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected,
+ *	   %-ENXIO if no suitable firmware interface is present.
+ */
+int fwnode_property_read_u16_array(struct fwnode_handle *fwnode,
+				   const char *propname, u16 *val, size_t nval)
+{
+	return FWNODE_PROP_READ_ARRAY(fwnode, propname, u16, DEV_PROP_U16,
+				      val, nval);
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_u16_array);
+
+/**
+ * fwnode_property_read_u32_array - return a u32 array property of firmware node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Read an array of u32 properties with @propname from @fwnode store them to
+ * @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected,
+ *	   %-ENXIO if no suitable firmware interface is present.
+ */
+int fwnode_property_read_u32_array(struct fwnode_handle *fwnode,
+				   const char *propname, u32 *val, size_t nval)
+{
+	return FWNODE_PROP_READ_ARRAY(fwnode, propname, u32, DEV_PROP_U32,
+				      val, nval);
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_u32_array);
+
+/**
+ * fwnode_property_read_u64_array - return a u64 array property firmware node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Read an array of u64 properties with @propname from @fwnode and store them to
+ * @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of numbers,
+ *	   %-EOVERFLOW if the size of the property is not as expected,
+ *	   %-ENXIO if no suitable firmware interface is present.
+ */
+int fwnode_property_read_u64_array(struct fwnode_handle *fwnode,
+				   const char *propname, u64 *val, size_t nval)
+{
+	return FWNODE_PROP_READ_ARRAY(fwnode, propname, u64, DEV_PROP_U64,
+				      val, nval);
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_u64_array);
+
+/**
+ * fwnode_property_read_string_array - return string array property of a node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Read an string list property @propname from the given firmware node and store
+ * them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO if the property is not an array of strings,
+ *	   %-EOVERFLOW if the size of the property is not as expected,
+ *	   %-ENXIO if no suitable firmware interface is present.
+ */
+int fwnode_property_read_string_array(struct fwnode_handle *fwnode,
+				      const char *propname, const char **val,
+				      size_t nval)
+{
+	if (is_of_node(fwnode))
+		return of_property_read_string_array(of_node(fwnode), propname,
+						     val, nval);
+	else if (is_acpi_node(fwnode))
+		return acpi_dev_prop_read(acpi_node(fwnode), propname,
+					  DEV_PROP_STRING, val, nval);
+
+	return -ENXIO;
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_string_array);
+
+/**
+ * fwnode_property_read_string - return a string property of a firmware node
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property
+ * @val: The value is stored here
+ *
+ * Read property @propname from the given firmware node and store the value into
+ * @val if found.  The value is checked to be a string.
+ *
+ * Return: %0 if the property was found (success),
+ *	   %-EINVAL if given arguments are not valid,
+ *	   %-ENODATA if the property does not have a value,
+ *	   %-EPROTO or %-EILSEQ if the property is not a string,
+ *	   %-ENXIO if no suitable firmware interface is present.
+ */
+int fwnode_property_read_string(struct fwnode_handle *fwnode,
+				const char *propname, const char **val)
+{
+	if (is_of_node(fwnode))
+		return of_property_read_string(of_node(fwnode),propname, val);
+	else if (is_acpi_node(fwnode))
+		return acpi_dev_prop_read(acpi_node(fwnode), propname,
+					  DEV_PROP_STRING, val, 1);
+
+	return -ENXIO;
+}
+EXPORT_SYMBOL_GPL(fwnode_property_read_string);
+
+/**
+ * device_get_next_child_node - Return the next child node handle for a device
+ * @dev: Device to find the next child node for.
+ * @child: Handle to one of the device's child nodes or a null handle.
+ */
+struct fwnode_handle *device_get_next_child_node(struct device *dev,
+						 struct fwnode_handle *child)
+{
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
+		struct device_node *node;
+
+		node = of_get_next_available_child(dev->of_node, of_node(child));
+		if (node)
+			return &node->fwnode;
+	} else if (IS_ENABLED(CONFIG_ACPI)) {
+		struct acpi_device *node;
+
+		node = acpi_get_next_child(dev, acpi_node(child));
+		if (node)
+			return acpi_fwnode_handle(node);
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(device_get_next_child_node);
+
+/**
+ * fwnode_handle_put - Drop reference to a device node
+ * @fwnode: Pointer to the device node to drop the reference to.
+ *
+ * This has to be used when terminating device_for_each_child_node() iteration
+ * with break or return to prevent stale device node references from being left
+ * behind.
+ */
+void fwnode_handle_put(struct fwnode_handle *fwnode)
+{
+	if (is_of_node(fwnode))
+		of_node_put(of_node(fwnode));
+}
+EXPORT_SYMBOL_GPL(fwnode_handle_put);
+
+/**
+ * device_get_child_node_count - return the number of child nodes for device
+ * @dev: Device to cound the child nodes for
+ */
+unsigned int device_get_child_node_count(struct device *dev)
+{
+	struct fwnode_handle *child;
+	unsigned int count = 0;
+
+	device_for_each_child_node(dev, child)
+		count++;
+
+	return count;
+}
+EXPORT_SYMBOL_GPL(device_get_child_node_count);
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1389,6 +1389,26 @@ int acpi_device_add(struct acpi_device *
 	return result;
 }
 
+struct acpi_device *acpi_get_next_child(struct device *dev,
+					struct acpi_device *child)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+	struct list_head *head, *next;
+
+	if (!adev)
+		return NULL;
+
+	head = &adev->children;
+	if (list_empty(head))
+		return NULL;
+
+	if (!child)
+		return list_first_entry(head, struct acpi_device, node);
+
+	next = child->node.next;
+	return next == head ? NULL : list_entry(next, struct acpi_device, node);
+}
+
 /* --------------------------------------------------------------------------
                                  Driver Management
    -------------------------------------------------------------------------- */
@@ -2008,6 +2028,7 @@ void acpi_init_device_object(struct acpi
 	device->device_type = type;
 	device->handle = handle;
 	device->parent = acpi_bus_get_parent(handle);
+	device->fwnode.type = FWNODE_ACPI;
 	acpi_set_device_status(device, sta);
 	acpi_device_get_busid(device);
 	acpi_set_pnp_ids(handle, &device->pnp, type);


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

* Re: [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware
  2014-11-03 22:04     ` Rafael J. Wysocki
@ 2014-11-04 17:01       ` Grant Likely
  2014-11-04 21:29         ` Rafael J. Wysocki
  0 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2014-11-04 17:01 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux Kernel Mailing List, Arnd Bergmann, ACPI Devel Maling List,
	Greg Kroah-Hartman, Darren Hart, Mika Westerberg, Aaron Lu,
	devicetree, Linus Walleij, Alexandre Courbot, Dmitry Torokhov,
	Bryan Wu

On Mon, Nov 3, 2014 at 10:04 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> I also found that this parser code doesn't correctly handle malformed
>> (unterminated) string properties. It will overflow. The existing
>> functions have the same problem, so it isn't something that you've
>> added. I've got a fix, and as a side effect the fix creates the _array
>> version basically for free as part of reworking
>> of_property_count_strings() and of_property_read_string_index()
>
> OK
>
> So can you please point me to a git branch containing the fix?  I'll rebase the
> patch on top of that then and everything should merge just fine.

git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git devicetree/merge

I'm going to let the patch sit in there for a few days to get
linux-next exposure before I ask Linus to pull. The pull req will go
out before the end of the week.

g.

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

* Re: [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware
  2014-11-04 17:01       ` Grant Likely
@ 2014-11-04 21:29         ` Rafael J. Wysocki
  0 siblings, 0 replies; 29+ messages in thread
From: Rafael J. Wysocki @ 2014-11-04 21:29 UTC (permalink / raw)
  To: Grant Likely
  Cc: Linux Kernel Mailing List, Arnd Bergmann, ACPI Devel Maling List,
	Greg Kroah-Hartman, Darren Hart, Mika Westerberg, Aaron Lu,
	devicetree, Linus Walleij, Alexandre Courbot, Dmitry Torokhov,
	Bryan Wu

On Tuesday, November 04, 2014 05:01:04 PM Grant Likely wrote:
> On Mon, Nov 3, 2014 at 10:04 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >> I also found that this parser code doesn't correctly handle malformed
> >> (unterminated) string properties. It will overflow. The existing
> >> functions have the same problem, so it isn't something that you've
> >> added. I've got a fix, and as a side effect the fix creates the _array
> >> version basically for free as part of reworking
> >> of_property_count_strings() and of_property_read_string_index()
> >
> > OK
> >
> > So can you please point me to a git branch containing the fix?  I'll rebase the
> > patch on top of that then and everything should merge just fine.
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git devicetree/merge
> 
> I'm going to let the patch sit in there for a few days to get
> linux-next exposure before I ask Linus to pull. The pull req will go
> out before the end of the week.

OK, thanks!

Pulled, rebased my device-properties branch, merged into linux-pm.git/linux-next
and pushed back.

Rafael


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

end of thread, other threads:[~2014-11-04 21:08 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-21 21:08 [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
2014-10-21 21:09 ` [PATCH v6 01/12] ACPI: Add support for device specific properties Rafael J. Wysocki
2014-10-21 21:15 ` [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware Rafael J. Wysocki
2014-11-03 15:40   ` Grant Likely
2014-11-03 22:04     ` Rafael J. Wysocki
2014-11-04 17:01       ` Grant Likely
2014-11-04 21:29         ` Rafael J. Wysocki
2014-11-04 15:51   ` Grant Likely
2014-11-04 16:20     ` Rafael J. Wysocki
2014-11-04 16:38   ` [Update][PATCH " Rafael J. Wysocki
2014-10-21 21:19 ` [PATCH v6 03/12] ACPI: Allow drivers to match using Device Tree compatible property Rafael J. Wysocki
2014-10-21 21:19 ` [PATCH v6 04/12] misc: at25: Make use of device property API Rafael J. Wysocki
2014-11-04 14:18   ` Grant Likely
2014-11-04 14:38     ` Mika Westerberg
2014-11-04 15:04       ` Grant Likely
2014-11-04 16:19         ` Rafael J. Wysocki
2014-11-04 16:07           ` Mika Westerberg
2014-10-21 21:20 ` [PATCH v6 05/12] gpio / ACPI: Add support for _DSD device properties Rafael J. Wysocki
2014-10-21 21:21 ` [PATCH v6 06/12] gpio: sch: Consolidate core and resume banks Rafael J. Wysocki
2014-10-21 21:21 ` [PATCH v6 07/12] leds: leds-gpio: Add support for GPIO descriptors Rafael J. Wysocki
2014-10-21 21:22 ` [PATCH v6 08/12] input: gpio_keys_polled: " Rafael J. Wysocki
2014-10-21 21:29 ` [PATCH v6 09/12] Driver core: Unified interface for firmware node properties Rafael J. Wysocki
2014-11-04 16:43   ` [Update][PATCH " Rafael J. Wysocki
2014-10-21 21:33 ` [PATCH v6 10/12] gpio: Support for unified device properties interface Rafael J. Wysocki
2014-10-21 21:35 ` [PATCH v6 11/12] leds: leds-gpio: Make use of device property API Rafael J. Wysocki
2014-10-21 21:37 ` [PATCH v6 12/12] input: gpio_keys_polled: " Rafael J. Wysocki
2014-10-24 22:10 ` [PATCH v6 00/12] Add ACPI _DSD and unified device properties support Rafael J. Wysocki
2014-11-04 15:49   ` Grant Likely
2014-11-04 16:20     ` Rafael J. Wysocki

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