[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200930120143.w7tddcta7sekdzju@ti.com>
Date: Wed, 30 Sep 2020 17:31:45 +0530
From: Pratyush Yadav <p.yadav@...com>
To: <Tudor.Ambarus@...rochip.com>
CC: <miquel.raynal@...tlin.com>, <richard@....at>, <vigneshr@...com>,
<linux-mtd@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
<nsekhar@...com>, <boris.brezillon@...labora.com>
Subject: Re: [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support
On 30/09/20 09:57AM, Tudor.Ambarus@...rochip.com wrote:
> On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> > Hi,
>
> Hello,
>
> >
> > This series adds support for Octal DTR flashes in the SPI NOR framework,
> > and then adds hooks for the Cypress Semper and Micron Xcella flashes to
> > allow running them in Octal DTR mode. This series assumes that the flash
> > is handed to the kernel in Legacy SPI mode.
> >
>
> I like this series. There are some comments that can be addressed, no big
> deal though.
>
> I think that we shouldn't let the door open for users with flashes that
> enter in a X-X-X mode in a non-volatile way. Think of two flashes that have
> the same X-X-X mode enable sequence, but in which only the EN bit differs:
> for one the EN bit is volatile and for the other it is non-volatile. Users
> of the later flash that try to enable the X-X-X mode (using our code) will
> end up with the flash in a mode from which they can't recover. Thus my advice
> is to consider by default all the flashes, as X-X-X mode non-volatile flashes,
> and to not let them use the X-X-X mode enable methods. Flashes that can enter
> X-X-X modes in a volatile way, should discover this capability by parsing the
> optional SFDP SCCR Map. Those that don't define this table, should pass this
> capability as a flash_info flag when declaring the flash. With these, users
> should be conscious about the V or NV modes, and the risk to end up with
> flashes for which there is no software to recover is diminished. This is what
> I tried in RFC 1/3 and RFC 3/3. I think those 2 patches should be part of
> this series. 14/15 and 15/15 should be updated accordingly. RFC 2/3 has room
> for discussions because it provides access system-wise, while ideally would be
> to do it at flash granularity. I'll wait for your feedback on those.
FWIW, I took a quick look at the RFC patches and I agree that patches
1/3 and 3/3 are good and I'll add them to my series. I don't like 2/3
mainly because of the same concerns that Vignesh has: wearing down the
NV bits. Also IMO we should avoid touching NV bits as much as we can
because I had used some on the Cypress flash during development, and I
ended up "bricking" the flash a few times (for example when I used the
wrong dummy cycles by mistake or the controller driver did something
wrong). Since I knew exactly which registers I was touching I could
recover it but the process is quite painful.
>
> > Tested on TI J721E EVM with 1-bit ECC on the Cypress flash.
>
> As a tip, when introducing some big changes to the core, would be nice to be
> assured that things that worked previously are still working now.
> An erase-write-read-back test in Quad SPI would suffice. Probably you have
> already did that, but I haven't seen it mentioned.
Ok, I'll include them next time. I tested this series on the CQSPI and
TI QSPI controllers on MT25Q, S25FL for regressions, and on MT35 and S28
for Octal DTR.
--
Regards,
Pratyush Yadav
Texas Instruments India
Powered by blists - more mailing lists