Skip site navigation (1)Skip section navigation (2)
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
--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 <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),

--Apple-Mail-13--586638903--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?B18180D6-5AA5-41A4-BF6B-3ACA09E9C544>