All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kok, Auke" <auke-jan.h.kok@intel.com>
To: andi@lisas.de
Cc: e1000-devel@lists.sourceforge.net, auke-jan.h.kok@intel.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	bunk@kernel.org
Subject: Re: [RFC/PATCH] e100 driver didn't support any MII-less PHYs...
Date: Sat, 29 Dec 2007 21:54:45 -0800	[thread overview]
Message-ID: <477732A5.5020501@intel.com> (raw)
In-Reply-To: <20071228235118.GA31059@rhlx01.hs-esslingen.de>

Andreas Mohr wrote:
> Hi all,
> 
> I was mildly annoyed when rebooting my _headless_ internet gateway after a
> hotplug -> udev migration and witnessing it not coming up again,
> which turned out to be due to an eepro100 / e100 loading conflict
> since eepro100 supported both of my Intel-based network cards,
> whereas e100 only supported the "newer" one and entirely failed on ifup...
> (udev had somehow managed to tweak loading sequence as compared to
> a hotplug setup, which caused the drivers to probe differently)
> 
> After investigating this e100 failure for half an hour it was obvious
> that it was failing in e100_hw_init() -> e100_phy_init() since the driver was
> prepared to handle MII-capable PHYs only, not certain older(?) MII-less
> PHYs such as 80c24 or i82503.
> Investigating some FreeBSD etc. drivers it became terribly clear that there
> are also some MII-less PHYs and that one would have to handle them properly.
> 
> Thus I decided to add support for those:
> - after PHY init failure, try to detect whether the EEPROM lists one of
>   the MII-less PHYs
> - if so, don't fatally fail PHY init function
> - avoid touching MII in various utility functions in case of MII-less
>   PHY (FIXME: this may need review, it was a quick hack in some places)
> - add some proper logging on init failure
> 
> Note that this is an initial, semi-rough patch only, would love to have
> it corrected/improved by the e1000 team.
> (I also added some spelling updates for good measure, these would have
> to be committed separately obviously)
> 
> Frankly I'm quite uncertain as to why one would try to actively deprecate
> a driver which works for many cards with a newer one which fails to work
> for several card types and doesn't seem clearly superiour in hindsight
> after going through it...
> Oh, right, that's in order to brute-force people to report any
> nagging problems with the new driver, which is... errm... very
> understandable after all ;)
> (I hope that me "reporting" this problem via a patch is ok ;)
> 
> For reference, I'm using a BNC/AUI/TP PCI combo card
> Intel 82557 645477-004 FCC ID EJMNPDEPR10PCTPCI
> 
> This mail written using a reassuringly stable connection over the newly
> adapted driver...


ok, barely glanced over the patch but it might just be fine. Can you split up this
patch and send a separate patch for the spelling mistakes? I'll then have some
quick testing done on the result and do a bit deeper review after newyears.

Cheers,

Auke

WARNING: multiple messages have this Message-ID (diff)
From: "Kok, Auke" <auke-jan.h.kok@intel.com>
To: andi@lisas.de
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	auke-jan.h.kok@intel.com, bunk@kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC/PATCH] e100 driver didn't support any MII-less PHYs...
Date: Sat, 29 Dec 2007 21:54:45 -0800	[thread overview]
Message-ID: <477732A5.5020501@intel.com> (raw)
In-Reply-To: <20071228235118.GA31059@rhlx01.hs-esslingen.de>

Andreas Mohr wrote:
> Hi all,
> 
> I was mildly annoyed when rebooting my _headless_ internet gateway after a
> hotplug -> udev migration and witnessing it not coming up again,
> which turned out to be due to an eepro100 / e100 loading conflict
> since eepro100 supported both of my Intel-based network cards,
> whereas e100 only supported the "newer" one and entirely failed on ifup...
> (udev had somehow managed to tweak loading sequence as compared to
> a hotplug setup, which caused the drivers to probe differently)
> 
> After investigating this e100 failure for half an hour it was obvious
> that it was failing in e100_hw_init() -> e100_phy_init() since the driver was
> prepared to handle MII-capable PHYs only, not certain older(?) MII-less
> PHYs such as 80c24 or i82503.
> Investigating some FreeBSD etc. drivers it became terribly clear that there
> are also some MII-less PHYs and that one would have to handle them properly.
> 
> Thus I decided to add support for those:
> - after PHY init failure, try to detect whether the EEPROM lists one of
>   the MII-less PHYs
> - if so, don't fatally fail PHY init function
> - avoid touching MII in various utility functions in case of MII-less
>   PHY (FIXME: this may need review, it was a quick hack in some places)
> - add some proper logging on init failure
> 
> Note that this is an initial, semi-rough patch only, would love to have
> it corrected/improved by the e1000 team.
> (I also added some spelling updates for good measure, these would have
> to be committed separately obviously)
> 
> Frankly I'm quite uncertain as to why one would try to actively deprecate
> a driver which works for many cards with a newer one which fails to work
> for several card types and doesn't seem clearly superiour in hindsight
> after going through it...
> Oh, right, that's in order to brute-force people to report any
> nagging problems with the new driver, which is... errm... very
> understandable after all ;)
> (I hope that me "reporting" this problem via a patch is ok ;)
> 
> For reference, I'm using a BNC/AUI/TP PCI combo card
> Intel 82557 645477-004 FCC ID EJMNPDEPR10PCTPCI
> 
> This mail written using a reassuringly stable connection over the newly
> adapted driver...


ok, barely glanced over the patch but it might just be fine. Can you split up this
patch and send a separate patch for the spelling mistakes? I'll then have some
quick testing done on the result and do a bit deeper review after newyears.

Cheers,

Auke

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

  reply	other threads:[~2007-12-30  5:56 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-28 23:51 [RFC/PATCH] e100 driver didn't support any MII-less PHYs Andreas Mohr
2007-12-30  5:54 ` Kok, Auke [this message]
2007-12-30  5:54   ` Kok, Auke
2008-01-01 20:09   ` Andreas Mohr
2008-01-29 23:03     ` Andreas Mohr
2008-01-29 23:09       ` Kok, Auke
2008-01-30  6:49         ` Andreas Mohr
2008-01-30 16:34           ` Kok, Auke
2008-11-03  8:04             ` Andreas Mohr
2008-11-03  9:34               ` Jeff Kirsher
2008-11-03  9:34                 ` Jeff Kirsher
2008-01-01 20:10   ` Andreas Mohr

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=477732A5.5020501@intel.com \
    --to=auke-jan.h.kok@intel.com \
    --cc=andi@lisas.de \
    --cc=bunk@kernel.org \
    --cc=e1000-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.