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. 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. Cheers James