[edk2] [PATCH] ShellPkg: ShellFileHandleReadLine must return UCS2 lines

2016-02-10 Thread Jim_Dailey
ShellPkg: ShellFileHandleReadLine must return UCS2 lines.

An earlier change had this function returning the type of lines that were in 
the file being read (ASCII or UCS2).  The way it is used, UCS2 output is 
expected, even when the file being read is ASCII. This change restores that 
behavior and documents it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jim Dailey 
---
diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c 
b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
index 4b53c70..abff0d3 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
@@ -4084,9 +4084,20 @@ ShellFileHandleReturnLine(
   If the position upon start is 0, then the Ascii Boolean will be set.  This 
should be
   maintained and not changed for all operations with the same file.
 
+  NOTE: LINES THAT ARE RETURNED BY THIS FUNCTION ARE UCS2, EVEN IF THE FILE 
BEING READ
+IS IN ASCII FORMAT.
+
   @param[in]   HandleSHELL_FILE_HANDLE to read from.
-  @param[in, out]  BufferThe pointer to buffer to read into.
-  @param[in, out]  Size  The pointer to number of bytes in Buffer.
+  @param[in, out]  BufferThe pointer to buffer to read into. If this 
function
+ returns EFI_SUCCESS, then on output Buffer 
will
+ contain a UCS2 string, even if the file being
+ read is ASCII.
+  @param[in, out]  Size  On input, pointer to number of bytes in 
Buffer.
+ On output, unchanged unless Buffer is too 
small
+ to contain the next line of the file. In that
+ case Size is set to the number of bytes needed
+ to hold the next line of the file (as a UCS2
+ string, even if it is an ASCII file).
   @param[in]   Truncate  If the buffer is large enough, this has no 
effect.
  If the buffer is is too small and Truncate is 
TRUE,
  the line will be truncated.
@@ -4165,43 +4176,27 @@ ShellFileHandleReadLine(
 //
 // if we have space save it...
 //
-if ((CountSoFar + 1) * CharSize < *Size){
+if ((CountSoFar+1)*sizeof(CHAR16) < *Size){
   ASSERT(Buffer != NULL);
-  if (*Ascii) {
-((CHAR8*)Buffer)[CountSoFar] = (CHAR8) CharBuffer;
-((CHAR8*)Buffer)[CountSoFar+1] = '\0';
-  }
-  else {
-((CHAR16*)Buffer)[CountSoFar] = CharBuffer;
-((CHAR16*)Buffer)[CountSoFar+1] = CHAR_NULL;
-  }
+  ((CHAR16*)Buffer)[CountSoFar] = CharBuffer;
+  ((CHAR16*)Buffer)[CountSoFar+1] = CHAR_NULL;
 }
   }
 
   //
   // if we ran out of space tell when...
   //
-  if (Status != EFI_END_OF_FILE){
-if ((CountSoFar + 1) * CharSize > *Size){
-  *Size = (CountSoFar + 1) * CharSize;
-  if (!Truncate) {
-gEfiShellProtocol->SetFilePosition(Handle, OriginalFilePosition);
-  } else {
-DEBUG((DEBUG_WARN, "The line was truncated in 
ShellFileHandleReadLine"));
-  }
-  return (EFI_BUFFER_TOO_SMALL);
-}
-
-if (*Ascii) {
-  if (CountSoFar && ((CHAR8*)Buffer)[CountSoFar - 1] == '\r') {
-((CHAR8*)Buffer)[CountSoFar - 1] = '\0';
-  }
-}
-else {
-  if (CountSoFar && Buffer[CountSoFar - 1] == L'\r') {
-Buffer[CountSoFar - 1] = CHAR_NULL;
-  }
+  if ((CountSoFar+1)*sizeof(CHAR16) > *Size){
+*Size = (CountSoFar+1)*sizeof(CHAR16);
+if (!Truncate) {
+  gEfiShellProtocol->SetFilePosition(Handle, OriginalFilePosition);
+} else {
+  DEBUG((DEBUG_WARN, "The line was truncated in ShellFileHandleReadLine"));
 }
+return (EFI_BUFFER_TOO_SMALL);
+  }
+  while(Buffer[StrLen(Buffer)-1] == L'\r') {
+Buffer[StrLen(Buffer)-1] = CHAR_NULL;
   }
 
   return (Status);

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH] Ax88772b: Fixing compilation error variable set but not used

2016-02-10 Thread Shivamurthy Shastri
error: pNicDevice variable set but not used

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Shivamurthy Shastri 
---
 OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/SimpleNetwork.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/SimpleNetwork.c 
b/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/SimpleNetwork.c
index 9eeb61f..c061a6b 100644
--- a/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/SimpleNetwork.c
+++ b/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/SimpleNetwork.c
@@ -700,10 +700,8 @@ SN_ReceiveFilters (
   EFI_SIMPLE_NETWORK_MODE * pMode;
   EFI_STATUS Status = EFI_SUCCESS;   
   EFI_TPL TplPrevious; 
-  NIC_DEVICE * pNicDevice;
 
   TplPrevious = gBS->RaiseTPL(TPL_CALLBACK);
-  pNicDevice = DEV_FROM_SIMPLE_NETWORK ( pSimpleNetwork );
   pMode = pSimpleNetwork->Mode;
 
   if (pSimpleNetwork == NULL) {
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH] Ax88772b: support for multiple dongles and chips

2016-02-10 Thread Shivamurthy Shastri
Driver code is modified to support multiple ethernet dongles, which uses
similar ASIX chips. Also, it can be used for multiple ASIX chips with
similar register map.

Enabled support for Apple Ethernet Adapter

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Shivamurthy Shastri 
---
 .../Bus/Usb/UsbNetworking/Ax88772b/Ax88772.h   | 13 -
 .../Bus/Usb/UsbNetworking/Ax88772b/DriverBinding.c | 33 +-
 2 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/Ax88772.h 
b/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/Ax88772.h
index ab75ec2..816f2b2 100644
--- a/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/Ax88772.h
+++ b/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/Ax88772.h
@@ -297,12 +297,23 @@
 #define AN_10_HDX   0x0020  ///<  1 = 10BASE-T support
 #define AN_CSMA_CD  0x0001  ///<  1 = IEEE 802.3 CSMA/CD 
support
 
-
+/* asix_flags defines */
+#define FLAG_NONE   0
+#define FLAG_TYPE_AX88172   (1U << 0)
+#define FLAG_TYPE_AX88772   (1U << 1)
+#define FLAG_TYPE_AX88772B  (1U << 2)
+#define FLAG_EEPROM_MAC (1U << 3) /* initial mac address in eeprom */
 
 
//--
 //  Data Types
 
//--
 
+struct ASIX_DONGLE {
+   UINT16  VendorId;
+   UINT16  ProductId;
+   INT32   Flags;
+};
+
 /**
   Ethernet header layout
 
diff --git a/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/DriverBinding.c 
b/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/DriverBinding.c
index 3b73040..6a10cbf 100644
--- a/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/DriverBinding.c
+++ b/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/DriverBinding.c
@@ -14,6 +14,13 @@
 
 #include "Ax88772.h"
 
+struct ASIX_DONGLE ASIX_DONGLES[] = {
+{ 0x05AC, 0x1402, FLAG_TYPE_AX88772 }, /* Apple USB Ethernet Adapter */
+/* ASIX 88772B */
+{ 0x0B95, 0x772B, FLAG_TYPE_AX88772B | FLAG_EEPROM_MAC },
+{ 0x, 0x, FLAG_NONE }   /* END - Do not remove */
+};
+
 /**
   Verify the controller type
 
@@ -36,6 +43,8 @@ DriverSupported (
   EFI_USB_DEVICE_DESCRIPTOR Device;
   EFI_USB_IO_PROTOCOL * pUsbIo;
   EFI_STATUS Status;
+  UINT32 i;
+
   //
   //  Connect to the USB stack
   //
@@ -60,19 +69,17 @@ DriverSupported (
 else {
   //
   //  Validate the adapter
-  //   
-  if ( VENDOR_ID == Device.IdVendor ) {
-
-if (PRODUCT_ID == Device.IdProduct) {
-DEBUG ((EFI_D_INFO, "Found the AX88772B\r\n"));
-}
-   else  {
-Status = EFI_UNSUPPORTED;
-   }
-  }
-   else {
- Status = EFI_UNSUPPORTED;
-   }   
+  //
+  for (i = 0; ASIX_DONGLES[i].VendorId != 0; i++) {
+   if (ASIX_DONGLES[i].VendorId == Device.IdVendor &&
+   ASIX_DONGLES[i].ProductId == Device.IdProduct) {
+ DEBUG ((EFI_D_INFO, "Found the AX88772B\r\n"));
+ break;
+   }
+   }
+
+   if (ASIX_DONGLES[i].VendorId == 0)
+   Status = EFI_UNSUPPORTED;
 }

 //
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH] Ax88772b: Workaround access to RXQTC register with Apple Ethernet Adapter

2016-02-10 Thread Shivamurthy Shastri
The USB command CMD_RXQTC ("RX Queue Cascade Threshold Control") tries
to access the register and is always failing when using the Apple
Ethernet adapter.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Shivamurthy Shastri 
---
 .../Bus/Usb/UsbNetworking/Ax88772b/Ax88772.c   | 15 +
 .../Bus/Usb/UsbNetworking/Ax88772b/Ax88772.h   |  2 ++
 .../Bus/Usb/UsbNetworking/Ax88772b/DriverBinding.c | 39 ++
 3 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/Ax88772.c 
b/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/Ax88772.c
index 45ba3e5..b3fefb1 100644
--- a/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/Ax88772.c
+++ b/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/Ax88772.c
@@ -625,15 +625,18 @@ Ax88772Reset (
   
   if (EFI_ERROR(Status)) goto err;  
 
-  SetupMsg.RequestType = USB_REQ_TYPE_VENDOR
+  if (pNicDevice->Flags != FLAG_TYPE_AX88772) {
+   SetupMsg.RequestType = USB_REQ_TYPE_VENDOR
 | USB_TARGET_DEVICE; 
-  SetupMsg.Request = CMD_RXQTC;
-  SetupMsg.Value = 0x8000;
-  SetupMsg.Index = 0x8001;
-  SetupMsg.Length = 0;
-  Status = Ax88772UsbCommand ( pNicDevice,
+   SetupMsg.Request = CMD_RXQTC;
+   SetupMsg.Value = 0x8000;
+   SetupMsg.Index = 0x8001;
+   SetupMsg.Length = 0;
+   Status = Ax88772UsbCommand ( pNicDevice,
   &SetupMsg,
   NULL ); 
+  }
+
 err:
   return Status;
 }
diff --git a/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/Ax88772.h 
b/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/Ax88772.h
index 816f2b2..a85c998 100644
--- a/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/Ax88772.h
+++ b/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/Ax88772.h
@@ -407,6 +407,8 @@ typedef struct {
   RX_PKT * pFirstFill;
   UINTN   PktCntInQueue;
   UINT8 * pBulkInBuff;
+
+  INT32 Flags;
  
 } NIC_DEVICE;
 
diff --git a/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/DriverBinding.c 
b/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/DriverBinding.c
index 6a10cbf..46a2ef9 100644
--- a/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/DriverBinding.c
+++ b/OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/DriverBinding.c
@@ -124,6 +124,8 @@ DriverStart (
UINTN   LengthInBytes;
EFI_DEVICE_PATH_PROTOCOL*ParentDevicePath = NULL;
MAC_ADDR_DEVICE_PATHMacDeviceNode;
+   EFI_USB_DEVICE_DESCRIPTOR   Device;
+   UINT32 i;
 
   //
//  Allocate the device structure
@@ -178,6 +180,43 @@ DriverStart (
  goto EXIT;
   }
 
+Status = pNicDevice->pUsbIo->UsbGetDeviceDescriptor ( pNicDevice->pUsbIo, 
&Device );
+if (EFI_ERROR ( Status )) {
+gBS->CloseProtocol (
+Controller,
+&gEfiUsbIoProtocolGuid,
+pThis->DriverBindingHandle,
+Controller
+);
+  gBS->FreePool ( pNicDevice );
+ goto EXIT;
+}
+else {
+  //
+  //  Validate the adapter
+  //
+  for (i = 0; ASIX_DONGLES[i].VendorId != 0; i++) {
+   if (ASIX_DONGLES[i].VendorId == Device.IdVendor &&
+   ASIX_DONGLES[i].ProductId == Device.IdProduct) {
+ break;
+   }
+   }
+
+   if (ASIX_DONGLES[i].VendorId == 0) {
+gBS->CloseProtocol (
+Controller,
+&gEfiUsbIoProtocolGuid,
+pThis->DriverBindingHandle,
+Controller
+);
+  gBS->FreePool ( pNicDevice );
+ goto EXIT;
+   }
+
+   pNicDevice->Flags = ASIX_DONGLES[i].Flags;
+}
+
+
//
   // Set Device Path
   //   
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] ShellPkg: ShellFileHandleReadLine must return UCS2 lines

2016-02-10 Thread Ryan Harkin
Hi Jim,

Thanks for the quick fix!  It works for me on ARM Juno R0, R1 and R2
and Versatile Express TC2.

On 10 February 2016 at 13:45,   wrote:
> ShellPkg: ShellFileHandleReadLine must return UCS2 lines.
>
> An earlier change had this function returning the type of lines that were in 
> the file being read (ASCII or UCS2).  The way it is used, UCS2 output is 
> expected, even when the file being read is ASCII. This change restores that 
> behavior and documents it.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jim Dailey
^^ your email address is missing from your sign-off, I think it's
mandatory, but don't hold me to that.

I haven't had chance to read and understand the code, but I can at
least provide:

Tested-by: Ryan Harkin 

Thanks,
Ryan.

> ---
> diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c 
> b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> index 4b53c70..abff0d3 100644
> --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> @@ -4084,9 +4084,20 @@ ShellFileHandleReturnLine(
>If the position upon start is 0, then the Ascii Boolean will be set.  This 
> should be
>maintained and not changed for all operations with the same file.
>
> +  NOTE: LINES THAT ARE RETURNED BY THIS FUNCTION ARE UCS2, EVEN IF THE FILE 
> BEING READ
> +IS IN ASCII FORMAT.
> +
>@param[in]   HandleSHELL_FILE_HANDLE to read from.
> -  @param[in, out]  BufferThe pointer to buffer to read into.
> -  @param[in, out]  Size  The pointer to number of bytes in Buffer.
> +  @param[in, out]  BufferThe pointer to buffer to read into. If this 
> function
> + returns EFI_SUCCESS, then on output Buffer 
> will
> + contain a UCS2 string, even if the file 
> being
> + read is ASCII.
> +  @param[in, out]  Size  On input, pointer to number of bytes in 
> Buffer.
> + On output, unchanged unless Buffer is too 
> small
> + to contain the next line of the file. In 
> that
> + case Size is set to the number of bytes 
> needed
> + to hold the next line of the file (as a UCS2
> + string, even if it is an ASCII file).
>@param[in]   Truncate  If the buffer is large enough, this has no 
> effect.
>   If the buffer is is too small and Truncate 
> is TRUE,
>   the line will be truncated.
> @@ -4165,43 +4176,27 @@ ShellFileHandleReadLine(
>  //
>  // if we have space save it...
>  //
> -if ((CountSoFar + 1) * CharSize < *Size){
> +if ((CountSoFar+1)*sizeof(CHAR16) < *Size){
>ASSERT(Buffer != NULL);
> -  if (*Ascii) {
> -((CHAR8*)Buffer)[CountSoFar] = (CHAR8) CharBuffer;
> -((CHAR8*)Buffer)[CountSoFar+1] = '\0';
> -  }
> -  else {
> -((CHAR16*)Buffer)[CountSoFar] = CharBuffer;
> -((CHAR16*)Buffer)[CountSoFar+1] = CHAR_NULL;
> -  }
> +  ((CHAR16*)Buffer)[CountSoFar] = CharBuffer;
> +  ((CHAR16*)Buffer)[CountSoFar+1] = CHAR_NULL;
>  }
>}
>
>//
>// if we ran out of space tell when...
>//
> -  if (Status != EFI_END_OF_FILE){
> -if ((CountSoFar + 1) * CharSize > *Size){
> -  *Size = (CountSoFar + 1) * CharSize;
> -  if (!Truncate) {
> -gEfiShellProtocol->SetFilePosition(Handle, OriginalFilePosition);
> -  } else {
> -DEBUG((DEBUG_WARN, "The line was truncated in 
> ShellFileHandleReadLine"));
> -  }
> -  return (EFI_BUFFER_TOO_SMALL);
> -}
> -
> -if (*Ascii) {
> -  if (CountSoFar && ((CHAR8*)Buffer)[CountSoFar - 1] == '\r') {
> -((CHAR8*)Buffer)[CountSoFar - 1] = '\0';
> -  }
> -}
> -else {
> -  if (CountSoFar && Buffer[CountSoFar - 1] == L'\r') {
> -Buffer[CountSoFar - 1] = CHAR_NULL;
> -  }
> +  if ((CountSoFar+1)*sizeof(CHAR16) > *Size){
> +*Size = (CountSoFar+1)*sizeof(CHAR16);
> +if (!Truncate) {
> +  gEfiShellProtocol->SetFilePosition(Handle, OriginalFilePosition);
> +} else {
> +  DEBUG((DEBUG_WARN, "The line was truncated in 
> ShellFileHandleReadLine"));
>  }
> +return (EFI_BUFFER_TOO_SMALL);
> +  }
> +  while(Buffer[StrLen(Buffer)-1] == L'\r') {
> +Buffer[StrLen(Buffer)-1] = CHAR_NULL;
>}
>
>return (Status);
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v4 0/3] support multiple PL061 gpio controllers

2016-02-10 Thread Ryan Harkin
Ping.  This disappeared down a black hole.

On 19 August 2015 at 08:56, Haojian Zhuang  wrote:
> Changelog:
>   v4:
> * Use 64-bit value on PL061 register base address.
> * Use fallback to be compatible with current PcdPL061GpioBase value
>   when platform gpio driver isn't present.
> * Remove the dependancy on PL061. Move the dependancy to platform
>   gpio driver instead.
>   v3:
> * Remove GPIO_PIN_MASK_HIGH_8BIT() and GPIO_PIN_MASK_LOW_8BIT().
> * Avoid to use MmioAnd8() on updating GPIO DATA register, since PL061
>   could access each bit by specified register offset.
> * Add PLATFORM_GPIO_CONTROLLER structure in embedded gpio.
> * Support multiple PL061 gpio controllers in one platform.
>   v2:
> * Append the patch to fix gpio pin mask macro.
>
> Haojian Zhuang (3):
>   ArmPlatformPkg: PL061: fix accessing GPIO DATA
>   EmbeddedPkg: enhance for multiple gpio controllers
>   ArmPlatformPkg: PL061: support multiple controller
>
>  ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c| 137 
> +++--
>  .../Drivers/PL061GpioDxe/PL061GpioDxe.inf  |   1 +
>  ArmPlatformPkg/Include/Drivers/PL061Gpio.h |  51 
>  EmbeddedPkg/EmbeddedPkg.dec|   1 +
>  EmbeddedPkg/Include/Protocol/EmbeddedGpio.h|  17 +++
>  5 files changed, 143 insertions(+), 64 deletions(-)
>
> --
> 2.1.4
>

I found this patch series sitting in my "review" folder.  I have some
really minor comments, but I'll reply to the individual patches.

In the meantime, I tested this on Juno which also has PL061 and it works fine.

So for the whole series:

Tested-by: Ryan Harkin 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v4 1/3] ArmPlatformPkg: PL061: fix accessing GPIO DATA

2016-02-10 Thread Ryan Harkin
On 19 August 2015 at 08:56, Haojian Zhuang  wrote:
> // All bits low except one bit high, restricted to 8 bits
> // (i.e. ensures zeros above 8bits)
>
> But '&&' is wrong at here. It'll only return 1 or 0 as the result
> of GPIO_PIN_MASK_HIGH_8BIT(Pin).
>
> Since PL061 spec said in below.
>
> In order to write to GPIODATA, the corresponding bits in the mask,
> resulting from the address bus, PADDR[9:2], must be HIGH. Otherwise
> the bit values remain unchanged by the write.
> Similarly, the values read from this register are determined for
> each bit, by the mask bit derived from the address used to access
> the data register, PADDR[9:2]. Bits that are 1 in the address mask
> cause the corresponding bits in GPIODATA to be read, and bits that
> are 0 in the address mask cause the corresponding bits in GPIODATA
> to be read as 0, regardless of their value.
>
> Now simplify the way to read/write bit of GPIO_DATA register.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Haojian Zhuang 
> ---
>  ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 16 
>  ArmPlatformPkg/Include/Drivers/PL061Gpio.h  |  4 
>  2 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c 
> b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
> index ff05662..042fc76 100644
> --- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
> +++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
> @@ -125,7 +125,7 @@ Get (
>  }
>}
>
> -  if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
> +  if (MmioRead8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2))) {

Shouldn't this be and OR "|", not a plus "+"?

I realise that they probably achieve the same thing here, but using a
"+" for masking is unusual.

The same comment applied to the other changes below:

>  *Value = 1;
>} else {
>  *Value = 0;
> @@ -181,21 +181,21 @@ Set (
>{
>  case GPIO_MODE_INPUT:
>// Set the corresponding direction bit to LOW for input
> -  MmioAnd8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_LOW_8BIT(Gpio));
> +  MmioAnd8 (PL061_GPIO_DIR_REG, ~GPIO_PIN_MASK(Gpio));
>break;
>
>  case GPIO_MODE_OUTPUT_0:
>// Set the corresponding data bit to LOW for 0
> -  MmioAnd8 (PL061_GPIO_DATA_REG, GPIO_PIN_MASK_LOW_8BIT(Gpio));
> +  MmioWrite8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2), 0);
>// Set the corresponding direction bit to HIGH for output
> -  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
> +  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio));
>break;
>
>  case GPIO_MODE_OUTPUT_1:
>// Set the corresponding data bit to HIGH for 1
> -  MmioOr8 (PL061_GPIO_DATA_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
> +  MmioWrite8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2), 0xff);
>// Set the corresponding direction bit to HIGH for output
> -  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
> +  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio));
>break;
>
>  default:
> @@ -250,9 +250,9 @@ GetMode (
>}
>
>// Check if it is input or output
> -  if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
> +  if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK(Gpio)) {
>  // Pin set to output
> -if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
> +if (MmioRead8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2))) {
>*Mode = GPIO_MODE_OUTPUT_1;
>  } else {
>*Mode = GPIO_MODE_OUTPUT_0;
> diff --git a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h 
> b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h
> index 38458f4..d436fd4 100644
> --- a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h
> +++ b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h
> @@ -46,9 +46,5 @@
>
>  // All bits low except one bit high, native bit length
>  #define GPIO_PIN_MASK(Pin)  (1UL << ((UINTN)(Pin)))
> -// All bits low except one bit high, restricted to 8 bits (i.e. ensures 
> zeros above 8bits)
> -#define GPIO_PIN_MASK_HIGH_8BIT(Pin)(GPIO_PIN_MASK(Pin) && 0xFF)
> -// All bits high except one bit low, restricted to 8 bits (i.e. ensures 
> zeros above 8bits)
> -#define GPIO_PIN_MASK_LOW_8BIT(Pin) ((~GPIO_PIN_MASK(Pin)) && 0xFF)
>
>  #endif  // __PL061_GPIO_H__
> --
> 2.1.4
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v4 1/3] ArmPlatformPkg: PL061: fix accessing GPIO DATA

2016-02-10 Thread Laszlo Ersek
On 02/10/16 16:00, Ryan Harkin wrote:
> On 19 August 2015 at 08:56, Haojian Zhuang  wrote:
>> // All bits low except one bit high, restricted to 8 bits
>> // (i.e. ensures zeros above 8bits)
>>
>> But '&&' is wrong at here. It'll only return 1 or 0 as the result
>> of GPIO_PIN_MASK_HIGH_8BIT(Pin).
>>
>> Since PL061 spec said in below.
>>
>> In order to write to GPIODATA, the corresponding bits in the mask,
>> resulting from the address bus, PADDR[9:2], must be HIGH. Otherwise
>> the bit values remain unchanged by the write.
>> Similarly, the values read from this register are determined for
>> each bit, by the mask bit derived from the address used to access
>> the data register, PADDR[9:2]. Bits that are 1 in the address mask
>> cause the corresponding bits in GPIODATA to be read, and bits that
>> are 0 in the address mask cause the corresponding bits in GPIODATA
>> to be read as 0, regardless of their value.
>>
>> Now simplify the way to read/write bit of GPIO_DATA register.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Haojian Zhuang 
>> ---
>>  ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 16 
>>  ArmPlatformPkg/Include/Drivers/PL061Gpio.h  |  4 
>>  2 files changed, 8 insertions(+), 12 deletions(-)
>>
>> diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c 
>> b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
>> index ff05662..042fc76 100644
>> --- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
>> +++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
>> @@ -125,7 +125,7 @@ Get (
>>  }
>>}
>>
>> -  if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
>> +  if (MmioRead8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2))) {
> 
> Shouldn't this be and OR "|", not a plus "+"?

Not knowing anything about what's going on: I don't think so. The above
does not manipulate a register value, it calculates a register offset.

> I realise that they probably achieve the same thing here, but using a
> "+" for masking is unusual.

It's not (value) masking, it's offset calculation.

(Whether the offset calculation is *correct* is of course over my head.)

> 
> The same comment applied to the other changes below:

Those changes are different:

>>  *Value = 1;
>>} else {
>>  *Value = 0;
>> @@ -181,21 +181,21 @@ Set (
>>{
>>  case GPIO_MODE_INPUT:
>>// Set the corresponding direction bit to LOW for input
>> -  MmioAnd8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_LOW_8BIT(Gpio));
>> +  MmioAnd8 (PL061_GPIO_DIR_REG, ~GPIO_PIN_MASK(Gpio));

MmioAnd8() reads the register at the given offset, masks the value read,
and then writes back the result.

>>break;
>>
>>  case GPIO_MODE_OUTPUT_0:
>>// Set the corresponding data bit to LOW for 0
>> -  MmioAnd8 (PL061_GPIO_DATA_REG, GPIO_PIN_MASK_LOW_8BIT(Gpio));
>> +  MmioWrite8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2), 0);

This replaces the read-mask-write triplet (using a constant offset) with
a simple write (to a dynamically calculated offset).


>>// Set the corresponding direction bit to HIGH for output
>> -  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
>> +  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio));
>>break;

read-set-write, only the or-mask changes

>>
>>  case GPIO_MODE_OUTPUT_1:
>>// Set the corresponding data bit to HIGH for 1
>> -  MmioOr8 (PL061_GPIO_DATA_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
>> +  MmioWrite8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2), 0xff);

replaces read-set-write to a constant register offset with a write to a
dynamically calculated offset

etc

Thanks
Laszlo

>>// Set the corresponding direction bit to HIGH for output
>> -  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
>> +  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio));
>>break;
>>
>>  default:
>> @@ -250,9 +250,9 @@ GetMode (
>>}
>>
>>// Check if it is input or output
>> -  if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
>> +  if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK(Gpio)) {
>>  // Pin set to output
>> -if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
>> +if (MmioRead8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2))) {
>>*Mode = GPIO_MODE_OUTPUT_1;
>>  } else {
>>*Mode = GPIO_MODE_OUTPUT_0;
>> diff --git a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h 
>> b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h
>> index 38458f4..d436fd4 100644
>> --- a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h
>> +++ b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h
>> @@ -46,9 +46,5 @@
>>
>>  // All bits low except one bit high, native bit length
>>  #define GPIO_PIN_MASK(Pin)  (1UL << ((UINTN)(Pin)))
>> -// All bits low except one bit high, restricted to 8 bits (i.e. ensures 
>> zeros above 8bits)
>> -#define GPIO_PIN_MASK_HIGH_8BIT(Pin)(GPI

Re: [edk2] [PATCH v4 3/3] ArmPlatformPkg: PL061: support multiple controller

2016-02-10 Thread Ryan Harkin
On 19 August 2015 at 08:56, Haojian Zhuang  wrote:
> Support multiple PL061 controllers. If platform gpio driver couldn't be found,
> PL061 gpio driver will continue to load PcdPL061GpioBase as the register base.
> It could be compatible with the use case of current PL061 gpio driver.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Haojian Zhuang 
> ---
>  ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c| 137 
> +++--
>  .../Drivers/PL061GpioDxe/PL061GpioDxe.inf  |   1 +
>  ArmPlatformPkg/Include/Drivers/PL061Gpio.h |  47 ---
>  3 files changed, 125 insertions(+), 60 deletions(-)
>
> diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c 
> b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
> index 042fc76..02da8e1 100644
> --- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
> +++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
> @@ -19,6 +19,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -28,6 +29,7 @@
>  #include 
>
>  BOOLEAN mPL061Initialized = FALSE;
> +PLATFORM_GPIO_CONTROLLER *mPL061PlatformGpio;
>
>  /**
>Function implementations
> @@ -38,20 +40,36 @@ PL061Identify (
>VOID
>)
>  {
> -  // Check if this is a PrimeCell Peripheral
> -  if ((MmioRead8 (PL061_GPIO_PCELL_ID0) != 0x0D)
> -  ||  (MmioRead8 (PL061_GPIO_PCELL_ID1) != 0xF0)
> -  ||  (MmioRead8 (PL061_GPIO_PCELL_ID2) != 0x05)
> -  ||  (MmioRead8 (PL061_GPIO_PCELL_ID3) != 0xB1)) {
> -return EFI_NOT_FOUND;
> +  UINTNIndex;
> +  UINTNRegisterBase;
> +
> +  if (   (mPL061PlatformGpio->GpioCount == 0)
> +  || (mPL061PlatformGpio->GpioControllerCount == 0)) {
> + return EFI_NOT_FOUND;
>}
>
> -  // Check if this PrimeCell Peripheral is the PL061 GPIO
> -  if ((MmioRead8 (PL061_GPIO_PERIPH_ID0) != 0x61)
> -  ||  (MmioRead8 (PL061_GPIO_PERIPH_ID1) != 0x10)
> -  ||  ((MmioRead8 (PL061_GPIO_PERIPH_ID2) & 0xF) != 0x04)
> -  ||  (MmioRead8 (PL061_GPIO_PERIPH_ID3) != 0x00)) {
> -return EFI_NOT_FOUND;
> +  for (Index = 0; Index < mPL061PlatformGpio->GpioControllerCount; Index++) {
> +if (mPL061PlatformGpio->GpioController[Index].InternalGpioCount != 
> PL061_GPIO_PINS) {
> +  return EFI_INVALID_PARAMETER;
> +}
> +
> +RegisterBase = mPL061PlatformGpio->GpioController[Index].RegisterBase;
> +
> +// Check if this is a PrimeCell Peripheral
> +if ((MmioRead8 (RegisterBase + PL061_GPIO_PCELL_ID0) != 0x0D)
> +||  (MmioRead8 (RegisterBase + PL061_GPIO_PCELL_ID1) != 0xF0)
> +||  (MmioRead8 (RegisterBase + PL061_GPIO_PCELL_ID2) != 0x05)
> +||  (MmioRead8 (RegisterBase + PL061_GPIO_PCELL_ID3) != 0xB1)) {
> +  return EFI_NOT_FOUND;
> +}
> +
> +// Check if this PrimeCell Peripheral is the PL061 GPIO
> +if ((MmioRead8 (RegisterBase + PL061_GPIO_PERIPH_ID0) != 0x61)
> +||  (MmioRead8 (RegisterBase + PL061_GPIO_PERIPH_ID1) != 0x10)
> +||  ((MmioRead8 (RegisterBase + PL061_GPIO_PERIPH_ID2) & 0xF) != 
> 0x04)
> +||  (MmioRead8 (RegisterBase + PL061_GPIO_PERIPH_ID3) != 0x00)) {
> +  return EFI_NOT_FOUND;
> +}
>}
>
>return EFI_SUCCESS;
> @@ -84,6 +102,31 @@ PL061Initialize (
>return Status;
>  }
>
> +EFI_STATUS
> +EFIAPI
> +PL061Locate (
> +  IN  EMBEDDED_GPIO_PIN Gpio,
> +  OUT UINTN *ControllerIndex,
> +  OUT UINTN *ControllerOffset,
> +  OUT UINTN *RegisterBase
> +  )
> +{
> +  UINT32Index;
> +
> +  for (Index = 0; Index < mPL061PlatformGpio->GpioControllerCount; Index++) {
> +if ((Gpio >= mPL061PlatformGpio->GpioController[Index].GpioIndex)
> +&&  (Gpio < mPL061PlatformGpio->GpioController[Index].GpioIndex
> + + mPL061PlatformGpio->GpioController[Index].InternalGpioCount)) 
> {
> +  *ControllerIndex = Index;
> +  *ControllerOffset = Gpio % 
> mPL061PlatformGpio->GpioController[Index].InternalGpioCount;
> +  *RegisterBase = mPL061PlatformGpio->GpioController[Index].RegisterBase;
> +  return EFI_SUCCESS;
> +}
> +  }
> +  DEBUG ((EFI_D_ERROR, "%a, failed to locate gpio %d\n", __func__, Gpio));
> +  return EFI_INVALID_PARAMETER;
> +}
> +
>  /**
>
>  Routine Description:
> @@ -110,11 +153,15 @@ Get (
>)
>  {
>EFI_STATUSStatus = EFI_SUCCESS;
> +  UINTN Index, Offset, RegisterBase;
>
> -  if ((Value == NULL)
> -  ||  (Gpio > LAST_GPIO_PIN))
> -  {
> -return EFI_INVALID_PARAMETER;
> +  Status = PL061Locate (Gpio, &Index, &Offset, &RegisterBase);
> +  if (EFI_ERROR (Status))
> +goto EXIT;
> +
> +  if (Value == NULL) {
> +Status = EFI_INVALID_PARAMETER;
> +goto EXIT;
>}
>
>// Initialize the hardware if not already done
> @@ -125,7 +172,7 @@ Get (
>  }
>}
>
> -  if (MmioRead8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2))) {
> +  if (MmioRead8 (RegisterBase + PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Offset) 
> << 2))) {

A

[edk2] [PATCH 2/2] ArmPlatformPkg/ArmJunoPkg: only have 1 PCD for the FDT

2016-02-10 Thread Ryan Harkin
Juno doesn't have lots of DTB files in NOR flash, it only has 1 file,
called "board.dtb" and the motherboard configuration makes the right
choice about which DTB file gets written as board.dtb in NOR.

The code attempts to select which DTB it should use based on the board
variant or configuration.  And this doesn't work because those DTB files
aren't present in NOR flash.

So remove the DTB variants and only load board.dtb.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ryan Harkin 
---
 ArmPlatformPkg/ArmJunoPkg/ArmJuno.dec  |  4 +---
 .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf   |  4 +---
 ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c  | 14 +-
 3 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/ArmPlatformPkg/ArmJunoPkg/ArmJuno.dec 
b/ArmPlatformPkg/ArmJunoPkg/ArmJuno.dec
index 040a906..7af8885 100644
--- a/ArmPlatformPkg/ArmJunoPkg/ArmJuno.dec
+++ b/ArmPlatformPkg/ArmJunoPkg/ArmJuno.dec
@@ -44,6 +44,4 @@ [PcdsFixedAtBuild.common]
   
gArmJunoTokenSpaceGuid.PcdSynopsysUsbEhciBaseAddress|0x7FFC|UINT32|0x0005
 
   # Juno Device Trees are loaded from NOR Flash
-  
gArmJunoTokenSpaceGuid.PcdJunoR0FdtDevicePath|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/juno.dtb"|VOID*|0x0006
-  
gArmJunoTokenSpaceGuid.PcdJunoR1A57x2FdtDevicePath|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/r1a57.dtb"|VOID*|0x0007
-  
gArmJunoTokenSpaceGuid.PcdJunoR1A57x2A53x4FdtDevicePath|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/r1a57a53.dtb"|VOID*|0x0008
+  
gArmJunoTokenSpaceGuid.PcdJunoFdtDevicePath|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/board.dtb"|VOID*|0x0008
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf 
b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
index d1f2f7b..6ab81e8 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
@@ -73,9 +73,7 @@ [FixedPcd]
   gArmJunoTokenSpaceGuid.PcdSynopsysUsbEhciBaseAddress
   gArmJunoTokenSpaceGuid.PcdSynopsysUsbOhciBaseAddress
 
-  gArmJunoTokenSpaceGuid.PcdJunoR0FdtDevicePath
-  gArmJunoTokenSpaceGuid.PcdJunoR1A57x2FdtDevicePath
-  gArmJunoTokenSpaceGuid.PcdJunoR1A57x2A53x4FdtDevicePath
+  gArmJunoTokenSpaceGuid.PcdJunoFdtDevicePath
 
   gArmPlatformTokenSpaceGuid.PcdDefaultBootDevicePath
   gArmPlatformTokenSpaceGuid.PcdDefaultBootArgument
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c 
b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
index 61b2401..40d341e 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
@@ -244,19 +244,7 @@ ArmJunoEntryPoint (
   //
   // Set up the device path to the FDT.
   //
-  switch (JunoRevision) {
-  case JUNO_REVISION_R0:
-TextDevicePath = (CHAR16*)FixedPcdGetPtr (PcdJunoR0FdtDevicePath);
-break;
-
-  case JUNO_REVISION_R1:
-TextDevicePath = (CHAR16*)FixedPcdGetPtr (PcdJunoR1A57x2FdtDevicePath);
-break;
-
-  default:
-TextDevicePath = NULL;
-  }
-
+  TextDevicePath = (CHAR16*)FixedPcdGetPtr (PcdJunoFdtDevicePath);
   if (TextDevicePath != NULL) {
 TextDevicePathSize = StrSize (TextDevicePath);
 Buffer = PcdSetPtr (PcdFdtDevicePaths, &TextDevicePathSize, 
TextDevicePath);
-- 
2.1.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH 0/2] ArmPlatformPkg/ArmJunoPkg: boot and FDT cleanup

2016-02-10 Thread Ryan Harkin
This small series removes a fair amount of broken code used to create
default boot entries that don't work and install DT blobs depending upon
that variant of the board this the code is running on, without
supporting all the board variants now available.

[PATCH 1/2] ArmPlatformPkg/ArmJunoPkg: don't create default boot entries
[PATCH 2/2] ArmPlatformPkg/ArmJunoPkg: only have 1 PCD for the FDT

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH 1/2] ArmPlatformPkg/ArmJunoPkg: don't create default boot entries

2016-02-10 Thread Ryan Harkin
Code was inserted to create default boot entries for Juno R1.  These
don't work, but they are also preventing the board from booting into the
default options that Intel BDS would otherwise boot.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ryan Harkin 
---
 .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 248 -
 1 file changed, 248 deletions(-)

diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c 
b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
index f4e6d35..61b2401 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
@@ -37,13 +37,6 @@
 //
 #define SKY2_MAC_ADDRESS_BOOTARG_LEN  47
 
-//
-// Function prototypes
-//
-STATIC EFI_STATUS SetJunoR1DefaultBootEntries (
-  VOID
-  );
-
 // This GUID must match the FILE_GUID in 
ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiTables.inf
 STATIC CONST EFI_GUID mJunoAcpiTableFile = { 0xa1dd808e, 0x1e95, 0x4399, { 
0xab, 0xc0, 0x65, 0x3c, 0x82, 0xe8, 0x53, 0x0c } };
 
@@ -82,75 +75,6 @@ STATIC CONST EFI_PCI_ROOT_BRIDGE_DEVICE_PATH 
mPciRootComplexDevicePath = {
 EFI_EVENT mAcpiRegistration = NULL;
 
 /**
- * Build and Set UEFI Variable Boot
- *
- * @param BootVariableName   Name of the UEFI Variable
- * @param Attributes 'Attributes' for the Boot variable as per 
UEFI spec
- * @param BootDescriptionDescription of the Boot variable
- * @param DevicePath EFI Device Path of the EFI Application to boot
- * @param OptionalData   Parameters to pass to the EFI application
- * @param OptionalDataSize   Size of the parameters to pass to the EFI 
application
- *
- * @return EFI_OUT_OF_RESOURCES  A memory allocation failed
- * @return   Return value of RT.SetVariable
- */
-STATIC
-EFI_STATUS
-BootOptionCreate (
-  IN  CHAR16BootVariableName[9],
-  IN  UINT32Attributes,
-  IN  CHAR16*   BootDescription,
-  IN  EFI_DEVICE_PATH_PROTOCOL* DevicePath,
-  IN  UINT8*OptionalData,
-  IN  UINTN OptionalDataSize
-  )
-{
-  UINTN VariableSize;
-  UINT8 *Variable;
-  UINT8 *VariablePtr;
-  UINTN FilePathListLength;
-  UINTN BootDescriptionSize;
-
-  FilePathListLength  = GetDevicePathSize (DevicePath);
-  BootDescriptionSize = StrSize (BootDescription);
-
-  // Each Boot variable is built as follow:
-  //   UINT32   Attributes
-  //   UINT16   FilePathListLength
-  //   CHAR16*  Description
-  //   EFI_DEVICE_PATH_PROTOCOL FilePathList[]
-  //   UINT8OptionalData[]
-  VariableSize = sizeof (UINT32) + sizeof (UINT16) +
-  BootDescriptionSize + FilePathListLength + OptionalDataSize;
-  Variable = AllocateZeroPool (VariableSize);
-  if (Variable == NULL) {
-return EFI_OUT_OF_RESOURCES;
-  }
-
-  // 'Attributes' field
-  *(UINT32*)Variable = Attributes;
-  // 'FilePathListLength' field
-  VariablePtr = Variable + sizeof (UINT32);
-  *(UINT16*)VariablePtr = FilePathListLength;
-  // 'Description' field
-  VariablePtr += sizeof (UINT16);
-  CopyMem (VariablePtr, BootDescription, BootDescriptionSize);
-  // 'FilePathList' field
-  VariablePtr += BootDescriptionSize;
-  CopyMem (VariablePtr, DevicePath, FilePathListLength);
-  // 'OptionalData' field
-  VariablePtr += FilePathListLength;
-  CopyMem (VariablePtr, OptionalData, OptionalDataSize);
-
-  return gRT->SetVariable (
-  BootVariableName,
-  &gEfiGlobalVariableGuid,
-  EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS,
-  VariableSize, Variable
-  );
-}
-
-/**
   Notification function of the event defined as belonging to the
   EFI_END_OF_DXE_EVENT_GROUP_GUID event group that was created in
   the entry point of the driver.
@@ -304,11 +228,6 @@ ArmJunoEntryPoint (
   // Set the R1 two boot options if not already done.
   //
   if (JunoRevision == JUNO_REVISION_R1) {
-Status = SetJunoR1DefaultBootEntries ();
-if (EFI_ERROR (Status)) {
-  return Status;
-}
-
 // Enable PCI enumeration
 PcdSetBool (PcdPciDisableBusEnumeration, FALSE);
 
@@ -356,170 +275,3 @@ ArmJunoEntryPoint (
 
   return Status;
 }
-
-/**
- * If no boot entry is currently defined, define the two default boot entries
- * for Juno R1.
- *
- * @return EFI_SUCCESS Some boot entries were already defined or
- * the default boot entries were set 
successfully.
- * @return EFI_OUT_OF_RESOURCESA memory allocation failed.
- * @return EFI_DEVICE_ERRORAn UEFI variable could not be saved due to 
a hardware failure.
- * @return EFI_WRITE_PROTECTED An UEFI variable is read-only.
- * @return EFI_SECURITY_VIOLATION  An UEFI v

[edk2] [RFC 0/3] Rework the pl061 driver for better readability

2016-02-10 Thread Leif Lindholm
Apologies to Haojian for not giving any feedback on his set of
significant improvements to the pl061 GPIO controller driver.
I found myself unable to review his modifications due to the complexity
of the original driver, so I decided to refactor it ... only to realise
I had no platform to actually test it on.

This series does not modify the external interfaces to the pl061 driver
in any way, so testing on existing platforms would be very appreciated.

Leif Lindholm (3):
  ArmPlatformPkg: PL061 - reformat copyright statement.
  ArmPlatformPkg: PL061 - only initialize on protocol load
  ArmPlatformPkg: PL061 - rewrite the hardware interaction

 ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 116 ++--
 ArmPlatformPkg/Include/Drivers/PL061Gpio.h  |   6 +-
 2 files changed, 71 insertions(+), 51 deletions(-)

-- 
2.1.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [RFC 1/3] ArmPlatformPkg: PL061 - reformat copyright statement.

2016-02-10 Thread Leif Lindholm
The copyright statement in PL061Gpio.c would not fit on a normal terminal
screen. Break lines at <= 80 characters.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm 
---
 ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c 
b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
index ff05662..492cd16 100644
--- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
+++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
@@ -1,11 +1,12 @@
 /** @file
 *
 *  Copyright (c) 2011, ARM Limited. All rights reserved.
+*  Copyright (c) 2015, Linaro Limited. All rights reserved.
 *
 *  This program and the accompanying materials
-*  are licensed and made available under the terms and conditions of the BSD 
License
-*  which accompanies this distribution.  The full text of the license may be 
found at
-*  http://opensource.org/licenses/bsd-license.php
+*  are licensed and made available under the terms and conditions of the BSD
+*  License which accompanies this distribution.  The full text of the license
+*  may be found at http://opensource.org/licenses/bsd-license.php
 *
 *  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
 *  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
-- 
2.1.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [RFC 2/3] ArmPlatformPkg: PL061 - only initialize on protocol load

2016-02-10 Thread Leif Lindholm
For whatever reason, every single operation on the PL061 looked for an
"Initialized" flag, and manually called the initialization function if
not set. Move this to a single call on protocol installation.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm 
---
 ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 39 -
 1 file changed, 6 insertions(+), 33 deletions(-)

diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c 
b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
index 492cd16..7fa9ab6 100644
--- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
+++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
@@ -28,7 +28,6 @@
 #include 
 #include 
 
-BOOLEAN mPL061Initialized = FALSE;
 
 /**
   Function implementations
@@ -79,8 +78,6 @@ PL061Initialize (
   //   // Ensure interrupts are disabled
   //}
 
-  mPL061Initialized = TRUE;
-
   EXIT:
   return Status;
 }
@@ -110,30 +107,19 @@ Get (
   OUT UINTN *Value
   )
 {
-  EFI_STATUSStatus = EFI_SUCCESS;
-
   if ((Value == NULL)
   ||  (Gpio > LAST_GPIO_PIN))
   {
 return EFI_INVALID_PARAMETER;
   }
 
-  // Initialize the hardware if not already done
-  if (!mPL061Initialized) {
-Status = PL061Initialize();
-if (EFI_ERROR(Status)) {
-  goto EXIT;
-}
-  }
-
   if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
 *Value = 1;
   } else {
 *Value = 0;
   }
 
-  EXIT:
-  return Status;
+  return EFI_SUCCESS;
 }
 
 /**
@@ -170,14 +156,6 @@ Set (
 goto EXIT;
   }
 
-  // Initialize the hardware if not already done
-  if (!mPL061Initialized) {
-Status = PL061Initialize();
-if (EFI_ERROR(Status)) {
-  goto EXIT;
-}
-  }
-
   switch (Mode)
   {
 case GPIO_MODE_INPUT:
@@ -234,22 +212,12 @@ GetMode (
   OUT EMBEDDED_GPIO_MODE  *Mode
   )
 {
-  EFI_STATUS Status;
-
   // Check for errors
   if ((Mode == NULL)
   ||  (Gpio > LAST_GPIO_PIN)) {
 return EFI_INVALID_PARAMETER;
   }
 
-  // Initialize the hardware if not already done
-  if (!mPL061Initialized) {
-Status = PL061Initialize();
-if (EFI_ERROR(Status)) {
-  return Status;
-}
-  }
-
   // Check if it is input or output
   if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
 // Pin set to output
@@ -330,6 +298,11 @@ PL061InstallProtocol (
   //
   ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gEmbeddedGpioProtocolGuid);
 
+  Status = PL061Initialize();
+  if (EFI_ERROR(Status)) {
+return EFI_DEVICE_ERROR;
+  }
+
   // Install the Embedded GPIO Protocol onto a new handle
   Handle = NULL;
   Status = gBS->InstallMultipleProtocolInterfaces(
-- 
2.1.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [RFC 3/3] ArmPlatformPkg: PL061 - rewrite the hardware interaction

2016-02-10 Thread Leif Lindholm
The PL061 GPIO controller is a bit of an anachronism, and the existing
driver does nothing to hide this - leading to it being very tricky to
read.

Rewrite it to document (in comments and code) what is actually
happening, and fix some bugs in the process.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm 
---
 ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 70 +
 ArmPlatformPkg/Include/Drivers/PL061Gpio.h  |  6 +--
 2 files changed, 61 insertions(+), 15 deletions(-)

diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c 
b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
index 7fa9ab6..4bfb8de 100644
--- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
+++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
@@ -28,6 +28,56 @@
 #include 
 #include 
 
+//
+// The PL061 is a strange beast. The 8-bit data register is aliased across a
+// region 0x400 bytes in size, with bits [9:2] of the address operating as a
+// mask for both read and write operations:
+// For reads:
+//   - All bits where their corresponding mask bit is 1 return the current
+// value of that bit in the GPIO_DATA register.
+//   - All bits where their corresponding mask bit is 0 return 0.
+// For writes:
+//   - All bits where their corresponding mask bit is 1 set the bit in the
+// GPIO_DATA register to the written value.
+//   - All bits where their corresponding mask bit is 0 are left untouched
+// in the GPIO_DATA register.
+//
+// To keep this driver intelligible, PL061EffectiveAddress, PL061GetPins and
+// Pl061SetPins provide an internal abstraction from this interface.
+
+STATIC
+UINTN
+EFIAPI
+PL061EffectiveAddress (
+  IN UINTN Address,
+  IN UINTN Mask
+  )
+{
+  return ((Address + PL061_GPIO_DATA_REG_OFFSET) + (Mask << 2));
+}
+
+STATIC
+UINTN
+EFIAPI
+PL061GetPins (
+  IN UINTN Address,
+  IN UINTN Mask
+  )
+{
+  return MmioRead8 (PL061EffectiveAddress (Address, Mask));
+}
+
+STATIC
+VOID
+EFIAPI
+PL061SetPins (
+  IN UINTN Address,
+  IN UINTN Mask,
+  IN UINTN Value
+  )
+{
+  MmioWrite8 (PL061EffectiveAddress (Address, Mask), Value);
+}
 
 /**
   Function implementations
@@ -113,7 +163,7 @@ Get (
 return EFI_INVALID_PARAMETER;
   }
 
-  if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
+  if (PL061GetPins (PL061_GPIO_DATA_REG, Gpio)) {
 *Value = 1;
   } else {
 *Value = 0;
@@ -160,21 +210,21 @@ Set (
   {
 case GPIO_MODE_INPUT:
   // Set the corresponding direction bit to LOW for input
-  MmioAnd8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_LOW_8BIT(Gpio));
+  MmioAnd8 (PL061_GPIO_DIR_REG, ~GPIO_PIN_MASK(Gpio) & 0xFF);
   break;
 
 case GPIO_MODE_OUTPUT_0:
-  // Set the corresponding data bit to LOW for 0
-  MmioAnd8 (PL061_GPIO_DATA_REG, GPIO_PIN_MASK_LOW_8BIT(Gpio));
   // Set the corresponding direction bit to HIGH for output
-  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
+  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio));
+  // Set the corresponding data bit to LOW for 0
+  PL061SetPins (PL061_GPIO_DATA_REG, GPIO_PIN_MASK(Gpio), 0);
   break;
 
 case GPIO_MODE_OUTPUT_1:
-  // Set the corresponding data bit to HIGH for 1
-  MmioOr8 (PL061_GPIO_DATA_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
   // Set the corresponding direction bit to HIGH for output
-  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
+  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio));
+  // Set the corresponding data bit to HIGH for 1
+  PL061SetPins (PL061_GPIO_DATA_REG, GPIO_PIN_MASK(Gpio), 1);
   break;
 
 default:
@@ -219,9 +269,9 @@ GetMode (
   }
 
   // Check if it is input or output
-  if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
+  if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK(Gpio)) {
 // Pin set to output
-if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
+if (PL061GetPins (PL061_GPIO_DATA_REG, GPIO_PIN_MASK(Gpio))) {
   *Mode = GPIO_MODE_OUTPUT_1;
 } else {
   *Mode = GPIO_MODE_OUTPUT_0;
diff --git a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h 
b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h
index 38458f4..9da68bd 100644
--- a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h
+++ b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h
@@ -19,6 +19,7 @@
 #include 
 
 // PL061 GPIO Registers
+#define PL061_GPIO_DATA_REG_OFFSET  ((UINTN) 0x000)
 #define PL061_GPIO_DATA_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 
0x000)
 #define PL061_GPIO_DIR_REG  ((UINT32)PcdGet32 (PcdPL061GpioBase) + 
0x400)
 #define PL061_GPIO_IS_REG   ((UINT32)PcdGet32 (PcdPL061GpioBase) + 
0x404)
@@ -40,15 +41,10 @@
 #define PL061_GPIO_PCELL_ID2((UINT32)PcdGet32 (PcdPL061GpioBase) + 
0xFF8)
 #define PL061_GPIO_PCELL_ID3((UINT32)PcdGet32 (PcdPL061GpioBase) + 
0xFFC)
 
-
 // GPIO pins are numbered 0..7
 #define LAST_GPIO_PIN   

Re: [edk2] [RFC 2/3] ArmPlatformPkg: PL061 - only initialize on protocol load

2016-02-10 Thread Ard Biesheuvel
On 10 February 2016 at 16:55, Leif Lindholm  wrote:
> For whatever reason, every single operation on the PL061 looked for an
> "Initialized" flag, and manually called the initialization function if
> not set. Move this to a single call on protocol installation.
>

I think it is not entirely unreasonable to only install the protocol
in the constructor, and not interface with the actual hardware unless
it is actually used.

However, in this case, the only initialization that takes places is
confirming that we are talking to a supported revision of the PL061,
which is something that /does/ make sense, since the protocol should
not be installed in the first place if that is not the case.
That does mean that it would be better to remove PL061Initialize()
entirely, and call PL061Identify() from the constructor instead.


> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Leif Lindholm 
> ---
>  ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 39 
> -
>  1 file changed, 6 insertions(+), 33 deletions(-)
>
> diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c 
> b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
> index 492cd16..7fa9ab6 100644
> --- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
> +++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
> @@ -28,7 +28,6 @@
>  #include 
>  #include 
>
> -BOOLEAN mPL061Initialized = FALSE;
>
>  /**
>Function implementations
> @@ -79,8 +78,6 @@ PL061Initialize (
>//   // Ensure interrupts are disabled
>//}
>
> -  mPL061Initialized = TRUE;
> -
>EXIT:
>return Status;
>  }
> @@ -110,30 +107,19 @@ Get (
>OUT UINTN *Value
>)
>  {
> -  EFI_STATUSStatus = EFI_SUCCESS;
> -
>if ((Value == NULL)
>||  (Gpio > LAST_GPIO_PIN))
>{
>  return EFI_INVALID_PARAMETER;
>}
>
> -  // Initialize the hardware if not already done
> -  if (!mPL061Initialized) {
> -Status = PL061Initialize();
> -if (EFI_ERROR(Status)) {
> -  goto EXIT;
> -}
> -  }
> -
>if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
>  *Value = 1;
>} else {
>  *Value = 0;
>}
>
> -  EXIT:
> -  return Status;
> +  return EFI_SUCCESS;
>  }
>
>  /**
> @@ -170,14 +156,6 @@ Set (
>  goto EXIT;
>}
>
> -  // Initialize the hardware if not already done
> -  if (!mPL061Initialized) {
> -Status = PL061Initialize();
> -if (EFI_ERROR(Status)) {
> -  goto EXIT;
> -}
> -  }
> -
>switch (Mode)
>{
>  case GPIO_MODE_INPUT:
> @@ -234,22 +212,12 @@ GetMode (
>OUT EMBEDDED_GPIO_MODE  *Mode
>)
>  {
> -  EFI_STATUS Status;
> -
>// Check for errors
>if ((Mode == NULL)
>||  (Gpio > LAST_GPIO_PIN)) {
>  return EFI_INVALID_PARAMETER;
>}
>
> -  // Initialize the hardware if not already done
> -  if (!mPL061Initialized) {
> -Status = PL061Initialize();
> -if (EFI_ERROR(Status)) {
> -  return Status;
> -}
> -  }
> -
>// Check if it is input or output
>if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
>  // Pin set to output
> @@ -330,6 +298,11 @@ PL061InstallProtocol (
>//
>ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gEmbeddedGpioProtocolGuid);
>
> +  Status = PL061Initialize();
> +  if (EFI_ERROR(Status)) {
> +return EFI_DEVICE_ERROR;
> +  }
> +
>// Install the Embedded GPIO Protocol onto a new handle
>Handle = NULL;
>Status = gBS->InstallMultipleProtocolInterfaces(
> --
> 2.1.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC 3/3] ArmPlatformPkg: PL061 - rewrite the hardware interaction

2016-02-10 Thread Ard Biesheuvel
On 10 February 2016 at 16:55, Leif Lindholm  wrote:
> The PL061 GPIO controller is a bit of an anachronism, and the existing
> driver does nothing to hide this - leading to it being very tricky to
> read.
>
> Rewrite it to document (in comments and code) what is actually
> happening, and fix some bugs in the process.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Leif Lindholm 
> ---
>  ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 70 
> +
>  ArmPlatformPkg/Include/Drivers/PL061Gpio.h  |  6 +--
>  2 files changed, 61 insertions(+), 15 deletions(-)
>
> diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c 
> b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
> index 7fa9ab6..4bfb8de 100644
> --- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
> +++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
> @@ -28,6 +28,56 @@
>  #include 
>  #include 
>
> +//
> +// The PL061 is a strange beast. The 8-bit data register is aliased across a
> +// region 0x400 bytes in size, with bits [9:2] of the address operating as a
> +// mask for both read and write operations:
> +// For reads:
> +//   - All bits where their corresponding mask bit is 1 return the current
> +// value of that bit in the GPIO_DATA register.
> +//   - All bits where their corresponding mask bit is 0 return 0.
> +// For writes:
> +//   - All bits where their corresponding mask bit is 1 set the bit in the
> +// GPIO_DATA register to the written value.
> +//   - All bits where their corresponding mask bit is 0 are left untouched
> +// in the GPIO_DATA register.
> +//
> +// To keep this driver intelligible, PL061EffectiveAddress, PL061GetPins and
> +// Pl061SetPins provide an internal abstraction from this interface.
> +
> +STATIC
> +UINTN
> +EFIAPI
> +PL061EffectiveAddress (
> +  IN UINTN Address,
> +  IN UINTN Mask
> +  )
> +{
> +  return ((Address + PL061_GPIO_DATA_REG_OFFSET) + (Mask << 2));
> +}
> +
> +STATIC
> +UINTN
> +EFIAPI
> +PL061GetPins (
> +  IN UINTN Address,
> +  IN UINTN Mask
> +  )
> +{
> +  return MmioRead8 (PL061EffectiveAddress (Address, Mask));
> +}
> +
> +STATIC
> +VOID
> +EFIAPI
> +PL061SetPins (
> +  IN UINTN Address,
> +  IN UINTN Mask,
> +  IN UINTN Value
> +  )
> +{
> +  MmioWrite8 (PL061EffectiveAddress (Address, Mask), Value);
> +}
>
>  /**
>Function implementations
> @@ -113,7 +163,7 @@ Get (
>  return EFI_INVALID_PARAMETER;
>}
>
> -  if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
> +  if (PL061GetPins (PL061_GPIO_DATA_REG, Gpio)) {
>  *Value = 1;
>} else {
>  *Value = 0;
> @@ -160,21 +210,21 @@ Set (
>{
>  case GPIO_MODE_INPUT:
>// Set the corresponding direction bit to LOW for input
> -  MmioAnd8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_LOW_8BIT(Gpio));
> +  MmioAnd8 (PL061_GPIO_DIR_REG, ~GPIO_PIN_MASK(Gpio) & 0xFF);
>break;
>
>  case GPIO_MODE_OUTPUT_0:
> -  // Set the corresponding data bit to LOW for 0
> -  MmioAnd8 (PL061_GPIO_DATA_REG, GPIO_PIN_MASK_LOW_8BIT(Gpio));
>// Set the corresponding direction bit to HIGH for output
> -  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
> +  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio));
> +  // Set the corresponding data bit to LOW for 0
> +  PL061SetPins (PL061_GPIO_DATA_REG, GPIO_PIN_MASK(Gpio), 0);
>break;
>
>  case GPIO_MODE_OUTPUT_1:
> -  // Set the corresponding data bit to HIGH for 1
> -  MmioOr8 (PL061_GPIO_DATA_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
>// Set the corresponding direction bit to HIGH for output
> -  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
> +  MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio));
> +  // Set the corresponding data bit to HIGH for 1
> +  PL061SetPins (PL061_GPIO_DATA_REG, GPIO_PIN_MASK(Gpio), 1);

Shouldn't you pass 0xff as value here? This way, you can only set pin 0 afaict

>break;
>
>  default:
> @@ -219,9 +269,9 @@ GetMode (
>}
>
>// Check if it is input or output
> -  if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
> +  if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK(Gpio)) {
>  // Pin set to output
> -if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
> +if (PL061GetPins (PL061_GPIO_DATA_REG, GPIO_PIN_MASK(Gpio))) {
>*Mode = GPIO_MODE_OUTPUT_1;
>  } else {
>*Mode = GPIO_MODE_OUTPUT_0;
> diff --git a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h 
> b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h
> index 38458f4..9da68bd 100644
> --- a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h
> +++ b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h
> @@ -19,6 +19,7 @@
>  #include 
>
>  // PL061 GPIO Registers
> +#define PL061_GPIO_DATA_REG_OFFSET  ((UINTN) 0x000)
>  #define PL061_GPIO_DATA_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) 
> + 0x000)
>  #define PL061_GPIO_DIR_REG  ((UIN

Re: [edk2] [PATCH 0/2] ArmPlatformPkg/ArmJunoPkg: boot and FDT cleanup

2016-02-10 Thread Ryan Harkin
Actually, this series expects some other patches to be applied that
aren't upstream.  I'll rebase my tree and re-submit so there is
dependency.


On 10 February 2016 at 15:51, Ryan Harkin  wrote:
> This small series removes a fair amount of broken code used to create
> default boot entries that don't work and install DT blobs depending upon
> that variant of the board this the code is running on, without
> supporting all the board variants now available.
>
> [PATCH 1/2] ArmPlatformPkg/ArmJunoPkg: don't create default boot entries
> [PATCH 2/2] ArmPlatformPkg/ArmJunoPkg: only have 1 PCD for the FDT
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] GenericBdsLib equivalent of 'BdsStartEfiApplication'

2016-02-10 Thread Michael Zimmermann
Hi,

I'm trying to switch from ARM's BdsLib to Intel's generic once.
the only function that's left is 'BdsStartEfiApplication'. apparently the
generic variant only allows booting via bootoptions.
So, do we plan to move these functions out of ARM's BdsLib or is there
another way?

to make it easier for you, this is the function's signature:
/**
  Start an EFI Application from a Device Path

  @param  ParentImageHandle Handle of the calling image
  @param  DevicePathLocation of the EFI Application

  @retval EFI_SUCCESS   All drivers have been connected
  @retval EFI_NOT_FOUND The Linux kernel Device Path has not been
found
  @retval EFI_OUT_OF_RESOURCES  There is not enough resource memory to
store the matching results.

**/
EFI_STATUS
BdsStartEfiApplication (
  IN EFI_HANDLE  ParentImageHandle,
  IN EFI_DEVICE_PATH_PROTOCOL*DevicePath,
  IN UINTN   LoadOptionsSize,
  IN VOID*   LoadOptions
  );



Michael
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC 2/3] ArmPlatformPkg: PL061 - only initialize on protocol load

2016-02-10 Thread Leif Lindholm
On Wed, Feb 10, 2016 at 05:11:25PM +0100, Ard Biesheuvel wrote:
> On 10 February 2016 at 16:55, Leif Lindholm  wrote:
> > For whatever reason, every single operation on the PL061 looked for an
> > "Initialized" flag, and manually called the initialization function if
> > not set. Move this to a single call on protocol installation.
> 
> I think it is not entirely unreasonable to only install the protocol
> in the constructor, and not interface with the actual hardware unless
> it is actually used.
> 
> However, in this case, the only initialization that takes places is
> confirming that we are talking to a supported revision of the PL061,
> which is something that /does/ make sense, since the protocol should
> not be installed in the first place if that is not the case.
> That does mean that it would be better to remove PL061Initialize()
> entirely, and call PL061Identify() from the constructor instead.

Yeah, that's a good point.
Since I knew I was invasively changing the world underneath Haojian's
series I was kind of trying to keep the diff minimal, and didn't
really look at what the result looked like :)

Fixing this for a future PATCH version.

Thanks!

/
Leif

> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Leif Lindholm 
> > ---
> >  ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 39 
> > -
> >  1 file changed, 6 insertions(+), 33 deletions(-)
> >
> > diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c 
> > b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
> > index 492cd16..7fa9ab6 100644
> > --- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
> > +++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c
> > @@ -28,7 +28,6 @@
> >  #include 
> >  #include 
> >
> > -BOOLEAN mPL061Initialized = FALSE;
> >
> >  /**
> >Function implementations
> > @@ -79,8 +78,6 @@ PL061Initialize (
> >//   // Ensure interrupts are disabled
> >//}
> >
> > -  mPL061Initialized = TRUE;
> > -
> >EXIT:
> >return Status;
> >  }
> > @@ -110,30 +107,19 @@ Get (
> >OUT UINTN *Value
> >)
> >  {
> > -  EFI_STATUSStatus = EFI_SUCCESS;
> > -
> >if ((Value == NULL)
> >||  (Gpio > LAST_GPIO_PIN))
> >{
> >  return EFI_INVALID_PARAMETER;
> >}
> >
> > -  // Initialize the hardware if not already done
> > -  if (!mPL061Initialized) {
> > -Status = PL061Initialize();
> > -if (EFI_ERROR(Status)) {
> > -  goto EXIT;
> > -}
> > -  }
> > -
> >if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
> >  *Value = 1;
> >} else {
> >  *Value = 0;
> >}
> >
> > -  EXIT:
> > -  return Status;
> > +  return EFI_SUCCESS;
> >  }
> >
> >  /**
> > @@ -170,14 +156,6 @@ Set (
> >  goto EXIT;
> >}
> >
> > -  // Initialize the hardware if not already done
> > -  if (!mPL061Initialized) {
> > -Status = PL061Initialize();
> > -if (EFI_ERROR(Status)) {
> > -  goto EXIT;
> > -}
> > -  }
> > -
> >switch (Mode)
> >{
> >  case GPIO_MODE_INPUT:
> > @@ -234,22 +212,12 @@ GetMode (
> >OUT EMBEDDED_GPIO_MODE  *Mode
> >)
> >  {
> > -  EFI_STATUS Status;
> > -
> >// Check for errors
> >if ((Mode == NULL)
> >||  (Gpio > LAST_GPIO_PIN)) {
> >  return EFI_INVALID_PARAMETER;
> >}
> >
> > -  // Initialize the hardware if not already done
> > -  if (!mPL061Initialized) {
> > -Status = PL061Initialize();
> > -if (EFI_ERROR(Status)) {
> > -  return Status;
> > -}
> > -  }
> > -
> >// Check if it is input or output
> >if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
> >  // Pin set to output
> > @@ -330,6 +298,11 @@ PL061InstallProtocol (
> >//
> >ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gEmbeddedGpioProtocolGuid);
> >
> > +  Status = PL061Initialize();
> > +  if (EFI_ERROR(Status)) {
> > +return EFI_DEVICE_ERROR;
> > +  }
> > +
> >// Install the Embedded GPIO Protocol onto a new handle
> >Handle = NULL;
> >Status = gBS->InstallMultipleProtocolInterfaces(
> > --
> > 2.1.4
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v3 1/4] EmbeddedPkg/Lan9118Dxe: use MemoryFence

2016-02-10 Thread Ard Biesheuvel
On 9 February 2016 at 20:23, Ryan Harkin  wrote:
> When reviewing my LAN9118 driver PCD patch [1], Ard Biesheuvel noted
> that most calls to gBS->Stall() in this driver seem to be used to
> prevent timing issues between the device updating data and the host
> reading the values.  And that replacing most of these calls with a
> MemoryFence() would be more robust.
>
> The only exceptions are the stalls that are enclosed inside retry loops:
>
>  - in the AutoNegotiate() function.
>This stall is waiting for the link to negotiate, which may require
>stalling until it is ready.
>
>  - in the Lan9118Initialize() function.
>These two stalls are waiting for devices and time out after a number
>of retries.
>
>  - in the SoftReset() function.
>This stall is inside a loop where the comment states:
>"If time taken exceeds 100us, then there was an error condition"
>
> In these instances, I kept the stall, but also added a MemoryFence().
>
> [1] http://article.gmane.org/gmane.comp.bios.edk2.devel/7389
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ryan Harkin 

Reviewed-by: Ard Biesheuvel 

> ---
>  EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c |  9 +++---
>  EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c | 40 
> ++---
>  2 files changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c 
> b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c
> index 4de5204..79bee3f 100644
> --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c
> +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c
> @@ -307,8 +307,7 @@ SnpInitialize (
>
>// Write the current configuration to the register
>MmioWrite32 (LAN9118_PMT_CTRL, PmConf);
> -  gBS->Stall (LAN9118_STALL);
> -  gBS->Stall (LAN9118_STALL);
> +  MemoryFence();
>
>// Configure GPIO and HW
>Status = ConfigureHardware (HW_CONF_USE_LEDS, Snp);
> @@ -431,7 +430,7 @@ SnpReset (
>
>// Write the current configuration to the register
>MmioWrite32 (LAN9118_PMT_CTRL, PmConf);
> -  gBS->Stall (LAN9118_STALL);
> +  MemoryFence();
>
>// Reactivate the LEDs
>Status = ConfigureHardware (HW_CONF_USE_LEDS, Snp);
> @@ -446,7 +445,7 @@ SnpReset (
>  HwConf |= HW_CFG_TX_FIFO_SIZE(gTxBuffer);// assign size chosen in 
> SnpInitialize
>
>  MmioWrite32 (LAN9118_HW_CFG, HwConf);// Write the conf
> -gBS->Stall (LAN9118_STALL);
> +MemoryFence();
>}
>
>// Enable the receiver and transmitter and clear their contents
> @@ -701,7 +700,7 @@ SnpReceiveFilters (
>// Write the options to the MAC_CSR
>//
>IndirectMACWrite32 (INDIRECT_MAC_INDEX_CR, MacCSRValue);
> -  gBS->Stall (LAN9118_STALL);
> +  MemoryFence();
>
>//
>// If we have to retrieve something, start packet reception.
> diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c 
> b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c
> index 9531b0b..2ef1ecb 100644
> --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c
> +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c
> @@ -236,7 +236,7 @@ IndirectEEPROMRead32 (
>
>// Write to Eeprom command register
>MmioWrite32 (LAN9118_E2P_CMD, EepromCmd);
> -  gBS->Stall (LAN9118_STALL);
> +  MemoryFence();
>
>// Wait until operation has completed
>while (MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY);
> @@ -284,7 +284,7 @@ IndirectEEPROMWrite32 (
>
>// Write to Eeprom command register
>MmioWrite32 (LAN9118_E2P_CMD, EepromCmd);
> -  gBS->Stall (LAN9118_STALL);
> +  MemoryFence();
>
>// Wait until operation has completed
>while (MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY);
> @@ -362,13 +362,14 @@ Lan9118Initialize (
>if (((MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_PM_MODE_MASK) >> 12) != 0) {
>  DEBUG ((DEBUG_NET, "Waking from reduced power state.\n"));
>  MmioWrite32 (LAN9118_BYTE_TEST, 0x);
> -gBS->Stall (LAN9118_STALL);
> +MemoryFence();
>}
>
>// Check that device is active
>Timeout = 20;
>while ((MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_READY) == 0 && --Timeout) {
>  gBS->Stall (LAN9118_STALL);
> +MemoryFence();
>}
>if (!Timeout) {
>  return EFI_TIMEOUT;
> @@ -378,6 +379,7 @@ Lan9118Initialize (
>Timeout = 20;
>while ((MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY) && --Timeout){
>  gBS->Stall (LAN9118_STALL);
> +MemoryFence();
>}
>if (!Timeout) {
>  return EFI_TIMEOUT;
> @@ -447,11 +449,12 @@ SoftReset (
>
>// Write the configuration
>MmioWrite32 (LAN9118_HW_CFG, HwConf);
> -  gBS->Stall (LAN9118_STALL);
> +  MemoryFence();
>
>// Wait for reset to complete
>while (MmioRead32 (LAN9118_HW_CFG) & HWCFG_SRST) {
>
> +MemoryFence();
>  gBS->Stall (LAN9118_STALL);
>  ResetTime += 1;
>
> @@ -500,7 +503,7 @@ PhySoftReset (
>
>  // Wait for completion
>  while (MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_PHY_RST) {
> -  gBS->Stall (LAN9118_STALL);
> +  MemoryFence();
>  

Re: [edk2] [PATCH v3 2/4] EmbeddedPkg/Lan9118Dxe: add PCD for negotiation timeout

2016-02-10 Thread Ard Biesheuvel
On 9 February 2016 at 20:23, Ryan Harkin  wrote:
> Add a PCD for the link negotiation timeout so the platform can over-ride
> the default value.
>
> The code previously did 2000 iterations of the loop with a 2us stall, so
> the code has been changed subtly to set the number of iterations equal
> to the PCD value divided by the stall time.
>
> Since the stall time has not changed, the default PCD value is set at
> 4000 so the original behaviour is not changed.
>
> The problems were discovered when the ARM Juno Development Platform used
> the "EFI Network" option with then LAN9118 driver.  It fails to boot the
> first time and so the board drops back to Shell again:
>
>   Warning: LAN9118 Driver in stopped state
>   Link timeout in auto-negotiation.
>   Lan9118: Auto Negociation not supported.
>   EhcExecTransfer: transfer failed with 2
>   EhcControlTransfer: error - Device Error, transfer - 2
>   Buffer: EFI Hard Drive
>   Booting EFI Misc Device
>   Booting EFI Misc Device 1
>   Booting EFI Hard Drive
>   Booting EFI Network
>   Warning: LAN9118 Driver not initialized
>   Link timeout in auto-negotiation.
>   Lan9118: Auto Negociation not supported.
>   Booting EFI Internal Shell
>
> Exiting Shell drops the user back to the Intel BDS UI.  Selecting
> "Continue" then succeeds in booting from the EFI Network:
>
>   Booting EFI Misc Device
>   Booting EFI Misc Device 1
>   Booting EFI Hard Drive
>   Booting EFI Network
>   ..MnpFreeTxBuf: Duplicated recycle report from SNP.
>   MnpFreeTxBuf: Duplicated recycle report from SNP.
>   [snip repeated errors]
>
> Discussion on the edk2-devel mailing list [1] prompted Laszlo Ersek to
> suggest the time taken for the NIC to negotiate was causing a problem.
> He suggested the solution contained in this patch to provide a PCD
> configurable by the platform.
>
> The default PCD value does not work for Juno.  Setting the PCD to a
> larger value works for Juno R0, R1 and R2.
>
> [1] http://article.gmane.org/gmane.comp.bios.edk2.devel/7341
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ryan Harkin 

Reviewed-by: Ard Biesheuvel 

> ---
>  EmbeddedPkg/EmbeddedPkg.dec | 1 +
>  EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf   | 1 +
>  EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c | 2 +-
>  3 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
> index f557527..cd0d96f 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -145,6 +145,7 @@ [PcdsFixedAtBuild.common]
># LAN9118 Ethernet Driver PCDs
>gEmbeddedTokenSpaceGuid.PcdLan9118DxeBaseAddress|0x0|UINT32|0x0025
>gEmbeddedTokenSpaceGuid.PcdLan9118DefaultMacAddress|0x0|UINT64|0x0026
> +  
> gEmbeddedTokenSpaceGuid.PcdLan9118DefaultNegotiationTimeout|4000|UINT32|0x0027
>
>#
># Android FastBoot
> diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf 
> b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf
> index 9e5f98b..3c2246f 100644
> --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf
> +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf
> @@ -51,6 +51,7 @@ [Protocols]
>  [FixedPcd]
>gEmbeddedTokenSpaceGuid.PcdLan9118DxeBaseAddress
>gEmbeddedTokenSpaceGuid.PcdLan9118DefaultMacAddress
> +  gEmbeddedTokenSpaceGuid.PcdLan9118DefaultNegotiationTimeout
>
>  [Depex]
>TRUE
> diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c 
> b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c
> index 2ef1ecb..c57c7ce 100644
> --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c
> +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c
> @@ -586,7 +586,7 @@ AutoNegotiate (
>// Check that link is up first
>if ((PhyStatus & PHYSTS_LINK_STS) == 0) {
>  // Wait until it is up or until Time Out
> -TimeOut = 2000;
> +TimeOut = FixedPcdGet32 (PcdLan9118DefaultNegotiationTimeout) / 
> LAN9118_STALL;
>  while ((IndirectPHYRead32 (PHY_INDEX_BASIC_STATUS) & PHYSTS_LINK_STS) == 
> 0) {
>MemoryFence();
>gBS->Stall (LAN9118_STALL);
> --
> 2.1.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v3 0/4] EmbeddedPkg/Lan9118Dxe: MemoryFence and PCD

2016-02-10 Thread Ard Biesheuvel
On 9 February 2016 at 20:29, Ryan Harkin  wrote:
> On 9 February 2016 at 19:23, Ryan Harkin  wrote:
>> This is a follow up from my previous patch [1] to add a PCD for the
>> auto-negotiation timeout and the v2 series that followed it.
>>
>> Review comments on the edk2-devel mailing list and on the
>> #linaro-enterprise IRC channel evolved the solution into two different
>> patches:
>>
>> [PATCH 1/4] EmbeddedPkg/Lan9118Dxe: use MemoryFence
>> [PATCH 2/4] EmbeddedPkg/Lan9118Dxe: add PCD for negotiation timeout
>>
>> Whilst I was editing the code, I also noticed a few non-functional
>> quirks that were easy to fix:
>>
>> [PATCH 3/4] EmbeddedPkg/Lan9118Dxe: minor DEBUG tidyup
>> [PATCH 4/4] EmbeddedPkg/Lan9118Dxe: rename TimeOut to Retries
>>
>> Changes since v2:
>> - The number of stalls replaced in the first patch has been reduced.
>>   Any loop that is time bound now contains a MemoryFence and a Stall
>> - The PCD was previously setting the Stall time for the auto-negotiation
>>   timeout.  It now sets the total time to wait for auto-negotiation.
>> - I dropped the Reviewed-by tags off patches 1 & 2 because the code has
>>   changed enough that I didn't think it was fair to keep them.
>>
>> [1] http://article.gmane.org/gmane.comp.bios.edk2.devel/7341
>>
>

OK, pushed to tianocore-edk2/master

Thanks,
Ard.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] ShellPkg: ShellFileHandleReadLine must return UCS2 lines

2016-02-10 Thread Carsey, Jaben
Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Ryan Harkin [mailto:ryan.har...@linaro.org]
> Sent: Wednesday, February 10, 2016 6:16 AM
> To: jim_dai...@dell.com
> Cc: edk2-devel@lists.01.org; Carsey, Jaben ; Qiu,
> Shumin 
> Subject: Re: [edk2] [PATCH] ShellPkg: ShellFileHandleReadLine must return
> UCS2 lines
> Importance: High
> 
> Hi Jim,
> 
> Thanks for the quick fix!  It works for me on ARM Juno R0, R1 and R2
> and Versatile Express TC2.
> 
> On 10 February 2016 at 13:45,   wrote:
> > ShellPkg: ShellFileHandleReadLine must return UCS2 lines.
> >
> > An earlier change had this function returning the type of lines that were in
> the file being read (ASCII or UCS2).  The way it is used, UCS2 output is
> expected, even when the file being read is ASCII. This change restores that
> behavior and documents it.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Jim Dailey
> ^^ your email address is missing from your sign-off, I think it's
> mandatory, but don't hold me to that.
> 
> I haven't had chance to read and understand the code, but I can at
> least provide:
> 
> Tested-by: Ryan Harkin 
> 
> Thanks,
> Ryan.
> 
> > ---
> > diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> > index 4b53c70..abff0d3 100644
> > --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> > +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> > @@ -4084,9 +4084,20 @@ ShellFileHandleReturnLine(
> >If the position upon start is 0, then the Ascii Boolean will be set.  
> > This
> should be
> >maintained and not changed for all operations with the same file.
> >
> > +  NOTE: LINES THAT ARE RETURNED BY THIS FUNCTION ARE UCS2, EVEN IF
> THE FILE BEING READ
> > +IS IN ASCII FORMAT.
> > +
> >@param[in]   HandleSHELL_FILE_HANDLE to read from.
> > -  @param[in, out]  BufferThe pointer to buffer to read into.
> > -  @param[in, out]  Size  The pointer to number of bytes in Buffer.
> > +  @param[in, out]  BufferThe pointer to buffer to read into. If 
> > this
> function
> > + returns EFI_SUCCESS, then on output 
> > Buffer will
> > + contain a UCS2 string, even if the file 
> > being
> > + read is ASCII.
> > +  @param[in, out]  Size  On input, pointer to number of bytes in 
> > Buffer.
> > + On output, unchanged unless Buffer is too 
> > small
> > + to contain the next line of the file. In 
> > that
> > + case Size is set to the number of bytes 
> > needed
> > + to hold the next line of the file (as a 
> > UCS2
> > + string, even if it is an ASCII file).
> >@param[in]   Truncate  If the buffer is large enough, this has 
> > no effect.
> >   If the buffer is is too small and 
> > Truncate is TRUE,
> >   the line will be truncated.
> > @@ -4165,43 +4176,27 @@ ShellFileHandleReadLine(
> >  //
> >  // if we have space save it...
> >  //
> > -if ((CountSoFar + 1) * CharSize < *Size){
> > +if ((CountSoFar+1)*sizeof(CHAR16) < *Size){
> >ASSERT(Buffer != NULL);
> > -  if (*Ascii) {
> > -((CHAR8*)Buffer)[CountSoFar] = (CHAR8) CharBuffer;
> > -((CHAR8*)Buffer)[CountSoFar+1] = '\0';
> > -  }
> > -  else {
> > -((CHAR16*)Buffer)[CountSoFar] = CharBuffer;
> > -((CHAR16*)Buffer)[CountSoFar+1] = CHAR_NULL;
> > -  }
> > +  ((CHAR16*)Buffer)[CountSoFar] = CharBuffer;
> > +  ((CHAR16*)Buffer)[CountSoFar+1] = CHAR_NULL;
> >  }
> >}
> >
> >//
> >// if we ran out of space tell when...
> >//
> > -  if (Status != EFI_END_OF_FILE){
> > -if ((CountSoFar + 1) * CharSize > *Size){
> > -  *Size = (CountSoFar + 1) * CharSize;
> > -  if (!Truncate) {
> > -gEfiShellProtocol->SetFilePosition(Handle, OriginalFilePosition);
> > -  } else {
> > -DEBUG((DEBUG_WARN, "The line was truncated in
> ShellFileHandleReadLine"));
> > -  }
> > -  return (EFI_BUFFER_TOO_SMALL);
> > -}
> > -
> > -if (*Ascii) {
> > -  if (CountSoFar && ((CHAR8*)Buffer)[CountSoFar - 1] == '\r') {
> > -((CHAR8*)Buffer)[CountSoFar - 1] = '\0';
> > -  }
> > -}
> > -else {
> > -  if (CountSoFar && Buffer[CountSoFar - 1] == L'\r') {
> > -Buffer[CountSoFar - 1] = CHAR_NULL;
> > -  }
> > +  if ((CountSoFar+1)*sizeof(CHAR16) > *Size){
> > +*Size = (CountSoFar+1)*sizeof(CHAR16);
> > +if (!Truncate) {
> > +  gEfiShellProtocol->SetFilePosition(Handle, OriginalFilePosition);
> > +} else {
> > +  DEBUG((DEBUG_WARN, "The line was truncated in
> ShellFileHandleReadLine"));
> >  }
> > +return (EFI_BUFFER_TOO_SMALL)

Re: [edk2] PCD Local Token Numbers in PEI/DXE

2016-02-10 Thread Kinney, Michael D
Hi Tim,

Your description looks correct to me.  The current design does have an 
assumption 
that the PCD database used in PEI is aligned with the PCD database used in DXE.

If the number of Dynamic/DynamicEx PCDs used in a platform changes, then the
PCD database associated with the PCD PEIM and PCD DXE Driver both need to be
updated.

I think it would be good to work on a method that allows the PEI Database and
DXE Database to be updated independently.

In general, if binary modules are used, then Dynamic PCDs can not be used.  
Instead
all Dynamic PCDs must be updated to by DynamicEx PCDs.  That is for binary
modules that performs PCD Get/Set operations through the PCD PPI/Protocol.

I think the gap here is that the PCD database does not have a build mode that 
forces use of only DynamicEx (TokenSpaceGuid, TokenNumber) for the entire 
database
contents.  If we added that build mode (so there are no "local token numbers")
then the PEI database and DXE database could be updated independently.

This build mode could only be enabled if there are no Dynamic PCDs.  In fact, 
this
build mode could be automatic if there are no Dynamic PCDs in DSC file.

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Tim 
> Lewis
> Sent: Tuesday, February 9, 2016 10:24 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] PCD Local Token Numbers in PEI/DXE
> 
> We have run into an interesting problem with the PCD database when the PEI 
> and DXE
> databases were not built at the same time. This happens with boot-block type
> arrangements. This is not a Dynamic vs. DynamicEx issue.
> 
> Short form:
> 
> 
> 1)  The standard PCD database for Dynamic/DynamicEx PCDs is broken into 
> two pieces,
> based on whether the PCD is access by a PEIM, a DXE driver, or both. The 
> pieces are
> embedded directly into the PCD PEIM and PCD DXE driver that produces the PCD 
> services.
> 
> 2)  Each Dynamic/DynamicEx PCD is assigned a unique "local token number" 
> This
> number is different than the token number which is in the PCD declaration in 
> the .dec
> file. This number is assigned at build time.
> 
> 3)  If a later version of the DXE PCD driver is a) built in a later 
> codebase where
> there are more or less PEI-access PCDs, but later b) executed with a version 
> of the PEI
> PCD database from the earlier codebase where there were fewer, it causes a 
> problem. For
> example, if the new PEI PCD database has 4 more, the new DXE PCD database 
> will start
> its numbering at +4. But when it is executed with the old PEI PCD database, 
> it will end
> up looking up the wrong PCD
> 
> We're not sure what the best course is to solve this. Frankly, the PCD 
> database format
> is a muddle. We have a temporary work-around, but we're wondering if anyone 
> has
> thoughts on a good solution.
> 
> Thanks,
> 
> Tim
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] PCD Local Token Numbers in PEI/DXE

2016-02-10 Thread Tim Lewis
Mike --

Yes, we use all DynamicEx PCDs because we use a large number of binary 
deliverables in certain segments. 

It would be a much simpler database design if the look up was purely a 
GUID/token number/SkuId look-up (no local token numbers at all). The existing 
Dynamic PCDs could be supported by assigning them a single GUID. That is, 
Dynamic PCDs would be translated up to DynamicEx by using gEfiDynamicPcdGuid.  
Or we could deprecate Dynamic. Or make it auto-translate to DynamicEx.

Tim


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Kinney, 
Michael D
Sent: Wednesday, February 10, 2016 9:41 AM
To: Tim Lewis ; edk2-devel@lists.01.org; Kinney, Michael 
D 
Subject: Re: [edk2] PCD Local Token Numbers in PEI/DXE

Hi Tim,

Your description looks correct to me.  The current design does have an 
assumption that the PCD database used in PEI is aligned with the PCD database 
used in DXE.

If the number of Dynamic/DynamicEx PCDs used in a platform changes, then the 
PCD database associated with the PCD PEIM and PCD DXE Driver both need to be 
updated.

I think it would be good to work on a method that allows the PEI Database and 
DXE Database to be updated independently.

In general, if binary modules are used, then Dynamic PCDs can not be used.  
Instead all Dynamic PCDs must be updated to by DynamicEx PCDs.  That is for 
binary modules that performs PCD Get/Set operations through the PCD 
PPI/Protocol.

I think the gap here is that the PCD database does not have a build mode that 
forces use of only DynamicEx (TokenSpaceGuid, TokenNumber) for the entire 
database contents.  If we added that build mode (so there are no "local token 
numbers") then the PEI database and DXE database could be updated independently.

This build mode could only be enabled if there are no Dynamic PCDs.  In fact, 
this build mode could be automatic if there are no Dynamic PCDs in DSC file.

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Tim Lewis
> Sent: Tuesday, February 9, 2016 10:24 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] PCD Local Token Numbers in PEI/DXE
> 
> We have run into an interesting problem with the PCD database when the 
> PEI and DXE databases were not built at the same time. This happens 
> with boot-block type arrangements. This is not a Dynamic vs. DynamicEx issue.
> 
> Short form:
> 
> 
> 1)  The standard PCD database for Dynamic/DynamicEx PCDs is broken into 
> two pieces,
> based on whether the PCD is access by a PEIM, a DXE driver, or both. 
> The pieces are embedded directly into the PCD PEIM and PCD DXE driver that 
> produces the PCD services.
> 
> 2)  Each Dynamic/DynamicEx PCD is assigned a unique "local token number" 
> This
> number is different than the token number which is in the PCD 
> declaration in the .dec file. This number is assigned at build time.
> 
> 3)  If a later version of the DXE PCD driver is a) built in a later 
> codebase where
> there are more or less PEI-access PCDs, but later b) executed with a 
> version of the PEI PCD database from the earlier codebase where there 
> were fewer, it causes a problem. For example, if the new PEI PCD 
> database has 4 more, the new DXE PCD database will start its numbering 
> at +4. But when it is executed with the old PEI PCD database, it will 
> end up looking up the wrong PCD
> 
> We're not sure what the best course is to solve this. Frankly, the PCD 
> database format is a muddle. We have a temporary work-around, but 
> we're wondering if anyone has thoughts on a good solution.
> 
> Thanks,
> 
> Tim
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] PCD Local Token Numbers in PEI/DXE

2016-02-10 Thread Kinney, Michael D
Tim,

All good ideas to evaluate.

We did design in the Dynamic PCDs with generated local tokens to minimize the 
size overhead
of the PCD database for source builds.  We can do some size impact analysis of 
these ideas 
to see which one makes the most sense.

The database is currently optimized for Dynamic PCDs.  When a DynamicEx PCD is 
used
it is internally translated to a Dynamic request.  I think all of the ideas 
here require
this concept to be reversed.  We need to optimized the database for DynamicEx 
and never
reference Dynamic part of database to process a DynamicEx request.  If Dynamic 
is used,
it can either be internally translated to a DynamicEx request with a fixed 
token space
guid or be processed as a local token.  In mixed Dynamic/DynamicEx 
environments, the 
same PCD may be accessed using both methods.  Current design supports this 
mixed env,
so we need to make sure that aspect is not broken if we change internal 
code/database.

Best regards,

Mike

> -Original Message-
> From: Tim Lewis [mailto:tim.le...@insyde.com]
> Sent: Wednesday, February 10, 2016 9:50 AM
> To: Kinney, Michael D ; edk2-devel@lists.01.org
> Subject: RE: PCD Local Token Numbers in PEI/DXE
> 
> Mike --
> 
> Yes, we use all DynamicEx PCDs because we use a large number of binary 
> deliverables in
> certain segments.
> 
> It would be a much simpler database design if the look up was purely a 
> GUID/token
> number/SkuId look-up (no local token numbers at all). The existing Dynamic 
> PCDs could
> be supported by assigning them a single GUID. That is, Dynamic PCDs would be 
> translated
> up to DynamicEx by using gEfiDynamicPcdGuid.  Or we could deprecate Dynamic. 
> Or make it
> auto-translate to DynamicEx.
> 
> Tim
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Kinney, Michael
> D
> Sent: Wednesday, February 10, 2016 9:41 AM
> To: Tim Lewis ; edk2-devel@lists.01.org; Kinney, 
> Michael D
> 
> Subject: Re: [edk2] PCD Local Token Numbers in PEI/DXE
> 
> Hi Tim,
> 
> Your description looks correct to me.  The current design does have an 
> assumption that
> the PCD database used in PEI is aligned with the PCD database used in DXE.
> 
> If the number of Dynamic/DynamicEx PCDs used in a platform changes, then the 
> PCD
> database associated with the PCD PEIM and PCD DXE Driver both need to be 
> updated.
> 
> I think it would be good to work on a method that allows the PEI Database and 
> DXE
> Database to be updated independently.
> 
> In general, if binary modules are used, then Dynamic PCDs can not be used.  
> Instead all
> Dynamic PCDs must be updated to by DynamicEx PCDs.  That is for binary 
> modules that
> performs PCD Get/Set operations through the PCD PPI/Protocol.
> 
> I think the gap here is that the PCD database does not have a build mode that 
> forces
> use of only DynamicEx (TokenSpaceGuid, TokenNumber) for the entire database 
> contents.
> If we added that build mode (so there are no "local token numbers") then the 
> PEI
> database and DXE database could be updated independently.
> 
> This build mode could only be enabled if there are no Dynamic PCDs.  In fact, 
> this
> build mode could be automatic if there are no Dynamic PCDs in DSC file.
> 
> Mike
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Tim Lewis
> > Sent: Tuesday, February 9, 2016 10:24 AM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] PCD Local Token Numbers in PEI/DXE
> >
> > We have run into an interesting problem with the PCD database when the
> > PEI and DXE databases were not built at the same time. This happens
> > with boot-block type arrangements. This is not a Dynamic vs. DynamicEx 
> > issue.
> >
> > Short form:
> >
> >
> > 1)  The standard PCD database for Dynamic/DynamicEx PCDs is broken into 
> > two
> pieces,
> > based on whether the PCD is access by a PEIM, a DXE driver, or both.
> > The pieces are embedded directly into the PCD PEIM and PCD DXE driver that 
> > produces
> the PCD services.
> >
> > 2)  Each Dynamic/DynamicEx PCD is assigned a unique "local token 
> > number" This
> > number is different than the token number which is in the PCD
> > declaration in the .dec file. This number is assigned at build time.
> >
> > 3)  If a later version of the DXE PCD driver is a) built in a later 
> > codebase
> where
> > there are more or less PEI-access PCDs, but later b) executed with a
> > version of the PEI PCD database from the earlier codebase where there
> > were fewer, it causes a problem. For example, if the new PEI PCD
> > database has 4 more, the new DXE PCD database will start its numbering
> > at +4. But when it is executed with the old PEI PCD database, it will
> > end up looking up the wrong PCD
> >
> > We're not sure what the best course is to solve this. Frankly, the PCD
> > database format is a muddle. We have a temporary work-around, but
> > we're wondering

[edk2] BaseTools build number after transition to git

2016-02-10 Thread Felix Poludov
Most of the EDKII tools support --version switch that returns tool version and 
build number.
We've been using SVN revision ID as a build number for all the tools.
Have there been any discussions on what should be used as a build number in the 
new (and hopefully merrier)  EDKII Git world?

Felix


The information contained in this message may be confidential and proprietary 
to American Megatrends, Inc.  This communication is intended to be read only by 
the individual or entity to whom it is addressed or by their designee. If the 
reader of this message is not the intended recipient, you are on notice that 
any distribution of this message, in any form, is strictly prohibited.  Please 
promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and 
then delete or destroy all copies of the transmission.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] ShellPkg: ShellFileHandleReadLine must return UCS2 lines

2016-02-10 Thread Ryan Harkin
On 10 February 2016 at 14:16, Ryan Harkin  wrote:
> Hi Jim,
>
> Thanks for the quick fix!  It works for me on ARM Juno R0, R1 and R2
> and Versatile Express TC2.
>
> On 10 February 2016 at 13:45,   wrote:
>> ShellPkg: ShellFileHandleReadLine must return UCS2 lines.
>>
>> An earlier change had this function returning the type of lines that were in 
>> the file being read (ASCII or UCS2).  The way it is used, UCS2 output is 
>> expected, even when the file being read is ASCII. This change restores that 
>> behavior and documents it.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jim Dailey
> ^^ your email address is missing from your sign-off, I think it's
> mandatory, but don't hold me to that.
>
> I haven't had chance to read and understand the code, but I can at
> least provide:
>
> Tested-by: Ryan Harkin 
>

I've just updated to the tip of tianocore and I see the same problem
as before.  I have made sure I'm running the binary I built from this
point in the tree, after the fix has been merged:

62989e0  2016-02-10  Merge branch 'master' of
https://github.com/tianocore/edk2 [Jaben Carsey]

And it doesn't work.  So I went back to this commit:

d2a0d2e  2016-02-08  ShellPkg: Fix ASCII and UNICODE file pipes.
 [jaben carsey]

And applied the patch myself from the original email in this thread
and it works for me.

So I've looked again.  If I checkout the tree right where the fix went in:

2dda8a1  2016-02-10  ShellPkg: ShellFileHandleReadLine must return
UCS2 lines.  [Jim Dailey]

It also works.

If I then checkout the tree at the next commit, a strange merge commit:

3a01358  2016-02-10  Merge branch 'master' of
https://github.com/tianocore/edk2 [Jaben Carsey]

Then part of the fix vanishes and the code no longer works.

If I do a "git show -1 -m 3a01358", then I can see that the merge
commit does indeed appear to add some of the code back.

A further merge commit in the tree:

62989e0  2016-02-10  Merge branch 'master' of
https://github.com/tianocore/edk2 [Jaben Carsey]

Has loads of other funky stuff in it and it adds part of the change
back, but only the comment part, the code code hunk that fixes my
problem.

Whilst I was surprised to see these merge commits in the tree in the
first place, I didn't think too much about it.  But now I'm seeing the
diffs they are introducing, I'm either going mad or I'm getting
worried about the workflow in the tree :(

Can someone have a look into the tree and please confirm either that
I've lost the plot or that something strange has happened?

> Thanks,
> Ryan.
>
>> ---
>> diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c 
>> b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
>> index 4b53c70..abff0d3 100644
>> --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
>> +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
>> @@ -4084,9 +4084,20 @@ ShellFileHandleReturnLine(
>>If the position upon start is 0, then the Ascii Boolean will be set.  
>> This should be
>>maintained and not changed for all operations with the same file.
>>
>> +  NOTE: LINES THAT ARE RETURNED BY THIS FUNCTION ARE UCS2, EVEN IF THE FILE 
>> BEING READ
>> +IS IN ASCII FORMAT.
>> +
>>@param[in]   HandleSHELL_FILE_HANDLE to read from.
>> -  @param[in, out]  BufferThe pointer to buffer to read into.
>> -  @param[in, out]  Size  The pointer to number of bytes in Buffer.
>> +  @param[in, out]  BufferThe pointer to buffer to read into. If 
>> this function
>> + returns EFI_SUCCESS, then on output Buffer 
>> will
>> + contain a UCS2 string, even if the file 
>> being
>> + read is ASCII.
>> +  @param[in, out]  Size  On input, pointer to number of bytes in 
>> Buffer.
>> + On output, unchanged unless Buffer is too 
>> small
>> + to contain the next line of the file. In 
>> that
>> + case Size is set to the number of bytes 
>> needed
>> + to hold the next line of the file (as a 
>> UCS2
>> + string, even if it is an ASCII file).
>>@param[in]   Truncate  If the buffer is large enough, this has no 
>> effect.
>>   If the buffer is is too small and Truncate 
>> is TRUE,
>>   the line will be truncated.
>> @@ -4165,43 +4176,27 @@ ShellFileHandleReadLine(
>>  //
>>  // if we have space save it...
>>  //
>> -if ((CountSoFar + 1) * CharSize < *Size){
>> +if ((CountSoFar+1)*sizeof(CHAR16) < *Size){
>>ASSERT(Buffer != NULL);
>> -  if (*Ascii) {
>> -((CHAR8*)Buffer)[CountSoFar] = (CHAR8) CharBuffer;
>> -((CHAR8*)Buffer)[CountSoFar+1] = '\0';
>> -  }
>> -  else {
>> -((CHAR16*)Buffer)[CountSoFar] = CharBuffer;
>> -((CHAR16*)Buff

[edk2] [PATCH v2 1/2] ArmPlatformPkg/ArmJunoPkg: don't create default boot entries

2016-02-10 Thread Ryan Harkin
Code was inserted to create default boot entries for Juno R1.  These
don't work, but they are also preventing the board from booting into the
default options that Intel BDS would otherwise boot.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ryan Harkin 
---
 .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 255 -
 1 file changed, 255 deletions(-)

diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c 
b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
index 756c275..205aead 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
@@ -31,13 +31,6 @@
 #include 
 
 //
-// Size in number of characters of the Linux boot argument
-// passing the MAC address to be used by the PCI GigaByte
-// Ethernet device : " sky2.mac_address=0x11,0x22,0x33,0x44,0x55,0x66"
-//
-#define SKY2_MAC_ADDRESS_BOOTARG_LEN  47
-
-//
 // Hardware platform identifiers
 //
 typedef enum {
@@ -46,13 +39,6 @@ typedef enum {
   JUNO_R1
 } JUNO_REVISION;
 
-//
-// Function prototypes
-//
-STATIC EFI_STATUS SetJunoR1DefaultBootEntries (
-  VOID
-  );
-
 // This GUID must match the FILE_GUID in 
ArmPlatformPkg/ArmJunoPkg/AcpiTables/AcpiTables.inf
 STATIC CONST EFI_GUID mJunoAcpiTableFile = { 0xa1dd808e, 0x1e95, 0x4399, { 
0xab, 0xc0, 0x65, 0x3c, 0x82, 0xe8, 0x53, 0x0c } };
 
@@ -91,75 +77,6 @@ STATIC CONST EFI_PCI_ROOT_BRIDGE_DEVICE_PATH 
mPciRootComplexDevicePath = {
 EFI_EVENT mAcpiRegistration = NULL;
 
 /**
- * Build and Set UEFI Variable Boot
- *
- * @param BootVariableName   Name of the UEFI Variable
- * @param Attributes 'Attributes' for the Boot variable as per 
UEFI spec
- * @param BootDescriptionDescription of the Boot variable
- * @param DevicePath EFI Device Path of the EFI Application to boot
- * @param OptionalData   Parameters to pass to the EFI application
- * @param OptionalDataSize   Size of the parameters to pass to the EFI 
application
- *
- * @return EFI_OUT_OF_RESOURCES  A memory allocation failed
- * @return   Return value of RT.SetVariable
- */
-STATIC
-EFI_STATUS
-BootOptionCreate (
-  IN  CHAR16BootVariableName[9],
-  IN  UINT32Attributes,
-  IN  CHAR16*   BootDescription,
-  IN  EFI_DEVICE_PATH_PROTOCOL* DevicePath,
-  IN  UINT8*OptionalData,
-  IN  UINTN OptionalDataSize
-  )
-{
-  UINTN VariableSize;
-  UINT8 *Variable;
-  UINT8 *VariablePtr;
-  UINTN FilePathListLength;
-  UINTN BootDescriptionSize;
-
-  FilePathListLength  = GetDevicePathSize (DevicePath);
-  BootDescriptionSize = StrSize (BootDescription);
-
-  // Each Boot variable is built as follow:
-  //   UINT32   Attributes
-  //   UINT16   FilePathListLength
-  //   CHAR16*  Description
-  //   EFI_DEVICE_PATH_PROTOCOL FilePathList[]
-  //   UINT8OptionalData[]
-  VariableSize = sizeof (UINT32) + sizeof (UINT16) +
-  BootDescriptionSize + FilePathListLength + OptionalDataSize;
-  Variable = AllocateZeroPool (VariableSize);
-  if (Variable == NULL) {
-return EFI_OUT_OF_RESOURCES;
-  }
-
-  // 'Attributes' field
-  *(UINT32*)Variable = Attributes;
-  // 'FilePathListLength' field
-  VariablePtr = Variable + sizeof (UINT32);
-  *(UINT16*)VariablePtr = FilePathListLength;
-  // 'Description' field
-  VariablePtr += sizeof (UINT16);
-  CopyMem (VariablePtr, BootDescription, BootDescriptionSize);
-  // 'FilePathList' field
-  VariablePtr += BootDescriptionSize;
-  CopyMem (VariablePtr, DevicePath, FilePathListLength);
-  // 'OptionalData' field
-  VariablePtr += FilePathListLength;
-  CopyMem (VariablePtr, OptionalData, OptionalDataSize);
-
-  return gRT->SetVariable (
-  BootVariableName,
-  &gEfiGlobalVariableGuid,
-  EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS,
-  VariableSize, Variable
-  );
-}
-
-/**
   Notification function of the event defined as belonging to the
   EFI_END_OF_DXE_EVENT_GROUP_GUID event group that was created in
   the entry point of the driver.
@@ -341,11 +258,6 @@ ArmJunoEntryPoint (
   // Set the R1 two boot options if not already done.
   //
   if (JunoRevision == JUNO_R1) {
-Status = SetJunoR1DefaultBootEntries ();
-if (EFI_ERROR (Status)) {
-  return Status;
-}
-
 // Enable PCI enumeration
 PcdSetBool (PcdPciDisableBusEnumeration, FALSE);
 
@@ -393,170 +305,3 @@ ArmJunoEntryPoint (
 
   return Status;
 }
-
-/**
- * If no boot entry is currently defined, define the two default boot entries
- * for Juno R1.
- *
- * @return EFI_SUCCESS Some boot entries were already defined or
- * the

[edk2] [PATCH v2 2/2] ArmPlatformPkg/ArmJunoPkg: only have 1 PCD for the FDT

2016-02-10 Thread Ryan Harkin
Juno doesn't have lots of DTB files in NOR flash, it only has 1 file,
called "board.dtb" and the motherboard configuration makes the right
choice about which DTB file gets written as board.dtb in NOR.

The code attempts to select which DTB it should use based on the board
variant or configuration.  And this doesn't work because those DTB files
aren't present in NOR flash.

So remove the DTB variants and only load board.dtb.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ryan Harkin 
---
 ArmPlatformPkg/ArmJunoPkg/ArmJuno.dec  |  4 +---
 .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf   |  4 +---
 ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c  | 14 +-
 3 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/ArmPlatformPkg/ArmJunoPkg/ArmJuno.dec 
b/ArmPlatformPkg/ArmJunoPkg/ArmJuno.dec
index 040a906..7af8885 100644
--- a/ArmPlatformPkg/ArmJunoPkg/ArmJuno.dec
+++ b/ArmPlatformPkg/ArmJunoPkg/ArmJuno.dec
@@ -44,6 +44,4 @@ [PcdsFixedAtBuild.common]
   
gArmJunoTokenSpaceGuid.PcdSynopsysUsbEhciBaseAddress|0x7FFC|UINT32|0x0005
 
   # Juno Device Trees are loaded from NOR Flash
-  
gArmJunoTokenSpaceGuid.PcdJunoR0FdtDevicePath|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/juno.dtb"|VOID*|0x0006
-  
gArmJunoTokenSpaceGuid.PcdJunoR1A57x2FdtDevicePath|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/r1a57.dtb"|VOID*|0x0007
-  
gArmJunoTokenSpaceGuid.PcdJunoR1A57x2A53x4FdtDevicePath|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/r1a57a53.dtb"|VOID*|0x0008
+  
gArmJunoTokenSpaceGuid.PcdJunoFdtDevicePath|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/board.dtb"|VOID*|0x0008
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf 
b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
index d1f2f7b..6ab81e8 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
@@ -73,9 +73,7 @@ [FixedPcd]
   gArmJunoTokenSpaceGuid.PcdSynopsysUsbEhciBaseAddress
   gArmJunoTokenSpaceGuid.PcdSynopsysUsbOhciBaseAddress
 
-  gArmJunoTokenSpaceGuid.PcdJunoR0FdtDevicePath
-  gArmJunoTokenSpaceGuid.PcdJunoR1A57x2FdtDevicePath
-  gArmJunoTokenSpaceGuid.PcdJunoR1A57x2A53x4FdtDevicePath
+  gArmJunoTokenSpaceGuid.PcdJunoFdtDevicePath
 
   gArmPlatformTokenSpaceGuid.PcdDefaultBootDevicePath
   gArmPlatformTokenSpaceGuid.PcdDefaultBootArgument
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c 
b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
index 205aead..dba1fcd 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
@@ -274,19 +274,7 @@ ArmJunoEntryPoint (
   //
   // Set up the device path to the FDT.
   //
-  switch (JunoRevision) {
-  case JUNO_R0:
-TextDevicePath = (CHAR16*)FixedPcdGetPtr (PcdJunoR0FdtDevicePath);
-break;
-
-  case JUNO_R1:
-TextDevicePath = (CHAR16*)FixedPcdGetPtr (PcdJunoR1A57x2FdtDevicePath);
-break;
-
-  default:
-TextDevicePath = NULL;
-  }
-
+  TextDevicePath = (CHAR16*)FixedPcdGetPtr (PcdJunoFdtDevicePath);
   if (TextDevicePath != NULL) {
 TextDevicePathSize = StrSize (TextDevicePath);
 Buffer = PcdSetPtr (PcdFdtDevicePaths, &TextDevicePathSize, 
TextDevicePath);
-- 
2.1.4

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH v2 0/2] ArmPlatformPkg/ArmJunoPkg: boot and FDT cleanup

2016-02-10 Thread Ryan Harkin
This small series removes a fair amount of broken code used to create
default boot entries and install DT blobs depending upon that variant of
the board this the code is running on.

Changes since v1:
- fixed rebase conflict in the 2nd patch where I accidentally left
  in a hunk from a different commit not due to be upstreamed.

[PATCH v2 1/2] ArmPlatformPkg/ArmJunoPkg: don't create default boot entries
[PATCH v2 2/2] ArmPlatformPkg/ArmJunoPkg: only have 1 PCD for the FDT

This series can be found here:

https://git.linaro.org/landing-teams/working/arm/edk2.git/shortlog/refs/tags/upstreaming-20160210-004
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] ShellPkg: ShellFileHandleReadLine must return UCS2 lines

2016-02-10 Thread Jordan Justen
On 2016-02-10 06:16:14, Ryan Harkin wrote:
> Hi Jim,
> 
> Thanks for the quick fix!  It works for me on ARM Juno R0, R1 and R2
> and Versatile Express TC2.
> 
> On 10 February 2016 at 13:45,   wrote:
> > ShellPkg: ShellFileHandleReadLine must return UCS2 lines.
> >
> > An earlier change had this function returning the type of lines that were 
> > in the file being read (ASCII or UCS2).  The way it is used, UCS2 output is 
> > expected, even when the file being read is ASCII. This change restores that 
> > behavior and documents it.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Jim Dailey
> ^^ your email address is missing from your sign-off, I think it's
> mandatory, but don't hold me to that.
> 

You are correct Ryan.

Jim,

If you have your name and email configured with git, then you can just
use '-s' with the git commit command to add the Signed-off-by.

Also, before submitting your patches, please run:

python BaseTools/Scripts/PatchCheck.py -1

If you have 2 patches to send, then you can use -2 rather than -1. You
can also run the script with python3.

-Jordan

> I haven't had chance to read and understand the code, but I can at
> least provide:
> 
> Tested-by: Ryan Harkin 
> 
> Thanks,
> Ryan.
> 
> > ---
> > diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c 
> > b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> > index 4b53c70..abff0d3 100644
> > --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> > +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> > @@ -4084,9 +4084,20 @@ ShellFileHandleReturnLine(
> >If the position upon start is 0, then the Ascii Boolean will be set.  
> > This should be
> >maintained and not changed for all operations with the same file.
> >
> > +  NOTE: LINES THAT ARE RETURNED BY THIS FUNCTION ARE UCS2, EVEN IF THE 
> > FILE BEING READ
> > +IS IN ASCII FORMAT.
> > +
> >@param[in]   HandleSHELL_FILE_HANDLE to read from.
> > -  @param[in, out]  BufferThe pointer to buffer to read into.
> > -  @param[in, out]  Size  The pointer to number of bytes in Buffer.
> > +  @param[in, out]  BufferThe pointer to buffer to read into. If 
> > this function
> > + returns EFI_SUCCESS, then on output 
> > Buffer will
> > + contain a UCS2 string, even if the file 
> > being
> > + read is ASCII.
> > +  @param[in, out]  Size  On input, pointer to number of bytes in 
> > Buffer.
> > + On output, unchanged unless Buffer is too 
> > small
> > + to contain the next line of the file. In 
> > that
> > + case Size is set to the number of bytes 
> > needed
> > + to hold the next line of the file (as a 
> > UCS2
> > + string, even if it is an ASCII file).
> >@param[in]   Truncate  If the buffer is large enough, this has 
> > no effect.
> >   If the buffer is is too small and 
> > Truncate is TRUE,
> >   the line will be truncated.
> > @@ -4165,43 +4176,27 @@ ShellFileHandleReadLine(
> >  //
> >  // if we have space save it...
> >  //
> > -if ((CountSoFar + 1) * CharSize < *Size){
> > +if ((CountSoFar+1)*sizeof(CHAR16) < *Size){
> >ASSERT(Buffer != NULL);
> > -  if (*Ascii) {
> > -((CHAR8*)Buffer)[CountSoFar] = (CHAR8) CharBuffer;
> > -((CHAR8*)Buffer)[CountSoFar+1] = '\0';
> > -  }
> > -  else {
> > -((CHAR16*)Buffer)[CountSoFar] = CharBuffer;
> > -((CHAR16*)Buffer)[CountSoFar+1] = CHAR_NULL;
> > -  }
> > +  ((CHAR16*)Buffer)[CountSoFar] = CharBuffer;
> > +  ((CHAR16*)Buffer)[CountSoFar+1] = CHAR_NULL;
> >  }
> >}
> >
> >//
> >// if we ran out of space tell when...
> >//
> > -  if (Status != EFI_END_OF_FILE){
> > -if ((CountSoFar + 1) * CharSize > *Size){
> > -  *Size = (CountSoFar + 1) * CharSize;
> > -  if (!Truncate) {
> > -gEfiShellProtocol->SetFilePosition(Handle, OriginalFilePosition);
> > -  } else {
> > -DEBUG((DEBUG_WARN, "The line was truncated in 
> > ShellFileHandleReadLine"));
> > -  }
> > -  return (EFI_BUFFER_TOO_SMALL);
> > -}
> > -
> > -if (*Ascii) {
> > -  if (CountSoFar && ((CHAR8*)Buffer)[CountSoFar - 1] == '\r') {
> > -((CHAR8*)Buffer)[CountSoFar - 1] = '\0';
> > -  }
> > -}
> > -else {
> > -  if (CountSoFar && Buffer[CountSoFar - 1] == L'\r') {
> > -Buffer[CountSoFar - 1] = CHAR_NULL;
> > -  }
> > +  if ((CountSoFar+1)*sizeof(CHAR16) > *Size){
> > +*Size = (CountSoFar+1)*sizeof(CHAR16);
> > +if (!Truncate) {
> > +  gEfiShellProtocol->SetFilePosition(Handle, OriginalFilePosition);
> > +} else {
> > +  DEBUG((DEBUG_WARN, "The line was truncated 

[edk2] [PATCH] EmbeddedPkg: Add ACPI6.0 GIC Definitions

2016-02-10 Thread Supreeth Venkatesh
Add ACPI6.0 macros for GIC Distributor Initialization, GICC Structure
Initialization and GIC Redistributor Initialization.

Contributed-under: TianoCore Contribution Agreement 1.0
Reported-by: Dennis Chen 
Signed-off-by: Supreeth Venkatesh 
Tested-by: Supreeth Venkatesh 
---
 EmbeddedPkg/Include/Library/AcpiLib.h | 20 
 1 file changed, 20 insertions(+)

diff --git a/EmbeddedPkg/Include/Library/AcpiLib.h 
b/EmbeddedPkg/Include/Library/AcpiLib.h
index 42710fd..e88e57f 100644
--- a/EmbeddedPkg/Include/Library/AcpiLib.h
+++ b/EmbeddedPkg/Include/Library/AcpiLib.h
@@ -38,6 +38,12 @@
 GicDistHwId, GicDistBase, GicDistVector, EFI_ACPI_RESERVED_DWORD \
   }
 
+#define EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT(GicDistHwId, GicDistBase, 
GicDistVector, GicVersion) \
+  { \
+EFI_ACPI_6_0_GICD, sizeof (EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE), 
EFI_ACPI_RESERVED_WORD, \
+GicDistHwId, GicDistBase, GicDistVector, GicVersion \
+  }
+
 // Note the parking protocol is configured by UEFI if required
 #define EFI_ACPI_5_0_GIC_STRUCTURE_INIT(GicId, AcpiCpuId, Flags, PmuIrq, 
GicBase) \
   { \
@@ -54,12 +60,26 @@
 GsivId, GicRBase, Mpidr
  \
   }
 
+// Note the parking protocol is configured by UEFI if required
+#define EFI_ACPI_6_0_GICC_STRUCTURE_INIT(GicId, AcpiCpuUid, Mpidr, Flags, 
PmuIrq,\
+GicBase, GicVBase, GicHBase, GsivId, GicRBase) 
  \
+  {
  \
+EFI_ACPI_6_0_GIC, sizeof (EFI_ACPI_6_0_GIC_STRUCTURE), 
EFI_ACPI_RESERVED_WORD,   \
+GicId, AcpiCpuUid, Flags, 0, PmuIrq, 0, GicBase, GicVBase, GicHBase,   
  \
+GsivId, GicRBase, Mpidr
  \
+  }
+
 #define EFI_ACPI_6_0_GIC_MSI_FRAME_INIT(GicMsiFrameId, PhysicalBaseAddress, 
Flags, SPICount, SPIBase) \
   { \
 EFI_ACPI_6_0_GIC_MSI_FRAME, sizeof (EFI_ACPI_6_0_GIC_MSI_FRAME_STRUCTURE), 
EFI_ACPI_RESERVED_WORD, \
 GicMsiFrameId, PhysicalBaseAddress, Flags, SPICount, SPIBase \
   }
 
+#define EFI_ACPI_6_0_GIC_REDISTRIBUTOR_INIT(RedisRegionAddr, RedisDiscLength) \
+  { \
+EFI_ACPI_6_0_GICR, sizeof (EFI_ACPI_6_0_GICR_STRUCTURE), 0, 
RedisRegionAddr, RedisDiscLength \
+  }
+
 //
 // SBSA Generic Watchdog
 //
-- 
2.6.3

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] BaseTools build number after transition to git

2016-02-10 Thread Laszlo Ersek
On 02/10/16 20:08, Felix Poludov wrote:
> Most of the EDKII tools support --version switch that returns tool version 
> and build number.
> We've been using SVN revision ID as a build number for all the tools.
> Have there been any discussions

Not to my knowledge.

> on what should be used as a build number in the new (and hopefully merrier)  
> EDKII Git world?

IMO, clearly the git commit hash of the current tree (possibly truncated
to 12 nibbles).

Unless official releases and matching git tagging are institutionalized
-- in that case, "git describe" is frequently used.

Thanks
Laszlo

> 
> Felix
> 
> 
> The information contained in this message may be confidential and proprietary 
> to American Megatrends, Inc.  This communication is intended to be read only 
> by the individual or entity to whom it is addressed or by their designee. If 
> the reader of this message is not the intended recipient, you are on notice 
> that any distribution of this message, in any form, is strictly prohibited.  
> Please promptly notify the sender by reply e-mail or by telephone at 
> 770-246-8600, and then delete or destroy all copies of the transmission.
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] BaseTools build number after transition to git

2016-02-10 Thread Andrew Fish

> On Feb 10, 2016, at 12:55 PM, Laszlo Ersek  wrote:
> 
> On 02/10/16 20:08, Felix Poludov wrote:
>> Most of the EDKII tools support --version switch that returns tool version 
>> and build number.
>> We've been using SVN revision ID as a build number for all the tools.
>> Have there been any discussions
> 
> Not to my knowledge.
> 
>> on what should be used as a build number in the new (and hopefully merrier)  
>> EDKII Git world?
> 
> IMO, clearly the git commit hash of the current tree (possibly truncated
> to 12 nibbles).
> 

I thought the --short output was shorter than that? 

> Unless official releases and matching git tagging are institutionalized
> -- in that case, "git describe" is frequently used.
> 

The other trick I've seen is to add a /D or -D CFLAG with the hash of the HEAD 
so you don't need an untracked file, or extra steps in the build process, to 
get this done.

Thanks,

Andrew Fish

> Thanks
> Laszlo
> 
>> 
>> Felix
>> 
>> 
>> The information contained in this message may be confidential and 
>> proprietary to American Megatrends, Inc.  This communication is intended to 
>> be read only by the individual or entity to whom it is addressed or by their 
>> designee. If the reader of this message is not the intended recipient, you 
>> are on notice that any distribution of this message, in any form, is 
>> strictly prohibited.  Please promptly notify the sender by reply e-mail or 
>> by telephone at 770-246-8600, and then delete or destroy all copies of the 
>> transmission.
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
>> 
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] BaseTools build number after transition to git

2016-02-10 Thread Laszlo Ersek
On 02/10/16 22:15, Andrew Fish wrote:
> 
>> On Feb 10, 2016, at 12:55 PM, Laszlo Ersek  wrote:
>>
>> On 02/10/16 20:08, Felix Poludov wrote:
>>> Most of the EDKII tools support --version switch that returns tool version 
>>> and build number.
>>> We've been using SVN revision ID as a build number for all the tools.
>>> Have there been any discussions
>>
>> Not to my knowledge.
>>
>>> on what should be used as a build number in the new (and hopefully merrier) 
>>>  EDKII Git world?
>>
>> IMO, clearly the git commit hash of the current tree (possibly truncated
>> to 12 nibbles).
>>
> 
> I thought the --short output was shorter than that? 

In my experience, git likes to work with 7 or 8 nibbles as a starting
point (IIRC 7 in "git rebase -i", 8 in "git blame"), and automatically
using longer hash prefixes if the 7 or 8 wouldn't be unique. (Again, I
don't have documentation or hard evidence about this, just what I seem
to recall.)

The Linux kernel recommends 12 nibbles for future proofing references in
commit messages and elsewhere. What suffices for Linux should suffice
for everything else (TM).

(Well, maybe not for Facebook, but they use Mercurial anyway.)

https://www.kernel.org/doc/Documentation/SubmittingPatches

Hm, this actually prompted me now to set core.abbrev = 12. :)

I also stumbled upon
.

[snip]

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Mangefeste, Tony
We need an issue tracking system for Tiano.  

The built-in issue tracking system that comes with GitHub isn't sufficient to 
satisfy a key requirement.  There needs to be support for multiple 
Tianocore-related programs.  As you know Intel has a system today that's 
internal to Intel where we track issues.  That does not meet the needs of the 
community.  And to help improve transparency, and better engage with the 
community I'm driving the discussion and bring up of a bug tracking system.

The goal is to have one operational by March 21, 2016 (WW13).  We're 6 weeks 
and counting from that deadline.  I'm interested in community feedback, 
gathering requirements, and feedback on proposals for which system to use.  

We're going to transform issue tracking on Tianocore a transparent, community 
driven behavior.

Key requirements for the system include (but not limited to):
* OSS (does not have to be free)
* Ability to bulk import/export databases, data (CSV)
* Secure, ability to shield sensitive issues
* Group credential management
* Supports mobile views (phone/tablet)
* Ability to generate reports
* Can be used to generate quick tasks for community members (e.g. Find a Task)
* Integrate with GitHub

I appreciate anyone's time and passion on this.  Let me know if you want to 
participate in such a task force.

BRs,
Tony



___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Laszlo Ersek
On 02/10/16 22:45, Mangefeste, Tony wrote:
> We need an issue tracking system for Tiano.  
> 
> The built-in issue tracking system that comes with GitHub isn't
> sufficient to satisfy a key requirement.  There needs to be support
> for multiple Tianocore-related programs.

I agree. The Bugzilla instance that Red Hat operates supports many
pieces of metadata, and "product" and "component" are two key fields. I
would find it useful if the metadata reflected even the edk2 module in
question, at least for long-established modules (library instances and
drivers). This would also enable fine-grained automatic assignment to a
maintainer.

> As you know Intel has a
> system today that's internal to Intel where we track issues.  That
> does not meet the needs of the community.  And to help improve
> transparency, and better engage with the community I'm driving the
> discussion and bring up of a bug tracking system.
> 
> The goal is to have one operational by March 21, 2016 (WW13).  We're
> 6 weeks and counting from that deadline.  I'm interested in community
> feedback, gathering requirements, and feedback on proposals for which
> system to use.

- Launchpad
  

  Cons:
  - proportional width font
  - forcefully reflows comments, even if it means breaking git commit
hashes into several parts, breaking copy & paste on double-click
  - truncates long comments in the "full bug view". If you'd like to
read a careful, detailed comment to end, you have to open the
comment in isolation. And then you can't look at the other comments
at the same time.

- Github issue tracker
  

  Pros:
  - some integration with the code repository
(automatic highlighting of commit hashes, for example; link leads to
commit when clicked)

  Cons:
  - social interaction oriented, with @person style "callouts"
  - doesn't support binary attachments
  - very basic, lacking metadata
  - bug data is held hostage, no way to mass-export it in an open format
/ schema
  - proportional width font, with MarkDown enabled by default
(interferes with ASCII diagrams and code pasting)
  - lackluster integration with email (can't send updates of one's own
bug actions)
  - unwieldy permalinks for comments, even from within other comments
on the same bug
  - allows anyone to reedit their own posted comments (that's a
negative, yes)

- Bugzilla
  
  
  
  

  Pros:
  - the gold standard for serious free and open source software
  - heavy local customization possible
  - splendid outgoing emails, including about your own actions;
metadata changes rendered in simple ASCII tables
  - heaps of metadata
  - public and private bugs
  - public and private comments and attachments
  - access to any individual bug can be restricted to specific groups
(partners, customers, security response team)
  - flags for release planning and stakeholder signoff
  - well defined and customizable bug states and transitions
  - monospace font for comments
  - simple permalinks (can be hand-constructed if you know the bug nr
and comment nr you want to refer to)
  - no markup beyond simple highlighting (as links) of a few patterns,
like "bug NNN", "comment ZZZ", and "bug PPP comment QQQ".
  - bug aliases (like CVE-2016-N) that are highlighted and resolved
to their respective bug numbers
  - dependency tracking between bugs, displayable in a tree-like view as
well
  - support for cloning bugs for other components and products
  - elaborate queries can be composed and saved
  - you can watch people, regardless on what bug they comment or work on
  - supposed to mass-export bug data in XML
  - ability to collapse and expand individual and all comments
  - comment-specific "reply" links prime a new comment with the comment
being replied to *correctly quoted*
  - ability to CC yourself and *others* on bugs
  - feedback can be requested from specific people or groups by setting
a conspicuous and *formal* NEEDINFO request on them --> triggers
separate email. NEEDINFO remains set until requestee answers or is
explicitly cleared.
  - all metadata changes are tracked with timestamps in-line with the
comments
  - comments are read-only once posted, unless special privileges are
granted
  - textual template can be specified for all new bug reports
  - ...

  Cons:
  - requires bug reporters to think first (oh wait, that's a pro)
  - HTTP request/response oriented, only minimally AJAX-y (oh wait,
that's a pro too!)
  - occasionally slow
  - requires serious, dedicated maintenance and/or perf tuning

> 
> We're going to transform issue tracking on Tianocore a transparent, community 
> driven behavior.
> 
> Key requirements for the system include (but not limited to):
> * OSS (does not have to be free)
> * Ability to

Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Mangefeste, Tony
I've gone down the track of using Bugzilla as well.  Aside from the massive 
list of pros listed below, gathering any other preference is welcomed.  I have 
used Bugzilla, but never been a maintainer on it.  We do have some resources 
lined up for management of Bugzilla, if needed, so that's not a barrier.  Of 
course, the devil's in the details.

So in short, Bugzilla is top of my mind right now.  I'm looking at other OSS 
projects and seeing what they use.  If anyone here sees one not listed below or 
has an opinion that they care to express please do so.  On the mobility view, 
I'll try to play around with that and see how it looks on my mobile devices. 

Welcome to real-time decision making, thought process spewing.

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Wednesday, February 10, 2016 2:44 PM
To: Mangefeste, Tony 
Cc: edk2-devel@lists.01.org 
Subject: Re: [edk2] Task: Issue Tracking System for Tianocore

On 02/10/16 22:45, Mangefeste, Tony wrote:
> We need an issue tracking system for Tiano.  
> 
> The built-in issue tracking system that comes with GitHub isn't 
> sufficient to satisfy a key requirement.  There needs to be support 
> for multiple Tianocore-related programs.

I agree. The Bugzilla instance that Red Hat operates supports many pieces of 
metadata, and "product" and "component" are two key fields. I would find it 
useful if the metadata reflected even the edk2 module in question, at least for 
long-established modules (library instances and drivers). This would also 
enable fine-grained automatic assignment to a maintainer.

> As you know Intel has a
> system today that's internal to Intel where we track issues.  That 
> does not meet the needs of the community.  And to help improve 
> transparency, and better engage with the community I'm driving the 
> discussion and bring up of a bug tracking system.
> 
> The goal is to have one operational by March 21, 2016 (WW13).  We're
> 6 weeks and counting from that deadline.  I'm interested in community 
> feedback, gathering requirements, and feedback on proposals for which 
> system to use.

- Launchpad
  

  Cons:
  - proportional width font
  - forcefully reflows comments, even if it means breaking git commit
hashes into several parts, breaking copy & paste on double-click
  - truncates long comments in the "full bug view". If you'd like to
read a careful, detailed comment to end, you have to open the
comment in isolation. And then you can't look at the other comments
at the same time.

- Github issue tracker
  

  Pros:
  - some integration with the code repository
(automatic highlighting of commit hashes, for example; link leads to
commit when clicked)

  Cons:
  - social interaction oriented, with @person style "callouts"
  - doesn't support binary attachments
  - very basic, lacking metadata
  - bug data is held hostage, no way to mass-export it in an open format
/ schema
  - proportional width font, with MarkDown enabled by default
(interferes with ASCII diagrams and code pasting)
  - lackluster integration with email (can't send updates of one's own
bug actions)
  - unwieldy permalinks for comments, even from within other comments
on the same bug
  - allows anyone to reedit their own posted comments (that's a
negative, yes)

- Bugzilla
  
  
  
  

  Pros:
  - the gold standard for serious free and open source software
  - heavy local customization possible
  - splendid outgoing emails, including about your own actions;
metadata changes rendered in simple ASCII tables
  - heaps of metadata
  - public and private bugs
  - public and private comments and attachments
  - access to any individual bug can be restricted to specific groups
(partners, customers, security response team)
  - flags for release planning and stakeholder signoff
  - well defined and customizable bug states and transitions
  - monospace font for comments
  - simple permalinks (can be hand-constructed if you know the bug nr
and comment nr you want to refer to)
  - no markup beyond simple highlighting (as links) of a few patterns,
like "bug NNN", "comment ZZZ", and "bug PPP comment QQQ".
  - bug aliases (like CVE-2016-N) that are highlighted and resolved
to their respective bug numbers
  - dependency tracking between bugs, displayable in a tree-like view as
well
  - support for cloning bugs for other components and products
  - elaborate queries can be composed and saved
  - you can watch people, regardless on what bug they comment or work on
  - supposed to mass-export bug data in XML
  - ability to collapse and expand individual and all comments
  - comment-specific "reply" links prime a new comment with the comment
being replied to *correctly quoted*
  - ability to CC

Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Laszlo Ersek
On 02/10/16 23:44, Laszlo Ersek wrote:

> - Bugzilla
>   
>   
>   
>   

Sorry, forgot to mention this one, even though it is pretty widely used:
. It tracks bugs for
.

Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH] ArmPkg: DefaultExceptionHandler fixes for use with DxeCore

2016-02-10 Thread Cohen, Eugene
Modify the DefaultExceptionHandler (uefi-variant) so it can be used by
DxeCore (via CpuExceptionHandlerLib) where the debug info table is not
yet published at library constructor time.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eugene Cohen 
---
 .../AArch64/DefaultExceptionHandler.c  |  2 -
 .../Arm/DefaultExceptionHandler.c  |  2 -
 .../DefaultExceptionHandlerLib.inf |  1 -
 .../DefaultExceptionHandlerUefi.c  | 43 +-
 4 files changed, 10 insertions(+), 38 deletions(-)

diff --git 
a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c 
b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c
index 49d36ed..37fd578 100644
--- 
a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c
+++ 
b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c
@@ -27,8 +27,6 @@
 #include 
 #include 
 
-EFI_DEBUG_IMAGE_INFO_TABLE_HEADER *gDebugImageTableHeader = NULL;
-
 STATIC CHAR8 *gExceptionTypeString[] = {
   "Synchronous",
   "IRQ",
diff --git 
a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c 
b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c
index 179fc22..aece263 100644
--- a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c
+++ b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c
@@ -27,8 +27,6 @@
 #include 
 #include 
 
-EFI_DEBUG_IMAGE_INFO_TABLE_HEADER *gDebugImageTableHeader = NULL;
-
 typedef struct {
   UINT32  BIT;
   CHAR8   Char;
diff --git 
a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf 
b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf
index da8190b..5d3ce89 100644
--- a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf
+++ b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf
@@ -20,7 +20,6 @@
   MODULE_TYPE= UEFI_DRIVER
   VERSION_STRING = 1.0
   LIBRARY_CLASS  = DefaultExceptionHandlerLib
-  CONSTRUCTOR= DefaultExceptionHandlerConstructor
 
 [Sources.common]
   DefaultExceptionHandlerUefi.c
diff --git 
a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerUefi.c 
b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerUefi.c
index b2d630c..02e6c9a 100644
--- a/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerUefi.c
+++ b/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerUefi.c
@@ -18,34 +18,6 @@
 
 #include 
 
-extern EFI_DEBUG_IMAGE_INFO_TABLE_HEADER *gDebugImageTableHeader;
-
-/**
-  The constructor function caches EFI Debug table information for use in the 
exception handler.
-
-
-  @param  ImageHandle   The firmware allocated handle for the EFI image.
-  @param  SystemTable   A pointer to the EFI System Table.
-
-  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
-
-**/
-EFI_STATUS
-EFIAPI
-DefaultExceptionHandlerConstructor (
-  IN EFI_HANDLEImageHandle,
-  IN EFI_SYSTEM_TABLE  *SystemTable
-  )
-{
-  EFI_STATUS  Status;
-
-  Status = EfiGetSystemConfigurationTable (&gEfiDebugImageInfoTableGuid, (VOID 
**)&gDebugImageTableHeader);
-  if (EFI_ERROR (Status)) {
-gDebugImageTableHeader = NULL;
-  }
-  return Status;
-}
-
 /**
   Use the EFI Debug Image Table to lookup the FaultAddress and find which 
PE/COFF image
   it came from. As long as the PE/COFF image contains a debug directory entry a
@@ -67,17 +39,22 @@ GetImageName (
   OUT UINTN  *PeCoffSizeOfHeaders
   )
 {
-  EFI_DEBUG_IMAGE_INFO  *DebugTable;
-  UINTN Entry;
-  CHAR8 *Address;
+  EFI_STATUS  Status;
+  EFI_DEBUG_IMAGE_INFO_TABLE_HEADER   *DebugTableHeader;
+  EFI_DEBUG_IMAGE_INFO*DebugTable;
+  UINTN   Entry;
+  CHAR8   *Address;
+
+  Status = EfiGetSystemConfigurationTable(&gEfiDebugImageInfoTableGuid, (VOID 
**)&DebugTableHeader);
+  if (EFI_ERROR(Status)) return NULL;
 
-  DebugTable = gDebugImageTableHeader->EfiDebugImageInfoTable;
+  DebugTable = DebugTableHeader->EfiDebugImageInfoTable;
   if (DebugTable == NULL) {
 return NULL;
   }
 
   Address = (CHAR8 *)(UINTN)FaultAddress;
-  for (Entry = 0; Entry < gDebugImageTableHeader->TableSize; Entry++, 
DebugTable++) {
+  for (Entry = 0; Entry < DebugTableHeader->TableSize; Entry++, DebugTable++) {
 if (DebugTable->NormalImage != NULL) {
   if ((DebugTable->NormalImage->ImageInfoType == 
EFI_DEBUG_IMAGE_INFO_TYPE_NORMAL) &&
   (DebugTable->NormalImage->LoadedImageProtocolInstance != NULL)) {
-- 
1.9.5.msysgit.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Laszlo Ersek
On 02/10/16 23:59, Mangefeste, Tony wrote:
> I've gone down the track of using Bugzilla as well.  Aside from the
> massive list of pros listed below, gathering any other preference is
> welcomed.  I have used Bugzilla, but never been a maintainer on it.
> We do have some resources lined up for management of Bugzilla, if
> needed, so that's not a barrier.  Of course, the devil's in the
> details.
> 
> So in short, Bugzilla is top of my mind right now.  I'm looking at
> other OSS projects and seeing what they use.  If anyone here sees one
> not listed below or has an opinion that they care to express please
> do so.  On the mobility view, I'll try to play around with that and
> see how it looks on my mobile devices.
> 
> Welcome to real-time decision making, thought process spewing.

I recall that Linaro uses JIRA:
https://cards.linaro.org/

Oh wait, there seems to be a new URL (still JIRA):
https://projects.linaro.org

I am (was?) subscribed to a minimal set of CARDs only, in the first
system; I don't have any real experience with JIRA. Ard, Leif, can you
please share your thoughts?

Thanks!
Laszlo

> 
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com] 
> Sent: Wednesday, February 10, 2016 2:44 PM
> To: Mangefeste, Tony 
> Cc: edk2-devel@lists.01.org 
> Subject: Re: [edk2] Task: Issue Tracking System for Tianocore
> 
> On 02/10/16 22:45, Mangefeste, Tony wrote:
>> We need an issue tracking system for Tiano.  
>>
>> The built-in issue tracking system that comes with GitHub isn't 
>> sufficient to satisfy a key requirement.  There needs to be support 
>> for multiple Tianocore-related programs.
> 
> I agree. The Bugzilla instance that Red Hat operates supports many pieces of 
> metadata, and "product" and "component" are two key fields. I would find it 
> useful if the metadata reflected even the edk2 module in question, at least 
> for long-established modules (library instances and drivers). This would also 
> enable fine-grained automatic assignment to a maintainer.
> 
>> As you know Intel has a
>> system today that's internal to Intel where we track issues.  That 
>> does not meet the needs of the community.  And to help improve 
>> transparency, and better engage with the community I'm driving the 
>> discussion and bring up of a bug tracking system.
>>
>> The goal is to have one operational by March 21, 2016 (WW13).  We're
>> 6 weeks and counting from that deadline.  I'm interested in community 
>> feedback, gathering requirements, and feedback on proposals for which 
>> system to use.
> 
> - Launchpad
>   
> 
>   Cons:
>   - proportional width font
>   - forcefully reflows comments, even if it means breaking git commit
> hashes into several parts, breaking copy & paste on double-click
>   - truncates long comments in the "full bug view". If you'd like to
> read a careful, detailed comment to end, you have to open the
> comment in isolation. And then you can't look at the other comments
> at the same time.
> 
> - Github issue tracker
>   
> 
>   Pros:
>   - some integration with the code repository
> (automatic highlighting of commit hashes, for example; link leads to
> commit when clicked)
> 
>   Cons:
>   - social interaction oriented, with @person style "callouts"
>   - doesn't support binary attachments
>   - very basic, lacking metadata
>   - bug data is held hostage, no way to mass-export it in an open format
> / schema
>   - proportional width font, with MarkDown enabled by default
> (interferes with ASCII diagrams and code pasting)
>   - lackluster integration with email (can't send updates of one's own
> bug actions)
>   - unwieldy permalinks for comments, even from within other comments
> on the same bug
>   - allows anyone to reedit their own posted comments (that's a
> negative, yes)
> 
> - Bugzilla
>   
>   
>   
>   
> 
>   Pros:
>   - the gold standard for serious free and open source software
>   - heavy local customization possible
>   - splendid outgoing emails, including about your own actions;
> metadata changes rendered in simple ASCII tables
>   - heaps of metadata
>   - public and private bugs
>   - public and private comments and attachments
>   - access to any individual bug can be restricted to specific groups
> (partners, customers, security response team)
>   - flags for release planning and stakeholder signoff
>   - well defined and customizable bug states and transitions
>   - monospace font for comments
>   - simple permalinks (can be hand-constructed if you know the bug nr
> and comment nr you want to refer to)
>   - no markup beyond simple highlighting (as links) of a few patterns,
> like "bug NNN", "comment ZZZ", and "bug PPP comment QQQ".
>   - bug aliases (like CVE-2016-N) that are highlighted 

Re: [edk2] How do you get current timestamp in ms or ns ?

2016-02-10 Thread Estelle Yeh
I am still stuck on those issues... It's been fun to work on developing my
UEFI library but I'm hitting a wall. It's probably very obvious for an
experience developer.

- Simple question: What's the difference between
DuetPkg\Library\DuetTimerLib\X86TimerLib.c and
MdePkg\Library\SecPeiDxeTimerLibCpu\X86TimerLib.c?

- What are the steps that I need to do to use functions that are in (for
example) MdePkg\Library\SecPeiDxeTimerLibCpu\X86TimerLib.c, meaning what do
I include and in which files?

- I tried adding
TimerLib|MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCpu.inf in
DuetPkgX64.dsc but I get the error: [Errno 2] No such file or directory:
'?\\c:\\edk2\\Build\\DuetPkgX64\\RELEASE_VS2012x86\\X64\\MdePkg\\Library\\SecPeiDxeTimerLibCpu\\SecPeiDxeTimerLibCpu\\OUTPUT\\SecPeiDxeTimerLibCpu.lib'"
Any ideas what can cause it?

If anybody could point me to the documentation that would answer my basic
questions, I would greatly appreciate it.

Thanks for any help.
Estelle

On Fri, Feb 5, 2016 at 2:35 PM, Estelle Yeh  wrote:

> Thanks a lot Andrew.
>
> - I built the DUET image according to the ReadMe that you mentioned but
> ran into issues while creating the bootable disk. Is that one needed?
>
> - Where should I see the DEBUG prints from the DXE core exactly?
>
> Thanks,
> Estelle
>
> On Thu, Feb 4, 2016 at 2:42 PM, Andrew Fish  wrote:
>
>>
>> On Feb 4, 2016, at 2:35 PM, Estelle Yeh  wrote:
>>
>> Thanks for the tips, Andrew.
>>
>> - It seems to be using DuetPkg/Library/DuetTimerLib/X86TimerLib.c, and I
>> get this from the report file:
>> c:\edk2\DuetPkg\Library\DuetTimerLib\DuetTimerLib.inf
>> {TimerLib}
>>
>>
>> That seems a bit strange. A Duet driver assumes it was boot on a system
>> that already had firmware run. So that might be your problem.
>> https://github.com/tianocore/edk2/blob/master/DuetPkg/ReadMe.txt
>>
>>
>> - I added the "INF  MdeModulePkg/Universal/TimestampDxe/TimestampDxe.inf"
>> entry to the .fdf but I still get "Not found" returned from LocateProtocol
>> (using %r in a Print). Does it matter where that TimestampDxe.inf entry is
>> placed in the .dsc and .fdf files?
>>
>>
>> You have to do both. DSC builds, FDF puts it in FV/FD. If you remove the
>> DSC entry it may seem to work, but after you do your 1st clean it will no
>> longer work.
>>
>> Who should be calling TimestampDriverInitialize()? Better question: How
>> can I make sure that my platform is producing that protocol? I don't see
>> any listed in the report file.
>>
>>
>> If it is in a FV that the DXE Core knows about it will get dispatched.
>> That driver has a Dependency Expression (depex) of TRUE so it should
>> dispatch early in DXE. You should see DEBUG prints from the DXE core about
>> what is getting dispatched. It will either show the driver name or the
>> FILE_GUID from the INF C10194E7-DEB2-4AF4-9EEE-BFFDE4D7D4C7
>>
>> Thanks,
>>
>> Andrew Fish
>>
>> Thanks,
>>
>> Estelle
>>
>>
>>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Jordan Justen
On 2016-02-10 15:08:16, Laszlo Ersek wrote:
> On 02/10/16 23:59, Mangefeste, Tony wrote:
> > I've gone down the track of using Bugzilla as well.  Aside from the
> > massive list of pros listed below, gathering any other preference is
> > welcomed.  I have used Bugzilla, but never been a maintainer on it.
> > We do have some resources lined up for management of Bugzilla, if
> > needed, so that's not a barrier.  Of course, the devil's in the
> > details.
> > 
> > So in short, Bugzilla is top of my mind right now.  I'm looking at
> > other OSS projects and seeing what they use.  If anyone here sees one
> > not listed below or has an opinion that they care to express please
> > do so.  On the mobility view, I'll try to play around with that and
> > see how it looks on my mobile devices.
> > 
> > Welcome to real-time decision making, thought process spewing.
> 
> I recall that Linaro uses JIRA:
> https://cards.linaro.org/
> 
> Oh wait, there seems to be a new URL (still JIRA):
> https://projects.linaro.org
> 
> I am (was?) subscribed to a minimal set of CARDs only, in the first
> system; I don't have any real experience with JIRA. Ard, Leif, can you
> please share your thoughts?
> 

As an user of GitHub, Bugzilla and Jira.

Jira: No advantages over Bugzilla. Bigger, and slower than bugzilla. I
think it tries to be flashier than bugzilla (and bugzilla is
admittedly a bit dated looking).

Bugzilla: Works fine. Pain in the but to administer. You have to pay
for hosting.

GitHub: Easy, but simplistic.

I think GitHub is the easier choice, and is good enough. Users will
likely expect to find it used since the source is currently hosted
there.

Actually, we are currently using GitHub, so we would be talking about
a change if we moved to anything else. We have 17 open and 18 closed
bugs currently which far surpasses the activity in the bug tracking
'trac' system that we had on sourceforge for several years. :) (And, I
think the same for the collabnet hosted bugs before that.)

I think bugzilla is an okay choice if it can be paid for (with several
years of funding...). I guess I'd recommend finding a service
dedicated to hosting bugzilla, and not try to set up a server and
self-host. It'd be a little annoying to have a separate bug tracker
account.

My vote is to stick with GitHub for now.

-Jordan

> 
> > 
> > -Original Message-
> > From: Laszlo Ersek [mailto:ler...@redhat.com] 
> > Sent: Wednesday, February 10, 2016 2:44 PM
> > To: Mangefeste, Tony 
> > Cc: edk2-devel@lists.01.org 
> > Subject: Re: [edk2] Task: Issue Tracking System for Tianocore
> > 
> > On 02/10/16 22:45, Mangefeste, Tony wrote:
> >> We need an issue tracking system for Tiano.  
> >>
> >> The built-in issue tracking system that comes with GitHub isn't 
> >> sufficient to satisfy a key requirement.  There needs to be support 
> >> for multiple Tianocore-related programs.
> > 
> > I agree. The Bugzilla instance that Red Hat operates supports many pieces 
> > of metadata, and "product" and "component" are two key fields. I would find 
> > it useful if the metadata reflected even the edk2 module in question, at 
> > least for long-established modules (library instances and drivers). This 
> > would also enable fine-grained automatic assignment to a maintainer.
> > 
> >> As you know Intel has a
> >> system today that's internal to Intel where we track issues.  That 
> >> does not meet the needs of the community.  And to help improve 
> >> transparency, and better engage with the community I'm driving the 
> >> discussion and bring up of a bug tracking system.
> >>
> >> The goal is to have one operational by March 21, 2016 (WW13).  We're
> >> 6 weeks and counting from that deadline.  I'm interested in community 
> >> feedback, gathering requirements, and feedback on proposals for which 
> >> system to use.
> > 
> > - Launchpad
> >   
> > 
> >   Cons:
> >   - proportional width font
> >   - forcefully reflows comments, even if it means breaking git commit
> > hashes into several parts, breaking copy & paste on double-click
> >   - truncates long comments in the "full bug view". If you'd like to
> > read a careful, detailed comment to end, you have to open the
> > comment in isolation. And then you can't look at the other comments
> > at the same time.
> > 
> > - Github issue tracker
> >   
> > 
> >   Pros:
> >   - some integration with the code repository
> > (automatic highlighting of commit hashes, for example; link leads to
> > commit when clicked)
> > 
> >   Cons:
> >   - social interaction oriented, with @person style "callouts"
> >   - doesn't support binary attachments
> >   - very basic, lacking metadata
> >   - bug data is held hostage, no way to mass-export it in an open format
> > / schema
> >   - proportional width font, with MarkDown enabled by default
> > (interferes with ASCII diagrams and code pasting)
> >   - lackluster integrati

[edk2] github development process

2016-02-10 Thread El-Haj-Mahmoud, Samer
So I have been itching to submit patches after the transition to github, when I 
stumbled across this. It looks like the development process described in: 
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process
 , does not match the process described in: 
https://raw.githubusercontent.com/tianocore/edk2/master/MdePkg/Contributions.txt

I followed the process in the wiki, and ended up creating a pull request for 
tiancore/edk for my contribution (https://github.com/tianocore/edk2/pull/55). I 
was also going to send a patch to EDK2 list (as the wiki suggests). Then I 
quickly realized that the maintainers are still working only with e-mail 
patches, and not pull requests, as clear from the conversations in 
closed/rejected pull requests:

https://github.com/tianocore/edk2/pull/45
https://github.com/tianocore/edk2/pull/51
https://github.com/tianocore/edk2/pull/41
etc..


I have no problem of sticking with the edk2-devel e-mail patches like we have 
been doing (all of my team from HPE contributes that way). But if that is the 
only/proper way to submit contributions, then the wiki need to be updated to 
drop the pull requests steps.

Thanks,
--Samer



___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH 1/2] MdePkg: Update Http11 with additional useful definitions

2016-02-10 Thread Samer El-Haj-Mahmoud
Add additional HTTP 1.1 definitions that are useful in HTTP
applications, such as User-Agent, Location, and HTTP Version string with
and without the CRLF.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Samer El-Haj-Mahmoud 
---
 MdePkg/Include/IndustryStandard/Http11.h | 51 
 1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/Http11.h 
b/MdePkg/Include/IndustryStandard/Http11.h
index 6c22f94..44de8c8 100644
--- a/MdePkg/Include/IndustryStandard/Http11.h
+++ b/MdePkg/Include/IndustryStandard/Http11.h
@@ -3,7 +3,7 @@
 
   This file contains common HTTP 1.1 definitions from RFC 2616 

-  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
+  (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -24,8 +24,8 @@
 /// The version of an HTTP message is indicated by an HTTP-Version field
 /// in the first line of the message.
 ///
-#define HTTP_VERSION"HTTP/1.1"
-
+#define HTTP_VERSION_STR   "HTTP/1.1"
+#define HTTP_VERSION_CRLF_STR  " HTTP/1.1\r\n"
 
 ///
 /// HTTP Request Method definitions
@@ -43,7 +43,11 @@
 #define HTTP_METHOD_CONNECT "CONNECT"
 #define HTTP_METHOD_PATCH   "PATCH"
 
-#define HTTP_METHOD_MAXIMUM_LEN  sizeof ("CONNECT")
+///
+/// Connect method has maximum length according to EFI_HTTP_METHOD defined in
+/// UEFI2.5 spec so use this.
+///
+#define HTTP_METHOD_MAXIMUM_LEN  sizeof (HTTP_METHOD_CONNECT)
 
 ///
 /// Accept Request Header
@@ -106,7 +110,7 @@
 #define HTTP_CONTENT_ENCODING_GZIP "gzip"  /// Content-Encoding: GNU 
zip format (described in RFC 1952).
 #define HTTP_CONTENT_ENCODING_COMPRESS "compress"  /// encoding format 
produced by the common UNIX file compression program "compress". 
 #define HTTP_CONTENT_ENCODING_DEFLATE  "deflate"   /// The "zlib" format 
defined in RFC 1950 in combination with the "deflate" 
-/// compression mechanism 
described in RFC 1951.
+   /// compression mechanism 
described in RFC 1951.
 
 
 ///
@@ -152,13 +156,37 @@
 
 
 ///
+/// User Agent Request Header
+/// 
+/// The User-Agent request-header field contains information about the user 
agent originating 
+/// the request. This is for statistical purposes, the tracing of protocol 
violations, and 
+/// automated recognition of user agents for the sake of tailoring responses 
to avoid 
+/// particular user agent limitations. User agents SHOULD include this field 
with requests. 
+/// The field can contain multiple product tokens and comments identifying the 
agent and any 
+/// subproducts which form a significant part of the user agent. 
+/// By convention, the product tokens are listed in order of their 
significance for 
+/// identifying the application.
+///
+#define HTTP_HEADER_USER_AGENT "User-Agent"
+
+///
 /// Host Request Header
 ///
 /// The Host request-header field specifies the Internet host and port number 
of the resource 
 /// being requested, as obtained from the original URI given by the user or 
referring resource 
 ///
-#define  HTTP_HEADER_HOST  "Host"
+#define HTTP_HEADER_HOST  "Host"
 
+///
+/// Location Response Header
+/// 
+/// The Location response-header field is used to redirect the recipient to a 
location other than 
+/// the Request-URI for completion of the request or identification of a new 
resource. 
+/// For 201 (Created) responses, the Location is that of the new resource 
which was created by 
+/// the request. For 3xx responses, the location SHOULD indicate the server's 
preferred URI for 
+/// automatic redirection to the resource. The field value consists of a 
single absolute URI.
+///
+#define HTTP_HEADER_LOCATION   "Location"
 
 ///
 /// The If-Match request-header field is used with a method to make it 
conditional.
@@ -170,7 +198,7 @@
 /// to prevent inadvertent modification of the wrong version of a resource. 
 /// As a special case, the value "*" matches any current entity of the 
resource.
 ///
-#define  HTTP_HEADER_IF_MATCH  "If-Match"
+#define HTTP_HEADER_IF_MATCH  "If-Match"
 
 
 ///
@@ -182,7 +210,7 @@
 /// to prevent a method (e.g. PUT) from inadvertently modifying an existing 
resource when the 
 /// client believes that the resource does not exist.
 ///
-#define  HTTP_HEADER_IF_NONE_MATCH "If-None-Match"
+#define HTTP_HEADER_IF_NONE_MATCH "If-None-Match"
 
 
 
@@ -192,17 +220,16 @@
 /// containing the authentication information of the user agent for
 /// the realm of the resource being requested.
 ///
-#define  HTTP_HEADER_AUTHORIZATION "Authorization"
+#define HTTP_HEADER_AUTHORIZATION "Authorization"
 
 ///
 /// ETAG 

[edk2] [PATCH 2/2] NetworkPkg: Use Http11 definitions in HttpDxe and HttpBootDxe

2016-02-10 Thread Samer El-Haj-Mahmoud
Change HttpDxe and HttpBootDxe to use the standard definitions from
Http11.h instead of private duplicate definitions.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Samer El-Haj-Mahmoud 
---
 NetworkPkg/HttpBootDxe/HttpBootClient.c |  7 ---
 NetworkPkg/HttpBootDxe/HttpBootClient.h |  4 +---
 NetworkPkg/HttpBootDxe/HttpBootDxe.h|  2 ++
 NetworkPkg/HttpDxe/HttpDriver.h |  3 ++-
 NetworkPkg/HttpDxe/HttpImpl.h   | 11 +--
 NetworkPkg/HttpDxe/HttpProto.c  | 16 ++--
 6 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c 
b/NetworkPkg/HttpBootDxe/HttpBootClient.c
index dd835c4..2ccac8c 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
@@ -2,6 +2,7 @@
   Implementation of the boot file download function.
 
 Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
+(C) Copyright 2016 Hewlett Packard Enterprise Development LP
 This program and the accompanying materials are licensed and made available 
under 
 the terms and conditions of the BSD License that accompanies this 
distribution.  
 The full text of the license may be found at
@@ -807,7 +808,7 @@ HttpBootGetBootFile (
   }
   Status = HttpBootSetHeader (
  HttpIoHeader,
- HTTP_FIELD_NAME_HOST,
+ HTTP_HEADER_HOST,
  HostName
  );
   FreePool (HostName);
@@ -820,7 +821,7 @@ HttpBootGetBootFile (
   //
   Status = HttpBootSetHeader (
  HttpIoHeader,
- HTTP_FIELD_NAME_ACCEPT,
+ HTTP_HEADER_ACCEPT,
  "*/*"
  );
   if (EFI_ERROR (Status)) {
@@ -832,7 +833,7 @@ HttpBootGetBootFile (
   //
   Status = HttpBootSetHeader (
  HttpIoHeader,
- HTTP_FIELD_NAME_USER_AGENT,
+ HTTP_HEADER_USER_AGENT,
  HTTP_USER_AGENT_EFI_HTTP_BOOT
  );
   if (EFI_ERROR (Status)) {
diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.h 
b/NetworkPkg/HttpBootDxe/HttpBootClient.h
index e618316..b929fa7 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.h
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.h
@@ -2,6 +2,7 @@
   Declaration of the boot file download function.
 
 Copyright (c) 2015, Intel Corporation. All rights reserved.
+(C) Copyright 2016 Hewlett Packard Enterprise Development LP
 This program and the accompanying materials are licensed and made available 
under 
 the terms and conditions of the BSD License that accompanies this 
distribution.  
 The full text of the license may be found at
@@ -18,9 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #define HTTP_BOOT_REQUEST_TIMEOUT5000  // 5 seconds in uints 
of millisecond.
 #define HTTP_BOOT_BLOCK_SIZE 1500
 
-#define HTTP_FIELD_NAME_USER_AGENT   "User-Agent"
-#define HTTP_FIELD_NAME_HOST "Host"
-#define HTTP_FIELD_NAME_ACCEPT   "Accept"
 
 
 #define HTTP_USER_AGENT_EFI_HTTP_BOOT"UefiHttpBoot/1.0"
diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.h 
b/NetworkPkg/HttpBootDxe/HttpBootDxe.h
index 452c8f4..fb6ab1a 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDxe.h
+++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.h
@@ -2,6 +2,7 @@
   UEFI HTTP boot driver's private data structure and interfaces declaration.
 
 Copyright (c) 2015, Intel Corporation. All rights reserved.
+(C) Copyright 2016 Hewlett Packard Enterprise Development LP
 This program and the accompanying materials are licensed and made available 
under 
 the terms and conditions of the BSD License that accompanies this 
distribution.  
 The full text of the license may be found at
@@ -16,6 +17,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #define __EFI_HTTP_BOOT_DXE_H__
 
 #include 
+#include 
 
 //
 // Libraries
diff --git a/NetworkPkg/HttpDxe/HttpDriver.h b/NetworkPkg/HttpDxe/HttpDriver.h
index 138f56c..4de06d7 100644
--- a/NetworkPkg/HttpDxe/HttpDriver.h
+++ b/NetworkPkg/HttpDxe/HttpDriver.h
@@ -2,7 +2,7 @@
   The header files of the driver binding and service binding protocol for 
HttpDxe driver.
 
   Copyright (c) 2015, Intel Corporation. All rights reserved.
-
+  (C) Copyright 2016 Hewlett Packard Enterprise Development LP
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -17,6 +17,7 @@
 #define __EFI_HTTP_DRIVER_H__
 
 #include 
+#include 
 
 //
 // Libraries
diff --git a/NetworkPkg/HttpDxe/HttpImpl.h b/NetworkPkg/HttpDxe/HttpImpl.h
index 3822842..8054371 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.h
+++ b/NetworkPkg/HttpDxe/HttpImpl.h
@@ -2,7 +2,7 @@
   The header files of implementation of EFI_HTTP_PROTOCOL protocol interfaces.
 
   Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
-
+ 

Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Bruce Cran

On 2/10/16 3:59 PM, Mangefeste, Tony wrote:

I've gone down the track of using Bugzilla as well.  Aside from the massive 
list of pros listed below, gathering any other preference is welcomed.  I have 
used Bugzilla, but never been a maintainer on it.  We do have some resources 
lined up for management of Bugzilla, if needed, so that's not a barrier.  Of 
course, the devil's in the details.

So in short, Bugzilla is top of my mind right now.  I'm looking at other OSS 
projects and seeing what they use.  If anyone here sees one not listed below or 
has an opinion that they care to express please do so.  On the mobility view, 
I'll try to play around with that and see how it looks on my mobile devices.



I'm currently hosting a mirror of the edk2 repo at 
https://edk2.bluestop.org/diffusion/EDK/ - Phabricator has a bug/task 
tracker module Maniphest that looks rather capable - 
https://phacility.com/phabricator/maniphest/ .


I just think it's a shame we've lost so many bugs by telling people to 
report them to the mailing list for a couple of years, so any issue 
tracker is good at this point!


--
Bruce
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Mangefeste, Tony
GitHub doesn't allow for data import/export or report generation as much as I'd 
like to stick with the GitHub issue management, and other limitations that will 
be pushed as we grow in GitHub usage.

I agree on the Bugzilla maintenance, we're looking into the specific scopes 
there too.

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jordan 
Justen
Sent: Wednesday, February 10, 2016 4:01 PM
To: Laszlo Ersek ; Leif Lindholm (Linaro address) 
; Ard Biesheuvel 
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] Task: Issue Tracking System for Tianocore

On 2016-02-10 15:08:16, Laszlo Ersek wrote:
> On 02/10/16 23:59, Mangefeste, Tony wrote:
> > I've gone down the track of using Bugzilla as well.  Aside from the 
> > massive list of pros listed below, gathering any other preference is 
> > welcomed.  I have used Bugzilla, but never been a maintainer on it.
> > We do have some resources lined up for management of Bugzilla, if 
> > needed, so that's not a barrier.  Of course, the devil's in the 
> > details.
> > 
> > So in short, Bugzilla is top of my mind right now.  I'm looking at 
> > other OSS projects and seeing what they use.  If anyone here sees 
> > one not listed below or has an opinion that they care to express 
> > please do so.  On the mobility view, I'll try to play around with 
> > that and see how it looks on my mobile devices.
> > 
> > Welcome to real-time decision making, thought process spewing.
> 
> I recall that Linaro uses JIRA:
> https://cards.linaro.org/
> 
> Oh wait, there seems to be a new URL (still JIRA):
> https://projects.linaro.org
> 
> I am (was?) subscribed to a minimal set of CARDs only, in the first 
> system; I don't have any real experience with JIRA. Ard, Leif, can you 
> please share your thoughts?
> 

As an user of GitHub, Bugzilla and Jira.

Jira: No advantages over Bugzilla. Bigger, and slower than bugzilla. I think it 
tries to be flashier than bugzilla (and bugzilla is admittedly a bit dated 
looking).

Bugzilla: Works fine. Pain in the but to administer. You have to pay for 
hosting.

GitHub: Easy, but simplistic.

I think GitHub is the easier choice, and is good enough. Users will likely 
expect to find it used since the source is currently hosted there.

Actually, we are currently using GitHub, so we would be talking about a change 
if we moved to anything else. We have 17 open and 18 closed bugs currently 
which far surpasses the activity in the bug tracking 'trac' system that we had 
on sourceforge for several years. :) (And, I think the same for the collabnet 
hosted bugs before that.)

I think bugzilla is an okay choice if it can be paid for (with several years of 
funding...). I guess I'd recommend finding a service dedicated to hosting 
bugzilla, and not try to set up a server and self-host. It'd be a little 
annoying to have a separate bug tracker account.

My vote is to stick with GitHub for now.

-Jordan

> 
> > 
> > -Original Message-
> > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > Sent: Wednesday, February 10, 2016 2:44 PM
> > To: Mangefeste, Tony 
> > Cc: edk2-devel@lists.01.org 
> > Subject: Re: [edk2] Task: Issue Tracking System for Tianocore
> > 
> > On 02/10/16 22:45, Mangefeste, Tony wrote:
> >> We need an issue tracking system for Tiano.  
> >>
> >> The built-in issue tracking system that comes with GitHub isn't 
> >> sufficient to satisfy a key requirement.  There needs to be support 
> >> for multiple Tianocore-related programs.
> > 
> > I agree. The Bugzilla instance that Red Hat operates supports many pieces 
> > of metadata, and "product" and "component" are two key fields. I would find 
> > it useful if the metadata reflected even the edk2 module in question, at 
> > least for long-established modules (library instances and drivers). This 
> > would also enable fine-grained automatic assignment to a maintainer.
> > 
> >> As you know Intel has a
> >> system today that's internal to Intel where we track issues.  That 
> >> does not meet the needs of the community.  And to help improve 
> >> transparency, and better engage with the community I'm driving the 
> >> discussion and bring up of a bug tracking system.
> >>
> >> The goal is to have one operational by March 21, 2016 (WW13).  
> >> We're
> >> 6 weeks and counting from that deadline.  I'm interested in 
> >> community feedback, gathering requirements, and feedback on 
> >> proposals for which system to use.
> > 
> > - Launchpad
> >   
> > 
> >   Cons:
> >   - proportional width font
> >   - forcefully reflows comments, even if it means breaking git commit
> > hashes into several parts, breaking copy & paste on double-click
> >   - truncates long comments in the "full bug view". If you'd like to
> > read a careful, detailed comment to end, you have to open the
> > comment in isolation. And then you can't look at the other comments
> > at the same time.
> > 
> > - Github issue tracker

Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Mangefeste, Tony
+1, we'll hit some rough spots in the coming months, but the end result will be 
achieved.   Thanks for the input.

-Original Message-
From: Bruce Cran [mailto:br...@cran.org.uk] 
Sent: Wednesday, February 10, 2016 4:21 PM
To: Mangefeste, Tony ; Laszlo Ersek 

Cc: edk2-devel@lists.01.org 
Subject: Re: [edk2] Task: Issue Tracking System for Tianocore

On 2/10/16 3:59 PM, Mangefeste, Tony wrote:
> I've gone down the track of using Bugzilla as well.  Aside from the massive 
> list of pros listed below, gathering any other preference is welcomed.  I 
> have used Bugzilla, but never been a maintainer on it.  We do have some 
> resources lined up for management of Bugzilla, if needed, so that's not a 
> barrier.  Of course, the devil's in the details.
>
> So in short, Bugzilla is top of my mind right now.  I'm looking at other OSS 
> projects and seeing what they use.  If anyone here sees one not listed below 
> or has an opinion that they care to express please do so.  On the mobility 
> view, I'll try to play around with that and see how it looks on my mobile 
> devices.


I'm currently hosting a mirror of the edk2 repo at 
https://edk2.bluestop.org/diffusion/EDK/ - Phabricator has a bug/task tracker 
module Maniphest that looks rather capable - 
https://phacility.com/phabricator/maniphest/ .

I just think it's a shame we've lost so many bugs by telling people to report 
them to the mailing list for a couple of years, so any issue tracker is good at 
this point!

--
Bruce
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Bruce Cran

On 2/10/16 4:08 PM, Laszlo Ersek wrote:


I am (was?) subscribed to a minimal set of CARDs only, in the first
system; I don't have any real experience with JIRA. Ard, Leif, can you
please share your thoughts?


We use JIRA at work, and I was the admin for a few years. It's big, 
complex, and not something I think we'd want to use with an open source 
project.  But then again the installation I was managing had around 100 
projects and 1000 users, so it might not be representative for a single, 
simpler project.


Bitbucket uses a simplified version of it, which can be seen at 
https://confluence.atlassian.com/bitbucket/use-the-issue-tracker-221449750.html 
.


--
Bruce
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] github development process

2016-02-10 Thread Jordan Justen
On 2016-02-10 16:03:26, El-Haj-Mahmoud, Samer wrote:
> So I have been itching to submit patches after the transition to
> github, when I stumbled across this. It looks like the development
> process described in:
> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process
> , does not match the process described in:
> https://raw.githubusercontent.com/tianocore/edk2/master/MdePkg/Contributions.txt
> 

There seems to be duplicated content on

https://github.com/tianocore/tianocore.github.io/wiki/SourceForge-to-Github-Quick-Start

And, I think the EDK-II-Development-Process version was a little
older. I updated it to match the other one.

Before and after, I only saw one mention of 'pull':

"7. Update the master branch (pull or fetch/merge)"

Is this what caused the confusion? Or, was there something else that
generated a pull request?

-Jordan

> I followed the process in the wiki, and ended up creating a pull
> request for tiancore/edk for my contribution
> (https://github.com/tianocore/edk2/pull/55). I was also going to
> send a patch to EDK2 list (as the wiki suggests). Then I quickly
> realized that the maintainers are still working only with e-mail
> patches, and not pull requests, as clear from the conversations in
> closed/rejected pull requests:
> 
> https://github.com/tianocore/edk2/pull/45
> https://github.com/tianocore/edk2/pull/51
> https://github.com/tianocore/edk2/pull/41
> etc..
> 
> 
> I have no problem of sticking with the edk2-devel e-mail patches
> like we have been doing (all of my team from HPE contributes that
> way). But if that is the only/proper way to submit contributions,
> then the wiki need to be updated to drop the pull requests steps.
> 
> Thanks,
> --Samer
> 
> 
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Leif Lindholm
On Thu, Feb 11, 2016 at 12:08:16AM +0100, Laszlo Ersek wrote:
> On 02/10/16 23:59, Mangefeste, Tony wrote:
> > I've gone down the track of using Bugzilla as well.  Aside from the
> > massive list of pros listed below, gathering any other preference is
> > welcomed.  I have used Bugzilla, but never been a maintainer on it.
> > We do have some resources lined up for management of Bugzilla, if
> > needed, so that's not a barrier.  Of course, the devil's in the
> > details.
> > 
> > So in short, Bugzilla is top of my mind right now.  I'm looking at
> > other OSS projects and seeing what they use.  If anyone here sees one
> > not listed below or has an opinion that they care to express please
> > do so.  On the mobility view, I'll try to play around with that and
> > see how it looks on my mobile devices.
> > 
> > Welcome to real-time decision making, thought process spewing.
> 
> I recall that Linaro uses JIRA:
> https://cards.linaro.org/

Retired.

> Oh wait, there seems to be a new URL (still JIRA):
> https://projects.linaro.org

Yup.

> I am (was?) subscribed to a minimal set of CARDs only, in the first
> system; I don't have any real experience with JIRA. Ard, Leif, can you
> please share your thoughts?

We use Jira for project management.
https://bugs.linaro.org/ is bugzilla.

Strongly support bugzilla over both Jira and the github one.

Regards,

Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Jordan Justen
On 2016-02-10 16:21:33, Mangefeste, Tony wrote:
> GitHub doesn't allow for data import/export or report generation as

They have an API that should allow for this:

https://developer.github.com/v3/issues/

> much as I'd like to stick with the GitHub issue management, and
> other limitations that will be pushed as we grow in GitHub usage.
> 

What other limitations?

> I agree on the Bugzilla maintenance, we're looking into the specific
> scopes there too.

Can you confirm that a non-free (as in money) service will be funded
for a substantial period of time? If we can pay someone reliable to
run bugzilla for us, that would be nice.

-Jordan

> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jordan 
> Justen
> Sent: Wednesday, February 10, 2016 4:01 PM
> To: Laszlo Ersek ; Leif Lindholm (Linaro address) 
> ; Ard Biesheuvel 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] Task: Issue Tracking System for Tianocore
> 
> On 2016-02-10 15:08:16, Laszlo Ersek wrote:
> > On 02/10/16 23:59, Mangefeste, Tony wrote:
> > > I've gone down the track of using Bugzilla as well.  Aside from the 
> > > massive list of pros listed below, gathering any other preference is 
> > > welcomed.  I have used Bugzilla, but never been a maintainer on it.
> > > We do have some resources lined up for management of Bugzilla, if 
> > > needed, so that's not a barrier.  Of course, the devil's in the 
> > > details.
> > > 
> > > So in short, Bugzilla is top of my mind right now.  I'm looking at 
> > > other OSS projects and seeing what they use.  If anyone here sees 
> > > one not listed below or has an opinion that they care to express 
> > > please do so.  On the mobility view, I'll try to play around with 
> > > that and see how it looks on my mobile devices.
> > > 
> > > Welcome to real-time decision making, thought process spewing.
> > 
> > I recall that Linaro uses JIRA:
> > https://cards.linaro.org/
> > 
> > Oh wait, there seems to be a new URL (still JIRA):
> > https://projects.linaro.org
> > 
> > I am (was?) subscribed to a minimal set of CARDs only, in the first 
> > system; I don't have any real experience with JIRA. Ard, Leif, can you 
> > please share your thoughts?
> > 
> 
> As an user of GitHub, Bugzilla and Jira.
> 
> Jira: No advantages over Bugzilla. Bigger, and slower than bugzilla. I think 
> it tries to be flashier than bugzilla (and bugzilla is admittedly a bit dated 
> looking).
> 
> Bugzilla: Works fine. Pain in the but to administer. You have to pay for 
> hosting.
> 
> GitHub: Easy, but simplistic.
> 
> I think GitHub is the easier choice, and is good enough. Users will likely 
> expect to find it used since the source is currently hosted there.
> 
> Actually, we are currently using GitHub, so we would be talking about a 
> change if we moved to anything else. We have 17 open and 18 closed bugs 
> currently which far surpasses the activity in the bug tracking 'trac' system 
> that we had on sourceforge for several years. :) (And, I think the same for 
> the collabnet hosted bugs before that.)
> 
> I think bugzilla is an okay choice if it can be paid for (with several years 
> of funding...). I guess I'd recommend finding a service dedicated to hosting 
> bugzilla, and not try to set up a server and self-host. It'd be a little 
> annoying to have a separate bug tracker account.
> 
> My vote is to stick with GitHub for now.
> 
> -Jordan
> 
> > 
> > > 
> > > -Original Message-
> > > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > > Sent: Wednesday, February 10, 2016 2:44 PM
> > > To: Mangefeste, Tony 
> > > Cc: edk2-devel@lists.01.org 
> > > Subject: Re: [edk2] Task: Issue Tracking System for Tianocore
> > > 
> > > On 02/10/16 22:45, Mangefeste, Tony wrote:
> > >> We need an issue tracking system for Tiano.  
> > >>
> > >> The built-in issue tracking system that comes with GitHub isn't 
> > >> sufficient to satisfy a key requirement.  There needs to be support 
> > >> for multiple Tianocore-related programs.
> > > 
> > > I agree. The Bugzilla instance that Red Hat operates supports many pieces 
> > > of metadata, and "product" and "component" are two key fields. I would 
> > > find it useful if the metadata reflected even the edk2 module in 
> > > question, at least for long-established modules (library instances and 
> > > drivers). This would also enable fine-grained automatic assignment to a 
> > > maintainer.
> > > 
> > >> As you know Intel has a
> > >> system today that's internal to Intel where we track issues.  That 
> > >> does not meet the needs of the community.  And to help improve 
> > >> transparency, and better engage with the community I'm driving the 
> > >> discussion and bring up of a bug tracking system.
> > >>
> > >> The goal is to have one operational by March 21, 2016 (WW13).  
> > >> We're
> > >> 6 weeks and counting from that deadline.  I'm interested in 
> > >> community feedback, gathering requirements, and feedback on 
> > >

Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Mangefeste, Tony
Yes, the funding for a service (like Bugzilla) is not at present time a 
concern.  But we're still investigating and modeling out what it will take to 
use it, and of course if it turns out to be unjustifiable, we'll find something 
else (including using GitHub issue tracking).

-Original Message-
From: Justen, Jordan L 
Sent: Wednesday, February 10, 2016 4:57 PM
To: Mangefeste, Tony ; Laszlo Ersek 
; Leif Lindholm (Linaro address) ; 
Ard Biesheuvel 
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] Task: Issue Tracking System for Tianocore

On 2016-02-10 16:21:33, Mangefeste, Tony wrote:
> GitHub doesn't allow for data import/export or report generation as

They have an API that should allow for this:

https://developer.github.com/v3/issues/

> much as I'd like to stick with the GitHub issue management, and other 
> limitations that will be pushed as we grow in GitHub usage.
> 

What other limitations?

> I agree on the Bugzilla maintenance, we're looking into the specific 
> scopes there too.

Can you confirm that a non-free (as in money) service will be funded for a 
substantial period of time? If we can pay someone reliable to run bugzilla for 
us, that would be nice.

-Jordan

> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Jordan Justen
> Sent: Wednesday, February 10, 2016 4:01 PM
> To: Laszlo Ersek ; Leif Lindholm (Linaro address) 
> ; Ard Biesheuvel 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] Task: Issue Tracking System for Tianocore
> 
> On 2016-02-10 15:08:16, Laszlo Ersek wrote:
> > On 02/10/16 23:59, Mangefeste, Tony wrote:
> > > I've gone down the track of using Bugzilla as well.  Aside from 
> > > the massive list of pros listed below, gathering any other 
> > > preference is welcomed.  I have used Bugzilla, but never been a 
> > > maintainer on it.
> > > We do have some resources lined up for management of Bugzilla, if 
> > > needed, so that's not a barrier.  Of course, the devil's in the 
> > > details.
> > > 
> > > So in short, Bugzilla is top of my mind right now.  I'm looking at 
> > > other OSS projects and seeing what they use.  If anyone here sees 
> > > one not listed below or has an opinion that they care to express 
> > > please do so.  On the mobility view, I'll try to play around with 
> > > that and see how it looks on my mobile devices.
> > > 
> > > Welcome to real-time decision making, thought process spewing.
> > 
> > I recall that Linaro uses JIRA:
> > https://cards.linaro.org/
> > 
> > Oh wait, there seems to be a new URL (still JIRA):
> > https://projects.linaro.org
> > 
> > I am (was?) subscribed to a minimal set of CARDs only, in the first 
> > system; I don't have any real experience with JIRA. Ard, Leif, can 
> > you please share your thoughts?
> > 
> 
> As an user of GitHub, Bugzilla and Jira.
> 
> Jira: No advantages over Bugzilla. Bigger, and slower than bugzilla. I think 
> it tries to be flashier than bugzilla (and bugzilla is admittedly a bit dated 
> looking).
> 
> Bugzilla: Works fine. Pain in the but to administer. You have to pay for 
> hosting.
> 
> GitHub: Easy, but simplistic.
> 
> I think GitHub is the easier choice, and is good enough. Users will likely 
> expect to find it used since the source is currently hosted there.
> 
> Actually, we are currently using GitHub, so we would be talking about 
> a change if we moved to anything else. We have 17 open and 18 closed 
> bugs currently which far surpasses the activity in the bug tracking 
> 'trac' system that we had on sourceforge for several years. :) (And, I 
> think the same for the collabnet hosted bugs before that.)
> 
> I think bugzilla is an okay choice if it can be paid for (with several years 
> of funding...). I guess I'd recommend finding a service dedicated to hosting 
> bugzilla, and not try to set up a server and self-host. It'd be a little 
> annoying to have a separate bug tracker account.
> 
> My vote is to stick with GitHub for now.
> 
> -Jordan
> 
> > 
> > > 
> > > -Original Message-
> > > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > > Sent: Wednesday, February 10, 2016 2:44 PM
> > > To: Mangefeste, Tony 
> > > Cc: edk2-devel@lists.01.org 
> > > Subject: Re: [edk2] Task: Issue Tracking System for Tianocore
> > > 
> > > On 02/10/16 22:45, Mangefeste, Tony wrote:
> > >> We need an issue tracking system for Tiano.  
> > >>
> > >> The built-in issue tracking system that comes with GitHub isn't 
> > >> sufficient to satisfy a key requirement.  There needs to be 
> > >> support for multiple Tianocore-related programs.
> > > 
> > > I agree. The Bugzilla instance that Red Hat operates supports many pieces 
> > > of metadata, and "product" and "component" are two key fields. I would 
> > > find it useful if the metadata reflected even the edk2 module in 
> > > question, at least for long-established modules (library instances and 
> > > drivers). This would also enable fine-grained automatic assignment to a 
>

Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Mangefeste, Tony
Leif,

Do you have funding estimates you would share with me from a Linaro 
perspective?  Of course, it doesn't have to be shared out in this context, but 
if you have such data it'd be interesting to compare our resources with what is 
required to run Bugzilla.

BRs,
Tony

-Original Message-
From: Leif Lindholm [mailto:leif.lindh...@linaro.org] 
Sent: Wednesday, February 10, 2016 4:53 PM
To: Laszlo Ersek 
Cc: Ard Biesheuvel ; Mangefeste, Tony 
; edk2-devel@lists.01.org 
Subject: Re: [edk2] Task: Issue Tracking System for Tianocore

On Thu, Feb 11, 2016 at 12:08:16AM +0100, Laszlo Ersek wrote:
> On 02/10/16 23:59, Mangefeste, Tony wrote:
> > I've gone down the track of using Bugzilla as well.  Aside from the 
> > massive list of pros listed below, gathering any other preference is 
> > welcomed.  I have used Bugzilla, but never been a maintainer on it.
> > We do have some resources lined up for management of Bugzilla, if 
> > needed, so that's not a barrier.  Of course, the devil's in the 
> > details.
> > 
> > So in short, Bugzilla is top of my mind right now.  I'm looking at 
> > other OSS projects and seeing what they use.  If anyone here sees 
> > one not listed below or has an opinion that they care to express 
> > please do so.  On the mobility view, I'll try to play around with 
> > that and see how it looks on my mobile devices.
> > 
> > Welcome to real-time decision making, thought process spewing.
> 
> I recall that Linaro uses JIRA:
> https://cards.linaro.org/

Retired.

> Oh wait, there seems to be a new URL (still JIRA):
> https://projects.linaro.org

Yup.

> I am (was?) subscribed to a minimal set of CARDs only, in the first 
> system; I don't have any real experience with JIRA. Ard, Leif, can you 
> please share your thoughts?

We use Jira for project management.
https://bugs.linaro.org/ is bugzilla.

Strongly support bugzilla over both Jira and the github one.

Regards,

Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Bruce Cran

On 2/10/16 5:58 PM, Mangefeste, Tony wrote:

Yes, the funding for a service (like Bugzilla) is not at present time a 
concern.  But we're still investigating and modeling out what it will take to 
use it, and of course if it turns out to be unjustifiable, we'll find something 
else (including using GitHub issue tracking).


Could an instance of bugzilla be hosted on tianocore.org if necessary? I 
presume tianocore.org is a dedicated Linux server somewhere that someone 
has root privileges on, and could probably be maintained by volunteers 
if we don't have sufficient sysadmins already?


--
Bruce
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Mangefeste, Tony
Now that's a great idea.  I'm already leaning towards whatever system we employ 
to have well documented methods for maintenance so that anyone (including 
myself) can contribute time to maintain.  Arguably, we can extend that to allow 
trusted members of the community rights to help maintain as well.

+1

And as for hosting, it's too early to tell, but like you I want it hosted off 
of Tianocore.org, there's some voodoo that can be employed to make that happen, 
and we'll work with our web backend contractor to see about making that happen.

-Original Message-
From: Bruce Cran [mailto:br...@cran.org.uk] 
Sent: Wednesday, February 10, 2016 5:02 PM
To: Mangefeste, Tony ; Justen, Jordan L 
; Laszlo Ersek ; Leif Lindholm 
(Linaro address) ; Ard Biesheuvel 

Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] Task: Issue Tracking System for Tianocore

On 2/10/16 5:58 PM, Mangefeste, Tony wrote:
> Yes, the funding for a service (like Bugzilla) is not at present time a 
> concern.  But we're still investigating and modeling out what it will take to 
> use it, and of course if it turns out to be unjustifiable, we'll find 
> something else (including using GitHub issue tracking).

Could an instance of bugzilla be hosted on tianocore.org if necessary? I 
presume tianocore.org is a dedicated Linux server somewhere that someone has 
root privileges on, and could probably be maintained by volunteers if we don't 
have sufficient sysadmins already?

--
Bruce
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Leif Lindholm
On Thu, Feb 11, 2016 at 12:59:52AM +, Mangefeste, Tony wrote:
> Do you have funding estimates you would share with me from a Linaro
> perspective?  Of course, it doesn't have to be shared out in this
> context, but if you have such data it'd be interesting to compare
> our resources with what is required to run Bugzilla.

Funding estimates for using Bugzilla? I mean, I could ask but I think
the answer would basically be "a machine running Linux".

I would expect the ticket load for Tianocore to be an order of
magnitude below what Linaro is seeing (across toolchains, Android,
kernel, OpenEmbedded, ...), and a couple of orders of magnitude
below what Red Hat are seeing.

/
Leif

> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org] 
> Sent: Wednesday, February 10, 2016 4:53 PM
> To: Laszlo Ersek 
> Cc: Ard Biesheuvel ; Mangefeste, Tony 
> ; edk2-devel@lists.01.org 
> Subject: Re: [edk2] Task: Issue Tracking System for Tianocore
> 
> On Thu, Feb 11, 2016 at 12:08:16AM +0100, Laszlo Ersek wrote:
> > On 02/10/16 23:59, Mangefeste, Tony wrote:
> > > I've gone down the track of using Bugzilla as well.  Aside from the 
> > > massive list of pros listed below, gathering any other preference is 
> > > welcomed.  I have used Bugzilla, but never been a maintainer on it.
> > > We do have some resources lined up for management of Bugzilla, if 
> > > needed, so that's not a barrier.  Of course, the devil's in the 
> > > details.
> > > 
> > > So in short, Bugzilla is top of my mind right now.  I'm looking at 
> > > other OSS projects and seeing what they use.  If anyone here sees 
> > > one not listed below or has an opinion that they care to express 
> > > please do so.  On the mobility view, I'll try to play around with 
> > > that and see how it looks on my mobile devices.
> > > 
> > > Welcome to real-time decision making, thought process spewing.
> > 
> > I recall that Linaro uses JIRA:
> > https://cards.linaro.org/
> 
> Retired.
> 
> > Oh wait, there seems to be a new URL (still JIRA):
> > https://projects.linaro.org
> 
> Yup.
> 
> > I am (was?) subscribed to a minimal set of CARDs only, in the first 
> > system; I don't have any real experience with JIRA. Ard, Leif, can you 
> > please share your thoughts?
> 
> We use Jira for project management.
> https://bugs.linaro.org/ is bugzilla.
> 
> Strongly support bugzilla over both Jira and the github one.
> 
> Regards,
> 
> Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Mangefeste, Tony
I understand better.  My concern is if there's some sort of ongoing headcount 
required to maintain Bugzilla.  As was stated before, if there's a maintenance 
requirement is that a hard requirement? Or can the software/system run 
autonomously for long periods of time without human intervention*.

*I have no direct experience administering Bugzilla, just using it.

-Original Message-
From: Leif Lindholm [mailto:leif.lindh...@linaro.org] 
Sent: Wednesday, February 10, 2016 5:26 PM
To: Mangefeste, Tony 
Cc: Laszlo Ersek ; Ard Biesheuvel 
; edk2-devel@lists.01.org 
Subject: Re: [edk2] Task: Issue Tracking System for Tianocore

On Thu, Feb 11, 2016 at 12:59:52AM +, Mangefeste, Tony wrote:
> Do you have funding estimates you would share with me from a Linaro 
> perspective?  Of course, it doesn't have to be shared out in this 
> context, but if you have such data it'd be interesting to compare our 
> resources with what is required to run Bugzilla.

Funding estimates for using Bugzilla? I mean, I could ask but I think the 
answer would basically be "a machine running Linux".

I would expect the ticket load for Tianocore to be an order of magnitude below 
what Linaro is seeing (across toolchains, Android, kernel, OpenEmbedded, ...), 
and a couple of orders of magnitude below what Red Hat are seeing.

/
Leif

> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Wednesday, February 10, 2016 4:53 PM
> To: Laszlo Ersek 
> Cc: Ard Biesheuvel ; Mangefeste, Tony 
> ; edk2-devel@lists.01.org 
> 
> Subject: Re: [edk2] Task: Issue Tracking System for Tianocore
> 
> On Thu, Feb 11, 2016 at 12:08:16AM +0100, Laszlo Ersek wrote:
> > On 02/10/16 23:59, Mangefeste, Tony wrote:
> > > I've gone down the track of using Bugzilla as well.  Aside from 
> > > the massive list of pros listed below, gathering any other 
> > > preference is welcomed.  I have used Bugzilla, but never been a 
> > > maintainer on it.
> > > We do have some resources lined up for management of Bugzilla, if 
> > > needed, so that's not a barrier.  Of course, the devil's in the 
> > > details.
> > > 
> > > So in short, Bugzilla is top of my mind right now.  I'm looking at 
> > > other OSS projects and seeing what they use.  If anyone here sees 
> > > one not listed below or has an opinion that they care to express 
> > > please do so.  On the mobility view, I'll try to play around with 
> > > that and see how it looks on my mobile devices.
> > > 
> > > Welcome to real-time decision making, thought process spewing.
> > 
> > I recall that Linaro uses JIRA:
> > https://cards.linaro.org/
> 
> Retired.
> 
> > Oh wait, there seems to be a new URL (still JIRA):
> > https://projects.linaro.org
> 
> Yup.
> 
> > I am (was?) subscribed to a minimal set of CARDs only, in the first 
> > system; I don't have any real experience with JIRA. Ard, Leif, can 
> > you please share your thoughts?
> 
> We use Jira for project management.
> https://bugs.linaro.org/ is bugzilla.
> 
> Strongly support bugzilla over both Jira and the github one.
> 
> Regards,
> 
> Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Mangefeste, Tony
BTW, I will start experimenting with it myself in the coming days setting up a 
virtual server, so I'll know better myself.

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Mangefeste, Tony
Sent: Wednesday, February 10, 2016 5:53 PM
To: Leif Lindholm 
Cc: edk2-devel@lists.01.org ; Laszlo Ersek 
; Ard Biesheuvel 
Subject: Re: [edk2] Task: Issue Tracking System for Tianocore

I understand better.  My concern is if there's some sort of ongoing headcount 
required to maintain Bugzilla.  As was stated before, if there's a maintenance 
requirement is that a hard requirement? Or can the software/system run 
autonomously for long periods of time without human intervention*.

*I have no direct experience administering Bugzilla, just using it.

-Original Message-
From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
Sent: Wednesday, February 10, 2016 5:26 PM
To: Mangefeste, Tony 
Cc: Laszlo Ersek ; Ard Biesheuvel 
; edk2-devel@lists.01.org 
Subject: Re: [edk2] Task: Issue Tracking System for Tianocore

On Thu, Feb 11, 2016 at 12:59:52AM +, Mangefeste, Tony wrote:
> Do you have funding estimates you would share with me from a Linaro 
> perspective?  Of course, it doesn't have to be shared out in this 
> context, but if you have such data it'd be interesting to compare our 
> resources with what is required to run Bugzilla.

Funding estimates for using Bugzilla? I mean, I could ask but I think the 
answer would basically be "a machine running Linux".

I would expect the ticket load for Tianocore to be an order of magnitude below 
what Linaro is seeing (across toolchains, Android, kernel, OpenEmbedded, ...), 
and a couple of orders of magnitude below what Red Hat are seeing.

/
Leif

> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Wednesday, February 10, 2016 4:53 PM
> To: Laszlo Ersek 
> Cc: Ard Biesheuvel ; Mangefeste, Tony 
> ; edk2-devel@lists.01.org 
> 
> Subject: Re: [edk2] Task: Issue Tracking System for Tianocore
> 
> On Thu, Feb 11, 2016 at 12:08:16AM +0100, Laszlo Ersek wrote:
> > On 02/10/16 23:59, Mangefeste, Tony wrote:
> > > I've gone down the track of using Bugzilla as well.  Aside from 
> > > the massive list of pros listed below, gathering any other 
> > > preference is welcomed.  I have used Bugzilla, but never been a 
> > > maintainer on it.
> > > We do have some resources lined up for management of Bugzilla, if 
> > > needed, so that's not a barrier.  Of course, the devil's in the 
> > > details.
> > > 
> > > So in short, Bugzilla is top of my mind right now.  I'm looking at 
> > > other OSS projects and seeing what they use.  If anyone here sees 
> > > one not listed below or has an opinion that they care to express 
> > > please do so.  On the mobility view, I'll try to play around with 
> > > that and see how it looks on my mobile devices.
> > > 
> > > Welcome to real-time decision making, thought process spewing.
> > 
> > I recall that Linaro uses JIRA:
> > https://cards.linaro.org/
> 
> Retired.
> 
> > Oh wait, there seems to be a new URL (still JIRA):
> > https://projects.linaro.org
> 
> Yup.
> 
> > I am (was?) subscribed to a minimal set of CARDs only, in the first 
> > system; I don't have any real experience with JIRA. Ard, Leif, can 
> > you please share your thoughts?
> 
> We use Jira for project management.
> https://bugs.linaro.org/ is bugzilla.
> 
> Strongly support bugzilla over both Jira and the github one.
> 
> Regards,
> 
> Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Task: Issue Tracking System for Tianocore

2016-02-10 Thread Bruce Cran

On 2/10/16 7:08 PM, Mangefeste, Tony wrote:

BTW, I will start experimenting with it myself in the coming days setting up a 
virtual server, so I'll know better myself.


https://wiki.mozilla.org/Bugzilla seems to have lots of information 
about Bugzilla, including 
https://wiki.mozilla.org/Bugzilla:FAQ#How_much_time_does_it_take_to_install_and_maintain_Bugzilla.3F


"Someone with a lot of Bugzilla experience can get you up and running in 
a few hours, and your Bugzilla install can run untended for years. 
Less-experienced administrators sometimes need a few days to set up 
Bugzilla."


--
Bruce

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel