Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754159AbbHXKs0 (ORCPT ); Mon, 24 Aug 2015 06:48:26 -0400 Received: from mail-out.m-online.net ([212.18.0.10]:47851 "EHLO mail-out.m-online.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751079AbbHXKsX (ORCPT ); Mon, 24 Aug 2015 06:48:23 -0400 X-Auth-Info: +FP8VxuXxhfDjA0X3CHik7UUfV98pVruYb2WOi7PMj8= From: Marek Vasut To: Cyrille Pitchen Subject: Re: [PATCH linux-next v4 3/5] mtd: spi-nor: allow to tune the number of dummy cycles Date: Mon, 24 Aug 2015 12:48:17 +0200 User-Agent: KMail/1.13.7 (Linux/3.14-2-amd64; KDE/4.13.1; x86_64; ; ) Cc: nicolas.ferre@atmel.com, broonie@kernel.org, linux-spi@vger.kernel.org, dwmw2@infradead.org, computersforpeace@gmail.com, zajec5@gmail.com, beanhuo@micron.com, juhosg@openwrt.org, shijie.huang@intel.com, ben@decadent.org.uk, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, linux-mtd@lists.infradead.org References: In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Message-Id: <201508241248.17466.marex@denx.de> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4665 Lines: 158 On Monday, August 24, 2015 at 12:13:58 PM, Cyrille Pitchen wrote: > The number of dummy cycles used during Fast Read commands can be reduced > to improve transfer performances. Each manufacturer has a dedicated set of > registers to provide the memory with the exact number of dummy cycles it > should expect. Both the memory and the (Q)SPI controller must agree on > this number of dummy cycles. > > The number of dummy cycles can be found into the memory datasheet and > mostly depends on the SPI clock frequency, the Fast Read op code and the > Single/Dual Data Rate mode. > > Probing JEDEC Serial Flash Discoverable Parameters (SFDP) tables would > only provide the driver with a high enough number of dummy cycles for each > Fast Read command to be used for all clock frequencies: this solution > would not be optimized. > > Signed-off-by: Cyrille Pitchen Hi! > drivers/mtd/spi-nor/spi-nor.c | 97 > ++++++++++++++++++++++++++++++++++--------- include/linux/mtd/spi-nor.h > | 2 + > 2 files changed, 80 insertions(+), 19 deletions(-) > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index e2a6029dc056..869e098a6841 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -119,24 +119,6 @@ static int read_cr(struct spi_nor *nor) > } > > /* > - * Dummy Cycle calculation for different type of read. > - * It can be used to support more commands with > - * different dummy cycle requirements. > - */ > -static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor) > -{ > - switch (nor->flash_read) { > - case SPI_NOR_FAST: > - case SPI_NOR_DUAL: > - case SPI_NOR_QUAD: > - return 8; > - case SPI_NOR_NORMAL: > - return 0; > - } > - return 0; > -} You can probably just soup up this function so that it sets the nor->read_dummy, no ? > -/* > * Write status register 1 byte > * Returns negative if error occurred. > */ > @@ -1012,6 +994,81 @@ static int set_quad_mode(struct spi_nor *nor, struct > flash_info *info) } > } > > +static int micron_set_dummy_cycles(struct spi_nor *nor) > +{ > + int ret; > + u8 val, mask; > + > + /* read the Volatile Configuration Register (VCR) */ NIT: If this is a sentence, start it with capital letter and end it with fullstop :) > + ret = nor->read_reg(nor, SPINOR_OP_RD_VCR, &val, 1); > + if (ret < 0) { > + dev_err(nor->dev, "error %d reading VCR\n", ret); > + return ret; > + } > + > + write_enable(nor); > + > + /* update the number of dummy into the VCR */ DTTO > + mask = GENMASK(7, 4); > + val &= ~mask; > + val |= (nor->read_dummy << 4) & mask; > + ret = nor->write_reg(nor, SPINOR_OP_WR_VCR, &val, 1, 0); > + if (ret < 0) { > + dev_err(nor->dev, "error while writing VCR register\n"); > + return ret; > + } > + > + ret = spi_nor_wait_till_ready(nor); > + if (ret) > + return ret; > + > + return 0; > +} > + > +/* > + * Dummy Cycle calculation for different type of read. > + * It can be used to support more commands with > + * different dummy cycle requirements. > + */ > +static int spi_nor_read_dummy_cycles(struct spi_nor *nor, > + const struct flash_info *info) > +{ > + struct device_node *np = nor->dev->of_node; > + u32 num_dummy_cycles; > + > + if (np && !of_property_read_u32(np, "m25p,num-dummy-cycles", > + &num_dummy_cycles)) { > + nor->read_dummy = num_dummy_cycles; > + > + /* > + * This switch block might be moved after the if...then...else > + * statement but it was not tested with all Spansion or Micron > + * memories. > + * Now the "m25p,num-dummy-cycles" property needs to be > + * explicitly set in the device tree so the switch statement is > + * executed. This should avoid unwanted side effects and keep > + * backward compatibility. > + */ > + switch (JEDEC_MFR(info)) { > + case CFI_MFR_ST: > + return micron_set_dummy_cycles(nor); > + default: If you do have m25p,num-dummy-cycles set for non-micron flash, you have a problem here I believe. > + break; > + } > + } else { The solution would be to drop this else {} bit here, so that if you fail in the DT-based configuration, you fall back to this old behavior. What do you think please ? :) > + switch (nor->flash_read) { > + case SPI_NOR_FAST: > + case SPI_NOR_DUAL: > + case SPI_NOR_QUAD: > + nor->read_dummy = 8; > + case SPI_NOR_NORMAL: > + nor->read_dummy = 0; > + } > + } > + > + return 0; > +} [...] -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/