Re: [PATCH v2 1/7] device property: add fwnode_property_read_string_index()

[Sakari Ailus]

Hi Clément,

Thanks for the set.

On Wed, Mar 23, 2022 at 10:18:04AM +0100, Clément Léger wrote:
> Add fwnode_property_read_string_index() function which allows to
> retrieve a string from an array by its index. This function is the
> equivalent of of_property_read_string_index() but for fwnode support.
> A .property_read_string_index callback is added to fwnode_ops to avoid
> doing a full allocation of an array just to retrieve one value.
> 
> Signed-off-by: Clément Léger <clement.leger@xxxxxxxxxxx>
> ---
>  drivers/base/property.c  | 26 ++++++++++++++++++++++++++
>  include/linux/fwnode.h   |  6 ++++++
>  include/linux/property.h |  3 +++
>  3 files changed, 35 insertions(+)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index e6497f6877ee..a8dd6e496a1d 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -451,6 +451,32 @@ int fwnode_property_match_string(const struct fwnode_handle *fwnode,
>  }
>  EXPORT_SYMBOL_GPL(fwnode_property_match_string);
>  
> +/**
> + * fwnode_property_read_string_index - read a string in an array using an index
> + * @fwnode: Firmware node to get the property of
> + * @propname: Name of the property holding the array
> + * @index: Index of the string to look for
> + * @string: Pointer to the string if found
> + *
> + * Find a string by a given index in a string array and if it is found return
> + * the string value in @string.
> + *
> + * Return: %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,
> + *	   %-ENXIO if no suitable firmware interface is present.
> + */
> +int fwnode_property_read_string_index(const struct fwnode_handle *fwnode,
> +				      const char *propname, int index,
> +				      const char **string)
> +{
> +	return fwnode_call_int_op(fwnode, property_read_string_index, propname,
> +				  index,
> +				  string);
> +}
> +EXPORT_SYMBOL_GPL(fwnode_property_read_string_index);
> +
>  /**
>   * fwnode_property_get_reference_args() - Find a reference with arguments
>   * @fwnode:	Firmware node where to look for the reference
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index 3a532ba66f6c..71ba8f53cf1e 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -93,6 +93,9 @@ struct fwnode_reference_args {
>   *			     success, a negative error code otherwise.
>   * @property_read_string_array: Read an array of string properties. Return zero
>   *				on success, a negative error code otherwise.
> + * @property_read_string_index: Read a string from a string array using an
> + *				index. Return zero on success, a negative error
> + *				code otherwise.
>   * @get_name: Return the name of an fwnode.
>   * @get_name_prefix: Get a prefix for a node (for printing purposes).
>   * @get_parent: Return the parent of an fwnode.
> @@ -123,6 +126,9 @@ struct fwnode_operations {
>  	(*property_read_string_array)(const struct fwnode_handle *fwnode_handle,
>  				      const char *propname, const char **val,
>  				      size_t nval);
> +	int (*property_read_string_index)(const struct fwnode_handle *fwnode,
> +					  const char *propname, int index,
> +					  const char **string);

Could this instead be done by adding an index argument to the
property_read_string_array?

The ACPI case is a bit more work but it guess it could be implemented later
as part of a more general cleanup there.

>  	const char *(*get_name)(const struct fwnode_handle *fwnode);
>  	const char *(*get_name_prefix)(const struct fwnode_handle *fwnode);
>  	struct fwnode_handle *(*get_parent)(const struct fwnode_handle *fwnode);
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 7399a0b45f98..a033920eb10a 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -70,6 +70,9 @@ int fwnode_property_read_string_array(const struct fwnode_handle *fwnode,
>  				      size_t nval);
>  int fwnode_property_read_string(const struct fwnode_handle *fwnode,
>  				const char *propname, const char **val);
> +int fwnode_property_read_string_index(const struct fwnode_handle *fwnode,
> +				      const char *propname, int index,
> +				      const char **string);
>  int fwnode_property_match_string(const struct fwnode_handle *fwnode,
>  				 const char *propname, const char *string);
>  int fwnode_property_get_reference_args(const struct fwnode_handle *fwnode,

-- 
Kind regards,

Sakari Ailus