From patchwork Thu Jun 13 07:32:21 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: avinash philip X-Patchwork-Id: 2714261 Return-Path: X-Original-To: patchwork-davinci@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id C562CC1459 for ; Thu, 13 Jun 2013 07:33:42 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4A9BE20179 for ; Thu, 13 Jun 2013 07:33:41 +0000 (UTC) Received: from comal.ext.ti.com (comal.ext.ti.com [198.47.26.152]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A4FCA20172 for ; Thu, 13 Jun 2013 07:33:39 +0000 (UTC) Received: from dlelxv90.itg.ti.com ([172.17.2.17]) by comal.ext.ti.com (8.13.7/8.13.7) with ESMTP id r5D7Xc64013277 for ; Thu, 13 Jun 2013 02:33:39 -0500 Received: from DFLE73.ent.ti.com (dfle73.ent.ti.com [128.247.5.110]) by dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id r5D7XcwR018571 for ; Thu, 13 Jun 2013 02:33:38 -0500 Received: from dlelxv23.itg.ti.com (172.17.1.198) by DFLE73.ent.ti.com (128.247.5.110) with Microsoft SMTP Server id 14.2.342.3; Thu, 13 Jun 2013 02:33:38 -0500 Received: from linux.omap.com (dlelxs01.itg.ti.com [157.170.227.31]) by dlelxv23.itg.ti.com (8.13.8/8.13.8) with ESMTP id r5D7Xc1V001314 for ; Thu, 13 Jun 2013 02:33:38 -0500 Received: from linux.omap.com (localhost [127.0.0.1]) by linux.omap.com (Postfix) with ESMTP id 95BBF8062A for ; Thu, 13 Jun 2013 02:33:38 -0500 (CDT) X-Original-To: davinci-linux-open-source@linux.davincidsp.com Delivered-To: davinci-linux-open-source@linux.davincidsp.com Received: from dbdlxv05.itg.ti.com (dbdlxv05.itg.ti.com [172.24.171.60]) by linux.omap.com (Postfix) with ESMTP id A5C4380626 for ; Thu, 13 Jun 2013 02:33:29 -0500 (CDT) Received: from DBDE73.ent.ti.com (dbde73.ent.ti.com [172.24.171.98]) by dbdlxv05.itg.ti.com (8.14.3/8.13.8) with ESMTP id r5D7XM5H030588; Thu, 13 Jun 2013 02:33:23 -0500 Received: from DBDE04.ent.ti.com ([fe80::21ac:d9f:f810:c8e7]) by DBDE73.ent.ti.com ([fe80::bc19:dc43:44da:7415%27]) with mapi id 14.02.0342.003; Thu, 13 Jun 2013 15:33:22 +0800 From: "Philip, Avinash" To: "Nori, Sekhar" Subject: RE: [PATCH 03/11] gpio: davinci: Modify to platform driver Thread-Topic: [PATCH 03/11] gpio: davinci: Modify to platform driver Thread-Index: AQHOVrtf6gwEFoq/8kygHsiS/vHYRpkwAbsAgACPoZCAALxDgIAAxFyQgAC16gCAAItCwA== Date: Thu, 13 Jun 2013 07:32:21 +0000 Deferred-Delivery: Thu, 13 Jun 2013 07:32:00 +0000 Message-ID: <518397C60809E147AF5323E0420B992E3EADA883@DBDE04.ent.ti.com> References: <1369206634-6778-1-git-send-email-avinashphilip@ti.com> <1369206634-6778-4-git-send-email-avinashphilip@ti.com> <51B71056.9010103@ti.com> <518397C60809E147AF5323E0420B992E3EAD9D75@DBDE04.ent.ti.com> <51B826BF.7050009@ti.com> <518397C60809E147AF5323E0420B992E3EADA252@DBDE04.ent.ti.com> <51B96410.7000807@ti.com> In-Reply-To: <51B96410.7000807@ti.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [172.24.170.142] MIME-Version: 1.0 CC: "davinci-linux-open-source@linux.davincidsp.com" , "linux@arm.linux.org.uk" , "khilman@deeprootsystems.com" , "linus.walleij@linaro.org" , "linux-kernel@vger.kernel.org" , "grant.likely@secretlab.ca" , "linux-arm-kernel@lists.infradead.org" X-BeenThere: davinci-linux-open-source@linux.davincidsp.com X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: Errors-To: davinci-linux-open-source-bounces+patchwork-davinci=patchwork.kernel.org@linux.davincidsp.com X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Jun 13, 2013 at 11:47:52, Nori, Sekhar wrote: > On 6/12/2013 5:40 PM, Philip, Avinash wrote: > > 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. > > You can always build it by enabling it in menuconfig. Its an ARMv6 > platform so you will have to disable other ARMv5 based platforms from > while enabling it. ARMv5 and ARMv6 cannot co-exist in the same image. I will try and update. > > > > >> > >> 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. > > Okay, please note these experiments in your cover letter. Its an 18usec > difference. I have no reference to say if that will affect any > application, but it will at least serve as information for someone who > may get affected by it. Ok I will give above details in commit message. > > > > >> 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 > > Where is this patch? This patch is not in mainline and got it from patchwork https://patchwork.kernel.org/patch/97853/ > What is the commit-id if it is in mainline? Where > is the failure log? With tnetv107x_defconfig build is failing arch/arm/mach-davinci/board-tnetv107x-evm.c:282:15: error: 'davinci_timer_init' undeclared here (not in a function) arch/arm/mach-davinci/board-tnetv107x-evm.c:284:15: error: 'davinci_init_late' undeclared here (not in a function) make[1]: *** [arch/arm/mach-davinci/board-tnetv107x-evm.o] Error 1 Following patch fixes the build above breakage > > > > > So I prefer to leave tnetv107x platform for now. > > I don't think that's acceptable. At least by me. I think 2 options are available 1. Convert gpio-tnetv107x.c to platform driver. This will help in removing gpio references in davinci_soc_info structure. 2. Remove inline gpio api support and start use gpiolib support. I prefer first option. It will help in removing . Thanks Avinash > > Thanks, > Sekhar > diff --git a/arch/arm/mach-davinci/board-tnetv107x-evm.c b/arch/arm/mach-davinci/board-tnetv107x-evm.c index ba79837..4a9c320 100644 --- a/arch/arm/mach-davinci/board-tnetv107x-evm.c +++ b/arch/arm/mach-davinci/board-tnetv107x-evm.c @@ -30,6 +30,7 @@ #include #include +#include #include #include #include @@ -147,7 +148,7 @@ static struct davinci_nand_pdata nand_config = { .ecc_bits = 1, }; -static struct davinci_uart_config serial_config __initconst = { +static struct davinci_uart_config serial_config = { .enabled_uarts = BIT(1), }; @@ -245,7 +246,7 @@ static struct ti_ssp_data ssp_config = { }, }; -static struct tnetv107x_device_info evm_device_info __initconst = { +static struct tnetv107x_device_info evm_device_info = { .serial_config = &serial_config, .mmc_config[1] = &mmc_config, /* controller 1 */ .nand_config[0] = &nand_config, /* chip select 0 */