[Skiboot] [PATCH] flash: Support adding the no-erase property to flash

[Suraj Jitindar Singh]

On Fri, 2017-09-01 at 17:01 +0000, William Kennington wrote:
> https://github.com/torvalds/linux/blob/master/Documentation/ABI/testi
> ng/sysfs-class-mtd#L65
> "No erase necessary"

Ok, thanks.

In that case I agree with this patch but think the commit message needs
slight rewording as below. :)

> 
> On Fri, Sep 1, 2017 at 12:16 AM Suraj Jitindar Singh <sjitindarsingh@
> gmail.com> wrote:
> > Hi,
> > 
> > On Mon, 2017-08-28 at 23:48 -0700, William A. Kennington III wrote:
> > > Currently, flash devices like mbox-flash ignore erase commands. 

mbox-flash now has erase capability (for V2 implementations any way) so
I think this sentence is misleading. You might be better off referring
to the fact that the mbox protocol explicitly states that an erase is
not required before a write, which is exactly what this flag is
designed to convey.

> > This
> > > means that issuing an erase from userspace, through the mtd
> > device,
> > > and
> > > back returns a successful operation that does nothing.
> > Unfortunately
> > > this makes userspace tools unhappy. Linux MTD devices support the
> > > MTD_NO_ERASE flag which conveys that writes do not require erases
> > on
> > > the
> > > underlying flash devices. We should set this property on all of
> > our
> > > devices which do not require erases to be performed.
> > >
> > > NOTE: This still requires a linux kernel component to set the
> > > MTD_NO_ERASE flag from the device tree property.
> > 
> > Can I just check, does the MTD_NO_ERASE flag mean that the mtd
> > device
> > doesn't have an erase function, or that an erase isn't necessary
> > before
> > a write?
> > 
> > Because the V2 mbox has an erase method which mbox-flash will call
> > when
> > an erase is performed. So if it means there isn't an erase function
> > then this is incorrect.
> > 
> > If it means that an erase isn't required before a write however
> > then I
> > support this as the mbox protocol explicitly states this to be the
> > case.
> > 
> > Thanks,
> > Suraj
> > 
> > >
> > > Signed-off-by: William A. Kennington III <wak at google.com>

With a slight commit message rewording,

Reviewed-by: Suraj Jitindar Singh <sjitindarsingh at gmail.com>

> > > ---
> > >  core/flash.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/core/flash.c b/core/flash.c
> > > index 53e6eba08..9e230174f 100644
> > > --- a/core/flash.c
> > > +++ b/core/flash.c
> > > @@ -31,6 +31,7 @@
> > >  struct flash {
> > >       struct list_node        list;
> > >       bool                    busy;
> > > +     bool                    no_erase;
> > >       struct blocklevel_device *bl;
> > >       uint64_t                size;
> > >       uint32_t                block_size;
> > > @@ -199,6 +200,8 @@ static struct dt_node
> > *flash_add_dt_node(struct
> > > flash *flash, int id)
> > >       dt_add_property_u64(flash_node, "reg", flash->size);
> > >       dt_add_property_cells(flash_node, "ibm,flash-block-size",
> > >                       flash->block_size);
> > > +     if (flash->no_erase)
> > > +             dt_add_property(flash_node, "no-erase", NULL, 0);
> > >  
> > >       /* we fix to 32-bits */
> > >       dt_add_property_cells(flash_node, "#address-cells", 1);
> > > @@ -281,6 +284,7 @@ int flash_register(struct blocklevel_device
> > *bl)
> > >  
> > >       flash->busy = false;
> > >       flash->bl = bl;
> > > +     flash->no_erase = !(bl->flags & WRITE_NEED_ERASE);
> > >       flash->size = size;
> > >       flash->block_size = block_size;
> > >       flash->id = num_flashes();
> >