gpio_acpi: Add allocate/release functions for GpioIo pins. master
authorImre Vadász <imre@vdsz.com>
Sat, 30 Apr 2016 16:46:11 +0000 (18:46 +0200)
committerImre Vadász <imre@vdsz.com>
Sat, 28 May 2016 23:39:43 +0000 (01:39 +0200)
* This slightly simplifies the read_pin and write_pin functions, and
  configuration errors should now be found when allocating the pin,
  instead of causing a panic in read_pin() or write_pin().

sys/bus/gpio/gpio_acpi/gpio_acpi.c
sys/bus/gpio/gpio_if.m
sys/bus/gpio/gpio_intel/gpio_cherryview.c
sys/bus/gpio/gpio_intel/gpio_intel.c
sys/bus/gpio/gpio_intel/gpio_intel_var.h

index 203dbf6..88d5738 100644 (file)
@@ -68,6 +68,9 @@ struct gpio_acpi_data {
 };
 
 static BOOLEAN gpio_acpi_check_gpioint(device_t dev, ACPI_RESOURCE_GPIO *gpio);
+static void    **gpioio_alloc_pins(device_t dev, device_t provider,
+                   ACPI_RESOURCE_GPIO *gpio, uint16_t idx, uint16_t length,
+                   void **buf);
 
 /* GPIO Address Space Handler */
 static void            gpio_acpi_install_address_space_handler(device_t dev,
@@ -151,6 +154,53 @@ gpio_acpi_check_gpioint(device_t dev, ACPI_RESOURCE_GPIO *gpio)
 }
 
 /*
+ * GpioIo ACPI resource handling
+ */
+
+static void **
+gpioio_alloc_pins(device_t dev, device_t provider, ACPI_RESOURCE_GPIO *gpio,
+    uint16_t idx, uint16_t length, void **buf)
+{
+       void **pins;
+       int flags, i, j;
+
+       if (buf == NULL) {
+               pins = kmalloc(sizeof(*pins) * length, M_DEVBUF,
+                   M_WAITOK | M_ZERO);
+       } else {
+               pins = buf;
+       }
+
+       if (gpio->IoRestriction == ACPI_IO_RESTRICT_INPUT) {
+               flags = (1U << 0);
+       } else if (gpio->IoRestriction ==
+           ACPI_IO_RESTRICT_OUTPUT) {
+               flags = (1U << 1);
+       } else {
+               flags = (1U << 0) | (1U << 1);
+       }
+       for (i = 0; i < length; i++) {
+               if (GPIO_ALLOC_IO_PIN(provider, gpio->PinTable[idx + i], flags,
+                   &pins[i]) != 0) {
+                       device_printf(dev, "Failed to alloc GpioIo pin %u on "
+                           "ResourceSource \"%s\"\n", gpio->PinTable[idx + i],
+                           gpio->ResourceSource.StringPtr);
+                       /* Release already alloc-ed pins */
+                       for (j = 0; j < i; j++)
+                               GPIO_RELEASE_IO_PIN(provider, pins[j]);
+                       goto err;
+               }
+       }
+
+       return (pins);
+
+err:
+       if (buf == NULL)
+               kfree(pins, M_DEVBUF);
+       return (NULL);
+}
+
+/*
  * GPIO Address space handler
  */
 
@@ -163,11 +213,6 @@ gpio_acpi_install_address_space_handler(device_t dev,
 
        handle = acpi_get_handle(dev);
        data->dev = dev;
-       /*
-        * XXX Should use the Address Space Setup Handler to check and
-        *     reserve the GPIO pins, to avoid checking on each READ/WRITE
-        *     call into the Adress Space Handler.
-        */
        s = AcpiInstallAddressSpaceHandler(handle, ACPI_ADR_SPACE_GPIO,
            &gpio_acpi_space_handler, NULL, data);
        if (ACPI_FAILURE(s)) {
@@ -204,57 +249,79 @@ gpio_acpi_space_handler(UINT32 Function, ACPI_PHYSICAL_ADDRESS Address,
        UINT8 action = Function & ACPI_IO_MASK;
        ACPI_RESOURCE *Resource;
        ACPI_STATUS s = AE_OK;
+       void **pins;
        int i;
 
+       if (Value == NULL)
+               return (AE_BAD_PARAMETER);
+
+       /* XXX probably unnecessary */
+       if (BitWidth == 0 || BitWidth > 64)
+               return (AE_BAD_PARAMETER);
+
        s = AcpiBufferToResource(info->Connection, info->Length, &Resource);
-       if (ACPI_FAILURE(s))
+       if (ACPI_FAILURE(s)) {
+               device_printf(dev, "AcpiBufferToResource failed\n");
                return (s);
-       if (Value == NULL || Resource->Type != ACPI_RESOURCE_TYPE_GPIO) {
+       }
+       if (Resource->Type != ACPI_RESOURCE_TYPE_GPIO) {
+               device_printf(dev, "Resource->Type is wrong\n");
                s = AE_BAD_PARAMETER;
                goto err;
        }
-
        gpio = &Resource->Data.Gpio;
        if (gpio->ConnectionType != ACPI_RESOURCE_GPIO_TYPE_IO) {
+               device_printf(dev, "gpio->ConnectionType is wrong\n");
                s = AE_BAD_PARAMETER;
                goto err;
        }
 
-       /* XXX probably unnecessary */
-       if (BitWidth == 0 || BitWidth > 64) {
+       if (Address + BitWidth > gpio->PinTableLength) {
+               device_printf(dev, "Address + BitWidth out of range\n");
+               s = AE_BAD_ADDRESS;
+               goto err;
+       }
+
+       if (gpio->IoRestriction == ACPI_IO_RESTRICT_OUTPUT &&
+           action == ACPI_READ) {
+               device_printf(dev,
+                   "IoRestriction is output only, but action is ACPI_READ\n");
+               s = AE_BAD_PARAMETER;
+               goto err;
+       }
+       if (gpio->IoRestriction == ACPI_IO_RESTRICT_INPUT &&
+           action == ACPI_WRITE) {
+               device_printf(dev,
+                   "IoRestriction is input only, but action is ACPI_WRITE\n");
                s = AE_BAD_PARAMETER;
                goto err;
        }
 
-       if (Address + BitWidth > gpio->PinTableLength) {
-               s = AE_BAD_ADDRESS;
+       /* Make sure we can access all pins, before trying actual read/write */
+       pins = gpioio_alloc_pins(dev, dev, gpio, Address, BitWidth, NULL);
+       if (pins == NULL) {
+               s = AE_BAD_PARAMETER;
                goto err;
        }
 
        if (action == ACPI_READ) {
-               if (gpio->IoRestriction == ACPI_IO_RESTRICT_OUTPUT) {
-                       s = AE_BAD_PARAMETER;
-                       goto err;
-               }
                *Value = 0;
                for (i = 0; i < BitWidth; i++) {
-                       val = GPIO_READ_PIN(dev, gpio->PinTable[Address + i]);
+                       val = GPIO_READ_PIN(dev, pins[i]);
                        *Value |= val << i;
                }
        } else {
-               if (gpio->IoRestriction == ACPI_IO_RESTRICT_INPUT) {
-                       s = AE_BAD_PARAMETER;
-                       goto err;
-               }
                for (i = 0; i < BitWidth; i++) {
-                       GPIO_WRITE_PIN(dev, gpio->PinTable[Address + i],
-                           *Value & (1ULL << i) ? 1 : 0);
+                       GPIO_WRITE_PIN(dev, pins[i],
+                           (*Value & (1ULL << i)) ? 1 : 0);
                }
        }
+       for (i = 0; i < BitWidth; i++)
+               GPIO_RELEASE_IO_PIN(dev, pins[i]);
+       kfree(pins, M_DEVBUF);
 
 err:
        ACPI_FREE(Resource);
-
        return (s);
 }
 
@@ -363,14 +430,14 @@ gpio_acpi_map_aei(device_t dev, int *num_aei)
                n = 0;
                for (res = (ACPI_RESOURCE *)b.Pointer; res < end;
                    res = ACPI_NEXT_RESOURCE(res)) {
-                       if (res->Type == ACPI_RESOURCE_TYPE_END_TAG)
+                       if (res->Type == ACPI_RESOURCE_TYPE_END_TAG) {
                                break;
-                       switch (res->Type) {
-                       case ACPI_RESOURCE_TYPE_GPIO:
+                       } else if (res->Type == ACPI_RESOURCE_TYPE_GPIO) {
                                n++;
-                               break;
-                       default:
-                               break;
+                       } else {
+                               device_printf(dev,
+                                   "Unexpected resource type %d\n",
+                                   res->Type);
                        }
                }
                if (n <= 0) {
@@ -384,17 +451,10 @@ gpio_acpi_map_aei(device_t dev, int *num_aei)
                    res = ACPI_NEXT_RESOURCE(res)) {
                        if (res->Type == ACPI_RESOURCE_TYPE_END_TAG)
                                break;
-                       switch (res->Type) {
-                       case ACPI_RESOURCE_TYPE_GPIO:
+                       if (res->Type == ACPI_RESOURCE_TYPE_GPIO) {
                                gpio = (ACPI_RESOURCE_GPIO *)&res->Data;
                                gpio_acpi_do_map_aei(dev, &infos[n++], gpio);
-                               break;
-                       default:
-                               device_printf(dev,
-                                   "Unexpected resource type %d\n",
-                                   res->Type);
-                               break;
-                       };
+                       }
                }
                AcpiOsFree(b.Pointer);
        }
index b639244..2b3e111 100644 (file)
@@ -77,17 +77,29 @@ METHOD void teardown_intr {
 };
 
 #
-# XXX Add a method for allocating pins for read/write IO.
-#     Allocating a pin for IO should perform the necessary checks to
-#     make sure that read_/write_pin doesn't trigger an assertion.
+# Reserve IO pin
 #
+METHOD int alloc_io_pin {
+       device_t dev;
+       u_int pin;
+       int flags;
+       void **cookiep;
+};
+
+#
+# Release IO pin
+#
+METHOD int release_io_pin {
+       device_t dev;
+       void *cookie;
+};
 
 #
 # Read pin value, returns 0 or 1.
 #
 METHOD int read_pin {
        device_t dev;
-       u_int pin;
+       void *cookie;
 };
 
 #
@@ -95,6 +107,6 @@ METHOD int read_pin {
 #
 METHOD void write_pin {
        device_t dev;
-       u_int pin;
+       void *cookie;
        int value;
 };
index e42e1d8..874ffef 100644 (file)
@@ -91,6 +91,8 @@ static void   gpio_cherryview_enable_intr(struct gpio_intel_softc *sc,
                    struct pin_intr_map *map);
 static void    gpio_cherryview_disable_intr(struct gpio_intel_softc *sc,
                    struct pin_intr_map *map);
+static int     gpio_cherryview_check_io_pin(struct gpio_intel_softc *sc,
+                   uint16_t pin, int flags);
 static int     gpio_cherryview_read_pin(struct gpio_intel_softc *sc,
                    uint16_t pin);
 static void    gpio_cherryview_write_pin(struct gpio_intel_softc *sc,
@@ -103,6 +105,7 @@ static struct gpio_intel_fns gpio_cherryview_fns = {
        .unmap_intr = gpio_cherryview_unmap_intr,
        .enable_intr = gpio_cherryview_enable_intr,
        .disable_intr = gpio_cherryview_disable_intr,
+       .check_io_pin = gpio_cherryview_check_io_pin,
        .read_pin = gpio_cherryview_read_pin,
        .write_pin = gpio_cherryview_write_pin,
 };
@@ -215,6 +218,7 @@ gpio_cherryview_intr(void *arg)
        int i;
 
        status = chvgpio_read(sc, CHV_GPIO_REG_IS);
+       KKASSERT(NELEM(sc->intrmaps) >= 16);
        for (i = 0; i < 16; i++) {
                if (status & (1U << i)) {
                        mapping = &sc->intrmaps[i];
@@ -442,6 +446,35 @@ gpio_cherryview_disable_intr(struct gpio_intel_softc *sc,
 }
 
 static int
+gpio_cherryview_check_io_pin(struct gpio_intel_softc *sc, uint16_t pin,
+    int flags)
+{
+       uint32_t reg1, reg2;
+
+       reg1 = chvgpio_read(sc, PIN_CTL0(pin));
+       if (flags & (1U << 0)) {
+               /* Verify that RX is enabled */
+               if ((reg1 & CHV_GPIO_CTL0_GPIOCFG_MASK) != 0 &&
+                   (reg1 & CHV_GPIO_CTL0_GPIOCFG_MASK) != 0x200) {
+                       return (0);
+               }
+       }
+       reg2 = chvgpio_read(sc, PIN_CTL1(pin));
+       if (flags & (1U << 1)) {
+               /* Verify that interrupt is disabled */
+               if ((reg2 & CHV_GPIO_CTL1_INTCFG_MASK) != 0)
+                       return (0);
+               /* Verify that TX is enabled */
+               if ((reg1 & CHV_GPIO_CTL0_GPIOCFG_MASK) != 0 &&
+                   (reg1 & CHV_GPIO_CTL0_GPIOCFG_MASK) != 0x100) {
+                       return (0);
+               }
+       }
+
+       return (1);
+}
+
+static int
 gpio_cherryview_read_pin(struct gpio_intel_softc *sc, uint16_t pin)
 {
        uint32_t reg;
@@ -463,20 +496,16 @@ gpio_cherryview_read_pin(struct gpio_intel_softc *sc, uint16_t pin)
 static void
 gpio_cherryview_write_pin(struct gpio_intel_softc *sc, uint16_t pin, int value)
 {
-       uint32_t reg1, reg2;
-
-       reg2 = chvgpio_read(sc, PIN_CTL1(pin));
-       /* Verify that interrupt is disabled */
-       KKASSERT((reg2 & CHV_GPIO_CTL1_INTCFG_MASK) == 0);
+       uint32_t reg;
 
-       reg1 = chvgpio_read(sc, PIN_CTL0(pin));
+       reg = chvgpio_read(sc, PIN_CTL0(pin));
        /* Verify that TX is enabled */
-       KKASSERT((reg1 & CHV_GPIO_CTL0_GPIOCFG_MASK) == 0 ||
-           (reg1 & CHV_GPIO_CTL0_GPIOCFG_MASK) == 0x100);
+       KKASSERT((reg & CHV_GPIO_CTL0_GPIOCFG_MASK) == 0 ||
+           (reg & CHV_GPIO_CTL0_GPIOCFG_MASK) == 0x100);
 
        if (value)
-               reg1 |= CHV_GPIO_CTL0_TXSTATE;
+               reg |= CHV_GPIO_CTL0_TXSTATE;
        else
-               reg1 &= ~CHV_GPIO_CTL0_TXSTATE;
-       chvgpio_write(sc, PIN_CTL0(pin), reg1);
+               reg &= ~CHV_GPIO_CTL0_TXSTATE;
+       chvgpio_write(sc, PIN_CTL0(pin), reg);
 }
index 09acffd..ee42cf4 100644 (file)
@@ -67,8 +67,11 @@ static void  gpio_intel_setup_intr(device_t dev, void *cookie, void *arg,
                    driver_intr_t *handler);
 static void    gpio_intel_teardown_intr(device_t dev, void *cookie);
 static void    gpio_intel_free_intr(device_t dev, void *cookie);
-static int     gpio_intel_read_pin(device_t dev, u_int pin);
-static void    gpio_intel_write_pin(device_t dev, u_int pin, int value);
+static int     gpio_intel_alloc_io_pin(device_t dev, u_int pin, int flags,
+                   void **cookiep);
+static void    gpio_intel_release_io_pin(device_t dev, void *cookie);
+static int     gpio_intel_read_pin(device_t dev, void *cookie);
+static void    gpio_intel_write_pin(device_t dev, void *cookie, int value);
 
 static void    gpio_intel_intr(void *arg);
 static int     gpio_intel_pin_exists(struct gpio_intel_softc *sc,
@@ -111,9 +114,12 @@ gpio_intel_attach(device_t dev)
                goto err;
        }
 
-       for (i = 0; i < 16; i++) {
+       for (i = 0; i < NELEM(sc->intrmaps); i++) {
                sc->intrmaps[i].pin = -1;
        }
+       for (i = 0; i < NELEM(sc->iomaps); i++) {
+               sc->iomaps[i].pin = -1;
+       }
 
        rid = 0;
        sc->mem_res = bus_alloc_resource_any(dev, SYS_RES_MEMORY,
@@ -208,9 +214,11 @@ gpio_intel_alloc_intr(device_t dev, u_int pin, int trigger, int polarity,
 
        if (gpio_intel_pin_exists(sc, pin)) {
                /* Make sure this pin isn't mapped yet */
-               for (i = 0; i < 16; i++) {
-                       if (sc->intrmaps[i].pin == pin)
-                       return (ENOMEM);
+               for (i = 0; i < NELEM(sc->intrmaps); i++) {
+                       if (sc->intrmaps[i].pin == pin) {
+                               lockmgr(&sc->lk, LK_RELEASE);
+                               return (EBUSY);
+                       }
                }
                ret = sc->fns->map_intr(sc, pin, trigger, polarity,
                    termination);
@@ -283,31 +291,92 @@ gpio_intel_teardown_intr(device_t dev, void *cookie)
 }
 
 static int
-gpio_intel_read_pin(device_t dev, u_int pin)
+gpio_intel_alloc_io_pin(device_t dev, u_int pin, int flags, void **cookiep)
 {
        struct gpio_intel_softc *sc = device_get_softc(dev);
+       struct pin_io_map *map = NULL;
+       int i, ret;
+
+       if (cookiep == NULL)
+               return (EINVAL);
+
+       if (!gpio_intel_pin_exists(sc, pin))
+               return (ENOENT);
+
+       lockmgr(&sc->lk, LK_EXCLUSIVE);
+
+       for (i = 0; i < NELEM(sc->iomaps); i++) {
+               if (sc->iomaps[i].pin == pin) {
+                       lockmgr(&sc->lk, LK_RELEASE);
+                       return (EBUSY);
+               }
+       }
+       for (i = 0; i < NELEM(sc->iomaps); i++) {
+               if (sc->iomaps[i].pin == -1) {
+                       map = &sc->iomaps[i];
+                       break;
+               }
+       }
+       if (map == NULL) {
+               ret = EBUSY;
+       } else if (sc->fns->check_io_pin(sc, pin, flags)) {
+               /*
+                * XXX It's possible that RX gets disabled when the interrupt
+                *     on this pin gets released.
+                */
+               map->pin = pin;
+               map->flags = flags;
+               *cookiep = map;
+               ret = 0;
+       } else {
+               ret = EBUSY;
+       }
+
+       lockmgr(&sc->lk, LK_RELEASE);
+
+       return (ret);
+}
+
+static void
+gpio_intel_release_io_pin(device_t dev, void *cookie)
+{
+       struct gpio_intel_softc *sc = device_get_softc(dev);
+       struct pin_io_map *map = (struct pin_io_map *)cookie;
+
+       lockmgr(&sc->lk, LK_EXCLUSIVE);
+       map->pin = -1;
+       map->flags = 0;
+       lockmgr(&sc->lk, LK_RELEASE);
+}
+
+static int
+gpio_intel_read_pin(device_t dev, void *cookie)
+{
+       struct gpio_intel_softc *sc = device_get_softc(dev);
+       struct pin_io_map *map = (struct pin_io_map *)cookie;
        int val;
 
-        /* This operation mustn't fail, otherwise ACPI would be in trouble */
-       KKASSERT(gpio_intel_pin_exists(sc, pin));
+       KKASSERT(map->pin >= 0);
+       KKASSERT(gpio_intel_pin_exists(sc, map->pin));
 
        lockmgr(&sc->lk, LK_EXCLUSIVE);
-       val = sc->fns->read_pin(sc, pin);
+       val = sc->fns->read_pin(sc, map->pin);
        lockmgr(&sc->lk, LK_RELEASE);
 
        return (val);
 }
 
 static void
-gpio_intel_write_pin(device_t dev, u_int pin, int value)
+gpio_intel_write_pin(device_t dev, void *cookie, int value)
 {
        struct gpio_intel_softc *sc = device_get_softc(dev);
+       struct pin_io_map *map = (struct pin_io_map *)cookie;
 
-        /* This operation mustn't fail, otherwise ACPI would be in trouble */
-       KKASSERT(gpio_intel_pin_exists(sc, pin));
+       KKASSERT(map->pin >= 0);
+       KKASSERT(gpio_intel_pin_exists(sc, map->pin));
 
        lockmgr(&sc->lk, LK_EXCLUSIVE);
-       sc->fns->write_pin(sc, pin, value);
+       sc->fns->write_pin(sc, map->pin, value);
        lockmgr(&sc->lk, LK_RELEASE);
 }
 
@@ -316,9 +385,9 @@ gpio_intel_intr(void *arg)
 {
        struct gpio_intel_softc *sc = (struct gpio_intel_softc *)arg;
 
-        lockmgr(&sc->lk, LK_EXCLUSIVE);
+       lockmgr(&sc->lk, LK_EXCLUSIVE);
        sc->fns->intr(arg);
-        lockmgr(&sc->lk, LK_RELEASE);
+       lockmgr(&sc->lk, LK_RELEASE);
 }
 
 static int
@@ -345,6 +414,8 @@ static device_method_t gpio_intel_methods[] = {
        DEVMETHOD(gpio_free_intr, gpio_intel_free_intr),
        DEVMETHOD(gpio_setup_intr, gpio_intel_setup_intr),
        DEVMETHOD(gpio_teardown_intr, gpio_intel_teardown_intr),
+       DEVMETHOD(gpio_alloc_io_pin, gpio_intel_alloc_io_pin),
+       DEVMETHOD(gpio_release_io_pin, gpio_intel_release_io_pin),
        DEVMETHOD(gpio_read_pin, gpio_intel_read_pin),
        DEVMETHOD(gpio_write_pin, gpio_intel_write_pin),
 
index b248416..286dde9 100644 (file)
@@ -16,6 +16,11 @@ struct pin_intr_map {
        uint32_t orig_gpiocfg;
 };
 
+struct pin_io_map {
+       int pin;
+       int flags;
+};
+
 struct gpio_intel_softc {
        device_t dev;
        struct resource *mem_res;
@@ -24,6 +29,7 @@ struct gpio_intel_softc {
        struct lock     lk;
        struct pinrange *ranges;
        struct pin_intr_map intrmaps[16];
+       struct pin_io_map iomaps[128];
        void            *acpireg;
        struct gpio_intel_fns *fns;
 };
@@ -37,6 +43,8 @@ typedef       void(*gpio_intel_enable_intr_fn)(struct gpio_intel_softc *sc,
            struct pin_intr_map *map);
 typedef        void(*gpio_intel_disable_intr_fn)(struct gpio_intel_softc *sc,
            struct pin_intr_map *map);
+typedef int(*gpio_intel_check_io_pin_fn)(struct gpio_intel_softc *sc,
+           uint16_t pin, int flags);
 typedef        void(*gpio_intel_write_pin_fn)(struct gpio_intel_softc *sc,
            uint16_t pin, int value);
 typedef        int(*gpio_intel_read_pin_fn)(struct gpio_intel_softc *sc,
@@ -49,6 +57,7 @@ struct gpio_intel_fns {
        gpio_intel_unmap_intr_fn unmap_intr;
        gpio_intel_enable_intr_fn enable_intr;
        gpio_intel_disable_intr_fn disable_intr;
+       gpio_intel_check_io_pin_fn check_io_pin;
        gpio_intel_write_pin_fn write_pin;
        gpio_intel_read_pin_fn  read_pin;
 };