All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data
@ 2020-04-07 17:38 Andy Shevchenko
  2020-04-07 17:38 ` [PATCH v1 2/9] pinctrl: mcp23s08: Deduplicate IRQ chip filling Andy Shevchenko
                   ` (9 more replies)
  0 siblings, 10 replies; 23+ messages in thread
From: Andy Shevchenko @ 2020-04-07 17:38 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio; +Cc: Andy Shevchenko

Platform data is a legacy interface to supply device properties
to the driver. In this case we even don't have in-kernel users
for it. Just remove it for good.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 42 +++++++++---------------------
 include/linux/spi/mcp23s08.h       | 18 -------------
 2 files changed, 12 insertions(+), 48 deletions(-)
 delete mode 100644 include/linux/spi/mcp23s08.h

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 3a235487e38d..2c8b8c45b70e 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -8,7 +8,6 @@
 #include <linux/gpio/driver.h>
 #include <linux/i2c.h>
 #include <linux/spi/spi.h>
-#include <linux/spi/mcp23s08.h>
 #include <linux/slab.h>
 #include <asm/byteorder.h>
 #include <linux/interrupt.h>
@@ -914,16 +913,9 @@ MODULE_DEVICE_TABLE(of, mcp23s08_i2c_of_match);
 static int mcp230xx_probe(struct i2c_client *client,
 				    const struct i2c_device_id *id)
 {
-	struct mcp23s08_platform_data *pdata, local_pdata;
 	struct mcp23s08 *mcp;
 	int status;
 
-	pdata = dev_get_platdata(&client->dev);
-	if (!pdata) {
-		pdata = &local_pdata;
-		pdata->base = -1;
-	}
-
 	mcp = devm_kzalloc(&client->dev, sizeof(*mcp), GFP_KERNEL);
 	if (!mcp)
 		return -ENOMEM;
@@ -937,7 +929,7 @@ static int mcp230xx_probe(struct i2c_client *client,
 	mcp->irq_chip.irq_bus_sync_unlock = mcp23s08_irq_bus_unlock;
 
 	status = mcp23s08_probe_one(mcp, &client->dev, client, client->addr,
-				    id->driver_data, pdata->base, 0);
+				    id->driver_data, -1, 0);
 	if (status)
 		return status;
 
@@ -986,13 +978,13 @@ static void mcp23s08_i2c_exit(void) { }
 
 static int mcp23s08_probe(struct spi_device *spi)
 {
-	struct mcp23s08_platform_data	*pdata, local_pdata;
 	unsigned			addr;
 	int				chips = 0;
 	struct mcp23s08_driver_data	*data;
 	int				status, type;
 	unsigned			ngpio = 0;
 	const struct			of_device_id *match;
+	u32				spi_present_mask;
 
 	match = of_match_device(of_match_ptr(mcp23s08_spi_of_match), &spi->dev);
 	if (match)
@@ -1000,32 +992,24 @@ static int mcp23s08_probe(struct spi_device *spi)
 	else
 		type = spi_get_device_id(spi)->driver_data;
 
-	pdata = dev_get_platdata(&spi->dev);
-	if (!pdata) {
-		pdata = &local_pdata;
-		pdata->base = -1;
-
+	status = device_property_read_u32(&spi->dev,
+			"microchip,spi-present-mask", &spi_present_mask);
+	if (status) {
 		status = device_property_read_u32(&spi->dev,
-			"microchip,spi-present-mask", &pdata->spi_present_mask);
+				"mcp,spi-present-mask", &spi_present_mask);
 		if (status) {
-			status = device_property_read_u32(&spi->dev,
-				"mcp,spi-present-mask",
-				&pdata->spi_present_mask);
-
-			if (status) {
-				dev_err(&spi->dev, "missing spi-present-mask");
-				return -ENODEV;
-			}
+			dev_err(&spi->dev, "missing spi-present-mask");
+			return -ENODEV;
 		}
 	}
 
-	if (!pdata->spi_present_mask || pdata->spi_present_mask > 0xff) {
+	if (!spi_present_mask || spi_present_mask > 0xff) {
 		dev_err(&spi->dev, "invalid spi-present-mask");
 		return -ENODEV;
 	}
 
 	for (addr = 0; addr < MCP_MAX_DEV_PER_CS; addr++) {
-		if (pdata->spi_present_mask & BIT(addr))
+		if (spi_present_mask & BIT(addr))
 			chips++;
 	}
 
@@ -1040,7 +1024,7 @@ static int mcp23s08_probe(struct spi_device *spi)
 	spi_set_drvdata(spi, data);
 
 	for (addr = 0; addr < MCP_MAX_DEV_PER_CS; addr++) {
-		if (!(pdata->spi_present_mask & BIT(addr)))
+		if (!(spi_present_mask & BIT(addr)))
 			continue;
 		chips--;
 		data->mcp[addr] = &data->chip[chips];
@@ -1054,12 +1038,10 @@ static int mcp23s08_probe(struct spi_device *spi)
 			mcp23s08_irq_bus_unlock;
 		status = mcp23s08_probe_one(data->mcp[addr], &spi->dev, spi,
 					    0x40 | (addr << 1), type,
-					    pdata->base, addr);
+					    -1, addr);
 		if (status < 0)
 			return status;
 
-		if (pdata->base != -1)
-			pdata->base += data->mcp[addr]->chip.ngpio;
 		ngpio += data->mcp[addr]->chip.ngpio;
 	}
 	data->ngpio = ngpio;
diff --git a/include/linux/spi/mcp23s08.h b/include/linux/spi/mcp23s08.h
deleted file mode 100644
index 738a45b435f2..000000000000
--- a/include/linux/spi/mcp23s08.h
+++ /dev/null
@@ -1,18 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-struct mcp23s08_platform_data {
-	/* For mcp23s08, up to 4 slaves (numbered 0..3) can share one SPI
-	 * chipselect, each providing 1 gpio_chip instance with 8 gpios.
-	 * For mpc23s17, up to 8 slaves (numbered 0..7) can share one SPI
-	 * chipselect, each providing 1 gpio_chip (port A + port B) with
-	 * 16 gpios.
-	 */
-	u32 spi_present_mask;
-
-	/* "base" is the number of the first GPIO or -1 for dynamic
-	 * assignment. If there are gaps in chip addressing the GPIO
-	 * numbers are sequential .. so for example if only slaves 0
-	 * and 3 are present, their GPIOs range from base to base+15
-	 * (or base+31 for s17 variant).
-	 */
-	unsigned	base;
-};
-- 
2.25.1


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

* [PATCH v1 2/9] pinctrl: mcp23s08: Deduplicate IRQ chip filling
  2020-04-07 17:38 [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data Andy Shevchenko
@ 2020-04-07 17:38 ` Andy Shevchenko
  2020-04-07 17:38 ` [PATCH v1 3/9] pinctrl: mcp23s08: Consolidate SPI and I²C code Andy Shevchenko
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2020-04-07 17:38 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio; +Cc: Andy Shevchenko

In both cases, SPI and I²C, IRQ chip is filled in the same way.
Deduplicate this by moving common part to mcp23s08_probe_one().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 2c8b8c45b70e..e05219d3331f 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -669,7 +669,14 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 
 	mcp->dev = dev;
 	mcp->addr = addr;
+
 	mcp->irq_active_high = false;
+	mcp->irq_chip.name = dev_name(dev);
+	mcp->irq_chip.irq_mask = mcp23s08_irq_mask;
+	mcp->irq_chip.irq_unmask = mcp23s08_irq_unmask;
+	mcp->irq_chip.irq_set_type = mcp23s08_irq_set_type;
+	mcp->irq_chip.irq_bus_lock = mcp23s08_irq_bus_lock;
+	mcp->irq_chip.irq_bus_sync_unlock = mcp23s08_irq_bus_unlock;
 
 	mcp->chip.direction_input = mcp23s08_direction_input;
 	mcp->chip.get = mcp23s08_get;
@@ -921,12 +928,6 @@ static int mcp230xx_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	mcp->irq = client->irq;
-	mcp->irq_chip.name = dev_name(&client->dev);
-	mcp->irq_chip.irq_mask = mcp23s08_irq_mask;
-	mcp->irq_chip.irq_unmask = mcp23s08_irq_unmask;
-	mcp->irq_chip.irq_set_type = mcp23s08_irq_set_type;
-	mcp->irq_chip.irq_bus_lock = mcp23s08_irq_bus_lock;
-	mcp->irq_chip.irq_bus_sync_unlock = mcp23s08_irq_bus_unlock;
 
 	status = mcp23s08_probe_one(mcp, &client->dev, client, client->addr,
 				    id->driver_data, -1, 0);
@@ -1029,13 +1030,6 @@ static int mcp23s08_probe(struct spi_device *spi)
 		chips--;
 		data->mcp[addr] = &data->chip[chips];
 		data->mcp[addr]->irq = spi->irq;
-		data->mcp[addr]->irq_chip.name = dev_name(&spi->dev);
-		data->mcp[addr]->irq_chip.irq_mask = mcp23s08_irq_mask;
-		data->mcp[addr]->irq_chip.irq_unmask = mcp23s08_irq_unmask;
-		data->mcp[addr]->irq_chip.irq_set_type = mcp23s08_irq_set_type;
-		data->mcp[addr]->irq_chip.irq_bus_lock = mcp23s08_irq_bus_lock;
-		data->mcp[addr]->irq_chip.irq_bus_sync_unlock =
-			mcp23s08_irq_bus_unlock;
 		status = mcp23s08_probe_one(data->mcp[addr], &spi->dev, spi,
 					    0x40 | (addr << 1), type,
 					    -1, addr);
-- 
2.25.1


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

* [PATCH v1 3/9] pinctrl: mcp23s08: Consolidate SPI and I²C code
  2020-04-07 17:38 [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data Andy Shevchenko
  2020-04-07 17:38 ` [PATCH v1 2/9] pinctrl: mcp23s08: Deduplicate IRQ chip filling Andy Shevchenko
@ 2020-04-07 17:38 ` Andy Shevchenko
  2020-04-07 17:38 ` [PATCH v1 4/9] pinctrl: mcp23s08: Drop unused parameter in mcp23s08_probe_one() Andy Shevchenko
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2020-04-07 17:38 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio; +Cc: Andy Shevchenko

There are three parts of each, SPI and I²C, driver spread over the code under
ifdeffery. Consolidate them so it will be only one for SPI and one for I²C.

The code has been cosmetically changed to avoid intrusive change.
Clean up is considered in the nearest future.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 461 +++++++++++++++--------------
 1 file changed, 233 insertions(+), 228 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index e05219d3331f..30a5b6bfbb86 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -27,8 +27,6 @@
 #define MCP_TYPE_S18    4
 #define MCP_TYPE_018    5
 
-#define MCP_MAX_DEV_PER_CS	8
-
 /* Registers are all 8 bits wide.
  *
  * The mcp23s17 has twice as many bits, and can be configured to work
@@ -308,80 +306,6 @@ static const struct pinconf_ops mcp_pinconf_ops = {
 
 /*----------------------------------------------------------------------*/
 
-#ifdef CONFIG_SPI_MASTER
-
-static int mcp23sxx_spi_write(void *context, const void *data, size_t count)
-{
-	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, }, };
-
-	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 mcp23sxx_spi_gather_write(void *context,
-				const void *reg, size_t reg_size,
-				const void *val, size_t val_size)
-{
-	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, }, };
-
-	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 mcp23sxx_spi_read(void *context, const void *reg, size_t reg_size,
-				void *val, size_t val_size)
-{
-	struct mcp23s08 *mcp = context;
-	struct spi_device *spi = to_spi_device(mcp->dev);
-	u8 tx[2];
-
-	if (reg_size != 1)
-		return -EINVAL;
-
-	tx[0] = mcp->addr | 0x01;
-	tx[1] = *((u8 *) reg);
-
-	return spi_write_then_read(spi, tx, sizeof(tx), val, val_size);
-}
-
-static const struct regmap_bus mcp23sxx_spi_regmap = {
-	.write = mcp23sxx_spi_write,
-	.gather_write = mcp23sxx_spi_gather_write,
-	.read = mcp23sxx_spi_read,
-};
-
-#endif /* CONFIG_SPI_MASTER */
-
-/*----------------------------------------------------------------------*/
-
-/* 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 int mcp23s08_direction_input(struct gpio_chip *chip, unsigned offset)
 {
 	struct mcp23s08	*mcp = gpiochip_get_data(chip);
@@ -656,14 +580,12 @@ static int mcp23s08_irqchip_setup(struct mcp23s08 *mcp)
 /*----------------------------------------------------------------------*/
 
 static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
-			      void *data, unsigned addr, unsigned type,
+			      unsigned int addr, unsigned int type,
 			      unsigned int base, int cs)
 {
 	int status, ret;
 	bool mirror = false;
 	bool open_drain = false;
-	struct regmap_config *one_regmap_config = NULL;
-	int raw_chip_address = (addr & ~0x40) >> 1;
 
 	mutex_init(&mcp->lock);
 
@@ -687,83 +609,6 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 	mcp->chip.of_node = dev->of_node;
 #endif
 
-	switch (type) {
-#ifdef CONFIG_SPI_MASTER
-	case MCP_TYPE_S08:
-	case MCP_TYPE_S17:
-		switch (type) {
-		case MCP_TYPE_S08:
-			one_regmap_config =
-				devm_kmemdup(dev, &mcp23x08_regmap,
-					sizeof(struct regmap_config), GFP_KERNEL);
-			mcp->reg_shift = 0;
-			mcp->chip.ngpio = 8;
-			mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL,
-					"mcp23s08.%d", raw_chip_address);
-			break;
-		case MCP_TYPE_S17:
-			one_regmap_config =
-				devm_kmemdup(dev, &mcp23x17_regmap,
-					sizeof(struct regmap_config), GFP_KERNEL);
-			mcp->reg_shift = 1;
-			mcp->chip.ngpio = 16;
-			mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL,
-					"mcp23s17.%d", raw_chip_address);
-			break;
-		}
-		if (!one_regmap_config)
-			return -ENOMEM;
-
-		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d", raw_chip_address);
-		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
-					       one_regmap_config);
-		break;
-
-	case MCP_TYPE_S18:
-		one_regmap_config =
-			devm_kmemdup(dev, &mcp23x17_regmap,
-				sizeof(struct regmap_config), GFP_KERNEL);
-		if (!one_regmap_config)
-			return -ENOMEM;
-		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
-					       one_regmap_config);
-		mcp->reg_shift = 1;
-		mcp->chip.ngpio = 16;
-		mcp->chip.label = "mcp23s18";
-		break;
-#endif /* CONFIG_SPI_MASTER */
-
-#if IS_ENABLED(CONFIG_I2C)
-	case MCP_TYPE_008:
-		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->regmap = devm_regmap_init_i2c(data, &mcp23x17_regmap);
-		mcp->reg_shift = 1;
-		mcp->chip.ngpio = 16;
-		mcp->chip.label = "mcp23017";
-		break;
-
-	case MCP_TYPE_018:
-		mcp->regmap = devm_regmap_init_i2c(data, &mcp23x17_regmap);
-		mcp->reg_shift = 1;
-		mcp->chip.ngpio = 16;
-		mcp->chip.label = "mcp23018";
-		break;
-#endif /* CONFIG_I2C */
-
-	default:
-		dev_err(dev, "invalid device type (%d)\n", type);
-		return -EINVAL;
-	}
-
-	if (IS_ERR(mcp->regmap))
-		return PTR_ERR(mcp->regmap);
-
 	mcp->chip.base = base;
 	mcp->chip.can_sleep = true;
 	mcp->chip.parent = dev;
@@ -822,14 +667,6 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 			goto fail;
 	}
 
-	if (one_regmap_config) {
-		mcp->pinctrl_desc.name = devm_kasprintf(dev, GFP_KERNEL,
-				"mcp23xxx-pinctrl.%d", raw_chip_address);
-		if (!mcp->pinctrl_desc.name)
-			return -ENOMEM;
-	} else {
-		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;
@@ -856,70 +693,13 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 
 /*----------------------------------------------------------------------*/
 
-#ifdef CONFIG_OF
-#ifdef CONFIG_SPI_MASTER
-static const struct of_device_id mcp23s08_spi_of_match[] = {
-	{
-		.compatible = "microchip,mcp23s08",
-		.data = (void *) MCP_TYPE_S08,
-	},
-	{
-		.compatible = "microchip,mcp23s17",
-		.data = (void *) MCP_TYPE_S17,
-	},
-	{
-		.compatible = "microchip,mcp23s18",
-		.data = (void *) MCP_TYPE_S18,
-	},
-/* NOTE: The use of the mcp prefix is deprecated and will be removed. */
-	{
-		.compatible = "mcp,mcp23s08",
-		.data = (void *) MCP_TYPE_S08,
-	},
-	{
-		.compatible = "mcp,mcp23s17",
-		.data = (void *) MCP_TYPE_S17,
-	},
-	{ },
-};
-MODULE_DEVICE_TABLE(of, mcp23s08_spi_of_match);
-#endif
-
-#if IS_ENABLED(CONFIG_I2C)
-static const struct of_device_id mcp23s08_i2c_of_match[] = {
-	{
-		.compatible = "microchip,mcp23008",
-		.data = (void *) MCP_TYPE_008,
-	},
-	{
-		.compatible = "microchip,mcp23017",
-		.data = (void *) MCP_TYPE_017,
-	},
-	{
-		.compatible = "microchip,mcp23018",
-		.data = (void *) MCP_TYPE_018,
-	},
-/* NOTE: The use of the mcp prefix is deprecated and will be removed. */
-	{
-		.compatible = "mcp,mcp23008",
-		.data = (void *) MCP_TYPE_008,
-	},
-	{
-		.compatible = "mcp,mcp23017",
-		.data = (void *) MCP_TYPE_017,
-	},
-	{ },
-};
-MODULE_DEVICE_TABLE(of, mcp23s08_i2c_of_match);
-#endif
-#endif /* CONFIG_OF */
-
-
 #if IS_ENABLED(CONFIG_I2C)
 
 static int mcp230xx_probe(struct i2c_client *client,
 				    const struct i2c_device_id *id)
 {
+	struct device *dev = &client->dev;
+	unsigned int type = id->driver_data;
 	struct mcp23s08 *mcp;
 	int status;
 
@@ -929,8 +709,39 @@ static int mcp230xx_probe(struct i2c_client *client,
 
 	mcp->irq = client->irq;
 
-	status = mcp23s08_probe_one(mcp, &client->dev, client, client->addr,
-				    id->driver_data, -1, 0);
+	switch (type) {
+	case MCP_TYPE_008:
+		mcp->regmap = devm_regmap_init_i2c(client, &mcp23x08_regmap);
+		mcp->reg_shift = 0;
+		mcp->chip.ngpio = 8;
+		mcp->chip.label = "mcp23008";
+		break;
+
+	case MCP_TYPE_017:
+		mcp->regmap = devm_regmap_init_i2c(client, &mcp23x17_regmap);
+		mcp->reg_shift = 1;
+		mcp->chip.ngpio = 16;
+		mcp->chip.label = "mcp23017";
+		break;
+
+	case MCP_TYPE_018:
+		mcp->regmap = devm_regmap_init_i2c(client, &mcp23x17_regmap);
+		mcp->reg_shift = 1;
+		mcp->chip.ngpio = 16;
+		mcp->chip.label = "mcp23018";
+		break;
+
+	default:
+		dev_err(dev, "invalid device type (%d)\n", type);
+		return -EINVAL;
+	}
+
+	if (IS_ERR(mcp->regmap))
+		return PTR_ERR(mcp->regmap);
+
+	mcp->pinctrl_desc.name = "mcp23xxx-pinctrl";
+
+	status = mcp23s08_probe_one(mcp, dev, client->addr, type, -1, 0);
 	if (status)
 		return status;
 
@@ -947,6 +758,34 @@ static const struct i2c_device_id mcp230xx_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, mcp230xx_id);
 
+#ifdef CONFIG_OF
+static const struct of_device_id mcp23s08_i2c_of_match[] = {
+	{
+		.compatible = "microchip,mcp23008",
+		.data = (void *) MCP_TYPE_008,
+	},
+	{
+		.compatible = "microchip,mcp23017",
+		.data = (void *) MCP_TYPE_017,
+	},
+	{
+		.compatible = "microchip,mcp23018",
+		.data = (void *) MCP_TYPE_018,
+	},
+/* NOTE: The use of the mcp prefix is deprecated and will be removed. */
+	{
+		.compatible = "mcp,mcp23008",
+		.data = (void *) MCP_TYPE_008,
+	},
+	{
+		.compatible = "mcp,mcp23017",
+		.data = (void *) MCP_TYPE_017,
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, mcp23s08_i2c_of_match);
+#endif /* CONFIG_OF */
+
 static struct i2c_driver mcp230xx_driver = {
 	.driver = {
 		.name	= "mcp230xx",
@@ -977,8 +816,138 @@ static void mcp23s08_i2c_exit(void) { }
 
 #ifdef CONFIG_SPI_MASTER
 
+#define MCP_MAX_DEV_PER_CS	8
+
+static int mcp23sxx_spi_write(void *context, const void *data, size_t count)
+{
+	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, }, };
+
+	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 mcp23sxx_spi_gather_write(void *context,
+				const void *reg, size_t reg_size,
+				const void *val, size_t val_size)
+{
+	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, }, };
+
+	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 mcp23sxx_spi_read(void *context, const void *reg, size_t reg_size,
+				void *val, size_t val_size)
+{
+	struct mcp23s08 *mcp = context;
+	struct spi_device *spi = to_spi_device(mcp->dev);
+	u8 tx[2];
+
+	if (reg_size != 1)
+		return -EINVAL;
+
+	tx[0] = mcp->addr | 0x01;
+	tx[1] = *((u8 *) reg);
+
+	return spi_write_then_read(spi, tx, sizeof(tx), val, val_size);
+}
+
+static const struct regmap_bus mcp23sxx_spi_regmap = {
+	.write = mcp23sxx_spi_write,
+	.gather_write = mcp23sxx_spi_gather_write,
+	.read = mcp23sxx_spi_read,
+};
+
+/* 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 int mcp23s08_spi_regmap_init(struct mcp23s08 *mcp, struct device *dev,
+				    unsigned int addr, unsigned int type)
+{
+	struct regmap_config *one_regmap_config = NULL;
+
+	switch (type) {
+	case MCP_TYPE_S08:
+	case MCP_TYPE_S17:
+		switch (type) {
+		case MCP_TYPE_S08:
+			one_regmap_config =
+				devm_kmemdup(dev, &mcp23x08_regmap,
+					sizeof(struct regmap_config), GFP_KERNEL);
+			mcp->reg_shift = 0;
+			mcp->chip.ngpio = 8;
+			mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL,
+					"mcp23s08.%d", addr);
+			break;
+		case MCP_TYPE_S17:
+			one_regmap_config =
+				devm_kmemdup(dev, &mcp23x17_regmap,
+					sizeof(struct regmap_config), GFP_KERNEL);
+			mcp->reg_shift = 1;
+			mcp->chip.ngpio = 16;
+			mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL,
+					"mcp23s17.%d", addr);
+			break;
+		}
+		if (!one_regmap_config)
+			return -ENOMEM;
+
+		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d", addr);
+		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
+					       one_regmap_config);
+		break;
+
+	case MCP_TYPE_S18:
+		one_regmap_config =
+			devm_kmemdup(dev, &mcp23x17_regmap,
+				sizeof(struct regmap_config), GFP_KERNEL);
+		if (!one_regmap_config)
+			return -ENOMEM;
+		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
+					       one_regmap_config);
+		mcp->reg_shift = 1;
+		mcp->chip.ngpio = 16;
+		mcp->chip.label = "mcp23s18";
+		break;
+
+	default:
+		dev_err(dev, "invalid device type (%d)\n", type);
+		return -EINVAL;
+	}
+
+	if (IS_ERR(mcp->regmap))
+		return PTR_ERR(mcp->regmap);
+
+	return 0;
+}
+
 static int mcp23s08_probe(struct spi_device *spi)
 {
+	struct device *dev = &spi->dev;
 	unsigned			addr;
 	int				chips = 0;
 	struct mcp23s08_driver_data	*data;
@@ -1030,9 +999,17 @@ static int mcp23s08_probe(struct spi_device *spi)
 		chips--;
 		data->mcp[addr] = &data->chip[chips];
 		data->mcp[addr]->irq = spi->irq;
-		status = mcp23s08_probe_one(data->mcp[addr], &spi->dev, spi,
-					    0x40 | (addr << 1), type,
-					    -1, addr);
+
+		status = mcp23s08_spi_regmap_init(data->mcp[addr], dev, addr, type);
+		if (status)
+			return status;
+
+		data->mcp[addr]->pinctrl_desc.name = devm_kasprintf(dev, GFP_KERNEL,
+				"mcp23xxx-pinctrl.%d", addr);
+		if (!data->mcp[addr]->pinctrl_desc.name)
+			return -ENOMEM;
+
+		status = mcp23s08_probe_one(data->mcp[addr], dev, 0x40 | (addr << 1), type, -1, addr);
 		if (status < 0)
 			return status;
 
@@ -1051,6 +1028,34 @@ static const struct spi_device_id mcp23s08_ids[] = {
 };
 MODULE_DEVICE_TABLE(spi, mcp23s08_ids);
 
+#ifdef CONFIG_OF
+static const struct of_device_id mcp23s08_spi_of_match[] = {
+	{
+		.compatible = "microchip,mcp23s08",
+		.data = (void *) MCP_TYPE_S08,
+	},
+	{
+		.compatible = "microchip,mcp23s17",
+		.data = (void *) MCP_TYPE_S17,
+	},
+	{
+		.compatible = "microchip,mcp23s18",
+		.data = (void *) MCP_TYPE_S18,
+	},
+/* NOTE: The use of the mcp prefix is deprecated and will be removed. */
+	{
+		.compatible = "mcp,mcp23s08",
+		.data = (void *) MCP_TYPE_S08,
+	},
+	{
+		.compatible = "mcp,mcp23s17",
+		.data = (void *) MCP_TYPE_S17,
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, mcp23s08_spi_of_match);
+#endif
+
 static struct spi_driver mcp23s08_driver = {
 	.probe		= mcp23s08_probe,
 	.id_table	= mcp23s08_ids,
-- 
2.25.1


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

* [PATCH v1 4/9] pinctrl: mcp23s08: Drop unused parameter in mcp23s08_probe_one()
  2020-04-07 17:38 [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data Andy Shevchenko
  2020-04-07 17:38 ` [PATCH v1 2/9] pinctrl: mcp23s08: Deduplicate IRQ chip filling Andy Shevchenko
  2020-04-07 17:38 ` [PATCH v1 3/9] pinctrl: mcp23s08: Consolidate SPI and I²C code Andy Shevchenko
@ 2020-04-07 17:38 ` Andy Shevchenko
  2020-04-07 17:38 ` [PATCH v1 5/9] pinctrl: mcp23s08: Refactor mcp23s08_spi_regmap_init() Andy Shevchenko
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2020-04-07 17:38 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio; +Cc: Andy Shevchenko

The cs parameter in mcp23s08_probe_one() is never used. Drop it for good.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 30a5b6bfbb86..513864c74860 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -581,7 +581,7 @@ static int mcp23s08_irqchip_setup(struct mcp23s08 *mcp)
 
 static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 			      unsigned int addr, unsigned int type,
-			      unsigned int base, int cs)
+			      unsigned int base)
 {
 	int status, ret;
 	bool mirror = false;
@@ -741,7 +741,7 @@ static int mcp230xx_probe(struct i2c_client *client,
 
 	mcp->pinctrl_desc.name = "mcp23xxx-pinctrl";
 
-	status = mcp23s08_probe_one(mcp, dev, client->addr, type, -1, 0);
+	status = mcp23s08_probe_one(mcp, dev, client->addr, type, -1);
 	if (status)
 		return status;
 
@@ -1009,7 +1009,7 @@ static int mcp23s08_probe(struct spi_device *spi)
 		if (!data->mcp[addr]->pinctrl_desc.name)
 			return -ENOMEM;
 
-		status = mcp23s08_probe_one(data->mcp[addr], dev, 0x40 | (addr << 1), type, -1, addr);
+		status = mcp23s08_probe_one(data->mcp[addr], dev, 0x40 | (addr << 1), type, -1);
 		if (status < 0)
 			return status;
 
-- 
2.25.1


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

* [PATCH v1 5/9] pinctrl: mcp23s08: Refactor mcp23s08_spi_regmap_init()
  2020-04-07 17:38 [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data Andy Shevchenko
                   ` (2 preceding siblings ...)
  2020-04-07 17:38 ` [PATCH v1 4/9] pinctrl: mcp23s08: Drop unused parameter in mcp23s08_probe_one() Andy Shevchenko
@ 2020-04-07 17:38 ` Andy Shevchenko
  2020-04-07 17:38 ` [PATCH v1 6/9] pinctrl: mcp23s08: Propagate error code from device_property_read_u32() Andy Shevchenko
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2020-04-07 17:38 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio; +Cc: Andy Shevchenko

There is a lot of duplication for one small helper function.
Refactor mcp23s08_spi_regmap_init() to prepare everything first
and then register regmap at the end.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 61 ++++++++++++++----------------
 1 file changed, 28 insertions(+), 33 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 513864c74860..07b69242a5b3 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -888,50 +888,38 @@ struct mcp23s08_driver_data {
 static int mcp23s08_spi_regmap_init(struct mcp23s08 *mcp, struct device *dev,
 				    unsigned int addr, unsigned int type)
 {
-	struct regmap_config *one_regmap_config = NULL;
+	const struct regmap_config *config;
+	struct regmap_config *copy;
+	const char *name;
 
 	switch (type) {
 	case MCP_TYPE_S08:
+		mcp->reg_shift = 0;
+		mcp->chip.ngpio = 8;
+		mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL, "mcp23s08.%d",
+						 addr);
+
+		config = &mcp23x08_regmap;
+		name = devm_kasprintf(dev, GFP_KERNEL, "%d", addr);
+		break;
+
 	case MCP_TYPE_S17:
-		switch (type) {
-		case MCP_TYPE_S08:
-			one_regmap_config =
-				devm_kmemdup(dev, &mcp23x08_regmap,
-					sizeof(struct regmap_config), GFP_KERNEL);
-			mcp->reg_shift = 0;
-			mcp->chip.ngpio = 8;
-			mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL,
-					"mcp23s08.%d", addr);
-			break;
-		case MCP_TYPE_S17:
-			one_regmap_config =
-				devm_kmemdup(dev, &mcp23x17_regmap,
-					sizeof(struct regmap_config), GFP_KERNEL);
-			mcp->reg_shift = 1;
-			mcp->chip.ngpio = 16;
-			mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL,
-					"mcp23s17.%d", addr);
-			break;
-		}
-		if (!one_regmap_config)
-			return -ENOMEM;
+		mcp->reg_shift = 1;
+		mcp->chip.ngpio = 16;
+		mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL, "mcp23s17.%d",
+						 addr);
 
-		one_regmap_config->name = devm_kasprintf(dev, GFP_KERNEL, "%d", addr);
-		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
-					       one_regmap_config);
+		config = &mcp23x17_regmap;
+		name = devm_kasprintf(dev, GFP_KERNEL, "%d", addr);
 		break;
 
 	case MCP_TYPE_S18:
-		one_regmap_config =
-			devm_kmemdup(dev, &mcp23x17_regmap,
-				sizeof(struct regmap_config), GFP_KERNEL);
-		if (!one_regmap_config)
-			return -ENOMEM;
-		mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp,
-					       one_regmap_config);
 		mcp->reg_shift = 1;
 		mcp->chip.ngpio = 16;
 		mcp->chip.label = "mcp23s18";
+
+		config = &mcp23x17_regmap;
+		name = config->name;
 		break;
 
 	default:
@@ -939,6 +927,13 @@ static int mcp23s08_spi_regmap_init(struct mcp23s08 *mcp, struct device *dev,
 		return -EINVAL;
 	}
 
+	copy = devm_kmemdup(dev, &config, sizeof(config), GFP_KERNEL);
+	if (!copy)
+		return -ENOMEM;
+
+	copy->name = name;
+
+	mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp, copy);
 	if (IS_ERR(mcp->regmap))
 		return PTR_ERR(mcp->regmap);
 
-- 
2.25.1


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

* [PATCH v1 6/9] pinctrl: mcp23s08: Propagate error code from device_property_read_u32()
  2020-04-07 17:38 [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data Andy Shevchenko
                   ` (3 preceding siblings ...)
  2020-04-07 17:38 ` [PATCH v1 5/9] pinctrl: mcp23s08: Refactor mcp23s08_spi_regmap_init() Andy Shevchenko
@ 2020-04-07 17:38 ` Andy Shevchenko
  2020-04-07 17:38 ` [PATCH v1 7/9] pinctrl: mcp23s08: Make use of device properties Andy Shevchenko
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2020-04-07 17:38 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio; +Cc: Andy Shevchenko

Return error code from device_property_read_u32() as is in mcp23s08_probe().
While here, drop status variable in mcp23s08_irq_set_type() which always 0.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 07b69242a5b3..515d1aa32732 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -485,7 +485,6 @@ static int mcp23s08_irq_set_type(struct irq_data *data, unsigned int type)
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
 	struct mcp23s08 *mcp = gpiochip_get_data(gc);
 	unsigned int pos = data->hwirq;
-	int status = 0;
 
 	if ((type & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH) {
 		mcp_set_bit(mcp, MCP_INTCON, pos, false);
@@ -508,7 +507,7 @@ static int mcp23s08_irq_set_type(struct irq_data *data, unsigned int type)
 	} else
 		return -EINVAL;
 
-	return status;
+	return 0;
 }
 
 static void mcp23s08_irq_bus_lock(struct irq_data *data)
@@ -964,7 +963,7 @@ static int mcp23s08_probe(struct spi_device *spi)
 				"mcp,spi-present-mask", &spi_present_mask);
 		if (status) {
 			dev_err(&spi->dev, "missing spi-present-mask");
-			return -ENODEV;
+			return status;
 		}
 	}
 
-- 
2.25.1


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

* [PATCH v1 7/9] pinctrl: mcp23s08: Make use of device properties
  2020-04-07 17:38 [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data Andy Shevchenko
                   ` (4 preceding siblings ...)
  2020-04-07 17:38 ` [PATCH v1 6/9] pinctrl: mcp23s08: Propagate error code from device_property_read_u32() Andy Shevchenko
@ 2020-04-07 17:38 ` Andy Shevchenko
  2020-04-07 17:38 ` [PATCH v1 8/9] pinctrl: mcp23s08: Use for_each_set_bit() and hweight_long() Andy Shevchenko
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2020-04-07 17:38 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio; +Cc: Andy Shevchenko

Device property API allows to gather device resources from different sources,
such as ACPI. Convert the drivers to unleash the power of device property API.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 515d1aa32732..330c2203e0f2 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -4,6 +4,7 @@
 #include <linux/kernel.h>
 #include <linux/device.h>
 #include <linux/mutex.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/gpio/driver.h>
 #include <linux/i2c.h>
@@ -11,7 +12,6 @@
 #include <linux/slab.h>
 #include <asm/byteorder.h>
 #include <linux/interrupt.h>
-#include <linux/of_device.h>
 #include <linux/regmap.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinconf.h>
@@ -20,12 +20,12 @@
 /*
  * MCP types supported by driver
  */
-#define MCP_TYPE_S08	0
-#define MCP_TYPE_S17	1
-#define MCP_TYPE_008	2
-#define MCP_TYPE_017	3
-#define MCP_TYPE_S18    4
-#define MCP_TYPE_018    5
+#define MCP_TYPE_S08	1
+#define MCP_TYPE_S17	2
+#define MCP_TYPE_008	3
+#define MCP_TYPE_017	4
+#define MCP_TYPE_S18	5
+#define MCP_TYPE_018	6
 
 /* Registers are all 8 bits wide.
  *
@@ -757,7 +757,6 @@ static const struct i2c_device_id mcp230xx_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, mcp230xx_id);
 
-#ifdef CONFIG_OF
 static const struct of_device_id mcp23s08_i2c_of_match[] = {
 	{
 		.compatible = "microchip,mcp23008",
@@ -783,12 +782,11 @@ static const struct of_device_id mcp23s08_i2c_of_match[] = {
 	{ },
 };
 MODULE_DEVICE_TABLE(of, mcp23s08_i2c_of_match);
-#endif /* CONFIG_OF */
 
 static struct i2c_driver mcp230xx_driver = {
 	.driver = {
 		.name	= "mcp230xx",
-		.of_match_table = of_match_ptr(mcp23s08_i2c_of_match),
+		.of_match_table = mcp23s08_i2c_of_match,
 	},
 	.probe		= mcp230xx_probe,
 	.id_table	= mcp230xx_id,
@@ -942,17 +940,17 @@ static int mcp23s08_spi_regmap_init(struct mcp23s08 *mcp, struct device *dev,
 static int mcp23s08_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
+	const void *match;
 	unsigned			addr;
 	int				chips = 0;
 	struct mcp23s08_driver_data	*data;
 	int				status, type;
 	unsigned			ngpio = 0;
-	const struct			of_device_id *match;
 	u32				spi_present_mask;
 
-	match = of_match_device(of_match_ptr(mcp23s08_spi_of_match), &spi->dev);
+	match = device_get_match_data(dev);
 	if (match)
-		type = (int)(uintptr_t)match->data;
+		type = (int)(uintptr_t)match;
 	else
 		type = spi_get_device_id(spi)->driver_data;
 
@@ -1022,7 +1020,6 @@ static const struct spi_device_id mcp23s08_ids[] = {
 };
 MODULE_DEVICE_TABLE(spi, mcp23s08_ids);
 
-#ifdef CONFIG_OF
 static const struct of_device_id mcp23s08_spi_of_match[] = {
 	{
 		.compatible = "microchip,mcp23s08",
@@ -1048,14 +1045,13 @@ static const struct of_device_id mcp23s08_spi_of_match[] = {
 	{ },
 };
 MODULE_DEVICE_TABLE(of, mcp23s08_spi_of_match);
-#endif
 
 static struct spi_driver mcp23s08_driver = {
 	.probe		= mcp23s08_probe,
 	.id_table	= mcp23s08_ids,
 	.driver = {
 		.name	= "mcp23s08",
-		.of_match_table = of_match_ptr(mcp23s08_spi_of_match),
+		.of_match_table = mcp23s08_spi_of_match,
 	},
 };
 
-- 
2.25.1


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

* [PATCH v1 8/9] pinctrl: mcp23s08: Use for_each_set_bit() and hweight_long()
  2020-04-07 17:38 [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data Andy Shevchenko
                   ` (5 preceding siblings ...)
  2020-04-07 17:38 ` [PATCH v1 7/9] pinctrl: mcp23s08: Make use of device properties Andy Shevchenko
@ 2020-04-07 17:38 ` Andy Shevchenko
  2020-04-07 17:38 ` [PATCH v1 9/9] pinctrl: mcp23s08: Split to three parts: core, I²C, SPI Andy Shevchenko
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2020-04-07 17:38 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio; +Cc: Andy Shevchenko

Here is a simplification of SPI code by using for_each_set_bit() and
hweight_long() library functions.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 30 +++++++++++-------------------
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 330c2203e0f2..ea8decc36d50 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /* MCP23S08 SPI/I2C GPIO driver */
 
+#include <linux/bitops.h>
 #include <linux/kernel.h>
 #include <linux/device.h>
 #include <linux/mutex.h>
@@ -940,13 +941,14 @@ static int mcp23s08_spi_regmap_init(struct mcp23s08 *mcp, struct device *dev,
 static int mcp23s08_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
+	unsigned long spi_present_mask;
 	const void *match;
+	int chips;
+	u32 v;
 	unsigned			addr;
-	int				chips = 0;
 	struct mcp23s08_driver_data	*data;
 	int				status, type;
 	unsigned			ngpio = 0;
-	u32				spi_present_mask;
 
 	match = device_get_match_data(dev);
 	if (match)
@@ -954,29 +956,22 @@ static int mcp23s08_probe(struct spi_device *spi)
 	else
 		type = spi_get_device_id(spi)->driver_data;
 
-	status = device_property_read_u32(&spi->dev,
-			"microchip,spi-present-mask", &spi_present_mask);
+	status = device_property_read_u32(dev, "microchip,spi-present-mask", &v);
 	if (status) {
-		status = device_property_read_u32(&spi->dev,
-				"mcp,spi-present-mask", &spi_present_mask);
+		status = device_property_read_u32(dev, "mcp,spi-present-mask", &v);
 		if (status) {
 			dev_err(&spi->dev, "missing spi-present-mask");
 			return status;
 		}
 	}
+	spi_present_mask = v;
 
-	if (!spi_present_mask || spi_present_mask > 0xff) {
+	if (!spi_present_mask || spi_present_mask >= BIT(MCP_MAX_DEV_PER_CS)) {
 		dev_err(&spi->dev, "invalid spi-present-mask");
 		return -ENODEV;
 	}
 
-	for (addr = 0; addr < MCP_MAX_DEV_PER_CS; addr++) {
-		if (spi_present_mask & BIT(addr))
-			chips++;
-	}
-
-	if (!chips)
-		return -ENODEV;
+	chips = hweight_long(spi_present_mask);
 
 	data = devm_kzalloc(&spi->dev,
 			    struct_size(data, chip, chips), GFP_KERNEL);
@@ -985,11 +980,8 @@ static int mcp23s08_probe(struct spi_device *spi)
 
 	spi_set_drvdata(spi, data);
 
-	for (addr = 0; addr < MCP_MAX_DEV_PER_CS; addr++) {
-		if (!(spi_present_mask & BIT(addr)))
-			continue;
-		chips--;
-		data->mcp[addr] = &data->chip[chips];
+	for_each_set_bit(addr, &spi_present_mask, MCP_MAX_DEV_PER_CS) {
+		data->mcp[addr] = &data->chip[--chips];
 		data->mcp[addr]->irq = spi->irq;
 
 		status = mcp23s08_spi_regmap_init(data->mcp[addr], dev, addr, type);
-- 
2.25.1


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

* [PATCH v1 9/9] pinctrl: mcp23s08: Split to three parts: core, I²C, SPI
  2020-04-07 17:38 [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data Andy Shevchenko
                   ` (6 preceding siblings ...)
  2020-04-07 17:38 ` [PATCH v1 8/9] pinctrl: mcp23s08: Use for_each_set_bit() and hweight_long() Andy Shevchenko
@ 2020-04-07 17:38 ` Andy Shevchenko
  2020-04-16 10:35 ` [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data Linus Walleij
  2021-09-16 12:51 ` Florian Eckert
  9 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2020-04-07 17:38 UTC (permalink / raw)
  To: Linus Walleij, linux-gpio; +Cc: Andy Shevchenko

Split the driver to three parts: core, I²C, SPI.
No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/Kconfig                |  13 +-
 drivers/pinctrl/Makefile               |   2 +
 drivers/pinctrl/pinctrl-mcp23s08.c     | 458 +------------------------
 drivers/pinctrl/pinctrl-mcp23s08.h     |  52 +++
 drivers/pinctrl/pinctrl-mcp23s08_i2c.c | 124 +++++++
 drivers/pinctrl/pinctrl-mcp23s08_spi.c | 262 ++++++++++++++
 6 files changed, 459 insertions(+), 452 deletions(-)
 create mode 100644 drivers/pinctrl/pinctrl-mcp23s08.h
 create mode 100644 drivers/pinctrl/pinctrl-mcp23s08_i2c.c
 create mode 100644 drivers/pinctrl/pinctrl-mcp23s08_spi.c

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 834c59950d1c..f646f070d98f 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -172,15 +172,22 @@ config PINCTRL_GEMINI
 	select GENERIC_PINCONF
 	select MFD_SYSCON
 
+config PINCTRL_MCP23S08_I2C
+	tristate
+	select REGMAP_I2C
+
+config PINCTRL_MCP23S08_SPI
+	tristate
+	select REGMAP_SPI
+
 config PINCTRL_MCP23S08
 	tristate "Microchip MCP23xxx I/O expander"
 	depends on SPI_MASTER || I2C
-	depends on I2C || I2C=n
 	select GPIOLIB
 	select GPIOLIB_IRQCHIP
-	select REGMAP_I2C if I2C
-	select REGMAP_SPI if SPI_MASTER
 	select GENERIC_PINCONF
+	select PINCTRL_MCP23S08_I2C if I2C
+	select PINCTRL_MCP23S08_SPI if SPI_MASTER
 	help
 	  SPI/I2C driver for Microchip MCP23S08 / MCP23S17 / MCP23S18 /
 	  MCP23008 / MCP23017 / MCP23018 I/O expanders.
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 0b36a1cfca8a..1731b2154df9 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -21,6 +21,8 @@ obj-$(CONFIG_PINCTRL_DIGICOLOR)	+= pinctrl-digicolor.o
 obj-$(CONFIG_PINCTRL_FALCON)	+= pinctrl-falcon.o
 obj-$(CONFIG_PINCTRL_GEMINI)	+= pinctrl-gemini.o
 obj-$(CONFIG_PINCTRL_MAX77620)	+= pinctrl-max77620.o
+obj-$(CONFIG_PINCTRL_MCP23S08_I2C)	+= pinctrl-mcp23s08_i2c.o
+obj-$(CONFIG_PINCTRL_MCP23S08_SPI)	+= pinctrl-mcp23s08_spi.o
 obj-$(CONFIG_PINCTRL_MCP23S08)	+= pinctrl-mcp23s08.o
 obj-$(CONFIG_PINCTRL_MESON)	+= meson/
 obj-$(CONFIG_PINCTRL_OXNAS)	+= pinctrl-oxnas.o
diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index ea8decc36d50..cb545557dcd4 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -7,9 +7,8 @@
 #include <linux/mutex.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/export.h>
 #include <linux/gpio/driver.h>
-#include <linux/i2c.h>
-#include <linux/spi/spi.h>
 #include <linux/slab.h>
 #include <asm/byteorder.h>
 #include <linux/interrupt.h>
@@ -18,15 +17,7 @@
 #include <linux/pinctrl/pinconf.h>
 #include <linux/pinctrl/pinconf-generic.h>
 
-/*
- * MCP types supported by driver
- */
-#define MCP_TYPE_S08	1
-#define MCP_TYPE_S17	2
-#define MCP_TYPE_008	3
-#define MCP_TYPE_017	4
-#define MCP_TYPE_S18	5
-#define MCP_TYPE_018	6
+#include "pinctrl-mcp23s08.h"
 
 /* Registers are all 8 bits wide.
  *
@@ -51,31 +42,6 @@
 #define MCP_GPIO	0x09
 #define MCP_OLAT	0x0a
 
-struct mcp23s08;
-
-struct mcp23s08 {
-	u8			addr;
-	bool			irq_active_high;
-	bool			reg_shift;
-
-	u16			irq_rise;
-	u16			irq_fall;
-	int			irq;
-	bool			irq_controller;
-	int			cached_gpio;
-	/* lock protects regmap access with bypass/cache flags */
-	struct mutex		lock;
-
-	struct gpio_chip	chip;
-	struct irq_chip		irq_chip;
-
-	struct regmap		*regmap;
-	struct device		*dev;
-
-	struct pinctrl_dev	*pctldev;
-	struct pinctrl_desc	pinctrl_desc;
-};
-
 static const struct reg_default mcp23x08_defaults[] = {
 	{.reg = MCP_IODIR,		.def = 0xff},
 	{.reg = MCP_IPOL,		.def = 0x00},
@@ -107,7 +73,7 @@ static const struct regmap_access_table mcp23x08_precious_table = {
 	.n_yes_ranges = 1,
 };
 
-static const struct regmap_config mcp23x08_regmap = {
+const struct regmap_config mcp23x08_regmap = {
 	.reg_bits = 8,
 	.val_bits = 8,
 
@@ -119,6 +85,7 @@ static const struct regmap_config mcp23x08_regmap = {
 	.cache_type = REGCACHE_FLAT,
 	.max_register = MCP_OLAT,
 };
+EXPORT_SYMBOL_GPL(mcp23x08_regmap);
 
 static const struct reg_default mcp23x16_defaults[] = {
 	{.reg = MCP_IODIR << 1,		.def = 0xffff},
@@ -151,7 +118,7 @@ static const struct regmap_access_table mcp23x16_precious_table = {
 	.n_yes_ranges = 1,
 };
 
-static const struct regmap_config mcp23x17_regmap = {
+const struct regmap_config mcp23x17_regmap = {
 	.reg_bits = 8,
 	.val_bits = 16,
 
@@ -164,6 +131,7 @@ static const struct regmap_config mcp23x17_regmap = {
 	.cache_type = REGCACHE_FLAT,
 	.val_format_endian = REGMAP_ENDIAN_LITTLE,
 };
+EXPORT_SYMBOL_GPL(mcp23x17_regmap);
 
 static int mcp_read(struct mcp23s08 *mcp, unsigned int reg, unsigned int *val)
 {
@@ -579,9 +547,8 @@ static int mcp23s08_irqchip_setup(struct mcp23s08 *mcp)
 
 /*----------------------------------------------------------------------*/
 
-static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
-			      unsigned int addr, unsigned int type,
-			      unsigned int base)
+int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
+		       unsigned int addr, unsigned int type, unsigned int base)
 {
 	int status, ret;
 	bool mirror = false;
@@ -690,411 +657,4 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 		dev_dbg(dev, "can't setup chip %d, --> %d\n", addr, ret);
 	return ret;
 }
-
-/*----------------------------------------------------------------------*/
-
-#if IS_ENABLED(CONFIG_I2C)
-
-static int mcp230xx_probe(struct i2c_client *client,
-				    const struct i2c_device_id *id)
-{
-	struct device *dev = &client->dev;
-	unsigned int type = id->driver_data;
-	struct mcp23s08 *mcp;
-	int status;
-
-	mcp = devm_kzalloc(&client->dev, sizeof(*mcp), GFP_KERNEL);
-	if (!mcp)
-		return -ENOMEM;
-
-	mcp->irq = client->irq;
-
-	switch (type) {
-	case MCP_TYPE_008:
-		mcp->regmap = devm_regmap_init_i2c(client, &mcp23x08_regmap);
-		mcp->reg_shift = 0;
-		mcp->chip.ngpio = 8;
-		mcp->chip.label = "mcp23008";
-		break;
-
-	case MCP_TYPE_017:
-		mcp->regmap = devm_regmap_init_i2c(client, &mcp23x17_regmap);
-		mcp->reg_shift = 1;
-		mcp->chip.ngpio = 16;
-		mcp->chip.label = "mcp23017";
-		break;
-
-	case MCP_TYPE_018:
-		mcp->regmap = devm_regmap_init_i2c(client, &mcp23x17_regmap);
-		mcp->reg_shift = 1;
-		mcp->chip.ngpio = 16;
-		mcp->chip.label = "mcp23018";
-		break;
-
-	default:
-		dev_err(dev, "invalid device type (%d)\n", type);
-		return -EINVAL;
-	}
-
-	if (IS_ERR(mcp->regmap))
-		return PTR_ERR(mcp->regmap);
-
-	mcp->pinctrl_desc.name = "mcp23xxx-pinctrl";
-
-	status = mcp23s08_probe_one(mcp, dev, client->addr, type, -1);
-	if (status)
-		return status;
-
-	i2c_set_clientdata(client, mcp);
-
-	return 0;
-}
-
-static const struct i2c_device_id mcp230xx_id[] = {
-	{ "mcp23008", MCP_TYPE_008 },
-	{ "mcp23017", MCP_TYPE_017 },
-	{ "mcp23018", MCP_TYPE_018 },
-	{ },
-};
-MODULE_DEVICE_TABLE(i2c, mcp230xx_id);
-
-static const struct of_device_id mcp23s08_i2c_of_match[] = {
-	{
-		.compatible = "microchip,mcp23008",
-		.data = (void *) MCP_TYPE_008,
-	},
-	{
-		.compatible = "microchip,mcp23017",
-		.data = (void *) MCP_TYPE_017,
-	},
-	{
-		.compatible = "microchip,mcp23018",
-		.data = (void *) MCP_TYPE_018,
-	},
-/* NOTE: The use of the mcp prefix is deprecated and will be removed. */
-	{
-		.compatible = "mcp,mcp23008",
-		.data = (void *) MCP_TYPE_008,
-	},
-	{
-		.compatible = "mcp,mcp23017",
-		.data = (void *) MCP_TYPE_017,
-	},
-	{ },
-};
-MODULE_DEVICE_TABLE(of, mcp23s08_i2c_of_match);
-
-static struct i2c_driver mcp230xx_driver = {
-	.driver = {
-		.name	= "mcp230xx",
-		.of_match_table = mcp23s08_i2c_of_match,
-	},
-	.probe		= mcp230xx_probe,
-	.id_table	= mcp230xx_id,
-};
-
-static int __init mcp23s08_i2c_init(void)
-{
-	return i2c_add_driver(&mcp230xx_driver);
-}
-
-static void mcp23s08_i2c_exit(void)
-{
-	i2c_del_driver(&mcp230xx_driver);
-}
-
-#else
-
-static int __init mcp23s08_i2c_init(void) { return 0; }
-static void mcp23s08_i2c_exit(void) { }
-
-#endif /* CONFIG_I2C */
-
-/*----------------------------------------------------------------------*/
-
-#ifdef CONFIG_SPI_MASTER
-
-#define MCP_MAX_DEV_PER_CS	8
-
-static int mcp23sxx_spi_write(void *context, const void *data, size_t count)
-{
-	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, }, };
-
-	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 mcp23sxx_spi_gather_write(void *context,
-				const void *reg, size_t reg_size,
-				const void *val, size_t val_size)
-{
-	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, }, };
-
-	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 mcp23sxx_spi_read(void *context, const void *reg, size_t reg_size,
-				void *val, size_t val_size)
-{
-	struct mcp23s08 *mcp = context;
-	struct spi_device *spi = to_spi_device(mcp->dev);
-	u8 tx[2];
-
-	if (reg_size != 1)
-		return -EINVAL;
-
-	tx[0] = mcp->addr | 0x01;
-	tx[1] = *((u8 *) reg);
-
-	return spi_write_then_read(spi, tx, sizeof(tx), val, val_size);
-}
-
-static const struct regmap_bus mcp23sxx_spi_regmap = {
-	.write = mcp23sxx_spi_write,
-	.gather_write = mcp23sxx_spi_gather_write,
-	.read = mcp23sxx_spi_read,
-};
-
-/* 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 int mcp23s08_spi_regmap_init(struct mcp23s08 *mcp, struct device *dev,
-				    unsigned int addr, unsigned int type)
-{
-	const struct regmap_config *config;
-	struct regmap_config *copy;
-	const char *name;
-
-	switch (type) {
-	case MCP_TYPE_S08:
-		mcp->reg_shift = 0;
-		mcp->chip.ngpio = 8;
-		mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL, "mcp23s08.%d",
-						 addr);
-
-		config = &mcp23x08_regmap;
-		name = devm_kasprintf(dev, GFP_KERNEL, "%d", addr);
-		break;
-
-	case MCP_TYPE_S17:
-		mcp->reg_shift = 1;
-		mcp->chip.ngpio = 16;
-		mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL, "mcp23s17.%d",
-						 addr);
-
-		config = &mcp23x17_regmap;
-		name = devm_kasprintf(dev, GFP_KERNEL, "%d", addr);
-		break;
-
-	case MCP_TYPE_S18:
-		mcp->reg_shift = 1;
-		mcp->chip.ngpio = 16;
-		mcp->chip.label = "mcp23s18";
-
-		config = &mcp23x17_regmap;
-		name = config->name;
-		break;
-
-	default:
-		dev_err(dev, "invalid device type (%d)\n", type);
-		return -EINVAL;
-	}
-
-	copy = devm_kmemdup(dev, &config, sizeof(config), GFP_KERNEL);
-	if (!copy)
-		return -ENOMEM;
-
-	copy->name = name;
-
-	mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp, copy);
-	if (IS_ERR(mcp->regmap))
-		return PTR_ERR(mcp->regmap);
-
-	return 0;
-}
-
-static int mcp23s08_probe(struct spi_device *spi)
-{
-	struct device *dev = &spi->dev;
-	unsigned long spi_present_mask;
-	const void *match;
-	int chips;
-	u32 v;
-	unsigned			addr;
-	struct mcp23s08_driver_data	*data;
-	int				status, type;
-	unsigned			ngpio = 0;
-
-	match = device_get_match_data(dev);
-	if (match)
-		type = (int)(uintptr_t)match;
-	else
-		type = spi_get_device_id(spi)->driver_data;
-
-	status = device_property_read_u32(dev, "microchip,spi-present-mask", &v);
-	if (status) {
-		status = device_property_read_u32(dev, "mcp,spi-present-mask", &v);
-		if (status) {
-			dev_err(&spi->dev, "missing spi-present-mask");
-			return status;
-		}
-	}
-	spi_present_mask = v;
-
-	if (!spi_present_mask || spi_present_mask >= BIT(MCP_MAX_DEV_PER_CS)) {
-		dev_err(&spi->dev, "invalid spi-present-mask");
-		return -ENODEV;
-	}
-
-	chips = hweight_long(spi_present_mask);
-
-	data = devm_kzalloc(&spi->dev,
-			    struct_size(data, chip, chips), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	spi_set_drvdata(spi, data);
-
-	for_each_set_bit(addr, &spi_present_mask, MCP_MAX_DEV_PER_CS) {
-		data->mcp[addr] = &data->chip[--chips];
-		data->mcp[addr]->irq = spi->irq;
-
-		status = mcp23s08_spi_regmap_init(data->mcp[addr], dev, addr, type);
-		if (status)
-			return status;
-
-		data->mcp[addr]->pinctrl_desc.name = devm_kasprintf(dev, GFP_KERNEL,
-				"mcp23xxx-pinctrl.%d", addr);
-		if (!data->mcp[addr]->pinctrl_desc.name)
-			return -ENOMEM;
-
-		status = mcp23s08_probe_one(data->mcp[addr], dev, 0x40 | (addr << 1), type, -1);
-		if (status < 0)
-			return status;
-
-		ngpio += data->mcp[addr]->chip.ngpio;
-	}
-	data->ngpio = ngpio;
-
-	return 0;
-}
-
-static const struct spi_device_id mcp23s08_ids[] = {
-	{ "mcp23s08", MCP_TYPE_S08 },
-	{ "mcp23s17", MCP_TYPE_S17 },
-	{ "mcp23s18", MCP_TYPE_S18 },
-	{ },
-};
-MODULE_DEVICE_TABLE(spi, mcp23s08_ids);
-
-static const struct of_device_id mcp23s08_spi_of_match[] = {
-	{
-		.compatible = "microchip,mcp23s08",
-		.data = (void *) MCP_TYPE_S08,
-	},
-	{
-		.compatible = "microchip,mcp23s17",
-		.data = (void *) MCP_TYPE_S17,
-	},
-	{
-		.compatible = "microchip,mcp23s18",
-		.data = (void *) MCP_TYPE_S18,
-	},
-/* NOTE: The use of the mcp prefix is deprecated and will be removed. */
-	{
-		.compatible = "mcp,mcp23s08",
-		.data = (void *) MCP_TYPE_S08,
-	},
-	{
-		.compatible = "mcp,mcp23s17",
-		.data = (void *) MCP_TYPE_S17,
-	},
-	{ },
-};
-MODULE_DEVICE_TABLE(of, mcp23s08_spi_of_match);
-
-static struct spi_driver mcp23s08_driver = {
-	.probe		= mcp23s08_probe,
-	.id_table	= mcp23s08_ids,
-	.driver = {
-		.name	= "mcp23s08",
-		.of_match_table = mcp23s08_spi_of_match,
-	},
-};
-
-static int __init mcp23s08_spi_init(void)
-{
-	return spi_register_driver(&mcp23s08_driver);
-}
-
-static void mcp23s08_spi_exit(void)
-{
-	spi_unregister_driver(&mcp23s08_driver);
-}
-
-#else
-
-static int __init mcp23s08_spi_init(void) { return 0; }
-static void mcp23s08_spi_exit(void) { }
-
-#endif /* CONFIG_SPI_MASTER */
-
-/*----------------------------------------------------------------------*/
-
-static int __init mcp23s08_init(void)
-{
-	int ret;
-
-	ret = mcp23s08_spi_init();
-	if (ret)
-		goto spi_fail;
-
-	ret = mcp23s08_i2c_init();
-	if (ret)
-		goto i2c_fail;
-
-	return 0;
-
- i2c_fail:
-	mcp23s08_spi_exit();
- spi_fail:
-	return ret;
-}
-/* register after spi/i2c postcore initcall and before
- * subsys initcalls that may rely on these GPIOs
- */
-subsys_initcall(mcp23s08_init);
-
-static void __exit mcp23s08_exit(void)
-{
-	mcp23s08_spi_exit();
-	mcp23s08_i2c_exit();
-}
-module_exit(mcp23s08_exit);
-
-MODULE_LICENSE("GPL");
+EXPORT_SYMBOL_GPL(mcp23s08_probe_one);
diff --git a/drivers/pinctrl/pinctrl-mcp23s08.h b/drivers/pinctrl/pinctrl-mcp23s08.h
new file mode 100644
index 000000000000..90dc27081a3c
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-mcp23s08.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* MCP23S08 SPI/I2C GPIO driver */
+
+#include <linux/gpio/driver.h>
+#include <linux/irq.h>
+#include <linux/mutex.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/types.h>
+
+/*
+ * MCP types supported by driver
+ */
+#define MCP_TYPE_S08	1
+#define MCP_TYPE_S17	2
+#define MCP_TYPE_008	3
+#define MCP_TYPE_017	4
+#define MCP_TYPE_S18	5
+#define MCP_TYPE_018	6
+
+struct device;
+struct regmap;
+
+struct pinctrl_dev;
+
+struct mcp23s08 {
+	u8			addr;
+	bool			irq_active_high;
+	bool			reg_shift;
+
+	u16			irq_rise;
+	u16			irq_fall;
+	int			irq;
+	bool			irq_controller;
+	int			cached_gpio;
+	/* lock protects regmap access with bypass/cache flags */
+	struct mutex		lock;
+
+	struct gpio_chip	chip;
+	struct irq_chip		irq_chip;
+
+	struct regmap		*regmap;
+	struct device		*dev;
+
+	struct pinctrl_dev	*pctldev;
+	struct pinctrl_desc	pinctrl_desc;
+};
+
+extern const struct regmap_config mcp23x08_regmap;
+extern const struct regmap_config mcp23x17_regmap;
+
+int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
+		       unsigned int addr, unsigned int type, unsigned int base);
diff --git a/drivers/pinctrl/pinctrl-mcp23s08_i2c.c b/drivers/pinctrl/pinctrl-mcp23s08_i2c.c
new file mode 100644
index 000000000000..e0b001c8c08c
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-mcp23s08_i2c.c
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* MCP23S08 I2C GPIO driver */
+
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "pinctrl-mcp23s08.h"
+
+static int mcp230xx_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	unsigned int type = id->driver_data;
+	struct mcp23s08 *mcp;
+	int ret;
+
+	mcp = devm_kzalloc(dev, sizeof(*mcp), GFP_KERNEL);
+	if (!mcp)
+		return -ENOMEM;
+
+	switch (type) {
+	case MCP_TYPE_008:
+		mcp->regmap = devm_regmap_init_i2c(client, &mcp23x08_regmap);
+		mcp->reg_shift = 0;
+		mcp->chip.ngpio = 8;
+		mcp->chip.label = "mcp23008";
+		break;
+
+	case MCP_TYPE_017:
+		mcp->regmap = devm_regmap_init_i2c(client, &mcp23x17_regmap);
+		mcp->reg_shift = 1;
+		mcp->chip.ngpio = 16;
+		mcp->chip.label = "mcp23017";
+		break;
+
+	case MCP_TYPE_018:
+		mcp->regmap = devm_regmap_init_i2c(client, &mcp23x17_regmap);
+		mcp->reg_shift = 1;
+		mcp->chip.ngpio = 16;
+		mcp->chip.label = "mcp23018";
+		break;
+
+	default:
+		dev_err(dev, "invalid device type (%d)\n", type);
+		return -EINVAL;
+	}
+
+	if (IS_ERR(mcp->regmap))
+		return PTR_ERR(mcp->regmap);
+
+	mcp->irq = client->irq;
+	mcp->pinctrl_desc.name = "mcp23xxx-pinctrl";
+
+	ret = mcp23s08_probe_one(mcp, dev, client->addr, type, -1);
+	if (ret)
+		return ret;
+
+	i2c_set_clientdata(client, mcp);
+
+	return 0;
+}
+
+static const struct i2c_device_id mcp230xx_id[] = {
+	{ "mcp23008", MCP_TYPE_008 },
+	{ "mcp23017", MCP_TYPE_017 },
+	{ "mcp23018", MCP_TYPE_018 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mcp230xx_id);
+
+static const struct of_device_id mcp23s08_i2c_of_match[] = {
+	{
+		.compatible = "microchip,mcp23008",
+		.data = (void *) MCP_TYPE_008,
+	},
+	{
+		.compatible = "microchip,mcp23017",
+		.data = (void *) MCP_TYPE_017,
+	},
+	{
+		.compatible = "microchip,mcp23018",
+		.data = (void *) MCP_TYPE_018,
+	},
+/* NOTE: The use of the mcp prefix is deprecated and will be removed. */
+	{
+		.compatible = "mcp,mcp23008",
+		.data = (void *) MCP_TYPE_008,
+	},
+	{
+		.compatible = "mcp,mcp23017",
+		.data = (void *) MCP_TYPE_017,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mcp23s08_i2c_of_match);
+
+static struct i2c_driver mcp230xx_driver = {
+	.driver = {
+		.name	= "mcp230xx",
+		.of_match_table = mcp23s08_i2c_of_match,
+	},
+	.probe		= mcp230xx_probe,
+	.id_table	= mcp230xx_id,
+};
+
+static int __init mcp23s08_i2c_init(void)
+{
+	return i2c_add_driver(&mcp230xx_driver);
+}
+
+/*
+ * Register after I²C postcore initcall and before
+ * subsys initcalls that may rely on these GPIOs.
+ */
+subsys_initcall(mcp23s08_i2c_init);
+
+static void mcp23s08_i2c_exit(void)
+{
+	i2c_del_driver(&mcp230xx_driver);
+}
+module_exit(mcp23s08_i2c_exit);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/pinctrl/pinctrl-mcp23s08_spi.c b/drivers/pinctrl/pinctrl-mcp23s08_spi.c
new file mode 100644
index 000000000000..e06fb885fd2b
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-mcp23s08_spi.c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* MCP23S08 SPI GPIO driver */
+
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+#include "pinctrl-mcp23s08.h"
+
+#define MCP_MAX_DEV_PER_CS	8
+
+/*
+ * 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 int mcp23sxx_spi_write(void *context, const void *data, size_t count)
+{
+	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, }, };
+
+	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 mcp23sxx_spi_gather_write(void *context,
+				const void *reg, size_t reg_size,
+				const void *val, size_t val_size)
+{
+	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, }, };
+
+	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 mcp23sxx_spi_read(void *context, const void *reg, size_t reg_size,
+				void *val, size_t val_size)
+{
+	struct mcp23s08 *mcp = context;
+	struct spi_device *spi = to_spi_device(mcp->dev);
+	u8 tx[2];
+
+	if (reg_size != 1)
+		return -EINVAL;
+
+	tx[0] = mcp->addr | 0x01;
+	tx[1] = *((u8 *) reg);
+
+	return spi_write_then_read(spi, tx, sizeof(tx), val, val_size);
+}
+
+static const struct regmap_bus mcp23sxx_spi_regmap = {
+	.write = mcp23sxx_spi_write,
+	.gather_write = mcp23sxx_spi_gather_write,
+	.read = mcp23sxx_spi_read,
+};
+
+static int mcp23s08_spi_regmap_init(struct mcp23s08 *mcp, struct device *dev,
+				    unsigned int addr, unsigned int type)
+{
+	const struct regmap_config *config;
+	struct regmap_config *copy;
+	const char *name;
+
+	switch (type) {
+	case MCP_TYPE_S08:
+		mcp->reg_shift = 0;
+		mcp->chip.ngpio = 8;
+		mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL, "mcp23s08.%d", addr);
+
+		config = &mcp23x08_regmap;
+		name = devm_kasprintf(dev, GFP_KERNEL, "%d", addr);
+		break;
+
+	case MCP_TYPE_S17:
+		mcp->reg_shift = 1;
+		mcp->chip.ngpio = 16;
+		mcp->chip.label = devm_kasprintf(dev, GFP_KERNEL, "mcp23s17.%d", addr);
+
+		config = &mcp23x17_regmap;
+		name = devm_kasprintf(dev, GFP_KERNEL, "%d", addr);
+		break;
+
+	case MCP_TYPE_S18:
+		mcp->reg_shift = 1;
+		mcp->chip.ngpio = 16;
+		mcp->chip.label = "mcp23s18";
+
+		config = &mcp23x17_regmap;
+		name = config->name;
+		break;
+
+	default:
+		dev_err(dev, "invalid device type (%d)\n", type);
+		return -EINVAL;
+	}
+
+	copy = devm_kmemdup(dev, &config, sizeof(config), GFP_KERNEL);
+	if (!copy)
+		return -ENOMEM;
+
+	copy->name = name;
+
+	mcp->regmap = devm_regmap_init(dev, &mcp23sxx_spi_regmap, mcp, copy);
+	if (IS_ERR(mcp->regmap))
+		return PTR_ERR(mcp->regmap);
+
+	return 0;
+}
+
+static int mcp23s08_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct mcp23s08_driver_data *data;
+	unsigned long spi_present_mask;
+	const void *match;
+	unsigned int addr;
+	unsigned int ngpio = 0;
+	int chips;
+	int type;
+	int ret;
+	u32 v;
+
+	match = device_get_match_data(dev);
+	if (match)
+		type = (int)(uintptr_t)match;
+	else
+		type = spi_get_device_id(spi)->driver_data;
+
+	ret = device_property_read_u32(dev, "microchip,spi-present-mask", &v);
+	if (ret) {
+		ret = device_property_read_u32(dev, "mcp,spi-present-mask", &v);
+		if (ret) {
+			dev_err(dev, "missing spi-present-mask");
+			return ret;
+		}
+	}
+	spi_present_mask = v;
+
+	if (!spi_present_mask || spi_present_mask >= BIT(MCP_MAX_DEV_PER_CS)) {
+		dev_err(dev, "invalid spi-present-mask");
+		return -ENODEV;
+	}
+
+	chips = hweight_long(spi_present_mask);
+
+	data = devm_kzalloc(dev, struct_size(data, chip, chips), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, data);
+
+	for_each_set_bit(addr, &spi_present_mask, MCP_MAX_DEV_PER_CS) {
+		data->mcp[addr] = &data->chip[--chips];
+		data->mcp[addr]->irq = spi->irq;
+
+		ret = mcp23s08_spi_regmap_init(data->mcp[addr], dev, addr, type);
+		if (ret)
+			return ret;
+
+		data->mcp[addr]->pinctrl_desc.name = devm_kasprintf(dev, GFP_KERNEL,
+								    "mcp23xxx-pinctrl.%d",
+								    addr);
+		if (!data->mcp[addr]->pinctrl_desc.name)
+			return -ENOMEM;
+
+		ret = mcp23s08_probe_one(data->mcp[addr], dev, 0x40 | (addr << 1), type, -1);
+		if (ret < 0)
+			return ret;
+
+		ngpio += data->mcp[addr]->chip.ngpio;
+	}
+	data->ngpio = ngpio;
+
+	return 0;
+}
+
+static const struct spi_device_id mcp23s08_ids[] = {
+	{ "mcp23s08", MCP_TYPE_S08 },
+	{ "mcp23s17", MCP_TYPE_S17 },
+	{ "mcp23s18", MCP_TYPE_S18 },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, mcp23s08_ids);
+
+static const struct of_device_id mcp23s08_spi_of_match[] = {
+	{
+		.compatible = "microchip,mcp23s08",
+		.data = (void *) MCP_TYPE_S08,
+	},
+	{
+		.compatible = "microchip,mcp23s17",
+		.data = (void *) MCP_TYPE_S17,
+	},
+	{
+		.compatible = "microchip,mcp23s18",
+		.data = (void *) MCP_TYPE_S18,
+	},
+/* NOTE: The use of the mcp prefix is deprecated and will be removed. */
+	{
+		.compatible = "mcp,mcp23s08",
+		.data = (void *) MCP_TYPE_S08,
+	},
+	{
+		.compatible = "mcp,mcp23s17",
+		.data = (void *) MCP_TYPE_S17,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mcp23s08_spi_of_match);
+
+static struct spi_driver mcp23s08_driver = {
+	.probe		= mcp23s08_probe,
+	.id_table	= mcp23s08_ids,
+	.driver = {
+		.name	= "mcp23s08",
+		.of_match_table = mcp23s08_spi_of_match,
+	},
+};
+
+static int __init mcp23s08_spi_init(void)
+{
+	return spi_register_driver(&mcp23s08_driver);
+}
+
+/*
+ * Register after SPI postcore initcall and before
+ * subsys initcalls that may rely on these GPIOs.
+ */
+subsys_initcall(mcp23s08_spi_init);
+
+static void mcp23s08_spi_exit(void)
+{
+	spi_unregister_driver(&mcp23s08_driver);
+}
+module_exit(mcp23s08_spi_exit);
+
+MODULE_LICENSE("GPL");
-- 
2.25.1


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

* Re: [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data
  2020-04-07 17:38 [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data Andy Shevchenko
                   ` (7 preceding siblings ...)
  2020-04-07 17:38 ` [PATCH v1 9/9] pinctrl: mcp23s08: Split to three parts: core, I²C, SPI Andy Shevchenko
@ 2020-04-16 10:35 ` Linus Walleij
  2020-10-09  9:15   ` Martin Hundebøll
  2021-09-16 12:51 ` Florian Eckert
  9 siblings, 1 reply; 23+ messages in thread
From: Linus Walleij @ 2020-04-16 10:35 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: open list:GPIO SUBSYSTEM

On Tue, Apr 7, 2020 at 7:38 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Platform data is a legacy interface to supply device properties
> to the driver. In this case we even don't have in-kernel users
> for it. Just remove it for good.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

All 9 patches applied.

Thanks!
Linus Walleij

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

* Re: Re: [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data
  2020-04-16 10:35 ` [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data Linus Walleij
@ 2020-10-09  9:15   ` Martin Hundebøll
  2020-10-09 14:02     ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Martin Hundebøll @ 2020-10-09  9:15 UTC (permalink / raw)
  To: Linus Walleij, Andy Shevchenko; +Cc: open list:GPIO SUBSYSTEM

Hi Andy,

On 16/04/2020 12.35, Linus Walleij wrote:
> On Tue, Apr 7, 2020 at 7:38 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
>> Platform data is a legacy interface to supply device properties
>> to the driver. In this case we even don't have in-kernel users
>> for it. Just remove it for good.
>>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> All 9 patches applied.

This series caused probing of my PiFace2 to fail:

 > [    1.019604] mcp23s08: probe of spi0.0 failed with error -22

I tried to bisect, but some of the commits failed to compile:

 > drivers/pinctrl/pinctrl-mcp23s08.c:959:39: error: 
'mcp23s08_spi_of_match' undeclared (first use in this function); did you 
mean 'mcp23s08_i2c_of_match'?

 >  959 |  match = of_match_device(of_match_ptr(mcp23s08_spi_of_match), 
&spi->dev);


However, bisecting did narrow it down to a few commits:

 > * 1ac30db20be2 - (refs/bisect/bad) pinctrl: mcp23s08: Make use of 
device properties (6 months ago) <Andy Shevchenko>

 > * 88af89b52a1b - 
(refs/bisect/skip-88af89b52a1b7493f1e5ec165856b4c6767cf654) pinctrl: 
mcp23s08: Propagate error code from device_property_read_u32() (6 months 
ago) <Andy Shevchenko>

 > * 0874758ecb2b - 
(refs/bisect/skip-0874758ecb2b3faa200a86dda45dbc29335f883e) pinctrl: 
mcp23s08: Refactor mcp23s08_spi_regmap_init() (6 months ago) <Andy 
Shevchenko>

 > * 0521701c8d10 - 
(refs/bisect/skip-0521701c8d10f832a401cc7ebfa92bb73782d792) pinctrl: 
mcp23s08: Drop unused parameter in mcp23s08_probe_one() (6 months ago) 
<Andy Shevchenko>

 > * d3da29b628a8 - 
(refs/bisect/skip-d3da29b628a86777d25c741c44b8af35f10020a0) pinctrl: 
mcp23s08: Consolidate SPI and I²C code (6 months ago) <Andy Shevchenko>

 > * 84d02e785d34 - 
(refs/bisect/good-84d02e785d34be9363a825d696cca1f07fac2634) pinctrl: 
mcp23s08: Deduplicate IRQ chip filling (6 months ago) <Andy Shevchenko>


I'm using the rpi3 device tree (broadcom/bcm2837-rpi-3-b.dtb) overlayed 
with a configuration for the mcp23s08:

/dts-v1/;

/plugin/;



/ {

         fragment@0 {

                 target-path = "/soc/spi@7e204000";



                 __overlay__ {

                         pinctrl-names = "default";

                         pinctrl-0 = <&spi0_gpio7>;

                         #address-cells = <1>;

                         #size-cells = <0>;

                         status = "okay";



                         gpio@0 {

                                 compatible = "microchip,mcp23s17";

                                 reg = <0>;

                                 gpio-controller;

                                 #gpio-cells = <2>;

                                 microchip,spi-present-mask = <0x01>;

                                 spi-max-frequency = <1000000>;

                         };

                 };

         };

};

I looked around the code a bit, and tried to compare it with v5.7, but 
didn't see any obvious candidates.

// Martin

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

* Re: Re: [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data
  2020-10-09  9:15   ` Martin Hundebøll
@ 2020-10-09 14:02     ` Andy Shevchenko
  2020-10-09 16:02       ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2020-10-09 14:02 UTC (permalink / raw)
  To: Martin Hundebøll; +Cc: Linus Walleij, open list:GPIO SUBSYSTEM

On Fri, Oct 09, 2020 at 11:15:57AM +0200, Martin Hundebøll wrote:
> On 16/04/2020 12.35, Linus Walleij wrote:
> > On Tue, Apr 7, 2020 at 7:38 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:

...

> This series caused probing of my PiFace2 to fail:

Thanks for report!

> > [    1.019604] mcp23s08: probe of spi0.0 failed with error -22

I suppose you are trying this on v5.9-rc8 with the same result?

This case without any error message and with -EINVAL points me to the only
possible call that may fail, i.e.

	ret = mcp23s08_spi_regmap_init(data->mcp[addr], dev, addr, type);
	if (ret)
		return ret;

Can you confirm this by adding a dev_info() call in between?

	ret = mcp23s08_spi_regmap_init(data->mcp[addr], dev, addr, type);
	if (ret) {
		dev_info(dev, "regmap init failed for %d@%u\n", type, addr);
		return ret;
	}

and share what it prints, please?

EINVAL can be returned either by wrong type (see switch-case inside that
function) or by devm_regmap_init().

> I tried to bisect, but some of the commits failed to compile:
> 
> > drivers/pinctrl/pinctrl-mcp23s08.c:959:39: error: 'mcp23s08_spi_of_match'
> undeclared (first use in this function); did you mean
> 'mcp23s08_i2c_of_match'?
> 
> >  959 |  match = of_match_device(of_match_ptr(mcp23s08_spi_of_match),
> &spi->dev);

At which patch this happens?

I compiled them individually but it might be slipped during rebase in between
of versions of the patch series.

> However, bisecting did narrow it down to a few commits:

You meant that revert of all of them helps?

Can you rather try to revert all patches against this driver until the one
which starts the series and bisect again? (Should be like 4-5 iterations only).

> > * 1ac30db20be2 - (refs/bisect/bad) pinctrl: mcp23s08: Make use of device
> properties (6 months ago) <Andy Shevchenko>
> 
> > * 88af89b52a1b -
> (refs/bisect/skip-88af89b52a1b7493f1e5ec165856b4c6767cf654) pinctrl:
> mcp23s08: Propagate error code from device_property_read_u32() (6 months
> ago) <Andy Shevchenko>
> 
> > * 0874758ecb2b -
> (refs/bisect/skip-0874758ecb2b3faa200a86dda45dbc29335f883e) pinctrl:
> mcp23s08: Refactor mcp23s08_spi_regmap_init() (6 months ago) <Andy
> Shevchenko>
> 
> > * 0521701c8d10 -
> (refs/bisect/skip-0521701c8d10f832a401cc7ebfa92bb73782d792) pinctrl:
> mcp23s08: Drop unused parameter in mcp23s08_probe_one() (6 months ago) <Andy
> Shevchenko>
> 
> > * d3da29b628a8 -
> (refs/bisect/skip-d3da29b628a86777d25c741c44b8af35f10020a0) pinctrl:
> mcp23s08: Consolidate SPI and I²C code (6 months ago) <Andy Shevchenko>
> 
> > * 84d02e785d34 -
> (refs/bisect/good-84d02e785d34be9363a825d696cca1f07fac2634) pinctrl:
> mcp23s08: Deduplicate IRQ chip filling (6 months ago) <Andy Shevchenko>
> 
> I'm using the rpi3 device tree (broadcom/bcm2837-rpi-3-b.dtb) overlayed with
> a configuration for the mcp23s08:

> /dts-v1/;
> /plugin/;
> / {
>         fragment@0 {
>                 target-path = "/soc/spi@7e204000";
>                 __overlay__ {
>                         pinctrl-names = "default";
>                         pinctrl-0 = <&spi0_gpio7>;
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>                         status = "okay";
>                         gpio@0 {
>                                 compatible = "microchip,mcp23s17";
>                                 reg = <0>;
>                                 gpio-controller;
>                                 #gpio-cells = <2>;
>                                 microchip,spi-present-mask = <0x01>;
>                                 spi-max-frequency = <1000000>;
>                         };
>                 };
>         };

Okay, there is no IRQ chip here... But this should be fine I think.

> };

> I looked around the code a bit, and tried to compare it with v5.7, but
> didn't see any obvious candidates.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: Re: [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data
  2020-10-09 14:02     ` Andy Shevchenko
@ 2020-10-09 16:02       ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2020-10-09 16:02 UTC (permalink / raw)
  To: Martin Hundebøll; +Cc: Linus Walleij, open list:GPIO SUBSYSTEM

On Fri, Oct 09, 2020 at 05:02:36PM +0300, Andy Shevchenko wrote:
> On Fri, Oct 09, 2020 at 11:15:57AM +0200, Martin Hundebøll wrote:
> > On 16/04/2020 12.35, Linus Walleij wrote:
> > > On Tue, Apr 7, 2020 at 7:38 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:

> > I tried to bisect, but some of the commits failed to compile:
> > 
> > > drivers/pinctrl/pinctrl-mcp23s08.c:959:39: error: 'mcp23s08_spi_of_match'
> > undeclared (first use in this function); did you mean
> > 'mcp23s08_i2c_of_match'?
> > 
> > >  959 |  match = of_match_device(of_match_ptr(mcp23s08_spi_of_match),
> > &spi->dev);
> 
> At which patch this happens?
> 
> I compiled them individually but it might be slipped during rebase in between
> of versions of the patch series.

JFYI: I have got Linux next and reverted all patches down to the one before my
series you complained about. Then I have compiled and reverted one-by-one.
Here is the log:

$ git reset --hard HEAD~1
HEAD is now at f018c29d2f05 Revert "pinctrl: mcp23s08: Deduplicate IRQ chip filling"
$ git reset --hard HEAD~1
HEAD is now at fd9a21af4720 Revert "pinctrl: mcp23s08: Consolidate SPI and I²C code"
$ git reset --hard HEAD~1
HEAD is now at efd1e6d2cdfe Revert "pinctrl: mcp23s08: Drop unused parameter in mcp23s08_probe_one()"
$ git reset --hard HEAD~1
HEAD is now at c0a36f9d6641 Revert "pinctrl: mcp23s08: Refactor mcp23s08_spi_regmap_init()"
$ git reset --hard HEAD~1
HEAD is now at b7cbbb8df01c Revert "pinctrl: mcp23s08: Propagate error code from device_property_read_u32()"
$ git reset --hard HEAD~1
HEAD is now at 819740665f29 Revert "pinctrl: mcp23s08: Make use of device properties"
$ git reset --hard HEAD~1
HEAD is now at 4a183ba59936 Revert "pinctrl: mcp23s08: Use for_each_set_bit() and hweight_long()"
$ git reset --hard HEAD~1
HEAD is now at 229eb0d949ee Revert "pinctrl: mcp23s08: Split to three parts: core, I²C, SPI"
$ git reset --hard HEAD~1
HEAD is now at 3d5db262b4cc Revert "pinctrl: mcp23s08: add module license"
$ git reset --hard HEAD~1
HEAD is now at 67d3e3edbab4 Revert "pinctrl: mcp23s08: Split to three parts: fix ptr_ret.cocci warnings"
$ git reset --hard HEAD~1
HEAD is now at d916cd9888b3 Revert "pinctrl: mcp23s08: Use irqchip template"
$ git reset --hard HEAD~1
HEAD is now at f0c94c7500ab Revert "pinctrl: mcp23s08: Improve error messaging in ->probe()"
$ git reset --hard HEAD~1
HEAD is now at 23593741ba99 Revert "pinctrl: mcp23s08: Fix mcp23x17_regmap initialiser"
$ git reset --hard HEAD~1
HEAD is now at 2f5dc6077839 Revert "pinctrl: mcp23s08: Fix mcp23x17 precious range"
$ git reset --hard HEAD~1
HEAD is now at ...

Every single patch is compiled perfectly. Are you sure you have clean tree?

Also I spent a bit of time to look again in the what may be possible of
breakage and found nothing alarming. Please, give more details about the issue.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data
  2020-04-07 17:38 [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data Andy Shevchenko
                   ` (8 preceding siblings ...)
  2020-04-16 10:35 ` [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data Linus Walleij
@ 2021-09-16 12:51 ` Florian Eckert
  2021-09-22  6:36   ` Andy Shevchenko
  9 siblings, 1 reply; 23+ messages in thread
From: Florian Eckert @ 2021-09-16 12:51 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Linus Walleij, linux-gpio, linux-gpio-owner

Hello Andy,

I just wanted to port my target an APU3 (x86_64) to kernel version 5.10 
in OpenWrt.
This is the next stable LTS kernel version in OpenWrt.

On my board I have this device as an I2c bus IO-expander with which I 
control additional LEDs.
For this purpose I have a platform driver. Which registers these 
additional LEDs.
I have now discovered that the platform driver no longer compiles due to 
this change, as the platform data for the mcp23017 have been removed by 
this change in kernel version 5.8.

I don't have control over the BIOS (coreboot) and I can't adjust the 
ACPI tables.
Since this is an x86 platform, I can't work with a device tree, can I?

--
Florian

On 2020-04-07 19:38, Andy Shevchenko wrote:
> Platform data is a legacy interface to supply device properties
> to the driver. In this case we even don't have in-kernel users
> for it. Just remove it for good.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/pinctrl/pinctrl-mcp23s08.c | 42 +++++++++---------------------
>  include/linux/spi/mcp23s08.h       | 18 -------------
>  2 files changed, 12 insertions(+), 48 deletions(-)
>  delete mode 100644 include/linux/spi/mcp23s08.h
> 
> diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c
> b/drivers/pinctrl/pinctrl-mcp23s08.c
> index 3a235487e38d..2c8b8c45b70e 100644
> --- a/drivers/pinctrl/pinctrl-mcp23s08.c
> +++ b/drivers/pinctrl/pinctrl-mcp23s08.c
> @@ -8,7 +8,6 @@
>  #include <linux/gpio/driver.h>
>  #include <linux/i2c.h>
>  #include <linux/spi/spi.h>
> -#include <linux/spi/mcp23s08.h>
>  #include <linux/slab.h>
>  #include <asm/byteorder.h>
>  #include <linux/interrupt.h>
> @@ -914,16 +913,9 @@ MODULE_DEVICE_TABLE(of, mcp23s08_i2c_of_match);
>  static int mcp230xx_probe(struct i2c_client *client,
>  				    const struct i2c_device_id *id)
>  {
> -	struct mcp23s08_platform_data *pdata, local_pdata;
>  	struct mcp23s08 *mcp;
>  	int status;
> 
> -	pdata = dev_get_platdata(&client->dev);
> -	if (!pdata) {
> -		pdata = &local_pdata;
> -		pdata->base = -1;
> -	}
> -
>  	mcp = devm_kzalloc(&client->dev, sizeof(*mcp), GFP_KERNEL);
>  	if (!mcp)
>  		return -ENOMEM;
> @@ -937,7 +929,7 @@ static int mcp230xx_probe(struct i2c_client 
> *client,
>  	mcp->irq_chip.irq_bus_sync_unlock = mcp23s08_irq_bus_unlock;
> 
>  	status = mcp23s08_probe_one(mcp, &client->dev, client, client->addr,
> -				    id->driver_data, pdata->base, 0);
> +				    id->driver_data, -1, 0);
>  	if (status)
>  		return status;
> 
> @@ -986,13 +978,13 @@ static void mcp23s08_i2c_exit(void) { }
> 
>  static int mcp23s08_probe(struct spi_device *spi)
>  {
> -	struct mcp23s08_platform_data	*pdata, local_pdata;
>  	unsigned			addr;
>  	int				chips = 0;
>  	struct mcp23s08_driver_data	*data;
>  	int				status, type;
>  	unsigned			ngpio = 0;
>  	const struct			of_device_id *match;
> +	u32				spi_present_mask;
> 
>  	match = of_match_device(of_match_ptr(mcp23s08_spi_of_match), 
> &spi->dev);
>  	if (match)
> @@ -1000,32 +992,24 @@ static int mcp23s08_probe(struct spi_device 
> *spi)
>  	else
>  		type = spi_get_device_id(spi)->driver_data;
> 
> -	pdata = dev_get_platdata(&spi->dev);
> -	if (!pdata) {
> -		pdata = &local_pdata;
> -		pdata->base = -1;
> -
> +	status = device_property_read_u32(&spi->dev,
> +			"microchip,spi-present-mask", &spi_present_mask);
> +	if (status) {
>  		status = device_property_read_u32(&spi->dev,
> -			"microchip,spi-present-mask", &pdata->spi_present_mask);
> +				"mcp,spi-present-mask", &spi_present_mask);
>  		if (status) {
> -			status = device_property_read_u32(&spi->dev,
> -				"mcp,spi-present-mask",
> -				&pdata->spi_present_mask);
> -
> -			if (status) {
> -				dev_err(&spi->dev, "missing spi-present-mask");
> -				return -ENODEV;
> -			}
> +			dev_err(&spi->dev, "missing spi-present-mask");
> +			return -ENODEV;
>  		}
>  	}
> 
> -	if (!pdata->spi_present_mask || pdata->spi_present_mask > 0xff) {
> +	if (!spi_present_mask || spi_present_mask > 0xff) {
>  		dev_err(&spi->dev, "invalid spi-present-mask");
>  		return -ENODEV;
>  	}
> 
>  	for (addr = 0; addr < MCP_MAX_DEV_PER_CS; addr++) {
> -		if (pdata->spi_present_mask & BIT(addr))
> +		if (spi_present_mask & BIT(addr))
>  			chips++;
>  	}
> 
> @@ -1040,7 +1024,7 @@ static int mcp23s08_probe(struct spi_device *spi)
>  	spi_set_drvdata(spi, data);
> 
>  	for (addr = 0; addr < MCP_MAX_DEV_PER_CS; addr++) {
> -		if (!(pdata->spi_present_mask & BIT(addr)))
> +		if (!(spi_present_mask & BIT(addr)))
>  			continue;
>  		chips--;
>  		data->mcp[addr] = &data->chip[chips];
> @@ -1054,12 +1038,10 @@ static int mcp23s08_probe(struct spi_device 
> *spi)
>  			mcp23s08_irq_bus_unlock;
>  		status = mcp23s08_probe_one(data->mcp[addr], &spi->dev, spi,
>  					    0x40 | (addr << 1), type,
> -					    pdata->base, addr);
> +					    -1, addr);
>  		if (status < 0)
>  			return status;
> 
> -		if (pdata->base != -1)
> -			pdata->base += data->mcp[addr]->chip.ngpio;
>  		ngpio += data->mcp[addr]->chip.ngpio;
>  	}
>  	data->ngpio = ngpio;
> diff --git a/include/linux/spi/mcp23s08.h 
> b/include/linux/spi/mcp23s08.h
> deleted file mode 100644
> index 738a45b435f2..000000000000
> --- a/include/linux/spi/mcp23s08.h
> +++ /dev/null
> @@ -1,18 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -struct mcp23s08_platform_data {
> -	/* For mcp23s08, up to 4 slaves (numbered 0..3) can share one SPI
> -	 * chipselect, each providing 1 gpio_chip instance with 8 gpios.
> -	 * For mpc23s17, up to 8 slaves (numbered 0..7) can share one SPI
> -	 * chipselect, each providing 1 gpio_chip (port A + port B) with
> -	 * 16 gpios.
> -	 */
> -	u32 spi_present_mask;
> -
> -	/* "base" is the number of the first GPIO or -1 for dynamic
> -	 * assignment. If there are gaps in chip addressing the GPIO
> -	 * numbers are sequential .. so for example if only slaves 0
> -	 * and 3 are present, their GPIOs range from base to base+15
> -	 * (or base+31 for s17 variant).
> -	 */
> -	unsigned	base;
> -};

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

* Re: [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data
  2021-09-16 12:51 ` Florian Eckert
@ 2021-09-22  6:36   ` Andy Shevchenko
  2021-09-22  7:53     ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2021-09-22  6:36 UTC (permalink / raw)
  To: Florian Eckert; +Cc: Linus Walleij, linux-gpio, linux-gpio-owner

On Thu, Sep 16, 2021 at 02:51:23PM +0200, Florian Eckert wrote:
> Hello Andy,
> 
> I just wanted to port my target an APU3 (x86_64) to kernel version 5.10 in
> OpenWrt.
> This is the next stable LTS kernel version in OpenWrt.
> 
> On my board I have this device as an I2c bus IO-expander with which I
> control additional LEDs.
> For this purpose I have a platform driver. Which registers these additional
> LEDs.
> I have now discovered that the platform driver no longer compiles due to
> this change, as the platform data for the mcp23017 have been removed by this
> change in kernel version 5.8.
> 
> I don't have control over the BIOS (coreboot)

It's open source.

> and I can't adjust the ACPI
> tables.

Actually you can (to some extend) even for closed sourced firmwares (either
with rewriting DSDT as Hackintosh project does, or via SSDT overlays, I gave a
talk on Linaro Connect about this).

> Since this is an x86 platform, I can't work with a device tree, can I?

Ideally you need to fix the ACPI tables, but this won't fix older firmwares
anyway. As a workaround you may convert platform data to use software nodes.

See the example (in v5.15-rc1):

	drivers/mfd/intel_quark_i2c_gpio.c
	drivers/gpio/gpio-dwapb.c
	drivers/i2c/busses/i2c-designware-platdrv.c

of such conversion.

> On 2020-04-07 19:38, Andy Shevchenko wrote:
> > Platform data is a legacy interface to supply device properties
> > to the driver. In this case we even don't have in-kernel users
> > for it. Just remove it for good.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data
  2021-09-22  6:36   ` Andy Shevchenko
@ 2021-09-22  7:53     ` Andy Shevchenko
  2021-09-23 14:17       ` Add a SSDT ACPI description to recognize my I2C device connected via SMBus Florian Eckert
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2021-09-22  7:53 UTC (permalink / raw)
  To: Florian Eckert; +Cc: Linus Walleij, linux-gpio, linux-gpio-owner

On Wed, Sep 22, 2021 at 09:36:21AM +0300, Andy Shevchenko wrote:
> On Thu, Sep 16, 2021 at 02:51:23PM +0200, Florian Eckert wrote:
> > Hello Andy,
> > 
> > I just wanted to port my target an APU3 (x86_64) to kernel version 5.10 in
> > OpenWrt.
> > This is the next stable LTS kernel version in OpenWrt.
> > 
> > On my board I have this device as an I2c bus IO-expander with which I
> > control additional LEDs.
> > For this purpose I have a platform driver. Which registers these additional
> > LEDs.
> > I have now discovered that the platform driver no longer compiles due to
> > this change, as the platform data for the mcp23017 have been removed by this
> > change in kernel version 5.8.

Ah, now I got it. You want to instantiate the I2C expander itself, so as I
mentioned below it can be easily done via SSDT overlays:

https://connect.linaro.org/resources/lvc21f/lvc21f-304/
https://www.youtube.com/watch?v=nlKjUAv3RL0&ab_channel=OSDNConf

https://stackoverflow.com/questions/65727454/
https://stackoverflow.com/questions/60105101/
https://stackoverflow.com/questions/54768841/
https://stackoverflow.com/questions/46095840/

https://github.com/westeri/meta-acpi/tree/master/recipes-bsp/acpi-tables/samples/

> > I don't have control over the BIOS (coreboot)
> 
> It's open source.
> 
> > and I can't adjust the ACPI
> > tables.
> 
> Actually you can (to some extend) even for closed sourced firmwares (either
> with rewriting DSDT as Hackintosh project does, or via SSDT overlays, I gave a
> talk on Linaro Connect about this).
> 
> > Since this is an x86 platform, I can't work with a device tree, can I?
> 
> Ideally you need to fix the ACPI tables, but this won't fix older firmwares
> anyway. As a workaround you may convert platform data to use software nodes.
> 
> See the example (in v5.15-rc1):
> 
> 	drivers/mfd/intel_quark_i2c_gpio.c
> 	drivers/gpio/gpio-dwapb.c
> 	drivers/i2c/busses/i2c-designware-platdrv.c
> 
> of such conversion.
> 
> > On 2020-04-07 19:38, Andy Shevchenko wrote:
> > > Platform data is a legacy interface to supply device properties
> > > to the driver. In this case we even don't have in-kernel users
> > > for it. Just remove it for good.

-- 
With Best Regards,
Andy Shevchenko



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

* Add a SSDT ACPI description to recognize my I2C device connected via SMBus
  2021-09-22  7:53     ` Andy Shevchenko
@ 2021-09-23 14:17       ` Florian Eckert
  2021-09-23 16:24         ` Mika Westerberg
  2021-09-23 20:26         ` Andy Shevchenko
  0 siblings, 2 replies; 23+ messages in thread
From: Florian Eckert @ 2021-09-23 14:17 UTC (permalink / raw)
  To: Andy Shevchenko, Enrico Weigelt, Mika Westerberg, Jean Delvare,
	Florian.Eckert
  Cc: linux-gpio, linux-acpi, linux-i2c

I am working wit OpenWrt which has recently switched the kernel version
from 5.4 to 5.10 on x86 Target [1] in its master branch.

I am using a APU3 board from PC-Engine [2].

The APU3 board has an SMBus [3] device (Intel PIIX4 and compatible
(ATI/AMD/Serverworks/Broadcom/SMSC) to communicate with additional 
connected I2C devices.

On This SMBus there is a IO expander from microchip connect [4] via the 
SMBus (i2c).
I used this microchip IO expander to control additional LEDs, as the 
APU3 only has 3.

So far, everything has worked fine, because I had wirten a platform 
device for this.
Everything was recognized and compiled cleanly and I could control the 
LEDs from the user-land.

Due to the following change [5] between 5.4 and 5.10 by removing the 
platform data support in
the IO expander mcp23s08, my plaform device does not compile anymore,
I can no longer use the platform device pattern for this kind of device.

The only possibility I can think of now is to make this device known
to the kernel via a dynamic ACPI SSDT table. I have already tried 
various
things but I can't get the driver [4] to feel responsible for this 
device.

I have used the following links that were provided by "Andy Shevchenko" 
to me
to understand the concept begind ACPI SSDT handling. Thanks for that.

https://connect.linaro.org/resources/lvc21f/lvc21f-304/
https://www.youtube.com/watch?v=nlKjUAv3RL0&ab_channel=OSDNConf
https://stackoverflow.com/questions/65727454/
https://stackoverflow.com/questions/60105101/
https://stackoverflow.com/questions/54768841/
https://stackoverflow.com/questions/46095840/
https://github.com/westeri/meta-acpi/tree/master/recipes-bsp/acpi-tables/samples/

This is my aml file that I tried with. It loads but nothing happens.

DefinitionBlock ("mcp23s08.aml", "SSDT", 5, "", "IO", 2)
{
     External (\_SB.PCI0.SBUS, DeviceObj)

     Device (\_SB.PCI0.SBUS.BUS0)
     {
         Name (_CID, "smbus")
         NAME (_ADR, Zero)
         Device (PIN)
         {
             Name (_HID, "PRP0001")
             Name (_DDN, "io expander")
             Name (_CRS, ResourceTemplate () {
                 I2cSerialBus (
                     0x24,                   // Bus address
                     ControllerInitiated,    // Don't care
                     400000,                 // Fast mode (400 kHz)
                     AddressingMode7Bit,     // 7-bit addressing
                     "\\_SB.PCI0.SBUS.BUS0", // I2C host controller
                     0                       // Must be 0
                 )
             })

             Name (_DSD, Package () {
                 ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
                 Package () {
                     Package () { "compatible", Package () { 
"microchip,mcp23017" } },
                 }
             })
         }
     }
}

In Coreboot the SMBus named SBUS and is on address 0x0014000 [7].

But I'm not sure if that's right at all.
Somehow I don't understand how the io expander is connected to SMBus.
According to my research, however, it should fit.

The SMBus device driver i2c-piix4 creates 3 I2C devices:
ls -la /sys/bus/i2c/devices/
../../../devices/pci0000:00/0000:00:14.0/i2c-0 (SMBus PIIX4 adapter port 
0 at 0b00)
../../../devices/pci0000:00/0000:00:14.0/i2c-1 (SMBus PIIX4 adapter port 
2 at 0b00)
../../../devices/pci0000:00/0000:00:14.0/i2c-2 (SMBus PIIX4 adapter port 
1 at 0b20)


The mcp23s08 is connected to the i2c-0 with address 0x24

Therefore I believe the following applies

+------+    +------+
| PCI0 |--->| SMB0 |--> i2c client A (0x24)
|      |    |      |
+------+    +------+


I have enabled the following kernel config parameters for ACPI SSDT:
CONFIG_ACPI_CUSTOM_METHOD
CONFIG_CONFIGFS_FS
CONFIG_ACPI_CONFIGFS
CONFIG_ACPI_DEBUG

The goal would be that the ACPI mapping for the i2c-pii4 and the 
connected pinctrl-mcp23s08 exactly works as
show in the video [5] from Andy Shevchenko.

I think others will have the same problem in the future when they update 
the kernel on an X86 embedded device
which does not support device trees and also now no platform data 
handling.


- Florian

[1] 
https://git.openwrt.org/?p=openwrt/openwrt.git;a=commit;h=64be0eadc17988f29d0a4b89569840d917746498
[2] https://pcengines.ch/apu.htm
[3] 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/i2c/busses/i2c-piix4.c?h=v5.10.68
[4] 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/pinctrl/pinctrl-max77620.c?h=v5.10.68
[5] 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/pinctrl/pinctrl-mcp23s08.c?h=v5.10.68&id=6aba6ed879b3471903c8ada28ba968a268df6143
[6] https://www.youtube.com/watch?v=nlKjUAv3RL0&ab_channel=OSDNConf
[7] 
https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/src/southbridge/amd/pi/hudson/acpi/fch.asl#29

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

* Re: Add a SSDT ACPI description to recognize my I2C device connected via SMBus
  2021-09-23 14:17       ` Add a SSDT ACPI description to recognize my I2C device connected via SMBus Florian Eckert
@ 2021-09-23 16:24         ` Mika Westerberg
  2021-09-23 20:26         ` Andy Shevchenko
  1 sibling, 0 replies; 23+ messages in thread
From: Mika Westerberg @ 2021-09-23 16:24 UTC (permalink / raw)
  To: Florian Eckert
  Cc: Andy Shevchenko, Enrico Weigelt, Jean Delvare, Florian.Eckert,
	linux-gpio, linux-acpi, linux-i2c

Hi,

On Thu, Sep 23, 2021 at 04:17:13PM +0200, Florian Eckert wrote:
> I am working wit OpenWrt which has recently switched the kernel version
> from 5.4 to 5.10 on x86 Target [1] in its master branch.
> 
> I am using a APU3 board from PC-Engine [2].
> 
> The APU3 board has an SMBus [3] device (Intel PIIX4 and compatible
> (ATI/AMD/Serverworks/Broadcom/SMSC) to communicate with additional connected
> I2C devices.
> 
> On This SMBus there is a IO expander from microchip connect [4] via the
> SMBus (i2c).
> I used this microchip IO expander to control additional LEDs, as the APU3
> only has 3.
> 
> So far, everything has worked fine, because I had wirten a platform device
> for this.
> Everything was recognized and compiled cleanly and I could control the LEDs
> from the user-land.
> 
> Due to the following change [5] between 5.4 and 5.10 by removing the
> platform data support in
> the IO expander mcp23s08, my plaform device does not compile anymore,
> I can no longer use the platform device pattern for this kind of device.
> 
> The only possibility I can think of now is to make this device known
> to the kernel via a dynamic ACPI SSDT table. I have already tried various
> things but I can't get the driver [4] to feel responsible for this device.
> 
> I have used the following links that were provided by "Andy Shevchenko" to
> me
> to understand the concept begind ACPI SSDT handling. Thanks for that.
> 
> https://connect.linaro.org/resources/lvc21f/lvc21f-304/
> https://www.youtube.com/watch?v=nlKjUAv3RL0&ab_channel=OSDNConf
> https://stackoverflow.com/questions/65727454/
> https://stackoverflow.com/questions/60105101/
> https://stackoverflow.com/questions/54768841/
> https://stackoverflow.com/questions/46095840/
> https://github.com/westeri/meta-acpi/tree/master/recipes-bsp/acpi-tables/samples/
> 
> This is my aml file that I tried with. It loads but nothing happens.
> 
> DefinitionBlock ("mcp23s08.aml", "SSDT", 5, "", "IO", 2)
> {
>     External (\_SB.PCI0.SBUS, DeviceObj)
> 
>     Device (\_SB.PCI0.SBUS.BUS0)
>     {
>         Name (_CID, "smbus")
>         NAME (_ADR, Zero)
>         Device (PIN)
>         {
>             Name (_HID, "PRP0001")
>             Name (_DDN, "io expander")
>             Name (_CRS, ResourceTemplate () {
>                 I2cSerialBus (
>                     0x24,                   // Bus address
>                     ControllerInitiated,    // Don't care
>                     400000,                 // Fast mode (400 kHz)
>                     AddressingMode7Bit,     // 7-bit addressing
>                     "\\_SB.PCI0.SBUS.BUS0", // I2C host controller
>                     0                       // Must be 0
>                 )
>             })
> 
>             Name (_DSD, Package () {
>                 ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>                 Package () {
>                     Package () { "compatible", Package () {
> "microchip,mcp23017" } },
>                 }
>             })
>         }
>     }
> }
> 
> In Coreboot the SMBus named SBUS and is on address 0x0014000 [7].
> 
> But I'm not sure if that's right at all.
> Somehow I don't understand how the io expander is connected to SMBus.
> According to my research, however, it should fit.
> 
> The SMBus device driver i2c-piix4 creates 3 I2C devices:
> ls -la /sys/bus/i2c/devices/
> ../../../devices/pci0000:00/0000:00:14.0/i2c-0 (SMBus PIIX4 adapter port 0
> at 0b00)
> ../../../devices/pci0000:00/0000:00:14.0/i2c-1 (SMBus PIIX4 adapter port 2
> at 0b00)
> ../../../devices/pci0000:00/0000:00:14.0/i2c-2 (SMBus PIIX4 adapter port 1
> at 0b20)
> 
> 
> The mcp23s08 is connected to the i2c-0 with address 0x24
> 
> Therefore I believe the following applies
> 
> +------+    +------+
> | PCI0 |--->| SMB0 |--> i2c client A (0x24)
> |      |    |      |
> +------+    +------+
> 
> 
> I have enabled the following kernel config parameters for ACPI SSDT:
> CONFIG_ACPI_CUSTOM_METHOD
> CONFIG_CONFIGFS_FS
> CONFIG_ACPI_CONFIGFS
> CONFIG_ACPI_DEBUG

How do you load the SSDT? Via initrd or something else? You may need to
enable CONFIG_ACPI_TABLE_UPGRADE too. Also do you see in dmesg that the
SSDT is loaded?

Then I suggest checking the below attributes:

 ../../../devices/pci0000:00/0000:00:14.0/firmware_node/path
 ../../../devices/pci0000:00/0000:00:14.0/i2c-0/firmware_node/path

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

* Re: Add a SSDT ACPI description to recognize my I2C device connected via SMBus
  2021-09-23 14:17       ` Add a SSDT ACPI description to recognize my I2C device connected via SMBus Florian Eckert
  2021-09-23 16:24         ` Mika Westerberg
@ 2021-09-23 20:26         ` Andy Shevchenko
  2021-09-23 20:29           ` Andy Shevchenko
  2021-09-29 22:40           ` Florian Eckert
  1 sibling, 2 replies; 23+ messages in thread
From: Andy Shevchenko @ 2021-09-23 20:26 UTC (permalink / raw)
  To: Florian Eckert
  Cc: Andy Shevchenko, Enrico Weigelt, Mika Westerberg, Jean Delvare,
	Florian.Eckert, linux-gpio, linux-acpi, linux-i2c

On Thu, Sep 23, 2021 at 5:26 PM Florian Eckert <fe@dev.tdt.de> wrote:

...

> This is my aml file that I tried with. It loads but nothing happens.

It's ASL, have you compiled it?

> DefinitionBlock ("mcp23s08.aml", "SSDT", 5, "", "IO", 2)
> {
>      External (\_SB.PCI0.SBUS, DeviceObj)
>
>      Device (\_SB.PCI0.SBUS.BUS0)
>      {

>          Name (_CID, "smbus")
>          NAME (_ADR, Zero)

This seems not right.

First of all, using _ADR along with _HID or _CID is against ACPI
specification. Second, the _CID value is against specification. (AR:
Please, drop _CID)

Third, what does _ADR == 0 mean? In the ACPI the _ADR == 0 for the PCI
device is only allowed for the PCI Host Bridge. What you need to put
here is the address of the PCI device, but it looks like you added the
BUS0 device which is not needed at all and ACPI tables already provide
it. Share (decompiled) DSDT of the device in question somewhere and we
can check this. (Okay, nevermind, I found something, see below)

>          Device (PIN)
>          {
>              Name (_HID, "PRP0001")
>              Name (_DDN, "io expander")
>              Name (_CRS, ResourceTemplate () {
>                  I2cSerialBus (
>                      0x24,                   // Bus address

Bus?! It's a slave address, i.e. your MCP chip address.

>                      ControllerInitiated,    // Don't care
>                      400000,                 // Fast mode (400 kHz)
>                      AddressingMode7Bit,     // 7-bit addressing
>                      "\\_SB.PCI0.SBUS.BUS0", // I2C host controller

Should be double checked, see above. Otherwise it seems good.

>                      0                       // Must be 0
>                  )
>              })
>
>              Name (_DSD, Package () {
>                  ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>                  Package () {
>                      Package () { "compatible", Package () {
> "microchip,mcp23017" } },

Have you read https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/pinctrl/pinctrl-mcp23s08.txt
just in case?

(I think compatible should be enough, but who knows)

>                  }
>              })
>          }
>      }
> }
>
> In Coreboot the SMBus named SBUS and is on address 0x0014000 [7].

Exactly. It means 00:14.0 in BDF notation.

> But I'm not sure if that's right at all.
> Somehow I don't understand how the io expander is connected to SMBus.
> According to my research, however, it should fit.
>
> The SMBus device driver i2c-piix4 creates 3 I2C devices:
> ls -la /sys/bus/i2c/devices/
> ../../../devices/pci0000:00/0000:00:14.0/i2c-0 (SMBus PIIX4 adapter port
> 0 at 0b00)
> ../../../devices/pci0000:00/0000:00:14.0/i2c-1 (SMBus PIIX4 adapter port
> 2 at 0b00)

Same I/O for two different ports?!

> ../../../devices/pci0000:00/0000:00:14.0/i2c-2 (SMBus PIIX4 adapter port
> 1 at 0b20)

Ah, it looks like a multifunctional device. In that case you have to
be sure the driver of the I2C controller is ready for the ACPI
enumeration (seems not). Basically you may use _ADR == 0, 1, ... for
children, but you need to document this and agree with AMD on the use.

Okay, it seems it has this:
  https://elixir.bootlin.com/linux/v5.15-rc2/source/drivers/i2c/i2c-mux.c#L396
which should populate a firmware node to a certain child.

> The mcp23s08 is connected to the i2c-0 with address 0x24

The mcp23s08 can not be connected to I2C. It's a SPI device.
Which chip do you actually have? I believe it's MCP23017 or MCP23018,
which is I2C.


Summary:
1) _CID notation is wrong in ASL;
2) driver seems supports the _ADR schema which you have used in ASL;
3) something fishy about I/O addresses in the sysfs (is it a typo when
you composed the email?);
4) it's unclear what you did with ASL to get it loaded;
5) as Mika suggested, have you checked the kernel configuration?

Otherwise I can't see anything else suspicious here.

-- 
With Best Regards,
Andy Shevchenko

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

* Re: Add a SSDT ACPI description to recognize my I2C device connected via SMBus
  2021-09-23 20:26         ` Andy Shevchenko
@ 2021-09-23 20:29           ` Andy Shevchenko
  2021-09-29 22:40           ` Florian Eckert
  1 sibling, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2021-09-23 20:29 UTC (permalink / raw)
  To: Florian Eckert
  Cc: Andy Shevchenko, Enrico Weigelt, Mika Westerberg, Jean Delvare,
	Florian.Eckert, linux-gpio, linux-acpi, linux-i2c

On Thu, Sep 23, 2021 at 11:26 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Sep 23, 2021 at 5:26 PM Florian Eckert <fe@dev.tdt.de> wrote:

...

> Summary:
> 1) _CID notation is wrong in ASL;
> 2) driver seems supports the _ADR schema which you have used in ASL;
> 3) something fishy about I/O addresses in the sysfs (is it a typo when
> you composed the email?);
> 4) it's unclear what you did with ASL to get it loaded;
> 5) as Mika suggested, have you checked the kernel configuration?
>
> Otherwise I can't see anything else suspicious here.

One more thing, I have no idea how max77620 is related to all this
(you mentioned it in [4]).


-- 
With Best Regards,
Andy Shevchenko

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

* Re: Add a SSDT ACPI description to recognize my I2C device connected via SMBus
  2021-09-23 20:26         ` Andy Shevchenko
  2021-09-23 20:29           ` Andy Shevchenko
@ 2021-09-29 22:40           ` Florian Eckert
  2021-09-30  6:15             ` Andy Shevchenko
  1 sibling, 1 reply; 23+ messages in thread
From: Florian Eckert @ 2021-09-29 22:40 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Enrico Weigelt, Mika Westerberg, Jean Delvare,
	Wolfram Sang, Eckert.Florian, linux-gpio, linux-acpi, linux-i2c


> The mcp23s08 can not be connected to I2C. It's a SPI device.
> Which chip do you actually have? I believe it's MCP23017 or MCP23018,
> which is I2C.

Yes the device is a mcp23017, but the driver is the pinctrl_mcp23s08 [1]

root@dev ~ # lsmod | grep mcp
pinctrl_mcp23s08       16384  0
regmap_core            45056  1 pinctrl_mcp23s08,[permanent]

> Summary:
> 1) _CID notation is wrong in ASL;

I got it

> 2) driver seems supports the _ADR schema which you have used in ASL;

This refers to the i2c-0, doesn't it?
My mcp23s08 device is located at the i2c-0 on address 0x24.

> 3) something fishy about I/O addresses in the sysfs (is it a typo when
> you composed the email?);

No

I have asked myself the same question.
Something is not right.
There was a change regarding the Hudson2 in the driver, maybe something 
went wrong [2]?

> 4) it's unclear what you did with ASL to get it loaded;

On my development device I did a `iasl dsl/mcp23017.dsl`
Of the following dsl

$ cat dsl/mcp23017.dsl
DefinitionBlock ("mcp23017.aml", "SSDT", 5, "", "MCP23017", 4)
{
     External (\_SB.PCI0.SBUS, DeviceObj)

     Scope (\_SB.PCI0.SBUS)
     {
         Device (GPIO)
         {
             Name (_HID, "PRP0001")
             Name (_DDN, "MCP23017 gpio expander")
             Name (_ADR, Zero)
             Name (_CRS, ResourceTemplate () {
                 I2cSerialBus (
                     0x24,                   // Bus address
                     ControllerInitiated,    // Don't care
                     400000,                 // Fast mode (400 kHz)
                     AddressingMode7Bit,     // 7-bit addressing
                     "\\_SB.PCI0.SBUS",      // I2C host controller
                     0                       // Must be 0
                 )
             })

             Name (_DSD, Package () {
                 ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
                 Package () {
                     Package () { "compatible", Package () { 
"microchip,mcp23017" } },
                 }
             })
         }
     }
}

After that I copied this to my APU3 Target and executed
  the following commands:
mkdir /sys/kernel/config/acpi/table/mcp23017

cat mcp23s08.aml > /sys/kernel/config/table/mcp23017

> 5) as Mika suggested, have you checked the kernel configuration?

I have now switched on the suggested option
CONFIG_ACPI_CUSTOM_METHOD=y
CONFIG_ACPI_TABLE_UPGRADE=y
CONFIG_CONFIGFS_FS=y
CONFIG_ACPI_CONFIGFS=y
CONFIG_ACPI_DEBUG=y

But this did not solved my issue loading ssdt during runtime.

The output of the aml on the target:

cat /sys/kernel/config/acpi/table/mcp23017/aml
SSDTsMCP23017INTL \/_SB_PCI0SBUSK
                                  \/_SB_PCI0SBUS[H
MCP23017 gpio expande_AD_CRS&
#$\_SB.PCI0.SBUS_DSD?
microchip,mcp23017


My iasl version:

iasl --version
Illegal option: --

Intel ACPI Component Architecture
ASL+ Optimizing Compiler/Disassembler version 20181213
Copyright (c) 2000 - 2018 Intel Corporation


What else can I do?
The initrd option does not work with OpenWrt.
Maybe I can get further if you can give me a good HEX for debug_level 
and the debug_layer?

---
Regards Florian

[1] 
https://github.com/torvalds/linux/blob/master/drivers/pinctrl/pinctrl-mcp23s08_i2c.c
[2] 
https://github.com/torvalds/linux/commit/528d53a1592b0e27c423f7cafc1df85f77fc1163#diff-aa95f6311d1fcc4d85955b153a2510e853807546ac8e0d3aa0310ac30d236147

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

* Re: Add a SSDT ACPI description to recognize my I2C device connected via SMBus
  2021-09-29 22:40           ` Florian Eckert
@ 2021-09-30  6:15             ` Andy Shevchenko
  2021-09-30  6:19               ` Andy Shevchenko
  0 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2021-09-30  6:15 UTC (permalink / raw)
  To: Florian Eckert
  Cc: Andy Shevchenko, Enrico Weigelt, Mika Westerberg, Jean Delvare,
	Wolfram Sang, Eckert.Florian, linux-gpio, linux-acpi, linux-i2c

On Thu, Sep 30, 2021 at 1:40 AM Florian Eckert <fe@dev.tdt.de> wrote:

...

> > 2) driver seems supports the _ADR schema which you have used in ASL;
>
> This refers to the i2c-0, doesn't it?

Quite likely, but no guarantees. The number of the controller is
issued in order of enumeration which may be not deterministic. What is
real _ADR mapping here is that:

0 == physical port 0
1 == physical port 1
...
n == physical port n

> My mcp23s08 device is located at the i2c-0 on address 0x24.

...

> > 4) it's unclear what you did with ASL to get it loaded;
>
> On my development device I did a `iasl dsl/mcp23017.dsl`
> Of the following dsl
>
> $ cat dsl/mcp23017.dsl
> DefinitionBlock ("mcp23017.aml", "SSDT", 5, "", "MCP23017", 4)
> {
>      External (\_SB.PCI0.SBUS, DeviceObj)
>
>      Scope (\_SB.PCI0.SBUS)
>      {

According to the 2) above and as we learnt from the actual code this
missed one level of the devices, i.e. I2C port

Device (I2C0)
{
        Name (_ADR, Zero)

>          Device (GPIO)
>          {

>              Name (_HID, "PRP0001")
>              Name (_ADR, Zero)

This is against specification as I told you already. _CID/_HID may not
be together with _ADR.

>              Name (_CRS, ResourceTemplate () {
>                  I2cSerialBus (
>                      0x24,                   // Bus address
>                      ControllerInitiated,    // Don't care
>                      400000,                 // Fast mode (400 kHz)
>                      AddressingMode7Bit,     // 7-bit addressing
>                      "\\_SB.PCI0.SBUS",      // I2C host controller
>                      0                       // Must be 0
>                  )
>              })
>
>              Name (_DSD, Package () {
>                  ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>                  Package () {
>                      Package () { "compatible", Package () {
> "microchip,mcp23017" } },
>                  }
>              })
>          }
>      }
> }
>
> After that I copied this to my APU3 Target and executed
>   the following commands:
> mkdir /sys/kernel/config/acpi/table/mcp23017
>
> cat mcp23s08.aml > /sys/kernel/config/table/mcp23017

Yeah, but your I2C controller already been enumerated, so this won't
have any effect due to half-baked ACPI table in the firmware
(coreboot) which should be

Device (SBUS)
{
    Name (_ADR, 0x00140000)
    Device (PORT0)
    {
       Name (_ADR, Zero)
    }
    ...
    Device(PORTn)
    {
        Name (_ADR, n)
    }
}

...

> > 5) as Mika suggested, have you checked the kernel configuration?
>
> I have now switched on the suggested option
> CONFIG_ACPI_CUSTOM_METHOD=y
> CONFIG_ACPI_TABLE_UPGRADE=y
> CONFIG_CONFIGFS_FS=y
> CONFIG_ACPI_CONFIGFS=y
> CONFIG_ACPI_DEBUG=y
>
> But this did not solved my issue loading ssdt during runtime.

It won't as explained in 4) above.

> The output of the aml on the target:
>
> cat /sys/kernel/config/acpi/table/mcp23017/aml

Don't use `cat` against binary files. But we don't really need that.
It's the same as after iasl.

...

> My iasl version:
>
> iasl --version
> Illegal option: --
>
> Intel ACPI Component Architecture
> ASL+ Optimizing Compiler/Disassembler version 20181213

Please, update to something newer.

> Copyright (c) 2000 - 2018 Intel Corporation

...

Current summary:
 1) you still have issues with ASL
 2) there is boot ordering issues with the controller itself

> What else can I do?

So, the course of actions as I see is:
- fix the asl
- fix the coreboot to represent ports of the I2C controller
- switch to use initramfs method (or EFI)
- if possible, to unbind and bind back the i2c controller after AML is
loaded via configfs (it may be not, if it's occupied by some
peripheral devices / drivers that are crucial to system to work)

> The initrd option does not work with OpenWrt.

Why not? If indeed so (but I don't believe in this), OpenWRT has a bug
/ missed feature, which must be fixed there.

-- 
With Best Regards,
Andy Shevchenko

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

* Re: Add a SSDT ACPI description to recognize my I2C device connected via SMBus
  2021-09-30  6:15             ` Andy Shevchenko
@ 2021-09-30  6:19               ` Andy Shevchenko
  0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2021-09-30  6:19 UTC (permalink / raw)
  To: Florian Eckert
  Cc: Andy Shevchenko, Enrico Weigelt, Mika Westerberg, Jean Delvare,
	Wolfram Sang, Eckert.Florian, linux-gpio, linux-acpi, linux-i2c

On Thu, Sep 30, 2021 at 9:15 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Sep 30, 2021 at 1:40 AM Florian Eckert <fe@dev.tdt.de> wrote:

...

> > > 5) as Mika suggested, have you checked the kernel configuration?
> >
> > I have now switched on the suggested option
> > CONFIG_ACPI_CUSTOM_METHOD=y
> > CONFIG_ACPI_TABLE_UPGRADE=y
> > CONFIG_CONFIGFS_FS=y
> > CONFIG_ACPI_CONFIGFS=y
> > CONFIG_ACPI_DEBUG=y
> >
> > But this did not solved my issue loading ssdt during runtime.
>
> It won't as explained in 4) above.

It won't _alone_ solve it, i.o.w. it's required but not sufficient.
Means you have to keep these configuration options enabled.

-- 
With Best Regards,
Andy Shevchenko

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

end of thread, other threads:[~2021-09-30  6:19 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-07 17:38 [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data Andy Shevchenko
2020-04-07 17:38 ` [PATCH v1 2/9] pinctrl: mcp23s08: Deduplicate IRQ chip filling Andy Shevchenko
2020-04-07 17:38 ` [PATCH v1 3/9] pinctrl: mcp23s08: Consolidate SPI and I²C code Andy Shevchenko
2020-04-07 17:38 ` [PATCH v1 4/9] pinctrl: mcp23s08: Drop unused parameter in mcp23s08_probe_one() Andy Shevchenko
2020-04-07 17:38 ` [PATCH v1 5/9] pinctrl: mcp23s08: Refactor mcp23s08_spi_regmap_init() Andy Shevchenko
2020-04-07 17:38 ` [PATCH v1 6/9] pinctrl: mcp23s08: Propagate error code from device_property_read_u32() Andy Shevchenko
2020-04-07 17:38 ` [PATCH v1 7/9] pinctrl: mcp23s08: Make use of device properties Andy Shevchenko
2020-04-07 17:38 ` [PATCH v1 8/9] pinctrl: mcp23s08: Use for_each_set_bit() and hweight_long() Andy Shevchenko
2020-04-07 17:38 ` [PATCH v1 9/9] pinctrl: mcp23s08: Split to three parts: core, I²C, SPI Andy Shevchenko
2020-04-16 10:35 ` [PATCH v1 1/9] pinctrl: mcp23s08: Get rid of legacy platform data Linus Walleij
2020-10-09  9:15   ` Martin Hundebøll
2020-10-09 14:02     ` Andy Shevchenko
2020-10-09 16:02       ` Andy Shevchenko
2021-09-16 12:51 ` Florian Eckert
2021-09-22  6:36   ` Andy Shevchenko
2021-09-22  7:53     ` Andy Shevchenko
2021-09-23 14:17       ` Add a SSDT ACPI description to recognize my I2C device connected via SMBus Florian Eckert
2021-09-23 16:24         ` Mika Westerberg
2021-09-23 20:26         ` Andy Shevchenko
2021-09-23 20:29           ` Andy Shevchenko
2021-09-29 22:40           ` Florian Eckert
2021-09-30  6:15             ` Andy Shevchenko
2021-09-30  6:19               ` Andy Shevchenko

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.