[Skiboot] [PATCH 2/4] core/flash: Make flash_reserve() and flash_release() use any flash

[Cyril Bur]

On Thu, 2017-06-29 at 11:23 +1000, Alistair Popple wrote:
> On Thu, 29 Jun 2017 09:38:02 AM Cyril Bur wrote:
> > Now that skiboot can have multiple flashes it doesn't make sense for
> > flash_reserve() and flash_release() to only operate on system_flash.
> > 
> > New functions system_flash_reserve() and system_flash_release() preserve
> > the current functionality of flash_reserve() and flash_release() and
> > flash_reserve() and flash_releasei() can now be used to mark any flash
> > as busy.
> > 
> > Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
> > ---
> >  core/flash.c       | 20 +++++++++++++++-----
> >  hw/ipmi/ipmi-sel.c |  4 ++--
> >  include/skiboot.h  |  4 ++--
> >  3 files changed, 19 insertions(+), 9 deletions(-)
> > 
> > diff --git a/core/flash.c b/core/flash.c
> > index f0390394..a905e986 100644
> > --- a/core/flash.c
> > +++ b/core/flash.c
> > @@ -48,15 +48,15 @@ static struct lock flash_lock;
> >  static struct flash *nvram_flash;
> >  static u32 nvram_offset, nvram_size;
> >  
> > -bool flash_reserve(void)
> > +static bool flash_reserve(struct flash *flash)
> >  {
> >  	bool rc = false;
> >  
> >  	if (!try_lock(&flash_lock))
> 
> So this is a global lock protecting all flash structures Skiboot knows about?
> Why wouldn't me make this more fine grained (ie. per struct)?

We totally could, my yak shaving razor is blunt.

> 
> >  		return false;
> >  
> > -	if (!system_flash->busy) {
> > -		system_flash->busy = true;
> > +	if (!flash->busy) {
> > +		flash->busy = true;
> >  		rc = true;
> >  	}
> >  	unlock(&flash_lock);
> > @@ -64,13 +64,23 @@ bool flash_reserve(void)
> >  	return rc;
> >  }
> >  
> > -void flash_release(void)
> > +static void flash_release(struct flash *flash)
> >  {
> >  	lock(&flash_lock);
> > -	system_flash->busy = false;
> > +	flash->busy = false;
> >  	unlock(&flash_lock);
> >  }
> >  
> > +bool system_flash_reserve(void)
> > +{
> > +	return flash_reserve(system_flash);
> > +}
> > +
> > +void system_flash_release(void)
> > +{
> > +	flash_release(system_flash);
> > +}
> > +
> >  static int flash_nvram_info(uint32_t *total_size)
> >  {
> >  	int rc;
> > diff --git a/hw/ipmi/ipmi-sel.c b/hw/ipmi/ipmi-sel.c
> > index 5c766472..14d10db2 100644
> > --- a/hw/ipmi/ipmi-sel.c
> > +++ b/hw/ipmi/ipmi-sel.c
> > @@ -475,7 +475,7 @@ static void sel_pnor(uint8_t access)
> >  			break;
> >  		}
> >  
> > -		granted = flash_reserve();
> > +		granted = system_flash_reserve();
> >  		if (granted)
> >  			occ_pnor_set_owner(PNOR_OWNER_EXTERNAL);
> >  		/* Ack the request */
> > @@ -484,7 +484,7 @@ static void sel_pnor(uint8_t access)
> >  		break;
> >  	case RELEASE_PNOR:
> >  		prlog(PR_NOTICE, "PNOR access released\n");
> > -		flash_release();
> > +		system_flash_release();
> >  		occ_pnor_set_owner(PNOR_OWNER_HOST);
> >  		break;
> >  	default:
> > diff --git a/include/skiboot.h b/include/skiboot.h
> > index 1a153b02..785986cf 100644
> > --- a/include/skiboot.h
> > +++ b/include/skiboot.h
> > @@ -235,8 +235,8 @@ extern int flash_register(struct blocklevel_device *bl);
> >  extern int flash_start_preload_resource(enum resource_id id, uint32_t subid,
> >  					void *buf, size_t *len);
> >  extern int flash_resource_loaded(enum resource_id id, uint32_t idx);
> > -extern bool flash_reserve(void);
> > -extern void flash_release(void);
> > +extern bool system_flash_reserve(void);
> > +extern void system_flash_release(void);
> >  #define FLASH_SUBPART_ALIGNMENT 0x1000
> >  #define FLASH_SUBPART_HEADER_SIZE FLASH_SUBPART_ALIGNMENT
> >  extern int flash_subpart_info(void *part_header, uint32_t header_len,
> > 
> 
>