[PATCH] mtdblock: warn if opened on NAND

[Ezequiel Garcia]

On Mon, Mar 28, 2022 at 1:11 PM Bjørn Mork <bjorn at mork.no> wrote:
>
> Warning on every translated mtd partition results in excessive log noise
> if this driver is loaded:
>
>   nand: device found, Manufacturer ID: 0xc2, Chip ID: 0xf1
>   nand: Macronix MX30LF1G18AC
>   nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
>   mt7621-nand 1e003000.nand: ECC strength adjusted to 4 bits
>   read_bbt: found bbt at block 1023
>   10 fixed-partitions partitions found on MTD device mt7621-nand
>   Creating 10 MTD partitions on "mt7621-nand":
>   0x000000000000-0x000000080000 : "Bootloader"
>   mtdblock: MTD device 'Bootloader' is NAND, please consider using UBI block devices instead.
>   0x000000080000-0x000000100000 : "Config"
>   mtdblock: MTD device 'Config' is NAND, please consider using UBI block devices instead.
>   0x000000100000-0x000000140000 : "Factory"
>   mtdblock: MTD device 'Factory' is NAND, please consider using UBI block devices instead.
>   0x000000140000-0x000002000000 : "Kernel"
>   mtdblock: MTD device 'Kernel' is NAND, please consider using UBI block devices instead.
>   0x000000540000-0x000002000000 : "ubi"
>   mtdblock: MTD device 'ubi' is NAND, please consider using UBI block devices instead.
>   0x000002140000-0x000004000000 : "Kernel2"
>   mtdblock: MTD device 'Kernel2' is NAND, please consider using UBI block devices instead.
>   0x000004000000-0x000004100000 : "wwan"
>   mtdblock: MTD device 'wwan' is NAND, please consider using UBI block devices instead.
>   0x000004100000-0x000005100000 : "data"
>   mtdblock: MTD device 'data' is NAND, please consider using UBI block devices instead.
>   0x000005100000-0x000005200000 : "rom-d"
>   mtdblock: MTD device 'rom-d' is NAND, please consider using UBI block devices instead.
>   0x000005200000-0x000005280000 : "reserve"
>   mtdblock: MTD device 'reserve' is NAND, please consider using UBI block devices instead.
>   mtk_soc_eth 1e100000.ethernet eth0: mediatek frame engine at 0xbe100000, irq 21
>
> This is more likely to annoy than to help users of embedded distros where
> this driver is enabled by default.  Making the blockdevs available does
> not imply that they are in use, and warning about bootloader partitions
> or other devices which obviously never will be mounted is more confusing
> than helpful.
>
> Move the warning to open(), where it will be of more use - actually warning
> anyone who mounts a file system on NAND using mtdblock.
>
> Fixes: e07403a8c6be ("mtdblock: Warn if added for a NAND device")
> Signed-off-by: Bjørn Mork <bjorn at mork.no>

Reviewed-by: Ezequiel Garcia <ezequiel at vanguardiasur.com.ar>

Given how annoying and noisy the above is on machines
where mtdblock is completely unused, I think this change
makes sense:

Thanks,
Ezequiel

> ---
> I don't know what to do about the mtdblock_ro part of the original
> patch since it doesn't have an open hook. Adding one just to print
> this warning seems a bit like overkill...  Maybe just drop the
> warning there? What harm is there accessing NAND devices anyway?
>
> Leaving that decision up to you experts :-)
>
> I'd really like to see this fix get into stable before the masses
> start noticing now that OpenWrt is moving to 5.15.  As you may or
> not know, OpenWrt unconditionally includes mtdblock whether the
> target is NAND or NOR.  So this will cause lots of noise on any
> NAND OpenWrt device.
>
>
> Bjørn
>
>  drivers/mtd/mtdblock.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
> index 03e3de3a5d79..1e94e7d10b8b 100644
> --- a/drivers/mtd/mtdblock.c
> +++ b/drivers/mtd/mtdblock.c
> @@ -257,6 +257,10 @@ static int mtdblock_open(struct mtd_blktrans_dev *mbd)
>                 return 0;
>         }
>
> +       if (mtd_type_is_nand(mbd->mtd))
> +               pr_warn("%s: MTD device '%s' is NAND, please consider using UBI block devices instead.\n",
> +                       mbd->tr->name, mbd->mtd->name);
> +
>         /* OK, it's not open. Create cache info for it */
>         mtdblk->count = 1;
>         mutex_init(&mtdblk->cache_mutex);
> @@ -322,10 +326,6 @@ static void mtdblock_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
>         if (!(mtd->flags & MTD_WRITEABLE))
>                 dev->mbd.readonly = 1;
>
> -       if (mtd_type_is_nand(mtd))
> -               pr_warn("%s: MTD device '%s' is NAND, please consider using UBI block devices instead.\n",
> -                       tr->name, mtd->name);
> -
>         if (add_mtd_blktrans_dev(&dev->mbd))
>                 kfree(dev);
>  }
> --
> 2.30.2
>