Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1632132 > unrolled thread

[PATCHv2 0/6] mcp23s08 pinconf support

Back to article view | Back to linux.kernel

Contents

  [PATCHv2 0/6] mcp23s08 pinconf support Sebastian Reichel <sebastian.reichel@collabora.co.uk> - 2017-04-27 16:30 +0200
    [PATCHv2 6/6] pinctrl: mcp23s08: drop pullup config from pdata Sebastian Reichel <sebastian.reichel@collabora.co.uk> - 2017-04-27 16:30 +0200
    [PATCHv2 1/6] gpio: mcp23s08: move to pinctrl Sebastian Reichel <sebastian.reichel@collabora.co.uk> - 2017-04-27 16:30 +0200
    [PATCHv2 2/6] blackfin: boards: MCP23S08 config has been renamed Sebastian Reichel <sebastian.reichel@collabora.co.uk> - 2017-04-27 16:30 +0200
    [PATCHv2 5/6] pinctrl: mcp23s08: add pinconf support Sebastian Reichel <sebastian.reichel@collabora.co.uk> - 2017-04-27 16:30 +0200
      Re: [PATCHv2 5/6] pinctrl: mcp23s08: add pinconf support Enric Balletbo i Serra <enric.balletbo@collabora.com> - 2017-05-10 16:40 +0200
    Re: [PATCHv2 0/6] mcp23s08 pinconf support Linus Walleij <linus.walleij@linaro.org> - 2017-04-28 10:30 +0200
      Re: [PATCHv2 0/6] mcp23s08 pinconf support Sebastian Reichel <sebastian.reichel@collabora.co.uk> - 2017-04-28 10:50 +0200
        Re: [PATCHv2 0/6] mcp23s08 pinconf support Linus Walleij <linus.walleij@linaro.org> - 2017-05-11 15:40 +0200

#1632132 — [PATCHv2 0/6] mcp23s08 pinconf support

Hi,

Back in January I sent patches adding pinconf support
for configuring mcp23s08's pull-ups. Apart from my
custom Raspberry Pi setup the pull-up support is also
needed by Toby Churchill SL50.

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

I did not add a stub for GPIO_MCP23S08 selecting the
new PINCTRL_MCP23S08, since that would require a copy
of all "depends on" information, which is quite messy.
Also it seems to be quite unusual (I found no instance
of a select being used for a renamed config option).

I see the following merge strategies:

 a) Ignore limited bisectability and merge all
    patches through their own subsystem. Compilation
	should always succeed, but the blackfin boards
	will be without mcp23s08 support with only one of
	patch 1 and 2 being applied.
 b) - Squash patch 1 & 2
    - Queue that patch into gpio/pinctrl
	- Provide immutable-branch for blackfin & arm
	- Blackfin & ARM can pull immutable-branch & apply defconfig patch
	- gpio/pinctrl can queue patch 5 & 6
 c) The same as b), but squash patch 1-4 to guarantee
    bisectability for defconfig.

-- Sebastian

Sebastian Reichel (6):
  gpio: mcp23s08: move to pinctrl
  blackfin: boards: MCP23S08 config has been renamed
  blackfin: defconfig: MCP23S08 config has been renamed
  arm: lpc32xx: defconfig: MCP23S08 config has been renamed
  pinctrl: mcp23s08: add pinconf support
  pinctrl: mcp23s08: drop pullup config from pdata

 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                            |  14 ++
 drivers/pinctrl/Makefile                           |   1 +
 .../gpio-mcp23s08.c => pinctrl/pinctrl-mcp23s08.c} | 206 +++++++++++++++++----
 include/linux/spi/mcp23s08.h                       |   1 -
 10 files changed, 196 insertions(+), 56 deletions(-)
 rename drivers/{gpio/gpio-mcp23s08.c => pinctrl/pinctrl-mcp23s08.c} (86%)

-- 
2.11.0

[toc] | [next] | [standalone]

#1632133 — [PATCHv2 6/6] pinctrl: mcp23s08: drop pullup config from pdata

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 8aacedcf814c..7fad3a9e2222 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

[toc] | [prev] | [next] | [standalone]

#1632134 — [PATCHv2 1/6] gpio: mcp23s08: move to pinctrl

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>
---
 drivers/gpio/Kconfig                                    | 17 -----------------
 drivers/gpio/Makefile                                   |  1 -
 drivers/pinctrl/Kconfig                                 | 13 +++++++++++++
 drivers/pinctrl/Makefile                                |  1 +
 .../gpio-mcp23s08.c => pinctrl/pinctrl-mcp23s08.c}      |  0
 5 files changed, 14 insertions(+), 18 deletions(-)
 rename drivers/{gpio/gpio-mcp23s08.c => pinctrl/pinctrl-mcp23s08.c} (100%)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 05043071fc98..4d64b109826c 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1217,23 +1217,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 becb96c724fe..c210ce4d91ac 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -76,7 +76,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 8f8c2af45781..cccff799a88a 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -135,6 +135,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 a251f439626f..7a0c6a9c166e 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -18,6 +18,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

[toc] | [prev] | [next] | [standalone]

#1632136 — [PATCHv2 2/6] blackfin: boards: MCP23S08 config has been renamed

The mcp23s08 driver now supports pinconf, so the config
option has been renamed from CONFIG_GPIO_MCP23S08 to
CONFIG_PINCTRL_MCP23S08.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 arch/blackfin/mach-bf527/boards/tll6527m.c | 4 ++--
 arch/blackfin/mach-bf609/boards/ezkit.c    | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

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
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1632137 — [PATCHv2 5/6] pinctrl: mcp23s08: add pinconf support

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 cccff799a88a..2ff95fe3017d 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -142,6 +142,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..8aacedcf814c 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 = pinctrl_register(&mcp->pinctrl_desc, dev, 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

[toc] | [prev] | [next] | [standalone]

#1638884 — Re: [PATCHv2 5/6] pinctrl: mcp23s08: add pinconf support

On 27/04/17 16:19, 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>
> ---
>  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 cccff799a88a..2ff95fe3017d 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -142,6 +142,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..8aacedcf814c 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 = pinctrl_register(&mcp->pinctrl_desc, dev, 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);
> 

Many thanks for this patch, tested on SL50 board by configuring the pins to read
the hardware revision that needs to have the pullup confifured, it works as
expected so,

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

[toc] | [prev] | [next] | [standalone]

#1632619

On Thu, Apr 27, 2017 at 4:19 PM, Sebastian Reichel
<sebastian.reichel@collabora.co.uk> wrote:

> Back in January I sent patches adding pinconf support
> for configuring mcp23s08's pull-ups. Apart from my
> custom Raspberry Pi setup the pull-up support is also
> needed by Toby Churchill SL50.
>
> 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

I like this and I would like to queue it early in the v4.13 development
cycle.

> I see the following merge strategies:
>
>  a) Ignore limited bisectability and merge all
>     patches through their own subsystem. Compilation
>         should always succeed, but the blackfin boards
>         will be without mcp23s08 support with only one of
>         patch 1 and 2 being applied.
>  b) - Squash patch 1 & 2
>     - Queue that patch into gpio/pinctrl
>         - Provide immutable-branch for blackfin & arm
>         - Blackfin & ARM can pull immutable-branch & apply defconfig patch
>         - gpio/pinctrl can queue patch 5 & 6
>  c) The same as b), but squash patch 1-4 to guarantee
>     bisectability for defconfig.

Can't I just get ACKs from the blackfin and ARM SoC maintainers
for their subsystems, merge it all into pinctrl and GPIO and provide them
an immutable branch from pinctrl to pull in if they need it?

I will anyway need to have an immutable branch between pinctrl
and GPIO for this.

I expect ARM and blackfin can optimistically ignore my branch unless
they get merge conflicts.

Yours,
Linus Walleij

[toc] | [prev] | [next] | [standalone]

#1632663

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Fri, Apr 28, 2017 at 10:22:24AM +0200, Linus Walleij wrote:
> On Thu, Apr 27, 2017 at 4:19 PM, Sebastian Reichel
> <sebastian.reichel@collabora.co.uk> wrote:
> 
> > Back in January I sent patches adding pinconf support
> > for configuring mcp23s08's pull-ups. Apart from my
> > custom Raspberry Pi setup the pull-up support is also
> > needed by Toby Churchill SL50.
> >
> > 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
> 
> I like this and I would like to queue it early in the v4.13
> development cycle.

Thanks.

> > I see the following merge strategies:
> >
> >  a) Ignore limited bisectability and merge all
> >     patches through their own subsystem. Compilation
> >         should always succeed, but the blackfin boards
> >         will be without mcp23s08 support with only one of
> >         patch 1 and 2 being applied.
> >  b) - Squash patch 1 & 2
> >     - Queue that patch into gpio/pinctrl
> >         - Provide immutable-branch for blackfin & arm
> >         - Blackfin & ARM can pull immutable-branch & apply defconfig patch
> >         - gpio/pinctrl can queue patch 5 & 6
> >  c) The same as b), but squash patch 1-4 to guarantee
> >     bisectability for defconfig.
> 
> Can't I just get ACKs from the blackfin and ARM SoC maintainers
> for their subsystems, merge it all into pinctrl and GPIO and provide them
> an immutable branch from pinctrl to pull in if they need it?
>
> I will anyway need to have an immutable branch between pinctrl
> and GPIO for this.
> 
> I expect ARM and blackfin can optimistically ignore my branch unless
> they get merge conflicts.

Yes, that should also work.

-- Sebastian

[toc] | [prev] | [next] | [standalone]

#1639441

On Fri, Apr 28, 2017 at 10:43 AM, Sebastian Reichel
<sebastian.reichel@collabora.co.uk> wrote:
> On Fri, Apr 28, 2017 at 10:22:24AM +0200, Linus Walleij wrote:
>> On Thu, Apr 27, 2017 at 4:19 PM, Sebastian Reichel
>> <sebastian.reichel@collabora.co.uk> wrote:
>>
>> > Back in January I sent patches adding pinconf support
>> > for configuring mcp23s08's pull-ups. Apart from my
>> > custom Raspberry Pi setup the pull-up support is also
>> > needed by Toby Churchill SL50.
>> >
>> > 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
>>
>> I like this and I would like to queue it early in the v4.13
>> development cycle.
>
> Thanks.

I wanna queue this.

Can you resend this patch set based on Torvald's HEAD or v4.12-rc1
when that is out after the weekend and:

- Request explicit ACKs from blackfin and ARM maintainers.
  (The ARM people are at arm@kernel.org)

- Include the regmap conversion in the last two patches you
  sent.

Yours,
Linus Walleij

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web