Re: [PATCH] dt: bindings: Add support for CSI1 bus

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Rob,

Thanks for the review.

On Tue, Jan 03, 2017 at 02:38:54PM -0600, Rob Herring wrote:
> On Wed, Dec 28, 2016 at 07:30:36PM +0100, Pavel Machek wrote:
> > From: Sakari Ailus <sakari.ailus@xxxxxx>
> > 
> > In the vast majority of cases the bus type is known to the driver(s)
> > since a receiver or transmitter can only support a single one. There
> > are cases however where different options are possible.
> 
> What cases specifically?

The existing V4L2 OF support tries to figure out the bus type and parse the
bus parameters based on that. This does not scale too well as there are
multiple serial busses that share common properties.

Some hardware also supports multiple types of busses on the same interfaces.

> 
> > Document the CSI1/CCP2 properties strobe_clk_inv and strobe_clock
> > properties. The former tells whether the strobe/clock signal is
> > inverted, while the latter signifies the clock or strobe mode.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxx>
> > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@xxxxxxxxx>
> > Signed-off-by: Pavel Machek <pavel@xxxxxx>
> > 
> > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
> > index 9cd2a36..f0523f7 100644
> > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> > @@ -76,6 +76,10 @@ Optional endpoint properties
> >    mode horizontal and vertical synchronization signals are provided to the
> >    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 - CSI2
> 
> As in MIPI CSI2?

Yeah, I guess it'd make sense to make this explicit.

> 
> > +  1 - parallel / Bt656
> > +  2 - CCP2
> >  - 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
> > @@ -110,9 +114,10 @@ Optional endpoint properties
> >    lane and followed by the data lanes in the same order as in data-lanes.
> >    Valid values are 0 (normal) and 1 (inverted). The length of the array
> >    should be the combined length of data-lanes and clock-lanes properties.
> > -  If the lane-polarities property is omitted, the value must be interpreted
> > -  as 0 (normal). This property is valid for serial busses only.
> 
> Why is this removed?

Must have been by mistake. :-)

> 
> > -
> > +- clock-inv: Clock or strobe signal inversion.
> > +  Possible values: 0 -- not inverted; 1 -- inverted
> 
> "invert" assumes I know what is normal and I do not. Define what is 
> "normal" and name the property the opposite of that. If normal is data 
> shifted on clock rising edge, then call the the property 
> "clock-shift-falling-edge" for example..

The hardware documentation says this is the "strobe/clock inversion control
signal". I'm not entirely sure whether this is just signal polarity (it's a
differential signal) or inversion of an internal signal of the CCP2 block.

It might make sense to make this a private property for the OMAP 3 ISP
instead. If it's seen elsewhere, then think about it again. I doubt it
would, as CCP2 is an old bus that's used on Nokia N9, N950 and N900.

As strobe is included, I'd add that to the name. Say, "ti,clock-strobe-inv".

> 
> > +- strobe: Whether the clock signal is used as clock or strobe. Used
> > +  with CCP2, for instance.
> >  
> >  Example
> >  -------
> > 
> > 

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@xxxxxx	XMPP: sailus@xxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux