pinctrl: mcp23s08: switch to regmap caching
authorSebastian Reichel <sebastian.reichel@collabora.co.uk>
Mon, 15 May 2017 09:24:28 +0000 (11:24 +0200)
committerLinus Walleij <linus.walleij@linaro.org>
Tue, 23 May 2017 07:47:28 +0000 (09:47 +0200)
Instead of using custom caching, this switches to regmap based
caching. Before the conversion the debugfs file used uncached
values, so that it was easily possible to see power-loss related
problems. The new code will check and recover at this place.

The patch will also ensure, that irqs are not cleared by checking
register status in debugfs.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
drivers/pinctrl/pinctrl-mcp23s08.c

index 8c684d1..be7ec7d 100644 (file)
@@ -67,14 +67,13 @@ struct mcp23s08 {
        bool                    irq_active_high;
        bool                    reg_shift;
 
-       u16                     cache[11];
        u16                     irq_rise;
        u16                     irq_fall;
        int                     irq;
        bool                    irq_controller;
-       /* lock protects the cached values */
+       int                     cached_gpio;
+       /* lock protects regmap access with bypass/cache flags */
        struct mutex            lock;
-       struct mutex            irq_lock;
 
        struct gpio_chip        chip;
 
@@ -85,20 +84,92 @@ struct mcp23s08 {
        struct pinctrl_desc     pinctrl_desc;
 };
 
+static const struct reg_default mcp23x08_defaults[] = {
+       {.reg = MCP_IODIR,              .def = 0xff},
+       {.reg = MCP_IPOL,               .def = 0x00},
+       {.reg = MCP_GPINTEN,            .def = 0x00},
+       {.reg = MCP_DEFVAL,             .def = 0x00},
+       {.reg = MCP_INTCON,             .def = 0x00},
+       {.reg = MCP_IOCON,              .def = 0x00},
+       {.reg = MCP_GPPU,               .def = 0x00},
+       {.reg = MCP_OLAT,               .def = 0x00},
+};
+
+static const struct regmap_range mcp23x08_volatile_range = {
+       .range_min = MCP_INTF,
+       .range_max = MCP_GPIO,
+};
+
+static const struct regmap_access_table mcp23x08_volatile_table = {
+       .yes_ranges = &mcp23x08_volatile_range,
+       .n_yes_ranges = 1,
+};
+
+static const struct regmap_range mcp23x08_precious_range = {
+       .range_min = MCP_GPIO,
+       .range_max = MCP_GPIO,
+};
+
+static const struct regmap_access_table mcp23x08_precious_table = {
+       .yes_ranges = &mcp23x08_precious_range,
+       .n_yes_ranges = 1,
+};
+
 static const struct regmap_config mcp23x08_regmap = {
        .reg_bits = 8,
        .val_bits = 8,
 
        .reg_stride = 1,
+       .volatile_table = &mcp23x08_volatile_table,
+       .precious_table = &mcp23x08_precious_table,
+       .reg_defaults = mcp23x08_defaults,
+       .num_reg_defaults = ARRAY_SIZE(mcp23x08_defaults),
+       .cache_type = REGCACHE_FLAT,
        .max_register = MCP_OLAT,
 };
 
+static const struct reg_default mcp23x16_defaults[] = {
+       {.reg = MCP_IODIR << 1,         .def = 0xffff},
+       {.reg = MCP_IPOL << 1,          .def = 0x0000},
+       {.reg = MCP_GPINTEN << 1,       .def = 0x0000},
+       {.reg = MCP_DEFVAL << 1,        .def = 0x0000},
+       {.reg = MCP_INTCON << 1,        .def = 0x0000},
+       {.reg = MCP_IOCON << 1,         .def = 0x0000},
+       {.reg = MCP_GPPU << 1,          .def = 0x0000},
+       {.reg = MCP_OLAT << 1,          .def = 0x0000},
+};
+
+static const struct regmap_range mcp23x16_volatile_range = {
+       .range_min = MCP_INTF << 1,
+       .range_max = MCP_GPIO << 1,
+};
+
+static const struct regmap_access_table mcp23x16_volatile_table = {
+       .yes_ranges = &mcp23x16_volatile_range,
+       .n_yes_ranges = 1,
+};
+
+static const struct regmap_range mcp23x16_precious_range = {
+       .range_min = MCP_GPIO << 1,
+       .range_max = MCP_GPIO << 1,
+};
+
+static const struct regmap_access_table mcp23x16_precious_table = {
+       .yes_ranges = &mcp23x16_precious_range,
+       .n_yes_ranges = 1,
+};
+
 static const struct regmap_config mcp23x17_regmap = {
        .reg_bits = 8,
        .val_bits = 16,
 
        .reg_stride = 2,
        .max_register = MCP_OLAT << 1,
+       .volatile_table = &mcp23x16_volatile_table,
+       .precious_table = &mcp23x16_precious_table,
+       .reg_defaults = mcp23x16_defaults,
+       .num_reg_defaults = ARRAY_SIZE(mcp23x16_defaults),
+       .cache_type = REGCACHE_FLAT,
        .val_format_endian = REGMAP_ENDIAN_LITTLE,
 };
 
@@ -112,27 +183,19 @@ static int mcp_write(struct mcp23s08 *mcp, unsigned int reg, unsigned int val)
        return regmap_write(mcp->regmap, reg << mcp->reg_shift, val);
 }
 
-static int mcp_set_bit(struct mcp23s08 *mcp, unsigned int reg,
-                      unsigned int pin, bool enabled)
+static int mcp_set_mask(struct mcp23s08 *mcp, unsigned int reg,
+                      unsigned int mask, bool enabled)
 {
        u16 val  = enabled ? 0xffff : 0x0000;
-       u16 mask = BIT(pin);
        return regmap_update_bits(mcp->regmap, reg << mcp->reg_shift,
                                  mask, val);
 }
 
-static int mcp_update_cache(struct mcp23s08 *mcp)
+static int mcp_set_bit(struct mcp23s08 *mcp, unsigned int reg,
+                      unsigned int pin, bool enabled)
 {
-       int ret, reg, i;
-
-       for (i = 0; i < ARRAY_SIZE(mcp->cache); i++) {
-               ret = mcp_read(mcp, i, &reg);
-               if (ret < 0)
-                       return ret;
-               mcp->cache[i] = reg;
-       }
-
-       return 0;
+       u16 mask = BIT(pin);
+       return mcp_set_mask(mcp, reg, mask, enabled);
 }
 
 static const struct pinctrl_pin_desc mcp23x08_pins[] = {
@@ -336,9 +399,9 @@ static int mcp23s08_direction_input(struct gpio_chip *chip, unsigned offset)
        int status;
 
        mutex_lock(&mcp->lock);
-       mcp->cache[MCP_IODIR] |= (1 << offset);
-       status = mcp_write(mcp, MCP_IODIR, mcp->cache[MCP_IODIR]);
+       status = mcp_set_bit(mcp, MCP_IODIR, offset, true);
        mutex_unlock(&mcp->lock);
+
        return status;
 }
 
@@ -353,33 +416,27 @@ static int mcp23s08_get(struct gpio_chip *chip, unsigned offset)
        ret = mcp_read(mcp, MCP_GPIO, &status);
        if (ret < 0)
                status = 0;
-       else {
-               mcp->cache[MCP_GPIO] = status;
+       else
                status = !!(status & (1 << offset));
-       }
+
+       mcp->cached_gpio = status;
+
        mutex_unlock(&mcp->lock);
        return status;
 }
 
-static int __mcp23s08_set(struct mcp23s08 *mcp, unsigned mask, int value)
+static int __mcp23s08_set(struct mcp23s08 *mcp, unsigned mask, bool value)
 {
-       unsigned olat = mcp->cache[MCP_OLAT];
-
-       if (value)
-               olat |= mask;
-       else
-               olat &= ~mask;
-       mcp->cache[MCP_OLAT] = olat;
-       return mcp_write(mcp, MCP_OLAT, olat);
+       return mcp_set_mask(mcp, MCP_OLAT, mask, value);
 }
 
 static void mcp23s08_set(struct gpio_chip *chip, unsigned offset, int value)
 {
        struct mcp23s08 *mcp = gpiochip_get_data(chip);
-       unsigned mask = 1 << offset;
+       unsigned mask = BIT(offset);
 
        mutex_lock(&mcp->lock);
-       __mcp23s08_set(mcp, mask, value);
+       __mcp23s08_set(mcp, mask, !!value);
        mutex_unlock(&mcp->lock);
 }
 
@@ -387,14 +444,13 @@ static int
 mcp23s08_direction_output(struct gpio_chip *chip, unsigned offset, int value)
 {
        struct mcp23s08 *mcp = gpiochip_get_data(chip);
-       unsigned mask = 1 << offset;
+       unsigned mask = BIT(offset);
        int status;
 
        mutex_lock(&mcp->lock);
        status = __mcp23s08_set(mcp, mask, value);
        if (status == 0) {
-               mcp->cache[MCP_IODIR] &= ~mask;
-               status = mcp_write(mcp, MCP_IODIR, mcp->cache[MCP_IODIR]);
+               status = mcp_set_mask(mcp, MCP_IODIR, mask, false);
        }
        mutex_unlock(&mcp->lock);
        return status;
@@ -404,7 +460,7 @@ mcp23s08_direction_output(struct gpio_chip *chip, unsigned offset, int value)
 static irqreturn_t mcp23s08_irq(int irq, void *data)
 {
        struct mcp23s08 *mcp = data;
-       int intcap, intf, i, gpio, gpio_orig, intcap_mask;
+       int intcap, intcon, intf, i, gpio, gpio_orig, intcap_mask, defval;
        unsigned int child_irq;
        bool intf_set, intcap_changed, gpio_bit_changed,
                defval_changed, gpio_set;
@@ -415,25 +471,31 @@ static irqreturn_t mcp23s08_irq(int irq, void *data)
                return IRQ_HANDLED;
        }
 
-       mcp->cache[MCP_INTF] = intf;
-
        if (mcp_read(mcp, MCP_INTCAP, &intcap) < 0) {
                mutex_unlock(&mcp->lock);
                return IRQ_HANDLED;
        }
 
-       mcp->cache[MCP_INTCAP] = intcap;
+       if (mcp_read(mcp, MCP_INTCON, &intcon) < 0) {
+               mutex_unlock(&mcp->lock);
+               return IRQ_HANDLED;
+       }
+
+       if (mcp_read(mcp, MCP_DEFVAL, &defval) < 0) {
+               mutex_unlock(&mcp->lock);
+               return IRQ_HANDLED;
+       }
 
        /* This clears the interrupt(configurable on S18) */
        if (mcp_read(mcp, MCP_GPIO, &gpio) < 0) {
                mutex_unlock(&mcp->lock);
                return IRQ_HANDLED;
        }
-       gpio_orig = mcp->cache[MCP_GPIO];
-       mcp->cache[MCP_GPIO] = gpio;
+       gpio_orig = mcp->cached_gpio;
+       mcp->cached_gpio = gpio;
        mutex_unlock(&mcp->lock);
 
-       if (mcp->cache[MCP_INTF] == 0) {
+       if (intf == 0) {
                /* There is no interrupt pending */
                return IRQ_HANDLED;
        }
@@ -461,7 +523,7 @@ static irqreturn_t mcp23s08_irq(int irq, void *data)
                 * to see if the input has changed.
                 */
 
-               intf_set = BIT(i) & mcp->cache[MCP_INTF];
+               intf_set = intf & BIT(i);
                if (i < 8 && intf_set)
                        intcap_mask = 0x00FF;
                else if (i >= 8 && intf_set)
@@ -470,14 +532,14 @@ static irqreturn_t mcp23s08_irq(int irq, void *data)
                        intcap_mask = 0x00;
 
                intcap_changed = (intcap_mask &
-                       (BIT(i) & mcp->cache[MCP_INTCAP])) !=
+                       (intcap & BIT(i))) !=
                        (intcap_mask & (BIT(i) & gpio_orig));
-               gpio_set = BIT(i) & mcp->cache[MCP_GPIO];
+               gpio_set = BIT(i) & gpio;
                gpio_bit_changed = (BIT(i) & gpio_orig) !=
-                       (BIT(i) & mcp->cache[MCP_GPIO]);
-               defval_changed = (BIT(i) & mcp->cache[MCP_INTCON]) &&
-                       ((BIT(i) & mcp->cache[MCP_GPIO]) !=
-                       (BIT(i) & mcp->cache[MCP_DEFVAL]));
+                       (BIT(i) & gpio);
+               defval_changed = (BIT(i) & intcon) &&
+                       ((BIT(i) & gpio) !=
+                       (BIT(i) & defval));
 
                if (((gpio_bit_changed || intcap_changed) &&
                        (BIT(i) & mcp->irq_rise) && gpio_set) ||
@@ -498,7 +560,7 @@ static void mcp23s08_irq_mask(struct irq_data *data)
        struct mcp23s08 *mcp = gpiochip_get_data(gc);
        unsigned int pos = data->hwirq;
 
-       mcp->cache[MCP_GPINTEN] &= ~BIT(pos);
+       mcp_set_bit(mcp, MCP_GPINTEN, pos, false);
 }
 
 static void mcp23s08_irq_unmask(struct irq_data *data)
@@ -507,7 +569,7 @@ static void mcp23s08_irq_unmask(struct irq_data *data)
        struct mcp23s08 *mcp = gpiochip_get_data(gc);
        unsigned int pos = data->hwirq;
 
-       mcp->cache[MCP_GPINTEN] |= BIT(pos);
+       mcp_set_bit(mcp, MCP_GPINTEN, pos, true);
 }
 
 static int mcp23s08_irq_set_type(struct irq_data *data, unsigned int type)
@@ -518,23 +580,23 @@ static int mcp23s08_irq_set_type(struct irq_data *data, unsigned int type)
        int status = 0;
 
        if ((type & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH) {
-               mcp->cache[MCP_INTCON] &= ~BIT(pos);
+               mcp_set_bit(mcp, MCP_INTCON, pos, false);
                mcp->irq_rise |= BIT(pos);
                mcp->irq_fall |= BIT(pos);
        } else if (type & IRQ_TYPE_EDGE_RISING) {
-               mcp->cache[MCP_INTCON] &= ~BIT(pos);
+               mcp_set_bit(mcp, MCP_INTCON, pos, false);
                mcp->irq_rise |= BIT(pos);
                mcp->irq_fall &= ~BIT(pos);
        } else if (type & IRQ_TYPE_EDGE_FALLING) {
-               mcp->cache[MCP_INTCON] &= ~BIT(pos);
+               mcp_set_bit(mcp, MCP_INTCON, pos, false);
                mcp->irq_rise &= ~BIT(pos);
                mcp->irq_fall |= BIT(pos);
        } else if (type & IRQ_TYPE_LEVEL_HIGH) {
-               mcp->cache[MCP_INTCON] |= BIT(pos);
-               mcp->cache[MCP_DEFVAL] &= ~BIT(pos);
+               mcp_set_bit(mcp, MCP_INTCON, pos, true);
+               mcp_set_bit(mcp, MCP_DEFVAL, pos, false);
        } else if (type & IRQ_TYPE_LEVEL_LOW) {
-               mcp->cache[MCP_INTCON] |= BIT(pos);
-               mcp->cache[MCP_DEFVAL] |= BIT(pos);
+               mcp_set_bit(mcp, MCP_INTCON, pos, true);
+               mcp_set_bit(mcp, MCP_DEFVAL, pos, true);
        } else
                return -EINVAL;
 
@@ -546,7 +608,8 @@ static void mcp23s08_irq_bus_lock(struct irq_data *data)
        struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
        struct mcp23s08 *mcp = gpiochip_get_data(gc);
 
-       mutex_lock(&mcp->irq_lock);
+       mutex_lock(&mcp->lock);
+       regcache_cache_only(mcp->regmap, true);
 }
 
 static void mcp23s08_irq_bus_unlock(struct irq_data *data)
@@ -554,12 +617,10 @@ static void mcp23s08_irq_bus_unlock(struct irq_data *data)
        struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
        struct mcp23s08 *mcp = gpiochip_get_data(gc);
 
-       mutex_lock(&mcp->lock);
-       mcp_write(mcp, MCP_GPINTEN, mcp->cache[MCP_GPINTEN]);
-       mcp_write(mcp, MCP_DEFVAL, mcp->cache[MCP_DEFVAL]);
-       mcp_write(mcp, MCP_INTCON, mcp->cache[MCP_INTCON]);
+       regcache_cache_only(mcp->regmap, false);
+       regcache_sync(mcp->regmap);
+
        mutex_unlock(&mcp->lock);
-       mutex_unlock(&mcp->irq_lock);
 }
 
 static struct irq_chip mcp23s08_irq_chip = {
@@ -577,8 +638,6 @@ static int mcp23s08_irq_setup(struct mcp23s08 *mcp)
        int err;
        unsigned long irqflags = IRQF_ONESHOT | IRQF_SHARED;
 
-       mutex_init(&mcp->irq_lock);
-
        if (mcp->irq_active_high)
                irqflags |= IRQF_TRIGGER_HIGH;
        else
@@ -618,6 +677,47 @@ static int mcp23s08_irq_setup(struct mcp23s08 *mcp)
 #include <linux/seq_file.h>
 
 /*
+ * This compares the chip's registers with the register
+ * cache and corrects any incorrectly set register. This
+ * can be used to fix state for MCP23xxx, that temporary
+ * lost its power supply.
+ */
+#define MCP23S08_CONFIG_REGS 8
+static int __check_mcp23s08_reg_cache(struct mcp23s08 *mcp)
+{
+       int cached[MCP23S08_CONFIG_REGS];
+       int err = 0, i;
+
+       /* read cached config registers */
+       for (i = 0; i < MCP23S08_CONFIG_REGS; i++) {
+               err = mcp_read(mcp, i, &cached[i]);
+               if (err)
+                       goto out;
+       }
+
+       regcache_cache_bypass(mcp->regmap, true);
+
+       for (i = 0; i < MCP23S08_CONFIG_REGS; i++) {
+               int uncached;
+               err = mcp_read(mcp, i, &uncached);
+               if (err)
+                       goto out;
+
+               if (uncached != cached[i]) {
+                       dev_err(mcp->dev, "restoring reg 0x%02x from 0x%04x to 0x%04x (power-loss?)\n",
+                               i, uncached, cached[i]);
+                       mcp_write(mcp, i, cached[i]);
+               }
+       }
+
+out:
+       if (err)
+               dev_err(mcp->dev, "read error: reg=%02x, err=%d", i, err);
+       regcache_cache_bypass(mcp->regmap, false);
+       return err;
+}
+
+/*
  * This shows more info than the generic gpio dump code:
  * pullups, deglitching, open drain drive.
  */
@@ -627,6 +727,7 @@ static void mcp23s08_dbg_show(struct seq_file *s, struct gpio_chip *chip)
        char            bank;
        int             t;
        unsigned        mask;
+       int iodir, gpio, gppu;
 
        mcp = gpiochip_get_data(chip);
 
@@ -634,14 +735,30 @@ static void mcp23s08_dbg_show(struct seq_file *s, struct gpio_chip *chip)
        bank = '0' + ((mcp->addr >> 1) & 0x7);
 
        mutex_lock(&mcp->lock);
-       t = mcp_update_cache(mcp);
-       if (t < 0) {
-               seq_printf(s, " I/O ERROR %d\n", t);
+
+       t = __check_mcp23s08_reg_cache(mcp);
+       if (t) {
+               seq_printf(s, " I/O Error\n");
+               goto done;
+       }
+       t = mcp_read(mcp, MCP_IODIR, &iodir);
+       if (t) {
+               seq_printf(s, " I/O Error\n");
+               goto done;
+       }
+       t = mcp_read(mcp, MCP_GPIO, &gpio);
+       if (t) {
+               seq_printf(s, " I/O Error\n");
+               goto done;
+       }
+       t = mcp_read(mcp, MCP_GPPU, &gppu);
+       if (t) {
+               seq_printf(s, " I/O Error\n");
                goto done;
        }
 
-       for (t = 0, mask = 1; t < chip->ngpio; t++, mask <<= 1) {
-               const char      *label;
+       for (t = 0, mask = BIT(0); t < chip->ngpio; t++, mask <<= 1) {
+               const char *label;
 
                label = gpiochip_is_requested(chip, t);
                if (!label)
@@ -649,9 +766,9 @@ static void mcp23s08_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 
                seq_printf(s, " gpio-%-3d P%c.%d (%-12s) %s %s %s",
                        chip->base + t, bank, t, label,
-                       (mcp->cache[MCP_IODIR] & mask) ? "in " : "out",
-                       (mcp->cache[MCP_GPIO] & mask) ? "hi" : "lo",
-                       (mcp->cache[MCP_GPPU] & mask) ? "up" : "  ");
+                       (iodir & mask) ? "in " : "out",
+                       (gpio & mask) ? "hi" : "lo",
+                       (gppu & mask) ? "up" : "  ");
                /* NOTE:  ignoring the irq-related registers */
                seq_puts(s, "\n");
        }
@@ -782,26 +899,6 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
                        goto fail;
        }
 
-       ret = mcp_update_cache(mcp);
-       if (ret < 0)
-               goto fail;
-
-       /* disable inverter on input */
-       if (mcp->cache[MCP_IPOL] != 0) {
-               mcp->cache[MCP_IPOL] = 0;
-               ret = mcp_write(mcp, MCP_IPOL, 0);
-               if (ret < 0)
-                       goto fail;
-       }
-
-       /* disable irqs */
-       if (mcp->cache[MCP_GPINTEN] != 0) {
-               mcp->cache[MCP_GPINTEN] = 0;
-               ret = mcp_write(mcp, MCP_GPINTEN, 0);
-               if (ret < 0)
-                       goto fail;
-       }
-
        ret = gpiochip_add_data(&mcp->chip, mcp);
        if (ret < 0)
                goto fail;