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 <[email protected]> > --- > 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
signature.asc
Description: PGP signature
