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
Cc: "Chen, Gilbert" <gilbert.chen@hpe.com>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v3 03/39] RiscVPkg/opensbi: EDK2 RISC-V OpenSBI support
Date: Thu, 19 Dec 2019 13:21:36 +0000	[thread overview]
Message-ID: <20191219132136.GU7359@bivouac.eciton.net> (raw)
In-Reply-To: <AT5PR8401MB1154380DF2E8A66C8699AE59FF520@AT5PR8401MB1154.NAMPRD84.PROD.OUTLOOK.COM>

On Thu, Dec 19, 2019 at 04:09:22 +0000, Abner Chang wrote:
> > > diff --git a/RiscVPkg/Include/sbi/sbi.h b/RiscVPkg/Include/sbi/sbi.h
> > > new file mode 100644 index 0000000..04e7f18
> > > --- /dev/null
> > > +++ b/RiscVPkg/Include/sbi/sbi.h
> > > @@ -0,0 +1,52 @@
> > > +/** @file
> > > +  SBI inline function calls.
> > > +
> > > +  Copyright (c) 2019, Hewlett Packard Enterprise Development LP. All
> > > + rights reserved.<BR>
> > > +
> > > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +
> > > +**/
> > > +
> > > +#ifndef EDK2_SBI_H_
> > > +#define EDK2_SBI_H_
> > > +
> > > +#include <sbi/sbi_types.h>  // Reference to header file wrapper
> > > +#include <include/sbi/riscv_asm.h> // Reference to header file in
> > > +opensbi
> > 
> > This whole sbi situation is too much of a hornets' nest, and really does need
> > to get sorted before we even push this to -staging.
> > (But I do think is is one of the last things we need to address.)
> > 
> > And I think the starting point to untangle it is to rename the wrapper include
> > files in RiscVPkg/Include/sbi (and indeed the directory
> > itself) to conform with the TianoCore standard.
> > 
> > That means:
> > 
> > RiscVPkg/Include/IndustryStandard/Sbi/Sbi.h
> > RiscVPkg/Include/IndustryStandard/Sbi/SbiBits.h
> > RiscVPkg/Include/IndustryStandard/Sbi/SbiTypes.h
> > 
> > Unless all of the wrappers can be combined into a single
> > RiscVPkg/Include/IndustryStandard/Sbi.h
> > 
> > ...and then tested on a case-sensitive filesystem.
> > 
> > For the record, doing that immediately results in the non-wrapper versions
> > being pulled in and then the build failing.
> 
> Leif,
> We can't change the naming of those header files under
> RiscVPkg/Include/sbi. We build opensbi source code in edk2 and those
> source files refer to those header file. And we are not going to
> change opensbi code as we discussed before.
>
> The only file we can change is sbi.h -> Sbi.h because this header
> file is created for edk2 RISC-V related drivers.
> 
> I think we can make it this way,
> Rename sbi.h to RiscVOpensbi.h and move it to under
> RiscVPkg/Include/IndustryStandard, no need "/Sbi" subdirectory.

That sounds good to me.

> Keep the naming  of sbi_bits.h and sbi_types.h and still leave those
> under RiscVPkg/Include/sbi.
>
> No naming changes in order to build opensbi source file. Can't even
> change "/sbi" subdirectory to "/Sbi". sbi_bits.h and sbi_types.h are
> the binding files for edk2 framework BTW.

Sure, if we separate interface for EDK2 apps from binding, it becomes
less bad. Plus sbi_bits.h needs to just go, but we'll fix that in
opensbi before merging Risc-V to edk2 master.
I think the proper fix for sbi_types.h also involves a change to
opensbi.

But if you can
- do those changes
- move/rename SbiFirmwareContext.h to live with RiscVOpensbi.h
- add a (short) Readme.md to RiscVPkg/Include/sbi explaining what
  these files are and why
I think that resolves my final reservations against bringing this set
onto a staging branch.

Regards,

Leif

  reply	other threads:[~2019-12-19 13:21 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-28  1:58 [edk2-staging/RISC-V-V2 PATCH v3 00/39] RISC-V EDK2 Port on Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 01/39] RiscVPkg: RISC-V processor package Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 02/39] RiscVPkg/Include: Add header files of RISC-V CPU package Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 03/39] RiscVPkg/opensbi: EDK2 RISC-V OpenSBI support Abner Chang
2019-11-21 16:24   ` Leif Lindholm
2019-12-19  4:09     ` [edk2-devel] " Abner Chang
2019-12-19 13:21       ` Leif Lindholm [this message]
2019-12-19 14:48         ` Abner Chang
     [not found]         ` <15E1CD20DD2FE7F1.29030@groups.io>
2019-12-20  3:04           ` Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 04/39] MdePkg: RISC-V RV64 binding in MdePkg Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 05/39] MdePkg/Include: RISC-V definitions Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 06/39] MdeModulePkg/CapsuleRuntimeDxe: Add RISCV64 arch Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 07/39] MdePkg/BaseLib: BaseLib for RISC-V RV64 Processor Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 08/39] MdePkg/BaseCacheMaintenanceLib: RISC-V cache maintenance implementation Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 09/39] MdePkg/BaseIoLibIntrinsic: RISC-V I/O intrinsic functions Abner Chang
2019-11-21 16:36   ` Leif Lindholm
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 10/39] MdePkg/BasePeCoff: Add RISC-V PE/Coff related code Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 11/39] MdePkg/BaseCpuLib: RISC-V Base CPU library implementation Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 12/39] MdePkg/BaseSynchronizationLib: RISC-V cache related code Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 13/39] MdeModulePkg/Logo Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 14/39] NetworkPkg Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 15/39] RiscVPkg/Library: RISC-V CPU library Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 16/39] RiscVPkg/Library: Add RISC-V exception library Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 17/39] RiscVPkg/Library: Add RISC-V timer library Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 18/39] RiscVPkg/Library: Add EDK2 RISC-V OpenSBI library Abner Chang
2019-11-21 16:48   ` [edk2-devel] " Leif Lindholm
2019-12-19  6:00     ` Abner Chang
2019-12-20 15:04       ` Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 19/39] RiscVPkg/Library: RISC-V platform level DxeIPL libraries Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 20/39] MdeModulePkg/DxeIplPeim : RISC-V platform level DxeIPL Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 21/39] RiscVPkg/PeiServicesTablePointerLibOpenSbi: RISC-V PEI Service Table Pointer library Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 22/39] RiscVPlatformPkg/RiscVPlatformTempMemoryInit: RISC-V Platform Temporary Memory library Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 23/39] RiscVPkg/CpuDxe: Add RISC-V CPU DXE driver Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 24/39] BaseTools: BaseTools changes for RISC-V platform Abner Chang
2019-11-21 16:55   ` Leif Lindholm
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 25/39] BaseTools/Scripts Abner Chang
2019-11-19 18:17   ` [edk2-devel] " Mark Salter
2019-11-20  8:23     ` Abner Chang
2019-11-21  7:40       ` Abner Chang
2019-11-21 16:56   ` Leif Lindholm
2019-11-22  3:49     ` Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 26/39] RiscVPkg/SmbiosDxe: Generic SMBIOS DXE driver for RISC-V platforms Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 27/39] RiscVPkg/RiscVOpensbLlib: Add submodule opensbi Abner Chang
2019-11-21 17:00   ` Leif Lindholm
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 28/39] RiscVPlatformPkg/FirmwareContextProcessorSpecificLib:Add FirmwareContextProcessorSpecificLib module Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 29/39] RiscVPlatformPkg/RealTimeClockLibNull: Null instance of RTC lib Abner Chang
2019-11-21 17:02   ` Leif Lindholm
2019-11-22 12:28     ` Abner Chang
2019-11-22 14:08       ` Leif Lindholm
2019-11-22 14:19         ` Abner Chang
2019-11-22 14:55           ` Leif Lindholm
2019-11-22 16:05             ` [edk2-devel] " Abner Chang
2019-11-22 16:32               ` Leif Lindholm
2019-11-23  7:38                 ` Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 30/39] RiscVPlatformPkg/OpensbiPlatformLibNull: NULL instance of RiscVOpensbiPlatformLib Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 31/39] RiscVPlatformPkg/PlatformMemoryTestLibNull: NULL instance of PlatformMemoryTestLib Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 32/39] RiscVPlatformPkg/PlatformUpdateProgressLibNull: NULL instance of PlatformUpdateProgressLib Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 33/39] RiscVPlatformPkg/PlatformBootManagerLib: Platform Boot Manager library Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 34/39] RiscVPkg/RiscVPlatformTimerLibNull: NULL instance of RISC-V platform timer library Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 35/39] RiscVPlatformPkg/SecMain: RISC-V SecMain module Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 36/39] RiscVPlatformPkg: Add RiscVPlatformPkg Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 37/39] edk2-staging:RISC-V-V2: Add RiscVEdk2Readme.md Abner Chang
2019-11-21 17:09   ` Leif Lindholm
2019-11-22  3:57     ` [edk2-devel] " Abner Chang
2019-11-22  9:34       ` Leif Lindholm
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 38/39] edk2-staging: Revise Maintainers.txt for RISC-V packages Abner Chang
2019-10-29  1:05   ` [edk2-devel] " Laszlo Ersek
2019-10-29  1:31     ` Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 39/39] edk2-staging: Revise Readme.md Abner Chang
2019-10-29  1:07   ` [edk2-devel] " Laszlo Ersek
2019-10-29  1:35     ` Abner Chang
2019-10-29  2:01     ` Leif Lindholm
2019-10-31  8:20       ` Laszlo Ersek
2019-11-21 17:15   ` Leif Lindholm
2019-11-22  2:05     ` Abner Chang
2019-11-22  9:42       ` Leif Lindholm
2019-11-22 11:46         ` 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=20191219132136.GU7359@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