All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] net: e100: ucode is optional in some cases
@ 2012-07-19 16:28 Bjørn Mork
  2012-07-19 17:50 ` David Miller
  0 siblings, 1 reply; 2+ messages in thread
From: Bjørn Mork @ 2012-07-19 16:28 UTC (permalink / raw)
  To: netdev
  Cc: e1000-devel, Bruce Allan, Jesse Brandeburg, John Ronciak,
	Bjørn Mork

  commit 9ac32e1b firmware: convert e100 driver to request_firmware()

did a straight conversion of the in-driver ucode to external
files.  This introduced the possibility of the driver failing
to enable an interface due to missing ucode. There was no
evaluation of the importance of the ucode at the time.

Based on comments in earlier versions of this driver, and in
the source code for the FreeBSD fxp driver, we can assume that
the ucode implements the "CPU Cycle Saver" feature on supported
adapters.  Although generally wanted, this is an optional
feature. The ucode source is not available, preventing it from
being included in free distributions. This creates unnecessary
problems for the end users. Doing a network install based on a
free distribution installer requires the user to download and
insert the ucode into the installer.

Making the ucode optional when possible improves the user
experience and driver usability.

The ucode for some adapters include a bugfix, making it
essential.  We continue to fail for these adapters unless the
ucode is available.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
v2: removed URLs from the patch, converting them to generic
    descriptions of the sources of information


 drivers/net/ethernet/intel/e100.c |   40 ++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
index ada720b..535f94f 100644
--- a/drivers/net/ethernet/intel/e100.c
+++ b/drivers/net/ethernet/intel/e100.c
@@ -1249,20 +1249,35 @@ static const struct firmware *e100_request_firmware(struct nic *nic)
 	const struct firmware *fw = nic->fw;
 	u8 timer, bundle, min_size;
 	int err = 0;
+	bool required = false;
 
 	/* do not load u-code for ICH devices */
 	if (nic->flags & ich)
 		return NULL;
 
-	/* Search for ucode match against h/w revision */
-	if (nic->mac == mac_82559_D101M)
+	/* Search for ucode match against h/w revision
+	 *
+	 * Based on comments in the source code for the FreeBSD fxp
+	 * driver, the FIRMWARE_D102E ucode includes both CPUSaver and
+	 *
+	 *    "fixes for bugs in the B-step hardware (specifically, bugs
+	 *     with Inline Receive)."
+	 *
+	 * So we must fail if it cannot be loaded.
+	 *
+	 * The other microcode files are only required for the optional
+	 * CPUSaver feature.  Nice to have, but no reason to fail.
+	 */
+	if (nic->mac == mac_82559_D101M) {
 		fw_name = FIRMWARE_D101M;
-	else if (nic->mac == mac_82559_D101S)
+	} else if (nic->mac == mac_82559_D101S) {
 		fw_name = FIRMWARE_D101S;
-	else if (nic->mac == mac_82551_F || nic->mac == mac_82551_10)
+	} else if (nic->mac == mac_82551_F || nic->mac == mac_82551_10) {
 		fw_name = FIRMWARE_D102E;
-	else /* No ucode on other devices */
+		required = true;
+	} else { /* No ucode on other devices */
 		return NULL;
+	}
 
 	/* If the firmware has not previously been loaded, request a pointer
 	 * to it. If it was previously loaded, we are reinitializing the
@@ -1273,10 +1288,17 @@ static const struct firmware *e100_request_firmware(struct nic *nic)
 		err = request_firmware(&fw, fw_name, &nic->pdev->dev);
 
 	if (err) {
-		netif_err(nic, probe, nic->netdev,
-			  "Failed to load firmware \"%s\": %d\n",
-			  fw_name, err);
-		return ERR_PTR(err);
+		if (required) {
+			netif_err(nic, probe, nic->netdev,
+				  "Failed to load firmware \"%s\": %d\n",
+				  fw_name, err);
+			return ERR_PTR(err);
+		} else {
+			netif_info(nic, probe, nic->netdev,
+				   "CPUSaver disabled. Needs \"%s\": %d\n",
+				   fw_name, err);
+			return NULL;
+		}
 	}
 
 	/* Firmware should be precisely UCODE_SIZE (words) plus three bytes
-- 
1.7.10.4


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] net: e100: ucode is optional in some cases
  2012-07-19 16:28 [PATCH v2] net: e100: ucode is optional in some cases Bjørn Mork
@ 2012-07-19 17:50 ` David Miller
  0 siblings, 0 replies; 2+ messages in thread
From: David Miller @ 2012-07-19 17:50 UTC (permalink / raw)
  To: bjorn
  Cc: netdev, jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
	carolyn.wyborny, donald.c.skidmore, gregory.v.rose,
	peter.p.waskiewicz.jr, alexander.h.duyck, john.ronciak,
	e1000-devel

From: Bjørn Mork <bjorn@mork.no>
Date: Thu, 19 Jul 2012 18:28:40 +0200

>   commit 9ac32e1b firmware: convert e100 driver to request_firmware()
> 
> did a straight conversion of the in-driver ucode to external
> files.  This introduced the possibility of the driver failing
> to enable an interface due to missing ucode. There was no
> evaluation of the importance of the ucode at the time.
> 
> Based on comments in earlier versions of this driver, and in
> the source code for the FreeBSD fxp driver, we can assume that
> the ucode implements the "CPU Cycle Saver" feature on supported
> adapters.  Although generally wanted, this is an optional
> feature. The ucode source is not available, preventing it from
> being included in free distributions. This creates unnecessary
> problems for the end users. Doing a network install based on a
> free distribution installer requires the user to download and
> insert the ucode into the installer.
> 
> Making the ucode optional when possible improves the user
> experience and driver usability.
> 
> The ucode for some adapters include a bugfix, making it
> essential.  We continue to fail for these adapters unless the
> ucode is available.
> 
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> ---
> v2: removed URLs from the patch, converting them to generic
>     descriptions of the sources of information

Applied.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2012-07-19 17:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-19 16:28 [PATCH v2] net: e100: ucode is optional in some cases Bjørn Mork
2012-07-19 17:50 ` David Miller

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.