"Hiremath, Vaibhav" <hvaibhav@xxxxxx> writes: > On Fri, Apr 20, 2012 at 06:04:20, Hilman, Kevin wrote: [...] >> Also note that because you're adding an offset of 0x10 to the start, the >> ioremap later is actually mapping 0x10 past the end. More on the base >> address later... >> > > Comment on top of this code will enough, isn't it? > No. ioremapping() the range that goes past the end of the device is just wrong. But if you fix the offset as described below, that would be fine. [...] >> > @@ -68,54 +66,43 @@ void read_persistent_clock(struct timespec *ts) >> > *ts = *tsp; >> > } >> > >> > -int __init omap_init_clocksource_32k(void) >> > +int __init omap_init_clocksource_32k(u32 pbase, unsigned long size) >> >> Now that this takes some arguments, this function wants some kerneldoc. >> > > Should I create new documentation inside Documentation/arm/OMAP/ > with name "counter_32k"? by kerneldoc, I mean comments in the code, not in Documenation. IOW, just add a comment before this function in the code, but make sure it is of the format described in Documentation/kernel-doc-nano-HOWTO.txt. >> In particular, you should document that 'pbase' is the address of the >> counter register, not the base of the IP. >> >> That being said, I think the pbase passed in should probably be the base >> of the IP register, not the address of the counter register. It seems to >> me that it's at offset 0x10 on all SoCs anyways. >> >> Doing that will also ensure that the ioremap'd range covers the whole >> IP, and not from the counter register to 0x10 past the end. >> > > Yeah, you are right, I can get rid of this offset and move it to > counter_32k.c file. Perfect. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html