Module Name: src Committed By: thorpej Date: Sun May 16 04:40:08 UTC 2021
Modified Files: src/sys/dev/i2c [thorpej-i2c-spi-conf]: i2c.c Log Message: Rather than allocating 8KB (!!) of space per i2c bus for a sparsely populated array of child devices, use a sorted list instead, optimized a bit for the common usage pattern. To generate a diff of this commit: cvs rdiff -u -r1.78.2.2 -r1.78.2.3 src/sys/dev/i2c/i2c.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/dev/i2c/i2c.c diff -u src/sys/dev/i2c/i2c.c:1.78.2.2 src/sys/dev/i2c/i2c.c:1.78.2.3 --- src/sys/dev/i2c/i2c.c:1.78.2.2 Sat May 8 11:34:38 2021 +++ src/sys/dev/i2c/i2c.c Sun May 16 04:40:08 2021 @@ -1,4 +1,4 @@ -/* $NetBSD: i2c.c,v 1.78.2.2 2021/05/08 11:34:38 thorpej Exp $ */ +/* $NetBSD: i2c.c,v 1.78.2.3 2021/05/16 04:40:08 thorpej Exp $ */ /*- * Copyright (c) 2021 The NetBSD Foundation, Inc. @@ -69,7 +69,7 @@ #endif #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: i2c.c,v 1.78.2.2 2021/05/08 11:34:38 thorpej Exp $"); +__KERNEL_RCSID(0, "$NetBSD: i2c.c,v 1.78.2.3 2021/05/16 04:40:08 thorpej Exp $"); #include <sys/param.h> #include <sys/systm.h> @@ -95,10 +95,20 @@ __KERNEL_RCSID(0, "$NetBSD: i2c.c,v 1.78 #define I2C_MAX_ADDR 0x3ff /* 10-bit address, max */ #endif +struct i2c_device_link { + TAILQ_ENTRY(i2c_device_link) l_list; + device_t l_device; + i2c_addr_t l_addr; +}; + +TAILQ_HEAD(i2c_devlist_head, i2c_device_link); + struct iic_softc { device_t sc_dev; i2c_tag_t sc_tag; - device_t sc_devices[I2C_MAX_ADDR + 1]; + + kmutex_t sc_devlist_lock; + struct i2c_devlist_head sc_devlist; }; static dev_type_open(iic_open); @@ -129,6 +139,179 @@ const struct cdevsw iic_cdevsw = { static void iic_smbus_intr_thread(void *); +static struct i2c_device_link * +iic_devslot_lookup(struct iic_softc *sc, i2c_addr_t addr) +{ + struct i2c_device_link *link; + + KASSERT(mutex_owned(&sc->sc_devlist_lock)); + + /* + * A common pattern is "reserve then insert or delete", and + * this is often done in increasing address order. So check + * if the last entry is the one we're looking for before we + * search the list from the front. + */ + link = TAILQ_LAST(&sc->sc_devlist, i2c_devlist_head); + if (link == NULL) { + /* List is empty. */ + return NULL; + } + if (link->l_addr == addr) { + return link; + } + + TAILQ_FOREACH(link, &sc->sc_devlist, l_list) { + /* + * The list is sorted, so if the current list element + * has an address larger than the one we're looking + * for, then it's not in the list. + */ + if (link->l_addr > addr) { + break; + } + if (link->l_addr == addr) { + return link; + } + } + return NULL; +} + +static bool +iic_devslot_reserve(struct iic_softc *sc, i2c_addr_t addr) +{ + struct i2c_device_link *link, *new_link; + + new_link = kmem_zalloc(sizeof(*new_link), KM_SLEEP); + new_link->l_addr = addr; + + mutex_enter(&sc->sc_devlist_lock); + + /* Optimize for reserving in increasing i2c address order. */ + link = TAILQ_LAST(&sc->sc_devlist, i2c_devlist_head); + if (link == NULL || link->l_addr < new_link->l_addr) { + TAILQ_INSERT_TAIL(&sc->sc_devlist, new_link, l_list); + new_link = NULL; + goto done; + } + KASSERT(!TAILQ_EMPTY(&sc->sc_devlist)); + + /* Sort the new entry into the list. */ + TAILQ_FOREACH(link, &sc->sc_devlist, l_list) { + if (link->l_addr < new_link->l_addr) { + continue; + } + if (link->l_addr == new_link->l_addr) { + /* Address is already reserved / in-use. */ + goto done; + } + /* + * If we get here, we know we should be inserted + * before this element, because we checked to see + * if we should be the last entry before entering + * the loop. + */ + KASSERT(link->l_addr > new_link->l_addr); + TAILQ_INSERT_BEFORE(link, new_link, l_list); + new_link = NULL; + break; + } + /* + * Because we checked for an empty list early, if we got + * here it means we inserted before "link". + */ + KASSERT(link != NULL); + KASSERT(TAILQ_NEXT(new_link, l_list) == link); + + done: + mutex_exit(&sc->sc_devlist_lock); + + if (new_link != NULL) { + kmem_free(new_link, sizeof(*new_link)); + return false; + } + return true; +} + +static bool +iic_devslot_insert(struct iic_softc *sc, device_t dev, i2c_addr_t addr) +{ + struct i2c_device_link *link; + bool rv = false; + + mutex_enter(&sc->sc_devlist_lock); + + link = iic_devslot_lookup(sc, addr); + if (link != NULL) { + if (link->l_device == NULL) { + link->l_device = dev; + rv = true; + } + } + + mutex_exit(&sc->sc_devlist_lock); + + return rv; +} + +static bool +iic_devslot_remove(struct iic_softc *sc, device_t dev, i2c_addr_t addr) +{ + struct i2c_device_link *link; + bool rv = false; + + mutex_enter(&sc->sc_devlist_lock); + + link = iic_devslot_lookup(sc, addr); + if (link != NULL) { + if (link->l_device == dev) { + TAILQ_REMOVE(&sc->sc_devlist, link, l_list); + rv = true; + } else { + link = NULL; + } + } + + mutex_exit(&sc->sc_devlist_lock); + + if (link != NULL) { + kmem_free(link, sizeof(*link)); + return false; + } + return rv; +} + +static bool +iic_devslot_set(struct iic_softc *sc, device_t dev, i2c_addr_t addr) +{ + return dev != NULL ? iic_devslot_insert(sc, dev, addr) + : iic_devslot_remove(sc, dev, addr); +} + +static bool +iic_devslot_lookup_addr(struct iic_softc *sc, device_t dev, i2c_addr_t *addrp) +{ + struct i2c_device_link *link; + bool rv = false; + + KASSERT(dev != NULL); + KASSERT(addrp != NULL); + + mutex_enter(&sc->sc_devlist_lock); + + TAILQ_FOREACH(link, &sc->sc_devlist, l_list) { + if (link->l_device == dev) { + *addrp = link->l_addr; + rv = true; + break; + } + } + + mutex_exit(&sc->sc_devlist_lock); + + return rv; +} + static int iic_print_direct(void *aux, const char *pnp) { @@ -346,6 +529,8 @@ iic_search(device_t parent, cfdata_t cf, for (ia.ia_addr = first_addr; ia.ia_addr <= last_addr; ia.ia_addr++) { int error, match_result; + device_t newdev; + bool rv __diagused; /* * Skip I2C addresses that are reserved for @@ -357,7 +542,7 @@ iic_search(device_t parent, cfdata_t cf, /* * Skip addresses where a device is already attached. */ - if (sc->sc_devices[ia.ia_addr] != NULL) + if (! iic_devslot_reserve(sc, ia.ia_addr)) continue; /* @@ -374,8 +559,11 @@ iic_search(device_t parent, cfdata_t cf, * is there. */ match_result = config_probe(parent, cf, &ia);/*XXX*/ - if (match_result <= 0) + if (match_result <= 0) { + rv = iic_devslot_remove(sc, NULL, ia.ia_addr); + KASSERT(rv); continue; + } /* * If the quality of the match by the driver was low @@ -384,11 +572,15 @@ iic_search(device_t parent, cfdata_t cf, * to see if it looks like something is really there. */ if (match_result == I2C_MATCH_ADDRESS_ONLY && - (error = (*probe_func)(sc, &ia, 0)) != 0) + (error = (*probe_func)(sc, &ia, 0)) != 0) { + rv = iic_devslot_remove(sc, NULL, ia.ia_addr); + KASSERT(rv); continue; + } - sc->sc_devices[ia.ia_addr] = - config_attach(parent, cf, &ia, iic_print, CFARG_EOL); + newdev = config_attach(parent, cf, &ia, iic_print, CFARG_EOL); + rv = iic_devslot_set(sc, newdev, ia.ia_addr); + KASSERT(rv); } return 0; @@ -398,13 +590,15 @@ static void iic_child_detach(device_t parent, device_t child) { struct iic_softc *sc = device_private(parent); - int i; + i2c_addr_t addr; + bool rv __diagused; - for (i = 0; i <= I2C_MAX_ADDR; i++) - if (sc->sc_devices[i] == child) { - sc->sc_devices[i] = NULL; - break; - } + if (! iic_devslot_lookup_addr(sc, child, &addr)) { + return; + } + + rv = iic_devslot_remove(sc, child, addr); + KASSERT(rv); } static int @@ -423,6 +617,8 @@ iic_enumerate_devices_callback(device_t { struct iic_softc *sc = device_private(self); int loc[IICCF_NLOCS] = { 0 }; + device_t newdev; + bool rv __diagused; args->count++; @@ -434,13 +630,14 @@ iic_enumerate_devices_callback(device_t args->ia->ia_addr); return true; /* keep enumerating */ } - if (sc->sc_devices[args->ia->ia_addr] == NULL) { - sc->sc_devices[args->ia->ia_addr] = - config_found(self, args->ia, iic_print_direct, - /* CFARG_SUBMATCH, config_stdsubmatch, XXX */ - CFARG_LOCATORS, loc, - CFARG_DEVHANDLE, args->ia->ia_devhandle, - CFARG_EOL); + if (iic_devslot_reserve(sc, args->ia->ia_addr)) { + newdev = config_found(self, args->ia, iic_print_direct, + /* CFARG_SUBMATCH, config_stdsubmatch, XXX */ + CFARG_LOCATORS, loc, + CFARG_DEVHANDLE, args->ia->ia_devhandle, + CFARG_EOL); + rv = iic_devslot_set(sc, newdev, args->ia->ia_addr); + KASSERT(rv); } return true; /* keep enumerating */ } @@ -469,6 +666,9 @@ iic_attach(device_t parent, device_t sel ic = sc->sc_tag; ic->ic_devname = device_xname(self); + TAILQ_INIT(&sc->sc_devlist); + mutex_init(&sc->sc_devlist_lock, MUTEX_DEFAULT, IPL_NONE); + LIST_INIT(&(sc->sc_tag->ic_list)); LIST_INIT(&(sc->sc_tag->ic_proc_list)); @@ -513,14 +713,25 @@ iic_detach(device_t self, int flags) { struct iic_softc *sc = device_private(self); i2c_tag_t ic = sc->sc_tag; - int i, error; + struct i2c_device_link *link; + device_t child; + int error; void *hdl; - for (i = 0; i <= I2C_MAX_ADDR; i++) { - if (sc->sc_devices[i]) { - error = config_detach(sc->sc_devices[i], flags); - if (error) - return error; + /* Detach all children in address order. */ + for (;;) { + mutex_enter(&sc->sc_devlist_lock); + link = TAILQ_FIRST(&sc->sc_devlist); + if (link == NULL) { + mutex_exit(&sc->sc_devlist_lock); + break; + } + child = link->l_device; + mutex_exit(&sc->sc_devlist_lock); + + error = config_detach(child, flags); + if (error) { + return error; } } @@ -762,12 +973,6 @@ iic_ioctl_exec(struct iic_softc *sc, i2c if (I2C_OP_WRITE_P(iie->iie_op) && (flag & FWRITE) == 0) return EBADF; -#if 0 - /* Disallow userspace access to devices that have drivers attached. */ - if (sc->sc_devices[iie->iie_addr] != NULL) - return EBUSY; -#endif - if (iie->iie_cmd != NULL) { cmd = kmem_alloc(iie->iie_cmdlen, KM_SLEEP); error = copyin(iie->iie_cmd, cmd, iie->iie_cmdlen);