Date: Wed, 11 Jul 2018 17:54:42 +0000 (UTC) From: Ian Lepore <ian@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r336202 - head/sys/dev/spibus Message-ID: <201807111754.w6BHsg8n050470@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: ian Date: Wed Jul 11 17:54:41 2018 New Revision: 336202 URL: https://svnweb.freebsd.org/changeset/base/336202 Log: Enhancements and fixes for the spigen(4) driver... - Resources used by spigen_mmap_single() are now tracked using devfs_set_cdevpriv() rather than in the softc. - Since resources are now tracked per-open-fd, there is no need to try to impose any exclusive-open logic, so flags related to that are removed. - Flags used to track open status to prevent detach() when the device is open are replaced with calls to device_busy()/device_unbusy(). That extends the protection up the hierarchy so that the spibus and hardware controller drivers also can't be detached while the device is open/in use. - Arbitrary limits on the maximum size of a transfer are removed, along with the sysctl variables that allowed the limits to be changed. There is just no reason to limit the size of a spi transfer to the machine's page size. Or to any other arbitrary value, really. - Most of the locking is removed. It was mostly protecting access to flags and fields in the softc that no longer exist. The locking that remains is just to prevent concurrent calls to device_[un]busy(). - The code was calling malloc() with M_WAITOK while holding a mutex in several places. Since most of the locking is gone, that's fixed. Modified: head/sys/dev/spibus/spigen.c Modified: head/sys/dev/spibus/spigen.c ============================================================================== --- head/sys/dev/spibus/spigen.c Wed Jul 11 16:44:14 2018 (r336201) +++ head/sys/dev/spibus/spigen.c Wed Jul 11 17:54:41 2018 (r336202) @@ -41,7 +41,6 @@ __FBSDID("$FreeBSD$"); #include <sys/proc.h> #include <sys/rwlock.h> #include <sys/spigenio.h> -#include <sys/sysctl.h> #include <sys/types.h> #include <vm/vm.h> @@ -55,13 +54,16 @@ __FBSDID("$FreeBSD$"); #ifdef FDT #include <dev/ofw/ofw_bus_subr.h> + +static struct ofw_compat_data compat_data[] = { + {"freebsd,spigen", true}, + {NULL, false} +}; + #endif #include "spibus_if.h" -#define SPIGEN_OPEN (1 << 0) -#define SPIGEN_MMAP_BUSY (1 << 1) - struct spigen_softc { device_t sc_dev; struct cdev *sc_cdev; @@ -69,15 +71,14 @@ struct spigen_softc { struct cdev *sc_adev; /* alias device */ #endif struct mtx sc_mtx; - uint32_t sc_command_length_max; /* cannot change while mmapped */ - uint32_t sc_data_length_max; /* cannot change while mmapped */ - vm_object_t sc_mmap_buffer; /* command, then data */ - vm_offset_t sc_mmap_kvaddr; - size_t sc_mmap_buffer_size; - int sc_debug; - int sc_flags; }; +struct spigen_mmap { + vm_object_t bufobj; + vm_offset_t kvaddr; + size_t bufsize; +}; + static int spigen_probe(device_t dev) { @@ -92,7 +93,7 @@ spigen_probe(device_t dev) #ifdef FDT if (ofw_bus_status_okay(dev) && - ofw_bus_is_compatible(dev, "freebsd,spigen")) + ofw_bus_search_compatible(dev, compat_data)->ocd_data) rv = BUS_PROBE_DEFAULT; #endif @@ -116,76 +117,6 @@ static struct cdevsw spigen_cdevsw = { }; static int -spigen_command_length_max_proc(SYSCTL_HANDLER_ARGS) -{ - struct spigen_softc *sc = (struct spigen_softc *)arg1; - uint32_t command_length_max; - int error; - - mtx_lock(&sc->sc_mtx); - command_length_max = sc->sc_command_length_max; - mtx_unlock(&sc->sc_mtx); - error = sysctl_handle_int(oidp, &command_length_max, - sizeof(command_length_max), req); - if (error == 0 && req->newptr != NULL) { - mtx_lock(&sc->sc_mtx); - if (sc->sc_mmap_buffer != NULL) - error = EBUSY; - else - sc->sc_command_length_max = command_length_max; - mtx_unlock(&sc->sc_mtx); - } - return (error); -} - -static int -spigen_data_length_max_proc(SYSCTL_HANDLER_ARGS) -{ - struct spigen_softc *sc = (struct spigen_softc *)arg1; - uint32_t data_length_max; - int error; - - mtx_lock(&sc->sc_mtx); - data_length_max = sc->sc_data_length_max; - mtx_unlock(&sc->sc_mtx); - error = sysctl_handle_int(oidp, &data_length_max, - sizeof(data_length_max), req); - if (error == 0 && req->newptr != NULL) { - mtx_lock(&sc->sc_mtx); - if (sc->sc_mmap_buffer != NULL) - error = EBUSY; - else - sc->sc_data_length_max = data_length_max; - mtx_unlock(&sc->sc_mtx); - } - return (error); -} - -static void -spigen_sysctl_init(struct spigen_softc *sc) -{ - struct sysctl_ctx_list *ctx; - struct sysctl_oid *tree_node; - struct sysctl_oid_list *tree; - - /* - * Add system sysctl tree/handlers. - */ - ctx = device_get_sysctl_ctx(sc->sc_dev); - tree_node = device_get_sysctl_tree(sc->sc_dev); - tree = SYSCTL_CHILDREN(tree_node); - SYSCTL_ADD_PROC(ctx, tree, OID_AUTO, "command_length_max", - CTLFLAG_MPSAFE | CTLFLAG_RW | CTLTYPE_UINT, sc, sizeof(*sc), - spigen_command_length_max_proc, "IU", "SPI command header portion (octets)"); - SYSCTL_ADD_PROC(ctx, tree, OID_AUTO, "data_length_max", - CTLFLAG_MPSAFE | CTLFLAG_RW | CTLTYPE_UINT, sc, sizeof(*sc), - spigen_data_length_max_proc, "IU", "SPI data trailer portion (octets)"); - SYSCTL_ADD_INT(ctx, tree, OID_AUTO, "data", CTLFLAG_RW, - &sc->sc_debug, 0, "debug flags"); - -} - -static int spigen_attach(device_t dev) { struct spigen_softc *sc; @@ -198,8 +129,6 @@ spigen_attach(device_t dev) sc = device_get_softc(dev); sc->sc_dev = dev; - sc->sc_command_length_max = PAGE_SIZE; - sc->sc_data_length_max = PAGE_SIZE; mtx_init(&sc->sc_mtx, device_get_nameunit(dev), NULL, MTX_DEF); @@ -230,30 +159,23 @@ spigen_attach(device_t dev) } #endif - spigen_sysctl_init(sc); - return (0); } static int spigen_open(struct cdev *cdev, int oflags, int devtype, struct thread *td) { - int error; device_t dev; struct spigen_softc *sc; - error = 0; dev = cdev->si_drv1; sc = device_get_softc(dev); mtx_lock(&sc->sc_mtx); - if (sc->sc_flags & SPIGEN_OPEN) - error = EBUSY; - else - sc->sc_flags |= SPIGEN_OPEN; + device_busy(sc->sc_dev); mtx_unlock(&sc->sc_mtx); - return (error); + return (0); } static int @@ -261,23 +183,16 @@ spigen_transfer(struct cdev *cdev, struct spigen_trans { struct spi_command transfer = SPI_COMMAND_INITIALIZER; device_t dev = cdev->si_drv1; - struct spigen_softc *sc = device_get_softc(dev); int error = 0; - mtx_lock(&sc->sc_mtx); - if (st->st_command.iov_len == 0) - error = EINVAL; - else if (st->st_command.iov_len > sc->sc_command_length_max || - st->st_data.iov_len > sc->sc_data_length_max) - error = ENOMEM; - mtx_unlock(&sc->sc_mtx); - if (error) - return (error); - #if 0 device_printf(dev, "cmd %p %u data %p %u\n", st->st_command.iov_base, st->st_command.iov_len, st->st_data.iov_base, st->st_data.iov_len); #endif + + if (st->st_command.iov_len == 0) + return (EINVAL); + transfer.tx_cmd = transfer.rx_cmd = malloc(st->st_command.iov_len, M_DEVBUF, M_WAITOK); if (st->st_data.iov_len > 0) { @@ -313,37 +228,22 @@ spigen_transfer_mmapped(struct cdev *cdev, struct spig { struct spi_command transfer = SPI_COMMAND_INITIALIZER; device_t dev = cdev->si_drv1; - struct spigen_softc *sc = device_get_softc(dev); - int error = 0; + struct spigen_mmap *mmap; + int error; - mtx_lock(&sc->sc_mtx); - if (sc->sc_flags & SPIGEN_MMAP_BUSY) - error = EBUSY; - else if (stm->stm_command_length > sc->sc_command_length_max || - stm->stm_data_length > sc->sc_data_length_max) - error = E2BIG; - else if (sc->sc_mmap_buffer == NULL) - error = EINVAL; - else if (sc->sc_mmap_buffer_size < - stm->stm_command_length + stm->stm_data_length) - error = ENOMEM; - if (error == 0) - sc->sc_flags |= SPIGEN_MMAP_BUSY; - mtx_unlock(&sc->sc_mtx); - if (error) + if ((error = devfs_get_cdevpriv((void **)&mmap)) != 0) return (error); - - transfer.tx_cmd = transfer.rx_cmd = (void *)sc->sc_mmap_kvaddr; + + if (mmap->bufsize < stm->stm_command_length + stm->stm_data_length) + return (E2BIG); + + transfer.tx_cmd = transfer.rx_cmd = (void *)((uintptr_t)mmap->kvaddr); transfer.tx_cmd_sz = transfer.rx_cmd_sz = stm->stm_command_length; transfer.tx_data = transfer.rx_data = - (void *)(sc->sc_mmap_kvaddr + stm->stm_command_length); + (void *)((uintptr_t)mmap->kvaddr + stm->stm_command_length); transfer.tx_data_sz = transfer.rx_data_sz = stm->stm_data_length; error = SPIBUS_TRANSFER(device_get_parent(dev), dev, &transfer); - mtx_lock(&sc->sc_mtx); - KASSERT((sc->sc_flags & SPIGEN_MMAP_BUSY), ("mmap no longer marked busy")); - sc->sc_flags &= ~(SPIGEN_MMAP_BUSY); - mtx_unlock(&sc->sc_mtx); return (error); } @@ -380,14 +280,26 @@ spigen_ioctl(struct cdev *cdev, u_long cmd, caddr_t da return (error); } +static void +spigen_mmap_cleanup(void *arg) +{ + struct spigen_mmap *mmap = arg; + + if (mmap->kvaddr != 0) + pmap_qremove(mmap->kvaddr, mmap->bufsize / PAGE_SIZE); + if (mmap->bufobj != NULL) + vm_object_deallocate(mmap->bufobj); + free(mmap, M_DEVBUF); +} + static int spigen_mmap_single(struct cdev *cdev, vm_ooffset_t *offset, vm_size_t size, struct vm_object **object, int nprot) { - device_t dev = cdev->si_drv1; - struct spigen_softc *sc = device_get_softc(dev); + struct spigen_mmap *mmap; vm_page_t *m; size_t n, pages; + int error; if (size == 0 || (nprot & (PROT_EXEC | PROT_READ | PROT_WRITE)) @@ -396,34 +308,38 @@ spigen_mmap_single(struct cdev *cdev, vm_ooffset_t *of size = roundup2(size, PAGE_SIZE); pages = size / PAGE_SIZE; - mtx_lock(&sc->sc_mtx); - if (sc->sc_mmap_buffer != NULL) { - mtx_unlock(&sc->sc_mtx); + if (devfs_get_cdevpriv((void **)&mmap) == 0) return (EBUSY); - } else if (size > sc->sc_command_length_max + sc->sc_data_length_max) { - mtx_unlock(&sc->sc_mtx); - return (E2BIG); + + mmap = malloc(sizeof(*mmap), M_DEVBUF, M_ZERO | M_WAITOK); + if ((mmap->kvaddr = kva_alloc(size)) == 0) { + spigen_mmap_cleanup(mmap); + return (ENOMEM); } - sc->sc_mmap_buffer_size = size; - *offset = 0; - sc->sc_mmap_buffer = *object = vm_pager_allocate(OBJT_PHYS, 0, size, - nprot, *offset, curthread->td_ucred); + mmap->bufsize = size; + mmap->bufobj = vm_pager_allocate(OBJT_PHYS, 0, size, nprot, 0, + curthread->td_ucred); + m = malloc(sizeof(*m) * pages, M_TEMP, M_WAITOK); - VM_OBJECT_WLOCK(*object); - vm_object_reference_locked(*object); // kernel and userland both + VM_OBJECT_WLOCK(mmap->bufobj); + vm_object_reference_locked(mmap->bufobj); // kernel and userland both for (n = 0; n < pages; n++) { - m[n] = vm_page_grab(*object, n, + m[n] = vm_page_grab(mmap->bufobj, n, VM_ALLOC_NOBUSY | VM_ALLOC_ZERO | VM_ALLOC_WIRED); m[n]->valid = VM_PAGE_BITS_ALL; } - VM_OBJECT_WUNLOCK(*object); - sc->sc_mmap_kvaddr = kva_alloc(size); - pmap_qenter(sc->sc_mmap_kvaddr, m, pages); + VM_OBJECT_WUNLOCK(mmap->bufobj); + pmap_qenter(mmap->kvaddr, m, pages); free(m, M_TEMP); - mtx_unlock(&sc->sc_mtx); - if (*object == NULL) - return (EINVAL); + if ((error = devfs_set_cdevpriv(mmap, spigen_mmap_cleanup)) != 0) { + /* Two threads were racing through this code; we lost. */ + spigen_mmap_cleanup(mmap); + return (error); + } + *offset = 0; + *object = mmap->bufobj; + return (0); } @@ -434,16 +350,7 @@ spigen_close(struct cdev *cdev, int fflag, int devtype struct spigen_softc *sc = device_get_softc(dev); mtx_lock(&sc->sc_mtx); - if (sc->sc_mmap_buffer != NULL) { - pmap_qremove(sc->sc_mmap_kvaddr, - sc->sc_mmap_buffer_size / PAGE_SIZE); - kva_free(sc->sc_mmap_kvaddr, sc->sc_mmap_buffer_size); - sc->sc_mmap_kvaddr = 0; - vm_object_deallocate(sc->sc_mmap_buffer); - sc->sc_mmap_buffer = NULL; - sc->sc_mmap_buffer_size = 0; - } - sc->sc_flags &= ~(SPIGEN_OPEN); + device_unbusy(sc->sc_dev); mtx_unlock(&sc->sc_mtx); return (0); } @@ -455,15 +362,6 @@ spigen_detach(device_t dev) sc = device_get_softc(dev); - mtx_lock(&sc->sc_mtx); - if (sc->sc_flags & SPIGEN_OPEN) { - mtx_unlock(&sc->sc_mtx); - return (EBUSY); - } - mtx_unlock(&sc->sc_mtx); - - mtx_destroy(&sc->sc_mtx); - #ifdef SPIGEN_LEGACY_CDEVNAME if (sc->sc_adev) destroy_dev(sc->sc_adev); @@ -472,6 +370,8 @@ spigen_detach(device_t dev) if (sc->sc_cdev) destroy_dev(sc->sc_cdev); + mtx_destroy(&sc->sc_mtx); + return (0); } @@ -494,3 +394,6 @@ static driver_t spigen_driver = { DRIVER_MODULE(spigen, spibus, spigen_driver, spigen_devclass, 0, 0); MODULE_DEPEND(spigen, spibus, 1, 1, 1); +#ifdef FDT +SIMPLEBUS_PNP_INFO(compat_data); +#endif
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201807111754.w6BHsg8n050470>