| Message ID | 1347020296-18796-6-git-send-email-maxime.ripard@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello Maxime, On Friday 07 September 2012 14:18:14 Maxime Ripard wrote: > The shift registers have an output pin that, when enabled, propagates > the values of its internal register to the pins. If another value comes > to the register while the output pin is disabled, this new value will > makae the older shift into the next register in the chain. > > This patch adds support for daisy-chaining the registers, using the > regular SPI chip select mechanism to manage the output pin, and the > registers-number dt property to set the number of chained registers. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- [snip] > static int __gen_74x164_write_config(struct gen_74x164_chip *chip) > { > - return spi_write(chip->spi, > - &chip->port_config, sizeof(chip->port_config)); > + struct spi_message message; > + struct spi_transfer *msg_buf; > + int i, ret; > + > + msg_buf = kzalloc(chip->registers * sizeof(struct spi_transfer), > + GFP_KERNEL); > + if (!msg_buf) > + return -ENOMEM; > + > + spi_message_init(&message); > + > + /* > + * Since the registers are chained, every byte sent will make > + * the previous byte shift to the next register in the > + * chain. Thus, the first byte send will end up in the last > + * register at the end of the transfer. So, to have a logical > + * numbering, send the bytes in reverse order so that the last > + * byte of the buffer will end up in the last register. > + */ > + for (i = chip->registers - 1; i >= 0; i--) { > + msg_buf[i].tx_buf = chip->buffer +i; > + msg_buf[i].len = sizeof(u8); > + spi_message_add_tail(msg_buf + i, &message); > + } > + > + ret = spi_sync(chip->spi, &message); > + if (ret) > + return ret; You are leaking msg_buf here in case of error. > + > + kfree(msg_buf); > + > + return 0; > } > > static int gen_74x164_get_value(struct gpio_chip *gc, unsigned offset) > { > struct gen_74x164_chip *chip = gpio_to_74x164_chip(gc); > + u8 bank = offset / 8; > + u8 pin = offset % 8; > int ret; > > mutex_lock(&chip->lock); > - ret = (chip->port_config >> offset) & 0x1; > + ret = (chip->buffer[bank] >> pin) & 0x1; > mutex_unlock(&chip->lock); > > return ret; > @@ -51,12 +87,14 @@ static void gen_74x164_set_value(struct gpio_chip *gc, > unsigned offset, int val) > { > struct gen_74x164_chip *chip = gpio_to_74x164_chip(gc); > + u8 bank = offset / 8; > + u8 pin = offset % 8; > > mutex_lock(&chip->lock); > if (val) > - chip->port_config |= (1 << offset); > + chip->buffer[bank] |= (1 << pin); > else > - chip->port_config &= ~(1 << offset); > + chip->buffer[bank] &= ~(1 << pin); > > __gen_74x164_write_config(chip); > mutex_unlock(&chip->lock); > @@ -75,6 +113,11 @@ static int __devinit gen_74x164_probe(struct spi_device *spi) > struct gen_74x164_chip_platform_data *pdata; > int ret; > > + if (!spi->dev.of_node) { > + dev_err(&spi->dev, "No device tree data available.\n"); > + return -EINVAL; > + } Should not this be folded in your previous patch? > + > /* > * bits_per_word cannot be configured in platform data > */ > @@ -104,7 +147,20 @@ static int __devinit gen_74x164_probe(struct spi_device *spi) > chip->gpio_chip.direction_output = gen_74x164_direction_output; > chip->gpio_chip.get = gen_74x164_get_value; > chip->gpio_chip.set = gen_74x164_set_value; > - chip->gpio_chip.ngpio = 8; > + > + if (of_property_read_u32(spi->dev.of_node, "registers-number", &chip- >registers)) { > + dev_err(&spi->dev, "Missing registers-number property in the DT. \n"); > + ret = -EINVAL; > + goto exit_destroy; > + } > + > + chip->gpio_chip.ngpio = GEN_74X164_NUMBER_GPIOS * chip->registers; > + chip->buffer = devm_kzalloc(&spi->dev, chip->gpio_chip.ngpio, GFP_KERNEL); > + if (!chip->buffer) { > + ret = -ENOMEM; > + goto exit_destroy; > + } > + > chip->gpio_chip.can_sleep = 1; > chip->gpio_chip.dev = &spi->dev; > chip->gpio_chip.owner = THIS_MODULE; > -- > 1.7.9.5 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Fri, Sep 7, 2012 at 2:18 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > The shift registers have an output pin that, when enabled, propagates > the values of its internal register to the pins. If another value comes > to the register while the output pin is disabled, this new value will > makae the older shift into the next register in the chain. > > This patch adds support for daisy-chaining the registers, using the > regular SPI chip select mechanism to manage the output pin, and the > registers-number dt property to set the number of chained registers. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> I'm stalling 5 thru 7 waiting for Florians comments to be addressed. Yours, Linus Walleij
On Fri, Sep 7, 2012 at 4:03 PM, Florian Fainelli <florian@openwrt.org> wrote: >> + if (!spi->dev.of_node) { >> + dev_err(&spi->dev, "No device tree data available.\n"); >> + return -EINVAL; >> + } > > Should not this be folded in your previous patch? True, but I've applied it, keep it here or split off as a separate fix. Yours, Linus Walleij
On Fri, Sep 07, 2012 at 11:07:33PM +0200, Linus Walleij wrote: > I'm stalling 5 thru 7 waiting for Florians comments to be addressed. > I expect that patch #7 does not apply on your tree at all, so I will take it via mxs/dt branch to ease the process.
On Mon, Sep 10, 2012 at 3:51 AM, Shawn Guo <shawn.guo@linaro.org> wrote: > On Fri, Sep 07, 2012 at 11:07:33PM +0200, Linus Walleij wrote: >> I'm stalling 5 thru 7 waiting for Florians comments to be addressed. >> > I expect that patch #7 does not apply on your tree at all, so I will > take it via mxs/dt branch to ease the process. Just that one patch? Or do you mean you want to take all of them? I really would prefer not have to rebase the GPIO tree... Yours, Linus Walleij
On Tue, Sep 11, 2012 at 06:57:15PM +0200, Linus Walleij wrote: > On Mon, Sep 10, 2012 at 3:51 AM, Shawn Guo <shawn.guo@linaro.org> wrote: > > On Fri, Sep 07, 2012 at 11:07:33PM +0200, Linus Walleij wrote: > >> I'm stalling 5 thru 7 waiting for Florians comments to be addressed. > >> > > I expect that patch #7 does not apply on your tree at all, so I will > > take it via mxs/dt branch to ease the process. > > Just that one patch? Or do you mean you want to take all of them? > I really would prefer not have to rebase the GPIO tree... > Just that one. Regards, Shawn
diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c index 2e31bd3..1831d0f 100644 --- a/drivers/gpio/gpio-74x164.c +++ b/drivers/gpio/gpio-74x164.c @@ -14,14 +14,18 @@ #include <linux/spi/spi.h> #include <linux/spi/74x164.h> #include <linux/gpio.h> +#include <linux/of_gpio.h> #include <linux/slab.h> #include <linux/module.h> +#define GEN_74X164_NUMBER_GPIOS 8 + struct gen_74x164_chip { struct spi_device *spi; + u8 *buffer; struct gpio_chip gpio_chip; struct mutex lock; - u8 port_config; + u32 registers; }; static struct gen_74x164_chip *gpio_to_74x164_chip(struct gpio_chip *gc) @@ -31,17 +35,49 @@ static struct gen_74x164_chip *gpio_to_74x164_chip(struct gpio_chip *gc) static int __gen_74x164_write_config(struct gen_74x164_chip *chip) { - return spi_write(chip->spi, - &chip->port_config, sizeof(chip->port_config)); + struct spi_message message; + struct spi_transfer *msg_buf; + int i, ret; + + msg_buf = kzalloc(chip->registers * sizeof(struct spi_transfer), + GFP_KERNEL); + if (!msg_buf) + return -ENOMEM; + + spi_message_init(&message); + + /* + * Since the registers are chained, every byte sent will make + * the previous byte shift to the next register in the + * chain. Thus, the first byte send will end up in the last + * register at the end of the transfer. So, to have a logical + * numbering, send the bytes in reverse order so that the last + * byte of the buffer will end up in the last register. + */ + for (i = chip->registers - 1; i >= 0; i--) { + msg_buf[i].tx_buf = chip->buffer +i; + msg_buf[i].len = sizeof(u8); + spi_message_add_tail(msg_buf + i, &message); + } + + ret = spi_sync(chip->spi, &message); + if (ret) + return ret; + + kfree(msg_buf); + + return 0; } static int gen_74x164_get_value(struct gpio_chip *gc, unsigned offset) { struct gen_74x164_chip *chip = gpio_to_74x164_chip(gc); + u8 bank = offset / 8; + u8 pin = offset % 8; int ret; mutex_lock(&chip->lock); - ret = (chip->port_config >> offset) & 0x1; + ret = (chip->buffer[bank] >> pin) & 0x1; mutex_unlock(&chip->lock); return ret; @@ -51,12 +87,14 @@ static void gen_74x164_set_value(struct gpio_chip *gc, unsigned offset, int val) { struct gen_74x164_chip *chip = gpio_to_74x164_chip(gc); + u8 bank = offset / 8; + u8 pin = offset % 8; mutex_lock(&chip->lock); if (val) - chip->port_config |= (1 << offset); + chip->buffer[bank] |= (1 << pin); else - chip->port_config &= ~(1 << offset); + chip->buffer[bank] &= ~(1 << pin); __gen_74x164_write_config(chip); mutex_unlock(&chip->lock); @@ -75,6 +113,11 @@ static int __devinit gen_74x164_probe(struct spi_device *spi) struct gen_74x164_chip_platform_data *pdata; int ret; + if (!spi->dev.of_node) { + dev_err(&spi->dev, "No device tree data available.\n"); + return -EINVAL; + } + /* * bits_per_word cannot be configured in platform data */ @@ -104,7 +147,20 @@ static int __devinit gen_74x164_probe(struct spi_device *spi) chip->gpio_chip.direction_output = gen_74x164_direction_output; chip->gpio_chip.get = gen_74x164_get_value; chip->gpio_chip.set = gen_74x164_set_value; - chip->gpio_chip.ngpio = 8; + + if (of_property_read_u32(spi->dev.of_node, "registers-number", &chip->registers)) { + dev_err(&spi->dev, "Missing registers-number property in the DT.\n"); + ret = -EINVAL; + goto exit_destroy; + } + + chip->gpio_chip.ngpio = GEN_74X164_NUMBER_GPIOS * chip->registers; + chip->buffer = devm_kzalloc(&spi->dev, chip->gpio_chip.ngpio, GFP_KERNEL); + if (!chip->buffer) { + ret = -ENOMEM; + goto exit_destroy; + } + chip->gpio_chip.can_sleep = 1; chip->gpio_chip.dev = &spi->dev; chip->gpio_chip.owner = THIS_MODULE;
The shift registers have an output pin that, when enabled, propagates the values of its internal register to the pins. If another value comes to the register while the output pin is disabled, this new value will makae the older shift into the next register in the chain. This patch adds support for daisy-chaining the registers, using the regular SPI chip select mechanism to manage the output pin, and the registers-number dt property to set the number of chained registers. Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> --- drivers/gpio/gpio-74x164.c | 70 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 63 insertions(+), 7 deletions(-)