netdev - RE: [Intel-wired-lan] [PATCH] igb: support BCM54616 PHY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 27 Jul 2017 18:35:31 +0000
From:   "Wyborny, Carolyn" <carolyn.wyborny@...el.com>
To:     Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        "Brown, Aaron F" <aaron.f.brown@...el.com>
CC:     "John W. Linville" <linville@...driver.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>
Subject: RE: [Intel-wired-lan] [PATCH] igb: support BCM54616 PHY

> -----Original Message-----
> From: netdev-owner@...r.kernel.org [mailto:netdev-
> owner@...r.kernel.org] On Behalf Of Florian Fainelli
> Sent: Thursday, July 27, 2017 10:58 AM
> To: Andrew Lunn <andrew@...n.ch>; Brown, Aaron F
> <aaron.f.brown@...el.com>
> Cc: John W. Linville <linville@...driver.com>; netdev@...r.kernel.org; intel-
> wired-lan@...ts.osuosl.org
> Subject: Re: [Intel-wired-lan] [PATCH] igb: support BCM54616 PHY
> 
> On 07/27/2017 08:37 AM, Andrew Lunn wrote:
> > On Thu, Jul 27, 2017 at 12:40:01AM +0000, Brown, Aaron F wrote:
> >>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@...osl.org] On
> Behalf
> >>> Of John W. Linville
> >>> Sent: Friday, July 21, 2017 11:12 AM
> >>> To: netdev@...r.kernel.org
> >>> Cc: intel-wired-lan@...ts.osuosl.org; John W. Linville
> >>> <linville@...driver.com>
> >>> Subject: [Intel-wired-lan] [PATCH] igb: support BCM54616 PHY
> >>>
> >>> The management port on an Edgecore AS7712-32 switch uses an igb MAC,
> >>> but
> >>> it uses a BCM54616 PHY. Without a patch like this, loading the igb
> >>> module produces dmesg output like this:
> >>>
> >>> [    3.439125] igb: Copyright (c) 2007-2014 Intel Corporation.
> >>> [    3.439866] igb: probe of 0000:00:14.0 failed with error -2
> >>>
> >>> Signed-off-by: John W. Linville <linville@...driver.com>
> >>> Cc: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
> >>> ---
> >>>  drivers/net/ethernet/intel/igb/e1000_82575.c   | 6 ++++++
> >>>  drivers/net/ethernet/intel/igb/e1000_defines.h | 1 +
> >>>  drivers/net/ethernet/intel/igb/e1000_hw.h      | 1 +
> >>>  3 files changed, 8 insertions(+)
> >>
> >> I do not have the specific hardware (Edgecore switch) but as far as
> regression tests go this works fine.
> >> Tested-by: Aaron Brown <aaron.f.brown@...el.com>
> >
> > Sorry, missed the initial post, so replying to a reply.
> >
> > Linux has supported the BCM54616 PHY since April 2015. If the Intel
> > drivers used the Linux PHY drivers, you would not of had this problem.
> >
> > It would be good if somebody spent the time to migrate these MAC
> > drivers to use the common Linux PHY infrastructure.
> 
> I suspect there is a design pattern within the Intel drivers to share as
> much low-level code as possible between OSes and only have some
> Linux-ism where necessary (e.g: net_device, ethtool etc.).
> 
> PHY code is a pain in general, especially if you are serious about
> testing interoperability (which is where you can spend tons of $$$ with
> little reward but just say: yes it works), so it may make sense to share
> it across different OSes.
> 
> I too, wish there was more sharing, but considering that this works for
> the Intel driver, there is little incentive in doing this I suppose...
> --
> Florian

Yes Florian you are correct generally, but future implementations have not been ruled out.  With this driver, we had our custom phy init code tested and released for several products before phylib existed.  I began research on a phylib implementation for it at one point, but internal product decisions ended up lowering the priority of that work.

Thanks,

Carolyn

Carolyn Wyborny 
Linux Development 
Networking Division 
Intel Corporation