Re: [PATCH v2 01/10] media: ipu3-cio2: Simplify cleanup code

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Andy,

Thank you for the patch. Glad to see interest in the CIO2 driver :-)

On Mon, Aug 17, 2020 at 07:07:24PM +0300, Andy Shevchenko wrote:
> The code looks more nicer if we use:
> 	while (i--)
> instead:
> 	for (i = i - 1; i >= 0; i--)
> 
> This would also allow making 'i' unsigned again.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: converted i to unsigned (Sakari)
>  drivers/media/pci/intel/ipu3/ipu3-cio2.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> index 92f5eadf2c99..cb74d49934f1 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> @@ -847,7 +847,7 @@ static int cio2_vb2_buf_init(struct vb2_buffer *vb)
>  	unsigned int lops = DIV_ROUND_UP(pages + 1, entries_per_page);
>  	struct sg_table *sg;
>  	struct sg_dma_page_iter sg_iter;
> -	int i, j;
> +	unsigned int i, j;
>  
>  	if (lops <= 0 || lops > CIO2_MAX_LOPS) {
>  		dev_err(dev, "%s: bad buffer size (%i)\n", __func__,
> @@ -887,7 +887,7 @@ static int cio2_vb2_buf_init(struct vb2_buffer *vb)
>  	b->lop[i][j] = cio2->dummy_page_bus_addr >> PAGE_SHIFT;
>  	return 0;
>  fail:
> -	for (i--; i >= 0; i--)
> +	while (i--)
>  		dma_free_coherent(dev, CIO2_PAGE_SIZE,
>  				  b->lop[i], b->lop_bus_addr[i]);

Looks good to me.

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

There's an additional issue though, if vb2_dma_sg_plane_desc() fails, we
should free all the allocated memory.

>  	return -ENOMEM;

-- 
Regards,

Laurent Pinchart