Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <45015436-cc46-a239-e73c-db23e2c161a8@linaro.org>
Date:   Fri, 25 Feb 2022 12:10:44 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH V3 3/3] nvmem: core: add cell name based matching of DT
 cell nodes
Content-Language: en-US
To:     =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= <zajec5@gmail.com>,
        Rob Herring <robh+dt@kernel.org>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Hauke Mehrtens <hauke@hauke-m.de>,
        bcm-kernel-feedback-list@broadcom.com, devicetree@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= <rafal@milecki.pl>
References: <20220124160300.25131-1-zajec5@gmail.com>
 <20220218070729.3256-1-zajec5@gmail.com>
 <20220218070729.3256-4-zajec5@gmail.com>
From:   Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
In-Reply-To: <20220218070729.3256-4-zajec5@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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]);
>   	}
>