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

[Svyatoslav Ryhel]

пт, 13 груд. 2024 р. о 16:33 Thierry Reding <thierry.reding at 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 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.
>

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