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
--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 <loos.br@gmail.com> wrote
> in <BEB9A0F8-560B-4937-8707-653988A26D85@gmail.com>:
>=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 <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),
--Apple-Mail-11-552019434--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?B97B1170-69AD-4AA2-A111-1B9539C71BC3>