From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH v4 3/4] tools: add tools support for Intel
	CDP
Date: Thu, 24 Sep 2015 11:56:49 +0100
Message-ID: <1443092209.10338.272.camel@citrix.com>
References: <1442482536-12024-1-git-send-email-he.chen@linux.intel.com>
	<1442482536-12024-4-git-send-email-he.chen@linux.intel.com>
	<55FA980C.6040401@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta14.messagelabs.com ([193.109.254.103])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <prvs=7027b08fe=Ian.Campbell@citrix.com>)
	id 1Zf4G3-0000Lb-Od
	for xen-devel@lists.xenproject.org; Thu, 24 Sep 2015 11:00:11 +0000
In-Reply-To: <55FA980C.6040401@citrix.com>
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: Andrew Cooper <andrew.cooper3@citrix.com>, He Chen <he.chen@linux.intel.com>, xen-devel@lists.xenproject.org
Cc: wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com, ian.jackson@eu.citrix.com, jbeulich@suse.com, chao.p.peng@linux.intel.com, keir@xen.org
List-Id: xen-devel@lists.xenproject.org

On Thu, 2015-09-17 at 11:38 +0100, Andrew Cooper wrote:
> On 17/09/15 10:35, He Chen wrote:
> > @@ -2798,7 +2798,9 @@ enum xc_psr_cmt_type {
> >  typedef enum xc_psr_cmt_type xc_psr_cmt_type;
> >  
> >  enum xc_psr_cat_type {
> > -    XC_PSR_CAT_L3_CBM = 1,
> > +    XC_PSR_CAT_L3_CBM  = 1,
> > +    XC_PSR_CAT_L3_CODE = 2,
> > +    XC_PSR_CAT_L3_DATA = 3,
> >  };
> 
> No need for the explicit assignments here.  The exact values are not
> interesting and guaranteed to be as currently assigned.

No harm though I suppose.

> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index ebbb9a5..8128f54 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -8390,6 +8390,10 @@ static int psr_cat_hwinfo(void)
> >          }
> >          printf("%-16s: %u\n", "Socket ID", socketid);
> >          printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
> > +        if (info->cdp_enabled)
> > +            printf("%-16s: Enabled\n", "CDP Status");
> > +        else
> > +            printf("%-16s: Disabled\n", "CDP Status");
> 
> printf("%-16s: %sabled\n", "CDP Status", info->cdp_enabled ? "En" :
> "Dis");
> 
> is rather shorter, if you prefer.

My preference would be along these lines but without the trick regarding
the common suffix on both words, i.e.

printf("%-16s: %s\n", "CDP Status",
       info->cdp_enabled ? "Enabled" : "Disabled");

As well as avoiding splitting the words this avoids duplicating the %-16s
and "CDP Status" compared with the original, which IMHO is the important
thing.

Ian.