[PATCH v2 1/2] driver: clk: tegra: partially support PLL clocks

[Thierry Reding]

On Thu, Dec 12, 2024 at 12:06:45PM +0200, Svyatoslav Ryhel wrote:
> Return PLL id into struct clk if PLL is parsed from device
> tree instead of throwing an error. Allow requesting PLL
> clock rate via get_rate op.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95 at gmail.com>
> ---
>  drivers/clk/tegra/tegra-car-clk.c | 49 ++++++++++++++++++++++++-------
>  1 file changed, 39 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/clk/tegra/tegra-car-clk.c b/drivers/clk/tegra/tegra-car-clk.c
> index 1d61f8dc378..dd4d7791120 100644
> --- a/drivers/clk/tegra/tegra-car-clk.c
> +++ b/drivers/clk/tegra/tegra-car-clk.c
> @@ -10,6 +10,9 @@
>  #include <asm/arch/clock.h>
>  #include <asm/arch-tegra/clk_rst.h>
>  
> +#define TEGRA_CAR_CLK_PLL	BIT(0)
> +#define TEGRA_CAR_CLK_PERIPH	BIT(1)
> +
>  static int tegra_car_clk_request(struct clk *clk)
>  {
>  	debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> @@ -20,30 +23,50 @@ static int tegra_car_clk_request(struct clk *clk)
>  	 * varies per SoC) are the peripheral clocks, which use a numbering
>  	 * scheme that matches HW registers 1:1. There are other clock IDs
>  	 * beyond this that are assigned arbitrarily by the Tegra CAR DT
> -	 * binding. Due to the implementation of this driver, it currently
> -	 * only supports the peripheral IDs.
> +	 * binding.
>  	 */
> -	if (clk->id >= PERIPH_ID_COUNT)
> -		return -EINVAL;
> +	if (!(clk->id >= PERIPH_ID_COUNT)) {

Maybe just do clk->id < PERIPH_ID_COUNT and get rid of the negation?
Seems a bit more straightforward.

> +		clk->data |= TEGRA_CAR_CLK_PERIPH;
> +		return 0;
> +	}
>  
> -	return 0;
> +	/* If check for periph failed, then check for PLL clock id */
> +	int id = clk_id_to_pll_id(clk->id);
> +
> +	if (clock_id_is_pll(id)) {
> +		clk->id = id;
> +		clk->data |= TEGRA_CAR_CLK_PLL;
> +		return 0;
> +	}
> +
> +	return -EINVAL;
>  }
>  
>  static ulong tegra_car_clk_get_rate(struct clk *clk)
>  {
> -	enum clock_id parent;
> +	if (clk->data & TEGRA_CAR_CLK_PLL)
> +		return clock_get_rate(clk->id);
>  
> -	debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
> -	      clk->id);
> +	if (clk->data & TEGRA_CAR_CLK_PERIPH) {
> +		enum clock_id parent;
>  
> -	parent = clock_get_periph_parent(clk->id);
> -	return clock_get_periph_rate(clk->id, parent);
> +		debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__,
> +		      clk, clk->dev, clk->id);

Is there any specific reason why you moved the debug() call into the
branch? It looks like it was supposed to be a simple function tracing
message, so it would probably be useful to have for PLL clocks as well.

> +
> +		parent = clock_get_periph_parent(clk->id);
> +		return clock_get_periph_rate(clk->id, parent);
> +	}
> +
> +	return -1U;
>  }
>  
>  static ulong tegra_car_clk_set_rate(struct clk *clk, ulong rate)
>  {
>  	enum clock_id parent;
>  
> +	if (clk->data & TEGRA_CAR_CLK_PLL)
> +		return 0;
> +
>  	debug("%s(clk=%p, rate=%lu) (dev=%p, id=%lu)\n", __func__, clk, rate,
>  	      clk->dev, clk->id);

Same here.

>  
> @@ -53,6 +76,9 @@ static ulong tegra_car_clk_set_rate(struct clk *clk, ulong rate)
>  
>  static int tegra_car_clk_enable(struct clk *clk)
>  {
> +	if (clk->data & TEGRA_CAR_CLK_PLL)
> +		return 0;
> +
>  	debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
>  	      clk->id);

And here.

>  
> @@ -63,6 +89,9 @@ static int tegra_car_clk_enable(struct clk *clk)
>  
>  static int tegra_car_clk_disable(struct clk *clk)
>  {
> +	if (clk->data & TEGRA_CAR_CLK_PLL)
> +		return 0;
> +
>  	debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
>  	      clk->id);
>  

And here, too.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20241213/938674b2/attachment.sig>