From patchwork Fri Jul 31 10:21:17 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roger Quadros X-Patchwork-Id: 6910071 Return-Path: X-Original-To: patchwork-linux-omap@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 4E8729F39D for ; Fri, 31 Jul 2015 10:22:48 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4A3FC20613 for ; Fri, 31 Jul 2015 10:22:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 29D5A205D1 for ; Fri, 31 Jul 2015 10:22:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751819AbbGaKWA (ORCPT ); Fri, 31 Jul 2015 06:22:00 -0400 Received: from devils.ext.ti.com ([198.47.26.153]:43721 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751161AbbGaKV6 (ORCPT ); Fri, 31 Jul 2015 06:21:58 -0400 Received: from dlelxv90.itg.ti.com ([172.17.2.17]) by devils.ext.ti.com (8.13.7/8.13.7) with ESMTP id t6VALKwo015432; Fri, 31 Jul 2015 05:21:20 -0500 Received: from DFLE72.ent.ti.com (dfle72.ent.ti.com [128.247.5.109]) by dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id t6VALKj4023714; Fri, 31 Jul 2015 05:21:20 -0500 Received: from dlep32.itg.ti.com (157.170.170.100) by DFLE72.ent.ti.com (128.247.5.109) with Microsoft SMTP Server id 14.3.224.2; Fri, 31 Jul 2015 05:21:20 -0500 Received: from [192.168.2.6] (ileax41-snat.itg.ti.com [10.172.224.153]) by dlep32.itg.ti.com (8.14.3/8.13.8) with ESMTP id t6VALHki002792; Fri, 31 Jul 2015 05:21:18 -0500 Message-ID: <55BB4C1D.5080905@ti.com> Date: Fri, 31 Jul 2015 13:21:17 +0300 From: Roger Quadros User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: Tony Lindgren CC: , , , , , , , Subject: Re: [PATCH 03/12] mtd: nand: omap: Move IRQ handling from GPMC to NAND driver References: <1436531019-18088-1-git-send-email-rogerq@ti.com> <1436531019-18088-4-git-send-email-rogerq@ti.com> <20150713071008.GC26485@atomide.com> <55A38D2E.9010500@ti.com> <20150713124059.GF26485@atomide.com> <55B8C1D5.3090807@ti.com> In-Reply-To: <55B8C1D5.3090807@ti.com> Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, 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 29/07/15 15:06, Roger Quadros wrote: > Tony, > > On 13/07/15 15:40, Tony Lindgren wrote: >> * Roger Quadros [150713 03:07]: >>> Tony, >>> >>> On 13/07/15 10:10, Tony Lindgren wrote: >>>> * Roger Quadros [150710 05:26]: >>>>> Since the Interrupt Events are used only by the NAND driver, >>>>> there is no point in managing the Interrupt registers >>>>> in the GPMC driver and complicating it with irqchip modeling. >>>> >>>> I don't think it's a good idea to allow external drivers to >>>> tinker directly with GPMC registers. How about just set up GPMC >>>> as an irqchip for the edge detection interrupts? >>>> >>>> I think we already have devices with multiple NAND chips. And >>>> there's nothing stopping other drivers from using the edge >>>> detection interrupts. >>> >>> OK. The GPMC_IRQ registers manage 2 NAND specific interrupts >>> (terminalcount and fifo) and 'n' WAIT pin edge interrupts. >>> >>> So we can model this as a irqchip with 'n + 2' interrupts. >> >> OK > > For the wait pins irqchip is not sufficient and it needs to be gpiochip > with irqchip. Waitpin status can be read from GPIO_STATUS register. > > Just getting the interrupt is not enough and we want to know if the > line is high or low. That is how nand->dev_ready works. > > How about having 2 IRQ domains? > One is irqchip with 2 interrupts (terminalcount and fifo) and second is > gpiochip + irqchip for the n wait pins. > > The nand driver can then be modified to use GPIO to get Read/Busy > pin status from the wait pin. One more observation I've had is that using irqchip modelling for the 2 NAND events causes a performance impact. Using mtd_oobtest I see the following on dra7-evm 1) v4.2-rc4 with prefetch-polled (no IRQs used) mtd_speedtest: eraseblock write speed is 7142 KiB/s mtd_speedtest: eraseblock read speed is 13721 KiB/s 2) v4.2-rc4 with prefetch-irq (IRQchip model) eraseblock write speed is 5475 KiB/s eraseblock read speed is 6420 KiB/s 3) this series (*) with prefetch-irq (no IRQchip model, nand driver directly accesses irqstatus/irqenable) eraseblock write speed is 6564 KiB/s eraseblock read speed is 10850 KiB/s (*) diff at the end is required on top to fix an issue with this series. So should we continue IRQchip modelling for the NAND events or use the GPMC interrupt as shared and add APIs to access the NAND bits of the IRQSTATUS/ENABLE register. cheers, -roger --- -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c index fecc054..26ef2bd 100644 --- a/drivers/mtd/nand/omap2.c +++ b/drivers/mtd/nand/omap2.c @@ -1832,13 +1832,14 @@ static int omap_get_dt_info(struct device *dev, struct omap_nand_info *info) for (i = 0; i < ARRAY_SIZE(nand_xfer_types); i++) { if (!strcasecmp(s, nand_xfer_types[i])) { info->xfer_type = i; - break; + goto next; } } dev_err(dev, "unrecognized value for ti,nand-xfer-type\n"); return -EINVAL; } +next: of_get_nand_on_flash_bbt(child); diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c index 26ef2bd..e8bdff5 100644 --- a/drivers/mtd/nand/omap2.c +++ b/drivers/mtd/nand/omap2.c @@ -670,17 +670,12 @@ static irqreturn_t omap_nand_irq(int this_irq, void *dev) goto done; } - /* Clear FIFOEVENT STATUS */ - irqstatus &= ~GPMC_IRQ_FIFOEVENT; - writel(irqstatus, info->reg.gpmc_irqstatus); - return IRQ_HANDLED; done: complete(&info->comp); - /* Clear FIFOEVENT and TERMCOUNT STATUS */ - irqstatus &= ~(GPMC_IRQ_TERMCOUNT | GPMC_IRQ_FIFOEVENT); + /* Clear IRQ STATUS */ writel(irqstatus, info->reg.gpmc_irqstatus); /* Disable Interrupt generation */