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@xxxxxxxxxxxxxxx> --- 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@xxxxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
