linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pratyush Yadav <p.yadav@ti.com>
To: Boris Brezillon <boris.brezillon@collabora.com>
Cc: <Tudor.Ambarus@microchip.com>, <alexandre.belloni@bootlin.com>,
	<vigneshr@ti.com>, <richard@nod.at>, <nsekhar@ti.com>,
	<Nicolas.Ferre@microchip.com>, <linux-kernel@vger.kernel.org>,
	<Ludovic.Desroches@microchip.com>, <broonie@kernel.org>,
	<linux-mtd@lists.infradead.org>, <miquel.raynal@bootlin.com>,
	<linux-spi@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v4 00/16] mtd: spi-nor: add xSPI Octal DTR support
Date: Mon, 11 May 2020 23:54:55 +0530	[thread overview]
Message-ID: <20200511182453.ivfwu4wlfn2ta5ag@ti.com> (raw)
In-Reply-To: <20200511112712.466f7246@collabora.com>

On 11/05/20 11:27AM, Boris Brezillon wrote:
> On Mon, 11 May 2020 09:00:35 +0000
> <Tudor.Ambarus@microchip.com> wrote:
> 
> > Hi, Pratyush, Boris,
> > 
> > On Friday, April 24, 2020 9:43:54 PM EEST Pratyush Yadav wrote:
> > > This series adds support for octal DTR flashes in the spi-nor framework,  
> > 
> > I'm still learning about this, but I can give you my 2 cents as of now, to 
> > open the discussion. Enabling 2-2-2, 4-4-4, and 8-8-8 modes is dangerous 
> > because the flash may not recover from unexpected resets. Entering one of 
> > these modes can be:
> > 1/ volatile selectable, the device return to the 1-1-1 protocol after the next 
> > power-on. I guess this is conditioned by the optional RESET pin, but I'll have 
> > to check. Also the flash can return to the 1-1-1 mode using the software reset 
> > or through writing to its Configuration Register, without power-on or power-
> > off.
> 
> My understanding is that there's no standard software reset procedure
> that guarantees no conflict with existing 1S commands, so even the
> software reset approach doesn't work here.
> 
> > 2/ non-volatile selectable in which RESET# and software reset are useless, the 
> > flash defaults to the mode selected in the non volatile Configuration Register 
> > bits. The only way to get back to 1-1-1 is to write to the Configuration 
> > Register.
> 
> I'm less worried about this case though, since I'd expect the ROM
> code and bootloaders to be able to deal with xD-xD-xD modes when the
> flash is set in this mode by default. That implies letting Linux know
> about this default mode of course, maybe through an extra DT
> property/cmdline param.
> 
> > 
> > Not recovering from unexpected resets is unacceptable. One should always 
> > prefer option 1/ and condition the entering in 2-2-2, 4-4-4 and 8-8-8 with the 
> > presence of the optional RESET pin.
> 
> Totally agree with you on that one, but we know what happens in
> practice...
> 
> > 
> > For the unfortunate flashes that support just option 2/, we should not enter 
> > these modes on our own, just by discovering the capabilities from the SFDP 
> > tables or by the flags in the flash_info struct. The best we can do for them 
> > is to move the responsibility to the user. Maybe to add a Kconfig option that 
> > is disabled by default with which we condition the entering in 2-2-2, 4-4-4 or 
> > 8-8-8 modes.
> 
> Hm, a Kconfig option doesn't sound like the right solution to the
> problem, since it should be a per-flash decision, not something you set
> system-wise.

Agreed.

Is there any such flash in use today? The two flashes the series adds 
support for both have volatile configuration for 8D mode. Unless we have 
to support a flash like this in practice, I think such a change is out 
of the scope of this series.
 
> > Once entered in one of these modes, if an unexpected reset comes, 
> > you most likely are doomed, because early stage bootloaders may not work in 
> > these modes and you'll not be able to boot the board. Assuming that one uses 
> > other environment to boot the board, we should at least make sure that the 
> > flash works in linux after an unexpected reset. We should try to determine in 
> > which mode we are at init, so maybe an extension of the default_init hook is 
> > needed. But all this looks like a BIG compromise, I'm not yet sure if we 
> > should adress 2/. Thoughts?
> 
> We should definitely not write non-volatile regs on our own, but
> instead use the mode that's been chosen there. I doubt anyone
> setting the non-volative conf to 8D-8D-8D will ever want to go back to
> 1S-1S-1S anyway, so 8D -> 1S transitions are not really an issue, right?
> 
> Of course, that still leaves us with the 'mode detection' issue, and I
> have no solution other than flagging it through the DT/cmdline for that
> one...

Correct. I tried doing it, and the best way I could figure out was to 
try reading the SFDP signature in 1S and 8D mode, and see where we get 
the correct value. But unfortunately, because the Read ID command is 
different in 8D mode for different flashes, we can't then figure out 
which flash it actually is.

-- 
Regards,
Pratyush Yadav
Texas Instruments India

  reply	other threads:[~2020-05-11 18:25 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-24 18:43 [PATCH v4 00/16] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
2020-04-24 18:43 ` [PATCH v4 01/16] spi: spi-mem: allow specifying whether an op is DTR or not Pratyush Yadav
2020-04-24 18:43 ` [PATCH v4 02/16] spi: atmel-quadspi: reject DTR ops Pratyush Yadav
2020-04-30 11:32   ` Mark Brown
2020-04-30 12:17     ` Pratyush Yadav
2020-04-30 12:19       ` Mark Brown
2020-04-24 18:43 ` [PATCH v4 03/16] spi: spi-mem: allow specifying a command's extension Pratyush Yadav
2020-04-24 18:43 ` [PATCH v4 04/16] mtd: spi-nor: add support for DTR protocol Pratyush Yadav
2020-04-24 18:43 ` [PATCH v4 05/16] mtd: spi-nor: default to address width of 3 for configurable widths Pratyush Yadav
2020-04-26  3:53   ` Yicong Yang
2020-04-27 17:23     ` Pratyush Yadav
2020-04-28  1:34       ` Yicong Yang
2020-04-28  5:25         ` Tudor.Ambarus
2020-04-24 18:44 ` [PATCH v4 06/16] mtd: spi-nor: prepare BFPT parsing for JESD216 rev D Pratyush Yadav
2020-04-24 18:44 ` [PATCH v4 07/16] mtd: spi-nor: get command opcode extension type from BFPT Pratyush Yadav
2020-04-24 18:44 ` [PATCH v4 08/16] mtd: spi-nor: parse xSPI Profile 1.0 table Pratyush Yadav
2020-04-24 18:44 ` [PATCH v4 09/16] mtd: spi-nor: use dummy cycle and address width info from SFDP Pratyush Yadav
2020-04-24 18:44 ` [PATCH v4 10/16] mtd: spi-nor: do 2 byte reads for SR and FSR in DTR mode Pratyush Yadav
2020-04-24 18:44 ` [PATCH v4 11/16] mtd: spi-nor: enable octal DTR mode when possible Pratyush Yadav
2020-04-24 18:44 ` [PATCH v4 12/16] mtd: spi-nor: perform a Soft Reset on shutdown Pratyush Yadav
2020-05-10 11:22   ` Tudor.Ambarus
2020-05-11 18:01     ` Pratyush Yadav
2020-04-24 18:44 ` [PATCH v4 13/16] mtd: spi-nor: Disable Octal DTR mode on suspend Pratyush Yadav
2020-04-24 18:44 ` [PATCH v4 14/16] mtd: spi-nor: expose spi_nor_default_setup() in core.h Pratyush Yadav
2020-04-24 18:44 ` [PATCH v4 15/16] mtd: spi-nor: add support for Cypress Semper flash Pratyush Yadav
2020-04-24 18:44 ` [PATCH v4 16/16] mtd: spi-nor: allow using MT35XU512ABA in Octal DTR mode Pratyush Yadav
2020-05-11  9:00 ` [PATCH v4 00/16] mtd: spi-nor: add xSPI Octal DTR support Tudor.Ambarus
2020-05-11  9:27   ` Boris Brezillon
2020-05-11 18:24     ` Pratyush Yadav [this message]
2020-05-12  6:16     ` Tudor.Ambarus
2020-05-12  9:49       ` Vignesh Raghavendra
2020-05-12 11:29         ` Tudor.Ambarus
2020-05-12 18:46           ` Pratyush Yadav
2020-05-11  9:43   ` Vignesh Raghavendra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200511182453.ivfwu4wlfn2ta5ag@ti.com \
    --to=p.yadav@ti.com \
    --cc=Ludovic.Desroches@microchip.com \
    --cc=Nicolas.Ferre@microchip.com \
    --cc=Tudor.Ambarus@microchip.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=boris.brezillon@collabora.com \
    --cc=broonie@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=nsekhar@ti.com \
    --cc=richard@nod.at \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).