On Tue, Mar 05, 2019 at 11:38:42AM -0600, Benoit Parrot wrote: > Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> wrote on Tue [2019-Mar-05 18:32:40 +0200]: > > Hi Benoit, > > > > On Tue, Mar 05, 2019 at 08:34:09AM -0600, Benoit Parrot wrote: > > > Sakari, > > > > > > Thank you for the patch. > > > > > > Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> wrote on Tue [2019-Mar-05 16:02:24 +0200]: > > > > ti-vpe driver parsed the remote endpoints for properties but ignored the > > > > local ones. Fix this by parsing the local endpoint properties instead. > > > > > > I am not sure I understand the logic here. For CSI2 sensor as far as I > > > understand the lane mapping (clock and data) is driven from the sensor > > > side. The bridge driver (in this case CAL) needs to setup the receiver side > > > based on what the sensor (aka remote endpoint) can provide. > > > > > > I failed to see how this fixes things here. > > > > > > Are you suggesting that sensor relevant properties be set (and effectively > > > duplicated) on the bridge/receiver side? > > > > Yes. The endpoint configuration in general is local to the device and > > should not be accessed from other device drivers. > > > > The lane mapping, for instance, is specific to a given device --- and may > > differ even between for two connected endpoints. It's used to reorder the > > PHY lanes (if the device supports that). Same goes for the clock lane. > > I did not see omap3isp having lane reorder capability, but I guess it would > be possible for instance, that a sensor uses clock lane 0 and data lane 1 > & 2 but the way it is wired on the board makes it that the receiver would see > sensor lane 0 on device lane 2 and so on... Not sure why you would wire it > up that way but who knows... I presume the feature is there to ease PCB design. > > > > > See e.g. arch/arm/boot/dts/omap3-n9.dts . ^ There it is. > > > > > > > > Some sensor can and do handle multiple data lanes configuration so the > > > sensor driver also needs to use those properties at probe time, duplicating > > > the lane data is just asking for a mismatch to happen, no? > > > > It's a different configuration on the sensor side. We currently have no > > checks in place to verify that the two would match. I haven't heard of this > > would have really been a problem though. > > I had just never thought about this cases, to me a single source of > information is better than 2. But anyhow I guess I'll have to update all of > my relevant dts files in the near future. Do you have in-kernel dts files using this? I presume the driver should then figure out whether the local endpoint has a configuration and if it doesn't, then look it up from the remote one. Otherwise old dts binaries will break. :-( -- Regards, Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxx
