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

[Matthias Schiffer <matthias.schiffer@ew.tq-group.com>]

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

I'm not sure if making mtd_op_erase() check for MTD_NO_ERASE is the
correct solution - it would obviously be fine for my MRAM, as erase is
a noop anyways, but I have no idea if there are other devices that set
the flag, but do support erase (and just don't need it for normal write
operations).

Anyways, I have no problem with implementing the fix that way if we
decide that these are the semantics we want.

Matthias


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