Hi Heinrich,
On Tue, Feb 27, 2024 at 02:37:17PM +0100, Heinrich Schuchardt wrote:
> On 27.02.24 11:04, Alexey Romanov wrote:
> > MTD block - abstraction over MTD subsystem, allowing
> > to read and write in blocks using BLK UCLASS.
> >
> > - Read algorithm:
> >
> > 1. Convert start block number to start address.
> > 2. Read block_dev->blksz bytes using mtd_read() and
> > add to start address pointer block_dev->blksz bytes,
> > until the requested number of blocks have been read.
> >
> > - Write algorithm:
> >
> > 1. Convert start block number to start address.
> > 2. Round this address down by mtd->erasesize.
> >
> > Erase addr Start addr
> > | |
> > v v
> > +----------------+----------------+----------------+
> > | blksz | blksz | blksz |
> > +----------------+----------------+----------------+
> >
> > 3. Calculate offset between this two addresses.
> > 4. Read mtd->erasesize bytes using mtd_read() into
> > temporary buffer from erase address.
> >
> > Erase addr Start addr
> > | |
> > v v
> > +----------------+----------------+----------------+
> > | blksz | blksz | blksz |
> > +----------------+----------------+----------------+
> > ^ mtd->erasesize = blksz * 3
>
> Can you really erase only the start of the erase block?
Yeah.
>
> Wouldn't you have also to round the end address up to the end of an
> erase block and read and rewrite the data between the end address and
> the end of the last erase block?
I think I didn't express myself quite correctly. Of coure, we erase
and write only by mtd->erasesize bytes and don't overwrite this field.
> > ^ mtd->erasesize = blksz * 3
I just wrote that for example mtd->erasesize can be equal to blksz * 3,
for the situtation I describe above. In reality it can be anything. I
will describe this point in more detail in v2.
>
> > |
> > |
> > |
> >
> > mtd_read()
> > from here
> >
> > 5. Copy data from user buffer to temporary buffer with offset,
> > calculated at step 3.
> > 6. Erase and write mtd->erasesize bytes at erase address
> > pointer using mtd_erase/mtd_write().
> > 7. Add to erase address pointer mtd->erasesize bytes.
> > 8. goto 1 until the requested number of blocks have
> > been written.
>
> Commit messages is not were developers typically look for documentation.
> Please, add the information to /doc/develop/ or to the source code.
Move it to docs in v2.
>
> Best regards
>
> Heinrich
>
> >
> > Signed-off-by: Alexey Romanov <avroma...@salutedevices.com>
> > ---
> > drivers/block/blk-uclass.c | 1 +
> > drivers/mtd/Makefile | 1 +
> > drivers/mtd/mtdblock.c | 170 +++++++++++++++++++++++++++++++++++++
> > include/linux/mtd/mtd.h | 12 +++
> > 4 files changed, 184 insertions(+)
> > create mode 100644 drivers/mtd/mtdblock.c
> >
> > diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> > index 77066da352..ab0a9105c9 100644
> > --- a/drivers/block/blk-uclass.c
> > +++ b/drivers/block/blk-uclass.c
> > @@ -37,6 +37,7 @@ static struct {
> > { UCLASS_PVBLOCK, "pvblock" },
> > { UCLASS_BLKMAP, "blkmap" },
> > { UCLASS_RKMTD, "rkmtd" },
> > + { UCLASS_MTD, "mtd" },
> > };
> >
> > static enum uclass_id uclass_name_to_iftype(const char *uclass_idname)
> > diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
> > index c638980ea2..993b122ac4 100644
> > --- a/drivers/mtd/Makefile
> > +++ b/drivers/mtd/Makefile
> > @@ -26,6 +26,7 @@ obj-y += onenand/
> > obj-y += spi/
> > obj-$(CONFIG_MTD_UBI) += ubi/
> > obj-$(CONFIG_NVMXIP) += nvmxip/
> > +obj-$(CONFIG_BLK) += mtdblock.o
> >
> > #SPL/TPL build
> > else
> > diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
> > new file mode 100644
> > index 0000000000..0563d0b795
> > --- /dev/null
> > +++ b/drivers/mtd/mtdblock.c
> > @@ -0,0 +1,170 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * (C) Copyright 2024 SaluteDevices, Inc.
> > + *
> > + * Author: Alexey Romanov <avroma...@salutedevices.com>
> > + */
> > +
> > +#include <blk.h>
> > +#include <dm/device.h>
> > +#include <dm/device-internal.h>
> > +#include <linux/mtd/mtd.h>
> > +
> > +int mtd_bind(struct udevice *dev, struct mtd_info **mtd)
> > +{
> > + struct blk_desc *bdesc;
> > + struct udevice *bdev;
> > + int ret;
> > +
> > + ret = blk_create_devicef(dev, "mtd_blk", "blk", UCLASS_MTD,
> > + dev_seq(dev), 512, 0, &bdev);
> > + if (ret) {
> > + pr_err("Cannot create block device");
> > + return ret;
> > + }
> > +
> > + bdesc = dev_get_uclass_plat(bdev);
> > + dev_set_priv(bdev, mtd);
> > + bdesc->bdev = bdev;
> > +
> > + return 0;
> > +}
> > +
> > +static ulong mtd_bread(struct udevice *dev, lbaint_t start, lbaint_t
> > blkcnt,
> > + void *dst)
> > +{
> > + struct blk_desc *block_dev = dev_get_uclass_plat(dev);
> > + struct mtd_info *mtd = blk_desc_to_mtd(block_dev);
> > + unsigned int sect_size = block_dev->blksz;
> > + lbaint_t cur = start;
> > + ulong read_cnt = 0;
> > +
> > + while (read_cnt < blkcnt) {
> > + int ret;
> > + loff_t sect_start = cur * sect_size;
> > + size_t retlen;
> > +
> > + ret = mtd_read(mtd, sect_start, sect_size, &retlen, dst);
> > + if (ret)
> > + return ret;
> > +
> > + if (retlen != sect_size) {
> > + pr_err("mtdblock: failed to read block 0x%lx\n", cur);
> > + return -EIO;
> > + }
> > +
> > + cur++;
> > + dst += sect_size;
> > + read_cnt++;
> > + }
> > +
> > + return read_cnt;
> > +}
> > +
> > +static int mtd_erase_write(struct mtd_info *mtd, uint64_t start, const
> > void *src)
> > +{
> > + int ret;
> > + size_t retlen;
> > + struct erase_info erase = { 0 };
> > +
> > + erase.mtd = mtd;
> > + erase.addr = start;
> > + erase.len = mtd->erasesize;
> > +
> > + ret = mtd_erase(mtd, &erase);
> > + if (ret)
> > + return ret;
> > +
> > + ret = mtd_write(mtd, start, mtd->erasesize, &retlen, src);
> > + if (ret)
> > + return ret;
> > +
> > + if (retlen != mtd->erasesize) {
> > + pr_err("mtdblock: failed to read block at 0x%llx\n", start);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static ulong mtd_bwrite(struct udevice *dev, lbaint_t start, lbaint_t
> > blkcnt,
> > + const void *src)
> > +{
> > + struct blk_desc *block_dev = dev_get_uclass_plat(dev);
> > + struct mtd_info *mtd = blk_desc_to_mtd(block_dev);
> > + unsigned int sect_size = block_dev->blksz;
> > + lbaint_t cur = start, blocks_todo = blkcnt;
> > + ulong write_cnt = 0;
> > + u8 *buf;
> > + int ret = 0;
> > +
> > + buf = malloc(mtd->erasesize);
> > + if (!buf)
> > + return -ENOMEM;
> > +
> > + while (blocks_todo > 0) {
> > + loff_t sect_start = cur * sect_size;
> > + loff_t erase_start = ALIGN_DOWN(sect_start, mtd->erasesize);
> > + u32 offset = sect_start - erase_start;
> > + size_t cur_size = min_t(size_t, mtd->erasesize - offset,
> > + blocks_todo * sect_size);
> > + size_t retlen;
> > + lbaint_t written;
> > +
> > + ret = mtd_read(mtd, erase_start, mtd->erasesize, &retlen, buf);
> > + if (ret)
> > + goto out;
> > +
> > + if (retlen != mtd->erasesize) {
> > + pr_err("mtdblock: failed to read block 0x%lx\n", cur);
> > + ret = -EIO;
> > + goto out;
> > + }
> > +
> > + memcpy(buf + offset, src, cur_size);
> > +
> > + ret = mtd_erase_write(mtd, erase_start, buf);
> > + if (ret)
> > + goto out;
> > +
> > + written = cur_size / sect_size;
> > +
> > + blocks_todo -= written;
> > + cur += written;
> > + src += cur_size;
> > + write_cnt += written;
> > + }
> > +
> > +out:
> > + free(buf);
> > +
> > + if (ret)
> > + return ret;
> > +
> > + return write_cnt;
> > +}
> > +
> > +static int mtd_blk_probe(struct udevice *dev)
> > +{
> > + int ret;
> > +
> > + ret = device_probe(dev);
> > + if (ret) {
> > + pr_err("Probing %s failed (err=%d)\n", dev->name, ret);
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct blk_ops mtd_blk_ops = {
> > + .read = mtd_bread,
> > + .write = mtd_bwrite,
> > +};
> > +
> > +U_BOOT_DRIVER(mtd_blk) = {
> > + .name = "mtd_blk",
> > + .id = UCLASS_BLK,
> > + .ops = &mtd_blk_ops,
> > + .probe = mtd_blk_probe,
> > +};
> > diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> > index 09f5269887..9b997fadd1 100644
> > --- a/include/linux/mtd/mtd.h
> > +++ b/include/linux/mtd/mtd.h
> > @@ -26,6 +26,9 @@
> > #include <dm/device.h>
> > #endif
> > #include <dm/ofnode.h>
> > +#if IS_ENABLED(CONFIG_BLK)
> > +#include <blk.h>
> > +#endif
> >
> > #define MAX_MTD_DEVICES 32
> > #endif
> > @@ -412,6 +415,15 @@ int mtd_write(struct mtd_info *mtd, loff_t to, size_t
> > len, size_t *retlen,
> > int mtd_panic_write(struct mtd_info *mtd, loff_t to, size_t len, size_t
> > *retlen,
> > const u_char *buf);
> >
> > +#if IS_ENABLED(CONFIG_BLK)
> > +static inline struct mtd_info *blk_desc_to_mtd(struct blk_desc *bdesc)
> > +{
> > + return *((struct mtd_info **)dev_get_priv(bdesc->bdev));
> > +}
> > +
> > +int mtd_bind(struct udevice *dev, struct mtd_info **mtd);
> > +#endif
> > +
> > int mtd_read_oob(struct mtd_info *mtd, loff_t from, struct mtd_oob_ops
> > *ops);
> > int mtd_write_oob(struct mtd_info *mtd, loff_t to, struct mtd_oob_ops
> > *ops);
> >
>
--
Thank you,
Alexey
