Re: [PATCH] media: tc358743: fix missing return of error code in tc358743_probe_of

[Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>]

Hi Josua

Thanks for the patch.

On Tue, 22 Jun 2021 at 21:33, Josua Mayer <josua@xxxxxxxxxxxxx> wrote:
>
> When device-tree configures an unsupported combinaion of number of lanes,

s/combinaion/combination

> and link frequency, e.g. by exceeding 1Gbps per lane, 0 is returned,
> wrongly indicating success. In this case, return EINVAL instead!

The change you've provided is in the check that the lane link
frequency is within range only. Nothing to do with the number of
lanes.

> This fixes a divide-by-zero crash in tc358743_num_csi_lanes_needed,
> where the divisor becomes zero because pll_fbd has been left at 0 by probe.

There's a very similar case in this function if the refclk frequency
is not supported.
That will cause a divide-by-zero when calculating pll_fbd with a 0
pll_prd value.

@@ -1980,6 +1980,7 @@ static int tc358743_probe_of(struct tc358743_state *state)
        default:
                dev_err(dev, "unsupported refclk rate: %u Hz\n",
                        state->pdata.refclk_hz);
+               ret = -EINVAL;
                goto disable_clk;
        }

Could you roll that one in too?

Cheers
  Dave

> Signed-off-by: Josua Mayer <josua@xxxxxxxxxxxxx>
> ---
>  drivers/media/i2c/tc358743.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> index 1b309bb743c7..f21da11caf22 100644
> --- a/drivers/media/i2c/tc358743.c
> +++ b/drivers/media/i2c/tc358743.c
> @@ -1974,6 +1974,7 @@ static int tc358743_probe_of(struct tc358743_state *state)
>         bps_pr_lane = 2 * endpoint.link_frequencies[0];
>         if (bps_pr_lane < 62500000U || bps_pr_lane > 1000000000U) {
>                 dev_err(dev, "unsupported bps per lane: %u bps\n", bps_pr_lane);
> +               ret = -EINVAL;
>                 goto disable_clk;
>         }
>
> --
> 2.32.0
>