[PATCH] mtdblock: warn if opened on NAND

[PATCH] mtdblock: warn if opened on NAND

[Bjørn Mork]

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@mork.no>
---
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


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtdblock: warn if opened on NAND

[Ezequiel Garcia]

H Bjørn,

Thanks a lot for taking care of this. I know it's been a pain point for users.

On Mon, Mar 28, 2022 at 1:11 PM Bjørn Mork <bjorn@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@mork.no>
> ---
> 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?
>

If I remember correctly, rhe reason for adding all these warnings came
after getting questions from developers unaware of UBI and UBI block.
These users/developers were building  products with
read-only filesystem on top of MTD block, on NAND devices.

I believe mtdblock itself isn't as needed as it used to be,
given you can mount JFFS2 directly on a MTD char device.

Internet's trucks remain heavily loaded with documentation and
tutorials pointing people to mtdblock. So the idea was to balance
that with some self-documentation in the form of a warning.

I think adding an mtdblock_ro open hook is totally justified if it points
users in the right direction. Alternatively, we can move the warning
to the upper mtd_blkdevs.c layer, and put it in blktrans_open.

mtd_blkdevs.c is used by some other drivers so maybe that's not desirable.

Regards,
Ezequiel

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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtdblock: warn if opened on NAND

[Tudor.Ambarus]

On 3/28/22 19:11, Bjørn Mork wrote:

Hi, Bjørn!

> 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@mork.no>
> ---
> 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

I don't see this as a fix, you just mask out the warning, you assume
that mtdblock is not used but you still keep it in your config, which
is wrong. You should instead update your config, remove mtdblock, and
fix the cause if it's possible.

> 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

Why does Openwrt include mtdblock?

I have encountered these messages in some configs that I'm using and I
ended up cleaning them. These messages helped me identify the problem and
ultimately to fix it. Let's give others the chance to do the same.

Cheers,
ta
> 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);
>  }

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtdblock: warn if opened on NAND

[Bjørn Mork]

<Tudor.Ambarus@microchip.com> writes:

> I don't see this as a fix, you just mask out the warning, you assume
> that mtdblock is not used

No, I don't.  No real warnings are masked.  Only bogus warnings.

I make no assumption about mtdblock usage. I moved the warning to open()
so that the driver complains when a NAND mtdblock partition is mounted.
I am sure this does happen from time to time, and agree 100% that a
warning is appropriate in those cases.

Commit e07403a8c6be is based on two false assumptions:
1) the presence of the mtdblock driver is a problem
2) all partitions will be mounted as mtdblock devices

Extensive log noise is a problem for end users.  We can obviously argue
if it is a bug or not.  I claim that it is because real warnings drown
in the noise, making real bugs much harder to spot.

> but you still keep it in your config, which
> is wrong. You should instead update your config, remove mtdblock, and
> fix the cause if it's possible.

So if you have a device with both NOR and NAND flash, then what?  Not
that unusual.

>> 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
>
> Why does Openwrt include mtdblock?

I don't know and I don't speak for OpenWrt, being only an end user.  But
I assume it's because OpenWrt builds semi-generic images for both NOR
and NAND devices, as well as devices with both flash types. They are
also trying to maintain flash layout compatibillty with OEM firmware,
which often does really stupid things.

Sure, OpenWrt could probably strip mtdblock from NAND only devices. But
I guess the priority is low since it doesn't fix any problem, and it
will complicate their build slightly. I'm sure patches are welcome
though.

> I have encountered these messages in some configs that I'm using and I
> ended up cleaning them. These messages helped me identify the problem and
> ultimately to fix it. Let's give others the chance to do the same.

Others will still get the warning iff there is a problem.

The fix is to prevent warnings when there isn't one.  If you warn 200
times (which IMHO is a low estimate here) for every real problem, then
no one will care in the end. "Oh, it's just mtdblock whining again.  It
does that all the time"


Bjørn

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtdblock: warn if opened on NAND

[Tudor.Ambarus]

Hi, Bjørn,

On 4/21/22 19:43, Bjørn Mork wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> <Tudor.Ambarus@microchip.com> writes:
> 
>> I don't see this as a fix, you just mask out the warning, you assume
>> that mtdblock is not used
> 
> No, I don't.  No real warnings are masked.  Only bogus warnings.
> 
> I make no assumption about mtdblock usage. I moved the warning to open()
> so that the driver complains when a NAND mtdblock partition is mounted.
> I am sure this does happen from time to time, and agree 100% that a
> warning is appropriate in those cases.
> 
> Commit e07403a8c6be is based on two false assumptions:
> 1) the presence of the mtdblock driver is a problem

It is a problem. Using it will wear out parts of your raw flash relatively
fast. There's no wear-leveling with mtdblock. IMO mtdblock should be used
just for testing purposes maybe, or for cases where users are forced to
use write capable block-based File Systems - which is still wrong in my
opinion.

> 2) all partitions will be mounted as mtdblock devices

Indeed, we should print only a message, not one for each partition. Even so,
wearing out parts of a partition damages your raw flash.

> 
> Extensive log noise is a problem for end users.  We can obviously argue
> if it is a bug or not.  I claim that it is because real warnings drown
> in the noise, making real bugs much harder to spot.
> 
>> but you still keep it in your config, which
>> is wrong. You should instead update your config, remove mtdblock, and
>> fix the cause if it's possible.
> 
> So if you have a device with both NOR and NAND flash, then what?  Not
> that unusual.

Sorry, I don't understand this argument. Both NAND and NOR flashes will
wear out if using mtdblock. NAND will wear faster than NOR, but NOR is
still affected.

> 
>>> 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
>>
>> Why does Openwrt include mtdblock?
> 
> I don't know and I don't speak for OpenWrt, being only an end user.  But
> I assume it's because OpenWrt builds semi-generic images for both NOR
> and NAND devices, as well as devices with both flash types. They are

Neither NOR, nor NAND should use mtdblock to enable write-capable
block-based File Systems in production. Raw flashes should be handled
with UBI instead.

> also trying to maintain flash layout compatibillty with OEM firmware,
> which often does really stupid things.
> 
> Sure, OpenWrt could probably strip mtdblock from NAND only devices. But
> I guess the priority is low since it doesn't fix any problem, and it
> will complicate their build slightly. I'm sure patches are welcome
> though.
> 
>> I have encountered these messages in some configs that I'm using and I
>> ended up cleaning them. These messages helped me identify the problem and
>> ultimately to fix it. Let's give others the chance to do the same.
> 
> Others will still get the warning iff there is a problem.

It is a problem in my opinion. Your flash will be irreversibly damaged
really fast if using mtdblock.

> 
> The fix is to prevent warnings when there isn't one.  If you warn 200
> times (which IMHO is a low estimate here) for every real problem, then
> no one will care in the end. "Oh, it's just mtdblock whining again.  It
> does that all the time"
> 

I agree we should improve this and pass a single warning. I would like to
still have this warning at boot if possible. I guess people have the
mtdblock config in their defconfigs, but never use mtdblock, at least that
was my case. Having that warning will give them the chance to fix their
defconfigs.

Btw, something similar was discussed at:
https://lore.kernel.org/lkml/876982414.38679.1635274892099.JavaMail.zimbra@nod.at/

Cheers,
ta

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtdblock: warn if opened on NAND

[Bjørn Mork]

<Tudor.Ambarus@microchip.com> writes:

>> Commit e07403a8c6be is based on two false assumptions:
>> 1) the presence of the mtdblock driver is a problem
>
> It is a problem. Using it will wear out parts of your raw flash relatively
> fast.

There is a significant difference between PRESENCE and USING.

I'll just stop this now. You're not arguing with me but woith some
straw man you created.





Bjørn

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtdblock: warn if opened on NAND

[Miquel Raynal]

Hi folks,

bjorn@mork.no wrote on Fri, 22 Apr 2022 08:38:27 +0200:

> <Tudor.Ambarus@microchip.com> writes:
> 
> >> Commit e07403a8c6be is based on two false assumptions:
> >> 1) the presence of the mtdblock driver is a problem  
> >
> > It is a problem. Using it will wear out parts of your raw flash relatively
> > fast.  
> 
> There is a significant difference between PRESENCE and USING.
> 
> I'll just stop this now. You're not arguing with me but woith some
> straw man you created.

Right now there is a message for _every_ mtd device that gets created
only if mtdblock support is built-in. Of course enabling mtdblock if
you don't use it is a waste of space, but it is far less an issue than
people actually using it and I think that is our main target.

The warnings from Ezequiel were a good idea but right now I think we
all agree that they are too frequent. A single warning would be an
improvement, but otherwise a warning only when ->open() gets called
seems a good enough step forward for me.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtdblock: warn if opened on NAND

[Ezequiel Garcia]

On Mon, Mar 28, 2022 at 1:11 PM Bjørn Mork <bjorn@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@mork.no>

Reviewed-by: Ezequiel Garcia <ezequiel@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
>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtdblock: warn if opened on NAND

[Miquel Raynal]

On Mon, 2022-03-28 at 16:11:08 UTC, =?utf-8?q?Bj=C3=B8rn_Mork?= 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@mork.no>
> Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.

Miquel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/