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.136, mailfrom: hao.a.wu@intel.com) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by groups.io with SMTP; Wed, 07 Aug 2019 19:37:00 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Aug 2019 19:37:00 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,358,1559545200"; d="scan'208";a="168835291" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga008.jf.intel.com with ESMTP; 07 Aug 2019 19:36:59 -0700 Received: from fmsmsx606.amr.corp.intel.com (10.18.126.86) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 7 Aug 2019 19:36:59 -0700 Received: from fmsmsx606.amr.corp.intel.com (10.18.126.86) by fmsmsx606.amr.corp.intel.com (10.18.126.86) 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:36:58 -0700 Received: from shsmsx154.ccr.corp.intel.com (10.239.6.54) by fmsmsx606.amr.corp.intel.com (10.18.126.86) 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:36:58 -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:36:56 +0800 From: "Wu, Hao A" To: "devel@edk2.groups.io" , "Albecki, Mateusz" Subject: Re: [edk2-devel] [PATCH 2/4] MdeModulePkg/UfsPassThruDxe: Refactor UfsExecUicCommand function Thread-Topic: [edk2-devel] [PATCH 2/4] MdeModulePkg/UfsPassThruDxe: Refactor UfsExecUicCommand function Thread-Index: AQHVTUBhKQlQhYkR+EqQ4xcm9lfNz6bwhAWQ Date: Thu, 8 Aug 2019 02:36:56 +0000 Message-ID: References: <20190807165107.688-1-mateusz.albecki@intel.com> <20190807165107.688-3-mateusz.albecki@intel.com> In-Reply-To: <20190807165107.688-3-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 > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Albecki, Mateusz > Sent: Thursday, August 08, 2019 12:51 AM > To: devel@edk2.groups.io > Cc: Albecki, Mateusz; Wu, Hao A > Subject: [edk2-devel] [PATCH 2/4] MdeModulePkg/UfsPassThruDxe: > Refactor UfsExecUicCommand function >=20 > https://bugzilla.tianocore.org/show_bug.cgi?id=3D1343 >=20 > UfsExecUicCommand function has been refactored to allow > the caller to check the command results which is important > for commands such as UIC read. >=20 > Cc: Hao A Wu > Signed-off-by: Mateusz Albecki > --- > MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 3 +- > .../Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 47 ++++++++++++---= ---- > --- > 2 files changed, 27 insertions(+), 23 deletions(-) >=20 > diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h > b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h > index 9b68db5ffe..b79be77709 100644 > --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h > +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h > @@ -1,6 +1,6 @@ > /** @file >=20 > - 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 >=20 > **/ > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include >=20 > #include > #include > diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c > b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c > index 912d6f8202..6ea27e473c 100644 > --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c > +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c > @@ -2,7 +2,7 @@ > UfsPassThruDxe driver is used to produce EFI_EXT_SCSI_PASS_THRU > protocol interface > for upper layer application to execute UFS-supported SCSI cmds. >=20 > - 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 >=20 > **/ > @@ -1633,11 +1633,8 @@ Exit1: > /** > Send UIC command. >=20 > - @param[in] Private The pointer to the > UFS_PASS_THRU_PRIVATE_DATA data structure. > - @param[in] UicOpcode The opcode of the UIC command. > - @param[in] Arg1 The value for 1st argument of the UIC com= mand. > - @param[in] Arg2 The value for 2nd argument of the UIC com= mand. > - @param[in] Arg3 The value for 3rd argument of the UIC com= mand. > + @param[in] Private The pointer to the > UFS_PASS_THRU_PRIVATE_DATA data structure. > + @param[in, out] UicCommand UIC command descriptor. On exit contains > UIC command results. >=20 > @return EFI_SUCCESS Successfully execute this UIC command and > detect attached UFS device. > @return EFI_DEVICE_ERROR Fail to execute this UIC command and detect > attached UFS device. > @@ -1646,10 +1643,7 @@ Exit1: > EFI_STATUS > UfsExecUicCommands ( > IN UFS_PASS_THRU_PRIVATE_DATA *Private, > - IN UINT8 UicOpcode, > - IN UINT32 Arg1, > - IN UINT32 Arg2, > - IN UINT32 Arg3 > + IN OUT EDKII_UIC_COMMAND *UicCommand > ) > { > EFI_STATUS Status; > @@ -1675,17 +1669,17 @@ UfsExecUicCommands ( > // only after all the UIC command argument registers (UICCMDARG1, > UICCMDARG2 and UICCMDARG3) > // are set. > // > - Status =3D UfsMmioWrite32 (Private, UFS_HC_UCMD_ARG1_OFFSET, Arg1); > + Status =3D UfsMmioWrite32 (Private, UFS_HC_UCMD_ARG1_OFFSET, > UicCommand->Arg1); > if (EFI_ERROR (Status)) { > return Status; > } >=20 > - Status =3D UfsMmioWrite32 (Private, UFS_HC_UCMD_ARG2_OFFSET, Arg2); > + Status =3D UfsMmioWrite32 (Private, UFS_HC_UCMD_ARG2_OFFSET, > UicCommand->Arg2); > if (EFI_ERROR (Status)) { > return Status; > } >=20 > - Status =3D UfsMmioWrite32 (Private, UFS_HC_UCMD_ARG3_OFFSET, Arg3); > + Status =3D UfsMmioWrite32 (Private, UFS_HC_UCMD_ARG3_OFFSET, > UicCommand->Arg3); > if (EFI_ERROR (Status)) { > return Status; > } > @@ -1698,7 +1692,7 @@ UfsExecUicCommands ( > return Status; > } >=20 > - Status =3D UfsMmioWrite32 (Private, UFS_HC_UIC_CMD_OFFSET, > (UINT32)UicOpcode); > + Status =3D UfsMmioWrite32 (Private, UFS_HC_UIC_CMD_OFFSET, > UicCommand->Opcode); > if (EFI_ERROR (Status)) { > return Status; > } > @@ -1712,14 +1706,18 @@ UfsExecUicCommands ( > return Status; > } >=20 > - if (UicOpcode !=3D UfsUicDmeReset) { > - Status =3D UfsMmioRead32 (Private, UFS_HC_UCMD_ARG2_OFFSET, > &Data); > + if (UicCommand->Opcode !=3D UfsUicDmeReset) { > + Status =3D UfsMmioRead32 (Private, UFS_HC_UCMD_ARG2_OFFSET, > &UicCommand->Arg2); > if (EFI_ERROR (Status)) { > return Status; > } > - if ((Data & 0xFF) !=3D 0) { > + Status =3D UfsMmioRead32 (Private, UFS_HC_UCMD_ARG3_OFFSET, > &UicCommand->Arg3); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + if ((UicCommand->Arg2 & 0xFF) !=3D 0) { > DEBUG_CODE_BEGIN(); > - DumpUicCmdExecResult (UicOpcode, (UINT8)(Data & 0xFF)); > + DumpUicCmdExecResult ((UINT8)UicCommand->Opcode, > (UINT8)(UicCommand->Arg2 & 0xFF)); > DEBUG_CODE_END(); > return EFI_DEVICE_ERROR; > } > @@ -1898,16 +1896,21 @@ UfsDeviceDetection ( > IN UFS_PASS_THRU_PRIVATE_DATA *Private > ) > { > - UINTN Retry; > - EFI_STATUS Status; > - UINT32 Data; > + UINTN Retry; > + EFI_STATUS Status; > + UINT32 Data; > + EDKII_UIC_COMMAND LinkStartupCommand; >=20 > // > // Start UFS device detection. > // Try up to 3 times for establishing data link with device. > // > for (Retry =3D 0; Retry < 3; Retry++) { > - Status =3D UfsExecUicCommands (Private, UfsUicDmeLinkStartup, 0, 0,= 0); > + LinkStartupCommand.Opcode =3D UfsUicDmeLinkStartup; > + LinkStartupCommand.Arg1 =3D 0; > + LinkStartupCommand.Arg2 =3D 0; > + LinkStartupCommand.Arg3 =3D 0; > + Status =3D UfsExecUicCommands (Private, &LinkStartupCommand); The patch looks good to me, Reviewed-by: Hao A Wu Best Regards, Hao Wu > if (EFI_ERROR (Status)) { > return EFI_DEVICE_ERROR; > } > -- > 2.14.1.windows.1 >=20 > -------------------------------------------------------------------- >=20 > Intel Technology Poland sp. z o.o. > ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII > Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957- > 07-52-316 | Kapital zakladowy 200.000 PLN. >=20 > Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego > adresata i moze zawierac informacje poufne. W razie przypadkowego > otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale > jej usuniecie; jakiekolwiek > przegladanie lub rozpowszechnianie jest zabronione. > This e-mail and any attachments may contain confidential material for th= e > sole use of the intended recipient(s). If you are not the intended recip= ient, > please contact the sender and delete all copies; any review or distribut= ion by > others is strictly prohibited. >=20 >=20 >=20