1. Patchew
  2. EDK2
  3. [edk2] [PATCH v2 0/4] support HiKey960
  4. View patch

[edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file

Haojian Zhuang posted 4 patches 6 years, 10 months ago
[edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
Posted by Haojian Zhuang 6 years, 10 months ago 
With the SerialPortLib and DebugLib, Dxe runtime driver can't
be executed well on HiKey. Serial logs are missing.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 Platform/Hisilicon/D02/Pv660D02.dsc | 2 ++
 Platform/Hisilicon/D03/D03.dsc      | 1 +
 Platform/Hisilicon/D05/D05.dsc      | 1 +
 Silicon/Hisilicon/Hisilicon.dsc.inc | 2 --
 4 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Platform/Hisilicon/D02/Pv660D02.dsc b/Platform/Hisilicon/D02/Pv660D02.dsc
index 9e826ae..018e149 100644
--- a/Platform/Hisilicon/D02/Pv660D02.dsc
+++ b/Platform/Hisilicon/D02/Pv660D02.dsc
@@ -80,6 +80,8 @@
 
 [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
+  SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf
+  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
 
 [BuildOptions]
   GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Hisilicon/Pv660/Include
diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
index c496306..b9bce66 100644
--- a/Platform/Hisilicon/D03/D03.dsc
+++ b/Platform/Hisilicon/D03/D03.dsc
@@ -97,6 +97,7 @@
 [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
   SerialPortLib|Silicon/Hisilicon/Hi1610/Library/Uart/LpcSerialPortLib/LpcSerialPortLib.inf
+  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
 
 [BuildOptions]
   GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Hisilicon/Hi1610/Include
diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
index 0792b08..dfee09b 100644
--- a/Platform/Hisilicon/D05/D05.dsc
+++ b/Platform/Hisilicon/D05/D05.dsc
@@ -105,6 +105,7 @@
 [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
   SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
+  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
 
 [BuildOptions]
   GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Hisilicon/Hi1616/Include
diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc b/Silicon/Hisilicon/Hisilicon.dsc.inc
index 5766829..b5b9e7e 100644
--- a/Silicon/Hisilicon/Hisilicon.dsc.inc
+++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
@@ -208,8 +208,6 @@
   MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
   ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
-  SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf
-  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
 
 [LibraryClasses.AARCH64]
   ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
Posted by Leif Lindholm 6 years, 10 months ago 
On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
> With the SerialPortLib and DebugLib, Dxe runtime driver can't
> be executed well on HiKey. Serial logs are missing.

"Can't be executed well"? Does this mean it crashes?

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  Platform/Hisilicon/D02/Pv660D02.dsc | 2 ++
>  Platform/Hisilicon/D03/D03.dsc      | 1 +
>  Platform/Hisilicon/D05/D05.dsc      | 1 +
>  Silicon/Hisilicon/Hisilicon.dsc.inc | 2 --
>  4 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/Hisilicon/D02/Pv660D02.dsc b/Platform/Hisilicon/D02/Pv660D02.dsc
> index 9e826ae..018e149 100644
> --- a/Platform/Hisilicon/D02/Pv660D02.dsc
> +++ b/Platform/Hisilicon/D02/Pv660D02.dsc
> @@ -80,6 +80,8 @@
>  
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>    I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
> +  SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf

This change I agree with - this is a clear fix.

> +  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf

But this one just causes duplication of boilerplate.
Could you instead put the fragment in Hisilicon.dsc.inc ...

>  
>  [BuildOptions]
>    GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Hisilicon/Pv660/Include
> diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
> index c496306..b9bce66 100644
> --- a/Platform/Hisilicon/D03/D03.dsc
> +++ b/Platform/Hisilicon/D03/D03.dsc
> @@ -97,6 +97,7 @@
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>    I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
>    SerialPortLib|Silicon/Hisilicon/Hi1610/Library/Uart/LpcSerialPortLib/LpcSerialPortLib.inf
> +  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>  
>  [BuildOptions]
>    GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Hisilicon/Hi1610/Include
> diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
> index 0792b08..dfee09b 100644
> --- a/Platform/Hisilicon/D05/D05.dsc
> +++ b/Platform/Hisilicon/D05/D05.dsc
> @@ -105,6 +105,7 @@
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>    I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
>    SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> +  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>  
>  [BuildOptions]
>    GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Hisilicon/Hi1616/Include
> diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc b/Silicon/Hisilicon/Hisilicon.dsc.inc
> index 5766829..b5b9e7e 100644
> --- a/Silicon/Hisilicon/Hisilicon.dsc.inc
> +++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
> @@ -208,8 +208,6 @@
>    MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>    ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf
>    CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
> -  SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf

... behind a conditional like

!ifndef CONFIG_NO_DEBUGLIB
> -  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
!endif

?

And correspondingly set CONFIG_NO_DEBUGLIB in hikey/hikey960 .dscs?

Also, if this is a problem causes by using Hisilicon.dsc.inc, it
should really go in before 2/4.
This is great, by the way - I was not expecting that you would be able
to reuse that, I thought there would be a separate .inc for
hikey/hikey960.

/
    Leif

>  
>  [LibraryClasses.AARCH64]
>    ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
> -- 
> 2.7.4
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
Posted by Haojian Zhuang 6 years, 10 months ago 
>From: Leif Lindholm <leif.lindholm@linaro.org>
>Sent: Monday, February 12, 2018 11:45 AM
>To: Haojian Zhuang
>Cc: edk2-devel@lists.01.org; linaro-uefi@lists.linaro.org; ard.sheuvel@linaro.org; heyi.guo@linaro.org
>Subject: Re: [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
>
>On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
>> With the SerialPortLib and DebugLib, Dxe runtime driver can't
>> be executed well on HiKey. Serial logs are missing.
>
>"Can't be executed well"? Does this mean it crashes?

No crash. Serial output are missing since SerialPortLib is different.

Best Regards
Haojian
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
Posted by Ard Biesheuvel 6 years, 10 months ago 
On 12 February 2018 at 11:47, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:
>>From: Leif Lindholm <leif.lindholm@linaro.org>
>>Sent: Monday, February 12, 2018 11:45 AM
>>To: Haojian Zhuang
>>Cc: edk2-devel@lists.01.org; linaro-uefi@lists.linaro.org; ard.sheuvel@linaro.org; heyi.guo@linaro.org
>>Subject: Re: [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
>>
>>On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
>>> With the SerialPortLib and DebugLib, Dxe runtime driver can't
>>> be executed well on HiKey. Serial logs are missing.
>>
>>"Can't be executed well"? Does this mean it crashes?
>
> No crash. Serial output are missing since SerialPortLib is different.
>

Does this driver take care to only create serial output at boot time?
Does it, e.g., call EfiAtRuntime() or use a notification callback at
ExitBootServices to make absolutely sure the serial port is no longer
used?
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
Posted by Haojian Zhuang 6 years, 10 months ago 
>From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>Sent: Monday, February 12, 2018 12:05 PM
>To: Haojian Zhuang
>Cc: Leif Lindholm; edk2-devel@lists.01.org; heyi.guo@linaro.org; ard.sheuvel@linaro.org; linaro-uefi@lists.linaro.org
>Subject: Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
>
>On 12 February 2018 at 11:47, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:
>>>From: Leif Lindholm <leif.lindholm@linaro.org>
>>>Sent: Monday, February 12, 2018 11:45 AM
>>>To: Haojian Zhuang
>>>Cc: edk2-devel@lists.01.org; linaro-uefi@lists.linaro.org; ard.sheuvel@linaro.org; heyi.guo@linaro.org
>>>Subject: Re: [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
>>>
>>>On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
>>>> With the SerialPortLib and DebugLib, Dxe runtime driver can't
>>>> be executed well on HiKey. Serial logs are missing.
>>>
>>>"Can't be executed well"? Does this mean it crashes?
>>
>> No crash. Serial output are missing since SerialPortLib is different.
>>
>
>Does this driver take care to only create serial output at boot time?
>Does it, e.g., call EfiAtRuntime() or use a notification callback at
>ExitBootServices to make absolutely sure the serial port is no longer
>used?

These drivers don't use serial port directly. But I tried to use DEBUG () 
function to dump some debug informations in these drivers. I found 
that I can't output anything on serial console.

Best Regards
Haojian
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
Posted by Ard Biesheuvel 6 years, 10 months ago 
On 12 February 2018 at 12:19, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:
>>From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>Sent: Monday, February 12, 2018 12:05 PM
>>To: Haojian Zhuang
>>Cc: Leif Lindholm; edk2-devel@lists.01.org; heyi.guo@linaro.org; ard.sheuvel@linaro.org; linaro-uefi@lists.linaro.org
>>Subject: Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
>>
>>On 12 February 2018 at 11:47, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:
>>>>From: Leif Lindholm <leif.lindholm@linaro.org>
>>>>Sent: Monday, February 12, 2018 11:45 AM
>>>>To: Haojian Zhuang
>>>>Cc: edk2-devel@lists.01.org; linaro-uefi@lists.linaro.org; ard.sheuvel@linaro.org; heyi.guo@linaro.org
>>>>Subject: Re: [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
>>>>
>>>>On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
>>>>> With the SerialPortLib and DebugLib, Dxe runtime driver can't
>>>>> be executed well on HiKey. Serial logs are missing.
>>>>
>>>>"Can't be executed well"? Does this mean it crashes?
>>>
>>> No crash. Serial output are missing since SerialPortLib is different.
>>>
>>
>>Does this driver take care to only create serial output at boot time?
>>Does it, e.g., call EfiAtRuntime() or use a notification callback at
>>ExitBootServices to make absolutely sure the serial port is no longer
>>used?
>
> These drivers don't use serial port directly. But I tried to use DEBUG ()
> function to dump some debug informations in these drivers. I found
> that I can't output anything on serial console.
>

But do those DEBUG() calls only occur at boot time? Or could they be
called at runtime as well?
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
Posted by Haojian Zhuang 6 years, 10 months ago 
>From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>Sent: Monday, February 12, 2018 12:22 PM
>To: Haojian Zhuang
>Cc: Leif Lindholm; edk2-devel@lists.01.org; heyi.guo@linaro.org; linaro-uefi@lists.linaro.org
>Subject: Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
>
>On 12 February 2018 at 12:19, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:
>>>From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>Sent: Monday, February 12, 2018 12:05 PM
>>>To: Haojian Zhuang
>>>Cc: Leif Lindholm; edk2-devel@lists.01.org; heyi.guo@linaro.org; ard.sheuvel@linaro.org; linaro-uefi@lists.linaro.org
>>>Subject: Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
>>>
>>>>On 12 February 2018 at 11:47, Haojian Zhuang <haojian.zhuang@linaro.org> wrote:
>>>>>From: Leif Lindholm <leif.lindholm@linaro.org>
>>>>>Sent: Monday, February 12, 2018 11:45 AM
>>>>>To: Haojian Zhuang
>>>>>Cc: edk2-devel@lists.01.org; linaro-uefi@lists.linaro.org; ard.sheuvel@linaro.org; heyi.guo@linaro.org
>>>>>Subject: Re: [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
>>>>>
>>>>>On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
>>>>>> With the SerialPortLib and DebugLib, Dxe runtime driver can't
>>>>>> be executed well on HiKey. Serial logs are missing.
>>>>>
>>>>>"Can't be executed well"? Does this mean it crashes?
>>>>
>>>> No crash. Serial output are missing since SerialPortLib is different.
>>>>
>>>
>>>Does this driver take care to only create serial output at boot time?
>>>Does it, e.g., call EfiAtRuntime() or use a notification callback at
>>>ExitBootServices to make absolutely sure the serial port is no longer
>>>used?
>>
>> These drivers don't use serial port directly. But I tried to use DEBUG ()
>> function to dump some debug informations in these drivers. I found
>> that I can't output anything on serial console.
>>
>
>But do those DEBUG() calls only occur at boot time? Or could they be
>called at runtime as well?

Excuse me that I didn't explain it clearly. 

At first, I need to make sure everything executed well when I switch to
the common dsc file. So I added some debug messages in those key
drivers.
In the second, I need to debug the boot flow later. I mean that I need some
debug message in the initialization of DXE runtime driver. I'm considering
to make use of EmuVariable and predefined emu variable region in RAM.
Then I could store the predefined boot options in emu variable region.
And I could re-use PlatformBootManager in ArmPlatformPkg without big
changes. So I need to add some debug messages in these DXE runtime
driver.

These two drivers in the common dsc file blocks me enabling debug
messages in the initialization code of DXE runtime driver. So I have
to move them out.

Best Regards
Haojian
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
Posted by Leif Lindholm 6 years, 10 months ago 
On Mon, Feb 12, 2018 at 11:45:06AM +0000, Leif Lindholm wrote:
> On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
> > With the SerialPortLib and DebugLib, Dxe runtime driver can't
> > be executed well on HiKey. Serial logs are missing.
> 
> "Can't be executed well"? Does this mean it crashes?

You replied to this question, but not the further ones below.
Can you have a look, please?

/
    Leif

> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> > ---
> >  Platform/Hisilicon/D02/Pv660D02.dsc | 2 ++
> >  Platform/Hisilicon/D03/D03.dsc      | 1 +
> >  Platform/Hisilicon/D05/D05.dsc      | 1 +
> >  Silicon/Hisilicon/Hisilicon.dsc.inc | 2 --
> >  4 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Platform/Hisilicon/D02/Pv660D02.dsc b/Platform/Hisilicon/D02/Pv660D02.dsc
> > index 9e826ae..018e149 100644
> > --- a/Platform/Hisilicon/D02/Pv660D02.dsc
> > +++ b/Platform/Hisilicon/D02/Pv660D02.dsc
> > @@ -80,6 +80,8 @@
> >  
> >  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> >    I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
> > +  SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf
> 
> This change I agree with - this is a clear fix.
> 
> > +  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
> 
> But this one just causes duplication of boilerplate.
> Could you instead put the fragment in Hisilicon.dsc.inc ...
> 
> >  
> >  [BuildOptions]
> >    GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Hisilicon/Pv660/Include
> > diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
> > index c496306..b9bce66 100644
> > --- a/Platform/Hisilicon/D03/D03.dsc
> > +++ b/Platform/Hisilicon/D03/D03.dsc
> > @@ -97,6 +97,7 @@
> >  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> >    I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
> >    SerialPortLib|Silicon/Hisilicon/Hi1610/Library/Uart/LpcSerialPortLib/LpcSerialPortLib.inf
> > +  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
> >  
> >  [BuildOptions]
> >    GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Hisilicon/Hi1610/Include
> > diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
> > index 0792b08..dfee09b 100644
> > --- a/Platform/Hisilicon/D05/D05.dsc
> > +++ b/Platform/Hisilicon/D05/D05.dsc
> > @@ -105,6 +105,7 @@
> >  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> >    I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
> >    SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> > +  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
> >  
> >  [BuildOptions]
> >    GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Hisilicon/Hi1616/Include
> > diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc b/Silicon/Hisilicon/Hisilicon.dsc.inc
> > index 5766829..b5b9e7e 100644
> > --- a/Silicon/Hisilicon/Hisilicon.dsc.inc
> > +++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
> > @@ -208,8 +208,6 @@
> >    MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> >    ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf
> >    CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
> > -  SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf
> 
> ... behind a conditional like
> 
> !ifndef CONFIG_NO_DEBUGLIB
> > -  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
> !endif
> 
> ?
> 
> And correspondingly set CONFIG_NO_DEBUGLIB in hikey/hikey960 .dscs?
> 
> Also, if this is a problem causes by using Hisilicon.dsc.inc, it
> should really go in before 2/4.
> This is great, by the way - I was not expecting that you would be able
> to reuse that, I thought there would be a separate .inc for
> hikey/hikey960.
> 
> /
>     Leif
> 
> >  
> >  [LibraryClasses.AARCH64]
> >    ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
> > -- 
> > 2.7.4
> > 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
Posted by Haojian Zhuang 6 years, 10 months ago 
On 02/15/2018 11:41 PM, Leif Lindholm wrote:
> On Mon, Feb 12, 2018 at 11:45:06AM +0000, Leif Lindholm wrote:
>> On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
>>> With the SerialPortLib and DebugLib, Dxe runtime driver can't
>>> be executed well on HiKey. Serial logs are missing.
>>
>> "Can't be executed well"? Does this mean it crashes?
> 
> You replied to this question, but not the further ones below.
> Can you have a look, please?

Excuse me that I missed comment in below.

> 
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>>> ---
>>>   Platform/Hisilicon/D02/Pv660D02.dsc | 2 ++
>>>   Platform/Hisilicon/D03/D03.dsc      | 1 +
>>>   Platform/Hisilicon/D05/D05.dsc      | 1 +
>>>   Silicon/Hisilicon/Hisilicon.dsc.inc | 2 --
>>>   4 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Platform/Hisilicon/D02/Pv660D02.dsc b/Platform/Hisilicon/D02/Pv660D02.dsc
>>> index 9e826ae..018e149 100644
>>> --- a/Platform/Hisilicon/D02/Pv660D02.dsc
>>> +++ b/Platform/Hisilicon/D02/Pv660D02.dsc
>>> @@ -80,6 +80,8 @@
>>>   
>>>   [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>>>     I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
>>> +  SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf
>>
>> This change I agree with - this is a clear fix.
>>
>>> +  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>>
>> But this one just causes duplication of boilerplate.
>> Could you instead put the fragment in Hisilicon.dsc.inc ...
>>
>>>   
>>>   [BuildOptions]
>>>     GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Hisilicon/Pv660/Include
>>> diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
>>> index c496306..b9bce66 100644
>>> --- a/Platform/Hisilicon/D03/D03.dsc
>>> +++ b/Platform/Hisilicon/D03/D03.dsc
>>> @@ -97,6 +97,7 @@
>>>   [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>>>     I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
>>>     SerialPortLib|Silicon/Hisilicon/Hi1610/Library/Uart/LpcSerialPortLib/LpcSerialPortLib.inf
>>> +  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>>>   
>>>   [BuildOptions]
>>>     GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Hisilicon/Hi1610/Include
>>> diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
>>> index 0792b08..dfee09b 100644
>>> --- a/Platform/Hisilicon/D05/D05.dsc
>>> +++ b/Platform/Hisilicon/D05/D05.dsc
>>> @@ -105,6 +105,7 @@
>>>   [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>>>     I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
>>>     SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
>>> +  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>>>   
>>>   [BuildOptions]
>>>     GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Hisilicon/Hi1616/Include
>>> diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc b/Silicon/Hisilicon/Hisilicon.dsc.inc
>>> index 5766829..b5b9e7e 100644
>>> --- a/Silicon/Hisilicon/Hisilicon.dsc.inc
>>> +++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
>>> @@ -208,8 +208,6 @@
>>>     MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>>>     ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf
>>>     CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
>>> -  SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf
>>
>> ... behind a conditional like
>>
>> !ifndef CONFIG_NO_DEBUGLIB
>>> -  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>> !endif
>>
>> ?
>>
>> And correspondingly set CONFIG_NO_DEBUGLIB in hikey/hikey960 .dscs?

OK. I'll create it.

>>
>> Also, if this is a problem causes by using Hisilicon.dsc.inc, it
>> should really go in before 2/4.

OK. I'll move it before [2/4].

>> This is great, by the way - I was not expecting that you would be able
>> to reuse that, I thought there would be a separate .inc for
>> hikey/hikey960.
>>
>> /
>>      Leif
>>
>>>   
>>>   [LibraryClasses.AARCH64]
>>>     ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
>>> -- 
>>> 2.7.4
>>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
Posted by Guo Heyi 6 years, 10 months ago 
Hi Haojian,

Dw8250SerialPortRuntimeLib actually depends on DW8250 hardware IP; if there
isn't such device on Hikey, then you can't use this library instance indeed.

But I think PeiDxeDebugLibReportStatusCode should be some common code, however
it depends on ReportStatusCodeLib and Status Code PEIM and Status code DXE
driver. Have you added them too?

Heyi

On Sat, Feb 10, 2018 at 01:31:06AM +0800, Haojian Zhuang wrote:
> With the SerialPortLib and DebugLib, Dxe runtime driver can't
> be executed well on HiKey. Serial logs are missing.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  Platform/Hisilicon/D02/Pv660D02.dsc | 2 ++
>  Platform/Hisilicon/D03/D03.dsc      | 1 +
>  Platform/Hisilicon/D05/D05.dsc      | 1 +
>  Silicon/Hisilicon/Hisilicon.dsc.inc | 2 --
>  4 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/Hisilicon/D02/Pv660D02.dsc b/Platform/Hisilicon/D02/Pv660D02.dsc
> index 9e826ae..018e149 100644
> --- a/Platform/Hisilicon/D02/Pv660D02.dsc
> +++ b/Platform/Hisilicon/D02/Pv660D02.dsc
> @@ -80,6 +80,8 @@
>  
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>    I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
> +  SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf
> +  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>  
>  [BuildOptions]
>    GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Hisilicon/Pv660/Include
> diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
> index c496306..b9bce66 100644
> --- a/Platform/Hisilicon/D03/D03.dsc
> +++ b/Platform/Hisilicon/D03/D03.dsc
> @@ -97,6 +97,7 @@
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>    I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
>    SerialPortLib|Silicon/Hisilicon/Hi1610/Library/Uart/LpcSerialPortLib/LpcSerialPortLib.inf
> +  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>  
>  [BuildOptions]
>    GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Hisilicon/Hi1610/Include
> diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
> index 0792b08..dfee09b 100644
> --- a/Platform/Hisilicon/D05/D05.dsc
> +++ b/Platform/Hisilicon/D05/D05.dsc
> @@ -105,6 +105,7 @@
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>    I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLibRuntime.inf
>    SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> +  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>  
>  [BuildOptions]
>    GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Silicon/Hisilicon/Hi1616/Include
> diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc b/Silicon/Hisilicon/Hisilicon.dsc.inc
> index 5766829..b5b9e7e 100644
> --- a/Silicon/Hisilicon/Hisilicon.dsc.inc
> +++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
> @@ -208,8 +208,6 @@
>    MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>    ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf
>    CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
> -  SerialPortLib|Silicon/Hisilicon/Library/Dw8250SerialPortRuntimeLib/Dw8250SerialPortRuntimeLib.inf
> -  DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
>  
>  [LibraryClasses.AARCH64]
>    ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
> -- 
> 2.7.4
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
Posted by Haojian Zhuang 6 years, 10 months ago 
On 02/13/2018 08:23 AM, Guo Heyi wrote:
> Hi Haojian,
> 
> Dw8250SerialPortRuntimeLib actually depends on DW8250 hardware IP; if there
> isn't such device on Hikey, then you can't use this library instance indeed.
> 
> But I think PeiDxeDebugLibReportStatusCode should be some common code, however
> it depends on ReportStatusCodeLib and Status Code PEIM and Status code DXE
> driver. Have you added them too?
>
If I leave PeiDxeDebugLibReportStatusCode and move Dw8250SerialPortRuntimeLib out,
I'll meet UEFI crash. It seems the debug lib is depended on serial port lib.

And I consider that Dw8250 serial port is only valid on D02. So I decide to move them out.

Best Regards
Haojian
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 3/4] Platform/Hisilicon: move out dxe runtime lib from common file
Posted by Guo Heyi 6 years, 10 months ago 
On Tue, Feb 13, 2018 at 12:59:50AM +0000, Haojian Zhuang wrote:
> On 02/13/2018 08:23 AM, Guo Heyi wrote:
> > Hi Haojian,
> > 
> > Dw8250SerialPortRuntimeLib actually depends on DW8250 hardware IP; if there
> > isn't such device on Hikey, then you can't use this library instance indeed.
> > 
> > But I think PeiDxeDebugLibReportStatusCode should be some common code, however
> > it depends on ReportStatusCodeLib and Status Code PEIM and Status code DXE
> > driver. Have you added them too?
> >
> If I leave PeiDxeDebugLibReportStatusCode and move Dw8250SerialPortRuntimeLib out,
> I'll meet UEFI crash. It seems the debug lib is depended on serial port lib.
This is strange, because you should have your own SerialPortLib of your
platform, otherwise error will occur when you build your UEFI.

> 
> And I consider that Dw8250 serial port is only valid on D02. So I decide to move them out.

I agree.

Regards,

Heyi

> 
> Best Regards
> Haojian
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.