* [PATCH 1/2] devicetree: gpio: 74x164: optional enable-gpios
@ 2017-08-07 12:27 Peng Fan
2017-08-07 12:27 ` [PATCH 2/2] gpio: 74x164: handling enable-gpios Peng Fan
0 siblings, 1 reply; 5+ messages in thread
From: Peng Fan @ 2017-08-07 12:27 UTC (permalink / raw)
To: linus.walleij, robh; +Cc: linux-gpio, devicetree, linux-kernel, Peng Fan
There is an OE(low active) input pin for 74hc595 and 74lv595.
To some boards, the OE pin is controlled by GPIO, so add an
optional property "enable-gpios".
Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
---
Documentation/devicetree/bindings/gpio/gpio-74x164.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-74x164.txt b/Documentation/devicetree/bindings/gpio/gpio-74x164.txt
index ce1b223..c878834 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-74x164.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-74x164.txt
@@ -12,6 +12,9 @@ Required properties:
1 = active low
- registers-number: Number of daisy-chained shift registers
+Optional properties:
+- enable-gpios: Specifier of the GPIO connected to OE pin
+
Example:
gpio5: gpio5@0 {
--
2.6.6
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] gpio: 74x164: handling enable-gpios
2017-08-07 12:27 [PATCH 1/2] devicetree: gpio: 74x164: optional enable-gpios Peng Fan
@ 2017-08-07 12:27 ` Peng Fan
2017-08-07 12:36 ` Fabio Estevam
2017-08-07 12:40 ` Fabio Estevam
0 siblings, 2 replies; 5+ messages in thread
From: Peng Fan @ 2017-08-07 12:27 UTC (permalink / raw)
To: linus.walleij, robh; +Cc: linux-gpio, devicetree, linux-kernel, Peng Fan
To 74hc595 and 74lv595, there is an OE(low active) input pin.
To some boards, this pin is controller by GPIO, so handling
this pin in driver. When driver probe, use GPIOD_OUT_LOW flag
when requesting the gpio, so OE is set to low when probe.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpio-74x164.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index a6607fa..e44422c 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -23,6 +23,7 @@ struct gen_74x164_chip {
struct gpio_chip gpio_chip;
struct mutex lock;
u32 registers;
+ struct gpio_desc *enable_gpio;
/*
* Since the registers are chained, every byte sent will make
* the previous byte shift to the next register in the
@@ -142,6 +143,12 @@ static int gen_74x164_probe(struct spi_device *spi)
chip->gpio_chip.parent = &spi->dev;
chip->gpio_chip.owner = THIS_MODULE;
+ chip->enable_gpio = devm_gpiod_get(&spi->dev, "enable", GPIOD_OUT_LOW);
+ if (IS_ERR(chip->enable_gpio)) {
+ dev_dbg(&spi->dev, "No enable-gpios property\n");
+ chip->enable_gpio = NULL;
+ }
+
mutex_init(&chip->lock);
ret = __gen_74x164_write_config(chip);
@@ -164,6 +171,8 @@ static int gen_74x164_remove(struct spi_device *spi)
{
struct gen_74x164_chip *chip = spi_get_drvdata(spi);
+ if (chip->enable_gpio)
+ gpiod_set_value(chip->enable_gpio, 0);
gpiochip_remove(&chip->gpio_chip);
mutex_destroy(&chip->lock);
--
2.6.6
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] gpio: 74x164: handling enable-gpios
2017-08-07 12:27 ` [PATCH 2/2] gpio: 74x164: handling enable-gpios Peng Fan
@ 2017-08-07 12:36 ` Fabio Estevam
2017-08-07 12:40 ` Fabio Estevam
1 sibling, 0 replies; 5+ messages in thread
From: Fabio Estevam @ 2017-08-07 12:36 UTC (permalink / raw)
To: Peng Fan; +Cc: Linus Walleij, Rob Herring, linux-gpio, devicetree, linux-kernel
Hi Peng,
On Mon, Aug 7, 2017 at 9:27 AM, Peng Fan <peng.fan@nxp.com> wrote:
> To 74hc595 and 74lv595, there is an OE(low active) input pin.
> To some boards, this pin is controller by GPIO, so handling
> this pin in driver. When driver probe, use GPIOD_OUT_LOW flag
> when requesting the gpio, so OE is set to low when probe.
Well, this depends of the GPIO polarity.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/gpio/gpio-74x164.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
> index a6607fa..e44422c 100644
> --- a/drivers/gpio/gpio-74x164.c
> +++ b/drivers/gpio/gpio-74x164.c
> @@ -23,6 +23,7 @@ struct gen_74x164_chip {
> struct gpio_chip gpio_chip;
> struct mutex lock;
> u32 registers;
> + struct gpio_desc *enable_gpio;
> /*
> * Since the registers are chained, every byte sent will make
> * the previous byte shift to the next register in the
> @@ -142,6 +143,12 @@ static int gen_74x164_probe(struct spi_device *spi)
> chip->gpio_chip.parent = &spi->dev;
> chip->gpio_chip.owner = THIS_MODULE;
>
> + chip->enable_gpio = devm_gpiod_get(&spi->dev, "enable", GPIOD_OUT_LOW);
It would be better to use devm_gpiod_get_optional instead.
> + if (IS_ERR(chip->enable_gpio)) {
> + dev_dbg(&spi->dev, "No enable-gpios property\n");
> + chip->enable_gpio = NULL;
> + }
> +
> mutex_init(&chip->lock);
>
> ret = __gen_74x164_write_config(chip);
> @@ -164,6 +171,8 @@ static int gen_74x164_remove(struct spi_device *spi)
> {
> struct gen_74x164_chip *chip = spi_get_drvdata(spi);
>
> + if (chip->enable_gpio)
> + gpiod_set_value(chip->enable_gpio, 0);
It would be better to use the cansleep variant instead.
Actually I have worked on adding this optional property, but haven't
submitted yet:
https://pastebin.com/u839DVD3
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] gpio: 74x164: handling enable-gpios
2017-08-07 12:27 ` [PATCH 2/2] gpio: 74x164: handling enable-gpios Peng Fan
2017-08-07 12:36 ` Fabio Estevam
@ 2017-08-07 12:40 ` Fabio Estevam
2017-08-08 1:15 ` Peng Fan
1 sibling, 1 reply; 5+ messages in thread
From: Fabio Estevam @ 2017-08-07 12:40 UTC (permalink / raw)
To: Peng Fan; +Cc: Linus Walleij, Rob Herring, linux-gpio, devicetree, linux-kernel
On Mon, Aug 7, 2017 at 9:27 AM, Peng Fan <peng.fan@nxp.com> wrote:
> + chip->enable_gpio = devm_gpiod_get(&spi->dev, "enable", GPIOD_OUT_LOW);
> + if (IS_ERR(chip->enable_gpio)) {
> + dev_dbg(&spi->dev, "No enable-gpios property\n");
> + chip->enable_gpio = NULL;
Also, the error handling here is not correct as it will never
propagate EPROBE_DEFER.
I will submit my version of the patch if you don't mind.
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 2/2] gpio: 74x164: handling enable-gpios
2017-08-07 12:40 ` Fabio Estevam
@ 2017-08-08 1:15 ` Peng Fan
0 siblings, 0 replies; 5+ messages in thread
From: Peng Fan @ 2017-08-08 1:15 UTC (permalink / raw)
To: Fabio Estevam
Cc: Linus Walleij, Rob Herring, linux-gpio, devicetree, linux-kernel
> > + chip->enable_gpio = devm_gpiod_get(&spi->dev, "enable",
> GPIOD_OUT_LOW);
> > + if (IS_ERR(chip->enable_gpio)) {
> > + dev_dbg(&spi->dev, "No enable-gpios property\n");
> > + chip->enable_gpio = NULL;
>
> Also, the error handling here is not correct as it will never propagate
> EPROBE_DEFER.
>
> I will submit my version of the patch if you don't mind.
That's ok if you have a better patch.
Regards,
Peng.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-08-08 1:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-07 12:27 [PATCH 1/2] devicetree: gpio: 74x164: optional enable-gpios Peng Fan
2017-08-07 12:27 ` [PATCH 2/2] gpio: 74x164: handling enable-gpios Peng Fan
2017-08-07 12:36 ` Fabio Estevam
2017-08-07 12:40 ` Fabio Estevam
2017-08-08 1:15 ` Peng Fan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).