Hi, Guennadi >-----Original Message----- >From: Guennadi Liakhovetski [mailto:g.liakhovetski@xxxxxx] >Sent: Tuesday, 05 March, 2013 18:08 >To: Albert Wang >Cc: corbet@xxxxxxx; linux-media@xxxxxxxxxxxxxxx; Libin Yang >Subject: Re: [REVIEW PATCH V4 05/12] [media] marvell-ccic: add new formats support >for marvell-ccic driver > >Looks pretty good to me now, just one nitpick: > >On Thu, 7 Feb 2013, Albert Wang wrote: > >> From: Libin Yang <lbyang@xxxxxxxxxxx> >> >> This patch adds the new formats support for marvell-ccic. >> >> Signed-off-by: Albert Wang <twang13@xxxxxxxxxxx> >> Signed-off-by: Libin Yang <lbyang@xxxxxxxxxxx> >> --- >> drivers/media/platform/marvell-ccic/mcam-core.c | 189 +++++++++++++++++++---- >> drivers/media/platform/marvell-ccic/mcam-core.h | 6 + >> 2 files changed, 162 insertions(+), 33 deletions(-) >> >> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c >b/drivers/media/platform/marvell-ccic/mcam-core.c >> index f89fce7..0f9604e 100755 >> --- a/drivers/media/platform/marvell-ccic/mcam-core.c >> +++ b/drivers/media/platform/marvell-ccic/mcam-core.c > >[snip] > >> @@ -971,11 +1062,35 @@ static void mcam_vb_buf_queue(struct vb2_buffer *vb) >> { >> struct mcam_vb_buffer *mvb = vb_to_mvb(vb); >> struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue); >> + struct v4l2_pix_format *fmt = &cam->pix_format; >> unsigned long flags; >> int start; >> + dma_addr_t dma_handle; >> + u32 pixel_count = fmt->width * fmt->height; >> >> spin_lock_irqsave(&cam->dev_lock, flags); >> + dma_handle = vb2_dma_contig_plane_dma_addr(vb, 0); >> + BUG_ON(!dma_handle); > >Again - a bit too strong. But the truth is - .buf_queue() cannot fail... >Would it be possible to move this pointer calculation to .buf_prepare(), >which does return an error code? > Yes, we will remove all BUG_ON macros, replace them with error or warning msg. >Thanks >Guennadi >--- >Guennadi Liakhovetski, Ph.D. >Freelance Open-Source Software Developer >http://www.open-technology.de/ Thanks Albert Wang 86-21-61092656 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
