RE: [PATCH v6 06/10] clk: realtek: Add support for mux clock

Yu-Chun Lin [林祐君]
2026-04-10T08:25:19Z

Hi Brian,

> Hi Yu-Chun and Cheng-Yu,
>
> On Thu, Apr 02, 2026 at 03:39:53PM +0800, Yu-Chun Lin wrote:
> > From: Cheng-Yu Lee <cylee12@realtek.com>
> >
> > Add a simple regmap-based clk_ops implementation for Realtek mux clocks.
> >
> > The implementation supports parent selection and rate determination
> > through regmap-backed register access.
> >
> > Signed-off-by: Cheng-Yu Lee <cylee12@realtek.com>
> > Co-developed-by: Yu-Chun Lin <eleanor.lin@realtek.com>
> > Signed-off-by: Yu-Chun Lin <eleanor.lin@realtek.com>
> > ---
> > Changes in v6:
> > - Add the headers used in c file to follow the "Include What You Use"
> principle.
> > ---
> > drivers/clk/realtek/Makefile | 1 +
> > drivers/clk/realtek/clk-regmap-mux.c | 48
> > ++++++++++++++++++++++++++++ drivers/clk/realtek/clk-regmap-mux.h |
> > 43 +++++++++++++++++++++++++
> > 3 files changed, 92 insertions(+)
> > create mode 100644 drivers/clk/realtek/clk-regmap-mux.c
> > create mode 100644 drivers/clk/realtek/clk-regmap-mux.h
> >
> > diff --git a/drivers/clk/realtek/Makefile
> > b/drivers/clk/realtek/Makefile index 74375f8127ac..f90dc57fcfdb 100644
> > --- a/drivers/clk/realtek/Makefile
> > +++ b/drivers/clk/realtek/Makefile
> > @@ -5,4 +5,5 @@ clk-rtk-y += common.o
> >
> > clk-rtk-y += clk-pll.o
> > clk-rtk-y += clk-regmap-gate.o
> > +clk-rtk-y += clk-regmap-mux.o
> > clk-rtk-y += freq_table.o
> > diff --git a/drivers/clk/realtek/clk-regmap-mux.c
> > b/drivers/clk/realtek/clk-regmap-mux.c
> > new file mode 100644
> > index 000000000000..068b056d61f0
> > --- /dev/null
> > +++ b/drivers/clk/realtek/clk-regmap-mux.c
> > @@ -0,0 +1,48 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2017 Realtek Semiconductor Corporation
> > + * Author: Cheng-Yu Lee <cylee12@realtek.com> */
> > +
> > +#include <linux/regmap.h>
> > +#include <linux/clk-provider.h>
>
> Sort the includes.
>

Ack.

> > +#include "clk-regmap-mux.h"
> > +
> > +static u8 clk_regmap_mux_get_parent(struct clk_hw *hw) {
> > + struct clk_regmap_mux *clkm = to_clk_regmap_mux(hw);
> > + int num_parents = clk_hw_get_num_parents(hw);
> > + u32 val;
> > + int ret;
> > +
> > + ret = regmap_read(clkm->clkr.regmap, clkm->mux_ofs, &val);
> > + if (ret)
> > + return 0;
>
> This is another case where it'd be nice to get the get_parent declaration fixed.
> Stephen recently linked to some work of his from 2022 here.
>
> https://lore.kernel.org/linux-clk/177431305509.5403.15386021337517970667
> @lazor/
>
> There's nothing for you to do right now.
>
> > +
> > + val = val >> clkm->shift & clkm->mask;
>
> I know there's the order of operations, however for clarity I would just include
> some () here to make it clear the expected order.

Ack.

> > +
> > + if (val >= num_parents)
>
> Remove newline before if.

Ack.

>
> > + return 0;
> > +
> > + return val;
>
> Or you could just use a ternary operator:
>
> return val >= num_parents ? 0 : val;
>

Ack.

> > +}
> > +
> > +static int clk_regmap_mux_set_parent(struct clk_hw *hw, u8 index) {
> > + struct clk_regmap_mux *clkm = to_clk_regmap_mux(hw);
> > +
> > + return regmap_update_bits(clkm->clkr.regmap, clkm->mux_ofs,
> > + clkm->mask << clkm->shift, index <<
> > +clkm->shift); }
> > +
> > +const struct clk_ops rtk_clk_regmap_mux_ops = {
> > + .set_parent = clk_regmap_mux_set_parent,
> > + .get_parent = clk_regmap_mux_get_parent,
> > + .determine_rate = __clk_mux_determine_rate, };
> > +EXPORT_SYMBOL_NS_GPL(rtk_clk_regmap_mux_ops, "REALTEK_CLK");
> > +
> > +const struct clk_ops rtk_clk_regmap_mux_ro_ops = {
> > + .get_parent = clk_regmap_mux_get_parent, };
> > +EXPORT_SYMBOL_NS_GPL(rtk_clk_regmap_mux_ro_ops, "REALTEK_CLK");
>
> rtk_clk_regmap_mux_ro_ops is exported, however the declaration is not
> actually declared in any header files.
>
> Brian

Since it is currently unused, I will drop the 'rtk_clk_regmap_mux_ro_ops'
implementation.

Best Regards,
Yu-Chun.