diff mbox series

[v2,09/11] media: ccs-pll: Better validate VT PLL branch

Message ID 20250417065354.311617-10-sakari.ailus@linux.intel.com
State New
Headers show
Series CCS PLL fixes and improvements | expand

Commit Message

Sakari Ailus April 17, 2025, 6:53 a.m. UTC
Check that the VT PLL dividers are actually found, don't trust they always
are even though they should be.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs-pll.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Laurent Pinchart April 21, 2025, 8:24 p.m. UTC | #1
Hi Sakari,

Thank you for the patch.

On Thu, Apr 17, 2025 at 09:53:52AM +0300, Sakari Ailus wrote:
> Check that the VT PLL dividers are actually found, don't trust they always
> are even though they should be.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/ccs-pll.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
> index 3f8153fb4af0..fc6f8aff5fd8 100644
> --- a/drivers/media/i2c/ccs-pll.c
> +++ b/drivers/media/i2c/ccs-pll.c
> @@ -449,7 +449,7 @@ static int ccs_pll_calculate_vt_tree(struct device *dev,
>  	return -EINVAL;
>  }
>  
> -static void
> +static int
>  ccs_pll_calculate_vt(struct device *dev, const struct ccs_pll_limits *lim,
>  		     const struct ccs_pll_branch_limits_bk *op_lim_bk,
>  		     struct ccs_pll *pll, struct ccs_pll_branch_fr *pll_fr,
> @@ -572,6 +572,8 @@ ccs_pll_calculate_vt(struct device *dev, const struct ccs_pll_limits *lim,
>  		if (best_pix_div < SHRT_MAX >> 1)
>  			break;
>  	}
> +	if (best_pix_div == SHRT_MAX >> 1)
> +		return -EINVAL;

I think I would have written

	if (vt_div > max_vt_div)
		return -EINVAL;

to match the for loop condition, this seems a bit more readable to me.
The result should be the same though, so either way,

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

>  
>  	pll->vt_bk.sys_clk_div = DIV_ROUND_UP(vt_div, best_pix_div);
>  	pll->vt_bk.pix_clk_div = best_pix_div;
> @@ -584,6 +586,8 @@ ccs_pll_calculate_vt(struct device *dev, const struct ccs_pll_limits *lim,
>  out_calc_pixel_rate:
>  	pll->pixel_rate_pixel_array =
>  		pll->vt_bk.pix_clk_freq_hz * pll->vt_lanes;
> +
> +	return 0;
>  }
>  
>  /*
> @@ -863,8 +867,10 @@ int ccs_pll_calculate(struct device *dev, const struct ccs_pll_limits *lim,
>  		if (pll->flags & CCS_PLL_FLAG_DUAL_PLL)
>  			break;
>  
> -		ccs_pll_calculate_vt(dev, lim, op_lim_bk, pll, op_pll_fr,
> -				     op_pll_bk, cphy, phy_const);
> +		rval = ccs_pll_calculate_vt(dev, lim, op_lim_bk, pll, op_pll_fr,
> +					    op_pll_bk, cphy, phy_const);
> +		if (rval)
> +			continue;
>  
>  		rval = check_bk_bounds(dev, lim, pll, PLL_VT);
>  		if (rval)
diff mbox series

Patch

diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
index 3f8153fb4af0..fc6f8aff5fd8 100644
--- a/drivers/media/i2c/ccs-pll.c
+++ b/drivers/media/i2c/ccs-pll.c
@@ -449,7 +449,7 @@  static int ccs_pll_calculate_vt_tree(struct device *dev,
 	return -EINVAL;
 }
 
-static void
+static int
 ccs_pll_calculate_vt(struct device *dev, const struct ccs_pll_limits *lim,
 		     const struct ccs_pll_branch_limits_bk *op_lim_bk,
 		     struct ccs_pll *pll, struct ccs_pll_branch_fr *pll_fr,
@@ -572,6 +572,8 @@  ccs_pll_calculate_vt(struct device *dev, const struct ccs_pll_limits *lim,
 		if (best_pix_div < SHRT_MAX >> 1)
 			break;
 	}
+	if (best_pix_div == SHRT_MAX >> 1)
+		return -EINVAL;
 
 	pll->vt_bk.sys_clk_div = DIV_ROUND_UP(vt_div, best_pix_div);
 	pll->vt_bk.pix_clk_div = best_pix_div;
@@ -584,6 +586,8 @@  ccs_pll_calculate_vt(struct device *dev, const struct ccs_pll_limits *lim,
 out_calc_pixel_rate:
 	pll->pixel_rate_pixel_array =
 		pll->vt_bk.pix_clk_freq_hz * pll->vt_lanes;
+
+	return 0;
 }
 
 /*
@@ -863,8 +867,10 @@  int ccs_pll_calculate(struct device *dev, const struct ccs_pll_limits *lim,
 		if (pll->flags & CCS_PLL_FLAG_DUAL_PLL)
 			break;
 
-		ccs_pll_calculate_vt(dev, lim, op_lim_bk, pll, op_pll_fr,
-				     op_pll_bk, cphy, phy_const);
+		rval = ccs_pll_calculate_vt(dev, lim, op_lim_bk, pll, op_pll_fr,
+					    op_pll_bk, cphy, phy_const);
+		if (rval)
+			continue;
 
 		rval = check_bk_bounds(dev, lim, pll, PLL_VT);
 		if (rval)