Subject: port-sparc/29516: comments are wrong, also splhigh not strictly necessary (patch included)
To: None <port-sparc-maintainer@netbsd.org, gnats-admin@netbsd.org,>
From: Iain Hibbert <plunky@rya-online.net>
List: netbsd-bugs
Date: 02/23/2005 22:08:00
>Number:         29516
>Category:       port-sparc
>Synopsis:       comments are wrong, also splhigh a bit excessive
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    port-sparc-maintainer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Feb 23 22:08:00 +0000 2005
>Originator:     Iain Hibbert
>Release:        NetBSD 2.0
>Organization:
>Environment:
System: NetBSD galant 2.0 NetBSD 2.0 (GALANT) #27: Wed Feb 23 12:26:51 GMT 2005 plunky@galant:/home/plunky/src/sys/arch/i386/compile/GALANT i386
Architecture: i386
Machine: i386
>Description:
I was looking at dev/sbus/magma.c recently using it as a reference and I suppose that when
it was modified to use the softintr framework, a couple of original comments were left and
are now wrong which confused me slightly. Anyway, the patch below changes the comments.

Also, since the hard interrupt is running at IPL_SERIAL, then in the soft interrupt routine when we
want to lock it out, we dont actually need to use splhigh() in two places, but splserial() will do
it instead. Patch also changes that.

And, one more thing that I'm really not sure about. Since the hard interrupt routine runs at
IPL_SERIAL now (originally IPL_TTY I believe), then all through the rest of the code there
are spltty() sections that I guess would like to not conflict with the interrupts. should
they be splserial() now instead? If the driver works then maybe not but I thought I would
mention it.

iain
>How-To-Repeat:
well, I cant get confused again I worked it out now..
>Fix:

--- magma.c.orig	2005-02-23 21:10:56.000000000 +0000
+++ magma.c	2005-02-23 21:17:44.000000000 +0000
@@ -504,7 +504,7 @@
  *
  *  returns 1 if it handled it, otherwise 0
  *
- *  runs at interrupt priority
+ *  runs at IPL_SERIAL
  */
 int
 magma_hard(arg)
@@ -725,9 +725,7 @@
 /*
  * magma soft interrupt handler
  *
- *  returns 1 if it handled it, 0 otherwise
- *
- *  runs at spltty()
+ * runs at IPL_SOFTSERIAL
  */
 void
 magma_soft(arg)
@@ -776,7 +774,7 @@
 			(*tp->t_linesw->l_rint)(data, tp);
 		}
 
-		s = splhigh();	/* block out hard interrupt routine */
+		s = splserial();	/* block out hard interrupt routine */
 		flags = mp->mp_flags;
 		CLR(mp->mp_flags, MTTYF_DONE | MTTYF_CARRIER_CHANGED | MTTYF_RING_OVERFLOW);
 		splx(s);	/* ok */
@@ -813,7 +811,7 @@
 		if( !ISSET(mp->mp_flags, MBPPF_OPEN) )
 			continue;
 
-		s = splhigh();
+		s = splserial();
 		flags = mp->mp_flags;
 		CLR(mp->mp_flags, MBPPF_WAKEUP);
 		splx(s);