From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga11.intel.com ([192.55.52.93]) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1XH0hO-0003Wi-0l for linux-mtd@lists.infradead.org; Tue, 12 Aug 2014 01:16:26 +0000 Date: Tue, 12 Aug 2014 09:16:07 +0800 From: Huang Shijie To: Brian Norris Subject: Re: [PATCH 6/8] mtd: spi-nor: drop replaceable wait-till-ready function pointer Message-ID: <20140812011607.GA2065@shldeISGChi005.sh.intel.com> References: <1407374222-8448-1-git-send-email-computersforpeace@gmail.com> <1407374222-8448-7-git-send-email-computersforpeace@gmail.com> <20140809095301.GA1076@localhost.localdomain> <20140811184311.GX3711@ld-irv-0074> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140811184311.GX3711@ld-irv-0074> Cc: Marek Vasut , Huang Shijie , zajec5@gmail.com, Huang Shijie , linux-mtd@lists.infradead.org List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Mon, Aug 11, 2014 at 11:43:11AM -0700, Brian Norris wrote: > On Sat, Aug 09, 2014 at 05:53:03PM +0800, Huang Shijie wrote: > > On Wed, Aug 06, 2014 at 06:17:00PM -0700, Brian Norris wrote: > > > We don't need to expose a 'wait-till-ready' interface to drivers. Status > > > register polling should be handled by the core spi-nor.c library, and as > > > of now, I see no need to provide a special driver-specific hook for it. > > > > Please do not drop this hook, please see why we add this hook: > > http://lists.infradead.org/pipermail/linux-mtd/2013-December/050561.html > > > > " > > The default implementation would do just as you suggest, and I would > > expect most H/W drivers to use the default implementation. However, some H/W > > Controllers can automate the polling of status registers, so having a hook allows > > the driver override the default behaviour. > > " > > > > The spi-nor framework will used by more drivers besides the m25p80 and > > fsl-quadspi. Some NOR flash controller may be very strange. > > OK, but the wait-till-ready hook should not look like the current one. > If it is used as an optimization of sorts, then we can provide it in > *addition* to the checks we do, but not *instead of*. I sincerely doubt > that any specialized SPI NOR controller will know how to check both SR > and FSR where appropriate, and it probably won't understand other > specialized sequences as they are developed / thrown on our lap by flash > vendors. > > So, the spi-nor.c "wait until ready" might have a sequence like this: > > 1. (Optionally) use low-level driver's "wait" function; skip if not > present > > 2. use the read register hooks to check SR/FSR to confirm completion > > I do not want to toss #2 into the low-level driver, so if there is a > need for #1, it should be done in addition to our spi-nor.c code, not > instead. (To this end, I believe Marek also complained about this when > we were adding the FSR-checking code; we should not have drivers and > spi-nor.c fighting over callback functions.) > > So I'm inclined to at most address #1 through an optional callback, and but where to put the optional callback? in the spi_nor{} ? > possibly even to ignore that until we see an actual driver that needs > it. If you insist to remove it, i will be okay too. anyway, we can add it back when an actual driver appears. thanks Huang Shijie