diff mbox

Bug with 'igb: Remove GS40G specific defines/functions'

Message ID 1476349095.18484.15.camel@maxwell
State Changes Requested
Headers show

Commit Message

Jochen Henneberg Oct. 13, 2016, 8:58 a.m. UTC
On Mi, 2016-10-12 at 21:18 +0000, Chris Arges wrote:
> 
> On 10/12/16, 3:46 PM, "Aaron Sierra" <asierra@xes-inc.com> wrote:
> 
>     Chris,
>     
>     > I’m not sure exactly which part I need to comment out to verify in
>     > ‘igb_init_phy_params_82575()’.
>     > Is there a fix already planned for this? Any additional information I can
>     > provide?
>     
>     Did you follow any of the "Next message" links? There is more to the
>     discussion than that first page that I linked to.
>     
>     In that discussion we determined that the PHY was somehow setup with a
>     non-zero register page setting. That was treated as a BIOS bug rather
>     than a driver bug.
>     
>     If you're having the same problem, then it could be possible to explicitly
>     set the default page to zero, but I'm not sure where that _should_ be done
>     or if that is generally the correct/safe thing to do.
>     
>     -Aaron
>     
> Aaron,
> Ah sorry about that, I thought you were referring just to that particular post.
> I’ll forward this along to the BIOS vendor.
> Thanks,
> --chris

Chris,

I was the original reporter of that issue. Find attached the patch that
I currently use to ensure correct page selection before the
configuration is started (id read).
I am not sure if this is a good fix (though it is quite small which I
would assume makes it good) and it applies to 4.5 kernel, I did not test
with current head.
The chips datasheet says that the initial page selection is 0, so this
is assumed to be obsolete, but the BIOS may mess up things and as not
everybody has means to modify or request a BIOS update it might make
sense to bring this patch into the driver. It comes with almost no cost
from what I can see.
Please let me know if this fixes your problem.

Regards
-Jochen


> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@lists.osuosl.org
> http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Comments

Chris Arges Oct. 13, 2016, 1:56 p.m. UTC | #1
On 10/13/16, 3:58 AM, "Jochen Henneberg" <jh@henneberg-systemdesign.com> wrote:

    On Mi, 2016-10-12 at 21:18 +0000, Chris Arges wrote:
    > 

    > On 10/12/16, 3:46 PM, "Aaron Sierra" <asierra@xes-inc.com> wrote:

    > 

    >     Chris,

    >     

    >     > I’m not sure exactly which part I need to comment out to verify in

    >     > ‘igb_init_phy_params_82575()’.

    >     > Is there a fix already planned for this? Any additional information I can

    >     > provide?

    >     

    >     Did you follow any of the "Next message" links? There is more to the

    >     discussion than that first page that I linked to.

    >     

    >     In that discussion we determined that the PHY was somehow setup with a

    >     non-zero register page setting. That was treated as a BIOS bug rather

    >     than a driver bug.

    >     

    >     If you're having the same problem, then it could be possible to explicitly

    >     set the default page to zero, but I'm not sure where that _should_ be done

    >     or if that is generally the correct/safe thing to do.

    >     

    >     -Aaron

    >     

    > Aaron,

    > Ah sorry about that, I thought you were referring just to that particular post.

    > I’ll forward this along to the BIOS vendor.

    > Thanks,

    > --chris

    
    Chris,
    
    I was the original reporter of that issue. Find attached the patch that
    I currently use to ensure correct page selection before the
    configuration is started (id read).
    I am not sure if this is a good fix (though it is quite small which I
    would assume makes it good) and it applies to 4.5 kernel, I did not test
    with current head.
    The chips datasheet says that the initial page selection is 0, so this
    is assumed to be obsolete, but the BIOS may mess up things and as not
    everybody has means to modify or request a BIOS update it might make
    sense to bring this patch into the driver. It comes with almost no cost
    from what I can see.
    Please let me know if this fixes your problem.
    
    Regards
    -Jochen
    

Jochen,

Thanks for this update. I tested this patch and it fixes my issue!
If there are no other ill effects from this patch perhaps it would be a good
candidate for mainline and stable.

--chris



    
    > 

    > _______________________________________________

    > Intel-wired-lan mailing list

    > Intel-wired-lan@lists.osuosl.org

    > http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

    
    -- 
    Henneberg - Systemdesign
    Jochen Henneberg
    Loehnfeld 26
    21423 Winsen (Luhe)
    --
    Fon: +49 4174 668 773
    Mobile: +49 172 160 14 69
    Fax: +49 321 210 761 64
    www: www.henneberg-systemdesign.com
Jochen Henneberg Oct. 14, 2016, 11:24 a.m. UTC | #2
On Do, 2016-10-13 at 13:56 +0000, Chris Arges wrote:
> Jochen,
> 
> Thanks for this update. I tested this patch and it fixes my issue!
> If there are no other ill effects from this patch perhaps it would be
> a good
> candidate for mainline and stable.

The patch is hacky, it does neither use the phy-ops.write_reg() nor does
it check for the correct chip before it applies the write.
I will try to prepare a clean fix and send a patch.

Regards
-Jochen

> 
> --chris
diff mbox

Patch

From f3e491156d6ba6b57e38f148a2ffe69ebf6fbfd4 Mon Sep 17 00:00:00 2001
From: Jochen Henneberg <jh@henneberg-systemdesign.com>
Date: Wed, 27 Apr 2016 10:18:36 +0000
Subject: [PATCH] igb: reset page select to 0 on initial phy id read

---
 drivers/net/ethernet/intel/igb/e1000_phy.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.c b/drivers/net/ethernet/intel/igb/e1000_phy.c
index 5b54254..17fa54d 100644
--- a/drivers/net/ethernet/intel/igb/e1000_phy.c
+++ b/drivers/net/ethernet/intel/igb/e1000_phy.c
@@ -77,6 +77,9 @@  s32 igb_get_phy_id(struct e1000_hw *hw)
 	s32 ret_val = 0;
 	u16 phy_id;
 
+        /* ensure phy page selection */
+        igb_write_phy_reg_82580(hw, I347AT4_PAGE_SELECT, 0);
+
 	ret_val = phy->ops.read_reg(hw, PHY_ID1, &phy_id);
 	if (ret_val)
 		goto out;
-- 
2.8.0.rc3