Received: by 2002:ac0:a5b6:0:0:0:0:0 with SMTP id m51-v6csp4287928imm; Mon, 18 Jun 2018 12:16:40 -0700 (PDT) X-Google-Smtp-Source: ADUXVKK7G4dITAaUYcyy3VMey+azpiET24XuhBeKYk/iBnyW5iJQPaH1iA2/JHHhT0rf7E+xevQJ X-Received: by 2002:a17:902:600a:: with SMTP id r10-v6mr15607497plj.70.1529349400594; Mon, 18 Jun 2018 12:16:40 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1529349400; cv=none; d=google.com; s=arc-20160816; b=gAJTuWi2MVEX+TH03USCYv6KgjlV0ik7d3zEu9ej+Op9/QlgO+0iap7Wc42vobuWr6 yL/kNEOZ5NYGD3XxJ2+b4V/aPkk5e9fB7vEP+8ryQUxFOSooigRIZbF0Ni7j85dIRCff jB6Xhn/bbmDEAIYVZ4Lrz0aaVt6zYAC4UgG8D/G2c+KmjZQBfw4AoBpkBCHTF6dL13AM 2kfKM17y9bU23n1z5/1tvtkJsFXHAi9VuAKb1DM5L/fpIzPWYYUkT0EYRW0JHZ/8TlUp 7Gfsn02tUy7rx3MzbXReIWGYemVHe6dO0HhUPHSldco8laAoxNP1SJyDWI6DI2+cyi0D irRQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:subject:cc:to:from:date :arc-authentication-results; bh=yfCpDlWpkx8VVXAg5fXCeVbKwHn0ksy23+Oj0tpRHeg=; b=pcclJ8s6/+ru75ejjUv+EOMIuEeWAhPbfTdmTiafaAe+xoTCiqAyTiui9I3cxYrbUB lx7kIjRE/duxek4kk2ftak0YaKthkeRmxZ7dqZpmPHnwDDujMu/gSbu62+mu+UOntgMz pd027JsIHWEpFQvXdI3UcVtFBOANw5EhnmqgYGORDpZJJv1IWGfr7cx5iNgslzx8178f f2Zthgu4OQPruGZ9G5hfdtsI3QiYqkbRF68y3AJTprLeNM0ZwwrUdViwIUDhqdhXBThi AI2p1RtQAhhx1PgynrfN2rVca8bRncxIgS2A0T3dzlh98lErIMA9vG+4wbvn8uzbAUHa 9oGQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id c3-v6si15235919pld.87.2018.06.18.12.16.26; Mon, 18 Jun 2018 12:16:40 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936145AbeFRTPk (ORCPT + 99 others); Mon, 18 Jun 2018 15:15:40 -0400 Received: from mail.bootlin.com ([62.4.15.54]:50507 "EHLO mail.bootlin.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935860AbeFRTPi (ORCPT ); Mon, 18 Jun 2018 15:15:38 -0400 Received: by mail.bootlin.com (Postfix, from userid 110) id 0A09B207C2; Mon, 18 Jun 2018 21:15:37 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on mail.bootlin.com X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT shortcircuit=ham autolearn=disabled version=3.4.0 Received: from bbrezillon (91-160-177-164.subs.proxad.net [91.160.177.164]) by mail.bootlin.com (Postfix) with ESMTPSA id 84F0A20733; Mon, 18 Jun 2018 21:15:36 +0200 (CEST) Date: Mon, 18 Jun 2018 21:15:36 +0200 From: Boris Brezillon To: Yogesh Narayan Gaur Cc: Fabio Estevam , David Wolfe , "dwmw2@infradead.org" , "richard@nod.at" , Prabhakar Kushwaha , Han Xu , "linux-kernel@vger.kernel.org" , "linux-spi@vger.kernel.org" , "marek.vasut@gmail.com" , Frieder Schrempf , "broonie@kernel.org" , "linux-mtd@lists.infradead.org" , "miquel.raynal@bootlin.com" , "computersforpeace@gmail.com" Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Message-ID: <20180618211536.0bf44f55@bbrezillon> In-Reply-To: References: <1527686082-15142-1-git-send-email-frieder.schrempf@exceet.de> <20180608145130.09f979f9@bbrezillon> <20180611094616.5c8f82cf@bbrezillon> <20180611121618.40f4b609@bbrezillon> <20180612091328.67734adb@bbrezillon> <20180615145019.734f23a9@bbrezillon> <20180615155541.4f43e9bb@bbrezillon> X-Mailer: Claws Mail 3.15.0-dirty (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Yogesh, On Mon, 18 Jun 2018 13:32:27 +0000 Yogesh Narayan Gaur wrote: > -----Original Message----- > From: Boris Brezillon [mailto:boris.brezillon@bootlin.com] > Sent: Friday, June 15, 2018 7:26 PM > To: Yogesh Narayan Gaur ; Fabio Estevam ; David Wolfe ; dwmw2@infradead.org > Cc: richard@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-spi@vger.kernel.org; marek.vasut@gmail.com; Frieder Schrempf ; broonie@kernel.org; linux-mtd@lists.infradead.org; miquel.raynal@bootlin.com; computersforpeace@gmail.com > Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller > > On Fri, 15 Jun 2018 13:42:12 +0000 > Yogesh Narayan Gaur wrote: > > > Hi Boris, > > > > I am still debugging the issue. > > With some analysis, able to check that proper values are not being written for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. > > > > In current code, value of map_addr are being assigned to these register. > > map_addr = q->memmap_phy + > > 2 * q->devtype_data->ahb_buf_size; > > > > qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); > > > > But instead of "q->devtype_data->ahb_buf_size" it should be flash size. > > No, because we're only using 2 * ->ahb_buf_size in the direct mapping for each device, and we're modifying the mapping dynamically based on the selected device. Maybe we got the logic wrong though. > > Yes, for register QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD, we need to save starting actual address from where this flash is getting started. > Thus, if my first flash size is 64MB, then register QUADSPI_SFA2AD would have value of q->memmap_phy + 0x4000000 i.e. (QUADSPI_SFA1AD + sizeof First Flash) > If second flash is of size 32MB, then register QUADSPI_SFB1AD would have value of value of QUADSPI_SFA2AD + sizeof second flash. Again, no, that's not what I'm trying to do, and the fact that it worked fine with CS0 makes me think you don't need to map the whole device to get it to work, just 2 * ->ahb_buf_size per device. > > > For my case flash size is 0x4000000 and with this hard coded value I am able to perform Write and Erase operation. > > One more change, I have to do is adding the flash_size when writing the base_address in SFAR register for case when "mem->spi->chip_select == 1" > > qspi_writel(q, q->memmap_phy + 0x4000000, base + QUADSPI_SFAR); > > I don't want to expose the full device in the direct mapping yet (that's part of the direct-mapping API I posted here [1]). What this version of the driver does is, map only 2 time the ahb_size so that we can bypass the internal cache of the QSPI engine. > > To perform any operation on second flash, we need to provide it's base address should be saved in SFAR register for this particular operation. That's what we tried to do, we tried to make all CS start at 0 when they are used and declare unused CS at having a size of 0. So, say you're trying to access CS1, you should have the following ranges: CS0: 0 -> 0 (size = 0) CS1: 0 -> 2 * ->ahb_buf_size (size = 2 * ->ahb_buf_size) CS2: 2 * ->ahb_buf_size -> 2 * ->ahb_buf_size (size = 0) CS3: 2 * ->ahb_buf_size -> 2 * ->ahb_buf_size (size = 0) now, if you're trying to access CS3: CS0: 0 -> 0 (size = 0) CS1: 0 -> 0 (size = 0) CS2: 0 -> 0 (size = 0) CS3: 0 -> 2 * ->ahb_buf_size (size = 2 * ->ahb_buf_size) maybe this approach does not work, but that's not clearly stated as 'not supported' in the datasheet. > Exposing only 2 time of ahb_size is design decision but value in SFAR register should be correct. > > > > > Thus, there should be mechanism or the entry in structure where we can have the information of the size of the connected slave device. > > Because that's exactly the kind of thing I'd like to avoid. What if the device is bigger than the reserved memory region? What if the sum of all devices does not fit in there? Here I tried to support all cases by just mapping the portion of memory we need. > > So IMO, there should be mechanism to have value of start address of each slave device. This might can be done from DTS entry of each slave device connected to the controller. Let's not put that in the DT. If we really can't re-use 0 as the start address and make some ranges 0 in size, then let's reserve 2 * ->ahb_buf_size per chip, and be done with it. This should leave us enough space in the AHB mem range to then support temporary direct mappings through the direct mapping API. Regards, Boris