[PATCH 09/15] blk: blkmap: Support mapping to device of any block size

[Bin Meng]

Hi Tobias,

On Wed, Sep 27, 2023 at 3:29 AM Tobias Waldekranz <tobias at waldekranz.com> wrote:
>
> On tis, sep 26, 2023 at 16:43, Bin Meng <bmeng at tinylab.org> wrote:
> > At present if a device to map has a block size other than 512,
> > the blkmap map process just fails. There is no reason why we
> > can't just use the block size of the mapped device.
>
> Won't this be very confusing to the user?

I don't see any confusion.

>
> The blkmap device uses a fixed block size of 512:
>
> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/block/blkmap.c?ref_type=heads#L393

Yes, the blkmap device was originally created with a fixed block size
of 512, and that's fine.

>
> So if I map a slice of a 4k device into a blkmap, then
>
>     blkmap read 0x80000000 0 1
>
> would copy 4k instead of 512 bytes from the lower device to 0x80000000,
> even though the blkmap reports a block size of 512.
>
> It seems to me that the expected behavior would be that only the first
> 512 bytes would be copied in the command above.

No, the blkmap block size was later updated to match the real
underlying device parameter during the map process. So it will copy
all 4k to 0x80000000.

>
> >
> > Signed-off-by: Bin Meng <bmeng at tinylab.org>
> > ---
> >
> >  drivers/block/blkmap.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
> > index f6acfa8927..149a4cac3e 100644
> > --- a/drivers/block/blkmap.c
> > +++ b/drivers/block/blkmap.c
> > @@ -171,11 +171,11 @@ int blkmap_map_linear(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
> >
> >       bd = dev_get_uclass_plat(bm->blk);
> >       lbd = dev_get_uclass_plat(lblk);
> > -     if (lbd->blksz != bd->blksz)
> > -             /* We could support block size translation, but we
> > -              * don't yet.
> > -              */
>
> Hence this comment ^

This comment was completely removed with the new updates. There is no
need to do any block size translation. We could just use whatever
block size the lower device is using, hence this patch.

>
> > -             return -EINVAL;
> > +     if (lbd->blksz != bd->blksz) {
> > +             /* update to match the mapped device */
> > +             bd->blksz = lbd->blksz;
> > +             bd->log2blksz = LOG2(bd->blksz);
> > +     }
> >
> >       linear = malloc(sizeof(*linear));
> >       if (!linear)
> > --

Regards,
Bin