[PATCHv3 00/14] mcp23s08 pinconf & cleanup

[PATCHv3 00/14] mcp23s08 pinconf & cleanup

[Sebastian Reichel]

Hi,

Here is a rebase of my PATCHv2 in combination with the second patchset, which
migrates to regmap based register caching. I also added a few more patches, that
do misc. cleanups in the driver and remove ~100 loc (that's 10%).

The first patch and the 12th patch ("simplify spi_present_mask handling") needs
Acked-by from Steven Miao for the blackfin architecture changes (defconfig, two
boardfiles) and from Vladimir Zapolskiy or Sylvain Lemieux for the lpc32xx_defconfig
changes.

Changes since PATCHv1:
 * Add patch moving mcp23s08 from gpio/ to pinctrl/
 * Add patches updating config references in arch/
 * Add patch removing pdata support for pullup config
Changes since PATCHv2:
 * rebase to v4.12-rc1
 * squash patch 1-4 for bisectability
 * use devm_pinctrl_register in patch adding pinctrl support
 * include patchset for regmap based caching
 * constify structs in patch switching to regmap caching
 * add new patches with more cleanups (-120 locs)

-- Sebastian

Sebastian Reichel (14):
  gpio: mcp23s08: move to pinctrl
  pinctrl: mcp23s08: add pinconf support
  pinctrl: mcp23s08: drop pullup config from pdata
  pinctrl: mcp23s08: switch to regmap caching
  pinctrl: mcp23s08: drop OF_GPIO dependency
  pinctrl: mcp23s08: irq mapping is already done
  pinctrl: mcp23s08: use managed kzalloc for mcp
  pinctrl: mcp23s08: switch to devm_gpiochip_add_data
  pinctrl: mcp23s08: simplify i2c pdata handling
  pinctrl: mcp23s08: simplify spi pdata handling
  pinctrl: mcp23s08: generalize irq property handling
  pinctrl: mcp23s08: simplify spi_present_mask handling
  pinctrl: mcp23s08: drop comment about missing irq support
  pinctrl: mcp23s08: fix comment for mcp23s08_platform_data.base

 arch/arm/configs/lpc32xx_defconfig                 |   2 +-
 arch/blackfin/configs/BF609-EZKIT_defconfig        |   2 +-
 arch/blackfin/mach-bf527/boards/tll6527m.c         |   8 +-
 arch/blackfin/mach-bf609/boards/ezkit.c            |   4 +-
 drivers/gpio/Kconfig                               |  17 -
 drivers/gpio/Makefile                              |   1 -
 drivers/pinctrl/Kconfig                            |  13 +
 drivers/pinctrl/Makefile                           |   1 +
 .../gpio-mcp23s08.c => pinctrl/pinctrl-mcp23s08.c} | 647 +++++++++++++--------
 include/linux/spi/mcp23s08.h                       |  38 +-
 10 files changed, 425 insertions(+), 308 deletions(-)
 rename drivers/{gpio/gpio-mcp23s08.c => pinctrl/pinctrl-mcp23s08.c} (64%)

-- 
2.11.0

[PATCHv3 01/14] gpio: mcp23s08: move to pinctrl

[Sebastian Reichel]

This moves the mcp23s08 driver from gpio to pinctrl. Actual
pinctrl support for configuration of the pull-up resistors
follows in its own patch.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 arch/arm/configs/lpc32xx_defconfig                      |  2 +-
 arch/blackfin/configs/BF609-EZKIT_defconfig             |  2 +-
 arch/blackfin/mach-bf527/boards/tll6527m.c              |  4 ++--
 arch/blackfin/mach-bf609/boards/ezkit.c                 |  4 ++--
 drivers/gpio/Kconfig                                    | 17 -----------------
 drivers/gpio/Makefile                                   |  1 -
 drivers/pinctrl/Kconfig                                 | 13 +++++++++++++
 drivers/pinctrl/Makefile                                |  1 +
 .../gpio-mcp23s08.c => pinctrl/pinctrl-mcp23s08.c}      |  0
 9 files changed, 20 insertions(+), 24 deletions(-)
 rename drivers/{gpio/gpio-mcp23s08.c => pinctrl/pinctrl-mcp23s08.c} (100%)

diff --git a/arch/arm/configs/lpc32xx_defconfig b/arch/arm/configs/lpc32xx_defconfig
index 6ba430d2b5b2..e15fa5f168bb 100644
--- a/arch/arm/configs/lpc32xx_defconfig
+++ b/arch/arm/configs/lpc32xx_defconfig
@@ -112,7 +112,7 @@ CONFIG_GPIO_SX150X=y
 CONFIG_GPIO_74X164=y
 CONFIG_GPIO_MAX7301=y
 CONFIG_GPIO_MC33880=y
-CONFIG_GPIO_MCP23S08=y
+CONFIG_PINCTRL_MCP23S08=y
 CONFIG_SENSORS_DS620=y
 CONFIG_SENSORS_MAX6639=y
 CONFIG_WATCHDOG=y
diff --git a/arch/blackfin/configs/BF609-EZKIT_defconfig b/arch/blackfin/configs/BF609-EZKIT_defconfig
index ba4267f658af..3ce77f07208a 100644
--- a/arch/blackfin/configs/BF609-EZKIT_defconfig
+++ b/arch/blackfin/configs/BF609-EZKIT_defconfig
@@ -105,7 +105,7 @@ CONFIG_SPI=y
 CONFIG_SPI_ADI_V3=y
 CONFIG_GPIOLIB=y
 CONFIG_GPIO_SYSFS=y
-CONFIG_GPIO_MCP23S08=y
+CONFIG_PINCTRL_MCP23S08=y
 # CONFIG_HWMON is not set
 CONFIG_WATCHDOG=y
 CONFIG_BFIN_WDT=y
diff --git a/arch/blackfin/mach-bf527/boards/tll6527m.c b/arch/blackfin/mach-bf527/boards/tll6527m.c
index c1acce4c2e45..be61477826f3 100644
--- a/arch/blackfin/mach-bf527/boards/tll6527m.c
+++ b/arch/blackfin/mach-bf527/boards/tll6527m.c
@@ -348,7 +348,7 @@ static struct platform_device bfin_i2s = {
 };
 #endif
 
-#if IS_ENABLED(CONFIG_GPIO_MCP23S08)
+#if IS_ENABLED(CONFIG_PINCTRL_MCP23S08)
 #include <linux/spi/mcp23s08.h>
 static const struct mcp23s08_platform_data bfin_mcp23s08_sys_gpio_info = {
 	.chip[0].is_present = true,
@@ -423,7 +423,7 @@ static struct spi_board_info bfin_spi_board_info[] __initdata = {
 		.mode = SPI_CPHA | SPI_CPOL,
 	},
 #endif
-#if IS_ENABLED(CONFIG_GPIO_MCP23S08)
+#if IS_ENABLED(CONFIG_PINCTRL_MCP23S08)
 	{
 		.modalias = "mcp23s08",
 		.platform_data = &bfin_mcp23s08_sys_gpio_info,
diff --git a/arch/blackfin/mach-bf609/boards/ezkit.c b/arch/blackfin/mach-bf609/boards/ezkit.c
index 9231e5a72b93..51157a255824 100644
--- a/arch/blackfin/mach-bf609/boards/ezkit.c
+++ b/arch/blackfin/mach-bf609/boards/ezkit.c
@@ -1887,7 +1887,7 @@ static struct platform_device i2c_bfin_twi1_device = {
 };
 #endif
 
-#if IS_ENABLED(CONFIG_GPIO_MCP23S08)
+#if IS_ENABLED(CONFIG_PINCTRL_MCP23S08)
 #include <linux/spi/mcp23s08.h>
 static const struct mcp23s08_platform_data bfin_mcp23s08_soft_switch0 = {
 	.base = 120,
@@ -1929,7 +1929,7 @@ static struct i2c_board_info __initdata bfin_i2c_board_info0[] = {
 		I2C_BOARD_INFO("ssm2602", 0x1b),
 	},
 #endif
-#if IS_ENABLED(CONFIG_GPIO_MCP23S08)
+#if IS_ENABLED(CONFIG_PINCTRL_MCP23S08)
 	{
 		I2C_BOARD_INFO("mcp23017", 0x21),
 		.platform_data = (void *)&bfin_mcp23s08_soft_switch0
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 23ca51ee6b28..5f88d7324e02 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1227,23 +1227,6 @@ config GPIO_PISOSR
 
 endmenu
 
-menu "SPI or I2C GPIO expanders"
-	depends on (SPI_MASTER && !I2C) || I2C
-
-config GPIO_MCP23S08
-	tristate "Microchip MCP23xxx I/O expander"
-	depends on OF_GPIO
-	select GPIOLIB_IRQCHIP
-	select REGMAP_I2C if I2C
-	select REGMAP if SPI_MASTER
-	help
-	  SPI/I2C driver for Microchip MCP23S08/MCP23S17/MCP23008/MCP23017
-	  I/O expanders.
-	  This provides a GPIO interface supporting inputs and outputs.
-	  The I2C versions of the chips can be used as interrupt-controller.
-
-endmenu
-
 menu "USB GPIO expanders"
 	depends on USB
 
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 68b96277d9fa..89f10061a5c1 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -77,7 +77,6 @@ obj-$(CONFIG_GPIO_MENZ127)	+= gpio-menz127.o
 obj-$(CONFIG_GPIO_MERRIFIELD)	+= gpio-merrifield.o
 obj-$(CONFIG_GPIO_MC33880)	+= gpio-mc33880.o
 obj-$(CONFIG_GPIO_MC9S08DZ60)	+= gpio-mc9s08dz60.o
-obj-$(CONFIG_GPIO_MCP23S08)	+= gpio-mcp23s08.o
 obj-$(CONFIG_GPIO_ML_IOH)	+= gpio-ml-ioh.o
 obj-$(CONFIG_GPIO_MM_LANTIQ)	+= gpio-mm-lantiq.o
 obj-$(CONFIG_GPIO_MOCKUP)      += gpio-mockup.o
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 37af5e3029d5..b5aa50c51633 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -146,6 +146,19 @@ config PINCTRL_FALCON
 	depends on SOC_FALCON
 	depends on PINCTRL_LANTIQ
 
+config PINCTRL_MCP23S08
+	tristate "Microchip MCP23xxx I/O expander"
+	depends on OF_GPIO
+	depends on SPI_MASTER || I2C
+	select GPIOLIB_IRQCHIP
+	select REGMAP_I2C if I2C
+	select REGMAP_SPI if SPI_MASTER
+	help
+	  SPI/I2C driver for Microchip MCP23S08/MCP23S17/MCP23008/MCP23017
+	  I/O expanders.
+	  This provides a GPIO interface supporting inputs and outputs.
+	  The I2C versions of the chips can be used as interrupt-controller.
+
 config PINCTRL_MESON
 	bool
 	depends on OF
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 0e9b2226a7c2..59d793aa3db3 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_PINCTRL_DA850_PUPD) += pinctrl-da850-pupd.o
 obj-$(CONFIG_PINCTRL_DIGICOLOR)	+= pinctrl-digicolor.o
 obj-$(CONFIG_PINCTRL_FALCON)	+= pinctrl-falcon.o
 obj-$(CONFIG_PINCTRL_MAX77620)	+= pinctrl-max77620.o
+obj-$(CONFIG_PINCTRL_MCP23S08)	+= pinctrl-mcp23s08.o
 obj-$(CONFIG_PINCTRL_MESON)	+= meson/
 obj-$(CONFIG_PINCTRL_OXNAS)	+= pinctrl-oxnas.o
 obj-$(CONFIG_PINCTRL_PALMAS)	+= pinctrl-palmas.o
diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
similarity index 100%
rename from drivers/gpio/gpio-mcp23s08.c
rename to drivers/pinctrl/pinctrl-mcp23s08.c
-- 
2.11.0

[PATCHv3 02/14] pinctrl: mcp23s08: add pinconf support

[Sebastian Reichel]

mcp23xxx device have configurable 100k pullup resistors. This adds
support for enabling them using pinctrl's pinconf interface.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/pinctrl/Kconfig            |   1 +
 drivers/pinctrl/pinctrl-mcp23s08.c | 199 ++++++++++++++++++++++++++++++++-----
 2 files changed, 176 insertions(+), 24 deletions(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index b5aa50c51633..1dabd1d79c1d 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -153,6 +153,7 @@ config PINCTRL_MCP23S08
 	select GPIOLIB_IRQCHIP
 	select REGMAP_I2C if I2C
 	select REGMAP_SPI if SPI_MASTER
+	select GENERIC_PINCONF
 	help
 	  SPI/I2C driver for Microchip MCP23S08/MCP23S17/MCP23008/MCP23017
 	  I/O expanders.
diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 2a57d024481d..d957c4bbc8c1 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -24,6 +24,9 @@
 #include <linux/of_irq.h>
 #include <linux/of_device.h>
 #include <linux/regmap.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
 
 /**
  * MCP types supported by driver
@@ -77,6 +80,9 @@ struct mcp23s08 {
 
 	struct regmap		*regmap;
 	struct device		*dev;
+
+	struct pinctrl_dev	*pctldev;
+	struct pinctrl_desc	pinctrl_desc;
 };
 
 static const struct regmap_config mcp23x08_regmap = {
@@ -96,6 +102,158 @@ static const struct regmap_config mcp23x17_regmap = {
 	.val_format_endian = REGMAP_ENDIAN_LITTLE,
 };
 
+static int mcp_read(struct mcp23s08 *mcp, unsigned int reg, unsigned int *val)
+{
+	return regmap_read(mcp->regmap, reg << mcp->reg_shift, val);
+}
+
+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)
+{
+	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)
+{
+	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;
+}
+
+static const struct pinctrl_pin_desc mcp23x08_pins[] = {
+	PINCTRL_PIN(0, "gpio0"),
+	PINCTRL_PIN(1, "gpio1"),
+	PINCTRL_PIN(2, "gpio2"),
+	PINCTRL_PIN(3, "gpio3"),
+	PINCTRL_PIN(4, "gpio4"),
+	PINCTRL_PIN(5, "gpio5"),
+	PINCTRL_PIN(6, "gpio6"),
+	PINCTRL_PIN(7, "gpio7"),
+};
+
+static const struct pinctrl_pin_desc mcp23x17_pins[] = {
+	PINCTRL_PIN(0, "gpio0"),
+	PINCTRL_PIN(1, "gpio1"),
+	PINCTRL_PIN(2, "gpio2"),
+	PINCTRL_PIN(3, "gpio3"),
+	PINCTRL_PIN(4, "gpio4"),
+	PINCTRL_PIN(5, "gpio5"),
+	PINCTRL_PIN(6, "gpio6"),
+	PINCTRL_PIN(7, "gpio7"),
+	PINCTRL_PIN(8, "gpio8"),
+	PINCTRL_PIN(9, "gpio9"),
+	PINCTRL_PIN(10, "gpio10"),
+	PINCTRL_PIN(11, "gpio11"),
+	PINCTRL_PIN(12, "gpio12"),
+	PINCTRL_PIN(13, "gpio13"),
+	PINCTRL_PIN(14, "gpio14"),
+	PINCTRL_PIN(15, "gpio15"),
+};
+
+static int mcp_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	return 0;
+}
+
+static const char *mcp_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
+						unsigned int group)
+{
+	return NULL;
+}
+
+static int mcp_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
+					unsigned int group,
+					const unsigned int **pins,
+					unsigned int *num_pins)
+{
+	return -ENOTSUPP;
+}
+
+static const struct pinctrl_ops mcp_pinctrl_ops = {
+	.get_groups_count = mcp_pinctrl_get_groups_count,
+	.get_group_name = mcp_pinctrl_get_group_name,
+	.get_group_pins = mcp_pinctrl_get_group_pins,
+#ifdef CONFIG_OF
+	.dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
+	.dt_free_map = pinconf_generic_dt_free_map,
+#endif
+};
+
+static int mcp_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
+			      unsigned long *config)
+{
+	struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
+	enum pin_config_param param = pinconf_to_config_param(*config);
+	unsigned int data, status;
+	int ret;
+
+	switch (param) {
+	case PIN_CONFIG_BIAS_PULL_UP:
+		ret = mcp_read(mcp, MCP_GPPU, &data);
+		if (ret < 0)
+			return ret;
+		status = (data & BIT(pin)) ? 1 : 0;
+		break;
+	default:
+		dev_err(mcp->dev, "Invalid config param %04x\n", param);
+		return -ENOTSUPP;
+	}
+
+	*config = 0;
+
+	return status ? 0 : -EINVAL;
+}
+
+static int mcp_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
+			      unsigned long *configs, unsigned int num_configs)
+{
+	struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
+	enum pin_config_param param;
+	u32 arg, mask;
+	u16 val;
+	int ret = 0;
+	int i;
+
+	for (i = 0; i < num_configs; i++) {
+		param = pinconf_to_config_param(configs[i]);
+		arg = pinconf_to_config_argument(configs[i]);
+
+		switch (param) {
+		case PIN_CONFIG_BIAS_PULL_UP:
+			val = arg ? 0xFFFF : 0x0000;
+			mask = BIT(pin);
+			ret = mcp_set_bit(mcp, MCP_GPPU, pin, arg);
+			break;
+		default:
+			dev_err(mcp->dev, "Invalid config param %04x\n", param);
+			return -ENOTSUPP;
+		}
+	}
+
+	return ret;
+}
+
+static const struct pinconf_ops mcp_pinconf_ops = {
+	.pin_config_get = mcp_pinconf_get,
+	.pin_config_set = mcp_pinconf_set,
+	.is_generic = true,
+};
+
 /*----------------------------------------------------------------------*/
 
 #ifdef CONFIG_SPI_MASTER
@@ -158,30 +316,6 @@ static const struct regmap_bus mcp23sxx_spi_regmap = {
 
 #endif /* CONFIG_SPI_MASTER */
 
-static int mcp_read(struct mcp23s08 *mcp, unsigned int reg, unsigned int *val)
-{
-	return regmap_read(mcp->regmap, reg << mcp->reg_shift, val);
-}
-
-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_update_cache(struct mcp23s08 *mcp)
-{
-	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;
-}
-
 /*----------------------------------------------------------------------*/
 
 /* A given spi_device can represent up to eight mcp23sxx chips
@@ -682,6 +816,23 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 		if (ret)
 			goto fail;
 	}
+
+	mcp->pinctrl_desc.name = "mcp23xxx-pinctrl";
+	mcp->pinctrl_desc.pctlops = &mcp_pinctrl_ops;
+	mcp->pinctrl_desc.confops = &mcp_pinconf_ops;
+	mcp->pinctrl_desc.npins = mcp->chip.ngpio;
+	if (mcp->pinctrl_desc.npins == 8)
+		mcp->pinctrl_desc.pins = mcp23x08_pins;
+	else if (mcp->pinctrl_desc.npins == 16)
+		mcp->pinctrl_desc.pins = mcp23x17_pins;
+	mcp->pinctrl_desc.owner = THIS_MODULE;
+
+	mcp->pctldev = devm_pinctrl_register(dev, &mcp->pinctrl_desc, mcp);
+	if (IS_ERR(mcp->pctldev)) {
+		ret = PTR_ERR(mcp->pctldev);
+		goto fail;
+	}
+
 fail:
 	if (ret < 0)
 		dev_dbg(dev, "can't setup chip %d, --> %d\n", addr, ret);
-- 
2.11.0

[PATCHv3 03/14] pinctrl: mcp23s08: drop pullup config from pdata

[Sebastian Reichel]

mcp23s08 support configuration of the pullups using the
pinconf framework. This removes the custom pullup configuration
from platform data, which has no upstream users.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 7 -------
 include/linux/spi/mcp23s08.h       | 1 -
 2 files changed, 8 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index d957c4bbc8c1..8c684d179e29 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -782,11 +782,6 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 			goto fail;
 	}
 
-	/* configure ~100K pullups */
-	ret = mcp_write(mcp, MCP_GPPU, pdata->chip[cs].pullups);
-	if (ret < 0)
-		goto fail;
-
 	ret = mcp_update_cache(mcp);
 	if (ret < 0)
 		goto fail;
@@ -911,7 +906,6 @@ static int mcp230xx_probe(struct i2c_client *client,
 	if (match) {
 		pdata = &local_pdata;
 		pdata->base = -1;
-		pdata->chip[0].pullups = 0;
 		pdata->irq_controller =	of_property_read_bool(
 					client->dev.of_node,
 					"interrupt-controller");
@@ -1031,7 +1025,6 @@ static int mcp23s08_probe(struct spi_device *spi)
 		pdata = &local_pdata;
 		pdata->base = -1;
 		for (addr = 0; addr < ARRAY_SIZE(pdata->chip); addr++) {
-			pdata->chip[addr].pullups = 0;
 			if (spi_present_mask & (1 << addr))
 				chips++;
 		}
diff --git a/include/linux/spi/mcp23s08.h b/include/linux/spi/mcp23s08.h
index aa07d7b32568..080ecc6bb270 100644
--- a/include/linux/spi/mcp23s08.h
+++ b/include/linux/spi/mcp23s08.h
@@ -3,7 +3,6 @@
 
 struct mcp23s08_chip_info {
 	bool		is_present;	/* true if populated */
-	unsigned	pullups;	/* BIT(x) means enable pullup x */
 };
 
 struct mcp23s08_platform_data {
-- 
2.11.0

[PATCHv3 04/14] pinctrl: mcp23s08: switch to regmap caching

[Sebastian Reichel]

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>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 287 +++++++++++++++++++++++++------------
 1 file changed, 192 insertions(+), 95 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 8c684d179e29..be7ec7ddbce0 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -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;
-- 
2.11.0

[PATCHv3 05/14] pinctrl: mcp23s08: drop OF_GPIO dependency

[Sebastian Reichel]

The driver compiles & works perfectly fine without OF_GPIO on x86,
so lets drop the dependency.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/pinctrl/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 1dabd1d79c1d..1c80b970554e 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -148,7 +148,6 @@ config PINCTRL_FALCON
 
 config PINCTRL_MCP23S08
 	tristate "Microchip MCP23xxx I/O expander"
-	depends on OF_GPIO
 	depends on SPI_MASTER || I2C
 	select GPIOLIB_IRQCHIP
 	select REGMAP_I2C if I2C
-- 
2.11.0

[PATCHv3 06/14] pinctrl: mcp23s08: irq mapping is already done

[Sebastian Reichel]

i2c-core and spi-core already assign the irq, so we
can drop the additional call from the mcp driver.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index be7ec7ddbce0..94d2c19a6989 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -21,7 +21,6 @@
 #include <linux/slab.h>
 #include <asm/byteorder.h>
 #include <linux/interrupt.h>
-#include <linux/of_irq.h>
 #include <linux/of_device.h>
 #include <linux/regmap.h>
 #include <linux/pinctrl/pinctrl.h>
@@ -1008,7 +1007,6 @@ static int mcp230xx_probe(struct i2c_client *client,
 					"interrupt-controller");
 		pdata->mirror = of_property_read_bool(client->dev.of_node,
 						      "microchip,irq-mirror");
-		client->irq = irq_of_parse_and_map(client->dev.of_node, 0);
 	} else {
 		pdata = dev_get_platdata(&client->dev);
 		if (!pdata) {
@@ -1164,8 +1162,6 @@ static int mcp23s08_probe(struct spi_device *spi)
 
 	spi_set_drvdata(spi, data);
 
-	spi->irq = irq_of_parse_and_map(spi->dev.of_node, 0);
-
 	for (addr = 0; addr < ARRAY_SIZE(pdata->chip); addr++) {
 		if (!(spi_present_mask & (1 << addr)))
 			continue;
-- 
2.11.0

[PATCHv3 07/14] pinctrl: mcp23s08: use managed kzalloc for mcp

[Sebastian Reichel]

Let's remove a few lines of code by using managed memory for mcp
variable.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 94d2c19a6989..4d2e1c3c0e87 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -1019,7 +1019,7 @@ static int mcp230xx_probe(struct i2c_client *client,
 		}
 	}
 
-	mcp = kzalloc(sizeof(*mcp), GFP_KERNEL);
+	mcp = devm_kzalloc(&client->dev, sizeof(*mcp), GFP_KERNEL);
 	if (!mcp)
 		return -ENOMEM;
 
@@ -1027,16 +1027,11 @@ static int mcp230xx_probe(struct i2c_client *client,
 	status = mcp23s08_probe_one(mcp, &client->dev, client, client->addr,
 				    id->driver_data, pdata, 0);
 	if (status)
-		goto fail;
+		return status;
 
 	i2c_set_clientdata(client, mcp);
 
 	return 0;
-
-fail:
-	kfree(mcp);
-
-	return status;
 }
 
 static int mcp230xx_remove(struct i2c_client *client)
@@ -1044,7 +1039,6 @@ static int mcp230xx_remove(struct i2c_client *client)
 	struct mcp23s08 *mcp = i2c_get_clientdata(client);
 
 	gpiochip_remove(&mcp->chip);
-	kfree(mcp);
 
 	return 0;
 }
-- 
2.11.0

[PATCHv3 08/14] pinctrl: mcp23s08: switch to devm_gpiochip_add_data

[Sebastian Reichel]

Switching to devm_gpiochip_add_data simplifies the driver's
cleanup routine and safes a few loc.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 40 ++------------------------------------
 1 file changed, 2 insertions(+), 38 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 4d2e1c3c0e87..3fc4195ad415 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -898,7 +898,7 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 			goto fail;
 	}
 
-	ret = gpiochip_add_data(&mcp->chip, mcp);
+	ret = devm_gpiochip_add_data(dev, &mcp->chip, mcp);
 	if (ret < 0)
 		goto fail;
 
@@ -1034,15 +1034,6 @@ static int mcp230xx_probe(struct i2c_client *client,
 	return 0;
 }
 
-static int mcp230xx_remove(struct i2c_client *client)
-{
-	struct mcp23s08 *mcp = i2c_get_clientdata(client);
-
-	gpiochip_remove(&mcp->chip);
-
-	return 0;
-}
-
 static const struct i2c_device_id mcp230xx_id[] = {
 	{ "mcp23008", MCP_TYPE_008 },
 	{ "mcp23017", MCP_TYPE_017 },
@@ -1056,7 +1047,6 @@ static struct i2c_driver mcp230xx_driver = {
 		.of_match_table = of_match_ptr(mcp23s08_i2c_of_match),
 	},
 	.probe		= mcp230xx_probe,
-	.remove		= mcp230xx_remove,
 	.id_table	= mcp230xx_id,
 };
 
@@ -1166,7 +1156,7 @@ static int mcp23s08_probe(struct spi_device *spi)
 					    0x40 | (addr << 1), type, pdata,
 					    addr);
 		if (status < 0)
-			goto fail;
+			return status;
 
 		if (pdata->base != -1)
 			pdata->base += data->mcp[addr]->chip.ngpio;
@@ -1180,31 +1170,6 @@ static int mcp23s08_probe(struct spi_device *spi)
 	 */
 
 	return 0;
-
-fail:
-	for (addr = 0; addr < ARRAY_SIZE(data->mcp); addr++) {
-
-		if (!data->mcp[addr])
-			continue;
-		gpiochip_remove(&data->mcp[addr]->chip);
-	}
-	return status;
-}
-
-static int mcp23s08_remove(struct spi_device *spi)
-{
-	struct mcp23s08_driver_data	*data = spi_get_drvdata(spi);
-	unsigned			addr;
-
-	for (addr = 0; addr < ARRAY_SIZE(data->mcp); addr++) {
-
-		if (!data->mcp[addr])
-			continue;
-
-		gpiochip_remove(&data->mcp[addr]->chip);
-	}
-
-	return 0;
 }
 
 static const struct spi_device_id mcp23s08_ids[] = {
@@ -1217,7 +1182,6 @@ MODULE_DEVICE_TABLE(spi, mcp23s08_ids);
 
 static struct spi_driver mcp23s08_driver = {
 	.probe		= mcp23s08_probe,
-	.remove		= mcp23s08_remove,
 	.id_table	= mcp23s08_ids,
 	.driver = {
 		.name	= "mcp23s08",
-- 
2.11.0

[PATCHv3 09/14] pinctrl: mcp23s08: simplify i2c pdata handling

[Sebastian Reichel]

Simplify i2c pdata handling, so that it uses pdata when available
and falls back to reading device properties otherwise.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 3fc4195ad415..aec2dad26560 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -995,28 +995,16 @@ static int mcp230xx_probe(struct i2c_client *client,
 	struct mcp23s08_platform_data *pdata, local_pdata;
 	struct mcp23s08 *mcp;
 	int status;
-	const struct of_device_id *match;
 
-	match = of_match_device(of_match_ptr(mcp23s08_i2c_of_match),
-					&client->dev);
-	if (match) {
+	pdata = dev_get_platdata(&client->dev);
+	if (!pdata) {
 		pdata = &local_pdata;
 		pdata->base = -1;
-		pdata->irq_controller =	of_property_read_bool(
-					client->dev.of_node,
-					"interrupt-controller");
-		pdata->mirror = of_property_read_bool(client->dev.of_node,
-						      "microchip,irq-mirror");
-	} else {
-		pdata = dev_get_platdata(&client->dev);
-		if (!pdata) {
-			pdata = devm_kzalloc(&client->dev,
-					sizeof(struct mcp23s08_platform_data),
-					GFP_KERNEL);
-			if (!pdata)
-				return -ENOMEM;
-			pdata->base = -1;
-		}
+
+		pdata->irq_controller = device_property_read_bool(
+			&client->dev, "interrupt-controller");
+		pdata->mirror = device_property_read_bool(
+			&client->dev, "microchip,irq-mirror");
 	}
 
 	mcp = devm_kzalloc(&client->dev, sizeof(*mcp), GFP_KERNEL);
-- 
2.11.0

[PATCHv3 10/14] pinctrl: mcp23s08: simplify spi pdata handling

[Sebastian Reichel]

Simplify spi pdata handling, so that it uses pdata when available
and falls back to reading device properties otherwise.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 67 ++++++++++++++++++--------------------
 1 file changed, 32 insertions(+), 35 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index aec2dad26560..541bf80a2a13 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -1071,58 +1071,55 @@ static int mcp23s08_probe(struct spi_device *spi)
 	u32				spi_present_mask = 0;
 
 	match = of_match_device(of_match_ptr(mcp23s08_spi_of_match), &spi->dev);
-	if (match) {
+	if (match)
 		type = (int)(uintptr_t)match->data;
-		status = of_property_read_u32(spi->dev.of_node,
-			    "microchip,spi-present-mask", &spi_present_mask);
+	else
+		type = spi_get_device_id(spi)->driver_data;
+
+	pdata = dev_get_platdata(&spi->dev);
+	if (!pdata) {
+		pdata = &local_pdata;
+		pdata->base = -1;
+
+		pdata->irq_controller = device_property_read_bool(&spi->dev,
+			"interrupt-controller");
+		pdata->mirror = device_property_read_bool(&spi->dev,
+			"microchip,irq-mirror");
+
+		status = device_property_read_u32(&spi->dev,
+			"microchip,spi-present-mask", &spi_present_mask);
 		if (status) {
-			status = of_property_read_u32(spi->dev.of_node,
-				    "mcp,spi-present-mask", &spi_present_mask);
+			status = device_property_read_u32(&spi->dev,
+				"mcp,spi-present-mask", &spi_present_mask);
+
 			if (status) {
-				dev_err(&spi->dev,
-					"DT has no spi-present-mask\n");
+				dev_err(&spi->dev, "missing spi-present-mask");
 				return -ENODEV;
 			}
 		}
-		if ((spi_present_mask <= 0) || (spi_present_mask >= 256)) {
-			dev_err(&spi->dev, "invalid spi-present-mask\n");
-			return -ENODEV;
-		}
-
-		pdata = &local_pdata;
-		pdata->base = -1;
-		for (addr = 0; addr < ARRAY_SIZE(pdata->chip); addr++) {
-			if (spi_present_mask & (1 << addr))
-				chips++;
-		}
-		pdata->irq_controller =	of_property_read_bool(
-					spi->dev.of_node,
-					"interrupt-controller");
-		pdata->mirror = of_property_read_bool(spi->dev.of_node,
-						      "microchip,irq-mirror");
 	} else {
-		type = spi_get_device_id(spi)->driver_data;
-		pdata = dev_get_platdata(&spi->dev);
-		if (!pdata) {
-			pdata = devm_kzalloc(&spi->dev,
-					sizeof(struct mcp23s08_platform_data),
-					GFP_KERNEL);
-			pdata->base = -1;
-		}
-
 		for (addr = 0; addr < ARRAY_SIZE(pdata->chip); addr++) {
 			if (!pdata->chip[addr].is_present)
 				continue;
-			chips++;
 			if ((type == MCP_TYPE_S08) && (addr > 3)) {
 				dev_err(&spi->dev,
-					"mcp23s08 only supports address 0..3\n");
+					"mcp23s08 only supports address 0..3");
 				return -EINVAL;
 			}
-			spi_present_mask |= 1 << addr;
+			spi_present_mask |= BIT(addr);
 		}
 	}
 
+	if (!spi_present_mask || spi_present_mask >= 256) {
+		dev_err(&spi->dev, "invalid spi-present-mask");
+		return -ENODEV;
+	}
+
+	for (addr = 0; addr < ARRAY_SIZE(pdata->chip); addr++) {
+		if (spi_present_mask & BIT(addr))
+			chips++;
+	}
+
 	if (!chips)
 		return -ENODEV;
 
-- 
2.11.0

[PATCHv3 11/14] pinctrl: mcp23s08: generalize irq property handling

[Sebastian Reichel]

This moves irq property handling from spi/i2c specific code into
the generic mcp23s08_probe_one. This is possible because the
device properties are named equally.

As a side-effect this drops support for setting the properties via
pdata, which has no mainline users. If boardcode wants to enable
the chip as interrupt controller it can attach the device properties
instead.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 27 +++++++++------------------
 include/linux/spi/mcp23s08.h       | 18 ------------------
 2 files changed, 9 insertions(+), 36 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 541bf80a2a13..b39da587f2fa 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -783,7 +783,7 @@ static void mcp23s08_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 
 static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 			      void *data, unsigned addr, unsigned type,
-			      struct mcp23s08_platform_data *pdata, int cs)
+			      unsigned int base, int cs)
 {
 	int status, ret;
 	bool mirror = false;
@@ -855,7 +855,7 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 	if (IS_ERR(mcp->regmap))
 		return PTR_ERR(mcp->regmap);
 
-	mcp->chip.base = pdata->base;
+	mcp->chip.base = base;
 	mcp->chip.can_sleep = true;
 	mcp->chip.parent = dev;
 	mcp->chip.owner = THIS_MODULE;
@@ -868,13 +868,14 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 	if (ret < 0)
 		goto fail;
 
-	mcp->irq_controller = pdata->irq_controller;
+	mcp->irq_controller =
+		device_property_read_bool(dev, "interrupt-controller");
 	if (mcp->irq && mcp->irq_controller) {
 		mcp->irq_active_high =
-			of_property_read_bool(mcp->chip.parent->of_node,
+			device_property_read_bool(dev,
 					      "microchip,irq-active-high");
 
-		mirror = pdata->mirror;
+		mirror = device_property_read_bool(dev, "microchip,irq-mirror");
 	}
 
 	if ((status & IOCON_SEQOP) || !(status & IOCON_HAEN) || mirror ||
@@ -1000,11 +1001,6 @@ static int mcp230xx_probe(struct i2c_client *client,
 	if (!pdata) {
 		pdata = &local_pdata;
 		pdata->base = -1;
-
-		pdata->irq_controller = device_property_read_bool(
-			&client->dev, "interrupt-controller");
-		pdata->mirror = device_property_read_bool(
-			&client->dev, "microchip,irq-mirror");
 	}
 
 	mcp = devm_kzalloc(&client->dev, sizeof(*mcp), GFP_KERNEL);
@@ -1013,7 +1009,7 @@ static int mcp230xx_probe(struct i2c_client *client,
 
 	mcp->irq = client->irq;
 	status = mcp23s08_probe_one(mcp, &client->dev, client, client->addr,
-				    id->driver_data, pdata, 0);
+				    id->driver_data, pdata->base, 0);
 	if (status)
 		return status;
 
@@ -1081,11 +1077,6 @@ static int mcp23s08_probe(struct spi_device *spi)
 		pdata = &local_pdata;
 		pdata->base = -1;
 
-		pdata->irq_controller = device_property_read_bool(&spi->dev,
-			"interrupt-controller");
-		pdata->mirror = device_property_read_bool(&spi->dev,
-			"microchip,irq-mirror");
-
 		status = device_property_read_u32(&spi->dev,
 			"microchip,spi-present-mask", &spi_present_mask);
 		if (status) {
@@ -1138,8 +1129,8 @@ static int mcp23s08_probe(struct spi_device *spi)
 		data->mcp[addr] = &data->chip[chips];
 		data->mcp[addr]->irq = spi->irq;
 		status = mcp23s08_probe_one(data->mcp[addr], &spi->dev, spi,
-					    0x40 | (addr << 1), type, pdata,
-					    addr);
+					    0x40 | (addr << 1), type,
+					    pdata->base, addr);
 		if (status < 0)
 			return status;
 
diff --git a/include/linux/spi/mcp23s08.h b/include/linux/spi/mcp23s08.h
index 080ecc6bb270..4af82ee63329 100644
--- a/include/linux/spi/mcp23s08.h
+++ b/include/linux/spi/mcp23s08.h
@@ -21,22 +21,4 @@ struct mcp23s08_platform_data {
 	 * base to base+15 (or base+31 for s17 variant).
 	 */
 	unsigned	base;
-	/* Marks the device as a interrupt controller.
-	 * NOTE: The interrupt functionality is only supported for i2c
-	 * versions of the chips. The spi chips can also do the interrupts,
-	 * but this is not supported by the linux driver yet.
-	 */
-	bool		irq_controller;
-
-	/* Sets the mirror flag in the IOCON register. Devices
-	 * with two interrupt outputs (these are the devices ending with 17 and
-	 * those that have 16 IOs) have two IO banks: IO 0-7 form bank 1 and
-	 * IO 8-15 are bank 2. These chips have two different interrupt outputs:
-	 * One for bank 1 and another for bank 2. If irq-mirror is set, both
-	 * interrupts are generated regardless of the bank that an input change
-	 * occurred on. If it is not set, the interrupt are only generated for
-	 * the bank they belong to.
-	 * On devices with only one interrupt output this property is useless.
-	 */
-	bool		mirror;
 };
-- 
2.11.0

[PATCHv3 12/14] pinctrl: mcp23s08: simplify spi_present_mask handling

[Sebastian Reichel]

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 arch/blackfin/mach-bf527/boards/tll6527m.c |  4 ++--
 drivers/pinctrl/pinctrl-mcp23s08.c         | 29 ++++++++++-------------------
 include/linux/spi/mcp23s08.h               |  6 +-----
 3 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/arch/blackfin/mach-bf527/boards/tll6527m.c b/arch/blackfin/mach-bf527/boards/tll6527m.c
index be61477826f3..ce5488e8226b 100644
--- a/arch/blackfin/mach-bf527/boards/tll6527m.c
+++ b/arch/blackfin/mach-bf527/boards/tll6527m.c
@@ -351,11 +351,11 @@ static struct platform_device bfin_i2s = {
 #if IS_ENABLED(CONFIG_PINCTRL_MCP23S08)
 #include <linux/spi/mcp23s08.h>
 static const struct mcp23s08_platform_data bfin_mcp23s08_sys_gpio_info = {
-	.chip[0].is_present = true,
+	.spi_present_mask = BIT(0),
 	.base = 0x30,
 };
 static const struct mcp23s08_platform_data bfin_mcp23s08_usr_gpio_info = {
-	.chip[2].is_present = true,
+	.spi_present_mask = BIT(2),
 	.base = 0x38,
 };
 #endif
diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index b39da587f2fa..29d9c1fd4309 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -36,6 +36,8 @@
 #define MCP_TYPE_017	3
 #define MCP_TYPE_S18    4
 
+#define MCP_MAX_DEV_PER_CS	8
+
 /* Registers are all 8 bits wide.
  *
  * The mcp23s17 has twice as many bits, and can be configured to work
@@ -1064,7 +1066,6 @@ static int mcp23s08_probe(struct spi_device *spi)
 	int				status, type;
 	unsigned			ngpio = 0;
 	const struct			of_device_id *match;
-	u32				spi_present_mask = 0;
 
 	match = of_match_device(of_match_ptr(mcp23s08_spi_of_match), &spi->dev);
 	if (match)
@@ -1078,36 +1079,26 @@ static int mcp23s08_probe(struct spi_device *spi)
 		pdata->base = -1;
 
 		status = device_property_read_u32(&spi->dev,
-			"microchip,spi-present-mask", &spi_present_mask);
+			"microchip,spi-present-mask", &pdata->spi_present_mask);
 		if (status) {
 			status = device_property_read_u32(&spi->dev,
-				"mcp,spi-present-mask", &spi_present_mask);
+				"mcp,spi-present-mask",
+				&pdata->spi_present_mask);
 
 			if (status) {
 				dev_err(&spi->dev, "missing spi-present-mask");
 				return -ENODEV;
 			}
 		}
-	} else {
-		for (addr = 0; addr < ARRAY_SIZE(pdata->chip); addr++) {
-			if (!pdata->chip[addr].is_present)
-				continue;
-			if ((type == MCP_TYPE_S08) && (addr > 3)) {
-				dev_err(&spi->dev,
-					"mcp23s08 only supports address 0..3");
-				return -EINVAL;
-			}
-			spi_present_mask |= BIT(addr);
-		}
 	}
 
-	if (!spi_present_mask || spi_present_mask >= 256) {
+	if (!pdata->spi_present_mask || pdata->spi_present_mask > 0xff) {
 		dev_err(&spi->dev, "invalid spi-present-mask");
 		return -ENODEV;
 	}
 
-	for (addr = 0; addr < ARRAY_SIZE(pdata->chip); addr++) {
-		if (spi_present_mask & BIT(addr))
+	for (addr = 0; addr < MCP_MAX_DEV_PER_CS; addr++) {
+		if (pdata->spi_present_mask & BIT(addr))
 			chips++;
 	}
 
@@ -1122,8 +1113,8 @@ static int mcp23s08_probe(struct spi_device *spi)
 
 	spi_set_drvdata(spi, data);
 
-	for (addr = 0; addr < ARRAY_SIZE(pdata->chip); addr++) {
-		if (!(spi_present_mask & (1 << addr)))
+	for (addr = 0; addr < MCP_MAX_DEV_PER_CS; addr++) {
+		if (!(pdata->spi_present_mask & BIT(addr)))
 			continue;
 		chips--;
 		data->mcp[addr] = &data->chip[chips];
diff --git a/include/linux/spi/mcp23s08.h b/include/linux/spi/mcp23s08.h
index 4af82ee63329..211f3c0ef49c 100644
--- a/include/linux/spi/mcp23s08.h
+++ b/include/linux/spi/mcp23s08.h
@@ -1,10 +1,6 @@
 
 /* FIXME driver should be able to handle IRQs...  */
 
-struct mcp23s08_chip_info {
-	bool		is_present;	/* true if populated */
-};
-
 struct mcp23s08_platform_data {
 	/* For mcp23s08, up to 4 slaves (numbered 0..3) can share one SPI
 	 * chipselect, each providing 1 gpio_chip instance with 8 gpios.
@@ -12,7 +8,7 @@ struct mcp23s08_platform_data {
 	 * chipselect, each providing 1 gpio_chip (port A + port B) with
 	 * 16 gpios.
 	 */
-	struct mcp23s08_chip_info	chip[8];
+	u32 spi_present_mask;
 
 	/* "base" is the number of the first GPIO.  Dynamic assignment is
 	 * not currently supported, and even if there are gaps in chip
-- 
2.11.0

[PATCHv3 13/14] pinctrl: mcp23s08: drop comment about missing irq support

[Sebastian Reichel]

The driver supports using mcp23xxx as interrupt controller, so
let's drop all comments stating otherwise.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 19 ++-----------------
 include/linux/spi/mcp23s08.h       |  3 ---
 2 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 29d9c1fd4309..3e40d4245512 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -1,14 +1,4 @@
-/*
- * MCP23S08 SPI/I2C GPIO gpio expander driver
- *
- * The inputs and outputs of the mcp23s08, mcp23s17, mcp23008 and mcp23017 are
- * supported.
- * For the I2C versions of the chips (mcp23008 and mcp23017) generation of
- * interrupts is also supported.
- * The hardware of the SPI versions of the chips (mcp23s08 and mcp23s17) is
- * also capable of generating interrupts, but the linux driver does not
- * support that yet.
- */
+/* MCP23S08 SPI/I2C GPIO driver */
 
 #include <linux/kernel.h>
 #include <linux/device.h>
@@ -27,7 +17,7 @@
 #include <linux/pinctrl/pinconf.h>
 #include <linux/pinctrl/pinconf-generic.h>
 
-/**
+/*
  * MCP types supported by driver
  */
 #define MCP_TYPE_S08	0
@@ -1131,11 +1121,6 @@ static int mcp23s08_probe(struct spi_device *spi)
 	}
 	data->ngpio = ngpio;
 
-	/* NOTE:  these chips have a relatively sane IRQ framework, with
-	 * per-signal masking and level/edge triggering.  It's not yet
-	 * handled here...
-	 */
-
 	return 0;
 }
 
diff --git a/include/linux/spi/mcp23s08.h b/include/linux/spi/mcp23s08.h
index 211f3c0ef49c..4354beefd584 100644
--- a/include/linux/spi/mcp23s08.h
+++ b/include/linux/spi/mcp23s08.h
@@ -1,6 +1,3 @@
-
-/* FIXME driver should be able to handle IRQs...  */
-
 struct mcp23s08_platform_data {
 	/* For mcp23s08, up to 4 slaves (numbered 0..3) can share one SPI
 	 * chipselect, each providing 1 gpio_chip instance with 8 gpios.
-- 
2.11.0

[PATCHv3 14/14] pinctrl: mcp23s08: fix comment for mcp23s08_platform_data.base

[Sebastian Reichel]

The comment does not match the driver, which actually supports
automatic assignment. Fix this by updating the comment.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 include/linux/spi/mcp23s08.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/spi/mcp23s08.h b/include/linux/spi/mcp23s08.h
index 4354beefd584..82d96a346e6f 100644
--- a/include/linux/spi/mcp23s08.h
+++ b/include/linux/spi/mcp23s08.h
@@ -7,11 +7,11 @@ struct mcp23s08_platform_data {
 	 */
 	u32 spi_present_mask;
 
-	/* "base" is the number of the first GPIO.  Dynamic assignment is
-	 * not currently supported, and even if there are gaps in chip
-	 * addressing the GPIO numbers are sequential .. so for example
-	 * if only slaves 0 and 3 are present, their GPIOs range from
-	 * base to base+15 (or base+31 for s17 variant).
+	/* "base" is the number of the first GPIO or -1 for dynamic
+	 * assignment. If there are gaps in chip addressing the GPIO
+	 * numbers are sequential .. so for example if only slaves 0
+	 * and 3 are present, their GPIOs range from base to base+15
+	 * (or base+31 for s17 variant).
 	 */
 	unsigned	base;
 };
-- 
2.11.0

Re: [PATCHv3 02/14] pinctrl: mcp23s08: add pinconf support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 8045 bytes --]

Hi,

On Mon, May 15, 2017 at 11:24:26AM +0200, Sebastian Reichel wrote:
> mcp23xxx device have configurable 100k pullup resistors. This adds
> support for enabling them using pinctrl's pinconf interface.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

I just noticed, that I forgot to add:

Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

-- Sebastian

> ---
>  drivers/pinctrl/Kconfig            |   1 +
>  drivers/pinctrl/pinctrl-mcp23s08.c | 199 ++++++++++++++++++++++++++++++++-----
>  2 files changed, 176 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index b5aa50c51633..1dabd1d79c1d 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -153,6 +153,7 @@ config PINCTRL_MCP23S08
>  	select GPIOLIB_IRQCHIP
>  	select REGMAP_I2C if I2C
>  	select REGMAP_SPI if SPI_MASTER
> +	select GENERIC_PINCONF
>  	help
>  	  SPI/I2C driver for Microchip MCP23S08/MCP23S17/MCP23008/MCP23017
>  	  I/O expanders.
> diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> index 2a57d024481d..d957c4bbc8c1 100644
> --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> @@ -24,6 +24,9 @@
>  #include <linux/of_irq.h>
>  #include <linux/of_device.h>
>  #include <linux/regmap.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinconf.h>
> +#include <linux/pinctrl/pinconf-generic.h>
>  
>  /**
>   * MCP types supported by driver
> @@ -77,6 +80,9 @@ struct mcp23s08 {
>  
>  	struct regmap		*regmap;
>  	struct device		*dev;
> +
> +	struct pinctrl_dev	*pctldev;
> +	struct pinctrl_desc	pinctrl_desc;
>  };
>  
>  static const struct regmap_config mcp23x08_regmap = {
> @@ -96,6 +102,158 @@ static const struct regmap_config mcp23x17_regmap = {
>  	.val_format_endian = REGMAP_ENDIAN_LITTLE,
>  };
>  
> +static int mcp_read(struct mcp23s08 *mcp, unsigned int reg, unsigned int *val)
> +{
> +	return regmap_read(mcp->regmap, reg << mcp->reg_shift, val);
> +}
> +
> +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)
> +{
> +	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)
> +{
> +	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;
> +}
> +
> +static const struct pinctrl_pin_desc mcp23x08_pins[] = {
> +	PINCTRL_PIN(0, "gpio0"),
> +	PINCTRL_PIN(1, "gpio1"),
> +	PINCTRL_PIN(2, "gpio2"),
> +	PINCTRL_PIN(3, "gpio3"),
> +	PINCTRL_PIN(4, "gpio4"),
> +	PINCTRL_PIN(5, "gpio5"),
> +	PINCTRL_PIN(6, "gpio6"),
> +	PINCTRL_PIN(7, "gpio7"),
> +};
> +
> +static const struct pinctrl_pin_desc mcp23x17_pins[] = {
> +	PINCTRL_PIN(0, "gpio0"),
> +	PINCTRL_PIN(1, "gpio1"),
> +	PINCTRL_PIN(2, "gpio2"),
> +	PINCTRL_PIN(3, "gpio3"),
> +	PINCTRL_PIN(4, "gpio4"),
> +	PINCTRL_PIN(5, "gpio5"),
> +	PINCTRL_PIN(6, "gpio6"),
> +	PINCTRL_PIN(7, "gpio7"),
> +	PINCTRL_PIN(8, "gpio8"),
> +	PINCTRL_PIN(9, "gpio9"),
> +	PINCTRL_PIN(10, "gpio10"),
> +	PINCTRL_PIN(11, "gpio11"),
> +	PINCTRL_PIN(12, "gpio12"),
> +	PINCTRL_PIN(13, "gpio13"),
> +	PINCTRL_PIN(14, "gpio14"),
> +	PINCTRL_PIN(15, "gpio15"),
> +};
> +
> +static int mcp_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
> +{
> +	return 0;
> +}
> +
> +static const char *mcp_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
> +						unsigned int group)
> +{
> +	return NULL;
> +}
> +
> +static int mcp_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
> +					unsigned int group,
> +					const unsigned int **pins,
> +					unsigned int *num_pins)
> +{
> +	return -ENOTSUPP;
> +}
> +
> +static const struct pinctrl_ops mcp_pinctrl_ops = {
> +	.get_groups_count = mcp_pinctrl_get_groups_count,
> +	.get_group_name = mcp_pinctrl_get_group_name,
> +	.get_group_pins = mcp_pinctrl_get_group_pins,
> +#ifdef CONFIG_OF
> +	.dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
> +	.dt_free_map = pinconf_generic_dt_free_map,
> +#endif
> +};
> +
> +static int mcp_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
> +			      unsigned long *config)
> +{
> +	struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
> +	enum pin_config_param param = pinconf_to_config_param(*config);
> +	unsigned int data, status;
> +	int ret;
> +
> +	switch (param) {
> +	case PIN_CONFIG_BIAS_PULL_UP:
> +		ret = mcp_read(mcp, MCP_GPPU, &data);
> +		if (ret < 0)
> +			return ret;
> +		status = (data & BIT(pin)) ? 1 : 0;
> +		break;
> +	default:
> +		dev_err(mcp->dev, "Invalid config param %04x\n", param);
> +		return -ENOTSUPP;
> +	}
> +
> +	*config = 0;
> +
> +	return status ? 0 : -EINVAL;
> +}
> +
> +static int mcp_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
> +			      unsigned long *configs, unsigned int num_configs)
> +{
> +	struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
> +	enum pin_config_param param;
> +	u32 arg, mask;
> +	u16 val;
> +	int ret = 0;
> +	int i;
> +
> +	for (i = 0; i < num_configs; i++) {
> +		param = pinconf_to_config_param(configs[i]);
> +		arg = pinconf_to_config_argument(configs[i]);
> +
> +		switch (param) {
> +		case PIN_CONFIG_BIAS_PULL_UP:
> +			val = arg ? 0xFFFF : 0x0000;
> +			mask = BIT(pin);
> +			ret = mcp_set_bit(mcp, MCP_GPPU, pin, arg);
> +			break;
> +		default:
> +			dev_err(mcp->dev, "Invalid config param %04x\n", param);
> +			return -ENOTSUPP;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct pinconf_ops mcp_pinconf_ops = {
> +	.pin_config_get = mcp_pinconf_get,
> +	.pin_config_set = mcp_pinconf_set,
> +	.is_generic = true,
> +};
> +
>  /*----------------------------------------------------------------------*/
>  
>  #ifdef CONFIG_SPI_MASTER
> @@ -158,30 +316,6 @@ static const struct regmap_bus mcp23sxx_spi_regmap = {
>  
>  #endif /* CONFIG_SPI_MASTER */
>  
> -static int mcp_read(struct mcp23s08 *mcp, unsigned int reg, unsigned int *val)
> -{
> -	return regmap_read(mcp->regmap, reg << mcp->reg_shift, val);
> -}
> -
> -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_update_cache(struct mcp23s08 *mcp)
> -{
> -	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;
> -}
> -
>  /*----------------------------------------------------------------------*/
>  
>  /* A given spi_device can represent up to eight mcp23sxx chips
> @@ -682,6 +816,23 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
>  		if (ret)
>  			goto fail;
>  	}
> +
> +	mcp->pinctrl_desc.name = "mcp23xxx-pinctrl";
> +	mcp->pinctrl_desc.pctlops = &mcp_pinctrl_ops;
> +	mcp->pinctrl_desc.confops = &mcp_pinconf_ops;
> +	mcp->pinctrl_desc.npins = mcp->chip.ngpio;
> +	if (mcp->pinctrl_desc.npins == 8)
> +		mcp->pinctrl_desc.pins = mcp23x08_pins;
> +	else if (mcp->pinctrl_desc.npins == 16)
> +		mcp->pinctrl_desc.pins = mcp23x17_pins;
> +	mcp->pinctrl_desc.owner = THIS_MODULE;
> +
> +	mcp->pctldev = devm_pinctrl_register(dev, &mcp->pinctrl_desc, mcp);
> +	if (IS_ERR(mcp->pctldev)) {
> +		ret = PTR_ERR(mcp->pctldev);
> +		goto fail;
> +	}
> +
>  fail:
>  	if (ret < 0)
>  		dev_dbg(dev, "can't setup chip %d, --> %d\n", addr, ret);
> -- 
> 2.11.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCHv3 01/14] gpio: mcp23s08: move to pinctrl

[Sylvain Lemieux]

On Mon, 2017-05-15 at 11:24 +0200, Sebastian Reichel wrote:
> This moves the mcp23s08 driver from gpio to pinctrl. Actual
> pinctrl support for configuration of the pull-up resistors
> follows in its own patch.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
>  arch/arm/configs/lpc32xx_defconfig                      |  2 +-
>  arch/blackfin/configs/BF609-EZKIT_defconfig             |  2 +-
>  arch/blackfin/mach-bf527/boards/tll6527m.c              |  4 ++--
>  arch/blackfin/mach-bf609/boards/ezkit.c                 |  4 ++--
>  drivers/gpio/Kconfig                                    | 17 -----------------
>  drivers/gpio/Makefile                                   |  1 -
>  drivers/pinctrl/Kconfig                                 | 13 +++++++++++++
>  drivers/pinctrl/Makefile                                |  1 +
>  .../gpio-mcp23s08.c => pinctrl/pinctrl-mcp23s08.c}      |  0
>  9 files changed, 20 insertions(+), 24 deletions(-)
>  rename drivers/{gpio/gpio-mcp23s08.c => pinctrl/pinctrl-mcp23s08.c} (100%)
> 
> diff --git a/arch/arm/configs/lpc32xx_defconfig b/arch/arm/configs/lpc32xx_defconfig
> index 6ba430d2b5b2..e15fa5f168bb 100644
> --- a/arch/arm/configs/lpc32xx_defconfig
> +++ b/arch/arm/configs/lpc32xx_defconfig
> @@ -112,7 +112,7 @@ CONFIG_GPIO_SX150X=y
>  CONFIG_GPIO_74X164=y
>  CONFIG_GPIO_MAX7301=y
>  CONFIG_GPIO_MC33880=y
> -CONFIG_GPIO_MCP23S08=y
> +CONFIG_PINCTRL_MCP23S08=y
>  CONFIG_SENSORS_DS620=y
>  CONFIG_SENSORS_MAX6639=y
>  CONFIG_WATCHDOG=y

Acked-by: Sylvain Lemieux <slemieux.tyco@gmail.com>

[...]

> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 23ca51ee6b28..5f88d7324e02 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1227,23 +1227,6 @@ config GPIO_PISOSR
>  
>  endmenu
>  
> -menu "SPI or I2C GPIO expanders"
> -	depends on (SPI_MASTER && !I2C) || I2C
> -
> -config GPIO_MCP23S08
> -	tristate "Microchip MCP23xxx I/O expander"
> -	depends on OF_GPIO
> -	select GPIOLIB_IRQCHIP
> -	select REGMAP_I2C if I2C
> -	select REGMAP if SPI_MASTER
> -	help
> -	  SPI/I2C driver for Microchip MCP23S08/MCP23S17/MCP23008/MCP23017
> -	  I/O expanders.
> -	  This provides a GPIO interface supporting inputs and outputs.
> -	  The I2C versions of the chips can be used as interrupt-controller.
> -
> -endmenu
> -
>  menu "USB GPIO expanders"
>  	depends on USB
>  
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 68b96277d9fa..89f10061a5c1 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -77,7 +77,6 @@ obj-$(CONFIG_GPIO_MENZ127)	+= gpio-menz127.o
>  obj-$(CONFIG_GPIO_MERRIFIELD)	+= gpio-merrifield.o
>  obj-$(CONFIG_GPIO_MC33880)	+= gpio-mc33880.o
>  obj-$(CONFIG_GPIO_MC9S08DZ60)	+= gpio-mc9s08dz60.o
> -obj-$(CONFIG_GPIO_MCP23S08)	+= gpio-mcp23s08.o
>  obj-$(CONFIG_GPIO_ML_IOH)	+= gpio-ml-ioh.o
>  obj-$(CONFIG_GPIO_MM_LANTIQ)	+= gpio-mm-lantiq.o
>  obj-$(CONFIG_GPIO_MOCKUP)      += gpio-mockup.o
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 37af5e3029d5..b5aa50c51633 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -146,6 +146,19 @@ config PINCTRL_FALCON
>  	depends on SOC_FALCON
>  	depends on PINCTRL_LANTIQ
>  
> +config PINCTRL_MCP23S08
> +	tristate "Microchip MCP23xxx I/O expander"
> +	depends on OF_GPIO
> +	depends on SPI_MASTER || I2C
> +	select GPIOLIB_IRQCHIP
> +	select REGMAP_I2C if I2C
> +	select REGMAP_SPI if SPI_MASTER
> +	help
> +	  SPI/I2C driver for Microchip MCP23S08/MCP23S17/MCP23008/MCP23017
> +	  I/O expanders.
> +	  This provides a GPIO interface supporting inputs and outputs.
> +	  The I2C versions of the chips can be used as interrupt-controller.
> +
>  config PINCTRL_MESON
>  	bool
>  	depends on OF
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index 0e9b2226a7c2..59d793aa3db3 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_PINCTRL_DA850_PUPD) += pinctrl-da850-pupd.o
>  obj-$(CONFIG_PINCTRL_DIGICOLOR)	+= pinctrl-digicolor.o
>  obj-$(CONFIG_PINCTRL_FALCON)	+= pinctrl-falcon.o
>  obj-$(CONFIG_PINCTRL_MAX77620)	+= pinctrl-max77620.o
> +obj-$(CONFIG_PINCTRL_MCP23S08)	+= pinctrl-mcp23s08.o
>  obj-$(CONFIG_PINCTRL_MESON)	+= meson/
>  obj-$(CONFIG_PINCTRL_OXNAS)	+= pinctrl-oxnas.o
>  obj-$(CONFIG_PINCTRL_PALMAS)	+= pinctrl-palmas.o
> diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
> similarity index 100%
> rename from drivers/gpio/gpio-mcp23s08.c
> rename to drivers/pinctrl/pinctrl-mcp23s08.c

Re: [PATCHv3 00/14] mcp23s08 pinconf & cleanup

[Linus Walleij]

On Mon, May 15, 2017 at 11:24 AM, Sebastian Reichel
<sebastian.reichel@collabora.co.uk> wrote:

> Here is a rebase of my PATCHv2 in combination with the second patchset, which
> migrates to regmap based register caching. I also added a few more patches, that
> do misc. cleanups in the driver and remove ~100 loc (that's 10%).
>
> The first patch and the 12th patch ("simplify spi_present_mask handling") needs
> Acked-by from Steven Miao for the blackfin architecture changes (defconfig, two
> boardfiles) and from Vladimir Zapolskiy or Sylvain Lemieux for the lpc32xx_defconfig
> changes.

I have applied all patches to the mcp23s08 branch in the pinctrl tree:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/

I have also merged this branch to devel and into the GPIO devel
branch for testing in linux-next.

I assume the blackfin maintainers are not upset, else they can say
so, an explicit ACK would be nice :)

Hats off for this very nice patch set, please test it in linux-next as
it trickles in there.

Yours,
Linus Walleij