Hi, Jonathan >-----Original Message----- >From: Jonathan Corbet [mailto:corbet@xxxxxxx] >Sent: Monday, 17 December, 2012 00:25 >To: Albert Wang >Cc: g.liakhovetski@xxxxxx; linux-media@xxxxxxxxxxxxxxx; Libin Yang >Subject: Re: [PATCH V3 09/15] [media] marvell-ccic: add get_mcam function for marvell- >ccic driver > >On Sat, 15 Dec 2012 17:57:58 +0800 >Albert Wang <twang13@xxxxxxxxxxx> wrote: > >> This patch adds get_mcam() inline function which is prepared for >> adding soc_camera support in marvell-ccic driver > >Time for a bikeshed moment: "get" generally is understood to mean >incrementing a reference count in kernel code. Can it have a name like >vbq_to_mcam() instead? > [Albert Wang] Sure. It looks your name is more professional. :) >Also: > >> @@ -1073,14 +1073,17 @@ static int mcam_vb_queue_setup(struct vb2_queue *vq, >> 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 mcam_camera *cam = get_mcam(vb->vb2_queue); >> struct v4l2_pix_format *fmt = &cam->pix_format; >> unsigned long flags; >> int start; >> dma_addr_t dma_handle; >> + unsigned long size; >> u32 pixel_count = fmt->width * fmt->height; >> >> spin_lock_irqsave(&cam->dev_lock, flags); >> + size = vb2_plane_size(vb, 0); >> + vb2_set_plane_payload(vb, 0, size); >> dma_handle = vb2_dma_contig_plane_dma_addr(vb, 0); >> BUG_ON(!dma_handle); >> start = (cam->state == S_BUFWAIT) && !list_empty(&cam->buffers); > >There is an unrelated change here that belongs in a separate patch. > [Albert Wang] OK >> @@ -1138,9 +1141,12 @@ static void mcam_vb_wait_finish(struct vb2_queue *vq) >> */ >> static int mcam_vb_start_streaming(struct vb2_queue *vq, unsigned int count) >> { >> - struct mcam_camera *cam = vb2_get_drv_priv(vq); >> + struct mcam_camera *cam = get_mcam(vq); >> unsigned int frame; >> >> + if (count < 2) >> + return -EINVAL; >> + > >Here too - unrelated change. > [Albert Wang] Em, it looks we should add a new patch to contain these changes. :) >> if (cam->state != S_IDLE) { >> INIT_LIST_HEAD(&cam->buffers); >> return -EINVAL; >> @@ -1170,7 +1176,7 @@ static int mcam_vb_start_streaming(struct vb2_queue *vq, >unsigned int count) >> >> static int mcam_vb_stop_streaming(struct vb2_queue *vq) >> { >> - struct mcam_camera *cam = vb2_get_drv_priv(vq); >> + struct mcam_camera *cam = get_mcam(vq); >> unsigned long flags; >> >> if (cam->state == S_BUFWAIT) { >> @@ -1181,6 +1187,7 @@ static int mcam_vb_stop_streaming(struct vb2_queue *vq) >> if (cam->state != S_STREAMING) >> return -EINVAL; >> mcam_ctlr_stop_dma(cam); >> + cam->state = S_IDLE; > >...and also here ... > >jon 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
