[PATCH] driver core: property: support for generic property

[PATCH] driver core: property: support for generic property

[Heikki Krogerus]

This extends the Unified Property Interface by adding
"Generic Property" to it for cases where device tree or ACPI
are not being used.

That makes the Unified Property Interface cover most of the
cases where information is extracted from custom
platform_data in the drivers, i.e. removing also the need
for them to handle custom platform data separately.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/property.c  | 188 +++++++++++++++++++++++++++++++++++++++++------
 include/linux/device.h   |   2 +
 include/linux/property.h |  13 ++++
 3 files changed, 182 insertions(+), 21 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index c458458..99990e2 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -15,6 +15,129 @@
 #include <linux/acpi.h>
 #include <linux/of.h>
 
+static struct dev_gen_prop *dev_prop_get(struct device *dev, const char *name)
+{
+	struct dev_gen_prop *prop;
+
+	if (!dev->gen_prop)
+		return NULL;
+
+	for (prop = dev->gen_prop; prop->name; prop++)
+		if (!strcmp(name, prop->name))
+			return prop;
+	return NULL;
+}
+
+static int dev_prop_copy_array_u8(const char **src, u8 *val, size_t nval)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < nval; i++) {
+		if (!src[i])
+			return -EOVERFLOW;
+		ret = kstrtou8(src[i], 0, &val[i]);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
+static int dev_prop_copy_array_u16(const char **src, u16 *val, size_t nval)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < nval; i++) {
+		if (!src[i])
+			return -EOVERFLOW;
+		ret = kstrtou16(src[i], 0, &val[i]);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
+static int dev_prop_copy_array_u32(const char **src, u32 *val, size_t nval)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < nval; i++) {
+		if (!src[i])
+			return -EOVERFLOW;
+		ret = kstrtou32(src[i], 0, &val[i]);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
+static int dev_prop_copy_array_u64(const char **src, u64 *val, size_t nval)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < nval; i++) {
+		if (!src[i])
+			return -EOVERFLOW;
+		ret = kstrtou64(src[i], 0, &val[i]);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
+static int dev_prop_copy_array_string(const char **src, const char **val,
+				      size_t nval)
+{
+	int i;
+
+	for (i = 0; i < nval; i++) {
+		if (!src[i])
+			return -EOVERFLOW;
+		val[i] = src[i];
+	}
+	return 0;
+}
+
+static int dev_prop_read_array(struct device *dev, const char *name,
+			       enum dev_prop_type type, void *val, size_t nval)
+{
+	struct dev_gen_prop *prop;
+	int ret = 0;
+
+	prop = dev_prop_get(dev, name);
+	if (!prop)
+		return -ENODATA;
+
+	if (prop->type != type)
+		return -EPROTO;
+
+	switch (prop->type) {
+	case DEV_PROP_U8:
+		ret = dev_prop_copy_array_u8(prop->val, (u8 *)val, nval);
+		break;
+	case DEV_PROP_U16:
+		ret = dev_prop_copy_array_u16(prop->val, (u16 *)val, nval);
+		break;
+	case DEV_PROP_U32:
+		ret = dev_prop_copy_array_u32(prop->val, (u32 *)val, nval);
+		break;
+	case DEV_PROP_U64:
+		ret = dev_prop_copy_array_u64(prop->val, (u64 *)val, nval);
+		break;
+	case DEV_PROP_STRING:
+		ret = dev_prop_copy_array_string(prop->val, (const char **)val,
+						 nval);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	return ret;
+}
+
 /**
  * device_property_present - check if a property of a device is present
  * @dev: Device whose property is being checked
@@ -26,8 +149,9 @@ 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);
+	if (ACPI_HANDLE(dev))
+		return !acpi_dev_prop_get(ACPI_COMPANION(dev), propname, NULL);
+	return !!dev_prop_get(dev, propname);
 }
 EXPORT_SYMBOL_GPL(device_property_present);
 
@@ -51,13 +175,6 @@ EXPORT_SYMBOL_GPL(fwnode_property_present);
 	(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
@@ -77,7 +194,13 @@ EXPORT_SYMBOL_GPL(fwnode_property_present);
 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);
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+		return OF_DEV_PROP_READ_ARRAY(dev->of_node, propname, u8,
+					      val, nval);
+	if (ACPI_HANDLE(dev))
+		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
+					  DEV_PROP_U8, val, nval);
+	return dev_prop_read_array(dev, propname, DEV_PROP_U8, val, nval);
 }
 EXPORT_SYMBOL_GPL(device_property_read_u8_array);
 
@@ -100,7 +223,13 @@ EXPORT_SYMBOL_GPL(device_property_read_u8_array);
 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);
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+		return OF_DEV_PROP_READ_ARRAY(dev->of_node, propname, u16, val,
+					      nval);
+	if (ACPI_HANDLE(dev))
+		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
+					  DEV_PROP_U16, val, nval);
+	return dev_prop_read_array(dev, propname, DEV_PROP_U16, val, nval);
 }
 EXPORT_SYMBOL_GPL(device_property_read_u16_array);
 
@@ -123,7 +252,13 @@ EXPORT_SYMBOL_GPL(device_property_read_u16_array);
 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);
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+		return OF_DEV_PROP_READ_ARRAY(dev->of_node, propname, u32, val,
+					      nval);
+	if (ACPI_HANDLE(dev))
+		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
+					  DEV_PROP_U32, val, nval);
+	return dev_prop_read_array(dev, propname, DEV_PROP_U32, val, nval);
 }
 EXPORT_SYMBOL_GPL(device_property_read_u32_array);
 
@@ -146,7 +281,13 @@ EXPORT_SYMBOL_GPL(device_property_read_u32_array);
 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);
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+		return OF_DEV_PROP_READ_ARRAY(dev->of_node, propname, u64, val,
+					      nval);
+	if (ACPI_HANDLE(dev))
+		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
+					  DEV_PROP_U64, val, nval);
+	return dev_prop_read_array(dev, propname, DEV_PROP_U64, val, nval);
 }
 EXPORT_SYMBOL_GPL(device_property_read_u64_array);
 
@@ -169,10 +310,13 @@ EXPORT_SYMBOL_GPL(device_property_read_u64_array);
 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);
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+		return of_property_read_string_array(dev->of_node, propname,
+						     val, nval);
+	if (ACPI_HANDLE(dev))
+		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
+					  DEV_PROP_STRING, val, nval);
+	return dev_prop_read_array(dev, propname, DEV_PROP_STRING, val, nval);
 }
 EXPORT_SYMBOL_GPL(device_property_read_string_array);
 
@@ -193,10 +337,12 @@ EXPORT_SYMBOL_GPL(device_property_read_string_array);
 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);
+	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+		return of_property_read_string(dev->of_node, propname, val);
+	if (ACPI_HANDLE(dev))
+		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
+					  DEV_PROP_STRING, val, 1);
+	return dev_prop_read_array(dev, propname, DEV_PROP_STRING, val, 1);
 }
 EXPORT_SYMBOL_GPL(device_property_read_string);
 
diff --git a/include/linux/device.h b/include/linux/device.h
index fb50673..bd1e292 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -27,6 +27,7 @@
 #include <linux/ratelimit.h>
 #include <linux/uidgid.h>
 #include <linux/gfp.h>
+#include <linux/property.h>
 #include <asm/device.h>
 
 struct device;
@@ -780,6 +781,7 @@ struct device {
 
 	struct device_node	*of_node; /* associated device tree node */
 	struct acpi_dev_node	acpi_node; /* associated ACPI device node */
+	struct dev_gen_prop	*gen_prop;
 
 	dev_t			devt;	/* dev_t, creates the sysfs "dev" */
 	u32			id;	/* device instance */
diff --git a/include/linux/property.h b/include/linux/property.h
index a6a3d98..6ac8e508 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -26,6 +26,19 @@ enum dev_prop_type {
 	DEV_PROP_MAX,
 };
 
+/**
+ * struct dev_gen_prop - Generic Device Property
+ * @name: property name
+ * @val: value array
+ *
+ * Used when of_node and acpi_node are missing.
+ */
+struct dev_gen_prop {
+	enum dev_prop_type type;
+	const char *name;
+	const char **val;
+};
+
 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);
-- 
2.1.4

Re: [PATCH] driver core: property: support for generic property

[Rafael J. Wysocki]

On Monday, January 12, 2015 01:15:44 PM Heikki Krogerus wrote:
> This extends the Unified Property Interface by adding
> "Generic Property" to it for cases where device tree or ACPI
> are not being used.
> 
> That makes the Unified Property Interface cover most of the
> cases where information is extracted from custom
> platform_data in the drivers, i.e. removing also the need
> for them to handle custom platform data separately.

I'm not sure I understand that correctly.  Do you mean that it is possible to
reduce code duplication between drivers this way?

> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/base/property.c  | 188 +++++++++++++++++++++++++++++++++++++++++------
>  include/linux/device.h   |   2 +
>  include/linux/property.h |  13 ++++
>  3 files changed, 182 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index c458458..99990e2 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -15,6 +15,129 @@
>  #include <linux/acpi.h>
>  #include <linux/of.h>
>  
> +static struct dev_gen_prop *dev_prop_get(struct device *dev, const char *name)
> +{
> +	struct dev_gen_prop *prop;
> +
> +	if (!dev->gen_prop)
> +		return NULL;
> +
> +	for (prop = dev->gen_prop; prop->name; prop++)
> +		if (!strcmp(name, prop->name))
> +			return prop;
> +	return NULL;
> +}
> +
> +static int dev_prop_copy_array_u8(const char **src, u8 *val, size_t nval)
> +{
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < nval; i++) {
> +		if (!src[i])
> +			return -EOVERFLOW;
> +		ret = kstrtou8(src[i], 0, &val[i]);
> +		if (ret)
> +			return ret;
> +	}
> +	return 0;
> +}
> +
> +static int dev_prop_copy_array_u16(const char **src, u16 *val, size_t nval)
> +{
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < nval; i++) {
> +		if (!src[i])
> +			return -EOVERFLOW;
> +		ret = kstrtou16(src[i], 0, &val[i]);
> +		if (ret)
> +			return ret;
> +	}
> +	return 0;
> +}
> +
> +static int dev_prop_copy_array_u32(const char **src, u32 *val, size_t nval)
> +{
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < nval; i++) {
> +		if (!src[i])
> +			return -EOVERFLOW;
> +		ret = kstrtou32(src[i], 0, &val[i]);
> +		if (ret)
> +			return ret;
> +	}
> +	return 0;
> +}
> +
> +static int dev_prop_copy_array_u64(const char **src, u64 *val, size_t nval)
> +{
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < nval; i++) {
> +		if (!src[i])
> +			return -EOVERFLOW;
> +		ret = kstrtou64(src[i], 0, &val[i]);
> +		if (ret)
> +			return ret;
> +	}
> +	return 0;
> +}
> +
> +static int dev_prop_copy_array_string(const char **src, const char **val,
> +				      size_t nval)
> +{
> +	int i;
> +
> +	for (i = 0; i < nval; i++) {
> +		if (!src[i])
> +			return -EOVERFLOW;
> +		val[i] = src[i];
> +	}
> +	return 0;
> +}
> +
> +static int dev_prop_read_array(struct device *dev, const char *name,
> +			       enum dev_prop_type type, void *val, size_t nval)
> +{
> +	struct dev_gen_prop *prop;
> +	int ret = 0;
> +
> +	prop = dev_prop_get(dev, name);
> +	if (!prop)
> +		return -ENODATA;
> +
> +	if (prop->type != type)
> +		return -EPROTO;
> +
> +	switch (prop->type) {
> +	case DEV_PROP_U8:
> +		ret = dev_prop_copy_array_u8(prop->val, (u8 *)val, nval);
> +		break;
> +	case DEV_PROP_U16:
> +		ret = dev_prop_copy_array_u16(prop->val, (u16 *)val, nval);
> +		break;
> +	case DEV_PROP_U32:
> +		ret = dev_prop_copy_array_u32(prop->val, (u32 *)val, nval);
> +		break;
> +	case DEV_PROP_U64:
> +		ret = dev_prop_copy_array_u64(prop->val, (u64 *)val, nval);
> +		break;
> +	case DEV_PROP_STRING:
> +		ret = dev_prop_copy_array_string(prop->val, (const char **)val,
> +						 nval);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +	return ret;
> +}
> +
>  /**
>   * device_property_present - check if a property of a device is present
>   * @dev: Device whose property is being checked
> @@ -26,8 +149,9 @@ 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);
> +	if (ACPI_HANDLE(dev))
> +		return !acpi_dev_prop_get(ACPI_COMPANION(dev), propname, NULL);
> +	return !!dev_prop_get(dev, propname);
>  }
>  EXPORT_SYMBOL_GPL(device_property_present);
>  
> @@ -51,13 +175,6 @@ EXPORT_SYMBOL_GPL(fwnode_property_present);
>  	(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_)
> -

The macro was used here for a reason.  Can't you replace it with an extended
macro instead?

>  /**
>   * device_property_read_u8_array - return a u8 array property of a device
>   * @dev: Device to get the property of
> @@ -77,7 +194,13 @@ EXPORT_SYMBOL_GPL(fwnode_property_present);
>  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);
> +	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> +		return OF_DEV_PROP_READ_ARRAY(dev->of_node, propname, u8,
> +					      val, nval);
> +	if (ACPI_HANDLE(dev))
> +		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
> +					  DEV_PROP_U8, val, nval);
> +	return dev_prop_read_array(dev, propname, DEV_PROP_U8, val, nval);
>  }
>  EXPORT_SYMBOL_GPL(device_property_read_u8_array);
>  
> @@ -100,7 +223,13 @@ EXPORT_SYMBOL_GPL(device_property_read_u8_array);
>  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);
> +	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> +		return OF_DEV_PROP_READ_ARRAY(dev->of_node, propname, u16, val,
> +					      nval);
> +	if (ACPI_HANDLE(dev))
> +		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
> +					  DEV_PROP_U16, val, nval);
> +	return dev_prop_read_array(dev, propname, DEV_PROP_U16, val, nval);
>  }
>  EXPORT_SYMBOL_GPL(device_property_read_u16_array);
>  
> @@ -123,7 +252,13 @@ EXPORT_SYMBOL_GPL(device_property_read_u16_array);
>  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);
> +	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> +		return OF_DEV_PROP_READ_ARRAY(dev->of_node, propname, u32, val,
> +					      nval);
> +	if (ACPI_HANDLE(dev))
> +		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
> +					  DEV_PROP_U32, val, nval);
> +	return dev_prop_read_array(dev, propname, DEV_PROP_U32, val, nval);
>  }
>  EXPORT_SYMBOL_GPL(device_property_read_u32_array);
>  
> @@ -146,7 +281,13 @@ EXPORT_SYMBOL_GPL(device_property_read_u32_array);
>  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);
> +	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> +		return OF_DEV_PROP_READ_ARRAY(dev->of_node, propname, u64, val,
> +					      nval);
> +	if (ACPI_HANDLE(dev))
> +		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
> +					  DEV_PROP_U64, val, nval);
> +	return dev_prop_read_array(dev, propname, DEV_PROP_U64, val, nval);
>  }
>  EXPORT_SYMBOL_GPL(device_property_read_u64_array);
>  
> @@ -169,10 +310,13 @@ EXPORT_SYMBOL_GPL(device_property_read_u64_array);
>  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);
> +	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> +		return of_property_read_string_array(dev->of_node, propname,
> +						     val, nval);
> +	if (ACPI_HANDLE(dev))
> +		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
> +					  DEV_PROP_STRING, val, nval);
> +	return dev_prop_read_array(dev, propname, DEV_PROP_STRING, val, nval);
>  }
>  EXPORT_SYMBOL_GPL(device_property_read_string_array);
>  
> @@ -193,10 +337,12 @@ EXPORT_SYMBOL_GPL(device_property_read_string_array);
>  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);
> +	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> +		return of_property_read_string(dev->of_node, propname, val);
> +	if (ACPI_HANDLE(dev))
> +		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
> +					  DEV_PROP_STRING, val, 1);
> +	return dev_prop_read_array(dev, propname, DEV_PROP_STRING, val, 1);
>  }
>  EXPORT_SYMBOL_GPL(device_property_read_string);
>  
> diff --git a/include/linux/device.h b/include/linux/device.h
> index fb50673..bd1e292 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -27,6 +27,7 @@
>  #include <linux/ratelimit.h>
>  #include <linux/uidgid.h>
>  #include <linux/gfp.h>
> +#include <linux/property.h>
>  #include <asm/device.h>
>  
>  struct device;
> @@ -780,6 +781,7 @@ struct device {
>  
>  	struct device_node	*of_node; /* associated device tree node */
>  	struct acpi_dev_node	acpi_node; /* associated ACPI device node */
> +	struct dev_gen_prop	*gen_prop;
>  
>  	dev_t			devt;	/* dev_t, creates the sysfs "dev" */
>  	u32			id;	/* device instance */
> diff --git a/include/linux/property.h b/include/linux/property.h
> index a6a3d98..6ac8e508 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -26,6 +26,19 @@ enum dev_prop_type {
>  	DEV_PROP_MAX,
>  };
>  
> +/**
> + * struct dev_gen_prop - Generic Device Property
> + * @name: property name
> + * @val: value array
> + *
> + * Used when of_node and acpi_node are missing.
> + */
> +struct dev_gen_prop {
> +	enum dev_prop_type type;
> +	const char *name;
> +	const char **val;
> +};
> +
>  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);
> 

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

Re: [PATCH] driver core: property: support for generic property

[Heikki Krogerus]

On Tue, Jan 13, 2015 at 10:35:09AM +0100, Rafael J. Wysocki wrote:
> On Monday, January 12, 2015 01:15:44 PM Heikki Krogerus wrote:
> > This extends the Unified Property Interface by adding
> > "Generic Property" to it for cases where device tree or ACPI
> > are not being used.
> > 
> > That makes the Unified Property Interface cover most of the
> > cases where information is extracted from custom
> > platform_data in the drivers, i.e. removing also the need
> > for them to handle custom platform data separately.
> 
> I'm not sure I understand that correctly.  Do you mean that it is possible to
> reduce code duplication between drivers this way?

I wan't to replace platform_data with unified device properties. So I
wan't to replace this kind of code in the drivers:

        if (pdata)
                val = pdata->val;
        else
                device_property_read_u32(dev, "magic", &val);

With just:

        device_property_read_u32(dev, "magic", &val);

> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  drivers/base/property.c  | 188 +++++++++++++++++++++++++++++++++++++++++------
> >  include/linux/device.h   |   2 +
> >  include/linux/property.h |  13 ++++
> >  3 files changed, 182 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > index c458458..99990e2 100644
> > --- a/drivers/base/property.c
> > +++ b/drivers/base/property.c
> > @@ -15,6 +15,129 @@
> >  #include <linux/acpi.h>
> >  #include <linux/of.h>
> >  
> > +static struct dev_gen_prop *dev_prop_get(struct device *dev, const char *name)
> > +{
> > +	struct dev_gen_prop *prop;
> > +
> > +	if (!dev->gen_prop)
> > +		return NULL;
> > +
> > +	for (prop = dev->gen_prop; prop->name; prop++)
> > +		if (!strcmp(name, prop->name))
> > +			return prop;
> > +	return NULL;
> > +}
> > +
> > +static int dev_prop_copy_array_u8(const char **src, u8 *val, size_t nval)
> > +{
> > +	int ret;
> > +	int i;
> > +
> > +	for (i = 0; i < nval; i++) {
> > +		if (!src[i])
> > +			return -EOVERFLOW;
> > +		ret = kstrtou8(src[i], 0, &val[i]);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int dev_prop_copy_array_u16(const char **src, u16 *val, size_t nval)
> > +{
> > +	int ret;
> > +	int i;
> > +
> > +	for (i = 0; i < nval; i++) {
> > +		if (!src[i])
> > +			return -EOVERFLOW;
> > +		ret = kstrtou16(src[i], 0, &val[i]);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int dev_prop_copy_array_u32(const char **src, u32 *val, size_t nval)
> > +{
> > +	int ret;
> > +	int i;
> > +
> > +	for (i = 0; i < nval; i++) {
> > +		if (!src[i])
> > +			return -EOVERFLOW;
> > +		ret = kstrtou32(src[i], 0, &val[i]);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int dev_prop_copy_array_u64(const char **src, u64 *val, size_t nval)
> > +{
> > +	int ret;
> > +	int i;
> > +
> > +	for (i = 0; i < nval; i++) {
> > +		if (!src[i])
> > +			return -EOVERFLOW;
> > +		ret = kstrtou64(src[i], 0, &val[i]);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int dev_prop_copy_array_string(const char **src, const char **val,
> > +				      size_t nval)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < nval; i++) {
> > +		if (!src[i])
> > +			return -EOVERFLOW;
> > +		val[i] = src[i];
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int dev_prop_read_array(struct device *dev, const char *name,
> > +			       enum dev_prop_type type, void *val, size_t nval)
> > +{
> > +	struct dev_gen_prop *prop;
> > +	int ret = 0;
> > +
> > +	prop = dev_prop_get(dev, name);
> > +	if (!prop)
> > +		return -ENODATA;
> > +
> > +	if (prop->type != type)
> > +		return -EPROTO;
> > +
> > +	switch (prop->type) {
> > +	case DEV_PROP_U8:
> > +		ret = dev_prop_copy_array_u8(prop->val, (u8 *)val, nval);
> > +		break;
> > +	case DEV_PROP_U16:
> > +		ret = dev_prop_copy_array_u16(prop->val, (u16 *)val, nval);
> > +		break;
> > +	case DEV_PROP_U32:
> > +		ret = dev_prop_copy_array_u32(prop->val, (u32 *)val, nval);
> > +		break;
> > +	case DEV_PROP_U64:
> > +		ret = dev_prop_copy_array_u64(prop->val, (u64 *)val, nval);
> > +		break;
> > +	case DEV_PROP_STRING:
> > +		ret = dev_prop_copy_array_string(prop->val, (const char **)val,
> > +						 nval);
> > +		break;
> > +	default:
> > +		ret = -EINVAL;
> > +		break;
> > +	}
> > +	return ret;
> > +}
> > +
> >  /**
> >   * device_property_present - check if a property of a device is present
> >   * @dev: Device whose property is being checked
> > @@ -26,8 +149,9 @@ 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);
> > +	if (ACPI_HANDLE(dev))
> > +		return !acpi_dev_prop_get(ACPI_COMPANION(dev), propname, NULL);
> > +	return !!dev_prop_get(dev, propname);
> >  }
> >  EXPORT_SYMBOL_GPL(device_property_present);
> >  
> > @@ -51,13 +175,6 @@ EXPORT_SYMBOL_GPL(fwnode_property_present);
> >  	(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_)
> > -
> 
> The macro was used here for a reason.  Can't you replace it with an extended
> macro instead?
> 
> >  /**
> >   * device_property_read_u8_array - return a u8 array property of a device
> >   * @dev: Device to get the property of
> > @@ -77,7 +194,13 @@ EXPORT_SYMBOL_GPL(fwnode_property_present);
> >  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);
> > +	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> > +		return OF_DEV_PROP_READ_ARRAY(dev->of_node, propname, u8,
> > +					      val, nval);
> > +	if (ACPI_HANDLE(dev))
> > +		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
> > +					  DEV_PROP_U8, val, nval);
> > +	return dev_prop_read_array(dev, propname, DEV_PROP_U8, val, nval);
> >  }
> >  EXPORT_SYMBOL_GPL(device_property_read_u8_array);
> >  
> > @@ -100,7 +223,13 @@ EXPORT_SYMBOL_GPL(device_property_read_u8_array);
> >  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);
> > +	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> > +		return OF_DEV_PROP_READ_ARRAY(dev->of_node, propname, u16, val,
> > +					      nval);
> > +	if (ACPI_HANDLE(dev))
> > +		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
> > +					  DEV_PROP_U16, val, nval);
> > +	return dev_prop_read_array(dev, propname, DEV_PROP_U16, val, nval);
> >  }
> >  EXPORT_SYMBOL_GPL(device_property_read_u16_array);
> >  
> > @@ -123,7 +252,13 @@ EXPORT_SYMBOL_GPL(device_property_read_u16_array);
> >  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);
> > +	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> > +		return OF_DEV_PROP_READ_ARRAY(dev->of_node, propname, u32, val,
> > +					      nval);
> > +	if (ACPI_HANDLE(dev))
> > +		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
> > +					  DEV_PROP_U32, val, nval);
> > +	return dev_prop_read_array(dev, propname, DEV_PROP_U32, val, nval);
> >  }
> >  EXPORT_SYMBOL_GPL(device_property_read_u32_array);
> >  
> > @@ -146,7 +281,13 @@ EXPORT_SYMBOL_GPL(device_property_read_u32_array);
> >  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);
> > +	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> > +		return OF_DEV_PROP_READ_ARRAY(dev->of_node, propname, u64, val,
> > +					      nval);
> > +	if (ACPI_HANDLE(dev))
> > +		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
> > +					  DEV_PROP_U64, val, nval);
> > +	return dev_prop_read_array(dev, propname, DEV_PROP_U64, val, nval);
> >  }
> >  EXPORT_SYMBOL_GPL(device_property_read_u64_array);
> >  
> > @@ -169,10 +310,13 @@ EXPORT_SYMBOL_GPL(device_property_read_u64_array);
> >  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);
> > +	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> > +		return of_property_read_string_array(dev->of_node, propname,
> > +						     val, nval);
> > +	if (ACPI_HANDLE(dev))
> > +		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
> > +					  DEV_PROP_STRING, val, nval);
> > +	return dev_prop_read_array(dev, propname, DEV_PROP_STRING, val, nval);
> >  }
> >  EXPORT_SYMBOL_GPL(device_property_read_string_array);
> >  
> > @@ -193,10 +337,12 @@ EXPORT_SYMBOL_GPL(device_property_read_string_array);
> >  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);
> > +	if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> > +		return of_property_read_string(dev->of_node, propname, val);
> > +	if (ACPI_HANDLE(dev))
> > +		return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
> > +					  DEV_PROP_STRING, val, 1);
> > +	return dev_prop_read_array(dev, propname, DEV_PROP_STRING, val, 1);
> >  }
> >  EXPORT_SYMBOL_GPL(device_property_read_string);
> >  
> > diff --git a/include/linux/device.h b/include/linux/device.h
> > index fb50673..bd1e292 100644
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -27,6 +27,7 @@
> >  #include <linux/ratelimit.h>
> >  #include <linux/uidgid.h>
> >  #include <linux/gfp.h>
> > +#include <linux/property.h>
> >  #include <asm/device.h>
> >  
> >  struct device;
> > @@ -780,6 +781,7 @@ struct device {
> >  
> >  	struct device_node	*of_node; /* associated device tree node */
> >  	struct acpi_dev_node	acpi_node; /* associated ACPI device node */
> > +	struct dev_gen_prop	*gen_prop;
> >  
> >  	dev_t			devt;	/* dev_t, creates the sysfs "dev" */
> >  	u32			id;	/* device instance */
> > diff --git a/include/linux/property.h b/include/linux/property.h
> > index a6a3d98..6ac8e508 100644
> > --- a/include/linux/property.h
> > +++ b/include/linux/property.h
> > @@ -26,6 +26,19 @@ enum dev_prop_type {
> >  	DEV_PROP_MAX,
> >  };
> >  
> > +/**
> > + * struct dev_gen_prop - Generic Device Property
> > + * @name: property name
> > + * @val: value array
> > + *
> > + * Used when of_node and acpi_node are missing.
> > + */
> > +struct dev_gen_prop {
> > +	enum dev_prop_type type;
> > +	const char *name;
> > +	const char **val;
> > +};
> > +
> >  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);
> > 
> 
> -- 
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

-- 
heikki

Re: [PATCH] driver core: property: support for generic property

[Heikki Krogerus]

On Tue, Jan 13, 2015 at 10:35:09AM +0100, Rafael J. Wysocki wrote:
> > @@ -51,13 +175,6 @@ EXPORT_SYMBOL_GPL(fwnode_property_present);
> >  	(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_)
> > -
> 
> The macro was used here for a reason.  Can't you replace it with an extended
> macro instead?

OK, I'll change it.

Thanks,

-- 
heikki