Re: [edk2] [Patch] UefiCpuPkg/PiSmmCpuDxeSmm: Add check to void use null pointer.

2017-10-08 Thread Dong, Eric 
Hao,

Agree with your suggestion, I will remove the if code when I merge the change.

Thanks,
Eric

-Original Message-
From: Wu, Hao A 
Sent: Monday, October 9, 2017 11:39 AM
To: Dong, Eric ; edk2-devel@lists.01.org
Cc: Ni, Ruiyu 
Subject: RE: [edk2] [Patch] UefiCpuPkg/PiSmmCpuDxeSmm: Add check to void use 
null pointer.

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Eric Dong
> Sent: Monday, October 09, 2017 11:18 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu
> Subject: [edk2] [Patch] UefiCpuPkg/PiSmmCpuDxeSmm: Add check to void 
> use null pointer.
> 
> Current code logic not check the pointer before use it. This may has 
> potential issue, this patch add code to check it.
> 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> index ef72b9b..2c1dc82 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> @@ -226,12 +226,17 @@ SetProcessorRegister (
>CPU_REGISTER_TABLE*RegisterTable;
> 
>InitApicId = GetInitialApicId ();
> +  RegisterTable = NULL;
>for (Index = 0; Index < RegisterTableCount; Index++) {
>  if (RegisterTables[Index].InitialApicId == InitApicId) {
>RegisterTable =  [Index];
>break;
>  }
>}
> +  ASSERT (RegisterTable != NULL);
> +  if (RegisterTable == NULL) {
> +return;
> +  }

Hi Eric,

If "RegisterTable == NULL" is a case that should never occur, my thought is 
that using "ASSERT" merely is enough. The 'if' statement above seems can be 
removed for me.


Best Regards,
Hao Wu

> 
>//
>// Traverse Register Table of this logical processor
> --
> 2.7.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] UefiCpuPkg/PiSmmCpuDxeSmm: Add check to void use null pointer.

2017-10-08 Thread Wu, Hao A 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Eric
> Dong
> Sent: Monday, October 09, 2017 11:18 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu
> Subject: [edk2] [Patch] UefiCpuPkg/PiSmmCpuDxeSmm: Add check to void use
> null pointer.
> 
> Current code logic not check the pointer before use it. This may
> has potential issue, this patch add code to check it.
> 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> index ef72b9b..2c1dc82 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> @@ -226,12 +226,17 @@ SetProcessorRegister (
>CPU_REGISTER_TABLE*RegisterTable;
> 
>InitApicId = GetInitialApicId ();
> +  RegisterTable = NULL;
>for (Index = 0; Index < RegisterTableCount; Index++) {
>  if (RegisterTables[Index].InitialApicId == InitApicId) {
>RegisterTable =  [Index];
>break;
>  }
>}
> +  ASSERT (RegisterTable != NULL);
> +  if (RegisterTable == NULL) {
> +return;
> +  }

Hi Eric,

If "RegisterTable == NULL" is a case that should never occur, my thought
is that using "ASSERT" merely is enough. The 'if' statement above seems
can be removed for me.


Best Regards,
Hao Wu

> 
>//
>// Traverse Register Table of this logical processor
> --
> 2.7.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] UefiCpuPkg/PiSmmCpuDxeSmm: Add check to void use null pointer.

2017-10-08 Thread Ni, Ruiyu 
Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Eric
> Dong
> Sent: Monday, October 9, 2017 11:18 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu 
> Subject: [edk2] [Patch] UefiCpuPkg/PiSmmCpuDxeSmm: Add check to void use
> null pointer.
> 
> Current code logic not check the pointer before use it. This may has potential
> issue, this patch add code to check it.
> 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> index ef72b9b..2c1dc82 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> @@ -226,12 +226,17 @@ SetProcessorRegister (
>CPU_REGISTER_TABLE*RegisterTable;
> 
>InitApicId = GetInitialApicId ();
> +  RegisterTable = NULL;
>for (Index = 0; Index < RegisterTableCount; Index++) {
>  if (RegisterTables[Index].InitialApicId == InitApicId) {
>RegisterTable =  [Index];
>break;
>  }
>}
> +  ASSERT (RegisterTable != NULL);
> +  if (RegisterTable == NULL) {
> +return;
> +  }
> 
>//
>// Traverse Register Table of this logical processor
> --
> 2.7.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3] Ifconfig : Fixed False information about Media State.

2017-10-08 Thread Wu, Jiaxin 
For me, the hard-code string looks good to me here. I'm also fine if you stick 
to move in UNI file.

Thanks,
Jiaxin 


> -Original Message-
> From: Ni, Ruiyu
> Sent: Monday, October 9, 2017 10:09 AM
> To: Wu, Jiaxin ; Meenakshi Aggarwal
> ; Carsey, Jaben ;
> edk2-devel@lists.01.org
> Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media State.
> 
> Do you need to put all the hard-code string in UNI file for localization?
> 
> Thanks/Ray
> 
> > -Original Message-
> > From: Wu, Jiaxin
> > Sent: Monday, October 9, 2017 9:29 AM
> > To: Meenakshi Aggarwal ; Carsey, Jaben
> > ; edk2-devel@lists.01.org; Ni, Ruiyu
> > 
> > Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media
> State.
> >
> > I agree with Jaben. If NetLibDetectMedia return error status, we can
> output as
> > below:
> >
> >   ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> state
> > unknown");
> >
> > Thanks,
> > Jiaxin
> >
> > > -Original Message-
> > > From: Meenakshi Aggarwal [mailto:meenakshi.aggar...@nxp.com]
> > > Sent: Sunday, October 8, 2017 4:49 PM
> > > To: Carsey, Jaben ; edk2-devel@lists.01.org;
> > > Wu, Jiaxin ; Ni, Ruiyu 
> > > Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media
> State.
> > >
> > > It is hard to say when can an API fail because its dependent on
> > > implementation.
> > >
> > >
> > > > -Original Message-
> > > > From: Carsey, Jaben [mailto:jaben.car...@intel.com]
> > > > Sent: Friday, October 06, 2017 7:32 PM
> > > > To: Meenakshi Aggarwal ; edk2-
> > > > de...@lists.01.org; Wu, Jiaxin ; Ni, Ruiyu
> > > > 
> > > > Subject: RE: [PATCH v3] Ifconfig : Fixed False information about
> > > > Media
> > > State.
> > > >
> > > > Reviewed-by: Jaben Carsey  Do you know
> under
> > > > what conditions the API will fail? Is it worth saying something like
> > > > media
> > > stats
> > > > unknown when the function fails?
> > > >
> > > > Ray,
> > > >
> > > > What do you think?
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Meenakshi Aggarwal [mailto:meenakshi.aggar...@nxp.com]
> > > > > Sent: Thursday, October 05, 2017 9:48 PM
> > > > > To: edk2-devel@lists.01.org; Wu, Jiaxin ;
> > > > > Carsey, Jaben ; Ni, Ruiyu
> > > > > 
> > > > > Cc: Meenakshi Aggarwal 
> > > > > Subject: [PATCH v3] Ifconfig : Fixed False information about Media
> State.
> > > > > Importance: High
> > > > >
> > > > > Issue : We were setting MediaPresent as TRUE (default) and not
> > > > > checking return status of NetLibDetectMedia().
> > > > > NetLibDetectMedia() sets MediaPresent FLAG in case of success only
> > > > > and dont change flag on error.
> > > > > So, Media State will display as 'Media Present', in case of error
> > > > > also.
> > > > >
> > > > > Fix : Check return value of NetLibDetectMedia()
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > >
> > > > > Signed-off-by: Meenakshi Aggarwal 
> > > > > ---
> > > > >  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 11
> > > > > +++--
> > > > > --
> > > > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git
> > > > > a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > > b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > > index 4db07b2..90ca724 100644
> > > > > --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > > +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > > @@ -576,11 +576,14 @@ IfConfigShowInterfaceInfo (
> > > > >  //
> > > > >  // Get Media State.
> > > > >  //
> > > > > -NetLibDetectMedia (IfCb->NicHandle, );
> > > > > -if (!MediaPresent) {
> > > > > -  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > > L"Media
> > > > > disconnected");
> > > > > +if (EFI_SUCCESS == NetLibDetectMedia (IfCb->NicHandle,
> > > > > )) {
> > > > > +  if (!MediaPresent) {
> > > > > +ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > > L"Media
> > > > > disconnected");
> > > > > +  } else {
> > > > > +ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > > L"Media
> > > > > present");
> > > > > +  }
> > > > >  } else {
> > > > > -  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > > (STR_IFCONFIG_INFO_MEDIA_STATE),

[edk2] [Patch] UefiCpuPkg/PiSmmCpuDxeSmm: Add check to void use null pointer.

2017-10-08 Thread Eric Dong 
Current code logic not check the pointer before use it. This may
has potential issue, this patch add code to check it.

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index ef72b9b..2c1dc82 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -226,12 +226,17 @@ SetProcessorRegister (
   CPU_REGISTER_TABLE*RegisterTable;
 
   InitApicId = GetInitialApicId ();
+  RegisterTable = NULL;
   for (Index = 0; Index < RegisterTableCount; Index++) {
 if (RegisterTables[Index].InitialApicId == InitApicId) {
   RegisterTable =  [Index];
   break;
 }
   }
+  ASSERT (RegisterTable != NULL);
+  if (RegisterTable == NULL) {
+return;
+  }
 
   //
   // Traverse Register Table of this logical processor
-- 
2.7.0.windows.1

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

Re: [edk2] [PATCH] MdeModulePkg/PciBus: Count multiple hotplug resource paddings

2017-10-08 Thread Zeng, Star 
Reviewed-by: Star Zeng 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Saturday, September 30, 2017 1:11 PM
To: edk2-devel@lists.01.org
Cc: Laszlo Ersek 
Subject: [edk2] [PATCH] MdeModulePkg/PciBus: Count multiple hotplug resource 
paddings

The current implementation assumes there is only one hotplug resource padding 
for each resource type. It's not true considering
DegradeResource(): MEM64 resource could be degraded to MEM32 resource.

The patch treat the resource paddings using the same logic as treating 
typical/actual resources and the total resource of a bridge is set to the MAX 
of typical/actual resources and resource paddings.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Laszlo Ersek 
---
 .../Bus/Pci/PciBusDxe/PciResourceSupport.c | 67 +++---
 1 file changed, 21 insertions(+), 46 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
index e93134613b..f086b1732d 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
@@ -343,14 +343,9 @@ CalculateResourceAperture (
   IN PCI_RESOURCE_NODE*Bridge
   )
 {
-  UINT64Aperture;
+  UINT64Aperture[2];
   LIST_ENTRY*CurrentLink;
   PCI_RESOURCE_NODE *Node;
-  UINT64PaddingAperture;
-  UINT64Offset;
-
-  Aperture= 0;
-  PaddingAperture = 0;
 
   if (Bridge == NULL) {
 return ;
@@ -362,6 +357,8 @@ CalculateResourceAperture (
 return ;
   }
 
+  Aperture[PciResUsageTypical] = 0;
+  Aperture[PciResUsagePadding] = 0;
   //
   // Assume the bridge is aligned
   //
@@ -369,58 +366,30 @@ CalculateResourceAperture (
   ; !IsNull (>ChildList, CurrentLink)
   ; CurrentLink = GetNextNode (>ChildList, CurrentLink)
   ) {
-
 Node = RESOURCE_NODE_FROM_LINK (CurrentLink);
-if (Node->ResourceUsage == PciResUsagePadding) {
-  ASSERT (PaddingAperture == 0);
-  PaddingAperture = Node->Length;
-  continue;
-}
 
 //
-// Apply padding resource if available
+// It's possible for a bridge to contain multiple padding resource
+// nodes due to DegradeResource().
 //
-Offset = Aperture & (Node->Alignment);
-
-if (Offset != 0) {
-
-  Aperture = Aperture + (Node->Alignment + 1) - Offset;
-
-}
-
+ASSERT ((Node->ResourceUsage == PciResUsageTypical) ||
+(Node->ResourceUsage == PciResUsagePadding));
+ASSERT (Node->ResourceUsage < ARRAY_SIZE (Aperture));
 //
 // Recode current aperture as a offset
-// this offset will be used in future real allocation
+// Apply padding resource to meet alignment requirement
+// Node offset will be used in future real allocation
 //
-Node->Offset = Aperture;
+Node->Offset = ALIGN_VALUE (Aperture[Node->ResourceUsage], 
+ Node->Alignment + 1);
 
 //
-// Increment aperture by the length of node
+// Record the total aperture.
 //
-Aperture += Node->Length;
-  }
-
-  //
-  // At last, adjust the aperture with the bridge's
-  // alignment
-  //
-  Offset = Aperture & (Bridge->Alignment);
-  if (Offset != 0) {
-Aperture = Aperture + (Bridge->Alignment + 1) - Offset;
+Aperture[Node->ResourceUsage] = Node->Offset + Node->Length;
   }
 
   //
-  // If the bridge has already padded the resource and the
-  // amount of padded resource is larger, then keep the
-  // padded resource
-  //
-  if (Bridge->Length < Aperture) {
-Bridge->Length = Aperture;
-  }
-
-  //
-  // Adjust the bridge's alignment to the first child's alignment
-  // if the bridge has at least one child
+  // Adjust the bridge's alignment to the MAX (first) alignment of all 
children.
   //
   CurrentLink = Bridge->ChildList.ForwardLink;
   if (CurrentLink != >ChildList) { @@ -431,10 +400,16 @@ 
CalculateResourceAperture (
   }
 
   //
+  // At last, adjust the aperture with the bridge's alignment  //  
+ Aperture[PciResUsageTypical] = ALIGN_VALUE 
+ (Aperture[PciResUsageTypical], Bridge->Alignment + 1);  
+ Aperture[PciResUsagePadding] = ALIGN_VALUE 
+ (Aperture[PciResUsagePadding], Bridge->Alignment + 1);
+
+  //
   // Hotplug controller needs padding resources.
   // Use the larger one between the padding resource and actual occupied 
resource.
   //
-  Bridge->Length = MAX (Bridge->Length, PaddingAperture);
+  Bridge->Length = MAX (Aperture[PciResUsageTypical], 
+ Aperture[PciResUsagePadding]);
 }
 
 /**
--
2.12.2.windows.2

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

Re: [edk2] [PATCH v3] Ifconfig : Fixed False information about Media State.

2017-10-08 Thread Ni, Ruiyu 
Do you need to put all the hard-code string in UNI file for localization?

Thanks/Ray

> -Original Message-
> From: Wu, Jiaxin
> Sent: Monday, October 9, 2017 9:29 AM
> To: Meenakshi Aggarwal ; Carsey, Jaben
> ; edk2-devel@lists.01.org; Ni, Ruiyu
> 
> Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media State.
> 
> I agree with Jaben. If NetLibDetectMedia return error status, we can output as
> below:
> 
>   ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media state
> unknown");
> 
> Thanks,
> Jiaxin
> 
> > -Original Message-
> > From: Meenakshi Aggarwal [mailto:meenakshi.aggar...@nxp.com]
> > Sent: Sunday, October 8, 2017 4:49 PM
> > To: Carsey, Jaben ; edk2-devel@lists.01.org;
> > Wu, Jiaxin ; Ni, Ruiyu 
> > Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media 
> > State.
> >
> > It is hard to say when can an API fail because its dependent on
> > implementation.
> >
> >
> > > -Original Message-
> > > From: Carsey, Jaben [mailto:jaben.car...@intel.com]
> > > Sent: Friday, October 06, 2017 7:32 PM
> > > To: Meenakshi Aggarwal ; edk2-
> > > de...@lists.01.org; Wu, Jiaxin ; Ni, Ruiyu
> > > 
> > > Subject: RE: [PATCH v3] Ifconfig : Fixed False information about
> > > Media
> > State.
> > >
> > > Reviewed-by: Jaben Carsey  Do you know under
> > > what conditions the API will fail? Is it worth saying something like
> > > media
> > stats
> > > unknown when the function fails?
> > >
> > > Ray,
> > >
> > > What do you think?
> > >
> > >
> > > > -Original Message-
> > > > From: Meenakshi Aggarwal [mailto:meenakshi.aggar...@nxp.com]
> > > > Sent: Thursday, October 05, 2017 9:48 PM
> > > > To: edk2-devel@lists.01.org; Wu, Jiaxin ;
> > > > Carsey, Jaben ; Ni, Ruiyu
> > > > 
> > > > Cc: Meenakshi Aggarwal 
> > > > Subject: [PATCH v3] Ifconfig : Fixed False information about Media 
> > > > State.
> > > > Importance: High
> > > >
> > > > Issue : We were setting MediaPresent as TRUE (default) and not
> > > > checking return status of NetLibDetectMedia().
> > > > NetLibDetectMedia() sets MediaPresent FLAG in case of success only
> > > > and dont change flag on error.
> > > > So, Media State will display as 'Media Present', in case of error
> > > > also.
> > > >
> > > > Fix : Check return value of NetLibDetectMedia()
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > >
> > > > Signed-off-by: Meenakshi Aggarwal 
> > > > ---
> > > >  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 11
> > > > +++--
> > > > --
> > > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git
> > > > a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > index 4db07b2..90ca724 100644
> > > > --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > @@ -576,11 +576,14 @@ IfConfigShowInterfaceInfo (
> > > >  //
> > > >  // Get Media State.
> > > >  //
> > > > -NetLibDetectMedia (IfCb->NicHandle, );
> > > > -if (!MediaPresent) {
> > > > -  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > L"Media
> > > > disconnected");
> > > > +if (EFI_SUCCESS == NetLibDetectMedia (IfCb->NicHandle,
> > > > )) {
> > > > +  if (!MediaPresent) {
> > > > +ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > L"Media
> > > > disconnected");
> > > > +  } else {
> > > > +ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > L"Media
> > > > present");
> > > > +  }
> > > >  } else {
> > > > -  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > L"Media
> > > > present");
> > > > +  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > L"Media
> > > > disconnected");
> > > >  }
> > > >
> > > >  //
> > > > --
> > > > 2.7.4

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

Re: [edk2] MTFTP file transfer timeout error

2017-10-08 Thread Fu, Siyuan 
Hi, Vabhav

Sorry for the late response, I just came back from the vacation.

The default retry count for tftp shell command is 6, which means the ACK packet 
will be retransmitted 6 times before the code goes to the 
Mtftp4CleanOperation() you marked below. The MTFTP driver always saves the last 
transmitted packet in Instance->LastPacket, and the Mtftp4Retransmit() will try 
to retransmit it if not reach the max retry count.

As you mentioned 1K block size is always success, I guess it's may because the 
IP fragment. A MTFTP(UDP) packet which is larger than 1.5K (the default MTU) 
will be fragmented to several IP frame. During the transmit, the whole UDP 
packet will be discarded by the IP layer if any of the IP fragment is lost in 
the network. As a result, using large MTFTP block size will increase the 
possibility of timeout in bad network connection.

I suggest you add some debug in Mtftp4RrqInput() in below lines to confirm if 
the EFI client can receive the MTFTP packet correctly, also you could use 
Wireshark in your server to check whether it receives the ACK from client.
switch (Opcode) {
case EFI_MTFTP4_OPCODE_DATA:
if ((Len > (UINT32) (MTFTP4_DATA_HEAD_LEN + Instance->BlkSize)) ||
 (Len < (UINT32) MTFTP4_DATA_HEAD_LEN)) {
goto ON_EXIT;
 }

 Status = Mtftp4RrqHandleData (Instance, Packet, Len, Multicast, 
);
 break;


BestRegards
Fu Siyuan


-Original Message-
From: Vabhav Sharma [mailto:vabhav.sha...@nxp.com] 
Sent: Friday, October 6, 2017 1:26 AM
To: Fu, Siyuan ; edk2-devel@lists.01.org
Subject: RE: MTFTP file transfer timeout error

Dear Experts,

I traced that timeout error for different block size during file transfer using 
tftp(rrq opcode)  is returned from function  Mtftp4OnTimerTick() TFTP layer in 
UEFI network stack.
Mtftp4OnTimerTick()
//
// Retransmit the packet if haven't reach the maxmium retry count,
// otherwise exit the transfer.
//
if (++Instance->CurRetry < Instance->MaxRetry) {
  Mtftp4Retransmit (Instance);
  Mtftp4SetTimeout (Instance);
} else {
  Mtftp4CleanOperation (Instance, EFI_TIMEOUT);//Timeout is set
  continue;
}

Once this is set, It gets populated to downloadfile() function in tftp shellpkg 
library.
There seems to be issue(corruption) with tfp client state machine 
-Not sending ACK for received data blocks
-Sending duplicate ACK

If server don't receive ACK, It stops sending data packets and timeout occurs 
after maximum retry.
Please suggest if this is new or known issue to be fixed?

Regards,
Vabhav

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Vabhav 
Sharma
Sent: Thursday, September 28, 2017 5:04 PM
To: siyuan...@intel.com
Cc: edk2-devel@lists.01.org; edk2-devel 
Subject: Re: [edk2] MTFTP file transfer timeout error

[This sender failed our fraud detection checks and may not be who they appear 
to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]

Hello Fu Siyuan,
I see that blocksize option with tftp command is introduced with commit 
2be45bfe2779043bc3566e879e7ec279412012dc.
Could you please help me clarify with the timeout error behavior observed in 
previous mail

Please note the behavior varies for different file type(Attached sheet) 
Regards, Vabhav

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Vabhav 
Sharma
Sent: Saturday, September 23, 2017 4:21 PM
To: edk2-devel@lists.01.org; edk2-devel 
Subject: [edk2] MTFTP file transfer timeout error

[This sender failed our fraud detection checks and may not be who they appear 
to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]

Hi All,
I am facing timeout error with file transfer using tftp on UEFI shell with ARM 
based SoCs Command used: tftp -s  -i   


File transfer with file size greater 50 or 60 MB is returning timeout(Also 
depends on type of file like data file, ASCII file, boot sector)

I verified by playing around with blocksize from 32K to 42K for different file 
size(100MB,200MB,500MB) and identify that increasing the block size for large 
file size helps with successful transfer.
File transfer is always successful with 1K blocksize but file transfer time is 
increased.

Please suggest if there is any link between block size with file size or anyone 
faced such issue? I assume expectation is to use any blocksize from 
512(default) to 64K.

Regards,
Vabhav
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org

Re: [edk2] [PATCH v3] Ifconfig : Fixed False information about Media State.

2017-10-08 Thread Wu, Jiaxin 
I agree with Jaben. If NetLibDetectMedia return error status, we can output as 
below:

  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media state 
unknown");

Thanks,
Jiaxin

> -Original Message-
> From: Meenakshi Aggarwal [mailto:meenakshi.aggar...@nxp.com]
> Sent: Sunday, October 8, 2017 4:49 PM
> To: Carsey, Jaben ; edk2-devel@lists.01.org; Wu,
> Jiaxin ; Ni, Ruiyu 
> Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media State.
> 
> It is hard to say when can an API fail because its dependent on
> implementation.
> 
> 
> > -Original Message-
> > From: Carsey, Jaben [mailto:jaben.car...@intel.com]
> > Sent: Friday, October 06, 2017 7:32 PM
> > To: Meenakshi Aggarwal ; edk2-
> > de...@lists.01.org; Wu, Jiaxin ; Ni, Ruiyu
> > 
> > Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media
> State.
> >
> > Reviewed-by: Jaben Carsey  Do you know under
> > what conditions the API will fail? Is it worth saying something like media
> stats
> > unknown when the function fails?
> >
> > Ray,
> >
> > What do you think?
> >
> >
> > > -Original Message-
> > > From: Meenakshi Aggarwal [mailto:meenakshi.aggar...@nxp.com]
> > > Sent: Thursday, October 05, 2017 9:48 PM
> > > To: edk2-devel@lists.01.org; Wu, Jiaxin ; Carsey,
> > > Jaben ; Ni, Ruiyu 
> > > Cc: Meenakshi Aggarwal 
> > > Subject: [PATCH v3] Ifconfig : Fixed False information about Media State.
> > > Importance: High
> > >
> > > Issue : We were setting MediaPresent as TRUE (default) and not
> > > checking return status of NetLibDetectMedia().
> > > NetLibDetectMedia() sets MediaPresent FLAG in case of success only and
> > > dont change flag on error.
> > > So, Media State will display as 'Media Present', in case of error
> > > also.
> > >
> > > Fix : Check return value of NetLibDetectMedia()
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > >
> > > Signed-off-by: Meenakshi Aggarwal 
> > > ---
> > >  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 11
> > > +++--
> > > --
> > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > index 4db07b2..90ca724 100644
> > > --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > @@ -576,11 +576,14 @@ IfConfigShowInterfaceInfo (
> > >  //
> > >  // Get Media State.
> > >  //
> > > -NetLibDetectMedia (IfCb->NicHandle, );
> > > -if (!MediaPresent) {
> > > -  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> L"Media
> > > disconnected");
> > > +if (EFI_SUCCESS == NetLibDetectMedia (IfCb->NicHandle,
> > > )) {
> > > +  if (!MediaPresent) {
> > > +ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> L"Media
> > > disconnected");
> > > +  } else {
> > > +ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> L"Media
> > > present");
> > > +  }
> > >  } else {
> > > -  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> L"Media
> > > present");
> > > +  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> L"Media
> > > disconnected");
> > >  }
> > >
> > >  //
> > > --
> > > 2.7.4

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

[edk2] [PATCH] MdeModulePkg/PciHostBridgeDxe: Fixed PCI DMA Map/Umap bounce buffer

2017-10-08 Thread Daniil Egranov 
The patch corrects the logic of transferring data between a bounce
buffer and a real buffer above 4GB:
1. In the case of mapping a bounce buffer for the write operation,
data from a real buffer should be copied into a bounce
buffer.
2.In the case of unmapping a bounce buffer for the read operation,
data should be copied from a bounce buffer into a real
buffer.

The patch resolves a Juno board issue with the the grub and SATA
drives.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Daniil Egranov 
---
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c 
b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
index dc06c16dc0..877fa2fd13 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -1153,12 +1153,12 @@ RootBridgeIoMap (
 }
 
 //
-// If this is a read operation from the Bus Master's point of view,
+// If this is a write operation from the Bus Master's point of view,
 // then copy the contents of the real buffer into the mapped buffer
 // so the Bus Master can read the contents of the real buffer.
 //
-if (Operation == EfiPciOperationBusMasterRead ||
-Operation == EfiPciOperationBusMasterRead64) {
+if (Operation == EfiPciOperationBusMasterWrite ||
+Operation == EfiPciOperationBusMasterWrite64) {
   CopyMem (
 (VOID *) (UINTN) MapInfo->MappedHostAddress,
 (VOID *) (UINTN) MapInfo->HostAddress,
@@ -1256,12 +1256,12 @@ RootBridgeIoUnmap (
   RemoveEntryList (>Link);
 
   //
-  // If this is a write operation from the Bus Master's point of view,
+  // If this is a read operation from the Bus Master's point of view,
   // then copy the contents of the mapped buffer into the real buffer
   // so the processor can read the contents of the real buffer.
   //
-  if (MapInfo->Operation == EfiPciOperationBusMasterWrite ||
-  MapInfo->Operation == EfiPciOperationBusMasterWrite64) {
+  if (MapInfo->Operation == EfiPciOperationBusMasterRead ||
+  MapInfo->Operation == EfiPciOperationBusMasterRead64) {
 CopyMem (
   (VOID *) (UINTN) MapInfo->HostAddress,
   (VOID *) (UINTN) MapInfo->MappedHostAddress,
-- 
2.11.0

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

[edk2] [platforms: PATCH v2 4/5] Marvell/Drivers: Pp2Dxe: Rework PHY handling

2017-10-08 Thread Marcin Wojtas 
Hitherto PHY handling in Pp2Dxe was not flexible. It allowed for using
only single MDIO controller, which may not be true on Armada 80x0 SoCs.
For this purpose introduce the MDIO description, using the new structures
and template in MvHwDescLib. This change enables addition of multiple
CP110 hardware blocks with MDIO controllers.

This change required different PHY handling and obtaining data over
desired MDIO bus. Now given Pp2 port is matched with the PHY via
its index in gMarvellTokenSpaceGuid.PcdPhyDeviceIds. The PHY itself
is mapped to the MDIO controller, using
gMarvellTokenSpaceGuid.PcdPhy2MdioController. Also obtaining
SMI addresses was moved to the PHY initialization routine.
All above allow for much cleaner and logical PHY description
in the .dsc file, which now uses macros for connection type
and speed.

Update PortingGuide documentation accordingly and Armada 70x0 DB
NIC/PHY description.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada/Armada.dsc.inc |  18 +++
 Platform/Marvell/Armada/Armada70x0.dsc |  10 +-
 Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.c |  35 --
 Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.inf   |   3 -
 Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c   | 122 

 Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h   |   2 -
 Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.inf |   4 +-
 Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c   |  16 +--
 Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h   |   2 +-
 Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf |   4 +-
 Platform/Marvell/Include/Library/MvHwDescLib.h |  23 
 Platform/Marvell/Include/Protocol/Mdio.h   |   6 +
 Platform/Marvell/Include/Protocol/MvPhy.h  |   1 +
 Platform/Marvell/Marvell.dec   |   8 +-
 Silicon/Marvell/Documentation/PortingGuide.txt |  66 +--
 15 files changed, 203 insertions(+), 117 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
b/Platform/Marvell/Armada/Armada.dsc.inc
index 7d0dc39..7258017 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -524,6 +524,24 @@
   DEFINE CP_RXAUI1  = 0x16
   DEFINE CP_SFI = 0x17
 
+  #Network interface speed
+  DEFINE PHY_SPEED_10   = 0x1
+  DEFINE PHY_SPEED_100  = 0x2
+  DEFINE PHY_SPEED_1000 = 0x3
+  DEFINE PHY_SPEED_2500 = 0x4
+  DEFINE PHY_SPEED_1= 0x5
+
+  #Network PHY type
+  DEFINE PHY_RGMII  = 0x0
+  DEFINE PHY_RGMII_ID   = 0x1
+  DEFINE PHY_RGMII_TXID = 0x2
+  DEFINE PHY_RGMII_RXID = 0x3
+  DEFINE PHY_SGMII  = 0x4
+  DEFINE PHY_RTBI   = 0x5
+  DEFINE PHY_XAUI   = 0x6
+  DEFINE PHY_RXAUI  = 0x7
+  DEFINE PHY_SFI= 0x8
+
   #UTMI PHY connection type
   DEFINE UTMI_USB_HOST0 = 0x0
   DEFINE UTMI_USB_HOST1 = 0x1
diff --git a/Platform/Marvell/Armada/Armada70x0.dsc 
b/Platform/Marvell/Armada/Armada70x0.dsc
index b40766b..430803c 100644
--- a/Platform/Marvell/Armada/Armada70x0.dsc
+++ b/Platform/Marvell/Armada/Armada70x0.dsc
@@ -115,18 +115,20 @@
   gMarvellTokenSpaceGuid.PcdUtmiPortType|{ $(UTMI_USB_HOST0), 
$(UTMI_USB_HOST1) }
 
   #MDIO
-  gMarvellTokenSpaceGuid.PcdMdioBaseAddress|0xF212A200
+  gMarvellTokenSpaceGuid.PcdMdioControllersEnabled|{ 0x1, 0x0 }
 
   #PHY
-  gMarvellTokenSpaceGuid.PcdPhyConnectionTypes|{ 0x8, 0x4, 0x0 }
+  gMarvellTokenSpaceGuid.PcdPhy2MdioController|{ 0x0, 0x0 }
   gMarvellTokenSpaceGuid.PcdPhyDeviceIds|{ 0x0, 0x0 }
+  gMarvellTokenSpaceGuid.PcdPhySmiAddresses|{ 0x0, 0x1 }
   gMarvellTokenSpaceGuid.PcdPhyStartupAutoneg|FALSE
 
   #NET
-  gMarvellTokenSpaceGuid.PcdPhySmiAddresses|{ 0xff, 0x0, 0x1 }
   gMarvellTokenSpaceGuid.PcdPp2GopIndexes|{ 0x0, 0x2, 0x3 }
   gMarvellTokenSpaceGuid.PcdPp2InterfaceAlwaysUp|{ 0x0, 0x0, 0x0 }
-  gMarvellTokenSpaceGuid.PcdPp2InterfaceSpeed|{ 0x5, 0x3, 0x3 }
+  gMarvellTokenSpaceGuid.PcdPp2InterfaceSpeed|{ $(PHY_SPEED_1), 
$(PHY_SPEED_1000), $(PHY_SPEED_1000) }
+  gMarvellTokenSpaceGuid.PcdPp2PhyConnectionTypes|{ $(PHY_SFI), $(PHY_SGMII), 
$(PHY_RGMII) }
+  gMarvellTokenSpaceGuid.PcdPp2PhyIndexes|{ 0xFF, 0x0, 0x1 }
   gMarvellTokenSpaceGuid.PcdPp2Port2Controller|{ 0x0, 0x0, 0x0 }
   gMarvellTokenSpaceGuid.PcdPp2PortIds|{ 0x0, 0x1, 0x2 }
   gMarvellTokenSpaceGuid.PcdPp2Controllers|{ 0x1 }
diff --git a/Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.c 
b/Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.c
index ae466d7..12aabad 100644
--- a/Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.c
+++ b/Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.c
@@ -46,7 +46,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include "MvMdioDxe.h"
 
-UINT64 MdioBase = 0;
+DECLARE_A7K8K_MDIO_TEMPLATE;
 
 STATIC
 EFI_STATUS
@@ -70,7 +70,7 @@ MdioCheckParam (
 STATIC

[edk2] [platforms: PATCH v2 5/5] Platform/Marvell/Armada: Remove ParsePcdLib

2017-10-08 Thread Marcin Wojtas 
Current PCD handling in libraries and drivers allow to get
rid of this code.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
Reviewed-by: Leif Lindholm 
Reviewed-by: Ard Biesheuvel 
---
 Platform/Marvell/Armada/Armada.dsc.inc   |   1 -
 Platform/Marvell/Include/Library/ParsePcdLib.h   |  46 
 Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.c   | 228 
 Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.inf |  50 -
 4 files changed, 325 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
b/Platform/Marvell/Armada/Armada.dsc.inc
index 7258017..89fb7e7 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -33,7 +33,6 @@
   
ArmPlatformLib|Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf
   ComPhyLib|Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf
   MppLib|Platform/Marvell/Library/MppLib/MppLib.inf
-  ParsePcdLib|Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.inf
   UtmiPhyLib|Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf
 
   DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
diff --git a/Platform/Marvell/Include/Library/ParsePcdLib.h 
b/Platform/Marvell/Include/Library/ParsePcdLib.h
deleted file mode 100644
index a255685..000
--- a/Platform/Marvell/Include/Library/ParsePcdLib.h
+++ /dev/null
@@ -1,46 +0,0 @@
-/
-Copyright (C) 2016 Marvell International Ltd.
-
-Marvell BSD License Option
-
-If you received this File from Marvell, you may opt to use, redistribute and/or
-modify this File under the following licensing terms.
-Redistribution and use in source and binary forms, with or without 
modification,
-are permitted provided that the following conditions are met:
-
-* Redistributions of source code must retain the above copyright notice,
-  this list of conditions and the following disclaimer.
-
-* Redistributions in binary form must reproduce the above copyright
-  notice, this list of conditions and the following disclaimer in the
-  documentation and/or other materials provided with the distribution.
-
-* Neither the name of Marvell nor the names of its contributors may be
-  used to endorse or promote products derived from this software without
-  specific prior written permission.
-
-THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
-ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
-WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
-DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
-ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
-(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
-LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
-ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
-SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-***/
-
-#ifndef __PARSEPCDLIB_H__
-#define __PARSEPCDLIB_H__
-
-EFI_STATUS
-ParsePcdString (
-  IN  CHAR16 *PcdString,
-  IN  UINT8  Count,
-  OUT UINTN  *ValueTable,
-  OUT CHAR16 **StrTable
-  );
-
-#endif
diff --git a/Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.c 
b/Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.c
deleted file mode 100644
index 9a4be8e..000
--- a/Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.c
+++ /dev/null
@@ -1,228 +0,0 @@
-/
-Copyright (C) 2016 Marvell International Ltd.
-
-Marvell BSD License Option
-
-If you received this File from Marvell, you may opt to use, redistribute and/or
-modify this File under the following licensing terms.
-Redistribution and use in source and binary forms, with or without 
modification,
-are permitted provided that the following conditions are met:
-
-* Redistributions of source code must Retain the above copyright notice,
-  this list of conditions and the following disclaimer.
-
-* Redistributions in binary form must reproduce the above copyright
-  notice, this list of conditions and the following disclaimer in the
-  documentation and/or other materials provided with the distribution.
-
-* Neither the name of Marvell nor the names of its contributors may be
-  used to endorse or promote products derived from this software without
-  specific prior written permission.
-
-THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
-ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
-WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE

[edk2] [platforms: PATCH v2 3/5] Marvell/Library: UtmiLib: Move devices description to MvHwDescLib

2017-10-08 Thread Marcin Wojtas 
This patch introduces UTMI description, using the new structures
and template in MvHwDescLib. This change enables more flexible
addition of multiple CP with UTMI PHY's and also significantly
reduces amount of used PCD's for that purpose. Update PortingGuide
documentation accordingly.

This patch replaces string-based description of Utmi on
Armada 70x0 DB with new, reduced format, which uses macros
in Armada.dsc.inc file for better readability.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada/Armada.dsc.inc |   5 +
 Platform/Marvell/Armada/Armada70x0.dsc |   7 +-
 Platform/Marvell/Include/Library/MvHwDescLib.h |  47 ++
 Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c   | 150 ++--
 Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h   |   1 -
 Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf |  11 +-
 Platform/Marvell/Marvell.dec   |   7 +-
 Silicon/Marvell/Documentation/PortingGuide.txt |  30 ++--
 8 files changed, 148 insertions(+), 110 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
b/Platform/Marvell/Armada/Armada.dsc.inc
index cd26506..7d0dc39 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -523,3 +523,8 @@
   DEFINE CP_RXAUI0  = 0x15
   DEFINE CP_RXAUI1  = 0x16
   DEFINE CP_SFI = 0x17
+
+  #UTMI PHY connection type
+  DEFINE UTMI_USB_HOST0 = 0x0
+  DEFINE UTMI_USB_HOST1 = 0x1
+  DEFINE UTMI_USB_DEVICE0   = 0x2
diff --git a/Platform/Marvell/Armada/Armada70x0.dsc 
b/Platform/Marvell/Armada/Armada70x0.dsc
index c11a973..b40766b 100644
--- a/Platform/Marvell/Armada/Armada70x0.dsc
+++ b/Platform/Marvell/Armada/Armada70x0.dsc
@@ -111,11 +111,8 @@
   gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|{ $(CP_1_25G), $(CP_5G), 
$(CP_10_3125G), $(CP_5G), $(CP_5G), $(CP_5G) }
 
   #UtmiPhy
-  gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUsbCfg|L"0xF2440420;0xF2440420"
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiCfg|L"0xF2440440;0xF2440444"
-  gMarvellTokenSpaceGuid.PcdUtmiPhyRegUtmiUnit|L"0xF258;0xF2581000"
-  gMarvellTokenSpaceGuid.PcdUtmiPhyUtmiPort|L"0x0;0x1"
+  gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled|{ 0x1, 0x1 }
+  gMarvellTokenSpaceGuid.PcdUtmiPortType|{ $(UTMI_USB_HOST0), 
$(UTMI_USB_HOST1) }
 
   #MDIO
   gMarvellTokenSpaceGuid.PcdMdioBaseAddress|0xF212A200
diff --git a/Platform/Marvell/Include/Library/MvHwDescLib.h 
b/Platform/Marvell/Include/Library/MvHwDescLib.h
index e029b50..6ad1bc2 100644
--- a/Platform/Marvell/Include/Library/MvHwDescLib.h
+++ b/Platform/Marvell/Include/Library/MvHwDescLib.h
@@ -117,6 +117,19 @@ typedef struct {
 } MVHW_RTC_DESC;
 
 //
+// UTMI PHY's description template definition
+//
+
+typedef struct {
+  UINT8 UtmiDevCount;
+  UINT32 UtmiPhyId[MVHW_MAX_XHCI_DEVS];
+  UINTN UtmiBaseAddresses[MVHW_MAX_XHCI_DEVS];
+  UINTN UtmiConfigAddresses[MVHW_MAX_XHCI_DEVS];
+  UINTN UtmiUsbConfigAddresses[MVHW_MAX_XHCI_DEVS];
+  UINTN UtmiMuxBitCount[MVHW_MAX_XHCI_DEVS];
+} MVHW_UTMI_DESC;
+
+//
 // Platform description of CommonPhy devices
 //
 #define MVHW_CP0_COMPHY_BASE   0xF2441000
@@ -217,4 +230,38 @@ MVHW_RTC_DESC mA7k8kRtcDescTemplate = {\
   { SIZE_4KB, SIZE_4KB }\
 }
 
+//
+// Platform description of UTMI PHY's
+//
+#define MVHW_CP0_UTMI0_BASE0xF258
+#define MVHW_CP0_UTMI0_CFG_BASE0xF2440440
+#define MVHW_CP0_UTMI0_USB_CFG_BASE0xF2440420
+#define MVHW_CP0_UTMI0_ID  0x0
+#define MVHW_CP0_UTMI1_BASE0xF2581000
+#define MVHW_CP0_UTMI1_CFG_BASE0xF2440444
+#define MVHW_CP0_UTMI1_USB_CFG_BASE0xF2440420
+#define MVHW_CP0_UTMI1_ID  0x1
+#define MVHW_CP1_UTMI0_BASE0xF458
+#define MVHW_CP1_UTMI0_CFG_BASE0xF4440440
+#define MVHW_CP1_UTMI0_USB_CFG_BASE0xF4440420
+#define MVHW_CP1_UTMI0_ID  0x0
+#define MVHW_CP1_UTMI1_BASE0xF4581000
+#define MVHW_CP1_UTMI1_CFG_BASE0xF4440444
+#define MVHW_CP1_UTMI1_USB_CFG_BASE0xF4440420
+#define MVHW_CP1_UTMI1_ID  0x1
+
+#define DECLARE_A7K8K_UTMI_TEMPLATE \
+STATIC \
+MVHW_UTMI_DESC mA7k8kUtmiDescTemplate = {\
+  4,\
+  { MVHW_CP0_UTMI0_ID, MVHW_CP0_UTMI1_ID,\
+MVHW_CP1_UTMI0_ID, MVHW_CP1_UTMI1_ID },\
+  { MVHW_CP0_UTMI0_BASE, MVHW_CP0_UTMI1_BASE,\
+MVHW_CP1_UTMI0_BASE, MVHW_CP1_UTMI1_BASE },\
+  { MVHW_CP0_UTMI0_CFG_BASE, MVHW_CP0_UTMI1_CFG_BASE,\
+MVHW_CP1_UTMI0_CFG_BASE, MVHW_CP1_UTMI1_CFG_BASE },\
+  { MVHW_CP0_UTMI0_USB_CFG_BASE, MVHW_CP0_UTMI1_USB_CFG_BASE,\
+MVHW_CP1_UTMI0_USB_CFG_BASE, MVHW_CP1_UTMI1_USB_CFG_BASE }\
+}
+
 #endif /* __MVHWDESCLIB_H__ */
diff --git a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c 
b/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
index 95b5698..f1819c4 100644
--- a/Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c
+++

[edk2] [platforms: PATCH v2 1/5] Marvell/Library: ComPhyLib: Remove PCD string parsing

2017-10-08 Thread Marcin Wojtas 
Simplify obtaining lane data, using arrays with direct enum values,
rather than strings. This is another step to completely remove
ParsePcdLib.

This patch replaces string-based description of ComPhy lanes
on Armada 70x0 DB with the enum values of type and speed -
for that purpose new [Defines] section was added to Armada.dsc.inc
file in order to increase readability.
PortingGuide is updated accordingly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada/Armada.dsc.inc   | 44 +
 Platform/Marvell/Armada/Armada70x0.dsc   | 11 +++-
 Platform/Marvell/Library/ComPhyLib/ComPhyLib.c   | 65 ---
 Platform/Marvell/Library/ComPhyLib/ComPhyLib.h   | 25 +++-
 Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf |  1 -
 Silicon/Marvell/Documentation/PortingGuide.txt   | 67 +++-
 6 files changed, 124 insertions(+), 89 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada.dsc.inc 
b/Platform/Marvell/Armada/Armada.dsc.inc
index 9549091..cd26506 100644
--- a/Platform/Marvell/Armada/Armada.dsc.inc
+++ b/Platform/Marvell/Armada/Armada.dsc.inc
@@ -479,3 +479,47 @@
   gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
   gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
   }
+
+
+#
+# Defines - platform description macros
+#
+
+[Defines]
+  # ComPhy speed
+  DEFINE CP_1_25G   = 0x1
+  DEFINE CP_1_5G= 0x2
+  DEFINE CP_2_5G= 0x3
+  DEFINE CP_3G  = 0x4
+  DEFINE CP_3_125G  = 0x5
+  DEFINE CP_5G  = 0x6
+  DEFINE CP_5_15625G= 0x7
+  DEFINE CP_6G  = 0x8
+  DEFINE CP_6_25G   = 0x9
+  DEFINE CP_10_3125G= 0xA
+
+  # ComPhy type
+  DEFINE CP_UNCONNECTED = 0x0
+  DEFINE CP_PCIE0   = 0x1
+  DEFINE CP_PCIE1   = 0x2
+  DEFINE CP_PCIE2   = 0x3
+  DEFINE CP_PCIE3   = 0x4
+  DEFINE CP_SATA0   = 0x5
+  DEFINE CP_SATA1   = 0x6
+  DEFINE CP_SATA2   = 0x7
+  DEFINE CP_SATA3   = 0x8
+  DEFINE CP_SGMII0  = 0x9
+  DEFINE CP_SGMII1  = 0xA
+  DEFINE CP_SGMII2  = 0xB
+  DEFINE CP_SGMII3  = 0xC
+  DEFINE CP_QSGMII  = 0xD
+  DEFINE CP_USB3_HOST0  = 0xE
+  DEFINE CP_USB3_HOST1  = 0xF
+  DEFINE CP_USB3_DEVICE = 0x10
+  DEFINE CP_XAUI0   = 0x11
+  DEFINE CP_XAUI1   = 0x12
+  DEFINE CP_XAUI2   = 0x13
+  DEFINE CP_XAUI3   = 0x14
+  DEFINE CP_RXAUI0  = 0x15
+  DEFINE CP_RXAUI1  = 0x16
+  DEFINE CP_SFI = 0x17
diff --git a/Platform/Marvell/Armada/Armada70x0.dsc 
b/Platform/Marvell/Armada/Armada70x0.dsc
index 467dfa3..dae9715 100644
--- a/Platform/Marvell/Armada/Armada70x0.dsc
+++ b/Platform/Marvell/Armada/Armada70x0.dsc
@@ -100,8 +100,15 @@
 
   #ComPhy
   gMarvellTokenSpaceGuid.PcdComPhyDevices|{ 0x1 }
-  
gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes|L"SGMII1;USB3_HOST0;SFI;SATA1;USB3_HOST1;PCIE2"
-  gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|L"1250;5000;10310;5000;5000;5000"
+  # ComPhy0
+  # 0: SGMII11.25 Gbps
+  # 1: USB3_HOST05 Gbps
+  # 2: SFI   10.31 Gbps
+  # 3: SATA1 5 Gbps
+  # 4: USB3_HOST15 Gbps
+  # 5: PCIE2 5 Gbps
+  gMarvellTokenSpaceGuid.PcdChip0ComPhyTypes|{ $(CP_SGMII1), $(CP_USB3_HOST0), 
$(CP_SFI), $(CP_SATA1), $(CP_USB3_HOST1), $(CP_PCIE2) }
+  gMarvellTokenSpaceGuid.PcdChip0ComPhySpeeds|{ $(CP_1_25G), $(CP_5G), 
$(CP_10_3125G), $(CP_5G), $(CP_5G), $(CP_5G) }
 
   #UtmiPhy
   gMarvellTokenSpaceGuid.PcdUtmiPhyCount|2
diff --git a/Platform/Marvell/Library/ComPhyLib/ComPhyLib.c 
b/Platform/Marvell/Library/ComPhyLib/ComPhyLib.c
index 3eb5d9f..bf21dca 100644
--- a/Platform/Marvell/Library/ComPhyLib/ComPhyLib.c
+++ b/Platform/Marvell/Library/ComPhyLib/ComPhyLib.c
@@ -113,47 +113,6 @@ RegSetSilent16(
   MmioWrite16 (Addr, RegData);
 }
 
-/* This function returns enum with SerDesType */
-UINT32
-ParseSerdesTypeString (
-  CHAR16* String
-  )
-{
-  UINT32 i;
-
-  if (String == NULL)
-return COMPHY_TYPE_INVALID;
-
-  for (i = 0; i < COMPHY_TYPE_MAX; i++) {
-if (StrCmp (String, TypeStringTable[i]) == 0) {
-  return i;
-}
-  }
-
-  /* PCD string doesn't match any supported SerDes Type */
-  return COMPHY_TYPE_INVALID;
-}
-
-/* This function converts SerDes speed in MHz to enum with SerDesSpeed */
-UINT32
-ParseSerdesSpeed (
-  UINT32 Value
-  )
-{
-  UINT32 i;
-  UINT32 ValueTable [] = {0, 1250, 1500, 2500, 3000, 3125,
-  5000, 5156, 6000, 6250, 10310};
-
-  for (i = 0; i < COMPHY_SPEED_MAX; i++) {
-if (Value == ValueTable[i]) {
-  return i;
-}
-  }
-
-  /* PCD SerDes speed value doesn't match any supported SerDes speed */
-  return COMPHY_SPEED_INVALID;
-}
-

[edk2] [platforms: PATCH v2 2/5] Marvell/Drivers: MvI2cDxe: Move devices description to MvHwDescLib

2017-10-08 Thread Marcin Wojtas 
This patch introduces I2c description, using the new structures
and template in MvHwDescLib. This change enables more flexible
addition of multiple I2c controllers and also allows for
removal of string PCD parsing. Update Armada 70x0 DB description
and PortingGuide accordingly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada/Armada70x0.dsc |  2 +-
 Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c   | 42 +++-
 Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf |  3 +-
 Platform/Marvell/Include/Library/MvHwDescLib.h | 25 
 Platform/Marvell/Marvell.dec   |  2 +-
 Silicon/Marvell/Documentation/PortingGuide.txt |  4 +-
 6 files changed, 54 insertions(+), 24 deletions(-)

diff --git a/Platform/Marvell/Armada/Armada70x0.dsc 
b/Platform/Marvell/Armada/Armada70x0.dsc
index dae9715..c11a973 100644
--- a/Platform/Marvell/Armada/Armada70x0.dsc
+++ b/Platform/Marvell/Armada/Armada70x0.dsc
@@ -78,7 +78,7 @@
   # I2C
   gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses|{ 0x50, 0x57, 0x60 }
   gMarvellTokenSpaceGuid.PcdI2cSlaveBuses|{ 0x0, 0x0, 0x0 }
-  gMarvellTokenSpaceGuid.PcdI2cBaseAddresses|L"0xF2701000;0xF2701100"
+  gMarvellTokenSpaceGuid.PcdI2cControllersEnabled|{ 0x1, 0x1 }
   gMarvellTokenSpaceGuid.PcdEepromI2cAddresses|{ 0x50, 0x57 }
   gMarvellTokenSpaceGuid.PcdEepromI2cBuses|{ 0x0, 0x0 }
   gMarvellTokenSpaceGuid.PcdI2cClockFrequency|25000
diff --git a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c 
b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
index fa79ebc..d85ee0b 100755
--- a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
+++ b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c
@@ -42,12 +42,14 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include 
 #include 
 #include 
-#include 
 #include 
+#include 
 #include 
 
 #include "MvI2cDxe.h"
 
+DECLARE_A7K8K_I2C_TEMPLATE;
+
 STATIC MV_I2C_BAUD_RATE baud_rate;
 
 STATIC MV_I2C_DEVICE_PATH MvI2cDevicePathProtocol = {
@@ -172,35 +174,39 @@ MvI2cInitialise (
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
 {
+  MVHW_I2C_DESC *Desc = 
+  UINT8 *I2cDeviceTable, Index;
   EFI_STATUS Status;
-  UINT32 BusCount;
-  EFI_PHYSICAL_ADDRESS I2cBaseAddresses[PcdGet32 (PcdI2cBusCount)];
-  INTN i;
 
-  BusCount = PcdGet32 (PcdI2cBusCount);
-  if (BusCount == 0)
-return EFI_SUCCESS;
+  /* Obtain table with enabled I2c devices */
+  I2cDeviceTable = (UINT8 *)PcdGetPtr (PcdI2cControllersEnabled);
+  if (I2cDeviceTable == NULL) {
+DEBUG ((DEBUG_ERROR, "Missing PcdI2cControllersEnabled\n"));
+return EFI_INVALID_PARAMETER;
+  }
 
-  Status = ParsePcdString (
-  (CHAR16 *) PcdGetPtr (PcdI2cBaseAddresses),
-  BusCount,
-  I2cBaseAddresses,
-  NULL
-  );
-  if (EFI_ERROR(Status))
-return Status;
+  if (PcdGetSize (PcdI2cControllersEnabled) > MVHW_MAX_I2C_DEVS) {
+DEBUG ((DEBUG_ERROR, "Wrong PcdI2cControllersEnabled format\n"));
+return EFI_INVALID_PARAMETER;
+  }
+
+  /* Initialize enabled chips */
+  for (Index = 0; Index < PcdGetSize (PcdI2cControllersEnabled); Index++) {
+if (!MVHW_DEV_ENABLED (I2c, Index)) {
+  DEBUG ((DEBUG_ERROR, "Skip I2c chip %d\n", Index));
+  continue;
+}
 
-  for (i = 0; i < BusCount; i++) {
 Status = MvI2cInitialiseController(
 ImageHandle,
 SystemTable,
-I2cBaseAddresses[i]
+Desc->I2cBaseAddresses[Index]
 );
 if (EFI_ERROR(Status))
   return Status;
   }
 
-  return Status;
+  return EFI_SUCCESS;
 }
 
 STATIC
diff --git a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf 
b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
index 16374ef..80655f1 100755
--- a/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
+++ b/Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf
@@ -55,7 +55,6 @@
   UefiLib
   UefiDriverEntryPoint
   UefiBootServicesTableLib
-  ParsePcdLib
 
 [Protocols]
   gEfiI2cMasterProtocolGuid
@@ -66,7 +65,7 @@
 [Pcd]
   gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses
   gMarvellTokenSpaceGuid.PcdI2cSlaveBuses
-  gMarvellTokenSpaceGuid.PcdI2cBaseAddresses
+  gMarvellTokenSpaceGuid.PcdI2cControllersEnabled
   gMarvellTokenSpaceGuid.PcdI2cClockFrequency
   gMarvellTokenSpaceGuid.PcdI2cBaudRate
   gMarvellTokenSpaceGuid.PcdI2cBusCount
diff --git a/Platform/Marvell/Include/Library/MvHwDescLib.h 
b/Platform/Marvell/Include/Library/MvHwDescLib.h
index 6a86865..e029b50 100644
--- a/Platform/Marvell/Include/Library/MvHwDescLib.h
+++ b/Platform/Marvell/Include/Library/MvHwDescLib.h
@@ -60,6 +60,16 @@ typedef struct {
 } MVHW_COMPHY_DESC;
 
 //
+// I2C devices description template definition
+//
+#define MVHW_MAX_I2C_DEVS 4
+
+typedef struct {
+  UINT8 I2cDevCount;
+  UINTN I2cBaseAddresses[MVHW_MAX_I2C_DEVS];
+} MVHW_I2C_DESC;
+
+//
 // NonDiscoverable devices description template definition
 //
 #define MVHW_MAX_XHCI_DEVS 4
@@ -130,6 +140,21 @@

[edk2] [platforms: PATCH v2 0/5] Armada 7k/8k - ParsePcdLib removal

2017-10-08 Thread Marcin Wojtas 
Hi,

This is a second version of the patchset with the complete
removal of ParsePcdLib and cleanup of boards' PCD representation.
According to v1 remarks, I modified PCD names and introduced macros
in Armada.dsc.inc, so that the readability of boards .dsc could
increse.
More details can befound in the changelog and commits.

The patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/parsepcd-upstream-r20171008

I'm looking forward to your comments or remarks.

Best regards,
Marcin

Changelog:
v1  -> v2
1/5
  * Add new [Defines] section was added to Armada.dsc.inc
file in order to increase readability.

2/5
  * s/PcdI2cControllers/PcdI2cControllersEnabled/

3/5
  * s/PcdUtmiControllers/PcdUtmiControllersEnabled/
  * Add defines for PcdUtmiPortType

4/5:
  * s/PcdMdioControllers/PcdMdioControllersEnabled/
  * Add macros for network PHY type and speed in Armada.dsc.inc

5/5
  * Add RBs

Marcin Wojtas (5):
  Marvell/Library: ComPhyLib: Remove PCD string parsing
  Marvell/Drivers: MvI2cDxe: Move devices description to MvHwDescLib
  Marvell/Library: UtmiLib: Move devices description to MvHwDescLib
  Marvell/Drivers: Pp2Dxe: Rework PHY handling
  Platform/Marvell/Armada: Remove ParsePcdLib

 Platform/Marvell/Armada/Armada.dsc.inc |  68 +-
 Platform/Marvell/Armada/Armada70x0.dsc |  30 +--
 Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.c   |  42 ++--
 Platform/Marvell/Drivers/I2c/MvI2cDxe/MvI2cDxe.inf |   3 +-
 Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.c |  35 ++-
 Platform/Marvell/Drivers/Net/MvMdioDxe/MvMdioDxe.inf   |   3 -
 Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.c   | 122 ++-
 Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.h   |   2 -
 Platform/Marvell/Drivers/Net/Phy/MvPhyDxe/MvPhyDxe.inf |   4 +-
 Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c   |  16 +-
 Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.h   |   2 +-
 Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.inf |   4 +-
 Platform/Marvell/Include/Library/MvHwDescLib.h |  95 
 Platform/Marvell/Include/Library/ParsePcdLib.h |  46 
 Platform/Marvell/Include/Protocol/Mdio.h   |   6 +
 Platform/Marvell/Include/Protocol/MvPhy.h  |   1 +
 Platform/Marvell/Library/ComPhyLib/ComPhyLib.c |  65 +-
 Platform/Marvell/Library/ComPhyLib/ComPhyLib.h |  25 +--
 Platform/Marvell/Library/ComPhyLib/ComPhyLib.inf   |   1 -
 Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.c | 228 

 Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.inf   |  50 -
 Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.c   | 150 ++---
 Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.h   |   1 -
 Platform/Marvell/Library/UtmiPhyLib/UtmiPhyLib.inf |  11 +-
 Platform/Marvell/Marvell.dec   |  17 +-
 Silicon/Marvell/Documentation/PortingGuide.txt | 165 --
 26 files changed, 528 insertions(+), 664 deletions(-)
 delete mode 100644 Platform/Marvell/Include/Library/ParsePcdLib.h
 delete mode 100644 Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.c
 delete mode 100644 Platform/Marvell/Library/ParsePcdLib/ParsePcdLib.inf

-- 
1.8.3.1

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

Re: [edk2] [PATCH v3] Ifconfig : Fixed False information about Media State.

2017-10-08 Thread Meenakshi Aggarwal 
It is hard to say when can an API fail because its dependent on implementation.


> -Original Message-
> From: Carsey, Jaben [mailto:jaben.car...@intel.com]
> Sent: Friday, October 06, 2017 7:32 PM
> To: Meenakshi Aggarwal ; edk2-
> de...@lists.01.org; Wu, Jiaxin ; Ni, Ruiyu
> 
> Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media State.
> 
> Reviewed-by: Jaben Carsey  Do you know under
> what conditions the API will fail? Is it worth saying something like media 
> stats
> unknown when the function fails?
> 
> Ray,
> 
> What do you think?
> 
> 
> > -Original Message-
> > From: Meenakshi Aggarwal [mailto:meenakshi.aggar...@nxp.com]
> > Sent: Thursday, October 05, 2017 9:48 PM
> > To: edk2-devel@lists.01.org; Wu, Jiaxin ; Carsey,
> > Jaben ; Ni, Ruiyu 
> > Cc: Meenakshi Aggarwal 
> > Subject: [PATCH v3] Ifconfig : Fixed False information about Media State.
> > Importance: High
> >
> > Issue : We were setting MediaPresent as TRUE (default) and not
> > checking return status of NetLibDetectMedia().
> > NetLibDetectMedia() sets MediaPresent FLAG in case of success only and
> > dont change flag on error.
> > So, Media State will display as 'Media Present', in case of error
> > also.
> >
> > Fix : Check return value of NetLibDetectMedia()
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> >
> > Signed-off-by: Meenakshi Aggarwal 
> > ---
> >  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 11
> > +++--
> > --
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > index 4db07b2..90ca724 100644
> > --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > @@ -576,11 +576,14 @@ IfConfigShowInterfaceInfo (
> >  //
> >  // Get Media State.
> >  //
> > -NetLibDetectMedia (IfCb->NicHandle, );
> > -if (!MediaPresent) {
> > -  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> > disconnected");
> > +if (EFI_SUCCESS == NetLibDetectMedia (IfCb->NicHandle,
> > )) {
> > +  if (!MediaPresent) {
> > +ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> > disconnected");
> > +  } else {
> > +ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> > present");
> > +  }
> >  } else {
> > -  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> > present");
> > +  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> > disconnected");
> >  }
> >
> >  //
> > --
> > 2.7.4

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