[PATCH v6 04/10] clk: realtek: Add support for phase locked loops (PLLs)
Yu-Chun Lin [林祐君]
eleanor.lin at realtek.com
Fri Apr 10 00:43:47 PDT 2026
Hi Brian,
> Hi Cheng-Yu,
>
> On Thu, Apr 02, 2026 at 03:39:51PM +0800, Yu-Chun Lin wrote:
> > From: Cheng-Yu Lee <cylee12 at realtek.com>
> >
> > Provide a full set of PLL operations for programmable PLLs and a
> > read-only variant for fixed or hardware-managed PLLs.
> >
> > Signed-off-by: Cheng-Yu Lee <cylee12 at realtek.com>
> > Co-developed-by: Yu-Chun Lin <eleanor.lin at realtek.com>
> > Signed-off-by: Yu-Chun Lin <eleanor.lin at realtek.com>
> > ---
> > Changes in v6:
> > - Add the headers used in c file to follow the "Include What You Use"
> principle.
> > - Move to_clk_pll() from clk-pll.h to clk-pll.c to limit its scope.
> > ---
> > drivers/clk/realtek/Makefile | 2 +
> > drivers/clk/realtek/clk-pll.c | 164
> +++++++++++++++++++++++++++++++
> > drivers/clk/realtek/clk-pll.h | 42 ++++++++
> > drivers/clk/realtek/freq_table.c | 36 +++++++
> > drivers/clk/realtek/freq_table.h | 21 ++++
> > 5 files changed, 265 insertions(+)
> > create mode 100644 drivers/clk/realtek/clk-pll.c create mode 100644
> > drivers/clk/realtek/clk-pll.h create mode 100644
> > drivers/clk/realtek/freq_table.c create mode 100644
> > drivers/clk/realtek/freq_table.h
> >
> > diff --git a/drivers/clk/realtek/Makefile
> > b/drivers/clk/realtek/Makefile index 377ec776ee47..a89ad77993e9 100644
> > --- a/drivers/clk/realtek/Makefile
> > +++ b/drivers/clk/realtek/Makefile
> > @@ -2,3 +2,5 @@
> > obj-$(CONFIG_RTK_CLK_COMMON) += clk-rtk.o
> >
> > clk-rtk-y += common.o
> > +clk-rtk-y += clk-pll.o
> > +clk-rtk-y += freq_table.o
> > diff --git a/drivers/clk/realtek/clk-pll.c
> > b/drivers/clk/realtek/clk-pll.c new file mode 100644 index
> > 000000000000..44730b22a94c
> > --- /dev/null
> > +++ b/drivers/clk/realtek/clk-pll.c
> > @@ -0,0 +1,164 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2024 Realtek Semiconductor Corporation
> > + * Author: Cheng-Yu Lee <cylee12 at realtek.com> */
> > +
> > +#include <linux/regmap.h>
> > +#include "clk-pll.h"
> > +
> > +#define TIMEOUT 2000
> > +
> > +static inline struct clk_pll *to_clk_pll(struct clk_hw *hw) {
> > + struct clk_regmap *clkr = to_clk_regmap(hw);
> > +
> > + return container_of(clkr, struct clk_pll, clkr); }
> > +
> > +static int wait_freq_ready(struct clk_pll *clkp) {
> > + u32 pollval;
> > +
> > + if (!clkp->freq_ready_valid)
> > + return 0;
> > +
> > + return regmap_read_poll_timeout_atomic(clkp->clkr.regmap,
> clkp->freq_ready_reg, pollval,
> > + (pollval &
> clkp->freq_ready_mask)
> > + ==
> clkp->freq_ready_val,
> > + 0, TIMEOUT);
>
> I would put the "(pollval & clkp->freq_ready_mask) == clkp->freq_ready_val"
> on the same line to improve readability. You can go out to 100 characters.
>
> Also should the delay be greater than 0 to avoid tons of constant retries?
>
Since aligning with the open parenthesis would exceed the 100-character limit,
I will use an extra tab for the continuation line.
Also, I have increased the delay_us parameter to 1 to avoid constant retries.
The code will look like this:
return regmap_read_poll_timeout_atomic(clkp->clkr.regmap, clkp->freq_ready_reg, pollval,
(pollval & clkp->freq_ready_mask) == clkp->freq_ready_val, 1, TIMEOUT);
> > +}
> > +
> > +static bool is_power_on(struct clk_pll *clkp) {
> > + u32 val;
> > +
> > + if (!clkp->power_reg)
> > + return true;
> > +
> > + if (regmap_read(clkp->clkr.regmap, clkp->power_reg, &val))
> > + return true;
>
> Is the intention if there is an error, then it marks it as success?
>
Returning true here is a conservative design choice. When reading the power
status fails, returning true prevents unconditionally turning off the clock.
> > +
> > + return (val & clkp->power_mask) == clkp->power_val_on; }
> > +
> > +static void clk_pll_disable(struct clk_hw *hw) {
> > + struct clk_pll *clkp = to_clk_pll(hw);
> > +
> > + if (!clkp->seq_power_off)
> > + return;
> > +
> > + regmap_multi_reg_write(clkp->clkr.regmap, clkp->seq_power_off,
> > + clkp->num_seq_power_off); }
> > +
> > +static int clk_pll_is_enabled(struct clk_hw *hw) {
> > + struct clk_pll *clkp = to_clk_pll(hw);
> > +
> > + return is_power_on(clkp);
> > +}
> > +
> > +static int clk_pll_determine_rate(struct clk_hw *hw,
> > + struct clk_rate_request *req) {
> > + struct clk_pll *clkp = to_clk_pll(hw);
> > + const struct freq_table *ftblv = NULL;
> > +
> > + ftblv = ftbl_find_by_rate(clkp->freq_tbl, req->rate);
> > + if (!ftblv)
> > + return -EINVAL;
> > +
> > + req->rate = ftblv->rate;
> > + return 0;
>
> Add newline before return.
>
Ack.
> > +}
> > +
> > +static unsigned long clk_pll_recalc_rate(struct clk_hw *hw,
> > + unsigned long parent_rate) {
> > + struct clk_pll *clkp = to_clk_pll(hw);
> > + const struct freq_table *fv;
> > + u32 freq_val;
> > +
> > + if (regmap_read(clkp->clkr.regmap, clkp->freq_reg, &freq_val))
> > + return 0;
> > +
> > + freq_val &= clkp->freq_mask;
> > +
> > + fv = ftbl_find_by_val_with_mask(clkp->freq_tbl, clkp->freq_mask,
> > + freq_val);
> > + return fv ? fv->rate : 0;
>
> Add newline before return.
>
Ack.
> > +}
> > +
(snip)
> > index 000000000000..272a10e75a54
> > --- /dev/null
> > +++ b/drivers/clk/realtek/freq_table.c
> > @@ -0,0 +1,36 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#include <linux/bitops.h>
> > +#include "freq_table.h"
> > +
> > +const struct freq_table *ftbl_find_by_rate(const struct freq_table *ftbl,
> > + unsigned long rate) {
> > + unsigned long best_rate = 0;
> > + const struct freq_table *best = NULL;
>
> Put variables in reverse Christmas tree order.
Ack
>
> > +
> > + for (; !IS_FREQ_TABLE_END(ftbl); ftbl++) {
> > + if (ftbl->rate == rate)
> > + return ftbl;
> > +
> > + if (ftbl->rate > rate)
> > + continue;
> > +
> > + if (ftbl->rate > best_rate) {
> > + best_rate = ftbl->rate;
> > + best = ftbl;
> > + }
> > + }
> > +
> > + return best;
> > +}
> > +
> > +const struct freq_table *
> > +ftbl_find_by_val_with_mask(const struct freq_table *ftbl, u32 mask,
> > +u32 value) {
> > + for (; !IS_FREQ_TABLE_END(ftbl); ftbl++) {
> > + if ((ftbl->val & mask) == (value & mask))
> > + return ftbl;
> > + }
> > + return NULL;
> > +};
> > diff --git a/drivers/clk/realtek/freq_table.h
> > b/drivers/clk/realtek/freq_table.h
> > new file mode 100644
> > index 000000000000..6d9116651105
> > --- /dev/null
> > +++ b/drivers/clk/realtek/freq_table.h
> > @@ -0,0 +1,21 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +struct freq_table {
> > + u32 val;
> > + unsigned long rate;
> > +};
> > +
> > +/* ofs check */
> > +#define CLK_OFS_INVALID -1
> > +#define CLK_OFS_IS_VALID(_ofs) ((_ofs) != CLK_OFS_INVALID)
>
> Is this used anywhere?
I will drop them.
Best Regards,
Yu-Chun
>
> Brian
More information about the linux-arm-kernel
mailing list