| Message ID | 1437761188-8179-9-git-send-email-Frank.Li@freescale.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Sat, Jul 25, 2015 at 02:06:28AM +0800, Frank.Li@freescale.com wrote: > From: Frank Li <Frank.Li@freescale.com> > > fsl-quadspi 21e0000.qspi: Unsupported cmd 0x20 > > when config CONFIG_MTD_SPI_NOR_USE_4K_SECTORS enable, > erase will use SPINOR_OP_BE_4K, which was not supported by fsl-quadspi > driver Slightly off topic: things looks pretty fragile here. /me thinks most of this can be written more cleanly... > Signed-off-by: Frank Li <Frank.Li@freescale.com> > Acked-by: Allen Xu <b45815@freescale.com> > --- > drivers/mtd/spi-nor/fsl-quadspi.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c > index 0f3f22d..e50da5c 100644 > --- a/drivers/mtd/spi-nor/fsl-quadspi.c > +++ b/drivers/mtd/spi-nor/fsl-quadspi.c > @@ -396,11 +396,11 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q) > lut_base = SEQID_SE * 4; > > if (q->nor_size <= SZ_16M) { > - cmd = SPINOR_OP_SE; > + cmd = q->nor[0].erase_opcode; > addrlen = ADDR24BIT; > } else { > /* use the 4-byte address */ > - cmd = SPINOR_OP_SE; > + cmd = q->nor[0].erase_opcode; > addrlen = ADDR32BIT; > } This whole block can be refactored to: cmd = q->nor[0].erase_opcode; addrlen = q->nor_size <= SZ_16M ? ADDR24BIT : ADDR32BIT; But really, this should be based on nor->addr_width... > > @@ -471,6 +471,8 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd) > case SPINOR_OP_BRWR: > return SEQID_BRWR; > default: > + if (cmd == q->nor[0].erase_opcode) Related question: what happens if you have multiple flash chips connected, and they don't need the same opcodes? It looks like you program the LUT only for the opcodes of the first flash, so the second wouldn't work right. > + return SEQID_SE; > dev_err(q->dev, "Unsupported cmd 0x%.2x\n", cmd); > break; > } Brian
On Fri, Jul 31, 2015 at 4:20 PM, Brian Norris <computersforpeace@gmail.com> wrote: > On Sat, Jul 25, 2015 at 02:06:28AM +0800, Frank.Li@freescale.com wrote: >> From: Frank Li <Frank.Li@freescale.com> >> >> fsl-quadspi 21e0000.qspi: Unsupported cmd 0x20 >> >> when config CONFIG_MTD_SPI_NOR_USE_4K_SECTORS enable, >> erase will use SPINOR_OP_BE_4K, which was not supported by fsl-quadspi >> driver > > Slightly off topic: things looks pretty fragile here. /me thinks most of > this can be written more cleanly... > >> Signed-off-by: Frank Li <Frank.Li@freescale.com> >> Acked-by: Allen Xu <b45815@freescale.com> >> --- >> drivers/mtd/spi-nor/fsl-quadspi.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c >> index 0f3f22d..e50da5c 100644 >> --- a/drivers/mtd/spi-nor/fsl-quadspi.c >> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c >> @@ -396,11 +396,11 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q) >> lut_base = SEQID_SE * 4; >> >> if (q->nor_size <= SZ_16M) { >> - cmd = SPINOR_OP_SE; >> + cmd = q->nor[0].erase_opcode; >> addrlen = ADDR24BIT; >> } else { >> /* use the 4-byte address */ >> - cmd = SPINOR_OP_SE; >> + cmd = q->nor[0].erase_opcode; >> addrlen = ADDR32BIT; >> } > > This whole block can be refactored to: > > cmd = q->nor[0].erase_opcode; > addrlen = q->nor_size <= SZ_16M ? ADDR24BIT : ADDR32BIT; > > But really, this should be based on nor->addr_width... > >> >> @@ -471,6 +471,8 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd) >> case SPINOR_OP_BRWR: >> return SEQID_BRWR; >> default: >> + if (cmd == q->nor[0].erase_opcode) > > Related question: what happens if you have multiple flash chips > connected, and they don't need the same opcodes? It looks like you > program the LUT only for the opcodes of the first flash, so the second > wouldn't work right. Fsl-qspi controller assume you connect two same flash chips. Two chip share the same opcodes. all other command like read, both chip also must use the same opcode. Frank Li > >> + return SEQID_SE; >> dev_err(q->dev, "Unsupported cmd 0x%.2x\n", cmd); >> break; >> } > > Brian
diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c index 0f3f22d..e50da5c 100644 --- a/drivers/mtd/spi-nor/fsl-quadspi.c +++ b/drivers/mtd/spi-nor/fsl-quadspi.c @@ -396,11 +396,11 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q) lut_base = SEQID_SE * 4; if (q->nor_size <= SZ_16M) { - cmd = SPINOR_OP_SE; + cmd = q->nor[0].erase_opcode; addrlen = ADDR24BIT; } else { /* use the 4-byte address */ - cmd = SPINOR_OP_SE; + cmd = q->nor[0].erase_opcode; addrlen = ADDR32BIT; } @@ -471,6 +471,8 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd) case SPINOR_OP_BRWR: return SEQID_BRWR; default: + if (cmd == q->nor[0].erase_opcode) + return SEQID_SE; dev_err(q->dev, "Unsupported cmd 0x%.2x\n", cmd); break; }