Date: Fri, 21 Jun 2013 18:05:26 -0300 From: Luiz Otavio O Souza <loos.br@gmail.com> To: Hiroki Sato <hrs@FreeBSD.org> Cc: freebsd-arch@FreeBSD.org Subject: Re: FDT Support for GPIO (gpiobus and friends) Message-ID: <B97B1170-69AD-4AA2-A111-1B9539C71BC3@gmail.com> In-Reply-To: <20121205.060056.592894859995638978.hrs@allbsd.org> References: <BEB9A0F8-560B-4937-8707-653988A26D85@gmail.com> <20121205.060056.592894859995638978.hrs@allbsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On Dec 4, 2012, at 7:00 PM, Hiroki Sato wrote:
> Luiz Otavio O Souza <loos.br@gmail.com> wrote
> in <BEB9A0F8-560B-4937-8707-653988A26D85@gmail.com>:
>
> 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 ?
>
> I like this idea, but it should be consistent with standard device
> tree bindings. For example,
>
> + gpiobus {
> + compatible = "gpiobus";
> +
> + /* Ok led */
> + led {
> + compatible = "gpioled";
> + label = "ok";
> + pins = <16>;
> + };
> + };
>
> should be something like the following:
>
> led {
> compatible = "gpio-leds";
> ok {
> gpios = <&foo 16 1>;
> label = "ok";
> };
> };
>
> A node using GPIOs must have gpios property for the gpio-controller
> node and pin number.
>
> -- 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
[-- Attachment #2 --]
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
[-- Attachment #3 --]
Index: dev/gpio/gpiobus.c
===================================================================
--- dev/gpio/gpiobus.c (revision 251700)
+++ dev/gpio/gpiobus.c (working copy)
@@ -27,6 +27,8 @@
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
+#include "opt_platform.h"
+
#include <sys/param.h>
#include <sys/systm.h>
#include <sys/malloc.h>
@@ -41,6 +43,12 @@
#include <sys/rman.h>
#include <machine/resource.h>
+#ifdef FDT
+#include <dev/ofw/ofw_bus.h>
+#include <dev/ofw/ofw_bus_subr.h>
+#include <dev/fdt/fdt_common.h>
+#endif
+
#include <sys/gpio.h>
#include <dev/gpio/gpiobusvar.h>
#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 <sys/param.h>
#include <sys/lock.h>
#include <sys/mutex.h>
+#ifdef FDT
+#include <dev/ofw/ofw_bus_subr.h>
+#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 <sys/cdefs.h>
__FBSDID("$FreeBSD$");
+#include "opt_platform.h"
+
#include <sys/param.h>
#include <sys/systm.h>
#include <sys/bio.h>
@@ -43,11 +45,20 @@
#include <sys/gpio.h>
#include "gpiobus_if.h"
+#ifdef FDT
+#include <dev/gpio/gpiobusvar.h>
+#include <dev/ofw/ofw_bus.h>
+#include <dev/ofw/ofw_bus_subr.h>
+#include <dev/fdt/fdt_common.h>
+#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),
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?B97B1170-69AD-4AA2-A111-1B9539C71BC3>