All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] mcp23s08 pinconf support
@ 2017-01-27 14:47 Sebastian Reichel
  2017-01-27 14:47 ` [PATCH 1/2] gpio: mcp23s08: use regmap Sebastian Reichel
  2017-01-27 14:47 ` [PATCH 2/2] gpio: mcp23s08: add pinconf support Sebastian Reichel
  0 siblings, 2 replies; 11+ messages in thread
From: Sebastian Reichel @ 2017-01-27 14:47 UTC (permalink / raw)
  To: Sebastian Reichel, Linus Walleij, Alexandre Courbot
  Cc: linux-gpio, linux-kernel

Hi,

Here are two patches for the mcp23s08 driver adding proper support for the
pull-up resistors. The regmap conversion was mainly done to improve the
debugging capabilities, but is also a nice cleanup. Further cleanups could
be done by using regmap's caching instead of the open-coded one.

The patches were tested with a couple of mcp23017 connected to the I²C bus of
an RPi (and modified DT).

-- Sebastian

Sebastian Reichel (2):
  gpio: mcp23s08: use regmap
  gpio: mcp23s08: add pinconf support

 drivers/gpio/gpio-mcp23s08.c | 430 ++++++++++++++++++++++++++-----------------
 1 file changed, 261 insertions(+), 169 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] gpio: mcp23s08: use regmap
  2017-01-27 14:47 [PATCH 0/2] mcp23s08 pinconf support Sebastian Reichel
@ 2017-01-27 14:47 ` Sebastian Reichel
  2017-01-30 14:58   ` Linus Walleij
  2017-01-27 14:47 ` [PATCH 2/2] gpio: mcp23s08: add pinconf support Sebastian Reichel
  1 sibling, 1 reply; 11+ messages in thread
From: Sebastian Reichel @ 2017-01-27 14:47 UTC (permalink / raw)
  To: Sebastian Reichel, Linus Walleij, Alexandre Courbot
  Cc: linux-gpio, linux-kernel

Use regmap API to save some lines of codes and have
debugfs support for all of the MCP's registers.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpio/gpio-mcp23s08.c | 320 ++++++++++++++++++-------------------------
 1 file changed, 130 insertions(+), 190 deletions(-)

diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/gpio/gpio-mcp23s08.c
index 504550665091..bdb692345428 100644
--- a/drivers/gpio/gpio-mcp23s08.c
+++ b/drivers/gpio/gpio-mcp23s08.c
@@ -23,6 +23,7 @@
 #include <linux/interrupt.h>
 #include <linux/of_irq.h>
 #include <linux/of_device.h>
+#include <linux/regmap.h>
 
 /**
  * MCP types supported by driver
@@ -58,16 +59,10 @@
 
 struct mcp23s08;
 
-struct mcp23s08_ops {
-	int	(*read)(struct mcp23s08 *mcp, unsigned reg);
-	int	(*write)(struct mcp23s08 *mcp, unsigned reg, unsigned val);
-	int	(*read_regs)(struct mcp23s08 *mcp, unsigned reg,
-			     u16 *vals, unsigned n);
-};
-
 struct mcp23s08 {
 	u8			addr;
 	bool			irq_active_high;
+	bool			reg_shift;
 
 	u16			cache[11];
 	u16			irq_rise;
@@ -80,188 +75,126 @@ struct mcp23s08 {
 
 	struct gpio_chip	chip;
 
-	const struct mcp23s08_ops	*ops;
-	void			*data; /* ops specific data */
+	struct regmap		*regmap;
+	struct device		*dev;
 };
 
-/* A given spi_device can represent up to eight mcp23sxx chips
- * sharing the same chipselect but using different addresses
- * (e.g. chips #0 and #3 might be populated, but not #1 or $2).
- * Driver data holds all the per-chip data.
- */
-struct mcp23s08_driver_data {
-	unsigned		ngpio;
-	struct mcp23s08		*mcp[8];
-	struct mcp23s08		chip[];
-};
+static const struct regmap_config mcp23x08_regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
 
-/*----------------------------------------------------------------------*/
-
-#if IS_ENABLED(CONFIG_I2C)
-
-static int mcp23008_read(struct mcp23s08 *mcp, unsigned reg)
-{
-	return i2c_smbus_read_byte_data(mcp->data, reg);
-}
-
-static int mcp23008_write(struct mcp23s08 *mcp, unsigned reg, unsigned val)
-{
-	return i2c_smbus_write_byte_data(mcp->data, reg, val);
-}
-
-static int
-mcp23008_read_regs(struct mcp23s08 *mcp, unsigned reg, u16 *vals, unsigned n)
-{
-	while (n--) {
-		int ret = mcp23008_read(mcp, reg++);
-		if (ret < 0)
-			return ret;
-		*vals++ = ret;
-	}
-
-	return 0;
-}
-
-static int mcp23017_read(struct mcp23s08 *mcp, unsigned reg)
-{
-	return i2c_smbus_read_word_data(mcp->data, reg << 1);
-}
-
-static int mcp23017_write(struct mcp23s08 *mcp, unsigned reg, unsigned val)
-{
-	return i2c_smbus_write_word_data(mcp->data, reg << 1, val);
-}
-
-static int
-mcp23017_read_regs(struct mcp23s08 *mcp, unsigned reg, u16 *vals, unsigned n)
-{
-	while (n--) {
-		int ret = mcp23017_read(mcp, reg++);
-		if (ret < 0)
-			return ret;
-		*vals++ = ret;
-	}
-
-	return 0;
-}
-
-static const struct mcp23s08_ops mcp23008_ops = {
-	.read		= mcp23008_read,
-	.write		= mcp23008_write,
-	.read_regs	= mcp23008_read_regs,
+	.reg_stride = 1,
+	.max_register = MCP_OLAT,
 };
 
-static const struct mcp23s08_ops mcp23017_ops = {
-	.read		= mcp23017_read,
-	.write		= mcp23017_write,
-	.read_regs	= mcp23017_read_regs,
-};
+static const struct regmap_config mcp23x17_regmap = {
+	.reg_bits = 8,
+	.val_bits = 16,
 
-#endif /* CONFIG_I2C */
+	.reg_stride = 2,
+	.max_register = MCP_OLAT << 1,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+};
 
 /*----------------------------------------------------------------------*/
 
 #ifdef CONFIG_SPI_MASTER
 
-static int mcp23s08_read(struct mcp23s08 *mcp, unsigned reg)
+static int mcp23sxx_spi_write(void *context, const void *data, size_t count)
 {
-	u8	tx[2], rx[1];
-	int	status;
+	struct mcp23s08 *mcp = context;
+	struct spi_device *spi = to_spi_device(mcp->dev);
+	struct spi_message m;
+	struct spi_transfer t[2] = { { .tx_buf = &mcp->addr, .len = 1, },
+				     { .tx_buf = data, .len = count, }, };
 
-	tx[0] = mcp->addr | 0x01;
-	tx[1] = reg;
-	status = spi_write_then_read(mcp->data, tx, sizeof(tx), rx, sizeof(rx));
-	return (status < 0) ? status : rx[0];
+	spi_message_init(&m);
+	spi_message_add_tail(&t[0], &m);
+	spi_message_add_tail(&t[1], &m);
+
+	return spi_sync(spi, &m);
 }
 
-static int mcp23s08_write(struct mcp23s08 *mcp, unsigned reg, unsigned val)
+static int mcp23sxx_spi_gather_write(void *context,
+				const void *reg, size_t reg_size,
+				const void *val, size_t val_size)
 {
-	u8	tx[3];
+	struct mcp23s08 *mcp = context;
+	struct spi_device *spi = to_spi_device(mcp->dev);
+	struct spi_message m;
+	struct spi_transfer t[3] = { { .tx_buf = &mcp->addr, .len = 1, },
+				     { .tx_buf = reg, .len = reg_size, },
+				     { .tx_buf = val, .len = val_size, }, };
 
-	tx[0] = mcp->addr;
-	tx[1] = reg;
-	tx[2] = val;
-	return spi_write_then_read(mcp->data, tx, sizeof(tx), NULL, 0);
+	spi_message_init(&m);
+	spi_message_add_tail(&t[0], &m);
+	spi_message_add_tail(&t[1], &m);
+	spi_message_add_tail(&t[2], &m);
+
+	return spi_sync(spi, &m);
 }
 
-static int
-mcp23s08_read_regs(struct mcp23s08 *mcp, unsigned reg, u16 *vals, unsigned n)
+static int mcp23sxx_spi_read(void *context, const void *reg, size_t reg_size,
+				void *val, size_t val_size)
 {
-	u8	tx[2], *tmp;
-	int	status;
+	struct mcp23s08 *mcp = context;
+	struct spi_device *spi = to_spi_device(mcp->dev);
+	u8 tx[2];
 
-	if ((n + reg) > sizeof(mcp->cache))
+	if (reg_size != 1)
 		return -EINVAL;
+
 	tx[0] = mcp->addr | 0x01;
-	tx[1] = reg;
+	tx[1] = *((u8 *) reg);
 
-	tmp = (u8 *)vals;
-	status = spi_write_then_read(mcp->data, tx, sizeof(tx), tmp, n);
-	if (status >= 0) {
-		while (n--)
-			vals[n] = tmp[n]; /* expand to 16bit */
-	}
-	return status;
+	return spi_write_then_read(spi, tx, sizeof(tx), val, val_size);
 }
 
-static int mcp23s17_read(struct mcp23s08 *mcp, unsigned reg)
-{
-	u8	tx[2], rx[2];
-	int	status;
+static const struct regmap_bus mcp23sxx_spi_regmap = {
+	.write = mcp23sxx_spi_write,
+	.gather_write = mcp23sxx_spi_gather_write,
+	.read = mcp23sxx_spi_read,
+};
 
-	tx[0] = mcp->addr | 0x01;
-	tx[1] = reg << 1;
-	status = spi_write_then_read(mcp->data, tx, sizeof(tx), rx, sizeof(rx));
-	return (status < 0) ? status : (rx[0] | (rx[1] << 8));
-}
+#endif /* CONFIG_SPI_MASTER */
 
-static int mcp23s17_write(struct mcp23s08 *mcp, unsigned reg, unsigned val)
+static int mcp_read(struct mcp23s08 *mcp, unsigned int reg, unsigned int *val)
 {
-	u8	tx[4];
-
-	tx[0] = mcp->addr;
-	tx[1] = reg << 1;
-	tx[2] = val;
-	tx[3] = val >> 8;
-	return spi_write_then_read(mcp->data, tx, sizeof(tx), NULL, 0);
+	return regmap_read(mcp->regmap, reg << mcp->reg_shift, val);
 }
 
-static int
-mcp23s17_read_regs(struct mcp23s08 *mcp, unsigned reg, u16 *vals, unsigned n)
+static int mcp_write(struct mcp23s08 *mcp, unsigned int reg, unsigned int val)
 {
-	u8	tx[2];
-	int	status;
+	return regmap_write(mcp->regmap, reg << mcp->reg_shift, val);
+}
 
-	if ((n + reg) > sizeof(mcp->cache))
-		return -EINVAL;
-	tx[0] = mcp->addr | 0x01;
-	tx[1] = reg << 1;
+static int mcp_update_cache(struct mcp23s08 *mcp)
+{
+	int ret, reg, i;
 
-	status = spi_write_then_read(mcp->data, tx, sizeof(tx),
-				     (u8 *)vals, n * 2);
-	if (status >= 0) {
-		while (n--)
-			vals[n] = __le16_to_cpu((__le16)vals[n]);
+	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 status;
+	return 0;
 }
 
-static const struct mcp23s08_ops mcp23s08_ops = {
-	.read		= mcp23s08_read,
-	.write		= mcp23s08_write,
-	.read_regs	= mcp23s08_read_regs,
-};
+/*----------------------------------------------------------------------*/
 
-static const struct mcp23s08_ops mcp23s17_ops = {
-	.read		= mcp23s17_read,
-	.write		= mcp23s17_write,
-	.read_regs	= mcp23s17_read_regs,
+/* A given spi_device can represent up to eight mcp23sxx chips
+ * sharing the same chipselect but using different addresses
+ * (e.g. chips #0 and #3 might be populated, but not #1 or $2).
+ * Driver data holds all the per-chip data.
+ */
+struct mcp23s08_driver_data {
+	unsigned		ngpio;
+	struct mcp23s08		*mcp[8];
+	struct mcp23s08		chip[];
 };
 
-#endif /* CONFIG_SPI_MASTER */
-
-/*----------------------------------------------------------------------*/
 
 static int mcp23s08_direction_input(struct gpio_chip *chip, unsigned offset)
 {
@@ -270,7 +203,7 @@ static int mcp23s08_direction_input(struct gpio_chip *chip, unsigned offset)
 
 	mutex_lock(&mcp->lock);
 	mcp->cache[MCP_IODIR] |= (1 << offset);
-	status = mcp->ops->write(mcp, MCP_IODIR, mcp->cache[MCP_IODIR]);
+	status = mcp_write(mcp, MCP_IODIR, mcp->cache[MCP_IODIR]);
 	mutex_unlock(&mcp->lock);
 	return status;
 }
@@ -278,13 +211,13 @@ static int mcp23s08_direction_input(struct gpio_chip *chip, unsigned offset)
 static int mcp23s08_get(struct gpio_chip *chip, unsigned offset)
 {
 	struct mcp23s08	*mcp = gpiochip_get_data(chip);
-	int status;
+	int status, ret;
 
 	mutex_lock(&mcp->lock);
 
 	/* REVISIT reading this clears any IRQ ... */
-	status = mcp->ops->read(mcp, MCP_GPIO);
-	if (status < 0)
+	ret = mcp_read(mcp, MCP_GPIO, &status);
+	if (ret < 0)
 		status = 0;
 	else {
 		mcp->cache[MCP_GPIO] = status;
@@ -303,7 +236,7 @@ static int __mcp23s08_set(struct mcp23s08 *mcp, unsigned mask, int value)
 	else
 		olat &= ~mask;
 	mcp->cache[MCP_OLAT] = olat;
-	return mcp->ops->write(mcp, MCP_OLAT, olat);
+	return mcp_write(mcp, MCP_OLAT, olat);
 }
 
 static void mcp23s08_set(struct gpio_chip *chip, unsigned offset, int value)
@@ -327,7 +260,7 @@ mcp23s08_direction_output(struct gpio_chip *chip, unsigned offset, int value)
 	status = __mcp23s08_set(mcp, mask, value);
 	if (status == 0) {
 		mcp->cache[MCP_IODIR] &= ~mask;
-		status = mcp->ops->write(mcp, MCP_IODIR, mcp->cache[MCP_IODIR]);
+		status = mcp_write(mcp, MCP_IODIR, mcp->cache[MCP_IODIR]);
 	}
 	mutex_unlock(&mcp->lock);
 	return status;
@@ -341,16 +274,14 @@ static irqreturn_t mcp23s08_irq(int irq, void *data)
 	unsigned int child_irq;
 
 	mutex_lock(&mcp->lock);
-	intf = mcp->ops->read(mcp, MCP_INTF);
-	if (intf < 0) {
+	if (mcp_read(mcp, MCP_INTF, &intf) < 0) {
 		mutex_unlock(&mcp->lock);
 		return IRQ_HANDLED;
 	}
 
 	mcp->cache[MCP_INTF] = intf;
 
-	intcap = mcp->ops->read(mcp, MCP_INTCAP);
-	if (intcap < 0) {
+	if (mcp_read(mcp, MCP_INTCAP, &intcap) < 0) {
 		mutex_unlock(&mcp->lock);
 		return IRQ_HANDLED;
 	}
@@ -435,9 +366,9 @@ static void mcp23s08_irq_bus_unlock(struct irq_data *data)
 	struct mcp23s08 *mcp = gpiochip_get_data(gc);
 
 	mutex_lock(&mcp->lock);
-	mcp->ops->write(mcp, MCP_GPINTEN, mcp->cache[MCP_GPINTEN]);
-	mcp->ops->write(mcp, MCP_DEFVAL, mcp->cache[MCP_DEFVAL]);
-	mcp->ops->write(mcp, MCP_INTCON, mcp->cache[MCP_INTCON]);
+	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]);
 	mutex_unlock(&mcp->lock);
 	mutex_unlock(&mcp->irq_lock);
 }
@@ -514,7 +445,7 @@ 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->ops->read_regs(mcp, 0, mcp->cache, ARRAY_SIZE(mcp->cache));
+	t = mcp_update_cache(mcp);
 	if (t < 0) {
 		seq_printf(s, " I/O ERROR %d\n", t);
 		goto done;
@@ -549,12 +480,12 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 			      void *data, unsigned addr, unsigned type,
 			      struct mcp23s08_platform_data *pdata, int cs)
 {
-	int status;
+	int status, ret;
 	bool mirror = false;
 
 	mutex_init(&mcp->lock);
 
-	mcp->data = data;
+	mcp->dev = dev;
 	mcp->addr = addr;
 	mcp->irq_active_high = false;
 
@@ -571,19 +502,25 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 	switch (type) {
 #ifdef CONFIG_SPI_MASTER
 	case MCP_TYPE_S08:
-		mcp->ops = &mcp23s08_ops;
+		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
+					       &mcp23x08_regmap);
+		mcp->reg_shift = 0;
 		mcp->chip.ngpio = 8;
 		mcp->chip.label = "mcp23s08";
 		break;
 
 	case MCP_TYPE_S17:
-		mcp->ops = &mcp23s17_ops;
+		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
+					       &mcp23x17_regmap);
+		mcp->reg_shift = 1;
 		mcp->chip.ngpio = 16;
 		mcp->chip.label = "mcp23s17";
 		break;
 
 	case MCP_TYPE_S18:
-		mcp->ops = &mcp23s17_ops;
+		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
+					       &mcp23x17_regmap);
+		mcp->reg_shift = 1;
 		mcp->chip.ngpio = 16;
 		mcp->chip.label = "mcp23s18";
 		break;
@@ -591,13 +528,15 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 
 #if IS_ENABLED(CONFIG_I2C)
 	case MCP_TYPE_008:
-		mcp->ops = &mcp23008_ops;
+		mcp->regmap = devm_regmap_init_i2c(data, &mcp23x08_regmap);
+		mcp->reg_shift = 0;
 		mcp->chip.ngpio = 8;
 		mcp->chip.label = "mcp23008";
 		break;
 
 	case MCP_TYPE_017:
-		mcp->ops = &mcp23017_ops;
+		mcp->regmap = devm_regmap_init_i2c(data, &mcp23x17_regmap);
+		mcp->reg_shift = 1;
 		mcp->chip.ngpio = 16;
 		mcp->chip.label = "mcp23017";
 		break;
@@ -608,6 +547,9 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 		return -EINVAL;
 	}
 
+	if (IS_ERR(mcp->regmap))
+		return PTR_ERR(mcp->regmap);
+
 	mcp->chip.base = pdata->base;
 	mcp->chip.can_sleep = true;
 	mcp->chip.parent = dev;
@@ -617,8 +559,8 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 	 * and MCP_IOCON.HAEN = 1, so we work with all chips.
 	 */
 
-	status = mcp->ops->read(mcp, MCP_IOCON);
-	if (status < 0)
+	ret = mcp_read(mcp, MCP_IOCON, &status);
+	if (ret < 0)
 		goto fail;
 
 	mcp->irq_controller = pdata->irq_controller;
@@ -646,51 +588,49 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 		if (type == MCP_TYPE_S18)
 			status |= IOCON_INTCC | (IOCON_INTCC << 8);
 
-		status = mcp->ops->write(mcp, MCP_IOCON, status);
-		if (status < 0)
+		ret = mcp_write(mcp, MCP_IOCON, status);
+		if (ret < 0)
 			goto fail;
 	}
 
 	/* configure ~100K pullups */
-	status = mcp->ops->write(mcp, MCP_GPPU, pdata->chip[cs].pullups);
-	if (status < 0)
+	ret = mcp_write(mcp, MCP_GPPU, pdata->chip[cs].pullups);
+	if (ret < 0)
 		goto fail;
 
-	status = mcp->ops->read_regs(mcp, 0, mcp->cache, ARRAY_SIZE(mcp->cache));
-	if (status < 0)
+	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;
-		status = mcp->ops->write(mcp, MCP_IPOL, 0);
-		if (status < 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;
-		status = mcp->ops->write(mcp, MCP_GPINTEN, 0);
-		if (status < 0)
+		ret = mcp_write(mcp, MCP_GPINTEN, 0);
+		if (ret < 0)
 			goto fail;
 	}
 
-	status = gpiochip_add_data(&mcp->chip, mcp);
-	if (status < 0)
+	ret = gpiochip_add_data(&mcp->chip, mcp);
+	if (ret < 0)
 		goto fail;
 
 	if (mcp->irq && mcp->irq_controller) {
-		status = mcp23s08_irq_setup(mcp);
-		if (status) {
+		ret = mcp23s08_irq_setup(mcp);
+		if (ret)
 			goto fail;
-		}
 	}
 fail:
-	if (status < 0)
-		dev_dbg(dev, "can't setup chip %d, --> %d\n",
-			addr, status);
-	return status;
+	if (ret < 0)
+		dev_dbg(dev, "can't setup chip %d, --> %d\n", addr, ret);
+	return ret;
 }
 
 /*----------------------------------------------------------------------*/
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] gpio: mcp23s08: add pinconf support
  2017-01-27 14:47 [PATCH 0/2] mcp23s08 pinconf support Sebastian Reichel
  2017-01-27 14:47 ` [PATCH 1/2] gpio: mcp23s08: use regmap Sebastian Reichel
@ 2017-01-27 14:47 ` Sebastian Reichel
  2017-01-27 17:16     ` kbuild test robot
                     ` (2 more replies)
  1 sibling, 3 replies; 11+ messages in thread
From: Sebastian Reichel @ 2017-01-27 14:47 UTC (permalink / raw)
  To: Sebastian Reichel, Linus Walleij, Alexandre Courbot
  Cc: linux-gpio, linux-kernel

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

Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
 drivers/gpio/gpio-mcp23s08.c | 200 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 176 insertions(+), 24 deletions(-)

diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/gpio/gpio-mcp23s08.c
index bdb692345428..96abecaccc12 100644
--- a/drivers/gpio/gpio-mcp23s08.c
+++ b/drivers/gpio/gpio-mcp23s08.c
@@ -24,6 +24,10 @@
 #include <linux/of_irq.h>
 #include <linux/of_device.h>
 #include <linux/regmap.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/pinconf-generic.h>
 
 /**
  * MCP types supported by driver
@@ -77,6 +81,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 +103,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 +317,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
@@ -627,6 +762,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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] gpio: mcp23s08: add pinconf support
  2017-01-27 14:47 ` [PATCH 2/2] gpio: mcp23s08: add pinconf support Sebastian Reichel
@ 2017-01-27 17:16     ` kbuild test robot
  2017-01-27 19:00     ` kbuild test robot
  2017-01-30 15:08   ` Linus Walleij
  2 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2017-01-27 17:16 UTC (permalink / raw)
  Cc: kbuild-all, Sebastian Reichel, Linus Walleij, Alexandre Courbot,
	linux-gpio, linux-kernel

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

Hi Sebastian,

[auto build test ERROR on gpio/for-next]
[also build test ERROR on v4.10-rc5 next-20170125]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sebastian-Reichel/mcp23s08-pinconf-support/20170127-230243
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: x86_64-randconfig-x003-201704 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

>> drivers/gpio/gpio-mcp23s08.c:86:22: error: field 'pinctrl_desc' has incomplete type
     struct pinctrl_desc pinctrl_desc;
                         ^~~~~~~~~~~~
>> drivers/gpio/gpio-mcp23s08.c:139:38: error: array type has incomplete element type 'struct pinctrl_pin_desc'
    static const struct pinctrl_pin_desc mcp23x08_pins[] = {
                                         ^~~~~~~~~~~~~
>> drivers/gpio/gpio-mcp23s08.c:140:2: error: implicit declaration of function 'PINCTRL_PIN' [-Werror=implicit-function-declaration]
     PINCTRL_PIN(0, "gpio0"),
     ^~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:150:38: error: array type has incomplete element type 'struct pinctrl_pin_desc'
    static const struct pinctrl_pin_desc mcp23x17_pins[] = {
                                         ^~~~~~~~~~~~~
>> drivers/gpio/gpio-mcp23s08.c:188:21: error: variable 'mcp_pinctrl_ops' has initializer but incomplete type
    static const struct pinctrl_ops mcp_pinctrl_ops = {
                        ^~~~~~~~~~~
>> drivers/gpio/gpio-mcp23s08.c:189:2: error: unknown field 'get_groups_count' specified in initializer
     .get_groups_count = mcp_pinctrl_get_groups_count,
     ^
>> drivers/gpio/gpio-mcp23s08.c:189:22: warning: excess elements in struct initializer
     .get_groups_count = mcp_pinctrl_get_groups_count,
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:189:22: note: (near initialization for 'mcp_pinctrl_ops')
>> drivers/gpio/gpio-mcp23s08.c:190:2: error: unknown field 'get_group_name' specified in initializer
     .get_group_name = mcp_pinctrl_get_group_name,
     ^
   drivers/gpio/gpio-mcp23s08.c:190:20: warning: excess elements in struct initializer
     .get_group_name = mcp_pinctrl_get_group_name,
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:190:20: note: (near initialization for 'mcp_pinctrl_ops')
>> drivers/gpio/gpio-mcp23s08.c:191:2: error: unknown field 'get_group_pins' specified in initializer
     .get_group_pins = mcp_pinctrl_get_group_pins,
     ^
   drivers/gpio/gpio-mcp23s08.c:191:20: warning: excess elements in struct initializer
     .get_group_pins = mcp_pinctrl_get_group_pins,
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:191:20: note: (near initialization for 'mcp_pinctrl_ops')
>> drivers/gpio/gpio-mcp23s08.c:193:2: error: unknown field 'dt_node_to_map' specified in initializer
     .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
     ^
>> drivers/gpio/gpio-mcp23s08.c:193:20: error: 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
     .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:193:20: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:193:20: note: (near initialization for 'mcp_pinctrl_ops')
>> drivers/gpio/gpio-mcp23s08.c:194:2: error: unknown field 'dt_free_map' specified in initializer
     .dt_free_map = pinconf_generic_dt_free_map,
     ^
>> drivers/gpio/gpio-mcp23s08.c:194:17: error: 'pinconf_generic_dt_free_map' undeclared here (not in a function)
     .dt_free_map = pinconf_generic_dt_free_map,
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:194:17: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:194:17: note: (near initialization for 'mcp_pinctrl_ops')
   drivers/gpio/gpio-mcp23s08.c: In function 'mcp_pinconf_get':
>> drivers/gpio/gpio-mcp23s08.c:201:25: error: implicit declaration of function 'pinctrl_dev_get_drvdata' [-Werror=implicit-function-declaration]
     struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
                            ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpio/gpio-mcp23s08.c:201:25: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
   drivers/gpio/gpio-mcp23s08.c: In function 'mcp_pinconf_set':
   drivers/gpio/gpio-mcp23s08.c:226:25: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
     struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
                            ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c: At top level:
>> drivers/gpio/gpio-mcp23s08.c:252:21: error: variable 'mcp_pinconf_ops' has initializer but incomplete type
    static const struct pinconf_ops mcp_pinconf_ops = {
                        ^~~~~~~~~~~
>> drivers/gpio/gpio-mcp23s08.c:253:2: error: unknown field 'pin_config_get' specified in initializer
     .pin_config_get = mcp_pinconf_get,
     ^
   drivers/gpio/gpio-mcp23s08.c:253:20: warning: excess elements in struct initializer
     .pin_config_get = mcp_pinconf_get,
                       ^~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:253:20: note: (near initialization for 'mcp_pinconf_ops')
>> drivers/gpio/gpio-mcp23s08.c:254:2: error: unknown field 'pin_config_set' specified in initializer
     .pin_config_set = mcp_pinconf_set,
     ^
   drivers/gpio/gpio-mcp23s08.c:254:20: warning: excess elements in struct initializer
     .pin_config_set = mcp_pinconf_set,
                       ^~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:254:20: note: (near initialization for 'mcp_pinconf_ops')
>> drivers/gpio/gpio-mcp23s08.c:255:2: error: unknown field 'is_generic' specified in initializer
     .is_generic = true,
     ^
   drivers/gpio/gpio-mcp23s08.c:255:16: warning: excess elements in struct initializer
     .is_generic = true,
                   ^~~~
   drivers/gpio/gpio-mcp23s08.c:255:16: note: (near initialization for 'mcp_pinconf_ops')
   drivers/gpio/gpio-mcp23s08.c: In function 'mcp23s08_probe_one':
>> drivers/gpio/gpio-mcp23s08.c:776:17: error: implicit declaration of function 'pinctrl_register' [-Werror=implicit-function-declaration]
     mcp->pctldev = pinctrl_register(&mcp->pinctrl_desc, dev, mcp);
                    ^~~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c: At top level:
>> drivers/gpio/gpio-mcp23s08.c:188:33: error: storage size of 'mcp_pinctrl_ops' isn't known
    static const struct pinctrl_ops mcp_pinctrl_ops = {
                                    ^~~~~~~~~~~~~~~

vim +/pinctrl_desc +86 drivers/gpio/gpio-mcp23s08.c

    80		struct gpio_chip	chip;
    81	
    82		struct regmap		*regmap;
    83		struct device		*dev;
    84	
    85		struct pinctrl_dev	*pctldev;
  > 86		struct pinctrl_desc	pinctrl_desc;
    87	};
    88	
    89	static const struct regmap_config mcp23x08_regmap = {
    90		.reg_bits = 8,
    91		.val_bits = 8,
    92	
    93		.reg_stride = 1,
    94		.max_register = MCP_OLAT,
    95	};
    96	
    97	static const struct regmap_config mcp23x17_regmap = {
    98		.reg_bits = 8,
    99		.val_bits = 16,
   100	
   101		.reg_stride = 2,
   102		.max_register = MCP_OLAT << 1,
   103		.val_format_endian = REGMAP_ENDIAN_LITTLE,
   104	};
   105	
   106	static int mcp_read(struct mcp23s08 *mcp, unsigned int reg, unsigned int *val)
   107	{
   108		return regmap_read(mcp->regmap, reg << mcp->reg_shift, val);
   109	}
   110	
   111	static int mcp_write(struct mcp23s08 *mcp, unsigned int reg, unsigned int val)
   112	{
   113		return regmap_write(mcp->regmap, reg << mcp->reg_shift, val);
   114	}
   115	
   116	static int mcp_set_bit(struct mcp23s08 *mcp, unsigned int reg,
   117			       unsigned int pin, bool enabled)
   118	{
   119		u16 val  = enabled ? 0xffff : 0x0000;
   120		u16 mask = BIT(pin);
   121		return regmap_update_bits(mcp->regmap, reg << mcp->reg_shift,
   122					  mask, val);
   123	}
   124	
   125	static int mcp_update_cache(struct mcp23s08 *mcp)
   126	{
   127		int ret, reg, i;
   128	
   129		for (i = 0; i < ARRAY_SIZE(mcp->cache); i++) {
   130			ret = mcp_read(mcp, i, &reg);
   131			if (ret < 0)
   132				return ret;
   133			mcp->cache[i] = reg;
   134		}
   135	
   136		return 0;
   137	}
   138	
 > 139	static const struct pinctrl_pin_desc mcp23x08_pins[] = {
 > 140		PINCTRL_PIN(0, "gpio0"),
   141		PINCTRL_PIN(1, "gpio1"),
   142		PINCTRL_PIN(2, "gpio2"),
   143		PINCTRL_PIN(3, "gpio3"),
   144		PINCTRL_PIN(4, "gpio4"),
   145		PINCTRL_PIN(5, "gpio5"),
   146		PINCTRL_PIN(6, "gpio6"),
   147		PINCTRL_PIN(7, "gpio7"),
   148	};
   149	
 > 150	static const struct pinctrl_pin_desc mcp23x17_pins[] = {
   151		PINCTRL_PIN(0, "gpio0"),
   152		PINCTRL_PIN(1, "gpio1"),
   153		PINCTRL_PIN(2, "gpio2"),
   154		PINCTRL_PIN(3, "gpio3"),
   155		PINCTRL_PIN(4, "gpio4"),
   156		PINCTRL_PIN(5, "gpio5"),
   157		PINCTRL_PIN(6, "gpio6"),
   158		PINCTRL_PIN(7, "gpio7"),
   159		PINCTRL_PIN(8, "gpio8"),
   160		PINCTRL_PIN(9, "gpio9"),
   161		PINCTRL_PIN(10, "gpio10"),
   162		PINCTRL_PIN(11, "gpio11"),
   163		PINCTRL_PIN(12, "gpio12"),
   164		PINCTRL_PIN(13, "gpio13"),
   165		PINCTRL_PIN(14, "gpio14"),
   166		PINCTRL_PIN(15, "gpio15"),
   167	};
   168	
   169	static int mcp_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
   170	{
   171		return 0;
   172	}
   173	
   174	static const char *mcp_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
   175							unsigned int group)
   176	{
   177		return NULL;
   178	}
   179	
   180	static int mcp_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
   181						unsigned int group,
   182						const unsigned int **pins,
   183						unsigned int *num_pins)
   184	{
   185		return -ENOTSUPP;
   186	}
   187	
 > 188	static const struct pinctrl_ops mcp_pinctrl_ops = {
 > 189		.get_groups_count = mcp_pinctrl_get_groups_count,
 > 190		.get_group_name = mcp_pinctrl_get_group_name,
 > 191		.get_group_pins = mcp_pinctrl_get_group_pins,
   192	#ifdef CONFIG_OF
 > 193		.dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
 > 194		.dt_free_map = pinconf_generic_dt_free_map,
   195	#endif
   196	};
   197	
   198	static int mcp_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
   199				      unsigned long *config)
   200	{
 > 201		struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
   202		enum pin_config_param param = pinconf_to_config_param(*config);
   203		unsigned int data, status;
   204		int ret;
   205	
   206		switch (param) {
   207		case PIN_CONFIG_BIAS_PULL_UP:
   208			ret = mcp_read(mcp, MCP_GPPU, &data);
   209			if (ret < 0)
   210				return ret;
   211			status = (data & BIT(pin)) ? 1 : 0;
   212			break;
   213		default:
   214			dev_err(mcp->dev, "Invalid config param %04x\n", param);
   215			return -ENOTSUPP;
   216		}
   217	
   218		*config = 0;
   219	
   220		return status ? 0 : -EINVAL;
   221	}
   222	
   223	static int mcp_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
   224				      unsigned long *configs, unsigned int num_configs)
   225	{
   226		struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
   227		enum pin_config_param param;
   228		u32 arg, mask;
   229		u16 val;
   230		int ret = 0;
   231		int i;
   232	
   233		for (i = 0; i < num_configs; i++) {
   234			param = pinconf_to_config_param(configs[i]);
   235			arg = pinconf_to_config_argument(configs[i]);
   236	
   237			switch (param) {
   238			case PIN_CONFIG_BIAS_PULL_UP:
   239				val = arg ? 0xFFFF : 0x0000;
   240				mask = BIT(pin);
   241				ret = mcp_set_bit(mcp, MCP_GPPU, pin, arg);
   242				break;
   243			default:
   244				dev_err(mcp->dev, "Invalid config param %04x\n", param);
   245				return -ENOTSUPP;
   246			}
   247		}
   248	
   249		return ret;
   250	}
   251	
 > 252	static const struct pinconf_ops mcp_pinconf_ops = {
 > 253		.pin_config_get = mcp_pinconf_get,
 > 254		.pin_config_set = mcp_pinconf_set,
 > 255		.is_generic = true,
   256	};
   257	
   258	/*----------------------------------------------------------------------*/

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24197 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] gpio: mcp23s08: add pinconf support
@ 2017-01-27 17:16     ` kbuild test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2017-01-27 17:16 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: kbuild-all, Sebastian Reichel, Linus Walleij, Alexandre Courbot,
	linux-gpio, linux-kernel

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

Hi Sebastian,

[auto build test ERROR on gpio/for-next]
[also build test ERROR on v4.10-rc5 next-20170125]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sebastian-Reichel/mcp23s08-pinconf-support/20170127-230243
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: x86_64-randconfig-x003-201704 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

>> drivers/gpio/gpio-mcp23s08.c:86:22: error: field 'pinctrl_desc' has incomplete type
     struct pinctrl_desc pinctrl_desc;
                         ^~~~~~~~~~~~
>> drivers/gpio/gpio-mcp23s08.c:139:38: error: array type has incomplete element type 'struct pinctrl_pin_desc'
    static const struct pinctrl_pin_desc mcp23x08_pins[] = {
                                         ^~~~~~~~~~~~~
>> drivers/gpio/gpio-mcp23s08.c:140:2: error: implicit declaration of function 'PINCTRL_PIN' [-Werror=implicit-function-declaration]
     PINCTRL_PIN(0, "gpio0"),
     ^~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:150:38: error: array type has incomplete element type 'struct pinctrl_pin_desc'
    static const struct pinctrl_pin_desc mcp23x17_pins[] = {
                                         ^~~~~~~~~~~~~
>> drivers/gpio/gpio-mcp23s08.c:188:21: error: variable 'mcp_pinctrl_ops' has initializer but incomplete type
    static const struct pinctrl_ops mcp_pinctrl_ops = {
                        ^~~~~~~~~~~
>> drivers/gpio/gpio-mcp23s08.c:189:2: error: unknown field 'get_groups_count' specified in initializer
     .get_groups_count = mcp_pinctrl_get_groups_count,
     ^
>> drivers/gpio/gpio-mcp23s08.c:189:22: warning: excess elements in struct initializer
     .get_groups_count = mcp_pinctrl_get_groups_count,
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:189:22: note: (near initialization for 'mcp_pinctrl_ops')
>> drivers/gpio/gpio-mcp23s08.c:190:2: error: unknown field 'get_group_name' specified in initializer
     .get_group_name = mcp_pinctrl_get_group_name,
     ^
   drivers/gpio/gpio-mcp23s08.c:190:20: warning: excess elements in struct initializer
     .get_group_name = mcp_pinctrl_get_group_name,
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:190:20: note: (near initialization for 'mcp_pinctrl_ops')
>> drivers/gpio/gpio-mcp23s08.c:191:2: error: unknown field 'get_group_pins' specified in initializer
     .get_group_pins = mcp_pinctrl_get_group_pins,
     ^
   drivers/gpio/gpio-mcp23s08.c:191:20: warning: excess elements in struct initializer
     .get_group_pins = mcp_pinctrl_get_group_pins,
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:191:20: note: (near initialization for 'mcp_pinctrl_ops')
>> drivers/gpio/gpio-mcp23s08.c:193:2: error: unknown field 'dt_node_to_map' specified in initializer
     .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
     ^
>> drivers/gpio/gpio-mcp23s08.c:193:20: error: 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
     .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:193:20: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:193:20: note: (near initialization for 'mcp_pinctrl_ops')
>> drivers/gpio/gpio-mcp23s08.c:194:2: error: unknown field 'dt_free_map' specified in initializer
     .dt_free_map = pinconf_generic_dt_free_map,
     ^
>> drivers/gpio/gpio-mcp23s08.c:194:17: error: 'pinconf_generic_dt_free_map' undeclared here (not in a function)
     .dt_free_map = pinconf_generic_dt_free_map,
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:194:17: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:194:17: note: (near initialization for 'mcp_pinctrl_ops')
   drivers/gpio/gpio-mcp23s08.c: In function 'mcp_pinconf_get':
>> drivers/gpio/gpio-mcp23s08.c:201:25: error: implicit declaration of function 'pinctrl_dev_get_drvdata' [-Werror=implicit-function-declaration]
     struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
                            ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpio/gpio-mcp23s08.c:201:25: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
   drivers/gpio/gpio-mcp23s08.c: In function 'mcp_pinconf_set':
   drivers/gpio/gpio-mcp23s08.c:226:25: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
     struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
                            ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c: At top level:
>> drivers/gpio/gpio-mcp23s08.c:252:21: error: variable 'mcp_pinconf_ops' has initializer but incomplete type
    static const struct pinconf_ops mcp_pinconf_ops = {
                        ^~~~~~~~~~~
>> drivers/gpio/gpio-mcp23s08.c:253:2: error: unknown field 'pin_config_get' specified in initializer
     .pin_config_get = mcp_pinconf_get,
     ^
   drivers/gpio/gpio-mcp23s08.c:253:20: warning: excess elements in struct initializer
     .pin_config_get = mcp_pinconf_get,
                       ^~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:253:20: note: (near initialization for 'mcp_pinconf_ops')
>> drivers/gpio/gpio-mcp23s08.c:254:2: error: unknown field 'pin_config_set' specified in initializer
     .pin_config_set = mcp_pinconf_set,
     ^
   drivers/gpio/gpio-mcp23s08.c:254:20: warning: excess elements in struct initializer
     .pin_config_set = mcp_pinconf_set,
                       ^~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c:254:20: note: (near initialization for 'mcp_pinconf_ops')
>> drivers/gpio/gpio-mcp23s08.c:255:2: error: unknown field 'is_generic' specified in initializer
     .is_generic = true,
     ^
   drivers/gpio/gpio-mcp23s08.c:255:16: warning: excess elements in struct initializer
     .is_generic = true,
                   ^~~~
   drivers/gpio/gpio-mcp23s08.c:255:16: note: (near initialization for 'mcp_pinconf_ops')
   drivers/gpio/gpio-mcp23s08.c: In function 'mcp23s08_probe_one':
>> drivers/gpio/gpio-mcp23s08.c:776:17: error: implicit declaration of function 'pinctrl_register' [-Werror=implicit-function-declaration]
     mcp->pctldev = pinctrl_register(&mcp->pinctrl_desc, dev, mcp);
                    ^~~~~~~~~~~~~~~~
   drivers/gpio/gpio-mcp23s08.c: At top level:
>> drivers/gpio/gpio-mcp23s08.c:188:33: error: storage size of 'mcp_pinctrl_ops' isn't known
    static const struct pinctrl_ops mcp_pinctrl_ops = {
                                    ^~~~~~~~~~~~~~~

vim +/pinctrl_desc +86 drivers/gpio/gpio-mcp23s08.c

    80		struct gpio_chip	chip;
    81	
    82		struct regmap		*regmap;
    83		struct device		*dev;
    84	
    85		struct pinctrl_dev	*pctldev;
  > 86		struct pinctrl_desc	pinctrl_desc;
    87	};
    88	
    89	static const struct regmap_config mcp23x08_regmap = {
    90		.reg_bits = 8,
    91		.val_bits = 8,
    92	
    93		.reg_stride = 1,
    94		.max_register = MCP_OLAT,
    95	};
    96	
    97	static const struct regmap_config mcp23x17_regmap = {
    98		.reg_bits = 8,
    99		.val_bits = 16,
   100	
   101		.reg_stride = 2,
   102		.max_register = MCP_OLAT << 1,
   103		.val_format_endian = REGMAP_ENDIAN_LITTLE,
   104	};
   105	
   106	static int mcp_read(struct mcp23s08 *mcp, unsigned int reg, unsigned int *val)
   107	{
   108		return regmap_read(mcp->regmap, reg << mcp->reg_shift, val);
   109	}
   110	
   111	static int mcp_write(struct mcp23s08 *mcp, unsigned int reg, unsigned int val)
   112	{
   113		return regmap_write(mcp->regmap, reg << mcp->reg_shift, val);
   114	}
   115	
   116	static int mcp_set_bit(struct mcp23s08 *mcp, unsigned int reg,
   117			       unsigned int pin, bool enabled)
   118	{
   119		u16 val  = enabled ? 0xffff : 0x0000;
   120		u16 mask = BIT(pin);
   121		return regmap_update_bits(mcp->regmap, reg << mcp->reg_shift,
   122					  mask, val);
   123	}
   124	
   125	static int mcp_update_cache(struct mcp23s08 *mcp)
   126	{
   127		int ret, reg, i;
   128	
   129		for (i = 0; i < ARRAY_SIZE(mcp->cache); i++) {
   130			ret = mcp_read(mcp, i, &reg);
   131			if (ret < 0)
   132				return ret;
   133			mcp->cache[i] = reg;
   134		}
   135	
   136		return 0;
   137	}
   138	
 > 139	static const struct pinctrl_pin_desc mcp23x08_pins[] = {
 > 140		PINCTRL_PIN(0, "gpio0"),
   141		PINCTRL_PIN(1, "gpio1"),
   142		PINCTRL_PIN(2, "gpio2"),
   143		PINCTRL_PIN(3, "gpio3"),
   144		PINCTRL_PIN(4, "gpio4"),
   145		PINCTRL_PIN(5, "gpio5"),
   146		PINCTRL_PIN(6, "gpio6"),
   147		PINCTRL_PIN(7, "gpio7"),
   148	};
   149	
 > 150	static const struct pinctrl_pin_desc mcp23x17_pins[] = {
   151		PINCTRL_PIN(0, "gpio0"),
   152		PINCTRL_PIN(1, "gpio1"),
   153		PINCTRL_PIN(2, "gpio2"),
   154		PINCTRL_PIN(3, "gpio3"),
   155		PINCTRL_PIN(4, "gpio4"),
   156		PINCTRL_PIN(5, "gpio5"),
   157		PINCTRL_PIN(6, "gpio6"),
   158		PINCTRL_PIN(7, "gpio7"),
   159		PINCTRL_PIN(8, "gpio8"),
   160		PINCTRL_PIN(9, "gpio9"),
   161		PINCTRL_PIN(10, "gpio10"),
   162		PINCTRL_PIN(11, "gpio11"),
   163		PINCTRL_PIN(12, "gpio12"),
   164		PINCTRL_PIN(13, "gpio13"),
   165		PINCTRL_PIN(14, "gpio14"),
   166		PINCTRL_PIN(15, "gpio15"),
   167	};
   168	
   169	static int mcp_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
   170	{
   171		return 0;
   172	}
   173	
   174	static const char *mcp_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
   175							unsigned int group)
   176	{
   177		return NULL;
   178	}
   179	
   180	static int mcp_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
   181						unsigned int group,
   182						const unsigned int **pins,
   183						unsigned int *num_pins)
   184	{
   185		return -ENOTSUPP;
   186	}
   187	
 > 188	static const struct pinctrl_ops mcp_pinctrl_ops = {
 > 189		.get_groups_count = mcp_pinctrl_get_groups_count,
 > 190		.get_group_name = mcp_pinctrl_get_group_name,
 > 191		.get_group_pins = mcp_pinctrl_get_group_pins,
   192	#ifdef CONFIG_OF
 > 193		.dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
 > 194		.dt_free_map = pinconf_generic_dt_free_map,
   195	#endif
   196	};
   197	
   198	static int mcp_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
   199				      unsigned long *config)
   200	{
 > 201		struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
   202		enum pin_config_param param = pinconf_to_config_param(*config);
   203		unsigned int data, status;
   204		int ret;
   205	
   206		switch (param) {
   207		case PIN_CONFIG_BIAS_PULL_UP:
   208			ret = mcp_read(mcp, MCP_GPPU, &data);
   209			if (ret < 0)
   210				return ret;
   211			status = (data & BIT(pin)) ? 1 : 0;
   212			break;
   213		default:
   214			dev_err(mcp->dev, "Invalid config param %04x\n", param);
   215			return -ENOTSUPP;
   216		}
   217	
   218		*config = 0;
   219	
   220		return status ? 0 : -EINVAL;
   221	}
   222	
   223	static int mcp_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
   224				      unsigned long *configs, unsigned int num_configs)
   225	{
   226		struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
   227		enum pin_config_param param;
   228		u32 arg, mask;
   229		u16 val;
   230		int ret = 0;
   231		int i;
   232	
   233		for (i = 0; i < num_configs; i++) {
   234			param = pinconf_to_config_param(configs[i]);
   235			arg = pinconf_to_config_argument(configs[i]);
   236	
   237			switch (param) {
   238			case PIN_CONFIG_BIAS_PULL_UP:
   239				val = arg ? 0xFFFF : 0x0000;
   240				mask = BIT(pin);
   241				ret = mcp_set_bit(mcp, MCP_GPPU, pin, arg);
   242				break;
   243			default:
   244				dev_err(mcp->dev, "Invalid config param %04x\n", param);
   245				return -ENOTSUPP;
   246			}
   247		}
   248	
   249		return ret;
   250	}
   251	
 > 252	static const struct pinconf_ops mcp_pinconf_ops = {
 > 253		.pin_config_get = mcp_pinconf_get,
 > 254		.pin_config_set = mcp_pinconf_set,
 > 255		.is_generic = true,
   256	};
   257	
   258	/*----------------------------------------------------------------------*/

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24197 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] gpio: mcp23s08: add pinconf support
  2017-01-27 14:47 ` [PATCH 2/2] gpio: mcp23s08: add pinconf support Sebastian Reichel
@ 2017-01-27 19:00     ` kbuild test robot
  2017-01-27 19:00     ` kbuild test robot
  2017-01-30 15:08   ` Linus Walleij
  2 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2017-01-27 19:00 UTC (permalink / raw)
  Cc: kbuild-all, Sebastian Reichel, Linus Walleij, Alexandre Courbot,
	linux-gpio, linux-kernel

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

Hi Sebastian,

[auto build test ERROR on gpio/for-next]
[also build test ERROR on v4.10-rc5 next-20170125]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sebastian-Reichel/mcp23s08-pinconf-support/20170127-230243
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: i386-randconfig-c0-01280104 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpio/gpio-mcp23s08.c:86:22: error: field 'pinctrl_desc' has incomplete type
     struct pinctrl_desc pinctrl_desc;
                         ^
>> drivers/gpio/gpio-mcp23s08.c:139:38: error: array type has incomplete element type
    static const struct pinctrl_pin_desc mcp23x08_pins[] = {
                                         ^
   drivers/gpio/gpio-mcp23s08.c:140:2: error: implicit declaration of function 'PINCTRL_PIN' [-Werror=implicit-function-declaration]
     PINCTRL_PIN(0, "gpio0"),
     ^
   drivers/gpio/gpio-mcp23s08.c:150:38: error: array type has incomplete element type
    static const struct pinctrl_pin_desc mcp23x17_pins[] = {
                                         ^
   drivers/gpio/gpio-mcp23s08.c:188:21: error: variable 'mcp_pinctrl_ops' has initializer but incomplete type
    static const struct pinctrl_ops mcp_pinctrl_ops = {
                        ^
   drivers/gpio/gpio-mcp23s08.c:189:2: error: unknown field 'get_groups_count' specified in initializer
     .get_groups_count = mcp_pinctrl_get_groups_count,
     ^
   drivers/gpio/gpio-mcp23s08.c:189:2: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:189:2: warning: (near initialization for 'mcp_pinctrl_ops')
   drivers/gpio/gpio-mcp23s08.c:190:2: error: unknown field 'get_group_name' specified in initializer
     .get_group_name = mcp_pinctrl_get_group_name,
     ^
   drivers/gpio/gpio-mcp23s08.c:190:2: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:190:2: warning: (near initialization for 'mcp_pinctrl_ops')
   drivers/gpio/gpio-mcp23s08.c:191:2: error: unknown field 'get_group_pins' specified in initializer
     .get_group_pins = mcp_pinctrl_get_group_pins,
     ^
   drivers/gpio/gpio-mcp23s08.c:191:2: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:191:2: warning: (near initialization for 'mcp_pinctrl_ops')
   drivers/gpio/gpio-mcp23s08.c:193:2: error: unknown field 'dt_node_to_map' specified in initializer
     .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
     ^
   drivers/gpio/gpio-mcp23s08.c:193:20: error: 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
     .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
                       ^
   drivers/gpio/gpio-mcp23s08.c:193:2: warning: excess elements in struct initializer
     .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
     ^
   drivers/gpio/gpio-mcp23s08.c:193:2: warning: (near initialization for 'mcp_pinctrl_ops')
   drivers/gpio/gpio-mcp23s08.c:194:2: error: unknown field 'dt_free_map' specified in initializer
     .dt_free_map = pinconf_generic_dt_free_map,
     ^
   drivers/gpio/gpio-mcp23s08.c:194:17: error: 'pinconf_generic_dt_free_map' undeclared here (not in a function)
     .dt_free_map = pinconf_generic_dt_free_map,
                    ^
   drivers/gpio/gpio-mcp23s08.c:194:2: warning: excess elements in struct initializer
     .dt_free_map = pinconf_generic_dt_free_map,
     ^
   drivers/gpio/gpio-mcp23s08.c:194:2: warning: (near initialization for 'mcp_pinctrl_ops')
   drivers/gpio/gpio-mcp23s08.c: In function 'mcp_pinconf_get':
   drivers/gpio/gpio-mcp23s08.c:201:9: error: implicit declaration of function 'pinctrl_dev_get_drvdata' [-Werror=implicit-function-declaration]
     struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
            ^
   drivers/gpio/gpio-mcp23s08.c:201:25: warning: initialization makes pointer from integer without a cast
     struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
                            ^
   drivers/gpio/gpio-mcp23s08.c: In function 'mcp_pinconf_set':
   drivers/gpio/gpio-mcp23s08.c:226:25: warning: initialization makes pointer from integer without a cast
     struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
                            ^
   drivers/gpio/gpio-mcp23s08.c: At top level:
   drivers/gpio/gpio-mcp23s08.c:252:21: error: variable 'mcp_pinconf_ops' has initializer but incomplete type
    static const struct pinconf_ops mcp_pinconf_ops = {
                        ^
   drivers/gpio/gpio-mcp23s08.c:253:2: error: unknown field 'pin_config_get' specified in initializer
     .pin_config_get = mcp_pinconf_get,
     ^
   drivers/gpio/gpio-mcp23s08.c:253:2: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:253:2: warning: (near initialization for 'mcp_pinconf_ops')
   drivers/gpio/gpio-mcp23s08.c:254:2: error: unknown field 'pin_config_set' specified in initializer
     .pin_config_set = mcp_pinconf_set,
     ^
   drivers/gpio/gpio-mcp23s08.c:254:2: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:254:2: warning: (near initialization for 'mcp_pinconf_ops')
   drivers/gpio/gpio-mcp23s08.c:255:2: error: unknown field 'is_generic' specified in initializer
     .is_generic = true,
     ^
   drivers/gpio/gpio-mcp23s08.c:255:2: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:255:2: warning: (near initialization for 'mcp_pinconf_ops')
   drivers/gpio/gpio-mcp23s08.c: In function 'mcp23s08_probe_one':
   drivers/gpio/gpio-mcp23s08.c:776:2: error: implicit declaration of function 'pinctrl_register' [-Werror=implicit-function-declaration]
     mcp->pctldev = pinctrl_register(&mcp->pinctrl_desc, dev, mcp);
     ^
   drivers/gpio/gpio-mcp23s08.c: At top level:
   drivers/gpio/gpio-mcp23s08.c:139:38: warning: 'mcp23x08_pins' defined but not used [-Wunused-variable]
    static const struct pinctrl_pin_desc mcp23x08_pins[] = {
                                         ^
   drivers/gpio/gpio-mcp23s08.c:150:38: warning: 'mcp23x17_pins' defined but not used [-Wunused-variable]
    static const struct pinctrl_pin_desc mcp23x17_pins[] = {
                                         ^
   cc1: some warnings being treated as errors

vim +139 drivers/gpio/gpio-mcp23s08.c

    80		struct gpio_chip	chip;
    81	
    82		struct regmap		*regmap;
    83		struct device		*dev;
    84	
    85		struct pinctrl_dev	*pctldev;
  > 86		struct pinctrl_desc	pinctrl_desc;
    87	};
    88	
    89	static const struct regmap_config mcp23x08_regmap = {
    90		.reg_bits = 8,
    91		.val_bits = 8,
    92	
    93		.reg_stride = 1,
    94		.max_register = MCP_OLAT,
    95	};
    96	
    97	static const struct regmap_config mcp23x17_regmap = {
    98		.reg_bits = 8,
    99		.val_bits = 16,
   100	
   101		.reg_stride = 2,
   102		.max_register = MCP_OLAT << 1,
   103		.val_format_endian = REGMAP_ENDIAN_LITTLE,
   104	};
   105	
   106	static int mcp_read(struct mcp23s08 *mcp, unsigned int reg, unsigned int *val)
   107	{
   108		return regmap_read(mcp->regmap, reg << mcp->reg_shift, val);
   109	}
   110	
   111	static int mcp_write(struct mcp23s08 *mcp, unsigned int reg, unsigned int val)
   112	{
   113		return regmap_write(mcp->regmap, reg << mcp->reg_shift, val);
   114	}
   115	
   116	static int mcp_set_bit(struct mcp23s08 *mcp, unsigned int reg,
   117			       unsigned int pin, bool enabled)
   118	{
   119		u16 val  = enabled ? 0xffff : 0x0000;
   120		u16 mask = BIT(pin);
   121		return regmap_update_bits(mcp->regmap, reg << mcp->reg_shift,
   122					  mask, val);
   123	}
   124	
   125	static int mcp_update_cache(struct mcp23s08 *mcp)
   126	{
   127		int ret, reg, i;
   128	
   129		for (i = 0; i < ARRAY_SIZE(mcp->cache); i++) {
   130			ret = mcp_read(mcp, i, &reg);
   131			if (ret < 0)
   132				return ret;
   133			mcp->cache[i] = reg;
   134		}
   135	
   136		return 0;
   137	}
   138	
 > 139	static const struct pinctrl_pin_desc mcp23x08_pins[] = {
   140		PINCTRL_PIN(0, "gpio0"),
   141		PINCTRL_PIN(1, "gpio1"),
   142		PINCTRL_PIN(2, "gpio2"),

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25466 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] gpio: mcp23s08: add pinconf support
@ 2017-01-27 19:00     ` kbuild test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2017-01-27 19:00 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: kbuild-all, Sebastian Reichel, Linus Walleij, Alexandre Courbot,
	linux-gpio, linux-kernel

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

Hi Sebastian,

[auto build test ERROR on gpio/for-next]
[also build test ERROR on v4.10-rc5 next-20170125]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sebastian-Reichel/mcp23s08-pinconf-support/20170127-230243
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: i386-randconfig-c0-01280104 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpio/gpio-mcp23s08.c:86:22: error: field 'pinctrl_desc' has incomplete type
     struct pinctrl_desc pinctrl_desc;
                         ^
>> drivers/gpio/gpio-mcp23s08.c:139:38: error: array type has incomplete element type
    static const struct pinctrl_pin_desc mcp23x08_pins[] = {
                                         ^
   drivers/gpio/gpio-mcp23s08.c:140:2: error: implicit declaration of function 'PINCTRL_PIN' [-Werror=implicit-function-declaration]
     PINCTRL_PIN(0, "gpio0"),
     ^
   drivers/gpio/gpio-mcp23s08.c:150:38: error: array type has incomplete element type
    static const struct pinctrl_pin_desc mcp23x17_pins[] = {
                                         ^
   drivers/gpio/gpio-mcp23s08.c:188:21: error: variable 'mcp_pinctrl_ops' has initializer but incomplete type
    static const struct pinctrl_ops mcp_pinctrl_ops = {
                        ^
   drivers/gpio/gpio-mcp23s08.c:189:2: error: unknown field 'get_groups_count' specified in initializer
     .get_groups_count = mcp_pinctrl_get_groups_count,
     ^
   drivers/gpio/gpio-mcp23s08.c:189:2: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:189:2: warning: (near initialization for 'mcp_pinctrl_ops')
   drivers/gpio/gpio-mcp23s08.c:190:2: error: unknown field 'get_group_name' specified in initializer
     .get_group_name = mcp_pinctrl_get_group_name,
     ^
   drivers/gpio/gpio-mcp23s08.c:190:2: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:190:2: warning: (near initialization for 'mcp_pinctrl_ops')
   drivers/gpio/gpio-mcp23s08.c:191:2: error: unknown field 'get_group_pins' specified in initializer
     .get_group_pins = mcp_pinctrl_get_group_pins,
     ^
   drivers/gpio/gpio-mcp23s08.c:191:2: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:191:2: warning: (near initialization for 'mcp_pinctrl_ops')
   drivers/gpio/gpio-mcp23s08.c:193:2: error: unknown field 'dt_node_to_map' specified in initializer
     .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
     ^
   drivers/gpio/gpio-mcp23s08.c:193:20: error: 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
     .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
                       ^
   drivers/gpio/gpio-mcp23s08.c:193:2: warning: excess elements in struct initializer
     .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
     ^
   drivers/gpio/gpio-mcp23s08.c:193:2: warning: (near initialization for 'mcp_pinctrl_ops')
   drivers/gpio/gpio-mcp23s08.c:194:2: error: unknown field 'dt_free_map' specified in initializer
     .dt_free_map = pinconf_generic_dt_free_map,
     ^
   drivers/gpio/gpio-mcp23s08.c:194:17: error: 'pinconf_generic_dt_free_map' undeclared here (not in a function)
     .dt_free_map = pinconf_generic_dt_free_map,
                    ^
   drivers/gpio/gpio-mcp23s08.c:194:2: warning: excess elements in struct initializer
     .dt_free_map = pinconf_generic_dt_free_map,
     ^
   drivers/gpio/gpio-mcp23s08.c:194:2: warning: (near initialization for 'mcp_pinctrl_ops')
   drivers/gpio/gpio-mcp23s08.c: In function 'mcp_pinconf_get':
   drivers/gpio/gpio-mcp23s08.c:201:9: error: implicit declaration of function 'pinctrl_dev_get_drvdata' [-Werror=implicit-function-declaration]
     struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
            ^
   drivers/gpio/gpio-mcp23s08.c:201:25: warning: initialization makes pointer from integer without a cast
     struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
                            ^
   drivers/gpio/gpio-mcp23s08.c: In function 'mcp_pinconf_set':
   drivers/gpio/gpio-mcp23s08.c:226:25: warning: initialization makes pointer from integer without a cast
     struct mcp23s08 *mcp = pinctrl_dev_get_drvdata(pctldev);
                            ^
   drivers/gpio/gpio-mcp23s08.c: At top level:
   drivers/gpio/gpio-mcp23s08.c:252:21: error: variable 'mcp_pinconf_ops' has initializer but incomplete type
    static const struct pinconf_ops mcp_pinconf_ops = {
                        ^
   drivers/gpio/gpio-mcp23s08.c:253:2: error: unknown field 'pin_config_get' specified in initializer
     .pin_config_get = mcp_pinconf_get,
     ^
   drivers/gpio/gpio-mcp23s08.c:253:2: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:253:2: warning: (near initialization for 'mcp_pinconf_ops')
   drivers/gpio/gpio-mcp23s08.c:254:2: error: unknown field 'pin_config_set' specified in initializer
     .pin_config_set = mcp_pinconf_set,
     ^
   drivers/gpio/gpio-mcp23s08.c:254:2: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:254:2: warning: (near initialization for 'mcp_pinconf_ops')
   drivers/gpio/gpio-mcp23s08.c:255:2: error: unknown field 'is_generic' specified in initializer
     .is_generic = true,
     ^
   drivers/gpio/gpio-mcp23s08.c:255:2: warning: excess elements in struct initializer
   drivers/gpio/gpio-mcp23s08.c:255:2: warning: (near initialization for 'mcp_pinconf_ops')
   drivers/gpio/gpio-mcp23s08.c: In function 'mcp23s08_probe_one':
   drivers/gpio/gpio-mcp23s08.c:776:2: error: implicit declaration of function 'pinctrl_register' [-Werror=implicit-function-declaration]
     mcp->pctldev = pinctrl_register(&mcp->pinctrl_desc, dev, mcp);
     ^
   drivers/gpio/gpio-mcp23s08.c: At top level:
   drivers/gpio/gpio-mcp23s08.c:139:38: warning: 'mcp23x08_pins' defined but not used [-Wunused-variable]
    static const struct pinctrl_pin_desc mcp23x08_pins[] = {
                                         ^
   drivers/gpio/gpio-mcp23s08.c:150:38: warning: 'mcp23x17_pins' defined but not used [-Wunused-variable]
    static const struct pinctrl_pin_desc mcp23x17_pins[] = {
                                         ^
   cc1: some warnings being treated as errors

vim +139 drivers/gpio/gpio-mcp23s08.c

    80		struct gpio_chip	chip;
    81	
    82		struct regmap		*regmap;
    83		struct device		*dev;
    84	
    85		struct pinctrl_dev	*pctldev;
  > 86		struct pinctrl_desc	pinctrl_desc;
    87	};
    88	
    89	static const struct regmap_config mcp23x08_regmap = {
    90		.reg_bits = 8,
    91		.val_bits = 8,
    92	
    93		.reg_stride = 1,
    94		.max_register = MCP_OLAT,
    95	};
    96	
    97	static const struct regmap_config mcp23x17_regmap = {
    98		.reg_bits = 8,
    99		.val_bits = 16,
   100	
   101		.reg_stride = 2,
   102		.max_register = MCP_OLAT << 1,
   103		.val_format_endian = REGMAP_ENDIAN_LITTLE,
   104	};
   105	
   106	static int mcp_read(struct mcp23s08 *mcp, unsigned int reg, unsigned int *val)
   107	{
   108		return regmap_read(mcp->regmap, reg << mcp->reg_shift, val);
   109	}
   110	
   111	static int mcp_write(struct mcp23s08 *mcp, unsigned int reg, unsigned int val)
   112	{
   113		return regmap_write(mcp->regmap, reg << mcp->reg_shift, val);
   114	}
   115	
   116	static int mcp_set_bit(struct mcp23s08 *mcp, unsigned int reg,
   117			       unsigned int pin, bool enabled)
   118	{
   119		u16 val  = enabled ? 0xffff : 0x0000;
   120		u16 mask = BIT(pin);
   121		return regmap_update_bits(mcp->regmap, reg << mcp->reg_shift,
   122					  mask, val);
   123	}
   124	
   125	static int mcp_update_cache(struct mcp23s08 *mcp)
   126	{
   127		int ret, reg, i;
   128	
   129		for (i = 0; i < ARRAY_SIZE(mcp->cache); i++) {
   130			ret = mcp_read(mcp, i, &reg);
   131			if (ret < 0)
   132				return ret;
   133			mcp->cache[i] = reg;
   134		}
   135	
   136		return 0;
   137	}
   138	
 > 139	static const struct pinctrl_pin_desc mcp23x08_pins[] = {
   140		PINCTRL_PIN(0, "gpio0"),
   141		PINCTRL_PIN(1, "gpio1"),
   142		PINCTRL_PIN(2, "gpio2"),

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25466 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] gpio: mcp23s08: use regmap
  2017-01-27 14:47 ` [PATCH 1/2] gpio: mcp23s08: use regmap Sebastian Reichel
@ 2017-01-30 14:58   ` Linus Walleij
  0 siblings, 0 replies; 11+ messages in thread
From: Linus Walleij @ 2017-01-30 14:58 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: Alexandre Courbot, linux-gpio, linux-kernel

On Fri, Jan 27, 2017 at 3:47 PM, Sebastian Reichel <sre@kernel.org> wrote:

> Use regmap API to save some lines of codes and have
> debugfs support for all of the MCP's registers.
>
> Signed-off-by: Sebastian Reichel <sre@kernel.org>

Patch applied. Irresistible cleanup!

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] gpio: mcp23s08: add pinconf support
  2017-01-27 14:47 ` [PATCH 2/2] gpio: mcp23s08: add pinconf support Sebastian Reichel
  2017-01-27 17:16     ` kbuild test robot
  2017-01-27 19:00     ` kbuild test robot
@ 2017-01-30 15:08   ` Linus Walleij
  2017-01-30 16:40     ` Sebastian Reichel
  2 siblings, 1 reply; 11+ messages in thread
From: Linus Walleij @ 2017-01-30 15:08 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: Alexandre Courbot, linux-gpio, linux-kernel

On Fri, Jan 27, 2017 at 3:47 PM, Sebastian Reichel <sre@kernel.org> wrote:

> mcp23xxx device have configurable 100k pullup resistors. This adds
> support for enabling them using pinctrl's pinconf interface.
>
> Signed-off-by: Sebastian Reichel <sre@kernel.org>

That's the right approach!

> diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/gpio/gpio-mcp23s08.c

Can we move the patch to drivers/pinctrl/* like all other mixed drivers
doing combined pinctrl and GPIO?

Also: no Kconfig changes? Surely you must select the right pinctrl
things, I guess you're just lucky that some other pin controller on your
system selects your infrastructure. I think that is why the build robot
is complaining.

> +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;
> +}

Why do you need a cache of register values when regmap already
supports this?

Instead I suggest: exploit the .volatile_reg() callback from
struct regmap_config and use regmap to maintain the cache.

Apart from this is is nice!

With the recent changes from Mika Westerberg scheduled for v4.11
the road is open to expose pullups all the way to userspace from
GPIOs chardev if we want to (some patches would be needed).

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] gpio: mcp23s08: add pinconf support
  2017-01-30 15:08   ` Linus Walleij
@ 2017-01-30 16:40     ` Sebastian Reichel
  2017-02-01 15:12       ` Linus Walleij
  0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Reichel @ 2017-01-30 16:40 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Alexandre Courbot, linux-gpio, linux-kernel

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

Hi Linus,

On Mon, Jan 30, 2017 at 04:08:01PM +0100, Linus Walleij wrote:
> On Fri, Jan 27, 2017 at 3:47 PM, Sebastian Reichel <sre@kernel.org> wrote:
> 
> > mcp23xxx device have configurable 100k pullup resistors. This adds
> > support for enabling them using pinctrl's pinconf interface.
> >
> > Signed-off-by: Sebastian Reichel <sre@kernel.org>
> 
> That's the right approach!
>
> > diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/gpio/gpio-mcp23s08.c
> 
> Can we move the patch to drivers/pinctrl/* like all other mixed drivers
> doing combined pinctrl and GPIO?

Sure. Two questions:

 * Should the config name change to PINCTRL_MCP23s08 (or PINCTRL_MCP23XXX)?
   This will mean people will have to fix their .config
 * Should there be a "SPI or I2C GPIO expanders" sub-menu for it as in the
   gpio Kconfig?

> Also: no Kconfig changes? Surely you must select the right pinctrl
> things, I guess you're just lucky that some other pin controller on your
> system selects your infrastructure. I think that is why the build robot
> is complaining.

Yes, it should select GENERIC_PINCONF as far as I can see.

> > +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;
> > +}
> 
> Why do you need a cache of register values when regmap already
> supports this?
>
> Instead I suggest: exploit the .volatile_reg() callback from
> struct regmap_config and use regmap to maintain the cache.

The mcp_update_cache method is just moved in this patch. I do not
need it, but the existing code uses it. Actually I only moved it, so
that it sticks together with the mcp_read and mcp_write functions,
which must be placed a bit earlier to be usable by the pinctrl stuff.

I did not convert the custom caching in the regmap patch, since it
should be done in its own cleanup patch. Introducing basic regmap
was much more straight forward than removing the open-coded caching.

> Apart from this is is nice!

I guess the custom platform data based pullup config could also
be removed. I just checked and there are not many users of the
platform data:

linux/arch $ git grep -l mcp23 | grep -v "/dts/"         
blackfin/mach-bf527/boards/tll6527m.c
blackfin/mach-bf609/boards/ezkit.c

None of them seems to use the pullups variable.

> With the recent changes from Mika Westerberg scheduled for v4.11
> the road is open to expose pullups all the way to userspace from
> GPIOs chardev if we want to (some patches would be needed).

I just added the pinctrl config to DT, but that sounds useful.

-- Sebastian

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] gpio: mcp23s08: add pinconf support
  2017-01-30 16:40     ` Sebastian Reichel
@ 2017-02-01 15:12       ` Linus Walleij
  0 siblings, 0 replies; 11+ messages in thread
From: Linus Walleij @ 2017-02-01 15:12 UTC (permalink / raw)
  To: Sebastian Reichel; +Cc: Alexandre Courbot, linux-gpio, linux-kernel

On Mon, Jan 30, 2017 at 5:40 PM, Sebastian Reichel <sre@kernel.org> wrote:

>> Can we move the patch to drivers/pinctrl/* like all other mixed drivers
>> doing combined pinctrl and GPIO?
>
> Sure. Two questions:
>
>  * Should the config name change to PINCTRL_MCP23s08 (or PINCTRL_MCP23XXX)?
>    This will mean people will have to fix their .config

I never understood what the right way is to deal with this.

What happens actually when you run "make oldconfig"?

Is there a way to supply a replacement Kconfig symbol in a good
way?

Else I guess we can keep a dangling GPIO_MCP23S08 bool
that just selects the new PINCTRL_MCP23XXX for now?
With a comment that it got moved?

(Please verify that it works though...)

>  * Should there be a "SPI or I2C GPIO expanders" sub-menu for it as in the
>    gpio Kconfig?

Maybe, that would be a separate refactoring thing for later.
I think it is this one and SX150X that use I2C right now.

> I did not convert the custom caching in the regmap patch, since it
> should be done in its own cleanup patch. Introducing basic regmap
> was much more straight forward than removing the open-coded caching.

OK I take it that it will be fixed later then, that's fine.

> I guess the custom platform data based pullup config could also
> be removed. I just checked and there are not many users of the
> platform data:
>
> linux/arch $ git grep -l mcp23 | grep -v "/dts/"
> blackfin/mach-bf527/boards/tll6527m.c
> blackfin/mach-bf609/boards/ezkit.c
>
> None of them seems to use the pullups variable.

Just kill it off, good riddance!

As separate patch before/after moving it I guess.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-02-01 15:12 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-27 14:47 [PATCH 0/2] mcp23s08 pinconf support Sebastian Reichel
2017-01-27 14:47 ` [PATCH 1/2] gpio: mcp23s08: use regmap Sebastian Reichel
2017-01-30 14:58   ` Linus Walleij
2017-01-27 14:47 ` [PATCH 2/2] gpio: mcp23s08: add pinconf support Sebastian Reichel
2017-01-27 17:16   ` kbuild test robot
2017-01-27 17:16     ` kbuild test robot
2017-01-27 19:00   ` kbuild test robot
2017-01-27 19:00     ` kbuild test robot
2017-01-30 15:08   ` Linus Walleij
2017-01-30 16:40     ` Sebastian Reichel
2017-02-01 15:12       ` Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.