[PATCH v1 1/4] drivers: introduce mtdblock abstraction

Alexey Romanov avromanov at salutedevices.com
Thu Mar 7 11:24:41 CET 2024


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 <avromanov at 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 <avromanov at 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


More information about the U-Boot mailing list