[edk2,v2,5/5] MdeModulePkg/UfsBlockIoPei: fix initialize OCS value to 0x0F

Message ID 1484569378-16126-6-git-send-email-haojian.zhuang@linaro.org
State New
Headers show
Series
  • fix UFS driver
Related show

Commit Message

Haojian Zhuang Jan. 16, 2017, 12:22 p.m.
The OCS value should be initiliazed as 0x0F according to UFS spec.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

---
 MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Jan. 16, 2017, 4:02 p.m. | #1
On Mon, Jan 16, 2017 at 08:22:58PM +0800, Haojian Zhuang wrote:
> The OCS value should be initiliazed as 0x0F according to UFS spec.

> 

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

> ---

>  MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 2 ++

>  1 file changed, 2 insertions(+)

> 

> diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c

> index cccacce..67042b7 100644

> --- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c

> +++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c

> @@ -480,6 +480,7 @@ UfsCreateScsiCommandDesc (

>    Trd->Int    = UFS_INTERRUPT_COMMAND;

>    Trd->Dd     = DataDirection;

>    Trd->Ct     = UFS_STORAGE_COMMAND_TYPE;

> +  Trd->Ocs    = 0x0F;


Could these be given some #define in UfsHci.h rather than inline magic
numbers?

Regards,

Leif

>    Trd->UcdBa  = (UINT32)RShiftU64 ((UINT64)(UINTN)CommandUpiu, 7);

>    Trd->UcdBaU = (UINT32)RShiftU64 ((UINT64)(UINTN)CommandUpiu, 32);

>    Trd->RuL    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_RESPONSE_UPIU)), sizeof (UINT32));

> @@ -637,6 +638,7 @@ UfsCreateNopCommandDesc (

>    Trd->Int    = UFS_INTERRUPT_COMMAND;

>    Trd->Dd     = 0x00;

>    Trd->Ct     = UFS_STORAGE_COMMAND_TYPE;

> +  Trd->Ocs    = 0x0F;

>    Trd->UcdBa  = (UINT32)RShiftU64 ((UINT64)(UINTN)NopOutUpiu, 7);

>    Trd->UcdBaU = (UINT32)RShiftU64 ((UINT64)(UINTN)NopOutUpiu, 32);

>    Trd->RuL    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_NOP_IN_UPIU)), sizeof (UINT32));

> -- 

> 2.7.4

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Tian, Feng Jan. 17, 2017, 2:06 a.m. | #2
Agree.

How about naming it as UFS_TRD_OCS_INIT_VALUE?

If there is no objection, I will help make this change and push these patches into EDKII trunk.

Series reviewed-by: Feng Tian <feng.tian@intel.com>

Thanks
Feng

-----Original Message-----
From: Leif Lindholm [mailto:leif.lindholm@linaro.org] 

Sent: Tuesday, January 17, 2017 12:03 AM
To: Haojian Zhuang <haojian.zhuang@linaro.org>
Cc: Tian, Feng <feng.tian@intel.com>; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
Subject: Re: [PATCH v2 5/5] MdeModulePkg/UfsBlockIoPei: fix initialize OCS value to 0x0F

On Mon, Jan 16, 2017 at 08:22:58PM +0800, Haojian Zhuang wrote:
> The OCS value should be initiliazed as 0x0F according to UFS spec.

> 

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

> ---

>  MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 2 ++

>  1 file changed, 2 insertions(+)

> 

> diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c 

> b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c

> index cccacce..67042b7 100644

> --- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c

> +++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c

> @@ -480,6 +480,7 @@ UfsCreateScsiCommandDesc (

>    Trd->Int    = UFS_INTERRUPT_COMMAND;

>    Trd->Dd     = DataDirection;

>    Trd->Ct     = UFS_STORAGE_COMMAND_TYPE;

> +  Trd->Ocs    = 0x0F;


Could these be given some #define in UfsHci.h rather than inline magic numbers?

Regards,

Leif

>    Trd->UcdBa  = (UINT32)RShiftU64 ((UINT64)(UINTN)CommandUpiu, 7);

>    Trd->UcdBaU = (UINT32)RShiftU64 ((UINT64)(UINTN)CommandUpiu, 32);

>    Trd->RuL    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_RESPONSE_UPIU)), sizeof (UINT32));

> @@ -637,6 +638,7 @@ UfsCreateNopCommandDesc (

>    Trd->Int    = UFS_INTERRUPT_COMMAND;

>    Trd->Dd     = 0x00;

>    Trd->Ct     = UFS_STORAGE_COMMAND_TYPE;

> +  Trd->Ocs    = 0x0F;

>    Trd->UcdBa  = (UINT32)RShiftU64 ((UINT64)(UINTN)NopOutUpiu, 7);

>    Trd->UcdBaU = (UINT32)RShiftU64 ((UINT64)(UINTN)NopOutUpiu, 32);

>    Trd->RuL    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_NOP_IN_UPIU)), sizeof (UINT32));

> --

> 2.7.4

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Jan. 17, 2017, 10:56 a.m. | #3
On Tue, Jan 17, 2017 at 02:06:51AM +0000, Tian, Feng wrote:
> Agree.

> 

> How about naming it as UFS_TRD_OCS_INIT_VALUE?

> 

> If there is no objection, I will help make this change and push

> these patches into EDKII trunk.


That sounds excellent to me, thanks!

/
    Leif

> Series reviewed-by: Feng Tian <feng.tian@intel.com>

> 

> Thanks

> Feng

> 

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

> From: Leif Lindholm [mailto:leif.lindholm@linaro.org] 

> Sent: Tuesday, January 17, 2017 12:03 AM

> To: Haojian Zhuang <haojian.zhuang@linaro.org>

> Cc: Tian, Feng <feng.tian@intel.com>; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org

> Subject: Re: [PATCH v2 5/5] MdeModulePkg/UfsBlockIoPei: fix initialize OCS value to 0x0F

> 

> On Mon, Jan 16, 2017 at 08:22:58PM +0800, Haojian Zhuang wrote:

> > The OCS value should be initiliazed as 0x0F according to UFS spec.

> > 

> > Contributed-under: TianoCore Contribution Agreement 1.0

> > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

> > ---

> >  MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 2 ++

> >  1 file changed, 2 insertions(+)

> > 

> > diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c 

> > b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c

> > index cccacce..67042b7 100644

> > --- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c

> > +++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c

> > @@ -480,6 +480,7 @@ UfsCreateScsiCommandDesc (

> >    Trd->Int    = UFS_INTERRUPT_COMMAND;

> >    Trd->Dd     = DataDirection;

> >    Trd->Ct     = UFS_STORAGE_COMMAND_TYPE;

> > +  Trd->Ocs    = 0x0F;

> 

> Could these be given some #define in UfsHci.h rather than inline magic numbers?

> 

> Regards,

> 

> Leif

> 

> >    Trd->UcdBa  = (UINT32)RShiftU64 ((UINT64)(UINTN)CommandUpiu, 7);

> >    Trd->UcdBaU = (UINT32)RShiftU64 ((UINT64)(UINTN)CommandUpiu, 32);

> >    Trd->RuL    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_RESPONSE_UPIU)), sizeof (UINT32));

> > @@ -637,6 +638,7 @@ UfsCreateNopCommandDesc (

> >    Trd->Int    = UFS_INTERRUPT_COMMAND;

> >    Trd->Dd     = 0x00;

> >    Trd->Ct     = UFS_STORAGE_COMMAND_TYPE;

> > +  Trd->Ocs    = 0x0F;

> >    Trd->UcdBa  = (UINT32)RShiftU64 ((UINT64)(UINTN)NopOutUpiu, 7);

> >    Trd->UcdBaU = (UINT32)RShiftU64 ((UINT64)(UINTN)NopOutUpiu, 32);

> >    Trd->RuL    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_NOP_IN_UPIU)), sizeof (UINT32));

> > --

> > 2.7.4

> > 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch hide | download patch | download mbox

diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index cccacce..67042b7 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -480,6 +480,7 @@  UfsCreateScsiCommandDesc (
   Trd->Int    = UFS_INTERRUPT_COMMAND;
   Trd->Dd     = DataDirection;
   Trd->Ct     = UFS_STORAGE_COMMAND_TYPE;
+  Trd->Ocs    = 0x0F;
   Trd->UcdBa  = (UINT32)RShiftU64 ((UINT64)(UINTN)CommandUpiu, 7);
   Trd->UcdBaU = (UINT32)RShiftU64 ((UINT64)(UINTN)CommandUpiu, 32);
   Trd->RuL    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_RESPONSE_UPIU)), sizeof (UINT32));
@@ -637,6 +638,7 @@  UfsCreateNopCommandDesc (
   Trd->Int    = UFS_INTERRUPT_COMMAND;
   Trd->Dd     = 0x00;
   Trd->Ct     = UFS_STORAGE_COMMAND_TYPE;
+  Trd->Ocs    = 0x0F;
   Trd->UcdBa  = (UINT32)RShiftU64 ((UINT64)(UINTN)NopOutUpiu, 7);
   Trd->UcdBaU = (UINT32)RShiftU64 ((UINT64)(UINTN)NopOutUpiu, 32);
   Trd->RuL    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_NOP_IN_UPIU)), sizeof (UINT32));