Attached below is a diff that adds GPIO support to wbsio(4) for the Nuvoton
NCT5104D variant which is found on most of the recent PC Engines APU boards,
(I'm personally developing this on an APU4).

I've hit a problem that it causes the kernel to crash when probing for
lm78(4) which can be found on /some/ wbsio(4) chips, but not this particular
variant, so there aren't any to find. If I comment out the "lm* at wbsio?" in
the kernel configuration or comment out the latter `config_found()` call I
added to attach the GPIO bus then the kernel boots fine.

The kernel panic I get is:

...
wbsio0 at isa0 port 0x2e/2: NCT5104D rev 0x53
panic: bus_space_map: bad bus space tag
Stopped at      db_enter+0x12:  popq    %r11
    TID    PID    UID     PRFLAGS     PFLAGS  CPU  COMMAND
*     0      0      0     0x10000      0x200    0K swapper
db_enter(9a1e376995dc152,100,10,ffffffff81f6aed8,202,8) at db_enter+0x12
panic(200,ffffffff81f6b168,1a,8,ffff8000000a3e58,ffffffff81cb6a60) at 
panic+0x120
bus_space_map(9a45ea3f225ed920,ffff8000000a3e58,ffffffff81f6b168,1a,ffffffff81c93da0,0)
 at bus_space_map+0x62
lm_wbsio_match(b99bd94bc1864cf1,ffffffff81c93da0,ffffffff81f6b080,1a,ffffffff81c93da0,ffffffff81c8fd76)
 at lm_wbsio_match+0x4d
mapply(9766dee4470b4208,ffffffff81c93da0,ffff8000000a3e00,1a,ffffffff81f6b080,ffffffff81c93c18)
 at mapply+0xbe
config_search(eef789b22a8888bf,ffffffff81f6b168,ffffffff810b7d40,0,ffff8000000a3e00,ffffffff81c331f0)
 at config_search+0xc0
config_found_sm(7a3bdef80501ee63,2e,0,ffff8000000a3e00,ffffffff814b088e,ffffffff81f6b110)
 at config_found_sm+0x2e
wbsio_attach(370b5c3cf5b5f4cc,ffff8000000a3c00,ffffffff81f6b558,ffffffff81c73448,ffff8000000a3e00,ffff8000000a3e24)
 at wbsio_attach+0x8de
config_attach(d3e35ce7cfd9a5c1,ffffffff81c93c18,ffff8000000a3e00,ffffffff81f6b888,ffff8000000a3c00,ffffffff81f6b558)
 at config_attach+0x1ee
isascan(370b5c3cf5b5fae4,ffffffff8155f110,ffff8000000a3c00,ffffffff81c8cc01,ffffffff81c93c18,ffffffff81c8fdfc)
 at isascan+0x1fb
config_scan(51e6e319330389e6,ffff800000084b00,ffffffff81f6bc98,ffffffff81c8ccf8,ffff8000000a3c00,ffff8000000a3c24)
 at config_scan+0xb6
config_attach(d3e35ce7cfa11aa1,ffff80000008b2c0,ffff80000001cf00,ffffffff81d2ecd8,0,ffff80000001cf24)
 at config_attach+0x1ee
pcib_callback(b0d20c41cc678cce,30193439aff14287,0,ffffffff81c740e8,ffffffff81c33320,ffffffff81c331f0)
 at pcib_callback+0x5b
config_process_deferred_children(d810f4a0da8adfd1,ffff800000023100,ffffffff81f6bd90,ffffffff81c74730,ffff80000001cf00,0)
 at config_process_deferred_children+0
x89
end trace frame: 0xffffffff81f6bd60, count: 0
https://www.openbsd.org/ddb.html describes the minimum info required in bug
reports.  Insufficient info makes it difficult to find and fix bugs.
ddb{0}> 

So it crashes in lm_wbsio_match() which is in lm78_isa.c. I'm not sure why
bus_space_map() is failing, the GPIO portion doesn't map any I/O space, it's
all driven through the standard chip configuration registers.

Could someone possibly indicate what I've done wrong here? I've based the
GPIO attachment code on the same thing I did when implementing skgpio(4) and
as I mentioned, if I disable lm78(4) then the GPIO driver appears to attach
and function. I suspect I've got some subtlety of autoconf wrong.

Thanks

Matt

diff --git a/sys/arch/amd64/conf/GENERIC b/sys/arch/amd64/conf/GENERIC
index 9a74efca601..5aaa1f55651 100644
--- a/sys/arch/amd64/conf/GENERIC
+++ b/sys/arch/amd64/conf/GENERIC
@@ -123,6 +123,7 @@ schsio*     at isa? port 0x164e
 wbsio* at isa? port 0x2e       # Winbond LPC Super I/O
 wbsio* at isa? port 0x4e
 lm*    at wbsio?
+gpio*  at wbsio?
 uguru0 at isa? disable port 0xe0       # ABIT uGuru
 
 aps0   at isa? port 0x1600             # ThinkPad Active Protection System
diff --git a/sys/arch/i386/conf/GENERIC b/sys/arch/i386/conf/GENERIC
index 40dc421cf45..e2349208064 100644
--- a/sys/arch/i386/conf/GENERIC
+++ b/sys/arch/i386/conf/GENERIC
@@ -147,6 +147,7 @@ viasio*     at isa? port 0x4e flags 0x0000
 wbsio* at isa? port 0x2e               # Winbond LPC Super I/O
 wbsio* at isa? port 0x4e
 lm*    at wbsio?
+gpio*  at wbsio?
 uguru0 at isa? disable port 0xe0       # ABIT uGuru
 fins0  at isa? port 0x4e               # Fintek F71805 Super I/O
 
diff --git a/sys/dev/isa/files.isa b/sys/dev/isa/files.isa
index 65877402ccf..f0fe6fc4e37 100644
--- a/sys/dev/isa/files.isa
+++ b/sys/dev/isa/files.isa
@@ -248,7 +248,7 @@ attach      lpt at isa with lpt_isa
 file   dev/isa/lpt_isa.c               lpt_isa
 
 # Winbond LPC Super I/O
-device wbsio {}
+device wbsio {} : gpiobus
 attach wbsio at isa
 file   dev/isa/wbsio.c                 wbsio
 
diff --git a/sys/dev/isa/wbsio.c b/sys/dev/isa/wbsio.c
index 7147d481732..31ad1cecec0 100644
--- a/sys/dev/isa/wbsio.c
+++ b/sys/dev/isa/wbsio.c
@@ -23,12 +23,15 @@
 #include <sys/device.h>
 #include <sys/kernel.h>
 #include <sys/systm.h>
+#include <sys/gpio.h>
 
 #include <machine/bus.h>
 
 #include <dev/isa/isavar.h>
 #include <dev/isa/wbsioreg.h>
 
+#include <dev/gpio/gpiovar.h>
+
 #ifdef WBSIO_DEBUG
 #define DPRINTF(x) printf x
 #else
@@ -40,12 +43,21 @@ struct wbsio_softc {
 
        bus_space_tag_t         sc_iot;
        bus_space_handle_t      sc_ioh;
+
+       struct gpio_chipset_tag sc_gpio_gc;
+       gpio_pin_t              sc_gpio_pins[WBSIO_GPIO_NPINS];
 };
 
 int    wbsio_probe(struct device *, void *, void *);
 void   wbsio_attach(struct device *, struct device *, void *);
 int    wbsio_print(void *, const char *);
 
+int    wbsio_gpio_pin_to_port(struct wbsio_softc *, int);
+int    wbsio_gpio_pin_read(struct wbsio_softc *, int);
+int    wbsio_gpio_read(void *, int);
+void   wbsio_gpio_write(void *, int, int);
+void   wbsio_gpio_ctl(void *, int, int);
+
 struct cfattach wbsio_ca = {
        sizeof(struct wbsio_softc),
        wbsio_probe,
@@ -129,8 +141,10 @@ wbsio_attach(struct device *parent, struct device *self, 
void *aux)
        struct wbsio_softc *sc = (void *)self;
        struct isa_attach_args *ia = aux;
        struct isa_attach_args nia;
-       u_int8_t devid, reg, reg0, reg1;
+       struct gpiobus_attach_args gba;
+       u_int8_t devid, reg, reg0, reg1, mf;
        u_int16_t iobase;
+       int i, npins = 0, port;
 
        /* Map ISA I/O space */
        sc->sc_iot = ia->ia_iot;
@@ -197,17 +211,103 @@ wbsio_attach(struct device *parent, struct device *self, 
void *aux)
 
        printf("\n");
 
-       /* Escape from configuration mode */
-       wbsio_conf_disable(sc->sc_iot, sc->sc_ioh);
+       if (iobase) {
+               nia = *ia;
+               nia.ia_iobase = iobase;
+               nia.ia_aux = (void *)(u_long)devid; /* pass devid down to 
wb_match() */
 
-       if (iobase == 0)
-               return;
+               config_found(self, &nia, wbsio_print);
+       }
+
+       if (devid != WBSIO_ID_NCT5104D)
+               goto done;
+
+       /* Read Multi Function Register */
+       mf = wbsio_conf_read(sc->sc_iot, sc->sc_ioh, WBSIO_MF);
+
+       /* Select GPIO logical device */
+       wbsio_conf_write(sc->sc_iot, sc->sc_ioh, WBSIO_LDN, WBSIO_LDN_GPIO1);
+       reg = wbsio_conf_read(sc->sc_iot, sc->sc_ioh, WBSIO_LDN_EN);
+
+       /* UART C is inactive, use the pins as GPIO instead */
+       if ((mf & WBSIO_MF_UARTC) == 0) {
+               reg |= WBSIO_GPIO0_EN;
+               npins += WBSIO_GPIO0_NPINS;
+       }
+
+       /* UART D is inactive, use the pins as GPIO instead */
+       if ((mf & WBSIO_MF_UARTD) == 0) {
+               reg |= WBSIO_GPIO1_EN;
+               npins += WBSIO_GPIO1_NPINS;
+       }
+
+       /* GP67 is available, use it as a GPIO pin */
+       if (((wbsio_conf_read(sc->sc_iot, sc->sc_ioh, WBSIO_CR27) &
+           (1 << 2)) == 0) && (mf & WBSIO_MF_GP67)) {
+               reg |= WBSIO_GPIO6_EN;
+               npins += WBSIO_GPIO6_NPINS;
+       }
+
+       if (npins == 0)
+               goto done;
+
+       /* Enable the GPIO pins */
+       wbsio_conf_write(sc->sc_iot, sc->sc_ioh, WBSIO_LDN_EN, reg);
+
+       for (i = 0; i < npins; i++) {
+               sc->sc_gpio_pins[i].pin_num = i;
+               sc->sc_gpio_pins[i].pin_caps = GPIO_PIN_INPUT |
+                   GPIO_PIN_OUTPUT | GPIO_PIN_OPENDRAIN | GPIO_PIN_PUSHPULL |
+                   GPIO_PIN_INVIN | GPIO_PIN_INVOUT;
+
+               /* Work out the GPIO owning this pin */
+               port = wbsio_gpio_pin_to_port(sc, i);
+
+               /* Read existing I/O direction */
+               reg = wbsio_conf_read(sc->sc_iot, sc->sc_ioh,
+                   WBSIO_GPIO_IO + (port << 2));
+               sc->sc_gpio_pins[i].pin_flags = (reg & (1 << (i % 8))) ?
+                    GPIO_PIN_INPUT : GPIO_PIN_OUTPUT;
+
+               /* Read existing inversion status */
+               reg = wbsio_conf_read(sc->sc_iot, sc->sc_ioh,
+                   WBSIO_GPIO_INVERT + (port << 2));
+               sc->sc_gpio_pins[i].pin_flags |= (reg & (1 << (i % 8))) ?
+                   GPIO_PIN_INVIN | GPIO_PIN_INVOUT : 0;
+
+               /* Read existing push-pull/open drain. Note the logical
+                * device change to read this
+                */
+               wbsio_conf_write(sc->sc_iot, sc->sc_ioh, WBSIO_LDN,
+                   WBSIO_LDN_GPIO3);
+               reg = wbsio_conf_read(sc->sc_iot, sc->sc_ioh,
+                   WBSIO_GPIO_PP_OD + port);
+               sc->sc_gpio_pins[i].pin_flags |= (reg & (1 << (i % 8))) ?
+                   GPIO_PIN_OPENDRAIN : GPIO_PIN_PUSHPULL;
 
-       nia = *ia;
-       nia.ia_iobase = iobase;
-       nia.ia_aux = (void *)(u_long)devid; /* pass devid down to wb_match() */
+               /* Read existing pin state switching back to the original
+                * logical device again
+                */
+               wbsio_conf_write(sc->sc_iot, sc->sc_ioh, WBSIO_LDN,
+                   WBSIO_LDN_GPIO1);
+               sc->sc_gpio_pins[i].pin_state = wbsio_gpio_pin_read(sc, i);
+       }
+
+       sc->sc_gpio_gc.gp_cookie = sc;
+       sc->sc_gpio_gc.gp_pin_read = wbsio_gpio_read;
+       sc->sc_gpio_gc.gp_pin_write = wbsio_gpio_write;
+       sc->sc_gpio_gc.gp_pin_ctl = wbsio_gpio_ctl;
+
+       gba.gba_name = "gpio";
+       gba.gba_gc = &sc->sc_gpio_gc;
+       gba.gba_pins = sc->sc_gpio_pins;
+       gba.gba_npins = npins;
 
-       config_found(self, &nia, wbsio_print);
+       config_found(&sc->sc_dev, &gba, gpiobus_print);
+
+done:
+       /* Escape from configuration mode */
+       wbsio_conf_disable(sc->sc_iot, sc->sc_ioh);
 }
 
 int
@@ -223,3 +323,146 @@ wbsio_print(void *aux, const char *pnp)
                printf("/%d", ia->ia_iosize);
        return (UNCONF);
 }
+
+/* Assumes the chip is unlocked and the correct logical device is selected */
+int
+wbsio_gpio_pin_to_port(struct wbsio_softc *sc, int pin)
+{
+       u_int8_t reg;
+       int i, c = 8;
+
+       /* Read the enabled GPIO ports, mask out any other potential bits */
+       reg = wbsio_conf_read(sc->sc_iot, sc->sc_ioh, WBSIO_LDN_EN) &
+           (WBSIO_GPIO0_EN | WBSIO_GPIO1_EN | WBSIO_GPIO6_EN);
+
+       for (i = 0; i < (pin / 8); i++)
+               reg &= reg - 1;
+
+       reg &= ~(reg - 1);
+       if (reg) c--;
+       if (reg & 0x0f) c -= 4;
+       if (reg & 0x33) c -= 2;
+       if (reg & 0x55) c -= 1;
+
+       return (c);
+}
+
+int
+wbsio_gpio_pin_read(struct wbsio_softc *sc, int pin)
+{
+       return (wbsio_conf_read(sc->sc_iot, sc->sc_ioh,
+           WBSIO_GPIO_DATA + (wbsio_gpio_pin_to_port(sc, pin) << 2)) &
+           (1 << (pin % 8))) ? GPIO_PIN_HIGH : GPIO_PIN_LOW;
+}
+
+int
+wbsio_gpio_read(void *arg, int pin)
+{
+       struct wbsio_softc *sc = arg;
+       int value;
+
+       /* Enter configuration mode */
+       wbsio_conf_enable(sc->sc_iot, sc->sc_ioh);
+
+       /* Select GPIO logical device */
+       wbsio_conf_write(sc->sc_iot, sc->sc_ioh, WBSIO_LDN, WBSIO_LDN_GPIO1);
+
+       /* Read pin state */
+       value = wbsio_gpio_pin_read(sc, pin);
+
+       /* Escape from configuration mode */
+       wbsio_conf_disable(sc->sc_iot, sc->sc_ioh);
+
+       return (value);
+}
+
+void
+wbsio_gpio_write(void *arg, int pin, int value)
+{
+       struct wbsio_softc *sc = arg;
+       u_int8_t data;
+       int port;
+
+       /* Enter configuration mode */
+       wbsio_conf_enable(sc->sc_iot, sc->sc_ioh);
+
+       /* Select GPIO logical device */
+       wbsio_conf_write(sc->sc_iot, sc->sc_ioh, WBSIO_LDN, WBSIO_LDN_GPIO1);
+
+       /* Work out the GPIO owning this pin */
+       port = wbsio_gpio_pin_to_port(sc, pin);
+
+       /* Write pin state */
+       data = wbsio_conf_read(sc->sc_iot, sc->sc_ioh,
+           WBSIO_GPIO_DATA + (port << 2));
+
+       if (value == GPIO_PIN_LOW)
+               data &= ~(1 << (pin % 8));
+       else if (value == GPIO_PIN_HIGH)
+               data |= (1 << (pin % 8));
+
+       wbsio_conf_write(sc->sc_iot, sc->sc_ioh, WBSIO_GPIO_DATA + (port << 2),
+           data);
+
+       /* Escape from configuration mode */
+       wbsio_conf_disable(sc->sc_iot, sc->sc_ioh);
+}
+
+void
+wbsio_gpio_ctl(void *arg, int pin, int flags)
+{
+       struct wbsio_softc *sc = arg;
+       u_int8_t data;
+       int port;
+
+       /* Enter configuration mode */
+       wbsio_conf_enable(sc->sc_iot, sc->sc_ioh);
+
+       /* Select GPIO logical device */
+       wbsio_conf_write(sc->sc_iot, sc->sc_ioh, WBSIO_LDN, WBSIO_LDN_GPIO1);
+
+       /* Work out the GPIO owning this pin */
+       port = wbsio_gpio_pin_to_port(sc, pin);
+
+       /* Set I/O direction */
+       data = wbsio_conf_read(sc->sc_iot, sc->sc_ioh,
+           WBSIO_GPIO_IO + (port << 2));
+
+       if (flags & GPIO_PIN_INPUT)
+               data |= (1 << (pin % 8));
+       if (flags & GPIO_PIN_OUTPUT)
+               data &= ~(1 << (pin % 8));
+
+       wbsio_conf_write(sc->sc_iot, sc->sc_ioh, WBSIO_GPIO_IO + (port << 2),
+           data);
+
+       /* Set inversion */
+       data = wbsio_conf_read(sc->sc_iot, sc->sc_ioh,
+           WBSIO_GPIO_INVERT + (port << 2));
+
+       if ((flags & (GPIO_PIN_INPUT | GPIO_PIN_INVIN)) ||
+           (flags & (GPIO_PIN_OUTPUT | GPIO_PIN_INVOUT)))
+               data |= (1 << (pin % 8));
+       else
+               data &= ~(1 << (pin % 8));
+
+       wbsio_conf_write(sc->sc_iot, sc->sc_ioh,
+           WBSIO_GPIO_INVERT + (port << 2), data);
+
+       /* Set push-pull/open drain */
+       wbsio_conf_write(sc->sc_iot, sc->sc_ioh, WBSIO_LDN, WBSIO_LDN_GPIO3);
+
+       data = wbsio_conf_read(sc->sc_iot, sc->sc_ioh,
+           WBSIO_GPIO_PP_OD + port);
+
+       if (flags & GPIO_PIN_OPENDRAIN)
+               flags |= (1 << (pin % 8));
+       if (flags & GPIO_PIN_PUSHPULL)
+               flags &= ~(1 << (pin % 8));
+
+       wbsio_conf_write(sc->sc_iot, sc->sc_ioh, WBSIO_GPIO_PP_OD + port,
+           data);
+
+       /* Escape from configuration mode */
+       wbsio_conf_disable(sc->sc_iot, sc->sc_ioh);
+}
diff --git a/sys/dev/isa/wbsioreg.h b/sys/dev/isa/wbsioreg.h
index 574a19f358b..ba8355a61c5 100644
--- a/sys/dev/isa/wbsioreg.h
+++ b/sys/dev/isa/wbsioreg.h
@@ -30,8 +30,15 @@
 
 /* Configuration Space Registers */
 #define WBSIO_LDN              0x07    /* Logical Device Number */
+#define WBSIO_MF               0x1c    /* Multi Function Selection */
+#define WBSIO_MF_UARTD         (1 << 2)
+#define WBSIO_MF_UARTC         (1 << 3)
+#define WBSIO_MF_GP67          (1 << 4)
 #define WBSIO_ID               0x20    /* Device ID */
 #define WBSIO_REV              0x21    /* Device Revision */
+#define WBSIO_CR27             0x27    /* Global Option */
+#define WBSIO_SF               0x2f    /* Strapping Function */
+#define WBSIO_LDN_EN           0x30    /* Logical Device Enable */
 
 #define WBSIO_ID_W83627HF      0x52
 #define WBSIO_ID_W83627THF     0x82
@@ -46,8 +53,29 @@
 #define WBSIO_ID_NCT5104D      0xc4
 
 /* Logical Device Number (LDN) Assignments */
+#define WBSIO_LDN_GPIO1                0x07
+#define WBSIO_LDN_GPIO2                0x09    /* Not used */
 #define WBSIO_LDN_HM           0x0b
+#define WBSIO_LDN_GPIO3                0x0f
 
 /* Hardware Monitor Control Registers (LDN B) */
 #define WBSIO_HM_ADDR_MSB      0x60    /* Address [15:8] */
 #define WBSIO_HM_ADDR_LSB      0x61    /* Address [7:0] */
+
+/* GPIO Control Registers (LDN 7) */
+/* GPIOn registers are offset by n*4 bytes */
+#define WBSIO_GPIO_IO          0xe0    /* GPIO Direction */
+#define WBSIO_GPIO_DATA                0xe1    /* GPIO Data */
+#define WBSIO_GPIO_INVERT      0xe2    /* GPIO Invert */
+#define WBSIO_GPIO_STATUS      0xe3    /* GPIO Status */
+#define WBSIO_GPIO0_EN         (1 << 0)
+#define WBSIO_GPIO1_EN         (1 << 1)
+#define WBSIO_GPIO6_EN         (1 << 6)
+#define WBSIO_GPIO0_NPINS      8
+#define WBSIO_GPIO1_NPINS      8
+#define WBSIO_GPIO6_NPINS      1
+#define WBSIO_GPIO_NPINS       
(WBSIO_GPIO0_NPINS+WBSIO_GPIO1_NPINS+WBSIO_GPIO6_NPINS)
+
+/* GPIO Control Registers (LDN F) */
+/* GPIOn register is offset by n bytes */
+#define WBSIO_GPIO_PP_OD       0xe0    /* GPIO Push-Pull/Open Drain */

Reply via email to