Re: [PATCH 00/04] Load keyspan firmware with hotplug

Re: [PATCH 00/04] Load keyspan firmware with hotplug

[Humberto Massa]

Sven Luther wrote:

>  On Tue, Apr 05, 2005 at 12:23:29AM -0400, Jan Harkes wrote:
> > This ofcourse doesn't actually solve Debian's distribution issues since
> > the keyspan firmware can only be distributed as part of 'Linux or other
> > Open Source operating system kernel'.
>
>
>  Well, if this is the case, it can be distributed on the non-free
>  archive.

Nope, looks awfully non-distributable for me, unless you put it into a 
kernel-non-free with the entire kernel -- but it says clearly that it's 
undistributable by itself.


HTH,
Massa

non-free firmware in kernel modules, aggregation and unclear copyright notice.

[Sven Luther]

Hello,

<quick sumary>
Current linux kernel source hold undistributable non-free firmware blobs, and
to consider them as mere agregation, a clear licence statement from the
copyright holders of said non-free firmware blobls is needed, read below for
details.
</quick sumary>

Please keep everyone in the CC, as not everyone reads debian-legal or LKML.

Some kernel modules present in the kernel sources as distributed from
ftp.kernel.org present some non-free binary only firmware that gets uploaded
in the target chip by the controler. tg3, qla2xxx, acenix and a couple of
others are example of such modules with non-free firmware blobs.

This is no major problem per see, since, as discussed in this thread :

  http://lists.debian.org/debian-legal/2005/03/msg00283.html

It is obvious in this context that the non-free firmware constitute a mere
aggregation and not an act of linking with the rest of the kernel. This is at
least the consensus that debian has reached with input from the debian-legal
lists, and what we will stand by this.

Naturally even if debian has come to the conclusion that these non-free
firmware blobs are not a violation of the rest of the kernel GPL licence, it
still doesn't make these non-free firmware blobs free software, and thus they
and the drivers which contain it will be removed from debian/main, and put
into the non-free section of our archive.

Now, these non-free firmware are distributed in the same file as the rest of
the module which uses it. This is still ok since it constitute agregation on
a same distribution media, where the distribution media is the file in this
case. 

But these files, as seen in the tg3.c case, have no special mention of the
firmware in the file header, nor are they distinguished in any way from the
rest of the content of that file, which places them de facto under the GPL,
since.

Accordying to the GPL, we thus needs the source files for this non-free
firmware, which is not available, and thus makes the files undistributable.
Even if we would consider these firmware as separate and not covered by the
de-facto GPLing of the files in question, we still would have no licence
allowing us to distribute those non-free firmware blobs, and thus we have
again no right to distribute them as part of the kernel.

The clean solution is to have a small notice in the header of those files or
in the toplevel COPYING file, excluding those firmware blobs from the general
GPLing of the files, and have a small comment inside the files to identify the
firmware blobs as such and again excluding them from the GPL, and possibly a
toplevel listing of all the files wich have such problems.

This is an easy fix, and i believe even those who held the above analysis as
non-sense or whatever will agree that this is something that should be done.
The real problem being that nobody except the copyright holder of those
firmware blobs is legally allowed to make said modification, and thus i bring
this issue to everyone's attention, for comment and feedback, before trying to
reach the copyright holders of those individual firmware blobs asking them to
clarify the situation. I believe many of those read this list anyway, so would
be able to fix the issue or comment on it without further proding needed.

In hopes of quick resolution of these murky legalese issues nobody is really
fond of, 

Friendly,

Sven Luther

Re: non-free firmware in kernel modules, aggregation and unclear copyright notice.

[Michael Poole]

Sven Luther writes:

> Hello,
>
> <quick sumary>
> Current linux kernel source hold undistributable non-free firmware blobs, and
> to consider them as mere agregation, a clear licence statement from the
> copyright holders of said non-free firmware blobls is needed, read below for
> details.
> </quick sumary>
>
> Please keep everyone in the CC, as not everyone reads debian-legal or LKML.

This question comes up every four or five months.  You might have even
been the last one to raise this question on one or more of the mailing
lists you cc'ed.  Please, go check the list archives for the previous
(lengthy and multiple) discussions about this topic.

Michael Poole

Re: non-free firmware in kernel modules, aggregation and unclear copyright notice.

[Sven Luther]

On Mon, Apr 04, 2005 at 09:26:58AM -0400, Michael Poole wrote:
> Sven Luther writes:
> 
> > Hello,
> >
> > <quick sumary>
> > Current linux kernel source hold undistributable non-free firmware blobs, and
> > to consider them as mere agregation, a clear licence statement from the
> > copyright holders of said non-free firmware blobls is needed, read below for
> > details.
> > </quick sumary>
> >
> > Please keep everyone in the CC, as not everyone reads debian-legal or LKML.
> 
> This question comes up every four or five months.  You might have even
> been the last one to raise this question on one or more of the mailing
> lists you cc'ed.  Please, go check the list archives for the previous
> (lengthy and multiple) discussions about this topic.

Sure, i raised this the last time, and it was discussed on debian-legal and
debian-kernel, and since nobody objected, and many where in accord with my
arguments in that thread i linked in the parent post, i believe consensus was
reached. This is basically the position debian has, and work has already been
started to move some of the affected modules in a separate package, which will
be distributed from non-free.

This is just the followup on said discussion, involving the larger LKML
audience, in order to get this fixed for good. As said, it is just a mere
technicality to get out of the muddy situation, all the people having
contributed source-less firmware blobs, need to give us (us being debian, but
also all the linux kernel community) either the source if they persist in
distributing the code under the GPL, or a clear distribution licence for these
firmware blobs, and clearly identificate them as not covered by the GPL that
the file they come in is.

Discussing legal issues is all cool and nice for those that appreciates such
sport, but it doesn't really make sense if it is not applied to acts later on.

Friendly,

Sven Luther

Re: non-free firmware in kernel modules, aggregation and unclear copyright notice.

[Greg KH]

On Mon, Apr 04, 2005 at 04:16:47PM +0200, Sven Luther wrote:
> This is just the followup on said discussion, involving the larger LKML
> audience, in order to get this fixed for good. As said, it is just a mere
> technicality to get out of the muddy situation, all the people having
> contributed source-less firmware blobs, need to give us (us being debian, but
> also all the linux kernel community) either the source if they persist in
> distributing the code under the GPL, or a clear distribution licence for these
> firmware blobs, and clearly identificate them as not covered by the GPL that
> the file they come in is.

What if we don't want to do so?  I know I personally posted a solution
for this _5_ years ago in debian-legal, and have yet to receive a
patch...

> Discussing legal issues is all cool and nice for those that appreciates such
> sport, but it doesn't really make sense if it is not applied to acts later on.

Then let's see some acts.  We (lkml) are not the ones with the percieved
problem, or the ones discussing it.

bleah.

greg k-h

Re: non-free firmware in kernel modules, aggregation and unclear copyright notice.

[Sven Luther]

On Mon, Apr 04, 2005 at 10:51:30AM -0700, Greg KH wrote:
> On Mon, Apr 04, 2005 at 04:16:47PM +0200, Sven Luther wrote:
> > This is just the followup on said discussion, involving the larger LKML
> > audience, in order to get this fixed for good. As said, it is just a mere
> > technicality to get out of the muddy situation, all the people having
> > contributed source-less firmware blobs, need to give us (us being debian, but
> > also all the linux kernel community) either the source if they persist in
> > distributing the code under the GPL, or a clear distribution licence for these
> > firmware blobs, and clearly identificate them as not covered by the GPL that
> > the file they come in is.
> 
> What if we don't want to do so?  I know I personally posted a solution
> for this _5_ years ago in debian-legal, and have yet to receive a
> patch...

Mmm, probably that 2001 discussion about the keyspan firmware, right ?

  http://lists.debian.org/debian-legal/2001/04/msg00145.html

Can you summarize the conclusion of the thread, or what you did get from it,
please ? 

Friendly,

Sven Luther

Re: non-free firmware in kernel modules, aggregation and unclear copyright notice.

[Greg KH]

On Mon, Apr 04, 2005 at 08:27:53PM +0200, Sven Luther wrote:
> Mmm, probably that 2001 discussion about the keyspan firmware, right ?
> 
>   http://lists.debian.org/debian-legal/2001/04/msg00145.html
> 
> Can you summarize the conclusion of the thread, or what you did get from it,
> please ? 

That people didn't like the inclusion of firmware, I posted how you can
fix it by moving it outside of the kernel, and asked for patches.

None have come.

So I refuse to listen to talk about this, as obviously, no one cares
enough about this to actually fix the issue.

People drag this up about once a year, so you are just following the
trend...

This is my last reply to this thread, unless it contains code.

greg k-h

[PATCH 00/04] Load keyspan firmware with hotplug

[Jan Harkes]

On Mon, Apr 04, 2005 at 12:17:46PM -0700, Greg KH wrote:
> On Mon, Apr 04, 2005 at 08:27:53PM +0200, Sven Luther wrote:
> > Mmm, probably that 2001 discussion about the keyspan firmware, right ?
> > 
> >   http://lists.debian.org/debian-legal/2001/04/msg00145.html
> > 
> > Can you summarize the conclusion of the thread, or what you did get from it,
> > please ? 
> 
> That people didn't like the inclusion of firmware, I posted how you can
> fix it by moving it outside of the kernel, and asked for patches.
> 
> None have come.

Didn't know you were waiting for it. How about something like the
following series of patches?

[01/04] - add simple Intel IHEX format parser to the firmware loader.
[02/04] - make the keyspan driver use request_firmware.
[03/04] - converter program used to dump the keyspan headers as IHex files.
[04/04] - result of running the previous program.

This ofcourse doesn't actually solve Debian's distribution issues since
the keyspan firmware can only be distributed as part of 'Linux or other
Open Source operating system kernel'.

> So I refuse to listen to talk about this, as obviously, no one cares
> enough about this to actually fix the issue.

I got tired of always building my own kernels on Debian just to get my
serial dongle to work since their included keyspan.ko driver is so
useless that it isn't even worth having. The only way to use it with a
Debian kernel is to have the dongle in a powered hub and first boot into
Windows or a normal kernel.org kernel to get the thing initialized.
Didn't send the patch earlier since I wanted to split off the
pre-numeration part of the driver so that after intialization we can
unload the unused parts of the driver as well as the the firmware class
module.

Jan

Re: [PATCH 00/04] Load keyspan firmware with hotplug

[Dmitry Torokhov]

On Monday 04 April 2005 23:23, Jan Harkes wrote:
> On Mon, Apr 04, 2005 at 12:17:46PM -0700, Greg KH wrote:
> > On Mon, Apr 04, 2005 at 08:27:53PM +0200, Sven Luther wrote:
> > > Mmm, probably that 2001 discussion about the keyspan firmware, right ?
> > > 
> > >   http://lists.debian.org/debian-legal/2001/04/msg00145.html
> > > 
> > > Can you summarize the conclusion of the thread, or what you did get from it,
> > > please ? 
> > 
> > That people didn't like the inclusion of firmware, I posted how you can
> > fix it by moving it outside of the kernel, and asked for patches.
> > 
> > None have come.
> 
> Didn't know you were waiting for it. How about something like the
> following series of patches?
> 
> [01/04] - add simple Intel IHEX format parser to the firmware loader.

Firmware loader is format-agnostic, I think having IHEX parser in a separate
file would be better...

-- 
Dmitry

Re: [PATCH 00/04] Load keyspan firmware with hotplug

[Kay Sievers]

On Mon, 2005-04-04 at 23:51 -0500, Dmitry Torokhov wrote:
> On Monday 04 April 2005 23:23, Jan Harkes wrote:
> > On Mon, Apr 04, 2005 at 12:17:46PM -0700, Greg KH wrote:
> > > On Mon, Apr 04, 2005 at 08:27:53PM +0200, Sven Luther wrote:
> > > > Mmm, probably that 2001 discussion about the keyspan firmware, right ?
> > > > 
> > > >   http://lists.debian.org/debian-legal/2001/04/msg00145.html
> > > > 
> > > > Can you summarize the conclusion of the thread, or what you did get from it,
> > > > please ? 
> > > 
> > > That people didn't like the inclusion of firmware, I posted how you can
> > > fix it by moving it outside of the kernel, and asked for patches.
> > > 
> > > None have come.
> > 
> > Didn't know you were waiting for it. How about something like the
> > following series of patches?
> > 
> > [01/04] - add simple Intel IHEX format parser to the firmware loader.
> 
> Firmware loader is format-agnostic, I think having IHEX parser in a separate
> file would be better...

Why should this be in-kernel at all? Convert the firmware into a binary
blob or do it in the userspace request.

Kay

Re: [PATCH 00/04] Load keyspan firmware with hotplug

[Jan Harkes]

On Tue, Apr 05, 2005 at 10:32:38AM +0200, Kay Sievers wrote:
> On Mon, 2005-04-04 at 23:51 -0500, Dmitry Torokhov wrote:
> > Firmware loader is format-agnostic, I think having IHEX parser in a separate
> > file would be better...
> 
> Why should this be in-kernel at all? Convert the firmware into a binary
> blob or do it in the userspace request.

Because the keyspan headers seem to have a specific sequence of
operations, something like 'load these 5 bytes at offset 10, load this
byte at offset 0, etc.' I don't know if there is some magic in that
sequence. Without knowing what is in the firmware, it looks to me like a
little bit of bootstrap code is loaded first.

Jan

Re: [PATCH 00/04] Load keyspan firmware with hotplug

[Marcel Holtmann]

Hi Jan,

> > > > Mmm, probably that 2001 discussion about the keyspan firmware, right ?
> > > > 
> > > >   http://lists.debian.org/debian-legal/2001/04/msg00145.html
> > > > 
> > > > Can you summarize the conclusion of the thread, or what you did get from it,
> > > > please ? 
> > > 
> > > That people didn't like the inclusion of firmware, I posted how you can
> > > fix it by moving it outside of the kernel, and asked for patches.
> > > 
> > > None have come.
> > 
> > Didn't know you were waiting for it. How about something like the
> > following series of patches?
> > 
> > [01/04] - add simple Intel IHEX format parser to the firmware loader.
> 
> Firmware loader is format-agnostic, I think having IHEX parser in a separate
> file would be better...

I agree with Dmitry on this point. The IHEX parser should not be inside
firmware_class.c. What about using keyspan_ihex.[ch] for it?

Regards

Marcel

Re: [PATCH 00/04] Load keyspan firmware with hotplug

[Jan Harkes]

On Tue, Apr 05, 2005 at 11:22:06AM +0200, Marcel Holtmann wrote:
> I agree with Dmitry on this point. The IHEX parser should not be inside
> firmware_class.c. What about using keyspan_ihex.[ch] for it?

That's what I had originally, actually called firmware_ihex.ko, since
the IHEX format parser is not in any way keyspan specific and there are
several usb-serial converters that seem to use the same IHEX->.h trick
which could trivially be modified to use this loader.

But the compiled parser fairly small (< 2KB) and adding it to the
existing module didn't effectively add any size to the firmware_class
module since things are rounded to a page boundary anyways.

Jan

Re: [PATCH 00/04] Load keyspan firmware with hotplug

[Marcel Holtmann]

Hi Jan,

> > I agree with Dmitry on this point. The IHEX parser should not be inside
> > firmware_class.c. What about using keyspan_ihex.[ch] for it?
> 
> That's what I had originally, actually called firmware_ihex.ko, since
> the IHEX format parser is not in any way keyspan specific and there are
> several usb-serial converters that seem to use the same IHEX->.h trick
> which could trivially be modified to use this loader.
> 
> But the compiled parser fairly small (< 2KB) and adding it to the
> existing module didn't effectively add any size to the firmware_class
> module since things are rounded to a page boundary anyways.

so it seems that this is usb-serial specific at the moment. Then I would
propose to add it to the core of the usb-serial driver. Unless no other
driver in the kernel needs a IHEX parser, I think it is bad idea to mess
it up at the moment. People are also working on a replacement for the
current request_firmware(), because the needs are changing. Try to keep
it close with the usb-serial for now.

Regards

Marcel

Re: [PATCH 00/04] Load keyspan firmware with hotplug

[Dmitry Torokhov]

On Apr 5, 2005 9:36 AM, Marcel Holtmann <marcel@holtmann.org> wrote:

> People are also working on a replacement for the
> current request_firmware(), because the needs are changing. Try to keep
> it close with the usb-serial for now.
> 

Could you elaborate on what do you think is needed? I have some of
patches to firmware loader and wondering if we should coordinate out
efforts. I have

1. convert from using a timer to wait_for_comletion_timeout which
simplifies logic
2. tightening of state transition rules and removing possible memory
leak (very unlikely)
3. converting firmware_priv to fw_class_dev to simplify memory management.
4. removing request_firmware_nowait as noone seems to be using it -
and I doubt one would ever want to request firmware from an interrupt.
5. Creating firmware class in a separate thread to work around selinux
(with prism54 firmware is loaded when interface is configured and thus
firware loader runs in context of /sbin/ip which may not have access
to sysfs. Having separate thread will ensure that firmware loader runs
in kernel context).

And I was thinking about caching firmware (siomething like if you do
"echo 2 > /sys/class/firmware/blah/loading" instead of 0 it will keep
a copy of the firmware in memory. One could see all firmwares in
/sys/class/firmware/loaded/<xxx> and by erase cached firmware by
echoing 0 into it).

What do you think?

-- 
Dmitry

Re: [PATCH 00/04] Load keyspan firmware with hotplug

[Marcel Holtmann]

Hi Dmitry,

> > People are also working on a replacement for the
> > current request_firmware(), because the needs are changing. Try to keep
> > it close with the usb-serial for now.
> > 
> 
> Could you elaborate on what do you think is needed? I have some of
> patches to firmware loader and wondering if we should coordinate out
> efforts. I have
> 
> 1. convert from using a timer to wait_for_comletion_timeout which
> simplifies logic
> 2. tightening of state transition rules and removing possible memory
> leak (very unlikely)
> 3. converting firmware_priv to fw_class_dev to simplify memory management.
> 4. removing request_firmware_nowait as noone seems to be using it -
> and I doubt one would ever want to request firmware from an interrupt.
> 5. Creating firmware class in a separate thread to work around selinux
> (with prism54 firmware is loaded when interface is configured and thus
> firware loader runs in context of /sbin/ip which may not have access
> to sysfs. Having separate thread will ensure that firmware loader runs
> in kernel context).
> 
> And I was thinking about caching firmware (siomething like if you do
> "echo 2 > /sys/class/firmware/blah/loading" instead of 0 it will keep
> a copy of the firmware in memory. One could see all firmwares in
> /sys/class/firmware/loaded/<xxx> and by erase cached firmware by
> echoing 0 into it).
> 
> What do you think?

actually there is a thread about firmware loading rewrite and POST
calling of programs. It must be on LKML or the hotplug mailing list.
However my backlog for the interesting topics of these lists increased
while I was traveling the last 5-6 weeks. I think you should simply
start a new one on LKML.

Regards

Marcel

Re: [PATCH 00/04] Load keyspan firmware with hotplug

[Jan Harkes]

On Tue, Apr 05, 2005 at 04:36:31PM +0200, Marcel Holtmann wrote:
> > > I agree with Dmitry on this point. The IHEX parser should not be inside
> > > firmware_class.c. What about using keyspan_ihex.[ch] for it?
> > 
> > That's what I had originally, actually called firmware_ihex.ko, since
> > the IHEX format parser is not in any way keyspan specific and there are
> > several usb-serial converters that seem to use the same IHEX->.h trick
> > which could trivially be modified to use this loader.
> > 
> > But the compiled parser fairly small (< 2KB) and adding it to the
> > existing module didn't effectively add any size to the firmware_class
> > module since things are rounded to a page boundary anyways.
> 
> so it seems that this is usb-serial specific at the moment. Then I would

I really don't see the point you are trying to argue. I said, sure it is
possible to make it a separate module, that is what my initial
implementation was. Why do you want to pigeon-hole it with anything but
the existing firmware loading code?

It is _not_ usb-serial specific, in fact once the device is initialized
this isn't even needed. And the initialization as far as I can see uses
little or no usb-serial code.

It happens that many usb-serial devices are built around the ezusb
chipset, which in turn seems to be a 8051-based microcontroller. The
compilers for such microcontrollers seem to generate IHEX formatted
output possibly because eprom burners generally support the format.

> it up at the moment. People are also working on a replacement for the
> current request_firmware(), because the needs are changing. Try to keep
> it close with the usb-serial for now.

What? I find the existing request_firmware fairly simple and
straightforward, it has a very KISS-like quality to it, it is nice and
small and even the userspace support is trivial. I only saw a mention
about 'replacing' it in the current thread which mostly involved
complaints but didn't actually see anyone volunteering to start working
on such a replacement.

If a driver wants to load 5 different chunks, just call request_firmware
5 times (i.e. drivers/bluetooth/bcm203x.c). If the data is a single
binary blob, just ask for the single binary blob. In this case there
seems to be some structure to the blob that I wanted to preserve, and
that would either be some custom binary format that contains
[<address><length><data>]... tuples, which ofcourse causes problems for
our big-endian brothers, or a similar ascii format, where the IHEX is
not only pretty much standardized, it is trivial to parse and even adds
checksum information.

The only thing that I see missing right now is a change to the makefiles
to have in-tree firmware files get installed in /lib/modules/`uname
-r`/firmware or some similar place. Ideally people would add a line
like,

    fw-$(CONFIG_FOO) = foo-firmware-blob.fw

And make install could drop it a place where hotplug can find it.

Jan

Re: [PATCH 00/04] Load keyspan firmware with hotplug

[Marcel Holtmann]

Hi Jan,

> > > > I agree with Dmitry on this point. The IHEX parser should not be inside
> > > > firmware_class.c. What about using keyspan_ihex.[ch] for it?
> > > 
> > > That's what I had originally, actually called firmware_ihex.ko, since
> > > the IHEX format parser is not in any way keyspan specific and there are
> > > several usb-serial converters that seem to use the same IHEX->.h trick
> > > which could trivially be modified to use this loader.
> > > 
> > > But the compiled parser fairly small (< 2KB) and adding it to the
> > > existing module didn't effectively add any size to the firmware_class
> > > module since things are rounded to a page boundary anyways.
> > 
> > so it seems that this is usb-serial specific at the moment. Then I would
> 
> I really don't see the point you are trying to argue. I said, sure it is
> possible to make it a separate module, that is what my initial
> implementation was. Why do you want to pigeon-hole it with anything but
> the existing firmware loading code?

the existing request_firmware() has nothing in common with IHEX parser
and such a parser should not belong there. So either make it a separate
module or add it to the module that is using it. In this case this is
the keyspan module or the usb-serial core.

> It is _not_ usb-serial specific, in fact once the device is initialized
> this isn't even needed. And the initialization as far as I can see uses
> little or no usb-serial code.
> 
> It happens that many usb-serial devices are built around the ezusb
> chipset, which in turn seems to be a 8051-based microcontroller. The
> compilers for such microcontrollers seem to generate IHEX formatted
> output possibly because eprom burners generally support the format.

Then make it at separate module.

> > it up at the moment. People are also working on a replacement for the
> > current request_firmware(), because the needs are changing. Try to keep
> > it close with the usb-serial for now.
> 
> What? I find the existing request_firmware fairly simple and
> straightforward, it has a very KISS-like quality to it, it is nice and
> small and even the userspace support is trivial. I only saw a mention
> about 'replacing' it in the current thread which mostly involved
> complaints but didn't actually see anyone volunteering to start working
> on such a replacement.
> 
> If a driver wants to load 5 different chunks, just call request_firmware
> 5 times (i.e. drivers/bluetooth/bcm203x.c). If the data is a single
> binary blob, just ask for the single binary blob. In this case there
> seems to be some structure to the blob that I wanted to preserve, and
> that would either be some custom binary format that contains
> [<address><length><data>]... tuples, which ofcourse causes problems for
> our big-endian brothers, or a similar ascii format, where the IHEX is
> not only pretty much standardized, it is trivial to parse and even adds
> checksum information.

I am not going to repeat the current arguments and it is not about
loading multiple firmware files (btw I wrote the bcm203x). Check the
mailing list archives for the details. I still need to catch up with the
discussion, but there is some ongoing work.

> The only thing that I see missing right now is a change to the makefiles
> to have in-tree firmware files get installed in /lib/modules/`uname
> -r`/firmware or some similar place. Ideally people would add a line
> like,
> 
>     fw-$(CONFIG_FOO) = foo-firmware-blob.fw
> 
> And make install could drop it a place where hotplug can find it.

This is another approach and if you want something like that, then send
a patch for it and let Sam and others comment on it.

Regards

Marcel

Re: [PATCH 00/04] Load keyspan firmware with hotplug

[Dmitry Torokhov]

On Apr 5, 2005 6:45 AM, Jan Harkes <jaharkes@cs.cmu.edu> wrote:
> On Tue, Apr 05, 2005 at 11:22:06AM +0200, Marcel Holtmann wrote:
> > I agree with Dmitry on this point. The IHEX parser should not be inside
> > firmware_class.c. What about using keyspan_ihex.[ch] for it?
> 
> That's what I had originally, actually called firmware_ihex.ko, since
> the IHEX format parser is not in any way keyspan specific and there are
> several usb-serial converters that seem to use the same IHEX->.h trick
> which could trivially be modified to use this loader.
> 
> But the compiled parser fairly small (< 2KB) and adding it to the
> existing module didn't effectively add any size to the firmware_class
> module since things are rounded to a page boundary anyways.
> 

It is not about the size, it is about source separation. Since it has
nothing to do with actualy loading of a BLOB from userspace to kernel
space I'd put it into lib/, like crc functions, etc. It does not
really have to work with "struct firmware *", "void *data, size_t len"
should be just as useable.

-- 
Dmitry

Re: [PATCH 00/04] Load keyspan firmware with hotplug

[Sven Luther]

On Tue, Apr 05, 2005 at 12:23:29AM -0400, Jan Harkes wrote:
> On Mon, Apr 04, 2005 at 12:17:46PM -0700, Greg KH wrote:
> > On Mon, Apr 04, 2005 at 08:27:53PM +0200, Sven Luther wrote:
> > > Mmm, probably that 2001 discussion about the keyspan firmware, right ?
> > > 
> > >   http://lists.debian.org/debian-legal/2001/04/msg00145.html
> > > 
> > > Can you summarize the conclusion of the thread, or what you did get from it,
> > > please ? 
> > 
> > That people didn't like the inclusion of firmware, I posted how you can
> > fix it by moving it outside of the kernel, and asked for patches.
> > 
> > None have come.
> 
> Didn't know you were waiting for it. How about something like the
> following series of patches?
> 
> [01/04] - add simple Intel IHEX format parser to the firmware loader.
> [02/04] - make the keyspan driver use request_firmware.
> [03/04] - converter program used to dump the keyspan headers as IHex files.
> [04/04] - result of running the previous program.
> 
> This ofcourse doesn't actually solve Debian's distribution issues since
> the keyspan firmware can only be distributed as part of 'Linux or other
> Open Source operating system kernel'.

Well, if this is the case, it can be distributed on the non-free archive.

> > So I refuse to listen to talk about this, as obviously, no one cares
> > enough about this to actually fix the issue.
> 
> I got tired of always building my own kernels on Debian just to get my
> serial dongle to work since their included keyspan.ko driver is so
> useless that it isn't even worth having. The only way to use it with a
> Debian kernel is to have the dongle in a powered hub and first boot into
> Windows or a normal kernel.org kernel to get the thing initialized.

The non-free driver package should solve that for you.

> Didn't send the patch earlier since I wanted to split off the
> pre-numeration part of the driver so that after intialization we can
> unload the unused parts of the driver as well as the the firmware class
> module.

Friendly,

Sven Luther