diff mbox

[3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked

Message ID 1418632754-16698-4-git-send-email-Yanjun.Zhu@windriver.com
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show

Commit Message

Zhu Yanjun Dec. 15, 2014, 8:39 a.m. UTC 
2.6.x kernels require a similar logic change as commit b7d6e335
[e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked]
introduces for newer kernels.

When PHY reset is intentionally blocked on 82577/8/9, do not toggle the
LANPHYPC value bit (essentially performing a hard power reset of the
device) otherwise the PHY can be put into an unknown state.

Cleanup whitespace in the same function.

[yanjun.zhu: whitespace remains unchanged]

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
---
 drivers/net/e1000e/ich8lan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Sergei Shtylyov Dec. 15, 2014, 12:01 p.m. UTC | #1 
Hello.

On 12/15/2014 11:39 AM, Zhu Yanjun wrote:

> 2.6.x kernels require a similar logic change as commit b7d6e335
> [e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked]
> introduces for newer kernels.

    Hm, so is this patch to 2.6.x-stable kernels or a recent kernel?
If the former, you should follow the rules in 
Documentation/stable_kernel_rules.txt.

> When PHY reset is intentionally blocked on 82577/8/9, do not toggle the
> LANPHYPC value bit (essentially performing a hard power reset of the
> device) otherwise the PHY can be put into an unknown state.

> Cleanup whitespace in the same function.

> [yanjun.zhu: whitespace remains unchanged]

> Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>

    So, is this your patch, or Bruce's? If the latter, you should add:

From: Bruce Allan <bruce.w.allan@intel.com>

at the start of the change log.

> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>

WBR, Sergei

--
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
Willy Tarreau Dec. 15, 2014, 12:16 p.m. UTC | #2 
Hello,

On Mon, Dec 15, 2014 at 03:01:48PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 12/15/2014 11:39 AM, Zhu Yanjun wrote:
> 
> >2.6.x kernels require a similar logic change as commit b7d6e335
> >[e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked]
> >introduces for newer kernels.
> 
>    Hm, so is this patch to 2.6.x-stable kernels or a recent kernel?
> If the former, you should follow the rules in 
> Documentation/stable_kernel_rules.txt.

I don't see anything there that does not comply with the rules. This
patchset looks fine and acceptable to me, I'll just wait a bit so that
if either Jeff or Bruce rejects it I respect their wish.

However you just made me realize that I can't find commit b7d6e335.
Zhu, please double-check that this commit (or an equivalent) was merged
upstream, *this* it a prerequisite for going into -stable.

> >When PHY reset is intentionally blocked on 82577/8/9, do not toggle the
> >LANPHYPC value bit (essentially performing a hard power reset of the
> >device) otherwise the PHY can be put into an unknown state.
> 
> >Cleanup whitespace in the same function.
> 
> >[yanjun.zhu: whitespace remains unchanged]
> 
> >Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
> 
>    So, is this your patch, or Bruce's? If the latter, you should add:
> 
> From: Bruce Allan <bruce.w.allan@intel.com>
> 
> at the start of the change log.
>
> >Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> >Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> >Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>

The case where a commit gets backported is always ambiguous, especially
when the context changes a lot. I'm personally in favor of keeping the
original signed-of-by chain related to the original fix, and adding some
text between it and the backporter's s-o-b indicating that the changes
were, so that original authors do not get blamed for mistakes.

Best regards,
Willy

--
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
yzhu1 Dec. 15, 2014, 1:21 p.m. UTC | #3 
Hi, Willy

Thanks for your reply.

This patch "[PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked" exists in tag v3.1.

Please follow these steps, this patch can be found:

1. git checkout -f v3.1

2. git log -p drivers/net/e1000e/ich8lan.c

3. search "b7d6e335"

Then we will find this patch.

Best Regards!
Zhu Yanjun
Willy Tarreau Dec. 15, 2014, 1:33 p.m. UTC | #4 
On Mon, Dec 15, 2014 at 01:21:43PM +0000, Zhu, Yanjun wrote:
> Hi, Willy
> 
> Thanks for your reply.
> 
> This patch "[PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked" exists in tag v3.1.
> 
> Please follow these steps, this patch can be found:
> 
> 1. git checkout -f v3.1
> 
> 2. git log -p drivers/net/e1000e/ich8lan.c
> 
> 3. search "b7d6e335"
> 
> Then we will find this patch.

Ah it's because you truncated the commit ID from the right instead of from
the left. Truncated commit IDs are valid from the left, not from the right.
In your case, the commit is 6cc7aaed70c96c3933fbacbad582fc79b7d6e335
("e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked"), so
the truncated ID is 6cc7aae, not b7d6e335. It's important to fix that in
your commit messages so that a "git show" works correctly (it failed for me
for this precise reason).

Thanks,
Willy

--
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
yzhu1 Dec. 16, 2014, 2:08 a.m. UTC | #5 
Hi, Willy

Thanks for your reply.

I will fix it and send V2.

Best Regards!
Zhu Yanjun
On 12/15/2014 09:33 PM, Willy Tarreau wrote:
> On Mon, Dec 15, 2014 at 01:21:43PM +0000, Zhu, Yanjun wrote:
>> Hi, Willy
>>
>> Thanks for your reply.
>>
>> This patch "[PATCH 3/5] e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked" exists in tag v3.1.
>>
>> Please follow these steps, this patch can be found:
>>
>> 1. git checkout -f v3.1
>>
>> 2. git log -p drivers/net/e1000e/ich8lan.c
>>
>> 3. search "b7d6e335"
>>
>> Then we will find this patch.
> Ah it's because you truncated the commit ID from the right instead of from
> the left. Truncated commit IDs are valid from the left, not from the right.
> In your case, the commit is 6cc7aaed70c96c3933fbacbad582fc79b7d6e335
> ("e1000e: do not toggle LANPHYPC value bit when PHY reset is blocked"), so
> the truncated ID is 6cc7aae, not b7d6e335. It's important to fix that in
> your commit messages so that a "git show" works correctly (it failed for me
> for this precise reason).
>
> Thanks,
> Willy
>

--
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
diff mbox

Patch

diff --git a/drivers/net/e1000e/ich8lan.c b/drivers/net/e1000e/ich8lan.c
index c4b2d15..8c7e4aa 100644
--- a/drivers/net/e1000e/ich8lan.c
+++ b/drivers/net/e1000e/ich8lan.c
@@ -280,7 +280,8 @@  static s32 e1000_init_phy_params_pchlan(struct e1000_hw *hw)
 	phy->ops.write_phy_reg_locked = e1000_write_phy_reg_hv_locked;
 	phy->autoneg_mask             = AUTONEG_ADVERTISE_SPEED_DEFAULT;
 
-	if (!(er32(FWSM) & E1000_ICH_FWSM_FW_VALID)) {
+	if (!(er32(FWSM) & E1000_ICH_FWSM_FW_VALID) && 
+		!e1000_check_reset_block(hw)) {
 		/*Set Phy Config Counter to 50msec */
 		ctrl = er32(FEXTNVM3);
 		ctrl &= ~E1000_FEXTNVM3_PHY_CFG_COUNTER_MASK;