Date: Sun, 22 Dec 2019 19:30:13 +0000 (UTC) From: Ian Lepore <ian@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r356022 - in stable/12: share/man/man9 sys/arm/broadcom/bcm2835 sys/arm/freescale/imx sys/arm/ti sys/dev/glxiic sys/dev/ichsmb sys/dev/ow sys/kern sys/sys Message-ID: <201912221930.xBMJUDtM035466@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: ian Date: Sun Dec 22 19:30:13 2019 New Revision: 356022 URL: https://svnweb.freebsd.org/changeset/base/356022 Log: MFC r355598, r355727, r355735, r355858 r355598: Do not attach children of owc_gpiobus until interrupts are working. The children of the bus need to do IO on the bus to probe for hardware presence. Doing IO means timing the bus states using sbinuptime(), and that requires working timecounters, which are not initialized until after device attachment has completed. PR: 242526 r355727 (by imp): Create new wrapper function: bus_delayed_attach_children() Delay the attachment of children, when requested, until after interrutps are running. This is often needed to allow children to run transactions on i2c or spi busses. It's a common enough idiom that it will be useful to have its own wrapper. Reviewed by: ian Differential Revision: https://reviews.freebsd.org/D21465 r355735 (by imp): Be consistent about checking return value from bus_delayed_attach_children. Most places checked, but a couple last minute changes didn't. Make them all use the return value. Noticed by: rpokala@ r355858: Update owc_gpiobus (one-wire over gpio) to the modern gpio_pin interface. It used to be required that a device be a child of gpiobus(4) to manipulate gpio pins. That requirement didn't work well for FDT-based systems with many cross-hierarchy users of gpio, so a more modern framework was created that removed the old hierarchy requirement. These changes adapt the owc_gpiobus driver to use the newer gpio_pin_* functions to acquire, release, and manipulate gpio pins. This allows a single driver to work for both hinted-attachment and fdt-based systems, and removes the requirement that any one-wire fdt nodes must appear at the root of the devicetree. Differential Revision: https://reviews.freebsd.org/D22710 Added: stable/12/share/man/man9/bus_delayed_attach_children.9 - copied unchanged from r355727, head/share/man/man9/bus_delayed_attach_children.9 Modified: stable/12/share/man/man9/Makefile stable/12/sys/arm/broadcom/bcm2835/bcm2835_bsc.c stable/12/sys/arm/freescale/imx/imx_i2c.c stable/12/sys/arm/freescale/imx/imx_spi.c stable/12/sys/arm/ti/ti_i2c.c stable/12/sys/dev/glxiic/glxiic.c stable/12/sys/dev/ichsmb/ichsmb.c stable/12/sys/dev/ow/owc_gpiobus.c stable/12/sys/kern/subr_bus.c stable/12/sys/sys/bus.h Directory Properties: stable/12/ (props changed) Modified: stable/12/share/man/man9/Makefile ============================================================================== --- stable/12/share/man/man9/Makefile Sun Dec 22 19:14:33 2019 (r356021) +++ stable/12/share/man/man9/Makefile Sun Dec 22 19:30:13 2019 (r356022) @@ -36,6 +36,7 @@ MAN= accept_filter.9 \ BUS_CHILD_DELETED.9 \ BUS_CHILD_DETACHED.9 \ BUS_CONFIG_INTR.9 \ + bus_delayed_attach_children.9 \ BUS_DESCRIBE_INTR.9 \ bus_dma.9 \ bus_generic_attach.9 \ Copied: stable/12/share/man/man9/bus_delayed_attach_children.9 (from r355727, head/share/man/man9/bus_delayed_attach_children.9) ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ stable/12/share/man/man9/bus_delayed_attach_children.9 Sun Dec 22 19:30:13 2019 (r356022, copy of r355727, head/share/man/man9/bus_delayed_attach_children.9) @@ -0,0 +1,55 @@ +.\" -*- nroff -*- +.\" +.\" Copyright (c) 2019 M. Warner Losh +.\" +.\" Redistribution and use in source and binary forms, with or without +.\" modification, are permitted provided that the following conditions +.\" are met: +.\" 1. Redistributions of source code must retain the above copyright +.\" notice, this list of conditions and the following disclaimer. +.\" 2. Redistributions in binary form must reproduce the above copyright +.\" notice, this list of conditions and the following disclaimer in the +.\" documentation and/or other materials provided with the distribution. +.\" +.\" THIS SOFTWARE IS PROVIDED BY THE DEVELOPERS ``AS IS'' AND ANY EXPRESS OR +.\" IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES +.\" OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. +.\" IN NO EVENT SHALL THE DEVELOPERS BE LIABLE FOR ANY DIRECT, INDIRECT, +.\" INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT +.\" NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +.\" DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +.\" THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +.\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF +.\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +.\" +.\" $FreeBSD$ +.\" +.Dd August 29, 2019 +.Dt BUS_DELAYED_ATTACH_CHILDREN 9 +.Os +.Sh NAME +.Nm bus_delayed_attach_children +.Nd "request that the children be attached when interrupts are enabled" +.Sh SYNOPSIS +.In sys/param.h +.In sys/bus.h +.Pp +.Ft int +.Fn bus_delayed_attach_children "device_t dev" +.Sh DESCRIPTION +The +.Fn bus_delayed_attach_children +function requests that the children of this device +be attached when interrupts are running. +If interrupts are currently running, this happens immediately. +If interrupts aren't yet running, this happens after interrupts are enabled, but +before the system mounts root. +.Sh RETURN VALUES +A zero return value indicates success. +.Sh SEE ALSO +.Xr bus 9 , +.Xr device 9 , +.Xr driver 9 +.Sh AUTHORS +This manual page was written by +.An Warner Losh Aq Mt imp@FreeBSD.org . Modified: stable/12/sys/arm/broadcom/bcm2835/bcm2835_bsc.c ============================================================================== --- stable/12/sys/arm/broadcom/bcm2835/bcm2835_bsc.c Sun Dec 22 19:14:33 2019 (r356021) +++ stable/12/sys/arm/broadcom/bcm2835/bcm2835_bsc.c Sun Dec 22 19:30:13 2019 (r356022) @@ -347,9 +347,7 @@ bcm_bsc_attach(device_t dev) } /* Probe and attach the iicbus when interrupts are available. */ - config_intrhook_oneshot((ich_func_t)bus_generic_attach, dev); - - return (0); + return (bus_delayed_attach_children(dev)); } static int Modified: stable/12/sys/arm/freescale/imx/imx_i2c.c ============================================================================== --- stable/12/sys/arm/freescale/imx/imx_i2c.c Sun Dec 22 19:14:33 2019 (r356021) +++ stable/12/sys/arm/freescale/imx/imx_i2c.c Sun Dec 22 19:30:13 2019 (r356022) @@ -449,8 +449,7 @@ no_recovery: /* We don't do a hardware reset here because iicbus_attach() does it. */ /* Probe and attach the iicbus when interrupts are available. */ - config_intrhook_oneshot((ich_func_t)bus_generic_attach, dev); - return (0); + return (bus_delayed_attach_children(dev)); } static int Modified: stable/12/sys/arm/freescale/imx/imx_spi.c ============================================================================== --- stable/12/sys/arm/freescale/imx/imx_spi.c Sun Dec 22 19:14:33 2019 (r356021) +++ stable/12/sys/arm/freescale/imx/imx_spi.c Sun Dec 22 19:30:13 2019 (r356022) @@ -566,9 +566,7 @@ spi_attach(device_t dev) * their attach. We can't do IO until timers and interrupts are working. */ sc->spibus = device_add_child(dev, "spibus", -1); - config_intrhook_oneshot((ich_func_t)bus_generic_attach, dev); - - return (0); + return (bus_delayed_attach_children(dev)); } static int Modified: stable/12/sys/arm/ti/ti_i2c.c ============================================================================== --- stable/12/sys/arm/ti/ti_i2c.c Sun Dec 22 19:14:33 2019 (r356021) +++ stable/12/sys/arm/ti/ti_i2c.c Sun Dec 22 19:30:13 2019 (r356022) @@ -905,7 +905,7 @@ ti_i2c_attach(device_t dev) } /* Probe and attach the iicbus when interrupts are available. */ - config_intrhook_oneshot((ich_func_t)bus_generic_attach, dev); + err = bus_delayed_attach_children(dev); out: if (err) { Modified: stable/12/sys/dev/glxiic/glxiic.c ============================================================================== --- stable/12/sys/dev/glxiic/glxiic.c Sun Dec 22 19:14:33 2019 (r356021) +++ stable/12/sys/dev/glxiic/glxiic.c Sun Dec 22 19:30:13 2019 (r356022) @@ -411,8 +411,7 @@ glxiic_attach(device_t dev) glxiic_smb_enable(sc, IIC_FASTEST, 0); /* Probe and attach the iicbus when interrupts are available. */ - config_intrhook_oneshot((ich_func_t)bus_generic_attach, dev); - error = 0; + error = bus_delayed_attach_children(dev); out: if (error != 0) { Modified: stable/12/sys/dev/ichsmb/ichsmb.c ============================================================================== --- stable/12/sys/dev/ichsmb/ichsmb.c Sun Dec 22 19:14:33 2019 (r356021) +++ stable/12/sys/dev/ichsmb/ichsmb.c Sun Dec 22 19:30:13 2019 (r356022) @@ -131,11 +131,8 @@ ichsmb_attach(device_t dev) goto fail; } - /* Probe and attach the smbus when interrupts are available */ - config_intrhook_oneshot((ich_func_t)bus_generic_attach, dev); - - return (0); - + /* Attach children when interrupts are available */ + return (bus_delayed_attach_children(dev)); fail: mtx_destroy(&sc->mutex); return (error); Modified: stable/12/sys/dev/ow/owc_gpiobus.c ============================================================================== --- stable/12/sys/dev/ow/owc_gpiobus.c Sun Dec 22 19:14:33 2019 (r356021) +++ stable/12/sys/dev/ow/owc_gpiobus.c Sun Dec 22 19:30:13 2019 (r356022) @@ -39,17 +39,21 @@ __FBSDID("$FreeBSD$"); #include <sys/module.h> #include <sys/mutex.h> +#include <dev/gpio/gpiobusvar.h> +#include <dev/ow/owll.h> + #ifdef FDT -#include <dev/fdt/fdt_common.h> #include <dev/ofw/ofw_bus.h> #include <dev/ofw/ofw_bus_subr.h> -#endif -#include <dev/gpio/gpiobusvar.h> -#include "gpiobus_if.h" +static struct ofw_compat_data compat_data[] = { + {"w1-gpio", true}, + {NULL, false} +}; +OFWBUS_PNP_INFO(compat_data); +SIMPLEBUS_PNP_INFO(compat_data); +#endif /* FDT */ -#include <dev/ow/owll.h> - #define OW_PIN 0 #define OWC_GPIOBUS_LOCK(_sc) mtx_lock(&(_sc)->sc_mtx) @@ -62,7 +66,7 @@ __FBSDID("$FreeBSD$"); struct owc_gpiobus_softc { device_t sc_dev; - device_t sc_busdev; + gpio_pin_t sc_pin; struct mtx sc_mtx; }; @@ -70,81 +74,86 @@ static int owc_gpiobus_probe(device_t); static int owc_gpiobus_attach(device_t); static int owc_gpiobus_detach(device_t); -#ifdef FDT -static void -owc_gpiobus_identify(driver_t *driver, device_t bus) +static int +owc_gpiobus_probe(device_t dev) { - phandle_t w1, root; + int rv; /* - * Find all the 1-wire bus pseudo-nodes that are - * at the top level of the FDT. Would be nice to - * somehow preserve the node name of these busses, - * but there's no good place to put it. The driver's - * name is used for the device name, and the 1-wire - * bus overwrites the description. + * By default we only bid to attach if specifically added by our parent + * (usually via hint.owc_gpiobus.#.at=busname). On FDT systems we bid + * as the default driver based on being configured in the FDT data. */ - root = OF_finddevice("/"); - if (root == -1) - return; - for (w1 = OF_child(root); w1 != 0; w1 = OF_peer(w1)) { - if (!fdt_is_compatible_strict(w1, "w1-gpio")) - continue; - if (!OF_hasprop(w1, "gpios")) - continue; - ofw_gpiobus_add_fdt_child(bus, driver->name, w1); - } -} -#endif + rv = BUS_PROBE_NOWILDCARD; -static int -owc_gpiobus_probe(device_t dev) -{ #ifdef FDT - if (!ofw_bus_status_okay(dev)) - return (ENXIO); + if (ofw_bus_status_okay(dev) && + ofw_bus_search_compatible(dev, compat_data)->ocd_data) + rv = BUS_PROBE_DEFAULT; +#endif - if (ofw_bus_is_compatible(dev, "w1-gpio")) { - device_set_desc(dev, "FDT GPIO attached one-wire bus"); - return (BUS_PROBE_DEFAULT); - } + device_set_desc(dev, "GPIO one-wire bus"); - return (ENXIO); -#else - device_set_desc(dev, "GPIO attached one-wire bus"); - return 0; -#endif + return (rv); } static int owc_gpiobus_attach(device_t dev) { struct owc_gpiobus_softc *sc; - device_t *kids; - int nkid; + int err; sc = device_get_softc(dev); sc->sc_dev = dev; - sc->sc_busdev = device_get_parent(dev); + +#ifdef FDT + /* Try to configure our pin from fdt data on fdt-based systems. */ + err = gpio_pin_get_by_ofw_idx(dev, ofw_bus_get_node(dev), OW_PIN, + &sc->sc_pin); +#else + err = ENOENT; +#endif + /* + * If we didn't get configured by fdt data and our parent is gpiobus, + * see if we can be configured by the bus (allows hinted attachment even + * on fdt-based systems). + */ + if (err != 0 && + strcmp("gpiobus", device_get_name(device_get_parent(dev))) == 0) + err = gpio_pin_get_by_child_index(dev, OW_PIN, &sc->sc_pin); + + /* If we didn't get configured by either method, whine and punt. */ + if (err != 0) { + device_printf(sc->sc_dev, + "cannot acquire gpio pin (config error)\n"); + return (err); + } + OWC_GPIOBUS_LOCK_INIT(sc); - nkid = 0; - if (device_get_children(dev, &kids, &nkid) == 0) - free(kids, M_TEMP); - if (nkid == 0) - device_add_child(dev, "ow", -1); - bus_generic_attach(dev); - return (0); + /* + * Add the ow bus as a child, but defer probing and attaching it until + * interrupts work, because we can't do IO for them until we can read + * the system timecounter (which initializes after device attachments). + */ + device_add_child(sc->sc_dev, "ow", -1); + return (bus_delayed_attach_children(dev)); } static int owc_gpiobus_detach(device_t dev) { struct owc_gpiobus_softc *sc; + int err; sc = device_get_softc(dev); + + if ((err = device_delete_children(dev)) != 0) + return (err); + + gpio_pin_release(sc->sc_pin); OWC_GPIOBUS_LOCK_DESTROY(sc); - bus_generic_detach(dev); + return (0); } @@ -157,18 +166,10 @@ owc_gpiobus_detach(device_t dev) * These macros let what why we're doing stuff shine in the code * below, and let the how be confined to here. */ -#define GETBUS(sc) GPIOBUS_ACQUIRE_BUS((sc)->sc_busdev, \ - (sc)->sc_dev, GPIOBUS_WAIT) -#define RELBUS(sc) GPIOBUS_RELEASE_BUS((sc)->sc_busdev, \ - (sc)->sc_dev) -#define OUTPIN(sc) GPIOBUS_PIN_SETFLAGS((sc)->sc_busdev, \ - (sc)->sc_dev, OW_PIN, GPIO_PIN_OUTPUT) -#define INPIN(sc) GPIOBUS_PIN_SETFLAGS((sc)->sc_busdev, \ - (sc)->sc_dev, OW_PIN, GPIO_PIN_INPUT) -#define GETPIN(sc, bit) GPIOBUS_PIN_GET((sc)->sc_busdev, \ - (sc)->sc_dev, OW_PIN, bit) -#define LOW(sc) GPIOBUS_PIN_SET((sc)->sc_busdev, \ - (sc)->sc_dev, OW_PIN, GPIO_PIN_LOW) +#define OUTPIN(sc) gpio_pin_setflags((sc)->sc_pin, GPIO_PIN_OUTPUT) +#define INPIN(sc) gpio_pin_setflags((sc)->sc_pin, GPIO_PIN_INPUT) +#define GETPIN(sc, bp) gpio_pin_is_active((sc)->sc_pin, (bp)) +#define LOW(sc) gpio_pin_set_active((sc)->sc_pin, false) /* * WRITE-ONE (see owll_if.m for timings) From Figure 4-1 AN-937 @@ -186,12 +187,8 @@ static int owc_gpiobus_write_one(device_t dev, struct ow_timing *t) { struct owc_gpiobus_softc *sc; - int error; sc = device_get_softc(dev); - error = GETBUS(sc); - if (error != 0) - return (error); critical_enter(); @@ -206,8 +203,6 @@ owc_gpiobus_write_one(device_t dev, struct ow_timing * critical_exit(); - RELBUS(sc); - return (0); } @@ -227,12 +222,8 @@ static int owc_gpiobus_write_zero(device_t dev, struct ow_timing *t) { struct owc_gpiobus_softc *sc; - int error; sc = device_get_softc(dev); - error = GETBUS(sc); - if (error != 0) - return (error); critical_enter(); @@ -247,8 +238,6 @@ owc_gpiobus_write_zero(device_t dev, struct ow_timing critical_exit(); - RELBUS(sc); - return (0); } @@ -271,13 +260,10 @@ static int owc_gpiobus_read_data(device_t dev, struct ow_timing *t, int *bit) { struct owc_gpiobus_softc *sc; - int error, sample; + bool sample; sbintime_t then, now; sc = device_get_softc(dev); - error = GETBUS(sc); - if (error != 0) - return (error); critical_enter(); @@ -296,7 +282,7 @@ owc_gpiobus_read_data(device_t dev, struct ow_timing * do { now = sbinuptime(); GETPIN(sc, &sample); - } while (now - then < (t->t_rdv + 2) * SBT_1US && sample == 0); + } while (now - then < (t->t_rdv + 2) * SBT_1US && sample == false); critical_exit(); if (now - then < t->t_rdv * SBT_1US) @@ -309,9 +295,7 @@ owc_gpiobus_read_data(device_t dev, struct ow_timing * now = sbinuptime(); } while (now - then < (t->t_slot + t->t_rec) * SBT_1US); - RELBUS(sc); - - return (error); + return (0); } /* @@ -327,32 +311,31 @@ owc_gpiobus_read_data(device_t dev, struct ow_timing * * |<tPDH>| * * Note: for Regular Speed operations, tRSTL + tR should be less than 960us to - * avoid interferring with other devices on the bus + * avoid interfering with other devices on the bus. + * + * Return values in *bit: + * -1 = Bus wiring error (stuck low). + * 0 = no presence pulse + * 1 = presence pulse detected */ static int owc_gpiobus_reset_and_presence(device_t dev, struct ow_timing *t, int *bit) { struct owc_gpiobus_softc *sc; - int error; - int buf = -1; + bool sample; sc = device_get_softc(dev); - error = GETBUS(sc); - if (error != 0) - return (error); /* * Read the current state of the bus. The steady state of an idle bus is * high. Badly wired buses that are missing the required pull up, or * that have a short circuit to ground cause all kinds of mischief when - * we try to read them later. Return EIO and release the bus if the bus - * is currently low. + * we try to read them later. Return EIO if the bus is currently low. */ INPIN(sc); - GETPIN(sc, &buf); - if (buf == 0) { + GETPIN(sc, &sample); + if (sample == false) { *bit = -1; - RELBUS(sc); return (EIO); } @@ -368,8 +351,8 @@ owc_gpiobus_reset_and_presence(device_t dev, struct ow DELAY(t->t_pdh + t->t_pdl / 2); /* Read presence pulse */ - GETPIN(sc, &buf); - *bit = !!buf; + GETPIN(sc, &sample); + *bit = sample; critical_exit(); @@ -380,15 +363,12 @@ owc_gpiobus_reset_and_presence(device_t dev, struct ow * window. It should return to high. If it is low, then we have some * problem and should abort the reset. */ - GETPIN(sc, &buf); - if (buf == 0) { + GETPIN(sc, &sample); + if (sample == false) { *bit = -1; - RELBUS(sc); return (EIO); } - RELBUS(sc); - return (0); } @@ -396,9 +376,6 @@ static devclass_t owc_gpiobus_devclass; static device_method_t owc_gpiobus_methods[] = { /* Device interface */ -#ifdef FDT - DEVMETHOD(device_identify, owc_gpiobus_identify), -#endif DEVMETHOD(device_probe, owc_gpiobus_probe), DEVMETHOD(device_attach, owc_gpiobus_attach), DEVMETHOD(device_detach, owc_gpiobus_detach), @@ -415,6 +392,10 @@ static driver_t owc_gpiobus_driver = { owc_gpiobus_methods, sizeof(struct owc_gpiobus_softc), }; + +#ifdef FDT +DRIVER_MODULE(owc_gpiobus, simplebus, owc_gpiobus_driver, owc_gpiobus_devclass, 0, 0); +#endif DRIVER_MODULE(owc_gpiobus, gpiobus, owc_gpiobus_driver, owc_gpiobus_devclass, 0, 0); MODULE_DEPEND(owc_gpiobus, ow, 1, 1, 1); Modified: stable/12/sys/kern/subr_bus.c ============================================================================== --- stable/12/sys/kern/subr_bus.c Sun Dec 22 19:14:33 2019 (r356021) +++ stable/12/sys/kern/subr_bus.c Sun Dec 22 19:30:13 2019 (r356022) @@ -3725,6 +3725,22 @@ bus_generic_attach(device_t dev) } /** + * @brief Helper function for delaying attaching children + * + * Many buses can't run transactions on the bus which children need to probe and + * attach until after interrupts and/or timers are running. This function + * delays their attach until interrupts and timers are enabled. + */ +int +bus_delayed_attach_children(device_t dev) +{ + /* Probe and attach the bus children when interrupts are available */ + config_intrhook_oneshot((ich_func_t)bus_generic_attach, dev); + + return (0); +} + +/** * @brief Helper function for implementing DEVICE_DETACH() * * This function can be used to help implement the DEVICE_DETACH() for Modified: stable/12/sys/sys/bus.h ============================================================================== --- stable/12/sys/sys/bus.h Sun Dec 22 19:14:33 2019 (r356021) +++ stable/12/sys/sys/bus.h Sun Dec 22 19:30:13 2019 (r356022) @@ -563,6 +563,7 @@ int bus_child_present(device_t child); int bus_child_pnpinfo_str(device_t child, char *buf, size_t buflen); int bus_child_location_str(device_t child, char *buf, size_t buflen); void bus_enumerate_hinted_children(device_t bus); +int bus_delayed_attach_children(device_t bus); static __inline struct resource * bus_alloc_resource_any(device_t dev, int type, int *rid, u_int flags)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201912221930.xBMJUDtM035466>