From patchwork Wed Jun 15 22:56:10 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kamal Dasu X-Patchwork-Id: 9179675 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 4F969604DB for ; Wed, 15 Jun 2016 22:56:53 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3FEBD27B13 for ; Wed, 15 Jun 2016 22:56:53 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3454127D5D; Wed, 15 Jun 2016 22:56:53 +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.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID 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 8A0FE27B13 for ; Wed, 15 Jun 2016 22:56:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932456AbcFOW4w (ORCPT ); Wed, 15 Jun 2016 18:56:52 -0400 Received: from mail-yw0-f172.google.com ([209.85.161.172]:33794 "EHLO mail-yw0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932327AbcFOW4v (ORCPT ); Wed, 15 Jun 2016 18:56:51 -0400 Received: by mail-yw0-f172.google.com with SMTP id c72so27539650ywb.1 for ; Wed, 15 Jun 2016 15:56:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=feoEusXzFTW962HTIgiipejUUcSYTDelNb1STxiw3hE=; b=A9OaiBUZHyOO+3lawLe4nHUf+lN3XFuTtAw8TM3XEtURPAi0YQYJ0Jt+gofc/HTKP/ tWJNygdRIEiGNIeQXF+g5uRcLAUFa7QVg/9jlhzVEF/RhynepCZVD9lgPbv94BMFuY7P ya9pe1LzG/e15DTzABIRkJukIFxAPMIYV9GE4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=feoEusXzFTW962HTIgiipejUUcSYTDelNb1STxiw3hE=; b=HAVmBd2IbZJeosM6LOxJh2OBpMv84IOa9BNVCVacEiYq48ZrgpucMA2AzLy46ovLcZ jLUWcS+qVB/yswNLKVDnva9URLH1T5AP+Mt0jL+ijgsIbjw9a9ZdrM78yNkvNu2pBwGo XUrSVRpzNjCoUDS0SrGxY5EqueVG2RXZdcq+q/44igl+J39v2HBqAUHQmQnkU2Sit6Or LPL2yRZHMlvj0v0kJZtSgqv+liUOARvM0ay+tcvLAJwJ8oSLpZtu4uB6nx1wmwFMkNwM f3NeOtyWitRAjRtdPXGAqDUA6uvc64pZqvq+wcMxDp5gzM3VUM6A6yAZ4fw3yDgJkk8H 3bKg== X-Gm-Message-State: ALyK8tJYFsARS6TO9iIJdJRS7in/I09I3vYrfPt2LheEbOVtcq1Ps+1puHXSqA8oLzjkPZ27DKMDsyLN2PYjxop/ X-Received: by 10.37.51.4 with SMTP id z4mr853228ybz.75.1466031410075; Wed, 15 Jun 2016 15:56:50 -0700 (PDT) MIME-Version: 1.0 Received: by 10.37.204.209 with HTTP; Wed, 15 Jun 2016 15:56:10 -0700 (PDT) In-Reply-To: <20160614165816.GT2282@sirena.org.uk> References: <1465589171-25575-1-git-send-email-kdasu.kdev@gmail.com> <1465589171-25575-2-git-send-email-kdasu.kdev@gmail.com> <20160613101352.GF2282@sirena.org.uk> <20160614165816.GT2282@sirena.org.uk> From: Kamal Dasu Date: Wed, 15 Jun 2016 18:56:10 -0400 Message-ID: Subject: Re: [V3, 2/4] spi: bcm-qspi: Add SPI flash and MSPI driver To: Mark Brown Cc: Kamal Dasu , linux-spi@vger.kernel.org, Florian Fainelli , bcm-kernel-feedback-list@broadcom.com, Vikram Prakash , Andy Fung , Jon Mason , Yendapally Reddy Dhananjaya Reddy Sender: linux-spi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Mark, I have implemented most of your suggestions. A few of them, actually just a couple, I wanted to discuss further. More importantly related to the spi_flash_read(). Please see my questions inline. On Tue, Jun 14, 2016 at 12:58 PM, Mark Brown wrote: > On Tue, Jun 14, 2016 at 11:43:56AM -0400, Kamal Dasu wrote: >> On Mon, Jun 13, 2016 at 6:13 AM, Mark Brown wrote: >> > On Fri, Jun 10, 2016 at 04:06:09PM -0400, Kamal Dasu wrote: > >> >> + /* deassert CS on the final byte */ >> >> + if (fnb & FNB_BREAK_DESELECT) { >> >> + mspi_cdram = read_cdram_slot(qspi, slot - 1) & >> >> + ~MSPI_CDRAM_CONT_BIT; >> >> + write_cdram_slot(qspi, slot - 1, mspi_cdram); >> >> + } > >> > Let the core handle asserting and deasserting chip select. > >> Its actually a peripheral CS (PCS) bit in the MSPI block. Peripheral >> chip selects are used to select an external device for serial data >> transfer. PCS[i] corresponds to pin SS[i]. > > That doesn't appear to address the issue, what you're describing sounds > like the normal function of the chip select? This bit has no effect and is not connected actually although software is manipulating it. The real CS is the one we use via the dt cs_reg. I will fix the comments/code to reflect this accordingly. > >> >> + switch (command) { >> >> + case SPINOR_OP_READ4_FAST: >> >> + if (!bcm_qspi_is_4_byte_mode(qspi)) > >> > No, this is not a good idea. You should not be attempting to parse >> > the data stream. If your controller has flash support it should >> > implement the flash interfaces, otherwise it should just pass the data >> > streams through. > >> This is only for read command we need to setup both tx and rx at the >> same time for improved performance. > > No, to repeat what I said if you want to support accelerated flash reads > implement the standard accelerated flash read operation we have > (spi_flash_read()). This is a fairly common feature and trying to open > code this in individual drivers would at the very least lead to a lot of > duplicated code and is the source of most of the problems in the code. I was looking at spi_flash_read() caled from m25p80_read() and spi core implementation. I have a case where when we have very small reads or unaligned buffers and addresses passed then I have to fall back to using standard MSPI reads. How best to handle this ?. One of the options without having to replicate code involves a minor change to m25p80 driver as shown below. --- spi_message_init(&m); --- I am open to any other suggestion that I can implement in my driver. In the previous version of using transfer_one_message() I could fallback in such cases. >> >> + spi_message_init(&m); >> >> + m.spi = &spi; > >> > I don't know why your driver is creating and trying to do SPI transfers >> > but that is completely inappropriate for a SPI controller. Whatever >> > functionality this is implementing should be in a separate SPI device >> > driver. > >> Currently not used. But I did not see a good way to be able to send >> commands, like finding the ID, setting 3/4 byte addressing mode , quad >> mode, on a PM resume(). Are you suggesting I make a driver similar to >> m25p80 ?. > > I am suggesting you implement the standard flash interfaces and as a > result directly use m25p80. Are you suggesting to use something like spi_read_then_write() in cases where I want to set the addressing and single/quad modes ?. Thanks Kamal -- To unsubscribe from this list: send the line "unsubscribe linux-spi" 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/devices/m25p80.c b/drivers/mtd/devices/m25p80.c index 9d68544..e98b4fa 100644 --- a/drivers/mtd/devices/m25p80.c +++ b/drivers/mtd/devices/m25p80.c @@ -149,8 +149,11 @@ static int m25p80_read(struct spi_nor *nor, loff_t from, size_t len, msg.data_nbits = m25p80_rx_nbits(nor); ret = spi_flash_read(spi, &msg); - *retlen = msg.retlen; - return ret; + /* check we need to fallback to normal spi read */ + if (ret != -EAGAIN) { + *retlen = msg.retlen; + return ret; + } }