Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 19 Aug 2012 17:17:23 +0900 (JST)
From:      Hiroki Sato <hrs@FreeBSD.org>
To:        gonzo@FreeBSD.org
Cc:        freebsd-arm@FreeBSD.org
Subject:   gpiobus_hinted_child >32 pins support, pin_getname method, and gpio-sysctl bridge patch
Message-ID:  <20120819.171723.523519054460575158.hrs@allbsd.org>

next in thread | raw e-mail | index | archive | help
----Security_Multipart0(Sun_Aug_19_17_17_23_2012_120)--
Content-Type: Multipart/Mixed;
	boundary="--Next_Part(Sun_Aug_19_17_17_23_2012_889)--"
Content-Transfer-Encoding: 7bit

----Next_Part(Sun_Aug_19_17_17_23_2012_889)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hi,

 Can you review the attached patches?  The first one is to add >32 pins
 support for hint.devname.%d.pins and pin_getname method into gpiobus
 similar to gpio interface.  After applying this, the following hint
 will be supported:

 hint.gpioled.0.at="gpiobus0"
 hint.gpioled.0.pins0="0x00000000" # 41
 hint.gpioled.0.pins1="0x00000200"

 hint.gpioled.0.pins can still be used when the pin number is lower
 than 32.  The addition of pin_getname method is to reuse pin name if
 it is already defined in the parent gpio driver.  The maximum of the
 pin number is limited by GPIOBUS_PINS_MAX (currently 128).

 Another one is a simple GPIO-sysctl bridge driver.  Currently it
 supports read operation only, but it is easy to add write one.

 I confirmed if the both work fine by using several Marvell
 88F628[12]-based boards.

-- Hiroki

----Next_Part(Sun_Aug_19_17_17_23_2012_889)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="gpiobus-20120819-1.diff"

Index: sys/dev/gpio/gpiobus.c
===================================================================
--- sys/dev/gpio/gpiobus.c	(revision 239381)
+++ sys/dev/gpio/gpiobus.c	(working copy)
@@ -40,6 +40,7 @@
 #include <machine/bus.h>
 #include <sys/rman.h>
 #include <machine/resource.h>
+#include <machine/_inttypes.h>

 #include <sys/gpio.h>
 #include <dev/gpio/gpiobusvar.h>
@@ -47,7 +48,7 @@
 #include "gpiobus_if.h"

 static void gpiobus_print_pins(struct gpiobus_ivar *);
-static int gpiobus_parse_pins(struct gpiobus_softc *, device_t, int);
+static int gpiobus_parse_pins(struct gpiobus_softc *, device_t, int *, size_t);
 static int gpiobus_probe(device_t);
 static int gpiobus_attach(device_t);
 static int gpiobus_detach(device_t);
@@ -69,6 +70,7 @@
 static int gpiobus_pin_setflags(device_t, device_t, uint32_t, uint32_t);
 static int gpiobus_pin_getflags(device_t, device_t, uint32_t, uint32_t*);
 static int gpiobus_pin_getcaps(device_t, device_t, uint32_t, uint32_t*);
+static int gpiobus_pin_getname(device_t, device_t, uint32_t, char *);
 static int gpiobus_pin_set(device_t, device_t, uint32_t, unsigned int);
 static int gpiobus_pin_get(device_t, device_t, uint32_t, unsigned int*);
 static int gpiobus_pin_toggle(device_t, device_t, uint32_t);
@@ -119,15 +121,18 @@
 }

 static int
-gpiobus_parse_pins(struct gpiobus_softc *sc, device_t child, int mask)
+gpiobus_parse_pins(struct gpiobus_softc *sc, device_t child, int *mask,
+    size_t masklen)
 {
 	struct gpiobus_ivar *devi = GPIOBUS_IVAR(child);
-	int i, npins;
+	int i, j, pin, npins;

 	npins = 0;
-	for (i = 0; i < 32; i++) {
-		if (mask & (1 << i))
-			npins++;
+	for (i = 0; i < masklen; i++) {
+		for (j = 0; j < sizeof(mask[i]) * 8; j++) {
+			if (mask[i] & (1 << j))
+				npins++;
+		}
 	}

 	if (npins == 0) {
@@ -143,27 +148,30 @@
 		return (ENOMEM);

 	npins = 0;
-	for (i = 0; i < 32; i++) {
+	for (i = 0; i < masklen; i++) {
+		for (j = 0; j < sizeof(mask[i]) * 8; j++) {
+			if ((mask[i] & (1 << j)) == 0)
+				continue;

-		if ((mask & (1 << i)) == 0)
-			continue;
-
-		if (i >= sc->sc_npins) {
-			device_printf(child,
-			    "invalid pin %d, max: %d\n", i, sc->sc_npins - 1);
-			return (EINVAL);
+			pin = i * sizeof(mask[i]) * 8 + j;
+			if (pin >= sc->sc_npins) {
+				device_printf(child,
+				    "invalid pin %d, max: %d\n", pin,
+				    sc->sc_npins - 1);
+				return (EINVAL);
+			}
+			devi->pins[npins++] = pin;
+			/*
+			 * Mark pin as mapped and give warning if it's
+			 * already mapped
+			 */
+			if (sc->sc_pins_mapped[pin]) {
+				device_printf(child,
+				    "warning: pin %d is already mapped\n", pin);
+				return (EINVAL);
+			}
+			sc->sc_pins_mapped[pin] = 1;
 		}
-
-		devi->pins[npins++] = i;
-		/*
-		 * Mark pin as mapped and give warning if it's already mapped
-		 */
-		if (sc->sc_pins_mapped[i]) {
-			device_printf(child,
-			    "warning: pin %d is already mapped\n", i);
-			return (EINVAL);
-		}
-		sc->sc_pins_mapped[i] = 1;
 	}

 	return (0);
@@ -192,6 +200,8 @@
 	 * Increase to get number of pins
 	 */
 	sc->sc_npins++;
+	if (bootverbose)
+		device_printf(dev, "number of pins = %d\n", sc->sc_npins);

 	KASSERT(sc->sc_npins != 0, ("GPIO device with no pins"));

@@ -304,19 +314,47 @@
 	return (child);
 }

+#define	GPIOBUS_PINS_MAX	128
+static int pinmask[(GPIOBUS_PINS_MAX - 1) / (sizeof(int) * 8) + 1];
+static char pinresname[] = "pinsNNN";
+
 static void
 gpiobus_hinted_child(device_t bus, const char *dname, int dunit)
 {
 	struct gpiobus_softc *sc = GPIOBUS_SOFTC(bus);
 	struct gpiobus_ivar *devi;
 	device_t child;
-	int pins;
+	int i, error;

+	bzero((char *)&pinmask, sizeof(pinmask));

 	child = BUS_ADD_CHILD(bus, 0, dname, dunit);
 	devi = GPIOBUS_IVAR(child);
-	resource_int_value(dname, dunit, "pins", &pins);
-	if (gpiobus_parse_pins(sc, child, pins))
+	if (resource_int_value(dname, dunit, "pins", &pinmask[0]) == 0) {
+		/*
+		 * Backward compatibility for 32-pin configuration in a
+		 * 32-bit hint:
+		 *
+		 * hint.devname.%d.pins=0x00000000
+		 */
+		i = 1;
+	} else {
+		/*
+		 * A 32-bit hint configures a 32-pin block:
+		 *
+		 * hint.devname.%d.pins0=0x00000000	#  0..31
+		 * hint.devname.%d.pins1=0x00000000	# 32..63
+		 * hint.devname.%d.pins2=0x00000000	# 64..91
+		 */
+		for (i = 0; i < sizeof(pinmask)/sizeof(pinmask[0]); i++) {
+			snprintf(pinresname, sizeof(pinresname), "pins%d", i);
+			error = resource_int_value(dname, dunit, pinresname,
+			     &pinmask[i]);
+			if (error)
+				break;
+		}
+	}
+	if (gpiobus_parse_pins(sc, child, pinmask, i))
 		device_delete_child(bus, child);
 }

@@ -409,6 +447,18 @@
 }

 static int
+gpiobus_pin_getname(device_t dev, device_t child, uint32_t pin, char *name)
+{
+	struct gpiobus_softc *sc = GPIOBUS_SOFTC(dev);
+	struct gpiobus_ivar *devi = GPIOBUS_IVAR(child);
+
+	if (pin >= devi->npins)
+		return (EINVAL);
+
+	return GPIO_PIN_GETNAME(sc->sc_dev, devi->pins[pin], name);
+}
+
+static int
 gpiobus_pin_set(device_t dev, device_t child, uint32_t pin,
     unsigned int value)
 {
@@ -469,6 +519,7 @@
 	DEVMETHOD(gpiobus_release_bus,	gpiobus_release_bus),
 	DEVMETHOD(gpiobus_pin_getflags,	gpiobus_pin_getflags),
 	DEVMETHOD(gpiobus_pin_getcaps,	gpiobus_pin_getcaps),
+	DEVMETHOD(gpiobus_pin_getname,	gpiobus_pin_getname),
 	DEVMETHOD(gpiobus_pin_setflags,	gpiobus_pin_setflags),
 	DEVMETHOD(gpiobus_pin_get,	gpiobus_pin_get),
 	DEVMETHOD(gpiobus_pin_set,	gpiobus_pin_set),
Index: sys/dev/gpio/gpiobus_if.m
===================================================================
--- sys/dev/gpio/gpiobus_if.m	(revision 239381)
+++ sys/dev/gpio/gpiobus_if.m	(working copy)
@@ -111,6 +111,16 @@
 };

 #
+# Get pin name
+#
+METHOD int pin_getname {
+	device_t dev;
+	device_t child;
+	uint32_t pin_num;
+	char *name;
+};
+
+#
 # Set current configuration and capabilities
 #
 METHOD int pin_setflags {
Index: sys/dev/gpio/gpioled.c
===================================================================
--- sys/dev/gpio/gpioled.c	(revision 239381)
+++ sys/dev/gpio/gpioled.c	(working copy)
@@ -94,20 +94,25 @@
 {
 	struct gpioled_softc *sc;
 	const char *name;
+	char nbuf[GPIOMAXNAME];

 	sc = device_get_softc(dev);
 	sc->sc_dev = dev;
 	sc->sc_busdev = device_get_parent(dev);
 	GPIOLED_LOCK_INIT(sc);
 	if (resource_string_value(device_get_name(dev),
-	    device_get_unit(dev), "name", &name))
-		name = NULL;
-
+	    device_get_unit(dev), "name", &name) != 0) {
+		GPIOBUS_PIN_GETNAME(sc->sc_busdev, sc->sc_dev, 0, nbuf);
+		nbuf[sizeof(nbuf) - 1] = '\0';
+		if (nbuf[0] != '\0')
+			name = nbuf;
+		else
+			name = device_get_nameunit(dev);
+	}
 	GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, GPIOLED_PIN,
 	    GPIO_PIN_OUTPUT);

-	sc->sc_leddev = led_create(gpioled_control, sc, name ? name :
-	    device_get_nameunit(dev));
+	sc->sc_leddev = led_create(gpioled_control, sc, name);

 	return (0);
 }

----Next_Part(Sun_Aug_19_17_17_23_2012_889)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="gpiosysctl-20120819-1.diff"

Index: sys/conf/files
===================================================================
--- sys/conf/files	(revision 239381)
+++ sys/conf/files	(working copy)
@@ -1268,6 +1268,7 @@
 	dependency	"gpio_if.h"
 dev/gpio/gpioiic.c		optional gpioiic
 dev/gpio/gpioled.c		optional gpioled
+dev/gpio/gpiosysctl.c		optional gpiosysctl
 dev/gpio/gpio_if.m		optional gpio
 dev/gpio/gpiobus_if.m		optional gpio
 dev/hatm/if_hatm.c		optional hatm pci
Index: sys/dev/gpio/gpiosysctl.c
===================================================================
--- sys/dev/gpio/gpiosysctl.c	(revision 0)
+++ sys/dev/gpio/gpiosysctl.c	(working copy)
@@ -0,0 +1,197 @@
+/*-
+ * Copyright (c) 2012 Hiroki Sato <hrs@FreeBSD.org>
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
+#include <sys/param.h>
+#include <sys/systm.h>
+#include <sys/kernel.h>
+#include <sys/module.h>
+#include <sys/bus.h>
+#include <sys/sysctl.h>
+#include <sys/lock.h>
+#include <sys/mutex.h>
+
+#include <machine/fdt.h>
+#include <dev/fdt/fdt_common.h>
+#include <dev/ofw/ofw_bus.h>
+#include <dev/ofw/ofw_bus_subr.h>
+
+#include <sys/gpio.h>
+
+#include "gpio_if.h"
+#include "gpiobus_if.h"
+
+struct gpiosysctl_softc
+{
+	device_t	sc_dev;
+	device_t	sc_busdev;
+	struct mtx	sc_mtx;
+};
+
+#define	GPIO_SYSCTL_DEVNAME		"gpiosysctl"
+#define	GPIO_SYSCTL_LOCK(sc)		mtx_lock(&(sc)->sc_mtx)
+#define	GPIO_SYSCTL_UNLOCK(sc)		mtx_unlock(&(sc)->sc_mtx)
+#define	GPIO_SYSCTL_LOCK_INIT(sc)	mtx_init(&(sc)->sc_mtx,		\
+					device_get_nameunit((sc)->sc_dev), \
+					GPIO_SYSCTL_DEVNAME, MTX_DEF)
+#define	GPIO_SYSCTL_LOCK_DESTROY(sc)	mtx_destroy(&(sc)->sc_mtx)
+
+static int gs_probe(device_t);
+static int gs_attach(device_t);
+static int gs_detach(device_t);
+
+static int
+gs_probe(device_t dev)
+{
+
+	device_set_desc(dev, "GPIO-sysctl bridge driver");
+
+	return (0);
+}
+
+static int
+gs_gpio_in(struct gpiosysctl_softc *sc)
+{
+	int val;
+
+	GPIO_SYSCTL_LOCK(sc);
+	GPIOBUS_LOCK_BUS(sc->sc_busdev);
+	GPIOBUS_ACQUIRE_BUS(sc->sc_busdev, sc->sc_dev);
+	GPIOBUS_PIN_GET(sc->sc_busdev, sc->sc_dev, 0, &val);
+	GPIOBUS_RELEASE_BUS(sc->sc_busdev, sc->sc_dev);
+	GPIOBUS_UNLOCK_BUS(sc->sc_busdev);
+	GPIO_SYSCTL_UNLOCK(sc);
+
+	return (val);
+}
+
+static void
+gs_gpio_out(struct gpiosysctl_softc *sc, int val)
+{
+	GPIO_SYSCTL_LOCK(sc);
+	GPIOBUS_LOCK_BUS(sc->sc_busdev);
+	GPIOBUS_ACQUIRE_BUS(sc->sc_busdev, sc->sc_dev);
+	GPIOBUS_PIN_SET(sc->sc_busdev, sc->sc_dev, 0,
+	    (val == 0) ? GPIO_PIN_LOW : GPIO_PIN_HIGH);
+	GPIOBUS_RELEASE_BUS(sc->sc_busdev, sc->sc_dev);
+	GPIOBUS_UNLOCK_BUS(sc->sc_busdev);
+	GPIO_SYSCTL_UNLOCK(sc);
+}
+
+static int
+gs_sysctl_handler(SYSCTL_HANDLER_ARGS)
+{
+	device_t dev;
+	struct gpiosysctl_softc *sc;
+	int val, ret;
+
+	dev = (device_t)arg1;
+	sc = device_get_softc(dev);
+	if (req->newptr == NULL) {
+		val = gs_gpio_in(sc);
+		ret = sysctl_handle_int(oidp, &val, 0, req);
+	} else {
+		ret = sysctl_handle_int(oidp, &val, 0, req);
+		if (ret < 0 || ret > 1) {
+			device_printf(dev, "%d: invalid value.\n", ret);
+			ret = 0;
+		}
+		gs_gpio_out(sc, val);
+		ret = val;
+	}
+
+	return (ret);
+}
+
+static int
+gs_attach(device_t dev)
+{
+	struct gpiosysctl_softc *sc;
+	struct sysctl_ctx_list *ctx;
+	struct sysctl_oid *oid;
+	device_t pdev;
+	const char *name;
+	char nbuf[GPIOMAXNAME];
+	int flags;
+
+	sc = device_get_softc(dev);
+	sc->sc_dev = dev;
+	sc->sc_busdev = device_get_parent(dev);
+	GPIO_SYSCTL_LOCK_INIT(sc);
+
+	if (resource_string_value(device_get_name(dev), device_get_unit(dev),
+	    "name", &name) != 0) {
+		GPIOBUS_PIN_GETNAME(sc->sc_busdev, sc->sc_dev, 0, nbuf);
+		nbuf[sizeof(nbuf) - 1] = '\0';
+		if (nbuf[0] != '\0')
+			name = nbuf;
+		else {
+			device_printf(dev, "no valid name.\n");
+			return (ENXIO);
+		}
+	}
+	ctx = device_get_sysctl_ctx(dev);
+
+	pdev = device_get_parent(dev);
+	oid = device_get_sysctl_tree(pdev);
+	flags = CTLTYPE_INT | CTLFLAG_RD;
+	SYSCTL_ADD_PROC(ctx, SYSCTL_CHILDREN(oid), OID_AUTO, name, flags, dev,
+	    0, gs_sysctl_handler, "I", NULL);
+
+	return (0);
+}
+
+static int
+gs_detach(device_t dev)
+{
+	struct gpiosysctl_softc *sc;
+
+	sc = device_get_softc(dev);
+	GPIO_SYSCTL_LOCK_DESTROY(sc);
+
+	return (0);
+}
+
+static devclass_t gs_devclass;
+
+static device_method_t gs_methods[] = {
+	/* Device interface */
+	DEVMETHOD(device_probe,		gs_probe),
+	DEVMETHOD(device_attach,	gs_attach),
+	DEVMETHOD(device_detach,	gs_detach),
+	{ 0, 0 },
+};
+
+static driver_t gs_driver = {
+	GPIO_SYSCTL_DEVNAME,
+	gs_methods,
+	sizeof(struct gpiosysctl_softc),
+};
+
+DRIVER_MODULE(gpiosysctl, gpiobus, gs_driver, gs_devclass, 0, 0);
+MODULE_VERSION(gpiosysctl, 1);

----Next_Part(Sun_Aug_19_17_17_23_2012_889)----

----Security_Multipart0(Sun_Aug_19_17_17_23_2012_120)--
Content-Type: application/pgp-signature
Content-Transfer-Encoding: 7bit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (FreeBSD)

iEYEABECAAYFAlAwoRMACgkQTyzT2CeTzy2ipwCgxfULT0udetgUXhdPkrcGxOFY
/MgAnRo+muFwVMIbiGA+kBXwswBYLiJQ
=PkE4
-----END PGP SIGNATURE-----

----Security_Multipart0(Sun_Aug_19_17_17_23_2012_120)----



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120819.171723.523519054460575158.hrs>