Hi,
On Fri, Mar 13, 2015 at 01:03:21AM +0200, Sakari Ailus wrote:
> [...]
>
> > > +Required properties
> > > +===================
> > > +
> > > +compatible : "ti,omap3-isp"
> >
> > I would rephrase that using the usual wording as "compatible: Must contain
> > "ti,omap3-isp".
>
> [...]
>
> > > +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?
In other drivers this kind of information is often extracted from the
compatible string. For example:
{ .compatible = "ti,omap34xx-isp", .data = OMAP3ISP_PHY_TYPE_COMPLEX_IO, },
{ .compatible = "ti,omap36xx-isp", .data = OMAP3ISP_PHY_TYPE_CSIPHY, },
...
> [...]
>
> > > +Example
> > > +=======
> > > +
> > > + omap3_isp: omap3_isp@480bc000 {
> >
> > DT node names traditionally use - as a separator. Furthermore the phandle
> > isn't needed. This should thus probably be
> >
> > omap3-isp@480bc000 {
>
> Fixed.
According to ePAPR this should be a generic name (page 19); For
example the i2c node name should be "i2c@address" instead of
"omap3-i2c@address". There is no recommended generic term for an
image signal processor, "isp" looks ok to me and seems to be
already used in NVIDIA Tegra's device tree files. So maybe:
isp@480bc000 {
> [...]
-- Sebastian
Attachment:
signature.asc
Description: Digital signature
