[PATCH 1/8] gpio: 74x164: Use module_spi_driver boiler plate function

[PATCH 1/8] gpio: 74x164: Use module_spi_driver boiler plate function

[Maxime Ripard]

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpio/gpio-74x164.c |   13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index a31ad6f..2975036 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -159,18 +159,7 @@ static struct spi_driver gen_74x164_driver = {
 	.probe		= gen_74x164_probe,
 	.remove		= __devexit_p(gen_74x164_remove),
 };
-
-static int __init gen_74x164_init(void)
-{
-	return spi_register_driver(&gen_74x164_driver);
-}
-subsys_initcall(gen_74x164_init);
-
-static void __exit gen_74x164_exit(void)
-{
-	spi_unregister_driver(&gen_74x164_driver);
-}
-module_exit(gen_74x164_exit);
+module_spi_driver(gen_74x164_driver);
 
 MODULE_AUTHOR("Gabor Juhos <juhosg@openwrt.org>");
 MODULE_AUTHOR("Miguel Gaio <miguel.gaio@efixo.com>");
-- 
1.7.9.5

[PATCH 2/8] gpio: 74x164: Use devm_kzalloc

[Maxime Ripard]

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpio/gpio-74x164.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index 2975036..604b998 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -90,7 +90,7 @@ static int __devinit gen_74x164_probe(struct spi_device *spi)
 	if (ret < 0)
 		return ret;
 
-	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+	chip = devm_kzalloc(&spi->dev, sizeof(*chip), GFP_KERNEL);
 	if (!chip)
 		return -ENOMEM;
 
@@ -125,7 +125,6 @@ static int __devinit gen_74x164_probe(struct spi_device *spi)
 exit_destroy:
 	dev_set_drvdata(&spi->dev, NULL);
 	mutex_destroy(&chip->lock);
-	kfree(chip);
 	return ret;
 }
 
@@ -141,10 +140,9 @@ static int __devexit gen_74x164_remove(struct spi_device *spi)
 	dev_set_drvdata(&spi->dev, NULL);
 
 	ret = gpiochip_remove(&chip->gpio_chip);
-	if (!ret) {
+	if (!ret)
 		mutex_destroy(&chip->lock);
-		kfree(chip);
-	} else
+	else
 		dev_err(&spi->dev, "Failed to remove the GPIO controller: %d\n",
 				ret);
 
-- 
1.7.9.5

[PATCH 2/8] gpio: 74x164: Use devm_kzalloc

[Florian Fainelli]

On Wednesday 05 September 2012 10:40:51 Maxime Ripard wrote:
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Acked-by: Florian Fainelli <florian@openwrt.org>

> ---
>  drivers/gpio/gpio-74x164.c |    8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
> index 2975036..604b998 100644
> --- a/drivers/gpio/gpio-74x164.c
> +++ b/drivers/gpio/gpio-74x164.c
> @@ -90,7 +90,7 @@ static int __devinit gen_74x164_probe(struct spi_device 
*spi)
>  	if (ret < 0)
>  		return ret;
>  
> -	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
> +	chip = devm_kzalloc(&spi->dev, sizeof(*chip), GFP_KERNEL);
>  	if (!chip)
>  		return -ENOMEM;
>  
> @@ -125,7 +125,6 @@ static int __devinit gen_74x164_probe(struct spi_device 
*spi)
>  exit_destroy:
>  	dev_set_drvdata(&spi->dev, NULL);
>  	mutex_destroy(&chip->lock);
> -	kfree(chip);
>  	return ret;
>  }
>  
> @@ -141,10 +140,9 @@ static int __devexit gen_74x164_remove(struct 
spi_device *spi)
>  	dev_set_drvdata(&spi->dev, NULL);
>  
>  	ret = gpiochip_remove(&chip->gpio_chip);
> -	if (!ret) {
> +	if (!ret)
>  		mutex_destroy(&chip->lock);
> -		kfree(chip);
> -	} else
> +	else
>  		dev_err(&spi->dev, "Failed to remove the GPIO controller: %d\n",
>  				ret);
>  
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/8] gpio: 74x164: Use devm_kzalloc

[Thomas Petazzoni]

Le Wed,  5 Sep 2012 10:40:51 +0200,
Maxime Ripard <maxime.ripard@free-electrons.com> a ?crit :

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 2/8] gpio: 74x164: Use devm_kzalloc

[Linus Walleij]

On Wed, Sep 5, 2012 at 10:40 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/gpio/gpio-74x164.c |    8 +++-----

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 2/8] gpio: 74x164: Use devm_kzalloc

[Linus Walleij]

On Wed, Sep 5, 2012 at 10:40 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Applied.

Thanks,
Linus Walleij

[PATCH 3/8] gpio: 74x164: Remove platform data and use dynamic gpio number assignment

[Maxime Ripard]

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpio/gpio-74x164.c |   10 +---------
 include/linux/spi/74x164.h |    9 ---------
 2 files changed, 1 insertion(+), 18 deletions(-)
 delete mode 100644 include/linux/spi/74x164.h

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index 604b998..24cfe4c 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -12,7 +12,6 @@
 #include <linux/init.h>
 #include <linux/mutex.h>
 #include <linux/spi/spi.h>
-#include <linux/spi/74x164.h>
 #include <linux/gpio.h>
 #include <linux/slab.h>
 #include <linux/module.h>
@@ -72,15 +71,8 @@ static int gen_74x164_direction_output(struct gpio_chip *gc,
 static int __devinit gen_74x164_probe(struct spi_device *spi)
 {
 	struct gen_74x164_chip *chip;
-	struct gen_74x164_chip_platform_data *pdata;
 	int ret;
 
-	pdata = spi->dev.platform_data;
-	if (!pdata || !pdata->base) {
-		dev_dbg(&spi->dev, "incorrect or missing platform data\n");
-		return -EINVAL;
-	}
-
 	/*
 	 * bits_per_word cannot be configured in platform data
 	 */
@@ -104,7 +96,7 @@ 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.base = pdata->base;
+	chip->gpio_chip.base = -1;
 	chip->gpio_chip.ngpio = 8;
 	chip->gpio_chip.can_sleep = 1;
 	chip->gpio_chip.dev = &spi->dev;
diff --git a/include/linux/spi/74x164.h b/include/linux/spi/74x164.h
deleted file mode 100644
index 0aa6acc..0000000
--- a/include/linux/spi/74x164.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef LINUX_SPI_74X164_H
-#define LINUX_SPI_74X164_H
-
-struct gen_74x164_chip_platform_data {
-	/* number assigned to the first GPIO */
-	unsigned	base;
-};
-
-#endif
-- 
1.7.9.5

[PATCH 3/8] gpio: 74x164: Remove platform data and use dynamic gpio number assignment

[Florian Fainelli]

On Wednesday 05 September 2012 10:40:52 Maxime Ripard wrote:
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

No, please don't do it that way, rather check if the platform_data pointer is 
null, and if it is, fall-back to dynamic gpio base assignment. There is a 
currently out of tree user of this for which it matters to be able to set a 
specific gpio base numbering.

> ---
>  drivers/gpio/gpio-74x164.c |   10 +---------
>  include/linux/spi/74x164.h |    9 ---------
>  2 files changed, 1 insertion(+), 18 deletions(-)
>  delete mode 100644 include/linux/spi/74x164.h
> 
> diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
> index 604b998..24cfe4c 100644
> --- a/drivers/gpio/gpio-74x164.c
> +++ b/drivers/gpio/gpio-74x164.c
> @@ -12,7 +12,6 @@
>  #include <linux/init.h>
>  #include <linux/mutex.h>
>  #include <linux/spi/spi.h>
> -#include <linux/spi/74x164.h>
>  #include <linux/gpio.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> @@ -72,15 +71,8 @@ static int gen_74x164_direction_output(struct gpio_chip 
*gc,
>  static int __devinit gen_74x164_probe(struct spi_device *spi)
>  {
>  	struct gen_74x164_chip *chip;
> -	struct gen_74x164_chip_platform_data *pdata;
>  	int ret;
>  
> -	pdata = spi->dev.platform_data;
> -	if (!pdata || !pdata->base) {
> -		dev_dbg(&spi->dev, "incorrect or missing platform data\n");
> -		return -EINVAL;
> -	}
> -
>  	/*
>  	 * bits_per_word cannot be configured in platform data
>  	 */
> @@ -104,7 +96,7 @@ 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.base = pdata->base;
> +	chip->gpio_chip.base = -1;
>  	chip->gpio_chip.ngpio = 8;
>  	chip->gpio_chip.can_sleep = 1;
>  	chip->gpio_chip.dev = &spi->dev;
> diff --git a/include/linux/spi/74x164.h b/include/linux/spi/74x164.h
> deleted file mode 100644
> index 0aa6acc..0000000
> --- a/include/linux/spi/74x164.h
> +++ /dev/null
> @@ -1,9 +0,0 @@
> -#ifndef LINUX_SPI_74X164_H
> -#define LINUX_SPI_74X164_H
> -
> -struct gen_74x164_chip_platform_data {
> -	/* number assigned to the first GPIO */
> -	unsigned	base;
> -};
> -
> -#endif
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/8] gpio: 74x164: Remove platform data and use dynamic gpio number assignment

[Thomas Petazzoni]

Le Wed, 05 Sep 2012 10:48:14 +0200,
Florian Fainelli <florian@openwrt.org> a ?crit :

> On Wednesday 05 September 2012 10:40:52 Maxime Ripard wrote:
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> No, please don't do it that way, rather check if the platform_data pointer is 
> null, and if it is, fall-back to dynamic gpio base assignment. There is a 
> currently out of tree user of this for which it matters to be able to set a 
> specific gpio base numbering.

Do we care about out of tree users?

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 3/8] gpio: 74x164: Remove platform data and use dynamic gpio number assignment

[Thomas Petazzoni]

Le Wed,  5 Sep 2012 10:40:52 +0200,
Maxime Ripard <maxime.ripard@free-electrons.com> a ?crit :

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

You should probably explicitly mention that the platform_data thing is
removed because it has no users in-tree.

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 4/8] gpio: 74x164: Add device tree support

[Maxime Ripard]

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpio/gpio-74x164.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index 24cfe4c..613067c 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -141,10 +141,17 @@ static int __devexit gen_74x164_remove(struct spi_device *spi)
 	return ret;
 }
 
+static const struct of_device_id gen_74x164_dt_ids[] = {
+	{ .compatible = "fairchild,74hc595" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, gen_74x164_dt_ids);
+
 static struct spi_driver gen_74x164_driver = {
 	.driver = {
 		.name		= "74x164",
 		.owner		= THIS_MODULE,
+		.of_match_table	= of_match_ptr(gen_74x164_dt_ids),
 	},
 	.probe		= gen_74x164_probe,
 	.remove		= __devexit_p(gen_74x164_remove),
-- 
1.7.9.5

[PATCH 4/8] gpio: 74x164: Add device tree support

[Florian Fainelli]

On Wednesday 05 September 2012 10:40:53 Maxime Ripard wrote:
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Acked-by: Florian Fainelli <florian@openwrt.org>

> ---
>  drivers/gpio/gpio-74x164.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
> index 24cfe4c..613067c 100644
> --- a/drivers/gpio/gpio-74x164.c
> +++ b/drivers/gpio/gpio-74x164.c
> @@ -141,10 +141,17 @@ static int __devexit gen_74x164_remove(struct 
spi_device *spi)
>  	return ret;
>  }
>  
> +static const struct of_device_id gen_74x164_dt_ids[] = {
> +	{ .compatible = "fairchild,74hc595" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, gen_74x164_dt_ids);
> +
>  static struct spi_driver gen_74x164_driver = {
>  	.driver = {
>  		.name		= "74x164",
>  		.owner		= THIS_MODULE,
> +		.of_match_table	= of_match_ptr(gen_74x164_dt_ids),
>  	},
>  	.probe		= gen_74x164_probe,
>  	.remove		= __devexit_p(gen_74x164_remove),
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/8] gpio: 74x164: Add device tree support

[Thomas Petazzoni]

Le Wed,  5 Sep 2012 10:40:53 +0200,
Maxime Ripard <maxime.ripard@free-electrons.com> a ?crit :

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 4/8] gpio: 74x164: Add device tree support

[Linus Walleij]

On Wed, Sep 5, 2012 at 10:40 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/gpio/gpio-74x164.c |    7 +++++++

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 5/8] gpio: 74x164: Add output pin support

[Maxime Ripard]

The shift registers have an output that, when enabled, propagates the
values of its internal register to the pins. Make use of this pin
through the output-latch-gpio dt property.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpio/gpio-74x164.c |   32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index 613067c..ff8d3d8 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -13,6 +13,7 @@
 #include <linux/mutex.h>
 #include <linux/spi/spi.h>
 #include <linux/gpio.h>
+#include <linux/of_gpio.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 
@@ -21,6 +22,7 @@ struct gen_74x164_chip {
 	struct gpio_chip	gpio_chip;
 	struct mutex		lock;
 	u8			port_config;
+	int			chip_select;
 };
 
 static struct gen_74x164_chip *gpio_to_74x164_chip(struct gpio_chip *gc)
@@ -30,8 +32,14 @@ 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,
+	int ret;
+
+	gpio_set_value(chip->chip_select, 0);
+	ret = spi_write(chip->spi,
 			 &chip->port_config, sizeof(chip->port_config));
+	gpio_set_value(chip->chip_select, 1);
+
+	return ret;
 }
 
 static int gen_74x164_get_value(struct gpio_chip *gc, unsigned offset)
@@ -73,6 +81,11 @@ static int __devinit gen_74x164_probe(struct spi_device *spi)
 	struct gen_74x164_chip *chip;
 	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
 	 */
@@ -102,6 +115,23 @@ static int __devinit gen_74x164_probe(struct spi_device *spi)
 	chip->gpio_chip.dev = &spi->dev;
 	chip->gpio_chip.owner = THIS_MODULE;
 
+	chip->chip_select = of_get_named_gpio(spi->dev.of_node, "output-latch-gpio", 0);
+	if (gpio_is_valid(chip->chip_select)) {
+		int flags = GPIOF_OUT_INIT_HIGH;
+		if (of_get_property(spi->dev.of_node, "latch-active-low", NULL))
+			flags = GPIOF_OUT_INIT_LOW;
+		ret = devm_gpio_request_one(&spi->dev, chip->chip_select,
+					    flags, "output-latch");
+		if (ret) {
+			dev_err(&spi->dev,
+				"failed to request gpio %d: %d\n",
+				chip->chip_select, ret);
+			goto exit_destroy;
+		}
+	}
+
+	gpio_set_value(chip->chip_select, 1);
+
 	ret = __gen_74x164_write_config(chip);
 	if (ret) {
 		dev_err(&spi->dev, "Failed writing: %d\n", ret);
-- 
1.7.9.5

[PATCH 5/8] gpio: 74x164: Add output pin support

[Thomas Petazzoni]

Le Wed,  5 Sep 2012 10:40:54 +0200,
Maxime Ripard <maxime.ripard@free-electrons.com> a ?crit :

> The shift registers have an output that, when enabled, propagates the
> values of its internal register to the pins. Make use of this pin
> through the output-latch-gpio dt property.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Well, thinking more about this, and reviewing the i.MX 28 datasheet a
bit more thoroughly, I don't think you need this GPIO thing. It is an
active-low pin that should be kept low during the entire duration of
the transfer, which basically is a chip select. And in turns out that
the CFA10049 board uses pins that can be controlled as chip selects
directly by the SPI controller, so you should rather use this rather
than introduce a specially handled GPIO. This needs testing, but I
don't see why it wouldn't work.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 5/8] gpio: 74x164: Add output pin support

[Eric Bénard]

Hi Thomas,

Le Wed, 5 Sep 2012 11:20:12 +0200,
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> a ?crit :

> Le Wed,  5 Sep 2012 10:40:54 +0200,
> Maxime Ripard <maxime.ripard@free-electrons.com> a ?crit :
> 
> > The shift registers have an output that, when enabled, propagates the
> > values of its internal register to the pins. Make use of this pin
> > through the output-latch-gpio dt property.
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> Well, thinking more about this, and reviewing the i.MX 28 datasheet a
> bit more thoroughly, I don't think you need this GPIO thing. It is an
> active-low pin that should be kept low during the entire duration of
> the transfer, which basically is a chip select. And in turns out that
> the CFA10049 board uses pins that can be controlled as chip selects
> directly by the SPI controller, so you should rather use this rather
> than introduce a specially handled GPIO. This needs testing, but I
> don't see why it wouldn't work.
> 
are you talking of the /OE pin of the 74HC595 ?

Eric

[PATCH 5/8] gpio: 74x164: Add output pin support

[Thomas Petazzoni]

Le Wed, 5 Sep 2012 11:46:45 +0200,
Eric B?nard <eric@eukrea.com> a ?crit :

> > Well, thinking more about this, and reviewing the i.MX 28 datasheet a
> > bit more thoroughly, I don't think you need this GPIO thing. It is an
> > active-low pin that should be kept low during the entire duration of
> > the transfer, which basically is a chip select. And in turns out that
> > the CFA10049 board uses pins that can be controlled as chip selects
> > directly by the SPI controller, so you should rather use this rather
> > than introduce a specially handled GPIO. This needs testing, but I
> > don't see why it wouldn't work.
> > 
> are you talking of the /OE pin of the 74HC595 ?

No. Our chip select lines are connected to the STCP pin. It is
technically not a chip select, but a low-to-high transition on this pin
triggers the apparition on the output pins of the 74HC595 of the
contents of the storage register. So, doing a high-to-low transition
before the SPI transfer, and then a low-to-high transition after the
SPI transfer will actually do what we want. But maybe this is an abuse
of the chip select concept, I don't know.

I don't think the /OE pin can be used as a chip select, because when it
is high, the state of the output pins is not maintained to their
previous state: the pins are turned into the high impedance state. If
used as a chip select, it would mean that the value of the output pins
transition from their old state to high impedance at the beginning of
the transfer, and then from the high impedance to their new state at
the end of the transfer. And of course, this signal is inverted
compared to a normal chip select.

Note that I may be completely wrong, I'm a software guy, not a hardware
one :)

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 5/8] gpio: 74x164: Add output pin support

[Eric Bénard]

Hi,

Le Wed, 5 Sep 2012 12:09:06 +0200,
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> a ?crit :
> I don't think the /OE pin can be used as a chip select, because when it
> is high, the state of the output pins is not maintained to their
> previous state: the pins are turned into the high impedance state. If
> used as a chip select, it would mean that the value of the output pins
> transition from their old state to high impedance at the beginning of
> the transfer, and then from the high impedance to their new state at
> the end of the transfer.

OK that's exactly what I was thinking to ;-)

> And of course, this signal is inverted compared to a normal chip select.

polarity of the chip select can vary (mc13xxx-spi has an active high CS
for example).

Eric

[PATCH 5/8] gpio: 74x164: Add output pin support

[Thomas Petazzoni]

Le Wed, 5 Sep 2012 12:26:52 +0200,
Eric B?nard <eric@eukrea.com> a ?crit :

> Le Wed, 5 Sep 2012 12:09:06 +0200,
> Thomas Petazzoni <thomas.petazzoni@free-electrons.com> a ?crit :
> > I don't think the /OE pin can be used as a chip select, because when it
> > is high, the state of the output pins is not maintained to their
> > previous state: the pins are turned into the high impedance state. If
> > used as a chip select, it would mean that the value of the output pins
> > transition from their old state to high impedance at the beginning of
> > the transfer, and then from the high impedance to their new state at
> > the end of the transfer.
> 
> OK that's exactly what I was thinking to ;-)

Good. So, do you think it's reasonable to use the STCP as a chip-select
for this device?

> > And of course, this signal is inverted compared to a normal chip select.
> 
> polarity of the chip select can vary (mc13xxx-spi has an active high CS
> for example).

Sure, but I guess it depends on capabilities of the SPI controller in
your SoC. I haven't seen a way of configuring the polarity of the chip
selects in the i.MX28 SPI controller, but maybe I've missed it.
Anyway, /OE isn't usable as a chip-select, so we don't care.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 5/8] gpio: 74x164: Add output pin support

[Eric Bénard]

Le Wed, 5 Sep 2012 13:56:46 +0200,
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> a ?crit :

> Le Wed, 5 Sep 2012 12:26:52 +0200,
> Eric B?nard <eric@eukrea.com> a ?crit :
> 
> > Le Wed, 5 Sep 2012 12:09:06 +0200,
> > Thomas Petazzoni <thomas.petazzoni@free-electrons.com> a ?crit :
> > > I don't think the /OE pin can be used as a chip select, because when it
> > > is high, the state of the output pins is not maintained to their
> > > previous state: the pins are turned into the high impedance state. If
> > > used as a chip select, it would mean that the value of the output pins
> > > transition from their old state to high impedance at the beginning of
> > > the transfer, and then from the high impedance to their new state at
> > > the end of the transfer.
> > 
> > OK that's exactly what I was thinking to ;-)
> 
> Good. So, do you think it's reasonable to use the STCP as a chip-select
> for this device?
> 
in your case maybe but that really depends on how the chip is wired to
the CPU so I'm not sure that can be a generic choice.

> > > And of course, this signal is inverted compared to a normal chip select.
> > 
> > polarity of the chip select can vary (mc13xxx-spi has an active high CS
> > for example).
> 
> Sure, but I guess it depends on capabilities of the SPI controller in
> your SoC. I haven't seen a way of configuring the polarity of the chip
> selects in the i.MX28 SPI controller, but maybe I've missed it.
> Anyway, /OE isn't usable as a chip-select, so we don't care.
> 
that's a reason why the chip select can be a simple gpio (even if
it's wired on a SPI chip select capable pin)

Eric

[PATCH 5/8] gpio: 74x164: Add output pin support

[Thomas Petazzoni]

Le Wed, 5 Sep 2012 14:22:44 +0200,
Eric B?nard <eric@eukrea.com> a ?crit :

> > > OK that's exactly what I was thinking to ;-)
> > 
> > Good. So, do you think it's reasonable to use the STCP as a chip-select
> > for this device?
>
> in your case maybe but that really depends on how the chip is wired to
> the CPU so I'm not sure that can be a generic choice.

I'm not sure to see how this chip could be wired differently to the
CPU. The CPU must do a low-to-high transition on STCP to get the values
out from the internal register.

So, in other words, we don't know how to choose between:

 *) Keep the current solution Maxime has implemented, where we have a
    specific latch GPIO property in the Device Tree for this 74HC595,
    and the 74HC595 driver is responsible for toggling this GPIO when
    needed.

 *) Use the internal SPI controller logic to control this pin as a
    chip-select, and therefore get rid of Maxime's code with the
    specific latch GPIO property in the Device Tree.

Thoughts?

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 5/8] gpio: 74x164: Add output pin support

[Eric Bénard]

Le Wed, 5 Sep 2012 14:29:52 +0200,
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> a ?crit :

> Le Wed, 5 Sep 2012 14:22:44 +0200,
> Eric B?nard <eric@eukrea.com> a ?crit :
> 
> > > > OK that's exactly what I was thinking to ;-)
> > > 
> > > Good. So, do you think it's reasonable to use the STCP as a chip-select
> > > for this device?
> >
> > in your case maybe but that really depends on how the chip is wired to
> > the CPU so I'm not sure that can be a generic choice.
> 
> I'm not sure to see how this chip could be wired differently to the
> CPU. The CPU must do a low-to-high transition on STCP to get the values
> out from the internal register.
> 
> So, in other words, we don't know how to choose between:
> 
>  *) Keep the current solution Maxime has implemented, where we have a
>     specific latch GPIO property in the Device Tree for this 74HC595,
>     and the 74HC595 driver is responsible for toggling this GPIO when
>     needed.
> 
>  *) Use the internal SPI controller logic to control this pin as a
>     chip-select, and therefore get rid of Maxime's code with the
>     specific latch GPIO property in the Device Tree.
> 
daisy chaining won't work in the second case (I may be wrong but IIRC
the chip select will toggle at each spi_write) unless you configure the
spi controller to send a 8x(number of 74HC595) bits word :

 	gpio_set_value(chip->chip_select, 0);
	for (i = chip->registers - 1; i >= 0; i--) {
		ret = spi_write(chip->spi,
				chip->buffer + i, sizeof(u8));
		if (ret)
			return ret;
	}

 	gpio_set_value(chip->chip_select, 1);

Eric

[PATCH 5/8] gpio: 74x164: Add output pin support

[Maxime Ripard]

Le 05/09/2012 14:54, Eric B?nard a ?crit :
> Le Wed, 5 Sep 2012 14:29:52 +0200,
> Thomas Petazzoni <thomas.petazzoni@free-electrons.com> a ?crit :
> 
>> Le Wed, 5 Sep 2012 14:22:44 +0200,
>> Eric B?nard <eric@eukrea.com> a ?crit :
>>
>>>>> OK that's exactly what I was thinking to ;-)
>>>>
>>>> Good. So, do you think it's reasonable to use the STCP as a chip-select
>>>> for this device?
>>>
>>> in your case maybe but that really depends on how the chip is wired to
>>> the CPU so I'm not sure that can be a generic choice.
>>
>> I'm not sure to see how this chip could be wired differently to the
>> CPU. The CPU must do a low-to-high transition on STCP to get the values
>> out from the internal register.
>>
>> So, in other words, we don't know how to choose between:
>>
>>  *) Keep the current solution Maxime has implemented, where we have a
>>     specific latch GPIO property in the Device Tree for this 74HC595,
>>     and the 74HC595 driver is responsible for toggling this GPIO when
>>     needed.
>>
>>  *) Use the internal SPI controller logic to control this pin as a
>>     chip-select, and therefore get rid of Maxime's code with the
>>     specific latch GPIO property in the Device Tree.
>>
> daisy chaining won't work in the second case (I may be wrong but IIRC
> the chip select will toggle at each spi_write) unless you configure the
> spi controller to send a 8x(number of 74HC595) bits word :
> 
>  	gpio_set_value(chip->chip_select, 0);
> 	for (i = chip->registers - 1; i >= 0; i--) {
> 		ret = spi_write(chip->spi,
> 				chip->buffer + i, sizeof(u8));
> 		if (ret)
> 			return ret;
> 	}
> 
>  	gpio_set_value(chip->chip_select, 1);

And what about using the spi_message structure and one single spi_sync
instead of several spi_write ? Would that work in our case ?


-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 5/8] gpio: 74x164: Add output pin support

[Eric Bénard]

Le Wed, 05 Sep 2012 15:02:09 +0200,
Maxime Ripard <maxime.ripard@free-electrons.com> a ?crit :
> And what about using the spi_message structure and one single spi_sync
> instead of several spi_write ? Would that work in our case ?
> 
if you are sure that the controler will keep the chip select low during
the whole transfer that should work (that means on some spi controllers
you will have to configure the chip select as a plain gpio depending on
the number of buffer you chain).

Eric

[PATCH 6/8] gpio: 74x164: Add support for daisy-chaining

[Maxime Ripard]

These types of shift registers can be daisy-chained, each new value sent
while the output pin is kept low will make the previous value shift to
the next register in the chain.

This patch add support for chained shift registers, through the
registers-number dt property.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpio/gpio-74x164.c |   42 +++++++++++++++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index ff8d3d8..79b3e10 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -19,9 +19,10 @@
 
 struct gen_74x164_chip {
 	struct spi_device	*spi;
+	u8			*buffer;
 	struct gpio_chip	gpio_chip;
 	struct mutex		lock;
-	u8			port_config;
+	u32			registers;
 	int			chip_select;
 };
 
@@ -32,23 +33,31 @@ static struct gen_74x164_chip *gpio_to_74x164_chip(struct gpio_chip *gc)
 
 static int __gen_74x164_write_config(struct gen_74x164_chip *chip)
 {
-	int ret;
+	int i, ret;
 
 	gpio_set_value(chip->chip_select, 0);
-	ret = spi_write(chip->spi,
-			 &chip->port_config, sizeof(chip->port_config));
+
+	for (i = chip->registers - 1; i >= 0; i--) {
+		ret = spi_write(chip->spi,
+				chip->buffer + i, sizeof(u8));
+		if (ret)
+			return ret;
+	}
+
 	gpio_set_value(chip->chip_select, 1);
 
-	return ret;
+	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;
@@ -58,12 +67,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);
@@ -110,7 +121,20 @@ static int __devinit gen_74x164_probe(struct spi_device *spi)
 	chip->gpio_chip.get = gen_74x164_get_value;
 	chip->gpio_chip.set = gen_74x164_set_value;
 	chip->gpio_chip.base = -1;
-	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 = 8 * 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

[PATCH 6/8] gpio: 74x164: Add support for daisy-chaining

[Thomas Petazzoni]

Le Wed,  5 Sep 2012 10:40:55 +0200,
Maxime Ripard <maxime.ripard@free-electrons.com> a ?crit :

> +	for (i = chip->registers - 1; i >= 0; i--) {
> +		ret = spi_write(chip->spi,
> +				chip->buffer + i, sizeof(u8));
> +		if (ret)
> +			return ret;
> +	}

I think you should have a comment on top of this loop to explain why
this loop is done in the reverse direction. Of course, I personally
know why, but the reviewers/readers may not.

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

	ret = (chip->buffer[bank] >> pin) & 0x1;

>  {
>  	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);

		chip->buffer[bank] |= (1 << pin)

>  	else
> -		chip->port_config &= ~(1 << offset);
> +		*(chip->buffer + bank) &= ~(1 << pin);

		chip->buffer[bank] &= ~(1 << pin)


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

I'm wondering whether this shouldn't be named daisy-chain-length or
something like that, but I'm not sure.


> +	chip->gpio_chip.ngpio = 8 * chip->registers;

Maybe make "8" a #define so that it becomes self-explanatory what this
8 is?

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 7/8] gpio: 74x164: dts: Add documentation for the dt binding

[Maxime Ripard]

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 .../devicetree/bindings/gpio/gpio-74x164.txt       |   26 ++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-74x164.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-74x164.txt b/Documentation/devicetree/bindings/gpio/gpio-74x164.txt
new file mode 100644
index 0000000..e2a40a5
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-74x164.txt
@@ -0,0 +1,26 @@
+* Generic 8-bits shift register GPIO driver
+
+Required properties:
+- compatible : Should be "fairchild,74hc595"
+- reg : chip select number
+- gpio-controller : Marks the device node as a gpio controller.
+- #gpio-cells : Should be two.  The first cell is the pin number and
+  the second cell is used to specify the gpio polarity:
+      0 = active high
+      1 = active low
+- register-number: Number of daisy-chained shift registers
+- output-latch-gpio: GPIO connected to the output pin of the registers
+
+Optional properties:
+- latch-active-low: If the output pin is active low
+
+Example:
+
+gpio5: gpio5 at 0 {
+	compatible = "fairchild,74hc595";
+	gpio-controller;
+	#gpio-cells = <2>;
+	registers-number = <4>;
+	output-latch-gpio = <&gpio0 26 0>;
+	spi-max-frequency = <100000>;
+};
-- 
1.7.9.5

[PATCH 8/8] ARM: dts: cfa10049: Add the 74HC595 gpio expanders

[Maxime Ripard]

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/imx28-cfa10049.dts |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/imx28-cfa10049.dts b/arch/arm/boot/dts/imx28-cfa10049.dts
index 1f7fa50..820153a 100644
--- a/arch/arm/boot/dts/imx28-cfa10049.dts
+++ b/arch/arm/boot/dts/imx28-cfa10049.dts
@@ -39,6 +39,26 @@
 				pinctrl-names = "default";
 				pinctrl-0 = <&spi3_pins_cfa10049>;
 				status = "okay";
+
+				gpio5: gpio5 at 0 {
+					compatible = "fairchild,74hc595";
+					gpio-controller;
+					#gpio-cells = <2>;
+					reg = <0>;
+					registers-number = <4>;
+					output-latch-gpio = <&gpio0 26 0>;
+					spi-max-frequency = <100000>;
+				};
+
+				gpio6: gpio6 at 1 {
+					compatible = "fairchild,74hc595";
+					gpio-controller;
+					#gpio-cells = <2>;
+					reg = <1>;
+					registers-number = <2>;
+					output-latch-gpio = <&gpio0 17 0>;
+					spi-max-frequency = <100000>;
+				};
 			};
 		};
 
-- 
1.7.9.5

[PATCH 1/8] gpio: 74x164: Use module_spi_driver boiler plate function

[Florian Fainelli]

On Wednesday 05 September 2012 10:40:50 Maxime Ripard wrote:
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Acked-by: Florian Fainelli <florian@openwrt.org>

> ---
>  drivers/gpio/gpio-74x164.c |   13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
> index a31ad6f..2975036 100644
> --- a/drivers/gpio/gpio-74x164.c
> +++ b/drivers/gpio/gpio-74x164.c
> @@ -159,18 +159,7 @@ static struct spi_driver gen_74x164_driver = {
>  	.probe		= gen_74x164_probe,
>  	.remove		= __devexit_p(gen_74x164_remove),
>  };
> -
> -static int __init gen_74x164_init(void)
> -{
> -	return spi_register_driver(&gen_74x164_driver);
> -}
> -subsys_initcall(gen_74x164_init);
> -
> -static void __exit gen_74x164_exit(void)
> -{
> -	spi_unregister_driver(&gen_74x164_driver);
> -}
> -module_exit(gen_74x164_exit);
> +module_spi_driver(gen_74x164_driver);
>  
>  MODULE_AUTHOR("Gabor Juhos <juhosg@openwrt.org>");
>  MODULE_AUTHOR("Miguel Gaio <miguel.gaio@efixo.com>");
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/8] gpio: 74x164: Use module_spi_driver boiler plate function

[Thomas Petazzoni]

Le Wed,  5 Sep 2012 10:40:50 +0200,
Maxime Ripard <maxime.ripard@free-electrons.com> a ?crit :

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 1/8] gpio: 74x164: Use module_spi_driver boiler plate function

[Linus Walleij]

On Wed, Sep 5, 2012 at 10:40 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/gpio/gpio-74x164.c |   13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)

Acked-by: Linus Walleij <linus.walleij@linaro.org>

I guess all of these will go through your kernel tree?
Tell me if you want me to apply these to my GPIO tree.

Yours,
Linus Walleij

[PATCH 1/8] gpio: 74x164: Use module_spi_driver boiler plate function

[Maxime Ripard]

Hi Linus,

Le 06/09/2012 09:21, Linus Walleij a ?crit :
> On Wed, Sep 5, 2012 at 10:40 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> 
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> ---
>>  drivers/gpio/gpio-74x164.c |   13 +------------
>>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> I guess all of these will go through your kernel tree?
> Tell me if you want me to apply these to my GPIO tree.

Since I don't have any kernel tree, I guess you can apply them in your
tree :)

-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 1/8] gpio: 74x164: Use module_spi_driver boiler plate function

[Linus Walleij]

On Thu, Sep 6, 2012 at 4:10 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi Linus,
> Le 06/09/2012 09:21, Linus Walleij a ?crit :
>>
>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> I guess all of these will go through your kernel tree?
>> Tell me if you want me to apply these to my GPIO tree.
>
> Since I don't have any kernel tree, I guess you can apply them in your
> tree :)

I was more thinking if the Free Electrons people were doing
composite pull requests to ARM SoC. Oh well, nevermind,
I'll take them.

Applied!

Linus Walleij