> +/* ISI interrupt service routine */ > +static irqreturn_t isi_interrupt(int irq, void *dev_id) > +{ > + struct atmel_isi *isi = dev_id; > + u32 status, mask, pending; > + irqreturn_t ret = IRQ_NONE; > + > + spin_lock(&isi->lock); > + > + status = isi_readl(isi, ISI_STATUS); > + mask = isi_readl(isi, ISI_INTMASK); > + pending = status & mask; > + > + if (pending & ISI_CTRL_SRST) { > + complete(&isi->isi_complete); > + isi_writel(isi, ISI_INTDIS, ISI_CTRL_SRST); > + ret = IRQ_HANDLED; > + } > + if (pending & ISI_CTRL_DIS) { > + complete(&isi->isi_complete); > + isi_writel(isi, ISI_INTDIS, ISI_CTRL_DIS); > + ret = IRQ_HANDLED; > + } no else here? > + > + if (pending & ISI_SR_VSYNC) { > + switch (isi->state) { > + case ISI_STATE_IDLE: > + isi->state = ISI_STATE_READY; > + wake_up_interruptible(&isi->capture_wq); > + break; > + } really switch here? > + } else if (likely(pending & ISI_SR_CXFR_DONE)) { > + ret = atmel_isi_handle_streaming(isi); > + } > + > + spin_unlock(&isi->lock); > + > + return ret; > +} > + > +#define WAIT_ISI_RESET 1 > +#define WAIT_ISI_DISABLE 0 > +static int atmel_isi_wait_status(int wait_reset, struct atmel_isi *isi) I thinkhave teh atmel_isti first parameter is better > +{ > + unsigned long timeout; > + /* > + * The reset or disable will only succeed if we have a > + * pixel clock from the camera. > + */ > + init_completion(&isi->isi_complete); > + > + if (wait_reset) { > + isi_writel(isi, ISI_INTEN, ISI_CTRL_SRST); > + isi_writel(isi, ISI_CTRL, ISI_CTRL_SRST); > + } else { > + isi_writel(isi, ISI_INTEN, ISI_CTRL_DIS); > + isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS); > + } > + > + timeout = wait_for_completion_timeout(&isi->isi_complete, > + msecs_to_jiffies(100)); > + if (timeout == 0) > + return -ETIMEDOUT; > + > + return 0; > +} > + > +/* ------------------------------------------------------------------ > + Videobuf operations > + ------------------------------------------------------------------*/ > +static int queue_setup(struct vb2_queue *vq, unsigned int *nbuffers, > + unsigned int *nplanes, unsigned long sizes[], > + void *alloc_ctxs[]) > +{ > + struct soc_camera_device *icd = soc_camera_from_vb2q(vq); > + struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent); > + struct atmel_isi *isi = ici->priv; > + unsigned long size; > + int ret, bytes_per_line; > + > + /* Reset ISI */ > + ret = atmel_isi_wait_status(WAIT_ISI_RESET, isi); > + if (ret < 0) { > + dev_err(icd->dev.parent, "Reset ISI timed out\n"); > + return ret; > + } > + /* Disable all interrupts */ > + isi_writel(isi, ISI_INTDIS, ~0UL); > + > + bytes_per_line = soc_mbus_bytes_per_line(icd->user_width, > + icd->current_fmt->host_fmt); > + > + if (bytes_per_line < 0) > + return bytes_per_line; > + > + size = bytes_per_line * icd->user_height; > + > + if (*nbuffers == 0) > + *nbuffers = MAX_BUFFER_NUMS; > + if (*nbuffers > MAX_BUFFER_NUMS) else here > + *nbuffers = MAX_BUFFER_NUMS; > + > + if (size * *nbuffers > VID_LIMIT_BYTES) > + *nbuffers = VID_LIMIT_BYTES / size; > + > + *nplanes = 1; > + sizes[0] = size; > + alloc_ctxs[0] = isi->alloc_ctx; > + > + isi->sequence = 0; > + isi->active = NULL; > + > + dev_dbg(icd->dev.parent, "%s, count=%d, size=%ld\n", __func__, > + *nbuffers, size); > + > + return 0; > +} > + > +static int buffer_init(struct vb2_buffer *vb) > +{ > + struct frame_buffer *buf = container_of(vb, struct frame_buffer, vb); > + > + buf->p_fb_desc = NULL; > + buf->fb_desc_phys = 0; memset 0? > + INIT_LIST_HEAD(&buf->list); > + > + return 0; > +} > + otherwise the patch look good if you fix the upper issue Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@xxxxxxxxxxxx> Best Regards, J. -- 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