Re: [PATCH 2/3] of: dynamic: add of_node_alloc() and of_node_free()

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 04/05/2022 à 17:40, Clément Léger a écrit :
> Add functions which allows to create and free nodes.
> 
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> ---
>   drivers/of/dynamic.c | 59 ++++++++++++++++++++++++++++++++++++--------
>   include/linux/of.h   |  9 +++++++
>   2 files changed, 58 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index e8700e509d2e..ec28e5ba2969 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -455,6 +455,54 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
>   				 prop->length, allocflags);
>   }
>   
> +/**
> + * of_node_free - Free a node allocated dynamically.
> + * @node:	Node to be freed
> + */
> +void of_node_free(const struct device_node *node)
> +{
> +	kfree(node->full_name);
> +	kfree(node->data);
> +	kfree(node);
> +}
> +EXPORT_SYMBOL(of_node_free);
> +
> +/**
> + * of_node_alloc - Allocate a node dynamically.
> + * @name:	Node name
> + * @allocflags:	Allocation flags (typically pass GFP_KERNEL)
> + *
> + * Create a node by dynamically allocating the memory of both the
> + * node structure and the node name & contents. The node's
> + * flags have the OF_DYNAMIC & OF_DETACHED bit set so that we can
> + * differentiate between dynamically allocated nodes and not.
> + *
> + * Return: The newly allocated node or NULL on out of memory error.
> + */
> +struct device_node *of_node_alloc(const char *name, gfp_t allocflags)
> +{
> +	struct device_node *node;
> +
> +	node = kzalloc(sizeof(*node), allocflags);
> +	if (!node)
> +		return NULL;
> +
> +	if (name) {
> +		node->full_name = kstrdup(name, allocflags);
> +		if (!node->full_name) {
> +			kfree(node);
> +			return NULL;
> +		}
> +	}
> +
> +	of_node_set_flag(node, OF_DYNAMIC);
> +	of_node_set_flag(node, OF_DETACHED);
> +	of_node_init(node);
> +
> +	return node;
> +}
> +EXPORT_SYMBOL(of_node_alloc);
> +
>   /**
>    * __of_node_dup() - Duplicate or create an empty device node dynamically.
>    * @np:		if not NULL, contains properties to be duplicated in new node
> @@ -471,18 +519,9 @@ struct device_node *__of_node_dup(const struct device_node *np,
>   {
>   	struct device_node *node;
>   
> -	node = kzalloc(sizeof(*node), GFP_KERNEL);
> +	node = of_node_alloc(NULL, GFP_KERNEL);
>   	if (!node)
>   		return NULL;
> -	node->full_name = kstrdup(full_name, GFP_KERNEL);
> -	if (!node->full_name) {
> -		kfree(node);
> -		return NULL;
> -	}
> -
> -	of_node_set_flag(node, OF_DYNAMIC);
> -	of_node_set_flag(node, OF_DETACHED);
> -	of_node_init(node);
>   
>   	/* Iterate over and duplicate all properties */
>   	if (np) {
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 6b345eb71c19..749ac07f4960 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -1463,6 +1463,8 @@ enum of_reconfig_change {
>   };
>   
>   #ifdef CONFIG_OF_DYNAMIC
> +extern struct device_node *of_node_alloc(const char *name, gfp_t allocflags);
> +extern void of_node_free(const struct device_node *node);

'extern' is pointless:

CHECK: extern prototypes should be avoided in .h files
#104: FILE: include/linux/of.h:1466:
+extern struct device_node *of_node_alloc(const char *name, gfp_t 
allocflags);

CHECK: extern prototypes should be avoided in .h files
#105: FILE: include/linux/of.h:1467:
+extern void of_node_free(const struct device_node *node);



>   extern struct property *of_property_alloc(const char *name, const void *value,
>   					  int value_len, int len,
>   					  gfp_t allocflags);
> @@ -1512,6 +1514,13 @@ static inline int of_changeset_update_property(struct of_changeset *ocs,
>   	return of_changeset_action(ocs, OF_RECONFIG_UPDATE_PROPERTY, np, prop);
>   }
>   #else /* CONFIG_OF_DYNAMIC */
> +static inline struct device_node *of_node_alloc(const char *name,
> +						gfp_t allocflags)
> +{
> +	return NULL;
> +}
> +
> +static inline void of_node_free(const struct device_node *node) {}
>   
>   static inline struct property *of_property_alloc(const char *name,
>   						 const void *value,