[edk2-platforms: PATCH] Marvell/Drivers: MvPhyDxe: Execute proper initialization callback

[edk2-platforms: PATCH] Marvell/Drivers: MvPhyDxe: Execute proper initialization callback

[Marcin Wojtas]

Hitherto code was executing by mistake the first
possible initialization routine (valid for 88E1512 PHY),
regardless the setting in gMarvellTokenSpaceGuid.PcdPhyDeviceIds.
Fix this.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.h |  3 ++-
 Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.c | 28 +++++++++-----------
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.h b/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.h
index 908b768..4d8aacf 100644
--- a/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.h
+++ b/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.h
@@ -49,7 +49,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 typedef enum {
   MV_PHY_DEVICE_1512,
-  MV_PHY_DEVICE_1112
+  MV_PHY_DEVICE_1112,
+  MV_PHY_DEVICE_ID_MAX
 } MV_PHY_DEVICE_ID;
 
 typedef
diff --git a/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.c b/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.c
index 2d2aad7..6e4a715 100644
--- a/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.c
+++ b/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.c
@@ -396,7 +396,7 @@ MvPhyInit (
   PHY_DEVICE *PhyDev;
   UINT8 *DeviceIds;
   UINT8 MdioIndex;
-  INTN i;
+  UINT8 PhyId;
 
   Status = gBS->LocateProtocol (
       &gMarvellMdioProtocolGuid,
@@ -415,6 +415,17 @@ MvPhyInit (
     return EFI_INVALID_PARAMETER;
   }
 
+  DeviceIds = PcdGetPtr (PcdPhyDeviceIds);
+  PhyId = DeviceIds[PhyIndex];
+  if (PhyId >= MV_PHY_DEVICE_ID_MAX) {
+    DEBUG ((DEBUG_ERROR,
+      "%a, Incorrect PHY ID (0x%x) for PHY#%d\n",
+      __FUNCTION__,
+      PhyId,
+      PhyIndex));
+    return EFI_INVALID_PARAMETER;
+  }
+
   /* perform setup common for all PHYs */
   PhyDev = AllocateZeroPool (sizeof (PHY_DEVICE));
   PhyDev->Addr = PhySmiAddresses[PhyIndex];
@@ -427,20 +438,7 @@ MvPhyInit (
     PhyConnection));
   *OutPhyDev = PhyDev;
 
-  DeviceIds = PcdGetPtr (PcdPhyDeviceIds);
-  for (i = 0; i < PcdGetSize (PcdPhyDeviceIds); i++) {
-    /* find MvPhyDevices fitting entry */
-    if (MvPhyDevices[i].DevId == DeviceIds[i]) {
-      ASSERT (MvPhyDevices[i].DevInit != NULL);
-      /* proceed with PHY-specific initialization */
-      return MvPhyDevices[i].DevInit (Snp, PhyDev);
-    }
-  }
-
-  /* if we are here, no matching DevId was found */
-  Status = EFI_INVALID_PARAMETER;
-  FreePool (PhyDev);
-  return Status;
+  return MvPhyDevices[PhyId].DevInit (Snp, PhyDev);
 }
 
 EFI_STATUS
-- 
2.7.4

Re: [edk2-platforms: PATCH] Marvell/Drivers: MvPhyDxe: Execute proper initialization callback

[Leif Lindholm]

On Mon, Oct 07, 2019 at 02:02:26AM +0200, Marcin Wojtas wrote:
> Hitherto code was executing by mistake the first
> possible initialization routine (valid for 88E1512 PHY),
> regardless the setting in gMarvellTokenSpaceGuid.PcdPhyDeviceIds.
> Fix this.
> 
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Pushed as 16eb2d4e9c7b.
Thanks!

> ---
>  Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.h |  3 ++-
>  Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.c | 28 +++++++++-----------
>  2 files changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.h b/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.h
> index 908b768..4d8aacf 100644
> --- a/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.h
> +++ b/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.h
> @@ -49,7 +49,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  typedef enum {
>    MV_PHY_DEVICE_1512,
> -  MV_PHY_DEVICE_1112
> +  MV_PHY_DEVICE_1112,
> +  MV_PHY_DEVICE_ID_MAX
>  } MV_PHY_DEVICE_ID;
>  
>  typedef
> diff --git a/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.c b/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.c
> index 2d2aad7..6e4a715 100644
> --- a/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.c
> +++ b/Silicon/Marvell/Drivers/Net/MvPhyDxe/MvPhyDxe.c
> @@ -396,7 +396,7 @@ MvPhyInit (
>    PHY_DEVICE *PhyDev;
>    UINT8 *DeviceIds;
>    UINT8 MdioIndex;
> -  INTN i;
> +  UINT8 PhyId;
>  
>    Status = gBS->LocateProtocol (
>        &gMarvellMdioProtocolGuid,
> @@ -415,6 +415,17 @@ MvPhyInit (
>      return EFI_INVALID_PARAMETER;
>    }
>  
> +  DeviceIds = PcdGetPtr (PcdPhyDeviceIds);
> +  PhyId = DeviceIds[PhyIndex];
> +  if (PhyId >= MV_PHY_DEVICE_ID_MAX) {
> +    DEBUG ((DEBUG_ERROR,
> +      "%a, Incorrect PHY ID (0x%x) for PHY#%d\n",
> +      __FUNCTION__,
> +      PhyId,
> +      PhyIndex));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    /* perform setup common for all PHYs */
>    PhyDev = AllocateZeroPool (sizeof (PHY_DEVICE));
>    PhyDev->Addr = PhySmiAddresses[PhyIndex];
> @@ -427,20 +438,7 @@ MvPhyInit (
>      PhyConnection));
>    *OutPhyDev = PhyDev;
>  
> -  DeviceIds = PcdGetPtr (PcdPhyDeviceIds);
> -  for (i = 0; i < PcdGetSize (PcdPhyDeviceIds); i++) {
> -    /* find MvPhyDevices fitting entry */
> -    if (MvPhyDevices[i].DevId == DeviceIds[i]) {
> -      ASSERT (MvPhyDevices[i].DevInit != NULL);
> -      /* proceed with PHY-specific initialization */
> -      return MvPhyDevices[i].DevInit (Snp, PhyDev);
> -    }
> -  }
> -
> -  /* if we are here, no matching DevId was found */
> -  Status = EFI_INVALID_PARAMETER;
> -  FreePool (PhyDev);
> -  return Status;
> +  return MvPhyDevices[PhyId].DevInit (Snp, PhyDev);
>  }
>  
>  EFI_STATUS
> -- 
> 2.7.4
>