Support ROHM BD79124 ADC

There are a few use-cases where child nodes with a specific name need to be parsed. Code like:

fwnode_for_each_child_node()

	if (fwnode_name_eq())
		...

can be found from a various drivers/subsystems. Adding a macro for this can simplify things a bit.

In a few cases the data from the found nodes is later added to an array, which is allocated based on the number of found nodes. One example of such use is the IIO subsystem's ADC channel nodes, where the relevant nodes are named as channel[@N].

Add helpers for iterating and counting device's sub-nodes with certain name instead of open-coding this in every user.

Suggested-by Jonathan Cameron <[email protected]>

Signed-off-by Matti Vaittinen <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

Reviewed-by Marcelo Schmitt <[email protected]>

---
Revision history:
v7 => v8:
- Fix the example in fwnode_get_named_child_node_count() documentation to use the fwnode_get_named_child_node_count() and not the device_get_named_child_node_count()
- Fix the rest of the new macro's indentiations
v6 => v7:
- Improve kerneldoc
- Inline device_get_named_child_node_count() and change it to call fwnode_get_named_child_node_count() inside
- Fix indentiation of the new macros
v5 => v6:
- Add helpers to also iterate through the nodes.
v4 => v5:
- Use given name instead of string 'channel' when counting the nodes
- Add also fwnode_get_child_node_count_named() as suggested by Rob.
v3 => v4:
- New patch as suggested by Jonathan, see discussion in:
https://lore.kernel.org/lkml/20250223161338.5c896280@jic23-huawei/

---
 drivers/base/property.c  | 27 +++++++++++++++++++++++++++
 include/linux/property.h | 24 ++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

drivers/base/property.c +27 -0

index c1392743df9c..f42f32ff45fc 100644
@@ -945,6 +945,33 @@ unsigned int device_get_child_node_count(const struct device *dev)
 }
 EXPORT_SYMBOL_GPL(device_get_child_node_count);
 
+/**
+ * fwnode_get_named_child_node_count - number of child nodes with given name
+ * @fwnode: Node which child nodes are counted.
+ * @name: String to match child node name against.
+ *
+ * Scan child nodes and count all the nodes with a specific name. Potential
+ * 'number' -ending after the 'at sign' for scanned names is ignored.
+ * E.g.::
+ *   fwnode_get_named_child_node_count(fwnode, "channel");
+ * would match all the nodes::
+ *   channel { }, channel@0 {}, channel@0xabba {}...
+ *
+ * Return: the number of child nodes with a matching name for a given device.
+ */
+unsigned int fwnode_get_named_child_node_count(const struct fwnode_handle *fwnode,
+					       const char *name)
+{
+	struct fwnode_handle *child;
+	unsigned int count = 0;
+
+	fwnode_for_each_named_child_node(fwnode, child, name)
+		count++;
+
+	return count;
+}
+EXPORT_SYMBOL_GPL(fwnode_get_named_child_node_count);
+
 bool device_dma_supported(const struct device *dev)
 {
 	return fwnode_call_bool_op(dev_fwnode(dev), device_dma_supported);

include/linux/property.h +2 -0

index e214ecd241eb..a1856e6b714c 100644
@@ -167,10 +167,18 @@ struct fwnode_handle *fwnode_get_next_available_child_node(
 	for (child = fwnode_get_next_child_node(fwnode, NULL); child;	\
 	     child = fwnode_get_next_child_node(fwnode, child))
 
+#define fwnode_for_each_named_child_node(fwnode, child, name)		\
+	fwnode_for_each_child_node(fwnode, child)			\

include/linux/property.h +16 -0

 struct fwnode_handle *device_get_next_child_node(const struct device *dev,
 						 struct fwnode_handle *child);
 

@@ -178,11 +186,19 @@ struct fwnode_handle *device_get_next_child_node(const struct device *dev,
 	for (child = device_get_next_child_node(dev, NULL); child;	\
 	     child = device_get_next_child_node(dev, child))
 
+#define device_for_each_named_child_node(dev, child, name)		\
+	device_for_each_child_node(dev, child)				\
+		if (!fwnode_name_eq(child, name)) { } else
+
 #define device_for_each_child_node_scoped(dev, child)			\
 	for (struct fwnode_handle *child __free(fwnode_handle) =	\
 		device_get_next_child_node(dev, NULL);			\
 	     child; child = device_get_next_child_node(dev, child))
 
+#define device_for_each_named_child_node_scoped(dev, child, name)	\
+	device_for_each_child_node_scoped(dev, child)			\
+		if (!fwnode_name_eq(child, name)) { } else
+
 struct fwnode_handle *fwnode_get_named_child_node(const struct fwnode_handle *fwnode,
 						  const char *childname);
 struct fwnode_handle *device_get_named_child_node(const struct device *dev,

@@ -210,6 +226,14 @@ int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name);
 
 unsigned int device_get_child_node_count(const struct device *dev);
 
+unsigned int fwnode_get_named_child_node_count(const struct fwnode_handle *fwnode,
+					       const char *name);
+static inline unsigned int device_get_named_child_node_count(const struct device *dev,
+							     const char *name)
+{
+	return fwnode_get_named_child_node_count(dev_fwnode(dev), name);
+}
+
 static inline int device_property_read_u8(const struct device *dev,
 					  const char *propname, u8 *val)
 {

Moi,

Quoted 101 lines · Matti Vaittinen

There are a few use-cases where child nodes with a specific name need to be parsed. Code like:

fwnode_for_each_child_node()

	if (fwnode_name_eq())
		...

can be found from a various drivers/subsystems. Adding a macro for this can simplify things a bit.

In a few cases the data from the found nodes is later added to an array, which is allocated based on the number of found nodes. One example of such use is the IIO subsystem's ADC channel nodes, where the relevant nodes are named as channel[@N].

Add helpers for iterating and counting device's sub-nodes with certain name instead of open-coding this in every user.

Suggested-by Jonathan Cameron <[email protected]>

Signed-off-by Matti Vaittinen <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

Reviewed-by Marcelo Schmitt <[email protected]>

---
Revision history:
v7 => v8:
- Fix the example in fwnode_get_named_child_node_count() documentation to use the fwnode_get_named_child_node_count() and not the device_get_named_child_node_count()
- Fix the rest of the new macro's indentiations
v6 => v7:
- Improve kerneldoc
- Inline device_get_named_child_node_count() and change it to call fwnode_get_named_child_node_count() inside
- Fix indentiation of the new macros
v5 => v6:
- Add helpers to also iterate through the nodes.
v4 => v5:
- Use given name instead of string 'channel' when counting the nodes
- Add also fwnode_get_child_node_count_named() as suggested by Rob.
v3 => v4:
- New patch as suggested by Jonathan, see discussion in:
https://lore.kernel.org/lkml/20250223161338.5c896280@jic23-huawei/

---
  drivers/base/property.c  | 27 +++++++++++++++++++++++++++
  include/linux/property.h | 24 ++++++++++++++++++++++++
  2 files changed, 51 insertions(+)

drivers/base/property.c +27 -0

index c1392743df9c..f42f32ff45fc 100644
@@ -945,6 +945,33 @@ unsigned int device_get_child_node_count(const struct device *dev)
  }
  EXPORT_SYMBOL_GPL(device_get_child_node_count);
 
+/**
+ * fwnode_get_named_child_node_count - number of child nodes with given name
+ * @fwnode: Node which child nodes are counted.
+ * @name: String to match child node name against.
+ *
+ * Scan child nodes and count all the nodes with a specific name. Potential
+ * 'number' -ending after the 'at sign' for scanned names is ignored.
+ * E.g.::
+ *   fwnode_get_named_child_node_count(fwnode, "channel");
+ * would match all the nodes::
+ *   channel { }, channel@0 {}, channel@0xabba {}...
+ *
+ * Return: the number of child nodes with a matching name for a given device.
+ */
+unsigned int fwnode_get_named_child_node_count(const struct fwnode_handle *fwnode,
+					       const char *name)
+{
+	struct fwnode_handle *child;
+	unsigned int count = 0;
+
+	fwnode_for_each_named_child_node(fwnode, child, name)
+		count++;
+
+	return count;
+}
+EXPORT_SYMBOL_GPL(fwnode_get_named_child_node_count);
+
  bool device_dma_supported(const struct device *dev)
  {
  	return fwnode_call_bool_op(dev_fwnode(dev), device_dma_supported);

include/linux/property.h +4 -0

index e214ecd241eb..a1856e6b714c 100644
@@ -167,10 +167,18 @@ struct fwnode_handle *fwnode_get_next_available_child_node(
  	for (child = fwnode_get_next_child_node(fwnode, NULL); child;	\
  	     child = fwnode_get_next_child_node(fwnode, child))
 
+#define fwnode_for_each_named_child_node(fwnode, child, name)		\
+	fwnode_for_each_child_node(fwnode, child)			\
+		if (!fwnode_name_eq(child, name)) { } else
+
  #define fwnode_for_each_available_child_node(fwnode, child)		       \
  	for (child = fwnode_get_next_available_child_node(fwnode, NULL); child;\
  	     child = fwnode_get_next_available_child_node(fwnode, child))
 

include/linux/property.h struct fwnode_handle *fwnode_get_next_available_child_node( · line 181

Quoted 4 lines

include/linux/property.h +4 -0

+#define fwnode_for_each_available_named_child_node(fwnode, child, name)	\
+	fwnode_for_each_available_child_node(fwnode, child)		\
+		if (!fwnode_name_eq(child, name)) { } else
+

OF only enumerates available nodes via the fwnode API, software nodes don't have the concept but on ACPI I guess you could have a difference in nodes where you have device sub-nodes that aren't available. Still, these ACPI device nodes don't have meaningful names in this context (they're 4-character object names) so you wouldn't use them like this anyway.

So my question is: is it useful to provide this besides fwnode_for_each_named_child_node(), given that both are effectively the same?

Moro Sakari,

Thanks for the review.

Quoted 103 lines · Sakari Ailus

Moi,

Quoted 100 lines · Matti Vaittinen

There are a few use-cases where child nodes with a specific name need to be parsed. Code like:

fwnode_for_each_child_node()

	if (fwnode_name_eq())
		...

can be found from a various drivers/subsystems. Adding a macro for this can simplify things a bit.

In a few cases the data from the found nodes is later added to an array, which is allocated based on the number of found nodes. One example of such use is the IIO subsystem's ADC channel nodes, where the relevant nodes are named as channel[@N].

Add helpers for iterating and counting device's sub-nodes with certain name instead of open-coding this in every user.

Suggested-by Jonathan Cameron <[email protected]>

Signed-off-by Matti Vaittinen <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

Reviewed-by Marcelo Schmitt <[email protected]>

---
Revision history:
v7 => v8:
- Fix the example in fwnode_get_named_child_node_count() documentation to use the fwnode_get_named_child_node_count() and not the device_get_named_child_node_count()
- Fix the rest of the new macro's indentiations
v6 => v7:
- Improve kerneldoc
- Inline device_get_named_child_node_count() and change it to call fwnode_get_named_child_node_count() inside
- Fix indentiation of the new macros
v5 => v6:
- Add helpers to also iterate through the nodes.
v4 => v5:
- Use given name instead of string 'channel' when counting the nodes
- Add also fwnode_get_child_node_count_named() as suggested by Rob.
v3 => v4:
- New patch as suggested by Jonathan, see discussion in:
https://lore.kernel.org/lkml/20250223161338.5c896280@jic23-huawei/

---
  drivers/base/property.c  | 27 +++++++++++++++++++++++++++
  include/linux/property.h | 24 ++++++++++++++++++++++++
  2 files changed, 51 insertions(+)

drivers/base/property.c +27 -0

index c1392743df9c..f42f32ff45fc 100644
@@ -945,6 +945,33 @@ unsigned int device_get_child_node_count(const struct device *dev)
  }
  EXPORT_SYMBOL_GPL(device_get_child_node_count);
+/**
+ * fwnode_get_named_child_node_count - number of child nodes with given name
+ * @fwnode: Node which child nodes are counted.
+ * @name: String to match child node name against.
+ *
+ * Scan child nodes and count all the nodes with a specific name. Potential
+ * 'number' -ending after the 'at sign' for scanned names is ignored.
+ * E.g.::
+ *   fwnode_get_named_child_node_count(fwnode, "channel");
+ * would match all the nodes::
+ *   channel { }, channel@0 {}, channel@0xabba {}...
+ *
+ * Return: the number of child nodes with a matching name for a given device.
+ */
+unsigned int fwnode_get_named_child_node_count(const struct fwnode_handle *fwnode,
+					       const char *name)
+{
+	struct fwnode_handle *child;
+	unsigned int count = 0;
+
+	fwnode_for_each_named_child_node(fwnode, child, name)
+		count++;
+
+	return count;
+}
+EXPORT_SYMBOL_GPL(fwnode_get_named_child_node_count);
+
  bool device_dma_supported(const struct device *dev)
  {
  	return fwnode_call_bool_op(dev_fwnode(dev), device_dma_supported);

include/linux/property.h +6 -0

index e214ecd241eb..a1856e6b714c 100644
@@ -167,10 +167,18 @@ struct fwnode_handle *fwnode_get_next_available_child_node(
  	for (child = fwnode_get_next_child_node(fwnode, NULL); child;	\
  	     child = fwnode_get_next_child_node(fwnode, child))
+#define fwnode_for_each_named_child_node(fwnode, child, name)		\
+	fwnode_for_each_child_node(fwnode, child)			\
+		if (!fwnode_name_eq(child, name)) { } else
+
  #define fwnode_for_each_available_child_node(fwnode, child)		       \
  	for (child = fwnode_get_next_available_child_node(fwnode, NULL); child;\
  	     child = fwnode_get_next_available_child_node(fwnode, child))
+#define fwnode_for_each_available_named_child_node(fwnode, child, name)	\
+	fwnode_for_each_available_child_node(fwnode, child)		\

Quoted 8 lines

include/linux/property.h struct fwnode_handle *fwnode_get_next_available_child_node( · line 179

Quoted 2 lines

include/linux/property.h +2 -0

+		if (!fwnode_name_eq(child, name)) { } else
+

OF only enumerates available nodes via the fwnode API, software nodes don't have the concept but on ACPI I guess you could have a difference in nodes where you have device sub-nodes that aren't available. Still, these ACPI device nodes don't have meaningful names in this context (they're 4-character object names) so you wouldn't use them like this anyway.

I believe you have far better understanding on these concepts than I do. The reason behind adding fwnode_for_each_available_child_node() was the patch 10/10:

(quoted diff) +3 -5

-	fwnode_for_each_available_child_node(sensors, node) {
-		if (fwnode_name_eq(node, "sensor")) {
-			if (!thp7312_sensor_parse_dt(thp7312, node))
-				num_sensors++;
-		}
+	fwnode_for_each_available_named_child_node(sensors, node, "sensor") {
+		if (!thp7312_sensor_parse_dt(thp7312, node))
+			num_sensors++;
 	}

Quoted 5 lines

So my question is: is it useful to provide this besides fwnode_for_each_named_child_node(), given that both are effectively the same?

So, I suppose you're saying the existing thp7312 -driver has no real reason to use the 'fwnode_for_each_available_child_node()', but it could be using fwnode_for_each_child_node() instead?

If so, I am Ok with dropping the 'fwnode_for_each_available_named_child_node()' and changing the 10/10 to:

(quoted diff) +3 -5

-	fwnode_for_each_available_child_node(sensors, node) {
-		if (fwnode_name_eq(node, "sensor")) {
-			if (!thp7312_sensor_parse_dt(thp7312, node))
-				num_sensors++;
-		}
+	fwnode_for_each_named_child_node(sensors, node, "sensor") {
+		if (!thp7312_sensor_parse_dt(thp7312, node))
+			num_sensors++;
 	}

Do you think that'd be correct?

Quoted 4 lines · Matti Vaittinen

Quoted 3 lines · Sakari Ailus

Quoted 2 lines · Matti Vaittinen

There are a few use-cases where child nodes with a specific name need to be parsed. Code like:

Quoted 59 lines

Quoted 14 lines

Quoted 14 lines

include/linux/property.h +6 -0

@@ -167,10 +167,18 @@ struct fwnode_handle *fwnode_get_next_available_child_node(
   	for (child = fwnode_get_next_child_node(fwnode, NULL); child;	\
   	     child = fwnode_get_next_child_node(fwnode, child))
+#define fwnode_for_each_named_child_node(fwnode, child, name)		\
+	fwnode_for_each_child_node(fwnode, child)			\
+		if (!fwnode_name_eq(child, name)) { } else
+
   #define fwnode_for_each_available_child_node(fwnode, child)		       \
   	for (child = fwnode_get_next_available_child_node(fwnode, NULL); child;\
   	     child = fwnode_get_next_available_child_node(fwnode, child))
+#define fwnode_for_each_available_named_child_node(fwnode, child, name)	\
+	fwnode_for_each_available_child_node(fwnode, child)		\

Quoted 8 lines

include/linux/property.h struct fwnode_handle *fwnode_get_next_available_child_node( · line 179

Quoted 2 lines

include/linux/property.h +2 -0

+		if (!fwnode_name_eq(child, name)) { } else
+

OF only enumerates available nodes via the fwnode API, software nodes don't have the concept but on ACPI I guess you could have a difference in nodes where you have device sub-nodes that aren't available. Still, these ACPI device nodes don't have meaningful names in this context (they're 4-character object names) so you wouldn't use them like this anyway.

I believe you have far better understanding on these concepts than I do. The reason behind adding fwnode_for_each_available_child_node() was the patch 10/10:

(quoted diff) +3 -5

-	fwnode_for_each_available_child_node(sensors, node) {
-		if (fwnode_name_eq(node, "sensor")) {
-			if (!thp7312_sensor_parse_dt(thp7312, node))
-				num_sensors++;
-		}
+	fwnode_for_each_available_named_child_node(sensors, node, "sensor") {
+		if (!thp7312_sensor_parse_dt(thp7312, node))
+			num_sensors++;
  	}

Quoted 5 lines

So my question is: is it useful to provide this besides fwnode_for_each_named_child_node(), given that both are effectively the same?

So, I suppose you're saying the existing thp7312 -driver has no real reason to use the 'fwnode_for_each_available_child_node()', but it could be using fwnode_for_each_child_node() instead?

If so, I am Ok with dropping the 'fwnode_for_each_available_named_child_node()' and changing the 10/10 to:

(quoted diff) +3 -5

-	fwnode_for_each_available_child_node(sensors, node) {
-		if (fwnode_name_eq(node, "sensor")) {
-			if (!thp7312_sensor_parse_dt(thp7312, node))
-				num_sensors++;
-		}
+	fwnode_for_each_named_child_node(sensors, node, "sensor") {
+		if (!thp7312_sensor_parse_dt(thp7312, node))
+			num_sensors++;
  	}

Quoted 1 line

Do you think that'd be correct?

I'd say so. Feel free to cc me to the last patch as well.

I guess one way to make this clearer is to switch to fwnode_for_each_child_node() in a separate patch before fwnode_for_each_named_child_node() conversion.

There are also just a handful of users of fwnode_for_each_available_child_node() and I guess these could be converted, too, but I think it's outside the scope of the set.

Quoted 5 lines · Sakari Ailus

Quoted 4 lines · Matti Vaittinen

Quoted 3 lines · Sakari Ailus

Quoted 2 lines · Matti Vaittinen

There are a few use-cases where child nodes with a specific name need to be parsed. Code like:

...

Quoted 59 lines

Quoted 59 lines

Quoted 14 lines

Quoted 14 lines

include/linux/property.h +6 -0

@@ -167,10 +167,18 @@ struct fwnode_handle *fwnode_get_next_available_child_node(
   	for (child = fwnode_get_next_child_node(fwnode, NULL); child;	\
   	     child = fwnode_get_next_child_node(fwnode, child))
+#define fwnode_for_each_named_child_node(fwnode, child, name)		\
+	fwnode_for_each_child_node(fwnode, child)			\
+		if (!fwnode_name_eq(child, name)) { } else
+
   #define fwnode_for_each_available_child_node(fwnode, child)		       \
   	for (child = fwnode_get_next_available_child_node(fwnode, NULL); child;\
   	     child = fwnode_get_next_available_child_node(fwnode, child))
+#define fwnode_for_each_available_named_child_node(fwnode, child, name)	\
+	fwnode_for_each_available_child_node(fwnode, child)		\

Quoted 8 lines

include/linux/property.h struct fwnode_handle *fwnode_get_next_available_child_node( · line 179

Quoted 2 lines

include/linux/property.h +2 -0

+		if (!fwnode_name_eq(child, name)) { } else
+  

OF only enumerates available nodes via the fwnode API, software nodes don't have the concept but on ACPI I guess you could have a difference in nodes where you have device sub-nodes that aren't available. Still, these ACPI device nodes don't have meaningful names in this context (they're 4-character object names) so you wouldn't use them like this anyway.

I believe you have far better understanding on these concepts than I do. The reason behind adding fwnode_for_each_available_child_node() was the patch 10/10:

(quoted diff) +3 -5

-	fwnode_for_each_available_child_node(sensors, node) {
-		if (fwnode_name_eq(node, "sensor")) {
-			if (!thp7312_sensor_parse_dt(thp7312, node))
-				num_sensors++;
-		}
+	fwnode_for_each_available_named_child_node(sensors, node, "sensor") {
+		if (!thp7312_sensor_parse_dt(thp7312, node))
+			num_sensors++;
  	}

Quoted 5 lines

So my question is: is it useful to provide this besides fwnode_for_each_named_child_node(), given that both are effectively the same?

So, I suppose you're saying the existing thp7312 -driver has no real reason to use the 'fwnode_for_each_available_child_node()', but it could be using fwnode_for_each_child_node() instead?

If so, I am Ok with dropping the 'fwnode_for_each_available_named_child_node()' and changing the 10/10 to:

(quoted diff) +3 -5

-	fwnode_for_each_available_child_node(sensors, node) {
-		if (fwnode_name_eq(node, "sensor")) {
-			if (!thp7312_sensor_parse_dt(thp7312, node))
-				num_sensors++;
-		}
+	fwnode_for_each_named_child_node(sensors, node, "sensor") {
+		if (!thp7312_sensor_parse_dt(thp7312, node))
+			num_sensors++;
  	}

Quoted 3 lines

Quoted 1 line

Do you think that'd be correct?

I'd say so. Feel free to cc me to the last patch as well.

Thanks. I'll drop the fwnode_for_each_available_named_child_node() then.

Quoted 3 lines

I guess one way to make this clearer is to switch to fwnode_for_each_child_node() in a separate patch before fwnode_for_each_named_child_node() conversion.

I suppose this makes sense.

I think this series can't make it to 6.15-rc1. Meaning, these *_named_*() APIs perhaps land in 6.16-rc1. I assume these *_named_*() APIs will go through the IIO. This rather simple IIO driver's review took longer than I predicted, with more versions I intended (as always)
- and I kind of dislike respinning the whole series, with this large audience, when changes are not interesting to the most.

Maybe it is simplest to drop the thp7312 (and gianfar) from this series, and respin them only when the 6.16-rc1 is out. It's going to be couple of months though - so there's always a risk that I forget.

The proposed change for the thp7312, from fwnode_for_each_available_child_node() to fwnode_for_each_child_node() can be done earlier though.

Quoted 3 lines

There are also just a handful of users of fwnode_for_each_available_child_node() and I guess these could be converted, too, but I think it's outside the scope of the set.

Definitely not in the scope of the bd79124 support :)

Yours,
-- Matti

There are ADC ICs which may have some of the AIN pins usable for other functions. These ICs may have some of the AIN pins wired so that they should not be used for ADC.

(Preferred?) way for marking pins which can be used as ADC inputs is to add corresponding channels@N nodes in the device tree as described in the ADC binding yaml.

Add couple of helper functions which can be used to retrieve the channel information from the device node.

The new devm_iio_adc_device_alloc_chaninfo_se() -helper is intended to help drivers avoid open-coding the for_each_node -loop for getting the channel IDs. The helper provides standard way to detect the ADC channel nodes (by the node name), and a standard way to convert the "reg" -properties to channel identification numbers, used in the struct iio_chan_spec. Furthermore, the helper can optionally check the found channel IDs are smaller than given maximum. This is useful for callers which later use the IDs for example for indexing a channel data array.

The original driver treated all found child nodes as channel nodes. The new helper requires channel nodes to be named channel[@N]. This should help avoid problems with devices which may contain also other but ADC child nodes. Quick grep from arch/* with the rzg2l_adc's compatible string didn't reveal any in-tree .dts with channel nodes named otherwise. Also, same grep shows all the .dts seem to have channel IDs between 0..num of channels.

Use the new helper.

The new devm_iio_adc_device_alloc_chaninfo_se() -helper is intended to help drivers avoid open-coding the for_each_node -loop for getting the channel IDs. The helper provides standard way to detect the ADC channel nodes (by the node name), and a standard way to convert the "reg" -properties to channel identification numbers, used in the struct iio_chan_spec. Furthermore, the helper can optionally check the found channel IDs are smaller than given maximum. This is useful for callers which later use the IDs for example for indexing a channel data array.

The original driver treated all found child nodes as channel nodes. The new helper requires channel nodes to be named channel[@N]. This should help avoid problems with devices which may contain also other but ADC child nodes. Quick grep from arch/* with the sun20i-gpadc's compatible string didn't reveal any in-tree .dts with channel nodes named otherwise. Also, same grep shows all the in-tree .dts seem to have channel IDs between 0..num of channels.

Use the new helper.

The ROHM BD79124 is a 12-bit, 8-channel, SAR ADC. The ADC supports an automatic measurement mode, with an alarm interrupt for out-of-window measurements. The window is configurable for each channel.

The I2C protocol for manual start of the measurement and data reading is somewhat peculiar. It requires the master to do clock stretching after sending the I2C slave-address until the slave has captured the data. Needless to say this is not well suopported by the I2C controllers.

Thus do not support the BD79124's manual measurement mode but implement the measurements using automatic measurement mode, relying on the BD79124's ability of storing latest measurements into register.

Support also configuring the threshold events for detecting the out-of-window events.

The BD79124 keeps asserting IRQ for as long as the measured voltage is out of the configured window. Thus, prevent the user-space from choking on the events and mask the received event for a fixed duration (1 second) when an event is handled.

The ADC input pins can be also configured as general purpose outputs. Make those pins which don't have corresponding ADC channel node in the device-tree controllable as GPO.

+#include <linux/byteorder/generic.h>

drivers/iio/adc/rohm-bd79124.c new file +3 -0

+#include <linux/iio/events.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/adc-helpers.h>

drivers/iio/adc/rohm-bd79124.c new file +9 -0

+	/*
+	 * The BD79124 does not allow disabling/enabling limit separately for
+	 * one direction only. Hence, we do the disabling by changing the limit
+	 * to maximum/minimum measurable value. This means we need to cache
+	 * the limit in order to maintain it over the time limit is disabled.
+	 */
+	u16 alarm_r_limit[BD79124_MAX_NUM_CHANNELS];
+	u16 alarm_f_limit[BD79124_MAX_NUM_CHANNELS];
+	/* Bitmask of disabled events (for rate limiting) for each channel. */

drivers/iio/adc/rohm-bd79124.c new file +1 -0

+	u8 gpio_valid_mask;

+}

+}

drivers/iio/adc/rohm-bd79124.c new file +5 -0

+struct bd79124_raw {
+	u8 bit0_3; /* Is set in high bits of the byte */
+	u8 bit4_11;
+};
+#define BD79124_RAW_TO_INT(r) ((r.bit4_11 << 4) | (r.bit0_3 >> 4))

+ * The value bits [11:4] are stored in the next register.
+ *
+ * Read data from register and convert to integer.
+ */

drivers/iio/adc/rohm-bd79124.c new file +7 -0

+		return ret;
+	}
+
+	*val = BD79124_RAW_TO_INT(raw);
+
+	return 0;
+}

+/*
+ * The high and low limits as well as the recent result values are stored in
+ * the same way in 2 consequent registers. The first register contains 4 bits
+ * of the value. These bits are stored in the high bits [7:4] of register, but
+ * they represent the low bits [3:0] of the value.
+ * The value bits [11:4] are stored in the next register.

+ * Convert the integer to register format and write it using rmw cycle.
+ */

drivers/iio/adc/rohm-bd79124.c new file +12 -0

+static int bd79124_stop_measurement(struct bd79124_data *data, int chan)
+{
+	unsigned int val;
+	int ret;
+
+	/* See if already stopped */
+	ret = regmap_read(data->map, BD79124_REG_AUTO_CHANNELS, &val);
+	if (!(val & BIT(chan)))
+		return 0;
+
+	ret = regmap_clear_bits(data->map, BD79124_REG_SEQ_CFG,
+				BD79124_MSK_SEQ_START);

drivers/iio/adc/rohm-bd79124.c new file +12 -0

+		int regval = FIELD_PREP(BD79124_MSK_CONV_MODE,
+					BD79124_CONV_MODE_MANSEQ);
+
+		ret = regmap_update_bits(data->map, BD79124_REG_OPMODE_CFG,
+					 BD79124_MSK_CONV_MODE, regval);
+		if (ret)
+			return ret;
+	}
+
+	return regmap_set_bits(data->map, BD79124_REG_SEQ_CFG,
+			       BD79124_MSK_SEQ_START);
+}

(quoted diff) +2 -0

+	if (ret)
+		return ret;

drivers/iio/adc/rohm-bd79124.c new file +26 -0

+			data->alarm_f_limit[chan->channel] = val;
+			reg = BD79124_GET_LOW_LIMIT_REG(chan->channel);
+		} else {
+			return -EINVAL;
+		}
+		/*
+		 * We don't want to enable the alarm if it is not enabled or
+		 * if it is suppressed. In that case skip writing to the
+		 * register.
+		 */
+		if (!(data->alarm_monitored[chan->channel] & BIT(dir)) ||
+		    data->alarm_suppressed[chan->channel] & BIT(dir))
+			return 0;
+
+		return bd79124_write_int_to_reg(data, reg, val);
+
+	case IIO_EV_INFO_HYSTERESIS:
+		reg = BD79124_GET_HYSTERESIS_REG(chan->channel);
+		val >>= 3;
+
+		return regmap_update_bits(data->map, reg, BD79124_MSK_HYSTERESIS,
+					  val);
+	default:
+		return -EINVAL;
+	}
+}

drivers/iio/adc/rohm-bd79124.c new file +15 -0

+static int bd79124_read_raw(struct iio_dev *iio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long m)
+{
+	struct bd79124_data *data = iio_priv(iio_dev);
+	int ret;
+
+	if (chan->channel >= BD79124_MAX_NUM_CHANNELS)
+		return -EINVAL;
+
+	switch (m) {
+	case IIO_CHAN_INFO_RAW:
+	{
+		unsigned int old_chan_cfg, regval;
+		int tmp;

drivers/iio/adc/rohm-bd79124.c new file +36 -0

+		ret = regmap_update_bits(data->map, BD79124_REG_OPMODE_CFG,
+					 BD79124_MSK_CONV_MODE, regval);
+		if (ret)
+			return ret;
+
+		ret = bd79124_single_chan_seq(data, chan->channel, &old_chan_cfg);
+		if (ret)
+			return ret;
+
+		/* The maximum conversion time is 6 uS. */
+		udelay(6);
+
+		ret = bd79124_read_reg_to_int(data,
+			BD79124_GET_RECENT_RES_REG(chan->channel), val);
+		/*
+		 * Return the old chan config even if data reading failed in
+		 * order to re-enable the event monitoring.
+		 */
+		tmp = bd79124_single_chan_seq_end(data, old_chan_cfg);
+		if (tmp)
+			dev_err(data->dev,
+				"Failed to return config. Alarms may be disabled\n");
+
+		if (ret)
+			return ret;
+
+		return IIO_VAL_INT;
+	}
+	case IIO_CHAN_INFO_SCALE:
+		*val = data->vmax / 1000;
+		*val2 = BD79124_ADC_BITS;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}

drivers/iio/adc/rohm-bd79124.c new file +9 -0

+static int __bd79124_event_ratelimit(struct bd79124_data *data, int reg,
+				     unsigned int limit)
+{
+	int ret;
+
+	if (limit > BD79124_HIGH_LIMIT_MAX)
+		return -EINVAL;
+
+	ret = bd79124_write_int_to_reg(data, reg, limit);

drivers/iio/adc/rohm-bd79124.c new file +17 -0

+	return 0;
+}
 
+static int bd79124_get_gpio_pins(const struct iio_chan_spec *cs, int num_channels)
+{
+	int i, gpio_channels;
+
+	/*
+	 * Let's initialize the mux config to say that all 8 channels are
+	 * GPIOs. Then we can just loop through the iio_chan_spec and clear the
+	 * bits for found ADC channels.
+	 */
+	gpio_channels = GENMASK(7, 0);
+	for (i = 0; i < num_channels; i++)
+		gpio_channels &= ~BIT(cs[i].channel);
+
+	return gpio_channels;
+}

Quoted 24 lines · Matti Vaittinen

The ROHM BD79124 is a 12-bit, 8-channel, SAR ADC. The ADC supports an automatic measurement mode, with an alarm interrupt for out-of-window measurements. The window is configurable for each channel.

The I2C protocol for manual start of the measurement and data reading is somewhat peculiar. It requires the master to do clock stretching after sending the I2C slave-address until the slave has captured the data. Needless to say this is not well suopported by the I2C controllers.

Thus do not support the BD79124's manual measurement mode but implement the measurements using automatic measurement mode, relying on the BD79124's ability of storing latest measurements into register.

Support also configuring the threshold events for detecting the out-of-window events.

The BD79124 keeps asserting IRQ for as long as the measured voltage is out of the configured window. Thus, prevent the user-space from choking on the events and mask the received event for a fixed duration (1 second) when an event is handled.

The ADC input pins can be also configured as general purpose outputs. Make those pins which don't have corresponding ADC channel node in the device-tree controllable as GPO.

...

drivers/iio/adc/rohm-bd79124.c line 11

Quoted 3 lines

drivers/iio/adc/rohm-bd79124.c new file +3 -0

+#include <linux/bitfield.h>
+#include <linux/bitmap.h>
+#include <linux/bits.h>

bits.h is guaranteed by bitmap.h, but it's up to you to leave or remove this line.

Quoted 1 line

+#include <linux/byteorder/generic.h>

This is incorrect. In some cases it even may produce build failures. Should be asm/byteorder.h...

Quoted 6 lines

drivers/iio/adc/rohm-bd79124.c new file +6 -0

+#include <linux/device.h>
+#include <linux/delay.h>
+#include <linux/devm-helpers.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/i2c.h>

drivers/iio/adc/rohm-bd79124.c line 24

Quoted 6 lines

drivers/iio/adc/rohm-bd79124.c new file +6 -0

+#include <linux/interrupt.h>
+#include <linux/irqreturn.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/regmap.h>
+#include <linux/types.h>

...somewhere here.

Quoted 4 lines

drivers/iio/adc/rohm-bd79124.c new file +3 -0

+#include <linux/iio/events.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/adc-helpers.h>

...

drivers/iio/adc/rohm-bd79124.c line 84

Quoted 2 lines

drivers/iio/adc/rohm-bd79124.c new file +2 -0

+#define BD79124_GET_LIMIT_REG(ch, dir) ((dir) == IIO_EV_DIR_RISING ?		\
+		BD79124_GET_HIGH_LIMIT_REG(ch) : BD79124_GET_LOW_LIMIT_REG(ch))

It's hard to read, better to split over few lines:

#define BD79124_GET_LIMIT_REG(ch, dir)						\
	((dir) == IIO_EV_DIR_RISING ?						\
		BD79124_GET_HIGH_LIMIT_REG(ch) : BD79124_GET_LOW_LIMIT_REG(ch))

or alike.

...

Quoted 17 lines

drivers/iio/adc/rohm-bd79124.c new file +17 -0

+	/*
+	 * The BD79124 does not allow disabling/enabling limit separately for
+	 * one direction only. Hence, we do the disabling by changing the limit
+	 * to maximum/minimum measurable value. This means we need to cache
+	 * the limit in order to maintain it over the time limit is disabled.
+	 */
+	u16 alarm_r_limit[BD79124_MAX_NUM_CHANNELS];
+	u16 alarm_f_limit[BD79124_MAX_NUM_CHANNELS];
+	/* Bitmask of disabled events (for rate limiting) for each channel. */
+	int alarm_suppressed[BD79124_MAX_NUM_CHANNELS];
+	/*
+	 * The BD79124 is configured to run the measurements in the background.
+	 * This is done for the event monitoring as well as for the read_raw().
+	 * Protect the measurement starting/stopping using a mutex.
+	 */
+	struct mutex mutex;
+	struct delayed_work alm_enable_work;

drivers/iio/adc/rohm-bd79124.c line 122

Quoted 1 line

drivers/iio/adc/rohm-bd79124.c new file +1 -0

+	struct gpio_chip gc;

Hmm... Have you tried to shuffle fields to see if you gain a better code generation.

Also I don't remember if I asked about struct device *dev to be the same as in regmap and would it be worse code without it?

Quoted 1 line

drivers/iio/adc/rohm-bd79124.c new file +1 -0

+	u8 gpio_valid_mask;

...

drivers/iio/adc/rohm-bd79124.c line 204

Quoted 8 lines

drivers/iio/adc/rohm-bd79124.c new file +8 -0

+static void bd79124gpo_set(struct gpio_chip *gc, unsigned int offset, int value)
+{
+	struct bd79124_data *data = gpiochip_get_data(gc);
+
+	if (value)
+		regmap_set_bits(data->map, BD79124_REG_GPO_VAL, BIT(offset));
+	else
+		regmap_clear_bits(data->map, BD79124_REG_GPO_VAL, BIT(offset));

regmap_assign_bits()

Quoted 2 lines

+}

...

Quoted 6 lines

drivers/iio/adc/rohm-bd79124.c new file +6 -0

+static void bd79124gpo_set_multiple(struct gpio_chip *gc, unsigned long *mask,
+				    unsigned long *bits)
+{
+	unsigned int val;
+	int ret;
+	struct bd79124_data *data = gpiochip_get_data(gc);

drivers/iio/adc/rohm-bd79124.c line 214

Quoted 1 line

drivers/iio/adc/rohm-bd79124.c new file +1 -0

+	/* Ensure all GPIOs in 'mask' are set to be GPIOs */

This sounds like it can utilise valid_mask, but it seems you already have it.

So the question is what is the practical issue here? I believe the condition below will never be the case.

Quoted 3 lines

drivers/iio/adc/rohm-bd79124.c new file +3 -0

+	ret = regmap_read(data->map, BD79124_REG_PINCFG, &val);
+	if (ret)
+		return;

drivers/iio/adc/rohm-bd79124.c line 225

Quoted 7 lines

drivers/iio/adc/rohm-bd79124.c new file +7 -0

+	if ((val & *mask) != *mask) {
+		dev_dbg(data->dev, "Invalid mux config. Can't set value.\n");
+		/* Do not set value for pins configured as ADC inputs */
+		*mask &= val;
+	}
+
+	regmap_update_bits(data->map, BD79124_REG_GPO_VAL, *mask, *bits);

Can you rather utilise the respective bitmap APIs?

Quoted 2 lines

+}

...

Quoted 9 lines

drivers/iio/adc/rohm-bd79124.c new file +9 -0

+struct bd79124_raw {
+	u8 bit0_3; /* Is set in high bits of the byte */
+	u8 bit4_11;
+};
+#define BD79124_RAW_TO_INT(r) ((r.bit4_11 << 4) | (r.bit0_3 >> 4))
+#define BD79124_INT_TO_RAW(val) {					\
+	.bit4_11 = (val) >> 4,						\
+	.bit0_3 = (val) << 4,						\
+}

drivers/iio/adc/rohm-bd79124.c line 265

Quoted 6 lines

drivers/iio/adc/rohm-bd79124.c new file +6 -0

+
+/*
+ * The high and low limits as well as the recent result values are stored in
+ * the same way in 2 consequent registers. The first register contains 4 bits
+ * of the value. These bits are stored in the high bits [7:4] of register, but
+ * they represent the low bits [3:0] of the value.

I believe this comment should go on top of the data structure.

Also this is
still confusing as variable name above suggests one bit mapping, while text here is referencing to another. Can you elaborate this more, please?

Like

	u8 foo_1; // represents bits 3 2 1 0 x x x x
	// ? or represents bits 0 1 2 3 x x x x

Quoted 4 lines

+ * The value bits [11:4] are stored in the next register.
+ *
+ * Read data from register and convert to integer.
+ */

...

Quoted 8 lines

drivers/iio/adc/rohm-bd79124.c new file +8 -0

+static int bd79124_read_reg_to_int(struct bd79124_data *data, int reg,
+				   unsigned int *val)
+{
+	int ret;
+	struct bd79124_raw raw;
+
+	ret = regmap_bulk_read(data->map, reg, &raw, sizeof(raw));
+	if (ret) {

drivers/iio/adc/rohm-bd79124.c line 278

Quoted 1 line

drivers/iio/adc/rohm-bd79124.c new file +1 -0

+		dev_dbg(data->dev, "bulk_read failed %d\n", ret);

Do we need this (and alike)? It can be achieved via regmap tracepoints.

Quoted 7 lines

drivers/iio/adc/rohm-bd79124.c new file +7 -0

+		return ret;
+	}
+
+	*val = BD79124_RAW_TO_INT(raw);
+
+	return 0;
+}

...

Quoted 6 lines

+/*
+ * The high and low limits as well as the recent result values are stored in
+ * the same way in 2 consequent registers. The first register contains 4 bits
+ * of the value. These bits are stored in the high bits [7:4] of register, but
+ * they represent the low bits [3:0] of the value.
+ * The value bits [11:4] are stored in the next register.

Same as above, rather comment once at data type.

Quoted 3 lines

+ * Convert the integer to register format and write it using rmw cycle.
+ */

...

drivers/iio/adc/rohm-bd79124.c line 308

Quoted 1 line

drivers/iio/adc/rohm-bd79124.c new file +1 -0

+	raw.bit0_3 |= (0xf & tmp);

Btw, why Yoda style?

Quoted 18 lines

drivers/iio/adc/rohm-bd79124.c new file +18 -0

+static int bd79124_stop_measurement(struct bd79124_data *data, int chan)
+{
+	unsigned int val;
+	int ret;
+
+	/* See if already stopped */
+	ret = regmap_read(data->map, BD79124_REG_AUTO_CHANNELS, &val);
+	if (!(val & BIT(chan)))
+		return 0;
+
+	ret = regmap_clear_bits(data->map, BD79124_REG_SEQ_CFG,
+				BD79124_MSK_SEQ_START);
+
+	/* Clear the channel from the measured channels */
+	ret = regmap_write(data->map, BD79124_REG_AUTO_CHANNELS,
+			   ~BIT(chan) & val);
+	if (ret)
+		return ret;

drivers/iio/adc/rohm-bd79124.c line 454

Quoted 6 lines

drivers/iio/adc/rohm-bd79124.c new file +6 -0

+
+	/*
+	 * Stop background conversion for power saving if it was the last
+	 * channel
+	 */
+	if (!(~BIT(chan) & val)) {

Seems to me as you have the same above, perhaps just update the val?

Quoted 12 lines

drivers/iio/adc/rohm-bd79124.c new file +12 -0

+		int regval = FIELD_PREP(BD79124_MSK_CONV_MODE,
+					BD79124_CONV_MODE_MANSEQ);
+
+		ret = regmap_update_bits(data->map, BD79124_REG_OPMODE_CFG,
+					 BD79124_MSK_CONV_MODE, regval);
+		if (ret)
+			return ret;
+	}
+
+	return regmap_set_bits(data->map, BD79124_REG_SEQ_CFG,
+			       BD79124_MSK_SEQ_START);
+}

Can be done here, as it's only 83 characters.

...

drivers/iio/adc/rohm-bd79124.c line 601

Quoted 11 lines

drivers/iio/adc/rohm-bd79124.c new file +11 -0

+{
+	struct bd79124_data *data = iio_priv(iio_dev);
+	int reg;
+
+	if (chan->channel >= BD79124_MAX_NUM_CHANNELS)
+		return -EINVAL;
+
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		if (dir == IIO_EV_DIR_RISING) {
+			guard(mutex)(&data->mutex);

What does this mutex protect? chan->channel access? I don't think so, then it can be scoped_guard().

drivers/iio/adc/rohm-bd79124.c line 606

Quoted 5 lines

drivers/iio/adc/rohm-bd79124.c new file +4 -0

+			data->alarm_r_limit[chan->channel] = val;
+			reg = BD79124_GET_HIGH_LIMIT_REG(chan->channel);
+		} else if (dir == IIO_EV_DIR_FALLING) {
+			guard(mutex)(&data->mutex);

Ditto.

Quoted 26 lines

drivers/iio/adc/rohm-bd79124.c new file +26 -0

+			data->alarm_f_limit[chan->channel] = val;
+			reg = BD79124_GET_LOW_LIMIT_REG(chan->channel);
+		} else {
+			return -EINVAL;
+		}
+		/*
+		 * We don't want to enable the alarm if it is not enabled or
+		 * if it is suppressed. In that case skip writing to the
+		 * register.
+		 */
+		if (!(data->alarm_monitored[chan->channel] & BIT(dir)) ||
+		    data->alarm_suppressed[chan->channel] & BIT(dir))
+			return 0;
+
+		return bd79124_write_int_to_reg(data, reg, val);
+
+	case IIO_EV_INFO_HYSTERESIS:
+		reg = BD79124_GET_HYSTERESIS_REG(chan->channel);
+		val >>= 3;
+
+		return regmap_update_bits(data->map, reg, BD79124_MSK_HYSTERESIS,
+					  val);
+	default:
+		return -EINVAL;
+	}
+}

...

Quoted 13 lines

drivers/iio/adc/rohm-bd79124.c new file +13 -0

+static int __bd79124_event_ratelimit(struct bd79124_data *data, int reg,
+				     unsigned int limit)
+{
+	int ret;
+
+	if (limit > BD79124_HIGH_LIMIT_MAX)
+		return -EINVAL;
+
+	ret = bd79124_write_int_to_reg(data, reg, limit);
+	if (ret)
+		return ret;
+
+	/*

drivers/iio/adc/rohm-bd79124.c line 826

Quoted 6 lines

drivers/iio/adc/rohm-bd79124.c new file +6 -0

+	 * We use 1 sec 'grace period'. At the moment I see no reason to make
+	 * this user configurable. We need an ABI for this if configuration is
+	 * needed.
+	 */
+	schedule_delayed_work(&data->alm_enable_work,
+			      msecs_to_jiffies(1000));

Perfectly one line even for the 80 character limit fanatics :-)

Quoted 19 lines

drivers/iio/adc/rohm-bd79124.c new file +17 -0

+	return 0;
+}
 
+static int bd79124_get_gpio_pins(const struct iio_chan_spec *cs, int num_channels)
+{
+	int i, gpio_channels;
+
+	/*
+	 * Let's initialize the mux config to say that all 8 channels are
+	 * GPIOs. Then we can just loop through the iio_chan_spec and clear the
+	 * bits for found ADC channels.
+	 */
+	gpio_channels = GENMASK(7, 0);
+	for (i = 0; i < num_channels; i++)
+		gpio_channels &= ~BIT(cs[i].channel);
+
+	return gpio_channels;
+}

...

drivers/iio/adc/rohm-bd79124.c line 1066

Quoted 7 lines

drivers/iio/adc/rohm-bd79124.c new file +7 -0

+	ret = devm_iio_adc_device_alloc_chaninfo_se(dev, template,
+		BD79124_MAX_NUM_CHANNELS - 1, &cs);
+	if (ret < 0) {
+		if (ret == -ENOENT)
+			goto register_gpios;
+		return ret;
+	}

Also can be written as

	ret = devm_iio_adc_device_alloc_chaninfo_se(dev, template,
		BD79124_MAX_NUM_CHANNELS - 1, &cs);
	/* Register all channels as GPIOs in case no ADC functionality required */

	if (ret == -ENOENT)
		goto register_gpios;
	if (ret < 0)
		return ret;

We can avoid open-coding the loop construct which counts firmware child nodes with a specific name by using the newly added device_get_child_node_count_named().

The gianfar driver has such open-coded loop. Replace it with the device_get_child_node_count_named().

Suggested-by Andy Shevchenko <[email protected]>

Signed-off-by Matti Vaittinen <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

Quoted 7 lines · Matti Vaittinen

We can avoid open-coding the loop construct which counts firmware child nodes with a specific name by using the newly added device_get_child_node_count_named().

The gianfar driver has such open-coded loop. Replace it with the device_get_child_node_count_named().

Quoted 3 lines

Suggested-by Andy Shevchenko <[email protected]>

Signed-off-by Matti Vaittinen <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

This patch looks good to me.
But I think it would be best to resubmit it, as a standalone patch for net-next, once it's dependencies are present in net-next.