On Mon, Apr 21, 2008 at 12:39 PM, Scott Wood <[EMAIL PROTECTED]> wrote: > On Mon, Apr 21, 2008 at 08:03:31AM -0600, Grant Likely wrote: > > > > + r) Freescale General-purpose Timers Module > > > + > > > + Required properties: > > > + - compatible : should be "fsl,gtm" ("fsl,qe-gtm" in addition for > QE > > > + GTMs or "fsl,cpm2-gtm" for CPM2 GTMs). > > > > I don't think this is specific enough. It is a very real possibility > > for Freescale to produce another part with a "general purpose timers > > module" that isn't register level compatible (and fsl,i2c is an > > example of what not to do). > > If that happens, we'll make up "fsl,gtm2". :-)
:-P > > The compatible string should include the > > exact chip version. Newer parts can also claim compatibility with > > older ones. > > > > Defining a 'generic' compatible value is also known as "making stuff up". > :-) > > Nothing wrong with making stuff up as long as we do it sanely. > > How about something like fsl,gtm-1.0? The problem I have with that is that there is always the potential that some rogue hardware will appear that messes up your sane scheme. Plus there is pretty much no downside to just choosing a device that the more recent parts reference. Example: consider 3 chips using the same i2c core; say MPC8313, MPC5200 & MPC8548. compatible lines could look like this: compatible = "fsl,mpc8313-i2c","fsl-i2c"; compatible = "fsl,mpc5200-i2c","fsl-i2c"; compatible = "fsl,mpc8548-i2c","fsl-i2c"; Or; (assuming 8313 is where it first appeared) it could look like this: compatible = "fsl,mpc8313-i2c"; compatible = "fsl,mpc5200-i2c","fsl,mpc8313-i2c"; compatible = "fsl,mpc8548-i2c","fsl,mpc8313-i2c"; To the driver it's just a different string ("fsl,mpc8313-i2c" instead of "fsl-i2c"), but with the second you *always* know what an mpc8131-i2c core is; the mpc8131 user guide tells you. Even if you are strict about your sane naming scheme, you cannot look into the future and someone will do something wonky on you. The other benefit is it avoids the temptation to define generic compatible values for things which will *never* be generic parts. Case in point; when I did the original mpc5200 bindings I did things like "mpc52xx-<blah>". However, there probably never will be another mpc52xx part; and if there is it will look vastly different from the current 5200. So I wasted all kinds of effort trying to be generic about stuff when it is almost certainly wasted effort and additional complexity to the naming conventions. > > > +static inline void gtm_ack_timer16(struct gtm_timer *tmr, u16 events) > > > +{ > > > + out_be16(tmr->gtevr, events); > > > +} > > > > Drop 'inline' and expect gcc to do the right thing. > > Not in a header... Oops, yeah; not sure what I was smoking. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev