[PATCH 1/2] device property: Add fwnode_property_match_string()

[PATCH 1/2] device property: Add fwnode_property_match_string()

[Mika Westerberg]

Sometimes it is useful to be able to extract an index of certain string
value from an array of strings. A typical use case is to give a name to a
DMA channel, PWM, clock and so on.

Provide an implementation using unified device property accessors that
follows of_property_match_string() but works for all supported fwnodes.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/base/property.c  | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/property.h |  4 +++
 2 files changed, 72 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 2d75366c61e0..a128db413f01 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -287,6 +287,28 @@ int device_property_read_string(struct device *dev, const char *propname,
 }
 EXPORT_SYMBOL_GPL(device_property_read_string);
 
+/**
+ * device_property_match_string - find a string in an array and return index
+ * @dev: Device to get the property of
+ * @propname: Name of the property holding the array
+ * @string: String to look for
+ *
+ * Find a given string in a string array and if it is found return the
+ * index back.
+ *
+ * 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 device_property_match_string(struct device *dev, const char *propname,
+				 const char *string)
+{
+	return fwnode_property_match_string(dev_fwnode(dev), propname, string);
+}
+EXPORT_SYMBOL_GPL(device_property_match_string);
+
 #define OF_DEV_PROP_READ_ARRAY(node, propname, type, val, nval) \
 	(val) ? of_property_read_##type##_array((node), (propname), (val), (nval)) \
 	      : of_property_count_elems_of_size((node), (propname), sizeof(type))
@@ -479,6 +501,52 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
 EXPORT_SYMBOL_GPL(fwnode_property_read_string);
 
 /**
+ * fwnode_property_match_string - find a string in an array and return index
+ * @fwnode: Firmware node to get the property of
+ * @propname: Name of the property holding the array
+ * @string: String to look for
+ *
+ * Find a given string in a string array and if it is found return the
+ * index back.
+ *
+ * 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_match_string(struct fwnode_handle *fwnode,
+	const char *propname, const char *string)
+{
+	const char **values;
+	int nval, ret, i;
+
+	nval = fwnode_property_read_string_array(fwnode, propname, NULL, 0);
+	if (nval < 0)
+		return nval;
+
+	values = kcalloc(nval, sizeof(*values), GFP_KERNEL);
+	if (!values)
+		return -ENOMEM;
+
+	ret = fwnode_property_read_string_array(fwnode, propname, values, nval);
+	if (ret < 0)
+		goto out;
+
+	ret = -ENODATA;
+	for (i = 0; i < nval; i++) {
+		if (!strcmp(values[i], string)) {
+			ret = i;
+			break;
+		}
+	}
+out:
+	kfree(values);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(fwnode_property_match_string);
+
+/**
  * device_get_next_child_node - Return the next child node handle for a device
  * @dev: Device to find the next child node for.
  * @child: Handle to one of the device's child nodes or a null handle.
diff --git a/include/linux/property.h b/include/linux/property.h
index a59c6ee566c2..463de52fe891 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -40,6 +40,8 @@ int device_property_read_string_array(struct device *dev, const char *propname,
 				      const char **val, size_t nval);
 int device_property_read_string(struct device *dev, const char *propname,
 				const char **val);
+int device_property_match_string(struct device *dev,
+				 const char *propname, const char *string);
 
 bool fwnode_property_present(struct fwnode_handle *fwnode, const char *propname);
 int fwnode_property_read_u8_array(struct fwnode_handle *fwnode,
@@ -59,6 +61,8 @@ int fwnode_property_read_string_array(struct fwnode_handle *fwnode,
 				      size_t nval);
 int fwnode_property_read_string(struct fwnode_handle *fwnode,
 				const char *propname, const char **val);
+int fwnode_property_match_string(struct fwnode_handle *fwnode,
+				 const char *propname, const char *string);
 
 struct fwnode_handle *device_get_next_child_node(struct device *dev,
 						 struct fwnode_handle *child);
-- 
2.5.1

[PATCH 2/2] acpi-dma: Add support for "dma-names" device property

[Mika Westerberg]

The current implementation hard codes the two supported channels so that
"tx" is always 0 and "rx" is always 1. This is because there has been no
suitable way in ACPI to name resources.

With _DSD device properties we can finally do this:

	Device (SPI1) {
	    Name (_CRS, ResourceTemplate () {
	        ...
	        FixedDMA (0x0000, 0x0000, Width32bit)
	        FixedDMA (0x0001, 0x0001, Width32bit)
	    })

	    Name (_DSD, Package () {
	        ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
	        Package () {
	            Package () {"dma-names", Package () {"tx", "rx"}}
	        },
	    })
	}

The names "tx" and "rx" now provide index of the FixedDMA resource in
question.

Modify acpi_dma_request_slave_chan_by_name() so that it looks for
"dma-names" property first and only then fall back using hardcoded indices.

The DT "dma-names" binding that we reuse for ACPI is documented in
Documentation/devicetree/bindings/dma/dma.txt.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/dma/acpi-dma.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/acpi-dma.c b/drivers/dma/acpi-dma.c
index 5a635646e05c..981a38fc4cb8 100644
--- a/drivers/dma/acpi-dma.c
+++ b/drivers/dma/acpi-dma.c
@@ -21,6 +21,7 @@
 #include <linux/ioport.h>
 #include <linux/acpi.h>
 #include <linux/acpi_dma.h>
+#include <linux/property.h>
 
 static LIST_HEAD(acpi_dma_list);
 static DEFINE_MUTEX(acpi_dma_lock);
@@ -413,21 +414,29 @@ EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_index);
  * translate the names "tx" and "rx" here based on the most common case where
  * the first FixedDMA descriptor is TX and second is RX.
  *
+ * If the device has "dma-names" property the FixedDMA descriptor indices
+ * are retrieved based on those. Otherwise the function falls back using
+ * hardcoded indices.
+ *
  * Return:
  * Pointer to appropriate dma channel on success or an error pointer.
  */
 struct dma_chan *acpi_dma_request_slave_chan_by_name(struct device *dev,
 		const char *name)
 {
-	size_t index;
-
-	if (!strcmp(name, "tx"))
-		index = 0;
-	else if (!strcmp(name, "rx"))
-		index = 1;
-	else
-		return ERR_PTR(-ENODEV);
+	int index;
+
+	index = device_property_match_string(dev, "dma-names", name);
+	if (index < 0) {
+		if (!strcmp(name, "tx"))
+			index = 0;
+		else if (!strcmp(name, "rx"))
+			index = 1;
+		else
+			return ERR_PTR(-ENODEV);
+	}
 
+	dev_dbg(dev, "found DMA channel \"%s\" at index %d\n", name, index);
 	return acpi_dma_request_slave_chan_by_index(dev, index);
 }
 EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_name);
-- 
2.5.1

Re: [PATCH 2/2] acpi-dma: Add support for "dma-names" device property

[Rafael J. Wysocki]

On Monday, September 14, 2015 05:37:36 PM Mika Westerberg wrote:
> The current implementation hard codes the two supported channels so that
> "tx" is always 0 and "rx" is always 1. This is because there has been no
> suitable way in ACPI to name resources.
> 
> With _DSD device properties we can finally do this:
> 
> 	Device (SPI1) {
> 	    Name (_CRS, ResourceTemplate () {
> 	        ...
> 	        FixedDMA (0x0000, 0x0000, Width32bit)
> 	        FixedDMA (0x0001, 0x0001, Width32bit)
> 	    })
> 
> 	    Name (_DSD, Package () {
> 	        ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> 	        Package () {
> 	            Package () {"dma-names", Package () {"tx", "rx"}}
> 	        },
> 	    })
> 	}
> 
> The names "tx" and "rx" now provide index of the FixedDMA resource in
> question.
> 
> Modify acpi_dma_request_slave_chan_by_name() so that it looks for
> "dma-names" property first and only then fall back using hardcoded indices.
> 
> The DT "dma-names" binding that we reuse for ACPI is documented in
> Documentation/devicetree/bindings/dma/dma.txt.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

This and the [1/2] both look reasonable to me, but I need an ACK from Vinod
for this one.

> ---
>  drivers/dma/acpi-dma.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma/acpi-dma.c b/drivers/dma/acpi-dma.c
> index 5a635646e05c..981a38fc4cb8 100644
> --- a/drivers/dma/acpi-dma.c
> +++ b/drivers/dma/acpi-dma.c
> @@ -21,6 +21,7 @@
>  #include <linux/ioport.h>
>  #include <linux/acpi.h>
>  #include <linux/acpi_dma.h>
> +#include <linux/property.h>
>  
>  static LIST_HEAD(acpi_dma_list);
>  static DEFINE_MUTEX(acpi_dma_lock);
> @@ -413,21 +414,29 @@ EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_index);
>   * translate the names "tx" and "rx" here based on the most common case where
>   * the first FixedDMA descriptor is TX and second is RX.
>   *
> + * If the device has "dma-names" property the FixedDMA descriptor indices
> + * are retrieved based on those. Otherwise the function falls back using
> + * hardcoded indices.
> + *
>   * Return:
>   * Pointer to appropriate dma channel on success or an error pointer.
>   */
>  struct dma_chan *acpi_dma_request_slave_chan_by_name(struct device *dev,
>  		const char *name)
>  {
> -	size_t index;
> -
> -	if (!strcmp(name, "tx"))
> -		index = 0;
> -	else if (!strcmp(name, "rx"))
> -		index = 1;
> -	else
> -		return ERR_PTR(-ENODEV);
> +	int index;
> +
> +	index = device_property_match_string(dev, "dma-names", name);
> +	if (index < 0) {
> +		if (!strcmp(name, "tx"))
> +			index = 0;
> +		else if (!strcmp(name, "rx"))
> +			index = 1;
> +		else
> +			return ERR_PTR(-ENODEV);
> +	}
>  
> +	dev_dbg(dev, "found DMA channel \"%s\" at index %d\n", name, index);
>  	return acpi_dma_request_slave_chan_by_index(dev, index);
>  }
>  EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_name);
> 

Thanks,
Rafael

Re: [PATCH 2/2] acpi-dma: Add support for "dma-names" device property

[Andy Shevchenko]

On Mon, 2015-09-14 at 17:37 +0300, Mika Westerberg wrote:
> The current implementation hard codes the two supported channels so 
> that
> "tx" is always 0 and "rx" is always 1. This is because there has been 
> no
> suitable way in ACPI to name resources.
> 
> With _DSD device properties we can finally do this:
> 
> 	Device (SPI1) {
> 	    Name (_CRS, ResourceTemplate () {
> 	        ...
> 	        FixedDMA (0x0000, 0x0000, Width32bit)
> 	        FixedDMA (0x0001, 0x0001, Width32bit)
> 	    })
> 
> 	    Name (_DSD, Package () {
> 	        ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> 	        Package () {
> 	            Package () {"dma-names", Package () {"tx", 
> "rx"}}
> 	        },
> 	    })
> 	}
> 
> The names "tx" and "rx" now provide index of the FixedDMA resource in
> question.
> 
> Modify acpi_dma_request_slave_chan_by_name() so that it looks for
> "dma-names" property first and only then fall back using hardcoded 
> indices.
> 
> The DT "dma-names" binding that we reuse for ACPI is documented in
> Documentation/devicetree/bindings/dma/dma.txt.
> 

For both
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/dma/acpi-dma.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma/acpi-dma.c b/drivers/dma/acpi-dma.c
> index 5a635646e05c..981a38fc4cb8 100644
> --- a/drivers/dma/acpi-dma.c
> +++ b/drivers/dma/acpi-dma.c
> @@ -21,6 +21,7 @@
>  #include <linux/ioport.h>
>  #include <linux/acpi.h>
>  #include <linux/acpi_dma.h>
> +#include <linux/property.h>
>  
>  static LIST_HEAD(acpi_dma_list);
>  static DEFINE_MUTEX(acpi_dma_lock);
> @@ -413,21 +414,29 @@ 
> EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_index);
>   * translate the names "tx" and "rx" here based on the most common 
> case where
>   * the first FixedDMA descriptor is TX and second is RX.
>   *
> + * If the device has "dma-names" property the FixedDMA descriptor 
> indices
> + * are retrieved based on those. Otherwise the function falls back 
> using
> + * hardcoded indices.
> + *
>   * Return:
>   * Pointer to appropriate dma channel on success or an error 
> pointer.
>   */
>  struct dma_chan *acpi_dma_request_slave_chan_by_name(struct device 
> *dev,
>  		const char *name)
>  {
> -	size_t index;
> -
> -	if (!strcmp(name, "tx"))
> -		index = 0;
> -	else if (!strcmp(name, "rx"))
> -		index = 1;
> -	else
> -		return ERR_PTR(-ENODEV);
> +	int index;
> +
> +	index = device_property_match_string(dev, "dma-names", 
> name);
> +	if (index < 0) {
> +		if (!strcmp(name, "tx"))
> +			index = 0;
> +		else if (!strcmp(name, "rx"))
> +			index = 1;
> +		else
> +			return ERR_PTR(-ENODEV);
> +	}
>  
> +	dev_dbg(dev, "found DMA channel \"%s\" at index %d\n", name, 
> index);
>  	return acpi_dma_request_slave_chan_by_index(dev, index);
>  }
>  EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_name);

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH 2/2] acpi-dma: Add support for "dma-names" device property

[Mika Westerberg]

On Tue, Sep 15, 2015 at 01:19:59AM +0200, Rafael J. Wysocki wrote:
> On Monday, September 14, 2015 05:37:36 PM Mika Westerberg wrote:
> > The current implementation hard codes the two supported channels so that
> > "tx" is always 0 and "rx" is always 1. This is because there has been no
> > suitable way in ACPI to name resources.
> > 
> > With _DSD device properties we can finally do this:
> > 
> > 	Device (SPI1) {
> > 	    Name (_CRS, ResourceTemplate () {
> > 	        ...
> > 	        FixedDMA (0x0000, 0x0000, Width32bit)
> > 	        FixedDMA (0x0001, 0x0001, Width32bit)
> > 	    })
> > 
> > 	    Name (_DSD, Package () {
> > 	        ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > 	        Package () {
> > 	            Package () {"dma-names", Package () {"tx", "rx"}}
> > 	        },
> > 	    })
> > 	}
> > 
> > The names "tx" and "rx" now provide index of the FixedDMA resource in
> > question.
> > 
> > Modify acpi_dma_request_slave_chan_by_name() so that it looks for
> > "dma-names" property first and only then fall back using hardcoded indices.
> > 
> > The DT "dma-names" binding that we reuse for ACPI is documented in
> > Documentation/devicetree/bindings/dma/dma.txt.
> > 
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> This and the [1/2] both look reasonable to me, but I need an ACK from Vinod
> for this one.

Vinod, any comments?

> 
> > ---
> >  drivers/dma/acpi-dma.c | 25 +++++++++++++++++--------
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/dma/acpi-dma.c b/drivers/dma/acpi-dma.c
> > index 5a635646e05c..981a38fc4cb8 100644
> > --- a/drivers/dma/acpi-dma.c
> > +++ b/drivers/dma/acpi-dma.c
> > @@ -21,6 +21,7 @@
> >  #include <linux/ioport.h>
> >  #include <linux/acpi.h>
> >  #include <linux/acpi_dma.h>
> > +#include <linux/property.h>
> >  
> >  static LIST_HEAD(acpi_dma_list);
> >  static DEFINE_MUTEX(acpi_dma_lock);
> > @@ -413,21 +414,29 @@ EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_index);
> >   * translate the names "tx" and "rx" here based on the most common case where
> >   * the first FixedDMA descriptor is TX and second is RX.
> >   *
> > + * If the device has "dma-names" property the FixedDMA descriptor indices
> > + * are retrieved based on those. Otherwise the function falls back using
> > + * hardcoded indices.
> > + *
> >   * Return:
> >   * Pointer to appropriate dma channel on success or an error pointer.
> >   */
> >  struct dma_chan *acpi_dma_request_slave_chan_by_name(struct device *dev,
> >  		const char *name)
> >  {
> > -	size_t index;
> > -
> > -	if (!strcmp(name, "tx"))
> > -		index = 0;
> > -	else if (!strcmp(name, "rx"))
> > -		index = 1;
> > -	else
> > -		return ERR_PTR(-ENODEV);
> > +	int index;
> > +
> > +	index = device_property_match_string(dev, "dma-names", name);
> > +	if (index < 0) {
> > +		if (!strcmp(name, "tx"))
> > +			index = 0;
> > +		else if (!strcmp(name, "rx"))
> > +			index = 1;
> > +		else
> > +			return ERR_PTR(-ENODEV);
> > +	}
> >  
> > +	dev_dbg(dev, "found DMA channel \"%s\" at index %d\n", name, index);
> >  	return acpi_dma_request_slave_chan_by_index(dev, index);
> >  }
> >  EXPORT_SYMBOL_GPL(acpi_dma_request_slave_chan_by_name);
> > 
> 
> Thanks,
> Rafael

Re: [PATCH 2/2] acpi-dma: Add support for "dma-names" device property

[Vinod Koul]

On Mon, Sep 14, 2015 at 05:37:36PM +0300, Mika Westerberg wrote:
> The current implementation hard codes the two supported channels so that
> "tx" is always 0 and "rx" is always 1. This is because there has been no
> suitable way in ACPI to name resources.
> 
> With _DSD device properties we can finally do this:
> 
> 	Device (SPI1) {
> 	    Name (_CRS, ResourceTemplate () {
> 	        ...
> 	        FixedDMA (0x0000, 0x0000, Width32bit)
> 	        FixedDMA (0x0001, 0x0001, Width32bit)
> 	    })
> 
> 	    Name (_DSD, Package () {
> 	        ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> 	        Package () {
> 	            Package () {"dma-names", Package () {"tx", "rx"}}
> 	        },
> 	    })
> 	}
> 
> The names "tx" and "rx" now provide index of the FixedDMA resource in
> question.
> 
> Modify acpi_dma_request_slave_chan_by_name() so that it looks for
> "dma-names" property first and only then fall back using hardcoded indices.
> 
> The DT "dma-names" binding that we reuse for ACPI is documented in
> Documentation/devicetree/bindings/dma/dma.txt.

Acked-by: Vinod Koul <vinod.koul@intel.com>

This is actually good and will help a lot. Btw would like to see some
tested-by tags, perhaps on dw dma or idma...

-- 
~Vinod

Re: [PATCH 2/2] acpi-dma: Add support for "dma-names" device property

[Rafael J. Wysocki]

On Thursday, September 24, 2015 09:05:28 PM Vinod Koul wrote:
> On Mon, Sep 14, 2015 at 05:37:36PM +0300, Mika Westerberg wrote:
> > The current implementation hard codes the two supported channels so that
> > "tx" is always 0 and "rx" is always 1. This is because there has been no
> > suitable way in ACPI to name resources.
> > 
> > With _DSD device properties we can finally do this:
> > 
> > 	Device (SPI1) {
> > 	    Name (_CRS, ResourceTemplate () {
> > 	        ...
> > 	        FixedDMA (0x0000, 0x0000, Width32bit)
> > 	        FixedDMA (0x0001, 0x0001, Width32bit)
> > 	    })
> > 
> > 	    Name (_DSD, Package () {
> > 	        ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > 	        Package () {
> > 	            Package () {"dma-names", Package () {"tx", "rx"}}
> > 	        },
> > 	    })
> > 	}
> > 
> > The names "tx" and "rx" now provide index of the FixedDMA resource in
> > question.
> > 
> > Modify acpi_dma_request_slave_chan_by_name() so that it looks for
> > "dma-names" property first and only then fall back using hardcoded indices.
> > 
> > The DT "dma-names" binding that we reuse for ACPI is documented in
> > Documentation/devicetree/bindings/dma/dma.txt.
> 
> Acked-by: Vinod Koul <vinod.koul@intel.com>
> 
> This is actually good and will help a lot.

Thanks!

Added [1/2] and this one to my device-properties branch.

Thanks,
Rafael