From 737361b49b1d4f61109a883539d69e85ed2ccbc3 Mon Sep 17 00:00:00 2001 From: imp Date: Wed, 1 Dec 1999 07:38:54 +0000 Subject: [PATCH] Fix the hang on card eject problem and maybe the hang on suspend problem. o Create new timeout routine so we don't detach the card inside a ISR but instead drop back to spl0 via a timeout of 0. o Actually delete the child of the pccard device rather than just faking it badly. o Fix sio, ed and ep to have pccard detach routines that are int rather than void. o Fix ep and ed pccard detach routines to use if_detach rather than just if_down. if_detach destroys the device, while if_down just marks it down. In this incarnation of the pccard things, we map the disable the slot action to detach the driver, which removes the driver from the device tree. When that is done, a panic would soon follow as the ifconfig tried to down the device. Didn't fix: o Should cache the pccard dev child's pointer in struct slot o remove now unused parts of struct slot o Any driver using softc after detach has been called. sio's softc used to be statically allocated, so you could check sc->gone, but that is now gone. o Didn't remove gone from softc of drivers that use the old pccard method. Didn't test: o ed driver changes o sio driver changes on pccards o suspend (no laptop or apm support on my desktop) --- sys/dev/ed/if_ed_pccard.c | 9 +++--- sys/dev/ep/if_ep_pccard.c | 7 +++-- sys/dev/sio/sio.c | 20 +++---------- sys/isa/sio.c | 20 +++---------- sys/pccard/pccard.c | 59 +++++++++++++++++++++++++-------------- 5 files changed, 55 insertions(+), 60 deletions(-) diff --git a/sys/dev/ed/if_ed_pccard.c b/sys/dev/ed/if_ed_pccard.c index df47ae56cc8e..549a9a343c65 100644 --- a/sys/dev/ed/if_ed_pccard.c +++ b/sys/dev/ed/if_ed_pccard.c @@ -48,7 +48,7 @@ */ static int ed_pccard_probe(device_t); static int ed_pccard_attach(device_t); -static void ed_pccard_detach(device_t); +static int ed_pccard_detach(device_t); static device_method_t ed_pccard_methods[] = { /* Device interface */ @@ -78,7 +78,7 @@ DRIVER_MODULE(ed, pccard, ed_pccard_driver, ed_pccard_devclass, 0, 0); * and ensure that any driver entry points such as * read and write do not hang. */ -static void +static int ed_pccard_detach(device_t dev) { struct ed_softc *sc = device_get_softc(dev); @@ -86,13 +86,14 @@ ed_pccard_detach(device_t dev) if (sc->gone) { device_printf(dev, "already unloaded\n"); - return; + return (0); } ifp->if_flags &= ~IFF_RUNNING; - if_down(ifp); + if_detach(ifp); bus_teardown_intr(dev, sc->irq_res, &sc->irq_handle); sc->gone = 1; device_printf(dev, "unload\n"); + return (0); } /* diff --git a/sys/dev/ep/if_ep_pccard.c b/sys/dev/ep/if_ep_pccard.c index 1435ce3ff0dc..ad3749b7a081 100644 --- a/sys/dev/ep/if_ep_pccard.c +++ b/sys/dev/ep/if_ep_pccard.c @@ -212,7 +212,7 @@ ep_pccard_attach(device_t dev) return (error); } -static void +static int ep_pccard_detach(device_t dev) { struct ep_softc *sc = device_get_softc(dev); @@ -221,14 +221,15 @@ ep_pccard_detach(device_t dev) if (sc->gone) { device_printf(dev, "already unloaded\n"); - return; + return (0); } sc->arpcom.ac_if.if_flags &= ~IFF_RUNNING; - if_down(&sc->arpcom.ac_if); + if_detach(&sc->arpcom.ac_if); sc->gone = 1; bus_teardown_intr(dev, sc->irq, sc->ep_intrhand); ep_free(dev); device_printf(dev, "unload\n"); + return (0); } static device_method_t ep_pccard_methods[] = { diff --git a/sys/dev/sio/sio.c b/sys/dev/sio/sio.c index fe8244e78984..4789c43dd104 100644 --- a/sys/dev/sio/sio.c +++ b/sys/dev/sio/sio.c @@ -87,15 +87,6 @@ #endif #include -#include "card.h" -#if NCARD > 0 -/* XXX should die XXX */ -#include -#include -#include -#include -#endif - #ifndef __i386__ #define disable_intr() #define enable_intr() @@ -487,13 +478,9 @@ static int sio_pccard_probe(dev) device_t dev; { - const char *name; - - name = pccard_get_name(dev); - if (strcmp(name, "sio")) - return ENXIO; - /* Do not probe IRQ - pccardd has not arranged for it yet */ + /* XXX Actually it has been asigned to you, but isn't activated */ + /* XXX until you specifically activate the resource for your use. */ SET_FLAG(dev, COM_C_NOPROBE); return (sioprobe(dev)); @@ -515,7 +502,7 @@ sio_pccard_attach(dev) * and ensure that any driver entry points such as * read and write do not hang. */ -static void +static int sio_pccard_detach(dev) device_t dev; { @@ -543,6 +530,7 @@ sio_pccard_detach(dev) free(com, M_DEVBUF); device_printf(dev, "unload,gone\n"); } + return (0); } #endif /* NCARD > 0 */ diff --git a/sys/isa/sio.c b/sys/isa/sio.c index fe8244e78984..4789c43dd104 100644 --- a/sys/isa/sio.c +++ b/sys/isa/sio.c @@ -87,15 +87,6 @@ #endif #include -#include "card.h" -#if NCARD > 0 -/* XXX should die XXX */ -#include -#include -#include -#include -#endif - #ifndef __i386__ #define disable_intr() #define enable_intr() @@ -487,13 +478,9 @@ static int sio_pccard_probe(dev) device_t dev; { - const char *name; - - name = pccard_get_name(dev); - if (strcmp(name, "sio")) - return ENXIO; - /* Do not probe IRQ - pccardd has not arranged for it yet */ + /* XXX Actually it has been asigned to you, but isn't activated */ + /* XXX until you specifically activate the resource for your use. */ SET_FLAG(dev, COM_C_NOPROBE); return (sioprobe(dev)); @@ -515,7 +502,7 @@ sio_pccard_attach(dev) * and ensure that any driver entry points such as * read and write do not hang. */ -static void +static int sio_pccard_detach(dev) device_t dev; { @@ -543,6 +530,7 @@ sio_pccard_detach(dev) free(com, M_DEVBUF); device_printf(dev, "unload,gone\n"); } + return (0); } #endif /* NCARD > 0 */ diff --git a/sys/pccard/pccard.c b/sys/pccard/pccard.c index c9f69408642f..ae09a7fa6455 100644 --- a/sys/pccard/pccard.c +++ b/sys/pccard/pccard.c @@ -93,6 +93,8 @@ SYSCTL_INT(_machdep_pccard, OID_AUTO, pcic_resume_reset, CTLFLAG_RW, static int allocate_driver(struct slot *, struct dev_desc *); static void inserted(void *); static void disable_slot(struct slot *); +static void disable_slot_spl0(struct slot *); +static void disable_slot_to(void *); static int invalid_io_memory(unsigned long, int); static void power_off_slot(void *); @@ -176,34 +178,52 @@ power_off_slot(void *arg) static void disable_slot(struct slot *slt) { + /* XXX Need to store pccarddev in slt. */ device_t pccarddev; - struct pccard_devinfo *devi; + device_t *kids; + int nkids; int i; + int ret; /* - * Unload all the drivers on this slot. Note we can't - * remove the device structures themselves, because this - * may be called from the event routine, which is called - * from the slot controller's ISR, and removing the structures - * shouldn't happen during the middle of some driver activity. - * * Note that a race condition is possible here; if a * driver is accessing the device and it is removed, then * all bets are off... */ - pccarddev = devclass_get_device(pccard_devclass, 0); - for (devi = slt->devices; devi; devi = devi->next) { - if (devi->isahd.id_device != 0) { - device_delete_child(pccarddev, devi->isahd.id_device); - devi->isahd.id_device = 0; - } - } + pccarddev = devclass_get_device(pccard_devclass, slt->slotnum); + device_get_children(pccarddev, &kids, &nkids); + for (i = 0; i < nkids; i++) { + if ((ret = device_delete_child(pccarddev, kids[i])) != 0) + printf("pccard: delete failed: %d\n", ret); + } /* Power off the slot 1/2 second after removal of the card */ slt->poff_ch = timeout(power_off_slot, (caddr_t)slt, hz / 2); slt->pwr_off_pending = 1; } +static void +disable_slot_to(void *argp) +{ + struct slot *slt = (struct slot *) argp; + + disable_slot(slt); + slt->state = empty; + printf("pccard: card removed, slot %d\n", slt->slotnum); + pccard_remove_beep(); + selwakeup(&slt->selp); +} + +/* + * Disables the slot later when we drop to spl0 via a timeout. + */ +static void +disable_slot_spl0(struct slot *slt) +{ + slt->disable_ch = timeout(disable_slot_to, (caddr_t) slt, 0); +} + + /* * APM hooks for suspending and resuming. */ @@ -226,6 +246,7 @@ slot_suspend(void *arg) * Disable any pending timeouts for this slot since we're * powering it down/disabling now. */ + untimeout(power_off_slot, (caddr_t)slt, slt->disable_ch); untimeout(power_off_slot, (caddr_t)slt, slt->poff_ch); slt->ctrl->disable(slt); return (0); @@ -296,6 +317,7 @@ pccard_alloc_slot(struct slot_ctrl *ctrl) } callout_handle_init(&slt->insert_ch); callout_handle_init(&slt->poff_ch); + callout_handle_init(&slt->disable_ch); #if NAPM > 0 { struct apmhook *ap; @@ -385,6 +407,7 @@ inserted(void *arg) * power it off right now. Then, re-enable the power using * the (possibly new) power settings. */ + untimeout(power_off_slot, (caddr_t)slt, slt->disable_ch); untimeout(power_off_slot, (caddr_t)slt, slt->poff_ch); power_off_slot(slt); slt->ctrl->power(slt); @@ -415,13 +438,7 @@ pccard_event(struct slot *slt, enum card_event event) * data structures are not unlinked. */ if (slt->state == filled) { - int s = splhigh(); - disable_slot(slt); - slt->state = empty; - splx(s); - printf("pccard: card removed, slot %d\n", slt->slotnum); - pccard_remove_beep(); - selwakeup(&slt->selp); + disable_slot_spl0(slt); } break; case card_inserted: