* [PATCH 0/3] Fwnode property API fixes for OF, pset
@ 2017-03-06 13:26 Sakari Ailus
2017-03-06 13:26 ` [PATCH 1/3] device property: fwnode_property_read_string_array() may return -EILSEQ Sakari Ailus
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Sakari Ailus @ 2017-03-06 13:26 UTC (permalink / raw)
To: linux-acpi, devicetree
Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
mark.rutland, broonie, robh, ahs3
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] device property: fwnode_property_read_string_array() may return -EILSEQ
2017-03-06 13:26 [PATCH 0/3] Fwnode property API fixes for OF, pset Sakari Ailus
@ 2017-03-06 13:26 ` Sakari Ailus
[not found] ` <1488806791-25488-2-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-03-06 13:26 ` [PATCH 2/3] device property: Fix reading pset strings using array access functions Sakari Ailus
2017-03-06 13:26 ` [PATCH 3/3] device property: of_property_read_string_array() returns number of strings Sakari Ailus
2 siblings, 1 reply; 14+ messages in thread
From: Sakari Ailus @ 2017-03-06 13:26 UTC (permalink / raw)
To: linux-acpi, devicetree
Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
mark.rutland, broonie, robh, ahs3
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] device property: Fix reading pset strings using array access functions
2017-03-06 13:26 [PATCH 0/3] Fwnode property API fixes for OF, pset Sakari Ailus
2017-03-06 13:26 ` [PATCH 1/3] device property: fwnode_property_read_string_array() may return -EILSEQ Sakari Ailus
@ 2017-03-06 13:26 ` Sakari Ailus
[not found] ` <1488806791-25488-3-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-03-08 5:41 ` [PATCH v1.1 " Sakari Ailus
2017-03-06 13:26 ` [PATCH 3/3] device property: of_property_read_string_array() returns number of strings Sakari Ailus
2 siblings, 2 replies; 14+ messages in thread
From: Sakari Ailus @ 2017-03-06 13:26 UTC (permalink / raw)
To: linux-acpi, devicetree
Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
mark.rutland, broonie, robh, ahs3
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] device property: of_property_read_string_array() returns number of strings
2017-03-06 13:26 [PATCH 0/3] Fwnode property API fixes for OF, pset Sakari Ailus
2017-03-06 13:26 ` [PATCH 1/3] device property: fwnode_property_read_string_array() may return -EILSEQ Sakari Ailus
2017-03-06 13:26 ` [PATCH 2/3] device property: Fix reading pset strings using array access functions Sakari Ailus
@ 2017-03-06 13:26 ` Sakari Ailus
2017-03-13 22:24 ` Rafael J. Wysocki
2017-03-15 13:31 ` [PATCH v1.1 3/3] device property: fwnode_property_read_string_array() returns nr " Sakari Ailus
2 siblings, 2 replies; 14+ messages in thread
From: Sakari Ailus @ 2017-03-06 13:26 UTC (permalink / raw)
To: linux-acpi, devicetree
Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
mark.rutland, broonie, robh, ahs3
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] device property: Fix reading pset strings using array access functions
[not found] ` <1488806791-25488-3-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2017-03-07 23:51 ` kbuild test robot
0 siblings, 0 replies; 14+ messages in thread
From: kbuild test robot @ 2017-03-07 23:51 UTC (permalink / raw)
To: Sakari Ailus
Cc: kbuild-all-JC7UmRfGjtg, linux-acpi-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, sudeep.holla-5wv7dgnIgG8,
lorenzo.pieralisi-5wv7dgnIgG8,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
rafael-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
broonie-DgEjT+Ai2ygdnm+yROfE0A, robh-DgEjT+Ai2ygdnm+yROfE0A,
ahs3-H+wXaHxf7aLQT0dZR+AlfA
[-- 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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1.1 2/3] device property: Fix reading pset strings using array access functions
2017-03-06 13:26 ` [PATCH 2/3] device property: Fix reading pset strings using array access functions Sakari Ailus
[not found] ` <1488806791-25488-3-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2017-03-08 5:41 ` Sakari Ailus
1 sibling, 0 replies; 14+ messages in thread
From: Sakari Ailus @ 2017-03-08 5:41 UTC (permalink / raw)
To: linux-acpi, devicetree
Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
mark.rutland, broonie, robh, ahs3
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] device property: of_property_read_string_array() returns number of strings
2017-03-06 13:26 ` [PATCH 3/3] device property: of_property_read_string_array() returns number of strings Sakari Ailus
@ 2017-03-13 22:24 ` Rafael J. Wysocki
[not found] ` <1840694.EMHZKJ1jEg-yvgW3jdyMHm1GS7QM15AGw@public.gmane.org>
2017-03-15 13:31 ` [PATCH v1.1 3/3] device property: fwnode_property_read_string_array() returns nr " Sakari Ailus
1 sibling, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2017-03-13 22:24 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-acpi, devicetree, sudeep.holla, lorenzo.pieralisi,
mika.westerberg, rafael, mark.rutland, broonie, robh, ahs3
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] device property: of_property_read_string_array() returns number of strings
[not found] ` <1840694.EMHZKJ1jEg-yvgW3jdyMHm1GS7QM15AGw@public.gmane.org>
@ 2017-03-14 7:17 ` Sakari Ailus
2017-03-14 16:57 ` Rafael J. Wysocki
0 siblings, 1 reply; 14+ messages in thread
From: Sakari Ailus @ 2017-03-14 7:17 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, sudeep.holla-5wv7dgnIgG8,
lorenzo.pieralisi-5wv7dgnIgG8,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
rafael-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
broonie-DgEjT+Ai2ygdnm+yROfE0A, robh-DgEjT+Ai2ygdnm+yROfE0A,
ahs3-H+wXaHxf7aLQT0dZR+AlfA
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] device property: of_property_read_string_array() returns number of strings
2017-03-14 7:17 ` Sakari Ailus
@ 2017-03-14 16:57 ` Rafael J. Wysocki
0 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2017-03-14 16:57 UTC (permalink / raw)
To: Sakari Ailus
Cc: Rafael J. Wysocki, ACPI Devel Maling List, devicetree,
Sudeep Holla, Lorenzo Pieralisi, Mika Westerberg,
Rafael J. Wysocki, Mark Rutland, Mark Brown, Rob Herring,
Al Stone
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1.1 3/3] device property: fwnode_property_read_string_array() returns nr of strings
2017-03-06 13:26 ` [PATCH 3/3] device property: of_property_read_string_array() returns number of strings Sakari Ailus
2017-03-13 22:24 ` Rafael J. Wysocki
@ 2017-03-15 13:31 ` Sakari Ailus
2017-03-27 13:36 ` Mika Westerberg
1 sibling, 1 reply; 14+ messages in thread
From: Sakari Ailus @ 2017-03-15 13:31 UTC (permalink / raw)
To: linux-acpi, devicetree
Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
mark.rutland, broonie, robh, ahs3
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1.1 3/3] device property: fwnode_property_read_string_array() returns nr of strings
2017-03-15 13:31 ` [PATCH v1.1 3/3] device property: fwnode_property_read_string_array() returns nr " Sakari Ailus
@ 2017-03-27 13:36 ` Mika Westerberg
2017-03-27 21:46 ` Sakari Ailus
0 siblings, 1 reply; 14+ messages in thread
From: Mika Westerberg @ 2017-03-27 13:36 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-acpi, devicetree, sudeep.holla, lorenzo.pieralisi, rafael,
mark.rutland, broonie, robh, ahs3
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] device property: fwnode_property_read_string_array() may return -EILSEQ
[not found] ` <1488806791-25488-2-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2017-03-27 13:53 ` Mika Westerberg
2017-03-27 21:39 ` Sakari Ailus
0 siblings, 1 reply; 14+ messages in thread
From: Mika Westerberg @ 2017-03-27 13:53 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, sudeep.holla-5wv7dgnIgG8,
lorenzo.pieralisi-5wv7dgnIgG8, rafael-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, broonie-DgEjT+Ai2ygdnm+yROfE0A,
robh-DgEjT+Ai2ygdnm+yROfE0A, ahs3-H+wXaHxf7aLQT0dZR+AlfA
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] device property: fwnode_property_read_string_array() may return -EILSEQ
2017-03-27 13:53 ` Mika Westerberg
@ 2017-03-27 21:39 ` Sakari Ailus
0 siblings, 0 replies; 14+ messages in thread
From: Sakari Ailus @ 2017-03-27 21:39 UTC (permalink / raw)
To: Mika Westerberg
Cc: linux-acpi, devicetree, sudeep.holla, lorenzo.pieralisi, rafael,
mark.rutland, broonie, robh, ahs3
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1.1 3/3] device property: fwnode_property_read_string_array() returns nr of strings
2017-03-27 13:36 ` Mika Westerberg
@ 2017-03-27 21:46 ` Sakari Ailus
0 siblings, 0 replies; 14+ messages in thread
From: Sakari Ailus @ 2017-03-27 21:46 UTC (permalink / raw)
To: Mika Westerberg
Cc: linux-acpi, devicetree, sudeep.holla, lorenzo.pieralisi, rafael,
mark.rutland, broonie, robh, ahs3
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
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-03-27 21:49 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-06 13:26 [PATCH 0/3] Fwnode property API fixes for OF, pset Sakari Ailus
2017-03-06 13:26 ` [PATCH 1/3] device property: fwnode_property_read_string_array() may return -EILSEQ Sakari Ailus
[not found] ` <1488806791-25488-2-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-03-27 13:53 ` Mika Westerberg
2017-03-27 21:39 ` Sakari Ailus
2017-03-06 13:26 ` [PATCH 2/3] device property: Fix reading pset strings using array access functions Sakari Ailus
[not found] ` <1488806791-25488-3-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-03-07 23:51 ` kbuild test robot
2017-03-08 5:41 ` [PATCH v1.1 " Sakari Ailus
2017-03-06 13:26 ` [PATCH 3/3] device property: of_property_read_string_array() returns number of strings Sakari Ailus
2017-03-13 22:24 ` Rafael J. Wysocki
[not found] ` <1840694.EMHZKJ1jEg-yvgW3jdyMHm1GS7QM15AGw@public.gmane.org>
2017-03-14 7:17 ` Sakari Ailus
2017-03-14 16:57 ` Rafael J. Wysocki
2017-03-15 13:31 ` [PATCH v1.1 3/3] device property: fwnode_property_read_string_array() returns nr " Sakari Ailus
2017-03-27 13:36 ` Mika Westerberg
2017-03-27 21:46 ` Sakari Ailus
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.