Re: [edk2] UEFI LinuxLoader app usage

[Jun Nie]

2017-02-09 21:44 GMT+08:00 Jun Nie <jun@linaro.org>:
>  I am new to UEFI and trying to boot Linux with LinuxLoader app. But I
> cannot find detail information for below questions. Could anyone point
> out where I can find related information or example code? Thanks for
> your time!
>
> 1. How to feed command line to this app so that ProcessAppCommandLine() can
> process it?
I know this app support parsing command line from shell. I am seeking
method to launch this app from BDS directly without shell but cannot
find where to specify value to LoadedImage->LoadOptions so that this
app can get command line from it.

>
> 2. And how should I generate  device path string for raw blockIO
> device with specified offset/size? UEFI 2.6 spec does not provide much
> information in 9.4.5 Media Device Path Rules.
>
> Thank you!
> Jun
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 1/2] EmbeddedPkg/AndroidBoot: boot android kernel from storage

[Jun Nie]

On 2017年07月28日 21:06, Leif Lindholm wrote:

On Fri, Jul 28, 2017 at 05:47:46PM +0800, Jun Nie wrote:

On 2017年07月27日 22:09, Leif Lindholm wrote:

On Thu, Jul 27, 2017 at 06:07:19PM +0800, Jun Nie wrote:

Add an android kernel loader that could load kernel from storage
device. This patch is from Haojian's code as below link. The minor
change is that alternative dtb is searched in second loader binary
of Android bootimage if dtb is not found after Linux kernel.
https://patches.linaro.org/patch/94683/

This android boot image BDS add addtitional cmdline/dtb/ramfs
support besides kernel that is introduced by Android boot header.


Getting there. A few more comments below.


Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
  ArmPkg/Include/Library/BdsLib.h|   3 +
  ArmPkg/Library/BdsLib/BdsFilePath.c|   3 -
  .../Application/AndroidBoot/AndroidBootApp.c   | 127 +++
  .../Application/AndroidBoot/AndroidBootApp.inf |  64 
  .../Application/AndroidFastboot/AndroidBootImg.c   |  35 +-
  .../AndroidFastboot/AndroidFastbootApp.h   |   1 +
  .../AndroidFastboot/Arm/BootAndroidBootImg.c   |   2 +-
  EmbeddedPkg/Include/Library/AndroidBootImgLib.h|  67 
  EmbeddedPkg/Include/Protocol/AndroidBootImg.h  |  47 +++
  .../Library/AndroidBootImgLib/AndroidBootImgLib.c  | 419 +
  .../AndroidBootImgLib/AndroidBootImgLib.inf|  48 +++
  11 files changed, 782 insertions(+), 34 deletions(-)
  create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
  create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
  create mode 100644 EmbeddedPkg/Include/Library/AndroidBootImgLib.h
  create mode 100644 EmbeddedPkg/Include/Protocol/AndroidBootImg.h
  create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
  create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf




diff --git a/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c 
b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
new file mode 100644
index 000..2de1d8a
--- /dev/null
+++ b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
@@ -0,0 +1,127 @@
+/** @file
+
+  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.
+  Copyright (c) 2017, Linaro. 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
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+EFI_STATUS
+EFIAPI
+AndroidBootAppEntryPoint (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+  CHAR16  *BootPathStr;
+  EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
+  EFI_DEVICE_PATH *DevicePath;
+  EFI_DEVICE_PATH_PROTOCOL*Node, *NextNode;
+  EFI_BLOCK_IO_PROTOCOL   *BlockIo;
+  UINT32  MediaId, BlockSize;
+  VOID*Buffer;
+  EFI_HANDLE  Handle;
+  UINTN   Size;
+
+  BootPathStr = (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);
+  ASSERT (BootPathStr != NULL);
+  Status = gBS->LocateProtocol (, NULL,
+(VOID **));
+  ASSERT_EFI_ERROR(Status);
+  DevicePath = (EFI_DEVICE_PATH 
*)EfiDevicePathFromTextProtocol->ConvertTextToDevicePath (BootPathStr);
+  ASSERT (DevicePath != NULL);
+
+  /* Find DevicePath node of Partition */
+  NextNode = DevicePath;
+  while (1) {


Should this not be while (NextNode != NULL), with some check that the
node was found before progressing?


(NextNode != NULL) is valid check.
The code check node before progressing as below, doesn't it?


My point is that if you never match the "if (IS_DEVICE_PATH_NODE "
condition, this loop will never terminate.

And if we update the loop condition to fix that, we end up calling
LocateDevicePath with a known bad parameter.


Yeah, besides the check in while(), I should add check before while loop.





+Node = NextNode;
+if (IS_DEVICE_PATH_NODE (Node, MEDIA_DEVICE_PATH, MEDIA_HARDDRIVE_DP)) {
+  break;
+}
+NextNode = NextDevicePathNode (Node);
+  }
+
+  Status = gBS->LocateDevicePath (,
+  , );


And should this not use  rather than ?

I suppose we should return error if no MEDIA_HARDDRIVE_DP node is found. I
ever did test and found orig

[edk2] [PATCH v4 1/4] ArmPkg: Move IS_DEVICE_PATH_NODE for sharing

[Jun Nie]

Move IS_DEVICE_PATH_NODE into header to share it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 ArmPkg/Include/Library/BdsLib.h | 3 +++
 ArmPkg/Library/BdsLib/BdsFilePath.c | 3 ---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ArmPkg/Include/Library/BdsLib.h b/ArmPkg/Include/Library/BdsLib.h
index c58f47e..4528c2e 100644
--- a/ArmPkg/Include/Library/BdsLib.h
+++ b/ArmPkg/Include/Library/BdsLib.h
@@ -15,6 +15,9 @@
 #ifndef __BDS_ENTRY_H__
 #define __BDS_ENTRY_H__
 
+#define IS_DEVICE_PATH_NODE(node,type,subtype)\
+(((node)->Type == (type)) && ((node)->SubType == (subtype)))
+
 /**
   This is defined by the UEFI specs, don't change it
 **/
diff --git a/ArmPkg/Library/BdsLib/BdsFilePath.c 
b/ArmPkg/Library/BdsLib/BdsFilePath.c
index f9d8c4c..41557bb 100644
--- a/ArmPkg/Library/BdsLib/BdsFilePath.c
+++ b/ArmPkg/Library/BdsLib/BdsFilePath.c
@@ -24,9 +24,6 @@
 #include 
 #include 
 
-
-#define IS_DEVICE_PATH_NODE(node,type,subtype) (((node)->Type == (type)) && 
((node)->SubType == (subtype)))
-
 /* Type and defines to set up the DHCP4 options */
 
 typedef struct {
-- 
1.9.1

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

[edk2] [PATCH v4 4/4] EmbeddedPkg: add Android boot device path and guid

[Jun Nie]

The device path specifies where to load android boot image.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/EmbeddedPkg.dec | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 4cd528a..8ad2a84 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -80,6 +80,7 @@
   gAndroidFastbootPlatformProtocolGuid =  { 0x524685a0, 0x89a0, 0x11e3, {0x9d, 
0x4d, 0xbf, 0xa9, 0xf6, 0xa4, 0x03, 0x08}}
   gUsbDeviceProtocolGuid =  { 0x021bd2ca, 0x51d2, 0x11e3, {0x8e, 0x56, 0xb7, 
0x54, 0x17, 0xc7,  0x0b, 0x44 }}
   gPlatformGpioProtocolGuid = { 0x52ce9845, 0x5af4, 0x43e2, {0xba, 0xfd, 0x23, 
0x08, 0x12, 0x54, 0x7a, 0xc2 }}
+  gAndroidBootImgProtocolGuid = { 0x9859bb19, 0x407c, 0x4f8b, {0xbc, 0xe1, 
0xf8, 0xda, 0x65, 0x65, 0xf4, 0xa5 }}
 
 [PcdsFeatureFlag.common]
   gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|FALSE|BOOLEAN|0x0001
@@ -181,6 +182,7 @@
   
gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbProductId|0xbeef|UINT32|0x0023
   gEmbeddedTokenSpaceGuid.PcdAndroidFastbootTcpPort|1234|UINT32|0x0024
 
+  gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath|L""|VOID*|0x0057
 
 [PcdsFixedAtBuild.ARM]
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|32|UINT8|0x0010
-- 
1.9.1

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

[edk2] [PATCH v4 2/4] EmbeddedPkg/AndroidFastboot: split android boot header

[Jun Nie]

Split android boot header definition to share code among
different applications and libraries.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 .../Application/AndroidFastboot/AndroidBootImg.c   | 35 +++
 .../AndroidFastboot/AndroidFastbootApp.h   |  1 +
 .../AndroidFastboot/Arm/BootAndroidBootImg.c   |  2 +-
 EmbeddedPkg/Include/Library/AndroidBootImgLib.h| 50 ++
 4 files changed, 57 insertions(+), 31 deletions(-)
 create mode 100644 EmbeddedPkg/Include/Library/AndroidBootImgLib.h

diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c 
b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
index f3e770b..2f7f093 100644
--- a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
+++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
@@ -14,32 +14,6 @@
 
 #include "AndroidFastbootApp.h"
 
-#define BOOT_MAGIC"ANDROID!"
-#define BOOT_MAGIC_LENGTH sizeof (BOOT_MAGIC) - 1
-
-// Check Val (unsigned) is a power of 2 (has only one bit set)
-#define IS_POWER_OF_2(Val) (Val != 0 && ((Val & (Val - 1)) == 0))
-
-// No documentation for this really - sizes of fields has been determined
-// empirically.
-#pragma pack(1)
-typedef struct {
-  CHAR8   BootMagic[BOOT_MAGIC_LENGTH];
-  UINT32  KernelSize;
-  UINT32  KernelAddress;
-  UINT32  RamdiskSize;
-  UINT32  RamdiskAddress;
-  UINT32  SecondStageBootloaderSize;
-  UINT32  SecondStageBootloaderAddress;
-  UINT32  KernelTaggsAddress;
-  UINT32  PageSize;
-  UINT32  Reserved[2];
-  CHAR8   ProductName[16];
-  CHAR8   KernelArgs[BOOTIMG_KERNEL_ARGS_SIZE];
-  UINT32  Id[32];
-} ANDROID_BOOTIMG_HEADER;
-#pragma pack()
-
 // Find the kernel and ramdisk in an Android boot.img.
 // return EFI_INVALID_PARAMTER if the boot.img is invalid (i.e. doesn't have 
the
 //  right magic value),
@@ -64,7 +38,8 @@ ParseAndroidBootImg (
 
   Header = (ANDROID_BOOTIMG_HEADER *) BootImg;
 
-  if (AsciiStrnCmp (Header->BootMagic, BOOT_MAGIC, BOOT_MAGIC_LENGTH) != 0) {
+  if (AsciiStrnCmp ((CONST CHAR8 *)Header->BootMagic, ANDROID_BOOT_MAGIC,
+ANDROID_BOOT_MAGIC_LENGTH) != 0) {
 return EFI_INVALID_PARAMETER;
   }
 
@@ -72,7 +47,7 @@ ParseAndroidBootImg (
 return EFI_NOT_FOUND;
   }
 
-  ASSERT (IS_POWER_OF_2 (Header->PageSize));
+  ASSERT (IS_VALID_ANDROID_PAGE_SIZE (Header->PageSize));
 
   *KernelSize = Header->KernelSize;
   *Kernel = BootImgBytePtr + Header->PageSize;
@@ -84,8 +59,8 @@ ParseAndroidBootImg (
  + ALIGN_VALUE (Header->KernelSize, Header->PageSize));
   }
 
-  AsciiStrnCpyS (KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE, Header->KernelArgs,
-BOOTIMG_KERNEL_ARGS_SIZE);
+  AsciiStrnCpyS (KernelArgs, ANDROID_BOOTIMG_KERNEL_ARGS_SIZE, 
Header->KernelArgs,
+ANDROID_BOOTIMG_KERNEL_ARGS_SIZE);
 
   return EFI_SUCCESS;
 }
diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h 
b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h
index f62660f..e4c5aa3 100644
--- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h
+++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h
@@ -15,6 +15,7 @@
 #ifndef __ANDROID_FASTBOOT_APP_H__
 #define __ANDROID_FASTBOOT_APP_H__
 
+#include 
 #include 
 #include 
 #include 
diff --git a/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c 
b/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c
index f446cce..1d9024b 100644
--- a/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c
+++ b/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c
@@ -112,7 +112,7 @@ BootAndroidBootImg (
   )
 {
   EFI_STATUS  Status;
-  CHAR8   KernelArgs[BOOTIMG_KERNEL_ARGS_SIZE];
+  CHAR8   
KernelArgs[ANDROID_BOOTIMG_KERNEL_ARGS_SIZE];
   VOID   *Kernel;
   UINTN   KernelSize;
   VOID   *Ramdisk;
diff --git a/EmbeddedPkg/Include/Library/AndroidBootImgLib.h 
b/EmbeddedPkg/Include/Library/AndroidBootImgLib.h
new file mode 100644
index 000..8116c7c
--- /dev/null
+++ b/EmbeddedPkg/Include/Library/AndroidBootImgLib.h
@@ -0,0 +1,50 @@
+/** @file
+
+  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.
+  Copyright (c) 2017, Linaro.
+
+  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
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __ABOOTIMG_H__
+#define __ABOOTIMG_H__
+
+#include 
+#include 
+#include 
+
+#i

[edk2] [PATCH v4 3/4] EmbeddedPkg/AndroidBoot: boot android kernel from storage

[Jun Nie]

Add an android kernel loader that could load kernel from storage
device. This patch is derived from Haojian's code as below link.
https://patches.linaro.org/patch/94683/

This android boot image BDS add addtitional cmdline/dtb/ramfs
support besides kernel that is introduced by Android boot header.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 .../Application/AndroidBoot/AndroidBootApp.c   | 140 +++
 .../Application/AndroidBoot/AndroidBootApp.inf |  64 +++
 EmbeddedPkg/Include/Library/AndroidBootImgLib.h|  17 +
 EmbeddedPkg/Include/Protocol/AndroidBootImg.h  |  47 +++
 .../Library/AndroidBootImgLib/AndroidBootImgLib.c  | 444 +
 .../AndroidBootImgLib/AndroidBootImgLib.inf|  48 +++
 6 files changed, 760 insertions(+)
 create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
 create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
 create mode 100644 EmbeddedPkg/Include/Protocol/AndroidBootImg.h
 create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
 create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf

diff --git a/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c 
b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
new file mode 100644
index 000..5069819
--- /dev/null
+++ b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
@@ -0,0 +1,140 @@
+/** @file
+
+  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.
+  Copyright (c) 2017, Linaro. 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
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+/* Validate the node is media hard drive type */
+EFI_STATUS
+ValidateAndroidMediaDevicePath (
+  IN EFI_DEVICE_PATH  *DevicePath
+  )
+{
+  EFI_DEVICE_PATH_PROTOCOL*Node, *NextNode;
+
+  NextNode = DevicePath;
+  while (NextNode != NULL) {
+Node = NextNode;
+if (IS_DEVICE_PATH_NODE (Node, MEDIA_DEVICE_PATH, MEDIA_HARDDRIVE_DP)) {
+  return EFI_SUCCESS;
+}
+NextNode = NextDevicePathNode (Node);
+  }
+  return EFI_INVALID_PARAMETER;
+}
+
+EFI_STATUS
+EFIAPI
+AndroidBootAppEntryPoint (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+  CHAR16  *BootPathStr;
+  EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
+  EFI_DEVICE_PATH *DevicePath;
+  EFI_BLOCK_IO_PROTOCOL   *BlockIo;
+  UINT32  MediaId, BlockSize;
+  VOID*Buffer;
+  EFI_HANDLE  Handle;
+  UINTN   BootImgSize;
+
+  BootPathStr = (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);
+  ASSERT (BootPathStr != NULL);
+  Status = gBS->LocateProtocol (, NULL,
+(VOID **));
+  ASSERT_EFI_ERROR(Status);
+  DevicePath = (EFI_DEVICE_PATH 
*)EfiDevicePathFromTextProtocol->ConvertTextToDevicePath (BootPathStr);
+  ASSERT (DevicePath != NULL);
+
+  Status = ValidateAndroidMediaDevicePath (DevicePath);
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  Status = gBS->LocateDevicePath (,
+  , );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  Status = gBS->OpenProtocol (
+  Handle,
+  ,
+  (VOID **) ,
+  gImageHandle,
+  NULL,
+  EFI_OPEN_PROTOCOL_GET_PROTOCOL
+  );
+  if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_ERROR, "Failed to get BlockIo: %r\n", Status));
+return Status;
+  }
+
+  MediaId = BlockIo->Media->MediaId;
+  BlockSize = BlockIo->Media->BlockSize;
+  Buffer = AllocatePages (EFI_SIZE_TO_PAGES (sizeof(ANDROID_BOOTIMG_HEADER)));
+  if (Buffer == NULL) {
+return EFI_BUFFER_TOO_SMALL;
+  }
+  /* Load header of boot.img */
+  Status = BlockIo->ReadBlocks (
+  BlockIo,
+  MediaId,
+  0,
+  BlockSize,
+  Buffer
+  );
+  Status = AndroidBootImgGetImgSize (Buffer, );
+  if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_ERROR, "Failed to get AndroidBootImg Size: %r\n", Status));
+return Status;
+  }
+  BootImgSize = ALIGN_VALUE (BootImgSi

[edk2] [PATCH v7] EmbeddedPkg/AndroidBoot: boot android kernel from storage

[Jun Nie]

Add an android kernel loader that could load kernel from storage
device.
This android boot image BDS add addtitional cmdline/dtb/ramfs
support besides kernel that is introduced by Android boot header.

This patch is derived from Haojian's code as below link.
https://patches.linaro.org/patch/94683/

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 .../Application/AndroidBoot/AndroidBootApp.c   | 140 +++
 .../Application/AndroidBoot/AndroidBootApp.inf |  64 +++
 EmbeddedPkg/EmbeddedPkg.dec|   2 +
 EmbeddedPkg/EmbeddedPkg.dsc|   2 +
 EmbeddedPkg/Include/Library/AndroidBootImgLib.h|  13 +
 EmbeddedPkg/Include/Protocol/AndroidBootImg.h  |  47 +++
 .../Library/AndroidBootImgLib/AndroidBootImgLib.c  | 463 +
 .../AndroidBootImgLib/AndroidBootImgLib.inf|  48 +++
 8 files changed, 779 insertions(+)
 create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
 create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
 create mode 100644 EmbeddedPkg/Include/Protocol/AndroidBootImg.h
 create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
 create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf

diff --git a/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c 
b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
new file mode 100644
index 000..977167d
--- /dev/null
+++ b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
@@ -0,0 +1,140 @@
+/** @file
+
+  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.
+  Copyright (c) 2017, Linaro. 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
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+/* Validate the node is media hard drive type */
+EFI_STATUS
+ValidateAndroidMediaDevicePath (
+  IN EFI_DEVICE_PATH  *DevicePath
+  )
+{
+  EFI_DEVICE_PATH_PROTOCOL*Node, *NextNode;
+
+  NextNode = DevicePath;
+  while (NextNode != NULL) {
+Node = NextNode;
+if (IS_DEVICE_PATH_NODE (Node, MEDIA_DEVICE_PATH, MEDIA_HARDDRIVE_DP)) {
+  return EFI_SUCCESS;
+}
+NextNode = NextDevicePathNode (Node);
+  }
+  return EFI_INVALID_PARAMETER;
+}
+
+EFI_STATUS
+EFIAPI
+AndroidBootAppEntryPoint (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+  CHAR16  *BootPathStr;
+  EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
+  EFI_DEVICE_PATH *DevicePath;
+  EFI_BLOCK_IO_PROTOCOL   *BlockIo;
+  UINT32  MediaId, BlockSize;
+  VOID*Buffer;
+  EFI_HANDLE  Handle;
+  UINTN   BootImgSize;
+
+  BootPathStr = (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);
+  ASSERT (BootPathStr != NULL);
+  Status = gBS->LocateProtocol (, NULL,
+(VOID **));
+  ASSERT_EFI_ERROR(Status);
+  DevicePath = (EFI_DEVICE_PATH 
*)EfiDevicePathFromTextProtocol->ConvertTextToDevicePath (BootPathStr);
+  ASSERT (DevicePath != NULL);
+
+  Status = ValidateAndroidMediaDevicePath (DevicePath);
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  Status = gBS->LocateDevicePath (,
+  , );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  Status = gBS->OpenProtocol (
+  Handle,
+  ,
+  (VOID **) ,
+  gImageHandle,
+  NULL,
+  EFI_OPEN_PROTOCOL_GET_PROTOCOL
+  );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Failed to get BlockIo: %r\n", Status));
+return Status;
+  }
+
+  MediaId = BlockIo->Media->MediaId;
+  BlockSize = BlockIo->Media->BlockSize;
+  Buffer = AllocatePages (EFI_SIZE_TO_PAGES (sizeof(ANDROID_BOOTIMG_HEADER)));
+  if (Buffer == NULL) {
+return EFI_BUFFER_TOO_SMALL;
+  }
+  /* Load header of boot.img */
+  Status = BlockIo->ReadBlocks (
+  BlockIo,
+  MediaId,
+  0,
+  BlockSize,
+  Buffer
+  );
+  Status = AndroidBootImgGetImgSize (Buffer, );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Failed to ge

Re: [edk2] [PATCH v2 2/2] EmbeddedPkg: add Android boot device path and guid

[Jun Nie]

2017-07-06 21:29 GMT+08:00 Jun Nie <jun@linaro.org>:
> The device path specifies where to load android boot image.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie <jun@linaro.org>
> ---
>  EmbeddedPkg/EmbeddedPkg.dec | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
> index 4cd528a..cf89af2 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -80,6 +80,7 @@
>gAndroidFastbootPlatformProtocolGuid =  { 0x524685a0, 0x89a0, 0x11e3, 
> {0x9d, 0x4d, 0xbf, 0xa9, 0xf6, 0xa4, 0x03, 0x08}}
>gUsbDeviceProtocolGuid =  { 0x021bd2ca, 0x51d2, 0x11e3, {0x8e, 0x56, 0xb7, 
> 0x54, 0x17, 0xc7,  0x0b, 0x44 }}
>gPlatformGpioProtocolGuid = { 0x52ce9845, 0x5af4, 0x43e2, {0xba, 0xfd, 
> 0x23, 0x08, 0x12, 0x54, 0x7a, 0xc2 }}
> +  gAbootimgProtocolGuid = { 0x9859bb19, 0x407c, 0x4f8b, {0xbc, 0xe1, 0xf8, 
> 0xda, 0x65, 0x65, 0xf4, 0xa5 }}
>
>  [PcdsFeatureFlag.common]
>gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|FALSE|BOOLEAN|0x0001
> @@ -181,6 +182,7 @@
>
> gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbProductId|0xbeef|UINT32|0x0023
>gEmbeddedTokenSpaceGuid.PcdAndroidFastbootTcpPort|1234|UINT32|0x0024

Just find that the ID 0x0024 conflict with other guid. Will change
it in next version.

>
> +  gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath|L""|VOID*|0x0025
>
>  [PcdsFixedAtBuild.ARM]
>gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|32|UINT8|0x0010
> --
> 1.9.1
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 2/2] EmbeddedPkg/DwEmmc: Adjust FIFO threshold

[Jun Nie]

2017-07-06 23:22 GMT+08:00 Leif Lindholm <leif.lindh...@linaro.org>:
> On Wed, Jul 05, 2017 at 04:27:08PM +0800, Jun Nie wrote:
>> Adjust FIFO threshold according to FIFO depth. Skip
>> the adjustment if we do not have FIFO depth info.
>>
>
> So, this is a big improvement in readability - but some of my generic
> style comments do not appear to have been addressed.
>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jun Nie <jun@linaro.org>
>> ---
>>  EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h  |  6 
>>  EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c   | 50 
>> +
>>  EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf |  1 +
>>  EmbeddedPkg/EmbeddedPkg.dec |  1 +
>>  4 files changed, 58 insertions(+)
>>
>> diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h 
>> b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h
>> index 055f1e0..90c7676 100644
>> --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h
>> +++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h
>> @@ -38,7 +38,10 @@
>>  #define DWEMMC_RINTSTS  ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) 
>> + 0x044)
>>  #define DWEMMC_STATUS   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) 
>> + 0x048)
>>  #define DWEMMC_FIFOTH   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) 
>> + 0x04c)
>> +#define DWEMMC_TCBCNT   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) 
>> + 0x05c)
>> +#define DWEMMC_TBBCNT   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) 
>> + 0x060)
>>  #define DWEMMC_DEBNCE   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) 
>> + 0x064)
>> +#define DWEMMC_HCON ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) 
>> + 0x070)
>>  #define DWEMMC_UHSREG   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) 
>> + 0x074)
>>  #define DWEMMC_BMOD ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) 
>> + 0x080)
>>  #define DWEMMC_DBADDR   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) 
>> + 0x088)
>> @@ -47,6 +50,7 @@
>>  #define DWEMMC_DSCADDR  ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) 
>> + 0x094)
>>  #define DWEMMC_BUFADDR  ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) 
>> + 0x098)
>>  #define DWEMMC_CARDTHRCTL   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) 
>> + 0X100)
>> +#define DWEMMC_DATA ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) 
>> + 0X200)
>>
>>  #define CMD_UPDATE_CLK  0x80202000
>>  #define CMD_START_BIT   (1 << 31)
>> @@ -124,4 +128,6 @@
>>  #define DWEMMC_CARD_RD_THR(x)   ((x & 0xfff) << 16)
>>  #define DWEMMC_CARD_RD_THR_EN   (1 << 0)
>>
>> +#define DWEMMC_GET_HDATA_WIDTH(x)   (((x) >> 7) & 0x7)
>> +
>>  #endif  // __DWEMMC_H__
>> diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c 
>> b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
>> index bb26b69..70e064d 100644
>> --- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
>> +++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
>> @@ -415,6 +415,55 @@ DwEmmcReceiveResponse (
>>return EFI_SUCCESS;
>>  }
>>
>> +VOID DwEmmcAdjustFifothreshold (
>
> This still needs to be:
> VOID
> DwEmmcAdjustFifoThreshold (
>
>> +  VOID
>> +  )
>> +{
>> +  /* DMA multiple transaction size map to reg value as array index */
>> +  CONST UINT32 BurstSize[] = {1, 4, 8, 16, 32, 64, 128, 256};
>> +  UINT32 BlkDepthInFifo, Fifoth, FifoWidth, FifoDepth;
>
> Fifoth -> FifoThreshold.
>
>> +  UINT32 BlkSize = DWEMMC_BLOCK_SIZE, Idx = 0, RxWmark = 1, TxWmark, 
>> TxWmarkInvers;
>
> RxWmark -> RxWaterMark.
> TxWmark -> TxWaterMark.
> TxWmarkInvers -> TxWaterMarkInverse.

My fault. The style is totally different with kernel and I am not
fully customized yet.

>
>> +
>> +  /* Skip FIFO adjustment if we do not have platform FIFO depth info */
>> +  FifoDepth = PcdGet32 (PcdDwEmmcDxeFifoDepth);
>> +  if (!FifoDepth) {
>> +return;
>> +  }
>> +
>> +  TxWmark = FifoDepth / 2;
>> +  TxWmarkInvers = FifoDepth - TxWmark;
>> +
>> +  FifoWidth = DWEMMC_GET_HDATA_WIDTH (MmioRead32 (DWEMMC_HCON));
>> +  if (!FifoWidth) {
>> +FifoWidth = 2;
>> +  } else if (FifoWidth == 2) {
>> +FifoWidth = 8;
>> +  } else {
>> +FifoWidth = 4;
>> +  }
>> +
>> +  BlkDepthInFifo = BlkSize / FifoWidth;
>> +
>> +  /* if BlkSize is not a multiple of the FI

[edk2] [PATCH v3] ArmPlatformPkg: Support different PL011 reg offset

[Jun Nie]

ZTE/SanChip version pl011 has different reg offset and bit offset
for some registers.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 ArmPlatformPkg/ArmPlatformPkg.dec  |  1 +
 ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf |  1 +
 ArmPlatformPkg/Include/Drivers/PL011Uart.h | 31 ++
 3 files changed, 33 insertions(+)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec 
b/ArmPlatformPkg/ArmPlatformPkg.dec
index d756fd2..b8a6b13 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -97,6 +97,7 @@
   gArmPlatformTokenSpaceGuid.PL011UartInteger|0|UINT32|0x0020
   gArmPlatformTokenSpaceGuid.PL011UartFractional|0|UINT32|0x002D
   gArmPlatformTokenSpaceGuid.PL011UartInterrupt|0x|UINT32|0x002F
+  gArmPlatformTokenSpaceGuid.PL011UartRegOffsetVariant|0|UINT8|0x003E
 
   ## PL011 Serial Debug UART
   
gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase|0x|UINT64|0x0030
diff --git a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf 
b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
index 0154f3b..3fd4602 100644
--- a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
+++ b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
@@ -39,3 +39,4 @@
 
   gArmPlatformTokenSpaceGuid.PL011UartInteger
   gArmPlatformTokenSpaceGuid.PL011UartFractional
+  gArmPlatformTokenSpaceGuid.PL011UartRegOffsetVariant
diff --git a/ArmPlatformPkg/Include/Drivers/PL011Uart.h 
b/ArmPlatformPkg/Include/Drivers/PL011Uart.h
index d5e88e8..4957dbf 100644
--- a/ArmPlatformPkg/Include/Drivers/PL011Uart.h
+++ b/ArmPlatformPkg/Include/Drivers/PL011Uart.h
@@ -18,7 +18,25 @@
 #include 
 #include 
 
+#define PL011_VARIANT_ZTE 1
+
 // PL011 Registers
+#if FixedPcdGet8 (PL011UartRegOffsetVariant) == PL011_VARIANT_ZTE
+#define UARTDR0x004
+#define UARTRSR   0x010
+#define UARTECR   0x010
+#define UARTFR0x014
+#define UARTIBRD  0x024
+#define UARTFBRD  0x028
+#define UARTLCR_H 0x030
+#define UARTCR0x034
+#define UARTIFLS  0x038
+#define UARTIMSC  0x040
+#define UARTRIS   0x044
+#define UARTMIS   0x048
+#define UARTICR   0x04c
+#define UARTDMACR 0x050
+#else
 #define UARTDR0x000
 #define UARTRSR   0x004
 #define UARTECR   0x004
@@ -34,6 +52,7 @@
 #define UARTMIS   0x040
 #define UARTICR   0x044
 #define UARTDMACR 0x048
+#endif
 
 #define UARTPID0  0xFE0
 #define UARTPID1  0xFE4
@@ -47,6 +66,17 @@
 #define UART_STATUS_ERROR_MASK0x0F
 
 // Flag reg bits
+#if FixedPcdGet8 (PL011UartRegOffsetVariant) == PL011_VARIANT_ZTE
+#define PL011_UARTFR_RI   (1 << 0)  // Ring indicator
+#define PL011_UARTFR_TXFE (1 << 7)  // Transmit FIFO empty
+#define PL011_UARTFR_RXFF (1 << 6)  // Receive  FIFO full
+#define PL011_UARTFR_TXFF (1 << 5)  // Transmit FIFO full
+#define PL011_UARTFR_RXFE (1 << 4)  // Receive  FIFO empty
+#define PL011_UARTFR_BUSY (1 << 8)  // UART busy
+#define PL011_UARTFR_DCD  (1 << 2)  // Data carrier detect
+#define PL011_UARTFR_DSR  (1 << 3)  // Data set ready
+#define PL011_UARTFR_CTS  (1 << 1)  // Clear to send
+#else
 #define PL011_UARTFR_RI   (1 << 8)  // Ring indicator
 #define PL011_UARTFR_TXFE (1 << 7)  // Transmit FIFO empty
 #define PL011_UARTFR_RXFF (1 << 6)  // Receive  FIFO full
@@ -56,6 +86,7 @@
 #define PL011_UARTFR_DCD  (1 << 2)  // Data carrier detect
 #define PL011_UARTFR_DSR  (1 << 1)  // Data set ready
 #define PL011_UARTFR_CTS  (1 << 0)  // Clear to send
+#endif
 
 // Flag reg bits - alternative names
 #define UART_TX_EMPTY_FLAG_MASK   PL011_UARTFR_TXFE
-- 
1.9.1

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

[edk2] [PATCH v3 2/2] EmbeddedPkg/DwEmmc: Adjust FIFO threshold

[Jun Nie]

Adjust FIFO threshold according to FIFO depth. Skip
the adjustment if we do not have FIFO depth info.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h  |  6 +
 EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c   | 42 +
 EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf |  1 +
 EmbeddedPkg/EmbeddedPkg.dec |  1 +
 4 files changed, 50 insertions(+)

diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h 
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h
index 055f1e0..90c7676 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h
@@ -38,7 +38,10 @@
 #define DWEMMC_RINTSTS  ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x044)
 #define DWEMMC_STATUS   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x048)
 #define DWEMMC_FIFOTH   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x04c)
+#define DWEMMC_TCBCNT   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x05c)
+#define DWEMMC_TBBCNT   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x060)
 #define DWEMMC_DEBNCE   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x064)
+#define DWEMMC_HCON ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x070)
 #define DWEMMC_UHSREG   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x074)
 #define DWEMMC_BMOD ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x080)
 #define DWEMMC_DBADDR   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x088)
@@ -47,6 +50,7 @@
 #define DWEMMC_DSCADDR  ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x094)
 #define DWEMMC_BUFADDR  ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x098)
 #define DWEMMC_CARDTHRCTL   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0X100)
+#define DWEMMC_DATA ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0X200)
 
 #define CMD_UPDATE_CLK  0x80202000
 #define CMD_START_BIT   (1 << 31)
@@ -124,4 +128,6 @@
 #define DWEMMC_CARD_RD_THR(x)   ((x & 0xfff) << 16)
 #define DWEMMC_CARD_RD_THR_EN   (1 << 0)
 
+#define DWEMMC_GET_HDATA_WIDTH(x)   (((x) >> 7) & 0x7)
+
 #endif  // __DWEMMC_H__
diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c 
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
index bb26b69..bd20f4b 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
@@ -415,6 +415,47 @@ DwEmmcReceiveResponse (
   return EFI_SUCCESS;
 }
 
+VOID
+DwEmmcAdjustFifothresholdreshold (
+  VOID
+  )
+{
+  /* DMA multiple transaction size map to reg value as array index */
+  CONST UINT32 BurstSize[] = {1, 4, 8, 16, 32, 64, 128, 256};
+  UINT32 BlkDepthInFifo, Fifothreshold, FifoWidth, FifoDepth;
+  UINT32 BlkSize = DWEMMC_BLOCK_SIZE, Idx = 0, RxWatermark = 1, TxWatermark, 
TxWatermarkInvers;
+
+  /* Skip FIFO adjustment if we do not have platform FIFO depth info */
+  FifoDepth = PcdGet32 (PcdDwEmmcDxeFifoDepth);
+  if (!FifoDepth) {
+return;
+  }
+
+  TxWatermark = FifoDepth / 2;
+  TxWatermarkInvers = FifoDepth - TxWatermark;
+
+  FifoWidth = DWEMMC_GET_HDATA_WIDTH (MmioRead32 (DWEMMC_HCON));
+  if (!FifoWidth) {
+FifoWidth = 2;
+  } else if (FifoWidth == 2) {
+FifoWidth = 8;
+  } else {
+FifoWidth = 4;
+  }
+
+  BlkDepthInFifo = BlkSize / FifoWidth;
+
+  Idx = ARRAY_SIZE (BurstSize) - 1;
+  while (Idx && ((BlkDepthInFifo % BurstSize[Idx]) || (TxWatermarkInvers % 
BurstSize[Idx]))) {
+Idx--;
+  }
+
+  RxWatermark = BurstSize[Idx] - 1;
+  Fifothreshold = DWEMMC_DMA_BURST_SIZE (Idx) | DWEMMC_FIFO_TWMARK 
(TxWatermark)
+   | DWEMMC_FIFO_RWMARK (RxWatermark);
+  MmioWrite32 (DWEMMC_FIFOTH, Fifothreshold);
+}
+
 EFI_STATUS
 PrepareDmaData (
   IN DWEMMC_IDMAC_DESCRIPTOR*IdmacDesc,
@@ -633,6 +674,7 @@ DwEmmcDxeInitialize (
 
   Handle = NULL;
 
+  DwEmmcAdjustFifothresholdreshold ();
   gpIdmacDesc = (DWEMMC_IDMAC_DESCRIPTOR *)AllocatePages 
(DWEMMC_MAX_DESC_PAGES);
   if (gpIdmacDesc == NULL) {
 return EFI_BUFFER_TOO_SMALL;
diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf 
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
index 99b4f99..bc4413e 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
@@ -49,6 +49,7 @@
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeClockFrequencyInHz
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeMaxClockFreqInHz
+  gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeFifoDepth
 
 [Depex]
   TRUE
diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 3cb30a4..4cd528a 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -168,6 +168,7 @@
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress|0x0|UINT32|0x0035
   gEmbeddedTokenSpaceGuid.PcdDwE

Re: [edk2] [PATCH v2 1/2] EmbeddedPkg/AndroidBoot: boot android kernel from storage

[Jun Nie]

2017-07-06 21:29 GMT+08:00 Jun Nie <jun@linaro.org>:
> Add an android kernel loader that could load kernel from storage
> device. This patch is from Haojian's code. The minor change
> is that alternative dtb is searched in second loader binary of
> Android bootimage if dtb is not found after Linux kernel.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie <jun@linaro.org>

Leif,

Could you help comment these two android boot BDS patches? Thank you!

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

[edk2] [PATCH v3 1/2] EmbeddedPkg/AndroidBoot: boot android kernel from storage

[Jun Nie]

Add an android kernel loader that could load kernel from storage
device. This patch is from Haojian's code as below link. The minor
change is that alternative dtb is searched in second loader binary
of Android bootimage if dtb is not found after Linux kernel.
https://patches.linaro.org/patch/94683/

This android boot image BDS add addtitional cmdline/dtb/ramfs
support besides kernel that is introduced by Android boot header.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 ArmPkg/Include/Library/BdsLib.h|   3 +
 ArmPkg/Library/BdsLib/BdsFilePath.c|   3 -
 .../Application/AndroidBoot/AndroidBootApp.c   | 127 +++
 .../Application/AndroidBoot/AndroidBootApp.inf |  64 
 .../Application/AndroidFastboot/AndroidBootImg.c   |  35 +-
 .../AndroidFastboot/AndroidFastbootApp.h   |   1 +
 .../AndroidFastboot/Arm/BootAndroidBootImg.c   |   2 +-
 EmbeddedPkg/Include/Library/AndroidBootImgLib.h|  67 
 EmbeddedPkg/Include/Protocol/AndroidBootImg.h  |  47 +++
 .../Library/AndroidBootImgLib/AndroidBootImgLib.c  | 419 +
 .../AndroidBootImgLib/AndroidBootImgLib.inf|  48 +++
 11 files changed, 782 insertions(+), 34 deletions(-)
 create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
 create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
 create mode 100644 EmbeddedPkg/Include/Library/AndroidBootImgLib.h
 create mode 100644 EmbeddedPkg/Include/Protocol/AndroidBootImg.h
 create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
 create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf

diff --git a/ArmPkg/Include/Library/BdsLib.h b/ArmPkg/Include/Library/BdsLib.h
index c58f47e..4528c2e 100644
--- a/ArmPkg/Include/Library/BdsLib.h
+++ b/ArmPkg/Include/Library/BdsLib.h
@@ -15,6 +15,9 @@
 #ifndef __BDS_ENTRY_H__
 #define __BDS_ENTRY_H__
 
+#define IS_DEVICE_PATH_NODE(node,type,subtype)\
+(((node)->Type == (type)) && ((node)->SubType == (subtype)))
+
 /**
   This is defined by the UEFI specs, don't change it
 **/
diff --git a/ArmPkg/Library/BdsLib/BdsFilePath.c 
b/ArmPkg/Library/BdsLib/BdsFilePath.c
index f9d8c4c..41557bb 100644
--- a/ArmPkg/Library/BdsLib/BdsFilePath.c
+++ b/ArmPkg/Library/BdsLib/BdsFilePath.c
@@ -24,9 +24,6 @@
 #include 
 #include 
 
-
-#define IS_DEVICE_PATH_NODE(node,type,subtype) (((node)->Type == (type)) && 
((node)->SubType == (subtype)))
-
 /* Type and defines to set up the DHCP4 options */
 
 typedef struct {
diff --git a/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c 
b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
new file mode 100644
index 000..2de1d8a
--- /dev/null
+++ b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
@@ -0,0 +1,127 @@
+/** @file
+
+  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.
+  Copyright (c) 2017, Linaro. 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
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+EFI_STATUS
+EFIAPI
+AndroidBootAppEntryPoint (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+  CHAR16  *BootPathStr;
+  EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
+  EFI_DEVICE_PATH *DevicePath;
+  EFI_DEVICE_PATH_PROTOCOL*Node, *NextNode;
+  EFI_BLOCK_IO_PROTOCOL   *BlockIo;
+  UINT32  MediaId, BlockSize;
+  VOID*Buffer;
+  EFI_HANDLE  Handle;
+  UINTN   Size;
+
+  BootPathStr = (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);
+  ASSERT (BootPathStr != NULL);
+  Status = gBS->LocateProtocol (, NULL,
+(VOID **));
+  ASSERT_EFI_ERROR(Status);
+  DevicePath = (EFI_DEVICE_PATH 
*)EfiDevicePathFromTextProtocol->ConvertTextToDevicePath (BootPathStr);
+  ASSERT (DevicePath != NULL);
+
+  /* Find DevicePath node of Partition */
+  NextNode = DevicePath;
+  while (1) {
+Node = NextNode;
+if (IS_DEVICE_PATH_NODE (Node, MEDIA_DEVICE_PATH, MEDIA_HARDDRIVE_DP)) {
+  break;
+}
+NextNode = NextDevicePathNode (Node);
+  }
+
+  Status = gBS->LocateDevicePath (,
+  , );
+  if (EFI_ERROR (Status)) {
+  

[edk2] [PATCH v3 2/2] EmbeddedPkg: add Android boot device path and guid

[Jun Nie]

The device path specifies where to load android boot image.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/EmbeddedPkg.dec | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 4cd528a..8ad2a84 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -80,6 +80,7 @@
   gAndroidFastbootPlatformProtocolGuid =  { 0x524685a0, 0x89a0, 0x11e3, {0x9d, 
0x4d, 0xbf, 0xa9, 0xf6, 0xa4, 0x03, 0x08}}
   gUsbDeviceProtocolGuid =  { 0x021bd2ca, 0x51d2, 0x11e3, {0x8e, 0x56, 0xb7, 
0x54, 0x17, 0xc7,  0x0b, 0x44 }}
   gPlatformGpioProtocolGuid = { 0x52ce9845, 0x5af4, 0x43e2, {0xba, 0xfd, 0x23, 
0x08, 0x12, 0x54, 0x7a, 0xc2 }}
+  gAndroidBootImgProtocolGuid = { 0x9859bb19, 0x407c, 0x4f8b, {0xbc, 0xe1, 
0xf8, 0xda, 0x65, 0x65, 0xf4, 0xa5 }}
 
 [PcdsFeatureFlag.common]
   gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|FALSE|BOOLEAN|0x0001
@@ -181,6 +182,7 @@
   
gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbProductId|0xbeef|UINT32|0x0023
   gEmbeddedTokenSpaceGuid.PcdAndroidFastbootTcpPort|1234|UINT32|0x0024
 
+  gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath|L""|VOID*|0x0057
 
 [PcdsFixedAtBuild.ARM]
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|32|UINT8|0x0010
-- 
1.9.1

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

Re: [edk2] [PATCH v3 1/2] EmbeddedPkg/AndroidBoot: boot android kernel from storage

[Jun Nie]

On 2017年07月27日 22:09, Leif Lindholm wrote:

On Thu, Jul 27, 2017 at 06:07:19PM +0800, Jun Nie wrote:

Add an android kernel loader that could load kernel from storage
device. This patch is from Haojian's code as below link. The minor
change is that alternative dtb is searched in second loader binary
of Android bootimage if dtb is not found after Linux kernel.
https://patches.linaro.org/patch/94683/

This android boot image BDS add addtitional cmdline/dtb/ramfs
support besides kernel that is introduced by Android boot header.


Getting there. A few more comments below.


Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
  ArmPkg/Include/Library/BdsLib.h|   3 +
  ArmPkg/Library/BdsLib/BdsFilePath.c|   3 -
  .../Application/AndroidBoot/AndroidBootApp.c   | 127 +++
  .../Application/AndroidBoot/AndroidBootApp.inf |  64 
  .../Application/AndroidFastboot/AndroidBootImg.c   |  35 +-
  .../AndroidFastboot/AndroidFastbootApp.h   |   1 +
  .../AndroidFastboot/Arm/BootAndroidBootImg.c   |   2 +-
  EmbeddedPkg/Include/Library/AndroidBootImgLib.h|  67 
  EmbeddedPkg/Include/Protocol/AndroidBootImg.h  |  47 +++
  .../Library/AndroidBootImgLib/AndroidBootImgLib.c  | 419 +
  .../AndroidBootImgLib/AndroidBootImgLib.inf|  48 +++
  11 files changed, 782 insertions(+), 34 deletions(-)
  create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
  create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
  create mode 100644 EmbeddedPkg/Include/Library/AndroidBootImgLib.h
  create mode 100644 EmbeddedPkg/Include/Protocol/AndroidBootImg.h
  create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
  create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf

diff --git a/ArmPkg/Include/Library/BdsLib.h b/ArmPkg/Include/Library/BdsLib.h
index c58f47e..4528c2e 100644
--- a/ArmPkg/Include/Library/BdsLib.h
+++ b/ArmPkg/Include/Library/BdsLib.h
@@ -15,6 +15,9 @@
  #ifndef __BDS_ENTRY_H__
  #define __BDS_ENTRY_H__
  
+#define IS_DEVICE_PATH_NODE(node,type,subtype)\

+(((node)->Type == (type)) && ((node)->SubType == (subtype)))
+
  /**
This is defined by the UEFI specs, don't change it
  **/
diff --git a/ArmPkg/Library/BdsLib/BdsFilePath.c 
b/ArmPkg/Library/BdsLib/BdsFilePath.c
index f9d8c4c..41557bb 100644
--- a/ArmPkg/Library/BdsLib/BdsFilePath.c
+++ b/ArmPkg/Library/BdsLib/BdsFilePath.c
@@ -24,9 +24,6 @@
  #include 
  #include 
  
-

-#define IS_DEVICE_PATH_NODE(node,type,subtype) (((node)->Type == (type)) && 
((node)->SubType == (subtype)))
-


Could you break these bits of moving macros and definitions into
common header files as a separate patch, preceding the rest of the changes?


Will do.



  /* Type and defines to set up the DHCP4 options */
  
  typedef struct {

diff --git a/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c 
b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
new file mode 100644
index 000..2de1d8a
--- /dev/null
+++ b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
@@ -0,0 +1,127 @@
+/** @file
+
+  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.
+  Copyright (c) 2017, Linaro. 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
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+EFI_STATUS
+EFIAPI
+AndroidBootAppEntryPoint (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+  CHAR16  *BootPathStr;
+  EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
+  EFI_DEVICE_PATH *DevicePath;
+  EFI_DEVICE_PATH_PROTOCOL*Node, *NextNode;
+  EFI_BLOCK_IO_PROTOCOL   *BlockIo;
+  UINT32  MediaId, BlockSize;
+  VOID*Buffer;
+  EFI_HANDLE  Handle;
+  UINTN   Size;
+
+  BootPathStr = (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);
+  ASSERT (BootPathStr != NULL);
+  Status = gBS->LocateProtocol (, NULL,
+(VOID **));
+  ASSERT_EFI_ERROR(Status);
+  DevicePath = (EFI_DEVICE_PATH 
*)EfiDevicePathFromTextProtocol->ConvertTextToDevicePath (BootPathStr);
+  ASSERT (DevicePath != NULL);
+
+  /* Find DevicePath node of 

Re: [edk2] [PATCH v2 1/2] EmbeddedPkg/AndroidBoot: boot android kernel from storage

[Jun Nie]

2017-07-19 0:04 GMT+08:00 Leif Lindholm <leif.lindh...@linaro.org>:
> On Thu, Jul 06, 2017 at 09:29:05PM +0800, Jun Nie wrote:
>> Add an android kernel loader that could load kernel from storage
>> device.
>
> UEFI can already load a kernel (with the EFI stub) from a storage
> device. Please explain in the commit message how this support differs
> from that.

OK, will add description for addtitional cmdline/dtb/ramfs support besides
 kernel that is introduced by Android boot header.

>
> What relation does this code have to AndroidFastbootApp?

It does not relate to AndroidFastbootApp directly because fastboot is tools
for image download/flash/etc. But both apps comply to Google's spec
and share some common data definition.

>
>> This patch is from Haojian's code.
>
> Could you put a link to the origin of the code (preferably the
> specific commit)?
Will do.
>
>> The minor change
>> is that alternative dtb is searched in second loader binary of
>> Android bootimage if dtb is not found after Linux kernel.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jun Nie <jun@linaro.org>
>> ---
>>  .../Application/AndroidBoot/AndroidBootApp.c   | 129 
>>  .../Application/AndroidBoot/AndroidBootApp.inf |  64 
>>  EmbeddedPkg/Include/Library/AbootimgLib.h  |  65 
>>  EmbeddedPkg/Include/Protocol/Abootimg.h|  47 +++
>>  EmbeddedPkg/Library/AbootimgLib/AbootimgLib.c  | 350 
>> +
>>  EmbeddedPkg/Library/AbootimgLib/AbootimgLib.inf|  48 +++
>
> For proper CamelCase, I think the library name should be
> AndroidBootImageLib, and the filenames (and macros) updated similarly.

Will do.
>
>>  6 files changed, 703 insertions(+)
>>  create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
>>  create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
>>  create mode 100644 EmbeddedPkg/Include/Library/AbootimgLib.h
>>  create mode 100644 EmbeddedPkg/Include/Protocol/Abootimg.h
>>  create mode 100644 EmbeddedPkg/Library/AbootimgLib/AbootimgLib.c
>>  create mode 100644 EmbeddedPkg/Library/AbootimgLib/AbootimgLib.inf
>>
>> diff --git a/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c 
>> b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
>> new file mode 100644
>> index 000..9ed931b
>> --- /dev/null
>> +++ b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
>> @@ -0,0 +1,129 @@
>> +/** @file
>> +
>> +  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.
>> +  Copyright (c) 2017, Linaro. 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
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
>> IMPLIED.
>> +
>> +**/
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>> +
>> +#define IS_DEVICE_PATH_NODE(node,type,subtype) (((node)->Type == (type)) && 
>> ((node)->SubType == (subtype)))
>
> Would prefer for this to be moved into a common header file (BdsLib.h
> ?) rather than copied into multiple .c files.

Will do.
>
>> +
>> +#define ALIGN(x, a)(((x) + ((a) - 1)) & ~((a) - 1))
>
> Rather use one of the ALIGN_ macros from Base.h.
>

Will do.
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +AndroidBootAppEntryPoint (
>> +  IN EFI_HANDLEImageHandle,
>> +  IN EFI_SYSTEM_TABLE  *SystemTable
>> +  )
>> +{
>> +  EFI_STATUS  Status;
>> +  CHAR16  *BootPathStr;
>> +  EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
>> +  EFI_DEVICE_PATH *DevicePath;
>> +  EFI_DEVICE_PATH_PROTOCOL*Node, *NextNode;
>> +  EFI_BLOCK_IO_PROTOCOL   *BlockIo;
>> +  UINT32  MediaId, BlockSize;
>> +  VOID*Buffer;
>> +  EFI_HANDLE  Handle;
>> +  UINTN   Size;
>> +
>> +  BootPathStr = (CHAR16 *)Pc

[edk2] [PATCH v4] EmbeddedPkg/MmcDxe: Align the ExtCSD buffer

[Jun Nie]

ExtCSD structure may be read via DMA. So align it to
page to avoid data corruption.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/Universal/MmcDxe/Mmc.h   |  2 +-
 EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 19 ++-
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h 
b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
index 8a7d5a3..f3e56ff 100644
--- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
+++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
@@ -319,7 +319,7 @@ typedef struct  {
   OCR   OCRData;
   CID   CIDData;
   CSD   CSDData;
-  ECSD  ECSDData; // MMC V4 extended card specific
+  ECSD  *ECSDData; // MMC V4 extended card specific
 } CARD_INFO;
 
 typedef struct _MMC_HOST_INSTANCE {
diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c 
b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
index c28207e..7f74c54 100644
--- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
+++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
@@ -13,6 +13,7 @@
 **/
 
 #include 
+#include 
 #include 
 
 #include "Mmc.h"
@@ -210,15 +211,19 @@ EmmcIdentificationMode (
   }
 
   // Fetch ECSD
+  MmcHostInstance->CardInfo.ECSDData = AllocatePages (EFI_SIZE_TO_PAGES 
(sizeof (ECSD)));
+  if (MmcHostInstance->CardInfo.ECSDData == NULL) {
+return EFI_OUT_OF_RESOURCES;
+  }
   Status = Host->SendCommand (Host, MMC_CMD8, 0);
   if (EFI_ERROR (Status)) {
 DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD fetch error, 
Status=%r.\n", Status));
   }
 
-  Status = Host->ReadBlockData (Host, 0, 512, (UINT32 
*)&(MmcHostInstance->CardInfo.ECSDData));
+  Status = Host->ReadBlockData (Host, 0, 512, (UINT32 
*)MmcHostInstance->CardInfo.ECSDData);
   if (EFI_ERROR (Status)) {
 DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD read error, 
Status=%r.\n", Status));
-return Status;
+goto FreePageExit;
   }
 
   // Make sure device exiting data mode
@@ -226,7 +231,7 @@ EmmcIdentificationMode (
 Status = EmmcGetDeviceState (MmcHostInstance, );
 if (EFI_ERROR (Status)) {
   DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): Failed to get device 
state, Status=%r.\n", Status));
-  return Status;
+  goto FreePageExit;
 }
   } while (State == EMMC_DATA_STATE);
 
@@ -237,12 +242,16 @@ EmmcIdentificationMode (
   Media->LogicalBlocksPerPhysicalBlock = 1;
   Media->IoAlign = 4;
   // Compute last block using bits [215:212] of the ECSD
-  Media->LastBlock = MmcHostInstance->CardInfo.ECSDData.SECTOR_COUNT - 1; // 
eMMC isn't supposed to report this for
+  Media->LastBlock = MmcHostInstance->CardInfo.ECSDData->SECTOR_COUNT - 1; // 
eMMC isn't supposed to report this for
   // Cards <2GB in size, but the model does.
 
   // Setup card type
   MmcHostInstance->CardInfo.CardType = EMMC_CARD;
   return EFI_SUCCESS;
+
+FreePageExit:
+  FreePages (MmcHostInstance->CardInfo.ECSDData, EFI_SIZE_TO_PAGES (sizeof 
(ECSD)));
+  return Status;
 }
 
 STATIC
@@ -258,7 +267,7 @@ InitializeEmmcDevice (
   UINT32 TimingMode[4] = {EMMCHS52DDR1V2, EMMCHS52DDR1V8, EMMCHS52, 
EMMCHS26};
 
   Host  = MmcHostInstance->MmcHost;
-  ECSDData = >CardInfo.ECSDData;
+  ECSDData = MmcHostInstance->CardInfo.ECSDData;
   if (ECSDData->DEVICE_TYPE == EMMCBACKWARD)
 return EFI_SUCCESS;
 
-- 
1.9.1

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

[edk2] How to add support to different reg offset definition to share the same driver code?

[Jun Nie]

Hi,

I am trying to add support to different reg offset and bit offset in
PL011 UART. It seems impossible to add macro in platform.dsc to enable
undef/redef in the header file with "#ifdef ZX_PL011_FLAG". Is there
any proper way to control the reg/bit offset definition? Or we have to
adopt the Linux driver method with a structure to hold different
offset value and wrap register access function as below? If so,
another Pcd is needed to specify the offset structure index for the
platforms.


static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
[REG_DR] = UART01x_DR,
[REG_ST_DMAWM] = ST_UART011_DMAWM,
[REG_ST_TIMEOUT] = ST_UART011_TIMEOUT,
...
}

static unsigned int pl011_read(const struct uart_amba_port *uap,
unsigned int reg)
{
void __iomem *addr = uap->port.membase + uap->reg_offset[reg];

return (uap->port.iotype == UPIO_MEM32) ?
readl_relaxed(addr) : readw_relaxed(addr);
}

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

[edk2] [PATCH v2] ArmPlatformPkg: Support different PL011 reg offset

[Jun Nie]

ZTE/SanChip version pl011 has different reg offset and bit offset
for some registers.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 ArmPlatformPkg/ArmPlatformPkg.dec  |  1 +
 ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf |  1 +
 ArmPlatformPkg/Include/Drivers/PL011Uart.h | 29 ++
 3 files changed, 31 insertions(+)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec 
b/ArmPlatformPkg/ArmPlatformPkg.dec
index d756fd2..3dd613c 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -97,6 +97,7 @@
   gArmPlatformTokenSpaceGuid.PL011UartInteger|0|UINT32|0x0020
   gArmPlatformTokenSpaceGuid.PL011UartFractional|0|UINT32|0x002D
   gArmPlatformTokenSpaceGuid.PL011UartInterrupt|0x|UINT32|0x002F
+  gArmPlatformTokenSpaceGuid.PL011UartZxRegOffset|0|UINT8|0
 
   ## PL011 Serial Debug UART
   
gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase|0x|UINT64|0x0030
diff --git a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf 
b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
index 0154f3b..257fbc7 100644
--- a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
+++ b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
@@ -39,3 +39,4 @@
 
   gArmPlatformTokenSpaceGuid.PL011UartInteger
   gArmPlatformTokenSpaceGuid.PL011UartFractional
+  gArmPlatformTokenSpaceGuid.PL011UartZxRegOffset
diff --git a/ArmPlatformPkg/Include/Drivers/PL011Uart.h 
b/ArmPlatformPkg/Include/Drivers/PL011Uart.h
index d5e88e8..09d548b 100644
--- a/ArmPlatformPkg/Include/Drivers/PL011Uart.h
+++ b/ArmPlatformPkg/Include/Drivers/PL011Uart.h
@@ -19,6 +19,7 @@
 #include 
 
 // PL011 Registers
+#if !FixedPcdGet8 (PL011UartZxRegOffset)
 #define UARTDR0x000
 #define UARTRSR   0x004
 #define UARTECR   0x004
@@ -34,6 +35,22 @@
 #define UARTMIS   0x040
 #define UARTICR   0x044
 #define UARTDMACR 0x048
+#else
+#define UARTDR0x004
+#define UARTRSR   0x010
+#define UARTECR   0x010
+#define UARTFR0x014
+#define UARTIBRD  0x024
+#define UARTFBRD  0x028
+#define UARTLCR_H 0x030
+#define UARTCR0x034
+#define UARTIFLS  0x038
+#define UARTIMSC  0x040
+#define UARTRIS   0x044
+#define UARTMIS   0x048
+#define UARTICR   0x04c
+#define UARTDMACR 0x050
+#endif
 
 #define UARTPID0  0xFE0
 #define UARTPID1  0xFE4
@@ -47,6 +64,7 @@
 #define UART_STATUS_ERROR_MASK0x0F
 
 // Flag reg bits
+#if !FixedPcdGet8 (PL011UartZxRegOffset)
 #define PL011_UARTFR_RI   (1 << 8)  // Ring indicator
 #define PL011_UARTFR_TXFE (1 << 7)  // Transmit FIFO empty
 #define PL011_UARTFR_RXFF (1 << 6)  // Receive  FIFO full
@@ -56,6 +74,17 @@
 #define PL011_UARTFR_DCD  (1 << 2)  // Data carrier detect
 #define PL011_UARTFR_DSR  (1 << 1)  // Data set ready
 #define PL011_UARTFR_CTS  (1 << 0)  // Clear to send
+#else
+#define PL011_UARTFR_RI   (1 << 0)  // Ring indicator
+#define PL011_UARTFR_TXFE (1 << 7)  // Transmit FIFO empty
+#define PL011_UARTFR_RXFF (1 << 6)  // Receive  FIFO full
+#define PL011_UARTFR_TXFF (1 << 5)  // Transmit FIFO full
+#define PL011_UARTFR_RXFE (1 << 4)  // Receive  FIFO empty
+#define PL011_UARTFR_BUSY (1 << 8)  // UART busy
+#define PL011_UARTFR_DCD  (1 << 2)  // Data carrier detect
+#define PL011_UARTFR_DSR  (1 << 3)  // Data set ready
+#define PL011_UARTFR_CTS  (1 << 1)  // Clear to send
+#endif
 
 // Flag reg bits - alternative names
 #define UART_TX_EMPTY_FLAG_MASK   PL011_UARTFR_TXFE
-- 
1.9.1

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

[edk2] [PATCH v5] EmbeddedPkg/MmcDxe: Align the ExtCSD buffer

[Jun Nie]

ExtCSD structure may be read via DMA. So align it to
page to avoid data corruption.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/Universal/MmcDxe/Mmc.c   |  3 +++
 EmbeddedPkg/Universal/MmcDxe/Mmc.h   |  2 +-
 EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 19 ++-
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.c 
b/EmbeddedPkg/Universal/MmcDxe/Mmc.c
index 30268e3..ce27150 100644
--- a/EmbeddedPkg/Universal/MmcDxe/Mmc.c
+++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.c
@@ -177,6 +177,9 @@ EFI_STATUS DestroyMmcHostInstance (
   if (MmcHostInstance->BlockIo.Media) {
 FreePool(MmcHostInstance->BlockIo.Media);
   }
+  if (MmcHostInstance->CardInfo.ECSDData) {
+FreePages (MmcHostInstance->CardInfo.ECSDData, EFI_SIZE_TO_PAGES (sizeof 
(ECSD)));
+  }
   FreePool (MmcHostInstance);
 
   return Status;
diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h 
b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
index b7b9454..eb4652b 100644
--- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
+++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
@@ -320,7 +320,7 @@ typedef struct  {
   OCR   OCRData;
   CID   CIDData;
   CSD   CSDData;
-  ECSD  ECSDData; // MMC V4 extended card specific
+  ECSD  *ECSDData; // MMC V4 extended card specific
 } CARD_INFO;
 
 typedef struct _MMC_HOST_INSTANCE {
diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c 
b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
index c28207e..7f74c54 100644
--- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
+++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
@@ -13,6 +13,7 @@
 **/
 
 #include 
+#include 
 #include 
 
 #include "Mmc.h"
@@ -210,15 +211,19 @@ EmmcIdentificationMode (
   }
 
   // Fetch ECSD
+  MmcHostInstance->CardInfo.ECSDData = AllocatePages (EFI_SIZE_TO_PAGES 
(sizeof (ECSD)));
+  if (MmcHostInstance->CardInfo.ECSDData == NULL) {
+return EFI_OUT_OF_RESOURCES;
+  }
   Status = Host->SendCommand (Host, MMC_CMD8, 0);
   if (EFI_ERROR (Status)) {
 DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD fetch error, 
Status=%r.\n", Status));
   }
 
-  Status = Host->ReadBlockData (Host, 0, 512, (UINT32 
*)&(MmcHostInstance->CardInfo.ECSDData));
+  Status = Host->ReadBlockData (Host, 0, 512, (UINT32 
*)MmcHostInstance->CardInfo.ECSDData);
   if (EFI_ERROR (Status)) {
 DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD read error, 
Status=%r.\n", Status));
-return Status;
+goto FreePageExit;
   }
 
   // Make sure device exiting data mode
@@ -226,7 +231,7 @@ EmmcIdentificationMode (
 Status = EmmcGetDeviceState (MmcHostInstance, );
 if (EFI_ERROR (Status)) {
   DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): Failed to get device 
state, Status=%r.\n", Status));
-  return Status;
+  goto FreePageExit;
 }
   } while (State == EMMC_DATA_STATE);
 
@@ -237,12 +242,16 @@ EmmcIdentificationMode (
   Media->LogicalBlocksPerPhysicalBlock = 1;
   Media->IoAlign = 4;
   // Compute last block using bits [215:212] of the ECSD
-  Media->LastBlock = MmcHostInstance->CardInfo.ECSDData.SECTOR_COUNT - 1; // 
eMMC isn't supposed to report this for
+  Media->LastBlock = MmcHostInstance->CardInfo.ECSDData->SECTOR_COUNT - 1; // 
eMMC isn't supposed to report this for
   // Cards <2GB in size, but the model does.
 
   // Setup card type
   MmcHostInstance->CardInfo.CardType = EMMC_CARD;
   return EFI_SUCCESS;
+
+FreePageExit:
+  FreePages (MmcHostInstance->CardInfo.ECSDData, EFI_SIZE_TO_PAGES (sizeof 
(ECSD)));
+  return Status;
 }
 
 STATIC
@@ -258,7 +267,7 @@ InitializeEmmcDevice (
   UINT32 TimingMode[4] = {EMMCHS52DDR1V2, EMMCHS52DDR1V8, EMMCHS52, 
EMMCHS26};
 
   Host  = MmcHostInstance->MmcHost;
-  ECSDData = >CardInfo.ECSDData;
+  ECSDData = MmcHostInstance->CardInfo.ECSDData;
   if (ECSDData->DEVICE_TYPE == EMMCBACKWARD)
 return EFI_SUCCESS;
 
-- 
1.9.1

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

Re: [edk2] How to add support to different reg offset definition to share the same driver code?

[Jun Nie]

2017-06-30 21:29 GMT+08:00 Gao, Liming <liming@intel.com>:
> Jun:
>  You can add C MACRO in [BuildOptions] of Platform.dsc, then use DSC flag to 
> control it.
>
> For example: Platform.dsc
> [Defines]
> DEFINE ZX_PL011_FLAG = FALSE
>
> [BuildOptions]
> !if $(ZX_PL011_FLAG) == TRUE
> *_*_*_CC_FLAGS = -D ZX_PL011_FLAG
> !endif
>
> Thanks
> Liming

Thanks for your demo code. It help a lot to a new comer.

Jun
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jun 
>> Nie
>> Sent: Friday, June 30, 2017 11:35 AM
>> To: Leif Lindholm <leif.lindh...@linaro.org>; Ard Biesheuvel 
>> <ard.biesheu...@linaro.org>; edk2-devel@lists.01.org;
>> linaro-u...@lists.linaro.org; alexei.fedo...@arm.com; evan.ll...@arm.com
>> Subject: [edk2] How to add support to different reg offset definition to 
>> share the same driver code?
>>
>> Hi,
>>
>> I am trying to add support to different reg offset and bit offset in
>> PL011 UART. It seems impossible to add macro in platform.dsc to enable
>> undef/redef in the header file with "#ifdef ZX_PL011_FLAG". Is there
>> any proper way to control the reg/bit offset definition? Or we have to
>> adopt the Linux driver method with a structure to hold different
>> offset value and wrap register access function as below? If so,
>> another Pcd is needed to specify the offset structure index for the
>> platforms.
>>
>>
>> static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
>> [REG_DR] = UART01x_DR,
>> [REG_ST_DMAWM] = ST_UART011_DMAWM,
>> [REG_ST_TIMEOUT] = ST_UART011_TIMEOUT,
>> ...
>> }
>>
>> static unsigned int pl011_read(const struct uart_amba_port *uap,
>> unsigned int reg)
>> {
>> void __iomem *addr = uap->port.membase + uap->reg_offset[reg];
>>
>> return (uap->port.iotype == UPIO_MEM32) ?
>> readl_relaxed(addr) : readw_relaxed(addr);
>> }
>>
>> Jun
>> ___
>> 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] [PATCH] ArmPlatformPkg: Support different reg offset to PL011

[Jun Nie]

ZTE SoC has different offset for some registers and bits.
Add a macro flag to undef/redef those value. The macro
flag can be enabled in BuildOptions section of platform.dsc.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 ArmPlatformPkg/Include/Drivers/PL011Uart.h | 40 ++
 1 file changed, 40 insertions(+)

diff --git a/ArmPlatformPkg/Include/Drivers/PL011Uart.h 
b/ArmPlatformPkg/Include/Drivers/PL011Uart.h
index d5e88e8..87fab60 100644
--- a/ArmPlatformPkg/Include/Drivers/PL011Uart.h
+++ b/ArmPlatformPkg/Include/Drivers/PL011Uart.h
@@ -40,6 +40,34 @@
 #define UARTPID2  0xFE8
 #define UARTPID3  0xFEC
 
+#ifdef ZX_PL011_FLAG
+#undef UARTDR
+#undef UARTFR
+#undef UARTIBRD
+#undef UARTFBRD
+#undef UARTLCR_H
+#undef UARTCR
+#undef UARTIFLS
+#undef UARTIMSC
+#undef UARTRIS
+#undef UARTMIS
+#undef UARTICR
+#undef UARTDMACR
+
+#define UARTDR0x004
+#define UARTFR0x014
+#define UARTIBRD  0x024
+#define UARTFBRD  0x028
+#define UARTLCR_H 0x030
+#define UARTCR0x034
+#define UARTIFLS  0x038
+#define UARTIMSC  0x040
+#define UARTRIS   0x044
+#define UARTMIS   0x048
+#define UARTICR   0x04c
+#define UARTDMACR 0x050
+#endif
+
 // Data status bits
 #define UART_DATA_ERROR_MASK  0x0F00
 
@@ -57,6 +85,18 @@
 #define PL011_UARTFR_DSR  (1 << 1)  // Data set ready
 #define PL011_UARTFR_CTS  (1 << 0)  // Clear to send
 
+#ifdef ZX_PL011_FLAG
+#undef PL011_UARTFR_RI
+#undef PL011_UARTFR_BUSY
+#undef PL011_UARTFR_DSR
+#undef PL011_UARTFR_CTS
+
+#define PL011_UARTFR_RI   0x001  // Ring indicator
+#define PL011_UARTFR_BUSY 0x100 // UART busy
+#define PL011_UARTFR_DSR  0x008  // Data set ready
+#define PL011_UARTFR_CTS  0x002  // Clear to send
+#endif
+
 // Flag reg bits - alternative names
 #define UART_TX_EMPTY_FLAG_MASK   PL011_UARTFR_TXFE
 #define UART_RX_FULL_FLAG_MASKPL011_UARTFR_RXFF
-- 
1.9.1

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

[edk2] [PATCH 2/2] EmbeddedPkg/DwEmmc: Adjust FIFO threshold

[Jun Nie]

Adjust FIFO threshold according to FIFO depth. Skip
the adjustment if we do not have FIFO depth info.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h  |  6 
 EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c   | 54 +
 EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf |  1 +
 EmbeddedPkg/EmbeddedPkg.dec |  1 +
 4 files changed, 62 insertions(+)

diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h 
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h
index 055f1e0..2b41539 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h
@@ -38,7 +38,10 @@
 #define DWEMMC_RINTSTS  ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x044)
 #define DWEMMC_STATUS   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x048)
 #define DWEMMC_FIFOTH   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x04c)
+#define DWEMMC_TCBCNT   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x05c)
+#define DWEMMC_TBBCNT   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x060)
 #define DWEMMC_DEBNCE   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x064)
+#define DWEMMC_HCON ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x070)
 #define DWEMMC_UHSREG   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x074)
 #define DWEMMC_BMOD ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x080)
 #define DWEMMC_DBADDR   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x088)
@@ -47,6 +50,7 @@
 #define DWEMMC_DSCADDR  ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x094)
 #define DWEMMC_BUFADDR  ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x098)
 #define DWEMMC_CARDTHRCTL   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0X100)
+#define DWEMMC_DATA ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0X200)
 
 #define CMD_UPDATE_CLK  0x80202000
 #define CMD_START_BIT   (1 << 31)
@@ -124,4 +128,6 @@
 #define DWEMMC_CARD_RD_THR(x)   ((x & 0xfff) << 16)
 #define DWEMMC_CARD_RD_THR_EN   (1 << 0)
 
+#define DWEMMC_GET_HDATA_WIDTH(x)   (((x)>>7) & 0x7)
+
 #endif  // __DWEMMC_H__
diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c 
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
index c67dd0d..cb32a7c 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
@@ -415,6 +415,59 @@ DwEmmcReceiveResponse (
   return EFI_SUCCESS;
 }
 
+VOID DwEmmcAdjustFifoth(
+  VOID
+  )
+{
+  const UINT32 Mszs[] = {1, 4, 8, 16, 32, 64, 128, 256};
+  UINT32 BlkSizeDepth, Fifoth, FifoWidth, FifoDepth;
+  UINT32 BlkSize = 512, Msize = 0, RxWmark = 1, TxWmark, TxWmarkInvers;
+  UINT32 Idx = ARRAY_SIZE(Mszs) - 1;
+
+  /* Skip FIFO adjustment if we do not have platform FIFO depth info */
+  FifoDepth = PcdGet32 (PcdDwEmmcDxeFifoDepth);
+  if (!FifoDepth)
+return;
+
+  TxWmark = FifoDepth / 2;
+  TxWmarkInvers = FifoDepth - TxWmark;
+
+  FifoWidth = DWEMMC_GET_HDATA_WIDTH(MmioRead32 (DWEMMC_HCON));
+  if (!FifoWidth) {
+FifoWidth = 2;
+  } else if (FifoWidth == 2) {
+FifoWidth = 8;
+  } else {
+FifoWidth = 4;
+  }
+
+  BlkSizeDepth = BlkSize / FifoWidth;
+
+  /*
+   * MSIZE is '1',
+   * if BlkSize is not a multiple of the FIFO width
+   */
+  if (BlkSize % FifoWidth) {
+goto done;
+  }
+
+  do {
+if (!((BlkSizeDepth % Mszs[Idx]) || (TxWmarkInvers % Mszs[Idx]))) {
+  Msize = Idx;
+  RxWmark = Mszs[Idx] - 1;
+  break;
+}
+  } while (--Idx > 0);
+  /*
+   * If Idx is '0', it won't be tried
+   * Thus, initial values are uesed
+   */
+done:
+  Fifoth = DWEMMC_DMA_BURST_SIZE(Msize) | DWEMMC_FIFO_TWMARK(TxWmark)
+   | DWEMMC_FIFO_RWMARK(RxWmark);
+  MmioWrite32 (DWEMMC_FIFOTH, Fifoth);
+}
+
 EFI_STATUS
 PrepareDmaData (
   IN DWEMMC_IDMAC_DESCRIPTOR*IdmacDesc,
@@ -632,6 +685,7 @@ DwEmmcDxeInitialize (
 
   Handle = NULL;
 
+  DwEmmcAdjustFifoth();
   gpIdmacDesc = (DWEMMC_IDMAC_DESCRIPTOR *)AllocatePages 
(DWEMMC_MAX_DESC_PAGES);
   if (gpIdmacDesc == NULL) {
 return EFI_BUFFER_TOO_SMALL;
diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf 
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
index 3582997..a3e10fe 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
@@ -49,6 +49,7 @@
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeClockFrequencyInHz
   gDwEmmcDxeTokenSpaceGuid.PcdDwEmmcDxeMaxClockFrequencyInHz
+  gDwEmmcDxeTokenSpaceGuid.PcdDwEmmcDxeFifoDepth
 
 [Depex]
   TRUE
diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index aec8259..f28a5d2 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -168,6 +168,7 @@
   gEmbeddedTokenSpaceGuid.PcdD

[edk2] [PATCH] EmbeddedPkg/DwEmmcDxe: limit max clock for platform

[Jun Nie]

Some boards may have max clock limitation. Add a Pcd to notify
driver.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c   | 4 
 EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf | 1 +
 EmbeddedPkg/EmbeddedPkg.dec | 1 +
 3 files changed, 6 insertions(+)

diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c 
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
index fe23d11..308f3a7 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
@@ -560,6 +560,10 @@ DwEmmcSetIos (
   EFI_STATUS Status = EFI_SUCCESS;
   UINT32Data;
 
+  if (BusClockFreq > PcdGet32 (PcdDwEmmcDxeMaxClockFrequencyInHz)) {
+return EFI_UNSUPPORTED;
+  }
+
   if (TimingMode != EMMCBACKWARD) {
 Data = MmioRead32 (DWEMMC_UHSREG);
 switch (TimingMode) {
diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf 
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
index e3c8313..3582997 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
@@ -48,6 +48,7 @@
 [Pcd]
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeClockFrequencyInHz
+  gDwEmmcDxeTokenSpaceGuid.PcdDwEmmcDxeMaxClockFrequencyInHz
 
 [Depex]
   TRUE
diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 0d4a062..aec8259 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -167,6 +167,7 @@
   # DwEmmc Driver PCDs
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress|0x0|UINT32|0x0035
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeClockFrequencyInHz|0x0|UINT32|0x0036
+  
gDwEmmcDxeTokenSpaceGuid.PcdDwEmmcDxeMaxClockFrequencyInHz|0x0|UINT32|4
 
   #
   # Android FastBoot
-- 
1.9.1

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

Re: [edk2] How to add support to different reg offset definition to share the same driver code?

[Jun Nie]

2017-06-30 19:01 GMT+08:00 Leif Lindholm <leif.lindh...@linaro.org>:
> Hi Jun,
>
> I think there is more than one benefit in mimicing the Linux driver,
> so I would lean towards the Pcd option. But as Ard points out to me,
> it needs to use a FixedPcd (using FixedPcdGet()) - this can only ever
> have a buildtime resolution.
>
> Regards,
>
> Leif (technically on holiday, so no patch review until Monday)


Just learn from Liming that a MACRO is OK to control the register
offset definition in header file as we can change build option in
platform.dsc file. This should be most clear method to minimize impact
to other platform.

Jun

>
> On Fri, Jun 30, 2017 at 11:35:26AM +0800, Jun Nie wrote:
>> Hi,
>>
>> I am trying to add support to different reg offset and bit offset in
>> PL011 UART. It seems impossible to add macro in platform.dsc to enable
>> undef/redef in the header file with "#ifdef ZX_PL011_FLAG". Is there
>> any proper way to control the reg/bit offset definition? Or we have to
>> adopt the Linux driver method with a structure to hold different
>> offset value and wrap register access function as below? If so,
>> another Pcd is needed to specify the offset structure index for the
>> platforms.
>>
>>
>> static u16 pl011_st_offsets[REG_ARRAY_SIZE] = {
>> [REG_DR] = UART01x_DR,
>> [REG_ST_DMAWM] = ST_UART011_DMAWM,
>> [REG_ST_TIMEOUT] = ST_UART011_TIMEOUT,
>> ...
>> }
>>
>> static unsigned int pl011_read(const struct uart_amba_port *uap,
>> unsigned int reg)
>> {
>> void __iomem *addr = uap->port.membase + uap->reg_offset[reg];
>>
>> return (uap->port.iotype == UPIO_MEM32) ?
>> readl_relaxed(addr) : readw_relaxed(addr);
>> }
>>
>> Jun
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 1/2] EmbeddedPkg/DwEmmcDxe: limit max clock for platform

[Jun Nie]

Some boards may have max clock limitation. Add a Pcd to notify
driver.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c   | 4 
 EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf | 1 +
 EmbeddedPkg/EmbeddedPkg.dec | 1 +
 3 files changed, 6 insertions(+)

diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c 
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
index fe23d11..bb26b69 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
@@ -560,6 +560,10 @@ DwEmmcSetIos (
   EFI_STATUS Status = EFI_SUCCESS;
   UINT32Data;
 
+  if ((PcdGet32 (PcdDwEmmcDxeMaxClockFreqInHz) != 0) &&
+  (BusClockFreq > PcdGet32 (PcdDwEmmcDxeMaxClockFreqInHz))) {
+return EFI_UNSUPPORTED;
+  }
   if (TimingMode != EMMCBACKWARD) {
 Data = MmioRead32 (DWEMMC_UHSREG);
 switch (TimingMode) {
diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf 
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
index e3c8313..99b4f99 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
@@ -48,6 +48,7 @@
 [Pcd]
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeClockFrequencyInHz
+  gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeMaxClockFreqInHz
 
 [Depex]
   TRUE
diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 5ea2f22..2da9b2f 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -169,6 +169,7 @@
   # DwEmmc Driver PCDs
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress|0x0|UINT32|0x0035
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeClockFrequencyInHz|0x0|UINT32|0x0036
+  gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeMaxClockFreqInHz|0x0|UINT32|0x0037
 
   #
   # Android FastBoot
-- 
1.9.1

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

[edk2] [PATCH v2 2/2] EmbeddedPkg/DwEmmc: Adjust FIFO threshold

[Jun Nie]

Adjust FIFO threshold according to FIFO depth. Skip
the adjustment if we do not have FIFO depth info.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h  |  6 
 EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c   | 50 +
 EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf |  1 +
 EmbeddedPkg/EmbeddedPkg.dec |  1 +
 4 files changed, 58 insertions(+)

diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h 
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h
index 055f1e0..90c7676 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmc.h
@@ -38,7 +38,10 @@
 #define DWEMMC_RINTSTS  ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x044)
 #define DWEMMC_STATUS   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x048)
 #define DWEMMC_FIFOTH   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x04c)
+#define DWEMMC_TCBCNT   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x05c)
+#define DWEMMC_TBBCNT   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x060)
 #define DWEMMC_DEBNCE   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x064)
+#define DWEMMC_HCON ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x070)
 #define DWEMMC_UHSREG   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x074)
 #define DWEMMC_BMOD ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x080)
 #define DWEMMC_DBADDR   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x088)
@@ -47,6 +50,7 @@
 #define DWEMMC_DSCADDR  ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x094)
 #define DWEMMC_BUFADDR  ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0x098)
 #define DWEMMC_CARDTHRCTL   ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0X100)
+#define DWEMMC_DATA ((UINT32)PcdGet32 (PcdDwEmmcDxeBaseAddress) + 
0X200)
 
 #define CMD_UPDATE_CLK  0x80202000
 #define CMD_START_BIT   (1 << 31)
@@ -124,4 +128,6 @@
 #define DWEMMC_CARD_RD_THR(x)   ((x & 0xfff) << 16)
 #define DWEMMC_CARD_RD_THR_EN   (1 << 0)
 
+#define DWEMMC_GET_HDATA_WIDTH(x)   (((x) >> 7) & 0x7)
+
 #endif  // __DWEMMC_H__
diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c 
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
index bb26b69..70e064d 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.c
@@ -415,6 +415,55 @@ DwEmmcReceiveResponse (
   return EFI_SUCCESS;
 }
 
+VOID DwEmmcAdjustFifothreshold (
+  VOID
+  )
+{
+  /* DMA multiple transaction size map to reg value as array index */
+  CONST UINT32 BurstSize[] = {1, 4, 8, 16, 32, 64, 128, 256};
+  UINT32 BlkDepthInFifo, Fifoth, FifoWidth, FifoDepth;
+  UINT32 BlkSize = DWEMMC_BLOCK_SIZE, Idx = 0, RxWmark = 1, TxWmark, 
TxWmarkInvers;
+
+  /* Skip FIFO adjustment if we do not have platform FIFO depth info */
+  FifoDepth = PcdGet32 (PcdDwEmmcDxeFifoDepth);
+  if (!FifoDepth) {
+return;
+  }
+
+  TxWmark = FifoDepth / 2;
+  TxWmarkInvers = FifoDepth - TxWmark;
+
+  FifoWidth = DWEMMC_GET_HDATA_WIDTH (MmioRead32 (DWEMMC_HCON));
+  if (!FifoWidth) {
+FifoWidth = 2;
+  } else if (FifoWidth == 2) {
+FifoWidth = 8;
+  } else {
+FifoWidth = 4;
+  }
+
+  BlkDepthInFifo = BlkSize / FifoWidth;
+
+  /* if BlkSize is not a multiple of the FIFO width */
+  if (BlkSize % FifoWidth) {
+goto done;
+  }
+
+  Idx = ARRAY_SIZE (BurstSize) - 1;
+  while (Idx && ((BlkDepthInFifo % BurstSize[Idx]) || (TxWmarkInvers % 
BurstSize[Idx]))) {
+Idx--;
+  }
+  RxWmark = BurstSize[Idx] - 1;
+  /*
+   * If Idx is '0', it won't be tried
+   * Thus, initial values are used
+   */
+done:
+  Fifoth = DWEMMC_DMA_BURST_SIZE (Idx) | DWEMMC_FIFO_TWMARK (TxWmark)
+   | DWEMMC_FIFO_RWMARK (RxWmark);
+  MmioWrite32 (DWEMMC_FIFOTH, Fifoth);
+}
+
 EFI_STATUS
 PrepareDmaData (
   IN DWEMMC_IDMAC_DESCRIPTOR*IdmacDesc,
@@ -633,6 +682,7 @@ DwEmmcDxeInitialize (
 
   Handle = NULL;
 
+  DwEmmcAdjustFifothreshold ();
   gpIdmacDesc = (DWEMMC_IDMAC_DESCRIPTOR *)AllocatePages 
(DWEMMC_MAX_DESC_PAGES);
   if (gpIdmacDesc == NULL) {
 return EFI_BUFFER_TOO_SMALL;
diff --git a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf 
b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
index 99b4f99..bc4413e 100644
--- a/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
+++ b/EmbeddedPkg/Drivers/DwEmmcDxe/DwEmmcDxe.inf
@@ -49,6 +49,7 @@
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeBaseAddress
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeClockFrequencyInHz
   gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeMaxClockFreqInHz
+  gEmbeddedTokenSpaceGuid.PcdDwEmmcDxeFifoDepth
 
 [Depex]
   TRUE
diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 2da9b2f..5f39d9d 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ 

[edk2] [PATCH v2 2/2] EmbeddedPkg: add Android boot device path and guid

[Jun Nie]

The device path specifies where to load android boot image.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/EmbeddedPkg.dec | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 4cd528a..cf89af2 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -80,6 +80,7 @@
   gAndroidFastbootPlatformProtocolGuid =  { 0x524685a0, 0x89a0, 0x11e3, {0x9d, 
0x4d, 0xbf, 0xa9, 0xf6, 0xa4, 0x03, 0x08}}
   gUsbDeviceProtocolGuid =  { 0x021bd2ca, 0x51d2, 0x11e3, {0x8e, 0x56, 0xb7, 
0x54, 0x17, 0xc7,  0x0b, 0x44 }}
   gPlatformGpioProtocolGuid = { 0x52ce9845, 0x5af4, 0x43e2, {0xba, 0xfd, 0x23, 
0x08, 0x12, 0x54, 0x7a, 0xc2 }}
+  gAbootimgProtocolGuid = { 0x9859bb19, 0x407c, 0x4f8b, {0xbc, 0xe1, 0xf8, 
0xda, 0x65, 0x65, 0xf4, 0xa5 }}
 
 [PcdsFeatureFlag.common]
   gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|FALSE|BOOLEAN|0x0001
@@ -181,6 +182,7 @@
   
gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbProductId|0xbeef|UINT32|0x0023
   gEmbeddedTokenSpaceGuid.PcdAndroidFastbootTcpPort|1234|UINT32|0x0024
 
+  gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath|L""|VOID*|0x0025
 
 [PcdsFixedAtBuild.ARM]
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|32|UINT8|0x0010
-- 
1.9.1

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

[edk2] [PATCH v2 1/2] EmbeddedPkg/AndroidBoot: boot android kernel from storage

[Jun Nie]

Add an android kernel loader that could load kernel from storage
device. This patch is from Haojian's code. The minor change
is that alternative dtb is searched in second loader binary of
Android bootimage if dtb is not found after Linux kernel.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 .../Application/AndroidBoot/AndroidBootApp.c   | 129 
 .../Application/AndroidBoot/AndroidBootApp.inf |  64 
 EmbeddedPkg/Include/Library/AbootimgLib.h  |  65 
 EmbeddedPkg/Include/Protocol/Abootimg.h|  47 +++
 EmbeddedPkg/Library/AbootimgLib/AbootimgLib.c  | 350 +
 EmbeddedPkg/Library/AbootimgLib/AbootimgLib.inf|  48 +++
 6 files changed, 703 insertions(+)
 create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
 create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
 create mode 100644 EmbeddedPkg/Include/Library/AbootimgLib.h
 create mode 100644 EmbeddedPkg/Include/Protocol/Abootimg.h
 create mode 100644 EmbeddedPkg/Library/AbootimgLib/AbootimgLib.c
 create mode 100644 EmbeddedPkg/Library/AbootimgLib/AbootimgLib.inf

diff --git a/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c 
b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
new file mode 100644
index 000..9ed931b
--- /dev/null
+++ b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
@@ -0,0 +1,129 @@
+/** @file
+
+  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.
+  Copyright (c) 2017, Linaro. 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
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#define IS_DEVICE_PATH_NODE(node,type,subtype) (((node)->Type == (type)) && 
((node)->SubType == (subtype)))
+
+#define ALIGN(x, a)(((x) + ((a) - 1)) & ~((a) - 1))
+
+EFI_STATUS
+EFIAPI
+AndroidBootAppEntryPoint (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+  CHAR16  *BootPathStr;
+  EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
+  EFI_DEVICE_PATH *DevicePath;
+  EFI_DEVICE_PATH_PROTOCOL*Node, *NextNode;
+  EFI_BLOCK_IO_PROTOCOL   *BlockIo;
+  UINT32  MediaId, BlockSize;
+  VOID*Buffer;
+  EFI_HANDLE  Handle;
+  UINTN   Size;
+
+  BootPathStr = (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);
+  ASSERT (BootPathStr != NULL);
+  Status = gBS->LocateProtocol (, NULL, 
(VOID **));
+  ASSERT_EFI_ERROR(Status);
+  DevicePath = (EFI_DEVICE_PATH 
*)EfiDevicePathFromTextProtocol->ConvertTextToDevicePath (BootPathStr);
+  ASSERT (DevicePath != NULL);
+
+  /* Find DevicePath node of Partition */
+  NextNode = DevicePath;
+  while (1) {
+Node = NextNode;
+if (IS_DEVICE_PATH_NODE (Node, MEDIA_DEVICE_PATH, MEDIA_HARDDRIVE_DP)) {
+  break;
+}
+NextNode = NextDevicePathNode (Node);
+  }
+
+  Status = gBS->LocateDevicePath (, , 
);
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  Status = gBS->OpenProtocol (
+  Handle,
+  ,
+  (VOID **) ,
+  gImageHandle,
+  NULL,
+  EFI_OPEN_PROTOCOL_GET_PROTOCOL
+  );
+  if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_ERROR, "Failed to get BlockIo: %r\n", Status));
+return Status;
+  }
+
+  MediaId = BlockIo->Media->MediaId;
+  BlockSize = BlockIo->Media->BlockSize;
+  Buffer = AllocatePages (1);
+  if (Buffer == NULL) {
+return EFI_BUFFER_TOO_SMALL;
+  }
+  /* Load header of boot.img */
+  Status = BlockIo->ReadBlocks (
+  BlockIo,
+  MediaId,
+  0,
+  BlockSize,
+  Buffer
+  );
+  Status = AbootimgGetImgSize (Buffer, );
+  if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_ERROR, "Failed to get Abootimg Size: %r\n", Status));
+return Status;
+  }
+  Size = ALIGN (Size, BlockSize);
+  FreePages (Buffer, 1);
+
+  /* Both PartitionStart and PartitionSize are counted as block size. */
+  Buffer = AllocatePages (EFI_SIZE_TO_PAGES (Size));
+  if (Buffer == NULL) {
+return EFI_BUFFER_TOO_SMALL;
+  }
+
+  /* Load heade

Re: [edk2] [PATCH v2] ArmPlatformPkg: Support different PL011 reg offset

[Jun Nie]

2017-07-06 0:36 GMT+08:00 Leif Lindholm <leif.lindh...@linaro.org>:
> On Tue, Jul 04, 2017 at 11:43:38PM +0800, Jun Nie wrote:
>> ZTE/SanChip version pl011 has different reg offset and bit offset
>> for some registers.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jun Nie <jun@linaro.org>
>> ---
>>  ArmPlatformPkg/ArmPlatformPkg.dec  |  1 +
>>  ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf |  1 +
>>  ArmPlatformPkg/Include/Drivers/PL011Uart.h | 29 
>> ++
>>  3 files changed, 31 insertions(+)
>>
>> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec 
>> b/ArmPlatformPkg/ArmPlatformPkg.dec
>> index d756fd2..3dd613c 100644
>> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
>> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
>> @@ -97,6 +97,7 @@
>>gArmPlatformTokenSpaceGuid.PL011UartInteger|0|UINT32|0x0020
>>gArmPlatformTokenSpaceGuid.PL011UartFractional|0|UINT32|0x002D
>>gArmPlatformTokenSpaceGuid.PL011UartInterrupt|0x|UINT32|0x002F
>> +  gArmPlatformTokenSpaceGuid.PL011UartZxRegOffset|0|UINT8|0
>
> I'm basically OK with this patch, but if we have multiple variants of
> this, as the Linux driver suggests, I think we're looking at something
> more like PL011UartRegOffsetVariant, with a numerical value for each
> special flavour.
>
>>
>>## PL011 Serial Debug UART
>>
>> gArmPlatformTokenSpaceGuid.PcdSerialDbgRegisterBase|0x|UINT64|0x0030
>> diff --git a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf 
>> b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
>> index 0154f3b..257fbc7 100644
>> --- a/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
>> +++ b/ArmPlatformPkg/Drivers/PL011Uart/PL011Uart.inf
>> @@ -39,3 +39,4 @@
>>
>>gArmPlatformTokenSpaceGuid.PL011UartInteger
>>gArmPlatformTokenSpaceGuid.PL011UartFractional
>> +  gArmPlatformTokenSpaceGuid.PL011UartZxRegOffset
>> diff --git a/ArmPlatformPkg/Include/Drivers/PL011Uart.h 
>> b/ArmPlatformPkg/Include/Drivers/PL011Uart.h
>> index d5e88e8..09d548b 100644
>> --- a/ArmPlatformPkg/Include/Drivers/PL011Uart.h
>> +++ b/ArmPlatformPkg/Include/Drivers/PL011Uart.h
>> @@ -19,6 +19,7 @@
>>  #include 
>>
>>  // PL011 Registers
>> +#if !FixedPcdGet8 (PL011UartZxRegOffset)
>
> And as such, more a test like...
> #if FixedPcdGet8 (PL011UartRegOffsetVariant) == PL011_VARIANT_ZTE

Yes, it is more generic. Which header is proper file to add value
definition of  PL011_VARIANT_ZTE that can be included by platform.dsc?

Jun

>
> /
> Leif
>
>>  #define UARTDR0x000
>>  #define UARTRSR   0x004
>>  #define UARTECR   0x004
>> @@ -34,6 +35,22 @@
>>  #define UARTMIS   0x040
>>  #define UARTICR   0x044
>>  #define UARTDMACR 0x048
>> +#else
>> +#define UARTDR0x004
>> +#define UARTRSR   0x010
>> +#define UARTECR   0x010
>> +#define UARTFR0x014
>> +#define UARTIBRD  0x024
>> +#define UARTFBRD  0x028
>> +#define UARTLCR_H 0x030
>> +#define UARTCR0x034
>> +#define UARTIFLS  0x038
>> +#define UARTIMSC  0x040
>> +#define UARTRIS   0x044
>> +#define UARTMIS   0x048
>> +#define UARTICR   0x04c
>> +#define UARTDMACR 0x050
>> +#endif
>>
>>  #define UARTPID0  0xFE0
>>  #define UARTPID1  0xFE4
>> @@ -47,6 +64,7 @@
>>  #define UART_STATUS_ERROR_MASK0x0F
>>
>>  // Flag reg bits
>> +#if !FixedPcdGet8 (PL011UartZxRegOffset)
>>  #define PL011_UARTFR_RI   (1 << 8)  // Ring indicator
>>  #define PL011_UARTFR_TXFE (1 << 7)  // Transmit FIFO empty
>>  #define PL011_UARTFR_RXFF (1 << 6)  // Receive  FIFO full
>> @@ -56,6 +74,17 @@
>>  #define PL011_UARTFR_DCD  (1 << 2)  // Data carrier detect
>>  #define PL011_UARTFR_DSR  (1 << 1)  // Data set ready
>>  #define PL011_UARTFR_CTS  (1 << 0)  // Clear to send
>> +#else
>> +#define PL011_UARTFR_RI   (1 << 0)  // Ring indicator
>> +#define PL011_UARTFR_TXFE (1 << 7)  // Transmit FIFO empty
>> +#define PL011_UARTFR_RXFF (1 << 6)  // Receive  FIFO full
>> +#define PL011_UARTFR_TXFF (1 << 5)  // Transmit FIFO full
>> +#define PL011_UARTFR_RXFE (1 << 4)  // Receive  FIFO empty
>> +#define PL011_UARTFR_BUSY (1 << 8)  // UART busy
>> +#define PL011_UARTFR_DCD  (1 << 2)  // Data carrier detect
>> +#define PL011_UARTFR_DSR  (1 << 3)  // Data set ready
>> +#define PL011_UARTFR_CTS  (1 << 1)  // Clear to send
>> +#endif
>>
>>  // Flag reg bits - alternative names
>>  #define UART_TX_EMPTY_FLAG_MASK   PL011_UARTFR_TXFE
>> --
>> 1.9.1
>>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] EmbeddedPkg/MmcDxe: Align the ExtCSD buffer

[Jun Nie]

ExtCSD structure may be read via DMA. So align it to
page to avoid data corruption.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/Universal/MmcDxe/Mmc.h   |  2 +-
 EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 11 ---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h 
b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
index 8a7d5a3..f3e56ff 100644
--- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
+++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
@@ -319,7 +319,7 @@ typedef struct  {
   OCR   OCRData;
   CID   CIDData;
   CSD   CSDData;
-  ECSD  ECSDData; // MMC V4 extended card specific
+  ECSD  *ECSDData; // MMC V4 extended card specific
 } CARD_INFO;
 
 typedef struct _MMC_HOST_INSTANCE {
diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c 
b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
index c28207e..6bb65c3 100644
--- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
+++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
@@ -13,6 +13,7 @@
 **/
 
 #include 
+#include 
 #include 
 
 #include "Mmc.h"
@@ -210,12 +211,16 @@ EmmcIdentificationMode (
   }
 
   // Fetch ECSD
+  MmcHostInstance->CardInfo.ECSDData = AllocatePages (1);
+  if (MmcHostInstance->CardInfo.ECSDData == NULL) {
+return EFI_BUFFER_TOO_SMALL;
+  }
   Status = Host->SendCommand (Host, MMC_CMD8, 0);
   if (EFI_ERROR (Status)) {
 DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD fetch error, 
Status=%r.\n", Status));
   }
 
-  Status = Host->ReadBlockData (Host, 0, 512, (UINT32 
*)&(MmcHostInstance->CardInfo.ECSDData));
+  Status = Host->ReadBlockData (Host, 0, 512, (UINT32 
*)MmcHostInstance->CardInfo.ECSDData);
   if (EFI_ERROR (Status)) {
 DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD read error, 
Status=%r.\n", Status));
 return Status;
@@ -237,7 +242,7 @@ EmmcIdentificationMode (
   Media->LogicalBlocksPerPhysicalBlock = 1;
   Media->IoAlign = 4;
   // Compute last block using bits [215:212] of the ECSD
-  Media->LastBlock = MmcHostInstance->CardInfo.ECSDData.SECTOR_COUNT - 1; // 
eMMC isn't supposed to report this for
+  Media->LastBlock = MmcHostInstance->CardInfo.ECSDData->SECTOR_COUNT - 1; // 
eMMC isn't supposed to report this for
   // Cards <2GB in size, but the model does.
 
   // Setup card type
@@ -258,7 +263,7 @@ InitializeEmmcDevice (
   UINT32 TimingMode[4] = {EMMCHS52DDR1V2, EMMCHS52DDR1V8, EMMCHS52, 
EMMCHS26};
 
   Host  = MmcHostInstance->MmcHost;
-  ECSDData = >CardInfo.ECSDData;
+  ECSDData = MmcHostInstance->CardInfo.ECSDData;
   if (ECSDData->DEVICE_TYPE == EMMCBACKWARD)
 return EFI_SUCCESS;
 
-- 
1.9.1

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

[edk2] [PATCH] EmbeddedPkg/MmcDxe: Correct argument of ECSD read

[Jun Nie]

The argument of CMD8 should be stuff bits according to standard
JESD84-A44.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c 
b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
index 4ce0ddd..c28207e 100644
--- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
+++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
@@ -210,7 +210,7 @@ EmmcIdentificationMode (
   }
 
   // Fetch ECSD
-  Status = Host->SendCommand (Host, MMC_CMD8, RCA);
+  Status = Host->SendCommand (Host, MMC_CMD8, 0);
   if (EFI_ERROR (Status)) {
 DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD fetch error, 
Status=%r.\n", Status));
   }
-- 
1.9.1

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

Re: [edk2] [PATCH] EmbeddedPkg/MmcDxe: Correct argument of ECSD read

[Jun Nie]

2017-06-29 20:09 GMT+08:00 Leif Lindholm <leif.lindh...@linaro.org>:
> On Thu, Jun 29, 2017 at 05:02:05PM +0800, Jun Nie wrote:
>> The argument of CMD8 should be stuff bits according to standard
>> JESD84-A44.
>
> OK, I realise that "stuff bits" is a term used by the spec, so that is
> probably sufficient explanation even though the term was known to me.
> And the MdeModulePkg driver seems to agree on the technical point.
> My question is why zeroes is the correct "stuff bits" value?

Yes, it is defined in page 2 in spec. I guess 0 is best filling value
than other value when we do not need a real value.
stuff bit: filling 0 bits to ensure fixed length frames for commands
and responses.

>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jun Nie <jun@linaro.org>
>> ---
>>  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c 
>> b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
>> index 4ce0ddd..c28207e 100644
>> --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
>> +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
>> @@ -210,7 +210,7 @@ EmmcIdentificationMode (
>>}
>>
>>// Fetch ECSD
>> -  Status = Host->SendCommand (Host, MMC_CMD8, RCA);
>> +  Status = Host->SendCommand (Host, MMC_CMD8, 0);
>>if (EFI_ERROR (Status)) {
>>  DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD fetch error, 
>> Status=%r.\n", Status));
>>}
>> --
>> 1.9.1
>>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 1/2] EmbeddedPkg/AndroidBoot: boot android kernel from storage

[Jun Nie]

2017-07-28 22:52 GMT+08:00 Leif Lindholm <leif.lindh...@linaro.org>:
> On Fri, Jul 28, 2017 at 10:18:49PM +0800, Jun Nie wrote:
>> On 2017年07月28日 21:06, Leif Lindholm wrote:
>> >On Fri, Jul 28, 2017 at 05:47:46PM +0800, Jun Nie wrote:
>> >>On 2017年07月27日 22:09, Leif Lindholm wrote:
>> >>>On Thu, Jul 27, 2017 at 06:07:19PM +0800, Jun Nie wrote:
>> >>>>Add an android kernel loader that could load kernel from storage
>> >>>>device. This patch is from Haojian's code as below link. The minor
>> >>>>change is that alternative dtb is searched in second loader binary
>> >>>>of Android bootimage if dtb is not found after Linux kernel.
>> >>>>https://patches.linaro.org/patch/94683/
>> >>>>
>> >>>>This android boot image BDS add addtitional cmdline/dtb/ramfs
>> >>>>support besides kernel that is introduced by Android boot header.
>> >>>
>> >>>Getting there. A few more comments below.
>> >>>
>> >>>>Contributed-under: TianoCore Contribution Agreement 1.0
>> >>>>Signed-off-by: Jun Nie <jun@linaro.org>
>> >>>>---
>> >>>>  ArmPkg/Include/Library/BdsLib.h|   3 +
>> >>>>  ArmPkg/Library/BdsLib/BdsFilePath.c|   3 -
>> >>>>  .../Application/AndroidBoot/AndroidBootApp.c   | 127 +++
>> >>>>  .../Application/AndroidBoot/AndroidBootApp.inf |  64 
>> >>>>  .../Application/AndroidFastboot/AndroidBootImg.c   |  35 +-
>> >>>>  .../AndroidFastboot/AndroidFastbootApp.h   |   1 +
>> >>>>  .../AndroidFastboot/Arm/BootAndroidBootImg.c   |   2 +-
>> >>>>  EmbeddedPkg/Include/Library/AndroidBootImgLib.h|  67 
>> >>>>  EmbeddedPkg/Include/Protocol/AndroidBootImg.h  |  47 +++
>> >>>>  .../Library/AndroidBootImgLib/AndroidBootImgLib.c  | 419 
>> >>>> +
>> >>>>  .../AndroidBootImgLib/AndroidBootImgLib.inf|  48 +++
>> >>>>  11 files changed, 782 insertions(+), 34 deletions(-)
>> >>>>  create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
>> >>>>  create mode 100644 
>> >>>> EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
>> >>>>  create mode 100644 EmbeddedPkg/Include/Library/AndroidBootImgLib.h
>> >>>>  create mode 100644 EmbeddedPkg/Include/Protocol/AndroidBootImg.h
>> >>>>  create mode 100644 
>> >>>> EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
>> >>>>  create mode 100644 
>> >>>> EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
>> >>>>
>> >
>
>> >>>>diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c 
>> >>>>b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
>> >>>>new file mode 100644
>> >>>>index 000..72c6322
>> >>>>--- /dev/null
>> >>>>+++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
>> >>>>@@ -0,0 +1,419 @@
>> >>>>+/** @file
>> >>>>+
>> >>>>+  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.
>> >>>>+  Copyright (c) 2017, Linaro. 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
>> >>>>+
>> >>>>+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> >>>>+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
>> >>>>IMPLIED.
>> >>>>+
>> >>>>+**/
>> >>>>+
>> >>>>+#include 
>> >>>>+#include 
>> >>>>+#include 
>> >>>>+#include 
>> >>>>+#include 
>> >>>>+
>> >>>>+#include 
>> >>>>+#include 
>> >>>>+
>> >>>>+#include 
>> >>>>+
>> >>>&g

[edk2] [PATCH v6 1/2] EmbeddedPkg/AndroidFastboot: split android boot header

[Jun Nie]

Split android boot header definition to share code among
different applications and libraries.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 .../Application/AndroidFastboot/AndroidBootImg.c   | 35 ++---
 .../AndroidFastboot/AndroidFastbootApp.h   |  1 +
 .../AndroidFastboot/Arm/BootAndroidBootImg.c   |  2 +-
 EmbeddedPkg/Include/Library/AndroidBootImgLib.h| 58 ++
 4 files changed, 65 insertions(+), 31 deletions(-)
 create mode 100644 EmbeddedPkg/Include/Library/AndroidBootImgLib.h

diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c 
b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
index f3e770b..2f7f093 100644
--- a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
+++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
@@ -14,32 +14,6 @@
 
 #include "AndroidFastbootApp.h"
 
-#define BOOT_MAGIC"ANDROID!"
-#define BOOT_MAGIC_LENGTH sizeof (BOOT_MAGIC) - 1
-
-// Check Val (unsigned) is a power of 2 (has only one bit set)
-#define IS_POWER_OF_2(Val) (Val != 0 && ((Val & (Val - 1)) == 0))
-
-// No documentation for this really - sizes of fields has been determined
-// empirically.
-#pragma pack(1)
-typedef struct {
-  CHAR8   BootMagic[BOOT_MAGIC_LENGTH];
-  UINT32  KernelSize;
-  UINT32  KernelAddress;
-  UINT32  RamdiskSize;
-  UINT32  RamdiskAddress;
-  UINT32  SecondStageBootloaderSize;
-  UINT32  SecondStageBootloaderAddress;
-  UINT32  KernelTaggsAddress;
-  UINT32  PageSize;
-  UINT32  Reserved[2];
-  CHAR8   ProductName[16];
-  CHAR8   KernelArgs[BOOTIMG_KERNEL_ARGS_SIZE];
-  UINT32  Id[32];
-} ANDROID_BOOTIMG_HEADER;
-#pragma pack()
-
 // Find the kernel and ramdisk in an Android boot.img.
 // return EFI_INVALID_PARAMTER if the boot.img is invalid (i.e. doesn't have 
the
 //  right magic value),
@@ -64,7 +38,8 @@ ParseAndroidBootImg (
 
   Header = (ANDROID_BOOTIMG_HEADER *) BootImg;
 
-  if (AsciiStrnCmp (Header->BootMagic, BOOT_MAGIC, BOOT_MAGIC_LENGTH) != 0) {
+  if (AsciiStrnCmp ((CONST CHAR8 *)Header->BootMagic, ANDROID_BOOT_MAGIC,
+ANDROID_BOOT_MAGIC_LENGTH) != 0) {
 return EFI_INVALID_PARAMETER;
   }
 
@@ -72,7 +47,7 @@ ParseAndroidBootImg (
 return EFI_NOT_FOUND;
   }
 
-  ASSERT (IS_POWER_OF_2 (Header->PageSize));
+  ASSERT (IS_VALID_ANDROID_PAGE_SIZE (Header->PageSize));
 
   *KernelSize = Header->KernelSize;
   *Kernel = BootImgBytePtr + Header->PageSize;
@@ -84,8 +59,8 @@ ParseAndroidBootImg (
  + ALIGN_VALUE (Header->KernelSize, Header->PageSize));
   }
 
-  AsciiStrnCpyS (KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE, Header->KernelArgs,
-BOOTIMG_KERNEL_ARGS_SIZE);
+  AsciiStrnCpyS (KernelArgs, ANDROID_BOOTIMG_KERNEL_ARGS_SIZE, 
Header->KernelArgs,
+ANDROID_BOOTIMG_KERNEL_ARGS_SIZE);
 
   return EFI_SUCCESS;
 }
diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h 
b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h
index f62660f..e4c5aa3 100644
--- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h
+++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h
@@ -15,6 +15,7 @@
 #ifndef __ANDROID_FASTBOOT_APP_H__
 #define __ANDROID_FASTBOOT_APP_H__
 
+#include 
 #include 
 #include 
 #include 
diff --git a/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c 
b/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c
index f446cce..1d9024b 100644
--- a/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c
+++ b/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c
@@ -112,7 +112,7 @@ BootAndroidBootImg (
   )
 {
   EFI_STATUS  Status;
-  CHAR8   KernelArgs[BOOTIMG_KERNEL_ARGS_SIZE];
+  CHAR8   
KernelArgs[ANDROID_BOOTIMG_KERNEL_ARGS_SIZE];
   VOID   *Kernel;
   UINTN   KernelSize;
   VOID   *Ramdisk;
diff --git a/EmbeddedPkg/Include/Library/AndroidBootImgLib.h 
b/EmbeddedPkg/Include/Library/AndroidBootImgLib.h
new file mode 100644
index 000..06da751
--- /dev/null
+++ b/EmbeddedPkg/Include/Library/AndroidBootImgLib.h
@@ -0,0 +1,58 @@
+/** @file
+
+  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.
+  Copyright (c) 2017, Linaro.
+
+  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
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __ABOOTIMG_H__
+#define __ABOOTIMG_H__
+
+#include 
+#include 
+#include 
+
+#i

[edk2] [PATCH v6 2/2] EmbeddedPkg/AndroidBoot: boot android kernel from storage

[Jun Nie]

Add an android kernel loader that could load kernel from storage
device.
This android boot image BDS add addtitional cmdline/dtb/ramfs
support besides kernel that is introduced by Android boot header.

This patch is derived from Haojian's code as below link.
https://patches.linaro.org/patch/94683/

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 .../Application/AndroidBoot/AndroidBootApp.c   | 140 ++
 .../Application/AndroidBoot/AndroidBootApp.inf |  64 +++
 EmbeddedPkg/EmbeddedPkg.dec|   2 +
 EmbeddedPkg/EmbeddedPkg.dsc|   2 +
 EmbeddedPkg/Include/Library/AndroidBootImgLib.h|  13 +
 EmbeddedPkg/Include/Protocol/AndroidBootImg.h  |  47 ++
 .../Library/AndroidBootImgLib/AndroidBootImgLib.c  | 471 +
 .../AndroidBootImgLib/AndroidBootImgLib.inf|  48 +++
 8 files changed, 787 insertions(+)
 create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
 create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
 create mode 100644 EmbeddedPkg/Include/Protocol/AndroidBootImg.h
 create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
 create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf

diff --git a/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c 
b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
new file mode 100644
index 000..977167d
--- /dev/null
+++ b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
@@ -0,0 +1,140 @@
+/** @file
+
+  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.
+  Copyright (c) 2017, Linaro. 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
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+/* Validate the node is media hard drive type */
+EFI_STATUS
+ValidateAndroidMediaDevicePath (
+  IN EFI_DEVICE_PATH  *DevicePath
+  )
+{
+  EFI_DEVICE_PATH_PROTOCOL*Node, *NextNode;
+
+  NextNode = DevicePath;
+  while (NextNode != NULL) {
+Node = NextNode;
+if (IS_DEVICE_PATH_NODE (Node, MEDIA_DEVICE_PATH, MEDIA_HARDDRIVE_DP)) {
+  return EFI_SUCCESS;
+}
+NextNode = NextDevicePathNode (Node);
+  }
+  return EFI_INVALID_PARAMETER;
+}
+
+EFI_STATUS
+EFIAPI
+AndroidBootAppEntryPoint (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+  CHAR16  *BootPathStr;
+  EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
+  EFI_DEVICE_PATH *DevicePath;
+  EFI_BLOCK_IO_PROTOCOL   *BlockIo;
+  UINT32  MediaId, BlockSize;
+  VOID*Buffer;
+  EFI_HANDLE  Handle;
+  UINTN   BootImgSize;
+
+  BootPathStr = (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);
+  ASSERT (BootPathStr != NULL);
+  Status = gBS->LocateProtocol (, NULL,
+(VOID **));
+  ASSERT_EFI_ERROR(Status);
+  DevicePath = (EFI_DEVICE_PATH 
*)EfiDevicePathFromTextProtocol->ConvertTextToDevicePath (BootPathStr);
+  ASSERT (DevicePath != NULL);
+
+  Status = ValidateAndroidMediaDevicePath (DevicePath);
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  Status = gBS->LocateDevicePath (,
+  , );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  Status = gBS->OpenProtocol (
+  Handle,
+  ,
+  (VOID **) ,
+  gImageHandle,
+  NULL,
+  EFI_OPEN_PROTOCOL_GET_PROTOCOL
+  );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Failed to get BlockIo: %r\n", Status));
+return Status;
+  }
+
+  MediaId = BlockIo->Media->MediaId;
+  BlockSize = BlockIo->Media->BlockSize;
+  Buffer = AllocatePages (EFI_SIZE_TO_PAGES (sizeof(ANDROID_BOOTIMG_HEADER)));
+  if (Buffer == NULL) {
+return EFI_BUFFER_TOO_SMALL;
+  }
+  /* Load header of boot.img */
+  Status = BlockIo->ReadBlocks (
+  BlockIo,
+  MediaId,
+  0,
+  BlockSize,
+  Buffer
+  );
+  Status = AndroidBootImgGetImgSize (Buffer, );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Failed to ge

[edk2] [PATCH v5 2/3] EmbeddedPkg/AndroidBoot: boot android kernel from storage

[Jun Nie]

Add an android kernel loader that could load kernel from storage
device.
This android boot image BDS add addtitional cmdline/dtb/ramfs
support besides kernel that is introduced by Android boot header.

This patch is derived from Haojian's code as below link.
https://patches.linaro.org/patch/94683/

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 .../Application/AndroidBoot/AndroidBootApp.c   | 140 ++
 .../Application/AndroidBoot/AndroidBootApp.inf |  64 +++
 EmbeddedPkg/Include/Library/AndroidBootImgLib.h|  13 +
 EmbeddedPkg/Include/Protocol/AndroidBootImg.h  |  47 ++
 .../Library/AndroidBootImgLib/AndroidBootImgLib.c  | 471 +
 .../AndroidBootImgLib/AndroidBootImgLib.inf|  48 +++
 6 files changed, 783 insertions(+)
 create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
 create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
 create mode 100644 EmbeddedPkg/Include/Protocol/AndroidBootImg.h
 create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
 create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf

diff --git a/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c 
b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
new file mode 100644
index 000..977167d
--- /dev/null
+++ b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
@@ -0,0 +1,140 @@
+/** @file
+
+  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.
+  Copyright (c) 2017, Linaro. 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
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+/* Validate the node is media hard drive type */
+EFI_STATUS
+ValidateAndroidMediaDevicePath (
+  IN EFI_DEVICE_PATH  *DevicePath
+  )
+{
+  EFI_DEVICE_PATH_PROTOCOL*Node, *NextNode;
+
+  NextNode = DevicePath;
+  while (NextNode != NULL) {
+Node = NextNode;
+if (IS_DEVICE_PATH_NODE (Node, MEDIA_DEVICE_PATH, MEDIA_HARDDRIVE_DP)) {
+  return EFI_SUCCESS;
+}
+NextNode = NextDevicePathNode (Node);
+  }
+  return EFI_INVALID_PARAMETER;
+}
+
+EFI_STATUS
+EFIAPI
+AndroidBootAppEntryPoint (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS  Status;
+  CHAR16  *BootPathStr;
+  EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
+  EFI_DEVICE_PATH *DevicePath;
+  EFI_BLOCK_IO_PROTOCOL   *BlockIo;
+  UINT32  MediaId, BlockSize;
+  VOID*Buffer;
+  EFI_HANDLE  Handle;
+  UINTN   BootImgSize;
+
+  BootPathStr = (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);
+  ASSERT (BootPathStr != NULL);
+  Status = gBS->LocateProtocol (, NULL,
+(VOID **));
+  ASSERT_EFI_ERROR(Status);
+  DevicePath = (EFI_DEVICE_PATH 
*)EfiDevicePathFromTextProtocol->ConvertTextToDevicePath (BootPathStr);
+  ASSERT (DevicePath != NULL);
+
+  Status = ValidateAndroidMediaDevicePath (DevicePath);
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  Status = gBS->LocateDevicePath (,
+  , );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  Status = gBS->OpenProtocol (
+  Handle,
+  ,
+  (VOID **) ,
+  gImageHandle,
+  NULL,
+  EFI_OPEN_PROTOCOL_GET_PROTOCOL
+  );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Failed to get BlockIo: %r\n", Status));
+return Status;
+  }
+
+  MediaId = BlockIo->Media->MediaId;
+  BlockSize = BlockIo->Media->BlockSize;
+  Buffer = AllocatePages (EFI_SIZE_TO_PAGES (sizeof(ANDROID_BOOTIMG_HEADER)));
+  if (Buffer == NULL) {
+return EFI_BUFFER_TOO_SMALL;
+  }
+  /* Load header of boot.img */
+  Status = BlockIo->ReadBlocks (
+  BlockIo,
+  MediaId,
+  0,
+  BlockSize,
+  Buffer
+  );
+  Status = AndroidBootImgGetImgSize (Buffer, );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Failed to get AndroidBootImg Size: %r\n", Status));
+return Status;
+  }
+  BootImgSize = ALIGN_VALUE (BootImgSi

Re: [edk2] [PATCH v4 3/4] EmbeddedPkg/AndroidBoot: boot android kernel from storage

[Jun Nie]

2017-08-01 23:50 GMT+08:00 Leif Lindholm <leif.lindh...@linaro.org>:
> On Tue, Aug 01, 2017 at 05:29:00PM +0800, Jun Nie wrote:
>> Add an android kernel loader that could load kernel from storage
>> device. This patch is derived from Haojian's code as below link.
>> https://patches.linaro.org/patch/94683/
>>
>> This android boot image BDS add addtitional cmdline/dtb/ramfs
>> support besides kernel that is introduced by Android boot header.
>
> This is nearly there - it's a big improvement, but a few more
> comments below.

Thank you for so much comments. All are addressed in v5.
Jun

>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jun Nie <jun@linaro.org>
>> ---
>>  .../Application/AndroidBoot/AndroidBootApp.c   | 140 +++
>>  .../Application/AndroidBoot/AndroidBootApp.inf |  64 +++
>>  EmbeddedPkg/Include/Library/AndroidBootImgLib.h|  17 +
>>  EmbeddedPkg/Include/Protocol/AndroidBootImg.h  |  47 +++
>>  .../Library/AndroidBootImgLib/AndroidBootImgLib.c  | 444 
>> +
>>  .../AndroidBootImgLib/AndroidBootImgLib.inf|  48 +++
>>  6 files changed, 760 insertions(+)
>>  create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
>>  create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
>>  create mode 100644 EmbeddedPkg/Include/Protocol/AndroidBootImg.h
>>  create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
>>  create mode 100644 
>> EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
>>
>> diff --git a/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c 
>> b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
>> new file mode 100644
>> index 000..5069819
>> --- /dev/null
>> +++ b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
>> @@ -0,0 +1,140 @@
>> +/** @file
>> +
>> +  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.
>> +  Copyright (c) 2017, Linaro. 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
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
>> IMPLIED.
>> +
>> +**/
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>> +
>> +/* Validate the node is media hard drive type */
>> +EFI_STATUS
>> +ValidateAndroidMediaDevicePath (
>> +  IN EFI_DEVICE_PATH  *DevicePath
>> +  )
>> +{
>> +  EFI_DEVICE_PATH_PROTOCOL*Node, *NextNode;
>> +
>> +  NextNode = DevicePath;
>> +  while (NextNode != NULL) {
>> +Node = NextNode;
>> +if (IS_DEVICE_PATH_NODE (Node, MEDIA_DEVICE_PATH, MEDIA_HARDDRIVE_DP)) {
>> +  return EFI_SUCCESS;
>> +}
>> +NextNode = NextDevicePathNode (Node);
>> +  }
>> +  return EFI_INVALID_PARAMETER;
>> +}
>> +
>> +EFI_STATUS
>> +EFIAPI
>> +AndroidBootAppEntryPoint (
>> +  IN EFI_HANDLEImageHandle,
>> +  IN EFI_SYSTEM_TABLE  *SystemTable
>> +  )
>> +{
>> +  EFI_STATUS  Status;
>> +  CHAR16  *BootPathStr;
>> +  EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
>> +  EFI_DEVICE_PATH *DevicePath;
>> +  EFI_BLOCK_IO_PROTOCOL   *BlockIo;
>> +  UINT32  MediaId, BlockSize;
>> +  VOID*Buffer;
>> +  EFI_HANDLE  Handle;
>> +  UINTN   BootImgSize;
>> +
>> +  BootPathStr = (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);
>> +  ASSERT (BootPathStr != NULL);
>> +  Status = gBS->LocateProtocol (, NULL,
>> +(VOID **));
>> +  ASSERT_EFI_ERROR(Status);
>> +  DevicePath = (EFI_DEVICE_PATH 
>> *)EfiDevicePathFromTextProtocol->ConvertTextToDevicePath (BootPathStr);
>> +  ASSERT (DevicePath != NULL);
>> +
>> +  Status = ValidateAndroidMediaDevicePath (DevicePath);
>> +  if (EFI_ERROR (Status)) {
>>

Re: [edk2] [PATCH v4 4/4] EmbeddedPkg: add Android boot device path and guid

[Jun Nie]

2017-08-02 0:19 GMT+08:00 Leif Lindholm <leif.lindh...@linaro.org>:
> Right, so this one confused me a bit, since it looked like it was
> breaking bisect again. It does, which caused me to take a closer
> look.
>
> First of all, you need to add
> EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf to
> EmbeddedPkg/EmbeddedPkg.dsc [Components.common].
>
> Once done, you should be able to build the application standalone with
>  build -a AARCH64 -t GCC5 -p EmbeddedPkg/EmbeddedPkg.dsc -m 
> EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
>
> For this to work, you also need to add a AndroidBootImgLib mapping to
> [LibraryClasses.common].
>
> Once that is done, the modifications here need to be squashed into
> 3/4.

This patch have to be the last one, later than AndroidBootImg patch.
Otherwise build failure happens due to lack of AndroidBootImgLib.

>
> /
> Leif
>
> On Tue, Aug 01, 2017 at 05:29:01PM +0800, Jun Nie wrote:
>> The device path specifies where to load android boot image.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jun Nie <jun@linaro.org>
>> ---
>>  EmbeddedPkg/EmbeddedPkg.dec | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
>> index 4cd528a..8ad2a84 100644
>> --- a/EmbeddedPkg/EmbeddedPkg.dec
>> +++ b/EmbeddedPkg/EmbeddedPkg.dec
>> @@ -80,6 +80,7 @@
>>gAndroidFastbootPlatformProtocolGuid =  { 0x524685a0, 0x89a0, 0x11e3, 
>> {0x9d, 0x4d, 0xbf, 0xa9, 0xf6, 0xa4, 0x03, 0x08}}
>>gUsbDeviceProtocolGuid =  { 0x021bd2ca, 0x51d2, 0x11e3, {0x8e, 0x56, 
>> 0xb7, 0x54, 0x17, 0xc7,  0x0b, 0x44 }}
>>gPlatformGpioProtocolGuid = { 0x52ce9845, 0x5af4, 0x43e2, {0xba, 0xfd, 
>> 0x23, 0x08, 0x12, 0x54, 0x7a, 0xc2 }}
>> +  gAndroidBootImgProtocolGuid = { 0x9859bb19, 0x407c, 0x4f8b, {0xbc, 0xe1, 
>> 0xf8, 0xda, 0x65, 0x65, 0xf4, 0xa5 }}
>>
>>  [PcdsFeatureFlag.common]
>>gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|FALSE|BOOLEAN|0x0001
>> @@ -181,6 +182,7 @@
>>
>> gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbProductId|0xbeef|UINT32|0x0023
>>gEmbeddedTokenSpaceGuid.PcdAndroidFastbootTcpPort|1234|UINT32|0x0024
>>
>> +  gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath|L""|VOID*|0x0057
>>
>>  [PcdsFixedAtBuild.ARM]
>>gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|32|UINT8|0x0010
>> --
>> 1.9.1
>>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v5 3/3] EmbeddedPkg: add Android boot build entry

[Jun Nie]

The device path specifies where to load android boot image.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/EmbeddedPkg.dec | 2 ++
 EmbeddedPkg/EmbeddedPkg.dsc | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 4cd528a..8ad2a84 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -80,6 +80,7 @@
   gAndroidFastbootPlatformProtocolGuid =  { 0x524685a0, 0x89a0, 0x11e3, {0x9d, 
0x4d, 0xbf, 0xa9, 0xf6, 0xa4, 0x03, 0x08}}
   gUsbDeviceProtocolGuid =  { 0x021bd2ca, 0x51d2, 0x11e3, {0x8e, 0x56, 0xb7, 
0x54, 0x17, 0xc7,  0x0b, 0x44 }}
   gPlatformGpioProtocolGuid = { 0x52ce9845, 0x5af4, 0x43e2, {0xba, 0xfd, 0x23, 
0x08, 0x12, 0x54, 0x7a, 0xc2 }}
+  gAndroidBootImgProtocolGuid = { 0x9859bb19, 0x407c, 0x4f8b, {0xbc, 0xe1, 
0xf8, 0xda, 0x65, 0x65, 0xf4, 0xa5 }}
 
 [PcdsFeatureFlag.common]
   gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|FALSE|BOOLEAN|0x0001
@@ -181,6 +182,7 @@
   
gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbProductId|0xbeef|UINT32|0x0023
   gEmbeddedTokenSpaceGuid.PcdAndroidFastbootTcpPort|1234|UINT32|0x0024
 
+  gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath|L""|VOID*|0x0057
 
 [PcdsFixedAtBuild.ARM]
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|32|UINT8|0x0010
diff --git a/EmbeddedPkg/EmbeddedPkg.dsc b/EmbeddedPkg/EmbeddedPkg.dsc
index 16b368e..4a34e34 100644
--- a/EmbeddedPkg/EmbeddedPkg.dsc
+++ b/EmbeddedPkg/EmbeddedPkg.dsc
@@ -52,6 +52,7 @@
   DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
 
 
+  AndroidBootImgLib|EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
   BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
   BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
   PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
@@ -272,6 +273,7 @@
   
TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
   }
 
+  EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
   EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.inf {
 
   # It depends on BdsLib that depends on TimerLib
-- 
1.9.1

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

[edk2] [PATCH v5 1/3] EmbeddedPkg/AndroidFastboot: split android boot header

[Jun Nie]

Split android boot header definition to share code among
different applications and libraries.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 .../Application/AndroidFastboot/AndroidBootImg.c   | 35 ++---
 .../AndroidFastboot/AndroidFastbootApp.h   |  1 +
 .../AndroidFastboot/Arm/BootAndroidBootImg.c   |  2 +-
 EmbeddedPkg/Include/Library/AndroidBootImgLib.h| 58 ++
 4 files changed, 65 insertions(+), 31 deletions(-)
 create mode 100644 EmbeddedPkg/Include/Library/AndroidBootImgLib.h

diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c 
b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
index f3e770b..2f7f093 100644
--- a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
+++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
@@ -14,32 +14,6 @@
 
 #include "AndroidFastbootApp.h"
 
-#define BOOT_MAGIC"ANDROID!"
-#define BOOT_MAGIC_LENGTH sizeof (BOOT_MAGIC) - 1
-
-// Check Val (unsigned) is a power of 2 (has only one bit set)
-#define IS_POWER_OF_2(Val) (Val != 0 && ((Val & (Val - 1)) == 0))
-
-// No documentation for this really - sizes of fields has been determined
-// empirically.
-#pragma pack(1)
-typedef struct {
-  CHAR8   BootMagic[BOOT_MAGIC_LENGTH];
-  UINT32  KernelSize;
-  UINT32  KernelAddress;
-  UINT32  RamdiskSize;
-  UINT32  RamdiskAddress;
-  UINT32  SecondStageBootloaderSize;
-  UINT32  SecondStageBootloaderAddress;
-  UINT32  KernelTaggsAddress;
-  UINT32  PageSize;
-  UINT32  Reserved[2];
-  CHAR8   ProductName[16];
-  CHAR8   KernelArgs[BOOTIMG_KERNEL_ARGS_SIZE];
-  UINT32  Id[32];
-} ANDROID_BOOTIMG_HEADER;
-#pragma pack()
-
 // Find the kernel and ramdisk in an Android boot.img.
 // return EFI_INVALID_PARAMTER if the boot.img is invalid (i.e. doesn't have 
the
 //  right magic value),
@@ -64,7 +38,8 @@ ParseAndroidBootImg (
 
   Header = (ANDROID_BOOTIMG_HEADER *) BootImg;
 
-  if (AsciiStrnCmp (Header->BootMagic, BOOT_MAGIC, BOOT_MAGIC_LENGTH) != 0) {
+  if (AsciiStrnCmp ((CONST CHAR8 *)Header->BootMagic, ANDROID_BOOT_MAGIC,
+ANDROID_BOOT_MAGIC_LENGTH) != 0) {
 return EFI_INVALID_PARAMETER;
   }
 
@@ -72,7 +47,7 @@ ParseAndroidBootImg (
 return EFI_NOT_FOUND;
   }
 
-  ASSERT (IS_POWER_OF_2 (Header->PageSize));
+  ASSERT (IS_VALID_ANDROID_PAGE_SIZE (Header->PageSize));
 
   *KernelSize = Header->KernelSize;
   *Kernel = BootImgBytePtr + Header->PageSize;
@@ -84,8 +59,8 @@ ParseAndroidBootImg (
  + ALIGN_VALUE (Header->KernelSize, Header->PageSize));
   }
 
-  AsciiStrnCpyS (KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE, Header->KernelArgs,
-BOOTIMG_KERNEL_ARGS_SIZE);
+  AsciiStrnCpyS (KernelArgs, ANDROID_BOOTIMG_KERNEL_ARGS_SIZE, 
Header->KernelArgs,
+ANDROID_BOOTIMG_KERNEL_ARGS_SIZE);
 
   return EFI_SUCCESS;
 }
diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h 
b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h
index f62660f..e4c5aa3 100644
--- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h
+++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h
@@ -15,6 +15,7 @@
 #ifndef __ANDROID_FASTBOOT_APP_H__
 #define __ANDROID_FASTBOOT_APP_H__
 
+#include 
 #include 
 #include 
 #include 
diff --git a/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c 
b/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c
index f446cce..1d9024b 100644
--- a/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c
+++ b/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c
@@ -112,7 +112,7 @@ BootAndroidBootImg (
   )
 {
   EFI_STATUS  Status;
-  CHAR8   KernelArgs[BOOTIMG_KERNEL_ARGS_SIZE];
+  CHAR8   
KernelArgs[ANDROID_BOOTIMG_KERNEL_ARGS_SIZE];
   VOID   *Kernel;
   UINTN   KernelSize;
   VOID   *Ramdisk;
diff --git a/EmbeddedPkg/Include/Library/AndroidBootImgLib.h 
b/EmbeddedPkg/Include/Library/AndroidBootImgLib.h
new file mode 100644
index 000..06da751
--- /dev/null
+++ b/EmbeddedPkg/Include/Library/AndroidBootImgLib.h
@@ -0,0 +1,58 @@
+/** @file
+
+  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.
+  Copyright (c) 2017, Linaro.
+
+  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
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __ABOOTIMG_H__
+#define __ABOOTIMG_H__
+
+#include 
+#include 
+#include 
+
+#i

Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: Use Trim instead of Erase for EraseBlocks

[Jun Nie]

On 2017年08月08日 10:04, Hao Wu wrote:

The current implementation of the Erase Block Protocol service
EraseBlocks() uses the erase command. According to spec eMMC Electrical
Standard 5.1, Section 6.6.9:

The erasable unit of the eMMC is the "Erase Group"; Erase group is
measured in write blocks that are the basic writable units of the Device.

However, code logic in function EmmcEraseBlocks() does not check whether
the starting logical block address to be erased and/or the size in bytes
to be erased is aligned with the erase group. Missing such checks will
erase the data on additional blocks on an eMMC device.

This commit will use the Trim command instead to perform the block erase
for eMMC devices. Unlike the Erase command, according to spec eMMC
Electrical Standard 5.1, Section 6.6.10:

The Trim operation is similar to the default erase operation described in
6.6.9. The Trim function applies the erase operation to write blocks
instead of erase groups.

Also, the Trim operation is mandatory for eMMC devices according to spec
eMMC Electrical Standard 5.1, Chapter 11, Table 220.

Cc: Star Zeng<star.z...@intel.com>
Cc: Ruiyu Ni<ruiyu...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu<hao.a...@intel.com>


Reviewed-by: Jun Nie <jun@linaro.org>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/4] Platforms: Add Sanchip Zx296718 basic library

[Jun Nie]

On 2017年08月10日 21:04, Leif Lindholm wrote:

You've reworked this to be targeted for edk2-platforms, which is
excellent! However:
- Please update subject line to match.
- Please also cc edk2-devel@lists.01.org (since you are with Linaro,
   it makes sense to still cc linaro-uefi)

To get the appropriate subject line for edk2-devel, as desribed by
https://github.com/tianocore/edk2-platforms/blob/about/Readme.md it
would also be helpful to add
--subject-prefix="edk2-platforms/master][PATCH" to git format-patch
command line (unless Laszlo can point out a better way of achieving
the same).

On Wed, Aug 09, 2017 at 10:12:36PM +0800, Jun Nie wrote:

Add Sanchip Zx296718 basic library files for Zx296718 SoC

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
  .../Library/Zx296718EvbLib/Zx296718Evb.c   | 132 +
  .../Library/Zx296718EvbLib/Zx296718EvbHelper.S |  50 
  .../Library/Zx296718EvbLib/Zx296718EvbLib.inf  |  53 +
  .../Library/Zx296718EvbLib/Zx296718EvbMem.c| 107 +
  Silicon/Sanchip/Zx296718/Include/Zx296718.h|  30 +
  Silicon/Sanchip/Zx296718/Zx296718.dec  |  32 +


It is more clear (especially when submitting new ports) to generate
the patches with --stat=1000, to make the full file paths visible.

See Laszlo's guide for helpful steps when submitting patches:
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers


  6 files changed, 404 insertions(+)
  create mode 100644 
Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718Evb.c
  create mode 100644 
Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbHelper.S
  create mode 100644 
Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbLib.inf
  create mode 100644 
Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718EvbMem.c
  create mode 100644 Silicon/Sanchip/Zx296718/Include/Zx296718.h
  create mode 100644 Silicon/Sanchip/Zx296718/Zx296718.dec


Since this ends up with a .dec, please rename
Silicon/Sanchip/Zx296718/ -> Silicon/Sanchip/Zx296718Pkg/.

That said, I don't see this .dec file providing any information that
is subsequently used enywhere. Is this in preparation for further
patches coming after this initial platform support?


No plan yet. So this file will be deleted.



diff --git a/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718Evb.c 
b/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718Evb.c
new file mode 100644
index 000..4e4eb54
--- /dev/null
+++ b/Platform/Sanchip/Zx296718Evb/Library/Zx296718EvbLib/Zx296718Evb.c
@@ -0,0 +1,132 @@
+/** @file
+*
+*  Copyright (C) 2017 Sanechips Technology Co., Ltd.


Just a clarification: Sanchip/Sanechips?
"The Internet" suggests Sanechips is the appropriate company name, so
should the directory name change >

+*  Copyright (c) 2017, Linaro Ltd.
+*
+*  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
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+*
+**/
+
+#include 
+#include 
+#include 
+#include 


Please sort include files alphabetically.


+
+#include 
+
+#include 
+
+ARM_CORE_INFO mZx296718EvbInfoTable[] = {
+  {
+// Cluster 0, Core 0
+0x0, 0x0,
+
+// MP Core MailBox Set/Get/Clear Addresses and Clear Value
+(UINT64)0x
+  },
+  {
+// Cluster 0, Core 1
+0x0, 0x1,
+
+// MP Core MailBox Set/Get/Clear Addresses and Clear Value
+(UINT64)0x
+  },
+  {
+// Cluster 0, Core 2
+0x0, 0x2,
+
+// MP Core MailBox Set/Get/Clear Addresses and Clear Value
+(UINT64)0x
+  },
+  {
+// Cluster 0, Core 3
+0x0, 0x3,
+
+// MP Core MailBox Set/Get/Clear Addresses and Clear Value
+(UINT64)0x
+  },
+};
+
+/**
+  Return the current Boot Mode
+
+  This function returns the boot reason on the platform
+
+  @return   Return the current Boot Mode of the platform
+
+**/
+EFI_BOOT_MODE
+ArmPlatformGetBootMode (
+  VOID
+  )
+{
+  return BOOT_WITH_FULL_CONFIGURATION;
+}
+
+/**
+  Initialize controllers that must setup in the normal world
+
+  This function is called by the ArmPlatformPkg/Pei or 
ArmPlatformPkg/Pei/PlatformPeim
+  in the PEI phase.
+
+**/
+RETURN_STATUS
+ArmPlatformInitialize (
+  IN  UINTN MpId
+  )
+{
+  return RETURN_SUCCESS;
+}
+
+/**
+  Initialize the system (or sometimes called permanent) memory
+
+  This memory is generally represented by the DRAM.
+
+**/
+VOID
+ArmPlatformInitializeSystemMemory (
+  VOID
+  )
+{
+}
+
+EFI_STAT

Re: [edk2] [PATCH 4/4] Platforms/zx: Add platform build system files

[Jun Nie]

On 2017年08月10日 23:00, Leif Lindholm wrote:

On Wed, Aug 09, 2017 at 10:12:39PM +0800, Jun Nie wrote:

Add platform build system files, including *.dsc *.fdf *.dec

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
  Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec |  33 ++
  Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc | 467 +++
  Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf | 331 +++
  3 files changed, 831 insertions(+)
  create mode 100644 Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
  create mode 100644 Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc
  create mode 100644 Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf

diff --git a/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec 
b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
new file mode 100644
index 000..078cfdf
--- /dev/null
+++ b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
@@ -0,0 +1,33 @@
+#
+#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
+#  Copyright (c) 2017, Linaro Ltd.
+#
+#  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
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+
+[Defines]
+  DEC_SPECIFICATION  = 0x00010005


0x00010019/1.25.


+  PACKAGE_NAME   = Zx296718Evb
+  PACKAGE_GUID   = d6db414d-ea67-4312-94d5-9c9e5b224d25
+  PACKAGE_VERSION= 0.1
+
+
+#
+# Include Section - list of Include Paths that are provided by this package.
+#   Comments are used for Keywords and Module Types.
+#
+# Supported Module Types:
+#  BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER 
DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION
+#
+
+[Includes.common]
+  Include# Root include for the package


This directory does not exist, breaking compilation. >

+
+[Guids.common]
+  gZx296718EvbTokenSpaceGuid  =  { 0x91148425, 0xcdd2, 0x4830, { 0x8b, 
0xd0, 0xc6, 0x1c, 0x6d, 0xea, 0x36, 0x21 } }


This does not appear to actually be used anywhere.


I already remove this whole file in next version as Ard suggested.



diff --git a/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc 
b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc
new file mode 100644
index 000..d104e7c
--- /dev/null
+++ b/Platform/Sanchip/Zx296718Evb/Zx296718Evb.dsc
@@ -0,0 +1,467 @@
+#
+#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
+#  Copyright (c) 2017, Linaro Ltd.
+#
+#  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
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+
+
+#
+# Defines Section - statements that will be processed to create a Makefile.
+#
+
+[Defines]
+  PLATFORM_NAME  = Zx296718Evb
+  PLATFORM_GUID  = 8edf1480-da5c-4857-bc02-7530bd8e7b7a
+  PLATFORM_VERSION   = 0.2
+  DSC_SPECIFICATION  = 0x00010005
+  OUTPUT_DIRECTORY   = Build/Zx296718Evb
+  SUPPORTED_ARCHITECTURES= AARCH64
+  BUILD_TARGETS  = DEBUG|RELEASE
+  SKUID_IDENTIFIER   = DEFAULT
+  FLASH_DEFINITION   = Platform/Sanchip/Zx296718Evb/Zx296718Evb.fdf
+
+[LibraryClasses.common]
+!if $(TARGET) == RELEASE
+  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
+!else
+  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
+!endif
+  
DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
+
+  ArmDisassemblerLib|ArmPkg/Library/ArmDisassemblerLib/ArmDisassemblerLib.inf
+  ArmGicLib|ArmPkg/Drivers/ArmGic/ArmGicLib.inf
+  ArmGicArchLib|ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
+  ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
+  ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
+  
ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
+
+  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
+  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+  
ArmPlatformLib|Platfor

Re: [edk2] [PATCH 2/4] Platforms: Add ZX RTC driver for Sanchip SoC

[Jun Nie]

On 2017年08月10日 22:03, Leif Lindholm wrote:

On Wed, Aug 09, 2017 at 10:12:37PM +0800, Jun Nie wrote:

Runtime service is not supported yet.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
  .../Zx6718RealTimeClockLib/Zx296718RealTimeClock.c | 376 +
  .../Zx6718RealTimeClockLib/Zx296718RealTimeClock.h | 102 ++
  .../Zx296718RealTimeClock.inf  |  42 +++
  3 files changed, 520 insertions(+)
  create mode 100644 
Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c
  create mode 100644 
Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.h
  create mode 100644 
Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.inf

diff --git 
a/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c 
b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c
new file mode 100644
index 000..af6e5bd
--- /dev/null
+++ b/Silicon/Sanchip/Library/Zx6718RealTimeClockLib/Zx296718RealTimeClock.c
@@ -0,0 +1,376 @@
+/** @file
+  Implement EFI RealTimeClock runtime services via RTC Lib.
+
+  Currently this driver does not support runtime virtual calling.
+
+  Copyright (C) 2017 Sanechips Technology Co., Ltd.
+  Copyright (c) 2017, Linaro Limited.
+
+  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
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+  Based on the files under 
ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
+
+**/
+
+#include 
+#include 


P before U.


+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+// Use EfiAtRuntime to check stage
+#include 


L (UefiRuntimeLib) before S (UefiRuntimeServices...).
No need for a comment explaining why we include headers.


+#include 
+#include "Zx296718RealTimeClock.h"
+
+STATIC UINTN   RtcBase;


mRtcBase.


+STATIC BOOLEAN RTCInitialized = FALSE;


mRTCInitialized.


+
+BOOLEAN
+EFIAPI
+IsTimeValid(
+  IN EFI_TIME *Time
+  )
+{
+  // Check the input parameters are within the range specified by UEFI
+  if ((Time->Year  < 2000) ||
+ (Time->Year   > 2099) ||
+ (Time->Month  < 1   ) ||
+ (Time->Month  > 12  ) ||
+ (Time->Day< 1   ) ||
+ (Time->Day> 31  ) ||
+ (Time->Hour   > 23  ) ||
+ (Time->Minute > 59  ) ||
+ (Time->Second > 59  ) ||
+ (Time->Nanosecond > 9) ||
+ (!((Time->TimeZone == EFI_UNSPECIFIED_TIMEZONE) || ((Time->TimeZone >= -1440) 
&& (Time->TimeZone <= 1440 ||
+ (Time->Daylight & (~(EFI_TIME_ADJUST_DAYLIGHT | EFI_TIME_IN_DAYLIGHT)))
+  ) {
+return FALSE;
+  }
+
+  return TRUE;
+}


This appears a direct copy of the version in
EmbeddedPkg/Library/TimeBaseLib. Can you add that TimeBaseLib library
dependency to decrease duplication?
(This may not have existed as a standalone library at the time you
started this port.)


+
+VOID


A lot of the functions in this file could do with a STATIC prefix,
since they are only used internally.


+Wait4Busy (


Semantically, this name is incorrect.
The function is waiting _while_ the state is RTC_BUSY, so seems to me
it should be called WaitBusy.


+  VOID
+  )
+{
+  UINT32 Val, Retry = 1000;
+  do {
+MicroSecondDelay (200);


Why 200?


It is just a suggested delay time according to RTC clock frequency. You 
want a RTC_WAIT_DELAY macro here for better understanding, right?



+Val = MmioRead32 (RtcBase + RTCSTS);


MmioRead32 does not imply any barrier semantics.
If this component will only ever be supported on ARM platforms, you
could insert an ArmDataMemoryBarrier (). Otherwise, MemoryFence ().


+  } while(Val & RTC_BUSY && Retry--);


Space before (.


+
+  if (!Retry)
+DEBUG((DEBUG_ERROR, "%a Rtc busy retry timeout\n", __func__));


Space after DEBUG.


+}
+
+VOID
+RTCWriteReg (
+  IN UINT32 Reg,
+  IN UINT32 Val
+  )
+{
+  Wait4Busy ();
+  MmioWrite32 (RtcBase + RTCCFGID, CONFIG_PARMETER);
+  Wait4Busy ();
+  MmioWrite32 (RtcBase + Reg, Val);
+}
+
+UINT32
+RTCReadReg (
+  IN UINT32 Reg
+  )
+{
+  Wait4Busy ();
+  return MmioRead32 (RtcBase + Reg);
+}
+
+VOID
+InitializeRTC (
+  VOID
+  )
+{
+  UINTN Val = (UINTN)FixedPcdGet64 (PcdZxRtcClockFreq);
+
+  RTCWriteReg (RTCCLKCNT, Val - 1);
+  Val = RTCReadReg (RTCPOWERINIT1);
+  if (RTC_POWER_INI1_PARA != Val) {
+RTCWriteReg (RTCCTL, 0);
+MicroSecondDelay (INIT_DELAY);
+Val = RTCReadReg (RTCCTL);
+Val |= RTC_CTRL_BIT6_1;
+RTCWriteReg (RTCCTL, Val);
+

Re: [edk2] [PATCH 3/4] Platforms/zx: Add boot manager lib and entries

[Jun Nie]

On 2017年08月10日 22:41, Leif Lindholm wrote:

On Wed, Aug 09, 2017 at 10:12:38PM +0800, Jun Nie wrote:

Add boot manager lib and entries, including Android and Grub.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
  .../Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c| 105 ++
  .../Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf  |  66 
  .../Library/PlatformBootManagerLib/PlatformBm.c| 404 +
  .../Library/PlatformBootManagerLib/PlatformBm.h|  30 ++
  .../PlatformBootManagerLib.inf |  91 +
  Silicon/Sanchip/SanchipPkg.dec |  29 ++
  6 files changed, 725 insertions(+)
  create mode 100644 
Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
  create mode 100644 
Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
  create mode 100644 Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.c
  create mode 100644 Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.h
  create mode 100644 
Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
  create mode 100644 Silicon/Sanchip/SanchipPkg.dec

diff --git 
a/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c 
b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
new file mode 100644
index 000..47d02bf
--- /dev/null
+++ b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
@@ -0,0 +1,105 @@
+/** @file
+*
+*  Copyright (C) 2017 Sanechips Technology Co., Ltd.
+*  Copyright (c) 2017, Linaro Ltd.
+*
+*  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
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+*
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 


Which aspect of BlockIo is used here?
On the whole, several of the above includes look unused - could you
prune them back a bit?


Sure.



+
+#include 
+
+EFI_STATUS
+EFIAPI
+AndroidBootImgAppendKernelArgs (
+  IN CHAR16*Args,
+  IN UINTN  Size
+  )
+{
+  UnicodeSPrint (
+Args + StrLen (Args), Size - StrLen (Args), L" efi=noruntime");


I am not sure what your intent is here, but I am entirely convinced
there is a better way to achieve it. Can you give some background,
please?


RTC driver is lack of runtime service, so register access result kernel 
panic. After RTC driver support runtime service, I already delete this 
function  in later version.



+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+AndroidBootImgUpdateDtb (
+  IN  EFI_PHYSICAL_ADDRESSOrigFdtBase,
+  OUT EFI_PHYSICAL_ADDRESS   *NewFdtBase
+  )
+{
+  UINTN FdtSize, NumPages;
+  INTN  err;
+  EFI_STATUSStatus;
+
+  //
+  // Store the FDT as Runtime Service Data to prevent the Kernel from
+  // overwritting its data.
+  //


This should really not be necessary and has never been needed
elsewhere. Have you seen this cause any change in behaviour in an
actual system?


This is copied from Hisilicon code and I had thought it is for safety. I 
already delete this function in later version.



+  FdtSize = fdt_totalsize ((VOID *)(UINTN)OrigFdtBase);
+  NumPages = EFI_SIZE_TO_PAGES (FdtSize) + 20;
+  Status = gBS->AllocatePages (
+  AllocateAnyPages, EfiRuntimeServicesData,
+  NumPages, NewFdtBase);
+  if (EFI_ERROR (Status)) {
+return EFI_BUFFER_TOO_SMALL;
+  }
+
+  CopyMem (
+(VOID*)(UINTN)*NewFdtBase,
+(VOID*)(UINTN)OrigFdtBase,
+FdtSize
+);
+
+  fdt_pack ((VOID*)(UINTN)*NewFdtBase);
+  err = fdt_check_header ((VOID*)(UINTN)*NewFdtBase);


This does not feel like it belongs here. Check it when you first load
it, by all means, but there is no need to keep checking it did not get
destroyed by normal execution.


Yes, it can be removed after check logic is added in library.



+  if (err != 0) {
+DEBUG ((DEBUG_ERROR, "ERROR: Device Tree header not valid (err:%d)\n", 
err));
+gBS->FreePages (*NewFdtBase, NumPages);
+return EFI_INVALID_PARAMETER;
+  }
+  return EFI_SUCCESS;
+}
+
+ANDROID_BOOTIMG_PROTOCOL mAndroidBootImg = {
+  AndroidBootImgAppendKernelArgs,
+  AndroidBootImgUpdateDtb
+};
+
+EFI_STATUS
+EFIAPI
+Zx296718EvbEntryPoint (
+  IN EFI_HANDLE ImageHandle,
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_STATUS   Status;
+
+  Status = gBS->InstallProtocolInterface (
+  ,
+  ,
+  EFI_NATIVE_INTERFACE,
+  
+  )

[edk2] [PATCH] EmbeddedPkg/MmcDxe: Add alignment for ECSD data

[Jun Nie]

Add alignment for ECSD data for DMA access. Otherwise
the data is corrupted on Sanechips platform.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/Universal/MmcDxe/Mmc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h 
b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
index 8a7d5a3..ca1a9d5 100644
--- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
+++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
@@ -319,7 +319,7 @@ typedef struct  {
   OCR   OCRData;
   CID   CIDData;
   CSD   CSDData;
-  ECSD  ECSDData; // MMC V4 extended card specific
+  ECSD  ECSDData __attribute__((aligned(8)));  // MMC V4 extended card 
specific
 } CARD_INFO;
 
 typedef struct _MMC_HOST_INSTANCE {
-- 
1.9.1

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

[edk2] [PATCH] EmbeddedPkg/MmcDxe: Add non-DDR timing mode support

[Jun Nie]

Only DDR mode is support for 8bit mode currently. Add
non-DDR case when configuring ECSD.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c 
b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
index 574a77e..5c0d7e7 100644
--- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
+++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
@@ -286,7 +286,10 @@ InitializeEmmcDevice (
 }
 Status = Host->SetIos (Host, BusClockFreq, 8, TimingMode[Idx]);
 if (!EFI_ERROR (Status)) {
-  Status = EmmcSetEXTCSD (MmcHostInstance, EXTCSD_BUS_WIDTH, 
EMMC_BUS_WIDTH_DDR_8BIT);
+  if (Idx < 2)
+Status = EmmcSetEXTCSD (MmcHostInstance, EXTCSD_BUS_WIDTH, 
EMMC_BUS_WIDTH_DDR_8BIT);
+  else
+Status = EmmcSetEXTCSD (MmcHostInstance, EXTCSD_BUS_WIDTH, 
EMMC_BUS_WIDTH_8BIT);
   if (EFI_ERROR (Status)) {
 DEBUG ((DEBUG_ERROR, "InitializeEmmcDevice(): Failed to set EXTCSD bus 
width, Status:%r\n", Status));
   }
-- 
1.9.1

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

Re: [edk2] [PATCH] EmbeddedPkg/MmcDxe: Add alignment for ECSD data

[Jun Nie]

2017-06-08 22:55 GMT+08:00 Andrew Fish <af...@apple.com>:
>
> On Jun 8, 2017, at 1:18 AM, Jun Nie <jun@linaro.org> wrote:
>
> Add alignment for ECSD data for DMA access. Otherwise
> the data is corrupted on Sanechips platform.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie <jun@linaro.org>
> ---
> EmbeddedPkg/Universal/MmcDxe/Mmc.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> index 8a7d5a3..ca1a9d5 100644
> --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> @@ -319,7 +319,7 @@ typedef struct  {
>   OCR   OCRData;
>   CID   CIDData;
>   CSD   CSDData;
> -  ECSD  ECSDData; // MMC V4 extended card
> specific
> +  ECSD  ECSDData __attribute__((aligned(8)));  // MMC V4 extended card
> specific
> } CARD_INFO;
>
>
> Jun,
>
> This structure does not look portable.
> 1) CARD_TYPE is an ENUM and the size of an enum is not a standard thing in
> C. Compiler is probably picking int and it seems to work.
> 2) I don't think  __attribute__((aligned(8))) is supported by all the edk2
> compilers (VC++ for example). It is an GNU extensions to C, not standard C,
> so we normally don't use it in edk2 code.
>
> While the alignment of types is not defined by the C standard, it is defined
> in the EFI ABI to be natural alignment. So you can add padding elements to a
> structure. You can also union with a UINT64 for force 8 byte alignment.
>
> typedef struct  {
>   UINT16RCA;
>   UINT32 CardType;   //CARD_TYPE
>   OCR   OCRData;
>   CID   CIDData;
>   CSD   CSDData;
>   UINT32Pad;
>   ECSD  ECSDData; // MMC V4 extended card
> specific
> } CARD_INFO;
>
> Thanks,
>
> Andrew Fish

Thanks for showing this robust method. Will change to this way.

Jun

>
> typedef struct _MMC_HOST_INSTANCE {
> --
> 1.9.1
>
> ___
> 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] EmbeddedPkg/MmcDxe: Add non-DDR timing mode support

[Jun Nie]

2017-06-09 0:55 GMT+08:00 Leif Lindholm <leif.lindh...@linaro.org>:
> On Thu, Jun 08, 2017 at 04:39:44PM +0800, Haojian Zhuang wrote:
>> On 2017/6/8 16:12, Jun Nie wrote:
>> >Only DDR mode is support for 8bit mode currently. Add
>> >non-DDR case when configuring ECSD.
>> >
>> >Contributed-under: TianoCore Contribution Agreement 1.0
>> >Signed-off-by: Jun Nie <jun@linaro.org>
>> >---
>> >  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 5 -
>> >  1 file changed, 4 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c 
>> >b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
>> >index 574a77e..5c0d7e7 100644
>> >--- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
>> >+++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
>> >@@ -286,7 +286,10 @@ InitializeEmmcDevice (
>> >  }
>> >  Status = Host->SetIos (Host, BusClockFreq, 8, TimingMode[Idx]);
>> >  if (!EFI_ERROR (Status)) {
>> >-  Status = EmmcSetEXTCSD (MmcHostInstance, EXTCSD_BUS_WIDTH, 
>> >EMMC_BUS_WIDTH_DDR_8BIT);
>> >+  if (Idx < 2)
>>
>> It's better to avoid hardcoded value at here. Maybe you can use switch &
>> case on TimingMode array at here.
>
> Yes, that would also work.
>
> Indeed, if any other possible values than EMMC_BUS_WIDTH_DDR_8BIT or
> EMMC_BUS_WIDTH_8BIT could be likely in the future, that would be the
> preferable solution.
>
> Regards,
>
> Leif


Yes, follow switch/case way is more extendable. Will add it and add
maintainer in next version.

Jun

>
>> >+Status = EmmcSetEXTCSD (MmcHostInstance, EXTCSD_BUS_WIDTH, 
>> >EMMC_BUS_WIDTH_DDR_8BIT);
>> >+  else
>> >+Status = EmmcSetEXTCSD (MmcHostInstance, EXTCSD_BUS_WIDTH, 
>> >EMMC_BUS_WIDTH_8BIT);
>> >if (EFI_ERROR (Status)) {
>> >  DEBUG ((DEBUG_ERROR, "InitializeEmmcDevice(): Failed to set 
>> > EXTCSD bus width, Status:%r\n", Status));
>> >}
>> >
>>
>> Best Regards
>> Haojian
>> ___
>> 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 v2] EmbeddedPkg/MmcDxe: Add alignment for ECSD data

[Jun Nie]

2017-06-12 23:53 GMT+08:00 Leif Lindholm <leif.lindh...@linaro.org>:
> On Mon, Jun 12, 2017 at 09:59:28AM +0800, Jun Nie wrote:
>> Add alignment for ECSD data for DMA access. Otherwise
>> the data is corrupted on Sanechips platform.
>
> I never did see a reply to my proposed solution, and the below is not
> it. Can you explain why you prefer this one?
>
> /
> Leif

Sorry, just see your email because that thread is not highlighted for
new email in gmail for unknown reason.
I have concern that "UINT64 VENDOR_SPECIFIC_FIELD[8]" cannot secure
the ECSD alignment because it is not the first member. Changing the
first member to "UINT64 RESERVED_1[2]" shall secure the alignment. But
I preferred Pad method. It is more readable if all ECSD member are
UINT8 type. It is also more clear to add alignment info in CARD_INFO,
just before ECSD member.
I do not get point of Andrew, maybe he share the same concern.

Jun

>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jun Nie <jun@linaro.org>
>> ---
>>  EmbeddedPkg/Universal/MmcDxe/Mmc.h | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h 
>> b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
>> index 8a7d5a3..6e3ab17 100644
>> --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
>> +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
>> @@ -319,6 +319,7 @@ typedef struct  {
>>OCR   OCRData;
>>CID   CIDData;
>>CSD   CSDData;
>> +  UINT64Pad;  // For 8 bytes alignment of 
>> ECSDData
>>ECSD  ECSDData; // MMC V4 extended card 
>> specific
>>  } CARD_INFO;
>>
>> --
>> 1.9.1
>>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] EmbeddedPkg/MmcDxe: Add alignment for ECSD data

[Jun Nie]

2017-06-13 12:01 GMT+08:00 Andrew Fish <af...@apple.com>:
>
>> On Jun 12, 2017, at 7:14 PM, Jun Nie <jun@linaro.org> wrote:
>>
>> 2017-06-12 23:53 GMT+08:00 Leif Lindholm <leif.lindh...@linaro.org>:
>>> On Mon, Jun 12, 2017 at 09:59:28AM +0800, Jun Nie wrote:
>>>> Add alignment for ECSD data for DMA access. Otherwise
>>>> the data is corrupted on Sanechips platform.
>>>
>>> I never did see a reply to my proposed solution, and the below is not
>>> it. Can you explain why you prefer this one?
>>>
>>> /
>>>Leif
>>
>> Sorry, just see your email because that thread is not highlighted for
>> new email in gmail for unknown reason.
>> I have concern that "UINT64 VENDOR_SPECIFIC_FIELD[8]" cannot secure
>> the ECSD alignment because it is not the first member. Changing the
>> first member to "UINT64 RESERVED_1[2]" shall secure the alignment. But
>> I preferred Pad method. It is more readable if all ECSD member are
>> UINT8 type. It is also more clear to add alignment info in CARD_INFO,
>> just before ECSD member.
>> I do not get point of Andrew, maybe he share the same concern.
>>
>
> Jun
>
> typedef enum {
>   UNKNOWN_CARD,
>   MMC_CARD,  //MMC card
>   MMC_CARD_HIGH, //MMC Card with High capacity
>   EMMC_CARD, //eMMC 4.41 card
>   SD_CARD,   //SD 1.1 card
>   SD_CARD_2, //SD 2.0 or above standard card
>   SD_CARD_2_HIGH //SD 2.0 or above high capacity card
> } CARD_TYPE;
>
> Per C spec sizeof(CARD_TYPE) can be 1, 2, 4, or 8 (64-bit integer), and it is 
> legal for the compiler to pick any of these. So it is not portable C code to 
> use an enum in a data structure when layout maters.
>
> Thanks,
>
> Andrew Fish

CARD_TYPE CardType is 2nd member of CARD_INFO, while ECSDData is the
last member and I just want to align it to 8 bytes. I had assume pad
will be added automatically by compiler if CARD_TYPE is not 8 bytes
aligned and UNIT64 type appear in following member. Does enum will
impact the later member alignment? Could you help elaborate more about
this?

Thank you!
Jun

>>
>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Jun Nie <jun@linaro.org>
>>>> ---
>>>> EmbeddedPkg/Universal/MmcDxe/Mmc.h | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h 
>>>> b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
>>>> index 8a7d5a3..6e3ab17 100644
>>>> --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
>>>> +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
>>>> @@ -319,6 +319,7 @@ typedef struct  {
>>>>   OCR   OCRData;
>>>>   CID   CIDData;
>>>>   CSD   CSDData;
>>>> +  UINT64Pad;  // For 8 bytes alignment of 
>>>> ECSDData
>>>>   ECSD  ECSDData; // MMC V4 extended card 
>>>> specific
>>>> } CARD_INFO;
>>>>
>>>> --
>>>> 1.9.1
>>>>
>> ___
>> 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] [PATCH v2] EmbeddedPkg/MmcDxe: Add alignment for ECSD data

[Jun Nie]

Add alignment for ECSD data for DMA access. Otherwise
the data is corrupted on Sanechips platform.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/Universal/MmcDxe/Mmc.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h 
b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
index 8a7d5a3..6e3ab17 100644
--- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
+++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
@@ -319,6 +319,7 @@ typedef struct  {
   OCR   OCRData;
   CID   CIDData;
   CSD   CSDData;
+  UINT64Pad;  // For 8 bytes alignment of 
ECSDData
   ECSD  ECSDData; // MMC V4 extended card specific
 } CARD_INFO;
 
-- 
1.9.1

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

[edk2] [PATCH v2] EmbeddedPkg/MmcDxe: Add non-DDR timing mode support

[Jun Nie]

Only DDR mode is support for 8bit mode currently. Add
non-DDR case when configuring ECSD.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jun Nie <jun@linaro.org>
---
 EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c 
b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
index 574a77e..4ce0ddd 100644
--- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
+++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
@@ -254,7 +254,7 @@ InitializeEmmcDevice (
   EFI_MMC_HOST_PROTOCOL *Host;
   EFI_STATUS Status = EFI_SUCCESS;
   ECSD   *ECSDData;
-  UINT32 BusClockFreq, Idx;
+  UINT32 BusClockFreq, Idx, BusMode;
   UINT32 TimingMode[4] = {EMMCHS52DDR1V2, EMMCHS52DDR1V8, EMMCHS52, 
EMMCHS26};
 
   Host  = MmcHostInstance->MmcHost;
@@ -286,7 +286,19 @@ InitializeEmmcDevice (
 }
 Status = Host->SetIos (Host, BusClockFreq, 8, TimingMode[Idx]);
 if (!EFI_ERROR (Status)) {
-  Status = EmmcSetEXTCSD (MmcHostInstance, EXTCSD_BUS_WIDTH, 
EMMC_BUS_WIDTH_DDR_8BIT);
+  switch (TimingMode[Idx]) {
+  case EMMCHS52DDR1V2:
+  case EMMCHS52DDR1V8:
+BusMode = EMMC_BUS_WIDTH_DDR_8BIT;
+break;
+  case EMMCHS52:
+  case EMMCHS26:
+BusMode = EMMC_BUS_WIDTH_8BIT;
+break;
+  default:
+return EFI_UNSUPPORTED;
+  }
+  Status = EmmcSetEXTCSD (MmcHostInstance, EXTCSD_BUS_WIDTH, BusMode);
   if (EFI_ERROR (Status)) {
 DEBUG ((DEBUG_ERROR, "InitializeEmmcDevice(): Failed to set EXTCSD bus 
width, Status:%r\n", Status));
   }
-- 
1.9.1

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

Re: [edk2] [PATCH v2] EmbeddedPkg/MmcDxe: Add alignment for ECSD data

[Jun Nie]

2017-06-13 17:18 GMT+08:00 Leif Lindholm <leif.lindh...@linaro.org>:
> On Tue, Jun 13, 2017 at 10:14:34AM +0800, Jun Nie wrote:
>> 2017-06-12 23:53 GMT+08:00 Leif Lindholm <leif.lindh...@linaro.org>:
>> > On Mon, Jun 12, 2017 at 09:59:28AM +0800, Jun Nie wrote:
>> >> Add alignment for ECSD data for DMA access. Otherwise
>> >> the data is corrupted on Sanechips platform.
>> >
>> > I never did see a reply to my proposed solution, and the below is not
>> > it. Can you explain why you prefer this one?
>>
>> Sorry, just see your email because that thread is not highlighted for
>> new email in gmail for unknown reason.
>> I have concern that "UINT64 VENDOR_SPECIFIC_FIELD[8]" cannot secure
>> the ECSD alignment because it is not the first member.
>
> It being the first member or not has no relevance.
> By my count, it starts at offset 64, with all preceding entries being
> UINT8.

  Maybe you are right. I had have concern that if preceding member of ECSDData,
CSDData, is not 8 bytes aligned in the tail, UINT8 RESERVED_1[] may start from
non 8 bytes aligned address.

>
>> Changing the first member to "UINT64 RESERVED_1[2]" shall secure the
>> alignment.
>
> Relying on a reserved field for setting alignment constraints risks
> having to find an alternative method at some point in the future, so I
> agree this is not a preferred solution.
>
>> But I preferred Pad method. It is more readable if all ECSD member
>> are UINT8 type.
>
> I confess am not familiar enough with eMMC to make a clean call here,
> but if real alignment constrains exist, using UINT8 is actively
> misleading, not to mention incorrect. If there is no real alignment
> constraint, this is not the code that needs fixing.

512 bytes ECSD data may be read either by CPU or DMA via dedicated
 eMMC command. DMA may require alignment for the structure and
DMA on different SoC may require different alignment. Hikey DMA is
OK with 4 bytes alignment, while ZTE SoC require 8 bytes alignment.
And pad must not be added in the 512 bytes structure body by compiler.
So all members in ECSD are defined as UINT8 is safe.

>
>> It is also more clear to add alignment info in
>> CARD_INFO, just before ECSD member.
>
> Why is it more clear to apply alignment constraints at point of use
> instead of point of definition?

Adding alignment attribute in definition is best way for the constraint, but I
 do not know how to make all compilers happy, just like my version 1 patch.
It is more or less obscure to change VENDOR_SPECIFIC_FIELD or RESERVED_1
to type UINT64.

I confess that adding a pad before ECSD usage is compromised method. It
serve as a reminder for developers too, and allow all members of ECSD are
defined with UINT8.

Another method is to define ECSDData as a pointer and allocate buffer for it.
Which method do you prefer?

>
> Is there any chance you can share the EFI_MMC_HOST_PROTOCOL
> implementation that triggers this error?

DWMMC controller on ZTE/Sanchip SoC will read corrupted data with current
DwMmc driver in edk2 96boards git repo. I just add several small patches to
enable it on new SoC. I will send all these changes in near future. Or
what detail
information do you need? I can share it here.

Jun

>
> /
> Leif
>
>> I do not get point of Andrew, maybe he share the same concern.
>>
>> Jun
>>
>> >
>> >> Contributed-under: TianoCore Contribution Agreement 1.0
>> >> Signed-off-by: Jun Nie <jun@linaro.org>
>> >> ---
>> >>  EmbeddedPkg/Universal/MmcDxe/Mmc.h | 1 +
>> >>  1 file changed, 1 insertion(+)
>> >>
>> >> diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h 
>> >> b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
>> >> index 8a7d5a3..6e3ab17 100644
>> >> --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
>> >> +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
>> >> @@ -319,6 +319,7 @@ typedef struct  {
>> >>OCR   OCRData;
>> >>CID   CIDData;
>> >>CSD   CSDData;
>> >> +  UINT64Pad;  // For 8 bytes alignment 
>> >> of ECSDData
>> >>ECSD  ECSDData; // MMC V4 extended card 
>> >> specific
>> >>  } CARD_INFO;
>> >>
>> >> --
>> >> 1.9.1
>> >>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] EmbeddedPkg/MmcDxe: Add alignment for ECSD data

[Jun Nie]

2017-06-13 12:25 GMT+08:00 Andrew Fish <af...@apple.com>:
>
> On Jun 12, 2017, at 9:13 PM, Jun Nie <jun@linaro.org> wrote:
>
> 2017-06-13 12:01 GMT+08:00 Andrew Fish <af...@apple.com>:
>
>
> On Jun 12, 2017, at 7:14 PM, Jun Nie <jun@linaro.org> wrote:
>
> 2017-06-12 23:53 GMT+08:00 Leif Lindholm <leif.lindh...@linaro.org>:
>
> On Mon, Jun 12, 2017 at 09:59:28AM +0800, Jun Nie wrote:
>
> Add alignment for ECSD data for DMA access. Otherwise
> the data is corrupted on Sanechips platform.
>
>
> I never did see a reply to my proposed solution, and the below is not
> it. Can you explain why you prefer this one?
>
> /
>   Leif
>
>
> Sorry, just see your email because that thread is not highlighted for
> new email in gmail for unknown reason.
> I have concern that "UINT64 VENDOR_SPECIFIC_FIELD[8]" cannot secure
> the ECSD alignment because it is not the first member. Changing the
> first member to "UINT64 RESERVED_1[2]" shall secure the alignment. But
> I preferred Pad method. It is more readable if all ECSD member are
> UINT8 type. It is also more clear to add alignment info in CARD_INFO,
> just before ECSD member.
> I do not get point of Andrew, maybe he share the same concern.
>
>
> Jun
>
> typedef enum {
>  UNKNOWN_CARD,
>  MMC_CARD,  //MMC card
>  MMC_CARD_HIGH, //MMC Card with High capacity
>  EMMC_CARD, //eMMC 4.41 card
>  SD_CARD,   //SD 1.1 card
>  SD_CARD_2, //SD 2.0 or above standard card
>  SD_CARD_2_HIGH //SD 2.0 or above high capacity card
> } CARD_TYPE;
>
> Per C spec sizeof(CARD_TYPE) can be 1, 2, 4, or 8 (64-bit integer), and it
> is legal for the compiler to pick any of these. So it is not portable C code
> to use an enum in a data structure when layout maters.
>
> Thanks,
>
> Andrew Fish
>
>
> CARD_TYPE CardType is 2nd member of CARD_INFO, while ECSDData is the
> last member and I just want to align it to 8 bytes. I had assume pad
> will be added automatically by compiler if CARD_TYPE is not 8 bytes
> aligned and UNIT64 type appear in following member. Does enum will
> impact the later member alignment? Could you help elaborate more about
> this?
>
>
> Sure
>
> type struct {
>   UINT16 RCA;
>   UINT8   CardType;
>   UINT32 OCRData;
> ...
>
> Has different alignment than:
> type struct {
>   UINT16 RCA;
>   UINT32 CardType;
>   UINT32 OCRData;
> ...
>
> Both are legal things for the C compiler to due given the type is an enum.
> 1st example OCRData starts at offset 4
> 2nd example OCRData starts at offset 8
>
> An integer type is not an int.
>
> Thanks,
>
> Andrew Fish

But I do not care about OCRData. I just care the alignment of
ECSDData, which is the last member and I assume it shall be impacted
just by the previous member alignment. Does ECSDData alignment cannot
be secured with Pad change if enum is the 2nd member? If so, is it
safe if I make sure the first member in ECSDData is UINT64?

 typedef struct  {
  UINT16 RCA;
  UINT8   CardType;
  OCR   OCRData;
  CID       CIDData;
  CSD   CSDData;
+  UINT64Pad;  // For 8 bytes
alignment of ECSDData
   ECSD  ECSDData; // MMC V4 extended card specific
 } CARD_INFO;

Thanks!
Jun

>
>
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie <jun@linaro.org>
> ---
> EmbeddedPkg/Universal/MmcDxe/Mmc.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> index 8a7d5a3..6e3ab17 100644
> --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> @@ -319,6 +319,7 @@ typedef struct  {
>  OCR   OCRData;
>  CID   CIDData;
>  CSD   CSDData;
> +  UINT64Pad;  // For 8 bytes alignment of
> ECSDData
>  ECSD  ECSDData; // MMC V4 extended card
> specific
> } CARD_INFO;
>
> --
> 1.9.1
>
> ___
> 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] [PATCH 1/2] MMC : Recieve response was missing after CMD12

[Jun Nie]

On 2017年08月31日 19:22, Leif Lindholm wrote:

On Wed, Aug 30, 2017 at 07:50:58PM +0530, Meenakshi Aggarwal wrote:

We are not recieving the response from memory card after
sending CMD 12. It was not resulting in any failure but
we should recieve response after sending a command.


Per spec, there is response data for CMD12. It is reasonable to read it.

Reviewed-by: Jun Nie <jun@linaro.org>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/2] SD : Updated CMD 6 implememtation.

[Jun Nie]

On 2017年08月31日 20:06, Leif Lindholm wrote:

On Wed, Aug 30, 2017 at 07:50:59PM +0530, Meenakshi Aggarwal wrote:

For setting high speed in SD card,
First CMD 6 (Switch) is send to check if card supports High Speed and
Second command is send to switch card to high speed mode.

In current inplementation, CMD 6 was sent only once to switch the
card into HS mode without checking if card supports HS or not, which is
not as per specification and also we are not setting the HS i.e. 5000
but directly asking the card to switch to 2600 which is incorrect as
SD card supports either 2500 or 5000.


Good catch, check should be done before setting function. And the 
setting result should be checked before return. Logic is correct in this 
patch.




Same as previous one: Jun, Haojian?

I do have a couple of style comments below.


Signed-off-by: Meenakshi Aggarwal 
---
  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 64 
  1 file changed, 55 insertions(+), 9 deletions(-)

diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c 
b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
index 7f74c54..3071b3b 100644
--- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
+++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
@@ -317,6 +317,24 @@ InitializeEmmcDevice (
return Status;
  }
  
+

+STATIC
+UINT32
+CreateSwitchCmdArgument (


This helper function is a good addition, thanks.


+  IN  UINT8  Mode,
+  IN  UINT8  Group,
+  IN  UINT8  Value
+  )
+{
+  UINT32 Argument;
+
+  Argument = Mode << 31 | 0x00FF;


Just because I hate implicit type promotion, could you make Mode
UINT32 in the input, please?


+  Argument &= ~(0xF << (Group * 4));
+  Argument |= Value << (Group * 4);
+
+  return Argument;
+}
+
  STATIC
  EFI_STATUS
  InitializeSdMmcDevice (
@@ -326,6 +344,7 @@ InitializeSdMmcDevice (
UINT32CmdArg;
UINT32Response[4];
UINT32Buffer[128];
+  UINT32Speed;
UINTN BlockSize;
UINTN CardSize;
UINTN NumBlocks;
@@ -334,6 +353,7 @@ InitializeSdMmcDevice (
EFI_STATUSStatus;
EFI_MMC_HOST_PROTOCOL *MmcHost;
  
+  Speed = 2500;


Could this be given a #define with a descriptive name, in Mmc.h?


MmcHost = MmcHostInstance->MmcHost;
  
// Send a command to get Card specific data

@@ -439,43 +459,69 @@ InitializeSdMmcDevice (
  }
}
if (CccSwitch) {
+/* SD Switch, Mode:0, Group:0, Value:0 */
+CmdArg = CreateSwitchCmdArgument(0, 0, 0);


A SD_MODE_CHECK/GET macro is clearer than 0 and 1 value for Mode.


+Status = MmcHost->SendCommand (MmcHost, MMC_CMD6, CmdArg);
+if (EFI_ERROR (Status)) {
+  DEBUG ((EFI_D_ERROR, "%a(MMC_CMD6): Failed with Status = %r\n", 
__func__, Status));
+   return Status;
+} else {
+  Status = MmcHost->ReadBlockData (MmcHost, 0, 64, Buffer);


What are 0 and 64?
I guess 64 is a size?
Is there a #define or a sizeof() that could make it more descriptive?


+  if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_ERROR, "%a(MMC_CMD6): ReadBlockData Failed with Status = 
%r\n", __func__, Status));
+return Status;
+  }
+}
+
+if (!(Buffer[3] & 0x2)) {


Bit 401 is HS support status. So bit in Buffer[12] should be tested.
Or I miss anything? I am checking "SD Specifications Part 1 Physical 
Layer Specification Version 2.00".




Is there no struct available to access this information in more human
readable form?

And a #define for the 0x2, please.


+  DEBUG ((EFI_D_ERROR, "%aHigh Speed not supported by Card %r\n", 
__func__, Status));
+  return Status;
+}
+
+Speed = 5000;   //High Speed for SD card is 50 MHZ


Could this be given a #define with a descriptive name, in Mmc.h?


+
  /* SD Switch, Mode:1, Group:0, Value:1 */
-CmdArg = 1 << 31 | 0x00FF;
-CmdArg &= ~(0xF << (0 * 4));
-CmdArg |= 1 << (0 * 4);
+CmdArg = CreateSwitchCmdArgument(1, 0, 1);
  Status = MmcHost->SendCommand (MmcHost, MMC_CMD6, CmdArg);
  if (EFI_ERROR (Status)) {
-  DEBUG ((EFI_D_ERROR, "%a(MMC_CMD6): Error and Status = %r\n", Status));
+  DEBUG ((EFI_D_ERROR, "%a(MMC_CMD6): Error and Status = %r\n", __func__, 
Status));


This looks like an unrelated bugfix? It is good, and thank you, but
could you break it out into its own patch please?
Also, __FUNCTION__ matches the coding style better (I know we have
both, but __func__ appears to be losing, and I would like to keep that
momentum up.


 return Status;
  } else {
Status = MmcHost->ReadBlockData (MmcHost, 0, 64, Buffer);
if (EFI_ERROR (Status)) {
-DEBUG ((EFI_D_ERROR, "%a(MMC_CMD6): ReadBlockData Error and Status = 
%r\n", Status));
+DEBUG ((EFI_D_ERROR, "%a(MMC_CMD6): ReadBlockData Error and Status = 
%r\n",__func__, Status));


Unrelated bugfix (same as comment above, and same patch please).


+return Status;
+  }
+
+