Re: [PATCH 09/12] clk: Rename clk_get_by_driver_info()

From: Sean Anderson <sean.anderson@seco.com>
To: Simon Glass <sjg@chromium.org>,
	U-Boot Mailing List <u-boot@lists.denx.de>
Cc: Tom Rini <trini@konsulko.com>
Subject: Re: [PATCH 09/12] clk: Rename clk_get_by_driver_info()
Date: Mon, 9 Aug 2021 12:41:17 -0400	[thread overview]
Message-ID: <cf1db7d3-1196-d804-3998-1feba8aea649@seco.com> (raw)
In-Reply-To: <20210807132413.3513724-10-sjg@chromium.org>

On 8/7/21 9:24 AM, Simon Glass wrote:
> This is actually a misnomer now, since the phandle info may contain
> a driver_info index or a udevice index. Rename it to use the word
> 'phandle', which seems more accurate. Add a comment while we are here.
> 
> Also add a test for this function.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>   drivers/clk/clk-uclass.c            |  7 +++----
>   drivers/mmc/ftsdc010_mci.c          |  2 +-
>   drivers/mmc/rockchip_dw_mmc.c       |  2 +-
>   drivers/mmc/rockchip_sdhci.c        |  2 +-
>   drivers/ram/rockchip/sdram_rk3399.c |  2 +-
>   drivers/spi/rk_spi.c                |  2 +-
>   include/clk.h                       | 31 ++++++++++++++++++++++++++---
>   test/dm/of_platdata.c               | 19 ++++++++++++++++++
>   8 files changed, 55 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> index 7b1ea076afd..493018b33eb 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -36,8 +36,8 @@ struct clk *dev_get_clk_ptr(struct udevice *dev)
>   }
>   
>   #if CONFIG_IS_ENABLED(OF_PLATDATA)
> -int clk_get_by_driver_info(struct udevice *dev, struct phandle_1_arg *cells,
> -			   struct clk *clk)
> +int clk_get_by_phandle(struct udevice *dev, const struct phandle_1_arg *cells,
> +		       struct clk *clk)
>   {
>   	int ret;
>   
> @@ -413,6 +413,7 @@ int clk_get_by_name(struct udevice *dev, const char *name, struct clk *clk)
>   
>   	return clk_get_by_index(dev, index, clk);
>   }
> +#endif /* OF_REAL */
>   
>   int clk_get_by_name_nodev(ofnode node, const char *name, struct clk *clk)
>   {
> @@ -465,8 +466,6 @@ int clk_release_all(struct clk *clk, int count)
>   	return 0;
>   }
>   
> -#endif /* OF_REAL */
> -
>   int clk_request(struct udevice *dev, struct clk *clk)
>   {
>   	const struct clk_ops *ops;
> diff --git a/drivers/mmc/ftsdc010_mci.c b/drivers/mmc/ftsdc010_mci.c
> index b8cafeb0431..570d54cf9d8 100644
> --- a/drivers/mmc/ftsdc010_mci.c
> +++ b/drivers/mmc/ftsdc010_mci.c
> @@ -433,7 +433,7 @@ static int ftsdc010_mmc_probe(struct udevice *dev)
>   	chip->priv = dev;
>   	chip->dev_index = 1;
>   	memcpy(priv->minmax, dtplat->clock_freq_min_max, sizeof(priv->minmax));
> -	ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->clk);
> +	ret = clk_get_by_phandle(dev, dtplat->clocks, &priv->clk);
>   	if (ret < 0)
>   		return ret;
>   #endif
> diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c
> index 855c0e7af52..7f8dea1e343 100644
> --- a/drivers/mmc/rockchip_dw_mmc.c
> +++ b/drivers/mmc/rockchip_dw_mmc.c
> @@ -123,7 +123,7 @@ static int rockchip_dwmmc_probe(struct udevice *dev)
>   	priv->minmax[0] = 400000;  /*  400 kHz */
>   	priv->minmax[1] = dtplat->max_frequency;
>   
> -	ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->clk);
> +	ret = clk_get_by_phandle(dev, dtplat->clocks, &priv->clk);
>   	if (ret < 0)
>   		return ret;
>   #else
> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
> index 93e58c2d3f1..a6ee7143953 100644
> --- a/drivers/mmc/rockchip_sdhci.c
> +++ b/drivers/mmc/rockchip_sdhci.c
> @@ -46,7 +46,7 @@ static int arasan_sdhci_probe(struct udevice *dev)
>   	host->name = dev->name;
>   	host->ioaddr = map_sysmem(dtplat->reg[0], dtplat->reg[1]);
>   	max_frequency = dtplat->max_frequency;
> -	ret = clk_get_by_driver_info(dev, dtplat->clocks, &clk);
> +	ret = clk_get_by_phandle(dev, dtplat->clocks, &clk);
>   #else
>   	max_frequency = dev_read_u32_default(dev, "max-frequency", 0);
>   	ret = clk_get_by_index(dev, 0, &clk);
> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
> index ce33fbbd7a6..c0a06dcaed0 100644
> --- a/drivers/ram/rockchip/sdram_rk3399.c
> +++ b/drivers/ram/rockchip/sdram_rk3399.c
> @@ -3107,7 +3107,7 @@ static int rk3399_dmc_init(struct udevice *dev)
>   	      priv->cic, priv->pmugrf, priv->pmusgrf, priv->pmucru, priv->pmu);
>   
>   #if CONFIG_IS_ENABLED(OF_PLATDATA)
> -	ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->ddr_clk);
> +	ret = clk_get_by_phandle(dev, dtplat->clocks, &priv->ddr_clk);
>   #else
>   	ret = clk_get_by_index(dev, 0, &priv->ddr_clk);
>   #endif
> diff --git a/drivers/spi/rk_spi.c b/drivers/spi/rk_spi.c
> index 8309a5301f2..cb80be77ae9 100644
> --- a/drivers/spi/rk_spi.c
> +++ b/drivers/spi/rk_spi.c
> @@ -183,7 +183,7 @@ static int conv_of_plat(struct udevice *dev)
>   
>   	plat->base = dtplat->reg[0];
>   	plat->frequency = 20000000;
> -	ret = clk_get_by_driver_info(dev, dtplat->clocks, &priv->clk);
> +	ret = clk_get_by_phandle(dev, dtplat->clocks, &priv->clk);
>   	if (ret < 0)
>   		return ret;
>   
> diff --git a/include/clk.h b/include/clk.h
> index f34401f48ee..a928879b122 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -89,11 +89,36 @@ struct clk_bulk {
>   
>   #if CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(CLK)
>   struct phandle_1_arg;
> -int clk_get_by_driver_info(struct udevice *dev,
> -			   struct phandle_1_arg *cells, struct clk *clk);
> +/**
> + * clk_get_by_phandle() - Get a clock by its phandle information (of-platadata)
> + *
> + * This function is used when of-platdata is enabled.
> + *
> + * This looks up a clock using the phandle info. With dtoc, each phandle in the
> + * 'clocks' property is transformed into an idx representing the device. For
> + * example:
> + *
> + *	clocks = <&dpll_mpu_ck 23>;
> + *
> + * might result in:
> + *
> + *	.clocks = {1, {23}},},

One too many closing braces?

> + *
> + * indicating that the clock is udevice idx 1 in dt-plat.c with an argument of
> + * 23. This function can return a valid clock given the above information. In
> + * this example it would return a clock containing the 'dpll_mpu_ck' device and
> + * the clock ID 23.
> + *
> + * @dev: Device containing the phandle
> + * @cells: Phandle info
> + * @clock: A pointer to a clock struct to initialise
> + * @return 0 if OK, or a negative error code.
> + */
> +int clk_get_by_phandle(struct udevice *dev, const struct phandle_1_arg *cells,
> +		       struct clk *clk);
>   
>   /**
> - * clk_get_by_index - Get/request a clock by integer index.
> + * clk_get_by_index() - Get/request a clock by integer index.
>    *
>    * This looks up and requests a clock. The index is relative to the client
>    * device; each device is assumed to have n clocks associated with it somehow,
> diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c
> index 0463cf0b433..c4a2d114104 100644
> --- a/test/dm/of_platdata.c
> +++ b/test/dm/of_platdata.c
> @@ -1,6 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0+
>   
>   #include <common.h>
> +#include <clk.h>
>   #include <dm.h>
>   #include <dt-structs.h>
>   #include <dm/test.h>
> @@ -222,3 +223,21 @@ static int dm_test_of_plat_parent(struct unit_test_state *uts)
>   }
>   DM_TEST(dm_test_of_plat_parent, UT_TESTF_SCAN_PDATA);
>   #endif
> +
> +/* Test clocks with of-platdata */
> +static int dm_test_of_plat_clk(struct unit_test_state *uts)
> +{
> +	struct dtd_sandbox_clk_test *plat;
> +	struct udevice *dev;
> +	struct clk clk;
> +
> +	ut_assertok(uclass_first_device_err(UCLASS_MISC, &dev));
> +	ut_asserteq_str("sandbox_clk_test", dev->name);
> +	plat = dev_get_plat(dev);
> +
> +	ut_assertok(clk_get_by_phandle(dev, &plat->clocks[0], &clk));
> +	ut_asserteq_str("sandbox_fixed_clock", clk.dev->name);
> +
> +	return 0;
> +}
> +DM_TEST(dm_test_of_plat_clk, UT_TESTF_SCAN_PDATA);
> 