On Wed, Oct 29, 2014 at 10:13 AM, James Hogan <james.hogan@xxxxxxxxxx> wrote: > On 29/10/14 16:55, Andrew Bresticker wrote: >> Hi James, >> >> On Wed, Oct 29, 2014 at 2:21 AM, James Hogan <james.hogan@xxxxxxxxxx> wrote: >>> Hi Andrew, >>> >>> On 29/10/14 00:12, Andrew Bresticker wrote: >>>> - changed compatible string to include CPU version >>> >>>> +Required properties: >>>> +- compatible : Should be "mti,<cpu>-gic". Supported variants: >>>> + - "mti,interaptiv-gic" >>> >>>> +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. >> >>> 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. >> >>> 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.