Patchwork net-bnx2x: Force link UP when the interface is in LOOPBACK mode

login
register
mail settings
Submitter Mahesh Bandewar
Date Jan. 30, 2013, 1:51 a.m.
Message ID <1359510715-12099-1-git-send-email-maheshb@google.com>
Download mbox | patch
Permalink /patch/216755/
State Superseded
Delegated to: David Miller
Headers show

Comments

Mahesh Bandewar - Jan. 30, 2013, 1:51 a.m.
When the interface does not have carrier but when it's put into
loopback mode (for tests), it does not make sense to not have
the carrier. So force it!

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c | 5 +++++
 1 file changed, 5 insertions(+)
Eilon Greenstein - Jan. 30, 2013, 10:24 a.m.
On Tue, 2013-01-29 at 17:51 -0800, Mahesh Bandewar wrote:
> When the interface does not have carrier but when it's put into
> loopback mode (for tests), it does not make sense to not have
> the carrier. So force it!
> 

Mahesh,

What is the motivation for this change? During the test, the device is
not really functional and cannot receive traffic to the loopback, so why
would you like to indicate internally that the link is up?

> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> ---
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
> index 859df751345e..91af55586bbc 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
> @@ -4748,6 +4748,11 @@ void bnx2x_link_status_update(struct link_params *params,
>  	vars->link_status = REG_RD(bp, params->shmem_base +
>  				   offsetof(struct shmem_region,
>  					    port_mb[port].link_status));
> +
> +	/* Force link UP in loopback mode */
> +	if (bp->link_params.loopback_mode != LOOPBACK_NONE)

The LOOPBACK_EXT requires an external loopback so we should not just
force link up indication for that kind of loopback.

> +		vars->link_status |= LINK_STATUS_LINK_UP;
> +
>  	if (bnx2x_eee_has_cap(params))
>  		vars->eee_status = REG_RD(bp, params->shmem2_base +
>  					  offsetof(struct shmem2_region,

Thanks,
Eilon


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mahesh Bandewar - Jan. 30, 2013, 4:52 p.m.
On Wed, Jan 30, 2013 at 2:24 AM, Eilon Greenstein <eilong@broadcom.com> wrote:
>
> On Tue, 2013-01-29 at 17:51 -0800, Mahesh Bandewar wrote:
> > When the interface does not have carrier but when it's put into
> > loopback mode (for tests), it does not make sense to not have
> > the carrier. So force it!
> >
>
> Mahesh,
>
> What is the motivation for this change? During the test, the device is
> not really functional and cannot receive traffic to the loopback, so why
> would you like to indicate internally that the link is up?
>
The device is capable of NETIF_F_LOOPBACK so when enabled, it does not
receive traffic from the wire but from the host-side it is functional.
Not having carrier is a problem in that scenario.

> > Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> > ---
> >  drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
> > index 859df751345e..91af55586bbc 100644
> > --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
> > +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
> > @@ -4748,6 +4748,11 @@ void bnx2x_link_status_update(struct link_params *params,
> >       vars->link_status = REG_RD(bp, params->shmem_base +
> >                                  offsetof(struct shmem_region,
> >                                           port_mb[port].link_status));
> > +
> > +     /* Force link UP in loopback mode */
> > +     if (bp->link_params.loopback_mode != LOOPBACK_NONE)
>
> The LOOPBACK_EXT requires an external loopback so we should not just
> force link up indication for that kind of loopback.
>
Yes, that's true! Let me correct this. I'll add a condition to check
that it's not EXT mode before forcing the link.
> > +             vars->link_status |= LINK_STATUS_LINK_UP;
> > +
> >       if (bnx2x_eee_has_cap(params))
> >               vars->eee_status = REG_RD(bp, params->shmem2_base +
> >                                         offsetof(struct shmem2_region,
>
> Thanks,
> Eilon
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
index 859df751345e..91af55586bbc 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
@@ -4748,6 +4748,11 @@  void bnx2x_link_status_update(struct link_params *params,
 	vars->link_status = REG_RD(bp, params->shmem_base +
 				   offsetof(struct shmem_region,
 					    port_mb[port].link_status));
+
+	/* Force link UP in loopback mode */
+	if (bp->link_params.loopback_mode != LOOPBACK_NONE)
+		vars->link_status |= LINK_STATUS_LINK_UP;
+
 	if (bnx2x_eee_has_cap(params))
 		vars->eee_status = REG_RD(bp, params->shmem2_base +
 					  offsetof(struct shmem2_region,