RE: [PATCH v14 06/11] clk: realtek: Add support for gate clock

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

Hi Jerome,

> > +static int rtk_clk_regmap_gate_enable(struct clk_hw *hw) {
> > +     struct rtk_clk_regmap_gate *clkg = to_rtk_clk_regmap_gate(hw);
> > +     unsigned int mask;
> > +     unsigned int val;
> > +
> > +     mask = BIT(clkg->bit_idx);
> > +     val = BIT(clkg->bit_idx);
> > +
> > +     if (clkg->write_en) {
> > +             mask |= BIT(clkg->bit_idx + 1);
> > +             val |= BIT(clkg->bit_idx + 1);
> > +     }
> > +
> > +     return regmap_update_bits(clkg->clkr.regmap, clkg->gate_ofs,
> > +mask, val); }
> > +
> > +static void rtk_clk_regmap_gate_disable(struct clk_hw *hw) {
> > +     struct rtk_clk_regmap_gate *clkg = to_rtk_clk_regmap_gate(hw);
> > +     unsigned int mask;
> > +     unsigned int val;
> > +
> > +     mask = BIT(clkg->bit_idx);
> > +     val = 0;
> > +
> > +     if (clkg->write_en) {
> > +             mask |= BIT(clkg->bit_idx + 1);
> > +             val |= BIT(clkg->bit_idx + 1);
> > +     }
> 
> Could add a comment explaining this write_en thing ? Some kind of volatile bit
> to commit the change ?
> 

Will add a comment and improve commit message.
It is a hardware-specific design. To modify the target bit, the adjacent
write-enable bit must be asserted.

> > +
> > +     regmap_update_bits(clkg->clkr.regmap, clkg->gate_ofs, mask,
> > + val);
> 
> Nitpick: the 2 functions above more less copy/paste.
> 

I will extract the common logic into a helper function.

Best Regards,
Yu-Chun