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

["Leif Lindholm" <leif.lindholm@linaro.org>]

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
>