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: mateusz.albecki@intel.com) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by groups.io with SMTP; Thu, 08 Aug 2019 13:52:33 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Aug 2019 13:52:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,362,1559545200"; d="scan'208";a="182708603" Received: from irsmsx103.ger.corp.intel.com ([163.33.3.157]) by FMSMGA003.fm.intel.com with ESMTP; 08 Aug 2019 13:52:32 -0700 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.59]) by IRSMSX103.ger.corp.intel.com ([169.254.3.45]) with mapi id 14.03.0439.000; Thu, 8 Aug 2019 21:52:31 +0100 From: "Albecki, Mateusz" To: "Wu, Hao A" , "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: AQHVTUBsbDHrNHBiVU6gPmWY3ZCJvabweSkAgAFCTXA= Date: Thu, 8 Aug 2019 20:52:30 +0000 Message-ID: <92CF190FF2351747A47C1708F0E09C0875EB4B63@IRSMSX102.ger.corp.intel.com> References: <20190807165107.688-1-mateusz.albecki@intel.com> <20190807165107.688-5-mateusz.albecki@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMGU3NTQ1ZGUtNzQ0NS00ZDM1LTk1MjAtM2RkNTcyZDg3MThiIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoicitEdDB0YVpnTkZ6WVJvRCtyZXptYlhjeitENitRYWlHNHVQdzZsTHFySXowTHZ6MmpqbmZhRzRkNkZTU2J4VyJ9 dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [163.33.239.181] MIME-Version: 1.0 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi, Yes that is bug. It should be placed before return EFI_SUCCESS. I will fix = it in v2. Thanks, Mateusz > -----Original Message----- > From: Wu, Hao A > Sent: Thursday, August 8, 2019 4:37 AM > To: Albecki, Mateusz ; devel@edk2.groups.io > Subject: RE: [PATCH 4/4] MdeModulePkg/UfsPassThruDxe: Implement > EDKII_UFS_HC_PLATFORM_PROTOCOL > = > 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 > > > > https://bugzilla.tianocore.org/show_bug.cgi?id=3D1343 > > > > 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. > > > > 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(-) > > > > 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 { > > > > UINT8 mUfsTargetId[TARGET_MAX_BYTES]; > > > > +GLOBAL_REMOVE_IF_UNREFERENCED > > EDKII_UFS_HC_PLATFORM_PROTOCOL *mUfsHcPlatform; > > + > > /** > > Sends a SCSI Request Packet to a SCSI device that is attached to > > the SCSI 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, > > + NULL, > > (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; > > > > UINT8 TaskTag; > > > > @@ -126,6 +127,13 @@ typedef struct { > > UFS_PASS_THRU_SIG \ > > ) > > > > +#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 > > ); > > > > +/** > > + Execute UIC command. > > + > > + @param[in] This Pointer to driver interface produced by = the 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. > > > > @@ -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; > > > > #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 dri= ver. > > # > > -# 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 > > > > [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; > > > > + if (mUfsHcPlatform !=3D NULL && mUfsHcPlatform->Callback !=3D NULL) { > > + Status =3D mUfsHcPlatform->Callback (Private->Handle, > > + EdkiiUfsHcPreHce, > > &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; > > } > > > > + if (mUfsHcPlatform !=3D NULL && mUfsHcPlatform->Callback !=3D NULL) { > > + Status =3D mUfsHcPlatform->Callback (Private->Handle, > > + EdkiiUfsHcPostHce, > > &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; > > } > > > > @@ -1901,6 +1917,14 @@ UfsDeviceDetection ( > > UINT32 Data; > > EDKII_UIC_COMMAND LinkStartupCommand; > > > > + 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 ( > > } > > } > > > > + 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 = UFS > 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; > > } > > > > @@ -2350,6 +2382,34 @@ ProcessAsyncTaskList ( > > } > > } > > > > +/** > > + Execute UIC command. > > + > > + @param[in] This Pointer to driver interface produced by = the 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. > > > > @@ -2380,6 +2440,14 @@ GetUfsHcInfo ( > > > > Private->UfsHcInfo.Capabilities =3D Data; > > > > + if (mUfsHcPlatform !=3D NULL && mUfsHcPlatform->OverrideHcInfo !=3D > > + NULL) > > { > > + 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; > > } > > > > -- > > 2.14.1.windows.1 -------------------------------------------------------------------- Intel Technology Poland sp. z o.o. ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydz= ial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-31= 6 | Kapital zakladowy 200.000 PLN. Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata= i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wi= adomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiek= olwiek przegladanie lub rozpowszechnianie jest zabronione. This e-mail and any attachments may contain confidential material for the s= ole use of the intended recipient(s). If you are not the intended recipient= , please contact the sender and delete all copies; any review or distributi= on by others is strictly prohibited.