пт, 13 груд. 2024 р. о 16:33 Thierry Reding thierry.reding@gmail.com пише:

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@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.

Agreed.

...

•         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.

I will check if this debug can be applied to plls as well as it is. If yes, I will move it higher. Thanks

...

•         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