[PATCH 0/3] Fwnode property API fixes for OF, pset

[PATCH 0/3] Fwnode property API fixes for OF, pset

[Sakari Ailus]

Hi folks,

Here are three patches that fix a few issues in the fwnode property API
implementation. They were previously a part of a larger patchset "Move
firmware specific code to firmware specific locations" here:

<URL:http://www.spinics.net/lists/linux-acpi/msg72174.html>

There are no dependencies between these three patches and other patches
I've sent recently.

-- 
Kind regards,
Sakari

[PATCH 1/3] device property: fwnode_property_read_string_array() may return -EILSEQ

[Sakari Ailus]

fwnode_property_read_string_array() may return -EILSEQ through
of_property_read_string_array(). Document this.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/base/property.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index c458c63..19a3dc5 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -589,7 +589,7 @@ static int __fwnode_property_read_string(struct fwnode_handle *fwnode,
  *         %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,
+ *	   %-EPROTO or %-EILSEQ 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.
  */
-- 
2.7.4

Re: [PATCH 1/3] device property: fwnode_property_read_string_array() may return -EILSEQ

[Mika Westerberg]

On Mon, Mar 06, 2017 at 03:26:29PM +0200, Sakari Ailus wrote:
> fwnode_property_read_string_array() may return -EILSEQ through
> of_property_read_string_array(). Document this.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Reviewed-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Though, I wonder if it would be good to change the function to only
return what the DT version does. Now the caller needs to check for both
values.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] device property: fwnode_property_read_string_array() may return -EILSEQ

[Sakari Ailus]

Hi Mika,

Mika Westerberg wrote:
> On Mon, Mar 06, 2017 at 03:26:29PM +0200, Sakari Ailus wrote:
>> fwnode_property_read_string_array() may return -EILSEQ through
>> of_property_read_string_array(). Document this.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thanks!

>
> Though, I wonder if it would be good to change the function to only
> return what the DT version does. Now the caller needs to check for both
> values.
>

It's rather that this value is currently returned by 
of_property_read_string_array(). I'd guess though that most callers 
don't care about the error code.

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

[PATCH 2/3] device property: Fix reading pset strings using array access functions

[Sakari Ailus]

The length field value of non-array string properties is the length of the
string itself. Non-array string properties thus require specific handling.
Fix this.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/base/property.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 19a3dc5..9224541a 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -552,13 +552,27 @@ static int __fwnode_property_read_string_array(struct fwnode_handle *fwnode,
 	else if (is_acpi_node(fwnode))
 		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
 					   val, nval);
-	else if (is_pset_node(fwnode))
-		return val ?
-			pset_prop_read_string_array(to_pset_node(fwnode),
-						    propname, val, nval) :
-			pset_prop_count_elems_of_size(to_pset_node(fwnode),
-						      propname,
-						      sizeof(const char *));
+	else if (is_pset_node(fwnode)) {
+		struct property_set *node = to_pset_node(fwnode);
+		struct property_entry *prop;
+
+		/* Read properties if val is non-NULL */
+		if (val)
+			return pset_prop_read_string_array(node, propname,
+							   val, nval);
+
+		prop = pset_prop_get(node, propname);
+		if (!prop)
+			return -EINVAL;
+
+		/* The array length for a non-array string property is 1. */
+		if (!prop->is_array)
+			return 1;
+
+		/* Return the length of an array. */
+		return pset_prop_count_elems_of_size(node, propname,
+						     sizeof(const char *));
+	}
 	return -ENXIO;
 }
 
-- 
2.7.4

Re: [PATCH 2/3] device property: Fix reading pset strings using array access functions

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]

Hi Sakari,

[auto build test WARNING on driver-core/driver-core-testing]
[also build test WARNING on v4.11-rc1]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sakari-Ailus/Fwnode-property-API-fixes-for-OF-pset/20170308-072028
config: i386-tinyconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/base/property.c: In function '__fwnode_property_read_string_array':
>> drivers/base/property.c:564:8: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      prop = pset_prop_get(node, propname);
           ^

vim +/const +564 drivers/base/property.c

   548			return val ?
   549				of_property_read_string_array(to_of_node(fwnode),
   550							      propname, val, nval) :
   551				of_property_count_strings(to_of_node(fwnode), propname);
   552		else if (is_acpi_node(fwnode))
   553			return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
   554						   val, nval);
   555		else if (is_pset_node(fwnode)) {
   556			struct property_set *node = to_pset_node(fwnode);
   557			struct property_entry *prop;
   558	
   559			/* Read properties if val is non-NULL */
   560			if (val)
   561				return pset_prop_read_string_array(node, propname,
   562								   val, nval);
   563	
 > 564			prop = pset_prop_get(node, propname);
   565			if (!prop)
   566				return -EINVAL;
   567	
   568			/* The array length for a non-array string property is 1. */
   569			if (!prop->is_array)
   570				return 1;
   571	
   572			/* Return the length of an array. */

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6490 bytes --]

[PATCH v1.1 2/3] device property: Fix reading pset strings using array access functions

[Sakari Ailus]

The length field value of non-array string properties is the length of the
string itself. Non-array string properties thus require specific handling.
Fix this.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---

since v1:

- Make prop const. pset_prop_get() returns const struct property_entry *
  now.

 drivers/base/property.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 19a3dc5..8c98390 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -552,13 +552,27 @@ static int __fwnode_property_read_string_array(struct fwnode_handle *fwnode,
 	else if (is_acpi_node(fwnode))
 		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
 					   val, nval);
-	else if (is_pset_node(fwnode))
-		return val ?
-			pset_prop_read_string_array(to_pset_node(fwnode),
-						    propname, val, nval) :
-			pset_prop_count_elems_of_size(to_pset_node(fwnode),
-						      propname,
-						      sizeof(const char *));
+	else if (is_pset_node(fwnode)) {
+		struct property_set *node = to_pset_node(fwnode);
+		const struct property_entry *prop;
+
+		/* Read properties if val is non-NULL */
+		if (val)
+			return pset_prop_read_string_array(node, propname,
+							   val, nval);
+
+		prop = pset_prop_get(node, propname);
+		if (!prop)
+			return -EINVAL;
+
+		/* The array length for a non-array string property is 1. */
+		if (!prop->is_array)
+			return 1;
+
+		/* Return the length of an array. */
+		return pset_prop_count_elems_of_size(node, propname,
+						     sizeof(const char *));
+	}
 	return -ENXIO;
 }
 
-- 
2.7.4

[PATCH 3/3] device property: of_property_read_string_array() returns number of strings

[Sakari Ailus]

of_property_read_string_array() returns number of strings read if the
target array of pointers is non-NULL. fwnode_property_read_string_array()
is documented to return 0 in that case. Fix this.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/base/property.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 9224541a..e67ec24 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -544,12 +544,24 @@ static 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 val ?
-			of_property_read_string_array(to_of_node(fwnode),
-						      propname, val, nval) :
-			of_property_count_strings(to_of_node(fwnode), propname);
-	else if (is_acpi_node(fwnode))
+	if (is_of_node(fwnode)) {
+		int rval;
+
+		if (!val)
+			return of_property_count_strings(to_of_node(fwnode),
+							 propname);
+
+		rval = of_property_read_string_array(to_of_node(fwnode),
+						     propname, val, nval);
+
+		if (rval < 0)
+			return rval;
+
+		if (rval == nval)
+			return 0;
+
+		return -EOVERFLOW;
+	} else if (is_acpi_node(fwnode))
 		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
 					   val, nval);
 	else if (is_pset_node(fwnode)) {
-- 
2.7.4

Re: [PATCH 3/3] device property: of_property_read_string_array() returns number of strings

[Rafael J. Wysocki]

On Monday, March 06, 2017 03:26:31 PM Sakari Ailus wrote:
> of_property_read_string_array() returns number of strings read if the
> target array of pointers is non-NULL. fwnode_property_read_string_array()
> is documented to return 0 in that case. Fix this.

Well, if we want people to use fwnode_property_read_string_array() instead of
of_property_read_string_array(), it should better behave analogously, shouldn't it?

> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/base/property.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 9224541a..e67ec24 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -544,12 +544,24 @@ static 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 val ?
> -			of_property_read_string_array(to_of_node(fwnode),
> -						      propname, val, nval) :
> -			of_property_count_strings(to_of_node(fwnode), propname);
> -	else if (is_acpi_node(fwnode))
> +	if (is_of_node(fwnode)) {
> +		int rval;
> +
> +		if (!val)
> +			return of_property_count_strings(to_of_node(fwnode),
> +							 propname);
> +
> +		rval = of_property_read_string_array(to_of_node(fwnode),
> +						     propname, val, nval);
> +
> +		if (rval < 0)
> +			return rval;
> +
> +		if (rval == nval)
> +			return 0;
> +
> +		return -EOVERFLOW;
> +	} else if (is_acpi_node(fwnode))
>  		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
>  					   val, nval);
>  	else if (is_pset_node(fwnode)) {
> 

Thanks,
Rafael

Re: [PATCH 3/3] device property: of_property_read_string_array() returns number of strings

[Sakari Ailus]

Hi Rafael,

On 03/14/17 00:24, Rafael J. Wysocki wrote:
> On Monday, March 06, 2017 03:26:31 PM Sakari Ailus wrote:
>> of_property_read_string_array() returns number of strings read if the
>> target array of pointers is non-NULL. fwnode_property_read_string_array()
>> is documented to return 0 in that case. Fix this.
> 
> Well, if we want people to use fwnode_property_read_string_array() instead of
> of_property_read_string_array(), it should better behave analogously, shouldn't it?

Not necessarily.

The documentation states that fwnode_property_read_string_array()
returns 0 on success. That makes sense since the callers often check
whether the return value is non-zero and then return based on that.
Returning zero on success is just simpler for the caller.

Often the number of elements to read from an array is known beforehand,
so there's little use for the actual number read.

Besides the fwnode variants, also the OF integer array access functions
return zero on success.

Rob had a similar comment but the discussion did not continue after my
initial reply. I can sure change that if you still think it's better the
other way.

-- 
Kind regards,

Sakari Ailus
sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] device property: of_property_read_string_array() returns number of strings

[Rafael J. Wysocki]

On Tue, Mar 14, 2017 at 8:17 AM, Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
> Hi Rafael,
>
> On 03/14/17 00:24, Rafael J. Wysocki wrote:
>> On Monday, March 06, 2017 03:26:31 PM Sakari Ailus wrote:
>>> of_property_read_string_array() returns number of strings read if the
>>> target array of pointers is non-NULL. fwnode_property_read_string_array()
>>> is documented to return 0 in that case. Fix this.
>>
>> Well, if we want people to use fwnode_property_read_string_array() instead of
>> of_property_read_string_array(), it should better behave analogously, shouldn't it?
>
> Not necessarily.
>
> The documentation states that fwnode_property_read_string_array()
> returns 0 on success. That makes sense since the callers often check
> whether the return value is non-zero and then return based on that.
> Returning zero on success is just simpler for the caller.
>
> Often the number of elements to read from an array is known beforehand,
> so there's little use for the actual number read.
>
> Besides the fwnode variants, also the OF integer array access functions
> return zero on success.
>
> Rob had a similar comment but the discussion did not continue after my
> initial reply. I can sure change that if you still think it's better the
> other way.

I understand the argumentation, but if X is supposed to be an easy
replacement for Y, it really should stick to the return value
conventions used by the latter or it is not easy any more.

So yes, please.

Thanks,
Rafael

[PATCH v1.1 3/3] device property: fwnode_property_read_string_array() returns nr of strings

[Sakari Ailus]

Functionally fwnode_property_read_string_array() should match
of_property_read_string_array() and work as a drop-in substitute for the
latter. of_property_read_string_array() returns the number of strings read
if the target string pointer array is non-NULL. Make
fwnode_property_read_string_array() do the same.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
This patch replaces v1 3/3 patch in this set.

Instead of changing the return value of fwnode / device property API
string array access on OF, change the behaviour on pset and ACPI instead.
This makes them to return the number of strings read on success.

I can merge this with patch 2/3 which is changing the same part of the
file, however I'm sending this separately at least for now as I think it's
easier to review this way, rather than making a bugfix and a change of the
behaviour in the same patch.

Regards,
Sakari

 drivers/base/property.c | 64 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 18 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 8c98390..82187ac 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -340,8 +340,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u64_array);
  * Function reads an array of string properties with @propname from the device
  * firmware description and stores them to @val if found.
  *
- * Return: number of values if @val was %NULL,
- *         %0 if the property was found (success),
+ * Return: number of values read on success if @val is non-NULL,
+ *	   number of values available on success if @val is NULL,
  *	   %-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,
@@ -549,29 +549,57 @@ static int __fwnode_property_read_string_array(struct fwnode_handle *fwnode,
 			of_property_read_string_array(to_of_node(fwnode),
 						      propname, val, nval) :
 			of_property_count_strings(to_of_node(fwnode), propname);
-	else if (is_acpi_node(fwnode))
-		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
-					   val, nval);
-	else if (is_pset_node(fwnode)) {
+	else if (is_acpi_node(fwnode)) {
+		int array_len =
+			acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
+					    NULL, nval);
+		int read_len;
+		int ret;
+
+		/* Return if val is NULL or if there was an error */
+		if (!val || array_len < 0)
+			return array_len;
+
+		read_len = min_t(int, nval, array_len);
+
+		ret = acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
+					  val, read_len);
+		if (ret < 0)
+			return ret;
+
+		return read_len;
+	} else if (is_pset_node(fwnode)) {
 		struct property_set *node = to_pset_node(fwnode);
 		const struct property_entry *prop;
-
-		/* Read properties if val is non-NULL */
-		if (val)
-			return pset_prop_read_string_array(node, propname,
-							   val, nval);
+		/* The array length for a non-array string property is 1. */
+		int array_len = 1;
+		int read_len;
+		int ret;
 
 		prop = pset_prop_get(node, propname);
 		if (!prop)
 			return -EINVAL;
 
-		/* The array length for a non-array string property is 1. */
-		if (!prop->is_array)
-			return 1;
+		/* Read the length of an array property. */
+		if (prop->is_array)
+			array_len = pset_prop_count_elems_of_size(
+				node, propname, sizeof(const char *));
+
+
+		/* Return if val is NULL or if there was an error */
+		if (!val || array_len < 0)
+			return array_len;
+
+		read_len = min_t(int, nval, array_len);
+
+		ret = pset_prop_read_string_array(node, propname, val,
+						  read_len);
+
+		if (ret < 0)
+			return ret;
 
 		/* Return the length of an array. */
-		return pset_prop_count_elems_of_size(node, propname,
-						     sizeof(const char *));
+		return read_len;
 	}
 	return -ENXIO;
 }
@@ -599,8 +627,8 @@ static int __fwnode_property_read_string(struct fwnode_handle *fwnode,
  * Read an string list property @propname from the given firmware node and store
  * them to @val if found.
  *
- * Return: number of values if @val was %NULL,
- *         %0 if the property was found (success),
+ * Return: number of values read on success if @val is non-NULL,
+ *	   number of values available on success if @val is NULL,
  *	   %-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,
-- 
2.7.4

Re: [PATCH v1.1 3/3] device property: fwnode_property_read_string_array() returns nr of strings

[Mika Westerberg]

On Wed, Mar 15, 2017 at 03:31:28PM +0200, Sakari Ailus wrote:
> Functionally fwnode_property_read_string_array() should match
> of_property_read_string_array() and work as a drop-in substitute for the
> latter. of_property_read_string_array() returns the number of strings read
> if the target string pointer array is non-NULL. Make
> fwnode_property_read_string_array() do the same.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> This patch replaces v1 3/3 patch in this set.
> 
> Instead of changing the return value of fwnode / device property API
> string array access on OF, change the behaviour on pset and ACPI instead.
> This makes them to return the number of strings read on success.
> 
> I can merge this with patch 2/3 which is changing the same part of the
> file, however I'm sending this separately at least for now as I think it's
> easier to review this way, rather than making a bugfix and a change of the
> behaviour in the same patch.
> 
> Regards,
> Sakari
> 
>  drivers/base/property.c | 64 +++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 46 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 8c98390..82187ac 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -340,8 +340,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u64_array);
>   * Function reads an array of string properties with @propname from the device
>   * firmware description and stores them to @val if found.
>   *
> - * Return: number of values if @val was %NULL,
> - *         %0 if the property was found (success),
> + * Return: number of values read on success if @val is non-NULL,
> + *	   number of values available on success if @val is NULL,
>   *	   %-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,
> @@ -549,29 +549,57 @@ static int __fwnode_property_read_string_array(struct fwnode_handle *fwnode,
>  			of_property_read_string_array(to_of_node(fwnode),
>  						      propname, val, nval) :
>  			of_property_count_strings(to_of_node(fwnode), propname);
> -	else if (is_acpi_node(fwnode))
> -		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
> -					   val, nval);
> -	else if (is_pset_node(fwnode)) {
> +	else if (is_acpi_node(fwnode)) {
> +		int array_len =
> +			acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,

Why not change acpi_node_prop_read() instead? This way you don't need to
add the extra code dealing with the return value here.

Ditto for the pset counterpart.

Re: [PATCH v1.1 3/3] device property: fwnode_property_read_string_array() returns nr of strings

[Sakari Ailus]

Hi Mika,

Mika Westerberg wrote:
> On Wed, Mar 15, 2017 at 03:31:28PM +0200, Sakari Ailus wrote:
>> Functionally fwnode_property_read_string_array() should match
>> of_property_read_string_array() and work as a drop-in substitute for the
>> latter. of_property_read_string_array() returns the number of strings read
>> if the target string pointer array is non-NULL. Make
>> fwnode_property_read_string_array() do the same.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> ---
>> This patch replaces v1 3/3 patch in this set.
>>
>> Instead of changing the return value of fwnode / device property API
>> string array access on OF, change the behaviour on pset and ACPI instead.
>> This makes them to return the number of strings read on success.
>>
>> I can merge this with patch 2/3 which is changing the same part of the
>> file, however I'm sending this separately at least for now as I think it's
>> easier to review this way, rather than making a bugfix and a change of the
>> behaviour in the same patch.
>>
>> Regards,
>> Sakari
>>
>>  drivers/base/property.c | 64 +++++++++++++++++++++++++++++++++++--------------
>>  1 file changed, 46 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/base/property.c b/drivers/base/property.c
>> index 8c98390..82187ac 100644
>> --- a/drivers/base/property.c
>> +++ b/drivers/base/property.c
>> @@ -340,8 +340,8 @@ EXPORT_SYMBOL_GPL(device_property_read_u64_array);
>>   * Function reads an array of string properties with @propname from the device
>>   * firmware description and stores them to @val if found.
>>   *
>> - * Return: number of values if @val was %NULL,
>> - *         %0 if the property was found (success),
>> + * Return: number of values read on success if @val is non-NULL,
>> + *	   number of values available on success if @val is NULL,
>>   *	   %-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,
>> @@ -549,29 +549,57 @@ static int __fwnode_property_read_string_array(struct fwnode_handle *fwnode,
>>  			of_property_read_string_array(to_of_node(fwnode),
>>  						      propname, val, nval) :
>>  			of_property_count_strings(to_of_node(fwnode), propname);
>> -	else if (is_acpi_node(fwnode))
>> -		return acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
>> -					   val, nval);
>> -	else if (is_pset_node(fwnode)) {
>> +	else if (is_acpi_node(fwnode)) {
>> +		int array_len =
>> +			acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
>
> Why not change acpi_node_prop_read() instead? This way you don't need to
> add the extra code dealing with the return value here.
>
> Ditto for the pset counterpart.

There are a few other users of acpi_node_prop_read() albeit there are 
not too many of them. acpi_node_prop_read() is just calling 
acpi_dev_prop_read() which is public but apparently has no users. I 
think changing that would be quite feasible, I'll take that into account 
in the next version. Same for pset.

-- 
Sakari Ailus
sakari.ailus@linux.intel.com