Query about Open drain flag with active LOW

Query about Open drain flag with active LOW

[Laxman Dewangan]

Hi Linus,
I need help to understand the open drain flag passing from the DT.
The DT binding header says: In the file include/dt-bindings/gpio/gpio.h

/*
* Open Drain/Collector is the combination of single-ended active low,
* Open Source/Emitter is the combination of single-ended active high.
*/
#define GPIO_OPEN_DRAIN (GPIO_SINGLE_ENDED | GPIO_ACTIVE_LOW)
#define GPIO_OPEN_SOURCE (GPIO_SINGLE_ENDED | GPIO_ACTIVE_HIGH)


I have the schematics, where I need to set it as
Pin state HIGH (by setting gpio-input mode) to enable the circuit.
Pin state LOW (by setting gpio-output mode with value to 0) to disable 
the circuit.


With this, I think the GPIO is active HIGH and open drain type.
If I set the active LOW from DT then I can not set pin to logical 1 
using the API gpiod_set_value() with “1”.
As this invert before calling _gpiod_set_raw_value() and so the value = 
0 and it set the pin in the gpio mode output-Low.

Shouldn'’t open drain pin are active HIGH?

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

Re: Query about Open drain flag with active LOW

[Linus Walleij]

On Mon, Mar 27, 2017 at 5:21 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:

> I have the schematics, where I need to set it as
> Pin state HIGH (by setting gpio-input mode) to enable the circuit.
> Pin state LOW (by setting gpio-output mode with value to 0) to disable the
> circuit.
(...)
> With this, I think the GPIO is active HIGH and open drain type.
> If I set the active LOW from DT then I can not set pin to logical 1 using
> the API gpiod_set_value() with “1”.
> As this invert before calling _gpiod_set_raw_value() and so the value = 0
> and it set the pin in the gpio mode output-Low.
>
> Shouldn'’t open drain pin are active HIGH?

Sorry I do not understand, have you solved it?

Else can you try to break down the problem, do you mean there is a bug
in inverted handling for open drain lines in gpiolib?

Yours,
Linus Walleij

Re: Query about Open drain flag with active LOW

[Laxman Dewangan]

On Tuesday 28 March 2017 02:36 PM, Linus Walleij wrote:
> On Mon, Mar 27, 2017 at 5:21 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>
>> I have the schematics, where I need to set it as
>> Pin state HIGH (by setting gpio-input mode) to enable the circuit.
>> Pin state LOW (by setting gpio-output mode with value to 0) to disable the
>> circuit.
> (...)
>> With this, I think the GPIO is active HIGH and open drain type.
>> If I set the active LOW from DT then I can not set pin to logical 1 using
>> the API gpiod_set_value() with “1”.
>> As this invert before calling _gpiod_set_raw_value() and so the value = 0
>> and it set the pin in the gpio mode output-Low.
>>
>> Shouldn'’t open drain pin are active HIGH?
> Sorry I do not understand, have you solved it?

I wanted to say that open drain flag should be with ACTIVE_HIGH | 
SINGLE_ENDED which is in inverted as of now.



> Else can you try to break down the problem, do you mean there is a bug
> in inverted handling for open drain lines in gpiolib?
>

Yes, I am suspecting the bug here with combination of ACTIVE state and 
SINGLE_ENDED flag. The logic to get OPEN DRAIN flag should be in 
inverted form.
Else we need to decouple the ACTIVE state with SINGLE_ENDED flag to get 
Open drain or open source.


If I want to say logical "1" for the open drain, it is setting pin in 
drive-0 mode. Actually, it should be in input mode so that pin state 
become HIGH.

In function of_find_gpio(), it is as follows:

         if (of_flags & OF_GPIO_SINGLE_ENDED) {
                 if (of_flags & OF_GPIO_ACTIVE_LOW)
                         *flags |= GPIO_OPEN_DRAIN;
                 else
                         *flags |= GPIO_OPEN_SOURCE;
         }


I think it should be
         if (of_flags & OF_GPIO_SINGLE_ENDED) {
                 if (of_flags & OF_GPIO_ACTIVE_LOW)
                         *flags |= GPIO_OPEN_SOURCE;
                 else
                         *flags |= GPIO_OPEN_DRAIN;
         }


Similar change is needed in the dtbinding gpio header also to invert the 
ACTIVE state with open drain/source.


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

Re: Query about Open drain flag with active LOW

[Linus Walleij]

On Tue, Mar 28, 2017 at 11:03 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:

> I wanted to say that open drain flag should be with ACTIVE_HIGH |
> SINGLE_ENDED which is in inverted as of now.

Oh that seems correct... can you send a patch for this?

>> Else can you try to break down the problem, do you mean there is a bug
>> in inverted handling for open drain lines in gpiolib?
>
> Yes, I am suspecting the bug here with combination of ACTIVE state and
> SINGLE_ENDED flag. The logic to get OPEN DRAIN flag should be in inverted
> form.
> Else we need to decouple the ACTIVE state with SINGLE_ENDED flag to get Open
> drain or open source.
>
> If I want to say logical "1" for the open drain, it is setting pin in
> drive-0 mode. Actually, it should be in input mode so that pin state become
> HIGH.
>
> In function of_find_gpio(), it is as follows:
>
>         if (of_flags & OF_GPIO_SINGLE_ENDED) {
>                 if (of_flags & OF_GPIO_ACTIVE_LOW)
>                         *flags |= GPIO_OPEN_DRAIN;
>                 else
>                         *flags |= GPIO_OPEN_SOURCE;
>         }
>
>
> I think it should be
>         if (of_flags & OF_GPIO_SINGLE_ENDED) {
>                 if (of_flags & OF_GPIO_ACTIVE_LOW)
>                         *flags |= GPIO_OPEN_SOURCE;
>                 else
>                         *flags |= GPIO_OPEN_DRAIN;
>         }
>
>
> Similar change is needed in the dtbinding gpio header also to invert the
> ACTIVE state with open drain/source.

OK I think there are no real users of this feature yet, so it would be great if
you could send a patch, especially if you also have hardware so you can test it.

Yours,
Linus Walleij

Re: Query about Open drain flag with active LOW

[Laxman Dewangan]

On Tuesday 28 March 2017 06:38 PM, Linus Walleij wrote:
> On Tue, Mar 28, 2017 at 11:03 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>
>
>
> I think it should be
>          if (of_flags & OF_GPIO_SINGLE_ENDED) {
>                  if (of_flags & OF_GPIO_ACTIVE_LOW)
>                          *flags |= GPIO_OPEN_SOURCE;
>                  else
>                          *flags |= GPIO_OPEN_DRAIN;
>          }
>
>
> Similar change is needed in the dtbinding gpio header also to invert the
> ACTIVE state with open drain/source.
> OK I think there are no real users of this feature yet, so it would be great if
> you could send a patch, especially if you also have hardware so you can test it.
>

Thinking more, active low/high for line is independent of the open 
drain/source behavior.
Per my understanding, Active low/high depends on the how external device 
provides signal or glue logic designed. Open drain/source is kind of 
setting pin state between GND or VDD or high impedance.

Let me send patch on decoupling the active HIGH/LOW with open 
drain/source and discuss more in the patch.


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