Re: [PATCH] environment: do not attempt to erase devices with MTD_NO_ERASE

[Sascha Hauer <s.hauer@pengutronix.de>]

On Thu, Nov 22, 2018 at 11:19:30AM +0100, Matthias Schiffer wrote:
> On Wed, 2018-11-21 at 08:56 +0100, Sascha Hauer wrote:
> > Hi Matthias,
> > 
> > On Tue, Nov 20, 2018 at 10:40:34AM +0100, 
> > matthias.schiffer@ew.tq-group.com wrote:
> > > From: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > > 
> > > Devices like MRAM do not need to be erased; in fact, trying to do a
> > > partial
> > > erase will fail with -EINVAL, as they don't have a proper erase
> > > block size
> > > defined.
> > 
> > Where does this -EINVAL come from? Wouldn't it be an option to check
> > for
> > the MTD_NO_ERASE flag mtd_op_erase() and return -EOPNOTSUPP there?
> > 
> > Sascha
> > 
> 
> Hmm, what are the expected semantics of MTD_NO_ERASE - "does not need
> erase" or "does not support erase"? According to code comments, it's
> the former.
> 
> The MRAM I'm working with reports an erase size equal to its total
> size:
> 
> barebox:/ devinfo m25p0
> Parameters:
>   erasesize: 131072 (type: uint32)
>   oobsize: 0 (type: uint32)
>   partitions: 128k(barebox-environment) (type: string)
>   size: 131072 (type: uint64)
>   writesize: 1 (type: uint32)
> 
> This matches what Linux reports, but it seems to me that this MRAM does
> not actually implement it: When I try to erase the whole flash (from
> Linux or Barebox), it does return success, but the data is unchanged.

So actually your device does not support erase. I'd say "does not
support erase" is the correct semantics for MTD_NO_ERASE, at least I
can't think of any devices that support an optional erase operation.

> When I configure the environment to span the whole flash, envfs_save()
> is working fine without my patch, as the erase is successful.
> 
> The problems start when I try to partition the MRAM, as envfs_save()
> will now try to erase a block smaller than the erasesize, leading to
> -EINVAL.

The MRAM specifies an erasesize of 128KiB and then afterwards an erase
operation is a no-op. This seems wrong, at least inconsistent. This
should be discussed on the Linux mtd list.

For barebox I think it's fine to return -EOPNOTSUPP for devices which
have the MTD_NO_ERASE flag. At least for your device this return value
really is correct.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox