Skip to contentSkip to conversation

gpio: of: Reflect decoupling of open collector and active low/high

PATCH Applied Charles Keepax 7 participants 14 messages May 23, 2017 – May 30, 2017 4 messages not in the archive

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/5 gpio: of: Reflect decoupling of open collector and active low/high

Commit 4c0facddb7d8 ("gpio: core: Decouple open drain/source flag with active low/high") decoupled the open collector outputs from active low/high but did not update the documentation.

Update the device tree documentation to correctly reflect this new separation between the two concepts.

---

New to the series.

Thanks,
Charles

 Documentation/devicetree/bindings/gpio/gpio.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/gpio/gpio.txt +4 -3
index 2209c16..bccb8623 100644
@@ -74,11 +74,12 @@ GPIO pin number, and GPIO flags as accepted by the "qe_pio_e" gpio-controller.
 Optional standard bitfield specifiers for the last cell:
 
 - Bit 0: 0 means active high, 1 means active low
-- Bit 1: 1 means single-ended wiring, see:
+- Bit 1: 0 mean push-pull wiring, see:
+           https://en.wikipedia.org/wiki/Push-pull_output
+         1 means single-ended wiring, see:
            https://en.wikipedia.org/wiki/Single-ended_triode
-	   When used with active-low, this means open drain/collector, see:
+- Bit 2: 0 means open-source, 1 means open drain, see:
            https://en.wikipedia.org/wiki/Open_collector
-	   When used with active-high, this means open source/emitter
 
 1.1) GPIO specifier best practices
 ----------------------------------
Signature · 6 lines
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#1 lore raw

PATCH v2 2/5 gpio: Add new flags to control sleep status of GPIOs

Add new flags to allow users to specify that they are not concerned with the status of GPIOs whilst in a sleep/low power state.

---

Changes since v1:
- Added much more core support for the SLEEP_MAY_LOOSE_VALUE flag
- Moved dt-binding new defines down a few lines

Rob, I carried your Ack since I assume you only care about the dt-bindings include, which has only had the previous defines moved down a couple of lines. I hope that is ok.

Thanks,
Charles

 drivers/gpio/gpiolib-of.c       |  3 +++
 drivers/gpio/gpiolib.c          | 12 ++++++++++++
 drivers/gpio/gpiolib.h          |  1 +
 include/dt-bindings/gpio/gpio.h |  4 ++++
 include/linux/gpio/driver.h     |  3 +++
 include/linux/gpio/machine.h    |  2 ++
 include/linux/of_gpio.h         |  1 +
 7 files changed, 26 insertions(+)
drivers/gpio/gpiolib-of.c +3 -0
index b13b7c7..e2abf0e 100644
@@ -153,6 +153,9 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 			*flags |= GPIO_OPEN_SOURCE;
 	}
 
+	if (of_flags & OF_GPIO_SLEEP_MAY_LOOSE_VALUE)
+		*flags |= GPIO_SLEEP_MAY_LOOSE_VALUE;
+
 	return desc;
 }
 
drivers/gpio/gpiolib.c +12 -0
index 5db4413..3cdddf4 100644
@@ -2869,6 +2869,16 @@ bool gpiochip_line_is_open_source(struct gpio_chip *chip, unsigned int offset)
 }
 EXPORT_SYMBOL_GPL(gpiochip_line_is_open_source);
 
+bool gpiochip_line_is_persistent(struct gpio_chip *chip, unsigned int offset)
+{
+	if (offset >= chip->ngpio)
+		return false;
+
+	return !test_bit(FLAG_SLEEP_MAY_LOOSE_VALUE,
+			 &chip->gpiodev->descs[offset].flags);
+}
+EXPORT_SYMBOL_GPL(gpiochip_line_is_persistent);
+
 /**
  * gpiod_get_raw_value_cansleep() - return a gpio's raw value
  * @desc: gpio whose value will be returned
@@ -3223,6 +3233,8 @@ static int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
 		set_bit(FLAG_OPEN_DRAIN, &desc->flags);
 	if (lflags & GPIO_OPEN_SOURCE)
 		set_bit(FLAG_OPEN_SOURCE, &desc->flags);
+	if (lflags & GPIO_SLEEP_MAY_LOOSE_VALUE)
+		set_bit(FLAG_SLEEP_MAY_LOOSE_VALUE, &desc->flags);
 
 	/* No particular flag request, return here... */
 	if (!(dflags & GPIOD_FLAGS_BIT_DIR_SET)) {
drivers/gpio/gpiolib.h +1 -0
index 2495b7e..f74b8a7 100644
@@ -190,6 +190,7 @@ struct gpio_desc {
 #define FLAG_OPEN_SOURCE 8	/* Gpio is open source type */
 #define FLAG_USED_AS_IRQ 9	/* GPIO is connected to an IRQ */
 #define FLAG_IS_HOGGED	11	/* GPIO is hogged */
+#define FLAG_SLEEP_MAY_LOOSE_VALUE 12	/* GPIO may loose value in sleep */
 
 	/* Connection label */
 	const char		*label;
include/dt-bindings/gpio/gpio.h +4 -0
index b4f54da..c507458 100644
@@ -28,4 +28,8 @@
 #define GPIO_OPEN_DRAIN (GPIO_SINGLE_ENDED | GPIO_LINE_OPEN_DRAIN)
 #define GPIO_OPEN_SOURCE (GPIO_SINGLE_ENDED | GPIO_LINE_OPEN_SOURCE)
 
+/* Bit 3 express GPIO suspend/resume persistence */
+#define GPIO_SLEEP_MAINTAIN_VALUE 0
+#define GPIO_SLEEP_MAY_LOOSE_VALUE 8
+
 #endif
include/linux/gpio/driver.h +3 -0
index 3935828..af20369 100644
@@ -213,6 +213,9 @@ bool gpiochip_line_is_irq(struct gpio_chip *chip, unsigned int offset);
 bool gpiochip_line_is_open_drain(struct gpio_chip *chip, unsigned int offset);
 bool gpiochip_line_is_open_source(struct gpio_chip *chip, unsigned int offset);
 
+/* Sleep persistence inquiry for drivers */
+bool gpiochip_line_is_persistent(struct gpio_chip *chip, unsigned int offset);
+
 /* get driver data */
 void *gpiochip_get_data(struct gpio_chip *chip);
 
include/linux/gpio/machine.h +2 -0
index c0d712d..13adadf 100644
@@ -9,6 +9,8 @@ enum gpio_lookup_flags {
 	GPIO_ACTIVE_LOW = (1 << 0),
 	GPIO_OPEN_DRAIN = (1 << 1),
 	GPIO_OPEN_SOURCE = (1 << 2),
+	GPIO_SLEEP_MAINTAIN_VALUE = (0 << 3),
+	GPIO_SLEEP_MAY_LOOSE_VALUE = (1 << 3),
 };
 
 /**
include/linux/of_gpio.h +1 -0
index 1e089d5..ca10f43 100644
@@ -31,6 +31,7 @@ enum of_gpio_flags {
 	OF_GPIO_ACTIVE_LOW = 0x1,
 	OF_GPIO_SINGLE_ENDED = 0x2,
 	OF_GPIO_OPEN_DRAIN = 0x4,
+	OF_GPIO_SLEEP_MAY_LOOSE_VALUE = 0x8,
 };
 
 #ifdef CONFIG_OF_GPIO
Signature · 1 line
2.1.4
#3 lore raw

PATCH v2 3/5 gpio: arizona: Add support for GPIOs that need to be maintained

In reply to Charles Keepax

The Arizona devices only maintain the state of output GPIOs whilst the CODEC is active, this can cause issues if the CODEC suspends whilst something is relying on the state of one of its GPIOs. However, in many systems the CODEC GPIOs are used for audio related features and thus the state of the GPIOs is unimportant whilst the CODEC is suspended. Often keeping the CODEC resumed in such a system would incur a power impact that is unacceptable.

Allow the user to select whether a GPIO output should keep the CODEC resumed, by adding a flag through the second cell of the GPIO specifier in device tree.

---

Changes since v1:
- Use the new gpiolib features to determine if the pin is persistent or not.

Thanks,
Charles

 drivers/gpio/gpio-arizona.c | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)
drivers/gpio/gpio-arizona.c +33 -2
index cd23fd7..d4e6ba0 100644
@@ -33,9 +33,23 @@ static int arizona_gpio_direction_in(struct gpio_chip *chip, unsigned offset)
 {
 	struct arizona_gpio *arizona_gpio = gpiochip_get_data(chip);
 	struct arizona *arizona = arizona_gpio->arizona;
+	bool persistent = gpiochip_line_is_persistent(chip, offset);
+	bool change;
+	int ret;
 
-	return regmap_update_bits(arizona->regmap, ARIZONA_GPIO1_CTRL + offset,
-				  ARIZONA_GPN_DIR, ARIZONA_GPN_DIR);
+	ret = regmap_update_bits_check(arizona->regmap,
+				       ARIZONA_GPIO1_CTRL + offset,
+				       ARIZONA_GPN_DIR, ARIZONA_GPN_DIR,
+				       &change);
+	if (ret < 0)
+		return ret;
+
+	if (change && persistent) {
+		pm_runtime_mark_last_busy(chip->parent);
+		pm_runtime_put_autosuspend(chip->parent);
+	}
+
+	return 0;
 }
 
 static int arizona_gpio_get(struct gpio_chip *chip, unsigned offset)
@@ -85,6 +99,21 @@ static int arizona_gpio_direction_out(struct gpio_chip *chip,
 {
 	struct arizona_gpio *arizona_gpio = gpiochip_get_data(chip);
 	struct arizona *arizona = arizona_gpio->arizona;
+	bool persistent = gpiochip_line_is_persistent(chip, offset);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(arizona->regmap, ARIZONA_GPIO1_CTRL + offset, &val);
+	if (ret < 0)
+		return ret;
+
+	if ((val & ARIZONA_GPN_DIR) && persistent) {
+		ret = pm_runtime_get_sync(chip->parent);
+		if (ret < 0) {
+			dev_err(chip->parent, "Failed to resume: %d\n", ret);
+			return ret;
+		}
+	}
 
 	if (value)
 		value = ARIZONA_GPN_LVL;
@@ -158,6 +187,8 @@ static int arizona_gpio_probe(struct platform_device *pdev)
 	else
 		arizona_gpio->gpio_chip.base = -1;
 
+	pm_runtime_enable(&pdev->dev);
+
 	ret = devm_gpiochip_add_data(&pdev->dev, &arizona_gpio->gpio_chip,
 				     arizona_gpio);
 	if (ret < 0) {
Signature · 1 line
2.1.4

A message that is not in the archive (sent off-list or not yet received): 1495550852-3672-3-git-send-email-ckeepax-yzvPICuk2AATkU/[email protected]

#5 lore raw

Re: [PATCH v2 3/5] gpio: arizona: Add support for GPIOs that need to be maintained

Quoted 12 lines · Charles Keepax

The Arizona devices only maintain the state of output GPIOs whilst the CODEC is active, this can cause issues if the CODEC suspends whilst something is relying on the state of one of its GPIOs. However, in many systems the CODEC GPIOs are used for audio related features and thus the state of the GPIOs is unimportant whilst the CODEC is suspended. Often keeping the CODEC resumed in such a system would incur a power impact that is unacceptable.

Allow the user to select whether a GPIO output should keep the CODEC resumed, by adding a flag through the second cell of the GPIO specifier in device tree.

Quoted 6 lines

---

Changes since v1:
- Use the new gpiolib features to determine if the pin is persistent or not.

Patch applied!

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#6 lore raw

PATCH v2 4/5 gpio: of: Add documentation of new sleep standard GPIO specifiers

In reply to Charles Keepax

Add documentation of new GPIO specifiers indicating if the state of an output pin should be maintained during sleep/low-power mode.

---

New to the series.

Thanks,
Charles

 Documentation/devicetree/bindings/gpio/gpio.txt | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/gpio/gpio.txt +2 -0
index bccb8623..802402f 100644
@@ -80,6 +80,8 @@ Optional standard bitfield specifiers for the last cell:
            https://en.wikipedia.org/wiki/Single-ended_triode
 - Bit 2: 0 means open-source, 1 means open drain, see:
            https://en.wikipedia.org/wiki/Open_collector
+- Bit 3: 0 means the output should be maintained during sleep/low-power mode
+         1 means the output state can be lost during sleep/low-power mode
 
 1.1) GPIO specifier best practices
 ----------------------------------
Signature · 1 line
2.1.4
Acked-by on v2 4/5
#7 lore raw

Re: [PATCH v2 4/5] gpio: of: Add documentation of new sleep standard GPIO specifiers

Quoted 5 lines · Charles Keepax

Add documentation of new GPIO specifiers indicating if the state of an output pin should be maintained during sleep/low-power mode.

---

Thanks,
Laxman

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

A message that is not in the archive (sent off-list or not yet received): 1495550852-3672-4-git-send-email-ckeepax-yzvPICuk2AATkU/[email protected]

#9 lore raw

Re: [PATCH v2 4/5] gpio: of: Add documentation of new sleep standard GPIO specifiers

Quoted 4 lines · Charles Keepax

Add documentation of new GPIO specifiers indicating if the state of an output pin should be maintained during sleep/low-power mode.

Patch applied with Laxman's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 1495550852-3672-1-git-send-email-ckeepax-yzvPICuk2AATkU/[email protected]

#11 lore raw

PATCH v2 5/5 mfd: arizona: Update GPIO binding for newly supported specifiers

Link to the generic GPIO specifier bindings now that the second cell of the binding has some support in the driver.

---

Changes since v1:
- Now just points at the standard gpio binding stuff

Thanks,
Charles

 Documentation/devicetree/bindings/mfd/arizona.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/mfd/arizona.txt +2 -1
index 8f2e282..b37bdde 100644
@@ -30,7 +30,8 @@ Required properties:
 
   - gpio-controller : Indicates this device is a GPIO controller.
   - #gpio-cells : Must be 2. The first cell is the pin number and the
-    second cell is used to specify optional parameters (currently unused).
+    second cell is used to specify optional parameters, see ../gpio/gpio.txt
+    for details.
 
   - AVDD-supply, DBVDD1-supply, CPVDD-supply : Power supplies for the device,
     as covered in Documentation/devicetree/bindings/regulator/regulator.txt
Signature · 6 lines
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#12 lore raw

Re: [PATCH v2 5/5] mfd: arizona: Update GPIO binding for newly supported specifiers

Quoted 6 lines · Charles Keepax

Link to the generic GPIO specifier bindings now that the second cell of the binding has some support in the driver.

---

Quoted 8 lines

Changes since v1:
- Now just points at the standard gpio binding stuff

Thanks,
Charles

 Documentation/devicetree/bindings/mfd/arizona.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

I'll take this if/when the GPIO changes are merged.

For my own reference:

Quoted 14 lines
Documentation/devicetree/bindings/mfd/arizona.txt +2 -1
index 8f2e282..b37bdde 100644
@@ -30,7 +30,8 @@ Required properties:
 
   - gpio-controller : Indicates this device is a GPIO controller.
   - #gpio-cells : Must be 2. The first cell is the pin number and the
-    second cell is used to specify optional parameters (currently unused).
+    second cell is used to specify optional parameters, see ../gpio/gpio.txt
+    for details.
 
   - AVDD-supply, DBVDD1-supply, CPVDD-supply : Power supplies for the device,
     as covered in Documentation/devicetree/bindings/regulator/regulator.txt
Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Reviewed-by
#13 lore raw

Re: [PATCH v2 5/5] mfd: arizona: Update GPIO binding for newly supported specifiers

Quoted 14 lines · Lee Jones
Quoted 12 lines · Charles Keepax

Link to the generic GPIO specifier bindings now that the second cell of the binding has some support in the driver.

---

Changes since v1:
- Now just points at the standard gpio binding stuff

Thanks,
Charles

Quoted 7 lines
Quoted 2 lines
 Documentation/devicetree/bindings/mfd/arizona.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

I'll take this if/when the GPIO changes are merged.

For my own reference:

I merged them today.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 1495550852-3672-5-git-send-email-ckeepax-yzvPICuk2AATkU/[email protected]

Applied
#15 lore raw

Re: [PATCH v2 5/5] mfd: arizona: Update GPIO binding for newly supported specifiers

Quoted 6 lines · Charles Keepax

Link to the generic GPIO specifier bindings now that the second cell of the binding has some support in the driver.

---

Quoted 8 lines

Changes since v1:
- Now just points at the standard gpio binding stuff

Thanks,
Charles

 Documentation/devicetree/bindings/mfd/arizona.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Applied, thanks.

Quoted 14 lines
Documentation/devicetree/bindings/mfd/arizona.txt +2 -1
index 8f2e282..b37bdde 100644
@@ -30,7 +30,8 @@ Required properties:
 
   - gpio-controller : Indicates this device is a GPIO controller.
   - #gpio-cells : Must be 2. The first cell is the pin number and the
-    second cell is used to specify optional parameters (currently unused).
+    second cell is used to specify optional parameters, see ../gpio/gpio.txt
+    for details.
 
   - AVDD-supply, DBVDD1-supply, CPVDD-supply : Power supplies for the device,
     as covered in Documentation/devicetree/bindings/regulator/regulator.txt
Signature · 8 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Acked-by on v2 1/5
#16 lore raw
In reply to Charles Keepax
Quoted 9 lines · Charles Keepax

Commit 4c0facddb7d8 ("gpio: core: Decouple open drain/source flag with active low/high") decoupled the open collector outputs from active low/high but did not update the documentation.

Update the device tree documentation to correctly reflect this new separation between the two concepts.

---

Thanks,
Laxman

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

#17 lore raw
In reply to Charles Keepax
Quoted 8 lines · Charles Keepax

Commit 4c0facddb7d8 ("gpio: core: Decouple open drain/source flag with active low/high") decoupled the open collector outputs from active low/high but did not update the documentation.

Update the device tree documentation to correctly reflect this new separation between the two concepts.

Awesome, patch applied.

Yours,
Linus Walleij