RE: [PATCH 6.12.y-cip 2/8] clk: renesas: r9a09g077: Add xSPI core and module clocks

Hi Pavel,

Thank you for the review.

> From: Pavel Machek <[email protected]>
> Sent: 16 July 2026 11:48
> To: Prabhakar Mahadev Lad <[email protected]>
> Cc: [email protected]; Nobuhiro Iwamatsu
> <[email protected]>; Pavel Machek <[email protected]>;
> Biju Das <[email protected]>
> Subject: Re: [PATCH 6.12.y-cip 2/8] clk: renesas: r9a09g077: Add xSPI core
> and module clocks
> 
> Hi!
> 
> Minor details here:
> 
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > [PL: manually applied the changes]
> > Signed-off-by: Lad Prabhakar <[email protected]>
> 
> > +++ b/drivers/clk/renesas/r9a09g077-cpg.c
> > @@ -295,6 +334,151 @@ r9a09g077_cpg_mux_clk_register(struct device *dev,
> >  	return clk_hw->clk;
> >  }
> >
> > +static unsigned int r9a09g077_cpg_fselxspi_get_divider(struct clk_hw
> *hw, unsigned long rate,
> > +						       unsigned int num_parents) {
> > +	struct clk_fixed_factor *ff;
> > +	struct clk_hw *parent_hw;
> > +	unsigned long best_rate;
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < num_parents; i++) {
> > +		parent_hw = clk_hw_get_parent_by_index(hw, i);
> > +		best_rate = clk_hw_round_rate(parent_hw, rate);
> > +
> > +		if (best_rate == rate) {
> > +			ff = to_clk_fixed_factor(parent_hw);
> > +			return ff->div;
> > +		}
> > +	}
> > +
> > +	/* No parent could provide the exact rate - this should not happen
> */
> > +	return 0;
> > +}
> 
> If it can't happen, WARN() may be suitable?
> 
Agreed.

> > +
> > +static struct clk * __init
> > +r9a09g077_cpg_fselxspi_div_clk_register(struct device *dev,
> > +					const struct cpg_core_clk *core,
> > +					void __iomem *addr,
> ...
> > +	if (!xspi_div_ops) {
> > +		xspi_div_ops = devm_kzalloc(dev, sizeof(*xspi_div_ops),
> GFP_KERNEL);
> > +		if (!xspi_div_ops)
> > +			return  ERR_PTR(-ENOMEM);
> 
> One space would be enough between return and ERR.
> 
Ack.

> > +		memcpy(xspi_div_ops, &clk_divider_ops,
> > +		       sizeof(const struct clk_ops));
> 
> And the memcpy does fit on single line.
> 
Ack.

> (May not be worth cleaning up unless you are doing changes for that
> anyway).
>
Agreed, I will make a note of it while doing changes to this driver.

Cheers,
Prabhakar