From owner-freebsd-arch@FreeBSD.ORG Wed Jul 3 13:19:29 2013 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 8798923F for ; Wed, 3 Jul 2013 13:19:29 +0000 (UTC) (envelope-from luiz.souza@ad.com.br) Received: from tx2outboundpool.messaging.microsoft.com (tx2ehsobe001.messaging.microsoft.com [65.55.88.11]) by mx1.freebsd.org (Postfix) with ESMTP id 4A21D1C2D for ; Wed, 3 Jul 2013 13:19:28 +0000 (UTC) Received: from mail139-tx2-R.bigfish.com (10.9.14.254) by TX2EHSOBE014.bigfish.com (10.9.40.34) with Microsoft SMTP Server id 14.1.225.23; Wed, 3 Jul 2013 13:19:21 +0000 Received: from mail139-tx2 (localhost [127.0.0.1]) by mail139-tx2-R.bigfish.com (Postfix) with ESMTP id 3D42B600AF; Wed, 3 Jul 2013 13:19:21 +0000 (UTC) X-Forefront-Antispam-Report: CIP:207.46.4.203; KIP:(null); UIP:(null); IPV:NLI; H:SN2PRD8002HT001.lamprd80.prod.outlook.com; RD:none; EFVD:NLI X-SpamScore: -4 X-BigFish: PS-4(zz98dI9371Ic85fh148cI1432Ide40hzz1f42h1ee6h1de0h1d18h1fdah2073h1202h1e76h1d1ah1d2ah1fc6hzzz2fh2a8h668h839hd25he5bhf0ah1288h12a5h12bdh137ah139eh1441h1504h1537h162dh1631h1662h1758h1898h18e1h1946h19b5h19ceh1ad9h1b0ah1bceh1d0ch1d2eh1d3fh1dfeh1dffh1e23h34h1155h) Received-SPF: pass (mail139-tx2: domain of ad.com.br designates 207.46.4.203 as permitted sender) client-ip=207.46.4.203; envelope-from=luiz.souza@ad.com.br; helo=SN2PRD8002HT001.lamprd80.prod.outlook.com ; .outlook.com ; Received: from mail139-tx2 (localhost.localdomain [127.0.0.1]) by mail139-tx2 (MessageSwitch) id 1372857559181984_22565; Wed, 3 Jul 2013 13:19:19 +0000 (UTC) Received: from TX2EHSMHS018.bigfish.com (unknown [10.9.14.246]) by mail139-tx2.bigfish.com (Postfix) with ESMTP id 1E5AD160CB3; Wed, 3 Jul 2013 13:19:19 +0000 (UTC) Received: from SN2PRD8002HT001.lamprd80.prod.outlook.com (207.46.4.203) by TX2EHSMHS018.bigfish.com (10.9.99.118) with Microsoft SMTP Server (TLS) id 14.1.225.23; Wed, 3 Jul 2013 13:19:18 +0000 Received: from [10.10.1.8] (201.72.203.67) by pod51028.outlook.com (10.27.50.185) with Microsoft SMTP Server (TLS) id 14.15.129.14; Wed, 3 Jul 2013 13:19:15 +0000 Subject: Re: FDT Support for GPIO (gpiobus and friends) MIME-Version: 1.0 (Apple Message framework v1085) Content-Type: multipart/mixed; boundary="Apple-Mail-13--586638903" From: Luiz Otavio O Souza In-Reply-To: <1142ABEB-7FDA-4CFE-9D12-F8FD2D4C85D6@bsdimp.com> Date: Wed, 3 Jul 2013 10:19:12 -0300 Message-ID: References: <20121205.060056.592894859995638978.hrs@allbsd.org> <04AEF097-025D-4B3F-A345-98878AE4A822@ad.com.br> <1142ABEB-7FDA-4CFE-9D12-F8FD2D4C85D6@bsdimp To: Warner Losh X-Mailer: Apple Mail (2.1085) X-Originating-IP: [201.72.203.67] X-OriginatorOrg: ad.com.br Cc: freebsd-arch@FreeBSD.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Jul 2013 13:19:29 -0000 --Apple-Mail-13--586638903 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="us-ascii" On Jun 24, 2013, at 11:52 AM, Warner Losh wrote: > I'm loving this patch, mostly. >=20 > But I don't see where the ivar gets freed... Perhaps this is because = we dont' support detaching GPIOs? >=20 No, you are right. Even if most (if not all) gpio drivers don't allow = the detach, gpiobus can be built as a module and as such it has a detach = routine which IMO should do the right thing. Thanks for noticing this. Here is a patch with a fix (i hope it's correct). Luiz --Apple-Mail-13--586638903 Content-Disposition: attachment; filename="gpioled-fdt.diff" Content-Type: application/octet-stream; name="gpioled-fdt.diff" Content-Transfer-Encoding: 7bit Index: dev/gpio/gpiobus.c =================================================================== --- dev/gpio/gpiobus.c (revision 251700) +++ dev/gpio/gpiobus.c (working copy) @@ -27,6 +27,8 @@ #include __FBSDID("$FreeBSD$"); +#include "opt_platform.h" + #include #include #include @@ -41,6 +43,12 @@ #include #include +#ifdef FDT +#include +#include +#include +#endif + #include #include #include "gpio_if.h" @@ -83,6 +91,130 @@ #define GPIOBUS_ASSERT_UNLOCKED(_sc) mtx_assert(&_sc->sc_mtx, MA_NOTOWNED); +#ifdef FDT +static int +gpiobus_fdt_parse_pins(device_t dev) +{ + struct gpiobus_ivar *devi; + struct gpiobus_softc *sc; + int i, len; + pcell_t *gpios; + phandle_t gpio, node; + + /* Retrieve the FDT node and check for gpios property. */ + node = ofw_bus_get_node(dev); + if ((len = OF_getproplen(node, "gpios")) < 0) + return (EINVAL); + + /* Retrieve the gpios property. */ + gpios = malloc(len, M_DEVBUF, M_NOWAIT | M_ZERO); + if (gpios == NULL) + return (ENOMEM); + if (OF_getprop(node, "gpios", gpios, len) < 0) { + free(gpios, M_DEVBUF); + return (EINVAL); + } + + /* + * The OF_getprop() is returning 4 pcells. + * The first one is the GPIO controller phandler. + * The last three are GPIO pin, GPIO pin direction and GPIO pin flags. + */ + if ((len / sizeof(pcell_t)) % 4) { + free(gpios, M_DEVBUF); + return (EINVAL); + } + devi = GPIOBUS_IVAR(dev); + devi->npins = len / (sizeof(pcell_t) * 4); + devi->pins = malloc(sizeof(uint32_t) * devi->npins, M_DEVBUF, + M_NOWAIT | M_ZERO); + if (devi->pins == NULL) { + free(gpios, M_DEVBUF); + return (ENOMEM); + } + + for (i = 0; i < devi->npins; i++) { + + /* Verify if we're attaching to the correct gpio controller. */ + gpio = OF_instance_to_package(fdt32_to_cpu(gpios[i * 4 + 0])); + if (!OF_hasprop(gpio, "gpio-controller") || + gpio != ofw_bus_get_node(device_get_parent( + device_get_parent(dev)))) { + free(devi->pins, M_DEVBUF); + free(gpios, M_DEVBUF); + return (EINVAL); + } + + /* Attach the child device to gpiobus. */ + sc = device_get_softc(device_get_parent(dev)); + + devi->pins[i] = fdt32_to_cpu(gpios[i * 4 + 1]); + /* (void)gpios[i * 4 + 2] - GPIO pin direction */ + /* (void)gpios[i * 4 + 3] - GPIO pin flags */ + + if (devi->pins[i] > sc->sc_npins) { + device_printf(dev, "invalid pin %d, max: %d\n", + devi->pins[i], sc->sc_npins - 1); + free(devi->pins, M_DEVBUF); + free(gpios, M_DEVBUF); + return (EINVAL); + } + + /* + * Mark pin as mapped and give warning if it's already mapped. + */ + if (sc->sc_pins_mapped[devi->pins[i]]) { + device_printf(dev, + "warning: pin %d is already mapped\n", + devi->pins[i]); + free(devi->pins, M_DEVBUF); + free(gpios, M_DEVBUF); + return (EINVAL); + } + sc->sc_pins_mapped[devi->pins[i]] = 1; + } + + free(gpios, M_DEVBUF); + return (0); +} + +int +gpiobus_fdt_add_child(driver_t *driver, device_t bus, phandle_t childnode) +{ + struct gpiobus_ivar *devi; + device_t child; + + devi = malloc(sizeof(*devi), M_DEVBUF, M_NOWAIT | M_ZERO); + if (devi == NULL) + return (-1); + + if (ofw_bus_gen_setup_devinfo(&devi->ofw, childnode) != 0) { + device_printf(bus, "could not set up devinfo\n"); + free(devi, M_DEVBUF); + return (-1); + } + + /* Add newbus device for the child. */ + child = device_add_child(bus, driver->name, -1); + if (child == NULL) { + device_printf(bus, "could not add child: %s\n", + devi->ofw.obd_name); + /* XXX should unmap */ + ofw_bus_gen_destroy_devinfo(&devi->ofw); + free(devi, M_DEVBUF); + return (-1); + } + device_set_ivars(child, devi); + if (gpiobus_fdt_parse_pins(child) != 0) { + device_delete_child(bus, child); + ofw_bus_gen_destroy_devinfo(&devi->ofw); + free(devi, M_DEVBUF); + return (-1); + } + return (0); +} +#endif + static void gpiobus_print_pins(struct gpiobus_ivar *devi) { @@ -151,6 +283,7 @@ if (i >= sc->sc_npins) { device_printf(child, "invalid pin %d, max: %d\n", i, sc->sc_npins - 1); + free(devi->pins, M_DEVBUF); return (EINVAL); } @@ -161,6 +294,7 @@ if (sc->sc_pins_mapped[i]) { device_printf(child, "warning: pin %d is already mapped\n", i); + free(devi->pins, M_DEVBUF); return (EINVAL); } sc->sc_pins_mapped[i] = 1; @@ -207,6 +341,7 @@ /* * Get parent's pins and mark them as unmapped */ + bus_generic_probe(dev); bus_enumerate_hinted_children(dev); return (bus_generic_attach(dev)); } @@ -218,19 +353,30 @@ static int gpiobus_detach(device_t dev) { - struct gpiobus_softc *sc = GPIOBUS_SOFTC(dev); - int err; + struct gpiobus_softc *sc; + struct gpiobus_ivar *devi; + device_t *devlist; + int i, err, ndevs; + sc = GPIOBUS_SOFTC(dev); KASSERT(mtx_initialized(&sc->sc_mtx), ("gpiobus mutex not initialized")); GPIOBUS_LOCK_DESTROY(sc); if ((err = bus_generic_detach(dev)) != 0) return (err); + if ((err = device_get_children(dev, &devlist, &ndevs)) != 0) + return (err); + for (i = 0; i < ndevs; i++) { + device_delete_child(dev, devlist[i]); + devi = GPIOBUS_IVAR(devlist[i]); + if (devi->pins) { + free(devi->pins, M_DEVBUF); + devi->pins = NULL; + } + } + free(devlist, M_TEMP); - /* detach and delete all children */ - device_delete_children(dev); - if (sc->sc_pins_mapped) { free(sc->sc_pins_mapped, M_DEVBUF); sc->sc_pins_mapped = NULL; @@ -445,6 +591,17 @@ return GPIO_PIN_TOGGLE(sc->sc_dev, devi->pins[pin]); } +#ifdef FDT +static const struct ofw_bus_devinfo * +gpiobus_get_devinfo(device_t bus, device_t child) +{ + struct gpiobus_ivar *devi; + + devi = device_get_ivars(child); + return (&devi->ofw); +} +#endif + static device_method_t gpiobus_methods[] = { /* Device interface */ DEVMETHOD(device_probe, gpiobus_probe), @@ -473,6 +630,16 @@ DEVMETHOD(gpiobus_pin_set, gpiobus_pin_set), DEVMETHOD(gpiobus_pin_toggle, gpiobus_pin_toggle), +#ifdef FDT + /* OFW bus interface */ + DEVMETHOD(ofw_bus_get_devinfo, gpiobus_get_devinfo), + DEVMETHOD(ofw_bus_get_compat, ofw_bus_gen_get_compat), + DEVMETHOD(ofw_bus_get_model, ofw_bus_gen_get_model), + DEVMETHOD(ofw_bus_get_name, ofw_bus_gen_get_name), + DEVMETHOD(ofw_bus_get_node, ofw_bus_gen_get_node), + DEVMETHOD(ofw_bus_get_type, ofw_bus_gen_get_type), +#endif + DEVMETHOD_END }; Index: dev/gpio/gpiobusvar.h =================================================================== --- dev/gpio/gpiobusvar.h (revision 251700) +++ dev/gpio/gpiobusvar.h (working copy) @@ -30,10 +30,16 @@ #ifndef __GPIOBUS_H__ #define __GPIOBUS_H__ +#include "opt_platform.h" + #include #include #include +#ifdef FDT +#include +#endif + #define GPIOBUS_IVAR(d) (struct gpiobus_ivar *) device_get_ivars(d) #define GPIOBUS_SOFTC(d) (struct gpiobus_softc *) device_get_softc(d) @@ -50,8 +56,15 @@ struct gpiobus_ivar { +#ifdef FDT + struct ofw_bus_devinfo ofw; /* FDT device info */ +#endif uint32_t npins; /* pins total */ uint32_t *pins; /* pins map */ }; +#ifdef FDT +int gpiobus_fdt_add_child(driver_t *, device_t, phandle_t); +#endif + #endif /* __GPIOBUS_H__ */ Index: dev/gpio/gpioled.c =================================================================== --- dev/gpio/gpioled.c (revision 251700) +++ dev/gpio/gpioled.c (working copy) @@ -27,6 +27,8 @@ #include __FBSDID("$FreeBSD$"); +#include "opt_platform.h" + #include #include #include @@ -43,11 +45,20 @@ #include #include "gpiobus_if.h" +#ifdef FDT +#include +#include +#include +#include +#endif + /* * Only one pin for led */ #define GPIOLED_PIN 0 +#define GPIOLED_MAXBUF 32 + #define GPIOLED_LOCK(_sc) mtx_lock(&(_sc)->sc_mtx) #define GPIOLED_UNLOCK(_sc) mtx_unlock(&(_sc)->sc_mtx) #define GPIOLED_LOCK_INIT(_sc) \ @@ -68,6 +79,35 @@ static int gpioled_attach(device_t); static int gpioled_detach(device_t); +#ifdef FDT +static void +gpioled_identify(driver_t *driver, device_t bus) +{ + phandle_t child, leds, root; + + root = OF_finddevice("/"); + if (root == 0) + return; + leds = fdt_find_compatible(root, "gpio-leds", 1); + if (leds == 0) + return; + for (child = OF_child(leds); child != 0; child = OF_peer(child)) { + + /* Check and process 'status' property. */ + if (!(fdt_is_enabled(child))) + continue; + + /* Property gpios must exist. */ + if (!OF_hasprop(child, "gpios")) + continue; + + /* Add the gpiobus child. */ + if (gpiobus_fdt_add_child(driver, bus, child) != 0) + continue; + } +} +#endif + static void gpioled_control(void *priv, int onoff) { @@ -93,15 +133,27 @@ gpioled_attach(device_t dev) { struct gpioled_softc *sc; - const char *name; + char *name; +#ifdef FDT + phandle_t node; +#endif sc = device_get_softc(dev); sc->sc_dev = dev; sc->sc_busdev = device_get_parent(dev); GPIOLED_LOCK_INIT(sc); +#ifdef FDT + name = malloc(GPIOLED_MAXBUF + 1, M_DEVBUF, M_NOWAIT | M_ZERO); + if (name == NULL) + return (ENOMEM); + node = ofw_bus_get_node(dev); + if (OF_getprop(node, "label", name, GPIOLED_MAXBUF) == -1) + OF_getprop(node, "name", name, GPIOLED_MAXBUF); +#else if (resource_string_value(device_get_name(dev), device_get_unit(dev), "name", &name)) name = NULL; +#endif GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, GPIOLED_PIN, GPIO_PIN_OUTPUT); @@ -109,6 +161,9 @@ sc->sc_leddev = led_create(gpioled_control, sc, name ? name : device_get_nameunit(dev)); +#ifdef FDT + free(name, M_DEVBUF); +#endif return (0); } @@ -130,6 +185,9 @@ static device_method_t gpioled_methods[] = { /* Device interface */ +#ifdef FDT + DEVMETHOD(device_identify, gpioled_identify), +#endif DEVMETHOD(device_probe, gpioled_probe), DEVMETHOD(device_attach, gpioled_attach), DEVMETHOD(device_detach, gpioled_detach), --Apple-Mail-13--586638903--