Ping? On Wed, Aug 01, 2018 at 02:16:27PM +0300, Sakari Ailus wrote: > Hi Rob, > > Thanks for the review. > > On Tue, Jul 31, 2018 at 03:32:10PM -0600, Rob Herring wrote: > > On Mon, Jul 23, 2018 at 04:46:50PM +0300, Sakari Ailus wrote: > > > Allow specifying the bus type explicitly for MIPI D-PHY, parallel and > > > Bt.656 busses. This is useful for devices that can make use of different > > > bus types. There are CSI-2 transmitters and receivers but the PHY > > > selection needs to be made between C-PHY and D-PHY; many devices also > > > support parallel and Bt.656 interfaces but the means to pass that > > > information to software wasn't there. > > > > > > Autodetection (value 0) is removed as an option as the property could be > > > simply omitted in that case. > > > > Presumably there are users, so you can't remove it. But documenting > > behavior when absent would be good. > > Well, it's effectively the same as having no such property at all: the type > is not specified. Generally there are two possibilities: the hardware > supports just a single bus or it supports more than one. If there's just > one, the type can be known by the driver. In that case there's no use for > autodetection. > > The second case is a bit more complicated: the bus type detection is solely > based on properties available in the endpoint, and I think that may have > been feasible approach when there were just parallel and Bt.656 busses that > were supported, but with the additional busses, the V4L2 fwnode framework > may no longer guess the bus in any meaningful way from the available > properties. I'd think the only known-good option here is to specify the > type explicitly in that case: there's no room for guessing. (This patchset > makes it possible for drivers to explicitly define the bus type, but the > autodetection support is maintained for backwards compatibility.) > > One of the existing issues is that there are combined parallel/Bt.656 > receivers that need to know the type of the bus. This is based on the > existence parallel interface only properties: if any of these exist, then > the interface is parallel, otherwise it is Bt.656. The DT bindings for the > same devices also define the defaults for the parallel interface. This > leaves the end result ambiguous: is it the parallel interface with the > default configuration or is it Bt.656? > > There will likely be similar issues for CSI-2 D-PHY and CSI-2 C-PHY. The > question there would be: is this CSI-2 C-PHY or CSI-2 D-PHY with default > clock lane configuration? > > In either case the autodetection option for the bus type provides no useful > information. If it exists in DT source, that's fine, there's just no use > for it. > > Let me know if you still think it should be maintained in binding > documentation. If you prefer to keep it, I'd propose to mark it as deprecated or something as it provides no information to software. > > > > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > > > --- > > > Documentation/devicetree/bindings/media/video-interfaces.txt | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt > > > index baf9d9756b3c..f884ada0bffc 100644 > > > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt > > > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt > > > @@ -100,10 +100,12 @@ Optional endpoint properties > > > slave device (data source) by the master device (data sink). In the master > > > mode the data source device is also the source of the synchronization signals. > > > - bus-type: data bus type. Possible values are: > > > - 0 - autodetect based on other properties (MIPI CSI-2 D-PHY, parallel or Bt656) > > > 1 - MIPI CSI-2 C-PHY > > > 2 - MIPI CSI1 > > > 3 - CCP2 > > > + 4 - MIPI CSI-2 D-PHY > > > + 5 - Parallel > > > > Is that really specific enough to be useful? > > Yes; see above. > > > > > > + 6 - Bt.656 > > > - bus-width: number of data lines actively used, valid for the parallel busses. > > > - data-shift: on the parallel data busses, if bus-width is used to specify the > > > number of data lines, data-shift can be used to specify which data lines are > -- Regards, Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxx
