devclass_find_free_unit
M. Warner Losh
imp at bsdimp.com
Wed Jun 10 16:22:33 UTC 2009
In message: <200906100822.15516.jhb at freebsd.org>
John Baldwin <jhb at freebsd.org> writes:
: On Tuesday 09 June 2009 7:42:49 pm M. Warner Losh wrote:
: > What purpose does devclass_find_free_unit serve? I think it can safely be
: > eliminated from the tree. The current design is racy.
: >
: > Comments?
: >
: > It is currently used:
: >
: > ./arm/xscale/ixp425/.svn/text-base/avila_ata.c.svn-base:
: device_add_child(dev, "ata", devclass_find_free_unit(ata_devclass, 0));
: > ./arm/xscale/ixp425/avila_ata.c: device_add_child(dev, "ata",
: devclass_find_free_unit(ata_devclass, 0));
: > ./arm/at91/.svn/text-base/at91_cfata.c.svn-base:
: device_add_child(dev, "ata", devclass_find_free_unit(ata_devclass, 0));
: > ./arm/at91/at91_cfata.c: device_add_child(dev, "ata",
: devclass_find_free_unit(ata_devclass, 0));
: > ./powerpc/psim/.svn/text-base/ata_iobus.c.svn-base:
: devclass_find_free_unit(ata_devclass, 0));
: >
: > # All the above can be replaced with a simple '-1'.
: >
: > ata/ata-pci.c: unit : devclass_find_free_unit(ata_devclass, 2));
: > ata/ata-usb.c: devclass_find_free_unit(ata_devclass, 2))) ==
: NULL) {
: >
: > These can likely be replaced by '2', but that may result in a warning
: > message being printed that likely can be eliminated...
:
: ata does this so it can reserve ata0 and ata1 for the "legacy" ATA channels on
: legacy ATA PCI adapters. That is, if you have both SATA controllers and a
: PATA controller, this allows the two PATA channels to always be ata0 and ata1
: and the PATA drivers to always be ad0 - ad3. You could perhaps implement
: this in 8.x now by a really horrendous hack of having ISA hints for ata0 and
: ata1 and letting bus_hint_device_unit() in the atapci driver claim those
: hints for the channels on PATA controllers.
I think it already does something akin to this:
/* attach all channels on this controller */
for (unit = 0; unit < ctlr->channels; unit++) {
if ((ctlr->ichannels & (1 << unit)) == 0)
continue;
child = device_add_child(dev, "ata",
((unit == 0 || unit == 1) && ctlr->legacy) ?
unit : devclass_find_free_unit(ata_devclass, 2));
if (child == NULL)
device_printf(dev, "failed to add ata child device\n");
else
device_set_ivars(child, (void *)(intptr_t)unit);
}
Why not just replace devclass_find_free_unit with '2'?
All the other users in the tree aer bogus and should be replaced by
-1. Well, I'm not 100% sure about the ata-usb.c patch, since that
would also be necessary to avoid collision. And the above code really
only applies to x86-based machine, right? There's no need to do that
for non-intel boxes. Or is the assumption on those boxes the
controller would never be in legacy.
Warner
-------------- next part --------------
Index: arm/xscale/ixp425/avila_ata.c
===================================================================
--- arm/xscale/ixp425/avila_ata.c (revision 193873)
+++ arm/xscale/ixp425/avila_ata.c (working copy)
@@ -248,7 +248,7 @@
NULL, ata_avila_intr, sc, &sc->sc_ih);
/* attach channel on this controller */
- device_add_child(dev, "ata", devclass_find_free_unit(ata_devclass, 0));
+ device_add_child(dev, "ata", -1);
bus_generic_attach(dev);
return 0;
Index: arm/at91/at91_cfata.c
===================================================================
--- arm/at91/at91_cfata.c (revision 193873)
+++ arm/at91/at91_cfata.c (working copy)
@@ -94,7 +94,7 @@
/* XXX: init CF controller? */
callout_init(&sc->tick, 1); /* Callout to poll the device. */
- device_add_child(dev, "ata", devclass_find_free_unit(ata_devclass, 0));
+ device_add_child(dev, "ata", -1);
bus_generic_attach(dev);
return (0);
}
Index: arm/at91/files.at91
===================================================================
--- arm/at91/files.at91 (revision 193873)
+++ arm/at91/files.at91 (working copy)
@@ -18,6 +18,11 @@
arm/at91/uart_bus_at91usart.c optional uart
arm/at91/uart_cpu_at91rm9200usart.c optional uart
arm/at91/uart_dev_at91usart.c optional uart
+
+dev/cfi/cfi_bus_lbc.c optional cfi
+dev/cfi/cfi_core.c optional cfi
+dev/cfi/cfi_dev.c optional cfi
+
#
# All the boards we support
#
Index: arm/at91/at91_machdep.c
===================================================================
--- arm/at91/at91_machdep.c (revision 193873)
+++ arm/at91/at91_machdep.c (working copy)
@@ -179,6 +179,14 @@
PTE_NOCACHE,
},
{
+ AT91RM92_FLS_BASE,
+ AT91RM92_FLS_PA_BASE,
+ AT91RM92_FLS_SIZE,
+ VM_PROT_READ|VM_PROT_WRITE,
+ PTE_NOCACHE,
+ },
+
+ {
/* CompactFlash controller. */
AT91RM92_CF_BASE,
AT91RM92_CF_PA_BASE,
Index: arm/at91/at91_mci.c
===================================================================
--- arm/at91/at91_mci.c (revision 193873)
+++ arm/at91/at91_mci.c (working copy)
@@ -62,8 +62,15 @@
#include "mmcbr_if.h"
-#define BBSZ 512
+// #define AT91_MCI_DEBUG
+#define MAX_SIZE 1
+#define BBSZ 512 * MAX_SIZE
+/*
+ * Note: This driver only supports the SlotA card. No attempt has been made
+ * to support SlotB.
+ */
+
struct at91_mci_softc {
void *intrhand; /* Interrupt handle */
device_t dev;
@@ -204,10 +211,16 @@
sc->host.f_min = 375000;
sc->host.f_max = at91_master_clock / 2; /* Typically 30MHz */
sc->host.host_ocr = MMC_OCR_320_330 | MMC_OCR_330_340;
+ sc->host.caps = 0;
+ /*
+ * The in-tree Linux driver doesn't allow 4-wire operation for the
+ * at91rm9200, but does for other members of the family. The atmel
+ * patches to this do allow it, or have in the past. It is unclear
+ * that the hardware even works, but my boot loader uses 4-bit bus
+ * in polling mode successfully.
+ */
if (sc->sc_cap & CAP_HAS_4WIRE)
- sc->host.caps = MMC_CAP_4_BIT_DATA;
- else
- sc->host.caps = 0;
+ sc->host.caps |= MMC_CAP_4_BIT_DATA;
child = device_add_child(dev, "mmc", 0);
device_set_ivars(dev, &sc->host);
err = bus_generic_attach(dev);
@@ -300,9 +313,9 @@
clkdiv = (at91_master_clock / ios->clock) / 2;
}
if (ios->bus_width == bus_width_4)
- WR4(sc, MCI_SDCR, RD4(sc, MCI_SDCR) | MCI_SDCR_SDCBUS);
+ WR4(sc, MCI_SDCR, MCI_SDCR_SDCBUS);
else
- WR4(sc, MCI_SDCR, RD4(sc, MCI_SDCR) & ~MCI_SDCR_SDCBUS);
+ WR4(sc, MCI_SDCR, 0);
WR4(sc, MCI_MR, (RD4(sc, MCI_MR) & ~MCI_MR_CLKDIV) | clkdiv);
/* Do we need a settle time here? */
/* XXX We need to turn the device on/off here with a GPIO pin */
@@ -341,7 +354,9 @@
if (!data) {
// The no data case is fairly simple
at91_mci_pdc_disable(sc);
-// printf("CMDR %x ARGR %x\n", cmdr, cmd->arg);
+#ifdef AT91_MCI_DEBUG
+ printf("CMDR %x ARGR %x\n", cmdr, cmd->arg);
+#endif
WR4(sc, MCI_ARGR, cmd->arg);
WR4(sc, MCI_CMDR, cmdr);
WR4(sc, MCI_IER, MCI_SR_ERROR | MCI_SR_CMDRDY);
@@ -399,7 +414,9 @@
ier = MCI_SR_TXBUFE;
}
}
-// printf("CMDR %x ARGR %x with data\n", cmdr, cmd->arg);
+#ifdef AT91_MCI_DEBUG
+ printf("CMDR %x ARGR %x with data\n", cmdr, cmd->arg);
+#endif
WR4(sc, MCI_ARGR, cmd->arg);
if (cmdr & MCI_CMDR_TRCMD_START) {
if (cmdr & MCI_CMDR_TRDIR) {
@@ -438,6 +455,14 @@
sc->req = NULL;
sc->curcmd = NULL;
req->done(req);
+ /*
+ * Attempted hack-a-round for the DMA bug for multiple reads.
+ */
+ if (req->cmd->opcode == MMC_READ_MULTIPLE_BLOCK) {
+ at91_mci_fini(sc->dev);
+ at91_mci_init(sc->dev);
+ at91_mci_update_ios(sc->dev, NULL);
+ }
}
static int
@@ -498,7 +523,9 @@
uint32_t *walker;
struct mmc_command *cmd;
int i, len;
-
+#ifdef AT91_MCI_DEBUG
+ char *w2;
+#endif
cmd = sc->curcmd;
bus_dmamap_sync(sc->dmatag, sc->map, BUS_DMASYNC_POSTREAD);
bus_dmamap_unload(sc->dmatag, sc->map);
@@ -509,6 +536,15 @@
for (i = 0; i < len; i++)
walker[i] = bswap32(walker[i]);
}
+#ifdef AT91_MCI_DEBUG
+ printf("Read data\n");
+ for (i = 0, w2 = cmd->data->data; i < cmd->data->len; i++) {
+ if (i % 16 == 0)
+ printf("%08x ", cmd->arg + i);
+ printf("%02x%s", w2[i], (i + 1) % 16 ? " " : "\n");
+ }
+ printf("\n");
+#endif
// Finish up the sequence...
WR4(sc, MCI_IDR, MCI_SR_ENDRX);
WR4(sc, MCI_IER, MCI_SR_RXBUFF);
@@ -544,14 +580,19 @@
if ((sr & MCI_SR_RCRCE) && (cmd->opcode == MMC_SEND_OP_COND ||
cmd->opcode == ACMD_SD_SEND_OP_COND))
cmd->error = MMC_ERR_NONE;
- else if (sr & (MCI_SR_RTOE | MCI_SR_DTOE))
+ else if (sr & (MCI_SR_RTOE | MCI_SR_DTOE)) {
+ printf("TIMEOUT %#x\n", sr);
cmd->error = MMC_ERR_TIMEOUT;
- else if (sr & (MCI_SR_RCRCE | MCI_SR_DCRCE))
+ } else if (sr & (MCI_SR_RCRCE | MCI_SR_DCRCE)) {
+ printf("CRC %#x\n", sr);
cmd->error = MMC_ERR_BADCRC;
- else if (sr & (MCI_SR_OVRE | MCI_SR_UNRE))
+ } else if (sr & (MCI_SR_OVRE | MCI_SR_UNRE)) {
+ printf("FIFO %#x\n", sr);
cmd->error = MMC_ERR_FIFO;
- else
+ } else {
+ printf("FAILED %#x\n", sr);
cmd->error = MMC_ERR_FAILED;
+ }
done = 1;
if (sc->mapped && cmd->error) {
bus_dmamap_unload(sc->dmatag, sc->map);
@@ -656,7 +697,7 @@
*(int *)result = sc->host.caps;
break;
case MMCBR_IVAR_MAX_DATA:
- *(int *)result = 1;
+ *(int *)result = MAX_SIZE;
break;
}
return (0);
Index: arm/at91/at91rm92reg.h
===================================================================
--- arm/at91/at91rm92reg.h (revision 193873)
+++ arm/at91/at91rm92reg.h (working copy)
@@ -341,6 +341,10 @@
#define AT91RM92_OHCI_PA_BASE 0x00300000
#define AT91RM92_OHCI_SIZE 0x00100000
+#define AT91RM92_FLS_BASE 0xdf000000
+#define AT91RM92_FLS_PA_BASE 0x10000000
+#define AT91RM92_FLS_SIZE 0x02000000 /* Support up to 32MB flash */
+
#define AT91RM92_CF_BASE 0xdfd00000
#define AT91RM92_CF_PA_BASE 0x51400000
#define AT91RM92_CF_SIZE 0x00100000
Index: powerpc/psim/ata_iobus.c
===================================================================
--- powerpc/psim/ata_iobus.c (revision 193873)
+++ powerpc/psim/ata_iobus.c (working copy)
@@ -114,9 +114,7 @@
* Add a single child per controller. Should be able
* to add two
*/
- device_add_child(dev, "ata",
- devclass_find_free_unit(ata_devclass, 0));
-
+ device_add_child(dev, "ata", -1);
return (bus_generic_attach(dev));
}
Index: dev/ata/ata-all.c
===================================================================
--- dev/ata/ata-all.c (revision 193873)
+++ dev/ata/ata-all.c (working copy)
@@ -663,7 +663,7 @@
btrim(atacap->serial, sizeof(atacap->serial));
bpack(atacap->serial, atacap->serial, sizeof(atacap->serial));
- if (bootverbose)
+ if (bootverbose || 1)
printf("ata%d-%s: pio=%s wdma=%s udma=%s cable=%s wire\n",
device_get_unit(ch->dev),
ata_unit2str(atadev),
Index: dev/ata/ata-usb.c
===================================================================
--- dev/ata/ata-usb.c (revision 193873)
+++ dev/ata/ata-usb.c (working copy)
@@ -414,11 +414,10 @@
/* ata channels are children to this USB control device */
for (i = 0; i <= sc->maxlun; i++) {
- if ((child = device_add_child(sc->dev, "ata",
- devclass_find_free_unit(ata_devclass, 2))) == NULL) {
+ if ((child = device_add_child(sc->dev, "ata", -1)) == NULL)
device_printf(sc->dev, "failed to add ata child device\n");
- } else
- device_set_ivars(child, (void *)(intptr_t)i);
+ else
+ device_set_ivars(child, (void *)(intptr_t)i);
}
bus_generic_attach(sc->dev);
More information about the freebsd-arch
mailing list