Date: Wed, 3 Jul 2013 10:19:12 -0300 From: Luiz Otavio O Souza <luiz.souza@ad.com.br> To: Warner Losh <imp@bsdimp.com> Cc: freebsd-arch@FreeBSD.org Subject: Re: FDT Support for GPIO (gpiobus and friends) Message-ID: <B18180D6-5AA5-41A4-BF6B-3ACA09E9C544@ad.com.br> In-Reply-To: <1142ABEB-7FDA-4CFE-9D12-F8FD2D4C85D6@bsdimp.com> References: <BEB9A0F8-560B-4937-8707-653988A26D85@gmail.com> <20121205.060056.592894859995638978.hrs@allbsd.org> <B97B1170-69AD-4AA2-A111-1B9539C71BC3@gmail.com> <04AEF097-025D-4B3F-A345-98878AE4A822@ad.com.br> <1142ABEB-7FDA-4CFE-9D12-F8FD2D4C85D6@bsdimp
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On Jun 24, 2013, at 11:52 AM, Warner Losh wrote:
> I'm loving this patch, mostly.
>
> But I don't see where the ivar gets freed... Perhaps this is because we dont' support detaching GPIOs?
>
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
[-- Attachment #2 --]
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(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 <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(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 <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(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),
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?B18180D6-5AA5-41A4-BF6B-3ACA09E9C544>