On Wed, Oct 29, 2014 at 10:13 AM, James Hogan [off-list ref] wrote:

On 29/10/14 16:55, Andrew Bresticker wrote:

quoted

Hi James,

On Wed, Oct 29, 2014 at 2:21 AM, James Hogan [off-list ref] wrote:

quoted

Hi Andrew,

On 29/10/14 00:12, Andrew Bresticker wrote:

quoted

 - changed compatible string to include CPU version

quoted

+Required properties:
+- compatible : Should be "mti,<cpu>-gic".  Supported variants:
+  - "mti,interaptiv-gic"

quoted

+Required properties for timer sub-node:
+- compatible : Should be "mti,<cpu>-gic-timer".  Supported variants:
+  - "mti,interaptiv-gic-timer"

Erm, I'm a bit confused...
Why do you include the core name in the compatible string?

You seem to be suggesting that:

1) The GIC/timer drivers need to know what core they're running on.

Is that really true?

They don't now, but it's possible that a future CPU has a newer
revision of the GIC which has some differences that need to be
accounted for in the driver.

quoted

2) It isn't possible to probe the core type.

But the kernel already knows this, so what's wrong with using
current_cpu_type() like everything else that needs to know?

3) Every new core should require a new compatible string to be added
before the GIC will work. You don't even have a generic compatible
string that DT can specify after the core specific one as a fallback.

Yes, adding a generic compatible string would be a good idea.

quoted

Please lets not do this unless it's actually necessary (which AFAICT it
really isn't).

The point of this was to future-proof these bindings and I though that
CPU type was the best way to indicate version in the compatible
string.  This is also how it's done for the ARM GIC and arch timers.
Perhaps the best thing to do is to require both a core-specific
("mti,interaptiv-gic") and generic ("mti,gic") compatible string and
just match on the generic one for now until there's a need to use the
core-specific one.  Thoughts?

FPGA boards like Malta are something else to consider (when it is
eventually converted to DT - Paul on CC knows more than me). You might
load an interAptiv, or a proAptiv, or a P5600 bitstream, and the gic
setup will be pretty much the same I think, since e.g. the address
depends on where it is convenient to put it in the address space of the
platform.

Ah, I didn't realize that the CPU bitstream could be changed
independently of the GIC.
In that case, the CPU revision isn't that useful.

Any thoughts on the existence of current_cpu_type(), and the GIC
revision register? They pretty much make encoding of core in compatible
string redundant I think.

Ok, I suppose using the revision register is fine then.