Re: [platforms: PATCH v2 07/10] Marvell/Drivers: Pp2Dxe: Change settings for the always-up link

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

On Fri, Oct 27, 2017 at 03:13:49AM +0200, Marcin Wojtas wrote:
> Currently initial forcing link status happened for all ports, not only
> marked as 'always-up'. Although this didn't actually matter for the MAC
> settings, because MAC is automatically updated with PHY HW polling
> feature of the controller, perform mv_gop110_fl_cfg only when
> the appropriate flag is true. Also in such case, force the link as up,
> using a new library routine.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c | 25 ++++++++++++++++++++
>  Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h |  6 +++++
>  Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c   |  6 ++++-
>  3 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c b/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c
> index 53154db..c2d0199 100644
> --- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c
> +++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.c
> @@ -4804,6 +4804,31 @@ MvGop110PortEventsMask (
>    return 0;
>  }
>  
> +/*
> + * Sets "Force Link Pass" and "Do Not Force Link Fail" bits.
> + * This function should only be called when the port is disabled.
> + */
> +VOID
> +MvGop110GmacForceLinkModeSet(
> +  IN PP2DXE_PORT *Port,
> +  IN BOOLEAN LinkUp
> +  )
> +{
> +  UINT32 RegVal;
> +
> +  RegVal = MvGop110GmacRead (Port, MVPP2_PORT_AUTO_NEG_CFG_REG);
> +
> +  if (LinkUp) {
> +    RegVal |= MVPP2_PORT_AUTO_NEG_CFG_FORCE_LINK_UP_MASK;
> +    RegVal &= ~MVPP2_PORT_AUTO_NEG_CFG_FORCE_LINK_DOWN_MASK;

So, I know UP and DOWN are separate bits (what's that about!?), but
unless you know that, the above two lines look like they're in the
wrong order.

If you flip those around (and make it like the 'else' clause):
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> +  } else {
> +    RegVal &= ~MVPP2_PORT_AUTO_NEG_CFG_FORCE_LINK_UP_MASK;
> +    RegVal |= MVPP2_PORT_AUTO_NEG_CFG_FORCE_LINK_DOWN_MASK;
> +  }
> +
> +  MvGop110GmacWrite (Port, MVPP2_PORT_AUTO_NEG_CFG_REG, RegVal);
> +}
> +
>  INT32
>  MvGop110FlCfg (
>    IN PP2DXE_PORT *Port
> diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h b/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h
> index a7011f7..3ebe294 100644
> --- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h
> +++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Mvpp2Lib.h
> @@ -504,6 +504,12 @@ MvGop110XlgPortLinkEventMask (
>    IN PP2DXE_PORT *Port
>    );
>  
> +VOID
> +MvGop110GmacForceLinkModeSet (
> +  IN PP2DXE_PORT *Port,
> +  IN BOOLEAN LinkUp
> +  );
> +
>  INT32
>  MvGop110FlCfg (
>    IN PP2DXE_PORT *Port
> diff --git a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> index 2827976..4a1b9d5 100644
> --- a/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> +++ b/Platform/Marvell/Drivers/Net/Pp2Dxe/Pp2Dxe.c
> @@ -1310,7 +1310,11 @@ Pp2DxeInitialiseController (
>      NetCompConfig |= MvpPp2xGop110NetcCfgCreate(&Pp2Context->Port);
>  
>      MvGop110PortInit(&Pp2Context->Port);
> -    MvGop110FlCfg(&Pp2Context->Port);
> +
> +    if (Pp2Context->Port.AlwaysUp == TRUE) {
> +      MvGop110GmacForceLinkModeSet (&Pp2Context->Port, TRUE);
> +      MvGop110FlCfg (&Pp2Context->Port);
> +    }
>  
>      Status = gBS->CreateEvent (
>                   EVT_SIGNAL_EXIT_BOOT_SERVICES,
> -- 
> 2.7.4
>