Hi Hans, On Monday 13 Feb 2017 15:19:13 Hans Verkuil wrote: > Hi Niklas, > > One general remark: in many commit logs you mistype 'subdeivce'. Can you > fix that for the v2? > > On 01/31/2017 04:40 PM, Niklas Söderlund wrote: > > Hi, > > > > This series address issues with the R-Car Gen2 VIN driver. The most > > serious issue is the OPS when unbind and rebinding the i2c driver for > > the video source subdevice which have popped up as a blocker for other > > work. > > > > This series is broken out of my much larger R-Car Gen3 enablement series > > '[PATCHv2 00/32] rcar-vin: Add Gen3 with media controller support'. I > > plan to remove that series form patchwork and focus on these fixes first > > as they are blocking other development. Once the blocking issues are > > removed I will rebase and repost the larger Gen3 series. > > > > Patch 1-4 fix simple problems found while testing > > > > 1-2 Fix format problems when the format is (re)set. > > 3 Fix media pad errors > > 4 Fix standard enumeration problem > > > > Patch 5 adds a wrapper function to retrieve the active video source > > subdevice. This is strictly not needed on Gen2 which only have one > > possible video source per VIN instance (This will change on Gen3). But > > patch 6-8,11 which fixes real issues on Gen2 make use of this wrapper, as > > not risk breaking things by removing this wrapper in this series and > > then readding it in a later Gen3 series I have chosen to keep the patch. > > Please let me know if I should drop it and rewrite patch 6-11 (if > > possible I would like to avoid that). > > > > Patch 6-8 deals with video source subdevice pad index handling by moving > > the information from struct rvin_dev to struct rvin_graph_entity and > > moving the pad index probing to the struct v4l2_async_notifier complete > > callback. This is needed to allow the bind/unbind fix in patch 10-11. > > > > Patch 9 use the pad information when calling enum_mbus_code. > > > > Patch 10-11 fix a OPS when unbinding/binding the video source subdevice. > > This will not help: you can unbind a subdev at any time, including when > it is in use. > > But why do you need this at all? You can also set suppress_bind_attrs in > the adv7180 driver to prevent the bind/unbind files from appearing. > > It really makes no sense for subdevs. In fact, all subdevs should set this > flag since in the current implementation this is completely impossible to > implement safely. > > I suggest you drop the patches relating to this and instead set the suppress > flag. The adv7180 is connected to an I2C controller that can be unbound. Setting the suppress_bind_attrs flag in the driver thus won't prevent the device from being unbound. suppress_bind_attrs is not a good solution for I2C drivers. -- Regards, Laurent Pinchart
