Hi Laurent, On Fri, Mar 13, 2015 at 01:11:03AM +0200, Laurent Pinchart wrote: > Hi Sakari, > > On Friday 13 March 2015 01:03:21 Sakari Ailus wrote: > > On Thu, Mar 12, 2015 at 01:39:07AM +0200, Laurent Pinchart wrote: > > > On Saturday 07 March 2015 23:41:11 Sakari Ailus wrote: > > > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxx> > > > > --- > > > > > > > > .../devicetree/bindings/media/ti,omap3isp.txt | 64 +++++++++++++ > > > > MAINTAINERS | 1 + > > > > 2 files changed, 65 insertions(+) > > > > create mode 100644 > > > > Documentation/devicetree/bindings/media/ti,omap3isp.txt > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/ti,omap3isp.txt > > > > b/Documentation/devicetree/bindings/media/ti,omap3isp.txt new file mode > > > > 100644 > > > > index 0000000..2059524 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/media/ti,omap3isp.txt > > [snip] > > > > > +syscon : syscon phandle and register offset > > > > > > We should document what the register offset is. > > > > This is SoC specific as is the base address. I'm not sure that would be > > relevant here. If you think so, shouldn't we also add the device base > > addresses and register block lengths? > > I meant something like > > syscon: the phandle and register offset to the Complex I/O or CSI-PHY > register. Oh, I misunderstood you. I'll use that text. > > > > +ti,phy-type : 0 -- 3430; 1 -- 3630 > > > > > > Would it make sense to add #define's for this ? > > > > I'll use OMAP3ISP_PHY_TYPE_COMPLEX_IO and OMAP3ISP_PHY_TYPE_CSIPHY as > > discussed. > > > > > It could also make sense to document/name them "Complex I/O" and "CSIPHY" > > > to avoid referring to the SoC that implements them, as the ISP is also > > > found in SoCs other than 3430 and 3630. > > > > > > Could the PHY type be derived from the ES revision that we query at > > > runtime ? > > > > I think this would work on 3430 and 3630 but I'm not certain about others. > > > > > We should also take into account the fact that the DM3730 has officially > > > no CSIPHY, but still seems to implement them in practice. > > > > The DT sources are for 36xx, but I'd guess it works on 37xx as well, doesn't > > it? > > I think so. > > > > > +#clock-cells : Must be 1 --- the ISP provides two external clocks, > > > > + cam_xclka and cam_xclkb, at indices 0 and 1, > > > > + respectively. Please find more information on common > > > > + clock bindings in ../clock/clock-bindings.txt. > > > > + > > > > +Port nodes (optional) > > > > +--------------------- > > > > > > This should refer to Documentation/devicetree/bindings/media/video- > > > interfaces.txt. > > > > There's a reference to video-interfaces.txt in the beginning of the file. Do > > you think that'd be enough? > > I've missed that. I think you could move the reference here. Done. -- 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
