[v5,05/11] device property: Add fwnode_get_name for returning the name of a node
diff mbox series

Message ID 20190902135732.23455-6-sakari.ailus@linux.intel.com
State Superseded, archived
Headers show
Series
  • Device property improvements, add %pfw format specifier
Related show

Commit Message

Sakari Ailus Sept. 2, 2019, 1:57 p.m. UTC
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>
Acked-by: Rob Herring <robh@kernel.org> (for OF)
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/property.c  | 26 ++++++++++++++++++++++++++
 drivers/base/property.c  | 11 +++++++++++
 drivers/base/swnode.c    | 20 ++++++++++++++++++++
 drivers/of/property.c    |  6 ++++++
 include/linux/fwnode.h   |  2 ++
 include/linux/property.h |  1 +
 6 files changed, 66 insertions(+)

Comments

Heikki Krogerus Sept. 3, 2019, 10:10 a.m. UTC | #1
Hi Sakari,

On Mon, Sep 02, 2019 at 04:57:26PM +0300, Sakari Ailus wrote:
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 951e7efd47c23..a4a0f5b80bad3 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -515,6 +515,25 @@ static int software_node_read_string_array(const struct fwnode_handle *fwnode,
>  						propname, val, nval);
>  }
>  
> +static const char *
> +software_node_get_name(const struct fwnode_handle *fwnode)
> +{
> +	const struct software_node *softnode = to_software_node(fwnode);
> +	const struct swnode *swnode = software_node_to_swnode(softnode);

Why not just:

        struct swnode *swnode = to_swnode(fwnode);

> +	struct fwnode_handle *parent;
> +
> +	if (!swnode)
> +		return "(null)";
> +
> +	parent = fwnode_get_parent(&swnode->fwnode);
> +	if (!parent)
> +		return "";

Please note that there is no root software node object (the kset is
the root), so you will get "" with most nodes. I'm assuming that is
not the intention, or is it?

> +	fwnode_handle_put(parent);
> +
> +	return kobject_name(&swnode->kobj);
> +}
> +
>  static struct fwnode_handle *
>  software_node_get_parent(const struct fwnode_handle *fwnode)
>  {
> @@ -615,6 +634,7 @@ static const struct fwnode_operations software_node_ops = {
>  	.property_present = software_node_property_present,
>  	.property_read_int_array = software_node_read_int_array,
>  	.property_read_string_array = software_node_read_string_array,
> +	.get_name = software_node_get_name,
>  	.get_parent = software_node_get_parent,
>  	.get_next_child_node = software_node_get_next_child,
>  	.get_named_child_node = software_node_get_named_child_node,

thanks,
Sakari Ailus Sept. 3, 2019, 11:24 a.m. UTC | #2
Moi,

Thanks for the comments.

On Tue, Sep 03, 2019 at 01:10:13PM +0300, Heikki Krogerus wrote:
> Hi Sakari,
> 
> On Mon, Sep 02, 2019 at 04:57:26PM +0300, Sakari Ailus wrote:
> > diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> > index 951e7efd47c23..a4a0f5b80bad3 100644
> > --- a/drivers/base/swnode.c
> > +++ b/drivers/base/swnode.c
> > @@ -515,6 +515,25 @@ static int software_node_read_string_array(const struct fwnode_handle *fwnode,
> >  						propname, val, nval);
> >  }
> >  
> > +static const char *
> > +software_node_get_name(const struct fwnode_handle *fwnode)
> > +{
> > +	const struct software_node *softnode = to_software_node(fwnode);
> > +	const struct swnode *swnode = software_node_to_swnode(softnode);
> 
> Why not just:
> 
>         struct swnode *swnode = to_swnode(fwnode);

Fixed.

> 
> > +	struct fwnode_handle *parent;
> > +
> > +	if (!swnode)
> > +		return "(null)";
> > +
> > +	parent = fwnode_get_parent(&swnode->fwnode);
> > +	if (!parent)
> > +		return "";
> 
> Please note that there is no root software node object (the kset is
> the root), so you will get "" with most nodes. I'm assuming that is
> not the intention, or is it?

Good point.

In practice this will happen rarely outside the tests, but indeed the root
node would usually not be a software node. I'll drop the above three lines
checking for the parent node, and change nodes created in the test
accordingly.

Patch
diff mbox series

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index ea3d700da3ca6..5a9397a390f41 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1311,6 +1311,31 @@  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;
+
+	/* Is this the root node? */
+	parent = fwnode_get_parent(fwnode);
+	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);
+	if (WARN_ON(!adev))
+		return NULL;
+
+	return acpi_device_bid(adev);
+}
+
 static struct fwnode_handle *
 acpi_fwnode_get_parent(struct fwnode_handle *fwnode)
 {
@@ -1351,6 +1376,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 f2e555e68b56a..9b5ec88e72d8b 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -556,6 +556,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/base/swnode.c b/drivers/base/swnode.c
index 951e7efd47c23..a4a0f5b80bad3 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -515,6 +515,25 @@  static int software_node_read_string_array(const struct fwnode_handle *fwnode,
 						propname, val, nval);
 }
 
+static const char *
+software_node_get_name(const struct fwnode_handle *fwnode)
+{
+	const struct software_node *softnode = to_software_node(fwnode);
+	const struct swnode *swnode = software_node_to_swnode(softnode);
+	struct fwnode_handle *parent;
+
+	if (!swnode)
+		return "(null)";
+
+	parent = fwnode_get_parent(&swnode->fwnode);
+	if (!parent)
+		return "";
+
+	fwnode_handle_put(parent);
+
+	return kobject_name(&swnode->kobj);
+}
+
 static struct fwnode_handle *
 software_node_get_parent(const struct fwnode_handle *fwnode)
 {
@@ -615,6 +634,7 @@  static const struct fwnode_operations software_node_ops = {
 	.property_present = software_node_property_present,
 	.property_read_int_array = software_node_read_int_array,
 	.property_read_string_array = software_node_read_string_array,
+	.get_name = software_node_get_name,
 	.get_parent = software_node_get_parent,
 	.get_next_child_node = software_node_get_next_child,
 	.get_named_child_node = software_node_get_named_child_node,
diff --git a/drivers/of/property.c b/drivers/of/property.c
index d7fa75e31f224..5bed634551ea6 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 a11c8c56c78b4..c331e0ef31e80 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -56,6 +56,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.
@@ -82,6 +83,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 5450e7ec219ac..ebc5e2016bb66 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -80,6 +80,7 @@  struct fwnode_handle *fwnode_find_reference(const struct fwnode_handle *fwnode,
 					    const char *name,
 					    unsigned int index);
 
+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);