Re: [PATCH] NET/PHY: Eliminate the forced speed reduction algorithm

2013-02-25 Thread David Miller
From: Kirill Kapranov 
Date: Mon, 25 Feb 2013 16:25:18 +0400


To be completely honest with you, I'm really getting tired of
making corrections to this one simple patch submission.

> From Kirill Kapranov  ,

It makes no sense to mention multiple email addresses in your
authorship line, reduce it to one.

If you cannot do things like this correctly on your own, have
automated tools (such as git) generate these patch emails for you.

> NET/PHY: Eliminate the forced speed reduction algorithm.

You do not need to mention the commit header line again, all
this does is make more work for me as I have to edit it out.
To one in the Subject line is sufficient, and you should not
use all-CAPS, and also the subsystem indication is not correct.
Your subject line should be something like:

"[PATCH] phy: Eliminate the forced speed reduction algorithm"

That is, remove the "NET/" part, leave "PHY" and make it all
lowercase.

> In case of fixed speed set up for a NIC (e.g. ethtool -s eth0 autoneg off 
> speed 100 duplex full) with an ethernet cable plugged off, the mentioned 
> algorithm slows down a NIC speed, so further hooking up a cable does not lead 
> to "link" state.

I also had to formate this paragraph down to 80 column lines instead
of one long one.

> Signed-off-by: Kirill Kapranov ,

Again, using two email address is not correct, just use one.

Please, do yourself a huge favor, look at how other people submit
accepted patches on this list.  Learn by example rather than making
many mistakes trying to figure things out purely on your own.

> I have looked up RFC-802.3, and found, that the mentioned algorithm is 
> neither quoted nor described. AFAIK, no one RFC describe the mentioned 
> algorithm, so it may be a witty invention of the developer(s).

At the time that autonegiation was a new or non-existing feature, this
approach of stepping down the link parameters trying different
settings one-by-one was an absolute necessity.  It's probably not
needed anymore in modern times.

> Tested at 2.6.38.7, applicable up to for 3.0.4. 

This patch does not apply to current upstream sources at all.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] NET/PHY: Eliminate the forced speed reduction algorithm

2013-02-25 Thread Kirill Kapranov
>From Kirill Kapranov  ,

NET/PHY: Eliminate the forced speed reduction algorithm.
In case of fixed speed set up for a NIC (e.g. ethtool -s eth0 autoneg off speed 
100 duplex full) with an ethernet cable plugged off, the mentioned algorithm 
slows down a NIC speed, so further hooking up a cable does not lead to "link" 
state.
Signed-off-by: Kirill Kapranov ,
---
The purpose of the introduced patch is deletion of the forced speed reduction 
algorithm realization from the driver module "phy".
The above mentioned algorithm works in the following way: if the PHY detected 
unlink line state (connector plugged off), NIC speed is decreased step-by-step 
in the sequence:
100 full duplex
100 half duplex
10 full duplex
10 half duplex
with the latency circa 10 s per step, and stops at 10-HD value.
I have looked up RFC-802.3, and found, that the mentioned algorithm is neither 
quoted nor described. AFAIK, no one RFC describe the mentioned algorithm, so it 
may be a witty invention of the developer(s).

In the case of the fixed speed and duplex set, with the autonegotiation off, 
for a NIC (e.g. # ethtool -s eth0 autoneg off speed 100 duplex full) with 
ethernet cable plugged off, mentioned algorithm slows down NIC speed, so when 
ethernet connector is plugged in, connection will be inoperative: an ethernet 
switch will try to connect with 100/full (e.g.), a NIC will stay at 10/half.
Thus, this algorithm is destructive for the fixed speed/duplex mode (with 
autonegotiation off).

In the AUTO mode, the mentioned algorithm is inessential. The autonegotiation 
procedure works fine regardless an speed/duplex settings at the moment of 
connector hooking up.
Thus, there is no point in using of this algorithm in driver.

Tested at 2.6.38.7, applicable up to for 3.0.4. 

--- linux/drivers/net/phy/phy.c.orig2011-05-22 02:13:59.0 +0400
+++ linux/drivers/net/phy/phy.c 2012-04-28 12:49:37.0 +0400
@@ -457,34 +457,6 @@ void phy_stop_machine(struct phy_device
 }
 
 /**
- * phy_force_reduction - reduce PHY speed/duplex settings by one step
- * @phydev: target phy_device struct
- *
- * Description: Reduces the speed/duplex settings by one notch,
- *   in this order--
- *   1000/FULL, 1000/HALF, 100/FULL, 100/HALF, 10/FULL, 10/HALF.
- *   The function bottoms out at 10/HALF.
- */
-static void phy_force_reduction(struct phy_device *phydev)
-{
-   int idx;
-
-   idx = phy_find_setting(phydev->speed, phydev->duplex);
-   
-   idx++;
-
-   idx = phy_find_valid(idx, phydev->supported);
-
-   phydev->speed = settings[idx].speed;
-   phydev->duplex = settings[idx].duplex;
-
-   pr_info("Trying %d/%s\n", phydev->speed,
-   DUPLEX_FULL == phydev->duplex ?
-   "FULL" : "HALF");
-}
-
-
-/**
  * phy_error - enter HALTED state for this PHY device
  * @phydev: target phy_device struct
  *
@@ -814,30 +786,12 @@ void phy_state_machine(struct work_struc
phydev->adjust_link(phydev->attached_dev);
 
} else if (0 == phydev->link_timeout--) {
-   int idx;
 
needs_aneg = 1;
/* If we have the magic_aneg bit,
 * we try again */
if (phydev->drv->flags & PHY_HAS_MAGICANEG)
break;
-
-   /* The timer expired, and we still
-* don't have a setting, so we try
-* forcing it until we find one that
-* works, starting from the fastest speed,
-* and working our way down */
-   idx = phy_find_valid(0, phydev->supported);
-
-   phydev->speed = settings[idx].speed;
-   phydev->duplex = settings[idx].duplex;
-
-   phydev->autoneg = AUTONEG_DISABLE;
-
-   pr_info("Trying %d/%s\n", phydev->speed,
-   DUPLEX_FULL ==
-   phydev->duplex ?
-   "FULL" : "HALF");
}
break;
case PHY_NOLINK:
@@ -863,7 +817,6 @@ void phy_state_machine(struct work_struc
netif_carrier_on(phydev->attached_dev);
} else {
if (0 == phydev->link_timeout--) {
-   phy_force_reduction(phydev);
needs_aneg = 1;
}
}


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

[PATCH] NET/PHY: Eliminate the forced speed reduction algorithm

2013-02-25 Thread Kirill Kapranov
From Kirill Kapranov  k...@nita.ru,kapran...@inbox.ru

NET/PHY: Eliminate the forced speed reduction algorithm.
In case of fixed speed set up for a NIC (e.g. ethtool -s eth0 autoneg off speed 
100 duplex full) with an ethernet cable plugged off, the mentioned algorithm 
slows down a NIC speed, so further hooking up a cable does not lead to link 
state.
Signed-off-by: Kirill Kapranov k...@nita.ru,kapran...@inbox.ru
---
The purpose of the introduced patch is deletion of the forced speed reduction 
algorithm realization from the driver module phy.
The above mentioned algorithm works in the following way: if the PHY detected 
unlink line state (connector plugged off), NIC speed is decreased step-by-step 
in the sequence:
100 full duplex
100 half duplex
10 full duplex
10 half duplex
with the latency circa 10 s per step, and stops at 10-HD value.
I have looked up RFC-802.3, and found, that the mentioned algorithm is neither 
quoted nor described. AFAIK, no one RFC describe the mentioned algorithm, so it 
may be a witty invention of the developer(s).

In the case of the fixed speed and duplex set, with the autonegotiation off, 
for a NIC (e.g. # ethtool -s eth0 autoneg off speed 100 duplex full) with 
ethernet cable plugged off, mentioned algorithm slows down NIC speed, so when 
ethernet connector is plugged in, connection will be inoperative: an ethernet 
switch will try to connect with 100/full (e.g.), a NIC will stay at 10/half.
Thus, this algorithm is destructive for the fixed speed/duplex mode (with 
autonegotiation off).

In the AUTO mode, the mentioned algorithm is inessential. The autonegotiation 
procedure works fine regardless an speed/duplex settings at the moment of 
connector hooking up.
Thus, there is no point in using of this algorithm in driver.

Tested at 2.6.38.7, applicable up to for 3.0.4. 

--- linux/drivers/net/phy/phy.c.orig2011-05-22 02:13:59.0 +0400
+++ linux/drivers/net/phy/phy.c 2012-04-28 12:49:37.0 +0400
@@ -457,34 +457,6 @@ void phy_stop_machine(struct phy_device
 }
 
 /**
- * phy_force_reduction - reduce PHY speed/duplex settings by one step
- * @phydev: target phy_device struct
- *
- * Description: Reduces the speed/duplex settings by one notch,
- *   in this order--
- *   1000/FULL, 1000/HALF, 100/FULL, 100/HALF, 10/FULL, 10/HALF.
- *   The function bottoms out at 10/HALF.
- */
-static void phy_force_reduction(struct phy_device *phydev)
-{
-   int idx;
-
-   idx = phy_find_setting(phydev-speed, phydev-duplex);
-   
-   idx++;
-
-   idx = phy_find_valid(idx, phydev-supported);
-
-   phydev-speed = settings[idx].speed;
-   phydev-duplex = settings[idx].duplex;
-
-   pr_info(Trying %d/%s\n, phydev-speed,
-   DUPLEX_FULL == phydev-duplex ?
-   FULL : HALF);
-}
-
-
-/**
  * phy_error - enter HALTED state for this PHY device
  * @phydev: target phy_device struct
  *
@@ -814,30 +786,12 @@ void phy_state_machine(struct work_struc
phydev-adjust_link(phydev-attached_dev);
 
} else if (0 == phydev-link_timeout--) {
-   int idx;
 
needs_aneg = 1;
/* If we have the magic_aneg bit,
 * we try again */
if (phydev-drv-flags  PHY_HAS_MAGICANEG)
break;
-
-   /* The timer expired, and we still
-* don't have a setting, so we try
-* forcing it until we find one that
-* works, starting from the fastest speed,
-* and working our way down */
-   idx = phy_find_valid(0, phydev-supported);
-
-   phydev-speed = settings[idx].speed;
-   phydev-duplex = settings[idx].duplex;
-
-   phydev-autoneg = AUTONEG_DISABLE;
-
-   pr_info(Trying %d/%s\n, phydev-speed,
-   DUPLEX_FULL ==
-   phydev-duplex ?
-   FULL : HALF);
}
break;
case PHY_NOLINK:
@@ -863,7 +817,6 @@ void phy_state_machine(struct work_struc
netif_carrier_on(phydev-attached_dev);
} else {
if (0 == phydev-link_timeout--) {
-   phy_force_reduction(phydev);
needs_aneg = 1;
}
}


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo 

Re: [PATCH] NET/PHY: Eliminate the forced speed reduction algorithm

2013-02-25 Thread David Miller
From: Kirill Kapranov kapran...@inbox.ru
Date: Mon, 25 Feb 2013 16:25:18 +0400


To be completely honest with you, I'm really getting tired of
making corrections to this one simple patch submission.

 From Kirill Kapranov  k...@nita.ru,kapran...@inbox.ru

It makes no sense to mention multiple email addresses in your
authorship line, reduce it to one.

If you cannot do things like this correctly on your own, have
automated tools (such as git) generate these patch emails for you.

 NET/PHY: Eliminate the forced speed reduction algorithm.

You do not need to mention the commit header line again, all
this does is make more work for me as I have to edit it out.
To one in the Subject line is sufficient, and you should not
use all-CAPS, and also the subsystem indication is not correct.
Your subject line should be something like:

[PATCH] phy: Eliminate the forced speed reduction algorithm

That is, remove the NET/ part, leave PHY and make it all
lowercase.

 In case of fixed speed set up for a NIC (e.g. ethtool -s eth0 autoneg off 
 speed 100 duplex full) with an ethernet cable plugged off, the mentioned 
 algorithm slows down a NIC speed, so further hooking up a cable does not lead 
 to link state.

I also had to formate this paragraph down to 80 column lines instead
of one long one.

 Signed-off-by: Kirill Kapranov k...@nita.ru,kapran...@inbox.ru

Again, using two email address is not correct, just use one.

Please, do yourself a huge favor, look at how other people submit
accepted patches on this list.  Learn by example rather than making
many mistakes trying to figure things out purely on your own.

 I have looked up RFC-802.3, and found, that the mentioned algorithm is 
 neither quoted nor described. AFAIK, no one RFC describe the mentioned 
 algorithm, so it may be a witty invention of the developer(s).

At the time that autonegiation was a new or non-existing feature, this
approach of stepping down the link parameters trying different
settings one-by-one was an absolute necessity.  It's probably not
needed anymore in modern times.

 Tested at 2.6.38.7, applicable up to for 3.0.4. 

This patch does not apply to current upstream sources at all.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] NET/PHY: Eliminate the forced speed reduction algorithm.

2013-02-19 Thread David Miller
From: Kirill Kapranov 
Date: Tue, 19 Feb 2013 13:53:48 +0400

> Tested at 2.6.38.7, applicable up to for 3.0.4. 
> Signed-off-by: Kirill Kapranov ,
>  --- linux/drivers/net/phy/phy.c.orig 2011-05-22 02:13:59.0 +0400
> +++ linux/drivers/net/phy/phy.c   2012-04-28 12:49:37.0 +0400

Your patches are continually poorly formatted, and corrupted by
your email client, which means that the patches cannot be applied
properly and all of our automated tools for patch tracking do not
recognize your submissions as a patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] NET/PHY: Eliminate the forced speed reduction algorithm.

2013-02-19 Thread Kirill Kapranov
NET/PHY: Eliminate the forced speed reduction algorithm.

The purpose of the introduced patch is deletion of the forced speed
reduction algorithm realisation from the driver module "phy".

The above mentioned algorithm works in the following way: if the phy
detected unlink line state (connector plugged off), NIC speed is
decreased step-by-step in the sequence:
100 full duplex
100 half duplex
10 full duplex
10 half duplex
with the latency circa 10 s per step, and stops at 10-HD value.
I have looked up RFC-802.3, and found, that the mentioned algorithm is
neither quoted nor described. AFAIK, no one RFC describe the mentioned
algorithm, so it may be a witty invention of the developer(s).

In the case of the fixed speed and duplex set, with the autonegotiation
off, for a NIC (e.g. # ethtool -s eth0 autoneg off speed 100 duplex
full) with ethernet cable plugged off, mentioned algorithm slows down
NIC speed, so when ethernet connector is plugged in, connection will be
inoperative: an ethernet switch will try to connect with 100/full
(e.g.), a NIC will stay at 10/half.
Thus, this algorithm is destructive for the fixed speed/duplex mode
(with autonegotiation off).

In the AUTO mode, the mentioned algorithm is inessential. The
autonegotiation procedure works fine regardless an speed/duplex settings
at the moment of connector hooking up.
Thus, there is no point in using of this algorithm in driver.

Thanks a lot Francois Romieu and David Miller for very constructive
advises.

Tested at 2.6.38.7, applicable up to for 3.0.4. 
Signed-off-by: Kirill Kapranov ,
 --- linux/drivers/net/phy/phy.c.orig   2011-05-22 02:13:59.0 +0400
+++ linux/drivers/net/phy/phy.c 2012-04-28 12:49:37.0 +0400
@@ -457,34 +457,6 @@ void phy_stop_machine(struct phy_device
 }
 
 /**
- * phy_force_reduction - reduce PHY speed/duplex settings by one step
- * @phydev: target phy_device struct
- *
- * Description: Reduces the speed/duplex settings by one notch,
- *   in this order--
- *   1000/FULL, 1000/HALF, 100/FULL, 100/HALF, 10/FULL, 10/HALF.
- *   The function bottoms out at 10/HALF.
- */
-static void phy_force_reduction(struct phy_device *phydev)
-{
-   int idx;
-
-   idx = phy_find_setting(phydev->speed, phydev->duplex);
-   
-   idx++;
-
-   idx = phy_find_valid(idx, phydev->supported);
-
-   phydev->speed = settings[idx].speed;
-   phydev->duplex = settings[idx].duplex;
-
-   pr_info("Trying %d/%s\n", phydev->speed,
-   DUPLEX_FULL == phydev->duplex ?
-   "FULL" : "HALF");
-}
-
-
-/**
  * phy_error - enter HALTED state for this PHY device
  * @phydev: target phy_device struct
  *
@@ -814,30 +786,12 @@ void phy_state_machine(struct work_struc
phydev->adjust_link(phydev->attached_dev);
 
} else if (0 == phydev->link_timeout--) {
-   int idx;
 
needs_aneg = 1;
/* If we have the magic_aneg bit,
 * we try again */
if (phydev->drv->flags & PHY_HAS_MAGICANEG)
break;
-
-   /* The timer expired, and we still
-* don't have a setting, so we try
-* forcing it until we find one that
-* works, starting from the fastest speed,
-* and working our way down */
-   idx = phy_find_valid(0, phydev->supported);
-
-   phydev->speed = settings[idx].speed;
-   phydev->duplex = settings[idx].duplex;
-
-   phydev->autoneg = AUTONEG_DISABLE;
-
-   pr_info("Trying %d/%s\n", phydev->speed,
-   DUPLEX_FULL ==
-   phydev->duplex ?
-   "FULL" : "HALF");
}
break;
case PHY_NOLINK:
@@ -863,7 +817,6 @@ void phy_state_machine(struct work_struc
netif_carrier_on(phydev->attached_dev);
} else {
if (0 == phydev->link_timeout--) {
-   phy_force_reduction(phydev);
needs_aneg = 1;
}
}



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] NET/PHY: Eliminate the forced speed reduction algorithm.

2013-02-19 Thread Kirill Kapranov
NET/PHY: Eliminate the forced speed reduction algorithm.

The purpose of the introduced patch is deletion of the forced speed
reduction algorithm realisation from the driver module phy.

The above mentioned algorithm works in the following way: if the phy
detected unlink line state (connector plugged off), NIC speed is
decreased step-by-step in the sequence:
100 full duplex
100 half duplex
10 full duplex
10 half duplex
with the latency circa 10 s per step, and stops at 10-HD value.
I have looked up RFC-802.3, and found, that the mentioned algorithm is
neither quoted nor described. AFAIK, no one RFC describe the mentioned
algorithm, so it may be a witty invention of the developer(s).

In the case of the fixed speed and duplex set, with the autonegotiation
off, for a NIC (e.g. # ethtool -s eth0 autoneg off speed 100 duplex
full) with ethernet cable plugged off, mentioned algorithm slows down
NIC speed, so when ethernet connector is plugged in, connection will be
inoperative: an ethernet switch will try to connect with 100/full
(e.g.), a NIC will stay at 10/half.
Thus, this algorithm is destructive for the fixed speed/duplex mode
(with autonegotiation off).

In the AUTO mode, the mentioned algorithm is inessential. The
autonegotiation procedure works fine regardless an speed/duplex settings
at the moment of connector hooking up.
Thus, there is no point in using of this algorithm in driver.

Thanks a lot Francois Romieu and David Miller for very constructive
advises.

Tested at 2.6.38.7, applicable up to for 3.0.4. 
Signed-off-by: Kirill Kapranov k...@nita.ru,kapran...@inbox.ru
 --- linux/drivers/net/phy/phy.c.orig   2011-05-22 02:13:59.0 +0400
+++ linux/drivers/net/phy/phy.c 2012-04-28 12:49:37.0 +0400
@@ -457,34 +457,6 @@ void phy_stop_machine(struct phy_device
 }
 
 /**
- * phy_force_reduction - reduce PHY speed/duplex settings by one step
- * @phydev: target phy_device struct
- *
- * Description: Reduces the speed/duplex settings by one notch,
- *   in this order--
- *   1000/FULL, 1000/HALF, 100/FULL, 100/HALF, 10/FULL, 10/HALF.
- *   The function bottoms out at 10/HALF.
- */
-static void phy_force_reduction(struct phy_device *phydev)
-{
-   int idx;
-
-   idx = phy_find_setting(phydev-speed, phydev-duplex);
-   
-   idx++;
-
-   idx = phy_find_valid(idx, phydev-supported);
-
-   phydev-speed = settings[idx].speed;
-   phydev-duplex = settings[idx].duplex;
-
-   pr_info(Trying %d/%s\n, phydev-speed,
-   DUPLEX_FULL == phydev-duplex ?
-   FULL : HALF);
-}
-
-
-/**
  * phy_error - enter HALTED state for this PHY device
  * @phydev: target phy_device struct
  *
@@ -814,30 +786,12 @@ void phy_state_machine(struct work_struc
phydev-adjust_link(phydev-attached_dev);
 
} else if (0 == phydev-link_timeout--) {
-   int idx;
 
needs_aneg = 1;
/* If we have the magic_aneg bit,
 * we try again */
if (phydev-drv-flags  PHY_HAS_MAGICANEG)
break;
-
-   /* The timer expired, and we still
-* don't have a setting, so we try
-* forcing it until we find one that
-* works, starting from the fastest speed,
-* and working our way down */
-   idx = phy_find_valid(0, phydev-supported);
-
-   phydev-speed = settings[idx].speed;
-   phydev-duplex = settings[idx].duplex;
-
-   phydev-autoneg = AUTONEG_DISABLE;
-
-   pr_info(Trying %d/%s\n, phydev-speed,
-   DUPLEX_FULL ==
-   phydev-duplex ?
-   FULL : HALF);
}
break;
case PHY_NOLINK:
@@ -863,7 +817,6 @@ void phy_state_machine(struct work_struc
netif_carrier_on(phydev-attached_dev);
} else {
if (0 == phydev-link_timeout--) {
-   phy_force_reduction(phydev);
needs_aneg = 1;
}
}



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] NET/PHY: Eliminate the forced speed reduction algorithm.

2013-02-19 Thread David Miller
From: Kirill Kapranov kapran...@inbox.ru
Date: Tue, 19 Feb 2013 13:53:48 +0400

 Tested at 2.6.38.7, applicable up to for 3.0.4. 
 Signed-off-by: Kirill Kapranov k...@nita.ru,kapran...@inbox.ru
  --- linux/drivers/net/phy/phy.c.orig 2011-05-22 02:13:59.0 +0400
 +++ linux/drivers/net/phy/phy.c   2012-04-28 12:49:37.0 +0400

Your patches are continually poorly formatted, and corrupted by
your email client, which means that the patches cannot be applied
properly and all of our automated tools for patch tracking do not
recognize your submissions as a patch.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/