Re: [PATCH v2 1/2] clk: Allow clk_get_by_name() with NULL name

From: Sean Anderson <seanga2@gmail.com>
To: Samuel Holland <samuel@sholland.org>,
	Lukasz Majewski <lukma@denx.de>, Simon Glass <sjg@chromium.org>
Cc: Dario Binacchi <dario.binacchi@amarulasolutions.com>,
	Michal Suchanek <msuchanek@suse.de>,
	u-boot@lists.denx.de
Subject: Re: [PATCH v2 1/2] clk: Allow clk_get_by_name() with NULL name
Date: Sun, 12 Feb 2023 12:51:42 -0500	[thread overview]
Message-ID: <57754ad1-9be0-9280-7912-d3d9a5d6dd8b@gmail.com> (raw)
In-Reply-To: <20230122000252.53642-1-samuel@sholland.org>

On 1/21/23 19:02, Samuel Holland wrote:
> This allows devm_clock_get(dev, NULL) to work and get the first clock,
> which is common in code ported from Linux.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
> Changes in v2:
>   - Move index error check inside if statement
>   - Update function comment
>   - Add unit test
> 
>   drivers/clk/clk-uclass.c | 12 +++++++-----
>   include/clk.h            |  4 ++--
>   test/dm/clk.c            |  5 +++++
>   3 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> index 2f9635524c..dc3e9d6a26 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -399,16 +399,18 @@ int clk_get_by_name(struct udevice *dev, const char *name, struct clk *clk)
>   
>   int clk_get_by_name_nodev(ofnode node, const char *name, struct clk *clk)
>   {
> -	int index;
> +	int index = 0;
>   
>   	debug("%s(node=%p, name=%s, clk=%p)\n", __func__,
>   		ofnode_get_name(node), name, clk);
>   	clk->dev = NULL;
>   
> -	index = ofnode_stringlist_search(node, "clock-names", name);
> -	if (index < 0) {
> -		debug("fdt_stringlist_search() failed: %d\n", index);
> -		return index;
> +	if (name) {
> +		index = ofnode_stringlist_search(node, "clock-names", name);
> +		if (index < 0) {
> +			debug("fdt_stringlist_search() failed: %d\n", index);
> +			return index;
> +		}
>   	}
>   
>   	return clk_get_by_index_nodev(node, index, clk);
> diff --git a/include/clk.h b/include/clk.h
> index 138766bd49..d91285235f 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -167,7 +167,7 @@ int clk_get_bulk(struct udevice *dev, struct clk_bulk *bulk);
>    * clk_get_by_name() - Get/request a clock by name.
>    * @dev:	The client device.
>    * @name:	The name of the clock to request, within the client's list of
> - *		clocks.
> + *		clocks, or NULL to request the first clock in the list.
>    * @clk:	A pointer to a clock struct to initialize.
>    *
>    * This looks up and requests a clock. The name is relative to the client
> @@ -184,7 +184,7 @@ int clk_get_by_name(struct udevice *dev, const char *name, struct clk *clk);
>    * clk_get_by_name_nodev - Get/request a clock by name without a device.
>    * @node:	The client ofnode.
>    * @name:	The name of the clock to request, within the client's list of
> - *		clocks.
> + *		clocks, or NULL to request the first clock in the list.
>    * @clk:	A pointer to a clock struct to initialize.
>    *
>    * Return: 0 if OK, or a negative error code.
> diff --git a/test/dm/clk.c b/test/dm/clk.c
> index 21997ed892..f48de05436 100644
> --- a/test/dm/clk.c
> +++ b/test/dm/clk.c
> @@ -26,6 +26,11 @@ static int dm_test_clk_base(struct unit_test_state *uts)
>   	ut_assertok(uclass_get_device_by_name(UCLASS_MISC, "clk-test", &dev));
>   
>   	/* Get the same clk port in 2 different ways and compare */
> +	ut_assertok(clk_get_by_index(dev, 0, &clk_method1));
> +	ut_assertok(clk_get_by_name(dev, NULL, &clk_method2));
> +	ut_asserteq(clk_is_match(&clk_method1, &clk_method2), true);
> +	ut_asserteq(clk_method1.id, clk_method2.id);
> +
>   	ut_assertok(clk_get_by_index(dev, 1, &clk_method1));
>   	ut_assertok(clk_get_by_index_nodev(dev_ofnode(dev), 1, &clk_method2));
>   	ut_asserteq(clk_is_match(&clk_method1, &clk_method2), true);

Reviewed-by: Sean Anderson <seanga2@gmail.com>