| Message ID | 20210820065839.10525-3-jaimeliao.tw@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | Add octal DTR support for Macronix flash | expand |
Hi, On 20/08/21 02:58PM, JaimeLiao wrote: > Power-on-Reset is a method to restore flash back to 1S-1S-1S mode from 8D-8D-8D > in the begging of probe. > > Command extension type is not standardized across flash vendors in DTR mode. > > For suiting different vendor flash devices, having second times Softreset with > different types is clumsy but useful in the begging of probe. Yes, it is indeed clumsy, and I am not convinced this is the right way to go. Firstly, you issue the reset twice. This is obviously not ideal and you have to hope the command with incorrect extension is ignored, and not interpreted as something different. But more importantly, you also do the same when called via spi_nor_remove(). At that point you have parsed SFDP and know the flash we are dealing with so you should already know which extension to use. So here is my suggestion: create a separate function, something like spi_nor_early_soft_reset(). In that function check a config variable to decide which extension to use and temporarily set nor->cmd_ext_type to that. Then in spi_nor_soft_reset() just use nor->cmd_ext_type, no need to hard code the extension. This way you will certainly use the correct extension at remove and will have a more accurate guess at probe time. I admit this isn't the cleanest solution, but this is the best I can come up with right now. > > Signed-off-by: JaimeLiao <jaimeliao.tw@gmail.com> > --- > drivers/mtd/spi/spi-nor-core.c | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > index 351ca9c3a8..707eb9c1d2 100644 > --- a/drivers/mtd/spi/spi-nor-core.c > +++ b/drivers/mtd/spi/spi-nor-core.c > @@ -3692,6 +3692,36 @@ static int spi_nor_soft_reset(struct spi_nor *nor) > */ > udelay(SPI_NOR_SRST_SLEEP_LEN); > > + /* Manufacturers with different command extension type. For suitting > + * different flash devices, using command extension type is equal "INVERT" > + * when second time Software Reset. > + */ > + > + nor->cmd_ext_type = SPI_NOR_EXT_INVERT; > + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 0), > + SPI_MEM_OP_NO_DUMMY, > + SPI_MEM_OP_NO_ADDR, > + SPI_MEM_OP_NO_DATA); > + spi_nor_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR); > + ret = spi_mem_exec_op(nor->spi, &op); > + if (ret) { > + dev_warn(nor->dev, "Software reset enable failed: %d\n", ret); > + goto out; > + } > + > + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST, 0), > + SPI_MEM_OP_NO_DUMMY, > + SPI_MEM_OP_NO_ADDR, > + SPI_MEM_OP_NO_DATA); > + spi_nor_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR); > + ret = spi_mem_exec_op(nor->spi, &op); > + if (ret) { > + dev_warn(nor->dev, "Software reset failed: %d\n", ret); > + goto out; > + } > + > + udelay(SPI_NOR_SRST_SLEEP_LEN); > + > out: > nor->cmd_ext_type = ext; > return ret; > -- > 2.17.1 >
Hi Pratyush
Thanks for your reply, I want to create "CONFIG_SPI_EXT_INVERT"
to separate extension types.
Changed as below, do you have any suggestion?
ext = nor->cmd_ext_type;
if (!nor->cmd_ext_type) {
nor->cmd_ext_type = SPI_NOR_EXT_REPEAT;
#ifdef CONFIG_SPI_NOR_EXT_INVERT
nor->cmd_ext_type = SPI_NOR_EXT_INVERT;
#endif
}
Pratyush Yadav <p.yadav@ti.com> 於 2021年8月30日 週一 下午7:30寫道：
> Hi,
>
> On 20/08/21 02:58PM, JaimeLiao wrote:
> > Power-on-Reset is a method to restore flash back to 1S-1S-1S mode from
> 8D-8D-8D
> > in the begging of probe.
> >
> > Command extension type is not standardized across flash vendors in DTR
> mode.
> >
> > For suiting different vendor flash devices, having second times
> Softreset with
> > different types is clumsy but useful in the begging of probe.
>
> Yes, it is indeed clumsy, and I am not convinced this is the right way
> to go.
>
> Firstly, you issue the reset twice. This is obviously not ideal and you
> have to hope the command with incorrect extension is ignored, and not
> interpreted as something different. But more importantly, you also do
> the same when called via spi_nor_remove(). At that point you have parsed
> SFDP and know the flash we are dealing with so you should already know
> which extension to use.
>
> So here is my suggestion: create a separate function, something like
> spi_nor_early_soft_reset(). In that function check a config variable to
> decide which extension to use and temporarily set nor->cmd_ext_type to
> that. Then in spi_nor_soft_reset() just use nor->cmd_ext_type, no need
> to hard code the extension. This way you will certainly use the correct
> extension at remove and will have a more accurate guess at probe time.
>
> I admit this isn't the cleanest solution, but this is the best I can
> come up with right now.
>
> >
> > Signed-off-by: JaimeLiao <jaimeliao.tw@gmail.com>
> > ---
> > drivers/mtd/spi/spi-nor-core.c | 30 ++++++++++++++++++++++++++++++
> > 1 file changed, 30 insertions(+)
> >
> > diff --git a/drivers/mtd/spi/spi-nor-core.c
> b/drivers/mtd/spi/spi-nor-core.c
> > index 351ca9c3a8..707eb9c1d2 100644
> > --- a/drivers/mtd/spi/spi-nor-core.c
> > +++ b/drivers/mtd/spi/spi-nor-core.c
> > @@ -3692,6 +3692,36 @@ static int spi_nor_soft_reset(struct spi_nor *nor)
> > */
> > udelay(SPI_NOR_SRST_SLEEP_LEN);
> >
> > + /* Manufacturers with different command extension type. For
> suitting
> > + * different flash devices, using command extension type is equal
> "INVERT"
> > + * when second time Software Reset.
> > + */
> > +
> > + nor->cmd_ext_type = SPI_NOR_EXT_INVERT;
> > + op = (struct
> spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 0),
> > + SPI_MEM_OP_NO_DUMMY,
> > + SPI_MEM_OP_NO_ADDR,
> > + SPI_MEM_OP_NO_DATA);
> > + spi_nor_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
> > + ret = spi_mem_exec_op(nor->spi, &op);
> > + if (ret) {
> > + dev_warn(nor->dev, "Software reset enable failed: %d\n",
> ret);
> > + goto out;
> > + }
> > +
> > + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST,
> 0),
> > + SPI_MEM_OP_NO_DUMMY,
> > + SPI_MEM_OP_NO_ADDR,
> > + SPI_MEM_OP_NO_DATA);
> > + spi_nor_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
> > + ret = spi_mem_exec_op(nor->spi, &op);
> > + if (ret) {
> > + dev_warn(nor->dev, "Software reset failed: %d\n", ret);
> > + goto out;
> > + }
> > +
> > + udelay(SPI_NOR_SRST_SLEEP_LEN);
> > +
> > out:
> > nor->cmd_ext_type = ext;
> > return ret;
> > --
> > 2.17.1
> >
>
> --
> Regards,
> Pratyush Yadav
> Texas Instruments Inc.
>
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 351ca9c3a8..707eb9c1d2 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3692,6 +3692,36 @@ static int spi_nor_soft_reset(struct spi_nor *nor) */ udelay(SPI_NOR_SRST_SLEEP_LEN); + /* Manufacturers with different command extension type. For suitting + * different flash devices, using command extension type is equal "INVERT" + * when second time Software Reset. + */ + + nor->cmd_ext_type = SPI_NOR_EXT_INVERT; + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 0), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_NO_ADDR, + SPI_MEM_OP_NO_DATA); + spi_nor_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR); + ret = spi_mem_exec_op(nor->spi, &op); + if (ret) { + dev_warn(nor->dev, "Software reset enable failed: %d\n", ret); + goto out; + } + + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST, 0), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_NO_ADDR, + SPI_MEM_OP_NO_DATA); + spi_nor_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR); + ret = spi_mem_exec_op(nor->spi, &op); + if (ret) { + dev_warn(nor->dev, "Software reset failed: %d\n", ret); + goto out; + } + + udelay(SPI_NOR_SRST_SLEEP_LEN); + out: nor->cmd_ext_type = ext; return ret;
Power-on-Reset is a method to restore flash back to 1S-1S-1S mode from 8D-8D-8D in the begging of probe. Command extension type is not standardized across flash vendors in DTR mode. For suiting different vendor flash devices, having second times Softreset with different types is clumsy but useful in the begging of probe. Signed-off-by: JaimeLiao <jaimeliao.tw@gmail.com> --- drivers/mtd/spi/spi-nor-core.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)