netdev - Re: [PATCH] firmware: convert e100 driver to request

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4855A617.90006@garzik.org>
Date:	Sun, 15 Jun 2008 19:30:31 -0400
From:	Jeff Garzik <jeff@...zik.org>
To:	David Woodhouse <dwmw2@...radead.org>
CC:	Jaswinder Singh <jaswinderlinux@...il.com>, netdev@...r.kernel.org,
	e1000-devel@...ts.sourceforge.net,
	Auke Kok <auke-jan.h.kok@...el.com>
Subject: Re: [PATCH] firmware: convert e100 driver to request_firmware()

David Woodhouse wrote:
> On Sun, 2008-06-15 at 14:44 -0400, Jeff Garzik wrote:
>> Each driver is _intimately_ tied to its firmware, so it only increases
>> headaches by separating two pieces of a single, cohesive whole.
> 
> On reflection, that argument _does_ make me inclined to modify my
> approach.
> 
> The _only_ thing that lets us justify the inclusion of this firmware
> blob in the kernel in the first place is the rather tenuous claim that
> it is "mere aggregation on a volume of a storage or distribution medium"
> -- as if it's just some kind of coincidence that these two pieces of
> work happen to be shipped together.
> 
> If they really are "intimately tied pieces of a single, cohesive whole",
> as you say, then the claim that it's "mere aggregation on a volume of a
> storage or distribution medium" loses whatever small amount of
> credibility it had in the first place, and we have no option but to
> remove the offending part immediately.
> 
> Revised patch follows....
> 
>  drivers/net/Kconfig |    1 
>  drivers/net/e100.c  |  281 +++++++++++++++-------------------------------------
>  2 files changed, 86 insertions(+), 196 deletions(-)

So, in response you separate them further?  No thanks.

Put the data in drivers/net/e100.firmware.  It makes no sense to have 
two parallel driver hierarchies, one for C source and one for firmwares.

And IMO I would think the best first step for all such drivers would be 
to add request_firmware() support _without_ touching the embedded 
firmware.  You are trying to stuff too much change into a single patch.

Such a step gets most of your driver modifications into the kernel with 
the least headache and controversy, while adding the quite-useful 
ability to have a driver load an external firmware instead of the 
embedded one.

request_firmware() infrastructure addition is clearly a separate and 
distinct change from moving/removing an embedded firmware.

	Jeff

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html