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

[Alistair Popple]

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

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