Re: [PATCH v2 04/11] media: ccs-pll: Correctly the upper limit of maximum op_pre_pll_clk_div

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Laurent,

On Mon, Apr 21, 2025 at 11:01:11PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> In the subject line, s/Correctly/Correct/ (or an equivalent change)
> 
> On Thu, Apr 17, 2025 at 09:53:47AM +0300, Sakari Ailus wrote:
> > The PLL calculator does a search of the PLL configuration space for all
> > valid OP pre-PLL clock dividers. The maximum did not take into account CCS
> 
> s/CCS/the CCS/
> 
> > PLL flag CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER in which case also odd PLL
> > dividers (other than 1) are valid. Do that now.
> > 
> > Fixes: 4e1e8d240dff ("media: ccs-pll: Add support for extended input PLL clock divider")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/i2c/ccs-pll.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
> > index 266fcd160da6..d985686b0a36 100644
> > --- a/drivers/media/i2c/ccs-pll.c
> > +++ b/drivers/media/i2c/ccs-pll.c
> > @@ -799,7 +799,7 @@ int ccs_pll_calculate(struct device *dev, const struct ccs_pll_limits *lim,
> >  		op_lim_fr->min_pre_pll_clk_div, op_lim_fr->max_pre_pll_clk_div);
> >  	max_op_pre_pll_clk_div =
> >  		min_t(u16, op_lim_fr->max_pre_pll_clk_div,
> > -		      clk_div_even(pll->ext_clk_freq_hz /
> > +		      DIV_ROUND_UP(pll->ext_clk_freq_hz,
> >  				   op_lim_fr->min_pll_ip_clk_freq_hz));
> 
> This doesn't take the CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER flag into account.
> If I understand the code right, it's not an issue, as
> max_op_pre_pll_clk_div is only used as an upper bound for the
> pre_pll_clk_div loop, which increments by 2 when
> CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER isn't set.

Correct.

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!

-- 
Regards,

Sakari Ailus