Re: [PATCH V3 3/3] nvmem: core: add cell name based matching of DT cell nodes

[Srinivas Kandagatla <srinivas.kandagatla@linaro.org>]

Hi Rafał

On 18/02/2022 07:07, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> When adding NVMEM cells defined by driver it's important to match them
> with DT nodes that specify matching names. That way other bindings &
> drivers can reference such "dynamic" NVMEM cells.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>   drivers/nvmem/core.c | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 53a43d843743..a891449c52f1 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -499,6 +499,31 @@ static int nvmem_cell_info_to_nvmem_cell_entry(struct nvmem_device *nvmem,
>   	return 0;
>   }
>   
> +/**
> + * nvmem_find_cell_of_node() - Find DT node matching nvmem cell
> + *
> + * @nvmem: nvmem provider
> + * @name: nvmem cell name
> + *
> + * Runtime created nvmem cells (those not coming from DT) may still need to be
> + * referenced in DT. This function allows finding DT node referencing nvmem cell
> + * by its name. Such a DT node can be then used by nvmem consumers.
> + *
> + * Return: NULL or pointer to DT node
> + */
> +static struct device_node *nvmem_find_cell_of_node(struct nvmem_device *nvmem,
> +						   const char *name)
> +{
> +	struct device_node *child;
> +
> +	for_each_child_of_node(nvmem->dev.of_node, child) {
> +		if (!strcmp(child->name, name))
> +			return child;
> +	}

Isn't this just

	return of_get_child_by_name(nvmem->dev.of_node, name);


> +
> +	return NULL;
> +}
> +
>   /**
>    * nvmem_add_cells() - Add cell information to an nvmem device
>    *
> @@ -532,6 +557,8 @@ static int nvmem_add_cells(struct nvmem_device *nvmem,
>   			goto err;
>   		}
>   
> +		cells[i]->np = nvmem_find_cell_of_node(nvmem, cells[i]->name);


This is really assuming that node name will be same as name passed in 
nvmem_cell_info which might not be always true.

This seems be very specific with brcm and this code does not belong in 
nvmem core.

How about adding device_node to struct nvmem_cell_info and update this 
of_node as part of brcm_nvram_parse()?


--srini


> +
>   		nvmem_cell_entry_add(cells[i]);
>   	}
>