linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Pratyush Yadav <p.yadav@ti.com>
To: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <tudor.ambarus@microchip.com>,
	Richard Weinberger <richard@nod.at>, Sekhar Nori <nsekhar@ti.com>,
	linux-kernel@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	linux-spi@vger.kernel.org, Mark Brown <broonie@kernel.org>,
	linux-mtd@lists.infradead.org,
	Miquel Raynal <miquel.raynal@bootlin.com>
Subject: Re: [PATCH v2 06/11] mtd: spi-nor: add support for DTR protocol
Date: Fri, 28 Feb 2020 15:06:58 +0530	[thread overview]
Message-ID: <20200228093658.zc3uifqg4zruokq3@ti.com> (raw)
In-Reply-To: <20200227175841.51435e3f@collabora.com>

Hi Boris,

On 27/02/20 05:58PM, Boris Brezillon wrote:
> On Wed, 26 Feb 2020 15:06:58 +0530
> Pratyush Yadav <p.yadav@ti.com> wrote:
> 
> > Double Transfer Rate (DTR) is SPI protocol in which data is transferred
> > on each clock edge as opposed to on each clock cycle. Make
> > framework-level changes to allow supporting flashes in DTR mode.
> > 
> > Right now, mixed DTR modes are not supported. So, for example a mode
> > like 4S-4D-4D will not work. All phases need to be either DTR or STR.
> 
> Didn't go deep into the patch but at first glance you don't seem to
> extend the framework to support stateful modes as I tried to do here
> [1]. That's really something we should address before considering
> supporting xD-xD-xD modes, unless the SPI-NOR only supports one
> stateful mode. If we don't do that first, we might face all sort of
> unpleasant issues:
> 
> * kexec not working correctly because the previous kernel left the NOR
>   in an unknown state
> * suspend/resume not working properly
> * linux not booting properly because the bootloader left the device in
>   its non-default mode
> * ...

Correct. I am working on a follow-up series that takes care of these 
problems. The series will allow spi-nor to detect what mode the flash is 
in and then run the SFPD procedure in that mode (or maybe switch to 
single SPI mode and then go about its business as usual? I haven't 
figured out all the details yet).

So for the context of this series, assume we are handed the flash in 
single SPI mode.
 
> [1]https://patchwork.kernel.org/cover/10638055/

BTW, I took a quick look at this series but I don't see any code that 
tries to detect which mode the flash is in (which is the troublesome 
part [0]). So, for example, if the bootloader leaves the flash in 
8D-8D-8D mode, how would your series handle that situation?

[0] There are multiple problems to take care of when trying to detect 
    which mode a flash is in. We can try reading SFDP in each mode and 
    whichever mode gives us the correct "SFDP" signature is the mode the 
    flash is in. But the problem is that even in xSPI standard Read SFDP 
    command is optional in 8D-8D-8D mode, let alone non-xSPI flashes. 
    Another problem is that the address bytes and dummy cycles for Read 
    SFDP are not the same for every flash. The xSPI standard says 
    address bytes can be 3/4 and dummy cycles can be 8/20. So, for 
    example, Cypress s28hs/s28ht family and Micron Xccela (mt35x) family 
    use 4 address bytes, but the Adesto ATXP032/ATXP032R flashes use 3 
    address bytes.

    Say that a flash supports Read SFDP in 8D-8D-8D mode and we try all 
    the combinations to find out which mode the flash is in, we now have 
    the problem of actually identifying the flash. Unfortunately, the 
    Read ID command is not uniform across flash vendors. The Micron 
    Xccela flashes use 8 dummy cycles and no address bytes for Read ID. 
    The Cypress s28hs/t family uses configurable dummy cycles 
    (defaulting to 3) and needs 4 dummy address bytes all of which are 
    0.

    If we can't find out which flash it is, we can't run its fixup 
    hooks, and might end up running it with incorrect settings. And all 
    this is assuming a flash even has SFDP and has it available in all 
    modes.

    So, the only solution I can now think of is having the flash name in 
    its compatible string in the device tree. This way we can skip all 
    the Read ID ugliness and can have flash-specific hooks to make it 
    easier to detect the mode it is in (though I wonder if it is even 
    possible to detect the mode in a flash that doesn't have SFDP in 
    8D-8D-8D).

    Thoughts? Is there a better way to solve this problem that I didn't 
    think of?

-- 
Regards,
Pratyush Yadav
Texas Instruments India

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply	other threads:[~2020-02-28  9:37 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-26  9:36 [PATCH v2 00/11] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
2020-02-26  9:36 ` [PATCH v2 01/11] dt-bindings: spi: allow expressing DTR capability Pratyush Yadav
2020-02-27 16:11   ` Boris Brezillon
2020-02-27 16:28     ` Mark Brown
2020-02-27 16:40       ` Geert Uytterhoeven
2020-02-27 16:44         ` Mark Brown
2020-02-27 17:03           ` Geert Uytterhoeven
2020-03-02  9:53             ` Pratyush Yadav
2020-02-27 17:06           ` Boris Brezillon
2020-02-27 16:29   ` Geert Uytterhoeven
2020-02-28  9:46     ` Pratyush Yadav
2020-02-26  9:36 ` [PATCH v2 02/11] spi: set mode bits for "spi-rx-dtr" and "spi-tx-dtr" Pratyush Yadav
2020-02-27 16:23   ` Boris Brezillon
2020-03-02  9:48     ` Pratyush Yadav
2020-03-02 10:20       ` Boris Brezillon
2020-02-26  9:36 ` [PATCH v2 03/11] spi: spi-mem: allow specifying whether an op is DTR or not Pratyush Yadav
2020-02-27 16:36   ` Boris Brezillon
2020-02-26  9:36 ` [PATCH v2 04/11] spi: spi-mem: allow specifying a command's extension Pratyush Yadav
2020-02-27 16:44   ` Boris Brezillon
2020-02-28  9:41     ` Pratyush Yadav
2020-02-26  9:36 ` [PATCH v2 05/11] spi: cadence-quadspi: Add support for octal DTR flashes Pratyush Yadav
2020-02-26  9:36 ` [PATCH v2 06/11] mtd: spi-nor: add support for DTR protocol Pratyush Yadav
2020-02-27 16:58   ` Boris Brezillon
2020-02-28  9:36     ` Pratyush Yadav [this message]
2020-02-28 10:53       ` Boris Brezillon
2020-02-28 12:07         ` Pratyush Yadav
2020-02-28 13:18           ` Boris Brezillon
2020-02-26  9:36 ` [PATCH v2 07/11] mtd: spi-nor: get command opcode extension type from BFPT Pratyush Yadav
2020-02-26  9:37 ` [PATCH v2 08/11] mtd: spi-nor: parse xSPI Profile 1.0 table Pratyush Yadav
2020-02-26  9:37 ` [PATCH v2 09/11] mtd: spi-nor: use dummy cycle and address width info from SFDP Pratyush Yadav
2020-02-26  9:37 ` [PATCH v2 10/11] mtd: spi-nor: enable octal DTR mode when possible Pratyush Yadav
2020-02-26  9:37 ` [PATCH v2 11/11] mtd: spi-nor: add support for Cypress Semper flash Pratyush Yadav

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=20200228093658.zc3uifqg4zruokq3@ti.com \
    --to=p.yadav@ti.com \
    --cc=boris.brezillon@collabora.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=nsekhar@ti.com \
    --cc=richard@nod.at \
    --cc=robh+dt@kernel.org \
    --cc=tudor.ambarus@microchip.com \
    --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).