Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Alessandro Suardi]

On Wed, Jan 7, 2009 at 3:24 PM, Alessandro Suardi
<alessandro.suardi@gmail.com> wrote:
> From dmesg:
>
> [asuardi@sandman src]$ dmesg | grep tg3
> tg3.c:v3.97 (December 10, 2008)
> tg3 0000:02:00.0: PCI INT A -> Link[LNKA] -> GSI 11 (level, low) -> IRQ 11
> tg3 0000:02:00.0: setting latency timer to 64
> tg3 0000:02:00.0: wake-up capability disabled by ACPI
> tg3 0000:02:00.0: PME# disabled
> tg3 0000:02:00.0: firmware: requesting tigon/tg3_tso.bin
> tg3: Failed to load firmware "tigon/tg3_tso.bin"
> tg3 0000:02:00.0: PCI INT A disabled
> tg3: probe of 0000:02:00.0 failed with error -2
>
>
> But /lib/firmware/tigon/tg3_tso.bin is actually present:
>
> [asuardi@sandman src]$ ls -l /lib/firmware/tigon/tg3_tso.bin
> -rw-r--r-- 1 root root 7004 2009-01-06 22:58 /lib/firmware/tigon/tg3_tso.bin
>
>
>
> Diffing the .configs one can see CONFIG_KMOD disappeared; and
>  the output of the kernel build in -git8 also has the IHEX parts about
>  tg3 firmware which are new (I don't recall seeing those in the last
>  1000 builds or so ;)
>
> [asuardi@sandman src]$ diff .config-2.6.28-git[78]
> 3,4c3,4
> < # Linux kernel version: 2.6.28-git7
> < # Mon Jan  5 17:34:11 2009
> ---
>> # Linux kernel version: 2.6.28-git8
>> # Tue Jan  6 22:06:19 2009
> 133d132
> < CONFIG_KMOD=y
> 1324a1324
>> # CONFIG_TWL4030_CORE is not set
> 1591d1590
> < CONFIG_HID_BRIGHT=y
> 1595d1593
> < CONFIG_HID_DELL=y
> 1602a1601
>> CONFIG_HID_NTRIG=y
> 1608a1608,1609
>> # CONFIG_GREENASIA_FF is not set
>> CONFIG_HID_TOPSEED=y
> 1821c1822
> < # CONFIG_OCFS2_COMPAT_JBD is not set
> ---
>> CONFIG_OCFS2_FS_POSIX_ACL=y
> 1825c1826,1832
> < # CONFIG_QUOTA is not set
> ---
>> CONFIG_QUOTA=y
>> CONFIG_QUOTA_NETLINK_INTERFACE=y
>> # CONFIG_PRINT_QUOTA_WARNING is not set
>> CONFIG_QUOTA_TREE=y
>> # CONFIG_QFMT_V1 is not set
>> CONFIG_QFMT_V2=y
>> CONFIG_QUOTACTL=y
>
>
>
> [asuardi@sandman src]$ diff /tmp/make-kernel-2.6.28-git[78].out
> 669a670
>>   CC [M]  fs/ocfs2/blockcheck.o
> 692a694,695
>>   CC [M]  fs/ocfs2/quota_local.o
>>   CC [M]  fs/ocfs2/quota_global.o
> 693a697
>>   CC [M]  fs/ocfs2/acl.o
> 810a815,818
>>   CC      fs/dquot.o
>>   CC      fs/quota_v2.o
>>   CC      fs/quota_tree.o
>>   CC      fs/quota.o
> 1045c1053
> < /share/src/linux-2.6.28-git7/arch/x86/include/asm/string_32.h:75:
> warning: array subscript is above array bounds
> ---
>> /share/src/linux-2.6.28-git8/arch/x86/include/asm/string_32.h:75: warning: array subscript is above array bounds
> 1286d1293
> <   CC      drivers/hid/hid-bright.o
> 1290d1296
> <   CC      drivers/hid/hid-dell.o
> 1295a1302
>>   CC      drivers/hid/hid-ntrig.o
> 1300a1308
>>   CC      drivers/hid/hid-topseed.o
> 1521c1529
> < /share/src/linux-2.6.28-git7/arch/x86/include/asm/string_32.h:75:
> warning: array subscript is above array bounds
> ---
>> /share/src/linux-2.6.28-git8/arch/x86/include/asm/string_32.h:75: warning: array subscript is above array bounds
> 1983d1990
> < net/rfkill/rfkill.c:59: warning: 'rfkill_led_trigger' defined but not used
> 2234,2235c2241,2242
> < System is 2000 kB
> < CRC 8dd05de2
> ---
>> System is 2013 kB
>> CRC 37411210
> 2646a2654,2656
>>   IHEX    firmware/tigon/tg3.bin
>>   IHEX    firmware/tigon/tg3_tso.bin
>>   IHEX    firmware/tigon/tg3_tso5.bin
>
>
> So which is it, whatever the IHEX part did or the disappearance of
>  CONFIG_KMOD (which is in the -git8 patch, indeed) to cause
>  this issue ?
>
> Note that I don't have CONFIG_FIRMWARE_IN_KERNEL, but on
>  the other hand I didn't earlier, either... and yes, CONFIG_MODULES=y.
>
>
> thanks in advance, ciao,

Still broken in -git10, cc'ing netdev... h/w is the onboard
 Gigabit ethernet on my Dell Latitude D610, that is

02:00.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5751
Gigabit Ethernet PCI Express (rev 01)

thanks,

--alessandro

 "Sun keeps rising in the west / I keep on waking fully confused"

   (The Replacements, "Within Your Reach")

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Jaswinder Singh Rajput]

Hello alessandro,

On Wed, Jan 7, 2009 at 7:54 PM, Alessandro Suardi
<alessandro.suardi@gmail.com> wrote:
>
> Note that I don't have CONFIG_FIRMWARE_IN_KERNEL, but on
>  the other hand I didn't earlier, either... and yes, CONFIG_MODULES=y.
>

Please try with CONFIG_FIRMWARE_IN_KERNEL , is this makes some difference.

--
JSR

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Alessandro Suardi]

On Thu, Jan 8, 2009 at 7:12 AM, Jaswinder Singh Rajput
<jaswinderlinux@gmail.com> wrote:
> Hello alessandro,
>
> On Wed, Jan 7, 2009 at 7:54 PM, Alessandro Suardi
> <alessandro.suardi@gmail.com> wrote:
>>
>> Note that I don't have CONFIG_FIRMWARE_IN_KERNEL, but on
>>  the other hand I didn't earlier, either... and yes, CONFIG_MODULES=y.
>>
>
> Please try with CONFIG_FIRMWARE_IN_KERNEL , is this makes some difference.

Yes it does, thank you - 2.6.28-git11 with CONFIG_FIRMWARE_IN_KERNEL
 gets my tg3 back to work on boot. Must be a requirement since -git8 then ?

Thanks again, ciao,

--alessandro

 "Sun keeps rising in the west / I keep on waking fully confused"

   (The Replacements, "Within Your Reach")

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Miller]

From: "Alessandro Suardi" <alessandro.suardi@gmail.com>
Date: Thu, 8 Jan 2009 21:33:35 +0100

> On Thu, Jan 8, 2009 at 7:12 AM, Jaswinder Singh Rajput
> <jaswinderlinux@gmail.com> wrote:
> > Hello alessandro,
> >
> > On Wed, Jan 7, 2009 at 7:54 PM, Alessandro Suardi
> > <alessandro.suardi@gmail.com> wrote:
> >>
> >> Note that I don't have CONFIG_FIRMWARE_IN_KERNEL, but on
> >>  the other hand I didn't earlier, either... and yes, CONFIG_MODULES=y.
> >>
> >
> > Please try with CONFIG_FIRMWARE_IN_KERNEL , is this makes some difference.
> 
> Yes it does, thank you - 2.6.28-git11 with CONFIG_FIRMWARE_IN_KERNEL
>  gets my tg3 back to work on boot. Must be a requirement since -git8 then ?

It is if you don't have the tg3 firmware files in your filesystem
already.

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Alessandro Suardi]

On Thu, Jan 8, 2009 at 9:53 PM, David Miller <davem@davemloft.net> wrote:
> From: "Alessandro Suardi" <alessandro.suardi@gmail.com>
> Date: Thu, 8 Jan 2009 21:33:35 +0100
>
>> On Thu, Jan 8, 2009 at 7:12 AM, Jaswinder Singh Rajput
>> <jaswinderlinux@gmail.com> wrote:
>> > Hello alessandro,
>> >
>> > On Wed, Jan 7, 2009 at 7:54 PM, Alessandro Suardi
>> > <alessandro.suardi@gmail.com> wrote:
>> >>
>> >> Note that I don't have CONFIG_FIRMWARE_IN_KERNEL, but on
>> >>  the other hand I didn't earlier, either... and yes, CONFIG_MODULES=y.
>> >>
>> >
>> > Please try with CONFIG_FIRMWARE_IN_KERNEL , is this makes some difference.
>>
>> Yes it does, thank you - 2.6.28-git11 with CONFIG_FIRMWARE_IN_KERNEL
>>  gets my tg3 back to work on boot. Must be a requirement since -git8 then ?
>
> It is if you don't have the tg3 firmware files in your filesystem
> already.

As I mentioned in the original email to LKML about -git8, and in
 the first CC to netdev about -git10, I appear to have these files
 in /lib/firmware/tigon. I save every single .config file I used in the
 last, what, four years ?

.config-2.6.28-git10:# CONFIG_FIRMWARE_IN_KERNEL is not set
.config-2.6.28-git11:CONFIG_FIRMWARE_IN_KERNEL=y
.config-2.6.28-git2:# CONFIG_FIRMWARE_IN_KERNEL is not set
.config-2.6.28-git3:# CONFIG_FIRMWARE_IN_KERNEL is not set
.config-2.6.28-git4:# CONFIG_FIRMWARE_IN_KERNEL is not set
.config-2.6.28-git5:# CONFIG_FIRMWARE_IN_KERNEL is not set
.config-2.6.28-git6:# CONFIG_FIRMWARE_IN_KERNEL is not set
.config-2.6.28-git7:# CONFIG_FIRMWARE_IN_KERNEL is not set
.config-2.6.28-git8:# CONFIG_FIRMWARE_IN_KERNEL is not set

-git-2 through -git-7 work
-git8 and -git10 don't work
-git11 works

Clearly the requirement changed in -git8, AFAICS.

thanks,

--alessandro

 "Sun keeps rising in the west / I keep on waking fully confused"

   (The Replacements, "Within Your Reach")

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Miller]

From: "Alessandro Suardi" <alessandro.suardi@gmail.com>
Date: Fri, 9 Jan 2009 18:30:40 +0100

> Clearly the requirement changed in -git8, AFAICS.

Because -git8 is where the firmware requirement got added.

If you build the TG3 driver statically into your kernel,
the only way to get the firmware properly loaded is to
enable that CONFIG_FIRMWARE_IN_KERNEL option.

Your filesystem isn't mounted when the driver initializes
in that case, therefore there is no way to get at the
firwware that might be present there.

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Alessandro Suardi]

On Fri, Jan 9, 2009 at 11:04 PM, David Miller <davem@davemloft.net> wrote:
> From: "Alessandro Suardi" <alessandro.suardi@gmail.com>
> Date: Fri, 9 Jan 2009 18:30:40 +0100
>
>> Clearly the requirement changed in -git8, AFAICS.
>
> Because -git8 is where the firmware requirement got added.

OK, thanks.

> If you build the TG3 driver statically into your kernel,
> the only way to get the firmware properly loaded is to
> enable that CONFIG_FIRMWARE_IN_KERNEL option.
>
> Your filesystem isn't mounted when the driver initializes
> in that case, therefore there is no way to get at the
> firwware that might be present there.

And indeed I do build tg3 in-kernel. But since the config
 option was present previously, perhaps it would have
 been the case to change the help text for tg3 explaining
 that now a non-modular build requires an extra step ?

Or maybe kbuild allows to select FIRMWARE_IN_KERNEL
 if tg3 is built non-modular ?

My only observation is that a make oldconfig shouldn't
 change a working setup to a non-working one (though
 it already happened once to me in a couple of weeks,
 the other being I915 DRM being silently skipped as a
 consequence of becoming dependent on CONFIG_FB)
 with a new kernel patch.

Clearly, if there is no easy way to avoid such a situation,
 it's okay - LKML and the other Linux-related lists are
 always helpful.

Thanks again, ciao,

--alessandro

 "Sun keeps rising in the west / I keep on waking fully confused"

   (The Replacements, "Within Your Reach")

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

On Fri, 09 Jan 2009 14:04:22 PST, David Miller said:
> From: "Alessandro Suardi" <alessandro.suardi@gmail.com>
> Date: Fri, 9 Jan 2009 18:30:40 +0100
> 
> > Clearly the requirement changed in -git8, AFAICS.
> 
> Because -git8 is where the firmware requirement got added.
> 
> If you build the TG3 driver statically into your kernel,
> the only way to get the firmware properly loaded is to
> enable that CONFIG_FIRMWARE_IN_KERNEL option.

Maybe this is one of those cases where we actually do want a SELECT?

	select FIRMWARE_IN_KERNEL if TIGON3=y

or similar?

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Miller]

From: Valdis.Kletnieks@vt.edu
Date: Sun, 11 Jan 2009 06:10:58 -0500

> On Fri, 09 Jan 2009 14:04:22 PST, David Miller said:
> > From: "Alessandro Suardi" <alessandro.suardi@gmail.com>
> > Date: Fri, 9 Jan 2009 18:30:40 +0100
> > 
> > > Clearly the requirement changed in -git8, AFAICS.
> > 
> > Because -git8 is where the firmware requirement got added.
> > 
> > If you build the TG3 driver statically into your kernel,
> > the only way to get the firmware properly loaded is to
> > enable that CONFIG_FIRMWARE_IN_KERNEL option.
> 
> Maybe this is one of those cases where we actually do want a SELECT?
> 
> 	select FIRMWARE_IN_KERNEL if TIGON3=y
> 
> or similar?

I have no idea how this is intended to work, David will
know.

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Woodhouse]

On Sun, 2009-01-11 at 04:08 -0800, David Miller wrote:
> From: Valdis.Kletnieks@vt.edu
> Date: Sun, 11 Jan 2009 06:10:58 -0500
> 
> > On Fri, 09 Jan 2009 14:04:22 PST, David Miller said:
> > > From: "Alessandro Suardi" <alessandro.suardi@gmail.com>
> > > Date: Fri, 9 Jan 2009 18:30:40 +0100
> > > 
> > > > Clearly the requirement changed in -git8, AFAICS.
> > > 
> > > Because -git8 is where the firmware requirement got added.
> > > 
> > > If you build the TG3 driver statically into your kernel,
> > > the only way to get the firmware properly loaded is to
> > > enable that CONFIG_FIRMWARE_IN_KERNEL option.
> > 
> > Maybe this is one of those cases where we actually do want a SELECT?
> > 
> > 	select FIRMWARE_IN_KERNEL if TIGON3=y
> > 
> > or similar?
> 
> I have no idea how this is intended to work, David will know.

Actually, I think the real issue here might be that the tg3 driver is
now behaving _differently_ to how other modern drivers work. It tries to
obtain the firmware once at initialisation time and if that fails it
doesn't register the device.

Other drivers will load the firmware later, at the time the device is
brought up. This means that even if you build the driver into the kernel
without its firmware, it can still request the firmware later, when you
try to start _using_ it. And when the file system is available.

I'll take a look and see if I can remedy that. Then we wouldn't _need_
the FIRMWARE_IN_KERNEL option.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Woodhouse]

On Sun, 2009-01-11 at 12:25 +0000, David Woodhouse wrote:
> I'll take a look and see if I can remedy that. Then we wouldn't _need_
> the FIRMWARE_IN_KERNEL option.

How about this? If it fails to load the firmware from userspace during
the initialisation, it'll try again later in tg3_open(). 

I _think_ that's fine, because we don't do anything else in the early
initialisation which requires the firmware to be loaded.

So if you build with CONFIG_TIGON3=y, CONFIG_FIRMWARE_IN_KERNEL=n, you
should see it fail to load the firmware at boot, but then it should load
it successfully when you bring the device up.

Untested-but-otherwise-Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 5e2dbae..f99218c 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -7535,11 +7535,45 @@ static int tg3_test_msi(struct tg3 *tp)
 	return err;
 }
 
+static int tg3_request_firmware(struct tg3 *tp)
+{
+	const __be32 *fw_data;
+
+	if (request_firmware(&tp->fw, tp->fw_needed, &tp->pdev->dev)) {
+		printk(KERN_ERR "tg3: Failed to load firmware \"%s\"\n",
+		       tp->fw_needed);
+		return -ENOENT;
+	}
+
+	fw_data = (void *)tp->fw->data;
+
+	/* Firmware blob starts with version numbers, followed by
+	   start address and _full_ length including BSS sections
+	   (which must be longer than the actual data, of course */
+
+	tp->fw_len = be32_to_cpu(fw_data[2]);	/* includes bss */
+	if (tp->fw_len < (tp->fw->size - 12)) {
+		printk(KERN_ERR "tg3: bogus length %d in \"%s\"\n",
+		       tp->fw_len, tp->fw_needed);
+		return -EINVAL;
+	}
+
+	/* We no longer need firmware; we have it. */
+	tp->fw_needed = NULL;
+	return 0;
+}
+
 static int tg3_open(struct net_device *dev)
 {
 	struct tg3 *tp = netdev_priv(dev);
 	int err;
 
+	if (tp->fw_needed) {
+		err = tg3_request_firmware(tp);
+		if (err)
+			return err;
+	}
+
 	netif_carrier_off(tp->dev);
 
 	err = tg3_set_power_state(tp, PCI_D0);
@@ -12934,7 +12968,6 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
 	struct net_device *dev;
 	struct tg3 *tp;
 	int err, pm_cap;
-	const char *fw_name = NULL;
 	char str[40];
 	u64 dma_mask, persist_dma_mask;
 
@@ -13091,7 +13124,7 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
 	tg3_init_bufmgr_config(tp);
 
 	if (tp->pci_chip_rev_id == CHIPREV_ID_5701_A0)
-		fw_name = FIRMWARE_TG3;
+		tp->fw_needed = FIRMWARE_TG3;
 
 	if (tp->tg3_flags2 & TG3_FLG2_HW_TSO) {
 		tp->tg3_flags2 |= TG3_FLG2_TSO_CAPABLE;
@@ -13107,34 +13140,19 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
 	}
 	if (tp->tg3_flags2 & TG3_FLG2_TSO_CAPABLE) {
 		if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5705)
-			fw_name = FIRMWARE_TG3TSO5;
+			tp->fw_needed = FIRMWARE_TG3TSO5;
 		else
-			fw_name = FIRMWARE_TG3TSO;
+			tp->fw_needed = FIRMWARE_TG3TSO;
 	}
 
-	if (fw_name) {
-		const __be32 *fw_data;
-
-		err = request_firmware(&tp->fw, fw_name, &tp->pdev->dev);
-		if (err) {
-			printk(KERN_ERR "tg3: Failed to load firmware \"%s\"\n",
-			       fw_name);
-			goto err_out_iounmap;
-		}
-
-		fw_data = (void *)tp->fw->data;
-
-		/* Firmware blob starts with version numbers, followed by
-		   start address and _full_ length including BSS sections
-		   (which must be longer than the actual data, of course */
-
-		tp->fw_len = be32_to_cpu(fw_data[2]);	/* includes bss */
-		if (tp->fw_len < (tp->fw->size - 12)) {
-			printk(KERN_ERR "tg3: bogus length %d in \"%s\"\n",
-			       tp->fw_len, fw_name);
-			err = -EINVAL;
+	if (tp->fw_needed) {
+		err = tg3_request_firmware(tp);
+		/* Failure to load firmware at this stage is not fatal; we'll
+		   try again in tg3_open(). So if you have the driver built
+		   into the kernel, you can still get the firmware loaded
+		   after userspace is running, when the device comes up. */
+		if (err != -ENOENT)
 			goto err_out_fw;
-		}
 	}
 
 	/* TSO is on by default on chips that support hardware TSO.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Alessandro Suardi]

On Sun, Jan 11, 2009 at 1:59 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Sun, 2009-01-11 at 12:25 +0000, David Woodhouse wrote:
>> I'll take a look and see if I can remedy that. Then we wouldn't _need_
>> the FIRMWARE_IN_KERNEL option.
>
> How about this? If it fails to load the firmware from userspace during
> the initialisation, it'll try again later in tg3_open().
>
> I _think_ that's fine, because we don't do anything else in the early
> initialisation which requires the firmware to be loaded.
>
> So if you build with CONFIG_TIGON3=y, CONFIG_FIRMWARE_IN_KERNEL=n, you
> should see it fail to load the firmware at boot, but then it should load
> it successfully when you bring the device up.
>
> Untested-but-otherwise-Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
>
> diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> index 5e2dbae..f99218c 100644
> --- a/drivers/net/tg3.c
> +++ b/drivers/net/tg3.c
> @@ -7535,11 +7535,45 @@ static int tg3_test_msi(struct tg3 *tp)
>        return err;
>  }
>
> +static int tg3_request_firmware(struct tg3 *tp)
> +{
> +       const __be32 *fw_data;
> +
> +       if (request_firmware(&tp->fw, tp->fw_needed, &tp->pdev->dev)) {
> +               printk(KERN_ERR "tg3: Failed to load firmware \"%s\"\n",
> +                      tp->fw_needed);
> +               return -ENOENT;
> +       }
> +
> +       fw_data = (void *)tp->fw->data;
> +
> +       /* Firmware blob starts with version numbers, followed by
> +          start address and _full_ length including BSS sections
> +          (which must be longer than the actual data, of course */
> +
> +       tp->fw_len = be32_to_cpu(fw_data[2]);   /* includes bss */
> +       if (tp->fw_len < (tp->fw->size - 12)) {
> +               printk(KERN_ERR "tg3: bogus length %d in \"%s\"\n",
> +                      tp->fw_len, tp->fw_needed);
> +               return -EINVAL;
> +       }
> +
> +       /* We no longer need firmware; we have it. */
> +       tp->fw_needed = NULL;
> +       return 0;
> +}
> +
>  static int tg3_open(struct net_device *dev)
>  {
>        struct tg3 *tp = netdev_priv(dev);
>        int err;
>
> +       if (tp->fw_needed) {
> +               err = tg3_request_firmware(tp);
> +               if (err)
> +                       return err;
> +       }
> +
>        netif_carrier_off(tp->dev);
>
>        err = tg3_set_power_state(tp, PCI_D0);
> @@ -12934,7 +12968,6 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
>        struct net_device *dev;
>        struct tg3 *tp;
>        int err, pm_cap;
> -       const char *fw_name = NULL;
>        char str[40];
>        u64 dma_mask, persist_dma_mask;
>
> @@ -13091,7 +13124,7 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
>        tg3_init_bufmgr_config(tp);
>
>        if (tp->pci_chip_rev_id == CHIPREV_ID_5701_A0)
> -               fw_name = FIRMWARE_TG3;
> +               tp->fw_needed = FIRMWARE_TG3;
>
>        if (tp->tg3_flags2 & TG3_FLG2_HW_TSO) {
>                tp->tg3_flags2 |= TG3_FLG2_TSO_CAPABLE;
> @@ -13107,34 +13140,19 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
>        }
>        if (tp->tg3_flags2 & TG3_FLG2_TSO_CAPABLE) {
>                if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5705)
> -                       fw_name = FIRMWARE_TG3TSO5;
> +                       tp->fw_needed = FIRMWARE_TG3TSO5;
>                else
> -                       fw_name = FIRMWARE_TG3TSO;
> +                       tp->fw_needed = FIRMWARE_TG3TSO;
>        }
>
> -       if (fw_name) {
> -               const __be32 *fw_data;
> -
> -               err = request_firmware(&tp->fw, fw_name, &tp->pdev->dev);
> -               if (err) {
> -                       printk(KERN_ERR "tg3: Failed to load firmware \"%s\"\n",
> -                              fw_name);
> -                       goto err_out_iounmap;
> -               }
> -
> -               fw_data = (void *)tp->fw->data;
> -
> -               /* Firmware blob starts with version numbers, followed by
> -                  start address and _full_ length including BSS sections
> -                  (which must be longer than the actual data, of course */
> -
> -               tp->fw_len = be32_to_cpu(fw_data[2]);   /* includes bss */
> -               if (tp->fw_len < (tp->fw->size - 12)) {
> -                       printk(KERN_ERR "tg3: bogus length %d in \"%s\"\n",
> -                              tp->fw_len, fw_name);
> -                       err = -EINVAL;
> +       if (tp->fw_needed) {
> +               err = tg3_request_firmware(tp);
> +               /* Failure to load firmware at this stage is not fatal; we'll
> +                  try again in tg3_open(). So if you have the driver built
> +                  into the kernel, you can still get the firmware loaded
> +                  after userspace is running, when the device comes up. */
> +               if (err != -ENOENT)
>                        goto err_out_fw;
> -               }
>        }
>
>        /* TSO is on by default on chips that support hardware TSO.

Patches cleanly but doesn't build in 2.6.29-rc1:

  CC      drivers/leds/led-core.o
  CC      drivers/leds/led-class.o
  LD      drivers/leds/built-in.o
  CC      drivers/net/tg3.o
drivers/net/tg3.c: In function 'tg3_request_firmware':
drivers/net/tg3.c:7542: error: 'struct tg3' has no member named 'fw_needed'
drivers/net/tg3.c:7544: error: 'struct tg3' has no member named 'fw_needed'
drivers/net/tg3.c:7557: error: 'struct tg3' has no member named 'fw_needed'
drivers/net/tg3.c:7562: error: 'struct tg3' has no member named 'fw_needed'
drivers/net/tg3.c: In function 'tg3_open':
drivers/net/tg3.c:7571: error: 'struct tg3' has no member named 'fw_needed'
drivers/net/tg3.c: In function 'tg3_init_one':
drivers/net/tg3.c:13127: error: 'struct tg3' has no member named 'fw_needed'
drivers/net/tg3.c:13143: error: 'struct tg3' has no member named 'fw_needed'
drivers/net/tg3.c:13145: error: 'struct tg3' has no member named 'fw_needed'
drivers/net/tg3.c:13148: error: 'struct tg3' has no member named 'fw_needed'
make[2]: *** [drivers/net/tg3.o] Error 1
make[1]: *** [drivers/net] Error 2
make: *** [drivers] Error 2
[asuardi@sandman linux-2.6.29-rc1]$


Indeed, struct tg3 in tg3.h doesn't have fw_needed here...


--alessandro

 "Sun keeps rising in the west / I keep on waking fully confused"

   (The Replacements, "Within Your Reach")

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Woodhouse]

On Sun, 2009-01-11 at 17:42 +0100, Alessandro Suardi wrote:
> Patches cleanly but doesn't build in 2.6.29-rc1:

Pants; sorry, I omitted this bit...

diff --git a/drivers/net/tg3.h b/drivers/net/tg3.h
index ae5da60..508def3 100644
--- a/drivers/net/tg3.h
+++ b/drivers/net/tg3.h
@@ -2764,6 +2764,7 @@ struct tg3 {
 	struct ethtool_coalesce		coal;
 
 	/* firmware info */
+	const char			*fw_needed;
 	const struct firmware		*fw;
 	u32				fw_len; /* includes BSS */
 };

-- 
dwmw2

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Alessandro Suardi]

On Sun, Jan 11, 2009 at 5:53 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Sun, 2009-01-11 at 17:42 +0100, Alessandro Suardi wrote:
>> Patches cleanly but doesn't build in 2.6.29-rc1:
>
> Pants; sorry, I omitted this bit...
>
> diff --git a/drivers/net/tg3.h b/drivers/net/tg3.h
> index ae5da60..508def3 100644
> --- a/drivers/net/tg3.h
> +++ b/drivers/net/tg3.h
> @@ -2764,6 +2764,7 @@ struct tg3 {
>        struct ethtool_coalesce         coal;
>
>        /* firmware info */
> +       const char                      *fw_needed;
>        const struct firmware           *fw;
>        u32                             fw_len; /* includes BSS */
>  };
>

This one works, thank you.

However, with the combination of in-kernel tg3 and
 CONFIG_IN_KERNEL_FIRMWARE=n there is at least a 30" delay
 during boot while waiting for the firmware request (which will fail).

Such wait didn't use to be there, and stuff worked. Could the wait
 be removed, to behave as it used to - as well ?

Thanks again, ciao,

--alessandro

 "Sun keeps rising in the west / I keep on waking fully confused"

   (The Replacements, "Within Your Reach")

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Woodhouse]

On Sun, 2009-01-11 at 20:24 +0100, Alessandro Suardi wrote:
> However, with the combination of in-kernel tg3 and
>  CONFIG_IN_KERNEL_FIRMWARE=n there is at least a 30" delay
>  during boot while waiting for the firmware request (which will fail).

Hm, ideally it should fail fast if the root file system isn't yet
mounted. I thought we'd seen and fixed a 'long delay' problem already.

I'll take another look.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]

On Sun, 11 Jan 2009 12:59:59 GMT, David Woodhouse said:
> On Sun, 2009-01-11 at 12:25 +0000, David Woodhouse wrote:
> > I'll take a look and see if I can remedy that. Then we wouldn't _need_
> > the FIRMWARE_IN_KERNEL option.
> 
> How about this? If it fails to load the firmware from userspace during
> the initialisation, it'll try again later in tg3_open(). 
> 
> I _think_ that's fine, because we don't do anything else in the early
> initialisation which requires the firmware to be loaded.
> 
> So if you build with CONFIG_TIGON3=y, CONFIG_FIRMWARE_IN_KERNEL=n, you
> should see it fail to load the firmware at boot, but then it should load
> it successfully when you bring the device up.
> 
> Untested-but-otherwise-Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>

I'll see if I can give it a test drive sometime in the next 24 hours or so.

One unanswered question:  What do we expect the system to do if they have this
patch, TIGON3=y, FIRMWARE_IN_KERNEL=n, and configure a netconsole for boot
messages?  I'm *hoping* the answer is "the netconsole doesn't come up at boot,
but can be re-enabled via the /sys/kernel/config/netconsole interface after
you've done an 'ifconfig eth0 up' or similar, or do a 'modprobe netconsole'.

Those seem like reasonable semantics to me - anybody got a different opinion?


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Woodhouse]

On Sun, 2009-01-11 at 11:48 -0500, Valdis.Kletnieks@vt.edu wrote:
> One unanswered question:  What do we expect the system to do if they have this
> patch, TIGON3=y, FIRMWARE_IN_KERNEL=n, and configure a netconsole for boot
> messages?  I'm *hoping* the answer is "the netconsole doesn't come up at boot,
> but can be re-enabled via the /sys/kernel/config/netconsole interface after
> you've done an 'ifconfig eth0 up' or similar, or do a 'modprobe netconsole'.
> 
> Those seem like reasonable semantics to me - anybody got a different opinion?

If netconsole follows the normal ->open() path, that sounds like what'll
happen.

-- 
dwmw2

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Miller]

From: Valdis.Kletnieks@vt.edu
Date: Sun, 11 Jan 2009 11:48:22 -0500

> One unanswered question:  What do we expect the system to do if they have this
> patch, TIGON3=y, FIRMWARE_IN_KERNEL=n, and configure a netconsole for boot
> messages?  I'm *hoping* the answer is "the netconsole doesn't come up at boot,
> but can be re-enabled via the /sys/kernel/config/netconsole interface after
> you've done an 'ifconfig eth0 up' or similar, or do a 'modprobe netconsole'.
> 
> Those seem like reasonable semantics to me - anybody got a different opinion?

Even better, how about nfsroot?  There is no "later", either you
can mount to root filesystem or you fail.

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]

On Sun, 11 Jan 2009 13:49:37 PST, David Miller said:
> From: Valdis.Kletnieks@vt.edu
> Date: Sun, 11 Jan 2009 11:48:22 -0500
> 
> > One unanswered question:  What do we expect the system to do if they have this
> > patch, TIGON3=y, FIRMWARE_IN_KERNEL=n, and configure a netconsole for boot
> > messages?  I'm *hoping* the answer is "the netconsole doesn't come up at boot,
> > but can be re-enabled via the /sys/kernel/config/netconsole interface after
> > you've done an 'ifconfig eth0 up' or similar, or do a 'modprobe netconsole'.
> > 
> > Those seem like reasonable semantics to me - anybody got a different opinion?
> 
> Even better, how about nfsroot?  There is no "later", either you
> can mount to root filesystem or you fail.

I don't see any sane way for an nfsroot to work unless you've built the kernel
with TIGON3=y, and FIRMWARE=y.  Anything else is just crazy talk.  For that
case, I'd suggest we just do this:

Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>

--- linux-2.6.28-mmotm0109/Documentation/filesystems/nfsroot.txt.dist	2008-12-24 18:26:37.000000000 -0500
+++ linux-2.6.28-mmotm0109/Documentation/filesystems/nfsroot.txt	2009-01-11 17:27:31.000000000 -0500
@@ -30,6 +30,9 @@ In the networking options, kernel level 
 along with the types of autoconfiguration to support. Selecting all of
 DHCP, BOOTP and RARP is safe.
 
+In addition, your network interface driver must be selected as built-in,
+and if the card is a Tigon3 or other card that requires a firmware load
+to become functional, you need to select FIRMWARE_IN_KERNEL.
 
 
 



[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Woodhouse]

On Sun, 2009-01-11 at 17:29 -0500, Valdis.Kletnieks@vt.edu wrote:
> On Sun, 11 Jan 2009 13:49:37 PST, David Miller said:
> > From: Valdis.Kletnieks@vt.edu
> > Date: Sun, 11 Jan 2009 11:48:22 -0500
> > 
> > > One unanswered question:  What do we expect the system to do if they have this
> > > patch, TIGON3=y, FIRMWARE_IN_KERNEL=n, and configure a netconsole for boot
> > > messages?  I'm *hoping* the answer is "the netconsole doesn't come up at boot,
> > > but can be re-enabled via the /sys/kernel/config/netconsole interface after
> > > you've done an 'ifconfig eth0 up' or similar, or do a 'modprobe netconsole'.
> > > 
> > > Those seem like reasonable semantics to me - anybody got a different opinion?
> > 
> > Even better, how about nfsroot?  There is no "later", either you
> > can mount to root filesystem or you fail.
> 
> I don't see any sane way for an nfsroot to work unless you've built the kernel
> with TIGON3=y, and FIRMWARE=y.  Anything else is just crazy talk.  For that
> case, I'd suggest we just do this:
> 
> Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
> 
> --- linux-2.6.28-mmotm0109/Documentation/filesystems/nfsroot.txt.dist	2008-12-24 18:26:37.000000000 -0500
> +++ linux-2.6.28-mmotm0109/Documentation/filesystems/nfsroot.txt	2009-01-11 17:27:31.000000000 -0500
> @@ -30,6 +30,9 @@ In the networking options, kernel level 
>  along with the types of autoconfiguration to support. Selecting all of
>  DHCP, BOOTP and RARP is safe.
>  
> +In addition, your network interface driver must be selected as built-in,
> +and if the card is a Tigon3 or other card that requires a firmware load
> +to become functional, you need to select FIRMWARE_IN_KERNEL.

Yeah, but it's not FIRMWARE_IN_KERNEL in the general case -- that's just
for a handful of 'speshul' legacy drivers. In the general case it's the
CONFIG_EXTRA_FIRMWARE option which needs to be set to include the
required firmware.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Miller]

From: David Woodhouse <dwmw2@infradead.org>
Date: Sun, 11 Jan 2009 12:59:59 +0000

> So if you build with CONFIG_TIGON3=y, CONFIG_FIRMWARE_IN_KERNEL=n, you
> should see it fail to load the firmware at boot, but then it should load
> it successfully when you bring the device up.

And you won't be able to mount an NFS root filesystem.

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 3010 bytes --]

On Sun, 11 Jan 2009 12:59:59 GMT, David Woodhouse said:
> On Sun, 2009-01-11 at 12:25 +0000, David Woodhouse wrote:
> > I'll take a look and see if I can remedy that. Then we wouldn't _need_
> > the FIRMWARE_IN_KERNEL option.
> 
> How about this? If it fails to load the firmware from userspace during
> the initialisation, it'll try again later in tg3_open(). 
> 
> I _think_ that's fine, because we don't do anything else in the early
> initialisation which requires the firmware to be loaded.
> 
> So if you build with CONFIG_TIGON3=y, CONFIG_FIRMWARE_IN_KERNEL=n, you
> should see it fail to load the firmware at boot, but then it should load
> it successfully when you bring the device up.
> 
> Untested-but-otherwise-Signed-off-by: David Woodhouse <David.Woodhouse@intel.
com>
> 
> diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c

So I took it for a test drive, using my currently-working .config, which has

CONFIG_TIGON3=y
CONFIG_FIRMWARE_IN_KERNEL=y

In the dmesg I see during early bootup:

[    0.694230] loop: module loaded
[    0.694249] tg3.c:v3.97 (December 10, 2008)
[    0.694261] vendor=8086 device=27d4
[    0.694265] tg3 0000:09:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
[    0.694274] tg3 0000:09:00.0: setting latency timer to 64
[    0.696087] tg3 0000:09:00.0: wake-up capability disabled by ACPI
[    0.696094] tg3 0000:09:00.0: PME# disabled
[    0.702276] tg3 0000:09:00.0: firmware: using built-in firmware tigon/tg3_tso.bin
[    0.702287] vendor=8086 device=27d4
[    0.702288] tg3 0000:09:00.0: PCI INT A disabled
[    0.702512] console [netcon0] enabled
[    0.702515] netconsole: network logging started
[    0.702575] Driver 'sd' needs updating - please use bus_type methods

but once we get to userspace, 'ifconfig' or 'ip link show' have *zero*
about an eth0 device.

For comparison, the dmesg if I revert your patch:

[    0.696638] loop: module loaded
[    0.696658] tg3.c:v3.97 (December 10, 2008)
[    0.696670] vendor=8086 device=27d4
[    0.696674] tg3 0000:09:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
[    0.696683] tg3 0000:09:00.0: setting latency timer to 64
[    0.698063] tg3 0000:09:00.0: wake-up capability disabled by ACPI
[    0.698070] tg3 0000:09:00.0: PME# disabled
[    0.704276] tg3 0000:09:00.0: firmware: using built-in firmware tigon/tg3_tso.bin
[    0.704445] eth0: Tigon3 [partno(BCM5752KFBG) rev 6002] (PCI Express) MAC address 00:15:c5:c8:33:4e
[    0.704448] eth0: attached PHY is 5752 (10/100/1000Base-T Ethernet) (WireSpeed[1])
[    0.704451] eth0: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1]
[    0.704453] eth0: dma_rwctrl[76180000] dma_mask[64-bit]
[    0.704653] console [netcon0] enabled
[    0.704656] netconsole: network logging started
[    0.704718] Driver 'sd' needs updating - please use bus_type methods

So it looks like the patch is failing to finish initialization of the card.
Damned if *I* can see what's breaking it, the conversion to use a helper
function tg3_request_firmware seems sane enough....


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 2657 bytes --]

On Sun, 11 Jan 2009 19:10:36 EST, Valdis.Kletnieks@vt.edu said:

> In the dmesg I see during early bootup:
> 
> [    0.694230] loop: module loaded
> [    0.694249] tg3.c:v3.97 (December 10, 2008)
> [    0.694261] vendor=8086 device=27d4
> [    0.694265] tg3 0000:09:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
> [    0.694274] tg3 0000:09:00.0: setting latency timer to 64
> [    0.696087] tg3 0000:09:00.0: wake-up capability disabled by ACPI
> [    0.696094] tg3 0000:09:00.0: PME# disabled
> [    0.702276] tg3 0000:09:00.0: firmware: using built-in firmware tigon/tg3_tso.bin
> [    0.702287] vendor=8086 device=27d4
> [    0.702288] tg3 0000:09:00.0: PCI INT A disabled
> [    0.702512] console [netcon0] enabled
> [    0.702515] netconsole: network logging started
> [    0.702575] Driver 'sd' needs updating - please use bus_type methods
> 
> but once we get to userspace, 'ifconfig' or 'ip link show' have *zero*
> about an eth0 device.
> 
> For comparison, the dmesg if I revert your patch:
> 
> [    0.696638] loop: module loaded
> [    0.696658] tg3.c:v3.97 (December 10, 2008)
> [    0.696670] vendor=8086 device=27d4
> [    0.696674] tg3 0000:09:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18
> [    0.696683] tg3 0000:09:00.0: setting latency timer to 64
> [    0.698063] tg3 0000:09:00.0: wake-up capability disabled by ACPI
> [    0.698070] tg3 0000:09:00.0: PME# disabled
> [    0.704276] tg3 0000:09:00.0: firmware: using built-in firmware tigon/tg3_tso.bin
> [    0.704445] eth0: Tigon3 [partno(BCM5752KFBG) rev 6002] (PCI Express) MAC address 00:15:c5:c8:33:4e
> [    0.704448] eth0: attached PHY is 5752 (10/100/1000Base-T Ethernet) (WireSpeed[1])
> [    0.704451] eth0: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1]
> [    0.704453] eth0: dma_rwctrl[76180000] dma_mask[64-bit]
> [    0.704653] console [netcon0] enabled
> [    0.704656] netconsole: network logging started
> [    0.704718] Driver 'sd' needs updating - please use bus_type methods
> 
> So it looks like the patch is failing to finish initialization of the card.
> Damned if *I* can see what's breaking it, the conversion to use a helper
> function tg3_request_firmware seems sane enough....

Damn.  I wonder if netconsole's initialization is turning around and stomping
on everything?  This looks suspicious:

static int tg3_open(struct net_device *dev)
 {
        struct tg3 *tp = netdev_priv(dev);
        int err;
 
+       if (tp->fw_needed) {

Do we know for sure that tp-> struct is the same one we set up back in
tg3_request_firmware, *and* that tg3_open() doesn't get called before
tg3_init_one() (which would result in an uninitialized fw_needed)?

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Woodhouse]

On Sun, 2009-01-11 at 19:10 -0500, Valdis.Kletnieks@vt.edu wrote:
> So it looks like the patch is failing to finish initialization of the card.
> Damned if *I* can see what's breaking it, the conversion to use a helper
> function tg3_request_firmware seems sane enough....

Ahem...

--- drivers/net/tg3.c.stupiddwmw2	2009-01-12 08:13:05.000000000 +0000
+++ drivers/net/tg3.c	2009-01-12 08:13:09.000000000 +0000
@@ -13151,7 +13151,7 @@ static int __devinit tg3_init_one(struct
 		   try again in tg3_open(). So if you have the driver built
 		   into the kernel, you can still get the firmware loaded
 		   after userspace is running, when the device comes up. */
-		if (err != -ENOENT)
+		if (err && err != -ENOENT)
 			goto err_out_fw;
 	}
 

-- 
dwmw2

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]

On Mon, 12 Jan 2009 08:13:59 GMT, David Woodhouse said:
> On Sun, 2009-01-11 at 19:10 -0500, Valdis.Kletnieks@vt.edu wrote:
> > So it looks like the patch is failing to finish initialization of the card.
> > Damned if *I* can see what's breaking it, the conversion to use a helper
> > function tg3_request_firmware seems sane enough....
> 
> Ahem...
> 
> --- drivers/net/tg3.c.stupiddwmw2	2009-01-12 08:13:05.000000000 +0000
> +++ drivers/net/tg3.c	2009-01-12 08:13:09.000000000 +0000
> @@ -13151,7 +13151,7 @@ static int __devinit tg3_init_one(struct

OK, tested on my laptop.  Results:

CONFIG_TIGON3=y CONFIG_PREVENT_FIRMWARE_BUILD=y works - the firmware gets
loaded during early system boot and all is as expected.

CONFIG_TIGON3=y CONFIG_FIRMWARE_IN_KERNEL=n wedges up - Booted with
'ignore_loglevel initcall_debug', and the last thing we get is:

tg3 0000:09:00.0: firmware: requesting tigon/tg3_tso.bin

At which point we hang because request_firmware apparently never returns.
It's wedged up *really* good at that point - I can't even use
alt-sysrq-anything.



[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

On Mon, 12 Jan 2009 08:13:59 GMT, David Woodhouse said:
> On Sun, 2009-01-11 at 19:10 -0500, Valdis.Kletnieks@vt.edu wrote:
> > So it looks like the patch is failing to finish initialization of the card.
> > Damned if *I* can see what's breaking it, the conversion to use a helper
> > function tg3_request_firmware seems sane enough....
> 
> Ahem...
> 
> --- drivers/net/tg3.c.stupiddwmw2	2009-01-12 08:13:05.000000000 +0000
> +++ drivers/net/tg3.c	2009-01-12 08:13:09.000000000 +0000
> @@ -13151,7 +13151,7 @@ static int __devinit tg3_init_one(struct

OK, tested on my laptop.  Results:

CONFIG_TIGON3=y CONFIG_PREVENT_FIRMWARE_BUILD=y works - the firmware gets
loaded during early system boot and all is as expected.

CONFIG_TIGON3=y CONFIG_FIRMWARE_IN_KERNEL=n wedges up - Booted with
'ignore_loglevel initcall_debug', and the last thing we get is:

tg3 0000:09:00.0: firmware: requesting tigon/tg3_tso.bin

At which point we hang because request_firmware apparently never returns.
It's wedged up *really* good at that point - I can't even use
alt-sysrq-anything.




[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Miller]

From: David Woodhouse <dwmw2@infradead.org>
Date: Sun, 11 Jan 2009 12:24:58 +0000

> Other drivers will load the firmware later, at the time the device is
> brought up. This means that even if you build the driver into the kernel
> without its firmware, it can still request the firmware later, when you
> try to start _using_ it. And when the file system is available.

For example, for mounting an NFS root using that device.... Oh, will I
need an initramfs for that once you pull the firmware-in-kernel
option?

To me, device probe is in fact the place to fail firmware discovery
for networking devices.  Because such a failure can mean you can't
mount your root filesystem.

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Woodhouse]

On Sun, 2009-01-11 at 13:39 -0800, David Miller wrote:
> For example, for mounting an NFS root using that device.... Oh, will I
> need an initramfs for that once you pull the firmware-in-kernel
> option?

The option to build arbitrary firmware into your kernel is never going
to be pulled. You can do nfsroot on your myri10ge today, without initrd,
even though the firmware is distributed separately from the kernel. That
doesn't prevent you from building it _in_ to your kernel, if you choose
to.

-- 
dwmw2

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Miller]

From: David Woodhouse <dwmw2@infradead.org>
Date: Sun, 11 Jan 2009 22:02:27 +0000

> On Sun, 2009-01-11 at 13:39 -0800, David Miller wrote:
> > For example, for mounting an NFS root using that device.... Oh, will I
> > need an initramfs for that once you pull the firmware-in-kernel
> > option?
> 
> The option to build arbitrary firmware into your kernel is never going
> to be pulled. You can do nfsroot on your myri10ge today, without initrd,
> even though the firmware is distributed separately from the kernel. That
> doesn't prevent you from building it _in_ to your kernel, if you choose
> to.

So what used to work out of the box by typing make now
will have all kinds of strange depencies, right?

This is a regression, no matter how you spin it, in my
book.

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Woodhouse]

On Sun, 2009-01-11 at 14:05 -0800, David Miller wrote:
> So what used to work out of the box by typing make now
> will have all kinds of strange depencies, right?
> 
> This is a regression, no matter how you spin it, in my
> book.

I know you think that, but I disagree. Yes, it's slightly less
convenient for a small handful of older drivers, but in the general case
it's a massive improvement -- because now we can have a single canonical
location to collect _all_ the firmware that is redistributable, for use
with the kernel. And we've already started adding a whole bunch of new
firmwares to that repository.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Willy Tarreau]

On Sun, Jan 11, 2009 at 10:41:38PM +0000, David Woodhouse wrote:
> On Sun, 2009-01-11 at 14:05 -0800, David Miller wrote:
> > So what used to work out of the box by typing make now
> > will have all kinds of strange depencies, right?
> > 
> > This is a regression, no matter how you spin it, in my
> > book.
> 
> I know you think that, but I disagree. Yes, it's slightly less
> convenient for a small handful of older drivers,

We're speaking about the driver for the on-board NIC which is present on
almost 100% of the servers built since the last 5 years. There are an
awful lot of users of this driver playing with PXE, nfsroot and other
non-legacy boot methods. So "small handful" or not, we must ensure not
to annoy a lot of users with strange bugs and awkward configurations.

> but in the general case
> it's a massive improvement -- because now we can have a single canonical
> location to collect _all_ the firmware that is redistributable, for use
> with the kernel. And we've already started adding a whole bunch of new
> firmwares to that repository.

In the past, the firmware distributed with the driver was always the
one known to work with that version of the driver. Now you'll have to
know what version of the firmware to use with what kernel version,
this is adding a new dimension to troubleshooting complexity.

I still think that these firmware hacks are a solution looking for
a problem to solve. We had something which has worked well for 15
years and now it's easy to see a machine hang at boot time for 60
seconds because a firmware file is missing or invalid.

Regression IMO, I agree with Davem.

Willy

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Woodhouse]

On Sun, 2009-01-11 at 13:39 -0800, David Miller wrote:
> 
> For example, for mounting an NFS root using that device.... Oh, will I
> need an initramfs for that once you pull the firmware-in-kernel
> option?

Of course, we've been talking for ages about pulling the dhcp and
nfsroot hacks from the kernel and requiring an initrd for that _anyway_,
but that's a different issue...

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Miller]

From: David Woodhouse <dwmw2@infradead.org>
Date: Sun, 11 Jan 2009 22:15:54 +0000

> On Sun, 2009-01-11 at 13:39 -0800, David Miller wrote:
> > 
> > For example, for mounting an NFS root using that device.... Oh, will I
> > need an initramfs for that once you pull the firmware-in-kernel
> > option?
> 
> Of course, we've been talking for ages about pulling the dhcp and
> nfsroot hacks from the kernel and requiring an initrd for that _anyway_,
> but that's a different issue...

It's not there now, so in the mean time....

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[Matt Carlson]

On Sun, Jan 11, 2009 at 01:39:29PM -0800, David Miller wrote:
> From: David Woodhouse <dwmw2@infradead.org>
> Date: Sun, 11 Jan 2009 12:24:58 +0000
> 
> > Other drivers will load the firmware later, at the time the device is
> > brought up. This means that even if you build the driver into the kernel
> > without its firmware, it can still request the firmware later, when you
> > try to start _using_ it. And when the file system is available.
> 
> For example, for mounting an NFS root using that device.... Oh, will I
> need an initramfs for that once you pull the firmware-in-kernel
> option?
> 
> To me, device probe is in fact the place to fail firmware discovery
> for networking devices.  Because such a failure can mean you can't
> mount your root filesystem.

David, is this the direction you want to take the fix?  I'm sitting on
a patch that elaborates on David Woodhouse's work which moves the
request_firmware call to tg3_open().  Before I posted it for comment,
I wanted to make sure the patch is moving in the right direction.

Re: 2.6.28-git8: tg3 doesn't work due to firmware not loading (-git7 is ok)

[David Miller]

From: "Matt Carlson" <mcarlson@broadcom.com>
Date: Tue, 13 Jan 2009 10:39:48 -0800

> On Sun, Jan 11, 2009 at 01:39:29PM -0800, David Miller wrote:
> > From: David Woodhouse <dwmw2@infradead.org>
> > Date: Sun, 11 Jan 2009 12:24:58 +0000
> > 
> > > Other drivers will load the firmware later, at the time the device is
> > > brought up. This means that even if you build the driver into the kernel
> > > without its firmware, it can still request the firmware later, when you
> > > try to start _using_ it. And when the file system is available.
> > 
> > For example, for mounting an NFS root using that device.... Oh, will I
> > need an initramfs for that once you pull the firmware-in-kernel
> > option?
> > 
> > To me, device probe is in fact the place to fail firmware discovery
> > for networking devices.  Because such a failure can mean you can't
> > mount your root filesystem.
> 
> David, is this the direction you want to take the fix?  I'm sitting on
> a patch that elaborates on David Woodhouse's work which moves the
> request_firmware call to tg3_open().  Before I posted it for comment,
> I wanted to make sure the patch is moving in the right direction.

You can post it, sure.

But it doesn't actually fix the nfsroot case, that will still be
broken.