Re: [edk2] [PATCH 3/3] MdeModulePkg/UfsPassThruDxe: Add UFS info protocol
Hello Mateusz,
A couple of general comments:
A. I suggest to break this commit into 2 patches:
The first one will just introduce the new protocol.
The second one will update the UfsPassThruDxe driver to consume this new
protocol.
B. There has been a Bugzilla tracker for the feature you add in this
patch:
https://bugzilla.tianocore.org/show_bug.cgi?id=1343
Could you help to add this information in the commit log message? Thanks.
C. I prefer the new protocol to have platform level singleton instance:
I do not see much value for a platform to produce multiple instances of
this protocol, I think the additional UIC commands needed during the UFS
HC initialization for a specific platform is deterministic.
Also, the selection of protocol instance implemented in function
GetUfsHcInfoProtocol() is by:
1) LocateHandleBuffer() to get all protocol instances;
2) Choose the 1st instance if the 'Supported' service returns without
error.
I think this will bring some kind of confusion to the protocol producers,
since they do not know which one will be selected when there are multiple
instances of the protocol.
> -Original Message-
> From: Albecki, Mateusz
> Sent: Thursday, March 28, 2019 9:56 PM
> To: edk2-devel@lists.01.org
> Cc: Albecki, Mateusz; Wu, Hao A
> Subject: [PATCH 3/3] MdeModulePkg/UfsPassThruDxe: Add UFS info protocol
>
> UFS host controller specification allows for implementation specific
> UIC programming to take place just after host controller enable and before
> device detection. Since there is no way for generic driver to anticipate
> implementation specific programming we add a UFS info protocol
> which allows the implementation specific code to pass this information
> to generic driver. UFS info protocol is located by the driver at the
> BindingStart call and the supported instance is retained throught entire
throught -> through
> driver lifetime. Presence of the protocol is optional.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Cc: Hao Wu
> Signed-off-by: Albecki Mateusz
> ---
> MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 62
> ++
> MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 2 +
> .../Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf | 1 +
> .../Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c| 60 +
> .../Include/Protocol/UfsHostControllerInfo.h | 75
> ++
> MdeModulePkg/MdeModulePkg.dec | 3 +
> 6 files changed, 203 insertions(+)
> create mode 100644 MdeModulePkg/Include/Protocol/UfsHostControllerInfo.h
>
> diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> index ea329618dc..a41079e311 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> @@ -40,6 +40,7 @@ UFS_PASS_THRU_PRIVATE_DATA gUfsPassThruTemplate
> = {
> UfsRwUfsAttribute
>},
>0, // UfsHostController
> + NULL, // UfsInfo
>0, // UfsHcBase
>0, // Capabilities
>0, // TaskTag
> @@ -776,6 +777,66 @@ UfsFinishDeviceInitialization (
>return EFI_SUCCESS;
> }
>
> +/**
> + Locates UFS HC infor protocol suitable for this controller.
infor -> info
> +
> + @param[in] DriverBinding Pointer to driver binding protocol
> + @param[in] Private Pointer to host controller private data
> + @param[in] ControllerHandle Controller to which driver is bound
> +**/
> +VOID
> +GetUfsHcInfoProtocol (
> + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding,
> + IN UFS_PASS_THRU_PRIVATE_DATA *Private,
> + IN EFI_HANDLE ControllerHandle
> + )
> +{
> + EFI_STATUSStatus;
> + EFI_HANDLE*ProtocolHandleArray;
> + UINTN NoHandles;
> + UINTN HandleIndex;
> + UFS_HC_INFO_PROTOCOL *UfsHcInfo;
> +
> + Status = gBS->LocateHandleBuffer (
> + ByProtocol,
> + ,
> + NULL,
> + ,
> +
> + );
> + if (EFI_ERROR (Status)) {
> +return;
> + }
> +
> + for (HandleIndex = 0; HandleIndex < NoHandles; HandleIndex++) {
> +Status = gBS->OpenProtocol (
> +ProtocolHandleArray[HandleIndex],
> +,
> +,
> +DriverBinding->DriverBindingHandle,
> +ControllerHandle,
> +EFI_OPEN_PROTOCOL_BY_DRIVER
> +);
> +if (EFI_ERROR (Status)) {
> + continue;
> +}
> +if (!EFI_ERROR(UfsHcInfo->Supported (UfsHcInfo, ControllerHandle))) {
> + Private->UfsHcInfo = UfsHcInfo;
> + break;
> +} else {
> + Status = gBS->CloseProtocol (
> +
[edk2] [PATCH 3/3] MdeModulePkg/UfsPassThruDxe: Add UFS info protocol
UFS host controller specification allows for implementation specific
UIC programming to take place just after host controller enable and before
device detection. Since there is no way for generic driver to anticipate
implementation specific programming we add a UFS info protocol
which allows the implementation specific code to pass this information
to generic driver. UFS info protocol is located by the driver at the
BindingStart call and the supported instance is retained throught entire
driver lifetime. Presence of the protocol is optional.
Contributed-under: TianoCore Contribution Agreement 1.1
Cc: Hao Wu
Signed-off-by: Albecki Mateusz
---
MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 62 ++
MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 2 +
.../Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf | 1 +
.../Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c| 60 +
.../Include/Protocol/UfsHostControllerInfo.h | 75 ++
MdeModulePkg/MdeModulePkg.dec | 3 +
6 files changed, 203 insertions(+)
create mode 100644 MdeModulePkg/Include/Protocol/UfsHostControllerInfo.h
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
index ea329618dc..a41079e311 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
@@ -40,6 +40,7 @@ UFS_PASS_THRU_PRIVATE_DATA gUfsPassThruTemplate = {
UfsRwUfsAttribute
},
0, // UfsHostController
+ NULL, // UfsInfo
0, // UfsHcBase
0, // Capabilities
0, // TaskTag
@@ -776,6 +777,66 @@ UfsFinishDeviceInitialization (
return EFI_SUCCESS;
}
+/**
+ Locates UFS HC infor protocol suitable for this controller.
+
+ @param[in] DriverBinding Pointer to driver binding protocol
+ @param[in] Private Pointer to host controller private data
+ @param[in] ControllerHandle Controller to which driver is bound
+**/
+VOID
+GetUfsHcInfoProtocol (
+ IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding,
+ IN UFS_PASS_THRU_PRIVATE_DATA *Private,
+ IN EFI_HANDLE ControllerHandle
+ )
+{
+ EFI_STATUSStatus;
+ EFI_HANDLE*ProtocolHandleArray;
+ UINTN NoHandles;
+ UINTN HandleIndex;
+ UFS_HC_INFO_PROTOCOL *UfsHcInfo;
+
+ Status = gBS->LocateHandleBuffer (
+ ByProtocol,
+ ,
+ NULL,
+ ,
+
+ );
+ if (EFI_ERROR (Status)) {
+return;
+ }
+
+ for (HandleIndex = 0; HandleIndex < NoHandles; HandleIndex++) {
+Status = gBS->OpenProtocol (
+ProtocolHandleArray[HandleIndex],
+,
+,
+DriverBinding->DriverBindingHandle,
+ControllerHandle,
+EFI_OPEN_PROTOCOL_BY_DRIVER
+);
+if (EFI_ERROR (Status)) {
+ continue;
+}
+if (!EFI_ERROR(UfsHcInfo->Supported (UfsHcInfo, ControllerHandle))) {
+ Private->UfsHcInfo = UfsHcInfo;
+ break;
+} else {
+ Status = gBS->CloseProtocol (
+ ProtocolHandleArray[HandleIndex],
+ ,
+ DriverBinding->DriverBindingHandle,
+ ControllerHandle
+ );
+ ASSERT_EFI_ERROR (Status);
+}
+ }
+
+ FreePool (ProtocolHandleArray);
+}
+
/**
Starts a device controller or a bus controller.
@@ -871,6 +932,7 @@ UfsPassThruDriverBindingStart (
Private->UfsHostController= UfsHc;
Private->UfsHcBase= UfsHcBase;
InitializeListHead (>Queue);
+ GetUfsHcInfoProtocol (This, Private, Controller);
//
// Initialize UFS Host Controller H/W.
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
index e591e78661..55a8ed9bdf 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include "UfsPassThruHci.h"
@@ -66,6 +67,7 @@ typedef struct _UFS_PASS_THRU_PRIVATE_DATA {
EFI_EXT_SCSI_PASS_THRU_PROTOCOL ExtScsiPassThru;
EFI_UFS_DEVICE_CONFIG_PROTOCOL UfsDevConfig;
EDKII_UFS_HOST_CONTROLLER_PROTOCOL *UfsHostController;
+ UFS_HC_INFO_PROTOCOL*UfsHcInfo;
UINTN UfsHcBase;
UINT32 Capabilities;
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf
b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf
index e550cd02b4..12b01771ff 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf
+++
