On Mon, Oct 14, 2019 at 11:27:51AM +0000, Abner Chang wrote: > > > > -----Original Message----- > > From: Leif Lindholm [mailto:leif.lindh...@linaro.org] > > Sent: Tuesday, October 1, 2019 6:40 AM > > To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist) > > <abner.ch...@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. > > > > 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 <abner.ch...@hpe.com> > > > --- > > > 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.<BR> > > > + > > > + 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 <IndustryStandard/SmBios.h> > > > > 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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48894): https://edk2.groups.io/g/devel/message/48894 Mute This Topic: https://groups.io/mt/34258224/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
