tg3 driver - make use of binary-only firmware optional

The Debian distribution recently removed the tg3 Gigabit Ethernet driver from its 2.6 kernels because it contains binary-only firmware, i.e. software without source code, which violates the
Debian Free Software Guildelines.

I not a kernel developer, but I looked at the tg3 driver and it seemed that it might work even without the firmware parts.

There are three different binary firmware parts in tg3.c:
- a firmware fix for the 5701_a0 chipset
- a firmware for TSO support
- another firmware for TSO support for the 5705 chipset

I moved these firmware parts from tg3.c to three separate firmware files (tg3-5701_a0-fw.h, tg3-tso-fw.h and tg3-tso5-fw.h), added a new CONFIG_TIGON3_FIRMWARE option to drivers/net/Kconfig and made the inclusion of the firmware files, the firmware loading functions and TSO support in tg3.c depend on CONFIG_TIGON3_FIRMWARE (see the attached patch against linux-2.6.5). This way the Debian distribution could just remove the firmware files from its kernel distribution and unset CONFIG_TIGON3_FIRMWARE.

The resulting driver (with firmware disabled) works fine with the onboard BCM5704 Gigabit Ethernet Controllers of my MSI K8D Dual-Opteron mainboard.

Regards
Andreas Jochens

Please Cc me when replying.

diff -urN linux-2.6.5.orig/drivers/net/Kconfig linux-2.6.5/drivers/net/Kconfig
@@ -2060,6 +2060,14 @@
 	  To compile this driver as a module, choose M here: the module
 	  will be called tg3.  This is recommended.
 
+config TIGON3_FIRMWARE
+	bool "Include firmware (for TSO support and for 5701_a0)" 
+	depends on TIGON3
+	default y
+	help
+	  This includes firmware for TSO support and a firmware fix for
+	  the 5701_a0 chipset. If unsure, say Y.
+
 endmenu
 
 #

Your patch works fine for me on my ASUA A7V8X with BCM5702 rev2.

The debian folks can ship the tg3 driver in whatever form they want to meet their political agenda.

However, that in no way means that Jeff and myself have to split the firmware out of the driver either. In fact, I do not want to as I like keeping all of the network drivers I write in single foo.c and foo.h files.

Would the patch be acceptable if the firmware parts were kept in tg3.c as they are now but #ifdef'd out when CONFIG_TIGON3_FIRMWARE is not set?

At least this would make it clear that the driver is usable even without the firmware. Or is there perhaps any technical problem which might occur when firmware loading is optionally disabled as indicated below?

Thank you for your attention.

Andreas Jochens

diff -urN linux-2.6.5.orig/drivers/net/Kconfig linux-2.6.5/drivers/net/Kconfig
@@ -2060,6 +2060,14 @@
 	  To compile this driver as a module, choose M here: the module
 	  will be called tg3.  This is recommended.
 
+config TIGON3_FIRMWARE
+	bool "Include firmware (for TSO support and for 5701_a0)" 
+	depends on TIGON3
+	default y
+	help
+	  This includes binary-only firmware for TSO support and a 
+          binary-only firmware fix for the 5701_a0 chipset. If unsure, say Y.
+
 endmenu
 
 #

diff -urN linux-2.6.5.orig/drivers/net/tg3.c linux-2.6.5/drivers/net/tg3.c
@@ -46,6 +46,10 @@
 #define TG3_VLAN_TAG_USED 0
 #endif
 
+#ifndef CONFIG_TIGON3_FIRMWARE
+#undef NETIF_F_TSO
+#endif
+
 #ifdef NETIF_F_TSO
 #define TG3_TSO_SUPPORT	1
 #else

@@ -3541,6 +3545,8 @@
 	return 0;
 }
 
+#ifdef CONFIG_TIGON3_FIRMWARE
+
 #define TG3_FW_RELEASE_MAJOR	0x0
 #define TG3_FW_RELASE_MINOR	0x0
 #define TG3_FW_RELEASE_FIX	0x0

@@ -4411,6 +4417,8 @@
 
 #endif /* TG3_TSO_SUPPORT != 0 */
 
+#endif /* CONFIG_TIGON3_FIRMWARE */
+
 /* tp->lock is held. */
 static void __tg3_set_mac_addr(struct tg3 *tp)
 {

@@ -4947,9 +4955,17 @@
 	tw32(SNDBDS_MODE, SNDBDS_MODE_ENABLE | SNDBDS_MODE_ATTN_ENABLE);
 
 	if (tp->pci_chip_rev_id == CHIPREV_ID_5701_A0) {
+#ifdef CONFIG_TIGON3_FIRMWARE
 		err = tg3_load_5701_a0_firmware_fix(tp);
 		if (err)
 			return err;
+#else
+		printk(KERN_ERR PFX "%s: tg3_reset_hardware - "
+			"5701_a0 firmware fix not loaded\n",
+			tp->dev->name );
+		return -ENODEV;
+#endif
+
 	}
 
 #if TG3_TSO_SUPPORT != 0

Is there any reason for not using the hotplug firmware infrastructure?

Wichert.

Wichert Akkerman <[email protected]>    It is simple to make things.
http://www.wiggy.net/                   It is hard to make things simple.

I had the hotplug mechanism in mind when I moved the three firmware images from tg3.c to three separate firmware files in my first patch. I thought of this as a first step which could be followed by converting the driver to use the hotplug firmware infrastructure and moving the firmware images to user space.

However, since a patch putting the firmware in separate files doesn't seem to be welcome, I doubt that a conversion to use the hotplug mechanism will be accepted because this would separate the firmware and the driver even more.

In any case, making the use of the firmware optional by introducing CONFIG_TIGON3_FIRMWARE seems to make sense. A later conversion to the hotplug firmware mechanism will always be possible.

Regards
Andreas Jochens

Right now there is not a high enough "Just Works" value to do this. The mechanisms for wide dissemination of necessary firmware, and automatic placement in initrd/initramfs, are not present yet.

This is a long term goal of all drivers that ship firmware, though.

Jeff

It's still a patch for more political purposes than technical ones.

See my other message -- when the Just Works(tm) value is high enough, we can use the kernel firmware loader, and the firmware will be outside the driver source code.

But since using the firmware loader _right now_ would kill loader-unaware situations like the installer or "kickstart", it's not feasible.

Jeff

Of course you are right. However, the main purpose of posting my patches was _not_ to get them into the mainline kernel but to get feedback regarding the technical possibility of using the driver without the firmware. I got some private mails confirming that the patched driver works for different controllers without the firmware.

While looking at the tg3.c sources I found the attached problem.

Regards
Andreas Jochens

diff -urN linux-2.6.5.orig/drivers/net/tg3.c linux-2.6.5/drivers/net/tg3.c
@@ -3842,7 +3842,7 @@
 #define TG3_TSO_FW_START_ADDR		0x08000000
 #define TG3_TSO_FW_TEXT_ADDR		0x08000000
 #define TG3_TSO_FW_TEXT_LEN		0x1a90
-#define TG3_TSO_FW_RODATA_ADDR		0x08001a900
+#define TG3_TSO_FW_RODATA_ADDR		0x08001a90
 #define TG3_TSO_FW_RODATA_LEN		0x60
 #define TG3_TSO_FW_DATA_ADDR		0x08001b20
 #define TG3_TSO_FW_DATA_LEN		0x20

Patch applied, thanks Andreas.

I will.