Leif,

2017-03-22 13:36 GMT+01:00 Leif Lindholm <leif.lindholm@linaro.org>:
On Wed, Mar 22, 2017 at 11:28:00AM +0100, Marcin Wojtas wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
>
> Fix compilation warning triggered by GCC 4.9. On the occasion
> change type of COMPHY_CHIP_INIT callback to VOID - the function
> is responsible for initializing all PHY's independently
> and returning overall Status does not make sense. For DEBUG build add
> ASSERT for switch default in case the SerDes lane type is not
> properly defined.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c | 7 +++----
>  Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h   | 4 ++--
>  2 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c b/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c
> index df8731d..bee3bbf 100755
> --- a/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c
> +++ b/Platforms/Marvell/Library/ComPhyLib/ComPhyCp110.c
> @@ -975,12 +975,12 @@ ComPhyMuxCp110 (
>        SerdesMap[Lane].Type = PHY_TYPE_UNCONNECTED;
>  }
>
> -EFI_STATUS
> +VOID
>  ComPhyCp110Init (
>    IN CHIP_COMPHY_CONFIG *PtrChipCfg
>    )
>  {
> -  EFI_STATUS Status;
> +  EFI_STATUS Status = EFI_SUCCESS;

This assignment shouldn't be necessary with the rest of the changes?
I can fold that in on commit if you agree.

In for loop we check for Status (if (EFI_ERROR(Status))), so unitialized value condition will still be there. I'll remove assginment from here and do 'Status = EFI_INVALID_PARAMETER' in switch default, so that all paths are covered, ok?
 

If so:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

>    COMPHY_MAP *PtrComPhyMap, *SerdesMap;
>    EFI_PHYSICAL_ADDRESS ComPhyBaseAddr, HpipeBaseAddr;
>    UINT32 ComPhyMaxCount, Lane;
> @@ -1036,12 +1036,11 @@ ComPhyCp110Init (
>      default:
>        DEBUG((DEBUG_ERROR, "Unknown SerDes Type, skip initialize SerDes %d\n",
>          Lane));
> +      ASSERT (FALSE);
>        break;
>      }
>      if (EFI_ERROR(Status))
>        DEBUG((DEBUG_ERROR, "PLL is not locked - Failed to initialize Lane %d\n",
>          Lane));
>    }
> -
> -  return EFI_SUCCESS;
>  }
> diff --git a/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h b/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h
> index 48cafc9..945f266 100644
> --- a/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h
> +++ b/Platforms/Marvell/Library/ComPhyLib/ComPhyLib.h
> @@ -394,7 +394,7 @@ typedef struct {
>  } PCD_LANE_MAP;
>
>  typedef
> -EFI_STATUS
> +VOID
>  (*COMPHY_CHIP_INIT) (
>    IN CHIP_COMPHY_CONFIG *PtrChipCfg
>    );
> @@ -417,7 +417,7 @@ ComPhyMuxInit (
>    IN EFI_PHYSICAL_ADDRESS SelectorBase
>    );
>
> -EFI_STATUS
> +VOID
>  ComPhyCp110Init (
>    IN CHIP_COMPHY_CONFIG * First
>    );
> --
> 1.8.3.1
>