public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wu, Hao A" <hao.a.wu@intel.com>
To: "Bandaru,
	Purna Chandra Rao" <purna.chandra.rao.bandaru@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Albecki, Mateusz" <mateusz.albecki@intel.com>,
	"Gao, Liming" <gaoliming@byosoft.com.cn>,
	"Liu, Zhiguang" <zhiguang.liu@intel.com>
Subject: Re: [PATCH] [PATCH v2] MdeModulePkg: Add bRefClkFreq card attribute programming support
Date: Tue, 8 Mar 2022 05:25:19 +0000	[thread overview]
Message-ID: <DM6PR11MB402593D8691229DDF024C481CA099@DM6PR11MB4025.namprd11.prod.outlook.com> (raw)
In-Reply-To: <1828e4b1d46a6ae53d275d374707909deba007d6.1646653492.git.purna.chandra.rao.bandaru@intel.com>

[-- Attachment #1: Type: text/plain, Size: 11238 bytes --]

Minor inline comments below on coding style from the uncrustify tool:





> -----Original Message-----

> From: Bandaru, Purna Chandra Rao <purna.chandra.rao.bandaru@intel.com>

> Sent: Monday, March 7, 2022 7:47 PM

> To: devel@edk2.groups.io

> Cc: Bandaru, Purna Chandra Rao <purna.chandra.rao.bandaru@intel.com>;

> Wu, Hao A <hao.a.wu@intel.com>; Albecki, Mateusz

> <mateusz.albecki@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>;

> Liu, Zhiguang <zhiguang.liu@intel.com>

> 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 <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>

> Cc: Albecki Mateusz <mateusz.albecki@intel.com<mailto:mateusz.albecki@intel.com>>

> Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>

> Cc: Zhiguang Liu <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>

>

> Signed-off-by: Purna Chandra Rao Bandaru

> <purna.chandra.rao.bandaru@intel.com<mailto:purna.chandra.rao.bandaru@intel.com>>

> ---

>  .../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.<BR>

> +  Copyright (c) 2014 - 2022, Intel Corporation. All rights

> + reserved.<BR>

>    Copyright (c) Microsoft Corporation.<BR>

>    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.<BR>

> +  Copyright (c) 2014 - 2022, Intel Corporation. All rights

> + reserved.<BR>

>    Copyright (c) Microsoft Corporation.<BR>

>    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.<BR>

> +Copyright (c) 2019 - 2022, Intel Corporation. All rights reserved.<BR>

>  SPDX-License-Identifier: BSD-2-Clause-Patent

>

>  **/

> @@ -11,7 +11,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent

>

>  #include <Protocol/UfsHostController.h>

>

> -#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



[-- Attachment #2: Type: text/html, Size: 34607 bytes --]

      reply	other threads:[~2022-03-08  5:25 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-07 11:47 [PATCH] [PATCH v2] MdeModulePkg: Add bRefClkFreq card attribute programming support Bandaru, Purna Chandra Rao
2022-03-08  5:25 ` Wu, Hao A [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR11MB402593D8691229DDF024C481CA099@DM6PR11MB4025.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox