Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 7 Dec 2019 17:10:04 +0000 (UTC)
From:      Ian Lepore <ian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org
Subject:   svn commit: r355490 - in stable/12: share/man/man4 sys/dev/gpio sys/dev/ofw
Message-ID:  <201912071710.xB7HA4ap011179@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: ian
Date: Sat Dec  7 17:10:03 2019
New Revision: 355490
URL: https://svnweb.freebsd.org/changeset/base/355490

Log:
  MFC r355214, r355239, r355274, r355276-r355277, r355295, r355298
  
  r355214:
  Ignore "gpio-hog" nodes when instantiating ofw_gpiobus children.  Also,
  in ofw_gpiobus_probe() return BUS_PROBE_DEFAULT rather than 0; we are not
  the only possible driver to handle this device, we're just slightly better
  than the base gpiobus (which probes at BUS_PROBE_GENERIC).
  
  In the time since this code was first written, the gpio controller bindings
  aquired the concept of a "hog" node which could be used to preset one or
  more gpio pins as input or output at a specified level.  This change doesn't
  fully implement the hogging concept, it just filters out hog nodes when
  instantiating child devices by scanning for child nodes in the fdt data.
  
  The whole concept of having child nodes under the controller node is not
  supported by the standard bindings, and appears to be a freebsd extension,
  probably left over from the days when we had no support for cross-tree
  phandle references in the fdt data.
  
  r355239:
  Add an OFWBUS_PNP_INFO() macro for devices that hang directly off the root
  ofwbus.  Also, apply some style(9) whitespace fixing to the
  SIMPLEBUS_PNP_INFO() macro (no functional change).
  
  r355274:
  Move most of the gpio_pin_* functions from ofw_gpiobus.c to gpiobus.c so
  that they can be used by drivers on non-FDT-configured systems.  Only the
  functions related to acquiring pins by parsing FDT data remain in
  ofw_gpiobus.  Also, add two new functions for acquiring gpio pins based on
  child device_t and index, or on the bus device_t and pin number.  And
  finally, defer reserving pins for gpiobus children until they acquire the
  pin, rather than reserving them as soon as the child is added (before it's
  even known whether the child will attach).
  
  This will allow drivers configured with hints (or any other mechanism) to
  use the same code as drivers configured via FDT data.  Until now, a hinted
  driver and an FDT driver had to be two completely different sets of code,
  because hinted drivers could only use gpiobus calls to manipulate pins,
  while fdt-configured drivers could not use that API (due to not always being
  children of the bus that owns the pins) and had to use the newer
  gpio_pin_xxxx() functions.  Now drivers can be written in the more
  traditional form, where most of the code is shared and only the resource
  acquisition code at attachment time changes.
  
  r355276:
  Rewrite gpioiic(4) to use the gpio_pin_* API, and to conform to the modern
  FDT bindings document for gpio-i2c devices.
  
  Using the gpio_pin_* functions to acquire/release/manipulate gpio pins
  removes the constraint that both gpio pins must belong to the same gpio
  controller/bank, and that the gpioiic instance must be a child of gpiobus.
  Removing those constraints allows the driver to be fully compatible with
  the modern dts bindings for a gpio bitbanged i2c bus.
  
  For hinted attachment, the two gpio pins still must be on the same gpiobus,
  and the device instance must be a child of that bus.  This preserves
  compatibility for existing installations that have use gpioiic(4) with hints.
  
  r355277:
  Fix leading whitespace (spaces->tabs) in comments; no functional change.
  
  r355295:
  Remove "all rights reserved" from copyright after getting a response from
  Luiz that he also was not intentionally asserting that right, it was already
  there when he added his name.
  
  r355298:
  Do not initialize the flags field in struct gpiobus_pin from the flags in
  struct gpio_pin.  It turns out these two sets of flags are completely
  unrelated to each other.
  
  Also, update the comment for GPIO_ACTIVE_LOW to reflect the fact that it
  does get set, somewhat unobviously, by code that parses FDT data.  The bits
  from the FDT cell containing flags are just copied to gpiobus_pin.flags, so
  there's never any obvious reference to the symbol GPIO_ACTIVE_LOW being
  stored into the flags field.

Modified:
  stable/12/share/man/man4/gpioiic.4
  stable/12/sys/dev/gpio/gpiobus.c
  stable/12/sys/dev/gpio/gpiobusvar.h
  stable/12/sys/dev/gpio/gpioiic.c
  stable/12/sys/dev/gpio/ofw_gpiobus.c
  stable/12/sys/dev/ofw/ofw_bus_subr.h
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/share/man/man4/gpioiic.4
==============================================================================
--- stable/12/share/man/man4/gpioiic.4	Sat Dec  7 16:45:12 2019	(r355489)
+++ stable/12/share/man/man4/gpioiic.4	Sat Dec  7 17:10:03 2019	(r355490)
@@ -24,7 +24,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd May 14, 2014
+.Dd December 1, 2019
 .Dt GPIOIIC 4
 .Os
 .Sh NAME
@@ -37,38 +37,42 @@ kernel configuration file:
 .Bd -ragged -offset indent
 .Cd "device gpio"
 .Cd "device gpioiic"
-.Cd "device iic"
 .Cd "device iicbb"
 .Cd "device iicbus"
 .Ed
+.Pp
+Alternatively, to load the driver as a
+module at boot time, place the following line in
+.Xr loader.conf 5 :
+.Bd -literal -offset indent
+gpioiic_load="YES"
+.Ed
 .Sh DESCRIPTION
 The
 .Nm
 driver provides an IIC bit-banging interface using two GPIO pins for the
-SCL and SDA on the
-.Nm gpiobus .
+SCL and SDA lines on the bus.
+.Pp
 .Nm
-implements an open collector kind of output, as recommended by the standard,
-when driving the pins on the
-.Nm gpiobus ,
-i.e, they are never switched to the logical value of '1',
-or they are '0' or simply open (Hi-Z/tri-state).
-So the pullup resistors are required so
-.Nm
-can work.
+simulates an open collector kind of output when managing the pins on the
+bus, even on systems which don't directly support configuring gpio pins
+in that mode.
+The pins are never driven to the logical value of '1'.
+They are driven to '0' or switched to input mode (Hi-Z/tri-state), and
+an external pullup resistor pulls the line to the 1 state unless some
+other device on the bus is driving it to 0.
 .Pp
+.Sh HINTS CONFIGURATION
 On a
 .Xr device.hints 5
-based system, like
-.Li MIPS ,
-these values are configurable for the
+based system, such as MIPS, these values are configurable for
 .Nm :
 .Bl -tag -width ".Va hint.gpioiic.%d.atXXX"
 .It Va hint.gpioiic.%d.at
 The
 .Nm gpiobus
 you are attaching to.
-Normally just gpiobus0.
+Normally just gpiobus0 on systems with a single bank of gpio pins.
 .It Va hint.gpioiic.%d.pins
 This is a bitmask of the pins on the
 .Nm gpiobus
@@ -78,6 +82,9 @@ To configure pin 0 and 7, use the bitmask of
 0b10000001 and convert it to a hexadecimal value of 0x0081.
 Please note that this mask should only ever have two bits set
 (any other bits - i.e., pins - will be ignored).
+Because
+.Nm
+must be a child of the gpiobus, both gpio pins must be part of that bus.
 .It Va hint.gpioiic.%d.scl
 Indicates which bit in the
 .Va hint.gpioiic.%d.pins
@@ -91,35 +98,32 @@ should be used as the SDATA
 source.
 Optional, defaults to 1.
 .El
-.Pp
-On a
+.Sh FDT CONFIGURATION
+On an
 .Xr FDT 4
-based system, like
-.Li ARM ,
-the DTS part for a
+based system, such as ARM, the DTS node for
 .Nm gpioiic
-device usually looks like:
+conforms to the standard bindings document i2c/i2c-gpio.yaml.
+The device node typically appears at the root of the device tree.
+The following is an example of a
+.Nm
+node with one slave device
+on the IIC bus:
 .Bd -literal
-gpio: gpio {
-
-	gpio-controller;
-	...
-
+/ {
 	gpioiic0 {
-		compatible = "gpioiic";
-		/*
-		 * Attach to GPIO pins 21 and 22.  Set them
-		 * initially as inputs.
-		 */
-		gpios = <&gpio 21 1 0
-			 &gpio 22 1 0>;
-		scl = <0>;		/* GPIO pin 21 - optional */
-		sda = <1>;		/* GPIO pin 22 - optional */
+		compatible = "i2c-gpio";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_gpioiic0>;
+		scl-gpios = <&gpio1  5 GPIO_ACTIVE_HIGH>;
+		sda-gpios = <&gpio7 11 GPIO_ACTIVE_HIGH>;
+		status = "okay";
 
-		/* This is an example of a gpioiic child. */
-		gpioiic-child0 {
-			compatible = "lm75";
-			i2c-address = <0x4f>;
+		/* One slave device on the i2c bus. */
+		rtc@51 {
+			compatible="nxp,pcf2127";
+			reg = <0x51>;
+			status = "okay";
 		};
 	};
 };
@@ -128,35 +132,19 @@ gpio: gpio {
 Where:
 .Bl -tag -width ".Va compatible"
 .It Va compatible
-Should always be set to "gpioiic".
-.It Va gpios
-The
-.Va gpios
-property indicates which GPIO pins should be used for SCLOCK and SDATA
-on the GPIO IIC bit-banging bus.
-For more details about the
-.Va gpios
-property, please consult
-.Pa /usr/src/sys/dts/bindings-gpio.txt .
-.It Va scl
-The
-.Va scl
-option indicates which bit in the
-.Va gpios
-should be used as the SCLOCK source.
-Optional, defaults to 0.
-.It Va sda
-The
-.Va sda
-option indicates which bit in the
-.Va gpios
-should be used as the SDATA source.
-Optional, defaults to 1.
+Should be set to "i2c-gpio".
+The deprecated string "gpioiic" is also accepted for backwards compatibility.
+.It Va scl-gpios Va sda-gpios
+These properties indicate which GPIO pins should be used for clock
+and data on the GPIO IIC bit-banging bus.
+There is no requirement that the two pins belong to the same gpio controller.
+.It Va pinctrl-names pinctrl-0
+These properties may be required to configure the chosen pins as gpio
+pins, unless the pins default to that state on your system.
 .El
 .Sh SEE ALSO
 .Xr fdt 4 ,
 .Xr gpio 4 ,
-.Xr gpioled 4 ,
 .Xr iic 4 ,
 .Xr iicbb 4 ,
 .Xr iicbus 4

Modified: stable/12/sys/dev/gpio/gpiobus.c
==============================================================================
--- stable/12/sys/dev/gpio/gpiobus.c	Sat Dec  7 16:45:12 2019	(r355489)
+++ stable/12/sys/dev/gpio/gpiobus.c	Sat Dec  7 17:10:03 2019	(r355490)
@@ -78,6 +78,18 @@ static int gpiobus_pin_get(device_t, device_t, uint32_
 static int gpiobus_pin_toggle(device_t, device_t, uint32_t);
 
 /*
+ * gpiobus_pin flags
+ *  The flags in struct gpiobus_pin are not related to the flags used by the
+ *  low-level controller driver in struct gpio_pin.  Currently, only pins
+ *  acquired via FDT data have gpiobus_pin.flags set, sourced from the flags in
+ *  the FDT properties.  In theory, these flags are defined per-platform.  In
+ *  practice they are always the flags from the dt-bindings/gpio/gpio.h file.
+ *  The only one of those flags we currently support is for handling active-low
+ *  pins, so we just define that flag here instead of including a GPL'd header.
+ */
+#define	GPIO_ACTIVE_LOW 1
+
+/*
  * XXX -> Move me to better place - gpio_subr.c?
  * Also, this function must be changed when interrupt configuration
  * data will be moved into struct resource.
@@ -135,6 +147,114 @@ gpio_check_flags(uint32_t caps, uint32_t flags)
 	return (0);
 }
 
+int
+gpio_pin_get_by_bus_pinnum(device_t busdev, uint32_t pinnum, gpio_pin_t *ppin)
+{
+	gpio_pin_t pin;
+	int err;
+
+	err = gpiobus_acquire_pin(busdev, pinnum);
+	if (err != 0)
+		return (EBUSY);
+
+	pin = malloc(sizeof(*pin), M_DEVBUF, M_WAITOK | M_ZERO);
+
+	pin->dev = device_get_parent(busdev);
+	pin->pin = pinnum;
+	pin->flags = 0;
+
+	*ppin = pin;
+	return (0);
+}
+
+int
+gpio_pin_get_by_child_index(device_t childdev, uint32_t idx, gpio_pin_t *ppin)
+{
+	struct gpiobus_ivar *devi;
+
+	devi = GPIOBUS_IVAR(childdev);
+	if (idx >= devi->npins)
+		return (EINVAL);
+
+	return (gpio_pin_get_by_bus_pinnum(device_get_parent(childdev),
+	    devi->pins[idx], ppin));
+}
+
+int
+gpio_pin_getcaps(gpio_pin_t pin, uint32_t *caps)
+{
+
+	KASSERT(pin != NULL, ("GPIO pin is NULL."));
+	KASSERT(pin->dev != NULL, ("GPIO pin device is NULL."));
+	return (GPIO_PIN_GETCAPS(pin->dev, pin->pin, caps));
+}
+
+int
+gpio_pin_is_active(gpio_pin_t pin, bool *active)
+{
+	int rv;
+	uint32_t tmp;
+
+	KASSERT(pin != NULL, ("GPIO pin is NULL."));
+	KASSERT(pin->dev != NULL, ("GPIO pin device is NULL."));
+	rv = GPIO_PIN_GET(pin->dev, pin->pin, &tmp);
+	if (rv  != 0) {
+		return (rv);
+	}
+
+	if (pin->flags & GPIO_ACTIVE_LOW)
+		*active = tmp == 0;
+	else
+		*active = tmp != 0;
+	return (0);
+}
+
+void
+gpio_pin_release(gpio_pin_t gpio)
+{
+	device_t busdev;
+
+	if (gpio == NULL)
+		return;
+
+	KASSERT(gpio->dev != NULL, ("GPIO pin device is NULL."));
+
+	busdev = GPIO_GET_BUS(gpio->dev);
+	if (busdev != NULL)
+		gpiobus_release_pin(busdev, gpio->pin);
+
+	free(gpio, M_DEVBUF);
+}
+
+int
+gpio_pin_set_active(gpio_pin_t pin, bool active)
+{
+	int rv;
+	uint32_t tmp;
+
+	if (pin->flags & GPIO_ACTIVE_LOW)
+		tmp = active ? 0 : 1;
+	else
+		tmp = active ? 1 : 0;
+
+	KASSERT(pin != NULL, ("GPIO pin is NULL."));
+	KASSERT(pin->dev != NULL, ("GPIO pin device is NULL."));
+	rv = GPIO_PIN_SET(pin->dev, pin->pin, tmp);
+	return (rv);
+}
+
+int
+gpio_pin_setflags(gpio_pin_t pin, uint32_t flags)
+{
+	int rv;
+
+	KASSERT(pin != NULL, ("GPIO pin is NULL."));
+	KASSERT(pin->dev != NULL, ("GPIO pin device is NULL."));
+
+	rv = GPIO_PIN_SETFLAGS(pin->dev, pin->pin, flags);
+	return (rv);
+}
+
 static void
 gpiobus_print_pins(struct gpiobus_ivar *devi, char *buf, size_t buflen)
 {
@@ -370,8 +490,6 @@ gpiobus_parse_pins(struct gpiobus_softc *sc, device_t 
 		devi->pins[npins++] = i;
 	}
 
-	if (gpiobus_acquire_child_pins(sc->sc_busdev, child) != 0)
-		return (EINVAL);
 	return (0);
 }
 
@@ -425,8 +543,6 @@ gpiobus_parse_pin_list(struct gpiobus_softc *sc, devic
 		p = endp + 1;
 	}
 
-	if (gpiobus_acquire_child_pins(sc->sc_busdev, child) != 0)
-		return (EINVAL);
 	return (0);
 }
 

Modified: stable/12/sys/dev/gpio/gpiobusvar.h
==============================================================================
--- stable/12/sys/dev/gpio/gpiobusvar.h	Sat Dec  7 16:45:12 2019	(r355489)
+++ stable/12/sys/dev/gpio/gpiobusvar.h	Sat Dec  7 17:10:03 2019	(r355490)
@@ -141,7 +141,7 @@ int ofw_gpiobus_parse_gpios(device_t, char *, struct g
 void ofw_gpiobus_register_provider(device_t);
 void ofw_gpiobus_unregister_provider(device_t);
 
-/* Consumers interface. */
+/* Acquire a pin by parsing FDT data. */
 int gpio_pin_get_by_ofw_name(device_t consumer, phandle_t node,
     char *name, gpio_pin_t *gpio);
 int gpio_pin_get_by_ofw_idx(device_t consumer, phandle_t node,
@@ -150,14 +150,29 @@ int gpio_pin_get_by_ofw_property(device_t consumer, ph
     char *name, gpio_pin_t *gpio);
 int gpio_pin_get_by_ofw_propidx(device_t consumer, phandle_t node,
     char *name, int idx, gpio_pin_t *gpio);
+#endif /* FDT */
+
+/* Acquire a pin by bus and pin number. */
+int gpio_pin_get_by_bus_pinnum(device_t _bus, uint32_t _pinnum, gpio_pin_t *_gp);
+
+/* Acquire a pin by child and index (used by direct children of gpiobus). */
+int gpio_pin_get_by_child_index(device_t _child, uint32_t _idx, gpio_pin_t *_gp);
+
+/* Release a pin acquired via any gpio_pin_get_xxx() function. */
 void gpio_pin_release(gpio_pin_t gpio);
+
+/* Work with gpio pins acquired using the functions above. */
 int gpio_pin_getcaps(gpio_pin_t pin, uint32_t *caps);
 int gpio_pin_is_active(gpio_pin_t pin, bool *active);
 int gpio_pin_set_active(gpio_pin_t pin, bool active);
 int gpio_pin_setflags(gpio_pin_t pin, uint32_t flags);
-#endif
 struct resource *gpio_alloc_intr_resource(device_t consumer_dev, int *rid,
     u_int alloc_flags, gpio_pin_t pin, uint32_t intr_mode);
+
+/*
+ * Functions shared between gpiobus and other bus classes that derive from it;
+ * these should not be called directly by other drivers.
+ */
 int gpio_check_flags(uint32_t, uint32_t);
 device_t gpiobus_attach_bus(device_t);
 int gpiobus_detach_bus(device_t);

Modified: stable/12/sys/dev/gpio/gpioiic.c
==============================================================================
--- stable/12/sys/dev/gpio/gpioiic.c	Sat Dec  7 16:45:12 2019	(r355489)
+++ stable/12/sys/dev/gpio/gpioiic.c	Sat Dec  7 17:10:03 2019	(r355490)
@@ -1,9 +1,9 @@
 /*-
- * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ * SPDX-License-Identifier: BSD-2-Clause
  *
  * Copyright (c) 2009 Oleksandr Tymoshenko <gonzo@freebsd.org>
  * Copyright (c) 2010 Luiz Otavio O Souza
- * All rights reserved.
+ * Copyright (c) 2019 Ian Lepore <ian@freebsd.org>
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -39,14 +39,8 @@ __FBSDID("$FreeBSD$");
 #include <sys/kernel.h>
 #include <sys/module.h>
 
-#ifdef FDT
-#include <dev/fdt/fdt_common.h>
-#include <dev/ofw/ofw_bus.h>
-#endif
-
 #include <dev/gpio/gpiobusvar.h>
 #include <dev/iicbus/iiconf.h>
-#include <dev/iicbus/iicbus.h>
 
 #include "gpiobus_if.h"
 #include "iicbb_if.h"
@@ -57,200 +51,281 @@ __FBSDID("$FreeBSD$");
 
 struct gpioiic_softc 
 {
-	device_t	sc_dev;
-	device_t	sc_busdev;
-	int		scl_pin;
-	int		sda_pin;
+	device_t	dev;
+	gpio_pin_t	sclpin;
+	gpio_pin_t	sdapin;
 };
 
-static int gpioiic_probe(device_t);
-static int gpioiic_attach(device_t);
+#ifdef FDT
 
-/* iicbb interface */
-static void gpioiic_reset_bus(device_t);
-static void gpioiic_setsda(device_t, int);
-static void gpioiic_setscl(device_t, int);
-static int gpioiic_getsda(device_t);
-static int gpioiic_getscl(device_t);
-static int gpioiic_reset(device_t, u_char, u_char, u_char *);
+#include <dev/ofw/ofw_bus.h>
 
+static struct ofw_compat_data compat_data[] = {
+	{"i2c-gpio",  true}, /* Standard devicetree compat string */
+	{"gpioiic",   true}, /* Deprecated old freebsd compat string */
+	{NULL,        false}
+};
+OFWBUS_PNP_INFO(compat_data);
+SIMPLEBUS_PNP_INFO(compat_data);
+
+static phandle_t
+gpioiic_get_node(device_t bus, device_t dev)
+{
+
+	/* Share our fdt node with iicbus so it can find its child nodes. */
+	return (ofw_bus_get_node(bus));
+}
+
 static int
-gpioiic_probe(device_t dev)
+gpioiic_setup_fdt_pins(struct gpioiic_softc *sc)
 {
-	struct gpiobus_ivar *devi;
+	phandle_t node;
+	int err;
 
-#ifdef FDT
-	if (!ofw_bus_status_okay(dev))
-		return (ENXIO);
-	if (!ofw_bus_is_compatible(dev, "gpioiic"))
-		return (ENXIO);
-#endif
-	devi = GPIOBUS_IVAR(dev);
-	if (devi->npins < GPIOIIC_MIN_PINS) {
-		device_printf(dev,
-		    "gpioiic needs at least %d GPIO pins (only %d given).\n",
-		    GPIOIIC_MIN_PINS, devi->npins);
-		return (ENXIO);
-	}
-	device_set_desc(dev, "GPIO I2C bit-banging driver");
+	node = ofw_bus_get_node(sc->dev);
 
-	return (BUS_PROBE_DEFAULT);
+	/*
+	 * Historically, we used the first two array elements of the gpios
+	 * property.  The modern bindings specify separate scl-gpios and
+	 * sda-gpios properties.  We cope with whichever is present.
+	 */
+	if (OF_hasprop(node, "gpios")) {
+		if ((err = gpio_pin_get_by_ofw_idx(sc->dev, node,
+		    GPIOIIC_SCL_DFLT, &sc->sclpin)) != 0) {
+			device_printf(sc->dev, "invalid gpios property\n");
+			return (err);
+		}
+		if ((err = gpio_pin_get_by_ofw_idx(sc->dev, node,
+		    GPIOIIC_SDA_DFLT, &sc->sdapin)) != 0) {
+			device_printf(sc->dev, "ivalid gpios property\n");
+			return (err);
+		}
+	} else {
+		if ((err = gpio_pin_get_by_ofw_property(sc->dev, node,
+		    "scl-gpios", &sc->sclpin)) != 0) {
+			device_printf(sc->dev, "missing scl-gpios property\n");
+			return (err);
+		}
+		if ((err = gpio_pin_get_by_ofw_property(sc->dev, node,
+		    "sda-gpios", &sc->sdapin)) != 0) {
+			device_printf(sc->dev, "missing sda-gpios property\n");
+			return (err);
+		}
+	}
+	return (0);
 }
+#endif /* FDT */
 
 static int
-gpioiic_attach(device_t dev)
+gpioiic_setup_hinted_pins(struct gpioiic_softc *sc)
 {
-	device_t		bitbang;
-#ifdef FDT
-	phandle_t		node;
-	pcell_t			pin;
-#endif
-	struct gpiobus_ivar	*devi;
-	struct gpioiic_softc	*sc;
+	device_t busdev;
+	const char *busname, *devname;
+	int err, numpins, sclnum, sdanum, unit;
 
-	sc = device_get_softc(dev);
-	sc->sc_dev = dev;
-	sc->sc_busdev = device_get_parent(dev);
-	if (resource_int_value(device_get_name(dev),
-		device_get_unit(dev), "scl", &sc->scl_pin))
-		sc->scl_pin = GPIOIIC_SCL_DFLT;
-	if (resource_int_value(device_get_name(dev),
-		device_get_unit(dev), "sda", &sc->sda_pin))
-		sc->sda_pin = GPIOIIC_SDA_DFLT;
+	devname = device_get_name(sc->dev);
+	unit = device_get_unit(sc->dev);
+	busdev = device_get_parent(sc->dev);
 
+	/*
+	 * If there is not an "at" hint naming our actual parent, then we
+	 * weren't instantiated as a child of gpiobus via hints, and we thus
+	 * can't access ivars that only exist for such children.
+	 */
+	if (resource_string_value(devname, unit, "at", &busname) != 0 ||
+	    (strcmp(busname, device_get_nameunit(busdev)) != 0 &&
+	     strcmp(busname, device_get_name(busdev)) != 0)) {
+		return (ENOENT);
+	}
+
+	/* Make sure there were hints for at least two pins. */
+	numpins = gpiobus_get_npins(sc->dev);
+	if (numpins < GPIOIIC_MIN_PINS) {
 #ifdef FDT
-	if ((node = ofw_bus_get_node(dev)) == -1)
-		return (ENXIO);
-	if (OF_getencprop(node, "scl", &pin, sizeof(pin)) > 0)
-		sc->scl_pin = (int)pin;
-	if (OF_getencprop(node, "sda", &pin, sizeof(pin)) > 0)
-		sc->sda_pin = (int)pin;
+		/*
+		 * Be silent when there are no hints on FDT systems; the FDT
+		 * data will provide the pin config (we'll whine if it doesn't).
+		 */
+		if (numpins == 0) {
+			return (ENOENT);
+		}
 #endif
+		device_printf(sc->dev, 
+		    "invalid pins hint; it must contain at least %d pins\n",
+		    GPIOIIC_MIN_PINS);
+		return (EINVAL);
+	}
 
-	if (sc->scl_pin < 0 || sc->scl_pin > 1)
-		sc->scl_pin = GPIOIIC_SCL_DFLT;
-	if (sc->sda_pin < 0 || sc->sda_pin > 1)
-		sc->sda_pin = GPIOIIC_SDA_DFLT;
+	/*
+	 * Our parent bus has already parsed the pins hint and it will use that
+	 * info when we call gpio_pin_get_by_child_index().  But we have to
+	 * handle the scl/sda index hints that tell us which of the two pins is
+	 * the clock and which is the data.  They're optional, but if present
+	 * they must be a valid index (0 <= index < numpins).
+	 */
+	if ((err = resource_int_value(devname, unit, "scl", &sclnum)) != 0)
+		sclnum = GPIOIIC_SCL_DFLT;
+	else if (sclnum < 0 || sclnum >= numpins) {
+		device_printf(sc->dev, "invalid scl hint %d\n", sclnum);
+		return (EINVAL);
+	}
+	if ((err = resource_int_value(devname, unit, "sda", &sdanum)) != 0)
+		sdanum = GPIOIIC_SDA_DFLT;
+	else if (sdanum < 0 || sdanum >= numpins) {
+		device_printf(sc->dev, "invalid sda hint %d\n", sdanum);
+		return (EINVAL);
+	}
 
-	devi = GPIOBUS_IVAR(dev);
-	device_printf(dev, "SCL pin: %d, SDA pin: %d\n",
-	    devi->pins[sc->scl_pin], devi->pins[sc->sda_pin]);
+	/* Allocate gpiobus_pin structs for the pins we found above. */
+	if ((err = gpio_pin_get_by_child_index(sc->dev, sclnum,
+	    &sc->sclpin)) != 0)
+		return (err);
+	if ((err = gpio_pin_get_by_child_index(sc->dev, sdanum,
+	    &sc->sdapin)) != 0)
+		return (err);
 
-	/* add generic bit-banging code */
-	bitbang = device_add_child(dev, "iicbb", -1);
-	device_probe_and_attach(bitbang);
-
 	return (0);
 }
 
-static int
-gpioiic_detach(device_t dev)
-{
-
-	bus_generic_detach(dev);
-	device_delete_children(dev);
-	return (0);
-}
-
-/*
- * Reset bus by setting SDA first and then SCL. 
- * Must always be called with gpio bus locked.
- */
 static void
-gpioiic_reset_bus(device_t dev)
+gpioiic_setsda(device_t dev, int val)
 {
-	struct gpioiic_softc		*sc = device_get_softc(dev);
+	struct gpioiic_softc *sc = device_get_softc(dev);
+	int err;
 
-	GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, sc->sda_pin,
-	    GPIO_PIN_INPUT);
-	GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, sc->scl_pin,
-	    GPIO_PIN_INPUT);
+	/*
+	 * Some controllers cannot set an output value while a pin is in input
+	 * mode; in that case we set the pin again after changing mode.
+	 */
+	err = gpio_pin_set_active(sc->sdapin, val);
+	gpio_pin_setflags(sc->sdapin, GPIO_PIN_OUTPUT | GPIO_PIN_OPENDRAIN);
+	if (err != 0)
+		gpio_pin_set_active(sc->sdapin, val);
 }
 
 static void
-gpioiic_setpin(struct gpioiic_softc *sc, int pin, int val)
+gpioiic_setscl(device_t dev, int val)
 {
-	int				err;
+	struct gpioiic_softc *sc = device_get_softc(dev);
 
-	if (val == 0) {
-		err = GPIOBUS_PIN_SET(sc->sc_busdev, sc->sc_dev, pin, 0);
-		GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, pin,
-		    GPIO_PIN_OUTPUT);
-
-		/*
-		 * Some controllers cannot set output value while a pin is in
-		 * input mode.
-		 */
-		if (err != 0)
-			GPIOBUS_PIN_SET(sc->sc_busdev, sc->sc_dev, pin, 0);
-	} else {
-		GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, pin,
-		    GPIO_PIN_INPUT);
-	}
+	gpio_pin_setflags(sc->sclpin, GPIO_PIN_OUTPUT | GPIO_PIN_OPENDRAIN);
+	gpio_pin_set_active(sc->sclpin, val);
 }
 
-static void
-gpioiic_setsda(device_t dev, int val)
+static int
+gpioiic_getscl(device_t dev)
 {
-	struct gpioiic_softc		*sc = device_get_softc(dev);
+	struct gpioiic_softc *sc = device_get_softc(dev);
+	bool val;
 
-	gpioiic_setpin(sc, sc->sda_pin, val);
+	gpio_pin_setflags(sc->sclpin, GPIO_PIN_INPUT);
+	gpio_pin_is_active(sc->sclpin, &val);
+	return (val);
 }
 
-static void
-gpioiic_setscl(device_t dev, int val)
+static int
+gpioiic_getsda(device_t dev)
 {
-	struct gpioiic_softc		*sc = device_get_softc(dev);
+	struct gpioiic_softc *sc = device_get_softc(dev);
+	bool val;
 
-	gpioiic_setpin(sc, sc->scl_pin, val);
+	gpio_pin_setflags(sc->sdapin, GPIO_PIN_INPUT);
+	gpio_pin_is_active(sc->sdapin, &val);
+	return (val);
 }
 
 static int
-gpioiic_getpin(struct gpioiic_softc *sc, int pin)
+gpioiic_reset(device_t dev, u_char speed, u_char addr, u_char *oldaddr)
 {
-	unsigned int			val;
+	struct gpioiic_softc *sc = device_get_softc(dev);
 
-	GPIOBUS_PIN_SETFLAGS(sc->sc_busdev, sc->sc_dev, pin, GPIO_PIN_INPUT);
-	GPIOBUS_PIN_GET(sc->sc_busdev, sc->sc_dev, pin, &val);
-	return ((int)val);
+	/* Stop driving the bus pins. */
+	gpio_pin_setflags(sc->sdapin, GPIO_PIN_INPUT);
+	gpio_pin_setflags(sc->sclpin, GPIO_PIN_INPUT);
+
+	/* Indicate that we have no slave address (master mode). */
+	return (IIC_ENOADDR);
 }
 
-static int
-gpioiic_getscl(device_t dev)
+static void
+gpioiic_cleanup(struct gpioiic_softc *sc)
 {
-	struct gpioiic_softc		*sc = device_get_softc(dev);
 
-	return (gpioiic_getpin(sc, sc->scl_pin));
+	device_delete_children(sc->dev);
+
+	if (sc->sclpin != NULL)
+		gpio_pin_release(sc->sclpin);
+
+	if (sc->sdapin != NULL)
+		gpio_pin_release(sc->sdapin);
 }
 
 static int
-gpioiic_getsda(device_t dev)
+gpioiic_probe(device_t dev)
 {
-	struct gpioiic_softc		*sc = device_get_softc(dev);
+	int rv;
 
-	return (gpioiic_getpin(sc, sc->sda_pin));
+	/*
+	 * By default we only bid to attach if specifically added by our parent
+	 * (usually via hint.gpioiic.#.at=busname).  On FDT systems we bid as
+	 * the default driver based on being configured in the FDT data.
+	 */
+	rv = BUS_PROBE_NOWILDCARD;
+
+#ifdef FDT
+	if (ofw_bus_status_okay(dev) &&
+	    ofw_bus_search_compatible(dev, compat_data)->ocd_data)
+                rv = BUS_PROBE_DEFAULT;
+#endif
+
+	device_set_desc(dev, "GPIO I2C");
+
+	return (rv);
 }
 
 static int
-gpioiic_reset(device_t dev, u_char speed, u_char addr, u_char *oldaddr)
+gpioiic_attach(device_t dev)
 {
-	struct gpioiic_softc		*sc;
+	struct gpioiic_softc *sc = device_get_softc(dev);
+	int err;
 
-	sc = device_get_softc(dev);
-	gpioiic_reset_bus(sc->sc_dev);
+	sc->dev = dev;
 
-	return (IIC_ENOADDR);
+	/* Acquire our gpio pins. */
+	err = gpioiic_setup_hinted_pins(sc);
+#ifdef FDT
+	if (err != 0)
+		err = gpioiic_setup_fdt_pins(sc);
+#endif
+	if (err != 0) {
+		device_printf(sc->dev, "no pins configured\n");
+		gpioiic_cleanup(sc);
+		return (ENXIO);
+	}
+
+	/* Say what we came up with for pin config. */
+	device_printf(dev, "SCL pin: %s:%d, SDA pin: %s:%d\n",
+	    device_get_nameunit(GPIO_GET_BUS(sc->sclpin->dev)), sc->sclpin->pin,
+	    device_get_nameunit(GPIO_GET_BUS(sc->sdapin->dev)), sc->sdapin->pin);
+
+	/* Add the bitbang driver as our only child; it will add iicbus. */
+	device_add_child(sc->dev, "iicbb", -1);
+	return (bus_generic_attach(dev));
 }
 
-#ifdef FDT
-static phandle_t
-gpioiic_get_node(device_t bus, device_t dev)
+static int
+gpioiic_detach(device_t dev)
 {
+	struct gpioiic_softc *sc = device_get_softc(dev);
+	int err;
 
-	/* We only have one child, the iicbb, which needs our own node. */
-	return (ofw_bus_get_node(bus));
+	if ((err = bus_generic_detach(dev)) != 0)
+		return (err);
+
+	gpioiic_cleanup(sc);
+
+	return (0);
 }
-#endif
 
 static devclass_t gpioiic_devclass;
 
@@ -282,6 +357,7 @@ static driver_t gpioiic_driver = {
 };
 
 DRIVER_MODULE(gpioiic, gpiobus, gpioiic_driver, gpioiic_devclass, 0, 0);
+DRIVER_MODULE(gpioiic, simplebus, gpioiic_driver, gpioiic_devclass, 0, 0);
 DRIVER_MODULE(iicbb, gpioiic, iicbb_driver, iicbb_devclass, 0, 0);
 MODULE_DEPEND(gpioiic, iicbb, IICBB_MINVER, IICBB_PREFVER, IICBB_MAXVER);
 MODULE_DEPEND(gpioiic, gpiobus, 1, 1, 1);

Modified: stable/12/sys/dev/gpio/ofw_gpiobus.c
==============================================================================
--- stable/12/sys/dev/gpio/ofw_gpiobus.c	Sat Dec  7 16:45:12 2019	(r355489)
+++ stable/12/sys/dev/gpio/ofw_gpiobus.c	Sat Dec  7 17:10:03 2019	(r355490)
@@ -43,8 +43,6 @@ __FBSDID("$FreeBSD$");
 
 #include "gpiobus_if.h"
 
-#define	GPIO_ACTIVE_LOW		1
-
 static struct ofw_gpiobus_devinfo *ofw_gpiobus_setup_devinfo(device_t,
 	device_t, phandle_t);
 static void ofw_gpiobus_destroy_devinfo(device_t, struct ofw_gpiobus_devinfo *);
@@ -140,82 +138,6 @@ gpio_pin_get_by_ofw_name(device_t consumer, phandle_t 
 	return (gpio_pin_get_by_ofw_idx(consumer, node, idx, pin));
 }
 
-void
-gpio_pin_release(gpio_pin_t gpio)
-{
-	device_t busdev;
-
-	if (gpio == NULL)
-		return;
-
-	KASSERT(gpio->dev != NULL, ("invalid pin state"));
-
-	busdev = GPIO_GET_BUS(gpio->dev);
-	if (busdev != NULL)
-		gpiobus_release_pin(busdev, gpio->pin);
-
-	/* XXXX Unreserve pin. */
-	free(gpio, M_DEVBUF);
-}
-
-int
-gpio_pin_getcaps(gpio_pin_t pin, uint32_t *caps)
-{
-
-	KASSERT(pin != NULL, ("GPIO pin is NULL."));
-	KASSERT(pin->dev != NULL, ("GPIO pin device is NULL."));
-	return (GPIO_PIN_GETCAPS(pin->dev, pin->pin, caps));
-}
-
-int
-gpio_pin_is_active(gpio_pin_t pin, bool *active)
-{
-	int rv;
-	uint32_t tmp;
-
-	KASSERT(pin != NULL, ("GPIO pin is NULL."));
-	KASSERT(pin->dev != NULL, ("GPIO pin device is NULL."));
-	rv = GPIO_PIN_GET(pin->dev, pin->pin, &tmp);
-	if (rv  != 0) {
-		return (rv);
-	}
-
-	if (pin->flags & GPIO_ACTIVE_LOW)
-		*active = tmp == 0;
-	else
-		*active = tmp != 0;
-	return (0);
-}
-
-int
-gpio_pin_set_active(gpio_pin_t pin, bool active)
-{
-	int rv;
-	uint32_t tmp;
-
-	if (pin->flags & GPIO_ACTIVE_LOW)
-		tmp = active ? 0 : 1;
-	else
-		tmp = active ? 1 : 0;
-
-	KASSERT(pin != NULL, ("GPIO pin is NULL."));
-	KASSERT(pin->dev != NULL, ("GPIO pin device is NULL."));
-	rv = GPIO_PIN_SET(pin->dev, pin->pin, tmp);
-	return (rv);
-}
-
-int
-gpio_pin_setflags(gpio_pin_t pin, uint32_t flags)
-{
-	int rv;
-
-	KASSERT(pin != NULL, ("GPIO pin is NULL."));
-	KASSERT(pin->dev != NULL, ("GPIO pin device is NULL."));
-
-	rv = GPIO_PIN_SETFLAGS(pin->dev, pin->pin, flags);
-	return (rv);
-}
-
 /*
  * OFW_GPIOBUS driver.
  */
@@ -492,7 +414,7 @@ ofw_gpiobus_probe(device_t dev)
 		return (ENXIO);
 	device_set_desc(dev, "OFW GPIO bus");
 
-	return (0);
+	return (BUS_PROBE_DEFAULT);
 }
 
 static int
@@ -511,6 +433,8 @@ ofw_gpiobus_attach(device_t dev)
 	 */
 	for (child = OF_child(ofw_bus_get_node(dev)); child != 0;
 	    child = OF_peer(child)) {
+		if (OF_hasprop(child, "gpio-hog"))
+			continue;
 		if (!OF_hasprop(child, "gpios"))
 			continue;
 		if (ofw_gpiobus_add_fdt_child(dev, NULL, child) == NULL)

Modified: stable/12/sys/dev/ofw/ofw_bus_subr.h
==============================================================================
--- stable/12/sys/dev/ofw/ofw_bus_subr.h	Sat Dec  7 16:45:12 2019	(r355489)
+++ stable/12/sys/dev/ofw/ofw_bus_subr.h	Sat Dec  7 17:10:03 2019	(r355490)
@@ -69,7 +69,8 @@ struct intr_map_data_fdt {
 #define FDTCOMPAT_PNP_INFO(t, busname) \
 	MODULE_PNP_INFO(FDTCOMPAT_PNP_DESCR, busname, t, t, sizeof(t) / sizeof(t[0]));
 
-#define SIMPLEBUS_PNP_INFO(t) FDTCOMPAT_PNP_INFO(t, simplebus)
+#define	OFWBUS_PNP_INFO(t)	FDTCOMPAT_PNP_INFO(t, ofwbus)
+#define	SIMPLEBUS_PNP_INFO(t)	FDTCOMPAT_PNP_INFO(t, simplebus)
 
 /* Generic implementation of ofw_bus_if.m methods and helper routines */
 int	ofw_bus_gen_setup_devinfo(struct ofw_bus_devinfo *, phandle_t);



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