Hello Hans,

On 19.01.2015 11:38, Hans Verkuil wrote:

Sorry for the delay.

No problem, thanks for your feedback.

quoted

Note: I'm intentionally using dma-contig instead of vmalloc, as the USB
core apparently _will_ try to use DMA for larger bulk transfers. 

As far as I can tell from looking through the usb core code it supports
scatter-gather DMA, so you should at least use dma-sg rather than dma-contig.
Physically contiguous memory should always be avoided.

OK, will this work transparently (i.e. just switch from *-contig-* to
*-sg-*)? If not, can you suggest an example driver to use as template?

I'm also missing a patch for the Kconfig that adds a dependency on MEDIA_USB_SUPPORT
and that selects VIDEOBUF2_DMA_SG.

Good point, will add that.

quoted

+err_unreg_video:
+	video_unregister_device(&sur40->vdev);
+err_unreg_v4l2:
+	v4l2_device_unregister(&sur40->v4l2);
 err_free_buffer:
 	kfree(sur40->bulk_in_buffer);
 err_free_polldev:

@@ -436,6 +604,10 @@ static void sur40_disconnect(struct usb_interface *interface)

Is this a hardwired device or hotpluggable? If it is hardwired, then this code is
OK, but if it is hotpluggable, then this isn't good enough.

It's hardwired. Out of curiosity, what would I have to change for a
hotpluggable one?

quoted

+	i->type = V4L2_INPUT_TYPE_CAMERA;
+	i->std = V4L2_STD_UNKNOWN;
+	strlcpy(i->name, "In-Cell Sensor", sizeof(i->name));

Perhaps just say "Sensor" here? I'm not sure what "In-Cell" means.

In-cell is referring to the concept of integrating sensor pixels
directly with LCD pixels, I think it's what Samsung calls it.

Thanks & best regards, Florian
-- 
SENT FROM MY DEC VT50 TERMINAL