[PATCH] PowerPC - Add a faster way to read the Time Base register

Roland McGrath roland@hack.frob.com
Fri May 25 23:43:00 GMT 2012


Some simple nits below.  Fix those up and then it's OK to commit.
The manual bits are still imperfect, but we can improve them later.

> +@node Adding Platform-specific
> +@appendixsubsec Platform-specific types, macros and functions

Throughout this section, say "header file", not "header".

> +@item
> +Each included symbol must have the prefix @code{__@var{arch}_}, such as
> +@code{__ppc_get_timebase}.
> +
> +@end itemize

Drop that blank line since there isn't one after @itemize.

> --- /dev/null
> +++ b/manual/platform.texi
> @@ -0,0 +1,32 @@
> +@node Platform, Contributors, Maintenance, Top
> +@c %MENU% Describe all platform-specific facilities provided
> +@appendix Summary of platform-specific facilities

It's not a summary, it's the only documentation there is.

> +@Theglibc{} can provide machine-specific functionality.
> +@ref{Adding Platform-specific} describes the process to include
> +these facilities in @theglibc{} and whether they should be included in
> +GCC as well.

This is a part of the manual for users, who may have no interest in ever
working on libc.  I don't think it should have an xref to something maint.texi.

> +@menu
> +* PowerPC::           Summary of Facilities Specific
> +                                   to the PowerPC Architecture
> +@end menu

Not a summary.

> +The @dfn{Time Base Register} is a 64-bit register that stores a monotonically
> +incremented value updated at a system-dependent frequency that may be
> +different from the processor frequency. More information in Power
> + ISA 2.06b - Book II - Section 5.2.

Two spaces between sentences.  Use proper grammar (that sentence no verb).
Use @cite.

> +/* Read the Time Base Register   */
> +static inline uint64_t
> +__ppc_get_timebase (void)

Period at the end of that sentence.

> +{
> +#ifdef __powerpc64__
> +  uint64_t __tb;
> +  /* "volatile" is necessary here, because the user expects this assembly
> +     isn't moved after an optimization.  */
> +  __asm__ volatile (
> +		    "mfspr %0, 268"
> +		    : "=r" (__tb));

We don't usually break the line after the ( like that.
This whole thing could be on line, so just do that.

> +  uint32_t __tbu, __tbl, __tmp; \
> +  __asm__ volatile (
> +		    "0:\n\t"

Likewise, here put "0..." right after the (.


Thanks,
Roland



More information about the Libc-alpha mailing list