* [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
[parent not found: <1488806791-25488-2-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* 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
* [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
[parent not found: <1488806791-25488-3-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* 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
* [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 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
[parent not found: <1840694.EMHZKJ1jEg-yvgW3jdyMHm1GS7QM15AGw@public.gmane.org>]
* 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 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.