Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1163267AbbKTTiP (ORCPT ); Fri, 20 Nov 2015 14:38:15 -0500 Received: from mail-qg0-f52.google.com ([209.85.192.52]:34773 "EHLO mail-qg0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751699AbbKTTiN (ORCPT ); Fri, 20 Nov 2015 14:38:13 -0500 Subject: Re: [PATCH v1 3/7] mtd: fsl-quadspi: Support both 24- and 32-bit addressed commands. To: Brian Norris , Han Xu References: <1436386881-28088-1-git-send-email-cory.tusar@pid1solutions.com> <1436386881-28088-4-git-send-email-cory.tusar@pid1solutions.com> <20151120192450.GW64635@google.com> From: Cory Tusar X-Enigmail-Draft-Status: N1110 Cc: robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, shawnguo@kernel.org, kernel@pengutronix.de, dwmw2@infradead.org, stefan@agner.ch, linux@arm.linux.org.uk, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mtd@lists.infradead.org, andrew@lunn.ch Message-ID: <564F76A2.9010905@pid1solutions.com> Date: Fri, 20 Nov 2015 14:38:10 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <20151120192450.GW64635@google.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9083 Lines: 269 -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 11/20/2015 02:24 PM, Brian Norris wrote: > Cory and Han, > > Did this series get dropped on the floor? I recall Han arguing > previously that this controller is always used with two identical chips. > But apparently that is not the case. > > If this request is not truly dead, I'd appreciate it if Han could > review/test. Hi Brian, Pretty sure this series will not apply & work cleanly with 4.4-rc1. IIRC, there were also some changes (apart from these) that reworked clocking and resulted in a fault the last time I tested this on actual hardware. I can see about rebasing and updating if there's additional interest. I don't know that there's anything about the controller itself that would preclude using different chips, apart from the overhead of having to reconfigure the LUT when switching between devices... We'd had a board which only included a device on QSPI0_B_CS0, whereas the driver assumed that the channels would be populated in order (e.g. QSPI0_A_CS0 first and then QSPI0_B_CS0)...that configuration was what originally drove my changes. - -Cory > On Wed, Jul 08, 2015 at 04:21:17PM -0400, Cory Tusar wrote: >> The current fsl-quadspi implementation assumes that all connected >> devices are of the same size and type. This commit adds lookup table >> entries for both 24- and 32-bit addressed variants of the read, sector >> erase, and page program operations as a precursor to later changes which >> generalize the flash layout parsing logic and allow for non-contiguous >> and non-homogeneous chip combinations. >> >> Signed-off-by: Cory Tusar >> --- >> drivers/mtd/spi-nor/fsl-quadspi.c | 116 ++++++++++++++++++++------------------ >> 1 file changed, 60 insertions(+), 56 deletions(-) >> >> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c >> index 52a872f..4b8038b 100644 >> --- a/drivers/mtd/spi-nor/fsl-quadspi.c >> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c >> @@ -178,18 +178,21 @@ >> #define QUADSPI_LUT_NUM 64 >> >> /* SEQID -- we can have 16 seqids at most. */ >> -#define SEQID_QUAD_READ 0 >> -#define SEQID_WREN 1 >> -#define SEQID_WRDI 2 >> -#define SEQID_RDSR 3 >> -#define SEQID_SE 4 >> -#define SEQID_CHIP_ERASE 5 >> -#define SEQID_PP 6 >> -#define SEQID_RDID 7 >> -#define SEQID_WRSR 8 >> -#define SEQID_RDCR 9 >> -#define SEQID_EN4B 10 >> -#define SEQID_BRWR 11 >> +#define SEQID_QUAD_READ_24 0 >> +#define SEQID_QUAD_READ_32 1 >> +#define SEQID_WREN 2 >> +#define SEQID_WRDI 3 >> +#define SEQID_RDSR 4 >> +#define SEQID_SE_24 5 >> +#define SEQID_SE_32 5 >> +#define SEQID_CHIP_ERASE 7 >> +#define SEQID_PP_24 8 >> +#define SEQID_PP_32 8 >> +#define SEQID_RDID 9 >> +#define SEQID_WRSR 10 >> +#define SEQID_RDCR 11 >> +#define SEQID_EN4B 12 >> +#define SEQID_BRWR 13 >> >> enum fsl_qspi_devtype { >> FSL_QUADSPI_VYBRID, >> @@ -287,7 +290,6 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q) >> void __iomem *base = q->iobase; >> int rxfifo = q->devtype_data->rxfifo; >> u32 lut_base; >> - u8 cmd, addrlen, dummy; >> int i; >> >> fsl_qspi_unlock_lut(q); >> @@ -297,22 +299,16 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q) >> writel(0, base + QUADSPI_LUT_BASE + i * 4); >> >> /* Quad Read */ >> - lut_base = SEQID_QUAD_READ * 4; >> - >> - if (q->nor_size <= SZ_16M) { >> - cmd = SPINOR_OP_READ_1_1_4; >> - addrlen = ADDR24BIT; >> - dummy = 8; >> - } else { >> - /* use the 4-byte address */ >> - cmd = SPINOR_OP_READ_1_1_4; >> - addrlen = ADDR32BIT; >> - dummy = 8; >> - } >> + lut_base = SEQID_QUAD_READ_24 * 4; >> + writel(LUT0(CMD, PAD1, SPINOR_OP_READ_1_1_4) | LUT1(ADDR, PAD1, ADDR24BIT), >> + base + QUADSPI_LUT(lut_base)); >> + writel(LUT0(DUMMY, PAD1, 8) | LUT1(READ, PAD4, rxfifo), >> + base + QUADSPI_LUT(lut_base + 1)); >> >> - writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen), >> + lut_base = SEQID_QUAD_READ_32 * 4; >> + writel(LUT0(CMD, PAD1, SPINOR_OP_READ_1_1_4) | LUT1(ADDR, PAD1, ADDR32BIT), >> base + QUADSPI_LUT(lut_base)); >> - writel(LUT0(DUMMY, PAD1, dummy) | LUT1(READ, PAD4, rxfifo), >> + writel(LUT0(DUMMY, PAD1, 8) | LUT1(READ, PAD4, rxfifo), >> base + QUADSPI_LUT(lut_base + 1)); >> >> /* Write enable */ >> @@ -320,18 +316,13 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q) >> writel(LUT0(CMD, PAD1, SPINOR_OP_WREN), base + QUADSPI_LUT(lut_base)); >> >> /* Page Program */ >> - lut_base = SEQID_PP * 4; >> - >> - if (q->nor_size <= SZ_16M) { >> - cmd = SPINOR_OP_PP; >> - addrlen = ADDR24BIT; >> - } else { >> - /* use the 4-byte address */ >> - cmd = SPINOR_OP_PP; >> - addrlen = ADDR32BIT; >> - } >> + lut_base = SEQID_PP_24 * 4; >> + writel(LUT0(CMD, PAD1, SPINOR_OP_PP) | LUT1(ADDR, PAD1, ADDR24BIT), >> + base + QUADSPI_LUT(lut_base)); >> + writel(LUT0(WRITE, PAD1, 0), base + QUADSPI_LUT(lut_base + 1)); >> >> - writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen), >> + lut_base = SEQID_PP_32 * 4; >> + writel(LUT0(CMD, PAD1, SPINOR_OP_PP) | LUT1(ADDR, PAD1, ADDR32BIT), >> base + QUADSPI_LUT(lut_base)); >> writel(LUT0(WRITE, PAD1, 0), base + QUADSPI_LUT(lut_base + 1)); >> >> @@ -341,18 +332,12 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q) >> base + QUADSPI_LUT(lut_base)); >> >> /* Erase a sector */ >> - lut_base = SEQID_SE * 4; >> - >> - if (q->nor_size <= SZ_16M) { >> - cmd = SPINOR_OP_SE; >> - addrlen = ADDR24BIT; >> - } else { >> - /* use the 4-byte address */ >> - cmd = SPINOR_OP_SE; >> - addrlen = ADDR32BIT; >> - } >> + lut_base = SEQID_SE_24 * 4; >> + writel(LUT0(CMD, PAD1, SPINOR_OP_SE) | LUT1(ADDR, PAD1, ADDR24BIT), >> + base + QUADSPI_LUT(lut_base)); >> >> - writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen), >> + lut_base = SEQID_SE_32 * 4; >> + writel(LUT0(CMD, PAD1, SPINOR_OP_SE) | LUT1(ADDR, PAD1, ADDR32BIT), >> base + QUADSPI_LUT(lut_base)); >> >> /* Erase the whole chip */ >> @@ -391,11 +376,17 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q) >> } >> >> /* Get the SEQID for the command */ >> -static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd) >> +static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd, u8 addr_width) >> { >> switch (cmd) { >> case SPINOR_OP_READ_1_1_4: >> - return SEQID_QUAD_READ; >> + if (addr_width == 3) >> + return SEQID_QUAD_READ_24; >> + if (addr_width == 4) >> + return SEQID_QUAD_READ_32; >> + dev_err(q->dev, "Unsupported addr_width (%d) for cmd 0x%.2x\n", >> + addr_width, cmd); >> + break; >> case SPINOR_OP_WREN: >> return SEQID_WREN; >> case SPINOR_OP_WRDI: >> @@ -403,11 +394,23 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd) >> case SPINOR_OP_RDSR: >> return SEQID_RDSR; >> case SPINOR_OP_SE: >> - return SEQID_SE; >> + if (addr_width == 3) >> + return SEQID_SE_24; >> + if (addr_width == 4) >> + return SEQID_SE_32; >> + dev_err(q->dev, "Unsupported addr_width (%d) for cmd 0x%.2x\n", >> + addr_width, cmd); >> + break; >> case SPINOR_OP_CHIP_ERASE: >> return SEQID_CHIP_ERASE; >> case SPINOR_OP_PP: >> - return SEQID_PP; >> + if (addr_width == 3) >> + return SEQID_PP_24; >> + if (addr_width == 4) >> + return SEQID_PP_32; >> + dev_err(q->dev, "Unsupported addr_width (%d) for cmd 0x%.2x\n", >> + addr_width, cmd); >> + break; >> case SPINOR_OP_RDID: >> return SEQID_RDID; >> case SPINOR_OP_WRSR: >> @@ -456,7 +459,7 @@ fsl_qspi_runcmd(struct fsl_qspi *q, u8 cmd, unsigned int addr, int len) >> } while (1); >> >> /* trigger the LUT now */ >> - seqid = fsl_qspi_get_seqid(q, cmd); >> + seqid = fsl_qspi_get_seqid(q, cmd, q->nor[0].addr_width); >> writel((seqid << QUADSPI_IPCR_SEQID_SHIFT) | len, base + QUADSPI_IPCR); >> >> /* Wait for the interrupt. */ >> @@ -601,9 +604,10 @@ static void fsl_qspi_init_abh_read(struct fsl_qspi *q) >> writel(0, base + QUADSPI_BUF2IND); >> >> /* Set the default lut sequence for AHB Read. */ >> - seqid = fsl_qspi_get_seqid(q, q->nor[0].read_opcode); >> + seqid = fsl_qspi_get_seqid(q, q->nor[0].read_opcode, >> + q->nor[0].addr_width); >> writel(seqid << QUADSPI_BFGENCR_SEQID_SHIFT, >> - q->iobase + QUADSPI_BFGENCR); >> + q->iobase + QUADSPI_BFGENCR); >> } >> >> /* We use this function to do some basic init for spi_nor_scan(). */ >> -- >> 2.3.6 >> - -- Cory Tusar Principal PID 1 Solutions, Inc. "There are two ways of constructing a software design. One way is to make it so simple that there are obviously no deficiencies, and the other way is to make it so complicated that there are no obvious deficiencies." --Sir Charles Anthony Richard Hoare -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAlZPdqIACgkQHT1tsfGwHJ/IOACdGu5LE0JC+jAjqa8k93vg4fU3 8KEAn2aGkmdWkq/7hfCOM8Nu+snBM5jG =qm3K -----END PGP SIGNATURE----- -- 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/