* [PATCH 0/3] gpio/mcp23s08: add support for i2c variants
@ 2011-07-14 19:59 Peter Korsgaard
2011-07-14 19:59 ` [PATCH 1/3] mcp23s08: remove unused work queue Peter Korsgaard
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Peter Korsgaard @ 2011-07-14 19:59 UTC (permalink / raw)
To: grant.likely, linux-kernel
The Microchip mcp23xxx I/O expanders exists both in variants with SPI
(23S) and I2C (230) interfaces, with either 8 or 16 I/O pins.
Until now the mcp23s08 driver only worked with the SPI variant. Adjust
the driver to work with the I2C ones as well.
drivers/gpio/Kconfig | 7 +-
drivers/gpio/mcp23s08.c | 260 +++++++++++++++++++++++++++++++++++++-----
include/linux/spi/mcp23s08.h | 4 +-
3 files changed, 237 insertions(+), 34 deletions(-)
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] mcp23s08: remove unused work queue
2011-07-14 19:59 [PATCH 0/3] gpio/mcp23s08: add support for i2c variants Peter Korsgaard
@ 2011-07-14 19:59 ` Peter Korsgaard
2011-07-15 2:53 ` Grant Likely
2011-07-14 19:59 ` [PATCH 2/3] mcp23s08: isolate spi specific parts Peter Korsgaard
2011-07-14 19:59 ` [PATCH 3/3] mcp23s08: add i2c support Peter Korsgaard
2 siblings, 1 reply; 10+ messages in thread
From: Peter Korsgaard @ 2011-07-14 19:59 UTC (permalink / raw)
To: grant.likely, linux-kernel; +Cc: Peter Korsgaard
Never accessed anywhere.
Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
---
drivers/gpio/mcp23s08.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/mcp23s08.c b/drivers/gpio/mcp23s08.c
index 40e0760..242a8a2 100644
--- a/drivers/gpio/mcp23s08.c
+++ b/drivers/gpio/mcp23s08.c
@@ -4,7 +4,6 @@
#include <linux/kernel.h>
#include <linux/device.h>
-#include <linux/workqueue.h>
#include <linux/mutex.h>
#include <linux/gpio.h>
#include <linux/spi/spi.h>
@@ -60,8 +59,6 @@ struct mcp23s08 {
struct gpio_chip chip;
- struct work_struct work;
-
const struct mcp23s08_ops *ops;
};
--
1.7.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] mcp23s08: isolate spi specific parts
2011-07-14 19:59 [PATCH 0/3] gpio/mcp23s08: add support for i2c variants Peter Korsgaard
2011-07-14 19:59 ` [PATCH 1/3] mcp23s08: remove unused work queue Peter Korsgaard
@ 2011-07-14 19:59 ` Peter Korsgaard
2011-07-15 2:53 ` Grant Likely
2011-07-14 19:59 ` [PATCH 3/3] mcp23s08: add i2c support Peter Korsgaard
2 siblings, 1 reply; 10+ messages in thread
From: Peter Korsgaard @ 2011-07-14 19:59 UTC (permalink / raw)
To: grant.likely, linux-kernel; +Cc: Peter Korsgaard
Change spi member of struct mcp23s08 to be a ops-specific opaque data
pointer, and move spi specific knowledge out of mcp23s08_probe_one().
No functional change, but is needed to add i2c support.
Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
---
drivers/gpio/mcp23s08.c | 75 ++++++++++++++++++++++++++++++++--------------
1 files changed, 52 insertions(+), 23 deletions(-)
diff --git a/drivers/gpio/mcp23s08.c b/drivers/gpio/mcp23s08.c
index 242a8a2..1494347 100644
--- a/drivers/gpio/mcp23s08.c
+++ b/drivers/gpio/mcp23s08.c
@@ -50,7 +50,6 @@ struct mcp23s08_ops {
};
struct mcp23s08 {
- struct spi_device *spi;
u8 addr;
u16 cache[11];
@@ -60,6 +59,7 @@ struct mcp23s08 {
struct gpio_chip chip;
const struct mcp23s08_ops *ops;
+ void *data; /* ops specific data */
};
/* A given spi_device can represent up to eight mcp23sxx chips
@@ -73,6 +73,8 @@ struct mcp23s08_driver_data {
struct mcp23s08 chip[];
};
+#ifdef CONFIG_SPI_MASTER
+
static int mcp23s08_read(struct mcp23s08 *mcp, unsigned reg)
{
u8 tx[2], rx[1];
@@ -80,7 +82,7 @@ static int mcp23s08_read(struct mcp23s08 *mcp, unsigned reg)
tx[0] = mcp->addr | 0x01;
tx[1] = reg;
- status = spi_write_then_read(mcp->spi, tx, sizeof tx, rx, sizeof rx);
+ status = spi_write_then_read(mcp->data, tx, sizeof tx, rx, sizeof rx);
return (status < 0) ? status : rx[0];
}
@@ -91,7 +93,7 @@ static int mcp23s08_write(struct mcp23s08 *mcp, unsigned reg, unsigned val)
tx[0] = mcp->addr;
tx[1] = reg;
tx[2] = val;
- return spi_write_then_read(mcp->spi, tx, sizeof tx, NULL, 0);
+ return spi_write_then_read(mcp->data, tx, sizeof tx, NULL, 0);
}
static int
@@ -106,7 +108,7 @@ mcp23s08_read_regs(struct mcp23s08 *mcp, unsigned reg, u16 *vals, unsigned n)
tx[1] = reg;
tmp = (u8 *)vals;
- status = spi_write_then_read(mcp->spi, tx, sizeof tx, tmp, n);
+ status = spi_write_then_read(mcp->data, tx, sizeof tx, tmp, n);
if (status >= 0) {
while (n--)
vals[n] = tmp[n]; /* expand to 16bit */
@@ -121,7 +123,7 @@ static int mcp23s17_read(struct mcp23s08 *mcp, unsigned reg)
tx[0] = mcp->addr | 0x01;
tx[1] = reg << 1;
- status = spi_write_then_read(mcp->spi, tx, sizeof tx, rx, sizeof rx);
+ status = spi_write_then_read(mcp->data, tx, sizeof tx, rx, sizeof rx);
return (status < 0) ? status : (rx[0] | (rx[1] << 8));
}
@@ -133,7 +135,7 @@ static int mcp23s17_write(struct mcp23s08 *mcp, unsigned reg, unsigned val)
tx[1] = reg << 1;
tx[2] = val;
tx[3] = val >> 8;
- return spi_write_then_read(mcp->spi, tx, sizeof tx, NULL, 0);
+ return spi_write_then_read(mcp->data, tx, sizeof tx, NULL, 0);
}
static int
@@ -147,7 +149,7 @@ mcp23s17_read_regs(struct mcp23s08 *mcp, unsigned reg, u16 *vals, unsigned n)
tx[0] = mcp->addr | 0x01;
tx[1] = reg << 1;
- status = spi_write_then_read(mcp->spi, tx, sizeof tx,
+ status = spi_write_then_read(mcp->data, tx, sizeof tx,
(u8 *)vals, n * 2);
if (status >= 0) {
while (n--)
@@ -169,6 +171,7 @@ static const struct mcp23s08_ops mcp23s17_ops = {
.read_regs = mcp23s17_read_regs,
};
+#endif /* CONFIG_SPI_MASTER */
/*----------------------------------------------------------------------*/
@@ -296,17 +299,16 @@ done:
/*----------------------------------------------------------------------*/
-static int mcp23s08_probe_one(struct spi_device *spi, unsigned addr,
+static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
+ void *data, unsigned addr,
unsigned type, unsigned base, unsigned pullups)
{
- struct mcp23s08_driver_data *data = spi_get_drvdata(spi);
- struct mcp23s08 *mcp = data->mcp[addr];
- int status;
+ int status;
mutex_init(&mcp->lock);
- mcp->spi = spi;
- mcp->addr = 0x40 | (addr << 1);
+ mcp->data = data;
+ mcp->addr = addr;
mcp->chip.direction_input = mcp23s08_direction_input;
mcp->chip.get = mcp23s08_get;
@@ -314,18 +316,29 @@ static int mcp23s08_probe_one(struct spi_device *spi, unsigned addr,
mcp->chip.set = mcp23s08_set;
mcp->chip.dbg_show = mcp23s08_dbg_show;
- if (type == MCP_TYPE_S17) {
- mcp->ops = &mcp23s17_ops;
- mcp->chip.ngpio = 16;
- mcp->chip.label = "mcp23s17";
- } else {
+ switch (type) {
+#ifdef CONFIG_SPI_MASTER
+ case MCP_TYPE_S08:
mcp->ops = &mcp23s08_ops;
mcp->chip.ngpio = 8;
mcp->chip.label = "mcp23s08";
+ break;
+
+ case MCP_TYPE_S17:
+ mcp->ops = &mcp23s17_ops;
+ mcp->chip.ngpio = 16;
+ mcp->chip.label = "mcp23s17";
+ break;
+#endif /* CONFIG_SPI_MASTER */
+
+ default:
+ dev_err(dev, "invalid device type (%d)\n", type);
+ return -EINVAL;
}
+
mcp->chip.base = base;
mcp->chip.can_sleep = 1;
- mcp->chip.dev = &spi->dev;
+ mcp->chip.dev = dev;
mcp->chip.owner = THIS_MODULE;
/* verify MCP_IOCON.SEQOP = 0, so sequential reads work,
@@ -371,11 +384,13 @@ static int mcp23s08_probe_one(struct spi_device *spi, unsigned addr,
status = gpiochip_add(&mcp->chip);
fail:
if (status < 0)
- dev_dbg(&spi->dev, "can't setup chip %d, --> %d\n",
- addr, status);
+ dev_dbg(dev, "can't setup chip %d, --> %d\n",
+ addr, status);
return status;
}
+#ifdef CONFIG_SPI_MASTER
+
static int mcp23s08_probe(struct spi_device *spi)
{
struct mcp23s08_platform_data *pdata;
@@ -418,7 +433,8 @@ static int mcp23s08_probe(struct spi_device *spi)
continue;
chips--;
data->mcp[addr] = &data->chip[chips];
- status = mcp23s08_probe_one(spi, addr, type, base,
+ status = mcp23s08_probe_one(data->mcp[addr], &spi->dev, spi,
+ 0x40 | (addr << 1), type, base,
pdata->chip[addr].pullups);
if (status < 0)
goto fail;
@@ -507,11 +523,21 @@ static struct spi_driver mcp23s08_driver = {
},
};
+#endif /* CONFIG_SPI_MASTER */
+
/*----------------------------------------------------------------------*/
static int __init mcp23s08_init(void)
{
- return spi_register_driver(&mcp23s08_driver);
+ int ret = 0;
+
+#ifdef CONFIG_SPI_MASTER
+ ret = spi_register_driver(&mcp23s08_driver);
+ if (ret)
+ return ret;
+#endif /* CONFIG_SPI_MASTER */
+
+ return ret;
}
/* register after spi postcore initcall and before
* subsys initcalls that may rely on these GPIOs
@@ -520,7 +546,10 @@ subsys_initcall(mcp23s08_init);
static void __exit mcp23s08_exit(void)
{
+#ifdef CONFIG_SPI_MASTER
spi_unregister_driver(&mcp23s08_driver);
+#endif /* CONFIG_SPI_MASTER */
+
}
module_exit(mcp23s08_exit);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] mcp23s08: add i2c support
2011-07-14 19:59 [PATCH 0/3] gpio/mcp23s08: add support for i2c variants Peter Korsgaard
2011-07-14 19:59 ` [PATCH 1/3] mcp23s08: remove unused work queue Peter Korsgaard
2011-07-14 19:59 ` [PATCH 2/3] mcp23s08: isolate spi specific parts Peter Korsgaard
@ 2011-07-14 19:59 ` Peter Korsgaard
2011-07-15 2:53 ` Grant Likely
2 siblings, 1 reply; 10+ messages in thread
From: Peter Korsgaard @ 2011-07-14 19:59 UTC (permalink / raw)
To: grant.likely, linux-kernel; +Cc: Peter Korsgaard
Add i2c bindings for the mcp230xx devices. This is quite a lot simpler
than the spi ones as there's no funky sub addressing done (one struct
i2c_client per struct gpio_chip).
The mcp23s08_platform_data structure is reused for i2c, even though
only a single mcp23s08_chip_info structure is needed.
To make the platform data bus independent, the setup/teardown prototypes
are slightly changed, so the bus specific struct (spi_device / i2c_client)
is passed as a void pointer instead.
There's no in-tree users of these callbacks.
To use, simply fill out a platform_data structure and pass it in
i2c_board_info, E.G.:
static const struct mcp23s08_platform_data mcp23017_data = {
.chip[0] = {
.pullups = 0x00ff,
},
.base = 240,
};
static struct i2c_board_info __initdata i2c_devs[] = {
{ I2C_BOARD_INFO("mcp23017", 0x20),
.platform_data = &smartview_mcp23017_data, },
...
};
Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
---
drivers/gpio/Kconfig | 7 +-
drivers/gpio/mcp23s08.c | 184 +++++++++++++++++++++++++++++++++++++++++-
include/linux/spi/mcp23s08.h | 4 +-
3 files changed, 186 insertions(+), 9 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 2967002..cf8a491 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -398,10 +398,11 @@ config GPIO_MAX7301
GPIO driver for Maxim MAX7301 SPI-based GPIO expander.
config GPIO_MCP23S08
- tristate "Microchip MCP23Sxx I/O expander"
- depends on SPI_MASTER
+ tristate "Microchip MCP23xxx I/O expander"
+ depends on SPI_MASTER || I2C
help
- SPI driver for Microchip MCP23S08/MPC23S17 I/O expanders.
+ SPI/I2C driver for Microchip MCP23S08/MCP23S17/MCP23008/MCP23017
+ I/O expanders.
This provides a GPIO interface supporting inputs and outputs.
config GPIO_MC33880
diff --git a/drivers/gpio/mcp23s08.c b/drivers/gpio/mcp23s08.c
index 1494347..5914018 100644
--- a/drivers/gpio/mcp23s08.c
+++ b/drivers/gpio/mcp23s08.c
@@ -1,11 +1,12 @@
/*
- * mcp23s08.c - SPI gpio expander driver
+ * mcp23s08.c - SPI/I2C gpio expander driver
*/
#include <linux/kernel.h>
#include <linux/device.h>
#include <linux/mutex.h>
#include <linux/gpio.h>
+#include <linux/i2c.h>
#include <linux/spi/spi.h>
#include <linux/spi/mcp23s08.h>
#include <linux/slab.h>
@@ -16,13 +17,13 @@
*/
#define MCP_TYPE_S08 0
#define MCP_TYPE_S17 1
+#define MCP_TYPE_008 2
+#define MCP_TYPE_017 3
/* Registers are all 8 bits wide.
*
* The mcp23s17 has twice as many bits, and can be configured to work
* with either 16 bit registers or with two adjacent 8 bit banks.
- *
- * Also, there are I2C versions of both chips.
*/
#define MCP_IODIR 0x00 /* init/reset: all ones */
#define MCP_IPOL 0x01
@@ -73,6 +74,72 @@ struct mcp23s08_driver_data {
struct mcp23s08 chip[];
};
+/*----------------------------------------------------------------------*/
+
+#ifdef CONFIG_I2C
+
+static int mcp23008_read(struct mcp23s08 *mcp, unsigned reg)
+{
+ return i2c_smbus_read_byte_data(mcp->data, reg);
+}
+
+static int mcp23008_write(struct mcp23s08 *mcp, unsigned reg, unsigned val)
+{
+ return i2c_smbus_write_byte_data(mcp->data, reg, val);
+}
+
+static int
+mcp23008_read_regs(struct mcp23s08 *mcp, unsigned reg, u16 *vals, unsigned n)
+{
+ while (n--) {
+ int ret = mcp23008_read(mcp, reg++);
+ if (ret < 0)
+ return ret;
+ *vals++ = ret;
+ }
+
+ return 0;
+}
+
+static int mcp23017_read(struct mcp23s08 *mcp, unsigned reg)
+{
+ return i2c_smbus_read_word_data(mcp->data, reg << 1);
+}
+
+static int mcp23017_write(struct mcp23s08 *mcp, unsigned reg, unsigned val)
+{
+ return i2c_smbus_write_word_data(mcp->data, reg << 1, val);
+}
+
+static int
+mcp23017_read_regs(struct mcp23s08 *mcp, unsigned reg, u16 *vals, unsigned n)
+{
+ while (n--) {
+ int ret = mcp23017_read(mcp, reg++);
+ if (ret < 0)
+ return ret;
+ *vals++ = ret;
+ }
+
+ return 0;
+}
+
+static const struct mcp23s08_ops mcp23008_ops = {
+ .read = mcp23008_read,
+ .write = mcp23008_write,
+ .read_regs = mcp23008_read_regs,
+};
+
+static const struct mcp23s08_ops mcp23017_ops = {
+ .read = mcp23017_read,
+ .write = mcp23017_write,
+ .read_regs = mcp23017_read_regs,
+};
+
+#endif /* CONFIG_I2C */
+
+/*----------------------------------------------------------------------*/
+
#ifdef CONFIG_SPI_MASTER
static int mcp23s08_read(struct mcp23s08 *mcp, unsigned reg)
@@ -331,6 +398,20 @@ static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
break;
#endif /* CONFIG_SPI_MASTER */
+#ifdef CONFIG_I2C
+ case MCP_TYPE_008:
+ mcp->ops = &mcp23008_ops;
+ mcp->chip.ngpio = 8;
+ mcp->chip.label = "mcp23008";
+ break;
+
+ case MCP_TYPE_017:
+ mcp->ops = &mcp23017_ops;
+ mcp->chip.ngpio = 16;
+ mcp->chip.label = "mcp23017";
+ break;
+#endif /* CONFIG_I2C */
+
default:
dev_err(dev, "invalid device type (%d)\n", type);
return -EINVAL;
@@ -389,6 +470,93 @@ fail:
return status;
}
+/*----------------------------------------------------------------------*/
+
+#ifdef CONFIG_I2C
+
+static int __devinit mcp230xx_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct mcp23s08_platform_data *pdata;
+ struct mcp23s08 *mcp;
+ int status;
+
+ pdata = client->dev.platform_data;
+ if (!pdata || !gpio_is_valid(pdata->base)) {
+ dev_dbg(&client->dev, "invalid or missing platform data\n");
+ return -EINVAL;
+ }
+
+ mcp = kzalloc(sizeof *mcp, GFP_KERNEL);
+ if (!mcp)
+ return -ENOMEM;
+
+ status = mcp23s08_probe_one(mcp, &client->dev, client, client->addr,
+ id->driver_data, pdata->base,
+ pdata->chip[0].pullups);
+ if (status)
+ goto fail;
+
+ i2c_set_clientdata(client, mcp);
+
+ if (pdata->setup) {
+ status = pdata->setup(client, pdata->base, mcp->chip.ngpio,
+ pdata->context);
+ if (status < 0)
+ dev_dbg(&client->dev, "setup --> %d\n", status);
+ }
+
+ return 0;
+
+fail:
+ kfree(mcp);
+
+ return status;
+}
+
+static int __devexit mcp230xx_remove(struct i2c_client *client)
+{
+ struct mcp23s08_platform_data *pdata = client->dev.platform_data;
+ struct mcp23s08 *mcp = i2c_get_clientdata(client);
+ int status;
+
+ if (pdata->teardown) {
+ status = pdata->teardown(client, pdata->base, mcp->chip.ngpio,
+ pdata->context);
+ if (status < 0) {
+ dev_err(&client->dev, "teardown --> %d\n", status);
+ return status;
+ }
+ }
+
+ status = gpiochip_remove(&mcp->chip);
+ if (status == 0)
+ kfree(mcp);
+
+ return status;
+}
+
+static const struct i2c_device_id mcp230xx_id[] = {
+ { "mcp23008", MCP_TYPE_008 },
+ { "mcp23017", MCP_TYPE_017 },
+ { },
+};
+MODULE_DEVICE_TABLE(i2c, mcp230xx_id);
+
+static struct i2c_driver mcp230xx_driver = {
+ .driver = {
+ .name = "mcp230xx",
+ .owner = THIS_MODULE,
+ },
+ .probe = mcp230xx_probe,
+ .remove = __devexit_p(mcp230xx_remove),
+ .id_table = mcp230xx_id,
+};
+
+#endif /* CONFIG_I2C */
+
+/*----------------------------------------------------------------------*/
+
#ifdef CONFIG_SPI_MASTER
static int mcp23s08_probe(struct spi_device *spi)
@@ -537,9 +705,13 @@ static int __init mcp23s08_init(void)
return ret;
#endif /* CONFIG_SPI_MASTER */
+#ifdef CONFIG_I2C
+ ret = i2c_add_driver(&mcp230xx_driver);
+#endif /* CONFIG_I2C */
+
return ret;
}
-/* register after spi postcore initcall and before
+/* register after spi/i2c postcore initcall and before
* subsys initcalls that may rely on these GPIOs
*/
subsys_initcall(mcp23s08_init);
@@ -550,6 +722,10 @@ static void __exit mcp23s08_exit(void)
spi_unregister_driver(&mcp23s08_driver);
#endif /* CONFIG_SPI_MASTER */
+#ifdef CONFIG_I2C
+ i2c_del_driver(&mcp230xx_driver);
+#endif /* CONFIG_I2C */
+
}
module_exit(mcp23s08_exit);
diff --git a/include/linux/spi/mcp23s08.h b/include/linux/spi/mcp23s08.h
index c42cff8..2eb4fc1 100644
--- a/include/linux/spi/mcp23s08.h
+++ b/include/linux/spi/mcp23s08.h
@@ -25,10 +25,10 @@ struct mcp23s08_platform_data {
void *context; /* param to setup/teardown */
- int (*setup)(struct spi_device *spi,
+ int (*setup)(void *data,
int gpio, unsigned ngpio,
void *context);
- int (*teardown)(struct spi_device *spi,
+ int (*teardown)(void *data,
int gpio, unsigned ngpio,
void *context);
};
--
1.7.5.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] mcp23s08: add i2c support
2011-07-14 19:59 ` [PATCH 3/3] mcp23s08: add i2c support Peter Korsgaard
@ 2011-07-15 2:53 ` Grant Likely
2011-07-15 7:03 ` Peter Korsgaard
0 siblings, 1 reply; 10+ messages in thread
From: Grant Likely @ 2011-07-15 2:53 UTC (permalink / raw)
To: Peter Korsgaard; +Cc: linux-kernel
On Thu, Jul 14, 2011 at 09:59:28PM +0200, Peter Korsgaard wrote:
> Add i2c bindings for the mcp230xx devices. This is quite a lot simpler
> than the spi ones as there's no funky sub addressing done (one struct
> i2c_client per struct gpio_chip).
>
> The mcp23s08_platform_data structure is reused for i2c, even though
> only a single mcp23s08_chip_info structure is needed.
>
> To make the platform data bus independent, the setup/teardown prototypes
> are slightly changed, so the bus specific struct (spi_device / i2c_client)
> is passed as a void pointer instead.
>
> There's no in-tree users of these callbacks.
Why don't we just remove them then? The notifier mechanism is more
generic anyway.
[...]
> @@ -537,9 +705,13 @@ static int __init mcp23s08_init(void)
> return ret;
> #endif /* CONFIG_SPI_MASTER */
>
> +#ifdef CONFIG_I2C
> + ret = i2c_add_driver(&mcp230xx_driver);
> +#endif /* CONFIG_I2C */
> +
Need to unwind on failure to register, or put the i2c driver into a
separate module so that each module gets it's own init/exit hooks.
> return ret;
> }
> -/* register after spi postcore initcall and before
> +/* register after spi/i2c postcore initcall and before
> * subsys initcalls that may rely on these GPIOs
> */
> subsys_initcall(mcp23s08_init);
> @@ -550,6 +722,10 @@ static void __exit mcp23s08_exit(void)
> spi_unregister_driver(&mcp23s08_driver);
> #endif /* CONFIG_SPI_MASTER */
>
> +#ifdef CONFIG_I2C
> + i2c_del_driver(&mcp230xx_driver);
> +#endif /* CONFIG_I2C */
> +
> }
> module_exit(mcp23s08_exit);
>
> diff --git a/include/linux/spi/mcp23s08.h b/include/linux/spi/mcp23s08.h
> index c42cff8..2eb4fc1 100644
> --- a/include/linux/spi/mcp23s08.h
> +++ b/include/linux/spi/mcp23s08.h
> @@ -25,10 +25,10 @@ struct mcp23s08_platform_data {
>
> void *context; /* param to setup/teardown */
>
> - int (*setup)(struct spi_device *spi,
> + int (*setup)(void *data,
> int gpio, unsigned ngpio,
> void *context);
> - int (*teardown)(struct spi_device *spi,
> + int (*teardown)(void *data,
> int gpio, unsigned ngpio,
> void *context);
> };
> --
> 1.7.5.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mcp23s08: remove unused work queue
2011-07-14 19:59 ` [PATCH 1/3] mcp23s08: remove unused work queue Peter Korsgaard
@ 2011-07-15 2:53 ` Grant Likely
2011-07-15 6:54 ` Peter Korsgaard
0 siblings, 1 reply; 10+ messages in thread
From: Grant Likely @ 2011-07-15 2:53 UTC (permalink / raw)
To: Peter Korsgaard; +Cc: linux-kernel
On Thu, Jul 14, 2011 at 09:59:26PM +0200, Peter Korsgaard wrote:
> Never accessed anywhere.
>
> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
I've applied this, but I had to do it manually. Please rebase the
remaining two patches on linux-next.
g.
> ---
> drivers/gpio/mcp23s08.c | 3 ---
> 1 files changed, 0 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpio/mcp23s08.c b/drivers/gpio/mcp23s08.c
> index 40e0760..242a8a2 100644
> --- a/drivers/gpio/mcp23s08.c
> +++ b/drivers/gpio/mcp23s08.c
> @@ -4,7 +4,6 @@
>
> #include <linux/kernel.h>
> #include <linux/device.h>
> -#include <linux/workqueue.h>
> #include <linux/mutex.h>
> #include <linux/gpio.h>
> #include <linux/spi/spi.h>
> @@ -60,8 +59,6 @@ struct mcp23s08 {
>
> struct gpio_chip chip;
>
> - struct work_struct work;
> -
> const struct mcp23s08_ops *ops;
> };
>
> --
> 1.7.5.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] mcp23s08: isolate spi specific parts
2011-07-14 19:59 ` [PATCH 2/3] mcp23s08: isolate spi specific parts Peter Korsgaard
@ 2011-07-15 2:53 ` Grant Likely
2011-07-15 6:57 ` Peter Korsgaard
0 siblings, 1 reply; 10+ messages in thread
From: Grant Likely @ 2011-07-15 2:53 UTC (permalink / raw)
To: Peter Korsgaard; +Cc: linux-kernel
On Thu, Jul 14, 2011 at 09:59:27PM +0200, Peter Korsgaard wrote:
> Change spi member of struct mcp23s08 to be a ops-specific opaque data
> pointer, and move spi specific knowledge out of mcp23s08_probe_one().
>
> No functional change, but is needed to add i2c support.
>
> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
> ---
> drivers/gpio/mcp23s08.c | 75 ++++++++++++++++++++++++++++++++--------------
> 1 files changed, 52 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpio/mcp23s08.c b/drivers/gpio/mcp23s08.c
> index 242a8a2..1494347 100644
> --- a/drivers/gpio/mcp23s08.c
> +++ b/drivers/gpio/mcp23s08.c
> @@ -50,7 +50,6 @@ struct mcp23s08_ops {
> };
>
> struct mcp23s08 {
> - struct spi_device *spi;
> u8 addr;
>
> u16 cache[11];
> @@ -60,6 +59,7 @@ struct mcp23s08 {
> struct gpio_chip chip;
>
> const struct mcp23s08_ops *ops;
> + void *data; /* ops specific data */
> };
>
> /* A given spi_device can represent up to eight mcp23sxx chips
> @@ -73,6 +73,8 @@ struct mcp23s08_driver_data {
> struct mcp23s08 chip[];
> };
>
> +#ifdef CONFIG_SPI_MASTER
> +
> static int mcp23s08_read(struct mcp23s08 *mcp, unsigned reg)
> {
> u8 tx[2], rx[1];
> @@ -80,7 +82,7 @@ static int mcp23s08_read(struct mcp23s08 *mcp, unsigned reg)
>
> tx[0] = mcp->addr | 0x01;
> tx[1] = reg;
> - status = spi_write_then_read(mcp->spi, tx, sizeof tx, rx, sizeof rx);
> + status = spi_write_then_read(mcp->data, tx, sizeof tx, rx, sizeof rx);
> return (status < 0) ? status : rx[0];
> }
>
> @@ -91,7 +93,7 @@ static int mcp23s08_write(struct mcp23s08 *mcp, unsigned reg, unsigned val)
> tx[0] = mcp->addr;
> tx[1] = reg;
> tx[2] = val;
> - return spi_write_then_read(mcp->spi, tx, sizeof tx, NULL, 0);
> + return spi_write_then_read(mcp->data, tx, sizeof tx, NULL, 0);
> }
>
> static int
> @@ -106,7 +108,7 @@ mcp23s08_read_regs(struct mcp23s08 *mcp, unsigned reg, u16 *vals, unsigned n)
> tx[1] = reg;
>
> tmp = (u8 *)vals;
> - status = spi_write_then_read(mcp->spi, tx, sizeof tx, tmp, n);
> + status = spi_write_then_read(mcp->data, tx, sizeof tx, tmp, n);
> if (status >= 0) {
> while (n--)
> vals[n] = tmp[n]; /* expand to 16bit */
> @@ -121,7 +123,7 @@ static int mcp23s17_read(struct mcp23s08 *mcp, unsigned reg)
>
> tx[0] = mcp->addr | 0x01;
> tx[1] = reg << 1;
> - status = spi_write_then_read(mcp->spi, tx, sizeof tx, rx, sizeof rx);
> + status = spi_write_then_read(mcp->data, tx, sizeof tx, rx, sizeof rx);
> return (status < 0) ? status : (rx[0] | (rx[1] << 8));
> }
>
> @@ -133,7 +135,7 @@ static int mcp23s17_write(struct mcp23s08 *mcp, unsigned reg, unsigned val)
> tx[1] = reg << 1;
> tx[2] = val;
> tx[3] = val >> 8;
> - return spi_write_then_read(mcp->spi, tx, sizeof tx, NULL, 0);
> + return spi_write_then_read(mcp->data, tx, sizeof tx, NULL, 0);
> }
>
> static int
> @@ -147,7 +149,7 @@ mcp23s17_read_regs(struct mcp23s08 *mcp, unsigned reg, u16 *vals, unsigned n)
> tx[0] = mcp->addr | 0x01;
> tx[1] = reg << 1;
>
> - status = spi_write_then_read(mcp->spi, tx, sizeof tx,
> + status = spi_write_then_read(mcp->data, tx, sizeof tx,
> (u8 *)vals, n * 2);
> if (status >= 0) {
> while (n--)
> @@ -169,6 +171,7 @@ static const struct mcp23s08_ops mcp23s17_ops = {
> .read_regs = mcp23s17_read_regs,
> };
>
> +#endif /* CONFIG_SPI_MASTER */
>
> /*----------------------------------------------------------------------*/
>
> @@ -296,17 +299,16 @@ done:
>
> /*----------------------------------------------------------------------*/
>
> -static int mcp23s08_probe_one(struct spi_device *spi, unsigned addr,
> +static int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
> + void *data, unsigned addr,
> unsigned type, unsigned base, unsigned pullups)
> {
> - struct mcp23s08_driver_data *data = spi_get_drvdata(spi);
> - struct mcp23s08 *mcp = data->mcp[addr];
> - int status;
> + int status;
>
> mutex_init(&mcp->lock);
>
> - mcp->spi = spi;
> - mcp->addr = 0x40 | (addr << 1);
> + mcp->data = data;
> + mcp->addr = addr;
>
> mcp->chip.direction_input = mcp23s08_direction_input;
> mcp->chip.get = mcp23s08_get;
> @@ -314,18 +316,29 @@ static int mcp23s08_probe_one(struct spi_device *spi, unsigned addr,
> mcp->chip.set = mcp23s08_set;
> mcp->chip.dbg_show = mcp23s08_dbg_show;
>
> - if (type == MCP_TYPE_S17) {
> - mcp->ops = &mcp23s17_ops;
> - mcp->chip.ngpio = 16;
> - mcp->chip.label = "mcp23s17";
> - } else {
> + switch (type) {
> +#ifdef CONFIG_SPI_MASTER
> + case MCP_TYPE_S08:
> mcp->ops = &mcp23s08_ops;
> mcp->chip.ngpio = 8;
> mcp->chip.label = "mcp23s08";
> + break;
> +
> + case MCP_TYPE_S17:
> + mcp->ops = &mcp23s17_ops;
> + mcp->chip.ngpio = 16;
> + mcp->chip.label = "mcp23s17";
> + break;
> +#endif /* CONFIG_SPI_MASTER */
> +
> + default:
> + dev_err(dev, "invalid device type (%d)\n", type);
> + return -EINVAL;
> }
> +
> mcp->chip.base = base;
> mcp->chip.can_sleep = 1;
> - mcp->chip.dev = &spi->dev;
> + mcp->chip.dev = dev;
> mcp->chip.owner = THIS_MODULE;
>
> /* verify MCP_IOCON.SEQOP = 0, so sequential reads work,
> @@ -371,11 +384,13 @@ static int mcp23s08_probe_one(struct spi_device *spi, unsigned addr,
> status = gpiochip_add(&mcp->chip);
> fail:
> if (status < 0)
> - dev_dbg(&spi->dev, "can't setup chip %d, --> %d\n",
> - addr, status);
> + dev_dbg(dev, "can't setup chip %d, --> %d\n",
> + addr, status);
> return status;
> }
>
> +#ifdef CONFIG_SPI_MASTER
> +
> static int mcp23s08_probe(struct spi_device *spi)
> {
> struct mcp23s08_platform_data *pdata;
> @@ -418,7 +433,8 @@ static int mcp23s08_probe(struct spi_device *spi)
> continue;
> chips--;
> data->mcp[addr] = &data->chip[chips];
> - status = mcp23s08_probe_one(spi, addr, type, base,
> + status = mcp23s08_probe_one(data->mcp[addr], &spi->dev, spi,
> + 0x40 | (addr << 1), type, base,
> pdata->chip[addr].pullups);
> if (status < 0)
> goto fail;
> @@ -507,11 +523,21 @@ static struct spi_driver mcp23s08_driver = {
> },
> };
>
> +#endif /* CONFIG_SPI_MASTER */
> +
> /*----------------------------------------------------------------------*/
>
> static int __init mcp23s08_init(void)
> {
> - return spi_register_driver(&mcp23s08_driver);
> + int ret = 0;
'= 0' is redundant.
> +
> +#ifdef CONFIG_SPI_MASTER
> + ret = spi_register_driver(&mcp23s08_driver);
> + if (ret)
> + return ret;
> +#endif /* CONFIG_SPI_MASTER */
> +
> + return ret;
This change really belongs in the 3rd patch that adds the i2c
registration.
> }
> /* register after spi postcore initcall and before
> * subsys initcalls that may rely on these GPIOs
> @@ -520,7 +546,10 @@ subsys_initcall(mcp23s08_init);
>
> static void __exit mcp23s08_exit(void)
> {
> +#ifdef CONFIG_SPI_MASTER
> spi_unregister_driver(&mcp23s08_driver);
> +#endif /* CONFIG_SPI_MASTER */
> +
Extra blank line.
> }
> module_exit(mcp23s08_exit);
>
> --
> 1.7.5.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] mcp23s08: remove unused work queue
2011-07-15 2:53 ` Grant Likely
@ 2011-07-15 6:54 ` Peter Korsgaard
0 siblings, 0 replies; 10+ messages in thread
From: Peter Korsgaard @ 2011-07-15 6:54 UTC (permalink / raw)
To: Grant Likely; +Cc: linux-kernel
>>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
Grant> On Thu, Jul 14, 2011 at 09:59:26PM +0200, Peter Korsgaard wrote:
>> Never accessed anywhere.
>>
>> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
Grant> I've applied this, but I had to do it manually. Please rebase the
Grant> remaining two patches on linux-next.
Thanks, I will.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] mcp23s08: isolate spi specific parts
2011-07-15 2:53 ` Grant Likely
@ 2011-07-15 6:57 ` Peter Korsgaard
0 siblings, 0 replies; 10+ messages in thread
From: Peter Korsgaard @ 2011-07-15 6:57 UTC (permalink / raw)
To: Grant Likely; +Cc: linux-kernel
>>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
Grant> On Thu, Jul 14, 2011 at 09:59:27PM +0200, Peter Korsgaard wrote:
>> Change spi member of struct mcp23s08 to be a ops-specific opaque data
>> pointer, and move spi specific knowledge out of mcp23s08_probe_one().
>>
>> No functional change, but is needed to add i2c support.
>>
>> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
>> ---
>> drivers/gpio/mcp23s08.c | 75 ++++++++++++++++++++++++++++++++--------------
>> static int __init mcp23s08_init(void)
>> {
>> - return spi_register_driver(&mcp23s08_driver);
>> + int ret = 0;
Grant> '= 0' is redundant.
Will fix.
>> +
>> +#ifdef CONFIG_SPI_MASTER
>> + ret = spi_register_driver(&mcp23s08_driver);
>> + if (ret)
>> + return ret;
>> +#endif /* CONFIG_SPI_MASTER */
>> +
>> + return ret;
Grant> This change really belongs in the 3rd patch that adds the i2c
Grant> registration.
You can argue for both ways. With my approach the 3rd patch doesn't
touch any of the spi stuff, but OK - I'll change.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] mcp23s08: add i2c support
2011-07-15 2:53 ` Grant Likely
@ 2011-07-15 7:03 ` Peter Korsgaard
0 siblings, 0 replies; 10+ messages in thread
From: Peter Korsgaard @ 2011-07-15 7:03 UTC (permalink / raw)
To: Grant Likely; +Cc: linux-kernel
>>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
Grant> On Thu, Jul 14, 2011 at 09:59:28PM +0200, Peter Korsgaard wrote:
>> Add i2c bindings for the mcp230xx devices. This is quite a lot simpler
>> than the spi ones as there's no funky sub addressing done (one struct
>> i2c_client per struct gpio_chip).
>>
>> The mcp23s08_platform_data structure is reused for i2c, even though
>> only a single mcp23s08_chip_info structure is needed.
>>
>> To make the platform data bus independent, the setup/teardown prototypes
>> are slightly changed, so the bus specific struct (spi_device / i2c_client)
>> is passed as a void pointer instead.
>>
>> There's no in-tree users of these callbacks.
Grant> Why don't we just remove them then? The notifier mechanism is more
Grant> generic anyway.
Ok, will do.
Grant> [...]
>> @@ -537,9 +705,13 @@ static int __init mcp23s08_init(void)
>> return ret;
>> #endif /* CONFIG_SPI_MASTER */
>>
>> +#ifdef CONFIG_I2C
>> + ret = i2c_add_driver(&mcp230xx_driver);
>> +#endif /* CONFIG_I2C */
>> +
Grant> Need to unwind on failure to register, or put the i2c driver into a
Grant> separate module so that each module gets it's own init/exit hooks.
Ok, will unwind.
Thanks for the quick review, will send an updated patch series shortly.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-07-15 7:03 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-14 19:59 [PATCH 0/3] gpio/mcp23s08: add support for i2c variants Peter Korsgaard
2011-07-14 19:59 ` [PATCH 1/3] mcp23s08: remove unused work queue Peter Korsgaard
2011-07-15 2:53 ` Grant Likely
2011-07-15 6:54 ` Peter Korsgaard
2011-07-14 19:59 ` [PATCH 2/3] mcp23s08: isolate spi specific parts Peter Korsgaard
2011-07-15 2:53 ` Grant Likely
2011-07-15 6:57 ` Peter Korsgaard
2011-07-14 19:59 ` [PATCH 3/3] mcp23s08: add i2c support Peter Korsgaard
2011-07-15 2:53 ` Grant Likely
2011-07-15 7:03 ` Peter Korsgaard
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.