From patchwork Thu Oct 12 20:29:15 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ladislav Michl X-Patchwork-Id: 10002867 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id C94D560325 for ; Thu, 12 Oct 2017 20:29:23 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B00B328D66 for ; Thu, 12 Oct 2017 20:29:23 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A374428D9F; Thu, 12 Oct 2017 20:29:23 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B451328D66 for ; Thu, 12 Oct 2017 20:29:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753220AbdJLU3V (ORCPT ); Thu, 12 Oct 2017 16:29:21 -0400 Received: from eddie.linux-mips.org ([148.251.95.138]:41532 "EHLO cvs.linux-mips.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753215AbdJLU3V (ORCPT ); Thu, 12 Oct 2017 16:29:21 -0400 Received: (from localhost user: 'ladis' uid#1021 fake: STDIN (ladis@eddie.linux-mips.org)) by eddie.linux-mips.org id S23992274AbdJLU3RbBtFj (ORCPT ); Thu, 12 Oct 2017 22:29:17 +0200 Date: Thu, 12 Oct 2017 22:29:15 +0200 From: Ladislav Michl To: Roger Quadros Cc: linux-omap@vger.kernel.org, Tony Lindgren Subject: Re: [PATCH v2] memory: omap-gpmc: use OneNAND base address from FDT Message-ID: <20171012202915.nokylct55mwg244b@lenoch> References: <20171008213317.iwlf65zbhcjg5b62@lenoch> <201710092229.vPbT8Rji%fengguang.wu@intel.com> <20171009205004.vkvxlyfuvmdmhzz4@lenoch> <3cc1f9a8-c81a-c6f0-55c3-ddf22aa7cef9@ti.com> <20171012122714.narzvmoptpetn5aj@lenoch> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Roger, On Thu, Oct 12, 2017 at 06:05:31PM +0300, Roger Quadros wrote: [snip] > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > On 12/10/17 15:27, Ladislav Michl wrote: > > Not being Tony, I try to answer anyway... > > > > I've done patchset which removes gpmc_probe_onenand_child and uses > > gpmc_probe_generic_child (renamed to gpmc_probe_child) to setup OneNAND > > async timings. Sync timings is setup from mtd/onenand/omap2.c after > > chip frequency is known. > > > > I've not sent it yet as device datasheet is available under NDA only, > > I do not have it and thus I'm left with equivalent source modifications > > without really understanding what should be done and why. > > > > That said, I sent this patch to fix OneNAND support (for IGEPv2 board > > at least) and leave discussion about making things nice for later. > > > > I'm proposing killing gpmc_onenand_data first and teaching OneNAND > > to get its configuration from DT. Later I'll cleanup sync/async > > patchset and send it for review unless someone is willing to take > > it over to speed things up a bit - then I'll send it as it is :-) > > sounds fine to me. > > > > Side question: in tree OneNAND DT nodes are providing timing information. > > I have no clue where this timing information is coming from and driver > > completely ignores it and computes timings on its own. Any hints here? > > My guess is nobody has access to the datasheet and we're relying on bootloader > set timings or timings that were already hardcoded in the kernel. > > You can use CONFIG_OMAP_GPMC_DEBUG to dump the timings set by the bootloader > in DT format. > > You can also compare the timings before and after gpmc driver loads. That will not help a lot, as these questions will remain unanswered using above method: - why SYNC_WRITE cannot be used on 24xx? - why are we letting gpio line settle only on 34xx? - why we are not using SYNC_WRITE with 54 MHz freq? - how should be omap_onenand_platform_data flags set by default? probably few more I do not remember, but will show up once I start polishing patchset :) Anyway, here's first untested version, just to show, that we need to touch arch/arm/mach-omap2/gpmc-onenand.c anyway, just to make gpmc_onenand_setup directly callable without global omap_onenand_platform_data (and later move it into omap-gpmc) So unless someone comes with better idea, I'll drop all global variables from arch/arm/mach-omap2/gpmc-onenand.c and export gpmc_onenand_setup to get rid of omap_onenand_platform_data. After that patch gets merged, we can make additional patch on top of following one, which will drop use of omap_onenand_platform_data from omap2 onenand mtd driver. There are simply too many trees to get things merged to make it easy task :-/ Also note that no DT only kernel sets flags, regulator, gpio and yet noone complained, so we could leave it for later - and use more recent APIs (gpiolib, etc). --- 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/onenand/omap2.c b/drivers/mtd/onenand/omap2.c index 29cc0c21138c..85fac7aed2dc 100644 --- a/drivers/mtd/onenand/omap2.c +++ b/drivers/mtd/onenand/omap2.c @@ -28,12 +28,15 @@ #include #include #include +#include +#include #include #include #include #include #include #include +#include #include #include @@ -59,7 +62,7 @@ struct omap2_onenand { int dma_channel; int freq; struct regulator *regulator; - u8 flags; + bool gpio_settle; }; static void omap2_onenand_dma_cb(int lch, u16 ch_status, void *data) @@ -152,7 +155,7 @@ static int omap2_onenand_wait(struct mtd_info *mtd, int state) if (!(syscfg & ONENAND_SYS_CFG1_IOBE)) { syscfg |= ONENAND_SYS_CFG1_IOBE; write_reg(c, syscfg, ONENAND_REG_SYS_CFG1); - if (c->flags & ONENAND_IN_OMAP34XX) + if (c->gpio_settle) /* Add a delay to let GPIO settle */ syscfg = read_reg(c, ONENAND_REG_SYS_CFG1); } @@ -606,6 +609,32 @@ static int omap2_onenand_disable(struct mtd_info *mtd) return ret; } +static int omap2_onenand_get_dt(struct device *dev, struct omap2_onenand *c) +{ + struct device_node *child = dev->of_node; + u32 cs, val; + + if (of_property_read_u32(child, "reg", &cs) < 0) { + dev_err(dev, "reg not found in DT\n"); + return -EINVAL; + } + c->gpmc_cs = cs; + + if (!of_property_read_u32(child, "dma-channel", &val)) + c->dma_channel = val; + else + c->dma_channel = -1; + + /* TODO: gpio, regulator, unlocking */ + + return 0; +} + +static const struct soc_device_attribute omap2_soc_devices[] = { + { .family = "OMAP2*" }, + { /* sentinel */ } +}; + static int omap2_onenand_probe(struct platform_device *pdev) { struct omap_onenand_platform_data *pdata; @@ -624,12 +653,12 @@ static int omap2_onenand_probe(struct platform_device *pdev) if (!c) return -ENOMEM; + r = omap2_onenand_get_dt(&pdev->dev, c); + if (r) + goto err_kfree; + init_completion(&c->irq_done); init_completion(&c->dma_done); - c->flags = pdata->flags; - c->gpmc_cs = pdata->cs; - c->gpio_irq = pdata->gpio_irq; - c->dma_channel = pdata->dma_channel; if (c->dma_channel < 0) { /* if -1, don't use DMA */ c->gpio_irq = 0; @@ -710,21 +739,22 @@ static int omap2_onenand_probe(struct platform_device *pdev) c->mtd.priv = &c->onenand; c->mtd.dev.parent = &pdev->dev; - mtd_set_of_node(&c->mtd, pdata->of_node); + mtd_set_of_node(&c->mtd, pdev->dev.of_node); this = &c->onenand; if (c->dma_channel >= 0) { this->wait = omap2_onenand_wait; - if (c->flags & ONENAND_IN_OMAP34XX) { - this->read_bufferram = omap3_onenand_read_bufferram; - this->write_bufferram = omap3_onenand_write_bufferram; - } else { + if (soc_device_match(omap2_soc_devices)) { this->read_bufferram = omap2_onenand_read_bufferram; this->write_bufferram = omap2_onenand_write_bufferram; + } else { + c->gpio_settle = true; + this->read_bufferram = omap3_onenand_read_bufferram; + this->write_bufferram = omap3_onenand_write_bufferram; } } - if (pdata->regulator_can_sleep) { + if (0 /* TODO: pdata->regulator_can_sleep */) { c->regulator = regulator_get(&pdev->dev, "vonenand"); if (IS_ERR(c->regulator)) { dev_err(&pdev->dev, "Failed to get regulator\n"); @@ -735,14 +765,13 @@ static int omap2_onenand_probe(struct platform_device *pdev) c->onenand.disable = omap2_onenand_disable; } - if (pdata->skip_initial_unlocking) + if (0 /* TODO: pdata->skip_initial_unlocking */) this->options |= ONENAND_SKIP_INITIAL_UNLOCKING; if ((r = onenand_scan(&c->mtd, 1)) < 0) goto err_release_regulator; - r = mtd_device_register(&c->mtd, pdata ? pdata->parts : NULL, - pdata ? pdata->nr_parts : 0); + r = mtd_device_register(&c->mtd, NULL, 0); if (r) goto err_release_onenand; @@ -792,12 +821,19 @@ static int omap2_onenand_remove(struct platform_device *pdev) return 0; } +static const struct of_device_id omap2_onenand_ids[] = { + { .compatible = "ti,omap2-onenand", }, + {}, +}; +MODULE_DEVICE_TABLE(of, omap2_onenand_ids); + static struct platform_driver omap2_onenand_driver = { .probe = omap2_onenand_probe, .remove = omap2_onenand_remove, .shutdown = omap2_onenand_shutdown, .driver = { .name = DRIVER_NAME, + .of_match_table = of_match_ptr(omap2_onenand_ids), }, };