Date: Thu, 24 Apr 2008 15:59:26 -0400 From: John Baldwin <jhb@freebsd.org> To: Marius Strobl <marius@alchemy.franken.de> Cc: cvs-src@freebsd.org, Marcel Moolenaar <xcllnt@mac.com>, src-committers@freebsd.org, cvs-all@freebsd.org, Peter Wemm <peter@wemm.org> Subject: Re: cvs commit: src/sys/amd64/amd64 machdep.c Message-ID: <200804241559.26710.jhb@freebsd.org> In-Reply-To: <20080424162722.GB66545@alchemy.franken.de> References: <200804190725.m3J7Pvie056329@repoman.freebsd.org> <8764A4AA-DE86-43A6-B161-3159DE7E5AB8@mac.com> <20080424162722.GB66545@alchemy.franken.de>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday 24 April 2008 12:27:23 pm Marius Strobl wrote:
> On Wed, Apr 23, 2008 at 06:26:14PM -0700, Marcel Moolenaar wrote:
> >
> > On Apr 23, 2008, at 5:40 PM, Peter Wemm wrote:
> >
> > >On Wed, Apr 23, 2008 at 5:58 AM, John Baldwin <jhb@freebsd.org> wrote:
> > >>On Saturday 19 April 2008 03:25:57 am Peter Wemm wrote:
> > >>>peter 2008-04-19 07:25:57 UTC
> > >>>
> > >>> FreeBSD src repository
> > >>>
> > >>> Modified files:
> > >>> sys/amd64/amd64 machdep.c
> > >>> Log:
> > >>> Put in a real isa_irq_pending() stub in order to remove two lines
> > >>>of
> > >>>dmesg noise from sio per unit. sio likes to probe if interrupts are
> > >>>configured correctly by looking at the pending bits of the atpic
> > >>>in order
> > >>>to put a non-fatal warning on the console. I think I'd rather
> > >>>read the
> > >>>pending bits from the apics, but I'm not sure its worth the hassle.
> > >>
> > >>Actually, the x86 interrupt sources have a pending method so this
> > >>can be
> > >>replaced. Could probably easily write something like this:
> > >>
> > >>int
> > >>intr_pending(u_int irq)
> > >>{
> > >> struct intsrc *isrc;
> > >>
> > >> isc = intr_lookup_source(irq);
> > >> if (isrc == NULL)
> > >> panic("bizarre");
> > >> return (isrc->is_pic->pic_pending(isrc));
> > >>}
> > >>
> > >>For intr_machdep.c and use this in sio:
> > >>
> > >>#if defined(__i386__) || defined(__amd64__)
> > >> foo = intr_pending(rman_get_start(irq_resource));
> > >>#else
> > >> foo = isa_irq_pending() & (1 << rman_get_start(irq_resource));
> > >>#endif
> > >>
> > >>or some such. I'd really prefer to kill isa_irq_pending().
> > >
> > >Let's just add intr_pending() to all MD backends that currently
> > >provide isa_irq_pending() for sio's benenfit. Either as a simple
> > >wrapper around the now-static local isa_irq_pending() (ia64), or by
> > >simplifying and converting isa_irq_pending() into intr_pending()
> > >(sparc64).
> >
> > sio() is only for i386 and amd64, isa_irq_pending() should be
> > removed from all MD code, except from i386 and amd64.
>
> How likely is it that another driver starts using
> isa_irq_pending() or a intr_pending()? I'd like to remove
> isa_irq_pending() from the sparc64 bits for quite some time
> now but so far saw no good reason to do so apart from there
> not being a relevant consumer. The alternative would be to
> properly implement a intr_pending() that works with devices
> on all kinds of busses but without a potential consumer
> that's a waste of time.
> this is pointless.
I'd say unlikely. I haven't tested this yet, but here is what I have so far
(compile tested on i386, but not run tested, and not updated for peter's
commit):
--- //depot/projects/smpng/sys/amd64/amd64/intr_machdep.c 2008/04/05 21:13:40
+++ //depot/user/jhb/intr/amd64/amd64/intr_machdep.c 2008/04/24 14:28:51
@@ -300,12 +299,24 @@
{
struct pic *pic;
sx_xlock(&intr_table_lock);
STAILQ_FOREACH(pic, &pics, pics) {
if (pic->pic_suspend != NULL)
pic->pic_suspend(pic);
}
sx_xunlock(&intr_table_lock);
+}
+
+int
+intr_pending(u_int irq)
+{
+ struct intsrc *isrc;
+
+ isrc = intr_lookup_source(irq);
+ if (isrc == NULL)
+ /* XXX: panic? */
+ return (0);
+ return (isrc->is_pic->pic_source_pending(isrc));
}
static int
--- //depot/projects/smpng/sys/amd64/amd64/machdep.c 2008/03/18 12:54:14
+++ //depot/user/jhb/intr/amd64/amd64/machdep.c 2008/04/24 14:32:37
@@ -845,16 +845,6 @@
sd->sd_gran = ssd->ssd_gran;
}
-#if !defined(DEV_ATPIC) && defined(DEV_ISA)
-#include <isa/isavar.h>
-u_int
-isa_irq_pending(void)
-{
-
- return (0);
-}
-#endif
-
u_int basemem;
/*
--- //depot/projects/smpng/sys/amd64/include/intr_machdep.h 2008/03/14 19:53:02
+++ //depot/user/jhb/intr/amd64/include/intr_machdep.h 2008/04/24 14:28:51
@@ -144,6 +144,7 @@
enum intr_polarity pol);
void intr_execute_handlers(struct intsrc *isrc, struct trapframe *frame);
struct intsrc *intr_lookup_source(int vector);
+int intr_pending(u_int irq);
int intr_register_pic(struct pic *pic);
int intr_register_source(struct intsrc *isrc);
int intr_remove_handler(void *cookie);
--- //depot/projects/smpng/sys/amd64/isa/atpic.c 2008/03/24 19:59:34
+++ //depot/user/jhb/intr/amd64/isa/atpic.c 2008/04/24 14:32:37
@@ -595,19 +595,4 @@
DRIVER_MODULE(atpic, isa, atpic_driver, atpic_devclass, 0, 0);
DRIVER_MODULE(atpic, acpi, atpic_driver, atpic_devclass, 0, 0);
-
-/*
- * Return a bitmap of the current interrupt requests. This is 8259-specific
- * and is only suitable for use at probe time.
- */
-intrmask_t
-isa_irq_pending(void)
-{
- u_char irr1;
- u_char irr2;
-
- irr1 = inb(IO_ICU1);
- irr2 = inb(IO_ICU2);
- return ((irr2 << 8) | irr1);
-}
#endif /* DEV_ISA */
--- //depot/projects/smpng/sys/dev/sio/sio.c 2008/01/21 18:58:30
+++ //depot/user/jhb/intr/dev/sio/sio.c 2008/04/24 14:51:42
@@ -85,6 +85,11 @@
#endif
#include <dev/ic/ns16550.h>
+#if defined(__i386__) || defined(__amd64__)
+#define PROBE_IRQ
+#include <machine/intr_machdep.h>
+#endif
+
#define LOTS_OF_EVENTS 64 /* helps separate urgent events from input */
#ifdef COM_MULTIPORT
@@ -430,11 +435,12 @@
int fn;
device_t idev;
Port_t iobase;
- intrmask_t irqmap[4];
- intrmask_t irqs;
+#ifdef PROBE_IRQ
+ int irqpending[4];
+ u_long xirq;
+#endif
u_char mcr_image;
int result;
- u_long xirq;
u_int flags = device_get_flags(dev);
int rid;
struct resource *port;
@@ -540,8 +546,13 @@
}
}
#endif /* COM_MULTIPORT */
- if (bus_get_resource(idev, SYS_RES_IRQ, 0, NULL, NULL) != 0)
+#ifdef PROBE_IRQ
+ if (bus_get_resource(idev, SYS_RES_IRQ, 0, &xirq, NULL) != 0) {
mcr_image = 0;
+ xirq = ~0ul;
+ }
+ bzero(irqpending, sizeof(irqpending));
+#endif
bzero(failures, sizeof failures);
iobase = rman_get_start(port);
@@ -608,7 +619,10 @@
sio_setreg(com, com_mcr, mcr_image);
sio_setreg(com, com_ier, 0);
DELAY(1000); /* XXX */
- irqmap[0] = isa_irq_pending();
+#ifdef PROBE_IRQ
+ if (xirq != ~0ul)
+ irqpending[0] = intr_pending(xirq);
+#endif
/*
* Attempt to set loopback mode so that we can send a null byte
@@ -716,10 +730,16 @@
failures[1] = sio_getreg(com, com_ier) - IER_ETXRDY;
failures[2] = sio_getreg(com, com_mcr) - mcr_image;
DELAY(10000); /* Some internal modems need this time */
- irqmap[1] = isa_irq_pending();
+#ifdef PROBE_IRQ
+ if (xirq != ~0ul)
+ irqpending[1] = intr_pending(xirq);
+#endif
failures[4] = (sio_getreg(com, com_iir) & IIR_IMASK) - IIR_TXRDY;
DELAY(1000); /* XXX */
- irqmap[2] = isa_irq_pending();
+#ifdef PROBE_IRQ
+ if (xirq != ~0ul)
+ irqpending[2] = intr_pending(xirq);
+#endif
failures[6] = (sio_getreg(com, com_iir) & IIR_IMASK) - IIR_NOPEND;
/*
@@ -735,25 +755,27 @@
sio_setreg(com, com_cfcr, CFCR_8BITS); /* dummy to avoid bus echo */
failures[7] = sio_getreg(com, com_ier);
DELAY(1000); /* XXX */
- irqmap[3] = isa_irq_pending();
+#ifdef PROBE_IRQ
+ if (xirq != ~0ul)
+ irqpending[3] = intr_pending(xirq);
+#endif
failures[9] = (sio_getreg(com, com_iir) & IIR_IMASK) - IIR_NOPEND;
mtx_unlock_spin(&sio_lock);
- irqs = irqmap[1] & ~irqmap[0];
- if (bus_get_resource(idev, SYS_RES_IRQ, 0, &xirq, NULL) == 0 &&
- ((1 << xirq) & irqs) == 0) {
- printf(
- "sio%d: configured irq %ld not in bitmap of probed irqs %#x\n",
- device_get_unit(dev), xirq, irqs);
- printf(
- "sio%d: port may not be enabled\n",
- device_get_unit(dev));
+#ifdef PROBE_IRQ
+ if (xirq != ~0ul) {
+ if (irqpending[1] != 0) {
+ device_printf(dev,
+ "configured irq %ld did not trigger\n", xirq);
+ device_printf(dev, "port may not be enabled\n");
+ }
+ if (bootverbose)
+ device_printf(dev, "irq pending: %d %d %d %d\n",
+ irqpending[0], irqpending[1], irqpending[2],
+ irqpending[3]);
}
- if (bootverbose)
- printf("sio%d: irq maps: %#x %#x %#x %#x\n",
- device_get_unit(dev),
- irqmap[0], irqmap[1], irqmap[2], irqmap[3]);
+#endif
result = 0;
for (fn = 0; fn < sizeof failures; ++fn)
--- //depot/projects/smpng/sys/i386/i386/intr_machdep.c 2008/04/05 21:13:40
+++ //depot/user/jhb/intr/i386/i386/intr_machdep.c 2008/04/24 14:28:51
@@ -288,12 +287,24 @@
{
struct pic *pic;
sx_xlock(&intr_table_lock);
STAILQ_FOREACH(pic, &pics, pics) {
if (pic->pic_suspend != NULL)
pic->pic_suspend(pic);
}
sx_xunlock(&intr_table_lock);
+}
+
+int
+intr_pending(u_int irq)
+{
+ struct intsrc *isrc;
+
+ isrc = intr_lookup_source(irq);
+ if (isrc == NULL)
+ /* XXX: panic? */
+ return (0);
+ return (isrc->is_pic->pic_source_pending(isrc));
}
static int
--- //depot/projects/smpng/sys/i386/include/intr_machdep.h 2008/03/14 19:53:02
+++ //depot/user/jhb/intr/i386/include/intr_machdep.h 2008/04/24 14:28:51
@@ -140,6 +140,7 @@
enum intr_polarity pol);
void intr_execute_handlers(struct intsrc *isrc, struct trapframe *frame);
struct intsrc *intr_lookup_source(int vector);
+int intr_pending(u_int irq);
int intr_register_pic(struct pic *pic);
int intr_register_source(struct intsrc *isrc);
int intr_remove_handler(void *cookie);
--- //depot/projects/smpng/sys/i386/isa/atpic.c 2008/03/24 19:59:34
+++ //depot/user/jhb/intr/i386/isa/atpic.c 2008/04/24 14:32:37
@@ -661,19 +661,4 @@
#ifndef PC98
DRIVER_MODULE(atpic, acpi, atpic_driver, atpic_devclass, 0, 0);
#endif
-
-/*
- * Return a bitmap of the current interrupt requests. This is 8259-specific
- * and is only suitable for use at probe time.
- */
-intrmask_t
-isa_irq_pending(void)
-{
- u_char irr1;
- u_char irr2;
-
- irr1 = inb(IO_ICU1);
- irr2 = inb(IO_ICU2);
- return ((irr2 << 8) | irr1);
-}
#endif /* DEV_ISA */
--- //depot/projects/smpng/sys/ia64/isa/isa.c 2007/02/27 20:53:48
+++ //depot/user/jhb/intr/ia64/isa/isa.c 2008/04/24 14:32:37
@@ -73,17 +73,6 @@
{
}
-intrmask_t
-isa_irq_pending(void)
-{
- u_char irr1;
- u_char irr2;
-
- irr1 = inb(IO_ICU1);
- irr2 = inb(IO_ICU2);
- return ((irr2 << 8) | irr1);
-}
-
/*
* This implementation simply passes the request up to the parent
* bus, which in our case is the special i386 nexus, substituting any
--- //depot/projects/smpng/sys/isa/isavar.h 2005/04/14 18:55:16
+++ //depot/user/jhb/intr/isa/isavar.h 2008/04/24 14:32:37
@@ -157,7 +157,6 @@
/* Device class for ISA bridges. */
extern devclass_t isab_devclass;
-extern intrmask_t isa_irq_pending(void);
extern void isa_probe_children(device_t dev);
void isa_dmacascade(int chan);
--- //depot/projects/smpng/sys/pc98/cbus/sio.c 2008/03/13 21:35:00
+++ //depot/user/jhb/intr/pc98/cbus/sio.c 2008/04/24 14:51:42
@@ -124,6 +124,11 @@
#include <dev/ic/rsa.h>
#endif
+#if defined(__i386__) || defined(__amd64__)
+#define PROBE_IRQ
+#include <machine/intr_machdep.h>
+#endif
+
#define LOTS_OF_EVENTS 64 /* helps separate urgent events from input */
/*
@@ -767,11 +772,12 @@
int fn;
device_t idev;
Port_t iobase;
- intrmask_t irqmap[4];
- intrmask_t irqs;
+#ifdef PROBE_IRQ
+ int irqpending[4];
+ u_long xirq;
+#endif
u_char mcr_image;
int result;
- u_long xirq;
u_int flags = device_get_flags(dev);
int rid;
struct resource *port;
@@ -936,7 +942,7 @@
tmp = ( inb( iod.ctrl ) & ~(IEN_Rx|IEN_TxEMP|IEN_Tx));
outb( iod.ctrl, tmp|IEN_TxEMP );
DELAY(10);
- result = isa_irq_pending() ? 0 : ENXIO;
+ result = intr_pending(iod.irq) ? 0 : ENXIO;
outb( iod.ctrl, tmp );
COM_INT_ENABLE
} else {
@@ -993,8 +999,13 @@
#endif
}
#endif /* COM_MULTIPORT */
- if (bus_get_resource(idev, SYS_RES_IRQ, 0, NULL, NULL) != 0)
+#ifdef PROBE_IRQ
+ if (bus_get_resource(idev, SYS_RES_IRQ, 0, &xirq, NULL) != 0) {
mcr_image = 0;
+ xirq = ~0ul;
+ }
+ bzero(irqpending, sizeof(irqpending));
+#endif
bzero(failures, sizeof failures);
iobase = rman_get_start(port);
@@ -1077,7 +1088,10 @@
sio_setreg(com, com_mcr, mcr_image);
sio_setreg(com, com_ier, 0);
DELAY(1000); /* XXX */
- irqmap[0] = isa_irq_pending();
+#ifdef PROBE_IRQ
+ if (xirq != ~0ul)
+ irqpending[0] = intr_pending(xirq);
+#endif
/*
* Attempt to set loopback mode so that we can send a null byte
@@ -1189,14 +1203,20 @@
failures[1] = sio_getreg(com, com_ier) - IER_ETXRDY;
failures[2] = sio_getreg(com, com_mcr) - mcr_image;
DELAY(10000); /* Some internal modems need this time */
- irqmap[1] = isa_irq_pending();
+#ifdef PROBE_IRQ
+ if (xirq != ~0ul)
+ irqpending[1] = intr_pending(xirq);
+#endif
failures[4] = (sio_getreg(com, com_iir) & IIR_IMASK) - IIR_TXRDY;
#ifdef PC98
if (iod.if_type == COM_IF_RSA98III)
inb(iobase + rsa_srr);
#endif
DELAY(1000); /* XXX */
- irqmap[2] = isa_irq_pending();
+#ifdef PROBE_IRQ
+ if (xirq != ~0ul)
+ irqpending[2] = intr_pending(xirq);
+#endif
failures[6] = (sio_getreg(com, com_iir) & IIR_IMASK) - IIR_NOPEND;
#ifdef PC98
if (iod.if_type == COM_IF_RSA98III)
@@ -1220,7 +1240,10 @@
outb(iobase + rsa_ier, 0x00);
#endif
DELAY(1000); /* XXX */
- irqmap[3] = isa_irq_pending();
+#ifdef PROBE_IRQ
+ if (xirq != ~0ul)
+ irqpending[3] = intr_pending(xirq);
+#endif
failures[9] = (sio_getreg(com, com_iir) & IIR_IMASK) - IIR_NOPEND;
#ifdef PC98
if (iod.if_type == COM_IF_RSA98III) {
@@ -1231,20 +1254,19 @@
mtx_unlock_spin(&sio_lock);
- irqs = irqmap[1] & ~irqmap[0];
- if (bus_get_resource(idev, SYS_RES_IRQ, 0, &xirq, NULL) == 0 &&
- ((1 << xirq) & irqs) == 0) {
- printf(
- "sio%d: configured irq %ld not in bitmap of probed irqs %#x\n",
- device_get_unit(dev), xirq, irqs);
- printf(
- "sio%d: port may not be enabled\n",
- device_get_unit(dev));
+#ifdef PROBE_IRQ
+ if (xirq != ~0ul) {
+ if (irqpending[1] != 0) {
+ device_printf(dev,
+ "configured irq %ld did not trigger\n", xirq);
+ device_printf(dev, "port may not be enabled\n");
+ }
+ if (bootverbose)
+ device_printf(dev, "irq pending: %d %d %d %d\n",
+ irqpending[0], irqpending[1], irqpending[2],
+ irqpending[3]);
}
- if (bootverbose)
- printf("sio%d: irq maps: %#x %#x %#x %#x\n",
- device_get_unit(dev),
- irqmap[0], irqmap[1], irqmap[2], irqmap[3]);
+#endif
result = 0;
for (fn = 0; fn < sizeof failures; ++fn)
--- //depot/projects/smpng/sys/sparc64/isa/isa.c 2007/12/20 22:29:11
+++ //depot/user/jhb/intr/sparc64/isa/isa.c 2008/04/24 14:32:37
@@ -64,7 +64,6 @@
static phandle_t isab_node;
static struct isa_ranges *isab_ranges;
static int isab_nrange;
-static ofw_pci_intr_t isa_ino[8];
static struct ofw_bus_iinfo isa_iinfo;
/*
@@ -82,23 +81,6 @@
static void isa_setup_children(device_t, phandle_t);
-intrmask_t
-isa_irq_pending(void)
-{
- intrmask_t pending;
- int i;
-
- /* XXX: Is this correct? */
- for (i = 7, pending = 0; i >= 0; i--) {
- pending <<= 1;
- if (isa_ino[i] != PCI_INVALID_IRQ) {
- pending |= (OFW_PCI_INTR_PENDING(isa_bus_device,
- isa_ino[i]) == 0) ? 0 : 1;
- }
- }
- return (pending);
-}
-
void
isa_init(device_t dev)
{
@@ -115,17 +97,6 @@
ofw_bus_setup_iinfo(isab_node, &isa_iinfo, sizeof(ofw_isa_intr_t));
- /*
- * This is really a bad kludge; however, it is needed to provide
- * isa_irq_pending(), which is unfortunately still used by some
- * drivers.
- * XXX: The only driver still using isa_irq_pending() is sio(4)
- * which we don't use on sparc64. Should we just drop support
- * for isa_irq_pending()?
- */
- for (i = 0; i < 8; i++)
- isa_ino[i] = PCI_INVALID_IRQ;
-
isa_setup_children(dev, isab_node);
for (i = isab_nrange - 1; i >= 0; i--) {
@@ -275,7 +246,6 @@
intrs[i], (unsigned long)node, name);
continue;
}
- isa_ino[intrs[i]] = rintr;
bus_set_resource(cdev, SYS_RES_IRQ, i, rintr, 1);
}
if (intrs != NULL)
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200804241559.26710.jhb>