Hi Maxime, On 2018-04-03 15:48:59 +0200, Maxime Ripard wrote: > Hi Niklas, > > On Thu, Mar 29, 2018 at 02:35:34PM +0200, Niklas Söderlund wrote: > > > + /* > > > + * Create a static mapping between the CSI virtual channels > > > + * and the input streams. > > > + * > > > + * This should be enhanced, but v4l2 lacks the support for > > > + * changing that mapping dynamically at the moment. > > > + * > > > + * We're protected from the userspace setting up links at the > > > + * same time by the upper layer having called > > > + * media_pipeline_start(). > > > + */ > > > + list_for_each_entry(link, &entity->links, list) { > > > > I wonder is this list_for_each_entry() really needed? Can't you simply > > iterate over all sink pads as with the loop bellow but drop the pad == > > link->sink check? Maybe I'm missing something. > > This was a review made by Sakari here: > https://patchwork.linuxtv.org/patch/44422/ > > The idea is that we need to know if the pad is enabled, and as far as > I know this information is only stored at the link level, not at the > pad level.
Ahh I see, you are correct. > > > Apart from this and the small nit-picks (one more bellow) I think this > > patch is fine. Once I understand this I be happy to add my tag to this > > change, great work! > > Is this a reviewed by? :) Yes, I now understand what I did not before :-) Feel free to add my Reviewed-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se> > > > I also think you shall consider to add a MAINTAINERS record for the RX > > and TX drivers. Maybe one entry for both drivers as they live in the > > same directory but I think one of the two should add it :-) > > Right, I'll do it, thanks! > Maxime > > -- > Maxime Ripard, Bootlin (formerly Free Electrons) > Embedded Linux and Kernel engineering > https://bootlin.com -- Regards, Niklas Söderlund