1. Patchew
  2. EDK2
  3. [edk2] [platforms: PATCH v2 00/10] Armada 7k/8k - misc improvements pt.2
  4. View patch

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

Marcin Wojtas posted 10 patches 7 years, 6 months ago
There is a newer version of this series
[edk2] [platforms: PATCH v2 07/10] Marvell/Drivers: Pp2Dxe: Change settings for the always-up link
Posted by Marcin Wojtas 7 years, 6 months ago 
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;
+  } 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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH v2 07/10] Marvell/Drivers: Pp2Dxe: Change settings for the always-up link
Posted by Leif Lindholm 7 years, 6 months ago 
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
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [platforms: PATCH v2 07/10] Marvell/Drivers: Pp2Dxe: Change settings for the always-up link
Posted by Marcin Wojtas 7 years, 6 months ago 
2017-10-27 16:36 GMT+02:00 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.
>

There is a 3rd case that if both are set to '1', autonegotiation is
forced, but we don't use it. I can even more simplify according to the
usecase (only 1 call in Pp2Dxe driver):

VOID
MvGop110GmacForceLinkUp(
  IN PP2DXE_PORT *Port
  )
{
  UINT32 RegVal;

  RegVal = MvGop110GmacRead (Port, MVPP2_PORT_AUTO_NEG_CFG_REG);

  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);
}

What do you think?

> If you flip those around (and make it like the 'else' clause):

Ok.

> 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
> >
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.