Hi, On Mon, 19 Jan 2015, William Towle wrote: > > On Mon, 19 Jan 2015, Guennadi Liakhovetski wrote: > > > > > On Thu, 18 Dec 2014, Ben Hutchings wrote: > > > Well, I thought that too. Will's submission from last week has that > > > change: > > > http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/87009 > > > Anyway, yes, that looks better! But I would still consider keeping buffers > > on the list in .buf_clean(), in which case you can remove it. And walk the > > list instead of the VB2 internal buffer array, as others have pointed out. > > Hi Guennadi, > Thanks for the clarification. Ian (when he was with us) did say "it > was particularly difficult to understand WTH this driver was doing". > > Regarding your first point, if it's safe to skip the actions left > in rcar_vin_videobuf_release() then I will do a further rework to > remove it completely. > > Regarding your second, in the patchset Ben linked to above we think > we have the appropriate loops: a for loop for queue_buf[], and > list_for_each_safe() for anything left in priv->capture; this is > consistent with rcar_vin_fill_hw_slot() setting up queue_buf[] with > pointers unlinked from priv->capture. This in turn suggests that we > are right not to call list_del_init() in both of > rcar_vin_stop_streaming()'s loops ... as long as I've correctly > interpreted the code and everyone's feedback thus far. I'm referring to this comment by Hans Verkuil of 14 August last year: > I'm assuming all buffers that are queued to the driver via buf_queue() are > linked into priv->capture. So you would typically call vb2_buffer_done > when you are walking that list: > > list_for_each_safe(buf_head, tmp, &priv->capture) { > // usually you go from buf_head to the real buffer struct > // containing a vb2_buffer struct > vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR); > list_del_init(buf_head); > } > > Please use this rather than looking into internal vb2_queue > datastructures. I think, that's the right way to implement that clean up loop. Thanks Guennadi -- 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