* [PATCH v2 0/3] Fwnode property API fixes for OF, pset and ACPI
@ 2017-03-28 12:22 Sakari Ailus
2017-03-28 12:22 ` [PATCH v2 1/3] device property: fwnode_property_read_string_array() may return -EILSEQ Sakari Ailus
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Sakari Ailus @ 2017-03-28 12:22 UTC (permalink / raw)
To: linux-acpi, devicetree, rafael
Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, mark.rutland,
broonie, robh, ahs3
Hi folks,
Here are three patches that fix a few issues in the fwnode property API
implementation. They were formerly 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>
The v1 (previous version) of the set is also available here:
<URL:http://www.spinics.net/lists/linux-acpi/msg72327.html>
These patches apply cleanly on v4.11-rc1 and linux-next with or without
the ACPI graph support patchset I've sent a moment ago.
changes since v1.1:
- Move fixes to fwnode_property_read_string_array() return value fixes to
respective implementations of those functions, rather than working
around them in the fwnode property interface functions.
- fwnode_property_read_string_array() now may return fewer strings than
requested also on ACPI in order to make behaviour of
fwnode_property_read_string_array() fully consistent.
changes since v1:
- Make prop const. pset_prop_get() returns const struct property_entry *
now in __fwnode_property_read_string_array().
- Make fwnode_property_read_string_array() behave as
of_property_read_string_array() does in terms of the return value.
Sakari Ailus (3):
device property: fwnode_property_read_string_array() may return
-EILSEQ
device property: Fix reading pset strings using array access functions
device property: fwnode_property_read_string_array() returns nr of
strings
drivers/acpi/property.c | 22 +++++++++++++++++-----
drivers/base/property.c | 46 ++++++++++++++++++++++++++++++++--------------
2 files changed, 49 insertions(+), 19 deletions(-)
--
Kind regards,
Sakari
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/3] device property: fwnode_property_read_string_array() may return -EILSEQ
2017-03-28 12:22 [PATCH v2 0/3] Fwnode property API fixes for OF, pset and ACPI Sakari Ailus
@ 2017-03-28 12:22 ` Sakari Ailus
2017-03-28 12:22 ` [PATCH v2 2/3] device property: Fix reading pset strings using array access functions Sakari Ailus
[not found] ` <1490703739-28270-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2 siblings, 0 replies; 7+ messages in thread
From: Sakari Ailus @ 2017-03-28 12:22 UTC (permalink / raw)
To: linux-acpi, devicetree, rafael
Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, 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>
Reviewed-by: Mika Westerberg <mika.westerberg@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] 7+ messages in thread
* [PATCH v2 2/3] device property: Fix reading pset strings using array access functions
2017-03-28 12:22 [PATCH v2 0/3] Fwnode property API fixes for OF, pset and ACPI Sakari Ailus
2017-03-28 12:22 ` [PATCH v2 1/3] device property: fwnode_property_read_string_array() may return -EILSEQ Sakari Ailus
@ 2017-03-28 12:22 ` Sakari Ailus
2017-03-28 12:48 ` Mika Westerberg
[not found] ` <1490703739-28270-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2 siblings, 1 reply; 7+ messages in thread
From: Sakari Ailus @ 2017-03-28 12:22 UTC (permalink / raw)
To: linux-acpi, devicetree, rafael
Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, 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 | 32 +++++++++++++++++++++++++-------
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 19a3dc5..686e985 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -146,14 +146,36 @@ static int pset_prop_read_string_array(struct property_set *pset,
const char *propname,
const char **strings, size_t nval)
{
+ const struct property_entry *prop;
const void *pointer;
- size_t length = nval * sizeof(*strings);
+ size_t array_len, length;
+
+ /* Find out the array length. */
+ prop = pset_prop_get(pset, propname);
+ if (!prop)
+ return -EINVAL;
+
+ if (!prop->is_array)
+ /* The array length for a non-array string property is 1. */
+ array_len = 1;
+ else
+ /* Find the length of an array. */
+ array_len = pset_prop_count_elems_of_size(pset, propname,
+ sizeof(const char *));
+
+ /* Return how many there are if strings is NULL. */
+ if (!strings)
+ return array_len;
+
+ array_len = min(nval, array_len);
+ length = array_len * sizeof(*strings);
pointer = pset_prop_find(pset, propname, length);
if (IS_ERR(pointer))
return PTR_ERR(pointer);
memcpy(strings, pointer, length);
+
return 0;
}
@@ -553,12 +575,8 @@ static int __fwnode_property_read_string_array(struct fwnode_handle *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 *));
+ return pset_prop_read_string_array(to_pset_node(fwnode),
+ propname, val, nval);
return -ENXIO;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] device property: fwnode_property_read_string_array() returns nr of strings
[not found] ` <1490703739-28270-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2017-03-28 12:22 ` Sakari Ailus
[not found] ` <1490703739-28270-4-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Sakari Ailus @ 2017-03-28 12:22 UTC (permalink / raw)
To: linux-acpi-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, rafael-DgEjT+Ai2ygdnm+yROfE0A
Cc: sudeep.holla-5wv7dgnIgG8, lorenzo.pieralisi-5wv7dgnIgG8,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA, mark.rutland-5wv7dgnIgG8,
broonie-DgEjT+Ai2ygdnm+yROfE0A, robh-DgEjT+Ai2ygdnm+yROfE0A,
ahs3-H+wXaHxf7aLQT0dZR+AlfA
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-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/acpi/property.c | 22 +++++++++++++++++-----
drivers/base/property.c | 12 ++++++------
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 3afddcd..06f8fda 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -699,6 +699,8 @@ static int acpi_data_prop_read_single(struct acpi_device_data *data,
return ret;
*(char **)val = obj->string.pointer;
+
+ return 1;
} else {
ret = -EINVAL;
}
@@ -708,7 +710,15 @@ static int acpi_data_prop_read_single(struct acpi_device_data *data,
int acpi_dev_prop_read_single(struct acpi_device *adev, const char *propname,
enum dev_prop_type proptype, void *val)
{
- return adev ? acpi_data_prop_read_single(&adev->data, propname, proptype, val) : -EINVAL;
+ int ret;
+
+ if (!adev)
+ return -EINVAL;
+
+ ret = acpi_data_prop_read_single(&adev->data, propname, proptype, val);
+ if (ret < 0 || proptype != ACPI_TYPE_STRING)
+ return ret;
+ return 0;
}
static int acpi_copy_property_array_u8(const union acpi_object *items, u8 *val,
@@ -784,7 +794,7 @@ static int acpi_copy_property_array_string(const union acpi_object *items,
val[i] = items[i].string.pointer;
}
- return 0;
+ return nval;
}
static int acpi_data_prop_read(struct acpi_device_data *data,
@@ -798,7 +808,7 @@ static int acpi_data_prop_read(struct acpi_device_data *data,
if (val && nval == 1) {
ret = acpi_data_prop_read_single(data, propname, proptype, val);
- if (!ret)
+ if (ret >= 0)
return ret;
}
@@ -809,7 +819,7 @@ static int acpi_data_prop_read(struct acpi_device_data *data,
if (!val)
return obj->package.count;
- if (nval > obj->package.count)
+ if (proptype != DEV_PROP_STRING && nval > obj->package.count)
return -EOVERFLOW;
else if (nval <= 0)
return -EINVAL;
@@ -830,7 +840,9 @@ static int acpi_data_prop_read(struct acpi_device_data *data,
ret = acpi_copy_property_array_u64(items, (u64 *)val, nval);
break;
case DEV_PROP_STRING:
- ret = acpi_copy_property_array_string(items, (char **)val, nval);
+ ret = acpi_copy_property_array_string(
+ items, (char **)val,
+ min_t(u32, nval, obj->package.count));
break;
default:
ret = -EINVAL;
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 686e985..b517485 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -176,7 +176,7 @@ static int pset_prop_read_string_array(struct property_set *pset,
memcpy(strings, pointer, length);
- return 0;
+ return array_len;
}
static int pset_prop_read_string(struct property_set *pset,
@@ -362,8 +362,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,
@@ -603,8 +603,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,
@@ -651,7 +651,7 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
!IS_ERR_OR_NULL(fwnode->secondary))
ret = __fwnode_property_read_string(fwnode->secondary,
propname, val);
- return ret;
+ return ret < 0 ? ret : 0;
}
EXPORT_SYMBOL_GPL(fwnode_property_read_string);
--
2.7.4
--
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 related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] device property: Fix reading pset strings using array access functions
2017-03-28 12:22 ` [PATCH v2 2/3] device property: Fix reading pset strings using array access functions Sakari Ailus
@ 2017-03-28 12:48 ` Mika Westerberg
0 siblings, 0 replies; 7+ messages in thread
From: Mika Westerberg @ 2017-03-28 12:48 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-acpi, devicetree, rafael, sudeep.holla, lorenzo.pieralisi,
mark.rutland, broonie, robh, ahs3
On Tue, Mar 28, 2017 at 03:22:18PM +0300, Sakari Ailus wrote:
> 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>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 3/3] device property: fwnode_property_read_string_array() returns nr of strings
[not found] ` <1490703739-28270-4-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2017-03-28 12:52 ` Mika Westerberg
[not found] ` <20170328125231.GP2957-3PARRvDOhMZrdx17CPfAsdBPR1lH4CV8@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Mika Westerberg @ 2017-03-28 12:52 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, rafael-DgEjT+Ai2ygdnm+yROfE0A,
sudeep.holla-5wv7dgnIgG8, lorenzo.pieralisi-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, broonie-DgEjT+Ai2ygdnm+yROfE0A,
robh-DgEjT+Ai2ygdnm+yROfE0A, ahs3-H+wXaHxf7aLQT0dZR+AlfA
On Tue, Mar 28, 2017 at 03:22:19PM +0300, Sakari Ailus wrote:
> - ret = acpi_copy_property_array_string(items, (char **)val, nval);
> + ret = acpi_copy_property_array_string(
> + items, (char **)val,
> + min_t(u32, nval, obj->package.count));
I think this looks better if written like:
ret = acpi_copy_property_array_string(items, (char **)val,
min_t(u32, nval, obj->package.count));
Regardless of that,
Reviewed-by: Mika Westerberg <mika.westerberg-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] 7+ messages in thread
* Re: [PATCH v2 3/3] device property: fwnode_property_read_string_array() returns nr of strings
[not found] ` <20170328125231.GP2957-3PARRvDOhMZrdx17CPfAsdBPR1lH4CV8@public.gmane.org>
@ 2017-03-28 13:07 ` Sakari Ailus
0 siblings, 0 replies; 7+ messages in thread
From: Sakari Ailus @ 2017-03-28 13:07 UTC (permalink / raw)
To: Mika Westerberg
Cc: Sakari Ailus, linux-acpi-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, rafael-DgEjT+Ai2ygdnm+yROfE0A,
sudeep.holla-5wv7dgnIgG8, lorenzo.pieralisi-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, broonie-DgEjT+Ai2ygdnm+yROfE0A,
robh-DgEjT+Ai2ygdnm+yROfE0A, ahs3-H+wXaHxf7aLQT0dZR+AlfA
Hi Mika,
Thank you for the review!
On Tue, Mar 28, 2017 at 03:52:31PM +0300, Mika Westerberg wrote:
> On Tue, Mar 28, 2017 at 03:22:19PM +0300, Sakari Ailus wrote:
> > - ret = acpi_copy_property_array_string(items, (char **)val, nval);
> > + ret = acpi_copy_property_array_string(
> > + items, (char **)val,
> > + min_t(u32, nval, obj->package.count));
>
> I think this looks better if written like:
>
> ret = acpi_copy_property_array_string(items, (char **)val,
> min_t(u32, nval, obj->package.count));
I prefer the former although I have no strong opinion either way. Using an
indentation that is automatically intended by a text editor is also an
advantage.
I can change it if you insist. :-)
>
> Regardless of that,
>
> Reviewed-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Thanks!
--
Regards,
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@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] 7+ messages in thread
end of thread, other threads:[~2017-03-28 13:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-28 12:22 [PATCH v2 0/3] Fwnode property API fixes for OF, pset and ACPI Sakari Ailus
2017-03-28 12:22 ` [PATCH v2 1/3] device property: fwnode_property_read_string_array() may return -EILSEQ Sakari Ailus
2017-03-28 12:22 ` [PATCH v2 2/3] device property: Fix reading pset strings using array access functions Sakari Ailus
2017-03-28 12:48 ` Mika Westerberg
[not found] ` <1490703739-28270-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-03-28 12:22 ` [PATCH v2 3/3] device property: fwnode_property_read_string_array() returns nr of strings Sakari Ailus
[not found] ` <1490703739-28270-4-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-03-28 12:52 ` Mika Westerberg
[not found] ` <20170328125231.GP2957-3PARRvDOhMZrdx17CPfAsdBPR1lH4CV8@public.gmane.org>
2017-03-28 13:07 ` 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.