From owner-freebsd-arch@FreeBSD.ORG Fri Jun 21 21:05:37 2013 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 4D462A51; Fri, 21 Jun 2013 21:05:37 +0000 (UTC) (envelope-from loos.br@gmail.com) Received: from mail-qa0-x236.google.com (mail-qa0-x236.google.com [IPv6:2607:f8b0:400d:c00::236]) by mx1.freebsd.org (Postfix) with ESMTP id EF37F1619; Fri, 21 Jun 2013 21:05:36 +0000 (UTC) Received: by mail-qa0-f54.google.com with SMTP id n20so926218qaj.6 for ; Fri, 21 Jun 2013 14:05:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:mime-version:content-type:from:in-reply-to:date:cc :message-id:references:to:x-mailer; bh=Dy2AOAZd2LMYahtP5nNxAYOAR7q5dBXdpzajrrPc9z8=; b=r1Zh4Q3IZR4l4G2e98tzkd5aDMr908tbGZXO6HE79dtLls6oh0vBHxC11prVrD1HXN jOfDeW09V1R/kAJ3jwbi5Sh4zuY4fI32vuSkV9y1aTMNhxPDvnCYx66uRixztstrB94m A8Wun2gO/rtnGuTnyfRrBF11pTUa+u8//XR8/NwkWnBfCuS49XL9IIijEqz2D1vQRla8 BcR4EXoXxR3uCaexc9/Je+YHKuAWzZ3wMnaKCChrPgAkCe6UDcx9JbFGvJ56sGjfV/6f G+W+YuP1FYgV8WJ0ybKhhRoB/vy/ljXrzTYKnxL6ZJ+U7TT9aVFU0QHUR/sEreTHwIs5 pkOg== X-Received: by 10.49.60.169 with SMTP id i9mr7266669qer.93.1371848736307; Fri, 21 Jun 2013 14:05:36 -0700 (PDT) Received: from [10.1.10.62] ([187.120.137.162]) by mx.google.com with ESMTPSA id s8sm8649681qat.4.2013.06.21.14.05.33 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 21 Jun 2013 14:05:35 -0700 (PDT) Subject: Re: FDT Support for GPIO (gpiobus and friends) Mime-Version: 1.0 (Apple Message framework v1085) Content-Type: multipart/mixed; boundary=Apple-Mail-11-552019434 From: Luiz Otavio O Souza In-Reply-To: <20121205.060056.592894859995638978.hrs@allbsd.org> Date: Fri, 21 Jun 2013 18:05:26 -0300 Message-Id: References: <20121205.060056.592894859995638978.hrs@allbsd.org> To: Hiroki Sato X-Mailer: Apple Mail (2.1085) 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: Fri, 21 Jun 2013 21:05:37 -0000 --Apple-Mail-11-552019434 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii On Dec 4, 2012, at 7:00 PM, Hiroki Sato wrote: > Luiz Otavio O Souza wrote > in : >=20 > lo> Hi, > lo> > lo> I've been playing with GPIO on RPi and found the missing support = of > lo> FDT on gpiobus very annoying. > lo> > lo> The following patch (gpio-fdt.diff) adds FDT support to GPIO = (gpiobus, > lo> gpioc, gpioled). > lo> > lo> The bcm2835_gpio.c.fdt.diff will add (a better) support of FDT on = RPi > lo> GPIO controller and the bcm2835.dts.diff has my changes on the RPi = dts > lo> for adding support of gpioled on 'ok' led (pin 16). > lo> > lo> Comments ? >=20 > I like this idea, but it should be consistent with standard device > tree bindings. For example, >=20 > + gpiobus { > + compatible =3D "gpiobus"; > + > + /* Ok led */ > + led { > + compatible =3D "gpioled"; > + label =3D "ok"; > + pins =3D <16>; > + }; > + }; >=20 > should be something like the following: >=20 > led { > compatible =3D "gpio-leds"; > ok { > gpios =3D <&foo 16 1>; > label =3D "ok"; > }; > }; >=20 > A node using GPIOs must have gpios property for the gpio-controller > node and pin number. >=20 > -- Hiroki Sorry for the long delay. And yes, i should have read the bindings-gpio.txt document first :) Is the attached patch better ? It implements the gpios property decoding = for gpioled (and could be used as a template for others gpio consumers) = and now it works (almost) out of box with the standard rpi dts file. The only change on the rpi dts file was to add the flags field and mark = the pin led as an output (even if this is unused by the current code - a = gpioled pin is always an output). This was necessary to match the format = documented on bindings-gpio.txt. Luiz --Apple-Mail-11-552019434 Content-Disposition: attachment; filename=bcm2835-rpi-b.diff Content-Type: application/octet-stream; name="bcm2835-rpi-b.diff" Content-Transfer-Encoding: 7bit Index: boot/fdt/dts/bcm2835-rpi-b.dts =================================================================== --- boot/fdt/dts/bcm2835-rpi-b.dts (revision 251700) +++ boot/fdt/dts/bcm2835-rpi-b.dts (working copy) @@ -518,7 +518,7 @@ ok { label = "ok"; - gpios = <&gpio 16 1>; + gpios = <&gpio 16 2 0>; /* Don't change this - it configures * how the led driver determines if --Apple-Mail-11-552019434 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(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, NULL, -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)); } @@ -445,6 +580,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 +619,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(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(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-11-552019434--