From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by mx.groups.io with SMTP id smtpd.web10.2444.1571054169072823852 for ; Mon, 14 Oct 2019 04:56:09 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=e8a4gAIx; spf=pass (domain: linaro.org, ip: 209.85.128.66, mailfrom: leif.lindholm@linaro.org) Received: by mail-wm1-f66.google.com with SMTP id y135so14337614wmc.1 for ; Mon, 14 Oct 2019 04:56:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=WWaH0/ytz778MAZW9kqPVpA3LnvpqHPn9hHFQZ2ztwE=; b=e8a4gAIxoxP0v5eQ6DKqgfJVK1r9OQyi/h87IhOtILgaa1eO77dAcwN0HI4RCmQW9f 1Se7swJ90u5uN2XaQYjyQMsE+/wQieTbveqHz8+9wuxOb2eBSa1zzpR6CEixVmzdct2O 2USMyc0u1FCsfb3pQ9DHjuD2xPeTgf+eZVBtTY1L9fwEmgfOCYfiz8Gs06CIaOMVZKOD Zqn2IThIxtrJI05Kz6+J4acJueu5E2pdjwtwWFaxTTaqAUimu+HCffH8Ck2LItcXuKQ1 Vgdjiqa6ekr3m6eBVcG3Rr6NZYHTpx9UAEOKb+5ozmNU0vtSaH85ktCa+1oRxu3C3gtf SJOA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=WWaH0/ytz778MAZW9kqPVpA3LnvpqHPn9hHFQZ2ztwE=; b=kD1duZOeR0SYKo2L7Ebp4wWIFhcCRjqkwji5oVzLaIyeVwGJIPQE2+EWwqT4PuDuHs T140MxIzmPO8WPvQB19o3tSnr8FRKoLj/NnO4RQyZCMi42n2dqyW2yP0yG1XFg24i3wT Cso2p2D4YbDSBUTWpSuNUZrTqWD1AtChVNsZvMLKXXsnf7d/tV0S64vxx5ASUJdrvI3a ijlsbKZehZf05nmcMTu4fOkE8DSIyGyTto21vhc0iHArbmHQl5+e3M+Dtl5KblfQKAlP bbFHKyM96kAumbq4zaWc1EM3UK2q1KNIbEGrFn+r9X1jnjjnctw326w8Lnue/HJwfELl 70kQ== X-Gm-Message-State: APjAAAWeZUmx2eQdD/qwykVEN2GgrKF1741+bdxSF/P3VYlsLlQxmKuJ wFpRBqFRUXxFMAScz+vA6pYSJQKseQE= X-Google-Smtp-Source: APXvYqzmBX/UYXJPtHhIprqp9W5heud7Eq1qR+n70KVzmzmZ9i4XkO/qgUXMzS+VrlEc5bIDYEUGNQ== X-Received: by 2002:a1c:7305:: with SMTP id d5mr14447502wmb.84.1571054167218; Mon, 14 Oct 2019 04:56:07 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id r140sm27106627wme.47.2019.10.14.04.56.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Oct 2019 04:56:06 -0700 (PDT) Date: Mon, 14 Oct 2019 12:56:05 +0100 From: "Leif Lindholm" To: devel@edk2.groups.io, abner.chang@hpe.com Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 26/29] RiscVPkg/SmbiosDxe: Generic SMBIOS DXE driver for RISC-V platforms. Message-ID: <20191014115605.GC25504@bivouac.eciton.net> References: <1569198715-31552-1-git-send-email-abner.chang@hpe.com> <1569198715-31552-28-git-send-email-abner.chang@hpe.com> <20190930223959.GJ25504@bivouac.eciton.net> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Oct 14, 2019 at 11:27:51AM +0000, Abner Chang wrote: > > > > -----Original Message----- > > From: Leif Lindholm [mailto:leif.lindholm@linaro.org] > > Sent: Tuesday, October 1, 2019 6:40 AM > > To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist) > > > > Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 26/29] > > RiscVPkg/SmbiosDxe: Generic SMBIOS DXE driver for RISC-V platforms. > > > > On Mon, Sep 23, 2019 at 08:31:52AM +0800, Abner Chang wrote: > > > RISC-V generic SMBIOS DXE driver for building up SMBIOS type 4, type 7 > > > and type 44 records. > > > > > > Signed-off-by: Abner Chang > > > --- > > > RiscVPkg/Include/ProcessorSpecificDataHob.h | 95 ++++++ > > > RiscVPkg/Include/SmbiosProcessorSpecificData.h | 58 ++++ > > > RiscVPkg/RiscVPkg.dec | 6 + > > > RiscVPkg/Universal/SmbiosDxe/RiscVSmbiosDxe.c | 339 > > +++++++++++++++++++++ > > > RiscVPkg/Universal/SmbiosDxe/RiscVSmbiosDxe.h | 32 ++ > > > RiscVPkg/Universal/SmbiosDxe/RiscVSmbiosDxe.inf | 58 ++++ > > > RiscVPkg/Universal/SmbiosDxe/RiscVSmbiosDxe.uni | 12 + > > > .../Universal/SmbiosDxe/RiscVSmbiosDxeExtra.uni | 13 + > > > 8 files changed, 613 insertions(+) > > > create mode 100644 RiscVPkg/Include/ProcessorSpecificDataHob.h > > > create mode 100644 RiscVPkg/Include/SmbiosProcessorSpecificData.h > > > create mode 100644 RiscVPkg/Universal/SmbiosDxe/RiscVSmbiosDxe.c > > > create mode 100644 RiscVPkg/Universal/SmbiosDxe/RiscVSmbiosDxe.h > > > create mode 100644 RiscVPkg/Universal/SmbiosDxe/RiscVSmbiosDxe.inf > > > create mode 100644 RiscVPkg/Universal/SmbiosDxe/RiscVSmbiosDxe.uni > > > create mode 100644 > > > RiscVPkg/Universal/SmbiosDxe/RiscVSmbiosDxeExtra.uni > > > > > > diff --git a/RiscVPkg/Include/ProcessorSpecificDataHob.h > > > b/RiscVPkg/Include/ProcessorSpecificDataHob.h > > > new file mode 100644 > > > index 0000000..6798a9d > > > --- /dev/null > > > +++ b/RiscVPkg/Include/ProcessorSpecificDataHob.h > > > > None of the things defined in here are HOBs, they are structures to hold > > information that will be put into HOBs. > > Can we merge all of these definitions into SmbiosProcessorSpecificData.h > > and delete this file? > > No. SmbiosProcessorSpecificData.h defines the structure declared in > RISC-V SMBIOS processor specific data spec > (https://github.com/riscv/riscv-smbios). > ProcessorSpecificDataHob is the implementation to deliver processor > information in HOB for building SMBIOS type 44 record. Fair enough. Nevertheless, I find it confusing to refer to the structures being bundled together into a HOB as if theye were themselves HOB. An alternative naming scheme for me woud be to rename the file ProcessorSpecificHobData.h and the structs _HOB_DATA instead of _DATA_HOB. But I am fully open to replacing these with better names. > I would like to keep these two files separately. This is fine, as long as the above concern is addressed. > > > @@ -0,0 +1,95 @@ > > > +/** @file > > > + Definition of Processor Specific Data HOB. > > > + > > > + Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All > > > + rights reserved.
> > > + > > > + SPDX-License-Identifier: BSD-2-Clause-Patent > > > + > > > +**/ > > > +#ifndef _RISC_V_PROCESSOR_SPECIFIC_DATA_HOB_H_ > > > +#define _RISC_V_PROCESSOR_SPECIFIC_DATA_HOB_H_ > > > > Please drop leading _. > > > > > + > > > +#include > > > > This file also uses Uefi.h, please include it, so we don't depend on other files > > pulling it in for us. > > > > > + > > > +#define TO_BE_FILLED 0 > > > +#define TO_BE_FILLED_BY_VENDOR 0 > > > +#define TO_BE_FILLED_BY_RISC_V_SMBIOS_DXE_DRIVER 0 #define > > > +TO_BE_FILLED_BY_CODE 0 > > > > These defines are never used, please drop, > > These definitions are used in platform code to indicates the value > is not set correctly and should be set by certain code. Yes they are, sorry. I had inadvertently checked out the wrong branch when searching for its use. This and all such related comments can be ignored. Best Regards, Leif