On 01/19/2015 03:11 PM, 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. Yes, that's safe. Just remove it altogether. The buf_init and buf_release ops are matching ops that are normally only used if you have to do per-buffer initialization and/or release. These are only called when the buffer memory changes. In most drivers including this one it's not needed at all. The same is true for rcar_vin_videobuf_init: it's pointless since the list initialization is done implicitly when you add the buffer to a list with list_add_tail(). Just drop the function. Regards, Hans > > 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. > > > Cheers, > Wills. > -- > 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 > -- 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
