On Wed, 14 Jul 2021 at 17:51, Marek Behún <marek.be...@nic.cz> wrote: > > 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.be...@nic.cz> > Tested-by: Masami Hiramatsu <masami.hirama...@linaro.org> > --- > drivers/mtd/mtdpart.c | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-)Reviewed-by: Simon Glass > <s...@chromium.org> >
Reviewed-by: Simon Glass <s...@chromium.org>