Re: [PATCH v2] ARM: CSR: Adding CSR SiRFprimaII board support

[Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>]

On Wed, Jul 06, 2011 at 02:01:24PM +0800, Barry Song wrote:
> 2011/7/6 Barry Song <21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> > 2011/7/6 Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>:
> >>>                       intc: interrupt-controller@0x80020000 {
> >>>                               #interrupt-cells = <1>;
> >>>                               interrupt-controller;
> >>>                               compatible = "sirf,intc", "sirf,prima2-intc";
> >>
> >> This looks backwards.  Most specific values should appear first.
> >> "sirf,intc" looks to be generic when compared to "sirf,prima2-intc".
> >> I'm also concerned that "sirf,intc" is trying to be too generic.  As
> >> much as possible compatible values should reflect a real
> >> implementation of the interface, not a generic abstract name.  Future
> >> hardware can always claim compatibility with existing older hardware.
> >> You're probably better off dropping "sirf,intc" entirely, and doing
> >> the same for similar properties throughout the file.
> >
> > i have been confused by compatible in dts and drivers for some time.
> > if an intc ip core is shared by two chips, for example, prima2 and
> > atlas6. is the following right?
> >
> > in dts for prima2: compatible = "sirf,prima2-intc";
> > in dts for atlas6:  compatible = "sirf,prima2-atlas6";
> 
> sorry for typo. in dts for atlas6:  compatible = "sirf,atlas6-intc";
> 
> > in intc drivers shared for both:     compatible = "sirf,prima2-intc",
> > "sirf,prima2-atlas6";
> 
> sorry for typo.  compatible = "sirf,prima2-intc", "sirf,atlas6-intc";
> 

Yes, that is one way to do it, just specify all the implementations
using the IP core.  If there are only a few, then this is easy.  If
there are a lot of implementations using the same core, then the list
gets rather large in a hurry.

The intent of compatible is to allow one device to claim compatibility
with another.  So, if the prima2 was already supported, and you were adding
atlas6 support, you could put the following into the atlas6 .dts:

	compatible = "sirf,atlas6-intc", "airf,prima2-intc";

and the existing driver would work.

The other option when working with IP cores it so encode the version
of the IP block into the compatible values, and make both boards use
that.  ie.  "sirf,intc-1.32".  *however* doing so requires that the
hardware folks actually do a good job of documenting the core versions
so that the compatible value you use accurately describes the
hardware.

It is also completely valid for a newer IP core to claim compatibility
with an older one in the .dts:

	ie: compatible = "sirf,intc-1.32", "sirf,intc-1.00";

Personally, I prefer anchoring compatible values to the specific
hardware implmentation rather than the IP core version.

> >
> > in my understanding, dts for special soc/board contains special
> > compatible for the chip, drivers shared by multi-soc/board contain all
> > compatible lists which can be supported by them?
> >
> > what's the generic way for this?
> >
> >>
> >> Also, for every new compatible value make sure you add documentation
> >> for it to Documentation/devicetree/bindings. (You may have already
> >> done so, it's just something I remind people about a lot).
> >>
> >> These comments also apply through the rest of the file.
> >>
> >>
> >