This patch removes the WARN_ONCE() test in spi_nor_write().
This macro triggers the display of a warning message almost every time we
use a UBI file-system because a write operation is performed at offset 64,
which is in the middle of the SPI NOR memory page. This is a valid
operation for ubifs.
Hence this warning is pretty annoying and useless so we just remove it.
Signed-off-by: Cyrille Pitchen <[email protected]>
Suggested-by: Richard Weinberger <[email protected]>
Suggested-by: Andras Szemzo <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index c9bcd05ca5d9..cdeb2c6b1492 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1265,9 +1265,6 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
ssize_t written;
page_offset = (to + i) & (nor->page_size - 1);
- WARN_ONCE(page_offset,
- "Writing at offset %zu into a NOR page. Writing partial pages may decrease reliability and increase wear of NOR flash.",
- page_offset);
/* the size of data remaining on the first page */
page_remain = min_t(size_t,
nor->page_size - page_offset, len - i);
--
2.7.4
On 12/06/2016 06:14 PM, Cyrille Pitchen wrote:
> This patch removes the WARN_ONCE() test in spi_nor_write().
> This macro triggers the display of a warning message almost every time we
> use a UBI file-system because a write operation is performed at offset 64,
> which is in the middle of the SPI NOR memory page. This is a valid
> operation for ubifs.
Is that a valid operation for all spi nors ?
> Hence this warning is pretty annoying and useless so we just remove it.
>
> Signed-off-by: Cyrille Pitchen <[email protected]>
> Suggested-by: Richard Weinberger <[email protected]>
> Suggested-by: Andras Szemzo <[email protected]>
> ---
> drivers/mtd/spi-nor/spi-nor.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index c9bcd05ca5d9..cdeb2c6b1492 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1265,9 +1265,6 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
> ssize_t written;
>
> page_offset = (to + i) & (nor->page_size - 1);
> - WARN_ONCE(page_offset,
> - "Writing at offset %zu into a NOR page. Writing partial pages may decrease reliability and increase wear of NOR flash.",
> - page_offset);
> /* the size of data remaining on the first page */
> page_remain = min_t(size_t,
> nor->page_size - page_offset, len - i);
>
--
Best regards,
Marek Vasut
On 12/07/2016 12:38 AM, Cyrille Pitchen wrote:
> Le 06/12/2016 ? 20:00, Marek Vasut a ?crit :
>> On 12/06/2016 06:14 PM, Cyrille Pitchen wrote:
>>> This patch removes the WARN_ONCE() test in spi_nor_write().
>>> This macro triggers the display of a warning message almost every time we
>>> use a UBI file-system because a write operation is performed at offset 64,
>>> which is in the middle of the SPI NOR memory page. This is a valid
>>> operation for ubifs.
>>
>> Is that a valid operation for all spi nors ?
>>
>
> AFAIK, yes, it is. First we need to erase a sector or a block, then we
> can send page program commands to write data into the memory. We cannot
> write more than a page size within a single page program command but you
> can write less and start in the middle of a page if you want.
I wasn't aware this partial and even unaligned programming was available
on all chips, OK.
> I don't know whether we could cross the page boundary if we start
> writing from the middle of a page as long as we write less data than a
> single page size. However spi_nor_write() don't do so, this is why it
> computes
> page_remain = min_t(size_t, nor->page_size - page_offset, len - i)
No, I don't think we can, I believe the PP would wrap around and program
the same page from the beginning.
> Well, now looking at the Spansion S25FL127S datasheet, the address is
> wrapped if we cross the page boundary:
Yeah, this matches my mental model.
> "Depending on the device configuration, the page size can either be 256
> or 512 bytes. Up to a page can be provided on SI after the 3-byte
> address with instruction 02h or 4-byte address with instruction 12h has
> been provided. If the 9 least significant address bits (A8-A0) are not
> all 0, all transmitted data that goes beyond the end of the current page
> are programmed from the start address of the same page (from the address
> whose 9 least significant bits (A8-A0) are all 0) i.e. the address wraps
> within the page aligned address boundaries. This is a result of only
> requiring the user to enter one single page address to cover the entire
> page boundary."
>
> Then from Adesto AT25DF321A datasheet:
> "If the starting memory address denoted by A23-A0 does not fall on an
> even 256-byte page boundary (A7-A0 are not all 0), then special
> circumstances regarding which memory locations to be programmed will
> apply. In this situation, any data that is sent to the device that goes
> beyond the end of the page will wrap around back to the beginning of the
> same page. For example, if the starting address denoted by A23-A0 is
> 0000FEh, and three bytes of data are sent to the device, then the first
> two bytes of data will be programmed at addresses 0000FEh and 0000FFh
> while the last byte of data will be programmed at address 000000h. The
> remaining bytes in the page (addresses 000001h through 0000FDh) will not
> be programmed and will remain in the erased state (FFh). In addition, if
> more than 256-bytes of data are sent to the device, then only the last
> 256-bytes sent will be latched into the internal buffer."
>
>
> Besides, the wear leveling is handled by the ubi layer I guess, at the
> spi-nor level we write raw data. Maybe Richard and Boris could tell us
> more but talking with them I've understood that's it is normal for the
> ubi layer to write at offset 64.
I'd understand RMW, but pure write seems a bit odd.
--
Best regards,
Marek Vasut
Hi Cyrille,
> -----Original Message-----
> From: linux-mtd [mailto:[email protected]] On Behalf
> Of Marek Vasut
> Sent: Wednesday, December 07, 2016 4:07 AM
> To: Cyrille Pitchen <[email protected]>; Cyrille Pitchen
> <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH 1/1] mtd: spi-nor: remove WARN_ONCE() message in
> spi_nor_write()
>
> On 12/07/2016 12:38 AM, Cyrille Pitchen wrote:
> > Le 06/12/2016 ? 20:00, Marek Vasut a ?crit :
> >> On 12/06/2016 06:14 PM, Cyrille Pitchen wrote:
> >>> This patch removes the WARN_ONCE() test in spi_nor_write().
> >>> This macro triggers the display of a warning message almost every
> >>> time we use a UBI file-system because a write operation is performed
> >>> at offset 64, which is in the middle of the SPI NOR memory page.
> >>> This is a valid operation for ubifs.
> >>
> >> Is that a valid operation for all spi nors ?
> >>
> >
> > AFAIK, yes, it is. First we need to erase a sector or a block, then we
> > can send page program commands to write data into the memory. We
> > cannot write more than a page size within a single page program
> > command but you can write less and start in the middle of a page if you
> want.
>
Technically you can, but for some chips this warning is indeed right, and at
least force the user to take a look. See this:
http://www.macronix.com/Lists/ApplicationNote/Attachments/1606/AN0302V1%20-%20MX25L_G%20Serial%20Flash%20Programming%20Guide.pdf
Thanks,
Marcin
> I wasn't aware this partial and even unaligned programming was available on
> all chips, OK.
>
> > I don't know whether we could cross the page boundary if we start
> > writing from the middle of a page as long as we write less data than a
> > single page size. However spi_nor_write() don't do so, this is why it
> > computes page_remain = min_t(size_t, nor->page_size - page_offset, len
> > - i)
>
> No, I don't think we can, I believe the PP would wrap around and program
> the same page from the beginning.
>
> > Well, now looking at the Spansion S25FL127S datasheet, the address is
> > wrapped if we cross the page boundary:
>
> Yeah, this matches my mental model.
>
> > "Depending on the device configuration, the page size can either be
> > 256 or 512 bytes. Up to a page can be provided on SI after the 3-byte
> > address with instruction 02h or 4-byte address with instruction 12h
> > has been provided. If the 9 least significant address bits (A8-A0) are
> > not all 0, all transmitted data that goes beyond the end of the
> > current page are programmed from the start address of the same page
> > (from the address whose 9 least significant bits (A8-A0) are all 0)
> > i.e. the address wraps within the page aligned address boundaries.
> > This is a result of only requiring the user to enter one single page
> > address to cover the entire page boundary."
> >
> > Then from Adesto AT25DF321A datasheet:
> > "If the starting memory address denoted by A23-A0 does not fall on an
> > even 256-byte page boundary (A7-A0 are not all 0), then special
> > circumstances regarding which memory locations to be programmed will
> > apply. In this situation, any data that is sent to the device that
> > goes beyond the end of the page will wrap around back to the beginning
> > of the same page. For example, if the starting address denoted by
> > A23-A0 is 0000FEh, and three bytes of data are sent to the device,
> > then the first two bytes of data will be programmed at addresses
> > 0000FEh and 0000FFh while the last byte of data will be programmed at
> > address 000000h. The remaining bytes in the page (addresses 000001h
> > through 0000FDh) will not be programmed and will remain in the erased
> > state (FFh). In addition, if more than 256-bytes of data are sent to
> > the device, then only the last 256-bytes sent will be latched into the
> internal buffer."
> >
> >
> > Besides, the wear leveling is handled by the ubi layer I guess, at the
> > spi-nor level we write raw data. Maybe Richard and Boris could tell us
> > more but talking with them I've understood that's it is normal for the
> > ubi layer to write at offset 64.
>
> I'd understand RMW, but pure write seems a bit odd.
>
>
> --
> Best regards,
> Marek Vasut
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
Le 06/12/2016 ? 20:00, Marek Vasut a ?crit :
> On 12/06/2016 06:14 PM, Cyrille Pitchen wrote:
>> This patch removes the WARN_ONCE() test in spi_nor_write().
>> This macro triggers the display of a warning message almost every time we
>> use a UBI file-system because a write operation is performed at offset 64,
>> which is in the middle of the SPI NOR memory page. This is a valid
>> operation for ubifs.
>
> Is that a valid operation for all spi nors ?
>
AFAIK, yes, it is. First we need to erase a sector or a block, then we
can send page program commands to write data into the memory. We cannot
write more than a page size within a single page program command but you
can write less and start in the middle of a page if you want.
I don't know whether we could cross the page boundary if we start
writing from the middle of a page as long as we write less data than a
single page size. However spi_nor_write() don't do so, this is why it
computes
page_remain = min_t(size_t, nor->page_size - page_offset, len - i)
Well, now looking at the Spansion S25FL127S datasheet, the address is
wrapped if we cross the page boundary:
"Depending on the device configuration, the page size can either be 256
or 512 bytes. Up to a page can be provided on SI after the 3-byte
address with instruction 02h or 4-byte address with instruction 12h has
been provided. If the 9 least significant address bits (A8-A0) are not
all 0, all transmitted data that goes beyond the end of the current page
are programmed from the start address of the same page (from the address
whose 9 least significant bits (A8-A0) are all 0) i.e. the address wraps
within the page aligned address boundaries. This is a result of only
requiring the user to enter one single page address to cover the entire
page boundary."
Then from Adesto AT25DF321A datasheet:
"If the starting memory address denoted by A23-A0 does not fall on an
even 256-byte page boundary (A7-A0 are not all 0), then special
circumstances regarding which memory locations to be programmed will
apply. In this situation, any data that is sent to the device that goes
beyond the end of the page will wrap around back to the beginning of the
same page. For example, if the starting address denoted by A23-A0 is
0000FEh, and three bytes of data are sent to the device, then the first
two bytes of data will be programmed at addresses 0000FEh and 0000FFh
while the last byte of data will be programmed at address 000000h. The
remaining bytes in the page (addresses 000001h through 0000FDh) will not
be programmed and will remain in the erased state (FFh). In addition, if
more than 256-bytes of data are sent to the device, then only the last
256-bytes sent will be latched into the internal buffer."
Besides, the wear leveling is handled by the ubi layer I guess, at the
spi-nor level we write raw data. Maybe Richard and Boris could tell us
more but talking with them I've understood that's it is normal for the
ubi layer to write at offset 64.
Best regards,
Cyrille
>> Hence this warning is pretty annoying and useless so we just remove it.
>>
>> Signed-off-by: Cyrille Pitchen <[email protected]>
>> Suggested-by: Richard Weinberger <[email protected]>
>> Suggested-by: Andras Szemzo <[email protected]>
>> ---
>> drivers/mtd/spi-nor/spi-nor.c | 3 ---
>> 1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index c9bcd05ca5d9..cdeb2c6b1492 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -1265,9 +1265,6 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>> ssize_t written;
>>
>> page_offset = (to + i) & (nor->page_size - 1);
>> - WARN_ONCE(page_offset,
>> - "Writing at offset %zu into a NOR page. Writing partial pages may decrease reliability and increase wear of NOR flash.",
>> - page_offset);
>> /* the size of data remaining on the first page */
>> page_remain = min_t(size_t,
>> nor->page_size - page_offset, len - i);
>>
>
>
On Wed, 7 Dec 2016 04:07:05 +0100
Marek Vasut <[email protected]> wrote:
> On 12/07/2016 12:38 AM, Cyrille Pitchen wrote:
> > Le 06/12/2016 à 20:00, Marek Vasut a écrit :
> >> On 12/06/2016 06:14 PM, Cyrille Pitchen wrote:
> >>> This patch removes the WARN_ONCE() test in spi_nor_write().
> >>> This macro triggers the display of a warning message almost every time we
> >>> use a UBI file-system because a write operation is performed at offset 64,
> >>> which is in the middle of the SPI NOR memory page. This is a valid
> >>> operation for ubifs.
> >>
> >> Is that a valid operation for all spi nors ?
> >>
> >
> > AFAIK, yes, it is. First we need to erase a sector or a block, then we
> > can send page program commands to write data into the memory. We cannot
> > write more than a page size within a single page program command but you
> > can write less and start in the middle of a page if you want.
>
> I wasn't aware this partial and even unaligned programming was available
> on all chips, OK.
>
> > I don't know whether we could cross the page boundary if we start
> > writing from the middle of a page as long as we write less data than a
> > single page size. However spi_nor_write() don't do so, this is why it
> > computes
> > page_remain = min_t(size_t, nor->page_size - page_offset, len - i)
>
> No, I don't think we can, I believe the PP would wrap around and program
> the same page from the beginning.
>
> > Well, now looking at the Spansion S25FL127S datasheet, the address is
> > wrapped if we cross the page boundary:
>
> Yeah, this matches my mental model.
>
> > "Depending on the device configuration, the page size can either be 256
> > or 512 bytes. Up to a page can be provided on SI after the 3-byte
> > address with instruction 02h or 4-byte address with instruction 12h has
> > been provided. If the 9 least significant address bits (A8-A0) are not
> > all 0, all transmitted data that goes beyond the end of the current page
> > are programmed from the start address of the same page (from the address
> > whose 9 least significant bits (A8-A0) are all 0) i.e. the address wraps
> > within the page aligned address boundaries. This is a result of only
> > requiring the user to enter one single page address to cover the entire
> > page boundary."
> >
> > Then from Adesto AT25DF321A datasheet:
> > "If the starting memory address denoted by A23-A0 does not fall on an
> > even 256-byte page boundary (A7-A0 are not all 0), then special
> > circumstances regarding which memory locations to be programmed will
> > apply. In this situation, any data that is sent to the device that goes
> > beyond the end of the page will wrap around back to the beginning of the
> > same page. For example, if the starting address denoted by A23-A0 is
> > 0000FEh, and three bytes of data are sent to the device, then the first
> > two bytes of data will be programmed at addresses 0000FEh and 0000FFh
> > while the last byte of data will be programmed at address 000000h. The
> > remaining bytes in the page (addresses 000001h through 0000FDh) will not
> > be programmed and will remain in the erased state (FFh). In addition, if
> > more than 256-bytes of data are sent to the device, then only the last
> > 256-bytes sent will be latched into the internal buffer."
> >
> >
> > Besides, the wear leveling is handled by the ubi layer I guess, at the
> > spi-nor level we write raw data. Maybe Richard and Boris could tell us
> > more but talking with them I've understood that's it is normal for the
> > ubi layer to write at offset 64.
>
> I'd understand RMW, but pure write seems a bit odd.
>
>
Well, UBI trusts what the MTD framwork says, and here [1] you say that
the minimum write unit is a byte, so UBI tries to optimize flash usage
by putting the EC and VID headers next to each other.
Now, if you want UBI to act differently, customize ->writesize on a,
but, by doing that, you may introduce regressions because some users
are already using UBI with this layout (ECH @0, VIDH @64).
[1]http://lxr.free-electrons.com/source/drivers/mtd/spi-nor/spi-nor.c#L1368
Le 07/12/2016 ? 07:21, Krzeminski, Marcin (Nokia - PL/Wroclaw) a ?crit :
> Hi Cyrille,
>
>> -----Original Message-----
>> From: linux-mtd [mailto:[email protected]] On Behalf
>> Of Marek Vasut
>> Sent: Wednesday, December 07, 2016 4:07 AM
>> To: Cyrille Pitchen <[email protected]>; Cyrille Pitchen
>> <[email protected]>
>> Cc: [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]
>> Subject: Re: [PATCH 1/1] mtd: spi-nor: remove WARN_ONCE() message in
>> spi_nor_write()
>>
>> On 12/07/2016 12:38 AM, Cyrille Pitchen wrote:
>>> Le 06/12/2016 ? 20:00, Marek Vasut a ?crit :
>>>> On 12/06/2016 06:14 PM, Cyrille Pitchen wrote:
>>>>> This patch removes the WARN_ONCE() test in spi_nor_write().
>>>>> This macro triggers the display of a warning message almost every
>>>>> time we use a UBI file-system because a write operation is performed
>>>>> at offset 64, which is in the middle of the SPI NOR memory page.
>>>>> This is a valid operation for ubifs.
>>>>
>>>> Is that a valid operation for all spi nors ?
>>>>
>>>
>>> AFAIK, yes, it is. First we need to erase a sector or a block, then we
>>> can send page program commands to write data into the memory. We
>>> cannot write more than a page size within a single page program
>>> command but you can write less and start in the middle of a page if you
>> want.
>>
> Technically you can, but for some chips this warning is indeed right, and at
> least force the user to take a look. See this:
>
> http://www.macronix.com/Lists/ApplicationNote/Attachments/1606/AN0302V1%20-%20MX25L_G%20Serial%20Flash%20Programming%20Guide.pdf
>
This Macronix document recommends to align both the start offset and the
length to a 16-byte boundary. However the WARN_ONCE() macro only checks the
start offset but doesn't test the length. Also it tests a page-size
alignment, which is a stronger constraint than a 16-byte alignment.
In the case of Macronix mx25l_g memories, when the UBI layer writes at
offset 64, the warning is a false positive, isn't it?
Also WARN_ONCE() dumps the call stack making people think of a kernel oops,
displayed once per boot when mounting a ubifs partition.
If the issue exists, printing a warning does not fix it.
So what should we do?
Best regards,
Cyrille
> Thanks,
> Marcin
>
>> I wasn't aware this partial and even unaligned programming was available on
>> all chips, OK.
>>
>>> I don't know whether we could cross the page boundary if we start
>>> writing from the middle of a page as long as we write less data than a
>>> single page size. However spi_nor_write() don't do so, this is why it
>>> computes page_remain = min_t(size_t, nor->page_size - page_offset, len
>>> - i)
>>
>> No, I don't think we can, I believe the PP would wrap around and program
>> the same page from the beginning.
>>
>>> Well, now looking at the Spansion S25FL127S datasheet, the address is
>>> wrapped if we cross the page boundary:
>>
>> Yeah, this matches my mental model.
>>
>>> "Depending on the device configuration, the page size can either be
>>> 256 or 512 bytes. Up to a page can be provided on SI after the 3-byte
>>> address with instruction 02h or 4-byte address with instruction 12h
>>> has been provided. If the 9 least significant address bits (A8-A0) are
>>> not all 0, all transmitted data that goes beyond the end of the
>>> current page are programmed from the start address of the same page
>>> (from the address whose 9 least significant bits (A8-A0) are all 0)
>>> i.e. the address wraps within the page aligned address boundaries.
>>> This is a result of only requiring the user to enter one single page
>>> address to cover the entire page boundary."
>>>
>>> Then from Adesto AT25DF321A datasheet:
>>> "If the starting memory address denoted by A23-A0 does not fall on an
>>> even 256-byte page boundary (A7-A0 are not all 0), then special
>>> circumstances regarding which memory locations to be programmed will
>>> apply. In this situation, any data that is sent to the device that
>>> goes beyond the end of the page will wrap around back to the beginning
>>> of the same page. For example, if the starting address denoted by
>>> A23-A0 is 0000FEh, and three bytes of data are sent to the device,
>>> then the first two bytes of data will be programmed at addresses
>>> 0000FEh and 0000FFh while the last byte of data will be programmed at
>>> address 000000h. The remaining bytes in the page (addresses 000001h
>>> through 0000FDh) will not be programmed and will remain in the erased
>>> state (FFh). In addition, if more than 256-bytes of data are sent to
>>> the device, then only the last 256-bytes sent will be latched into the
>> internal buffer."
>>>
>>>
>>> Besides, the wear leveling is handled by the ubi layer I guess, at the
>>> spi-nor level we write raw data. Maybe Richard and Boris could tell us
>>> more but talking with them I've understood that's it is normal for the
>>> ubi layer to write at offset 64.
>>
>> I'd understand RMW, but pure write seems a bit odd.
>>
>>
>> --
>> Best regards,
>> Marek Vasut
>>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
> -----Original Message-----
> From: Cyrille Pitchen [mailto:[email protected]]
> Sent: Wednesday, December 07, 2016 2:08 PM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <[email protected]>; Marek Vasut <[email protected]>;
> Cyrille Pitchen <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH 1/1] mtd: spi-nor: remove WARN_ONCE() message in
> spi_nor_write()
>
> Le 07/12/2016 ? 07:21, Krzeminski, Marcin (Nokia - PL/Wroclaw) a ?crit :
> > Hi Cyrille,
> >
> >> -----Original Message-----
> >> From: linux-mtd [mailto:[email protected]] On
> >> Behalf Of Marek Vasut
> >> Sent: Wednesday, December 07, 2016 4:07 AM
> >> To: Cyrille Pitchen <[email protected]>; Cyrille Pitchen
> >> <[email protected]>
> >> Cc: [email protected]; [email protected];
> >> [email protected]; [email protected];
> >> [email protected]
> >> Subject: Re: [PATCH 1/1] mtd: spi-nor: remove WARN_ONCE() message in
> >> spi_nor_write()
> >>
> >> On 12/07/2016 12:38 AM, Cyrille Pitchen wrote:
> >>> Le 06/12/2016 ? 20:00, Marek Vasut a ?crit :
> >>>> On 12/06/2016 06:14 PM, Cyrille Pitchen wrote:
> >>>>> This patch removes the WARN_ONCE() test in spi_nor_write().
> >>>>> This macro triggers the display of a warning message almost every
> >>>>> time we use a UBI file-system because a write operation is
> >>>>> performed at offset 64, which is in the middle of the SPI NOR memory
> page.
> >>>>> This is a valid operation for ubifs.
> >>>>
> >>>> Is that a valid operation for all spi nors ?
> >>>>
> >>>
> >>> AFAIK, yes, it is. First we need to erase a sector or a block, then
> >>> we can send page program commands to write data into the memory.
> We
> >>> cannot write more than a page size within a single page program
> >>> command but you can write less and start in the middle of a page if
> >>> you
> >> want.
> >>
> > Technically you can, but for some chips this warning is indeed right,
> > and at least force the user to take a look. See this:
> >
> >
> http://www.macronix.com/Lists/ApplicationNote/Attachments/1606/AN030
> 2V
> > 1%20-%20MX25L_G%20Serial%20Flash%20Programming%20Guide.pdf
> >
>
> This Macronix document recommends to align both the start offset and the
> length to a 16-byte boundary. However the WARN_ONCE() macro only
> checks the start offset but doesn't test the length. Also it tests a page-size
> alignment, which is a stronger constraint than a 16-byte alignment.
>
> In the case of Macronix mx25l_g memories, when the UBI layer writes at
> offset 64, the warning is a false positive, isn't it?
They are also saying about writing full pages at once (chapter 5).
>
> Also WARN_ONCE() dumps the call stack making people think of a kernel
> oops, displayed once per boot when mounting a ubifs partition.
>
> If the issue exists, printing a warning does not fix it.
>
If it generate to many noise and confuse users then this patch is really needed.
Thanks,
Marcin
> So what should we do?
>
> Best regards,
>
> Cyrille
>
> > Thanks,
> > Marcin
> >
> >> I wasn't aware this partial and even unaligned programming was
> >> available on all chips, OK.
> >>
> >>> I don't know whether we could cross the page boundary if we start
> >>> writing from the middle of a page as long as we write less data than
> >>> a single page size. However spi_nor_write() don't do so, this is why
> >>> it computes page_remain = min_t(size_t, nor->page_size -
> >>> page_offset, len
> >>> - i)
> >>
> >> No, I don't think we can, I believe the PP would wrap around and
> >> program the same page from the beginning.
> >>
> >>> Well, now looking at the Spansion S25FL127S datasheet, the address
> >>> is wrapped if we cross the page boundary:
> >>
> >> Yeah, this matches my mental model.
> >>
> >>> "Depending on the device configuration, the page size can either be
> >>> 256 or 512 bytes. Up to a page can be provided on SI after the
> >>> 3-byte address with instruction 02h or 4-byte address with
> >>> instruction 12h has been provided. If the 9 least significant
> >>> address bits (A8-A0) are not all 0, all transmitted data that goes
> >>> beyond the end of the current page are programmed from the start
> >>> address of the same page (from the address whose 9 least significant
> >>> bits (A8-A0) are all 0) i.e. the address wraps within the page aligned
> address boundaries.
> >>> This is a result of only requiring the user to enter one single page
> >>> address to cover the entire page boundary."
> >>>
> >>> Then from Adesto AT25DF321A datasheet:
> >>> "If the starting memory address denoted by A23-A0 does not fall on
> >>> an even 256-byte page boundary (A7-A0 are not all 0), then special
> >>> circumstances regarding which memory locations to be programmed will
> >>> apply. In this situation, any data that is sent to the device that
> >>> goes beyond the end of the page will wrap around back to the
> >>> beginning of the same page. For example, if the starting address
> >>> denoted by
> >>> A23-A0 is 0000FEh, and three bytes of data are sent to the device,
> >>> then the first two bytes of data will be programmed at addresses
> >>> 0000FEh and 0000FFh while the last byte of data will be programmed
> >>> at address 000000h. The remaining bytes in the page (addresses
> >>> 000001h through 0000FDh) will not be programmed and will remain in
> >>> the erased state (FFh). In addition, if more than 256-bytes of data
> >>> are sent to the device, then only the last 256-bytes sent will be
> >>> latched into the
> >> internal buffer."
> >>>
> >>>
> >>> Besides, the wear leveling is handled by the ubi layer I guess, at
> >>> the spi-nor level we write raw data. Maybe Richard and Boris could
> >>> tell us more but talking with them I've understood that's it is
> >>> normal for the ubi layer to write at offset 64.
> >>
> >> I'd understand RMW, but pure write seems a bit odd.
> >>
> >>
> >> --
> >> Best regards,
> >> Marek Vasut
> >>
> >> ______________________________________________________
> >> Linux MTD discussion mailing list
> >> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> >
On Tue, 6 Dec 2016 18:14:24 +0100
Cyrille Pitchen <[email protected]> wrote:
> This patch removes the WARN_ONCE() test in spi_nor_write().
> This macro triggers the display of a warning message almost every time we
> use a UBI file-system because a write operation is performed at offset 64,
> which is in the middle of the SPI NOR memory page. This is a valid
> operation for ubifs.
>
> Hence this warning is pretty annoying and useless so we just remove it.
>
> Signed-off-by: Cyrille Pitchen <[email protected]>
> Suggested-by: Richard Weinberger <[email protected]>
> Suggested-by: Andras Szemzo <[email protected]>
Acked-by: Boris Brezillon <[email protected]>
> ---
> drivers/mtd/spi-nor/spi-nor.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index c9bcd05ca5d9..cdeb2c6b1492 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1265,9 +1265,6 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
> ssize_t written;
>
> page_offset = (to + i) & (nor->page_size - 1);
> - WARN_ONCE(page_offset,
> - "Writing at offset %zu into a NOR page. Writing partial pages may decrease reliability and increase wear of NOR flash.",
> - page_offset);
> /* the size of data remaining on the first page */
> page_remain = min_t(size_t,
> nor->page_size - page_offset, len - i);
Le 08/12/2016 ? 16:31, Boris Brezillon a ?crit :
> On Tue, 6 Dec 2016 18:14:24 +0100
> Cyrille Pitchen <[email protected]> wrote:
>
>> This patch removes the WARN_ONCE() test in spi_nor_write().
>> This macro triggers the display of a warning message almost every time we
>> use a UBI file-system because a write operation is performed at offset 64,
>> which is in the middle of the SPI NOR memory page. This is a valid
>> operation for ubifs.
>>
>> Hence this warning is pretty annoying and useless so we just remove it.
>>
>> Signed-off-by: Cyrille Pitchen <[email protected]>
>> Suggested-by: Richard Weinberger <[email protected]>
>> Suggested-by: Andras Szemzo <[email protected]>
>
> Acked-by: Boris Brezillon <[email protected]>
>
Applied to git://github.com/spi-nor/linux.git
>> ---
>> drivers/mtd/spi-nor/spi-nor.c | 3 ---
>> 1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index c9bcd05ca5d9..cdeb2c6b1492 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -1265,9 +1265,6 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>> ssize_t written;
>>
>> page_offset = (to + i) & (nor->page_size - 1);
>> - WARN_ONCE(page_offset,
>> - "Writing at offset %zu into a NOR page. Writing partial pages may decrease reliability and increase wear of NOR flash.",
>> - page_offset);
>> /* the size of data remaining on the first page */
>> page_remain = min_t(size_t,
>> nor->page_size - page_offset, len - i);
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>