Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 24 Jun 2013 09:20:53 -0300
From:      Luiz Otavio O Souza <luiz.souza@ad.com.br>
To:        Luiz Otavio O Souza <loos.br@gmail.com>
Cc:        freebsd-arch@FreeBSD.org
Subject:   Re: FDT Support for GPIO (gpiobus and friends)
Message-ID:  <04AEF097-025D-4B3F-A345-98878AE4A822@ad.com.br>
In-Reply-To: <B97B1170-69AD-4AA2-A111-1B9539C71BC3@gmail.com>
References:  <BEB9A0F8-560B-4937-8707-653988A26D85@gmail.com> <20121205.060056.592894859995638978.hrs@allbsd.org> <B97B1170-69AD-4AA2-A111-1B9539C71BC3@gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
--Apple-Mail-14-779745624
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="us-ascii"

On Jun 21, 2013, at 6:05 PM, Luiz Otavio O Souza wrote:

+int
+gpiobus_fdt_add_child(device_t bus, phandle_t childnode)
+{
[...]
+	/* Add newbus device for the child. */
+	child =3D device_add_child(bus, NULL, -1);


This is obviously wrong.. it should only probe the specified type of =
drivers...

The attached patch fix this problem. Now i can have things like:

Index: bcm2835-rpi-b.dts
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- bcm2835-rpi-b.dts   (revision 251700)
+++ bcm2835-rpi-b.dts   (working copy)
@@ -518,7 +518,7 @@
=20
                ok {
                        label =3D "ok";
-                       gpios =3D <&gpio 16 1>;
+                       gpios =3D <&gpio 16 2 0>;
=20
                        /* Don't change this - it configures
                         * how the led driver determines if
@@ -529,8 +529,24 @@
                        /* This is the real default state. */
                        linux,default-trigger =3D "default-on";
                };
+
+               blue {
+                       label =3D "blue";
+                       gpios =3D <&sr1 3 2 0>;
+               };
        };
=20
+       shift-registers {
+               compatible =3D "gpio-shiftregister";
+
+               sr1: sr1 {
+                       gpios =3D <&gpio 17 2 0
+                                &gpio 21 2 0
+                                &gpio 22 2 0>;
+                       gpio-controller;
+               };
+       };
+
        power: regulator {
                compatible =3D "broadcom,bcm2835-power-mgr", =
"broadcom,bcm2708-power-mgr", "simple-bus";
                #address-cells =3D <1>;


And everything attaches correctly:

gpio0: <BCM2708/2835 GPIO controller> mem 0x20200000-0x202000af irq =
57,59,58,60 on simplebus0
gpio0: read-only pins: 46,47,48,49,50,51,52,53.
gpio0: reserved pins: 48,49,50,51,52,53.
gpioc0: <GPIO controller> on gpio0
gpiobus0: <GPIO bus> on gpio0
gpioled0: <GPIO led> at pin(s) 16 on gpiobus0
gpioshiftreg0: <GPIO Shift-Register expander> at pin(s) 17,21-22 on =
gpiobus0
gpioc1: <GPIO controller> on gpioshiftreg0
gpiobus1: <GPIO bus> on gpioshiftreg0
gpioled1: <GPIO led> at pin(s) 3 on gpiobus1


Regards,
Luiz



--Apple-Mail-14-779745624
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));
 }
@@ -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(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-14-779745624--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?04AEF097-025D-4B3F-A345-98878AE4A822>