public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif.lindholm@linaro.org>
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.
Date: Mon, 14 Oct 2019 12:56:05 +0100	[thread overview]
Message-ID: <20191014115605.GC25504@bivouac.eciton.net> (raw)
In-Reply-To: <CS1PR8401MB1192B670EBAC93398C420EAEFF900@CS1PR8401MB1192.NAMPRD84.PROD.OUTLOOK.COM>

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)
> > <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.
> > 
> > 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.chang@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

  reply	other threads:[~2019-10-14 11:56 UTC|newest]

Thread overview: 108+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-23  0:31 [edk2-staging/RISC-V-V2 PATCH v2 00/29] RISC-V EDK2 Port on Abner Chang
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 01/29] RiscVPkg: RISC-V processor package Abner Chang
2019-09-26 22:26   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 02/29] RiscVPkg/Include: Add header files of RISC-V CPU package Abner Chang
2019-09-26 22:29   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 03/29] RiscVPkg/opensbi: EDK2 RISC-V OpenSBI support Abner Chang
2019-09-26 22:41   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 04/29] MdePkg: RISC-V RV64 binding in MdePkg Abner Chang
2019-09-26 22:44   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 05/29] MdePkg/Include: RISC-V definitions Abner Chang
2019-09-26 22:45   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 06/29] MdeModulePkg/CapsuleRuntimeDxe: Add RISCV64 arch Abner Chang
2019-09-26 22:46   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 07/29] MdePkg/BaseLib: BaseLib for RISC-V RV64 Processor Abner Chang
2019-09-26 22:56   ` [edk2-devel] " Leif Lindholm
2019-10-14 16:47     ` Abner Chang
2019-10-14 18:23       ` Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 08/29] MdePkg/BaseCacheMaintenanceLib: RISC-V cache maintenance implementation Abner Chang
2019-10-01  8:44   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-23  0:31 ` Abner Chang
2019-09-26 23:30   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 09/29] MdePkg/BaseIoLibIntrinsic: RISC-V I/O intrinsic functions Abner Chang
2019-09-26 23:39   ` [edk2-devel] " Leif Lindholm
2019-10-01  8:49     ` Philippe Mathieu-Daudé
2019-10-01  9:07       ` Leif Lindholm
2019-10-02  1:30         ` Abner Chang
2019-10-02  9:13           ` Leif Lindholm
2019-10-02 16:14             ` Abner Chang
2019-10-02 16:27               ` Andrew Fish
2019-10-02 16:35                 ` Leif Lindholm
2019-10-03  0:52                   ` Abner Chang
2019-10-03  8:38                     ` Leif Lindholm
2019-10-03 11:34                       ` Abner Chang
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 10/29] MdePkg/BasePeCoff: Add RISC-V PE/Coff related code Abner Chang
2019-09-26 23:46   ` [edk2-devel] " Leif Lindholm
2019-10-15  4:02     ` Abner Chang
2019-10-15 10:31       ` Leif Lindholm
2019-10-15 10:56         ` Abner Chang
     [not found]     ` <15CDB6324F411B37.30896@groups.io>
2019-10-15  4:26       ` Abner Chang
2019-10-15 10:41         ` Leif Lindholm
2019-10-15 10:59           ` Abner Chang
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 11/29] MdePkg/BaseCpuLib: RISC-V Base CPU library implementation Abner Chang
2019-09-26 23:47   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 12/29] MdePkg/BaseSynchronizationLib: RISC-V cache related code Abner Chang
2019-09-27  0:19   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 13/29] MdeModulePkg/Logo Abner Chang
2019-09-30 22:51   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 14/29] NetworkPkg Abner Chang
2019-09-30 22:51   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 15/29] RiscVPkg/Library: RISC-V CPU library Abner Chang
2019-09-30 18:31   ` [edk2-devel] " Leif Lindholm
2019-10-15  2:32     ` Abner Chang
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 16/29] RiscVPkg/Library: Add RISC-V exception library Abner Chang
2019-09-30 19:15   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 17/29] RiscVPkg/Library: Add RISC-V timer library Abner Chang
2019-09-30 19:46   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 18/29] RiscVPkg/Library: Add EDK2 RISC-V OpenSBI library Abner Chang
2019-09-30 20:03   ` [edk2-devel] " Leif Lindholm
2019-10-15  1:21     ` Abner Chang
2019-10-15  8:35       ` Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 19/29] RiscVPkg/Library: RISC-V platform level DxeIPL libraries Abner Chang
2019-09-30 20:15   ` [edk2-devel] " Leif Lindholm
2019-09-30 20:44     ` Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 20/29] MdeModulePkg/DxeIplPeim : RISC-V platform level DxeIPL Abner Chang
2019-09-30 20:31   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 21/29] RiscVPkg/PeiServicesTablePointerLibOpenSbi: RISC-V PEI Service Table Pointer library Abner Chang
2019-09-30 20:54   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 22/29] RiscVPkg/RiscVPlatformTempMemoryInit: RISC-V Platform Temporary Memory library Abner Chang
2019-09-30 20:56   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 23/29] RiscVPkg/CpuDxe: Add RISC-V CPU DXE driver Abner Chang
2019-09-30 21:11   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 24/29] BaseTools: BaseTools changes for RISC-V platform Abner Chang
2019-09-26 22:09   ` [edk2-devel] " Leif Lindholm
2019-10-15  6:18     ` Abner Chang
2019-10-15 10:56       ` Leif Lindholm
2019-10-15 11:13         ` Abner Chang
2019-10-16  5:06         ` Abner Chang
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 25/29] BaseTools/Scripts Abner Chang
2019-09-26 20:50   ` [edk2-devel] " Leif Lindholm
2019-10-15  6:31     ` Abner Chang
2019-10-15 11:00       ` Leif Lindholm
2019-10-15 11:03         ` Abner Chang
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 26/29] RiscVPkg/SmbiosDxe: Generic SMBIOS DXE driver for RISC-V platforms Abner Chang
2019-09-30 22:39   ` [edk2-devel] " Leif Lindholm
2019-10-14 11:27     ` Abner Chang
2019-10-14 11:56       ` Leif Lindholm [this message]
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 27/29] edk2-staging/RISC-V-V2: Add submodule Abner Chang
2019-09-26 22:24   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 28/29] edk2-staging/RISC-V-V2: Add ReadMe Abner Chang
2019-09-30 22:48   ` [edk2-devel] " Leif Lindholm
2019-09-23  0:31 ` [edk2-staging/RISC-V-V2 PATCH v2 29/29] edk2-staging: Update Maintainers.txt Abner Chang
2019-09-30 22:50   ` [edk2-devel] " Leif Lindholm
     [not found] ` <15C6EB9824DD2A88.29693@groups.io>
2019-09-24  1:52   ` [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 04/29] MdePkg: RISC-V RV64 binding in MdePkg Abner Chang
     [not found] ` <15C6EB994C26E5C4.2053@groups.io>
2019-09-24  1:52   ` [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 05/29] MdePkg/Include: RISC-V definitions Abner Chang
     [not found] ` <15C6EB9950232DB5.29693@groups.io>
2019-09-24  1:53   ` [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 07/29] MdePkg/BaseLib: BaseLib for RISC-V RV64 Processor Abner Chang
     [not found] ` <15C6EB9A049FF8A4.24160@groups.io>
2019-09-24  1:54   ` [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 09/29] MdePkg/BaseIoLibIntrinsic: RISC-V I/O intrinsic functions Abner Chang
     [not found] ` <15C6EB9B3E887BEB.29693@groups.io>
2019-09-24  1:55   ` [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 13/29] MdeModulePkg/Logo Abner Chang
     [not found] ` <15C6EB9A40C408A0.24160@groups.io>
2019-09-24  1:56   ` [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 10/29] MdePkg/BasePeCoff: Add RISC-V PE/Coff related code Abner Chang
     [not found] ` <15C6EB9B872A5B83.24160@groups.io>
2019-09-24  1:57   ` [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 14/29] NetworkPkg Abner Chang
     [not found] ` <15C6EB99CBC780B5.2053@groups.io>
2019-09-24  1:57   ` [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 08/29] MdePkg/BaseCacheMaintenanceLib: RISC-V cache maintenance implementation Abner Chang
     [not found] ` <15C6EB9A9BD83853.2053@groups.io>
2019-09-24  1:58   ` [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 11/29] MdePkg/BaseCpuLib: RISC-V Base CPU library implementation Abner Chang
     [not found] ` <15C6EB9AEB7BB057.24160@groups.io>
2019-09-24  1:58   ` [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 12/29] MdePkg/BaseSynchronizationLib: RISC-V cache related code Abner Chang
     [not found] ` <15C6EB99608359A3.24160@groups.io>
2019-09-24  1:59   ` [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 08/29] MdePkg/BaseCacheMaintenanceLib: RISC-V cache maintenance implementation Abner Chang
     [not found] ` <15C6EB9D6C0EC3B0.29693@groups.io>
2019-09-24  2:00   ` [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 20/29] MdeModulePkg/DxeIplPeim : RISC-V platform level DxeIPL Abner Chang
     [not found] ` <15C6EB98AD6CCCEB.24160@groups.io>
2019-09-24  2:01   ` [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 06/29] MdeModulePkg/CapsuleRuntimeDxe: Add RISCV64 arch Abner Chang
     [not found] ` <15C6EB9F04387439.29693@groups.io>
2019-09-24  2:02   ` [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 25/29] BaseTools/Scripts Abner Chang
2019-09-26 22:22 ` [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 00/29] RISC-V EDK2 Port on Leif Lindholm
2019-10-15  6:39   ` Abner Chang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191014115605.GC25504@bivouac.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox