[Skiboot] [PATCH 2/3] external/common: Add default erase chip implmentation

Cyril Bur cyril.bur at au1.ibm.com
Tue Nov 8 09:16:57 AEDT 2016


On Mon, 2016-11-07 at 21:53 +1030, Joel Stanley wrote:
> On Mon, Nov 7, 2016 at 4:58 PM, Cyril Bur <cyril.bur at au1.ibm.com>
> wrote:
> > Just blocklevel_erase() from zero to sizeof.
> > 
> > Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
> > ---
> >  external/common/arch_flash_common.c | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/external/common/arch_flash_common.c
> > b/external/common/arch_flash_common.c
> > index ba06fb2..b6c87af 100644
> > --- a/external/common/arch_flash_common.c
> > +++ b/external/common/arch_flash_common.c
> > @@ -13,15 +13,30 @@
> >   * See the License for the specific language governing permissions
> > and
> >   * limitations under the License.
> >   */
> > +#include <stdlib.h>
> > 
> >  #include <libflash/blocklevel.h>
> > 
> >  #include "arch_flash.h"
> > 
> >  /* Default implmentations */
> > +
> > +/*
> > + * This just assumes that an erase from zero to total size is
> > + * 'correct'.
> > + * This is true for both powerpc and x86 and ARM has its own
> 
> a && b && c? Perhaps you're missing some parenthesis?
> 

Ah yeah that makes little sense...

What I meant to say:
An erase from zero to total size is the correct approach for powerpc
and x86. ARM has it own function which also includes a call to the
flash driver.

> This implementation is the same as the one you just wrote for ARM.
> Why
> not allow ARM to use this one?
> 

If the ARM was inited with an actual driver (something that isn't
possible on powerpc or x86) then the last line of the function will
run:
return flash_erase_chip(arch_data.flash_chip);

This differs. I'm sure I could have done it a better way.

> If you have a reason not to allow that then can you explain why you
> add a weakly linked version too?
> 
> > + * implmentation.
> 
> Spelling.

Oops. My forte strikes again.

Thanks,

Cyril
> 
> > + */
> >  int __attribute__((weak)) arch_flash_erase_chip(struct
> > blocklevel_device *bl)
> >  {
> > -       return -1;
> > +       int rc;
> > +       uint64_t total_size;
> > +
> > +       rc = blocklevel_get_info(bl, NULL, &total_size, NULL);
> > +       if (rc)
> > +               return rc;
> > +
> > +       return blocklevel_erase(bl, 0, total_size);
> >  }
> > 
> >  int __attribute__((weak)) arch_flash_4b_mode(struct
> > blocklevel_device *bl, int set_4b)
> > --
> > 2.10.2
> > 
> 
> 



More information about the Skiboot mailing list