From: "Marek Behún" <kabel@kernel.org>
To: Jagan Teki <jagan@amarulasolutions.com>, Tom Rini <trini@konsulko.com>
Cc: u-boot@lists.denx.de,
"Patrick Delaunay" <patrick.delaunay@st.com>,
"Pali Rohár" <pali@kernel.org>,
"Patrice Chotard" <patrice.chotard@foss.st.com>,
"Marek Vasut" <marex@denx.de>, "Marek Behún" <marek.behun@nic.cz>,
"Simon Glass" <sjg@chromium.org>,
"Masami Hiramatsu" <masami.hiramatsu@linaro.org>
Subject: [PATCH u-boot-spi v2 8/9] mtd: mtdpart: Make mtdpart's _erase method sane
Date: Sat, 25 Sep 2021 19:33:17 +0200 [thread overview]
Message-ID: <20210925173318.25804-9-kabel@kernel.org> (raw)
In-Reply-To: <20210925173318.25804-1-kabel@kernel.org>
From: Marek Behún <marek.behun@nic.cz>
The _erase() method of the mtdpart driver, part_erase(), currently
implements offset shifting (for given mtdpart partition) in a weird way:
1. part_erase() adds partition offset to block address
2. parent driver's _erase() method is called
3. parent driver's _erase() method calls mtd_erase_callback()
4. mtd_erase_callback() subtracts partition offset from block address
so that the callback function is given correct address
The problem here is that if the parent's driver does not call
mtd_erase_callback() in some scenario (this was recently a case for
spi_nor_erase(), which did not call mtd_erase_callback() at all), the
offset is not shifted back.
Moreover the code would be more readable if part_erase() not only added
partition offset before calling parent's _erase(), but also subtracted
it back afterwards. Currently the mtd_erase_callback() is expected to do
this subtracting since it does have to do it anyway.
Add the more steps to this procedure:
5. mtd_erase_callback() adds partition offset to block address so that
it returns the the erase_info structure members as it received them
6. part_erase() subtracts partition offset from block address
This makes the code more logical and also prevents errors in case
parent's driver does not call mtd_erase_callback() for some reason.
(BTW, the purpose of mtd_erase_callback() in Linux is to inform the
caller that it is done, since in Linux erasing is done asynchronously.
We are abusing the purpose of mtd_erase_callback() in U-Boot for
completely different purpose. The callback function itself has empty
implementation in all cases in U-Boot.)
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
drivers/mtd/mtdpart.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index aa58f722da..6ab481a7b1 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -446,24 +446,34 @@ static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
int ret;
instr->addr += mtd->offset;
+
ret = mtd->parent->_erase(mtd->parent, instr);
- if (ret) {
- if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
- instr->fail_addr -= mtd->offset;
- instr->addr -= mtd->offset;
- }
+ if (ret && instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
+ instr->fail_addr -= mtd->offset;
+
+ instr->addr -= mtd->offset;
+
return ret;
}
void mtd_erase_callback(struct erase_info *instr)
{
- if (instr->mtd->_erase == part_erase) {
+ if (!instr->callback)
+ return;
+
+ if (instr->mtd->_erase == part_erase && instr->len) {
if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
instr->fail_addr -= instr->mtd->offset;
instr->addr -= instr->mtd->offset;
}
- if (instr->callback)
- instr->callback(instr);
+
+ instr->callback(instr);
+
+ if (instr->mtd->_erase == part_erase && instr->len) {
+ if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
+ instr->fail_addr += instr->mtd->offset;
+ instr->addr += instr->mtd->offset;
+ }
}
EXPORT_SYMBOL_GPL(mtd_erase_callback);
--
2.32.0
next prev parent reply other threads:[~2021-09-25 17:35 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-25 17:33 [PATCH u-boot-spi v2 0/9] Fix `mtd erase` when used with mtdpart Marek Behún
2021-09-25 17:33 ` [PATCH u-boot-spi v2 1/9] mtd: spi-nor-core: Try cleaning up in case writing BAR failed Marek Behún
2021-09-28 16:45 ` Pratyush Yadav
2021-09-25 17:33 ` [PATCH u-boot-spi v2 2/9] mtd: spi-nor-core: Check return value of write_enable() in spi_nor_erase() Marek Behún
2021-09-28 16:48 ` Pratyush Yadav
2021-09-25 17:33 ` [PATCH u-boot-spi v2 3/9] mtd: spi-nor-core: Don't overwrite return value if it is non-zero Marek Behún
2021-09-25 17:33 ` [PATCH u-boot-spi v2 4/9] mtd: spi-nor-core: Check return value of write_disable() in spi_nor_erase() Marek Behún
2021-09-25 17:33 ` [PATCH u-boot-spi v2 5/9] mtd: spi-nor-core: Don't check for zero length " Marek Behún
2021-09-28 16:59 ` Pratyush Yadav
2021-10-01 9:25 ` Marek Behún
2021-10-01 10:30 ` Pratyush Yadav
2021-09-25 17:33 ` [PATCH u-boot-spi v2 6/9] mtd: spi-nor-core: Call mtd_erase_callback() from spi_nor_erase() Marek Behún
2021-09-25 17:33 ` [PATCH u-boot-spi v2 7/9] mtd: spi-nor-core: Check for ctrlc() in spi_nor_erase() Marek Behún
2021-09-25 17:33 ` Marek Behún [this message]
2021-09-25 17:33 ` [PATCH u-boot-spi v2 9/9] mtd: Remove mtd_erase_callback() entirely Marek Behún
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=20210925173318.25804-9-kabel@kernel.org \
--to=kabel@kernel.org \
--cc=jagan@amarulasolutions.com \
--cc=marek.behun@nic.cz \
--cc=marex@denx.de \
--cc=masami.hiramatsu@linaro.org \
--cc=pali@kernel.org \
--cc=patrice.chotard@foss.st.com \
--cc=patrick.delaunay@st.com \
--cc=sjg@chromium.org \
--cc=trini@konsulko.com \
--cc=u-boot@lists.denx.de \
/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.