linux-kernel - RE: [PATCH 03/11] gpio: davinci: Modify to platform driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <518397C60809E147AF5323E0420B992E3EADA252@DBDE04.ent.ti.com>
Date:	Wed, 12 Jun 2013 12:10:22 +0000
From:	"Philip, Avinash" <avinashphilip@...com>
To:	"Nori, Sekhar" <nsekhar@...com>
CC:	"khilman@...prootsystems.com" <khilman@...prootsystems.com>,
	"linux@....linux.org.uk" <linux@....linux.org.uk>,
	"grant.likely@...retlab.ca" <grant.likely@...retlab.ca>,
	"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"davinci-linux-open-source@...ux.davincidsp.com" 
	<davinci-linux-open-source@...ux.davincidsp.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 03/11] gpio: davinci: Modify to platform driver

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

>