Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1618027 > unrolled thread

[PATCH 1/1] gpio: core: Decouple open drain/source flag with active low/high

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/1] gpio: core: Decouple open drain/source flag with active low/high Laxman Dewangan <ldewangan@nvidia.com> - 2017-04-06 16:00 +0200
    Re: [PATCH 1/1] gpio: core: Decouple open drain/source flag with  active low/high Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-04-06 18:20 +0200
      Re: [PATCH 1/1] gpio: core: Decouple open drain/source flag with  active low/high Laxman Dewangan <ldewangan@nvidia.com> - 2017-04-06 19:30 +0200
        Re: [PATCH 1/1] gpio: core: Decouple open drain/source flag with  active low/high Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-04-06 19:40 +0200
    Re: [PATCH 1/1] gpio: core: Decouple open drain/source flag with  active low/high Linus Walleij <linus.walleij@linaro.org> - 2017-04-07 12:30 +0200
      Re: [PATCH 1/1] gpio: core: Decouple open drain/source flag with  active low/high Laxman Dewangan <ldewangan@nvidia.com> - 2017-04-07 12:40 +0200

#1618027 — [PATCH 1/1] gpio: core: Decouple open drain/source flag with active low/high

Currently, the GPIO interface is said to Open Drain if it is Single
Ended and active LOW. Similarly, it is said as Open Source if it is
Single Ended and active HIGH.

The active HIGH/LOW is used in the interface for setting the pin
state to HIGH or LOW when enabling/disabling the interface.

In Open Drain interface, pin is set to HIGH by putting pin in
high impedance and LOW by driving to the LOW.

In Open Source interface, pin is set to HIGH by driving pin to
HIGH and set to LOW by putting pin in high impedance.

With above, the Open Drain/Source is unrelated to the active LOW/HIGH
in interface. There is interface where the enable/disable of interface
is ether active LOW or HIGH but it is Open Drain type.

Hence decouple the Open Drain with Single Ended + Active LOW and
Open Source with Single Ended + Active HIGH.

Adding different flag for the Open Drain/Open Source which is valid
only when Single ended flag is enabled.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/gpio/gpiolib-of.c       |  2 +-
 drivers/gpio/gpiolib.c          |  4 +++-
 include/dt-bindings/gpio/gpio.h | 12 ++++++++----
 include/linux/of_gpio.h         |  1 +
 4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 975b9f6..b13b7c7 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -147,7 +147,7 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 		*flags |= GPIO_ACTIVE_LOW;
 
 	if (of_flags & OF_GPIO_SINGLE_ENDED) {
-		if (of_flags & OF_GPIO_ACTIVE_LOW)
+		if (of_flags & OF_GPIO_OPEN_DRAIN)
 			*flags |= GPIO_OPEN_DRAIN;
 		else
 			*flags |= GPIO_OPEN_SOURCE;
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 4aa1e78..c22e572 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3333,6 +3333,7 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 	unsigned long lflags = 0;
 	bool active_low = false;
 	bool single_ended = false;
+	bool open_drain = false;
 	int ret;
 
 	if (!fwnode)
@@ -3346,6 +3347,7 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 		if (!IS_ERR(desc)) {
 			active_low = flags & OF_GPIO_ACTIVE_LOW;
 			single_ended = flags & OF_GPIO_SINGLE_ENDED;
+			open_drain = flags & OF_GPIO_OPEN_DRAIN;
 		}
 	} else if (is_acpi_node(fwnode)) {
 		struct acpi_gpio_info info;
@@ -3366,7 +3368,7 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 		lflags |= GPIO_ACTIVE_LOW;
 
 	if (single_ended) {
-		if (active_low)
+		if (open_drain)
 			lflags |= GPIO_OPEN_DRAIN;
 		else
 			lflags |= GPIO_OPEN_SOURCE;
diff --git a/include/dt-bindings/gpio/gpio.h b/include/dt-bindings/gpio/gpio.h
index c673d2c..b4f54da 100644
--- a/include/dt-bindings/gpio/gpio.h
+++ b/include/dt-bindings/gpio/gpio.h
@@ -17,11 +17,15 @@
 #define GPIO_PUSH_PULL 0
 #define GPIO_SINGLE_ENDED 2
 
+/* Bit 2 express Open drain or open source */
+#define GPIO_LINE_OPEN_SOURCE 0
+#define GPIO_LINE_OPEN_DRAIN 4
+
 /*
- * Open Drain/Collector is the combination of single-ended active low,
- * Open Source/Emitter is the combination of single-ended active high.
+ * Open Drain/Collector is the combination of single-ended open drain interface.
+ * Open Source/Emitter is the combination of single-ended open source interface.
  */
-#define GPIO_OPEN_DRAIN (GPIO_SINGLE_ENDED | GPIO_ACTIVE_LOW)
-#define GPIO_OPEN_SOURCE (GPIO_SINGLE_ENDED | GPIO_ACTIVE_HIGH)
+#define GPIO_OPEN_DRAIN (GPIO_SINGLE_ENDED | GPIO_LINE_OPEN_DRAIN)
+#define GPIO_OPEN_SOURCE (GPIO_SINGLE_ENDED | GPIO_LINE_OPEN_SOURCE)
 
 #endif
diff --git a/include/linux/of_gpio.h b/include/linux/of_gpio.h
index 3f87ea5..1e089d5 100644
--- a/include/linux/of_gpio.h
+++ b/include/linux/of_gpio.h
@@ -30,6 +30,7 @@ struct device_node;
 enum of_gpio_flags {
 	OF_GPIO_ACTIVE_LOW = 0x1,
 	OF_GPIO_SINGLE_ENDED = 0x2,
+	OF_GPIO_OPEN_DRAIN = 0x4,
 };
 
 #ifdef CONFIG_OF_GPIO
-- 
2.1.4

[toc] | [next] | [standalone]

#1618153 — Re: [PATCH 1/1] gpio: core: Decouple open drain/source flag with active low/high

On Thu, Apr 6, 2017 at 4:35 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> Currently, the GPIO interface is said to Open Drain if it is Single
> Ended and active LOW. Similarly, it is said as Open Source if it is
> Single Ended and active HIGH.
>
> The active HIGH/LOW is used in the interface for setting the pin
> state to HIGH or LOW when enabling/disabling the interface.
>
> In Open Drain interface, pin is set to HIGH by putting pin in
> high impedance and LOW by driving to the LOW.
>
> In Open Source interface, pin is set to HIGH by driving pin to
> HIGH and set to LOW by putting pin in high impedance.
>
> With above, the Open Drain/Source is unrelated to the active LOW/HIGH
> in interface. There is interface where the enable/disable of interface
> is ether active LOW or HIGH but it is Open Drain type.
>
> Hence decouple the Open Drain with Single Ended + Active LOW and
> Open Source with Single Ended + Active HIGH.
>
> Adding different flag for the Open Drain/Open Source which is valid
> only when Single ended flag is enabled.

>         if (single_ended) {
> -               if (active_low)
> +               if (open_drain)

This breaks ACPI case, right?

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1618210 — Re: [PATCH 1/1] gpio: core: Decouple open drain/source flag with active low/high

On Thursday 06 April 2017 09:40 PM, Andy Shevchenko wrote:
> On Thu, Apr 6, 2017 at 4:35 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>> Currently, the GPIO interface is said to Open Drain if it is Single
>> Ended and active LOW. Similarly, it is said as Open Source if it is
>> Single Ended and active HIGH.
>>
>> The active HIGH/LOW is used in the interface for setting the pin
>> state to HIGH or LOW when enabling/disabling the interface.
>>
>> In Open Drain interface, pin is set to HIGH by putting pin in
>> high impedance and LOW by driving to the LOW.
>>
>> In Open Source interface, pin is set to HIGH by driving pin to
>> HIGH and set to LOW by putting pin in high impedance.
>>
>> With above, the Open Drain/Source is unrelated to the active LOW/HIGH
>> in interface. There is interface where the enable/disable of interface
>> is ether active LOW or HIGH but it is Open Drain type.
>>
>> Hence decouple the Open Drain with Single Ended + Active LOW and
>> Open Source with Single Ended + Active HIGH.
>>
>> Adding different flag for the Open Drain/Open Source which is valid
>> only when Single ended flag is enabled.
>>          if (single_ended) {
>> -               if (active_low)
>> +               if (open_drain)
> This breaks ACPI case, right?
>

In acpi case, single_ended is not handled. It only handles the active LOW.

 From code:


        bool active_low = false;
         bool single_ended = false;
         int ret;

         if (!fwnode)
                 return ERR_PTR(-EINVAL);

         if (is_of_node(fwnode)) {
                 enum of_gpio_flags flags;

                 desc = of_get_named_gpiod_flags(to_of_node(fwnode), 
propname,
                                                 index, &flags);
                 if (!IS_ERR(desc)) {
                         active_low = flags & OF_GPIO_ACTIVE_LOW;
                         single_ended = flags & OF_GPIO_SINGLE_ENDED;
                 }
         } else if (is_acpi_node(fwnode)) {
                 struct acpi_gpio_info info;

                 desc = acpi_node_get_gpiod(fwnode, propname, index, &info);
                 if (!IS_ERR(desc))
                         active_low = info.polarity == GPIO_ACTIVE_LOW;
         }

[toc] | [prev] | [next] | [standalone]

#1618217 — Re: [PATCH 1/1] gpio: core: Decouple open drain/source flag with active low/high

On Thu, Apr 6, 2017 at 7:56 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> On Thursday 06 April 2017 09:40 PM, Andy Shevchenko wrote:
>> On Thu, Apr 6, 2017 at 4:35 PM, Laxman Dewangan <ldewangan@nvidia.com>
>> wrote:

>>> Adding different flag for the Open Drain/Open Source which is valid
>>> only when Single ended flag is enabled.
>>>          if (single_ended) {
>>> -               if (active_low)
>>> +               if (open_drain)
>>
>> This breaks ACPI case, right?

> In acpi case, single_ended is not handled. It only handles the active LOW.
>
> From code:

Fair enough.
I would look into ACPI spec for possibility to add this kind of functionality.

Thanks.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1618663 — Re: [PATCH 1/1] gpio: core: Decouple open drain/source flag with active low/high

On Thu, Apr 6, 2017 at 3:35 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:

> Currently, the GPIO interface is said to Open Drain if it is Single
> Ended and active LOW. Similarly, it is said as Open Source if it is
> Single Ended and active HIGH.
>
> The active HIGH/LOW is used in the interface for setting the pin
> state to HIGH or LOW when enabling/disabling the interface.
>
> In Open Drain interface, pin is set to HIGH by putting pin in
> high impedance and LOW by driving to the LOW.
>
> In Open Source interface, pin is set to HIGH by driving pin to
> HIGH and set to LOW by putting pin in high impedance.
>
> With above, the Open Drain/Source is unrelated to the active LOW/HIGH
> in interface. There is interface where the enable/disable of interface
> is ether active LOW or HIGH but it is Open Drain type.
>
> Hence decouple the Open Drain with Single Ended + Active LOW and
> Open Source with Single Ended + Active HIGH.
>
> Adding different flag for the Open Drain/Open Source which is valid
> only when Single ended flag is enabled.
>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

Patch applied.

Good that you found this and fixed it before someone git hurt.

Sorry for screwing it up, my fault.

Yours,
Linus Walleij

[toc] | [prev] | [next] | [standalone]

#1618669 — Re: [PATCH 1/1] gpio: core: Decouple open drain/source flag with active low/high

On Friday 07 April 2017 03:55 PM, Linus Walleij wrote:
> On Thu, Apr 6, 2017 at 3:35 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>
>> Currently, the GPIO interface is said to Open Drain if it is Single
>> Ended and active LOW. Similarly, it is said as Open Source if it is
>> Single Ended and active HIGH.
>>
>> The active HIGH/LOW is used in the interface for setting the pin
>> state to HIGH or LOW when enabling/disabling the interface.
>>
>> In Open Drain interface, pin is set to HIGH by putting pin in
>> high impedance and LOW by driving to the LOW.
>>
>> In Open Source interface, pin is set to HIGH by driving pin to
>> HIGH and set to LOW by putting pin in high impedance.
>>
>> With above, the Open Drain/Source is unrelated to the active LOW/HIGH
>> in interface. There is interface where the enable/disable of interface
>> is ether active LOW or HIGH but it is Open Drain type.
>>
>> Hence decouple the Open Drain with Single Ended + Active LOW and
>> Open Source with Single Ended + Active HIGH.
>>
>> Adding different flag for the Open Drain/Open Source which is valid
>> only when Single ended flag is enabled.
>>
>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> Patch applied.
>
> Good that you found this and fixed it before someone git hurt.

Thanks you very much.


Now we are going to use the descriptor based GPIO APIs as this is very 
powerful and passing this info from DT is very useful.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web