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

Groups > linux.kernel > #1390891 > unrolled thread

Re: [PATCH V5 0/4] gpio: tegra: Cleanups and support for debounce

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V5 0/4] gpio: tegra: Cleanups and support for debounce Linus Walleij <linus.walleij@linaro.org> - 2016-04-29 11:10 +0200
    Re: [PATCH V5 0/4] gpio: tegra: Cleanups and support for debounce Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-29 11:40 +0200
      Re: [PATCH V5 0/4] gpio: tegra: Cleanups and support for debounce Linus Walleij <linus.walleij@linaro.org> - 2016-04-30 13:10 +0200
        Re: [PATCH V5 0/4] gpio: tegra: Cleanups and support for debounce Laxman Dewangan <ldewangan@nvidia.com> - 2016-05-02 09:00 +0200
        Re: [PATCH V5 0/4] gpio: tegra: Cleanups and support for debounce Stephen Warren <swarren@wwwdotorg.org> - 2016-05-02 18:20 +0200
          Re: [PATCH V5 0/4] gpio: tegra: Cleanups and support for debounce Laxman Dewangan <ldewangan@nvidia.com> - 2016-05-02 20:20 +0200
            Re: [PATCH V5 0/4] gpio: tegra: Cleanups and support for debounce Stephen Warren <swarren@wwwdotorg.org> - 2016-05-02 20:50 +0200
              Re: [PATCH V5 0/4] gpio: tegra: Cleanups and support for debounce Laxman Dewangan <ldewangan@nvidia.com> - 2016-05-02 21:20 +0200
                Re: [PATCH V5 0/4] gpio: tegra: Cleanups and support for debounce Stephen Warren <swarren@wwwdotorg.org> - 2016-05-03 18:00 +0200

#1390891 — Re: [PATCH V5 0/4] gpio: tegra: Cleanups and support for debounce

On Mon, Apr 25, 2016 at 12:38 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:

> Add support for the debounce as Tegra210 support debounce in HW.
> Also do the clenaups to remove all global variables.

OK this v5 is applied.

Laxman does this GPIO also have open drain and/or open source
handling?

Then you might want to look into supporting that too as I just added
support for native single-endedness to gpiolib, c.f.:
http://marc.info/?l=linux-gpio&m=146011780301280&w=2

It would be nice if you also implement .get_direction() which
makes debugfs and initial reading of the state of the lines
more accurate.

Yours,
Linus Walleij

[toc] | [next] | [standalone]

#1390943

On Friday 29 April 2016 02:37 PM, Linus Walleij wrote:
> On Mon, Apr 25, 2016 at 12:38 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>
>> Add support for the debounce as Tegra210 support debounce in HW.
>> Also do the clenaups to remove all global variables.
> OK this v5 is applied.
>
> Laxman does this GPIO also have open drain and/or open source
> handling?

Some of the pins support the open drain and these are part of pinmux 
register set.
For that we have property for setting open drain.

Is it possible to link the gpio APIs to pincontrol for setting that pin?


> Then you might want to look into supporting that too as I just added
> support for native single-endedness to gpiolib, c.f.:
> http://marc.info/?l=linux-gpio&m=146011780301280&w=2

Yaah, Some of PMIC's (which I am handling) gpios support open drain and 
I think I can have changes for the PMIC gpio driver.

>
> It would be nice if you also implement .get_direction() which
> makes debugfs and initial reading of the state of the lines
> more accurate.

Sure, this can be implemented.
Will post the patch on top of the series soon.

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

#1391591

On Fri, Apr 29, 2016 at 11:20 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> On Friday 29 April 2016 02:37 PM, Linus Walleij wrote:
>> On Mon, Apr 25, 2016 at 12:38 PM, Laxman Dewangan <ldewangan@nvidia.com>
>> wrote:
>>
>>> Add support for the debounce as Tegra210 support debounce in HW.
>>> Also do the clenaups to remove all global variables.
>>
>> OK this v5 is applied.
>>
>> Laxman does this GPIO also have open drain and/or open source
>> handling?
>
>
> Some of the pins support the open drain and these are part of pinmux
> register set.
> For that we have property for setting open drain.
>
> Is it possible to link the gpio APIs to pincontrol for setting that pin?

I have the same issue with Nomadik pin control that I use as a
testbed: there is a backend in pin control to the GPIO side.

I was thinking about adding a new cross call. We now have this:

/* External interface to pin control */
extern int pinctrl_request_gpio(unsigned gpio);
extern void pinctrl_free_gpio(unsigned gpio);
extern int pinctrl_gpio_direction_input(unsigned gpio);
extern int pinctrl_gpio_direction_output(unsigned gpio);

I was going to add:

extern int pinctrl_gpio_set_config(unsigned gpio, unsigned long config);

That can be used by GPIO drivers to call back into pincontrol
and set up any config flags using the conventions of the
corresponding pin control back-end.

This could be used for as well open drain as other things (like
pull-up) as the userspace ABI matures (it currently only has
in/out and open drain/source).

What do you think about this idea?

>> Then you might want to look into supporting that too as I just added
>> support for native single-endedness to gpiolib, c.f.:
>> http://marc.info/?l=linux-gpio&m=146011780301280&w=2
>
> Yaah, Some of PMIC's (which I am handling) gpios support open drain and I
> think I can have changes for the PMIC gpio driver.

Nice! :)

>> It would be nice if you also implement .get_direction() which
>> makes debugfs and initial reading of the state of the lines
>> more accurate.
>
> Sure, this can be implemented.
> Will post the patch on top of the series soon.

Thanks.

Yours,
Linus Walleij

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

#1392014

On Saturday 30 April 2016 04:37 PM, Linus Walleij wrote:
> On Fri, Apr 29, 2016 at 11:20 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>> On Friday 29 April 2016 02:37 PM, Linus Walleij wrote:
>>> On Mon, Apr 25, 2016 at 12:38 PM, Laxman Dewangan <ldewangan@nvidia.com>
>>> wrote:
>>>
>>>> Add support for the debounce as Tegra210 support debounce in HW.
>>>> Also do the clenaups to remove all global variables.
>>> OK this v5 is applied.
>>>
>>> Laxman does this GPIO also have open drain and/or open source
>>> handling?
>>
>> Some of the pins support the open drain and these are part of pinmux
>> register set.
>> For that we have property for setting open drain.
>>
>> Is it possible to link the gpio APIs to pincontrol for setting that pin?
> I have the same issue with Nomadik pin control that I use as a
> testbed: there is a backend in pin control to the GPIO side.
>
> I was thinking about adding a new cross call. We now have this:
>
> /* External interface to pin control */
> extern int pinctrl_request_gpio(unsigned gpio);
> extern void pinctrl_free_gpio(unsigned gpio);
> extern int pinctrl_gpio_direction_input(unsigned gpio);
> extern int pinctrl_gpio_direction_output(unsigned gpio);
>
> I was going to add:
>
> extern int pinctrl_gpio_set_config(unsigned gpio, unsigned long config);
>
> That can be used by GPIO drivers to call back into pincontrol
> and set up any config flags using the conventions of the
> corresponding pin control back-end.
>
> This could be used for as well open drain as other things (like
> pull-up) as the userspace ABI matures (it currently only has
> in/out and open drain/source).
>
> What do you think about this idea?
>

Yes, this will be great.
We will have generic interface which help in extending it in option.

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

#1392347

On 04/30/2016 05:07 AM, Linus Walleij wrote:
> On Fri, Apr 29, 2016 at 11:20 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>> On Friday 29 April 2016 02:37 PM, Linus Walleij wrote:
>>> On Mon, Apr 25, 2016 at 12:38 PM, Laxman Dewangan <ldewangan@nvidia.com>
>>> wrote:
>>>
>>>> Add support for the debounce as Tegra210 support debounce in HW.
>>>> Also do the clenaups to remove all global variables.
>>>
>>> OK this v5 is applied.
>>>
>>> Laxman does this GPIO also have open drain and/or open source
>>> handling?
>>
>>
>> Some of the pins support the open drain and these are part of pinmux
>> register set.
>> For that we have property for setting open drain.

IIRC, Tegra has open-drain control in both the GPIO controller for all 
pins (OE bit) and in the pinmux controller for a small subset of pins. 
For GPIOs, why wouldn't we just use the control bit in the GPIO 
controller for all GPIOs. This would avoid any special-cases, and 
minimize coupling between the GPIO and pinctrl drivers.

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

#1392433

On Monday 02 May 2016 09:42 PM, Stephen Warren wrote:
> On 04/30/2016 05:07 AM, Linus Walleij wrote:
>> On Fri, Apr 29, 2016 at 11:20 AM, Laxman Dewangan 
>> <ldewangan@nvidia.com> wrote:
>>> On Friday 29 April 2016 02:37 PM, Linus Walleij wrote:
>>>> On Mon, Apr 25, 2016 at 12:38 PM, Laxman Dewangan 
>>>> <ldewangan@nvidia.com>
>>>> wrote:
>>>>
>>>>> Add support for the debounce as Tegra210 support debounce in HW.
>>>>> Also do the clenaups to remove all global variables.
>>>>
>>>> OK this v5 is applied.
>>>>
>>>> Laxman does this GPIO also have open drain and/or open source
>>>> handling?
>>>
>>>
>>> Some of the pins support the open drain and these are part of pinmux
>>> register set.
>>> For that we have property for setting open drain.
>
> IIRC, Tegra has open-drain control in both the GPIO controller for all 
> pins (OE bit) and in the pinmux controller for a small subset of pins. 
> For GPIOs, why wouldn't we just use the control bit in the GPIO 
> controller for all GPIOs. This would avoid any special-cases, and 
> minimize coupling between the GPIO and pinctrl drivers.


Toggling OE bit is something emulating the open drain here.

I think idea is that when we configure the pin in open drain then it 
should be automatically handled by HW  when we want to set pin state 
high or low. When we set low, the pin should be driven and when high 
then it should be tristated input. We should not need any direction bit 
setting.

Otherwise, if pin is configured as open drain then:

Set out = 0

and when it need to set pin to high then oe = 0 else oe =1. Do not 
toggle any other bits.

On this case, we need to store that pin is configured as open drain so 
that set_value should toggle OE instead of OUT.

Or do you want to have different implementation?

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

#1392452

On 05/02/2016 11:58 AM, Laxman Dewangan wrote:
>
> On Monday 02 May 2016 09:42 PM, Stephen Warren wrote:
>> On 04/30/2016 05:07 AM, Linus Walleij wrote:
>>> On Fri, Apr 29, 2016 at 11:20 AM, Laxman Dewangan
>>> <ldewangan@nvidia.com> wrote:
>>>> On Friday 29 April 2016 02:37 PM, Linus Walleij wrote:
>>>>> On Mon, Apr 25, 2016 at 12:38 PM, Laxman Dewangan
>>>>> <ldewangan@nvidia.com>
>>>>> wrote:
>>>>>
>>>>>> Add support for the debounce as Tegra210 support debounce in HW.
>>>>>> Also do the clenaups to remove all global variables.
>>>>>
>>>>> OK this v5 is applied.
>>>>>
>>>>> Laxman does this GPIO also have open drain and/or open source
>>>>> handling?
>>>>
>>>>
>>>> Some of the pins support the open drain and these are part of pinmux
>>>> register set.
>>>> For that we have property for setting open drain.
>>
>> IIRC, Tegra has open-drain control in both the GPIO controller for all
>> pins (OE bit) and in the pinmux controller for a small subset of pins.
>> For GPIOs, why wouldn't we just use the control bit in the GPIO
>> controller for all GPIOs. This would avoid any special-cases, and
>> minimize coupling between the GPIO and pinctrl drivers.
>
>
> Toggling OE bit is something emulating the open drain here.

 From the perspective of the external HW that's attached to the GPIO, I 
believe there's no difference.

> I think idea is that when we configure the pin in open drain then it
> should be automatically handled by HW  when we want to set pin state
> high or low. When we set low, the pin should be driven and when high
> then it should be tristated input. We should not need any direction bit
> setting.

I don't imagine anything in the kernel cares, so long as the correct 
logic level is present on the pin based on whatever GPIO API was last 
called.

I'd be very surprised if there wasn't hardware that could only implement 
open-drain by this "emulation" method, so I'd be very surprised if 
something prohibited that implementation style.

> Otherwise, if pin is configured as open drain then:
>
> Set out = 0
>
> and when it need to set pin to high then oe = 0 else oe =1. Do not
> toggle any other bits.
>
> On this case, we need to store that pin is configured as open drain so
> that set_value should toggle OE instead of OUT.

That sounds like a reasonable implementation.

> Or do you want to have different implementation?

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

#1392473

On Tuesday 03 May 2016 12:14 AM, Stephen Warren wrote:
> On 05/02/2016 11:58 AM, Laxman Dewangan wrote:
>>
>>
>> Toggling OE bit is something emulating the open drain here.
>
> From the perspective of the external HW that's attached to the GPIO, I 
> believe there's no difference.
>
>> I think idea is that when we configure the pin in open drain then it
>> should be automatically handled by HW  when we want to set pin state
>> high or low. When we set low, the pin should be driven and when high
>> then it should be tristated input. We should not need any direction bit
>> setting.
>
> I don't imagine anything in the kernel cares, so long as the correct 
> logic level is present on the pin based on whatever GPIO API was last 
> called.
>
> I'd be very surprised if there wasn't hardware that could only 
> implement open-drain by this "emulation" method, so I'd be very 
> surprised if something prohibited that implementation style.
>

The emulation method implemented just to not drive high for open drain.
Recently, proper callback added for hw control for open drain and hence 
emulation method is not needed for such HW.

I think if HW support the callback to implement the open drain then use 
the HW method otherwise fallback to emulation method.


>> Otherwise, if pin is configured as open drain then:
>>
>> Set out = 0
>>
>> and when it need to set pin to high then oe = 0 else oe =1. Do not
>> toggle any other bits.
>>
>> On this case, we need to store that pin is configured as open drain so
>> that set_value should toggle OE instead of OUT.
>
> That sounds like a reasonable implementation.
>

Let me create the patch for review and further discussion.

>> Or do you want to have different implementation?
>

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

#1393560

On 05/02/2016 01:06 PM, Laxman Dewangan wrote:
>
> On Tuesday 03 May 2016 12:14 AM, Stephen Warren wrote:
>> On 05/02/2016 11:58 AM, Laxman Dewangan wrote:
>>>
>>>
>>> Toggling OE bit is something emulating the open drain here.
>>
>> From the perspective of the external HW that's attached to the GPIO, I
>> believe there's no difference.
>>
>>> I think idea is that when we configure the pin in open drain then it
>>> should be automatically handled by HW  when we want to set pin state
>>> high or low. When we set low, the pin should be driven and when high
>>> then it should be tristated input. We should not need any direction bit
>>> setting.
>>
>> I don't imagine anything in the kernel cares, so long as the correct
>> logic level is present on the pin based on whatever GPIO API was last
>> called.
>>
>> I'd be very surprised if there wasn't hardware that could only
>> implement open-drain by this "emulation" method, so I'd be very
>> surprised if something prohibited that implementation style.
>>
>
> The emulation method implemented just to not drive high for open drain.
> Recently, proper callback added for hw control for open drain and hence
> emulation method is not needed for such HW.
>
> I think if HW support the callback to implement the open drain then use
> the HW method otherwise fallback to emulation method.

I don't see any benefit to that. It makes the code more complex without 
enabling any more features.

For reference, on Tegra124 and earlier, very few pins have open-drain 
control in HW (pinmux) whereas you can emulate it in the GPIO module for 
any pin. In Tegra210 and Tegra186, many pins have open-drain control in 
HW (pinmux) yet a good number still don't, yet you can still emulate 
this in the GPIO module for any pin.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web