| Message ID | 1346834457-6257-5-git-send-email-maxime.ripard@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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
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
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
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
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
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 ?
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
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);
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(-)