linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Firmware loading problem
@ 2003-07-22 14:45 Marcel Holtmann
  2003-07-22 14:55 ` Manuel Estrada Sainz
  0 siblings, 1 reply; 10+ messages in thread
From: Marcel Holtmann @ 2003-07-22 14:45 UTC (permalink / raw)
  To: Linux Kernel Mailing List; +Cc: Manuel Estrada Sainz

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Firmware loading problem
  2003-07-22 14:45 Firmware loading problem Marcel Holtmann
@ 2003-07-22 14:55 ` Manuel Estrada Sainz
  2003-07-22 15:38   ` Marcel Holtmann
  0 siblings, 1 reply; 10+ messages in thread
From: Manuel Estrada Sainz @ 2003-07-22 14:55 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Linux Kernel Mailing List

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 <ranty@debian.org>
                         <ranty@bigfoot.com>
			 <ranty@users.sourceforge.net>
------------------------ <manuel.estrada@hispalinux.es> -------------------
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Firmware loading problem
  2003-07-22 14:55 ` Manuel Estrada Sainz
@ 2003-07-22 15:38   ` Marcel Holtmann
  2003-07-26  9:04     ` [PATCH] " Manuel Estrada Sainz
  0 siblings, 1 reply; 10+ messages in thread
From: Marcel Holtmann @ 2003-07-22 15:38 UTC (permalink / raw)
  To: Manuel Estrada Sainz; +Cc: Linux Kernel Mailing List

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

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


[-- Attachment #2: fwldtest.c --]
[-- Type: text/x-c, Size: 2330 bytes --]

/*
 *
 *  Firmware loader test driver
 *
 *  Copyright (C) 2003  Marcel Holtmann <marcel@holtmann.org>
 *
 *
 *  This program is free software; you can redistribute it and/or modify
 *  it under the terms of the GNU General Public License as published by
 *  the Free Software Foundation; either version 2 of the License, or
 *  (at your option) any later version.
 *
 *  This program is distributed in the hope that it will be useful,
 *  but WITHOUT ANY WARRANTY; without even the implied warranty of
 *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 *  GNU General Public License for more details.
 *
 *  You should have received a copy of the GNU General Public License
 *  along with this program; if not, write to the Free Software
 *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 *
 */

#include <linux/config.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/init.h>

#include <linux/device.h>
#include <linux/firmware.h>

#include <linux/usb.h>


static struct usb_device_id fwldtest_table[] = {
	{ USB_DEVICE(0x057c, 0x2200) },

	{ }	/* Terminating entry */
};

MODULE_DEVICE_TABLE(usb, fwldtest_table);


static int fwldtest_probe(struct usb_interface *iface, const struct usb_device_id *id)
{
	const struct firmware *firmware;
	struct usb_device *udev = interface_to_usbdev(iface);

	if (request_firmware(&firmware, "firmware.bin", &udev->dev) < 0) {
		printk(KERN_ERR "Firmware request failed\n");
		goto error;
	}

	printk(KERN_INFO "firmware data %p size %d\n", firmware->data, firmware->size);

error:
	release_firmware(firmware);

	return -EIO;
}

static void fwldtest_disconnect(struct usb_interface *iface)
{
}

static struct usb_driver fwldtest_driver = {
	.owner		= THIS_MODULE,
	.name		= "fwldtest",
	.probe		= fwldtest_probe,
	.disconnect	= fwldtest_disconnect,
	.id_table	= fwldtest_table,
};

static int __init fwldtest_init(void)
{
	return usb_register(&fwldtest_driver);
}

static void __exit fwldtest_cleanup(void)
{
	usb_deregister(&fwldtest_driver);
}

module_init(fwldtest_init);
module_exit(fwldtest_cleanup);

MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
MODULE_DESCRIPTION("Firmware loader test driver");
MODULE_LICENSE("GPL");

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] Re: Firmware loading problem
  2003-07-22 15:38   ` Marcel Holtmann
@ 2003-07-26  9:04     ` Manuel Estrada Sainz
  2003-07-26 11:14       ` Marcel Holtmann
  2003-07-27 19:21       ` Matthew Wilcox
  0 siblings, 2 replies; 10+ messages in thread
From: Manuel Estrada Sainz @ 2003-07-26  9:04 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Linux Kernel Mailing List, greg, willy

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

On Tue, Jul 22, 2003 at 05:38:15PM +0200, Marcel Holtmann wrote:
> 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.

 You are right, the problem was in sysfs, attached goes a patch that
 WorksForMe (tm), please test and report.

> 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.

 About the patch:
	- undo recent change, made in the believe that "buffer" was the
	  size of the whole file, it is just PAGE_SIZE in size.
		- Since files are allowed to have unknown sizes, by
		  setting their size to 0, we can't preallocate a buffer
		  of their size on open.

	- don't use any offset when handling buffer
		- simplifies code
		- since it is temporary storage it doesn't really matter

	- undo relevant changes to request_firmware() code.

	- hopefully adapt drivers/pci/pci-sysfs.c to this changes
		- Please double check, I didn't look very carefully on
		  this.

 Have a nice day

 	Manuel
-- 
--- Manuel Estrada Sainz <ranty@debian.org>
                         <ranty@bigfoot.com>
			 <ranty@users.sourceforge.net>
------------------------ <manuel.estrada@hispalinux.es> -------------------
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.

[-- Attachment #2: sysfs-bin-unbreak.diff --]
[-- Type: text/plain, Size: 3299 bytes --]

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);

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Re: Firmware loading problem
  2003-07-26  9:04     ` [PATCH] " Manuel Estrada Sainz
@ 2003-07-26 11:14       ` Marcel Holtmann
  2003-07-26 11:26         ` Manuel Estrada Sainz
  2003-07-27 19:21       ` Matthew Wilcox
  1 sibling, 1 reply; 10+ messages in thread
From: Marcel Holtmann @ 2003-07-26 11:14 UTC (permalink / raw)
  To: Manuel Estrada Sainz; +Cc: Linux Kernel Mailing List, Greg Kroah-Hartman, willy

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Re: Firmware loading problem
  2003-07-26 11:14       ` Marcel Holtmann
@ 2003-07-26 11:26         ` Manuel Estrada Sainz
  0 siblings, 0 replies; 10+ messages in thread
From: Manuel Estrada Sainz @ 2003-07-26 11:26 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Linux Kernel Mailing List, Greg Kroah-Hartman, willy

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 <ranty@debian.org>
                         <ranty@bigfoot.com>
			 <ranty@users.sourceforge.net>
------------------------ <manuel.estrada@hispalinux.es> -------------------
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Re: Firmware loading problem
  2003-07-26  9:04     ` [PATCH] " Manuel Estrada Sainz
  2003-07-26 11:14       ` Marcel Holtmann
@ 2003-07-27 19:21       ` Matthew Wilcox
  2003-07-27 21:21         ` Manuel Estrada Sainz
  1 sibling, 1 reply; 10+ messages in thread
From: Matthew Wilcox @ 2003-07-27 19:21 UTC (permalink / raw)
  To: Manuel Estrada Sainz
  Cc: Marcel Holtmann, Linux Kernel Mailing List, greg, willy, Patrick Mochel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Re: Firmware loading problem
  2003-07-27 19:21       ` Matthew Wilcox
@ 2003-07-27 21:21         ` Manuel Estrada Sainz
  2003-08-01 15:05           ` Manuel Estrada Sainz
  0 siblings, 1 reply; 10+ messages in thread
From: Manuel Estrada Sainz @ 2003-07-27 21:21 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Marcel Holtmann, Linux Kernel Mailing List, greg, Patrick Mochel

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 <ranty@debian.org>
                         <ranty@bigfoot.com>
			 <ranty@users.sourceforge.net>
------------------------ <manuel.estrada@hispalinux.es> -------------------
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Re: Firmware loading problem
  2003-07-27 21:21         ` Manuel Estrada Sainz
@ 2003-08-01 15:05           ` Manuel Estrada Sainz
  2003-08-01 18:50             ` Manuel Estrada Sainz
  0 siblings, 1 reply; 10+ messages in thread
From: Manuel Estrada Sainz @ 2003-08-01 15:05 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Marcel Holtmann, Linux Kernel Mailing List, greg, Patrick Mochel

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 <ranty@debian.org>
                         <ranty@bigfoot.com>
			 <ranty@users.sourceforge.net>
------------------------ <manuel.estrada@hispalinux.es> -------------------
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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Re: Firmware loading problem
  2003-08-01 15:05           ` Manuel Estrada Sainz
@ 2003-08-01 18:50             ` Manuel Estrada Sainz
  0 siblings, 0 replies; 10+ messages in thread
From: Manuel Estrada Sainz @ 2003-08-01 18:50 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Marcel Holtmann, Linux Kernel Mailing List, greg, Patrick Mochel,
	Andrew Morton

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

On Fri, Aug 01, 2003 at 05:05:38PM +0200, Manuel Estrada Sainz wrote:
> 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.

 OK, this time I have tested the PCI changes and it works:

 the patches:

 - sysfs-bin-unbreak-2-main.diff:
	- undo recent change, made in the believe that "buffer" was the
	  size of the whole file, it is just PAGE_SIZE in size. This was
	  causing kernel memory corruption.

		- Since files are allowed to have unknown sizes, by
		  setting their size to 0, we can't preallocate a buffer
		  of their size on open.

 - sysfs-bin-unbreak-2-request_firmware.diff:
	- Adapt to the above sysfs change.

 - sysfs-bin-unbreak-2-pci.diff:
  	- hopefully adapt drivers/pci/pci-sysfs.c to this changes.
		- Matthew can probably make it look prettier, but for
		  now it works.

 Have a nice day

 	Manuel


-- 
--- Manuel Estrada Sainz <ranty@debian.org>
                         <ranty@bigfoot.com>
			 <ranty@users.sourceforge.net>
------------------------ <manuel.estrada@hispalinux.es> -------------------
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.

[-- Attachment #2: sysfs-bin-unbreak-2-main.diff --]
[-- Type: text/plain, Size: 565 bytes --]

--- fs/sysfs/bin.c	4 Jul 2003 02:21:18 -0000	1.9
+++ fs/sysfs/bin.c	1 Aug 2003 14:26:45 -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);

[-- Attachment #3: sysfs-bin-unbreak-2-pci.diff --]
[-- Type: text/plain, Size: 2190 bytes --]

--- drivers/pci/pci-sysfs.c	4 Jul 2003 02:21:18 -0000	1.6
+++ drivers/pci/pci-sysfs.c	1 Aug 2003 14:26:43 -0000
@@ -67,6 +67,7 @@
 {
 	struct pci_dev *dev = to_pci_dev(container_of(kobj,struct device,kobj));
 	unsigned int size = 64;
+	loff_t init_off = off;
 
 	/* Several chips lock up trying to read undefined config space */
 	if (capable(CAP_SYS_ADMIN)) {
@@ -87,7 +88,7 @@
 	while (off & 3) {
 		unsigned char val;
 		pci_read_config_byte(dev, off, &val);
-		buf[off] = val;
+		buf[off - init_off] = val;
 		off++;
 		if (--size == 0)
 			break;
@@ -96,10 +97,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[off - init_off] = val & 0xff;
+		buf[off - init_off + 1] = (val >> 8) & 0xff;
+		buf[off - init_off + 2] = (val >> 16) & 0xff;
+		buf[off - init_off + 3] = (val >> 24) & 0xff;
 		off += 4;
 		size -= 4;
 	}
@@ -107,7 +108,7 @@
 	while (size > 0) {
 		unsigned char val;
 		pci_read_config_byte(dev, off, &val);
-		buf[off] = val;
+		buf[off - init_off] = val;
 		off++;
 		--size;
 	}
@@ -120,6 +121,7 @@
 {
 	struct pci_dev *dev = to_pci_dev(container_of(kobj,struct device,kobj));
 	unsigned int size = count;
+	loff_t init_off = off;
 
 	if (off > 256)
 		return 0;
@@ -129,24 +131,24 @@
 	}
 
 	while (off & 3) {
-		pci_write_config_byte(dev, off, buf[off]);
+		pci_write_config_byte(dev, off, buf[off - init_off]);
 		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[off - init_off];
+		val |= (unsigned int) buf[off - init_off + 1] << 8;
+		val |= (unsigned int) buf[off - init_off + 2] << 16;
+		val |= (unsigned int) buf[off - init_off + 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[off - init_off]);
 		off++;
 		--size;
 	}

[-- Attachment #4: sysfs-bin-unbreak-2-request_firmware.diff --]
[-- Type: text/plain, Size: 543 bytes --]

--- 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);
 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2003-08-01 18:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-07-22 14:45 Firmware loading problem Marcel Holtmann
2003-07-22 14:55 ` Manuel Estrada Sainz
2003-07-22 15:38   ` Marcel Holtmann
2003-07-26  9:04     ` [PATCH] " Manuel Estrada Sainz
2003-07-26 11:14       ` Marcel Holtmann
2003-07-26 11:26         ` Manuel Estrada Sainz
2003-07-27 19:21       ` Matthew Wilcox
2003-07-27 21:21         ` Manuel Estrada Sainz
2003-08-01 15:05           ` Manuel Estrada Sainz
2003-08-01 18:50             ` Manuel Estrada Sainz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).