[PATCH 1/1] mtd: spi-nor: remove WARN_ONCE() message in spi_nor_write()

Wed Dec 7 05:50:11 PST 2016

> -----Original Message-----
> From: Cyrille Pitchen [mailto:cyrille.pitchen at atmel.com]
> Sent: Wednesday, December 07, 2016 2:08 PM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <marcin.krzeminski at nokia.com>; Marek Vasut <marek.vasut at gmail.com>;
> Cyrille Pitchen <cyrille.pitchen at wedev4u.fr>
> Cc: boris.brezillon at free-electrons.com; computersforpeace at gmail.com;
> linux-mtd at lists.infradead.org; linux-kernel at vger.kernel.org;
> richard at nod.at
> 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:linux-mtd-bounces at lists.infradead.org] On
> >> Behalf Of Marek Vasut
> >> Sent: Wednesday, December 07, 2016 4:07 AM
> >> To: Cyrille Pitchen <cyrille.pitchen at wedev4u.fr>; Cyrille Pitchen
> >> <cyrille.pitchen at atmel.com>
> >> Cc: boris.brezillon at free-electrons.com; computersforpeace at gmail.com;
> >> linux-mtd at lists.infradead.org; linux-kernel at vger.kernel.org;
> >> richard at nod.at
> >> 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/
> >