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

Message ID 20170628233804.18412-2-cyril.bur@au1.ibm.com
State New
Headers show

Commit Message

Cyril Bur June 28, 2017, 11:38 p.m.
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@au1.ibm.com>
---
 core/flash.c       | 20 +++++++++++++++-----
 hw/ipmi/ipmi-sel.c |  4 ++--
 include/skiboot.h  |  4 ++--
 3 files changed, 19 insertions(+), 9 deletions(-)

Comments

Alistair Popple June 29, 2017, 1:23 a.m. | #1
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@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)?

>  		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,
>
Cyril Bur June 29, 2017, 1:28 a.m. | #2
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@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,
> > 
> 
>
Alistair Popple June 29, 2017, 1:37 a.m. | #3
On Thu, 29 Jun 2017 11:28:33 AM Cyril Bur wrote:
> 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@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.

Haha. Better get a grind stone out then! I don't think it's a big deal so long
as it isn't preventing simultaneous access to different flash chips. Which I
assume is what the next patch in the series fixes?

> > 
> > >  		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,
> > > 
> > 
> > 
>
Cyril Bur June 29, 2017, 1:40 a.m. | #4
On Thu, 2017-06-29 at 11:37 +1000, Alistair Popple wrote:
> On Thu, 29 Jun 2017 11:28:33 AM Cyril Bur wrote:
> > 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@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.
> 
> Haha. Better get a grind stone out then! I don't think it's a big deal so long
> as it isn't preventing simultaneous access to different flash chips. Which I
> assume is what the next patch in the series fixes?
> 

The next patch does improve on the current situation yes.

> > > 
> > > >  		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,
> > > > 
> > > 
> > > 
> 
>
Alistair Popple June 29, 2017, 1:47 a.m. | #5
On Thu, 29 Jun 2017 11:40:23 AM Cyril Bur wrote:
> On Thu, 2017-06-29 at 11:37 +1000, Alistair Popple wrote:
> > On Thu, 29 Jun 2017 11:28:33 AM Cyril Bur wrote:
> > > 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

I didn't see the flash_releasei() function defined anywhere.

> > > > > as busy.
> > > > > 
> > > > > Signed-off-by: Cyril Bur <cyril.bur@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.
> > 
> > Haha. Better get a grind stone out then! I don't think it's a big deal so long
> > as it isn't preventing simultaneous access to different flash chips. Which I
> > assume is what the next patch in the series fixes?
> > 
> 
> The next patch does improve on the current situation yes.
> 
> > > > 
> > > > >  		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)

Are you sure you didn't mean flash_releasei() here?

> > > > >  {
> > > > >  	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,
> > > > > 
> > > > 
> > > > 
> > 
> > 
>
Cyril Bur June 29, 2017, 1:51 a.m. | #6
On Thu, 2017-06-29 at 11:47 +1000, Alistair Popple wrote:
> On Thu, 29 Jun 2017 11:40:23 AM Cyril Bur wrote:
> > On Thu, 2017-06-29 at 11:37 +1000, Alistair Popple wrote:
> > > On Thu, 29 Jun 2017 11:28:33 AM Cyril Bur wrote:
> > > > 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
> 
> I didn't see the flash_releasei() function defined anywhere.
> 

Damn you win :(

> > > > > > as busy.
> > > > > > 
> > > > > > Signed-off-by: Cyril Bur <cyril.bur@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.
> > > 
> > > Haha. Better get a grind stone out then! I don't think it's a big deal so long
> > > as it isn't preventing simultaneous access to different flash chips. Which I
> > > assume is what the next patch in the series fixes?
> > > 
> > 
> > The next patch does improve on the current situation yes.
> > 
> > > > > 
> > > > > >  		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)
> 
> Are you sure you didn't mean flash_releasei() here?
> 

Pretty sure, but since I didn't declare it... did I? who am I?

> > > > > >  {
> > > > > >  	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,
> > > > > > 
> > > > > 
> > > > > 
> > > 
> > > 
> 
>
Alistair Popple June 29, 2017, 1:53 a.m. | #7
On Thu, 29 Jun 2017 11:51:02 AM Cyril Bur wrote:
> On Thu, 2017-06-29 at 11:47 +1000, Alistair Popple wrote:
> > On Thu, 29 Jun 2017 11:40:23 AM Cyril Bur wrote:
> > > On Thu, 2017-06-29 at 11:37 +1000, Alistair Popple wrote:
> > > > On Thu, 29 Jun 2017 11:28:33 AM Cyril Bur wrote:
> > > > > 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
> > 
> > I didn't see the flash_releasei() function defined anywhere.
> > 
> 
> Damn you win :(

I'm pretty sure you can bribe maintainers to fix commit message typos up for you
:-)

Reviewed-by: Alistair Popple <alistair@popple.id.au>

> > > > > > > as busy.
> > > > > > > 
> > > > > > > Signed-off-by: Cyril Bur <cyril.bur@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.
> > > > 
> > > > Haha. Better get a grind stone out then! I don't think it's a big deal so long
> > > > as it isn't preventing simultaneous access to different flash chips. Which I
> > > > assume is what the next patch in the series fixes?
> > > > 
> > > 
> > > The next patch does improve on the current situation yes.
> > > 
> > > > > > 
> > > > > > >  		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)
> > 
> > Are you sure you didn't mean flash_releasei() here?
> > 
> 
> Pretty sure, but since I didn't declare it... did I? who am I?
> 
> > > > > > >  {
> > > > > > >  	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,
> > > > > > > 
> > > > > > 
> > > > > > 
> > > > 
> > > > 
> > 
> > 
>
Cyril Bur June 29, 2017, 1:55 a.m. | #8
On Thu, 2017-06-29 at 11:53 +1000, Alistair Popple wrote:
> On Thu, 29 Jun 2017 11:51:02 AM Cyril Bur wrote:
> > On Thu, 2017-06-29 at 11:47 +1000, Alistair Popple wrote:
> > > On Thu, 29 Jun 2017 11:40:23 AM Cyril Bur wrote:
> > > > On Thu, 2017-06-29 at 11:37 +1000, Alistair Popple wrote:
> > > > > On Thu, 29 Jun 2017 11:28:33 AM Cyril Bur wrote:
> > > > > > 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
> > > 
> > > I didn't see the flash_releasei() function defined anywhere.
> > > 
> > 
> > Damn you win :(
> 
> I'm pretty sure you can bribe maintainers to fix commit message typos up for you
> :-)

Beersi on me Stewart :)


> 
> Reviewed-by: Alistair Popple <alistair@popple.id.au>
> 

Thanks.

> > > > > > > > as busy.
> > > > > > > > 
> > > > > > > > Signed-off-by: Cyril Bur <cyril.bur@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.
> > > > > 
> > > > > Haha. Better get a grind stone out then! I don't think it's a big deal so long
> > > > > as it isn't preventing simultaneous access to different flash chips. Which I
> > > > > assume is what the next patch in the series fixes?
> > > > > 
> > > > 
> > > > The next patch does improve on the current situation yes.
> > > > 
> > > > > > > 
> > > > > > > >  		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)
> > > 
> > > Are you sure you didn't mean flash_releasei() here?
> > > 
> > 
> > Pretty sure, but since I didn't declare it... did I? who am I?
> > 
> > > > > > > >  {
> > > > > > > >  	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,
> > > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > 
> > > > > 
> > > 
> > > 
> 
>

Patch

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))
 		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,