[PATCH v2 2/4] mtd: spi-nor-core: Adding different type of command extension in Soft Reset

[liao jaime]

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 at 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 at 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.
>