netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: ehea: fix wrongly-reported supported modes
@ 2011-05-03 19:42 Kleber Sacilotto de Souza
  2011-05-03 20:16 ` Ben Hutchings
  0 siblings, 1 reply; 4+ messages in thread
From: Kleber Sacilotto de Souza @ 2011-05-03 19:42 UTC (permalink / raw)
  To: netdev

Currently EHEA reports to ethtool as supporting 10000baseT_Full and
FIBRE independent of the hardware configuration. However, these
capabilities should be reported only if the physical port and
the medium support them, which is the case where the physical port
is connected at 10Gb.

Signed-off-by: Kleber Sacilotto de Souza <klebers@linux.vnet.ibm.com>
---
 drivers/net/ehea/ehea_ethtool.c |   21 ++++++++++++++-------
 1 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ehea/ehea_ethtool.c b/drivers/net/ehea/ehea_ethtool.c
index 3e2e734..04716c2 100644
--- a/drivers/net/ehea/ehea_ethtool.c
+++ b/drivers/net/ehea/ehea_ethtool.c
@@ -55,15 +55,22 @@ static int ehea_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 		cmd->duplex = -1;
 	}

-	cmd->supported = (SUPPORTED_10000baseT_Full | SUPPORTED_1000baseT_Full
-		       | SUPPORTED_100baseT_Full |  SUPPORTED_100baseT_Half
-		       | SUPPORTED_10baseT_Full | SUPPORTED_10baseT_Half
-		       | SUPPORTED_Autoneg | SUPPORTED_FIBRE);
+	cmd->supported = (SUPPORTED_1000baseT_Full | SUPPORTED_100baseT_Full
+		       | SUPPORTED_100baseT_Half | SUPPORTED_10baseT_Full
+		       | SUPPORTED_10baseT_Half | SUPPORTED_Autoneg);

-	cmd->advertising = (ADVERTISED_10000baseT_Full | ADVERTISED_Autoneg
-			 | ADVERTISED_FIBRE);
+	cmd->advertising = ADVERTISED_Autoneg;
+
+	if (cmd->speed == SPEED_10000) {
+		cmd->supported |= (SUPPORTED_10000baseT_Full | SUPPORTED_FIBRE);
+		cmd->advertising |= (ADVERTISED_10000baseT_Full | ADVERTISED_FIBRE);
+		cmd->port = PORT_FIBRE;
+	} else {
+		cmd->supported |= SUPPORTED_TP;
+		cmd->advertising |= (ADVERTISED_1000baseT_Full | ADVERTISED_TP);
+		cmd->port = PORT_TP;
+	}

-	cmd->port = PORT_FIBRE;
 	cmd->autoneg = port->autoneg == 1 ? AUTONEG_ENABLE : AUTONEG_DISABLE;

 	return 0;
-- 
1.7.1




^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] net: ehea: fix wrongly-reported supported modes
  2011-05-03 19:42 [PATCH] net: ehea: fix wrongly-reported supported modes Kleber Sacilotto de Souza
@ 2011-05-03 20:16 ` Ben Hutchings
  2011-05-03 21:34   ` Ben Hutchings
  0 siblings, 1 reply; 4+ messages in thread
From: Ben Hutchings @ 2011-05-03 20:16 UTC (permalink / raw)
  To: Kleber Sacilotto de Souza; +Cc: netdev

On Tue, 2011-05-03 at 16:42 -0300, Kleber Sacilotto de Souza wrote:
> Currently EHEA reports to ethtool as supporting 10000baseT_Full and
> FIBRE independent of the hardware configuration. However, these
> capabilities should be reported only if the physical port and
> the medium support them, which is the case where the physical port
> is connected at 10Gb.
>
> Signed-off-by: Kleber Sacilotto de Souza <klebers@linux.vnet.ibm.com>
> ---
>  drivers/net/ehea/ehea_ethtool.c |   21 ++++++++++++++-------
>  1 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ehea/ehea_ethtool.c b/drivers/net/ehea/ehea_ethtool.c
> index 3e2e734..04716c2 100644
> --- a/drivers/net/ehea/ehea_ethtool.c
> +++ b/drivers/net/ehea/ehea_ethtool.c
> @@ -55,15 +55,22 @@ static int ehea_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
>  		cmd->duplex = -1;
>  	}
> 
> -	cmd->supported = (SUPPORTED_10000baseT_Full | SUPPORTED_1000baseT_Full
> -		       | SUPPORTED_100baseT_Full |  SUPPORTED_100baseT_Half
> -		       | SUPPORTED_10baseT_Full | SUPPORTED_10baseT_Half
> -		       | SUPPORTED_Autoneg | SUPPORTED_FIBRE);
> +	cmd->supported = (SUPPORTED_1000baseT_Full | SUPPORTED_100baseT_Full
> +		       | SUPPORTED_100baseT_Half | SUPPORTED_10baseT_Full
> +		       | SUPPORTED_10baseT_Half | SUPPORTED_Autoneg);
> 
> -	cmd->advertising = (ADVERTISED_10000baseT_Full | ADVERTISED_Autoneg
> -			 | ADVERTISED_FIBRE);
> +	cmd->advertising = ADVERTISED_Autoneg;
> +
> +	if (cmd->speed == SPEED_10000) {
> +		cmd->supported |= (SUPPORTED_10000baseT_Full | SUPPORTED_FIBRE);
> +		cmd->advertising |= (ADVERTISED_10000baseT_Full | ADVERTISED_FIBRE);
> +		cmd->port = PORT_FIBRE;
> +	} else {
> +		cmd->supported |= SUPPORTED_TP;
> +		cmd->advertising |= (ADVERTISED_1000baseT_Full | ADVERTISED_TP);
> +		cmd->port = PORT_TP;
> +	}

This doesn't make any sense.  If the current speed is 10G, then the
driver also claims to support speeds of 10M, 100M, 1G and 10G.  But then
if the speed actually is <10G, the driver claims to support TP.  What's
going on here?

(Also, claiming to support BASE-T modes on non-TP media is bogus, though
I understand why people are doing it.)

Ben.

> -	cmd->port = PORT_FIBRE;
>  	cmd->autoneg = port->autoneg == 1 ? AUTONEG_ENABLE : AUTONEG_DISABLE;
> 
>  	return 0;

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] net: ehea: fix wrongly-reported supported modes
  2011-05-03 20:16 ` Ben Hutchings
@ 2011-05-03 21:34   ` Ben Hutchings
  2011-05-04 21:59     ` Kleber Sacilotto de Souza
  0 siblings, 1 reply; 4+ messages in thread
From: Ben Hutchings @ 2011-05-03 21:34 UTC (permalink / raw)
  To: Kleber Sacilotto de Souza; +Cc: netdev

On Tue, 2011-05-03 at 21:16 +0100, Ben Hutchings wrote:
> On Tue, 2011-05-03 at 16:42 -0300, Kleber Sacilotto de Souza wrote:
> > Currently EHEA reports to ethtool as supporting 10000baseT_Full and
> > FIBRE independent of the hardware configuration. However, these
> > capabilities should be reported only if the physical port and
> > the medium support them, which is the case where the physical port
> > is connected at 10Gb.
> >
> > Signed-off-by: Kleber Sacilotto de Souza <klebers@linux.vnet.ibm.com>
> > ---
> >  drivers/net/ehea/ehea_ethtool.c |   21 ++++++++++++++-------
> >  1 files changed, 14 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/net/ehea/ehea_ethtool.c b/drivers/net/ehea/ehea_ethtool.c
> > index 3e2e734..04716c2 100644
> > --- a/drivers/net/ehea/ehea_ethtool.c
> > +++ b/drivers/net/ehea/ehea_ethtool.c
> > @@ -55,15 +55,22 @@ static int ehea_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> >  		cmd->duplex = -1;
> >  	}
> > 
> > -	cmd->supported = (SUPPORTED_10000baseT_Full | SUPPORTED_1000baseT_Full
> > -		       | SUPPORTED_100baseT_Full |  SUPPORTED_100baseT_Half
> > -		       | SUPPORTED_10baseT_Full | SUPPORTED_10baseT_Half
> > -		       | SUPPORTED_Autoneg | SUPPORTED_FIBRE);
> > +	cmd->supported = (SUPPORTED_1000baseT_Full | SUPPORTED_100baseT_Full
> > +		       | SUPPORTED_100baseT_Half | SUPPORTED_10baseT_Full
> > +		       | SUPPORTED_10baseT_Half | SUPPORTED_Autoneg);
> > 
> > -	cmd->advertising = (ADVERTISED_10000baseT_Full | ADVERTISED_Autoneg
> > -			 | ADVERTISED_FIBRE);
> > +	cmd->advertising = ADVERTISED_Autoneg;
> > +
> > +	if (cmd->speed == SPEED_10000) {
> > +		cmd->supported |= (SUPPORTED_10000baseT_Full | SUPPORTED_FIBRE);
> > +		cmd->advertising |= (ADVERTISED_10000baseT_Full | ADVERTISED_FIBRE);
> > +		cmd->port = PORT_FIBRE;
> > +	} else {
> > +		cmd->supported |= SUPPORTED_TP;
> > +		cmd->advertising |= (ADVERTISED_1000baseT_Full | ADVERTISED_TP);
> > +		cmd->port = PORT_TP;
> > +	}
> 
> This doesn't make any sense.  If the current speed is 10G, then the
> driver also claims to support speeds of 10M, 100M, 1G and 10G.  But then
                                                               ^
                                                           on fibre

> if the speed actually is <10G, the driver claims to support TP.  What's
> going on here?
> 
> (Also, claiming to support BASE-T modes on non-TP media is bogus, though
> I understand why people are doing it.)
> 
> Ben.
> 
> > -	cmd->port = PORT_FIBRE;
> >  	cmd->autoneg = port->autoneg == 1 ? AUTONEG_ENABLE : AUTONEG_DISABLE;
> > 
> >  	return 0;
> 

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] net: ehea: fix wrongly-reported supported modes
  2011-05-03 21:34   ` Ben Hutchings
@ 2011-05-04 21:59     ` Kleber Sacilotto de Souza
  0 siblings, 0 replies; 4+ messages in thread
From: Kleber Sacilotto de Souza @ 2011-05-04 21:59 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev

On Tue, 2011-05-03 at 22:34 +0100, Ben Hutchings wrote:
> On Tue, 2011-05-03 at 21:16 +0100, Ben Hutchings wrote:
> > On Tue, 2011-05-03 at 16:42 -0300, Kleber Sacilotto de Souza wrote:
> > > Currently EHEA reports to ethtool as supporting 10000baseT_Full and
> > > FIBRE independent of the hardware configuration. However, these
> > > capabilities should be reported only if the physical port and
> > > the medium support them, which is the case where the physical port
> > > is connected at 10Gb.
> > >
> > > Signed-off-by: Kleber Sacilotto de Souza <klebers@linux.vnet.ibm.com>
> > > ---
> > >  drivers/net/ehea/ehea_ethtool.c |   21 ++++++++++++++-------
> > >  1 files changed, 14 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/net/ehea/ehea_ethtool.c b/drivers/net/ehea/ehea_ethtool.c
> > > index 3e2e734..04716c2 100644
> > > --- a/drivers/net/ehea/ehea_ethtool.c
> > > +++ b/drivers/net/ehea/ehea_ethtool.c
> > > @@ -55,15 +55,22 @@ static int ehea_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> > >  		cmd->duplex = -1;
> > >  	}
> > > 
> > > -	cmd->supported = (SUPPORTED_10000baseT_Full | SUPPORTED_1000baseT_Full
> > > -		       | SUPPORTED_100baseT_Full |  SUPPORTED_100baseT_Half
> > > -		       | SUPPORTED_10baseT_Full | SUPPORTED_10baseT_Half
> > > -		       | SUPPORTED_Autoneg | SUPPORTED_FIBRE);
> > > +	cmd->supported = (SUPPORTED_1000baseT_Full | SUPPORTED_100baseT_Full
> > > +		       | SUPPORTED_100baseT_Half | SUPPORTED_10baseT_Full
> > > +		       | SUPPORTED_10baseT_Half | SUPPORTED_Autoneg);
> > > 
> > > -	cmd->advertising = (ADVERTISED_10000baseT_Full | ADVERTISED_Autoneg
> > > -			 | ADVERTISED_FIBRE);
> > > +	cmd->advertising = ADVERTISED_Autoneg;
> > > +
> > > +	if (cmd->speed == SPEED_10000) {
> > > +		cmd->supported |= (SUPPORTED_10000baseT_Full | SUPPORTED_FIBRE);
> > > +		cmd->advertising |= (ADVERTISED_10000baseT_Full | ADVERTISED_FIBRE);
> > > +		cmd->port = PORT_FIBRE;
> > > +	} else {
> > > +		cmd->supported |= SUPPORTED_TP;
> > > +		cmd->advertising |= (ADVERTISED_1000baseT_Full | ADVERTISED_TP);
> > > +		cmd->port = PORT_TP;
> > > +	}
> > 
> > This doesn't make any sense.  If the current speed is 10G, then the
> > driver also claims to support speeds of 10M, 100M, 1G and 10G.  But then
>                                                                ^
>                                                            on fibre
> 
> > if the speed actually is <10G, the driver claims to support TP.  What's
> > going on here?

You are right. This patch was based on very vague hardware specs that
wasn't making much sense. I have more details about the hardware now, I
will send another patch soon.

> > 
> > (Also, claiming to support BASE-T modes on non-TP media is bogus, though
> > I understand why people are doing it.)
> > 
> > Ben.
> > 
> > > -	cmd->port = PORT_FIBRE;
> > >  	cmd->autoneg = port->autoneg == 1 ? AUTONEG_ENABLE : AUTONEG_DISABLE;
> > > 
> > >  	return 0;
> > 
> 

-- 
Kleber S. Souza
IBM Linux Technology Center


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-05-04 21:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-03 19:42 [PATCH] net: ehea: fix wrongly-reported supported modes Kleber Sacilotto de Souza
2011-05-03 20:16 ` Ben Hutchings
2011-05-03 21:34   ` Ben Hutchings
2011-05-04 21:59     ` Kleber Sacilotto de Souza

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).