Minor inline comments below on coding style from the uncrustify tool: > -----Original Message----- > From: Bandaru, Purna Chandra Rao > Sent: Monday, March 7, 2022 7:47 PM > To: devel@edk2.groups.io > Cc: Bandaru, Purna Chandra Rao ; > Wu, Hao A ; Albecki, Mateusz > ; Gao, Liming ; > Liu, Zhiguang > Subject: [PATCH] [PATCH v2] MdeModulePkg: Add bRefClkFreq card > attribute programming support > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3851 > > When the UFS card comes out of Manufacturer, bRefClkFreq attribute is set > to 1h on the UFS card as per the "MDV” (Manufacturer Default Value) "MDV" (seems like the second quotation mark is a non-ascii character) > specified by the spec JESD220*. However, depends on the UFS host system > environment, it need to be set to correct value. > > Reference Clock Frequency value > 0h:19.2 MHz > 1h: 26 MHz > 2h: 38.4 MHz > 3h: Obsolete > Others: Reserved > > Cc: Wu Hao A > > Cc: Albecki Mateusz > > Cc: Liming Gao > > Cc: Zhiguang Liu > > > Signed-off-by: Purna Chandra Rao Bandaru > > > --- > .../Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 50 ++++++++++++++++++- > .../Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 10 +--- > .../Protocol/UfsHostControllerPlatform.h | 15 +++++- > 3 files changed, 63 insertions(+), 12 deletions(-) > > diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c > b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c > index 4c2d6ae27f..638b14b0c1 100644 > --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c > +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c > @@ -1,6 +1,6 @@ > /** @file > > - Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.
> + Copyright (c) 2014 - 2022, Intel Corporation. All rights > + reserved.
> Copyright (c) Microsoft Corporation.
> SPDX-License-Identifier: BSD-2-Clause-Patent > > @@ -843,6 +843,8 @@ UfsPassThruDriverBindingStart ( > UFS_DEV_DESC DeviceDescriptor; > UINT32 UnitDescriptorSize; > UINT32 DeviceDescriptorSize; > + EDKII_UFS_CARD_REF_CLK_FREQ_ATTRIBUTE Attributes; > + UINT8 RefClkAttr; Please help to keep 2 spaces between the data type and the variable name and keep all other variables’ indent aligned. EFI_STATUS Status; EDKII_UFS_HOST_CONTROLLER_PROTOCOL *UfsHc; UFS_PASS_THRU_PRIVATE_DATA *Private; UINTN UfsHcBase; UINT32 Index; UFS_UNIT_DESC UnitDescriptor; UFS_DEV_DESC DeviceDescriptor; UINT32 UnitDescriptorSize; UINT32 DeviceDescriptorSize; EDKII_UFS_CARD_REF_CLK_FREQ_ATTRIBUTE Attributes; UINT8 RefClkAttr; > > Status = EFI_SUCCESS; > UfsHc = NULL; > @@ -916,6 +918,52 @@ UfsPassThruDriverBindingStart ( > DEBUG ((DEBUG_ERROR, "Ufs Host Controller Initialization Error, Status > = %r\n", Status)); > goto Error; > } > + if ((mUfsHcPlatform != NULL) && > + ((mUfsHcPlatform->RefClkFreq == EdkiiUfsCardRefClkFreq19p2Mhz) || > + (mUfsHcPlatform->RefClkFreq == EdkiiUfsCardRefClkFreq26Mhz) || > + (mUfsHcPlatform->RefClkFreq == EdkiiUfsCardRefClkFreq38p4Mhz))) { > + RefClkAttr = UfsAttrRefClkFreq; > + Attributes = EdkiiUfsCardRefClkFreqObsolete; > + Status = UfsRwAttributes (Private, TRUE, RefClkAttr, 0, 0, (UINT32 *) > &Attributes); > + if (!EFI_ERROR (Status)) { > + if (Attributes != mUfsHcPlatform->RefClkFreq) { > + Attributes = mUfsHcPlatform->RefClkFreq; > + DEBUG ( > + (DEBUG_INFO, > + "Setting bRefClkFreq attribute(%x) to %x\n 0 -> 19.2 Mhz\n 1 -> 26 > Mhz\n 2 -> 38.4 Mhz\n 3 -> Obsolete\n", > + RefClkAttr, > + Attributes) > + ); > + Status = UfsRwAttributes (Private, FALSE, RefClkAttr, 0, 0, (UINT32 *) > &Attributes); > + if (EFI_ERROR (Status)) { > + DEBUG ( > + (DEBUG_ERROR, > + "Failed to Change Reference Clock Attribute to %d, Status = %r \n", > + mUfsHcPlatform->RefClkFreq, > + Status) > + ); > + } > + } > + } else { > + DEBUG ( > + (DEBUG_ERROR, > + "Failed to Read Reference Clock Attribute, Status = %r \n", > + Status) > + ); > + } > + } > + > + if ((mUfsHcPlatform != NULL) && (mUfsHcPlatform->Callback != NULL)) { > + Status = mUfsHcPlatform->Callback (Private->Handle, > EdkiiUfsHcPostLinkStartup, &Private->UfsHcDriverInterface); > + if (EFI_ERROR (Status)) { > + DEBUG ( > + (DEBUG_ERROR, > + "Failure from platform driver during EdkiiUfsHcPostLinkStartup, Status > = %r\n", > + Status) > + ); > + return Status; > + } > + } Please help to update the above new codes to: Status = UfsControllerInit (Private); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_ERROR, "Ufs Host Controller Initialization Error, Status = %r\n", Status)); goto Error; } if ((mUfsHcPlatform != NULL) && ((mUfsHcPlatform->RefClkFreq == EdkiiUfsCardRefClkFreq19p2Mhz) || (mUfsHcPlatform->RefClkFreq == EdkiiUfsCardRefClkFreq26Mhz) || (mUfsHcPlatform->RefClkFreq == EdkiiUfsCardRefClkFreq38p4Mhz))) { RefClkAttr = UfsAttrRefClkFreq; Attributes = EdkiiUfsCardRefClkFreqObsolete; Status = UfsRwAttributes (Private, TRUE, RefClkAttr, 0, 0, (UINT32 *)&Attributes); if (!EFI_ERROR (Status)) { if (Attributes != mUfsHcPlatform->RefClkFreq) { Attributes = mUfsHcPlatform->RefClkFreq; DEBUG ( (DEBUG_INFO, "Setting bRefClkFreq attribute(%x) to %x\n 0 -> 19.2 Mhz\n 1 -> 26 Mhz\n 2 -> 38.4 Mhz\n 3 -> Obsolete\n", RefClkAttr, Attributes) ); Status = UfsRwAttributes (Private, FALSE, RefClkAttr, 0, 0, (UINT32 *)&Attributes); if (EFI_ERROR (Status)) { DEBUG ( (DEBUG_ERROR, "Failed to Change Reference Clock Attribute to %d, Status = %r \n", mUfsHcPlatform->RefClkFreq, Status) ); } } } else { DEBUG ( (DEBUG_ERROR, "Failed to Read Reference Clock Attribute, Status = %r \n", Status) ); } } if ((mUfsHcPlatform != NULL) && (mUfsHcPlatform->Callback != NULL)) { Status = mUfsHcPlatform->Callback (Private->Handle, EdkiiUfsHcPostLinkStartup, &Private->UfsHcDriverInterface); if (EFI_ERROR (Status)) { DEBUG ( (DEBUG_ERROR, "Failure from platform driver during EdkiiUfsHcPostLinkStartup, Status = %r\n", Status) ); return Status; } } Best Regards, Hao Wu > > // > // UFS 2.0 spec Section 13.1.3.3: > diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c > b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c > index eba35cc669..4a9fa01e7d 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. > > - Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.
> + Copyright (c) 2014 - 2022, Intel Corporation. All rights > + reserved.
> Copyright (c) Microsoft Corporation.
> SPDX-License-Identifier: BSD-2-Clause-Patent > > @@ -1970,14 +1970,6 @@ UfsDeviceDetection ( > return EFI_DEVICE_ERROR; > } > } else { > - if ((mUfsHcPlatform != NULL) && (mUfsHcPlatform->Callback != NULL)) { > - Status = mUfsHcPlatform->Callback (Private->Handle, > EdkiiUfsHcPostLinkStartup, &Private->UfsHcDriverInterface); > - if (EFI_ERROR (Status)) { > - DEBUG ((DEBUG_ERROR, "Failure from platform driver during > EdkiiUfsHcPostLinkStartup, Status = %r\n", Status)); > - return Status; > - } > - } > - > return EFI_SUCCESS; > } > } > diff --git a/MdeModulePkg/Include/Protocol/UfsHostControllerPlatform.h > b/MdeModulePkg/Include/Protocol/UfsHostControllerPlatform.h > index faa82d0c4e..32e9f6488c 100644 > --- a/MdeModulePkg/Include/Protocol/UfsHostControllerPlatform.h > +++ b/MdeModulePkg/Include/Protocol/UfsHostControllerPlatform.h > @@ -1,7 +1,7 @@ > /** @file > EDKII_UFS_HC_PLATFORM_PROTOCOL definition. > > -Copyright (c) 2019, Intel Corporation. All rights reserved.
> +Copyright (c) 2019 - 2022, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -11,7 +11,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #include > > -#define EDKII_UFS_HC_PLATFORM_PROTOCOL_VERSION 1 > +#define EDKII_UFS_HC_PLATFORM_PROTOCOL_VERSION 2 > > extern EFI_GUID gEdkiiUfsHcPlatformProtocolGuid; > > @@ -83,6 +83,13 @@ typedef enum { > EdkiiUfsHcPostLinkStartup > } EDKII_UFS_HC_PLATFORM_CALLBACK_PHASE; > > +typedef enum { > + EdkiiUfsCardRefClkFreq19p2Mhz, > + EdkiiUfsCardRefClkFreq26Mhz, > + EdkiiUfsCardRefClkFreq38p4Mhz, > + EdkiiUfsCardRefClkFreqObsolete > +} EDKII_UFS_CARD_REF_CLK_FREQ_ATTRIBUTE; > + > /** > Callback function for platform driver. > > @@ -118,6 +125,10 @@ struct _EDKII_UFS_HC_PLATFORM_PROTOCOL { > /// for host controller. > /// > EDKII_UFS_HC_PLATFORM_CALLBACK Callback; > + /// > + /// Reference Clock Frequency Ufs Card Attribute that need to be set in > this Ufs Host Environment. > + /// > + EDKII_UFS_CARD_REF_CLK_FREQ_ATTRIBUTE RefClkFreq; > }; > > #endif > -- > 2.31.1.windows.1