lkml.org 
[lkml]   [2013]   [Jun]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH 03/11] gpio: davinci: Modify to platform driver
    Date
    On Wed, Jun 12, 2013 at 13:13:59, Nori, Sekhar wrote:
    > On 6/11/2013 6:25 PM, Philip, Avinash wrote:
    >
    > > On Tue, Jun 11, 2013 at 17:26:06, Nori, Sekhar wrote:
    >
    > >> On 5/22/2013 12:40 PM, Philip Avinash wrote:
    >
    > >>> @@ -179,13 +204,10 @@ static int __init davinci_gpio_setup(void)
    > >>> gpiochip_add(&ctlrs[i].chip);
    > >>> }
    > >>>
    > >>> - soc_info->gpio_ctlrs = ctlrs;
    > >>
    > >>> - soc_info->gpio_ctlrs_num = DIV_ROUND_UP(ngpio, 32);
    > >>
    > >> You drop setting gpio_ctlrs_num here and don't introduce it anywhere
    > >> else in the patchset so in effect you render the inline gpio get/set API
    > >> useless. Looks like this initialization should be moved to platform code?
    > >
    > > With [PATCH 08/11] ARM: davinci: start using gpiolib support gpio get/set API
    > > Has no more dependency on soc_info->gpio_ctlrs_num.
    >
    > With this series, you have removed support for inline gpio get/set API.
    > I see that the inline functions are still available for use on
    > tnetv107x. I wonder why it is important to keep these for tnetv107x when
    > not necessary for other DaVinci devices?

    To support DT boot in da850, gpio davinci has to be converted to a driver and
    remove references to davinci_soc_info from driver. But tnetv107x has
    separate GPIO driver and reference to davinci_soc_info can also be removed.
    But I didn't found defconfig support for tnetv107x platforms and left untouched.
    As I will not be able to build and test on tnetv107x, I prefer to not touch it.

    >
    > When you are removing this feature, please note it prominently in your
    > cover letter and patch description.

    Ok

    > Also, please provide some data on
    > how the latency now compares to that of inline access earlier. This is
    > important especially for the read.

    I am not sure whether I understood correctly or not? Meanwhile I had done
    an experiment by reading printk timing before and after gpio_get_value from
    a test module. I think this will help in software latency for inline access over
    gpiolib specific access.

    gpio_get_value latency testing code

    +
    + local_irq_disable();
    + pr_emerg("%d %x\n", __LINE__, jiffies);
    + gpio_get_value(gpio_num);
    + pr_emerg("%d %x\n", __LINE__, jiffies);
    + local_irq_enable();

    inline gpio functions with interrupt disabled
    [ 29.734337] 81 ffff966c
    [ 29.736847] 83 ffff966c

    Time diff = 0.00251

    gpio library with interrupt disabled

    [ 272.876763] 81 fffff567
    [ 272.879291] 83 fffff567

    Time diff = 0.002528

    Inline function takes less time as expected.

    > For the writes, gpio clock will
    > mostly determine how soon the value changes on the pin.
    >
    > I am okay with removing the inline access feature. It helps simplify
    > code and most arm machines don't use them. I would just like to see some
    > data for justification as this can be seen as feature regression. Also,
    > if we are removing it, its better to also remove it completely and get
    > the LOC savings instead of just stopping its usage.

    I see build failure with below patch for tnetv107x
    [v6,6/6] Davinci: tnetv107x default configuration

    So I prefer to leave tnetv107x platform for now.

    Thanks
    Avinash

    >

    \
     
     \ /
      Last update: 2013-06-12 15:01    [W:8.019 / U:0.184 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site