* [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
* 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 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
* [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
* 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 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
* [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 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox