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

["Yu-Chun Lin [林祐君]" <eleanor.lin@realtek.com>]

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.