Hi,
I installed linux-2.6.0-test1-ac2 and tried to port my driver for the
BlueFRITZ! USB Bluetooth dongle to 2.6. This device needs a firmware
download and I want to use the new firmware class for getting the
firmware file from userspace. After reading the documentation and
testing the driver samples I got the results that I expected.
My problem is now that the firmware loader is not working with my
firmware file and it seems that this is a problem of the file size,
because copying small files through the same interface is working fine.
This is the file I want to load:
-rw-r--r-- 1 holtmann staff 418352 Jul 11 12:38 bfubase.frm
I have written my own firmware.agent hotplug script, which looks in
general something like this:
echo 1 > $LOADING
cp bfubase.frm $DATA
echo 0 > $LOADING
Loading the above firmware file through this interface results in
different behaviours. The results are complete freezes, instant reboots,
X server crashes with black screens and sometimes I see an oops about
virtual memory, but it goes bye bye too fast to let me do anything
useful with it.
Are their any limitations with the sysfs binary file interface?
Regards
Marcel
On Tue, Jul 22, 2003 at 04:45:28PM +0200, Marcel Holtmann wrote:
> Hi,
>
> I installed linux-2.6.0-test1-ac2 and tried to port my driver for the
> BlueFRITZ! USB Bluetooth dongle to 2.6. This device needs a firmware
> download and I want to use the new firmware class for getting the
> firmware file from userspace. After reading the documentation and
> testing the driver samples I got the results that I expected.
>
> My problem is now that the firmware loader is not working with my
> firmware file and it seems that this is a problem of the file size,
> because copying small files through the same interface is working fine.
> This is the file I want to load:
>
> -rw-r--r-- 1 holtmann staff 418352 Jul 11 12:38 bfubase.frm
>
> I have written my own firmware.agent hotplug script, which looks in
> general something like this:
>
> echo 1 > $LOADING
> cp bfubase.frm $DATA
> echo 0 > $LOADING
>
> Loading the above firmware file through this interface results in
> different behaviours. The results are complete freezes, instant reboots,
> X server crashes with black screens and sometimes I see an oops about
> virtual memory, but it goes bye bye too fast to let me do anything
> useful with it.
Could you send me a tarball with a sample showing the problem. If
possible I would like to do "make test" and have it compile and crash
the system appropriately :)
> Are their any limitations with the sysfs binary file interface?
Not that I know of. But I am willing to learn :)
Thanks
Manuel
--
--- Manuel Estrada Sainz <[email protected]>
<[email protected]>
<[email protected]>
------------------------ <[email protected]> -------------------
Let us have the serenity to accept the things we cannot change, courage to
change the things we can, and wisdom to know the difference.
Hi Manuel,
> > I installed linux-2.6.0-test1-ac2 and tried to port my driver for the
> > BlueFRITZ! USB Bluetooth dongle to 2.6. This device needs a firmware
> > download and I want to use the new firmware class for getting the
> > firmware file from userspace. After reading the documentation and
> > testing the driver samples I got the results that I expected.
> >
> > My problem is now that the firmware loader is not working with my
> > firmware file and it seems that this is a problem of the file size,
> > because copying small files through the same interface is working fine.
> > This is the file I want to load:
> >
> > -rw-r--r-- 1 holtmann staff 418352 Jul 11 12:38 bfubase.frm
> >
> > I have written my own firmware.agent hotplug script, which looks in
> > general something like this:
> >
> > echo 1 > $LOADING
> > cp bfubase.frm $DATA
> > echo 0 > $LOADING
> >
> > Loading the above firmware file through this interface results in
> > different behaviours. The results are complete freezes, instant reboots,
> > X server crashes with black screens and sometimes I see an oops about
> > virtual memory, but it goes bye bye too fast to let me do anything
> > useful with it.
>
> Could you send me a tarball with a sample showing the problem. If
> possible I would like to do "make test" and have it compile and crash
> the system appropriately :)
I tracked down the problem to request_firmware() or a sysfs problem.
With the firmware included in a header file the driver itself works
perfect.
Attached is a sample of how I use the request_firmware() and from the
documentation it seems correct to me.
Regards
Marcel
Index: drivers/base/firmware_class.c
===================================================================
RCS file: /home/cvs/linux-2.5/drivers/base/firmware_class.c,v
retrieving revision 1.3
diff -u -r1.3 firmware_class.c
--- drivers/base/firmware_class.c 4 Jul 2003 02:21:18 -0000 1.3
+++ drivers/base/firmware_class.c 26 Jul 2003 06:53:47 -0000
@@ -149,7 +149,7 @@
if (offset + count > fw->size)
count = fw->size - offset;
- memcpy(buffer + offset, fw->data + offset, count);
+ memcpy(buffer, fw->data + offset, count);
return count;
}
static int
@@ -198,7 +198,7 @@
if (retval)
return retval;
- memcpy(fw->data + offset, buffer + offset, count);
+ memcpy(fw->data + offset, buffer, count);
fw->size = max_t(size_t, offset + count, fw->size);
Index: drivers/pci/pci-sysfs.c
===================================================================
RCS file: /home/cvs/linux-2.5/drivers/pci/pci-sysfs.c,v
retrieving revision 1.6
diff -u -r1.6 pci-sysfs.c
--- drivers/pci/pci-sysfs.c 4 Jul 2003 02:21:18 -0000 1.6
+++ drivers/pci/pci-sysfs.c 26 Jul 2003 06:53:50 -0000
@@ -87,7 +87,7 @@
while (off & 3) {
unsigned char val;
pci_read_config_byte(dev, off, &val);
- buf[off] = val;
+ buf[0] = val;
off++;
if (--size == 0)
break;
@@ -96,10 +96,10 @@
while (size > 3) {
unsigned int val;
pci_read_config_dword(dev, off, &val);
- buf[off] = val & 0xff;
- buf[off + 1] = (val >> 8) & 0xff;
- buf[off + 2] = (val >> 16) & 0xff;
- buf[off + 3] = (val >> 24) & 0xff;
+ buf[0] = val & 0xff;
+ buf[1] = (val >> 8) & 0xff;
+ buf[2] = (val >> 16) & 0xff;
+ buf[3] = (val >> 24) & 0xff;
off += 4;
size -= 4;
}
@@ -107,7 +107,7 @@
while (size > 0) {
unsigned char val;
pci_read_config_byte(dev, off, &val);
- buf[off] = val;
+ buf[0] = val;
off++;
--size;
}
@@ -129,24 +129,24 @@
}
while (off & 3) {
- pci_write_config_byte(dev, off, buf[off]);
+ pci_write_config_byte(dev, off, buf[0]);
off++;
if (--size == 0)
break;
}
while (size > 3) {
- unsigned int val = buf[off];
- val |= (unsigned int) buf[off + 1] << 8;
- val |= (unsigned int) buf[off + 2] << 16;
- val |= (unsigned int) buf[off + 3] << 24;
+ unsigned int val = buf[0];
+ val |= (unsigned int) buf[1] << 8;
+ val |= (unsigned int) buf[2] << 16;
+ val |= (unsigned int) buf[3] << 24;
pci_write_config_dword(dev, off, val);
off += 4;
size -= 4;
}
while (size > 0) {
- pci_write_config_byte(dev, off, buf[off]);
+ pci_write_config_byte(dev, off, buf[0]);
off++;
--size;
}
Index: fs/sysfs/bin.c
===================================================================
RCS file: /home/cvs/linux-2.5/fs/sysfs/bin.c,v
retrieving revision 1.9
diff -u -r1.9 bin.c
--- fs/sysfs/bin.c 4 Jul 2003 02:21:18 -0000 1.9
+++ fs/sysfs/bin.c 26 Jul 2003 06:53:59 -0000
@@ -47,7 +47,7 @@
return ret;
count = ret;
- if (copy_to_user(userbuf, buffer + offs, count) != 0)
+ if (copy_to_user(userbuf, buffer, count) != 0)
return -EINVAL;
pr_debug("offs = %lld, *off = %lld, count = %zd\n", offs, *off, count);
@@ -83,7 +83,7 @@
count = size - offs;
}
- if (copy_from_user(buffer + offs, userbuf, count))
+ if (copy_from_user(buffer, userbuf, count))
return -EFAULT;
count = flush_write(dentry, buffer, offs, count);
Hi Manuel,
> > I tracked down the problem to request_firmware() or a sysfs problem.
> > With the firmware included in a header file the driver itself works
> > perfect.
>
> You are right, the problem was in sysfs, attached goes a patch that
> WorksForMe (tm), please test and report.
the firmware loading of my driver is now working, thanks. If someone has
double checked the pci-sysfs.c change, please forward this patch to
Linus.
What did Marcelo say about including your backport into 2.4?
> > Attached is a sample of how I use the request_firmware() and from the
> > documentation it seems correct to me.
>
> Not what I was asking for, but it seams OK.
I know, but I don't have such an easy package you requested.
Regards
Marcel
On Sat, Jul 26, 2003 at 01:14:17PM +0200, Marcel Holtmann wrote:
> Hi Manuel,
>
> > > I tracked down the problem to request_firmware() or a sysfs problem.
> > > With the firmware included in a header file the driver itself works
> > > perfect.
> >
> > You are right, the problem was in sysfs, attached goes a patch that
> > WorksForMe (tm), please test and report.
>
> the firmware loading of my driver is now working, thanks. If someone has
> double checked the pci-sysfs.c change, please forward this patch to
> Linus.
I'll wait for Greg and/or Matthew to say something about it.
> What did Marcelo say about including your backport into 2.4?
Nothing, he didn't even bother to answer :-(. Maybe someone more
relevant than myself could dig on the issue.
> > > Attached is a sample of how I use the request_firmware() and from the
> > > documentation it seems correct to me.
> >
> > Not what I was asking for, but it seams OK.
>
> I know, but I don't have such an easy package you requested.
Never mind, it is fixed now.
Regards
Manuel
--
--- Manuel Estrada Sainz <[email protected]>
<[email protected]>
<[email protected]>
------------------------ <[email protected]> -------------------
Let us have the serenity to accept the things we cannot change, courage to
change the things we can, and wisdom to know the difference.
On Sun, Jul 27, 2003 at 08:21:11PM +0100, Matthew Wilcox wrote:
> On Sat, Jul 26, 2003 at 11:04:58AM +0200, Manuel Estrada Sainz wrote:
> > - hopefully adapt drivers/pci/pci-sysfs.c to this changes
> > - Please double check, I didn't look very carefully on
> > this.
>
> Definitely wrong. I was going to undo this change since I realised how
> it doesn't work for you; but the change you made to the PCI code is wrong.
> It ends up copying everything to offset 0 from the buf address.
Exactly, and that is what sysfs code expects (with the rest of the
patch), the buffer is just temporary storage, it doesn't really matter
what offset you use as long as you don't write further than
buffer+PAGE_SIZE and both sides of the issue agree.
> I wish Pat had cc'd me when making the change to the interface
> originally ;-(
>
> I'll whip up a patch in a few minutes.
Great.
Have a nice day
Manuel
--
--- Manuel Estrada Sainz <[email protected]>
<[email protected]>
<[email protected]>
------------------------ <[email protected]> -------------------
Let us have the serenity to accept the things we cannot change, courage to
change the things we can, and wisdom to know the difference.
On Sat, Jul 26, 2003 at 11:04:58AM +0200, Manuel Estrada Sainz wrote:
> - hopefully adapt drivers/pci/pci-sysfs.c to this changes
> - Please double check, I didn't look very carefully on
> this.
Definitely wrong. I was going to undo this change since I realised how
it doesn't work for you; but the change you made to the PCI code is wrong.
It ends up copying everything to offset 0 from the buf address. I wish
Pat had cc'd me when making the change to the interface originally ;-(
I'll whip up a patch in a few minutes.
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
On Sun, Jul 27, 2003 at 11:21:34PM +0200, Manuel Estrada Sainz wrote:
> On Sun, Jul 27, 2003 at 08:21:11PM +0100, Matthew Wilcox wrote:
> > On Sat, Jul 26, 2003 at 11:04:58AM +0200, Manuel Estrada Sainz wrote:
> > > - hopefully adapt drivers/pci/pci-sysfs.c to this changes
> > > - Please double check, I didn't look very carefully on
> > > this.
> >
> > Definitely wrong. I was going to undo this change since I realised how
> > it doesn't work for you; but the change you made to the PCI code is wrong.
> > It ends up copying everything to offset 0 from the buf address.
>
> Exactly, and that is what sysfs code expects (with the rest of the
> patch), the buffer is just temporary storage, it doesn't really matter
> what offset you use as long as you don't write further than
> buffer+PAGE_SIZE and both sides of the issue agree.
My fault, I was severely misguided here, Matthew is of course write.
Now that I understand the issue a little deeper I'll try send a correct
patch to get the issue done with.
Have a nice day
Manuel
--
--- Manuel Estrada Sainz <[email protected]>
<[email protected]>
<[email protected]>
------------------------ <[email protected]> -------------------
Let us have the serenity to accept the things we cannot change, courage to
change the things we can, and wisdom to know the difference.
--- drivers/base/firmware_class.c 26 Jul 2003 08:38:07 -0000
+++ drivers/base/firmware_class.c 1 Aug 2003 14:26:41 -0000
@@ -151,7 +151,7 @@
if (offset + count > fw->size)
count = fw->size - offset;
- memcpy(buffer + offset, fw->data + offset, count);
+ memcpy(buffer, fw->data + offset, count);
return count;
}
static int
@@ -200,7 +200,7 @@
if (retval)
return retval;
- memcpy(fw->data + offset, buffer + offset, count);
+ memcpy(fw->data + offset, buffer, count);
fw->size = max_t(size_t, offset + count, fw->size);