netdev - Re: via velocity trivial patch

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

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

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

Date:	Fri, 08 May 2009 13:32:30 +0200
From:	Radek Podgorny <radek@...gorny.cz>
To:	David Miller <davem@...emloft.net>
CC:	netdev@...r.kernel.org
Subject: Re: via velocity trivial patch

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi David,

thanks for the feedback. I apologize for the inconvenience, this is my
first try on kernel contribution.

The "some garbage" are just my words for what Jeff Garzik called "also
bail if 0xffffffff (hardware fault / hardware unplugged)" in
http://lkml.indiana.edu/hypermail/linux/kernel/0406.0/0500.html in 2004.
Should I change that?

Anyway, I've verified this is really happening on my hardware when
multiple via velocity devices share the same interrupt. The one the
interrupt is for reads the correct ISR value, the others sometimes read
0 and sometimes 0xffffffff. I'm not educated enough to tell why.

Reasons to include the fix:
1) Someone smarter than me suggested it (Jeff Garzik).
2) Fixed my problem.
3) Does not introduce a new problem.

So I'm attaching the patch again. Now with the proper sign-off.

Sincerely
Radek Podgorny

Signed-off-by: Radek Podgorny <radek@...gorny.cz>

diff -ru linux-2.6.29.1-orig/drivers/net/via-velocity.c linux-2.6.29.1/drivers/net/via-velocity.c
- --- linux-2.6.29.1-orig/drivers/net/via-velocity.c	2009-04-02 22:55:27.000000000 +0200
+++ linux-2.6.29.1/drivers/net/via-velocity.c	2009-04-24 13:09:45.945318255 +0200
@@ -2226,8 +2226,8 @@
 	spin_lock(&vptr->lock);
 	isr_status = mac_read_isr(vptr->mac_regs);
 
- -	/* Not us ? */
- -	if (isr_status == 0) {
+	/* Not us or garbage ? */
+	if (isr_status == 0 || isr_status == 0xffffffff) {
 		spin_unlock(&vptr->lock);
 		return IRQ_NONE;
 	}
@@ -2236,10 +2236,11 @@
 
 	/*
 	 *	Keep processing the ISR until we have completed
- -	 *	processing and the isr_status becomes zero
+	 *	processing and the isr_status becomes zero or
+	 *	we read out some garbage
 	 */
 
- -	while (isr_status != 0) {
+	while (isr_status != 0 && isr_status != 0xffffffff) {
 		mac_write_isr(vptr->mac_regs, isr_status);
 		if (isr_status & (~(ISR_PRXI | ISR_PPRXI | ISR_PTXI | ISR_PPTXI)))
 			velocity_error(vptr, isr_status);

David Miller wrote:
> From: Radek Podgorny <radek@...gorny.cz>
> Date: Wed, 06 May 2009 23:07:02 +0200
> 
>> I've sent the following mail to Francois Romieu <romieu@...zoreil.com>
>> but did not get any reply. So I'm sending directly here. It's a trivial
>> patch for via-velocity driver.
> 
> "some garbage"...
> 
> Why would the interrupt status register give us all 1's
> if it is properly initialized?  I should report a zero,
> rather than "some garbage".
> 
> You also failed to provide a proper sign-off as described
> in Documentation/SubmittingPatches
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkoEGE0ACgkQ7mej6pjlbYSKVgCcCIr64sZq9N+yQhdjMgui3HfP
6ZcAnjXkaeVuiCA4vXkwK1+IQNfLO/9Q
=XZap
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html