From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: hao.a.wu@intel.com) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by groups.io with SMTP; Wed, 07 Aug 2019 19:37:29 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Aug 2019 19:37:29 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,358,1559545200"; d="scan'208";a="350026699" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga005.jf.intel.com with ESMTP; 07 Aug 2019 19:37:28 -0700 Received: from fmsmsx607.amr.corp.intel.com (10.18.126.87) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 7 Aug 2019 19:37:28 -0700 Received: from fmsmsx607.amr.corp.intel.com (10.18.126.87) by fmsmsx607.amr.corp.intel.com (10.18.126.87) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Wed, 7 Aug 2019 19:37:28 -0700 Received: from shsmsx154.ccr.corp.intel.com (10.239.6.54) by fmsmsx607.amr.corp.intel.com (10.18.126.87) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Wed, 7 Aug 2019 19:37:27 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.112]) by SHSMSX154.ccr.corp.intel.com ([169.254.7.249]) with mapi id 14.03.0439.000; Thu, 8 Aug 2019 10:37:26 +0800 From: "Wu, Hao A" To: "Albecki, Mateusz" , "devel@edk2.groups.io" Subject: Re: [PATCH 4/4] MdeModulePkg/UfsPassThruDxe: Implement EDKII_UFS_HC_PLATFORM_PROTOCOL Thread-Topic: [PATCH 4/4] MdeModulePkg/UfsPassThruDxe: Implement EDKII_UFS_HC_PLATFORM_PROTOCOL Thread-Index: AQHVTUBs2YyyAIVtEkOstqMzuZUsoabwiHpQ Date: Thu, 8 Aug 2019 02:37:26 +0000 Message-ID: References: <20190807165107.688-1-mateusz.albecki@intel.com> <20190807165107.688-5-mateusz.albecki@intel.com> In-Reply-To: <20190807165107.688-5-mateusz.albecki@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: hao.a.wu@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hello Mateusz, One inline comment below: > -----Original Message----- > From: Albecki, Mateusz > Sent: Thursday, August 08, 2019 12:51 AM > To: devel@edk2.groups.io > Cc: Albecki, Mateusz; Wu, Hao A > Subject: [PATCH 4/4] MdeModulePkg/UfsPassThruDxe: Implement > EDKII_UFS_HC_PLATFORM_PROTOCOL >=20 > https://bugzilla.tianocore.org/show_bug.cgi?id=3D1343 >=20 > This commit adds EDKII_UFS_HC_PLATFORM_PROTOCOL implementation > in UfsPassThruDxe driver in version 1. Driver assumes that at > most one instance of the protocol exists in the system. Presence > of the protocol is not mandatory. >=20 > Cc: Hao A Wu > Signed-off-by: Mateusz Albecki > --- > MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 17 ++++++ > MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 26 +++++++++ > .../Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf | 3 +- > .../Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 68 > ++++++++++++++++++++++ > 4 files changed, 113 insertions(+), 1 deletion(-) >=20 > diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c > b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c > index 09333c51d6..1559efe191 100644 > --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c > +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c > @@ -36,6 +36,7 @@ UFS_PASS_THRU_PRIVATE_DATA > gUfsPassThruTemplate =3D { > 0, // UfsHostController > 0, // UfsHcBase > {0, 0}, // UfsHcInfo > + {NULL, NULL}, // UfsHcDriverInterface > 0, // TaskTag > 0, // UtpTrlBase > 0, // Nutrs > @@ -92,6 +93,8 @@ UFS_DEVICE_PATH mUfsDevicePathTemplate =3D { >=20 > UINT8 mUfsTargetId[TARGET_MAX_BYTES]; >=20 > +GLOBAL_REMOVE_IF_UNREFERENCED > EDKII_UFS_HC_PLATFORM_PROTOCOL *mUfsHcPlatform; > + > /** > Sends a SCSI Request Packet to a SCSI device that is attached to the S= CSI > channel. This function > supports both blocking I/O and nonblocking I/O. The blocking I/O > functionality is required, and the > @@ -864,7 +867,21 @@ UfsPassThruDriverBindingStart ( > Private->ExtScsiPassThru.Mode =3D &Private->ExtScsiPassThruMode; > Private->UfsHostController =3D UfsHc; > Private->UfsHcBase =3D UfsHcBase; > + Private->Handle =3D Controller; > + Private->UfsHcDriverInterface.UfsHcProtocol =3D UfsHc; > + Private->UfsHcDriverInterface.UfsExecUicCommand =3D > UfsHcDriverInterfaceExecUicCommand; > InitializeListHead (&Private->Queue); > + > + // > + // This has to be done before initializing UfsHcInfo or calling the > UfsControllerInit > + // > + if (mUfsHcPlatform =3D=3D NULL) { > + Status =3D gBS->LocateProtocol (&gEdkiiUfsHcPlatformProtocolGuid, NU= LL, > (VOID**)&mUfsHcPlatform); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_INFO, "No UfsHcPlatformProtocol present\n")); > + } > + } > + > Status =3D GetUfsHcInfo (Private); > if (EFI_ERROR (Status)) { > DEBUG ((DEBUG_ERROR, "Failed to initialize UfsHcInfo\n")); > diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h > b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h > index c511aa8c7a..cbc0c2126e 100644 > --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h > +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h > @@ -63,6 +63,7 @@ typedef struct _UFS_PASS_THRU_PRIVATE_DATA { > EDKII_UFS_HOST_CONTROLLER_PROTOCOL *UfsHostController; > UINTN UfsHcBase; > EDKII_UFS_HC_INFO UfsHcInfo; > + EDKII_UFS_HC_DRIVER_INTERFACE UfsHcDriverInterface; >=20 > UINT8 TaskTag; >=20 > @@ -126,6 +127,13 @@ typedef struct { > UFS_PASS_THRU_SIG \ > ) >=20 > +#define UFS_PASS_THRU_PRIVATE_DATA_FROM_DRIVER_INTF(a) \ > + CR (a, \ > + UFS_PASS_THRU_PRIVATE_DATA, \ > + UfsHcDriverInterface, \ > + UFS_PASS_THRU_SIG \ > + ) > + > typedef struct _UFS_DEVICE_MANAGEMENT_REQUEST_PACKET { > UINT64 Timeout; > VOID *DataBuffer; > @@ -959,6 +967,23 @@ UfsRwUfsAttribute ( > IN OUT UINT32 *AttrSize > ); >=20 > +/** > + Execute UIC command. > + > + @param[in] This Pointer to driver interface produced by th= e UFS > controller. > + @param[in, out] UicCommand Descriptor of the command that will be > executed. > + > + @retval EFI_SUCCESS Command executed successfully. > + @retval EFI_INVALID_PARAMETER This or UicCommand is NULL. > + @retval Others Command failed to execute. > +**/ > +EFI_STATUS > +EFIAPI > +UfsHcDriverInterfaceExecUicCommand ( > + IN EDKII_UFS_HC_DRIVER_INTERFACE *This, > + IN OUT EDKII_UIC_COMMAND *UicCommand > + ); > + > /** > Initializes UfsHcInfo field in private data. >=20 > @@ -975,5 +1000,6 @@ GetUfsHcInfo ( > extern EFI_COMPONENT_NAME_PROTOCOL > gUfsPassThruComponentName; > extern EFI_COMPONENT_NAME2_PROTOCOL > gUfsPassThruComponentName2; > extern EFI_DRIVER_BINDING_PROTOCOL gUfsPassThruDriverBinding; > +extern EDKII_UFS_HC_PLATFORM_PROTOCOL *mUfsHcPlatform; >=20 > #endif > diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf > b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf > index 24f5ea3a8f..92dc25714b 100644 > --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf > +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf > @@ -1,7 +1,7 @@ > ## @file > # Description file for the Universal Flash Storage (UFS) Pass Thru drive= r. > # > -# Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
> +# Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.
> # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -54,6 +54,7 @@ > gEfiExtScsiPassThruProtocolGuid ## BY_START > gEfiUfsDeviceConfigProtocolGuid ## BY_START > gEdkiiUfsHostControllerProtocolGuid ## TO_START > + gEdkiiUfsHcPlatformProtocolGuid ## SOMETIMES_CONSUMES >=20 > [UserExtensions.TianoCore."ExtraFiles"] > UfsPassThruExtra.uni > diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c > b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c > index 74be3efc41..e2c104b103 100644 > --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c > +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c > @@ -1835,6 +1835,14 @@ UfsEnableHostController ( > EFI_STATUS Status; > UINT32 Data; >=20 > + if (mUfsHcPlatform !=3D NULL && mUfsHcPlatform->Callback !=3D NULL) { > + Status =3D mUfsHcPlatform->Callback (Private->Handle, EdkiiUfsHcPreH= ce, > &Private->UfsHcDriverInterface); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Failure from platform driver during > EdkiiUfsHcPreHce, Status =3D %r\n", Status)); > + return Status; > + } > + } > + > // > // UFS 2.0 spec section 7.1.1 - Host Controller Initialization > // > @@ -1878,6 +1886,14 @@ UfsEnableHostController ( > return EFI_DEVICE_ERROR; > } >=20 > + if (mUfsHcPlatform !=3D NULL && mUfsHcPlatform->Callback !=3D NULL) { > + Status =3D mUfsHcPlatform->Callback (Private->Handle, EdkiiUfsHcPost= Hce, > &Private->UfsHcDriverInterface); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Failure from platform driver during > EdkiiUfsHcPostHce, Status =3D %r\n", Status)); > + return Status; > + } > + } > + > return EFI_SUCCESS; > } >=20 > @@ -1901,6 +1917,14 @@ UfsDeviceDetection ( > UINT32 Data; > EDKII_UIC_COMMAND LinkStartupCommand; >=20 > + if (mUfsHcPlatform !=3D NULL && mUfsHcPlatform->Callback !=3D NULL) { > + Status =3D mUfsHcPlatform->Callback (Private->Handle, > EdkiiUfsHcPreLinkStartup, &Private->UfsHcDriverInterface); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Failure from platform driver during > EdkiiUfsHcPreLinkStartup, Status =3D %r\n", Status)); > + return Status; > + } > + } > + > // > // Start UFS device detection. > // Try up to 3 times for establishing data link with device. > @@ -1930,6 +1954,14 @@ UfsDeviceDetection ( > } > } >=20 > + if (mUfsHcPlatform !=3D NULL && mUfsHcPlatform->Callback !=3D NULL) { > + Status =3D mUfsHcPlatform->Callback (Private->Handle, > EdkiiUfsHcPostLinkStartup, &Private->UfsHcDriverInterface); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Failure from platform driver during > EdkiiUfsHcPostLinkStartup, Status =3D %r\n", Status)); > + return Status; > + } > + } > + It looks to me that the 'PostLinkStartup' callback is triggered when the UF= S device detection fails (placed above "return EFI_NOT_FOUND;"). Per my understanding, the callback should be invoked when such detection process succeeds. Could you help to confirm on this? Best Regards, Hao Wu > return EFI_NOT_FOUND; > } >=20 > @@ -2350,6 +2382,34 @@ ProcessAsyncTaskList ( > } > } >=20 > +/** > + Execute UIC command. > + > + @param[in] This Pointer to driver interface produced by th= e UFS > controller. > + @param[in, out] UicCommand Descriptor of the command that will be > executed. > + > + @retval EFI_SUCCESS Command executed successfully. > + @retval EFI_INVALID_PARAMETER This or UicCommand is NULL. > + @retval Others Command failed to execute. > +**/ > +EFI_STATUS > +EFIAPI > +UfsHcDriverInterfaceExecUicCommand ( > + IN EDKII_UFS_HC_DRIVER_INTERFACE *This, > + IN OUT EDKII_UIC_COMMAND *UicCommand > + ) > +{ > + UFS_PASS_THRU_PRIVATE_DATA *Private; > + > + if (This =3D=3D NULL || UicCommand =3D=3D NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + Private =3D UFS_PASS_THRU_PRIVATE_DATA_FROM_DRIVER_INTF (This); > + > + return UfsExecUicCommands (Private, UicCommand); > +} > + > /** > Initializes UfsHcInfo field in private data. >=20 > @@ -2380,6 +2440,14 @@ GetUfsHcInfo ( >=20 > Private->UfsHcInfo.Capabilities =3D Data; >=20 > + if (mUfsHcPlatform !=3D NULL && mUfsHcPlatform->OverrideHcInfo !=3D NU= LL) > { > + Status =3D mUfsHcPlatform->OverrideHcInfo (Private->Handle, &Private= - > >UfsHcInfo); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Failure from platform on OverrideHcInfo, > Status =3D %r\n", Status)); > + return Status; > + } > + } > + > return EFI_SUCCESS; > } >=20 > -- > 2.14.1.windows.1