Re: Re: [PATCH v13 2/3] clk: eswin: Add eic7700 clock driver

[Brian Masney <bmasney@redhat.com>]

On Wed, Feb 25, 2026 at 02:55:20PM +0800, Xuyang Dong wrote:
> > > +
> > > +int eswin_clk_register_divider(struct device *dev,
> > > +			       struct eswin_divider_clock *clks,
> > > +			       int nums, struct eswin_clock_data *data)
> > > +{
> > > +	struct clk_hw *clk_hw;
> > > +	int i;
> > > +
> > > +	for (i = 0; i < nums; i++) {
> > > +		clk_hw = clk_hw_register_divider_parent_data
> > > +				(dev, clks[i].name, clks[i].parent_data,
> > > +				clks[i].flags, data->base + clks[i].offset,
> > > +				clks[i].shift, clks[i].width, clks[i].div_flags,
> > > +				&data->lock);
> > > +
> > > +		if (IS_ERR(clk_hw)) {
> > > +			while (i--)
> > > +				clk_hw_unregister_divider
> > > +					(data->clk_data.hws[clks[i].id]);
> > 
> > All of the other places you are using the devm_ variant to automate the
> > cleanup, such as devm_clk_hw_register_gate_parent_data(),
> > devm_clk_hw_register_mux_parent_data_table(), and 
> > devm_clk_hw_register_divider_parent_hw(). What do you think about adding
> > a devm_clk_hw_register_divider_parent_data() for consistency?
> > 
> 
> Hi Brian and Stephen,
> 
> Thank you for the suggestions. We agree that implementing
> devm_clk_hw_register_divider_parent_data() is a good approach.
> In v14, we'll add this function in clk-provider.h as a separate preparatory patch.
> The ESWIN clock driver will then switch to using
> devm_clk_hw_register_divider_parent_data() instead of
> clk_hw_register_divider_parent_data(), with the driver patch depending on the former.
> Does this approach better align with upstream conventions?

Yes, that sounds good.

> > You can now go out to 100 characters for the line lengths instead of 80, however,
> > I'm not sure how Stephen feels about that. Personally I think it'd make this
> > block, plus some others in this series a bit cleaner. Taking into account the
> > current indentation, this block could become this with 100 characters as the max:
> > 
> >     hw = eswin_register_clkdiv(dev, div->id, div->name, phw,
> >                                div->flags, data->base + div->offset,
> >                                div->shift, div->width, div->div_flags,
> >                                div->priv_flag, &data->lock);
> > 
> 
> Stephen,
> Brian's feedback on line length was very helpful. 
> For v14, we've kept lines within 80 characters wherever possible.
> In a few cases, such as function calls with long parameter lists, we've kept 
> slightly longer lines to preserve readability, 
> but we're happy to rewrap them if preferred.
> Does this approach work for the clk subsystem?

I don't know when Stephen will be able to respond. I recommend just
posting a new version.

I apologize in advance if he asks you to do the opposite later.

Brian