Re: [PATCH RESEND u-boot-spi 0/8] Fix `mtd erase` when used with mtdpart

From: Jagan Teki <jagan@amarulasolutions.com>
To: "Marek Behún" <marek.behun@nic.cz>
Cc: "Masami Hiramatsu" <masami.hiramatsu@linaro.org>,
	"Simon Glass" <sjg@chromium.org>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Tom Rini" <trini@konsulko.com>,
	U-Boot-Denx <u-boot@lists.denx.de>,
	"Patrice Chotard" <patrice.chotard@foss.st.com>,
	"Patrick Delaunay" <patrick.delaunay@st.com>,
	"Heiko Schocher" <hs@denx.de>, "Pali Rohár" <pali@kernel.org>
Subject: Re: [PATCH RESEND u-boot-spi 0/8] Fix `mtd erase` when used with mtdpart
Date: Wed, 21 Jul 2021 21:46:56 +0530	[thread overview]
Message-ID: <CAMty3ZA4GsgPG84hOXXucjUosGQA117tSwwq9d2XQdDsrrDZqA@mail.gmail.com> (raw)
In-Reply-To: <20210714235109.25228-1-marek.behun@nic.cz>

Hi Marek,

On Thu, Jul 15, 2021 at 5:21 AM Marek Behún <marek.behun@nic.cz> wrote:
>
> Hello,
>
> I accidentally forgot to send this series to U-Boot's mailing list last
> time, meaning it did not end up in patchwork, so now I am resending it.
> Sorry for this mess.
>
> The original cover letter said:
>
> this patch series fixes the `mtd erase` command when used with mtdpart
> with a partition of non-zero offset.
>
> Currently when the `mtd erase` command is used for such a partition,
> it does not erase all blocks. Instead after a block is erased, the next
> block address not current block address + block size, but current block
> address + block size + partition offset, due to spi_nor_erase() not
> calling mtd_erase_callback():
>   => mtd erase "Rescue system"
>   Erasing 0x00000000 ... 0x006fffff (1792 eraseblock(s))
>   jedec_spi_nor spi-nor@0: at 0x100000, len 4096
>   jedec_spi_nor spi-nor@0: at 0x201000, len 4096
>   jedec_spi_nor spi-nor@0: at 0x302000, len 4096
>   jedec_spi_nor spi-nor@0: at 0x403000, len 4096
>   jedec_spi_nor spi-nor@0: at 0x504000, len 4096
>   jedec_spi_nor spi-nor@0: at 0x605000, len 4096
>   jedec_spi_nor spi-nor@0: at 0x706000, len 4096
>
> This series adds some fixes to spi_nor_erase() function, then adds
> calling of mtd_erase_callback() to fix this bug.
>
> The series also contains an improvement - adding the posibility to
> interrupt spi_nor_erase() with Ctrl+C; and another one - making mtdpart's
> _erase() method more sane so that the above mentioned bug will not occur
> even if underlying driver does not call mtd_erase_callback().
>
> Moreover I would also like to start a discussion regarding the MTD
> subsystem:
> - U-Boot's MTD subsystem is based on Linux's, with many #ifdef __U_BOOT__
>   macros
> - this was done to make it easier to port Linux's patches to U-Boot
> - the problem is that it seems nobody did port Linux's MTD patches to
>   U-Boot for a long time, the code is many times different from Linux',
>   and it would be very hard to align it
> - therefore I propose to get rid of all the #ifdefs, remove the Linux
>   specific code, and continue developing the code independently from
>   Linux. This would make it impossible to apply Linux patches in some
>   kind of automatic way, but this is currently already impossible
>   anyway
> What do you guys think?
>
> Marek
>
> Marek Behún (8):
>   mtd: spi-nor-core: Try cleaning up in case writing BAR failed
>   mtd: spi-nor-core: Check return value of write_enable() in
>     spi_nor_erase()
>   mtd: spi-nor-core: Don't overwrite return value if it is non-zero
>   mtd: spi-nor-core: Check return value of write_disable() in
>     spi_nor_erase()
>   mtd: spi-nor-core: Don't check for zero length in spi_nor_erase()
>   mtd: spi-nor-core: Call mtd_erase_callback() from spi_nor_erase()
>   mtd: spi-nor-core: Check for ctrlc() in spi_nor_erase()
>   mtd: mtdpart: Make mtdpart's _erase method sane

Found the build error with CI [1], would you please check?

[1] https://source.denx.de/u-boot/custodians/u-boot-spi/-/pipelines/8345

Jagan.