2000-12-06 07:57:11

by Linus Torvalds

[permalink] [raw]
Subject: test12-pre6


Ok, I almost called this the final test12, but I wanted to get some more
feedback on the keyboard controller stuff and PCI irq routing.

The biggest part of this is the budding parisc stuff, but it's unlikely
that we'll see full parisc support in 2.4.0 - the remaining pieces that
actually touch generic code aren't going to be as brainless to integrate.

Of this, the "exec_usermodehelper()" changes should fix a few problems for
people who rely heavily on automatic module loading, and the PCI irq
routing changes should hopefully fix a number of laptops.

Concering the PCI irq routing fixes in particular, I'd ask people with
laptops to start testing their kernels with PnP OS set to "yes" in the
BIOS setup. We shoul dbe at a stage where it should basically work all the
time, and it would be interesting to hear about cases that we don't handle
right.

(Anybody who has had problems with USB interrupts seemingly not being
delivered and similar is also encouraged to test this new kernel).

The APIC setup order should get older SMP machines working again (which
broke when we fixed some newer machines - let's hope that this time we fix
one machine without breaking another one).

Linus

----
- pre6:
- Alan Cox: synch. PA-RISC arch and bitops cleanups
- Maciej Rozycki: even more proper apic setup order.
- Andrew Morton: exec_usermodehelper fixes
- Adam Richter, Kai Germaschewski, me: PCI irq routing.
- revert A20 code changes. We really need to use the keyboard
controller if one exists. But decrease timeouts.
- Johannes Erdfelt: USB updates
- Ralf Baechle: MIPS memmove() fix.

- pre5:
- Jaroslav Kysela: ymfpci driver
- me: get rid of bogus MS_INVALIDATE semantics
- me: final part of the PageDirty() saga
- Rusty Russell: 4-way SMP iptables fix
- Al Viro: oops - bad ext2 inode dirty block bug

- pre4:
- Andries Brouwer: final isofs pieces.
- Kai Germaschewski: ISDN
- play CD audio correctly, don't stop after 12 minutes.
- Anton Altaparmakov: disable NTFS mmap for now, as it doesn't work.
- Stephen Tweedie: fix inode dirty block handling
- Bill Hartner: reschedule_idle - prefer right cpu
- Johannes Erdfelt: USB updates
- Alan Cox: synchronize
- Richard Henderson: alpha updates and optimizations
- Geert Uytterhoeven: fbdev could be fooled into crashing fix
- Trond Myklebust: NFS filehandles in inode rather than dentry

- pre3:
- me: more PageDirty / swapcache handling
- Neil Brown: raid and md init fixes
- David Brownell: pci hotplug sanitization.
- Kanoj Sarcar: mips64 update
- Kai Germaschewski: ISDN sync
- Andreas Bombe: ieee1394 cleanups and fixes
- Johannes Erdfelt: USB update
- David Miller: Sparc and net update
- Trond Myklebust: RPC layer SMP fixes
- Thomas Sailer: mixed sound driver fixes
- Tigran Aivazian: use atomic_dec_and_lock() for free_uid()

- pre2:
- Peter Anvin: more P4 configuration parsing
- Stephen Tweedie: O_SYNC patches. Make O_SYNC/fsync/fdatasync
do the right thing.
- Keith Owens: make mdule loading use the right struct module size
- Boszormenyi Zoltan: get MTRR's right for the >32-bit case
- Alan Cox: various random documentation etc
- Dario Ballabio: EATA and u14-34f update
- Ivan Kokshaysky: unbreak alpha ruffian
- Richard Henderson: PCI bridge initialization on alpha
- Zach Brown: correct locking in Maestro driver
- Geert Uytterhoeven: more m68k updates
- Andrey Savochkin: eepro100 update
- Dag Brattli: irda update
- Johannes Erdfelt: USB update

- pre1: (for ISDN synchronization _ONLY_! Not complete!)
- Byron Stanoszek: correct decimal precision for CPU MHz in
/proc/cpuinfo
- Ollie Lho: SiS pirq routing.
- Andries Brouwer: isofs cleanups
- Matt Kraai: /proc read() on directories should return EISDIR, not EINVAL
- me: be stricter about what we accept as a PCI bridge setup.
- me: always set PCI interrupts to be level-triggered when we enable them.
- me: updated PageDirty and swap cache handling
- Peter Anvin: update A20 code to work without keyboard controller
- Kai Germaschewski: ISDN updates
- Russell King: ARM updates
- Geert Uytterhoeven: m68k updates


2000-12-06 08:21:19

by Alexander Viro

[permalink] [raw]
Subject: Re: test12-pre6



On Tue, 5 Dec 2000, Linus Torvalds wrote:

>
> Ok, I almost called this the final test12, but I wanted to get some more
> feedback on the keyboard controller stuff and PCI irq routing.

Linus, could you apply the following bunch of fixes (split into separate
emails):
* dentry leak upon the attempt of cross-device link().
* recovery after get_block() failure in block_prepare_write()
* ext2_update_inode() missing some cases when it should mark
* bogus return values of truncate() and ftruncate().
fs as having large files
* fix to ext2_get_block() - it should return -EFBIG if the block
number is too large, not -EIO.

All of these are real bugs and the first 3 are pretty bad.

The first one (not my catch) - sys_link() should do path_release() if
it decides to fail with -EXDEV. It doesn't. Result: exploitable dentry
leak. Please, apply.

diff -urN rc12-pre6/fs/namei.c rc12-pre6-link/fs/namei.c
--- rc12-pre6/fs/namei.c Tue Dec 5 02:03:15 2000
+++ rc12-pre6-link/fs/namei.c Tue Dec 5 14:55:57 2000
@@ -1611,7 +1611,7 @@
goto out;
error = -EXDEV;
if (old_nd.mnt != nd.mnt)
- goto out;
+ goto out2;
new_dentry = lookup_create(&nd, 0);
error = PTR_ERR(new_dentry);
if (!IS_ERR(new_dentry)) {
@@ -1619,6 +1619,7 @@
dput(new_dentry);
}
up(&nd.dentry->d_inode->i_sem);
+out2:
path_release(&nd);
out:
path_release(&old_nd);

2000-12-06 08:25:00

by Alexander Viro

[permalink] [raw]
Subject: [PATCH] Re: test12-pre6

ext2_update_inode() marks the filesystems as having large files
if there the file becomes too large for old driver (2.2 one). Unfortunately,
it gets the limit wrong - it's not 2^32, it's 2^31. So we should check
for ->i_size_high being non zero _or_ ->i_size having bit 31 set. Please,
apply.

diff -urN rc12-pre6/fs/ext2/inode.c rc12-pre6-ext2-LFS/fs/ext2/inode.c
--- rc12-pre6/fs/ext2/inode.c Tue Dec 5 02:03:14 2000
+++ rc12-pre6-ext2-LFS/fs/ext2/inode.c Tue Dec 5 15:00:27 2000
@@ -1188,7 +1188,7 @@
raw_inode->i_dir_acl = cpu_to_le32(inode->u.ext2_i.i_dir_acl);
else {
raw_inode->i_size_high = cpu_to_le32(inode->i_size >> 32);
- if (raw_inode->i_size_high) {
+ if (raw_inode->i_size_high || (inode->i_size & (1<<31))) {
struct super_block *sb = inode->i_sb;
struct ext2_super_block *es = sb->u.ext2_sb.s_es;
if (!(es->s_feature_ro_compat & cpu_to_le32(EXT2_FEATURE_RO_COMPAT_LARGE_FILE))) {


2000-12-06 08:34:01

by Alexander Viro

[permalink] [raw]
Subject: [PATCH] Re: test12-pre6

If get_block() fails in block_prepare_write() we should zero
the blocks we've allocated and mark them dirty. We don't (i.e. we leave
the random junk in file in that case). Fix: do it. Moreover, if
->prepare_write() fails generic_file_write() is not going to write the user's
data into that page. It's OK if we were in hole (we got zeroes in all
new blocks, so it's equivalent to doing nothing with that page), but it's
not OK if we extended the file (we would get the file silently extended
with zeroes we've never intended to write there or we would get blocks
allocated past the ->i_size - the former screws applications, the latter
makes fsck unhappy). Fix: if the failed call extended the file call
vmtruncate() to clean the mess up.

Please, apply.

diff -urN rc12-pre6/fs/buffer.c rc12-pre6-get_block/fs/buffer.c
--- rc12-pre6/fs/buffer.c Tue Dec 5 02:03:14 2000
+++ rc12-pre6-get_block/fs/buffer.c Tue Dec 5 14:56:20 2000
@@ -1710,6 +1710,15 @@
}
return 0;
out:
+ bh = head;
+ do {
+ if (buffer_new(bh) && !buffer_uptodate(bh)) {
+ memset(bh->b_data, 0, bh->b_size);
+ set_bit(BH_Uptodate, &bh->b_state);
+ mark_buffer_dirty(bh);
+ }
+ bh = bh->b_this_page;
+ } while (bh != head);
return err;
}

diff -urN rc12-pre6/mm/filemap.c rc12-pre6-get_block/mm/filemap.c
--- rc12-pre6/mm/filemap.c Tue Dec 5 02:03:20 2000
+++ rc12-pre6-get_block/mm/filemap.c Tue Dec 5 14:56:20 2000
@@ -2382,6 +2382,7 @@
unsigned long written;
long status;
int err;
+ unsigned bytes;

cached_page = NULL;

@@ -2426,7 +2427,7 @@
}

while (count) {
- unsigned long bytes, index, offset;
+ unsigned long index, offset;
char *kaddr;

/*
@@ -2451,7 +2452,7 @@

status = mapping->a_ops->prepare_write(file, page, offset, offset+bytes);
if (status)
- goto unlock;
+ goto sync_failure;
kaddr = page_address(page);
status = copy_from_user(kaddr+offset, buf, bytes);
flush_dcache_page(page);
@@ -2476,6 +2477,7 @@
if (status < 0)
break;
}
+done:
*ppos = pos;

if (cached_page)
@@ -2496,6 +2498,13 @@
ClearPageUptodate(page);
kunmap(page);
goto unlock;
+sync_failure:
+ UnlockPage(page);
+ deactivate_page(page);
+ page_cache_release(page);
+ if (pos + bytes > inode->i_size)
+ vmtruncate(inode, inode->i_size);
+ goto done;
}

void __init page_cache_init(unsigned long mempages)


2000-12-06 08:37:21

by Alexander Viro

[permalink] [raw]
Subject: [PATCH] Re: test12-pre6

truncate() and ftruncate() return values are different from the
POSIX requirements and from the values returned by other Unices (and from
our own manpages, BTW). Trivial fix follows. Rationale: see truncate(2),
ftruncate(2), POSIX or try to call the thing on other Unices.

Please, apply.

diff -urN rc12-pre5/fs/open.c rc12-pre5-truncate/fs/open.c
--- rc12-pre5/fs/open.c Wed Nov 29 21:37:31 2000
+++ rc12-pre5-truncate/fs/open.c Tue Dec 5 14:53:59 2000
@@ -102,7 +102,12 @@
goto out;
inode = nd.dentry->d_inode;

- error = -EACCES;
+ /* For directories it's -EISDIR, for other non-regulars - -EINVAL */
+ error = -EISDIR;
+ if (S_ISDIR(inode->i_mode))
+ goto dput_and_out;
+
+ error = -EINVAL;
if (!S_ISREG(inode->i_mode))
goto dput_and_out;

@@ -163,7 +168,7 @@
goto out;
dentry = file->f_dentry;
inode = dentry->d_inode;
- error = -EACCES;
+ error = -EINVAL;
if (!S_ISREG(inode->i_mode) || !(file->f_mode & FMODE_WRITE))
goto out_putf;
error = -EPERM;

2000-12-06 08:42:01

by Alexander Viro

[permalink] [raw]
Subject: [PATCH] Re: test12-pre6

ext2_get_block() should return -EFBIG if the block number is
too large for our block size. It returns -EIO and prints a warning
right now. Fix: don't generate bogus warnings (we can legitimately
get larger-than-maximum arguments; there is no way in hell VFS or VM
could know the ext2-specific size limits), return correct error value.
Documentation fixed to reflect the change. Additionally, ext2_get_branch()
became non-inlined - obviously correct, saves code size and makes the
things actually faster (it's too big for inlining).

Please, apply

diff -urN rc12-pre6/fs/ext2/inode.c rc12-pre6-ext2_get_block/fs/ext2/inode.c
--- rc12-pre6/fs/ext2/inode.c Tue Dec 5 02:03:14 2000
+++ rc12-pre6-ext2_get_block/fs/ext2/inode.c Tue Dec 5 14:59:18 2000
@@ -153,11 +153,13 @@
* This function translates the block number into path in that tree -
* return value is the path length and @offsets[n] is the offset of
* pointer to (n+1)th node in the nth one. If @block is out of range
- * (negative or too large) warning is printed and zero returned.
+ * (negative or too large) we return zero. Warning is printed if @block
+ * is negative - that should never happen. Too large value is OK, it
+ * just means that ext2_get_block() should return -%EFBIG.
*
* Note: function doesn't find node addresses, so no IO is needed. All
* we need to know is the capacity of indirect blocks (taken from the
- * inode->i_sb).
+ * @inode->i_sb).
*/

/*
@@ -196,7 +198,7 @@
offsets[n++] = (i_block >> ptrs_bits) & (ptrs - 1);
offsets[n++] = i_block & (ptrs - 1);
} else {
- ext2_warning (inode->i_sb, "ext2_block_to_path", "block > big");
+ /* Too large, nothing to do here */
}
return n;
}
@@ -216,7 +218,7 @@
* i.e. little-endian 32-bit), chain[i].p contains the address of that
* number (it points into struct inode for i==0 and into the bh->b_data
* for i>0) and chain[i].bh points to the buffer_head of i-th indirect
- * block for i>0 and NULL for i==0. In other words, it holds the block
+ * block for i>0 and %NULL for i==0. In other words, it holds the block
* numbers of the chain, addresses they were taken from (and where we can
* verify that chain did not change) and buffer_heads hosting these
* numbers.
@@ -230,11 +232,11 @@
* or when it reads all @depth-1 indirect blocks successfully and finds
* the whole chain, all way to the data (returns %NULL, *err == 0).
*/
-static inline Indirect *ext2_get_branch(struct inode *inode,
- int depth,
- int *offsets,
- Indirect chain[4],
- int *err)
+static Indirect *ext2_get_branch(struct inode *inode,
+ int depth,
+ int *offsets,
+ Indirect chain[4],
+ int *err)
{
kdev_t dev = inode->i_dev;
int size = inode->i_sb->s_blocksize;
@@ -505,7 +507,7 @@

static int ext2_get_block(struct inode *inode, long iblock, struct buffer_head *bh_result, int create)
{
- int err = -EIO;
+ int err = -EFBIG;
int offsets[4];
Indirect chain[4];
Indirect *partial;


2000-12-06 08:55:38

by Tigran Aivazian

[permalink] [raw]
Subject: Re: [PATCH] Re: test12-pre6

Oh, Alexander your patch conflicts (well, not strictly but causes
patch(1) to spew warnings) with the one I just wanted to send to Linus 10
minutes ago... maybe you could merge it into yours and re-send to Linus?

Mine is obvious, see below (vfs_permission does have enough to fail with
EROFS on a readonly fs for files/directories/symlinks and truncate(2) is
undefined beyond them):

--- linux/fs/open.c Thu Oct 26 16:11:21 2000
+++ work/fs/open.c Wed Dec 6 06:24:43 2000
@@ -110,10 +110,6 @@
if (error)
goto dput_and_out;

- error = -EROFS;
- if (IS_RDONLY(inode))
- goto dput_and_out;
-
error = -EPERM;
if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
goto dput_and_out;

2000-12-06 09:07:21

by Tigran Aivazian

[permalink] [raw]
Subject: Re: [PATCH] Re: test12-pre6

On Wed, 6 Dec 2000, Tigran Aivazian wrote:
> minutes ago... maybe you could merge it into yours and re-send to Linus?

here is the merged patch:

--- linux/fs/open.c Thu Oct 26 16:11:21 2000
+++ work/fs/open.c Wed Dec 6 07:38:30 2000
@@ -102,7 +102,12 @@
goto out;
inode = nd.dentry->d_inode;

- error = -EACCES;
+ /* For directories it's -EISDIR, for other non-regulars - -EINVAL */
+ error = -EISDIR;
+ if (S_ISDIR(inode->i_mode))
+ goto dput_and_out;
+
+ error = -EINVAL;
if (!S_ISREG(inode->i_mode))
goto dput_and_out;

@@ -110,10 +115,6 @@
if (error)
goto dput_and_out;

- error = -EROFS;
- if (IS_RDONLY(inode))
- goto dput_and_out;
-
error = -EPERM;
if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
goto dput_and_out;
@@ -163,7 +164,7 @@
goto out;
dentry = file->f_dentry;
inode = dentry->d_inode;
- error = -EACCES;
+ error = -EINVAL;
if (!S_ISREG(inode->i_mode) || !(file->f_mode & FMODE_WRITE))
goto out_putf;
error = -EPERM;

2000-12-06 09:25:11

by Tigran Aivazian

[permalink] [raw]
Subject: Re: [PATCH] Re: test12-pre6

On Wed, 6 Dec 2000, Tigran Aivazian wrote:
> error = -EPERM;
> if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
> goto dput_and_out;

also, while we are here -- are you sure that EPERM is a good idea for
IS_IMMUTABLE(inode)? Other parts of Linux return EACCES in this
case. Maybe it would be more consistent to do EACCES here too? This would
simply mean remove IS_IMMUTABLE() from the above if because
vfs_permission() does return -EACCES if we ask MAY_WRITE for IS_IMMUTABLE
inode.

Since, the SuSv2 standard is silent on the issue of immutable files (they
are Linux-specific) then it may make sense to be consistent?

Regards,
Tigran

2000-12-06 09:38:14

by Alexander Viro

[permalink] [raw]
Subject: Re: [PATCH] Re: test12-pre6



On Wed, 6 Dec 2000, Tigran Aivazian wrote:

> On Wed, 6 Dec 2000, Tigran Aivazian wrote:
> > error = -EPERM;
> > if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
> > goto dput_and_out;
>
> also, while we are here -- are you sure that EPERM is a good idea for
> IS_IMMUTABLE(inode)? Other parts of Linux return EACCES in this
> case. Maybe it would be more consistent to do EACCES here too? This would
> simply mean remove IS_IMMUTABLE() from the above if because
> vfs_permission() does return -EACCES if we ask MAY_WRITE for IS_IMMUTABLE
> inode.
>
> Since, the SuSv2 standard is silent on the issue of immutable files (they
> are Linux-specific) then it may make sense to be consistent?

They are not Linux-specific (check where they came from), so I would rather
check 4.4BSD and SuSv2[1] be damned. I'll look it up tomorrow, right now I'm
going down. Sorry.

[1] gotta love the CaPiTaLi2aTi0n, BTW.

2000-12-06 09:58:22

by Tigran Aivazian

[permalink] [raw]
Subject: Re: [PATCH] Re: test12-pre6

On Wed, 6 Dec 2000, Alexander Viro wrote:
> They are not Linux-specific (check where they came from), so I would rather
> check 4.4BSD and SuSv2[1] be damned. I'll look it up tomorrow, right now I'm
> going down. Sorry.

4.4BSD (FreeBSD 4.2-release) returns EPERM for truncate on immutable (schg
or uchg), in which case current Linux truncate(2) is broken. So if we want
to return the same as FreeBSD then we either need to:

a) change
vfs_permission() (and make sure all other cases returning EPERM for
immutable is ok) or

b) we need to move that explicit check in
do_sys_truncate() above the call to permission().

Regards,
Tigran

2000-12-06 12:08:15

by Wakko Warner

[permalink] [raw]
Subject: Re: test12-pre6

> - pre6:
> - Andrew Morton: exec_usermodehelper fixes

pre4 oopsed all over the place on my alpha with modules and autoloading
turned on as soon as it mounted / and freed unused memory. I take it this
was seen on i386 as well?

Will try pre6.

--
Lab tests show that use of micro$oft causes cancer in lab animals

2000-12-06 12:35:13

by Andrew Morton

[permalink] [raw]
Subject: Re: test12-pre6

Wakko Warner wrote:
>
> > - pre6:
> > - Andrew Morton: exec_usermodehelper fixes
>
> pre4 oopsed all over the place on my alpha with modules and autoloading
> turned on as soon as it mounted / and freed unused memory. I take it this
> was seen on i386 as well?

No... The problems showed themselves a little more subtly than that,
although in the pre5 version there was potential for schedule_task tasks to
be queued before the kernel thread which handles them was started.

This shouldn't normally be a problem, but it becomes a fatal problem if
someone tries to schedule a task and then synchronously waits
for it to complete, as the new exec_usermodehelper does.

This change didn't affect module loading per-se. But the
kernel does try to run /sbin/hotplug from deep within sys_insert_module
and sys_delete_module, so they're related.

> Will try pre6.

Please.

-

2000-12-06 13:37:51

by Tobias Ringstrom

[permalink] [raw]
Subject: Re: test12-pre6

On Tue, 5 Dec 2000, Linus Torvalds wrote:
> Concering the PCI irq routing fixes in particular, I'd ask people with
> laptops to start testing their kernels with PnP OS set to "yes" in the
> BIOS setup. We shoul dbe at a stage where it should basically work all the
> time, and it would be interesting to hear about cases that we don't handle
> right.

It works fine here on a Mitac laptop (the one that needed a Win98
warm-boot a few weeks ago), but it is quite noisy about IRQs that are also
used for other devices. (PCI: The same IRQ used for device 00:08.0)

The way I see it, 2.4.0-test12-pre6 is just a much longer name for 2.4.0.
Keep going like this and we may end up calling you Linus "Santa" Torvalds!
It has a nice ring to it, don't you think? :-) Or should that be *-<:-)

/Tobias

2000-12-06 14:01:33

by Miles Lane

[permalink] [raw]
Subject: Re: test12-pre6

Tobias Ringstrom wrote:

<snip>

> The way I see it, 2.4.0-test12-pre6 is just a much longer name for 2.4.0.
> Keep going like this and we may end up calling you Linus "Santa" Torvalds!
> It has a nice ring to it, don't you think? :-) Or should that be *-<:-)

I appreciate your enthusiasm, but I'd love it if I could use
both my 3c575 and Belkin BusPort Mobile Cardbus cards
simultaneously without them thrashing over IRQ11. This is
still happening in test12-pre6 and seems to me to indicate
that all is not well somewhere in IRQ management land.

Just call me Scrooge. :-) On the other hand, maybe
I wasn't nice this year and am getting coal for
<insert holiday of preference here>.


00:00.0 Host bridge: Intel Corporation 440BX/ZX - 82443BX/ZX Host bridge
(AGP disabled) (rev 02)
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR+ FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort-
<MAbort+ >SERR- <PERR+
Latency: 64 set
Region 0: Memory at <unassigned> (32-bit, prefetchable) [size=64M]

00:02.0 VGA compatible controller: Neomagic Corporation NM2160
[MagicGraph 128XD] (rev 01) (prog-if 00 [VGA])
Subsystem: Dell Computer Corporation: Unknown device 007e
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort-
<MAbort- >SERR- <PERR-
Latency: 16 min, 255 max, 128 set
Interrupt: pin A routed to IRQ 0
Region 0: Memory at fd000000 (32-bit, prefetchable) [size=16M]
Region 1: Memory at fea00000 (32-bit, non-prefetchable) [size=2M]
Region 2: Memory at fed00000 (32-bit, non-prefetchable) [size=1M]

00:04.0 CardBus bridge: Texas Instruments PCI1131 (rev 01)
Subsystem: Dell Computer Corporation: Unknown device 007e
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort-
<MAbort- >SERR- <PERR-
Latency: 168 set, cache line size 08
Interrupt: pin A routed to IRQ 11
Region 0: Memory at 10000000 (32-bit, non-prefetchable) [size=4K]
Bus: primary=00, secondary=01, subordinate=01, sec-latency=176
Memory window 0: 10400000-107ff000 (prefetchable)
Memory window 1: 10800000-10bff000
I/O window 0: 00001000-000010ff
I/O window 1: 00001400-000014ff
BridgeCtl: Parity- SERR- ISA- VGA- MAbort- >Reset- 16bInt- PostWrite+
16-bit legacy interface ports at 0001

00:04.1 CardBus bridge: Texas Instruments PCI1131 (rev 01)
Subsystem: Dell Computer Corporation: Unknown device 007e
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort-
<MAbort- >SERR- <PERR-
Latency: 168 set, cache line size 08
Interrupt: pin B routed to IRQ 11
Region 0: Memory at 10001000 (32-bit, non-prefetchable) [size=4K]
Bus: primary=00, secondary=05, subordinate=05, sec-latency=176
Memory window 0: 10c00000-10fff000 (prefetchable)
Memory window 1: 11000000-113ff000
I/O window 0: 00001800-000018ff
I/O window 1: 00001c00-00001cff
BridgeCtl: Parity- SERR- ISA- VGA- MAbort- >Reset- 16bInt- PostWrite+
16-bit legacy interface ports at 0001

00:07.0 Bridge: Intel Corporation 82371AB PIIX4 ISA (rev 02)
Control: I/O+ Mem+ BusMaster+ SpecCycle+ MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort-
<MAbort- >SERR- <PERR-
Latency: 0 set

00:07.1 IDE interface: Intel Corporation 82371AB PIIX4 IDE (rev 01)
(prog-if 80 [Master])
Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort-
<MAbort- >SERR- <PERR-
Latency: 64 set
Region 4: I/O ports at fcf0 [size=16]

(Ignore the follow USB host-controller entry. It's the built-in,
not the Cardbus USB host-controller).

00:07.2 USB Controller: Intel Corporation 82371AB PIIX4 USB (rev 01)
(prog-if 00 [UHCI])
Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort-
<MAbort- >SERR- <PERR-
Interrupt: pin D routed to IRQ 0
Region 4: I/O ports at fcc0 [disabled] [size=32]

00:07.3 Bridge: Intel Corporation 82371AB PIIX4 ACPI (rev 02)
Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort-
<MAbort- >SERR- <PERR-

(Here are the two conflicting cards)

01:00.0 Ethernet controller: 3Com Corporation 3c575 [Megahertz] 10/100
LAN CardBus (rev 01)
Subsystem: 3Com Corporation: Unknown device 5b57
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B-
Status: Cap+ 66Mhz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort-
<MAbort- >SERR- <PERR-
Latency: 10 min, 5 max, 64 set
Interrupt: pin A routed to IRQ 11
Region 0: I/O ports at 1000 [size=128]
Region 1: Memory at 10800000 (32-bit, non-prefetchable) [size=128]
Region 2: Memory at 10800080 (32-bit, non-prefetchable) [size=128]
Expansion ROM at 10400000 [size=128K]
Capabilities: [50] Power Management version 1
Flags: PMEClk- AuxPwr- DSI- D1+ D2+ PME-
Status: D0 PME-Enable- DSel=0 DScale=0 PME-

05:00.0 USB Controller: OPTi Inc. 82C861 (rev 10) (prog-if 10 [OHCI])
Subsystem: OPTi Inc.: Unknown device c861
Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort-
<MAbort- >SERR- <PERR-
Interrupt: pin A routed to IRQ 11
Region 0: Memory at 11000000 (32-bit, non-prefetchable) [size=4K]


Ho ho ho,

Miles

2000-12-06 18:40:15

by Erik Mouw

[permalink] [raw]
Subject: Re: test12-pre6

On Tue, Dec 05, 2000 at 11:25:55PM -0800, Linus Torvalds wrote:
> Concering the PCI irq routing fixes in particular, I'd ask people with
> laptops to start testing their kernels with PnP OS set to "yes" in the
> BIOS setup. We shoul dbe at a stage where it should basically work all the
> time, and it would be interesting to hear about cases that we don't handle
> right.

I noticed that in recent Phoenix BIOSes (06/21/00), this option has a
slightly name:

OS: [Win95/98/2000] [Other]

Choosing "OS = WinXX " doesn't assign PCI resources, so it is indeed
the same as "PnP OS = yes".

> (Anybody who has had problems with USB interrupts seemingly not being
> delivered and similar is also encouraged to test this new kernel).

Sorry, it still doesn't work on my Asus P8300 (440MX chipset), though
the PCI resources are allocated differently from test12-pre3. Here is
the boot output (with DEBUG enabled in the PCI stuff):


PCI: BIOS32 Service Directory structure at 0xc00f6bf0
PCI: BIOS32 Service Directory entry at 0xfd762
PCI: BIOS probe returned s=00 hw=01 ver=02.10 l=01
PCI: PCI BIOS revision 2.10 entry at 0xfd987, last bus=1
PCI: Using configuration type 1
PCI: Probing PCI hardware
PCI: IDE base address fixup for 00:07.1
PCI: Scanning for ghost devices on bus 0
PCI: IRQ init
PCI: Interrupt Routing Table found at 0xc00fdf50
00:07 slot=00 0:00/def8 1:00/def8 2:00/def8 3:63/0800
00:09 slot=00 0:62/0800 1:00/def8 2:00/def8 3:00/def8
01:04 slot=00 0:60/0800 1:00/def8 2:00/def8 3:00/def8
01:08 slot=00 0:60/0800 1:00/def8 2:00/def8 3:00/def8
00:0a slot=00 0:60/0800 1:00/def8 2:00/def8 3:00/def8
00:02 slot=00 0:60/0800 1:00/def8 2:00/def8 3:00/def8
00:00 slot=00 0:00/def8 1:61/08e0 2:00/def8 3:00/def8
PCI: Using IRQ router PIIX [8086/7198] at 00:07.0
PCI: IRQ fixup
00:00.1: ignoring bogus IRQ 255
00:07.2: ignoring bogus IRQ 255
00:09.0: ignoring bogus IRQ 255
00:0a.0: ignoring bogus IRQ 255
IRQ for 00:00.1(1) via 00:00.1 -> PIRQ 61, mask 08e0, excl 0000 -> newirq=0 ... failed
IRQ for 00:07.2(3) via 00:07.2 -> PIRQ 63, mask 0800, excl 0000 -> newirq=0 ... failed
IRQ for 00:09.0(0) via 00:09.0 -> PIRQ 62, mask 0800, excl 0000 -> newirq=0 ... failed
IRQ for 00:0a.0(0) via 00:0a.0 -> PIRQ 60, mask 0800, excl 0000 -> newirq=0 -> got IRQ 11
PCI: Found IRQ 11 for device 00:0a.0
PCI: The same IRQ used for device 00:02.0
PCI: Allocating resources
PCI: Resource f8000000-fbffffff (f=200, d=0, p=0)
PCI: Resource 0000fcf0-0000fcff (f=101, d=0, p=0)
PCI: Resource 0000f800-0000f8ff (f=101, d=1, p=1)
PCI: Resource 0000fc00-0000fc3f (f=101, d=1, p=1)
PCI: Resource 0000fcc0-0000fcdf (f=101, d=1, p=1)
PCI: Resource fedffc00-fedffcff (f=200, d=1, p=1)
PCI: Resource 0000fce8-0000fcef (f=109, d=1, p=1)
PCI: Resource 0000f400-0000f4ff (f=101, d=1, p=1)
got res[10000000:10000fff] for resource 0 of Ricoh Co Ltd RL5c475
PCI: Sorting device list...
isapnp: Scanning for Pnp cards...
isapnp: No Plug & Play device found

[...]

Linux PCMCIA Card Services 3.1.22
options: [pci] [cardbus] [pm]
IRQ for 00:0a.0(0) via 00:0a.0 -> PIRQ 60, mask 0800, excl 0000 -> newirq=11 -> got IRQ 11
PCI: Found IRQ 11 for device 00:0a.0
PCI: The same IRQ used for device 00:02.0
Intel PCIC probe: not found.
NET4: Linux TCP/IP 1.0 for NET4.0
IP Protocols: ICMP, UDP, TCP, IGMP
IP: routing cache hash table of 512 buckets, 4Kbytes
TCP: Hash tables configured (established 8192 bind 8192)
Yenta IRQ list 06b8, PCI irq11
Socket status: 30000416

[...]

usb.c: registered new driver usbdevfs
usb.c: registered new driver hub
usb-uhci.c: $Revision: 1.251 $ time 10:08:49 Dec 6 2000
usb-uhci.c: High bandwidth mode enabled
PCI: Enabling device 00:07.2 (0000 -> 0001)
IRQ for 00:07.2(3) via 00:07.2 -> PIRQ 63, mask 0800, excl 0000 -> newirq=11 -> assigning IRQ 11 ... OK
PCI: Assigned IRQ 11 for device 00:07.2
usb-uhci.c: USB UHCI at I/O 0xfcc0, IRQ 11
usb-uhci.c: Detected 2 ports
usb.c: new USB bus registered, assigned bus number 1
usb.c: kmalloc IF c24c6900, numif 1
usb.c: new device strings: Mfr=0, Product=2, SerialNumber=1
usb.c: USB device number 1 default language ID 0x0
Product: USB UHCI Root Hub
SerialNumber: fcc0

[...]

PCI: Enabling device 00:00.1 (0000 -> 0001)
IRQ for 00:00.1(1) via 00:00.1 -> PIRQ 61, mask 08e0, excl 0000 -> newirq=5 -> assigning IRQ 5 -> edge ... OK
PCI: Assigned IRQ 5 for device 00:00.1
PCI: Setting latency timer of device 00:00.1 to 64


So at first the PCI code can't allocate an IRQ for devices 00:00.1
(audio), 00:07.2 (USB), and 00:09.0 (winmodem), but after the audio and
USB modules get inserted, IRQ 5 and 11 get allocated.

Here is the output from "lscpi -vv" for this configuration:


00:00.0 Host bridge: Intel Corporation 82440MX I/O Controller (rev 01)
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort+ >SERR- <PERR-
Latency: 64

00:00.1 Multimedia audio controller: Intel Corporation 82440MX AC'97 Audio Controller
Subsystem: Asustek Computer, Inc.: Unknown device 1063
Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR-
Latency: 0
Interrupt: pin B routed to IRQ 5
Region 0: I/O ports at f800 [size=256]
Region 1: I/O ports at fc00 [size=64]

00:02.0 VGA compatible controller: Silicon Motion, Inc. SM720 Lynx3DM (rev a2) (prog-if 00 [VGA])
Subsystem: Asustek Computer, Inc.: Unknown device 1332
Control: I/O+ Mem+ BusMaster+ SpecCycle+ MemWINV+ VGASnoop- ParErr- Stepping- SERR- FastB2B-
Status: Cap+ 66Mhz+ UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
Latency: 64
Interrupt: pin A routed to IRQ 11
Region 0: Memory at f8000000 (32-bit, non-prefetchable) [size=64M]
Capabilities: [40] Power Management version 1
Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
Status: D0 PME-Enable- DSel=0 DScale=0 PME-

00:07.0 ISA bridge: Intel Corporation 82440MX PCI to ISA Bridge (rev 01)
Control: I/O+ Mem+ BusMaster+ SpecCycle+ MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
Latency: 0

00:07.1 IDE interface: Intel Corporation 82440MX EIDE Controller (prog-if 80 [Master])
Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
Latency: 64
Region 4: I/O ports at fcf0 [size=16]

00:07.2 USB Controller: Intel Corporation 82440MX USB Universal Host Controller (prog-if 00 [UHCI])
Control: I/O+ Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
Interrupt: pin D routed to IRQ 11
Region 4: I/O ports at fcc0 [size=32]

00:07.3 Bridge: Intel Corporation 82440MX Power Management Controller
Control: I/O+ Mem+ BusMaster- SpecCycle+ MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-

00:09.0 Communication controller: Lucent Microelectronics WinModem 56k (rev 01)
Subsystem: Action Tec Electronics Inc: Unknown device 2400
Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
Status: Cap+ 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
Interrupt: pin A routed to IRQ 0
Region 0: Memory at fedffc00 (32-bit, non-prefetchable) [disabled] [size=256]
Region 1: I/O ports at fce8 [disabled] [size=8]
Region 2: I/O ports at f400 [disabled] [size=256]
Capabilities: [f8] Power Management version 2
Flags: PMEClk- DSI+ D1- D2+ AuxCurrent=0mA PME(D0-,D1-,D2+,D3hot+,D3cold+)
Status: D0 PME-Enable- DSel=0 DScale=0 PME-

00:0a.0 CardBus bridge: Ricoh Co Ltd RL5c475 (rev 80)
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
Status: Cap+ 66Mhz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
Latency: 168
Interrupt: pin A routed to IRQ 11
Region 0: Memory at 10000000 (32-bit, non-prefetchable) [size=4K]
Bus: primary=00, secondary=01, subordinate=01, sec-latency=176
Memory window 0: 10400000-107ff000 (prefetchable)
Memory window 1: 10800000-10bff000
I/O window 0: 00001000-000010ff
I/O window 1: 00001400-000014ff
BridgeCtl: Parity- SERR- ISA- VGA- MAbort- >Reset- 16bInt+ PostWrite+
16-bit legacy interface ports at 0001


Let me know if you need more information.


Erik

--
J.A.K. (Erik) Mouw, Information and Communication Theory Group, Department
of Electrical Engineering, Faculty of Information Technology and Systems,
Delft University of Technology, PO BOX 5031, 2600 GA Delft, The Netherlands
Phone: +31-15-2783635 Fax: +31-15-2781843 Email: [email protected]
WWW: http://www-ict.its.tudelft.nl/~erik/

2000-12-06 19:10:00

by Linus Torvalds

[permalink] [raw]
Subject: Re: test12-pre6



On Wed, 6 Dec 2000, Erik Mouw wrote:
>
> So at first the PCI code can't allocate an IRQ for devices 00:00.1
> (audio), 00:07.2 (USB), and 00:09.0 (winmodem), but after the audio and
> USB modules get inserted, IRQ 5 and 11 get allocated.

No, the irq stuff is a two-stage process: at first it only _reads_ the irq
config stuff for every device - whether they have a driver or not - and at
this stage it will not ever actually allocate and set up a new route. It
will just see if a route has already been set up by the BIOS.

Then, when a driver actually does a pci_enable_device(), it will do the
second stage of PCI irq routing, which is to actually set up a route if
none originally existed. So this is why you first se "failed" messages
(the generic "test if there is a route" code) and then later when loading
the module you see "allocated irq XX" messages.

So your dmesg output looks fine, and everything is ok at that level. The
fact that something still doesn't work for you indicates that we still
have problems, of course.

Can you tell me what device it is that doesn't work for you?

Linus


2000-12-06 19:39:04

by Erik Mouw

[permalink] [raw]
Subject: Re: test12-pre6

On Wed, Dec 06, 2000 at 10:38:51AM -0800, Linus Torvalds wrote:
> On Wed, 6 Dec 2000, Erik Mouw wrote:
> > So at first the PCI code can't allocate an IRQ for devices 00:00.1
> > (audio), 00:07.2 (USB), and 00:09.0 (winmodem), but after the audio and
> > USB modules get inserted, IRQ 5 and 11 get allocated.
>
> No, the irq stuff is a two-stage process: at first it only _reads_ the irq
> config stuff for every device - whether they have a driver or not - and at
> this stage it will not ever actually allocate and set up a new route. It
> will just see if a route has already been set up by the BIOS.
>
> Then, when a driver actually does a pci_enable_device(), it will do the
> second stage of PCI irq routing, which is to actually set up a route if
> none originally existed. So this is why you first se "failed" messages
> (the generic "test if there is a route" code) and then later when loading
> the module you see "allocated irq XX" messages.

Thanks for explaining.

> So your dmesg output looks fine, and everything is ok at that level. The
> fact that something still doesn't work for you indicates that we still
> have problems, of course.
>
> Can you tell me what device it is that doesn't work for you?

The USB controller. That's device 00:07.2:

00:07.2 USB Controller: Intel Corporation 82440MX USB Universal Host Controller (prog-if 00 [UHCI])
Control: I/O+ Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
Status: Cap- 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
Interrupt: pin D routed to IRQ 11
Region 4: I/O ports at fcc0 [size=32]

I never tried the Winmodem, because I don't need a modem. It came free
with the laptop.


Erik

--
J.A.K. (Erik) Mouw, Information and Communication Theory Group, Department
of Electrical Engineering, Faculty of Information Technology and Systems,
Delft University of Technology, PO BOX 5031, 2600 GA Delft, The Netherlands
Phone: +31-15-2783635 Fax: +31-15-2781843 Email: [email protected]
WWW: http://www-ict.its.tudelft.nl/~erik/

2000-12-06 20:09:55

by Linus Torvalds

[permalink] [raw]
Subject: Re: test12-pre6



On Wed, 6 Dec 2000, Erik Mouw wrote:
> >
> > Can you tell me what device it is that doesn't work for you?
>
> The USB controller. That's device 00:07.2:
>
> 00:07.2 USB Controller: Intel Corporation 82440MX USB Universal Host Controller (prog-if 00 [UHCI])
> Control: I/O+ Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
> Status: Cap- 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
> Interrupt: pin D routed to IRQ 11
> Region 4: I/O ports at fcc0 [size=32]

Now, this is with a bog-standard PIIX irq router, so we definitely know
that we have the pirq table parsing right. I even have unofficial
confirmation from intel engineers on this.

But I see something obviously wrong there: you have busmaster disabled.

Looking into the UHCI controller code, I notice that neither UHCI driver
actually does the (required)

pci_set_master(dev);

Please add that to just after a successful "pci_enable_device(dev)", and I
just bet your USB will start working.

Johannes, Georg, the above is a fairly embarrassing bug, and is likely to
explain a _lot_ of USB failures (the OHCI driver does do this, btw).

Linus

2000-12-06 20:40:55

by Erik Mouw

[permalink] [raw]
Subject: Re: test12-pre6

On Wed, Dec 06, 2000 at 11:38:30AM -0800, Linus Torvalds wrote:
> But I see something obviously wrong there: you have busmaster disabled.
>
> Looking into the UHCI controller code, I notice that neither UHCI driver
> actually does the (required)
>
> pci_set_master(dev);
>
> Please add that to just after a successful "pci_enable_device(dev)", and I
> just bet your USB will start working.

Yes, that did the trick! Problem solved, thanks a lot!

> Johannes, Georg, the above is a fairly embarrassing bug, and is likely to
> explain a _lot_ of USB failures (the OHCI driver does do this, btw).

Here is the patch, I don't know if it is enabled in the right place,
but it definatively fixes the problem.

--- drivers/usb/uhci.c.old Wed Dec 6 20:55:05 2000
+++ drivers/usb/uhci.c Wed Dec 6 20:55:37 2000
@@ -2383,6 +2383,8 @@
if (pci_enable_device(dev) < 0)
return -ENODEV;

+ pci_set_master(dev);
+
if (!dev->irq) {
err("found UHCI device with no IRQ assigned. check BIOS
settings!");
return -ENODEV;
--- drivers/usb/usb-uhci.c.old Wed Dec 6 20:53:58 2000
+++ drivers/usb/usb-uhci.c Wed Dec 6 20:54:48 2000
@@ -2941,6 +2941,8 @@

if (pci_enable_device(dev) < 0)
return -ENODEV;
+
+ pci_set_master(dev);

/* Search for the IO base address.. */
for (i = 0; i < 6; i++) {

Thanks again,


Erik
[listening to an MP3 with the USB audio device]

--
J.A.K. (Erik) Mouw, Information and Communication Theory Group, Department
of Electrical Engineering, Faculty of Information Technology and Systems,
Delft University of Technology, PO BOX 5031, 2600 GA Delft, The Netherlands
Phone: +31-15-2783635 Fax: +31-15-2781843 Email: [email protected]
WWW: http://www-ict.its.tudelft.nl/~erik/

2000-12-06 21:37:37

by Jeff Garzik

[permalink] [raw]
Subject: Re: test12-pre6

Erik Mouw wrote:
>
> On Wed, Dec 06, 2000 at 11:38:30AM -0800, Linus Torvalds wrote:
> > But I see something obviously wrong there: you have busmaster disabled.
> >
> > Looking into the UHCI controller code, I notice that neither UHCI driver
> > actually does the (required)
> >
> > pci_set_master(dev);
> >
> > Please add that to just after a successful "pci_enable_device(dev)", and I
> > just bet your USB will start working.
>
> Yes, that did the trick! Problem solved, thanks a lot!
>
> > Johannes, Georg, the above is a fairly embarrassing bug, and is likely to
> > explain a _lot_ of USB failures (the OHCI driver does do this, btw).
>
> Here is the patch, I don't know if it is enabled in the right place,
> but it definatively fixes the problem.
>
> --- drivers/usb/uhci.c.old Wed Dec 6 20:55:05 2000
> +++ drivers/usb/uhci.c Wed Dec 6 20:55:37 2000
> @@ -2383,6 +2383,8 @@
> if (pci_enable_device(dev) < 0)
> return -ENODEV;
>
> + pci_set_master(dev);
> +
> if (!dev->irq) {
> err("found UHCI device with no IRQ assigned. check BIOS
> settings!");
> return -ENODEV;
> --- drivers/usb/usb-uhci.c.old Wed Dec 6 20:53:58 2000
> +++ drivers/usb/usb-uhci.c Wed Dec 6 20:54:48 2000
> @@ -2941,6 +2941,8 @@
>
> if (pci_enable_device(dev) < 0)
> return -ENODEV;
> +
> + pci_set_master(dev);
>
> /* Search for the IO base address.. */
> for (i = 0; i < 6; i++) {
>

I didn't test your patch, but it looks good. Immediately following
pci_enable_device is generally a really good place for the call to
pci_set_master.

Jeff


--
Jeff Garzik |
Building 1024 | These are not the J's you're lookin' for.
MandrakeSoft | It's an old Jedi mind trick.

2000-12-06 23:30:35

by Miles Lane

[permalink] [raw]
Subject: Re: test12-pre6

Hmm. Your patch doesn't test whether pci_enable_device(dev)
was successful, does it?

I think what you want is:

diff -u --new-file drivers/usb/uhci.c~ drivers/usb/uhci.c
--- drivers/usb/uhci.c~ Tue Dec 5 23:55:38 2000
+++ drivers/usb/uhci.c Wed Dec 6 14:50:00 2000
@@ -2380,8 +2380,10 @@
/* disable legacy emulation */
pci_write_config_word(dev, USBLEGSUP, 0);

-
if (pci_enable_device(dev) < 0)
+
if (pci_enable_device(dev) < 0) {
+
pci_set_master(dev);
return -ENODEV;
+
}

if (!dev->irq) {
err("found UHCI device with no IRQ assigned. check BIOS settings!");
diff -u --new-file drivers/usb/usb-uhci.c~ drivers/usb/usb-uhci.c
--- drivers/usb/usb-uhci.c~ Tue Dec 5 23:55:38 2000
+++ drivers/usb/usb-uhci.c Wed Dec 6 14:50:09 2000
@@ -2939,8 +2939,10 @@
{
int i;

-
if (pci_enable_device(dev) < 0)
+
if (pci_enable_device(dev) < 0) {
+
pci_set_master(dev);
return -ENODEV;
+
}

/* Search for the IO base address.. */
for (i = 0; i < 6; i++) {
diff -u --new-file drivers/usb/usb-ohci.c# drivers/usb/usb-ohci.c
--- drivers/usb/usb-ohci.c# Wed Dec 6 14:56:12 2000
+++ drivers/usb/usb-ohci.c Wed Dec 6 14:49:34 2000
@@ -2320,8 +2320,10 @@
unsigned long mem_resource, mem_len;
void *mem_base;

-
if (pci_enable_device(dev) < 0)
+
if (pci_enable_device(dev) < 0) {
+
pci_set_master(dev);
return -ENODEV;
+
}

/* we read its hardware registers as memory */
mem_resource = pci_resource_start(dev, 0);




>> if (pci_enable_device(dev) < 0)
>> return -ENODEV;
>>
>> + pci_set_master(dev);
>> +

2000-12-06 23:41:40

by Johannes Erdfelt

[permalink] [raw]
Subject: Re: test12-pre6

On Wed, Dec 06, 2000, Miles Lane <[email protected]> wrote:
> Hmm. Your patch doesn't test whether pci_enable_device(dev)
> was successful, does it?

Umm, it does. If pci_enable_device wasn't successful, it returns -ENODEV.

Your patch below calls pci_set_master if enabling the device fails and
then ignores the device.

Is that what you meant?

JE

> I think what you want is:
>
> diff -u --new-file drivers/usb/uhci.c~ drivers/usb/uhci.c
> --- drivers/usb/uhci.c~ Tue Dec 5 23:55:38 2000
> +++ drivers/usb/uhci.c Wed Dec 6 14:50:00 2000
> @@ -2380,8 +2380,10 @@
> /* disable legacy emulation */
> pci_write_config_word(dev, USBLEGSUP, 0);
>
> -
> if (pci_enable_device(dev) < 0)
> +
> if (pci_enable_device(dev) < 0) {
> +
> pci_set_master(dev);
> return -ENODEV;
> +
> }
>
> if (!dev->irq) {
> err("found UHCI device with no IRQ assigned. check BIOS settings!");
> diff -u --new-file drivers/usb/usb-uhci.c~ drivers/usb/usb-uhci.c
> --- drivers/usb/usb-uhci.c~ Tue Dec 5 23:55:38 2000
> +++ drivers/usb/usb-uhci.c Wed Dec 6 14:50:09 2000
> @@ -2939,8 +2939,10 @@
> {
> int i;
>
> -
> if (pci_enable_device(dev) < 0)
> +
> if (pci_enable_device(dev) < 0) {
> +
> pci_set_master(dev);
> return -ENODEV;
> +
> }
>
> /* Search for the IO base address.. */
> for (i = 0; i < 6; i++) {
> diff -u --new-file drivers/usb/usb-ohci.c# drivers/usb/usb-ohci.c
> --- drivers/usb/usb-ohci.c# Wed Dec 6 14:56:12 2000
> +++ drivers/usb/usb-ohci.c Wed Dec 6 14:49:34 2000
> @@ -2320,8 +2320,10 @@
> unsigned long mem_resource, mem_len;
> void *mem_base;
>
> -
> if (pci_enable_device(dev) < 0)
> +
> if (pci_enable_device(dev) < 0) {
> +
> pci_set_master(dev);
> return -ENODEV;
> +
> }
>
> /* we read its hardware registers as memory */
> mem_resource = pci_resource_start(dev, 0);

2000-12-06 23:44:50

by Jeff Garzik

[permalink] [raw]
Subject: Re: test12-pre6

Miles Lane wrote:
>
> Hmm. Your patch doesn't test whether pci_enable_device(dev)
> was successful, does it?

eh? It's self-evident from Erik's patch that pci_enable_device's return
call is already being tested, thus you only need to add a call to
pci_set_master.

Jeff


--
Jeff Garzik |
Building 1024 | These are not the J's you're lookin' for.
MandrakeSoft | It's an old Jedi mind trick.

2000-12-06 23:50:30

by Miles Lane

[permalink] [raw]
Subject: Re: test12-pre6

Jeff Garzik wrote:

<snip>

> eh? It's self-evident from Erik's patch that pci_enable_device's return
> call is already being tested, thus you only need to add a call to
> pci_set_master.

Sorry. I'll shut up now and go back to doing something
I actually am somewhat knowledgable about -- namely, testing.

Please forgive the noise,

Miles

2000-12-07 00:37:19

by Miles Lane

[permalink] [raw]
Subject: Re: test12-pre6

Erik Mouw wrote:

> On Wed, Dec 06, 2000 at 11:38:30AM -0800, Linus Torvalds wrote:
>
>> But I see something obviously wrong there: you have busmaster disabled.
>>
>> Looking into the UHCI controller code, I notice that neither UHCI driver
>> actually does the (required)
>>
>> pci_set_master(dev);
>>
>> Please add that to just after a successful "pci_enable_device(dev)", and I
>> just bet your USB will start working.


I tested this change applied to usb-ohci.
I did not fix the problem where eth0 spits
out a ton of error messages:

eth0: Host error, FIFO diagnostic register 0000.
eth0: using default media MII
eth0: Host error, FIFO diagnostic register 0000.
eth0: using default media MII
eth0: Too much work in interrupt, status e003.
eth0: Host error, FIFO diagnostic register 0000.
eth0: using default media MII

This error occurs when I am trasfering data over my
DSL connection while moving my USB mouse.

2000-12-07 02:01:34

by Dunlap, Randy

[permalink] [raw]
Subject: RE: test12-pre6

> From: Linus Torvalds [mailto:[email protected]]
>
...
> Now, this is with a bog-standard PIIX irq router, so we
> definitely know
> that we have the pirq table parsing right. I even have unofficial
> confirmation from intel engineers on this.
>
> But I see something obviously wrong there: you have busmaster
> disabled.
>
> Looking into the UHCI controller code, I notice that neither
> UHCI driver actually does the (required)
>
> pci_set_master(dev);
>
> Please add that to just after a successful
> "pci_enable_device(dev)", and I
> just bet your USB will start working.
>
> Johannes, Georg, the above is a fairly embarrassing bug, and
> is likely to
> explain a _lot_ of USB failures (the OHCI driver does do this, btw).
>
> Linus

Gawd, I'm embarrassed even if they aren't.
You probably just wiped out a significant portion of the
USB bug list.

Thanks,
~Randy

2000-12-07 02:05:04

by Wakko Warner

[permalink] [raw]
Subject: Re: test12-pre6

Andrew Morton wrote:
> Wakko Warner wrote:
> >
> > > - pre6:
> > > - Andrew Morton: exec_usermodehelper fixes
> >
> > pre4 oopsed all over the place on my alpha with modules and autoloading
> > turned on as soon as it mounted / and freed unused memory. I take it this
> > was seen on i386 as well?
>
> No... The problems showed themselves a little more subtly than that,
> although in the pre5 version there was potential for schedule_task tasks to
> be queued before the kernel thread which handles them was started.
>
> This shouldn't normally be a problem, but it becomes a fatal problem if
> someone tries to schedule a task and then synchronously waits
> for it to complete, as the new exec_usermodehelper does.
>
> This change didn't affect module loading per-se. But the
> kernel does try to run /sbin/hotplug from deep within sys_insert_module
> and sys_delete_module, so they're related.
>
> > Will try pre6.
>
> Please.

test12pre6 boots on my alpha and it works with modules enabled (and
autoloading). Now, if I could only reiserfs worked on alpha!

--
Lab tests show that use of micro$oft causes cancer in lab animals