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 >