linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] Device property improvements, add %pfw format specifier
@ 2019-03-26 12:41 Sakari Ailus
  2019-03-26 12:41 ` [PATCH v2 1/6] device property: Add functions for accessing node's parents Sakari Ailus
                   ` (5 more replies)
  0 siblings, 6 replies; 35+ messages in thread
From: Sakari Ailus @ 2019-03-26 12:41 UTC (permalink / raw)
  To: Petr Mladek, linux-kernel, rafael
  Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring

Hi all,

This set adds functionality into the device property API (counting a
node's parents as well as obtaining its name) in order to support printing
fwnode names using a new conversion specifier "%pfw". The names that are
produced are equivalent to its OF counterpart "%pOF" on OF systems for the
two supported modifiers ("f" and "P").

Printing a node's name is something that's been available on OF for a long
time and if something is converted to device property API (such as the
V4L2 fwnode framework) it always got removed of a nice feature that was
sometimes essential in debugging. With this set, that no longer is the
case.

This set depends on a cleanup fully releasing the now-deprecated %pf for
other use:

<URL:https://lore.kernel.org/lkml/20190325211906.GA24180@google.com/T/#t>

I was a bit hesitant on how much of this should be covered by the fwnode
ops and how much would be just better to reside in lib/vsprintf.c, but
opted for putting it to the fwnode interface. This keeps it clean in
vsprintf.c and still rather lean on the fwnode ops side while making it
possible to add other firmware types (although that is unlikely I guess).

since v1:

- Add patch to remove support for %pf and %pF (depends on another patch
  removing all use of %pf and %pF) (now 4th patch)

- Fix kerneldoc argument documentation for fwnode_get_name (2nd patch)

- Align kerneldoc style with the rest of drivers/base/property.c (no extra
  newline after function name)

- Make checkpatch.pl complain about "%pf" not followed by "w" (6th patch)

- WARN_ONCE() on use of invalid conversion specifiers ("%pf" not followed
  by "w")

Sakari Ailus (6):
  device property: Add functions for accessing node's parents
  device property: Add fwnode_get_name for returning the name of a node
  device property: Add a function to obtain a node's prefix
  lib/vsprintf: Remove support for %pF and %pf in favour of %pS and %ps
  lib/vsprintf: Make use of fwnode API to obtain node names and
    separators
  lib/vsprintf: Add %pfw conversion specifier for printing fwnode names

 Documentation/core-api/printk-formats.rst |  34 +++++++---
 drivers/acpi/property.c                   |  46 ++++++++++++++
 drivers/base/property.c                   |  82 +++++++++++++++++++++---
 drivers/of/property.c                     |  16 +++++
 include/linux/fwnode.h                    |   4 ++
 include/linux/property.h                  |   5 ++
 lib/vsprintf.c                            | 101 +++++++++++++++++++++---------
 scripts/checkpatch.pl                     |   4 +-
 8 files changed, 245 insertions(+), 47 deletions(-)

-- 
2.11.0


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v2 1/6] device property: Add functions for accessing node's parents
  2019-03-26 12:41 [PATCH v2 0/6] Device property improvements, add %pfw format specifier Sakari Ailus
@ 2019-03-26 12:41 ` Sakari Ailus
  2019-03-27 12:26   ` Petr Mladek
  2019-03-26 12:41 ` [PATCH v2 2/6] device property: Add fwnode_get_name for returning the name of a node Sakari Ailus
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 35+ messages in thread
From: Sakari Ailus @ 2019-03-26 12:41 UTC (permalink / raw)
  To: Petr Mladek, linux-kernel, rafael
  Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring

Add two convenience functions for accessing node's parents:

fwnode_count_parents() returns the number of parent nodes a given node
has. fwnode_get_nth_parent() returns node's parent at a given distance
from the node itself.

Also reorder fwnode_get_parent() in property.c according to the same order
as in property.h.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/base/property.c  | 59 ++++++++++++++++++++++++++++++++++++++++++------
 include/linux/property.h |  3 +++
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 8b91ab380d14..61eb6ceacc3f 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -533,6 +533,19 @@ int device_add_properties(struct device *dev,
 EXPORT_SYMBOL_GPL(device_add_properties);
 
 /**
+ * fwnode_get_parent - Return parent firwmare node
+ * @fwnode: Firmware whose parent is retrieved
+ *
+ * Return parent firmware node of the given node if possible or %NULL if no
+ * parent was available.
+ */
+struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode)
+{
+	return fwnode_call_ptr_op(fwnode, get_parent);
+}
+EXPORT_SYMBOL_GPL(fwnode_get_parent);
+
+/**
  * fwnode_get_next_parent - Iterate to the node's parent
  * @fwnode: Firmware whose parent is retrieved
  *
@@ -554,17 +567,49 @@ struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
 EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
 
 /**
- * fwnode_get_parent - Return parent firwmare node
- * @fwnode: Firmware whose parent is retrieved
+ * fwnode_count_parents - Return the number of parents a node has
+ * @fwnode: The node the parents of which are to be counted
  *
- * Return parent firmware node of the given node if possible or %NULL if no
- * parent was available.
+ * Returns the number of parents a node has.
  */
-struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode)
+unsigned int fwnode_count_parents(struct fwnode_handle *fwnode)
 {
-	return fwnode_call_ptr_op(fwnode, get_parent);
+	unsigned int count;
+
+	fwnode_handle_get(fwnode);
+
+	for (count = 0; fwnode; count++)
+		fwnode = fwnode_get_next_parent(fwnode);
+
+	return count - 1;
 }
-EXPORT_SYMBOL_GPL(fwnode_get_parent);
+EXPORT_SYMBOL_GPL(fwnode_count_parents);
+
+/**
+ * fwnode_get_nth_parent - Return an nth parent of a node
+ * @fwnode: The node the parent of which is requested
+ * @depth: Distance of the parent from the node
+ *
+ * Returns the nth parent of a node. If @depth is 0, the functionality is
+ * equivalent to fwnode_handle_get(). For @depth == 1, it is fwnode_get_parent()
+ * and so on.
+ *
+ * The caller is responsible for calling fwnode_handle_put() for the returned
+ * node.
+ */
+struct fwnode_handle *fwnode_get_nth_parent(struct fwnode_handle *fwnode,
+					    unsigned int depth)
+{
+	unsigned int i;
+
+	fwnode_handle_get(fwnode);
+
+	for (i = 0; i < depth && fwnode; i++)
+		fwnode = fwnode_get_next_parent(fwnode);
+
+	return fwnode;
+}
+EXPORT_SYMBOL_GPL(fwnode_get_nth_parent);
 
 /**
  * fwnode_get_next_child_node - Return the next child node handle for a node
diff --git a/include/linux/property.h b/include/linux/property.h
index 65d3420dd5d1..1164ac974ee8 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -81,6 +81,9 @@ int fwnode_property_get_reference_args(const struct fwnode_handle *fwnode,
 struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode);
 struct fwnode_handle *fwnode_get_next_parent(
 	struct fwnode_handle *fwnode);
+unsigned int fwnode_count_parents(struct fwnode_handle *fwn);
+struct fwnode_handle *fwnode_get_nth_parent(struct fwnode_handle *fwn,
+					    unsigned int depth);
 struct fwnode_handle *fwnode_get_next_child_node(
 	const struct fwnode_handle *fwnode, struct fwnode_handle *child);
 struct fwnode_handle *fwnode_get_next_available_child_node(
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 2/6] device property: Add fwnode_get_name for returning the name of a node
  2019-03-26 12:41 [PATCH v2 0/6] Device property improvements, add %pfw format specifier Sakari Ailus
  2019-03-26 12:41 ` [PATCH v2 1/6] device property: Add functions for accessing node's parents Sakari Ailus
@ 2019-03-26 12:41 ` Sakari Ailus
  2019-03-28  9:45   ` Rafael J. Wysocki
  2019-03-31  6:42   ` Rob Herring
  2019-03-26 12:41 ` [PATCH v2 3/6] device property: Add a function to obtain a node's prefix Sakari Ailus
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 35+ messages in thread
From: Sakari Ailus @ 2019-03-26 12:41 UTC (permalink / raw)
  To: Petr Mladek, linux-kernel, rafael
  Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring

The fwnode framework did not have means to obtain the name of a node. Add
that now, in form of the fwnode_get_name() function and a corresponding
get_name fwnode op. OF and ACPI support is included.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c  | 24 ++++++++++++++++++++++++
 drivers/base/property.c  | 11 +++++++++++
 drivers/of/property.c    |  6 ++++++
 include/linux/fwnode.h   |  2 ++
 include/linux/property.h |  1 +
 5 files changed, 44 insertions(+)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 77abe0ec4043..8c9a4c02cde2 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1291,6 +1291,29 @@ acpi_fwnode_get_reference_args(const struct fwnode_handle *fwnode,
 						  args_count, args);
 }
 
+static const char *acpi_fwnode_get_name(const struct fwnode_handle *fwnode)
+{
+	const struct acpi_device *adev;
+	struct fwnode_handle *parent;
+
+	parent = fwnode_get_parent(fwnode);
+	/* Is this the root node? */
+	if (!parent)
+		return "\\";
+
+	fwnode_handle_put(parent);
+
+	if (is_acpi_data_node(fwnode)) {
+		const struct acpi_data_node *dn = to_acpi_data_node(fwnode);
+
+		return dn->name;
+	}
+
+	adev = to_acpi_device_node(fwnode);
+
+	return adev ? acpi_device_bid(adev) : NULL;
+}
+
 static struct fwnode_handle *
 acpi_fwnode_get_parent(struct fwnode_handle *fwnode)
 {
@@ -1331,6 +1354,7 @@ acpi_fwnode_device_get_match_data(const struct fwnode_handle *fwnode,
 		.get_parent = acpi_node_get_parent,			\
 		.get_next_child_node = acpi_get_next_subnode,		\
 		.get_named_child_node = acpi_fwnode_get_named_child_node, \
+		.get_name = acpi_fwnode_get_name,			\
 		.get_reference_args = acpi_fwnode_get_reference_args,	\
 		.graph_get_next_endpoint =				\
 			acpi_graph_get_next_endpoint,			\
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 61eb6ceacc3f..04a8591cd1b0 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -533,6 +533,17 @@ int device_add_properties(struct device *dev,
 EXPORT_SYMBOL_GPL(device_add_properties);
 
 /**
+ * fwnode_get_name - Return the name of a node
+ * @fwnode: The firmware node
+ *
+ * Returns a pointer to the node name.
+ */
+const char *fwnode_get_name(const struct fwnode_handle *fwnode)
+{
+	return fwnode_call_ptr_op(fwnode, get_name);
+}
+
+/**
  * fwnode_get_parent - Return parent firwmare node
  * @fwnode: Firmware whose parent is retrieved
  *
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 8631efa1daa1..f0a7b78f2d9f 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -872,6 +872,11 @@ of_fwnode_property_read_string_array(const struct fwnode_handle *fwnode,
 		of_property_count_strings(node, propname);
 }
 
+static const char *of_fwnode_get_name(const struct fwnode_handle *fwnode)
+{
+	return kbasename(to_of_node(fwnode)->full_name);
+}
+
 static struct fwnode_handle *
 of_fwnode_get_parent(const struct fwnode_handle *fwnode)
 {
@@ -993,6 +998,7 @@ const struct fwnode_operations of_fwnode_ops = {
 	.property_present = of_fwnode_property_present,
 	.property_read_int_array = of_fwnode_property_read_int_array,
 	.property_read_string_array = of_fwnode_property_read_string_array,
+	.get_name = of_fwnode_get_name,
 	.get_parent = of_fwnode_get_parent,
 	.get_next_child_node = of_fwnode_get_next_child_node,
 	.get_named_child_node = of_fwnode_get_named_child_node,
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index faebf0ca0686..85b7fa4dc727 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -59,6 +59,7 @@ struct fwnode_reference_args {
  *				 otherwise.
  * @property_read_string_array: Read an array of string properties. Return zero
  *				on success, a negative error code otherwise.
+ * @get_name: Return the name of an fwnode.
  * @get_parent: Return the parent of an fwnode.
  * @get_next_child_node: Return the next child node in an iteration.
  * @get_named_child_node: Return a child node with a given name.
@@ -85,6 +86,7 @@ struct fwnode_operations {
 	(*property_read_string_array)(const struct fwnode_handle *fwnode_handle,
 				      const char *propname, const char **val,
 				      size_t nval);
+	const char *(*get_name)(const struct fwnode_handle *fwnode);
 	struct fwnode_handle *(*get_parent)(const struct fwnode_handle *fwnode);
 	struct fwnode_handle *
 	(*get_next_child_node)(const struct fwnode_handle *fwnode,
diff --git a/include/linux/property.h b/include/linux/property.h
index 1164ac974ee8..2f5df900e310 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -78,6 +78,7 @@ int fwnode_property_get_reference_args(const struct fwnode_handle *fwnode,
 				       unsigned int nargs, unsigned int index,
 				       struct fwnode_reference_args *args);
 
+const char *fwnode_get_name(const struct fwnode_handle *fwnode);
 struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode);
 struct fwnode_handle *fwnode_get_next_parent(
 	struct fwnode_handle *fwnode);
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 3/6] device property: Add a function to obtain a node's prefix
  2019-03-26 12:41 [PATCH v2 0/6] Device property improvements, add %pfw format specifier Sakari Ailus
  2019-03-26 12:41 ` [PATCH v2 1/6] device property: Add functions for accessing node's parents Sakari Ailus
  2019-03-26 12:41 ` [PATCH v2 2/6] device property: Add fwnode_get_name for returning the name of a node Sakari Ailus
@ 2019-03-26 12:41 ` Sakari Ailus
  2019-03-26 13:16   ` Andy Shevchenko
  2019-03-31  6:42   ` Rob Herring
  2019-03-26 12:41 ` [PATCH v2 4/6] lib/vsprintf: Remove support for %pF and %pf in favour of %pS and %ps Sakari Ailus
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 35+ messages in thread
From: Sakari Ailus @ 2019-03-26 12:41 UTC (permalink / raw)
  To: Petr Mladek, linux-kernel, rafael
  Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring

The prefix is used for printing purpose before a node, and it also works
as a separator between two nodes.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c  | 22 ++++++++++++++++++++++
 drivers/base/property.c  | 12 ++++++++++++
 drivers/of/property.c    | 10 ++++++++++
 include/linux/fwnode.h   |  2 ++
 include/linux/property.h |  1 +
 5 files changed, 47 insertions(+)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 8c9a4c02cde2..c167fd748f86 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1314,6 +1314,27 @@ static const char *acpi_fwnode_get_name(const struct fwnode_handle *fwnode)
 	return adev ? acpi_device_bid(adev) : NULL;
 }
 
+static const char *
+acpi_fwnode_get_name_prefix(const struct fwnode_handle *fwnode)
+{
+	struct fwnode_handle *parent;
+
+	parent = fwnode_get_parent(fwnode);
+	/* Root node. */
+	if (!parent)
+		return "";
+
+	parent = fwnode_get_next_parent(parent);
+	/* Second node from the root; no prefix here either. */
+	if (!parent)
+		return "";
+
+	fwnode_handle_put(parent);
+
+	/* ACPI device or data node. */
+	return ".";
+}
+
 static struct fwnode_handle *
 acpi_fwnode_get_parent(struct fwnode_handle *fwnode)
 {
@@ -1355,6 +1376,7 @@ acpi_fwnode_device_get_match_data(const struct fwnode_handle *fwnode,
 		.get_next_child_node = acpi_get_next_subnode,		\
 		.get_named_child_node = acpi_fwnode_get_named_child_node, \
 		.get_name = acpi_fwnode_get_name,			\
+		.get_name_prefix = acpi_fwnode_get_name_prefix,		\
 		.get_reference_args = acpi_fwnode_get_reference_args,	\
 		.graph_get_next_endpoint =				\
 			acpi_graph_get_next_endpoint,			\
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 04a8591cd1b0..2e5b826a2376 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -544,6 +544,18 @@ const char *fwnode_get_name(const struct fwnode_handle *fwnode)
 }
 
 /**
+ * fwnode_get_name_prefix - Return the prefix of node for printing purposes
+ * @fwnode: The firmware node
+ *
+ * Returns the prefix of a node, intended to be printed right before the node.
+ * The prefix works also as a separator between the nodes.
+ */
+const char *fwnode_get_name_prefix(const struct fwnode_handle *fwnode)
+{
+	return fwnode_call_ptr_op(fwnode, get_name_prefix);
+}
+
+/**
  * fwnode_get_parent - Return parent firwmare node
  * @fwnode: Firmware whose parent is retrieved
  *
diff --git a/drivers/of/property.c b/drivers/of/property.c
index f0a7b78f2d9f..8fed3b142b41 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -877,6 +877,15 @@ static const char *of_fwnode_get_name(const struct fwnode_handle *fwnode)
 	return kbasename(to_of_node(fwnode)->full_name);
 }
 
+static const char *of_fwnode_get_name_prefix(const struct fwnode_handle *fwnode)
+{
+	/* Root needs no prefix here (its name is "/"). */
+	if (!to_of_node(fwnode)->parent)
+		return "";
+
+	return "/";
+}
+
 static struct fwnode_handle *
 of_fwnode_get_parent(const struct fwnode_handle *fwnode)
 {
@@ -999,6 +1008,7 @@ const struct fwnode_operations of_fwnode_ops = {
 	.property_read_int_array = of_fwnode_property_read_int_array,
 	.property_read_string_array = of_fwnode_property_read_string_array,
 	.get_name = of_fwnode_get_name,
+	.get_name_prefix = of_fwnode_get_name_prefix,
 	.get_parent = of_fwnode_get_parent,
 	.get_next_child_node = of_fwnode_get_next_child_node,
 	.get_named_child_node = of_fwnode_get_named_child_node,
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 85b7fa4dc727..592ac6936e14 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -60,6 +60,7 @@ struct fwnode_reference_args {
  * @property_read_string_array: Read an array of string properties. 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.
  * @get_next_child_node: Return the next child node in an iteration.
  * @get_named_child_node: Return a child node with a given name.
@@ -87,6 +88,7 @@ struct fwnode_operations {
 				      const char *propname, const char **val,
 				      size_t nval);
 	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);
 	struct fwnode_handle *
 	(*get_next_child_node)(const struct fwnode_handle *fwnode,
diff --git a/include/linux/property.h b/include/linux/property.h
index 2f5df900e310..0ea466ee8c3d 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -79,6 +79,7 @@ int fwnode_property_get_reference_args(const struct fwnode_handle *fwnode,
 				       struct fwnode_reference_args *args);
 
 const char *fwnode_get_name(const struct fwnode_handle *fwnode);
+const char *fwnode_get_name_prefix(const struct fwnode_handle *fwnode);
 struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode);
 struct fwnode_handle *fwnode_get_next_parent(
 	struct fwnode_handle *fwnode);
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 4/6] lib/vsprintf: Remove support for %pF and %pf in favour of %pS and %ps
  2019-03-26 12:41 [PATCH v2 0/6] Device property improvements, add %pfw format specifier Sakari Ailus
                   ` (2 preceding siblings ...)
  2019-03-26 12:41 ` [PATCH v2 3/6] device property: Add a function to obtain a node's prefix Sakari Ailus
@ 2019-03-26 12:41 ` Sakari Ailus
  2019-03-26 12:41 ` [PATCH v2 5/6] lib/vsprintf: Make use of fwnode API to obtain node names and separators Sakari Ailus
  2019-03-26 12:41 ` [PATCH v2 6/6] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names Sakari Ailus
  5 siblings, 0 replies; 35+ messages in thread
From: Sakari Ailus @ 2019-03-26 12:41 UTC (permalink / raw)
  To: Petr Mladek, linux-kernel, rafael
  Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring

%pS and %ps are now the preferred conversion specifiers to print function
%names. The functionality is equivalent; remove the old, deprecated %pF
%and %pf support.

Depends-on: ("treewide: Switch printk users from %pf and %pF to %ps and
	      %pS, respectively")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/core-api/printk-formats.rst | 10 ----------
 lib/vsprintf.c                            |  8 ++------
 scripts/checkpatch.pl                     |  1 -
 3 files changed, 2 insertions(+), 17 deletions(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index c37ec7cd9c06..c90826a1ff17 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -78,8 +78,6 @@ Symbols/Function Pointers
 
 	%pS	versatile_init+0x0/0x110
 	%ps	versatile_init
-	%pF	versatile_init+0x0/0x110
-	%pf	versatile_init
 	%pSR	versatile_init+0x9/0x110
 		(with __builtin_extract_return_addr() translation)
 	%pB	prev_fn_of_versatile_init+0x88/0x88
@@ -89,14 +87,6 @@ The ``S`` and ``s`` specifiers are used for printing a pointer in symbolic
 format. They result in the symbol name with (S) or without (s)
 offsets. If KALLSYMS are disabled then the symbol address is printed instead.
 
-Note, that the ``F`` and ``f`` specifiers are identical to ``S`` (``s``)
-and thus deprecated. We have ``F`` and ``f`` because on ia64, ppc64 and
-parisc64 function pointers are indirect and, in fact, are function
-descriptors, which require additional dereferencing before we can lookup
-the symbol. As of now, ``S`` and ``s`` perform dereferencing on those
-platforms (when needed), so ``F`` and ``f`` exist for compatibility
-reasons only.
-
 The ``B`` specifier results in the symbol name with offsets and should be
 used when printing stack backtraces. The specifier takes into
 consideration the effect of compiler optimisations which may occur
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 791b6fa36905..5f60b8d41277 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -797,7 +797,7 @@ char *symbol_string(char *buf, char *end, void *ptr,
 #ifdef CONFIG_KALLSYMS
 	if (*fmt == 'B')
 		sprint_backtrace(sym, value);
-	else if (*fmt != 'f' && *fmt != 's')
+	else if (*fmt != 's')
 		sprint_symbol(sym, value);
 	else
 		sprint_symbol_no_offset(sym, value);
@@ -1853,9 +1853,7 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
  *
  * - 'S' For symbolic direct pointers (or function descriptors) with offset
  * - 's' For symbolic direct pointers (or function descriptors) without offset
- * - 'F' Same as 'S'
- * - 'f' Same as 's'
- * - '[FfSs]R' as above with __builtin_extract_return_addr() translation
+ * - '[Ss]R' as above with __builtin_extract_return_addr() translation
  * - 'B' For backtraced symbolic direct pointers with offset
  * - 'R' For decoded struct resource, e.g., [mem 0x0-0x1f 64bit pref]
  * - 'r' For raw struct resource, e.g., [mem 0x0-0x1f flags 0x201]
@@ -1970,8 +1968,6 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 	}
 
 	switch (*fmt) {
-	case 'F':
-	case 'f':
 	case 'S':
 	case 's':
 		ptr = dereference_symbol_descriptor(ptr);
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 5b756278df13..b2e80259fb8c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5994,7 +5994,6 @@ sub process {
 					my $ext_type = "Invalid";
 					my $use = "";
 					if ($bad_specifier =~ /p[Ff]/) {
-						$ext_type = "Deprecated";
 						$use = " - use %pS instead";
 						$use =~ s/pS/ps/ if ($bad_specifier =~ /pf/);
 					}
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 5/6] lib/vsprintf: Make use of fwnode API to obtain node names and separators
  2019-03-26 12:41 [PATCH v2 0/6] Device property improvements, add %pfw format specifier Sakari Ailus
                   ` (3 preceding siblings ...)
  2019-03-26 12:41 ` [PATCH v2 4/6] lib/vsprintf: Remove support for %pF and %pf in favour of %pS and %ps Sakari Ailus
@ 2019-03-26 12:41 ` Sakari Ailus
  2019-03-26 12:41 ` [PATCH v2 6/6] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names Sakari Ailus
  5 siblings, 0 replies; 35+ messages in thread
From: Sakari Ailus @ 2019-03-26 12:41 UTC (permalink / raw)
  To: Petr Mladek, linux-kernel, rafael
  Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring

Instead of implementing our own means of discovering parent nodes, node
names or counting how many parents a node has, use the newly added
functions in the fwnode API to obtain that information.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 lib/vsprintf.c | 37 +++++++++++++++----------------------
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 5f60b8d41277..91f2a3e4892e 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -37,6 +37,7 @@
 #include <net/addrconf.h>
 #include <linux/siphash.h>
 #include <linux/compiler.h>
+#include <linux/property.h>
 #ifdef CONFIG_BLOCK
 #include <linux/blkdev.h>
 #endif
@@ -1720,32 +1721,23 @@ char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt)
 	return format_flags(buf, end, flags, names);
 }
 
-static const char *device_node_name_for_depth(const struct device_node *np, int depth)
-{
-	for ( ; np && depth; depth--)
-		np = np->parent;
-
-	return kbasename(np->full_name);
-}
-
 static noinline_for_stack
-char *device_node_gen_full_name(const struct device_node *np, char *buf, char *end)
+char *fwnode_gen_full_name(struct fwnode_handle *fwnode, char *buf, char *end)
 {
 	int depth;
-	const struct device_node *parent = np->parent;
 
-	/* special case for root node */
-	if (!parent)
-		return string(buf, end, "/", default_str_spec);
+	for (depth = fwnode_count_parents(fwnode); depth >= 0; depth--) {
+		struct fwnode_handle *__fwnode =
+			fwnode_get_nth_parent(fwnode, depth);
 
-	for (depth = 0; parent->parent; depth++)
-		parent = parent->parent;
-
-	for ( ; depth >= 0; depth--) {
-		buf = string(buf, end, "/", default_str_spec);
-		buf = string(buf, end, device_node_name_for_depth(np, depth),
+		buf = string(buf, end, fwnode_get_name_prefix(__fwnode),
+			     default_str_spec);
+		buf = string(buf, end, fwnode_get_name(__fwnode),
 			     default_str_spec);
+
+		fwnode_handle_put(__fwnode);
 	}
+
 	return buf;
 }
 
@@ -1790,10 +1782,11 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
 
 		switch (*fmt) {
 		case 'f':	/* full_name */
-			buf = device_node_gen_full_name(dn, buf, end);
+			buf = fwnode_gen_full_name(of_fwnode_handle(dn), buf,
+						   end);
 			break;
 		case 'n':	/* name */
-			p = kbasename(of_node_full_name(dn));
+			p = fwnode_get_name(of_fwnode_handle(dn));
 			precision = str_spec.precision;
 			str_spec.precision = strchrnul(p, '@') - p;
 			buf = string(buf, end, p, str_spec);
@@ -1803,7 +1796,7 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
 			buf = number(buf, end, (unsigned int)dn->phandle, num_spec);
 			break;
 		case 'P':	/* path-spec */
-			p = kbasename(of_node_full_name(dn));
+			p = fwnode_get_name(of_fwnode_handle(dn));
 			if (!p[1])
 				p = "/";
 			buf = string(buf, end, p, str_spec);
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v2 6/6] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names
  2019-03-26 12:41 [PATCH v2 0/6] Device property improvements, add %pfw format specifier Sakari Ailus
                   ` (4 preceding siblings ...)
  2019-03-26 12:41 ` [PATCH v2 5/6] lib/vsprintf: Make use of fwnode API to obtain node names and separators Sakari Ailus
@ 2019-03-26 12:41 ` Sakari Ailus
  2019-03-26 13:11   ` Rasmus Villemoes
  2019-03-26 20:18   ` Joe Perches
  5 siblings, 2 replies; 35+ messages in thread
From: Sakari Ailus @ 2019-03-26 12:41 UTC (permalink / raw)
  To: Petr Mladek, linux-kernel, rafael
  Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring

Add support for %pfw conversion specifier (with "f" and "P" modifiers) to
support printing full path of the node, including its name ("f") and only
the node's name ("P") in the printk family of functions. The two flags
have equivalent functionality to existing %pOF with the same two modifiers
("f" and "P") on OF based systems. The ability to do the same on ACPI
based systems is added by this patch.

On ACPI based systems the resulting strings look like

	\_SB.PCI0.CIO2.port@1.endpoint@0

where the nodes are separated by a dot (".") and the first three are
ACPI device nodes and the latter two ACPI data nodes.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/core-api/printk-formats.rst | 24 +++++++++++++
 lib/vsprintf.c                            | 56 +++++++++++++++++++++++++++++++
 scripts/checkpatch.pl                     |  3 +-
 3 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index c90826a1ff17..209625e0c61d 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -410,6 +410,30 @@ Examples::
 
 Passed by reference.
 
+Fwnode handles
+--------------
+
+::
+
+	%pfw[fP]
+
+For printing information on fwnode handles. The default is to print the full
+node name, including the path. The modifiers are functionally equivalent to
+%pOF above.
+
+	- f - full name of the node, including the path
+	- P - the name of the node including an address (if there is one)
+
+Examples (ACPI):
+
+	%pfwf	\_SB.PCI0.CIO2.port@1.endpoint@0	- Full node name
+	%pfwP	endpoint@0				- Node name
+
+Examples (OF):
+
+	%pfwf	/ocp@68000000/i2c@48072000/camera@10/port/endpoint - Full name
+	%pfwP	endpoint				- Node name
+
 Time and date (struct rtc_time)
 -------------------------------
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 91f2a3e4892e..43df0472d001 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1834,6 +1834,48 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
 	return widen_string(buf, buf - buf_start, end, spec);
 }
 
+static noinline_for_stack
+char *fwnode_string(char *buf, char *end, struct fwnode_handle *fwnode,
+		    struct printf_spec spec, const char *fmt)
+{
+	const char * const modifiers = "fP";
+	struct printf_spec str_spec = spec;
+	char *buf_start = buf;
+	bool pass;
+
+	str_spec.field_width = -1;
+
+	if ((unsigned long)fwnode < PAGE_SIZE)
+		return string(buf, end, "(null)", spec);
+
+	/* simple case without anything any more format specifiers */
+	fmt++;
+	if (fmt[0] == '\0' || strcspn(fmt, modifiers) > 0)
+		fmt = "f";
+
+	for (pass = false; strspn(fmt, modifiers); fmt++, pass = true) {
+		if (pass) {
+			if (buf < end)
+				*buf = ':';
+			buf++;
+		}
+
+		switch (*fmt) {
+		case 'f':	/* full_name */
+			buf = fwnode_gen_full_name(fwnode, buf, end);
+			break;
+		case 'P':	/* name */
+			buf = string(buf, end, fwnode_get_name(fwnode),
+				     str_spec);
+			break;
+		default:
+			break;
+		}
+	}
+
+	return widen_string(buf, buf - buf_start, end, spec);
+}
+
 /*
  * Show a '%p' thing.  A kernel extension is that the '%p' is followed
  * by an extra set of alphanumeric characters that are extended format
@@ -1936,6 +1978,10 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
  *                  F device node flags
  *                  c major compatible string
  *                  C full compatible string
+ * - 'fw[fP]'	For a firmware node (struct fwnode_handle) pointer
+ *		Without an option prints the full name of the node
+ *		f full name
+ *		P node name, including a possible unit address
  * - 'x' For printing the address. Equivalent to "%lx".
  *
  * ** When making changes please also update:
@@ -2060,6 +2106,16 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 			return device_node_string(buf, end, ptr, spec, fmt + 1);
 		}
 		break;
+	case 'f':
+		switch (fmt[1]) {
+		case 'w':
+			return fwnode_string(buf, end, ptr, spec, fmt + 1);
+		default:
+			WARN_ONCE(1, "Unsupported pointer conversion %%p%c\n",
+				  fmt[1]);
+			return buf;
+		}
+		break;
 	case 'x':
 		return pointer_string(buf, end, ptr, spec);
 	}
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b2e80259fb8c..031db629e309 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5977,7 +5977,8 @@ sub process {
 				while ($fmt =~ /(\%[\*\d\.]*p(\w))/g) {
 					$specifier = $1;
 					$extension = $2;
-					if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOx]/) {
+					if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOxf]/ ||
+					    $extension =~ /f[^w]/) {
 						$bad_specifier = $specifier;
 						last;
 					}
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 6/6] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names
  2019-03-26 12:41 ` [PATCH v2 6/6] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names Sakari Ailus
@ 2019-03-26 13:11   ` Rasmus Villemoes
  2019-03-26 13:24     ` Andy Shevchenko
  2019-03-26 20:18   ` Joe Perches
  1 sibling, 1 reply; 35+ messages in thread
From: Rasmus Villemoes @ 2019-03-26 13:11 UTC (permalink / raw)
  To: Sakari Ailus, Petr Mladek, linux-kernel, rafael
  Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring

On 26/03/2019 13.41, Sakari Ailus wrote:
> Add support for %pfw conversion specifier (with "f" and "P" modifiers) to
> support printing full path of the node, including its name ("f") and only
> the node's name ("P") in the printk family of functions. The two flags
> have equivalent functionality to existing %pOF with the same two modifiers
> ("f" and "P") on OF based systems. The ability to do the same on ACPI
> based systems is added by this patch.
> 
>  
> +static noinline_for_stack
> +char *fwnode_string(char *buf, char *end, struct fwnode_handle *fwnode,
> +		    struct printf_spec spec, const char *fmt)
> +{
> +	const char * const modifiers = "fP";
> +	struct printf_spec str_spec = spec;
> +	char *buf_start = buf;
> +	bool pass;
> +
> +	str_spec.field_width = -1;
> +
> +	if ((unsigned long)fwnode < PAGE_SIZE)
> +		return string(buf, end, "(null)", spec);
> +
> +	/* simple case without anything any more format specifiers */
> +	fmt++;
> +	if (fmt[0] == '\0' || strcspn(fmt, modifiers) > 0)
> +		fmt = "f";
> +
> +	for (pass = false; strspn(fmt, modifiers); fmt++, pass = true) {
> +		if (pass) {
> +			if (buf < end)
> +				*buf = ':';
> +			buf++;
> +		}
> +
> +		switch (*fmt) {
> +		case 'f':	/* full_name */
> +			buf = fwnode_gen_full_name(fwnode, buf, end);
> +			break;
> +		case 'P':	/* name */
> +			buf = string(buf, end, fwnode_get_name(fwnode),
> +				     str_spec);
> +			break;
> +		default:
> +			break;
> +		}
> +	}

This seems awfully complicated. Why would anyone ever pass more than one
of 'f' and 'P'? Why not just

switch(*fmt) {
case 'P':
   ...
case 'f':
default:
   ...
}

which avoids the loop and the strcspn. Or, drop the default: case and
don't have logic at all for falling back to 'f' if neither is present.

> +	return widen_string(buf, buf - buf_start, end, spec);
> +}
> +
>  /*
>   * Show a '%p' thing.  A kernel extension is that the '%p' is followed
>   * by an extra set of alphanumeric characters that are extended format
> @@ -1936,6 +1978,10 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
>   *                  F device node flags
>   *                  c major compatible string
>   *                  C full compatible string
> + * - 'fw[fP]'	For a firmware node (struct fwnode_handle) pointer
> + *		Without an option prints the full name of the node
> + *		f full name
> + *		P node name, including a possible unit address
>   * - 'x' For printing the address. Equivalent to "%lx".
>   *
>   * ** When making changes please also update:
> @@ -2060,6 +2106,16 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
>  			return device_node_string(buf, end, ptr, spec, fmt + 1);
>  		}
>  		break;
> +	case 'f':
> +		switch (fmt[1]) {
> +		case 'w':
> +			return fwnode_string(buf, end, ptr, spec, fmt + 1);

Why not pass fmt+2; we know that fmt+1 points at a 'w'. Just to avoid
doing the fmt++ inside fwnode_string().

Rasmus

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 3/6] device property: Add a function to obtain a node's prefix
  2019-03-26 12:41 ` [PATCH v2 3/6] device property: Add a function to obtain a node's prefix Sakari Ailus
@ 2019-03-26 13:16   ` Andy Shevchenko
  2019-03-26 13:32     ` Sakari Ailus
  2019-03-31  6:42   ` Rob Herring
  1 sibling, 1 reply; 35+ messages in thread
From: Andy Shevchenko @ 2019-03-26 13:16 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Petr Mladek, linux-kernel, rafael, linux-acpi, devicetree, Rob Herring

On Tue, Mar 26, 2019 at 02:41:03PM +0200, Sakari Ailus wrote:
> The prefix is used for printing purpose before a node, and it also works
> as a separator between two nodes.
> 

One nit below.

> +static const char *
> +acpi_fwnode_get_name_prefix(const struct fwnode_handle *fwnode)
> +{
> +	struct fwnode_handle *parent;
> +

> +	parent = fwnode_get_parent(fwnode);
> +	/* Root node. */

I guess a comment could be easier to read if it goes before parent assignment
line.

> +	if (!parent)
> +		return "";
> +
> +	parent = fwnode_get_next_parent(parent);
> +	/* Second node from the root; no prefix here either. */

Ditto.

> +	if (!parent)
> +		return "";
> +
> +	fwnode_handle_put(parent);
> +
> +	/* ACPI device or data node. */
> +	return ".";
> +}

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 6/6] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names
  2019-03-26 13:11   ` Rasmus Villemoes
@ 2019-03-26 13:24     ` Andy Shevchenko
  2019-03-26 13:31       ` Sakari Ailus
  2019-03-28 14:29       ` Petr Mladek
  0 siblings, 2 replies; 35+ messages in thread
From: Andy Shevchenko @ 2019-03-26 13:24 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Sakari Ailus, Petr Mladek, linux-kernel, rafael, linux-acpi,
	devicetree, Rob Herring

On Tue, Mar 26, 2019 at 02:11:35PM +0100, Rasmus Villemoes wrote:
> On 26/03/2019 13.41, Sakari Ailus wrote:
> > Add support for %pfw conversion specifier (with "f" and "P" modifiers) to
> > support printing full path of the node, including its name ("f") and only
> > the node's name ("P") in the printk family of functions. The two flags
> > have equivalent functionality to existing %pOF with the same two modifiers
> > ("f" and "P") on OF based systems. The ability to do the same on ACPI
> > based systems is added by this patch.

> > +	for (pass = false; strspn(fmt, modifiers); fmt++, pass = true) {
> > +		if (pass) {
> > +			if (buf < end)
> > +				*buf = ':';
> > +			buf++;
> > +		}
> > +
> > +		switch (*fmt) {
> > +		case 'f':	/* full_name */
> > +			buf = fwnode_gen_full_name(fwnode, buf, end);
> > +			break;
> > +		case 'P':	/* name */
> > +			buf = string(buf, end, fwnode_get_name(fwnode),
> > +				     str_spec);
> > +			break;
> > +		default:
> > +			break;
> > +		}
> > +	}
> 
> This seems awfully complicated. Why would anyone ever pass more than one
> of 'f' and 'P'? Why not just
> 
> switch(*fmt) {
> case 'P':
>    ...
> case 'f':
> default:
>    ...
> }
> 
> which avoids the loop and the strcspn. Or, drop the default: case and
> don't have logic at all for falling back to 'f' if neither is present.
> 
> > +	return widen_string(buf, buf - buf_start, end, spec);
> > +}

My point as well (as per sent comments against previous version).
Sakari, can you add test cases at the same time?

> >  			return device_node_string(buf, end, ptr, spec, fmt + 1);

> > +			return fwnode_string(buf, end, ptr, spec, fmt + 1);
> 
> Why not pass fmt+2; we know that fmt+1 points at a 'w'. Just to avoid
> doing the fmt++ inside fwnode_string().

I guess in order to be consistent with existing %pOF case. But wouldn't be
better to fix %pOF for that sooner or later?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 6/6] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names
  2019-03-26 13:24     ` Andy Shevchenko
@ 2019-03-26 13:31       ` Sakari Ailus
  2019-03-28 14:29       ` Petr Mladek
  1 sibling, 0 replies; 35+ messages in thread
From: Sakari Ailus @ 2019-03-26 13:31 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rasmus Villemoes, Petr Mladek, linux-kernel, rafael, linux-acpi,
	devicetree, Rob Herring

Hi Andy, Rasmus,

Thanks for the comments.

On Tue, Mar 26, 2019 at 03:24:50PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 26, 2019 at 02:11:35PM +0100, Rasmus Villemoes wrote:
> > On 26/03/2019 13.41, Sakari Ailus wrote:
> > > Add support for %pfw conversion specifier (with "f" and "P" modifiers) to
> > > support printing full path of the node, including its name ("f") and only
> > > the node's name ("P") in the printk family of functions. The two flags
> > > have equivalent functionality to existing %pOF with the same two modifiers
> > > ("f" and "P") on OF based systems. The ability to do the same on ACPI
> > > based systems is added by this patch.
> 
> > > +	for (pass = false; strspn(fmt, modifiers); fmt++, pass = true) {
> > > +		if (pass) {
> > > +			if (buf < end)
> > > +				*buf = ':';
> > > +			buf++;
> > > +		}
> > > +
> > > +		switch (*fmt) {
> > > +		case 'f':	/* full_name */
> > > +			buf = fwnode_gen_full_name(fwnode, buf, end);
> > > +			break;
> > > +		case 'P':	/* name */
> > > +			buf = string(buf, end, fwnode_get_name(fwnode),
> > > +				     str_spec);
> > > +			break;
> > > +		default:
> > > +			break;
> > > +		}
> > > +	}
> > 
> > This seems awfully complicated. Why would anyone ever pass more than one
> > of 'f' and 'P'? Why not just
> > 
> > switch(*fmt) {
> > case 'P':
> >    ...
> > case 'f':
> > default:
> >    ...
> > }
> > 
> > which avoids the loop and the strcspn. Or, drop the default: case and
> > don't have logic at all for falling back to 'f' if neither is present.

There are just two modifiers supported now ('f' and 'P') but should more be
needed in the future, we'd need the loop again. This is basically modelled
after %pOF implementation.

I can simplify this if you both still think that's the way to go, and
make the implementations diverge.

> > 
> > > +	return widen_string(buf, buf - buf_start, end, spec);
> > > +}
> 
> My point as well (as per sent comments against previous version).
> Sakari, can you add test cases at the same time?
> 
> > >  			return device_node_string(buf, end, ptr, spec, fmt + 1);
> 
> > > +			return fwnode_string(buf, end, ptr, spec, fmt + 1);
> > 
> > Why not pass fmt+2; we know that fmt+1 points at a 'w'. Just to avoid
> > doing the fmt++ inside fwnode_string().
> 
> I guess in order to be consistent with existing %pOF case. But wouldn't be
> better to fix %pOF for that sooner or later?

Agreed.

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 3/6] device property: Add a function to obtain a node's prefix
  2019-03-26 13:16   ` Andy Shevchenko
@ 2019-03-26 13:32     ` Sakari Ailus
  2019-03-27 12:36       ` Petr Mladek
  0 siblings, 1 reply; 35+ messages in thread
From: Sakari Ailus @ 2019-03-26 13:32 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Petr Mladek, linux-kernel, rafael, linux-acpi, devicetree, Rob Herring

Hi Andy,

On Tue, Mar 26, 2019 at 03:16:26PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 26, 2019 at 02:41:03PM +0200, Sakari Ailus wrote:
> > The prefix is used for printing purpose before a node, and it also works
> > as a separator between two nodes.
> > 
> 
> One nit below.
> 
> > +static const char *
> > +acpi_fwnode_get_name_prefix(const struct fwnode_handle *fwnode)
> > +{
> > +	struct fwnode_handle *parent;
> > +
> 
> > +	parent = fwnode_get_parent(fwnode);
> > +	/* Root node. */
> 
> I guess a comment could be easier to read if it goes before parent assignment
> line.

Only if the comments are changed accordingly. What we're doing here is
trying to figure out whether this is the root node. I can do that for v3.

> 
> > +	if (!parent)
> > +		return "";
> > +
> > +	parent = fwnode_get_next_parent(parent);
> > +	/* Second node from the root; no prefix here either. */
> 
> Ditto.
> 
> > +	if (!parent)
> > +		return "";
> > +
> > +	fwnode_handle_put(parent);
> > +
> > +	/* ACPI device or data node. */
> > +	return ".";
> > +}

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 6/6] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names
  2019-03-26 12:41 ` [PATCH v2 6/6] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names Sakari Ailus
  2019-03-26 13:11   ` Rasmus Villemoes
@ 2019-03-26 20:18   ` Joe Perches
       [not found]     ` <20190328114845.giusyarjibvg5ru7@kekkonen.localdomain>
  1 sibling, 1 reply; 35+ messages in thread
From: Joe Perches @ 2019-03-26 20:18 UTC (permalink / raw)
  To: Sakari Ailus, Petr Mladek, linux-kernel, rafael
  Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring

On Tue, 2019-03-26 at 14:41 +0200, Sakari Ailus wrote:
> Add support for %pfw conversion specifier (with "f" and "P" modifiers) to
> support printing full path of the node, including its name ("f") and only
> the node's name ("P") in the printk family of functions. The two flags
> have equivalent functionality to existing %pOF with the same two modifiers
> ("f" and "P") on OF based systems. The ability to do the same on ACPI
> based systems is added by this patch.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -5977,7 +5977,8 @@ sub process {
>  				while ($fmt =~ /(\%[\*\d\.]*p(\w))/g) {
>  					$specifier = $1;
>  					$extension = $2;
> -					if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOx]/) {
> +					if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOxf]/ ||
> +					    $extension =~ /f[^w]/) {
>  						$bad_specifier = $specifier;
>  						last;
>  					}

You also need to remove the bit about using %pF or %pf below this

					if ($bad_specifier =~ /p[Ff]/) {
						$ext_type = "Deprecated";
						$use = " - use %pS instead";
						$use =~ s/pS/ps/ if ($bad_specifier =~ /pf/);
					}



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 1/6] device property: Add functions for accessing node's parents
  2019-03-26 12:41 ` [PATCH v2 1/6] device property: Add functions for accessing node's parents Sakari Ailus
@ 2019-03-27 12:26   ` Petr Mladek
  2019-03-27 14:20     ` Sakari Ailus
  0 siblings, 1 reply; 35+ messages in thread
From: Petr Mladek @ 2019-03-27 12:26 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-kernel, rafael, Andy Shevchenko, linux-acpi, devicetree,
	Rob Herring

On Tue 2019-03-26 14:41:01, Sakari Ailus wrote:
> Add two convenience functions for accessing node's parents:
> 
> fwnode_count_parents() returns the number of parent nodes a given node
> has. fwnode_get_nth_parent() returns node's parent at a given distance
> from the node itself.
> 
> Also reorder fwnode_get_parent() in property.c according to the same order
> as in property.h.
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 8b91ab380d14..61eb6ceacc3f 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -554,17 +567,49 @@ struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
>  EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
>  
>  /**
> - * fwnode_get_parent - Return parent firwmare node
> - * @fwnode: Firmware whose parent is retrieved
> + * fwnode_count_parents - Return the number of parents a node has
> + * @fwnode: The node the parents of which are to be counted
>   *
> - * Return parent firmware node of the given node if possible or %NULL if no
> - * parent was available.
> + * Returns the number of parents a node has.
>   */
> -struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode)
> +unsigned int fwnode_count_parents(struct fwnode_handle *fwnode)
>  {
> -	return fwnode_call_ptr_op(fwnode, get_parent);
> +	unsigned int count;
> +
> +	fwnode_handle_get(fwnode);
> +
> +	for (count = 0; fwnode; count++)
> +		fwnode = fwnode_get_next_parent(fwnode);

Is it guaranteed that all parents stay when
fwnode_get_next_parent() releases the reference count
for each counted member?

> +
> +	return count - 1;

We could start counting from count = -1;

>  }
> -EXPORT_SYMBOL_GPL(fwnode_get_parent);
> +EXPORT_SYMBOL_GPL(fwnode_count_parents);
> +

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 3/6] device property: Add a function to obtain a node's prefix
  2019-03-26 13:32     ` Sakari Ailus
@ 2019-03-27 12:36       ` Petr Mladek
  2019-03-28 11:43         ` Sakari Ailus
  0 siblings, 1 reply; 35+ messages in thread
From: Petr Mladek @ 2019-03-27 12:36 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Andy Shevchenko, linux-kernel, rafael, linux-acpi, devicetree,
	Rob Herring

On Tue 2019-03-26 15:32:48, Sakari Ailus wrote:
> Hi Andy,
> 
> On Tue, Mar 26, 2019 at 03:16:26PM +0200, Andy Shevchenko wrote:
> > On Tue, Mar 26, 2019 at 02:41:03PM +0200, Sakari Ailus wrote:
> > > The prefix is used for printing purpose before a node, and it also works
> > > as a separator between two nodes.
> > > 
> > 
> > One nit below.
> > 
> > > +static const char *
> > > +acpi_fwnode_get_name_prefix(const struct fwnode_handle *fwnode)
> > > +{
> > > +	struct fwnode_handle *parent;
> > > +
> > 
> > > +	parent = fwnode_get_parent(fwnode);
> > > +	/* Root node. */
> > 
> > I guess a comment could be easier to read if it goes before parent assignment
> > line.
> 
> Only if the comments are changed accordingly. What we're doing here is
> trying to figure out whether this is the root node. I can do that for v3.

IMHO, the comment in acpi_fwnode_get_name() is easier to understand:

/* Is this the root node? */

> > 
> > > +	if (!parent)
> > > +		return "";
> > > +
> > > +	parent = fwnode_get_next_parent(parent);
> > > +	/* Second node from the root; no prefix here either. */
> > 
> > Ditto.

/* Is this 2nd node from the root? */

> > > +	if (!parent)
> > > +		return "";
> > > +
> > > +	fwnode_handle_put(parent);
> > > +
> > > +	/* ACPI device or data node. */
> > > +	return ".";
> > > +}

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 1/6] device property: Add functions for accessing node's parents
  2019-03-27 12:26   ` Petr Mladek
@ 2019-03-27 14:20     ` Sakari Ailus
  2019-03-28  9:38       ` Rafael J. Wysocki
  2019-03-28 14:10       ` Petr Mladek
  0 siblings, 2 replies; 35+ messages in thread
From: Sakari Ailus @ 2019-03-27 14:20 UTC (permalink / raw)
  To: Petr Mladek
  Cc: linux-kernel, rafael, Andy Shevchenko, linux-acpi, devicetree,
	Rob Herring

Hi Petr,

On Wed, Mar 27, 2019 at 01:26:25PM +0100, Petr Mladek wrote:
> On Tue 2019-03-26 14:41:01, Sakari Ailus wrote:
> > Add two convenience functions for accessing node's parents:
> > 
> > fwnode_count_parents() returns the number of parent nodes a given node
> > has. fwnode_get_nth_parent() returns node's parent at a given distance
> > from the node itself.
> > 
> > Also reorder fwnode_get_parent() in property.c according to the same order
> > as in property.h.
> > 
> > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > index 8b91ab380d14..61eb6ceacc3f 100644
> > --- a/drivers/base/property.c
> > +++ b/drivers/base/property.c
> > @@ -554,17 +567,49 @@ struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
> >  EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
> >  
> >  /**
> > - * fwnode_get_parent - Return parent firwmare node
> > - * @fwnode: Firmware whose parent is retrieved
> > + * fwnode_count_parents - Return the number of parents a node has
> > + * @fwnode: The node the parents of which are to be counted
> >   *
> > - * Return parent firmware node of the given node if possible or %NULL if no
> > - * parent was available.
> > + * Returns the number of parents a node has.
> >   */
> > -struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode)
> > +unsigned int fwnode_count_parents(struct fwnode_handle *fwnode)
> >  {
> > -	return fwnode_call_ptr_op(fwnode, get_parent);
> > +	unsigned int count;
> > +
> > +	fwnode_handle_get(fwnode);
> > +
> > +	for (count = 0; fwnode; count++)
> > +		fwnode = fwnode_get_next_parent(fwnode);
> 
> Is it guaranteed that all parents stay when
> fwnode_get_next_parent() releases the reference count
> for each counted member?

fwnode_get_next_parent() only releases the child node after it has acquired
the parent. The only implementation with refcounting for single nodes is
actually OF.

> 
> > +
> > +	return count - 1;
> 
> We could start counting from count = -1;

We could, but then count would need to be made signed (unless overflowing
is preferred instead).

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 1/6] device property: Add functions for accessing node's parents
  2019-03-27 14:20     ` Sakari Ailus
@ 2019-03-28  9:38       ` Rafael J. Wysocki
  2019-03-28 11:13         ` Sakari Ailus
  2019-03-28 14:10       ` Petr Mladek
  1 sibling, 1 reply; 35+ messages in thread
From: Rafael J. Wysocki @ 2019-03-28  9:38 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Petr Mladek, Linux Kernel Mailing List, Rafael J. Wysocki,
	Andy Shevchenko, ACPI Devel Maling List, devicetree, Rob Herring

On Wed, Mar 27, 2019 at 3:20 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Petr,
>
> On Wed, Mar 27, 2019 at 01:26:25PM +0100, Petr Mladek wrote:
> > On Tue 2019-03-26 14:41:01, Sakari Ailus wrote:
> > > Add two convenience functions for accessing node's parents:
> > >
> > > fwnode_count_parents() returns the number of parent nodes a given node
> > > has. fwnode_get_nth_parent() returns node's parent at a given distance
> > > from the node itself.
> > >
> > > Also reorder fwnode_get_parent() in property.c according to the same order
> > > as in property.h.
> > >
> > > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > > index 8b91ab380d14..61eb6ceacc3f 100644
> > > --- a/drivers/base/property.c
> > > +++ b/drivers/base/property.c
> > > @@ -554,17 +567,49 @@ struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
> > >  EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
> > >
> > >  /**
> > > - * fwnode_get_parent - Return parent firwmare node
> > > - * @fwnode: Firmware whose parent is retrieved
> > > + * fwnode_count_parents - Return the number of parents a node has
> > > + * @fwnode: The node the parents of which are to be counted
> > >   *
> > > - * Return parent firmware node of the given node if possible or %NULL if no
> > > - * parent was available.
> > > + * Returns the number of parents a node has.
> > >   */
> > > -struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode)
> > > +unsigned int fwnode_count_parents(struct fwnode_handle *fwnode)
> > >  {
> > > -   return fwnode_call_ptr_op(fwnode, get_parent);
> > > +   unsigned int count;
> > > +
> > > +   fwnode_handle_get(fwnode);
> > > +
> > > +   for (count = 0; fwnode; count++)
> > > +           fwnode = fwnode_get_next_parent(fwnode);
> >
> > Is it guaranteed that all parents stay when
> > fwnode_get_next_parent() releases the reference count
> > for each counted member?
>
> fwnode_get_next_parent() only releases the child node after it has acquired
> the parent. The only implementation with refcounting for single nodes is
> actually OF.
>
> >
> > > +
> > > +   return count - 1;
> >
> > We could start counting from count = -1;
>
> We could, but then count would need to be made signed (unless overflowing
> is preferred instead).

What if there are no parents?

Or is it guaranteed that there always will be at least one?

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 2/6] device property: Add fwnode_get_name for returning the name of a node
  2019-03-26 12:41 ` [PATCH v2 2/6] device property: Add fwnode_get_name for returning the name of a node Sakari Ailus
@ 2019-03-28  9:45   ` Rafael J. Wysocki
  2019-03-28 11:31     ` Sakari Ailus
  2019-03-31  6:42   ` Rob Herring
  1 sibling, 1 reply; 35+ messages in thread
From: Rafael J. Wysocki @ 2019-03-28  9:45 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Petr Mladek, Linux Kernel Mailing List, Rafael J. Wysocki,
	Andy Shevchenko, ACPI Devel Maling List, devicetree, Rob Herring

On Tue, Mar 26, 2019 at 1:41 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> The fwnode framework did not have means to obtain the name of a node. Add
> that now, in form of the fwnode_get_name() function and a corresponding
> get_name fwnode op. OF and ACPI support is included.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/acpi/property.c  | 24 ++++++++++++++++++++++++
>  drivers/base/property.c  | 11 +++++++++++
>  drivers/of/property.c    |  6 ++++++
>  include/linux/fwnode.h   |  2 ++
>  include/linux/property.h |  1 +
>  5 files changed, 44 insertions(+)
>
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 77abe0ec4043..8c9a4c02cde2 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -1291,6 +1291,29 @@ acpi_fwnode_get_reference_args(const struct fwnode_handle *fwnode,
>                                                   args_count, args);
>  }
>
> +static const char *acpi_fwnode_get_name(const struct fwnode_handle *fwnode)
> +{
> +       const struct acpi_device *adev;
> +       struct fwnode_handle *parent;
> +
> +       parent = fwnode_get_parent(fwnode);
> +       /* Is this the root node? */
> +       if (!parent)
> +               return "\\";
> +
> +       fwnode_handle_put(parent);
> +
> +       if (is_acpi_data_node(fwnode)) {
> +               const struct acpi_data_node *dn = to_acpi_data_node(fwnode);
> +
> +               return dn->name;
> +       }
> +
> +       adev = to_acpi_device_node(fwnode);
> +
> +       return adev ? acpi_device_bid(adev) : NULL;

I would do

if (WARN_ON(!adev))
         return NULL;

return acpi_device_bid(adev);

> +}
> +
>  static struct fwnode_handle *
>  acpi_fwnode_get_parent(struct fwnode_handle *fwnode)
>  {
> @@ -1331,6 +1354,7 @@ acpi_fwnode_device_get_match_data(const struct fwnode_handle *fwnode,
>                 .get_parent = acpi_node_get_parent,                     \
>                 .get_next_child_node = acpi_get_next_subnode,           \
>                 .get_named_child_node = acpi_fwnode_get_named_child_node, \
> +               .get_name = acpi_fwnode_get_name,                       \
>                 .get_reference_args = acpi_fwnode_get_reference_args,   \
>                 .graph_get_next_endpoint =                              \
>                         acpi_graph_get_next_endpoint,                   \
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 61eb6ceacc3f..04a8591cd1b0 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -533,6 +533,17 @@ int device_add_properties(struct device *dev,
>  EXPORT_SYMBOL_GPL(device_add_properties);
>
>  /**
> + * fwnode_get_name - Return the name of a node
> + * @fwnode: The firmware node
> + *
> + * Returns a pointer to the node name.
> + */
> +const char *fwnode_get_name(const struct fwnode_handle *fwnode)
> +{
> +       return fwnode_call_ptr_op(fwnode, get_name);
> +}
> +
> +/**
>   * fwnode_get_parent - Return parent firwmare node
>   * @fwnode: Firmware whose parent is retrieved
>   *
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 8631efa1daa1..f0a7b78f2d9f 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -872,6 +872,11 @@ of_fwnode_property_read_string_array(const struct fwnode_handle *fwnode,
>                 of_property_count_strings(node, propname);
>  }
>
> +static const char *of_fwnode_get_name(const struct fwnode_handle *fwnode)
> +{
> +       return kbasename(to_of_node(fwnode)->full_name);
> +}
> +
>  static struct fwnode_handle *
>  of_fwnode_get_parent(const struct fwnode_handle *fwnode)
>  {
> @@ -993,6 +998,7 @@ const struct fwnode_operations of_fwnode_ops = {
>         .property_present = of_fwnode_property_present,
>         .property_read_int_array = of_fwnode_property_read_int_array,
>         .property_read_string_array = of_fwnode_property_read_string_array,
> +       .get_name = of_fwnode_get_name,
>         .get_parent = of_fwnode_get_parent,
>         .get_next_child_node = of_fwnode_get_next_child_node,
>         .get_named_child_node = of_fwnode_get_named_child_node,
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index faebf0ca0686..85b7fa4dc727 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -59,6 +59,7 @@ struct fwnode_reference_args {
>   *                              otherwise.
>   * @property_read_string_array: Read an array of string properties. Return zero
>   *                             on success, a negative error code otherwise.
> + * @get_name: Return the name of an fwnode.
>   * @get_parent: Return the parent of an fwnode.
>   * @get_next_child_node: Return the next child node in an iteration.
>   * @get_named_child_node: Return a child node with a given name.
> @@ -85,6 +86,7 @@ struct fwnode_operations {
>         (*property_read_string_array)(const struct fwnode_handle *fwnode_handle,
>                                       const char *propname, const char **val,
>                                       size_t nval);
> +       const char *(*get_name)(const struct fwnode_handle *fwnode);
>         struct fwnode_handle *(*get_parent)(const struct fwnode_handle *fwnode);
>         struct fwnode_handle *
>         (*get_next_child_node)(const struct fwnode_handle *fwnode,
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 1164ac974ee8..2f5df900e310 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -78,6 +78,7 @@ int fwnode_property_get_reference_args(const struct fwnode_handle *fwnode,
>                                        unsigned int nargs, unsigned int index,
>                                        struct fwnode_reference_args *args);
>
> +const char *fwnode_get_name(const struct fwnode_handle *fwnode);
>  struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode);
>  struct fwnode_handle *fwnode_get_next_parent(
>         struct fwnode_handle *fwnode);
> --

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 1/6] device property: Add functions for accessing node's parents
  2019-03-28  9:38       ` Rafael J. Wysocki
@ 2019-03-28 11:13         ` Sakari Ailus
  2019-03-28 14:52           ` Rafael J. Wysocki
  0 siblings, 1 reply; 35+ messages in thread
From: Sakari Ailus @ 2019-03-28 11:13 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Petr Mladek, Linux Kernel Mailing List, Andy Shevchenko,
	ACPI Devel Maling List, devicetree, Rob Herring

Hi Rafael,

On Thu, Mar 28, 2019 at 10:38:01AM +0100, Rafael J. Wysocki wrote:
> On Wed, Mar 27, 2019 at 3:20 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Petr,
> >
> > On Wed, Mar 27, 2019 at 01:26:25PM +0100, Petr Mladek wrote:
> > > On Tue 2019-03-26 14:41:01, Sakari Ailus wrote:
> > > > Add two convenience functions for accessing node's parents:
> > > >
> > > > fwnode_count_parents() returns the number of parent nodes a given node
> > > > has. fwnode_get_nth_parent() returns node's parent at a given distance
> > > > from the node itself.
> > > >
> > > > Also reorder fwnode_get_parent() in property.c according to the same order
> > > > as in property.h.
> > > >
> > > > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > > > index 8b91ab380d14..61eb6ceacc3f 100644
> > > > --- a/drivers/base/property.c
> > > > +++ b/drivers/base/property.c
> > > > @@ -554,17 +567,49 @@ struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
> > > >  EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
> > > >
> > > >  /**
> > > > - * fwnode_get_parent - Return parent firwmare node
> > > > - * @fwnode: Firmware whose parent is retrieved
> > > > + * fwnode_count_parents - Return the number of parents a node has
> > > > + * @fwnode: The node the parents of which are to be counted
> > > >   *
> > > > - * Return parent firmware node of the given node if possible or %NULL if no
> > > > - * parent was available.
> > > > + * Returns the number of parents a node has.
> > > >   */
> > > > -struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode)
> > > > +unsigned int fwnode_count_parents(struct fwnode_handle *fwnode)
> > > >  {
> > > > -   return fwnode_call_ptr_op(fwnode, get_parent);
> > > > +   unsigned int count;
> > > > +
> > > > +   fwnode_handle_get(fwnode);
> > > > +
> > > > +   for (count = 0; fwnode; count++)
> > > > +           fwnode = fwnode_get_next_parent(fwnode);
> > >
> > > Is it guaranteed that all parents stay when
> > > fwnode_get_next_parent() releases the reference count
> > > for each counted member?
> >
> > fwnode_get_next_parent() only releases the child node after it has acquired
> > the parent. The only implementation with refcounting for single nodes is
> > actually OF.
> >
> > >
> > > > +
> > > > +   return count - 1;
> > >
> > > We could start counting from count = -1;
> >
> > We could, but then count would need to be made signed (unless overflowing
> > is preferred instead).
> 
> What if there are no parents?
> 
> Or is it guaranteed that there always will be at least one?

If there are no parents, the function will return 0 --- the node itself is
counted in the loop, too, hence the need to decrement by one.

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 2/6] device property: Add fwnode_get_name for returning the name of a node
  2019-03-28  9:45   ` Rafael J. Wysocki
@ 2019-03-28 11:31     ` Sakari Ailus
  0 siblings, 0 replies; 35+ messages in thread
From: Sakari Ailus @ 2019-03-28 11:31 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Petr Mladek, Linux Kernel Mailing List, Andy Shevchenko,
	ACPI Devel Maling List, devicetree, Rob Herring

Hi Rafael,

On Thu, Mar 28, 2019 at 10:45:57AM +0100, Rafael J. Wysocki wrote:
> On Tue, Mar 26, 2019 at 1:41 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > The fwnode framework did not have means to obtain the name of a node. Add
> > that now, in form of the fwnode_get_name() function and a corresponding
> > get_name fwnode op. OF and ACPI support is included.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/acpi/property.c  | 24 ++++++++++++++++++++++++
> >  drivers/base/property.c  | 11 +++++++++++
> >  drivers/of/property.c    |  6 ++++++
> >  include/linux/fwnode.h   |  2 ++
> >  include/linux/property.h |  1 +
> >  5 files changed, 44 insertions(+)
> >
> > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> > index 77abe0ec4043..8c9a4c02cde2 100644
> > --- a/drivers/acpi/property.c
> > +++ b/drivers/acpi/property.c
> > @@ -1291,6 +1291,29 @@ acpi_fwnode_get_reference_args(const struct fwnode_handle *fwnode,
> >                                                   args_count, args);
> >  }
> >
> > +static const char *acpi_fwnode_get_name(const struct fwnode_handle *fwnode)
> > +{
> > +       const struct acpi_device *adev;
> > +       struct fwnode_handle *parent;
> > +
> > +       parent = fwnode_get_parent(fwnode);
> > +       /* Is this the root node? */
> > +       if (!parent)
> > +               return "\\";
> > +
> > +       fwnode_handle_put(parent);
> > +
> > +       if (is_acpi_data_node(fwnode)) {
> > +               const struct acpi_data_node *dn = to_acpi_data_node(fwnode);
> > +
> > +               return dn->name;
> > +       }
> > +
> > +       adev = to_acpi_device_node(fwnode);
> > +
> > +       return adev ? acpi_device_bid(adev) : NULL;
> 
> I would do
> 
> if (WARN_ON(!adev))
>          return NULL;
> 
> return acpi_device_bid(adev);

Done.

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 3/6] device property: Add a function to obtain a node's prefix
  2019-03-27 12:36       ` Petr Mladek
@ 2019-03-28 11:43         ` Sakari Ailus
  0 siblings, 0 replies; 35+ messages in thread
From: Sakari Ailus @ 2019-03-28 11:43 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Andy Shevchenko, linux-kernel, rafael, linux-acpi, devicetree,
	Rob Herring

On Wed, Mar 27, 2019 at 01:36:59PM +0100, Petr Mladek wrote:
> On Tue 2019-03-26 15:32:48, Sakari Ailus wrote:
> > Hi Andy,
> > 
> > On Tue, Mar 26, 2019 at 03:16:26PM +0200, Andy Shevchenko wrote:
> > > On Tue, Mar 26, 2019 at 02:41:03PM +0200, Sakari Ailus wrote:
> > > > The prefix is used for printing purpose before a node, and it also works
> > > > as a separator between two nodes.
> > > > 
> > > 
> > > One nit below.
> > > 
> > > > +static const char *
> > > > +acpi_fwnode_get_name_prefix(const struct fwnode_handle *fwnode)
> > > > +{
> > > > +	struct fwnode_handle *parent;
> > > > +
> > > 
> > > > +	parent = fwnode_get_parent(fwnode);
> > > > +	/* Root node. */
> > > 
> > > I guess a comment could be easier to read if it goes before parent assignment
> > > line.
> > 
> > Only if the comments are changed accordingly. What we're doing here is
> > trying to figure out whether this is the root node. I can do that for v3.
> 
> IMHO, the comment in acpi_fwnode_get_name() is easier to understand:
> 
> /* Is this the root node? */
> 
> > > 
> > > > +	if (!parent)
> > > > +		return "";
> > > > +
> > > > +	parent = fwnode_get_next_parent(parent);
> > > > +	/* Second node from the root; no prefix here either. */
> > > 
> > > Ditto.
> 
> /* Is this 2nd node from the root? */

I used both the strings.

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 6/6] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names
       [not found]     ` <20190328114845.giusyarjibvg5ru7@kekkonen.localdomain>
@ 2019-03-28 11:51       ` Sakari Ailus
  0 siblings, 0 replies; 35+ messages in thread
From: Sakari Ailus @ 2019-03-28 11:51 UTC (permalink / raw)
  To: Joe Perches
  Cc: Petr Mladek, linux-kernel, rafael, Andy Shevchenko, linux-acpi,
	devicetree, Rob Herring

On Thu, Mar 28, 2019 at 01:48:45PM +0200, Sakari Ailus wrote:
> I actually intended to leave this --- it points to anyone accidentally
> trying to use %pf to what it needs to be replaced with. But the Deprecated
> should be replaced with Invalid (i.e. remove that assignment).

This is actually already a part of an earlier patch, "[PATCH v2 4/6]
lib/vsprintf: Remove support for %pF and %pf in favour of %pS and %ps".

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 1/6] device property: Add functions for accessing node's parents
  2019-03-27 14:20     ` Sakari Ailus
  2019-03-28  9:38       ` Rafael J. Wysocki
@ 2019-03-28 14:10       ` Petr Mladek
  2019-03-29 13:04         ` Sakari Ailus
  1 sibling, 1 reply; 35+ messages in thread
From: Petr Mladek @ 2019-03-28 14:10 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-kernel, rafael, Andy Shevchenko, linux-acpi, devicetree,
	Rob Herring

On Wed 2019-03-27 16:20:37, Sakari Ailus wrote:
> Hi Petr,
> 
> On Wed, Mar 27, 2019 at 01:26:25PM +0100, Petr Mladek wrote:
> > On Tue 2019-03-26 14:41:01, Sakari Ailus wrote:
> > > Add two convenience functions for accessing node's parents:
> > > 
> > > fwnode_count_parents() returns the number of parent nodes a given node
> > > has. fwnode_get_nth_parent() returns node's parent at a given distance
> > > from the node itself.
> > > 
> > > Also reorder fwnode_get_parent() in property.c according to the same order
> > > as in property.h.
> > > 
> > > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > > index 8b91ab380d14..61eb6ceacc3f 100644
> > > --- a/drivers/base/property.c
> > > +++ b/drivers/base/property.c
> > > @@ -554,17 +567,49 @@ struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
> > >  EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
> > >  
> > >  /**
> > > - * fwnode_get_parent - Return parent firwmare node
> > > - * @fwnode: Firmware whose parent is retrieved
> > > + * fwnode_count_parents - Return the number of parents a node has
> > > + * @fwnode: The node the parents of which are to be counted
> > >   *
> > > - * Return parent firmware node of the given node if possible or %NULL if no
> > > - * parent was available.
> > > + * Returns the number of parents a node has.
> > >   */
> > > -struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode)
> > > +unsigned int fwnode_count_parents(struct fwnode_handle *fwnode)
> > >  {
> > > -	return fwnode_call_ptr_op(fwnode, get_parent);
> > > +	unsigned int count;
> > > +
> > > +	fwnode_handle_get(fwnode);
> > > +
> > > +	for (count = 0; fwnode; count++)
> > > +		fwnode = fwnode_get_next_parent(fwnode);
> > 
> > Is it guaranteed that all parents stay when
> > fwnode_get_next_parent() releases the reference count
> > for each counted member?
> 
> fwnode_get_next_parent() only releases the child node after it has acquired
> the parent. The only implementation with refcounting for single nodes is
> actually OF.

I am still a bit confused. The function is later used the following way:

	for (depth = fwnode_count_parents(fwnode); depth >= 0; depth--) {
		struct fwnode_handle *__fwnode =
			fwnode_get_nth_parent(fwnode, depth);

		buf = string(buf, end, fwnode_get_name_prefix(__fwnode),
			     default_str_spec);
		buf = string(buf, end, fwnode_get_name(__fwnode),
			     default_str_spec);

		fwnode_handle_put(__fwnode);
	}

It uses the number to walk the hierarchy over and over again to print
the path from root.

I would expect that parents could not get removed until all children
are removed. And that the most bottom child (fwnode) must not get
removed. Otherwise we would access freed memory in each cycle.

Then there is a question why we need to get the reference of the
parents
at all.

It might be because we do not need touch refcounting of the parents
in this scenario. But we are re-using existing functions where
the ref-counting hang-over is important.

Or we really need to increase refcounting of parents. Then it looks
suspicious because we want to traverse the same path several times
and always have only one node locked (refcounted).

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 6/6] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names
  2019-03-26 13:24     ` Andy Shevchenko
  2019-03-26 13:31       ` Sakari Ailus
@ 2019-03-28 14:29       ` Petr Mladek
  2019-03-29 13:10         ` Sakari Ailus
  1 sibling, 1 reply; 35+ messages in thread
From: Petr Mladek @ 2019-03-28 14:29 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rasmus Villemoes, Sakari Ailus, linux-kernel, rafael, linux-acpi,
	devicetree, Rob Herring

On Tue 2019-03-26 15:24:50, Andy Shevchenko wrote:
> On Tue, Mar 26, 2019 at 02:11:35PM +0100, Rasmus Villemoes wrote:
> > On 26/03/2019 13.41, Sakari Ailus wrote:
> > > Add support for %pfw conversion specifier (with "f" and "P" modifiers) to
> > > support printing full path of the node, including its name ("f") and only
> > > the node's name ("P") in the printk family of functions. The two flags
> > > have equivalent functionality to existing %pOF with the same two modifiers
> > > ("f" and "P") on OF based systems. The ability to do the same on ACPI
> > > based systems is added by this patch.
> 
> > > +	for (pass = false; strspn(fmt, modifiers); fmt++, pass = true) {
> > > +		if (pass) {
> > > +			if (buf < end)
> > > +				*buf = ':';
> > > +			buf++;
> > > +		}
> > > +
> > > +		switch (*fmt) {
> > > +		case 'f':	/* full_name */
> > > +			buf = fwnode_gen_full_name(fwnode, buf, end);
> > > +			break;
> > > +		case 'P':	/* name */
> > > +			buf = string(buf, end, fwnode_get_name(fwnode),
> > > +				     str_spec);
> > > +			break;
> > > +		default:
> > > +			break;
> > > +		}
> > > +	}
> > 
> > This seems awfully complicated. Why would anyone ever pass more than one
> > of 'f' and 'P'? Why not just
> > 
> > switch(*fmt) {
> > case 'P':
> >    ...
> > case 'f':
> > default:
> >    ...
> > }
> > 
> > which avoids the loop and the strcspn. Or, drop the default: case and
> > don't have logic at all for falling back to 'f' if neither is present.
> > 
> > > +	return widen_string(buf, buf - buf_start, end, spec);
> > > +}
> 
> My point as well (as per sent comments against previous version).
> Sakari, can you add test cases at the same time?
> 
> > >  			return device_node_string(buf, end, ptr, spec, fmt + 1);
> 
> > > +			return fwnode_string(buf, end, ptr, spec, fmt + 1);
> > 
> > Why not pass fmt+2; we know that fmt+1 points at a 'w'. Just to avoid
> > doing the fmt++ inside fwnode_string().
> 
> I guess in order to be consistent with existing %pOF case. But wouldn't be
> better to fix %pOF for that sooner or later?

Good question. Are there any %pOF users that would want to use the for
cycle and printk more values by single %pOF? Would the output be human
readable without any delimiters or words around?

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 1/6] device property: Add functions for accessing node's parents
  2019-03-28 11:13         ` Sakari Ailus
@ 2019-03-28 14:52           ` Rafael J. Wysocki
  2019-03-29 13:15             ` Sakari Ailus
  0 siblings, 1 reply; 35+ messages in thread
From: Rafael J. Wysocki @ 2019-03-28 14:52 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Rafael J. Wysocki, Petr Mladek, Linux Kernel Mailing List,
	Andy Shevchenko, ACPI Devel Maling List, devicetree, Rob Herring

On Thu, Mar 28, 2019 at 12:13 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Rafael,
>
> On Thu, Mar 28, 2019 at 10:38:01AM +0100, Rafael J. Wysocki wrote:
> > On Wed, Mar 27, 2019 at 3:20 PM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > >
> > > Hi Petr,
> > >
> > > On Wed, Mar 27, 2019 at 01:26:25PM +0100, Petr Mladek wrote:
> > > > On Tue 2019-03-26 14:41:01, Sakari Ailus wrote:
> > > > > Add two convenience functions for accessing node's parents:
> > > > >
> > > > > fwnode_count_parents() returns the number of parent nodes a given node
> > > > > has. fwnode_get_nth_parent() returns node's parent at a given distance
> > > > > from the node itself.
> > > > >
> > > > > Also reorder fwnode_get_parent() in property.c according to the same order
> > > > > as in property.h.
> > > > >
> > > > > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > > > > index 8b91ab380d14..61eb6ceacc3f 100644
> > > > > --- a/drivers/base/property.c
> > > > > +++ b/drivers/base/property.c
> > > > > @@ -554,17 +567,49 @@ struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
> > > > >  EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
> > > > >
> > > > >  /**
> > > > > - * fwnode_get_parent - Return parent firwmare node
> > > > > - * @fwnode: Firmware whose parent is retrieved
> > > > > + * fwnode_count_parents - Return the number of parents a node has
> > > > > + * @fwnode: The node the parents of which are to be counted
> > > > >   *
> > > > > - * Return parent firmware node of the given node if possible or %NULL if no
> > > > > - * parent was available.
> > > > > + * Returns the number of parents a node has.
> > > > >   */
> > > > > -struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode)
> > > > > +unsigned int fwnode_count_parents(struct fwnode_handle *fwnode)
> > > > >  {
> > > > > -   return fwnode_call_ptr_op(fwnode, get_parent);
> > > > > +   unsigned int count;
> > > > > +
> > > > > +   fwnode_handle_get(fwnode);
> > > > > +
> > > > > +   for (count = 0; fwnode; count++)
> > > > > +           fwnode = fwnode_get_next_parent(fwnode);
> > > >
> > > > Is it guaranteed that all parents stay when
> > > > fwnode_get_next_parent() releases the reference count
> > > > for each counted member?
> > >
> > > fwnode_get_next_parent() only releases the child node after it has acquired
> > > the parent. The only implementation with refcounting for single nodes is
> > > actually OF.
> > >
> > > >
> > > > > +
> > > > > +   return count - 1;
> > > >
> > > > We could start counting from count = -1;
> > >
> > > We could, but then count would need to be made signed (unless overflowing
> > > is preferred instead).
> >
> > What if there are no parents?
> >
> > Or is it guaranteed that there always will be at least one?
>
> If there are no parents, the function will return 0 --- the node itself is
> counted in the loop, too, hence the need to decrement by one.

Right, sorry.

Then I don't see why count cannot be int.  Or call it "iter" to avoid
confusion with negative "counts". :-)

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 1/6] device property: Add functions for accessing node's parents
  2019-03-28 14:10       ` Petr Mladek
@ 2019-03-29 13:04         ` Sakari Ailus
  2019-03-29 14:14           ` Andy Shevchenko
  0 siblings, 1 reply; 35+ messages in thread
From: Sakari Ailus @ 2019-03-29 13:04 UTC (permalink / raw)
  To: Petr Mladek
  Cc: linux-kernel, rafael, Andy Shevchenko, linux-acpi, devicetree,
	Rob Herring

Hi Petr,

On Thu, Mar 28, 2019 at 03:10:36PM +0100, Petr Mladek wrote:
> On Wed 2019-03-27 16:20:37, Sakari Ailus wrote:
> > Hi Petr,
> > 
> > On Wed, Mar 27, 2019 at 01:26:25PM +0100, Petr Mladek wrote:
> > > On Tue 2019-03-26 14:41:01, Sakari Ailus wrote:
> > > > Add two convenience functions for accessing node's parents:
> > > > 
> > > > fwnode_count_parents() returns the number of parent nodes a given node
> > > > has. fwnode_get_nth_parent() returns node's parent at a given distance
> > > > from the node itself.
> > > > 
> > > > Also reorder fwnode_get_parent() in property.c according to the same order
> > > > as in property.h.
> > > > 
> > > > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > > > index 8b91ab380d14..61eb6ceacc3f 100644
> > > > --- a/drivers/base/property.c
> > > > +++ b/drivers/base/property.c
> > > > @@ -554,17 +567,49 @@ struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
> > > >  EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
> > > >  
> > > >  /**
> > > > - * fwnode_get_parent - Return parent firwmare node
> > > > - * @fwnode: Firmware whose parent is retrieved
> > > > + * fwnode_count_parents - Return the number of parents a node has
> > > > + * @fwnode: The node the parents of which are to be counted
> > > >   *
> > > > - * Return parent firmware node of the given node if possible or %NULL if no
> > > > - * parent was available.
> > > > + * Returns the number of parents a node has.
> > > >   */
> > > > -struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode)
> > > > +unsigned int fwnode_count_parents(struct fwnode_handle *fwnode)
> > > >  {
> > > > -	return fwnode_call_ptr_op(fwnode, get_parent);
> > > > +	unsigned int count;
> > > > +
> > > > +	fwnode_handle_get(fwnode);
> > > > +
> > > > +	for (count = 0; fwnode; count++)
> > > > +		fwnode = fwnode_get_next_parent(fwnode);
> > > 
> > > Is it guaranteed that all parents stay when
> > > fwnode_get_next_parent() releases the reference count
> > > for each counted member?
> > 
> > fwnode_get_next_parent() only releases the child node after it has acquired
> > the parent. The only implementation with refcounting for single nodes is
> > actually OF.
> 
> I am still a bit confused. The function is later used the following way:
> 
> 	for (depth = fwnode_count_parents(fwnode); depth >= 0; depth--) {
> 		struct fwnode_handle *__fwnode =
> 			fwnode_get_nth_parent(fwnode, depth);
> 
> 		buf = string(buf, end, fwnode_get_name_prefix(__fwnode),
> 			     default_str_spec);
> 		buf = string(buf, end, fwnode_get_name(__fwnode),
> 			     default_str_spec);
> 
> 		fwnode_handle_put(__fwnode);
> 	}
> 
> It uses the number to walk the hierarchy over and over again to print
> the path from root.
> 
> I would expect that parents could not get removed until all children
> are removed. And that the most bottom child (fwnode) must not get
> removed. Otherwise we would access freed memory in each cycle.
> 
> Then there is a question why we need to get the reference of the
> parents
> at all.
> 
> It might be because we do not need touch refcounting of the parents
> in this scenario. But we are re-using existing functions where
> the ref-counting hang-over is important.
> 
> Or we really need to increase refcounting of parents. Then it looks
> suspicious because we want to traverse the same path several times
> and always have only one node locked (refcounted).

I hope I understand your concern better this time.

You basically have to have a reference to a node until you're done
accessing it. The refcounting in the fwnode framework is based on the OF
API --- otherwise you couldn't access OF nodes using the framework. ACPI
does not have those (as you can't dynamically remove nodes). The newly
added swnode does have refcounting of individual nodes as well.

Even if a node's parent did disappear while printing an fwnode full name,
it'd mean that

1) fwnode_count_parents() returned an incorrect value (less than expected)
   or

2) some of the node names could be NULL if node references could not be
   obtained.

Both would result into garbled output in that case. I have to admit I'm not
sure if this is possible with OF.

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 6/6] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names
  2019-03-28 14:29       ` Petr Mladek
@ 2019-03-29 13:10         ` Sakari Ailus
  0 siblings, 0 replies; 35+ messages in thread
From: Sakari Ailus @ 2019-03-29 13:10 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Andy Shevchenko, Rasmus Villemoes, linux-kernel, rafael,
	linux-acpi, devicetree, Rob Herring

Hi Petr,

On Thu, Mar 28, 2019 at 03:29:27PM +0100, Petr Mladek wrote:
> On Tue 2019-03-26 15:24:50, Andy Shevchenko wrote:
> > On Tue, Mar 26, 2019 at 02:11:35PM +0100, Rasmus Villemoes wrote:
> > > On 26/03/2019 13.41, Sakari Ailus wrote:
> > > > Add support for %pfw conversion specifier (with "f" and "P" modifiers) to
> > > > support printing full path of the node, including its name ("f") and only
> > > > the node's name ("P") in the printk family of functions. The two flags
> > > > have equivalent functionality to existing %pOF with the same two modifiers
> > > > ("f" and "P") on OF based systems. The ability to do the same on ACPI
> > > > based systems is added by this patch.
> > 
> > > > +	for (pass = false; strspn(fmt, modifiers); fmt++, pass = true) {
> > > > +		if (pass) {
> > > > +			if (buf < end)
> > > > +				*buf = ':';
> > > > +			buf++;
> > > > +		}
> > > > +
> > > > +		switch (*fmt) {
> > > > +		case 'f':	/* full_name */
> > > > +			buf = fwnode_gen_full_name(fwnode, buf, end);
> > > > +			break;
> > > > +		case 'P':	/* name */
> > > > +			buf = string(buf, end, fwnode_get_name(fwnode),
> > > > +				     str_spec);
> > > > +			break;
> > > > +		default:
> > > > +			break;
> > > > +		}
> > > > +	}
> > > 
> > > This seems awfully complicated. Why would anyone ever pass more than one
> > > of 'f' and 'P'? Why not just
> > > 
> > > switch(*fmt) {
> > > case 'P':
> > >    ...
> > > case 'f':
> > > default:
> > >    ...
> > > }
> > > 
> > > which avoids the loop and the strcspn. Or, drop the default: case and
> > > don't have logic at all for falling back to 'f' if neither is present.
> > > 
> > > > +	return widen_string(buf, buf - buf_start, end, spec);
> > > > +}
> > 
> > My point as well (as per sent comments against previous version).
> > Sakari, can you add test cases at the same time?
> > 
> > > >  			return device_node_string(buf, end, ptr, spec, fmt + 1);
> > 
> > > > +			return fwnode_string(buf, end, ptr, spec, fmt + 1);
> > > 
> > > Why not pass fmt+2; we know that fmt+1 points at a 'w'. Just to avoid
> > > doing the fmt++ inside fwnode_string().
> > 
> > I guess in order to be consistent with existing %pOF case. But wouldn't be
> > better to fix %pOF for that sooner or later?
> 
> Good question. Are there any %pOF users that would want to use the for
> cycle and printk more values by single %pOF? Would the output be human
> readable without any delimiters or words around?

The delimiter is ':'. What comes to users, quick grepping tells the only
users are actually the unit tests, and this is also present in the
documentation.

Based on that I think we can safely omit that on %pfw actually. I thought
it was actually used somewhere.

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 1/6] device property: Add functions for accessing node's parents
  2019-03-28 14:52           ` Rafael J. Wysocki
@ 2019-03-29 13:15             ` Sakari Ailus
  2019-03-29 14:15               ` Andy Shevchenko
  2019-03-29 23:00               ` Rafael J. Wysocki
  0 siblings, 2 replies; 35+ messages in thread
From: Sakari Ailus @ 2019-03-29 13:15 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Petr Mladek, Linux Kernel Mailing List, Andy Shevchenko,
	ACPI Devel Maling List, devicetree, Rob Herring

On Thu, Mar 28, 2019 at 03:52:35PM +0100, Rafael J. Wysocki wrote:
> On Thu, Mar 28, 2019 at 12:13 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Rafael,
> >
> > On Thu, Mar 28, 2019 at 10:38:01AM +0100, Rafael J. Wysocki wrote:
> > > On Wed, Mar 27, 2019 at 3:20 PM Sakari Ailus
> > > <sakari.ailus@linux.intel.com> wrote:
> > > >
> > > > Hi Petr,
> > > >
> > > > On Wed, Mar 27, 2019 at 01:26:25PM +0100, Petr Mladek wrote:
> > > > > On Tue 2019-03-26 14:41:01, Sakari Ailus wrote:
> > > > > > Add two convenience functions for accessing node's parents:
> > > > > >
> > > > > > fwnode_count_parents() returns the number of parent nodes a given node
> > > > > > has. fwnode_get_nth_parent() returns node's parent at a given distance
> > > > > > from the node itself.
> > > > > >
> > > > > > Also reorder fwnode_get_parent() in property.c according to the same order
> > > > > > as in property.h.
> > > > > >
> > > > > > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > > > > > index 8b91ab380d14..61eb6ceacc3f 100644
> > > > > > --- a/drivers/base/property.c
> > > > > > +++ b/drivers/base/property.c
> > > > > > @@ -554,17 +567,49 @@ struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
> > > > > >  EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
> > > > > >
> > > > > >  /**
> > > > > > - * fwnode_get_parent - Return parent firwmare node
> > > > > > - * @fwnode: Firmware whose parent is retrieved
> > > > > > + * fwnode_count_parents - Return the number of parents a node has
> > > > > > + * @fwnode: The node the parents of which are to be counted
> > > > > >   *
> > > > > > - * Return parent firmware node of the given node if possible or %NULL if no
> > > > > > - * parent was available.
> > > > > > + * Returns the number of parents a node has.
> > > > > >   */
> > > > > > -struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode)
> > > > > > +unsigned int fwnode_count_parents(struct fwnode_handle *fwnode)
> > > > > >  {
> > > > > > -   return fwnode_call_ptr_op(fwnode, get_parent);
> > > > > > +   unsigned int count;
> > > > > > +
> > > > > > +   fwnode_handle_get(fwnode);
> > > > > > +
> > > > > > +   for (count = 0; fwnode; count++)
> > > > > > +           fwnode = fwnode_get_next_parent(fwnode);
> > > > >
> > > > > Is it guaranteed that all parents stay when
> > > > > fwnode_get_next_parent() releases the reference count
> > > > > for each counted member?
> > > >
> > > > fwnode_get_next_parent() only releases the child node after it has acquired
> > > > the parent. The only implementation with refcounting for single nodes is
> > > > actually OF.
> > > >
> > > > >
> > > > > > +
> > > > > > +   return count - 1;
> > > > >
> > > > > We could start counting from count = -1;
> > > >
> > > > We could, but then count would need to be made signed (unless overflowing
> > > > is preferred instead).
> > >
> > > What if there are no parents?
> > >
> > > Or is it guaranteed that there always will be at least one?
> >
> > If there are no parents, the function will return 0 --- the node itself is
> > counted in the loop, too, hence the need to decrement by one.
> 
> Right, sorry.
> 
> Then I don't see why count cannot be int.  Or call it "iter" to avoid
> confusion with negative "counts". :-)

Another option would be changing where the counting starts --- the first
parent. I.e.

fwnode = fwnode_get_parent(fwnode);

for (count = 0; fwnode; count++)
	fwnode = fwnode_get_next_parent(fwnode);

return count;

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 1/6] device property: Add functions for accessing node's parents
  2019-03-29 13:04         ` Sakari Ailus
@ 2019-03-29 14:14           ` Andy Shevchenko
  2019-03-29 15:31             ` Sakari Ailus
  0 siblings, 1 reply; 35+ messages in thread
From: Andy Shevchenko @ 2019-03-29 14:14 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Petr Mladek, linux-kernel, rafael, linux-acpi, devicetree, Rob Herring

On Fri, Mar 29, 2019 at 03:04:36PM +0200, Sakari Ailus wrote:
> On Thu, Mar 28, 2019 at 03:10:36PM +0100, Petr Mladek wrote:

> You basically have to have a reference to a node until you're done
> accessing it. The refcounting in the fwnode framework is based on the OF
> API --- otherwise you couldn't access OF nodes using the framework.

> ACPI
> does not have those (as you can't dynamically remove nodes).

What about configfs where we can load / unload tables at run time?

> The newly
> added swnode does have refcounting of individual nodes as well.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 1/6] device property: Add functions for accessing node's parents
  2019-03-29 13:15             ` Sakari Ailus
@ 2019-03-29 14:15               ` Andy Shevchenko
  2019-03-29 14:46                 ` Sakari Ailus
  2019-03-29 23:00               ` Rafael J. Wysocki
  1 sibling, 1 reply; 35+ messages in thread
From: Andy Shevchenko @ 2019-03-29 14:15 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Rafael J. Wysocki, Petr Mladek, Linux Kernel Mailing List,
	ACPI Devel Maling List, devicetree, Rob Herring

On Fri, Mar 29, 2019 at 03:15:07PM +0200, Sakari Ailus wrote:
> On Thu, Mar 28, 2019 at 03:52:35PM +0100, Rafael J. Wysocki wrote:
> > On Thu, Mar 28, 2019 at 12:13 PM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > > On Thu, Mar 28, 2019 at 10:38:01AM +0100, Rafael J. Wysocki wrote:
> > > > On Wed, Mar 27, 2019 at 3:20 PM Sakari Ailus
> > > > <sakari.ailus@linux.intel.com> wrote:
> > > > > On Wed, Mar 27, 2019 at 01:26:25PM +0100, Petr Mladek wrote:
> > > > > > On Tue 2019-03-26 14:41:01, Sakari Ailus wrote:

> Another option would be changing where the counting starts --- the first
> parent. I.e.

Looks better to me, though can you put comment how parents are being counted?

> fwnode = fwnode_get_parent(fwnode);
> 
> for (count = 0; fwnode; count++)
> 	fwnode = fwnode_get_next_parent(fwnode);
> 
> return count;



-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 1/6] device property: Add functions for accessing node's parents
  2019-03-29 14:15               ` Andy Shevchenko
@ 2019-03-29 14:46                 ` Sakari Ailus
  0 siblings, 0 replies; 35+ messages in thread
From: Sakari Ailus @ 2019-03-29 14:46 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J. Wysocki, Petr Mladek, Linux Kernel Mailing List,
	ACPI Devel Maling List, devicetree, Rob Herring

Hi Andy,

On Fri, Mar 29, 2019 at 04:15:49PM +0200, Andy Shevchenko wrote:
> On Fri, Mar 29, 2019 at 03:15:07PM +0200, Sakari Ailus wrote:
> > On Thu, Mar 28, 2019 at 03:52:35PM +0100, Rafael J. Wysocki wrote:
> > > On Thu, Mar 28, 2019 at 12:13 PM Sakari Ailus
> > > <sakari.ailus@linux.intel.com> wrote:
> > > > On Thu, Mar 28, 2019 at 10:38:01AM +0100, Rafael J. Wysocki wrote:
> > > > > On Wed, Mar 27, 2019 at 3:20 PM Sakari Ailus
> > > > > <sakari.ailus@linux.intel.com> wrote:
> > > > > > On Wed, Mar 27, 2019 at 01:26:25PM +0100, Petr Mladek wrote:
> > > > > > > On Tue 2019-03-26 14:41:01, Sakari Ailus wrote:
> 
> > Another option would be changing where the counting starts --- the first
> > parent. I.e.
> 
> Looks better to me, though can you put comment how parents are being counted?

To the code? The kerneldoc comment contains one, and the code below is
pretty simple.

> 
> > fwnode = fwnode_get_parent(fwnode);
> > 
> > for (count = 0; fwnode; count++)
> > 	fwnode = fwnode_get_next_parent(fwnode);
> > 
> > return count;
> 

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 1/6] device property: Add functions for accessing node's parents
  2019-03-29 14:14           ` Andy Shevchenko
@ 2019-03-29 15:31             ` Sakari Ailus
  0 siblings, 0 replies; 35+ messages in thread
From: Sakari Ailus @ 2019-03-29 15:31 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Petr Mladek, linux-kernel, rafael, linux-acpi, devicetree, Rob Herring

Hi Andy,

On Fri, Mar 29, 2019 at 04:14:19PM +0200, Andy Shevchenko wrote:
> On Fri, Mar 29, 2019 at 03:04:36PM +0200, Sakari Ailus wrote:
> > On Thu, Mar 28, 2019 at 03:10:36PM +0100, Petr Mladek wrote:
> 
> > You basically have to have a reference to a node until you're done
> > accessing it. The refcounting in the fwnode framework is based on the OF
> > API --- otherwise you couldn't access OF nodes using the framework.
> 
> > ACPI
> > does not have those (as you can't dynamically remove nodes).
> 
> What about configfs where we can load / unload tables at run time?

Hmm. I have to admit I missed this one.

Well, I guess that this feature depends on the user making sure there are
no references to the tables that are to be removed, as there is no
refcounting. Let me know if I'm missing something, but nothing appears to
prevent releasing nodes that are in use.

If that is really so, then refcounting or another mechanism to achieve
something similar should be added to make removal safe, but this isn't
related to printing the node names anymore nor such fixes belong to this
patchset.

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 1/6] device property: Add functions for accessing node's parents
  2019-03-29 13:15             ` Sakari Ailus
  2019-03-29 14:15               ` Andy Shevchenko
@ 2019-03-29 23:00               ` Rafael J. Wysocki
  1 sibling, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2019-03-29 23:00 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Rafael J. Wysocki, Petr Mladek, Linux Kernel Mailing List,
	Andy Shevchenko, ACPI Devel Maling List, devicetree, Rob Herring

On Friday, March 29, 2019 2:15:07 PM CET Sakari Ailus wrote:
> On Thu, Mar 28, 2019 at 03:52:35PM +0100, Rafael J. Wysocki wrote:
> > On Thu, Mar 28, 2019 at 12:13 PM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > >
> > > Hi Rafael,
> > >
> > > On Thu, Mar 28, 2019 at 10:38:01AM +0100, Rafael J. Wysocki wrote:
> > > > On Wed, Mar 27, 2019 at 3:20 PM Sakari Ailus
> > > > <sakari.ailus@linux.intel.com> wrote:
> > > > >
> > > > > Hi Petr,
> > > > >
> > > > > On Wed, Mar 27, 2019 at 01:26:25PM +0100, Petr Mladek wrote:
> > > > > > On Tue 2019-03-26 14:41:01, Sakari Ailus wrote:
> > > > > > > Add two convenience functions for accessing node's parents:
> > > > > > >
> > > > > > > fwnode_count_parents() returns the number of parent nodes a given node
> > > > > > > has. fwnode_get_nth_parent() returns node's parent at a given distance
> > > > > > > from the node itself.
> > > > > > >
> > > > > > > Also reorder fwnode_get_parent() in property.c according to the same order
> > > > > > > as in property.h.
> > > > > > >
> > > > > > > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > > > > > > index 8b91ab380d14..61eb6ceacc3f 100644
> > > > > > > --- a/drivers/base/property.c
> > > > > > > +++ b/drivers/base/property.c
> > > > > > > @@ -554,17 +567,49 @@ struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
> > > > > > >  EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
> > > > > > >
> > > > > > >  /**
> > > > > > > - * fwnode_get_parent - Return parent firwmare node
> > > > > > > - * @fwnode: Firmware whose parent is retrieved
> > > > > > > + * fwnode_count_parents - Return the number of parents a node has
> > > > > > > + * @fwnode: The node the parents of which are to be counted
> > > > > > >   *
> > > > > > > - * Return parent firmware node of the given node if possible or %NULL if no
> > > > > > > - * parent was available.
> > > > > > > + * Returns the number of parents a node has.
> > > > > > >   */
> > > > > > > -struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode)
> > > > > > > +unsigned int fwnode_count_parents(struct fwnode_handle *fwnode)
> > > > > > >  {
> > > > > > > -   return fwnode_call_ptr_op(fwnode, get_parent);
> > > > > > > +   unsigned int count;
> > > > > > > +
> > > > > > > +   fwnode_handle_get(fwnode);
> > > > > > > +
> > > > > > > +   for (count = 0; fwnode; count++)
> > > > > > > +           fwnode = fwnode_get_next_parent(fwnode);
> > > > > >
> > > > > > Is it guaranteed that all parents stay when
> > > > > > fwnode_get_next_parent() releases the reference count
> > > > > > for each counted member?
> > > > >
> > > > > fwnode_get_next_parent() only releases the child node after it has acquired
> > > > > the parent. The only implementation with refcounting for single nodes is
> > > > > actually OF.
> > > > >
> > > > > >
> > > > > > > +
> > > > > > > +   return count - 1;
> > > > > >
> > > > > > We could start counting from count = -1;
> > > > >
> > > > > We could, but then count would need to be made signed (unless overflowing
> > > > > is preferred instead).
> > > >
> > > > What if there are no parents?
> > > >
> > > > Or is it guaranteed that there always will be at least one?
> > >
> > > If there are no parents, the function will return 0 --- the node itself is
> > > counted in the loop, too, hence the need to decrement by one.
> > 
> > Right, sorry.
> > 
> > Then I don't see why count cannot be int.  Or call it "iter" to avoid
> > confusion with negative "counts". :-)
> 
> Another option would be changing where the counting starts --- the first
> parent. I.e.
> 
> fwnode = fwnode_get_parent(fwnode);
> 
> for (count = 0; fwnode; count++)
> 	fwnode = fwnode_get_next_parent(fwnode);
> 
> return count;

LGTM, thanks!


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 2/6] device property: Add fwnode_get_name for returning the name of a node
  2019-03-26 12:41 ` [PATCH v2 2/6] device property: Add fwnode_get_name for returning the name of a node Sakari Ailus
  2019-03-28  9:45   ` Rafael J. Wysocki
@ 2019-03-31  6:42   ` Rob Herring
  1 sibling, 0 replies; 35+ messages in thread
From: Rob Herring @ 2019-03-31  6:42 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Petr Mladek, linux-kernel, rafael, Andy Shevchenko, linux-acpi,
	devicetree

On Tue, Mar 26, 2019 at 02:41:02PM +0200, Sakari Ailus wrote:
> The fwnode framework did not have means to obtain the name of a node. Add
> that now, in form of the fwnode_get_name() function and a corresponding
> get_name fwnode op. OF and ACPI support is included.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/acpi/property.c  | 24 ++++++++++++++++++++++++
>  drivers/base/property.c  | 11 +++++++++++
>  drivers/of/property.c    |  6 ++++++

Acked-by: Rob Herring <robh@kernel.org>

>  include/linux/fwnode.h   |  2 ++
>  include/linux/property.h |  1 +
>  5 files changed, 44 insertions(+)


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v2 3/6] device property: Add a function to obtain a node's prefix
  2019-03-26 12:41 ` [PATCH v2 3/6] device property: Add a function to obtain a node's prefix Sakari Ailus
  2019-03-26 13:16   ` Andy Shevchenko
@ 2019-03-31  6:42   ` Rob Herring
  1 sibling, 0 replies; 35+ messages in thread
From: Rob Herring @ 2019-03-31  6:42 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Petr Mladek, linux-kernel, rafael, Andy Shevchenko, linux-acpi,
	devicetree

On Tue, Mar 26, 2019 at 02:41:03PM +0200, Sakari Ailus wrote:
> The prefix is used for printing purpose before a node, and it also works
> as a separator between two nodes.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/acpi/property.c  | 22 ++++++++++++++++++++++
>  drivers/base/property.c  | 12 ++++++++++++
>  drivers/of/property.c    | 10 ++++++++++

Acked-by: Rob Herring <robh@kernel.org>

>  include/linux/fwnode.h   |  2 ++
>  include/linux/property.h |  1 +
>  5 files changed, 47 insertions(+)


^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2019-03-31  6:42 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-26 12:41 [PATCH v2 0/6] Device property improvements, add %pfw format specifier Sakari Ailus
2019-03-26 12:41 ` [PATCH v2 1/6] device property: Add functions for accessing node's parents Sakari Ailus
2019-03-27 12:26   ` Petr Mladek
2019-03-27 14:20     ` Sakari Ailus
2019-03-28  9:38       ` Rafael J. Wysocki
2019-03-28 11:13         ` Sakari Ailus
2019-03-28 14:52           ` Rafael J. Wysocki
2019-03-29 13:15             ` Sakari Ailus
2019-03-29 14:15               ` Andy Shevchenko
2019-03-29 14:46                 ` Sakari Ailus
2019-03-29 23:00               ` Rafael J. Wysocki
2019-03-28 14:10       ` Petr Mladek
2019-03-29 13:04         ` Sakari Ailus
2019-03-29 14:14           ` Andy Shevchenko
2019-03-29 15:31             ` Sakari Ailus
2019-03-26 12:41 ` [PATCH v2 2/6] device property: Add fwnode_get_name for returning the name of a node Sakari Ailus
2019-03-28  9:45   ` Rafael J. Wysocki
2019-03-28 11:31     ` Sakari Ailus
2019-03-31  6:42   ` Rob Herring
2019-03-26 12:41 ` [PATCH v2 3/6] device property: Add a function to obtain a node's prefix Sakari Ailus
2019-03-26 13:16   ` Andy Shevchenko
2019-03-26 13:32     ` Sakari Ailus
2019-03-27 12:36       ` Petr Mladek
2019-03-28 11:43         ` Sakari Ailus
2019-03-31  6:42   ` Rob Herring
2019-03-26 12:41 ` [PATCH v2 4/6] lib/vsprintf: Remove support for %pF and %pf in favour of %pS and %ps Sakari Ailus
2019-03-26 12:41 ` [PATCH v2 5/6] lib/vsprintf: Make use of fwnode API to obtain node names and separators Sakari Ailus
2019-03-26 12:41 ` [PATCH v2 6/6] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names Sakari Ailus
2019-03-26 13:11   ` Rasmus Villemoes
2019-03-26 13:24     ` Andy Shevchenko
2019-03-26 13:31       ` Sakari Ailus
2019-03-28 14:29       ` Petr Mladek
2019-03-29 13:10         ` Sakari Ailus
2019-03-26 20:18   ` Joe Perches
     [not found]     ` <20190328114845.giusyarjibvg5ru7@kekkonen.localdomain>
2019-03-28 11:51       ` Sakari Ailus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).