Re: [Xen-devel] [PATCH v4 02/11] x86/intel_pstate: add some calculation related support

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.06.15 at 13:14, <wei.w.wang@xxxxxxxxx> wrote:
> The added calculation related functions will be used in the intel_pstate.c.
> They are copied from the Linux kernel(commit 2418f4f2, f3002134, eb18cba7).
> 
> v4 changes:
> 1) in commit message, "kernel" changed to "Linux kernel"
> 2) if-else coding style change.

Despite of this, ...

> +static inline uint64_t div64_u64(uint64_t dividend, uint64_t divisor)
> +{
> +    uint32_t high = divisor >> 32;
> +    uint64_t quot;
> +
> +    if (high == 0)

... this and further if()-s below are still not in line with ./CODING_STYLE
(missing blanks inside parentheses).

> +        quot = div_u64(dividend, divisor);
> +    else
> +    {
> +        int n = 1 + fls(high);
> +        quot = div_u64(dividend >> n, divisor >> n);

Blank line between declaration(s) and statement(s) please.

> --- a/xen/include/xen/kernel.h
> +++ b/xen/include/xen/kernel.h
> @@ -42,6 +42,18 @@
>  #define MIN(x,y) ((x) < (y) ? (x) : (y))
>  #define MAX(x,y) ((x) > (y) ? (x) : (y))
>  
> +/*
> + * clamp_t - return a value clamped to a given range using a given type
> + * @type: the type of variable to use
> + * @val: current value
> + * @lo: minimum allowable value
> + * @hi: maximum allowable value
> + *
> + * This macro does no typechecking and uses temporary variables of type
> + * 'type' to make all the comparisons.
> + */
> +#define clamp_t(type, val, lo, hi) min_t(type, max_t(type, val, lo), hi)

Shouldn't you also add a type checking variant then (which ought
to be used instead of the one above wherever possible)?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel