Hi Mauro,

Thank you for pointing to this.

On  1.11.2017 14:16, Mauro Carvalho Chehab wrote:
> if output->wm_num is bigger than 1, the value for reg is
If output->wn_num equals 2, we handle all cases (i == 0, i == 1) and set reg 
properly.
If output->wn_num is bigger than 2, then reg will not be initialized. However 
this is something that "cannot happen" and because of this the case is not 
handled.

So I think that there is nothing wrong really but we have to do something to 
remove the warning. I agree with your patch, it is technically not a right 
value for reg but any cases in which wm_num is bigger than 2 are not supported 
anyway and should not happen.

> not initialized, as warned by smatch:
>       drivers/media/platform/qcom/camss-8x16/camss-vfe.c:633 
> vfe_set_xbar_cfg() error: uninitialized symbol 'reg'.
>       drivers/media/platform/qcom/camss-8x16/camss-vfe.c:637 
> vfe_set_xbar_cfg() error: uninitialized symbol 'reg'.
> 
> I didn't check the logic into its details, but there is at least
> one point where wm_num is made equal to two. So, something
> seem broken.
> 
> For now, I just reset it to zero, and added a FIXME. Hopefully,
> the driver authors will know if this is OK, or if something else
> is needed there.
> 
> Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
> ---
>  drivers/media/platform/qcom/camss-8x16/camss-vfe.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c 
> b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
> index b22d2dfcd3c2..388431f747fa 100644
> --- a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
> @@ -622,6 +622,8 @@ static void vfe_set_xbar_cfg(struct vfe_device *vfe, 
> struct vfe_output *output,
>                       reg = VFE_0_BUS_XBAR_CFG_x_M_PAIR_STREAM_EN;
>                       if (p == V4L2_PIX_FMT_NV12 || p == V4L2_PIX_FMT_NV16)
>                               reg |= 
> VFE_0_BUS_XBAR_CFG_x_M_PAIR_STREAM_SWAP_INTER_INTRA;
> +             } else {
> +                     reg = 0;        /* FIXME: is it the right value for i > 
> 1? */
>               }
>  
>               if (output->wm_idx[i] % 2 == 1)
> 

-- 
Best regards,
Todor Tomov

Reply via email to