Date: Sat, 21 Feb 2009 22:57:26 +0000 (UTC) From: Alexander Motin <mav@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r188903 - in head/sys/dev/ata: . chipsets Message-ID: <200902212257.n1LMvQs1077550@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mav Date: Sat Feb 21 22:57:26 2009 New Revision: 188903 URL: http://svn.freebsd.org/changeset/base/188903 Log: Improve ata_reinit(): - protect againtst recursions, - add new devices detection using ata_identify(). Improve ata_identify(): - do not add duplicate device if device already exist. Rework SATA hot-plug events handling. Instead of unsafe duplicate implementation use common ata_reinit() to handle all state changes. All together this gives quite stable and robust cold- and hot-plug operation, invariant to false, lost and duplicate events. Modified: head/sys/dev/ata/ata-all.c head/sys/dev/ata/ata-all.h head/sys/dev/ata/ata-pci.h head/sys/dev/ata/ata-sata.c head/sys/dev/ata/chipsets/ata-promise.c Modified: head/sys/dev/ata/ata-all.c ============================================================================== --- head/sys/dev/ata/ata-all.c Sat Feb 21 21:12:54 2009 (r188902) +++ head/sys/dev/ata/ata-all.c Sat Feb 21 22:57:26 2009 (r188903) @@ -62,6 +62,7 @@ static struct cdevsw ata_cdevsw = { /* prototypes */ static void ata_boot_attach(void); static device_t ata_add_child(device_t, struct ata_device *, int); +static void ata_conn_event(void *, int); static void bswap(int8_t *, int); static void btrim(int8_t *, int); static void bpack(int8_t *, int8_t *, int); @@ -127,6 +128,7 @@ ata_attach(device_t dev) bzero(&ch->queue_mtx, sizeof(struct mtx)); mtx_init(&ch->queue_mtx, "ATA queue lock", NULL, MTX_DEF); TAILQ_INIT(&ch->ata_queue); + TASK_INIT(&ch->conntask, 0, ata_conn_event, dev); /* reset the controller HW, the channel and device(s) */ while (ATA_LOCKING(dev, ATA_LF_LOCK) != ch->unit) @@ -181,6 +183,7 @@ ata_detach(device_t dev) device_delete_child(dev, children[i]); free(children, M_TEMP); } + taskqueue_drain(taskqueue_thread, &ch->conntask); /* release resources */ bus_teardown_intr(dev, ch->r_irq, ch->ih); @@ -196,6 +199,14 @@ ata_detach(device_t dev) return 0; } +static void +ata_conn_event(void *context, int dummy) +{ + device_t dev = (device_t)context; + + ata_reinit(dev); +} + int ata_reinit(device_t dev) { @@ -217,6 +228,11 @@ ata_reinit(device_t dev) /* catch eventual request in ch->running */ mtx_lock(&ch->state_mtx); + if (ch->state & ATA_STALL_QUEUE) { + /* Recursive reinits and reinits during detach prohobited. */ + mtx_unlock(&ch->state_mtx); + return (ENXIO); + } if ((request = ch->running)) callout_stop(&request->callout); ch->running = NULL; @@ -274,6 +290,9 @@ ata_reinit(device_t dev) mtx_unlock(&ch->state_mtx); ATA_LOCKING(dev, ATA_LF_UNLOCK); + /* Add new children. */ + ata_identify(dev); + if (bootverbose) device_printf(dev, "reinit done ..\n"); @@ -684,14 +703,27 @@ ata_identify(device_t dev) { struct ata_channel *ch = device_get_softc(dev); struct ata_device *atadev; + device_t *children; device_t child; - int i; + int nchildren, i, n = ch->devices; if (bootverbose) - device_printf(dev, "identify ch->devices=%08x\n", ch->devices); + device_printf(dev, "Identifying devices: %08x\n", ch->devices); + mtx_lock(&Giant); + /* Skip existing devices. */ + if (!device_get_children(dev, &children, &nchildren)) { + for (i = 0; i < nchildren; i++) { + if (children[i] && (atadev = device_get_softc(children[i]))) + n &= ~((ATA_ATA_MASTER | ATA_ATAPI_MASTER) << atadev->unit); + } + free(children, M_TEMP); + } + /* Create new devices. */ + if (bootverbose) + device_printf(dev, "New devices: %08x\n", n); for (i = 0; i < ATA_PM; ++i) { - if (ch->devices & (((ATA_ATA_MASTER | ATA_ATAPI_MASTER) << i))) { + if (n & (((ATA_ATA_MASTER | ATA_ATAPI_MASTER) << i))) { int unit = -1; if (!(atadev = malloc(sizeof(struct ata_device), @@ -701,7 +733,7 @@ ata_identify(device_t dev) } atadev->unit = i; #ifdef ATA_STATIC_ID - if (ch->devices & ((ATA_ATA_MASTER << i))) + if (n & (ATA_ATA_MASTER << i)) unit = (device_get_unit(dev) << 1) + i; #endif if ((child = ata_add_child(dev, atadev, unit))) { @@ -716,6 +748,7 @@ ata_identify(device_t dev) } bus_generic_probe(dev); bus_generic_attach(dev); + mtx_unlock(&Giant); return 0; } Modified: head/sys/dev/ata/ata-all.h ============================================================================== --- head/sys/dev/ata/ata-all.h Sat Feb 21 21:12:54 2009 (r188902) +++ head/sys/dev/ata/ata-all.h Sat Feb 21 22:57:26 2009 (r188903) @@ -530,6 +530,7 @@ struct ata_channel { TAILQ_HEAD(, ata_request) ata_queue; /* head of ATA queue */ struct ata_request *freezepoint; /* composite freezepoint */ struct ata_request *running; /* currently running request */ + struct task conntask; /* PHY events handling task */ }; /* disk bay/enclosure related */ Modified: head/sys/dev/ata/ata-pci.h ============================================================================== --- head/sys/dev/ata/ata-pci.h Sat Feb 21 21:12:54 2009 (r188902) +++ head/sys/dev/ata/ata-pci.h Sat Feb 21 22:57:26 2009 (r188903) @@ -66,15 +66,6 @@ struct ata_pci_controller { } interrupt[8]; /* XXX SOS max ch# for now */ }; -/* structure for SATA connection update hotplug/hotswap support */ -struct ata_connect_task { - struct task task; - device_t dev; - int action; -#define ATA_C_ATTACH 1 -#define ATA_C_DETACH 2 -}; - /* defines for known chipset PCI id's */ #define ATA_ACARD_ID 0x1191 #define ATA_ATP850 0x00021191 @@ -451,7 +442,6 @@ int ata_check_80pin(device_t dev, int mo int ata_mode2idx(int mode); /* global prototypes ata-sata.c */ -void ata_sata_phy_event(void *context, int dummy); void ata_sata_phy_check_events(device_t dev); int ata_sata_phy_reset(device_t dev); void ata_sata_setmode(device_t dev, int mode); Modified: head/sys/dev/ata/ata-sata.c ============================================================================== --- head/sys/dev/ata/ata-sata.c Sat Feb 21 21:12:54 2009 (r188902) +++ head/sys/dev/ata/ata-sata.c Sat Feb 21 22:57:26 2009 (r188903) @@ -50,41 +50,6 @@ __FBSDID("$FreeBSD$"); #include <dev/ata/ata-pci.h> #include <ata_if.h> -/* - * SATA support functions - */ -void -ata_sata_phy_event(void *context, int dummy) -{ - struct ata_connect_task *tp = (struct ata_connect_task *)context; - struct ata_channel *ch = device_get_softc(tp->dev); - device_t *children; - int nchildren, i; - - mtx_lock(&Giant); /* newbus suckage it needs Giant */ - if (tp->action == ATA_C_ATTACH) { - if (bootverbose) - device_printf(tp->dev, "CONNECTED\n"); - ATA_RESET(tp->dev); - ata_identify(tp->dev); - } - if (tp->action == ATA_C_DETACH) { - if (!device_get_children(tp->dev, &children, &nchildren)) { - for (i = 0; i < nchildren; i++) - if (children[i]) - device_delete_child(tp->dev, children[i]); - free(children, M_TEMP); - } - mtx_lock(&ch->state_mtx); - ch->state = ATA_IDLE; - mtx_unlock(&ch->state_mtx); - if (bootverbose) - device_printf(tp->dev, "DISCONNECTED\n"); - } - mtx_unlock(&Giant); /* suckage code dealt with, release Giant */ - free(tp, M_ATA); -} - void ata_sata_phy_check_events(device_t dev) { @@ -94,32 +59,17 @@ ata_sata_phy_check_events(device_t dev) /* clear error bits/interrupt */ ATA_IDX_OUTL(ch, ATA_SERROR, error); - /* do we have any events flagged ? */ - if (error) { - struct ata_connect_task *tp; - u_int32_t status = ATA_IDX_INL(ch, ATA_SSTATUS); - - /* if we have a connection event deal with it */ - if ((error & ATA_SE_PHY_CHANGED) && - (tp = (struct ata_connect_task *) - malloc(sizeof(struct ata_connect_task), - M_ATA, M_NOWAIT | M_ZERO))) { - + /* if we have a connection event deal with it */ + if (error & ATA_SE_PHY_CHANGED) { + if (bootverbose) { + u_int32_t status = ATA_IDX_INL(ch, ATA_SSTATUS); if (((status & ATA_SS_CONWELL_MASK) == ATA_SS_CONWELL_GEN1) || ((status & ATA_SS_CONWELL_MASK) == ATA_SS_CONWELL_GEN2)) { - if (bootverbose) device_printf(dev, "CONNECT requested\n"); - tp->action = ATA_C_ATTACH; - } - else { - if (bootverbose) + } else device_printf(dev, "DISCONNECT requested\n"); - tp->action = ATA_C_DETACH; - } - tp->dev = dev; - TASK_INIT(&tp->task, 0, ata_sata_phy_event, tp); - taskqueue_enqueue(taskqueue_thread, &tp->task); } + taskqueue_enqueue(taskqueue_thread, &ch->conntask); } } Modified: head/sys/dev/ata/chipsets/ata-promise.c ============================================================================== --- head/sys/dev/ata/chipsets/ata-promise.c Sat Feb 21 21:12:54 2009 (r188902) +++ head/sys/dev/ata/chipsets/ata-promise.c Sat Feb 21 22:57:26 2009 (r188903) @@ -637,7 +637,6 @@ ata_promise_mio_status(device_t dev) { struct ata_pci_controller *ctlr = device_get_softc(device_get_parent(dev)); struct ata_channel *ch = device_get_softc(dev); - struct ata_connect_task *tp; u_int32_t fake_reg, stat_reg, vector, status; switch (ctlr->chip->cfg2) { @@ -663,31 +662,17 @@ ata_promise_mio_status(device_t dev) ATA_OUTL(ctlr->r_res2, stat_reg, status & (0x00000011 << ch->unit)); /* check for and handle disconnect events */ - if ((status & (0x00000001 << ch->unit)) && - (tp = (struct ata_connect_task *) - malloc(sizeof(struct ata_connect_task), - M_ATA, M_NOWAIT | M_ZERO))) { - + if (status & (0x00000001 << ch->unit)) { if (bootverbose) device_printf(dev, "DISCONNECT requested\n"); - tp->action = ATA_C_DETACH; - tp->dev = dev; - TASK_INIT(&tp->task, 0, ata_sata_phy_event, tp); - taskqueue_enqueue(taskqueue_thread, &tp->task); + taskqueue_enqueue(taskqueue_thread, &ch->conntask); } /* check for and handle connect events */ - if ((status & (0x00000010 << ch->unit)) && - (tp = (struct ata_connect_task *) - malloc(sizeof(struct ata_connect_task), - M_ATA, M_NOWAIT | M_ZERO))) { - + if (status & (0x00000010 << ch->unit)) { if (bootverbose) device_printf(dev, "CONNECT requested\n"); - tp->action = ATA_C_ATTACH; - tp->dev = dev; - TASK_INIT(&tp->task, 0, ata_sata_phy_event, tp); - taskqueue_enqueue(taskqueue_thread, &tp->task); + taskqueue_enqueue(taskqueue_thread, &ch->conntask); } /* do we have any device action ? */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200902212257.n1LMvQs1077550>