From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58054) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTVOU-0002vb-Bz for qemu-devel@nongnu.org; Wed, 10 Feb 2016 09:05:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTVOP-0000nD-8A for qemu-devel@nongnu.org; Wed, 10 Feb 2016 09:05:22 -0500 Received: from e06smtp08.uk.ibm.com ([195.75.94.104]:41554) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTVOO-0000m0-VV for qemu-devel@nongnu.org; Wed, 10 Feb 2016 09:05:17 -0500 Received: from localhost by e06smtp08.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 10 Feb 2016 14:05:14 -0000 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id A4B272190068 for ; Wed, 10 Feb 2016 14:04:57 +0000 (GMT) Received: from d06av05.portsmouth.uk.ibm.com (d06av05.portsmouth.uk.ibm.com [9.149.37.229]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u1AE5BDe19988690 for ; Wed, 10 Feb 2016 14:05:11 GMT Received: from d06av05.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av05.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u1AE5A27014970 for ; Wed, 10 Feb 2016 07:05:11 -0700 References: <1455020010-17532-1-git-send-email-clg@fr.ibm.com> <56BA2F29.8010109@mvista.com> From: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= Message-ID: <56BB4390.5040107@fr.ibm.com> Date: Wed, 10 Feb 2016 15:05:04 +0100 MIME-Version: 1.0 In-Reply-To: <56BA2F29.8010109@mvista.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 0/8] ipmi: a couple of enhancements to the BMC simulator (round 2) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Corey Minyard Cc: Greg Kurz , qemu-devel@nongnu.org, "Michael S. Tsirkin" Hello Corey, On 02/09/2016 07:25 PM, Corey Minyard wrote: > On 02/09/2016 06:13 AM, Cédric Le Goater wrote: >> The first patches are cleanups and prepare ground for an extension of >> the BMC simulator providing a SDR loader using a file. A simple FRU >> support comes next. >> >> The last patches introduce APIs to populate a guest device tree with >> the available sensors and to generate events, which is needed by >> platforms in some occasions. >> > > I have reviewed all of these, and they look good. I only have one > comment: The naming of the properties probably needs to be > fixed. > > "sdr" should be "sdrfile" to be consistent with everything else. yes. I agree. I am glad you took a look. > Technically, a "FRU" is a piece of hardware that can be replaced > in the field, "FRU data" is the data describing that FRU, and a "FRU > area" is the memory used to store FRU data. I know this is mixed > up a lot (and I have done so) but some people are picky about this. > > With the renaming of sdr (fru is your option): I will rename the "sdr" property to "sdrfile". As for FRU, you would rather have the code use FruData than Fru ? Something like: typedef struct IPMIFruData { char *filename; unsigned int nentries; uint16_t size; uint8_t *area; } IPMIFruData; The code using the IPMIFruData structure would look like : uint8_t *fru_area; ... fru_area = &ibs->frudata.area[fruid * ibs->frudata.size]; ... Changing all the names is not a problem. Let's get them right. And, so, the properties would become : DEFINE_PROP_UINT16("frudatasize", IPMIBmcSim, frudata.size, 1024), DEFINE_PROP_STRING("frudatafile", IPMIBmcSim, frudata.filename), DEFINE_PROP_STRING("sdrfile", IPMIBmcSim, sdr_filename), > Acked-by: Corey Minyard > > for all patches. > > Oh, and I assume you need to add documentation for the > properties to qemu-options.hx. Yes. Forgot that. Thanks, C. > -corey > >> Based on e4a096b1cd43 and also available here : >> >> https://github.com/legoater/qemu/commits/ipmi >> >> Thanks, >> >> C. >> >> Cédric Le Goater (8): >> ipmi: add a realize function to the device class >> ipmi: use a function to initialize the SDR table >> ipmi: remove the need of an ending record in the SDR table >> ipmi: add some local variables in ipmi_sdr_init >> ipmi: use a file to load SDRs >> ipmi: provide support for FRUs >> ipmi: introduce an ipmi_bmc_sdr_find() API >> ipmi: introduce an ipmi_bmc_gen_event() API >> >> hw/ipmi/ipmi_bmc_sim.c | 256 +++++++++++++++++++++++++++++++++++++++++++------ >> include/hw/ipmi/ipmi.h | 4 + >> 2 files changed, 233 insertions(+), 27 deletions(-) >> >