[RFC PATCH 0/2] software node: API documentation

[RFC PATCH 0/2] software node: API documentation

[Heikki Krogerus]

Hi,

I'm sending this as an RFC first. I would like to wait for the API
change Dmitry proposed [1] before we add the final API documentation
for the software nodes.

At this point I would like to know if the generic use is explained
clearly enough and if there is anything missing that should be added.

[1] https://lkml.org/lkml/2019/9/11/8

thanks,

Heikki Krogerus (2):
  software node: Add missing kernel-doc function descriptions
  software node: Add documentation

 Documentation/driver-api/software_node.rst | 197 +++++++++++++++++++++
 drivers/base/swnode.c                      |  23 +++
 2 files changed, 220 insertions(+)
 create mode 100644 Documentation/driver-api/software_node.rst

-- 
2.23.0

[RFC PATCH 1/2] software node: Add missing kernel-doc function descriptions

[Heikki Krogerus]

Some of the functions were missing kernel-doc style
descriptions.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/swnode.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index a1f3f0994f9f..51c9e6c56c26 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -33,6 +33,10 @@ static struct kset *swnode_kset;
 
 static const struct fwnode_operations software_node_ops;
 
+/**
+ * is_software_node - Test if fwnode handle is software node
+ * @fwnode: The fwnode handle to test
+ */
 bool is_software_node(const struct fwnode_handle *fwnode)
 {
 	return !IS_ERR_OR_NULL(fwnode) && fwnode->ops == &software_node_ops;
@@ -71,6 +75,10 @@ software_node_to_swnode(const struct software_node *node)
 	return swnode;
 }
 
+/**
+ * to_software_node - Convert fwnode handle to software node
+ * @fwnode: The fwnode handle of the software node
+ */
 const struct software_node *to_software_node(struct fwnode_handle *fwnode)
 {
 	struct swnode *swnode = to_swnode(fwnode);
@@ -79,6 +87,10 @@ const struct software_node *to_software_node(struct fwnode_handle *fwnode)
 }
 EXPORT_SYMBOL_GPL(to_software_node);
 
+/**
+ * software_node_fwnode - Convert software node to fwnode handle
+ * @swnode: The software node
+ */
 struct fwnode_handle *software_node_fwnode(const struct software_node *node)
 {
 	struct swnode *swnode = software_node_to_swnode(node);
@@ -802,6 +814,13 @@ int software_node_register(const struct software_node *node)
 }
 EXPORT_SYMBOL_GPL(software_node_register);
 
+/**
+ * fwnode_create_software_node - Create and register a software node
+ * @properties: Device properties for the software node
+ * @parent: The parent node
+ *
+ * Returnes the fwnode handle of the software node on success.
+ */
 struct fwnode_handle *
 fwnode_create_software_node(const struct property_entry *properties,
 			    const struct fwnode_handle *parent)
@@ -834,6 +853,10 @@ fwnode_create_software_node(const struct property_entry *properties,
 }
 EXPORT_SYMBOL_GPL(fwnode_create_software_node);
 
+/**
+ * fwnode_remove_software_node - Remove a software node
+ * @fwnode: The fwnode handle of the software node
+ */
 void fwnode_remove_software_node(struct fwnode_handle *fwnode)
 {
 	struct swnode *swnode = to_swnode(fwnode);
-- 
2.23.0

[RFC PATCH 2/2] software node: Add documentation

[Heikki Krogerus]

API documentation for the software nodes.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 Documentation/driver-api/software_node.rst | 197 +++++++++++++++++++++
 1 file changed, 197 insertions(+)
 create mode 100644 Documentation/driver-api/software_node.rst

diff --git a/Documentation/driver-api/software_node.rst b/Documentation/driver-api/software_node.rst
new file mode 100644
index 000000000000..cf8a05c34e9e
--- /dev/null
+++ b/Documentation/driver-api/software_node.rst
@@ -0,0 +1,197 @@
+
+.. _software_node:
+
+==============
+Software nodes
+==============
+
+Introduction
+============
+
+Software node is a :c:type:`struct fwnode_handle <fwnode_handle>` type,
+analogous to the ACPI and DT firmware nodes except that the software nodes are
+created in kernel code (hence the name "software" node). The software nodes can
+be used to complement fwnodes representing real firmware nodes when they are
+incomplete, for example missing device properties, and to supply the primary
+fwnode when the firmware lacks hardware description for a device completely.
+
+NOTE! The primary hardware description should always come from either ACPI
+tables or DT. Describing an entire system with software nodes, though possible,
+is not acceptable! The software nodes should only complement the primary
+hardware description.
+
+Hierarchy
+=========
+
+The software nodes support hierarchy (i.e. the software nodes can have child
+software nodes and a parent software node) just like ACPI and DT firmware nodes,
+but there is no dedicated root software node object. It means that a software
+node at the root level does not have a parent.
+
+Note! Only other software nodes can be children and the parent for a software
+node.
+
+Device properties
+=================
+
+The software node device properties are described with :c:type:`struct
+property_entry <property_entry>`. When a software node is created that has
+device properties, it is supplied with a zero terminated array of property
+entries. Normally the properties are described with helper macros::
+
+	static const u8 u8_array[] = { 0, 1, 2, 3 };
+	static const u16 u16_array[] = { 0, 1, 2, 3 };
+	static const u32 u32_array[] = { 0, 1, 2, 3 };
+	static const u64 u64_array[] = { 0, 1, 2, 3 };
+
+	static const struct property_entry my_props[] = {
+		PROPERTY_ENTRY_U8_ARRAY("u8_array_example", u8_array),
+		PROPERTY_ENTRY_U16_ARRAY("u16_array_example", u16_array),
+		PROPERTY_ENTRY_U32_ARRAY("u32_array_example", u32_array),
+		PROPERTY_ENTRY_U64_ARRAY("u64_array_example", u64_array),
+		PROPERTY_ENTRY_U8("u8_example", 0xff),
+		PROPERTY_ENTRY_U16("u16_example", 0xffff),
+		PROPERTY_ENTRY_U32("u32_example", 0xffffffff),
+		PROPERTY_ENTRY_U64("u64_example", 0xffffffffffffffff),
+		PROPERTY_ENTRY_STRING("string_example", "string"),
+		{ }
+	};
+
+Note! If "build-in" device properties are supplied to already existing device
+entries by using :c:func:`device_add_properties`, a software node is actually
+created for that device. That software node is just assigned to the device
+automatically in the function.
+
+Usage
+=====
+
+Node creation
+-------------
+
+Static nodes
+~~~~~~~~~~~~
+
+In a normal case the software nodes are described statically with
+:c:type:`struct software_node <software_node>`, and then registered with
+:c:func:`software_node_register`. Usually there is more then one software node
+that needs to be registered. A helper :c:func:`software_node_register_nodes`
+registers a zero terminated array of software nodes::
+
+	static const struct property_entry props[] = {
+		PROPERTY_ENTRY_STRING("foo", "bar"),
+		{ }
+	};
+
+	static const struct software_node my_nodes[] = {
+		{ "grandparent" },		/* no parent nor properties */
+		{ "parent", &my_nodes[0] },	/* parent, but no propreties */
+		{ "child", &my_nodes[1], props }, /* parent and properties */
+		{ }
+	};
+
+	static int my_init(void)
+	{
+		return software_node_register_nodes(my_nodes);
+	}
+
+Note! The above example names the nodes "grandparent", "parent" and "child", but
+the software nodes don't actually have to be named. If no name is supplied for a
+software node when it's being registered, the API names the node "node<n>" where
+<n> is index number.
+
+Dynamic nodes
+~~~~~~~~~~~~~
+
+The Quick (and dirty) method. A software node can be created on the fly with
+:c:func:`fwnode_create_software_node`. The nodes create "on-the-fly" don't
+differ from statically described software nodes in any way, but for now the API
+does not support naming of these nodes::
+
+	static const struct property_entry my_props[] = {
+		PROPERTY_ENTRY_STRING("foo", "bar"),
+		{ }
+	};
+
+	static int my_init(void)
+	{
+		struct fwnode_handle *fwnode;
+
+		/* Software node without a parent. */
+		fwnode = fwnode_create_software_node(my_props, NULL);
+		if (IS_ERR(fwnode))
+			return PTR_ERR(fwnode);
+
+		return -1;
+	}
+
+Linking software nodes with the device (struct device) entries
+--------------------------------------------------------------
+
+Ideally the software node should be assigned to the device entry before the
+device is registered (just like any other fwnode).
+
+When the software node is the primary fwnode for the device, the fwnode of the
+device needs to simply point to the software node fwnode. Using a helper
+:c:func:`software_node_fwnode` in the following example::
+
+	static const struct software_node my_node = {
+		.name = "thenode",
+	};
+
+	static int my_init(void)
+	{
+		struct device my_device = { };
+		int ret;
+
+		ret = software_node_register(my_node);
+		if (ret)
+			return ret;
+
+		device_initialize(&my_device);
+		dev_set_name(&my_device, "thedevice")
+
+		my_device.fwnode = software_node_fwnode(swnode);
+
+		return device_add(&my_device);
+	}
+
+When the software node is the secondary fwnode for a device, i.e. the device has
+either ACPI or DT (or something else) firmware node as the primary fwnode, the
+node should be assigned with :c:func:`set_secondary_fwnode`. If the
+``secondary`` member of the primary fwnode needs to be manually made to point
+to the software node, the code needs to make sure that the ``secondary`` member
+of the software node fwnode points to a specific value of ``ERR_PTR(-ENODEV)``::
+
+	struct fwnode_handle *fwnode = software_node_fwnode(swnode);
+
+	fwnode->secondary = ERR_PTR(-ENODEV);
+	dev->fwnode->secondary = fwnode;
+
+Note! There is no requirement to bind a software node with a device entry, i.e.
+having "sub-nodes" that represent for example certain resources the parent
+software node has is not a problem.
+
+Node References
+---------------
+
+TODO
+
+Device graph
+------------
+
+TODO
+
+API
+===
+
+.. kernel-doc:: include/linux/property.h
+   :internal: software_node
+
+.. kernel-doc:: include/linux/fwnode.h
+   :internal: fwnode_handle
+
+.. kernel-doc:: drivers/base/core.c
+   :functions: set_secondary_fwnode
+
+.. kernel-doc:: drivers/base/swnode.c
+   :functions: is_software_node is_software_node software_node_fwnode software_node_find_by_name software_node_register_nodes software_node_unregister_nodes software_node_register software_node_register fwnode_create_software_node fwnode_remove_software_node
-- 
2.23.0

Re: [RFC PATCH 2/2] software node: Add documentation

[Randy Dunlap]

Hi,
Below are a few doc edits for you.


On 10/2/19 5:33 AM, Heikki Krogerus wrote:
> API documentation for the software nodes.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  Documentation/driver-api/software_node.rst | 197 +++++++++++++++++++++
>  1 file changed, 197 insertions(+)
>  create mode 100644 Documentation/driver-api/software_node.rst
> 
> diff --git a/Documentation/driver-api/software_node.rst b/Documentation/driver-api/software_node.rst
> new file mode 100644
> index 000000000000..cf8a05c34e9e
> --- /dev/null
> +++ b/Documentation/driver-api/software_node.rst
> @@ -0,0 +1,197 @@
> +
> +.. _software_node:
> +
> +==============
> +Software nodes
> +==============
> +
> +Introduction
> +============
> +
> +Software node is a :c:type:`struct fwnode_handle <fwnode_handle>` type,
> +analogous to the ACPI and DT firmware nodes except that the software nodes are
> +created in kernel code (hence the name "software" node). The software nodes can
> +be used to complement fwnodes representing real firmware nodes when they are
> +incomplete, for example missing device properties, and to supply the primary
> +fwnode when the firmware lacks hardware description for a device completely.
> +
> +NOTE! The primary hardware description should always come from either ACPI
> +tables or DT. Describing an entire system with software nodes, though possible,
> +is not acceptable! The software nodes should only complement the primary
> +hardware description.
> +
> +Hierarchy
> +=========
> +
> +The software nodes support hierarchy (i.e. the software nodes can have child

I would s/child/children/

> +software nodes and a parent software node) just like ACPI and DT firmware nodes,
> +but there is no dedicated root software node object. It means that a software
> +node at the root level does not have a parent.
> +
> +Note! Only other software nodes can be children and the parent for a software
> +node.
> +
> +Device properties
> +=================
> +
> +The software node device properties are described with :c:type:`struct
> +property_entry <property_entry>`. When a software node is created that has
> +device properties, it is supplied with a zero terminated array of property
> +entries. Normally the properties are described with helper macros::
> +
> +	static const u8 u8_array[] = { 0, 1, 2, 3 };
> +	static const u16 u16_array[] = { 0, 1, 2, 3 };
> +	static const u32 u32_array[] = { 0, 1, 2, 3 };
> +	static const u64 u64_array[] = { 0, 1, 2, 3 };
> +
> +	static const struct property_entry my_props[] = {
> +		PROPERTY_ENTRY_U8_ARRAY("u8_array_example", u8_array),
> +		PROPERTY_ENTRY_U16_ARRAY("u16_array_example", u16_array),
> +		PROPERTY_ENTRY_U32_ARRAY("u32_array_example", u32_array),
> +		PROPERTY_ENTRY_U64_ARRAY("u64_array_example", u64_array),
> +		PROPERTY_ENTRY_U8("u8_example", 0xff),
> +		PROPERTY_ENTRY_U16("u16_example", 0xffff),
> +		PROPERTY_ENTRY_U32("u32_example", 0xffffffff),
> +		PROPERTY_ENTRY_U64("u64_example", 0xffffffffffffffff),
> +		PROPERTY_ENTRY_STRING("string_example", "string"),
> +		{ }
> +	};
> +
> +Note! If "build-in" device properties are supplied to already existing device

            "built-in"

> +entries by using :c:func:`device_add_properties`, a software node is actually
> +created for that device. That software node is just assigned to the device
> +automatically in the function.
> +
> +Usage
> +=====
> +
> +Node creation
> +-------------
> +
> +Static nodes
> +~~~~~~~~~~~~
> +
> +In a normal case the software nodes are described statically with
> +:c:type:`struct software_node <software_node>`, and then registered with
> +:c:func:`software_node_register`. Usually there is more then one software node

                                                           than

> +that needs to be registered. A helper :c:func:`software_node_register_nodes`
> +registers a zero terminated array of software nodes::
> +
> +	static const struct property_entry props[] = {
> +		PROPERTY_ENTRY_STRING("foo", "bar"),
> +		{ }
> +	};
> +
> +	static const struct software_node my_nodes[] = {
> +		{ "grandparent" },		/* no parent nor properties */
> +		{ "parent", &my_nodes[0] },	/* parent, but no propreties */

		                                                  properties

> +		{ "child", &my_nodes[1], props }, /* parent and properties */
> +		{ }
> +	};
> +
> +	static int my_init(void)
> +	{
> +		return software_node_register_nodes(my_nodes);
> +	}
> +
> +Note! The above example names the nodes "grandparent", "parent" and "child", but
> +the software nodes don't actually have to be named. If no name is supplied for a
> +software node when it's being registered, the API names the node "node<n>" where
> +<n> is index number.
> +
> +Dynamic nodes
> +~~~~~~~~~~~~~
> +
> +The Quick (and dirty) method. A software node can be created on the fly with
> +:c:func:`fwnode_create_software_node`. The nodes create "on-the-fly" don't

                                                    created

> +differ from statically described software nodes in any way, but for now the API
> +does not support naming of these nodes::
> +
> +	static const struct property_entry my_props[] = {
> +		PROPERTY_ENTRY_STRING("foo", "bar"),
> +		{ }
> +	};
> +
> +	static int my_init(void)
> +	{
> +		struct fwnode_handle *fwnode;
> +
> +		/* Software node without a parent. */
> +		fwnode = fwnode_create_software_node(my_props, NULL);
> +		if (IS_ERR(fwnode))
> +			return PTR_ERR(fwnode);
> +
> +		return -1;
> +	}
> +
> +Linking software nodes with the device (struct device) entries
> +--------------------------------------------------------------
> +
> +Ideally the software node should be assigned to the device entry before the
> +device is registered (just like any other fwnode).
> +
> +When the software node is the primary fwnode for the device, the fwnode of the
> +device needs to simply point to the software node fwnode. Using a helper
> +:c:func:`software_node_fwnode` in the following example::
> +
> +	static const struct software_node my_node = {
> +		.name = "thenode",
> +	};
> +
> +	static int my_init(void)
> +	{
> +		struct device my_device = { };
> +		int ret;
> +
> +		ret = software_node_register(my_node);
> +		if (ret)
> +			return ret;
> +
> +		device_initialize(&my_device);
> +		dev_set_name(&my_device, "thedevice")
> +
> +		my_device.fwnode = software_node_fwnode(swnode);
> +
> +		return device_add(&my_device);
> +	}
> +
> +When the software node is the secondary fwnode for a device, i.e. the device has
> +either ACPI or DT (or something else) firmware node as the primary fwnode, the
> +node should be assigned with :c:func:`set_secondary_fwnode`. If the
> +``secondary`` member of the primary fwnode needs to be manually made to point
> +to the software node, the code needs to make sure that the ``secondary`` member
> +of the software node fwnode points to a specific value of ``ERR_PTR(-ENODEV)``::
> +
> +	struct fwnode_handle *fwnode = software_node_fwnode(swnode);
> +
> +	fwnode->secondary = ERR_PTR(-ENODEV);
> +	dev->fwnode->secondary = fwnode;
> +
> +Note! There is no requirement to bind a software node with a device entry, i.e.
> +having "sub-nodes" that represent for example certain resources the parent
> +software node has is not a problem.
> +
> +Node References
> +---------------
> +
> +TODO
> +
> +Device graph
> +------------
> +
> +TODO
> +
> +API
> +===
> +
> +.. kernel-doc:: include/linux/property.h
> +   :internal: software_node
> +
> +.. kernel-doc:: include/linux/fwnode.h
> +   :internal: fwnode_handle
> +
> +.. kernel-doc:: drivers/base/core.c
> +   :functions: set_secondary_fwnode
> +
> +.. kernel-doc:: drivers/base/swnode.c
> +   :functions: is_software_node is_software_node software_node_fwnode software_node_find_by_name software_node_register_nodes software_node_unregister_nodes software_node_register software_node_register fwnode_create_software_node fwnode_remove_software_node
> 

Thanks for the doc.
-- 
~Randy

Re: [RFC PATCH 2/2] software node: Add documentation

[Heikki Krogerus]

On Thu, Oct 03, 2019 at 07:56:59PM -0700, Randy Dunlap wrote:
> Hi,
> Below are a few doc edits for you.

Thank you for the review! I'll fix the v2 according to your comments.

thanks,

-- 
heikki