From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jan Beulich" <JBeulich@suse.com>
Subject: Re: [PATCH v4 02/11] x86/intel_pstate: add some
 calculation related support
Date: Fri, 24 Jul 2015 07:16:20 -0600
Message-ID: <55B256C40200007800095317@prv-mh.provo.novell.com>
References: <1435230882-21865-1-git-send-email-wei.w.wang@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1435230882-21865-1-git-send-email-wei.w.wang@intel.com>
Content-Disposition: inline
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Wei Wang <wei.w.wang@intel.com>
Cc: andrew.cooper3@citrix.com, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

>>> On 25.06.15 at 13:14, <wei.w.wang@intel.com> 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