All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pratyush Yadav <p.yadav@ti.com>
To: Mark Brown <broonie@kernel.org>
Cc: Tudor Ambarus <tudor.ambarus@microchip.com>,
	Michael Walle <michael@walle.cc>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	<linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
	<linux-spi@vger.kernel.org>
Subject: Re: [PATCH 4/6] spi: spi-mem: reject partial cycle transfers in 8D-8D-8D mode
Date: Fri, 7 May 2021 19:26:33 +0530	[thread overview]
Message-ID: <20210507135631.maue7gorfzsv4qpk@ti.com> (raw)
In-Reply-To: <20210507125533.GA6383@sirena.org.uk>

On 07/05/21 01:55PM, Mark Brown wrote:
> On Fri, May 07, 2021 at 12:48:27AM +0530, Pratyush Yadav wrote:
> > In 8D-8D-8D mode two bytes are transferred per cycle. So an odd number
> > of bytes cannot be transferred because it would leave a residual half
> > cycle at the end. Consider such a transfer invalid and reject it.
> > 
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > 
> > ---
> > This patch should go through the SPI tree but I have included it in this
> > series because if it goes in before patches 1-3, Micron MT35XU and
> > Cypress S28HS flashes will stop working correctly.
> 
> It seems like this should probably even go in as a fix even if nothing
> is broken with mainline right now, it's the sort of thing some out of
> tree patch could easily trigger.  Unless anyone objects I'll do that.

If it does get backported to stable branches, patches 1-3 would have to 
go in _before_ this one. Otherwise it will break the two 8D-8D-8D 
flashes: Micron MT35XU512ABA and Cypress S28HS512T. Without patch 1 
8D-8D-8D mode will not be selected correctly on these two flashes. The 
supports_op() will reject it because of 1 data byte. This is a 
relatively safe patch for backporting to a stable branch.

Patches 2 and 3 are a slightly different matter. They add an extra 
register write. But most controllers I've come across don't support 
1-byte writes in 8D mode. It is likely that they are sending 
bogus/undefined values in the second byte and deasserting CS only after 
the cycle is done. So they should _in theory_ change undefined behaviour 
to defined behaviour.

Still, they introduce an extra register write. I'm not sure how 
risk-tolerant you want to be for stable backports. I will leave the 
judgement to you or Tudor or Vignesh.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

WARNING: multiple messages have this Message-ID (diff)
From: Pratyush Yadav <p.yadav@ti.com>
To: Mark Brown <broonie@kernel.org>
Cc: Tudor Ambarus <tudor.ambarus@microchip.com>,
	Michael Walle <michael@walle.cc>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	<linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
	<linux-spi@vger.kernel.org>
Subject: Re: [PATCH 4/6] spi: spi-mem: reject partial cycle transfers in 8D-8D-8D mode
Date: Fri, 7 May 2021 19:26:33 +0530	[thread overview]
Message-ID: <20210507135631.maue7gorfzsv4qpk@ti.com> (raw)
In-Reply-To: <20210507125533.GA6383@sirena.org.uk>

On 07/05/21 01:55PM, Mark Brown wrote:
> On Fri, May 07, 2021 at 12:48:27AM +0530, Pratyush Yadav wrote:
> > In 8D-8D-8D mode two bytes are transferred per cycle. So an odd number
> > of bytes cannot be transferred because it would leave a residual half
> > cycle at the end. Consider such a transfer invalid and reject it.
> > 
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > 
> > ---
> > This patch should go through the SPI tree but I have included it in this
> > series because if it goes in before patches 1-3, Micron MT35XU and
> > Cypress S28HS flashes will stop working correctly.
> 
> It seems like this should probably even go in as a fix even if nothing
> is broken with mainline right now, it's the sort of thing some out of
> tree patch could easily trigger.  Unless anyone objects I'll do that.

If it does get backported to stable branches, patches 1-3 would have to 
go in _before_ this one. Otherwise it will break the two 8D-8D-8D 
flashes: Micron MT35XU512ABA and Cypress S28HS512T. Without patch 1 
8D-8D-8D mode will not be selected correctly on these two flashes. The 
supports_op() will reject it because of 1 data byte. This is a 
relatively safe patch for backporting to a stable branch.

Patches 2 and 3 are a slightly different matter. They add an extra 
register write. But most controllers I've come across don't support 
1-byte writes in 8D mode. It is likely that they are sending 
bogus/undefined values in the second byte and deasserting CS only after 
the cycle is done. So they should _in theory_ change undefined behaviour 
to defined behaviour.

Still, they introduce an extra register write. I'm not sure how 
risk-tolerant you want to be for stable backports. I will leave the 
judgement to you or Tudor or Vignesh.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

  reply	other threads:[~2021-05-07 13:56 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-06 19:18 [PATCH 0/6] Avoid odd length/address read/writes in 8D-8D-8D mode Pratyush Yadav
2021-05-06 19:18 ` Pratyush Yadav
2021-05-06 19:18 ` [PATCH 1/6] mtd: spi-nor: core: use 2 data bytes for template ops Pratyush Yadav
2021-05-06 19:18   ` Pratyush Yadav
2021-05-06 19:18 ` [PATCH 2/6] mtd: spi-nor: spansion: write 2 bytes when disabling Octal DTR mode Pratyush Yadav
2021-05-06 19:18   ` Pratyush Yadav
2021-05-06 19:18 ` [PATCH 3/6] mtd: spi-nor: micron-st: " Pratyush Yadav
2021-05-06 19:18   ` Pratyush Yadav
2021-05-06 19:18 ` [PATCH 4/6] spi: spi-mem: reject partial cycle transfers in 8D-8D-8D mode Pratyush Yadav
2021-05-06 19:18   ` Pratyush Yadav
2021-05-07 12:55   ` Mark Brown
2021-05-07 12:55     ` Mark Brown
2021-05-07 13:56     ` Pratyush Yadav [this message]
2021-05-07 13:56       ` Pratyush Yadav
2021-05-07 15:31       ` Mark Brown
2021-05-07 15:31         ` Mark Brown
2021-05-07 15:48   ` Mark Brown
2021-05-07 15:48     ` Mark Brown
2021-05-07 16:49     ` Pratyush Yadav
2021-05-07 16:49       ` Pratyush Yadav
2021-05-06 19:18 ` [PATCH 5/6] mtd: spi-nor: core; avoid odd length/address reads on " Pratyush Yadav
2021-05-06 19:18   ` Pratyush Yadav
2021-05-07 15:51   ` Michael Walle
2021-05-07 15:51     ` Michael Walle
2021-05-07 18:04     ` Pratyush Yadav
2021-05-07 18:04       ` Pratyush Yadav
2021-05-07 18:14       ` Michael Walle
2021-05-07 18:14         ` Michael Walle
2021-05-07 18:23         ` Pratyush Yadav
2021-05-07 18:23           ` Pratyush Yadav
2021-05-06 19:18 ` [PATCH 6/6] mtd: spi-nor: core; avoid odd length/address writes in " Pratyush Yadav
2021-05-06 19:18   ` Pratyush Yadav
2021-05-07 15:56   ` Michael Walle
2021-05-07 15:56     ` Michael Walle
2021-05-07 17:02     ` Pratyush Yadav
2021-05-07 17:02       ` Pratyush Yadav
2021-05-07 15:50 ` [PATCH 0/6] Avoid odd length/address read/writes " Mark Brown
2021-05-07 15:50   ` Mark Brown

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=20210507135631.maue7gorfzsv4qpk@ti.com \
    --to=p.yadav@ti.com \
    --cc=broonie@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=michael@walle.cc \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.