[PATCH v1 1/3] device property: check fwnode type in to_of_node()

[PATCH v1 1/3] device property: check fwnode type in to_of_node()

[Andy Shevchenko]

Potentially one of platform can support both ACPI and OF. In that case when we
call to_of_node() for non-OF fwnode types we will get non-NULL result, which is
wrong. Check for the type and return a correspondent result.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/of.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/of.h b/include/linux/of.h
index edc068d..2194b8c 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -136,7 +136,8 @@ static inline bool is_of_node(struct fwnode_handle *fwnode)
 
 static inline struct device_node *to_of_node(struct fwnode_handle *fwnode)
 {
-	return fwnode ? container_of(fwnode, struct device_node, fwnode) : NULL;
+	return is_of_node(fwnode) ?
+		container_of(fwnode, struct device_node, fwnode) : NULL;
 }
 
 static inline bool of_have_populated_dt(void)
-- 
2.5.0

[PATCH v1 2/3] device property: fallback to pset when gettng one string

[Andy Shevchenko]

The one string as an equivalent to an array of one element. Allow user to read
one string as a plain string.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/property.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 37a7bb7..841b15c 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -462,7 +462,8 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
 		return acpi_dev_prop_read(to_acpi_node(fwnode), propname,
 					  DEV_PROP_STRING, val, 1);
 
-	return -ENXIO;
+	return pset_prop_read_array(to_pset(fwnode), propname,
+				    DEV_PROP_STRING, val, 1);
 }
 EXPORT_SYMBOL_GPL(fwnode_property_read_string);
 
-- 
2.5.0

[PATCH v1 3/3] device property: attach 'else if' to the proper 'if'

[Andy Shevchenko]

Obviously in the current place the 'else' keyword is redundant, though it seems
quite correct when we check if nval is in allowed range.

Reattach the condition branch there.

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

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 7836e2e..a28752c 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -528,13 +528,14 @@ int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
 
 	if (!val)
 		return obj->package.count;
-	else if (nval <= 0)
-		return -EINVAL;
 
 	if (nval > obj->package.count)
 		return -EOVERFLOW;
+	else if (nval <= 0)
+		return -EINVAL;
 
 	items = obj->package.elements;
+
 	switch (proptype) {
 	case DEV_PROP_U8:
 		ret = acpi_copy_property_array_u8(items, (u8 *)val, nval);
@@ -552,8 +553,7 @@ int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
 		ret = acpi_copy_property_array_string(items, (char **)val, nval);
 		break;
 	default:
-		ret = -EINVAL;
-		break;
+		return -EINVAL;
 	}
 	return ret;
 }
-- 
2.5.0

Re: [PATCH v1 3/3] device property: attach 'else if' to the proper 'if'

[Andy Shevchenko]

On Mon, 2015-08-10 at 19:56 +0300, Andy Shevchenko wrote:
> Obviously in the current place the 'else' keyword is redundant, 
> though it seems
> quite correct when we check if nval is in allowed range.
> 
> Reattach the condition branch there.

Rafael, it would be nice to have these fixes in 4.3 as well if you have
no objections.

One comment below.

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/acpi/property.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 7836e2e..a28752c 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -528,13 +528,14 @@ int acpi_dev_prop_read(struct acpi_device 
> *adev, const char *propname,
>  
>  	if (!val)
>  		return obj->package.count;
> -	else if (nval <= 0)
> -		return -EINVAL;
>  
>  	if (nval > obj->package.count)
>  		return -EOVERFLOW;
> +	else if (nval <= 0)
> +		return -EINVAL;
>  
>  	items = obj->package.elements;
> +

This…

>  	switch (proptype) {
>  	case DEV_PROP_U8:
>  		ret = acpi_copy_property_array_u8(items, (u8 *)val, 
> nval);
> @@ -552,8 +553,7 @@ int acpi_dev_prop_read(struct acpi_device *adev, 
> const char *propname,
>  		ret = acpi_copy_property_array_string(items, (char 
> **)val, nval);
>  		break;
>  	default:
> -		ret = -EINVAL;
> -		break;
> +		return -EINVAL;

…and this seem left overs. If you wish I can resend this patch without
them.

>  	}
>  	return ret;
>  }

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

Re: [PATCH v1 1/3] device property: check fwnode type in to_of_node()

[Mika Westerberg]

On Mon, Aug 10, 2015 at 07:56:46PM +0300, Andy Shevchenko wrote:
> Potentially one of platform can support both ACPI and OF. In that case when we
> call to_of_node() for non-OF fwnode types we will get non-NULL result, which is
> wrong. Check for the type and return a correspondent result.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH v1 2/3] device property: fallback to pset when gettng one string

[Mika Westerberg]

On Mon, Aug 10, 2015 at 07:56:47PM +0300, Andy Shevchenko wrote:
> The one string as an equivalent to an array of one element. Allow user to read
> one string as a plain string.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH v1 3/3] device property: attach 'else if' to the proper 'if'

[Mika Westerberg]

On Mon, Aug 10, 2015 at 07:56:48PM +0300, Andy Shevchenko wrote:
> Obviously in the current place the 'else' keyword is redundant, though it seems
> quite correct when we check if nval is in allowed range.
> 
> Reattach the condition branch there.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

When the leftovers you mentioned are dropped you can add my,

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH v1 1/3] device property: check fwnode type in to_of_node()

[Rafael J. Wysocki]

On Monday, August 10, 2015 07:56:46 PM Andy Shevchenko wrote:
> Potentially one of platform can support both ACPI and OF. In that case when we
> call to_of_node() for non-OF fwnode types we will get non-NULL result, which is
> wrong. Check for the type and return a correspondent result.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Added [1/3] to the 4.3 queue with a minor change in [3/3] (dropped the last
hunk) and the Reviewed-by tags from Mika.

Thanks!


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