Re: [PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address writes in 8D-8D-8D mode

Re: [PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address writes in 8D-8D-8D mode

[Tudor Ambarus]

On 4/29/25 10:03 AM, Luke Wang wrote:
> Hi Pratyush,
> 
>  
> 
> I'm following up on this patch series [1] Avoid odd length/address read/
> writes in 8D-8D-8D mode. While some of the series has been merged, the
> patch 4-6 remains unmerged.
> 
>  
> 
> In fact, we also encountered similar read/write issue of odd address/
> length with NXP FSPI controller (spi-nxp-fspi.c). Currently, we handled
> the odd address/length in the controller driver, but I think this should
> be a common issue in the octal dtr mode. Was there a technical reason
> for not merging the core layer solution?

I guess I stumbled on those small comments and did not consider the
greater benefit of taking the patches. No one cared and we forgot about
it. Please address the comments and resubmit.
> 
>  
> 
> [1] Original submission:  https://lore.kernel.org/
> all/20210531181757.19458-1-p.yadav@ti.com/ <https://lore.kernel.org/
> all/20210531181757.19458-1-p.yadav@ti.com/>
> 
>  
> 
> Regards,
> 
> Luke Wang
> 
>  
> 

Re: [PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address writes in 8D-8D-8D mode

[Pratyush Yadav]

Hi Luke,

On Tue, Apr 29 2025, Tudor Ambarus wrote:

> On 4/29/25 10:03 AM, Luke Wang wrote:
>> Hi Pratyush,
>> 
>> I'm following up on this patch series [1] Avoid odd length/address read/
>> writes in 8D-8D-8D mode. While some of the series has been merged, the
>> patch 4-6 remains unmerged.
>> 
>> In fact, we also encountered similar read/write issue of odd address/
>> length with NXP FSPI controller (spi-nxp-fspi.c). Currently, we handled
>> the odd address/length in the controller driver, but I think this should
>> be a common issue in the octal dtr mode. Was there a technical reason
>> for not merging the core layer solution?
>
> I guess I stumbled on those small comments and did not consider the
> greater benefit of taking the patches. No one cared and we forgot about
> it. Please address the comments and resubmit.

Yes, it should have been a simple next revision from me but apparently
it fell through the cracks. I do strongly agree that this should be done
in SPI NOR, and not in controller drivers. So it would be great if you
can respin the remaining patches of the series.

-- 
Regards,
Pratyush Yadav

Re: [PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address writes in 8D-8D-8D mode

[Miquel Raynal]

Hello,

On 07/05/2025 at 09:43:25 GMT, Pratyush Yadav <pratyush@kernel.org> wrote:

> Hi Luke,
>
> On Tue, Apr 29 2025, Tudor Ambarus wrote:
>
>> On 4/29/25 10:03 AM, Luke Wang wrote:
>>> Hi Pratyush,
>>> 
>>> I'm following up on this patch series [1] Avoid odd length/address read/
>>> writes in 8D-8D-8D mode. While some of the series has been merged, the
>>> patch 4-6 remains unmerged.
>>> 
>>> In fact, we also encountered similar read/write issue of odd address/
>>> length with NXP FSPI controller (spi-nxp-fspi.c). Currently, we handled
>>> the odd address/length in the controller driver, but I think this should
>>> be a common issue in the octal dtr mode. Was there a technical reason
>>> for not merging the core layer solution?
>>
>> I guess I stumbled on those small comments and did not consider the
>> greater benefit of taking the patches. No one cared and we forgot about
>> it. Please address the comments and resubmit.
>
> Yes, it should have been a simple next revision from me but apparently
> it fell through the cracks. I do strongly agree that this should be done
> in SPI NOR, and not in controller drivers. So it would be great if you
> can respin the remaining patches of the series.

The fact is that we will have octal DTR support in SPI NAND as well at some
point, hence a common solution would be welcome as we will likely face
similar problems when performing these unaligned accesses. I don't know
how feasible it is yet, but if we have a fix for SPI NOR, we will need
something similar for SPI NAND.

Thanks,
Miquèl

RE: [PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address writes in 8D-8D-8D mode

[Bough Chen]

> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: 2025年5月12日 15:57
> To: Pratyush Yadav <pratyush@kernel.org>
> Cc: Tudor Ambarus <tudor.ambarus@linaro.org>; Luke Wang
> <ziniu.wang_1@nxp.com>; broonie@kernel.org; linux-kernel@vger.kernel.org;
> linux-mtd@lists.infradead.org; linux-spi@vger.kernel.org; michael@walle.cc;
> p.yadav@ti.com; richard@nod.at; vigneshr@ti.com; Bough Chen
> <haibo.chen@nxp.com>; Han Xu <han.xu@nxp.com>
> Subject: Re: [PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address writes
> in 8D-8D-8D mode
> 
> Hello,
> 
> On 07/05/2025 at 09:43:25 GMT, Pratyush Yadav <pratyush@kernel.org>
> wrote:
> 
> > Hi Luke,
> >
> > On Tue, Apr 29 2025, Tudor Ambarus wrote:
> >
> >> On 4/29/25 10:03 AM, Luke Wang wrote:
> >>> Hi Pratyush,
> >>>
> >>> I'm following up on this patch series [1] Avoid odd length/address
> >>> read/ writes in 8D-8D-8D mode. While some of the series has been
> >>> merged, the patch 4-6 remains unmerged.
> >>>
> >>> In fact, we also encountered similar read/write issue of odd
> >>> address/ length with NXP FSPI controller (spi-nxp-fspi.c).
> >>> Currently, we handled the odd address/length in the controller
> >>> driver, but I think this should be a common issue in the octal dtr
> >>> mode. Was there a technical reason for not merging the core layer
> solution?
> >>
> >> I guess I stumbled on those small comments and did not consider the
> >> greater benefit of taking the patches. No one cared and we forgot
> >> about it. Please address the comments and resubmit.
> >
> > Yes, it should have been a simple next revision from me but apparently
> > it fell through the cracks. I do strongly agree that this should be
> > done in SPI NOR, and not in controller drivers. So it would be great
> > if you can respin the remaining patches of the series.
> 
> The fact is that we will have octal DTR support in SPI NAND as well at some
> point, hence a common solution would be welcome as we will likely face similar
> problems when performing these unaligned accesses. I don't know how feasible
> it is yet, but if we have a fix for SPI NOR, we will need something similar for SPI
> NAND.

Currently for octal DTR SPI NAND, seems do not support 8D-8D-8D, I check winbond, only support 1S-1D-8D mode. SPI NAND and SPI NOR share spi-mem.c, and in spi-mem.c, spi_mem_default_supports_op(), for DTR mode, the command must be DTR, so do not support 1S-1D-8D mode. This is the problem I can see.

We have local patch to handle the odd/even length/address in flexspi driver, I will prepare to move these to spi-nor core and send out for review first, but for SPI-NAND DTR mode, may need further efforts.

Regards
Haibo Chen 
> 
> Thanks,
> Miquèl

Re: [PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address writes in 8D-8D-8D mode

[Pratyush Yadav]

On Mon, May 12 2025, Miquel Raynal wrote:

> Hello,
>
> On 07/05/2025 at 09:43:25 GMT, Pratyush Yadav <pratyush@kernel.org> wrote:
>
>> Hi Luke,
>>
>> On Tue, Apr 29 2025, Tudor Ambarus wrote:
>>
>>> On 4/29/25 10:03 AM, Luke Wang wrote:
>>>> Hi Pratyush,
>>>> 
>>>> I'm following up on this patch series [1] Avoid odd length/address read/
>>>> writes in 8D-8D-8D mode. While some of the series has been merged, the
>>>> patch 4-6 remains unmerged.
>>>> 
>>>> In fact, we also encountered similar read/write issue of odd address/
>>>> length with NXP FSPI controller (spi-nxp-fspi.c). Currently, we handled
>>>> the odd address/length in the controller driver, but I think this should
>>>> be a common issue in the octal dtr mode. Was there a technical reason
>>>> for not merging the core layer solution?
>>>
>>> I guess I stumbled on those small comments and did not consider the
>>> greater benefit of taking the patches. No one cared and we forgot about
>>> it. Please address the comments and resubmit.
>>
>> Yes, it should have been a simple next revision from me but apparently
>> it fell through the cracks. I do strongly agree that this should be done
>> in SPI NOR, and not in controller drivers. So it would be great if you
>> can respin the remaining patches of the series.
>
> The fact is that we will have octal DTR support in SPI NAND as well at some
> point, hence a common solution would be welcome as we will likely face
> similar problems when performing these unaligned accesses. I don't know
> how feasible it is yet, but if we have a fix for SPI NOR, we will need
> something similar for SPI NAND.

The common solution would then probably be in SPI MEM? Since you need to
make sure you don't do an out of bounds read, you need to know the size
of the flash at least. That is recorded in the dirmap operations, so
perhaps we can have this logic in spi_mem_dirmap_{read,write}()? Haven't
thought too deeply so not sure if it would end up being a good idea.

-- 
Regards,
Pratyush Yadav

RE: [PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address writes in 8D-8D-8D mode

[Bough Chen]

> -----Original Message-----
> From: Pratyush Yadav <pratyush@kernel.org>
> Sent: 2025年5月12日 17:35
> To: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Pratyush Yadav <pratyush@kernel.org>; Tudor Ambarus
> <tudor.ambarus@linaro.org>; Luke Wang <ziniu.wang_1@nxp.com>;
> broonie@kernel.org; linux-kernel@vger.kernel.org;
> linux-mtd@lists.infradead.org; linux-spi@vger.kernel.org; michael@walle.cc;
> p.yadav@ti.com; richard@nod.at; vigneshr@ti.com; Bough Chen
> <haibo.chen@nxp.com>; Han Xu <han.xu@nxp.com>
> Subject: Re: [PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address writes
> in 8D-8D-8D mode
> 
> [Some people who received this message don't often get email from
> pratyush@kernel.org. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Mon, May 12 2025, Miquel Raynal wrote:
> 
> > Hello,
> >
> > On 07/05/2025 at 09:43:25 GMT, Pratyush Yadav <pratyush@kernel.org>
> wrote:
> >
> >> Hi Luke,
> >>
> >> On Tue, Apr 29 2025, Tudor Ambarus wrote:
> >>
> >>> On 4/29/25 10:03 AM, Luke Wang wrote:
> >>>> Hi Pratyush,
> >>>>
> >>>> I'm following up on this patch series [1] Avoid odd length/address
> >>>> read/ writes in 8D-8D-8D mode. While some of the series has been
> >>>> merged, the patch 4-6 remains unmerged.
> >>>>
> >>>> In fact, we also encountered similar read/write issue of odd
> >>>> address/ length with NXP FSPI controller (spi-nxp-fspi.c).
> >>>> Currently, we handled the odd address/length in the controller
> >>>> driver, but I think this should be a common issue in the octal dtr
> >>>> mode. Was there a technical reason for not merging the core layer
> solution?
> >>>
> >>> I guess I stumbled on those small comments and did not consider the
> >>> greater benefit of taking the patches. No one cared and we forgot
> >>> about it. Please address the comments and resubmit.
> >>
> >> Yes, it should have been a simple next revision from me but
> >> apparently it fell through the cracks. I do strongly agree that this
> >> should be done in SPI NOR, and not in controller drivers. So it would
> >> be great if you can respin the remaining patches of the series.
> >
> > The fact is that we will have octal DTR support in SPI NAND as well at
> > some point, hence a common solution would be welcome as we will likely
> > face similar problems when performing these unaligned accesses. I
> > don't know how feasible it is yet, but if we have a fix for SPI NOR,
> > we will need something similar for SPI NAND.
> 
> The common solution would then probably be in SPI MEM? Since you need to
> make sure you don't do an out of bounds read, you need to know the size of the
> flash at least. That is recorded in the dirmap operations, so perhaps we can have
> this logic in spi_mem_dirmap_{read,write}()? Haven't thought too deeply so not
> sure if it would end up being a good idea.

Seems reasonable, I will have a try.

Regards
Haibo Chen
> 
> --
> Regards,
> Pratyush Yadav

RE: [PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address writes in 8D-8D-8D mode

[Luke Wang]

> -----Original Message-----
> From: Bough Chen <haibo.chen@nxp.com>
> Sent: Monday, May 12, 2025 7:10 PM
> To: Pratyush Yadav <pratyush@kernel.org>; Miquel Raynal
> <miquel.raynal@bootlin.com>
> Cc: Tudor Ambarus <tudor.ambarus@linaro.org>; Luke Wang
> <ziniu.wang_1@nxp.com>; broonie@kernel.org; linux-
> kernel@vger.kernel.org; linux-mtd@lists.infradead.org; linux-
> spi@vger.kernel.org; michael@walle.cc; p.yadav@ti.com; richard@nod.at;
> vigneshr@ti.com; Han Xu <han.xu@nxp.com>
> Subject: RE: [PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address
> writes in 8D-8D-8D mode
> 
> > -----Original Message-----
> > From: Pratyush Yadav <pratyush@kernel.org>
> > Sent: 2025年5月12日 17:35
> > To: Miquel Raynal <miquel.raynal@bootlin.com>
> > Cc: Pratyush Yadav <pratyush@kernel.org>; Tudor Ambarus
> > <tudor.ambarus@linaro.org>; Luke Wang <ziniu.wang_1@nxp.com>;
> > broonie@kernel.org; linux-kernel@vger.kernel.org;
> > linux-mtd@lists.infradead.org; linux-spi@vger.kernel.org; michael@walle.cc;
> > p.yadav@ti.com; richard@nod.at; vigneshr@ti.com; Bough Chen
> > <haibo.chen@nxp.com>; Han Xu <han.xu@nxp.com>
> > Subject: Re: [PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address
> writes
> > in 8D-8D-8D mode
> >
> > [Some people who received this message don't often get email from
> > pratyush@kernel.org. Learn why this is important at
> > https://aka.ms/LearnAboutSenderIdentification ]
> >
> > On Mon, May 12 2025, Miquel Raynal wrote:
> >
> > > Hello,
> > >
> > > On 07/05/2025 at 09:43:25 GMT, Pratyush Yadav <pratyush@kernel.org>
> > wrote:
> > >
> > >> Hi Luke,
> > >>
> > >> On Tue, Apr 29 2025, Tudor Ambarus wrote:
> > >>
> > >>> On 4/29/25 10:03 AM, Luke Wang wrote:
> > >>>> Hi Pratyush,
> > >>>>
> > >>>> I'm following up on this patch series [1] Avoid odd length/address
> > >>>> read/ writes in 8D-8D-8D mode. While some of the series has been
> > >>>> merged, the patch 4-6 remains unmerged.
> > >>>>
> > >>>> In fact, we also encountered similar read/write issue of odd
> > >>>> address/ length with NXP FSPI controller (spi-nxp-fspi.c).
> > >>>> Currently, we handled the odd address/length in the controller
> > >>>> driver, but I think this should be a common issue in the octal dtr
> > >>>> mode. Was there a technical reason for not merging the core layer
> > solution?
> > >>>
> > >>> I guess I stumbled on those small comments and did not consider the
> > >>> greater benefit of taking the patches. No one cared and we forgot
> > >>> about it. Please address the comments and resubmit.
> > >>
> > >> Yes, it should have been a simple next revision from me but
> > >> apparently it fell through the cracks. I do strongly agree that this
> > >> should be done in SPI NOR, and not in controller drivers. So it would
> > >> be great if you can respin the remaining patches of the series.
> > >
> > > The fact is that we will have octal DTR support in SPI NAND as well at
> > > some point, hence a common solution would be welcome as we will likely
> > > face similar problems when performing these unaligned accesses. I
> > > don't know how feasible it is yet, but if we have a fix for SPI NOR,
> > > we will need something similar for SPI NAND.
> >
> > The common solution would then probably be in SPI MEM? Since you need
> to
> > make sure you don't do an out of bounds read, you need to know the size
> of the
> > flash at least. That is recorded in the dirmap operations, so perhaps we can
> have
> > this logic in spi_mem_dirmap_{read,write}()? Haven't thought too deeply so
> not
> > sure if it would end up being a good idea.
> 
> Seems reasonable, I will have a try.
> 

Sorry for delayed response.

After reviewing the SPI NAND driver, I noticed that the addr and len alignment has already been implemented in spinand_read_from_cache_op() and spinand_write_to_cache_op() functions.

Additionally, using 0xff padding in spi_mem_dirmap_write() might not be suitable for non-flash memory devices.

I can respin the remaining patches of the series if you agree.

Regards,
Luke Wang

> Regards
> Haibo Chen
> >
> > --
> > Regards,
> > Pratyush Yadav

Re: [PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address writes in 8D-8D-8D mode

[Pratyush Yadav]

On Tue, Jul 01 2025, Luke Wang wrote:

>> > On Mon, May 12 2025, Miquel Raynal wrote:
>> >
>> > > Hello,
>> > >
>> > > On 07/05/2025 at 09:43:25 GMT, Pratyush Yadav <pratyush@kernel.org>
[...]
>> > >
>> > > The fact is that we will have octal DTR support in SPI NAND as well at
>> > > some point, hence a common solution would be welcome as we will likely
>> > > face similar problems when performing these unaligned accesses. I
>> > > don't know how feasible it is yet, but if we have a fix for SPI NOR,
>> > > we will need something similar for SPI NAND.
[...]
>
> Sorry for delayed response.
>
> After reviewing the SPI NAND driver, I noticed that the addr and len
> alignment has already been implemented in spinand_read_from_cache_op()
> and spinand_write_to_cache_op() functions.

Right. I took a very quick look as well and it seems that SPI NAND only
does page sized reads and writes in spinand_write_to_cache_op() and
spinand_read_from_cache_op(). So it should not be a problem.

Miquel, Luke sent a respin [0] for fixing this in SPI NOR and I need to
decide if I should take them or push for a more generic fix. Did I miss
some place where SPI NAND can do odd-length reads or writes? If not, I'd
rather just take the respinned patches.

>
> Additionally, using 0xff padding in spi_mem_dirmap_write() might not
> be suitable for non-flash memory devices.

NAND also seems to be using 0xff though. From spinand_write_to_cache_op():

	nbytes = nanddev_page_size(nand) + nanddev_per_page_oobsize(nand);
	memset(spinand->databuf, 0xff, nanddev_page_size(nand));

Anyway, if needed we can solve that with a field in struct
spi_mem_dirmap_info I guess.

[0] https://lore.kernel.org/linux-mtd/20250708091646.292-1-ziniu.wang_1@nxp.com/T/#u

-- 
Regards,
Pratyush Yadav

[PATCH v2 0/6] Avoid odd length/address read/writes in 8D-8D-8D mode.

[Pratyush Yadav]

Hi,

On Octal DTR flashes like Micron Xcella or Cypress S28 family, reads or
writes cannot start at an odd address in 8D-8D-8D mode. Neither can they
be odd bytes long. Upper layers like filesystems don't know what mode
the flash is in, and hence don't know the read/write address or length
limitations. They might issue reads or writes that leave the flash in an
error state. In fact, using UBIFS on top of the flash was how I first
noticed this problem.

This series fixes that problem by padding the read/write with extra
bytes to make sure the final operation has an even address and length.
More info in patches 5 and 6.

Patches 1-3 fix some existing odd-byte long reads. Patch 4 adds checks
to disallow odd length command/address/dummy/data phases in 8D-8D-8D
mode. Note that this does not restrict the _value_ of the address from
being odd since this is a restriction on the flash, not the protocol
itself.

Patch 4 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.

Tested on TI J721E for Micron MT35 and on TI J7200 for Cypress S28.

Changes in v2:
Collect R-bys and cosmetic fixes. No functional changes. See the patches
for detailed changelog.

Pratyush Yadav (6):
  mtd: spi-nor: core: use 2 data bytes for template ops
  mtd: spi-nor: spansion: write 2 bytes when disabling Octal DTR mode
  mtd: spi-nor: micron-st: write 2 bytes when disabling Octal DTR mode
  spi: spi-mem: reject partial cycle transfers in 8D-8D-8D mode
  mtd: spi-nor: core: avoid odd length/address reads on 8D-8D-8D mode
  mtd: spi-nor: core: avoid odd length/address writes in 8D-8D-8D mode

 drivers/mtd/spi-nor/core.c      | 159 +++++++++++++++++++++++++++++++-
 drivers/mtd/spi-nor/micron-st.c |  22 ++++-
 drivers/mtd/spi-nor/spansion.c  |  18 +++-
 drivers/spi/spi-mem.c           |  12 ++-
 4 files changed, 196 insertions(+), 15 deletions(-)

-- 
2.30.0

[PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address writes in 8D-8D-8D mode

[Pratyush Yadav]

On Octal DTR capable flashes like Micron Xcella the writes cannot start
or end at an odd address in Octal DTR mode. Extra 0xff bytes need to be
appended or prepended to make sure the start address and end address are
even. 0xff is used because on NOR flashes a program operation can only
flip bits from 1 to 0, not the other way round. 0 to 1 flip needs to
happen via erases.

Signed-off-by: Pratyush Yadav <p.yadav@ti.com>

---

Changes in v2:
- Replace semicolon in subject with colon.
- Add a comment that ret < 0 after adjusting for extra bytes is not
  possible, and add a WARN_ON() on the condition to make sure it gets
  spotted quickly when some change triggers this bug.

 drivers/mtd/spi-nor/core.c | 73 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index a696af6a1b71..d2a7e16e667d 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2023,6 +2023,72 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
 	return ret;
 }
 
+/*
+ * On Octal DTR capable flashes like Micron Xcella the writes cannot start or
+ * end at an odd address in Octal DTR mode. Extra 0xff bytes need to be appended
+ * or prepended to make sure the start address and end address are even. 0xff is
+ * used because on NOR flashes a program operation can only flip bits from 1 to
+ * 0, not the other way round. 0 to 1 flip needs to happen via erases.
+ */
+static int spi_nor_octal_dtr_write(struct spi_nor *nor, loff_t to, size_t len,
+				   const u8 *buf)
+{
+	u8 *tmp_buf;
+	size_t bytes_written;
+	loff_t start, end;
+	int ret;
+
+	if (IS_ALIGNED(to, 2) && IS_ALIGNED(len, 2))
+		return spi_nor_write_data(nor, to, len, buf);
+
+	tmp_buf = kmalloc(nor->page_size, GFP_KERNEL);
+	if (!tmp_buf)
+		return -ENOMEM;
+
+	memset(tmp_buf, 0xff, nor->page_size);
+
+	start = round_down(to, 2);
+	end = round_up(to + len, 2);
+
+	memcpy(tmp_buf + (to - start), buf, len);
+
+	ret = spi_nor_write_data(nor, start, end - start, tmp_buf);
+	if (ret == 0) {
+		ret = -EIO;
+		goto out;
+	}
+	if (ret < 0)
+		goto out;
+
+	/*
+	 * More bytes are written than actually requested, but that number can't
+	 * be reported to the calling function or it will confuse its
+	 * calculations. Calculate how many of the _requested_ bytes were
+	 * written.
+	 */
+	bytes_written = ret;
+
+	if (to != start)
+		ret -= to - start;
+
+	/*
+	 * Only account for extra bytes at the end if they were actually
+	 * written. For example, if for some reason the controller could only
+	 * complete a partial write then the adjustment for the extra bytes at
+	 * the end is not needed.
+	 */
+	if (start + bytes_written == end)
+		ret -= end - (to + len);
+
+	/* Should not be possible. */
+	if (WARN_ON(ret < 0))
+		ret = -EIO;
+
+out:
+	kfree(tmp_buf);
+	return ret;
+}
+
 /*
  * Write an address range to the nor chip.  Data must be written in
  * FLASH_PAGESIZE chunks.  The address range may be any size provided
@@ -2067,7 +2133,12 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
 		if (ret)
 			goto write_err;
 
-		ret = spi_nor_write_data(nor, addr, page_remain, buf + i);
+		if (nor->write_proto == SNOR_PROTO_8_8_8_DTR)
+			ret = spi_nor_octal_dtr_write(nor, addr, page_remain,
+						      buf + i);
+		else
+			ret = spi_nor_write_data(nor, addr, page_remain,
+						 buf + i);
 		if (ret < 0)
 			goto write_err;
 		written = ret;
-- 
2.30.0

Re: [PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address writes in 8D-8D-8D mode

[Michael Walle]

Am 2021-05-31 20:17, schrieb Pratyush Yadav:
> On Octal DTR capable flashes like Micron Xcella the writes cannot start
> or end at an odd address in Octal DTR mode. Extra 0xff bytes need to be
> appended or prepended to make sure the start address and end address 
> are
> even. 0xff is used because on NOR flashes a program operation can only
> flip bits from 1 to 0, not the other way round. 0 to 1 flip needs to
> happen via erases.
> 
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>

Reviewed-by: Michael Walle <michael@walle.cc>

Re: [PATCH v2 6/6] mtd: spi-nor: core: avoid odd length/address writes in 8D-8D-8D mode

[Tudor.Ambarus]

On 5/31/21 9:17 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Octal DTR capable flashes like Micron Xcella the writes cannot start
> or end at an odd address in Octal DTR mode. Extra 0xff bytes need to be
> appended or prepended to make sure the start address and end address are
> even. 0xff is used because on NOR flashes a program operation can only
> flip bits from 1 to 0, not the other way round. 0 to 1 flip needs to
> happen via erases.
> 
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> 
> ---
> 
> Changes in v2:
> - Replace semicolon in subject with colon.
> - Add a comment that ret < 0 after adjusting for extra bytes is not
>   possible, and add a WARN_ON() on the condition to make sure it gets
>   spotted quickly when some change triggers this bug.
> 
>  drivers/mtd/spi-nor/core.c | 73 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 72 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index a696af6a1b71..d2a7e16e667d 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2023,6 +2023,72 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
>         return ret;
>  }
> 
> +/*

let's add kernel-doc comments for new methods.

> + * On Octal DTR capable flashes like Micron Xcella the writes cannot start or

strip "the" from "the writes". But I think I would get rid of the Micron Xcella example,
we're using these methods for all the 8D-8D-8D flashes. You can mention Micron in the
commit message if you want, but we shouldn't mention manufacturers in the core.

> + * end at an odd address in Octal DTR mode. Extra 0xff bytes need to be appended
> + * or prepended to make sure the start address and end address are even. 0xff is
> + * used because on NOR flashes a program operation can only flip bits from 1 to
> + * 0, not the other way round. 0 to 1 flip needs to happen via erases.
> + */
> +static int spi_nor_octal_dtr_write(struct spi_nor *nor, loff_t to, size_t len,
> +                                  const u8 *buf)
> +{
> +       u8 *tmp_buf;
> +       size_t bytes_written;
> +       loff_t start, end;
> +       int ret;
> +
> +       if (IS_ALIGNED(to, 2) && IS_ALIGNED(len, 2))
> +               return spi_nor_write_data(nor, to, len, buf);
> +
> +       tmp_buf = kmalloc(nor->page_size, GFP_KERNEL);> +       if (!tmp_buf)
> +               return -ENOMEM;
> +
> +       memset(tmp_buf, 0xff, nor->page_size);
> +
> +       start = round_down(to, 2);
> +       end = round_up(to + len, 2);
> +
> +       memcpy(tmp_buf + (to - start), buf, len);
> +
> +       ret = spi_nor_write_data(nor, start, end - start, tmp_buf);
> +       if (ret == 0) {
> +               ret = -EIO;
> +               goto out;
> +       }
> +       if (ret < 0)
> +               goto out;
> +
> +       /*
> +        * More bytes are written than actually requested, but that number can't
> +        * be reported to the calling function or it will confuse its
> +        * calculations. Calculate how many of the _requested_ bytes were
> +        * written.
> +        */
> +       bytes_written = ret;
> +
> +       if (to != start)
> +               ret -= to - start;
> +
> +       /*
> +        * Only account for extra bytes at the end if they were actually
> +        * written. For example, if for some reason the controller could only
> +        * complete a partial write then the adjustment for the extra bytes at
> +        * the end is not needed.
> +        */
> +       if (start + bytes_written == end)
> +               ret -= end - (to + len);
> +
> +       /* Should not be possible. */
> +       if (WARN_ON(ret < 0))
> +               ret = -EIO;

Is this really needed? Also, I think I would squash patch 5 and 6,
it's the same idea, and reads and writes are tied together.

Looks good!

> +
> +out:
> +       kfree(tmp_buf);
> +       return ret;
> +}
> +
>  /*
>   * Write an address range to the nor chip.  Data must be written in
>   * FLASH_PAGESIZE chunks.  The address range may be any size provided
> @@ -2067,7 +2133,12 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>                 if (ret)
>                         goto write_err;
> 
> -               ret = spi_nor_write_data(nor, addr, page_remain, buf + i);
> +               if (nor->write_proto == SNOR_PROTO_8_8_8_DTR)
> +                       ret = spi_nor_octal_dtr_write(nor, addr, page_remain,
> +                                                     buf + i);
> +               else
> +                       ret = spi_nor_write_data(nor, addr, page_remain,
> +                                                buf + i);
>                 if (ret < 0)
>                         goto write_err;
>                 written = ret;
> --
> 2.30.0
>