2007-09-21 11:01:48

by Denys Vlasenko

[permalink] [raw]
Subject: [PATCH 1/2] bnx2: factor out gzip unpacker

Hi Jeff,

This patch modifies gzip unpacking code in bnx2 driver so that
it does not depend on bnx2 internals. I will move this code
out of the driver and into zlib in follow-on patch.

It can be useful in other drivers which need to store firmwares
or any other relatively big binary blobs - fonts, cursor bitmaps,
whatever.

Patch is run tested by Michael Chan (driver author).

Michael, can you add your ACK?

Signed-off-by: Denys Vlasenko <[email protected]>
--
vda


Attachments:
(No filename) (477.00 B)
linux-2.6.23-rc6.bnx2-1.patch (12.22 kB)
linux-2.6.23-rc6.bnx2-2.patch (9.13 kB)
Download all attachments

2007-09-21 11:03:52

by Denys Vlasenko

[permalink] [raw]
Subject: [PATCH 2/2] bnx2: move gzip unpacker to zlib

On Friday 21 September 2007 12:01, Denys Vlasenko wrote:
> I will move this code
> out of the driver and into zlib in follow-on patch.

No, I won't. I accidentally attached both patches to first email,
you can find it there. Sorry.
--
vda

2007-09-21 16:14:41

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

From: Denys Vlasenko <[email protected]>
Date: Fri, 21 Sep 2007 12:01:24 +0100

> Hi Jeff,

BNX2 and TG3 patches goes through Michael Chan and myself,
and I usually merge them in instead of Jeff.

2007-09-21 17:04:19

by Denys Vlasenko

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

On Friday 21 September 2007 17:14, David Miller wrote:
> From: Denys Vlasenko <[email protected]>
> Date: Fri, 21 Sep 2007 12:01:24 +0100
>
> > Hi Jeff,
>
> BNX2 and TG3 patches goes through Michael Chan and myself,
> and I usually merge them in instead of Jeff.

Didn't know that, sorry.

Do patches look ok to you?
--
vda

2007-09-21 17:49:22

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

From: Denys Vlasenko <[email protected]>
Date: Fri, 21 Sep 2007 18:03:55 +0100

> Do patches look ok to you?

I'm travelling so I haven't looked closely yet :-)

Michael can take a look and I'll try to do so as well
tonight.

2007-09-21 18:05:43

by Denys Vlasenko

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

On Friday 21 September 2007 18:49, David Miller wrote:
> From: Denys Vlasenko <[email protected]>
> Date: Fri, 21 Sep 2007 18:03:55 +0100
>
> > Do patches look ok to you?
>
> I'm travelling so I haven't looked closely yet :-)
>
> Michael can take a look and I'll try to do so as well
> tonight.

Good.

I plan to use gzip compression on following drivers' firmware,
if patches will be accepted:

text data bss dec hex filename
17653 109968 240 127861 1f375 drivers/net/acenic.o
6628 120448 4 127080 1f068 drivers/net/dgrs.o
^^^^^^

--
vda

2007-09-21 18:37:20

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

On Fri, 21 Sep 2007 19:05:23 BST, Denys Vlasenko said:

> I plan to use gzip compression on following drivers' firmware,
> if patches will be accepted:
>
> text data bss dec hex filename
> 17653 109968 240 127861 1f375 drivers/net/acenic.o
> 6628 120448 4 127080 1f068 drivers/net/dgrs.o
> ^^^^^^

Should this be redone to use the existing firmware loading framework to
load the firmware instead?


Attachments:
(No filename) (226.00 B)

2007-09-21 19:18:24

by Denys Vlasenko

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

On Friday 21 September 2007 19:36, [email protected] wrote:
> On Fri, 21 Sep 2007 19:05:23 BST, Denys Vlasenko said:
>
> > I plan to use gzip compression on following drivers' firmware,
> > if patches will be accepted:
> >
> > text data bss dec hex filename
> > 17653 109968 240 127861 1f375 drivers/net/acenic.o
> > 6628 120448 4 127080 1f068 drivers/net/dgrs.o
> > ^^^^^^
>
> Should this be redone to use the existing firmware loading framework to
> load the firmware instead?

Not in every case.

For example, bnx2 maintainer says that driver and
firmware are closely tied for his driver. IOW: you upgrade kernel
and your NIC is not working anymore.

Another argument is to make kernel be able to bring up NICs
without needing firmware images in initramfs/initrd/hard drive.
--
vda

2007-09-21 19:33:21

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

On Fri, 21 Sep 2007 20:18:06 BST, Denys Vlasenko said:
> On Friday 21 September 2007 19:36, [email protected] wrote:
> > Should this be redone to use the existing firmware loading framework to
> > load the firmware instead?
>
> Not in every case.
>
> For example, bnx2 maintainer says that driver and
> firmware are closely tied for his driver. IOW: you upgrade kernel
> and your NIC is not working anymore.
>
> Another argument is to make kernel be able to bring up NICs
> without needing firmware images in initramfs/initrd/hard drive.

OK, I can live with "considered and decided against". :)


Attachments:
(No filename) (226.00 B)

2007-09-21 20:09:44

by Krzysztof Oledzki

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker



On Fri, 21 Sep 2007, Denys Vlasenko wrote:

> On Friday 21 September 2007 19:36, [email protected] wrote:
>> On Fri, 21 Sep 2007 19:05:23 BST, Denys Vlasenko said:
>>
>>> I plan to use gzip compression on following drivers' firmware,
>>> if patches will be accepted:
>>>
>>> text data bss dec hex filename
>>> 17653 109968 240 127861 1f375 drivers/net/acenic.o
>>> 6628 120448 4 127080 1f068 drivers/net/dgrs.o
>>> ^^^^^^
>>
>> Should this be redone to use the existing firmware loading framework to
>> load the firmware instead?
>
> Not in every case.
>
> For example, bnx2 maintainer says that driver and
> firmware are closely tied for his driver. IOW: you upgrade kernel
> and your NIC is not working anymore.
Firmware may come with a kernel. We have a "install modules", we can also
add "install firmware".

> Another argument is to make kernel be able to bring up NICs
> without needing firmware images in initramfs/initrd/hard drive.

It is not possible to bring up things like FC or WiFi without firmware,
what special is in classic NICs?

Best regards,

Krzysztof Ol?dzki

2007-09-21 20:14:01

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

Denys Vlasenko <[email protected]> writes:
>
> I plan to use gzip compression on following drivers' firmware,
> if patches will be accepted:
>
> text data bss dec hex filename
> 17653 109968 240 127861 1f375 drivers/net/acenic.o
> 6628 120448 4 127080 1f068 drivers/net/dgrs.o
> ^^^^^^

Just change the makefiles to always install gzip'ed modules
modutils knows how to unzip them on the fly.

That will catch all the firmware and all the other code too.

-Andi

2007-09-21 20:31:21

by Roland Dreier

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

> Just change the makefiles to always install gzip'ed modules
> modutils knows how to unzip them on the fly.

But that leaves the uncompressed firmware blobs in .data that ends up
in unswappable kernel memory.

- R.

2007-09-21 21:03:35

by Alan

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

> For example, bnx2 maintainer says that driver and
> firmware are closely tied for his driver. IOW: you upgrade kernel
> and your NIC is not working anymore.
>
> Another argument is to make kernel be able to bring up NICs
> without needing firmware images in initramfs/initrd/hard drive.

dgrs should be using the request_firmware interface. Actually dgrs is
probably a good candidate for /dev/null

Alan

2007-09-21 22:52:50

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

Alan Cox wrote:
>> For example, bnx2 maintainer says that driver and
>> firmware are closely tied for his driver. IOW: you upgrade kernel
>> and your NIC is not working anymore.
>>
>> Another argument is to make kernel be able to bring up NICs
>> without needing firmware images in initramfs/initrd/hard drive.
>
> dgrs should be using the request_firmware interface. Actually dgrs is
> probably a good candidate for /dev/null

According to an earlier thread, dgrs was never really maintained,
written for hardware that was never really distributed widely, and very
likely hasn't had users in years... if ever.

If that picture is accurate (it's a story I was told), then I am
definitely queueing up a deletion patch.

Jeff



2007-09-21 22:54:12

by Denys Vlasenko

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

On Friday 21 September 2007 21:13, Andi Kleen wrote:
> Denys Vlasenko <[email protected]> writes:
> >
> > I plan to use gzip compression on following drivers' firmware,
> > if patches will be accepted:
> >
> > text data bss dec hex filename
> > 17653 109968 240 127861 1f375 drivers/net/acenic.o
> > 6628 120448 4 127080 1f068 drivers/net/dgrs.o
> > ^^^^^^
>
> Just change the makefiles to always install gzip'ed modules
> modutils knows how to unzip them on the fly.

But I compile net/* into bzImage. I like netbooting :)
--
vda

2007-09-21 22:55:27

by Alan

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

> According to an earlier thread, dgrs was never really maintained,
> written for hardware that was never really distributed widely, and very
> likely hasn't had users in years... if ever.
>
> If that picture is accurate (it's a story I was told), then I am
> definitely queueing up a deletion patch.

I think thats sensible. If someone whines it can be put back but I really
don't think anyone will

2007-09-21 22:56:37

by Denys Vlasenko

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

On Friday 21 September 2007 20:33, Krzysztof Oledzki wrote:
>
> On Fri, 21 Sep 2007, Denys Vlasenko wrote:
>
> > On Friday 21 September 2007 19:36, [email protected] wrote:
> >> On Fri, 21 Sep 2007 19:05:23 BST, Denys Vlasenko said:
> >>
> >>> I plan to use gzip compression on following drivers' firmware,
> >>> if patches will be accepted:
> >>>
> >>> text data bss dec hex filename
> >>> 17653 109968 240 127861 1f375 drivers/net/acenic.o
> >>> 6628 120448 4 127080 1f068 drivers/net/dgrs.o
> >>> ^^^^^^
> >>
> >> Should this be redone to use the existing firmware loading framework to
> >> load the firmware instead?
> >
> > Not in every case.
> >
> > For example, bnx2 maintainer says that driver and
> > firmware are closely tied for his driver. IOW: you upgrade kernel
> > and your NIC is not working anymore.
>
> Firmware may come with a kernel. We have a "install modules", we can also
> add "install firmware".

Install where? I boot my machine over NFS, and it has no hard drive.

> > Another argument is to make kernel be able to bring up NICs
> > without needing firmware images in initramfs/initrd/hard drive.
>
> It is not possible to bring up things like FC or WiFi without firmware,
> what special is in classic NICs?

Nothing.

It is just not (yet?) decreed from The Very Top that all and every
firmware image should be loaded using request_firmware().

Also people may want to gzip something else than firmware.
--
vda

2007-09-21 22:59:14

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

Denys Vlasenko wrote:
> On Friday 21 September 2007 20:33, Krzysztof Oledzki wrote:
>> On Fri, 21 Sep 2007, Denys Vlasenko wrote:
>>
>>> On Friday 21 September 2007 19:36, [email protected] wrote:
>>>> On Fri, 21 Sep 2007 19:05:23 BST, Denys Vlasenko said:
>>>>
>>>>> I plan to use gzip compression on following drivers' firmware,
>>>>> if patches will be accepted:
>>>>>
>>>>> text data bss dec hex filename
>>>>> 17653 109968 240 127861 1f375 drivers/net/acenic.o
>>>>> 6628 120448 4 127080 1f068 drivers/net/dgrs.o
>>>>> ^^^^^^
>>>> Should this be redone to use the existing firmware loading framework to
>>>> load the firmware instead?
>>> Not in every case.
>>>
>>> For example, bnx2 maintainer says that driver and
>>> firmware are closely tied for his driver. IOW: you upgrade kernel
>>> and your NIC is not working anymore.
>> Firmware may come with a kernel. We have a "install modules", we can also
>> add "install firmware".
>
> Install where? I boot my machine over NFS, and it has no hard drive.

Special cases already fail when using distro-linked targets like "make
install."

Jeff



2007-09-22 00:02:45

by maximilian attems

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

On Fri, Sep 21, 2007 at 11:48:05PM +0100, Alan Cox wrote:
> > According to an earlier thread, dgrs was never really maintained,
> > written for hardware that was never really distributed widely, and very
> > likely hasn't had users in years... if ever.
> >
> > If that picture is accurate (it's a story I was told), then I am
> > definitely queueing up a deletion patch.
>
> I think thats sensible. If someone whines it can be put back but I really
> don't think anyone will

nobody did yet, please yell if you need a rebased patch.

--
maks

2007-09-22 01:49:20

by Michael Chan

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

On Fri, 2007-09-21 at 10:49 -0700, David Miller wrote:
> From: Denys Vlasenko <[email protected]>
> Date: Fri, 21 Sep 2007 18:03:55 +0100
>
> > Do patches look ok to you?
>
> I'm travelling so I haven't looked closely yet :-)
>
> Michael can take a look and I'll try to do so as well
> tonight.
>

I've already reviewed the earlier versions of the patch and have made
some suggestions. This latest one looks ok to me and tested ok.

I'll follow up later with another patch to remove all the zeros in other
firmware sections, and to remove the gzip headers completely.

Acked-by: Michael Chan <[email protected]>

2007-09-24 17:32:36

by Lennart Sorensen

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

On Fri, Sep 21, 2007 at 11:37:52PM +0100, Denys Vlasenko wrote:
> But I compile net/* into bzImage. I like netbooting :)

Isn't it possible to netboot with an initramfs image? I am pretty sure
I have seen some systems do exactly that.

--
Len Sorensen

2007-09-25 04:20:29

by Kyle Moffett

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

On Sep 24, 2007, at 13:32:23, Lennart Sorensen wrote:
> On Fri, Sep 21, 2007 at 11:37:52PM +0100, Denys Vlasenko wrote:
>> But I compile net/* into bzImage. I like netbooting :)
>
> Isn't it possible to netboot with an initramfs image? I am pretty
> sure I have seen some systems do exactly that.

Yeah, I've got Debian boxes that have never *not* netbooted (one Dell
Op^?^?Craptiplex box whose BIOS and ACPI sucks so bad it can't even
load GRUB/LILO, although Windows somehow works fine). So they boot
PXELinux using the PXE boot ROM on the NICs and it loads both a
kernel and an initramfs into memory. Kernel is stock Debian and
hardly has enough built-in to spit at you, let alone find network/
disks, but it manages to load everything it needs off the
automagically-generated initramfs.

Cheers,
Kyle Moffett

2007-10-01 00:57:37

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 1/2] bnx2: factor out gzip unpacker

From: "Michael Chan" <[email protected]>
Date: Fri, 21 Sep 2007 19:47:17 -0700

> On Fri, 2007-09-21 at 10:49 -0700, David Miller wrote:
> > From: Denys Vlasenko <[email protected]>
> > Date: Fri, 21 Sep 2007 18:03:55 +0100
> >
> > > Do patches look ok to you?
> >
> > I'm travelling so I haven't looked closely yet :-)
> >
> > Michael can take a look and I'll try to do so as well
> > tonight.
> >
>
> I've already reviewed the earlier versions of the patch and have made
> some suggestions. This latest one looks ok to me and tested ok.
>
> I'll follow up later with another patch to remove all the zeros in other
> firmware sections, and to remove the gzip headers completely.
>
> Acked-by: Michael Chan <[email protected]>

I've added these patches to net-2.6.24, thanks.