[Skiboot] [PATCH V4 4/6] opal/errorlog : Generalizes the error log read path

[Vasant Hegde]

On 07/26/2016 11:15 AM, Mukesh Ojha wrote:
> Introduces platform hooks for error log info, read, ack, and resend pending logs
> by adding four new function pointers in the platform structure.
>
> Adds opal generic function for read, info, ack and resend pending logs as
> callback, which will be called from host.
>
> Moving the opal call registration from fsp-elog-read.c to core/errorlog.c
> and the calling the fsp specific function via platform hooks, which will
> be initialised once the platform is known, otherwise it will be NULL.
> Declarations of fsp specific routine are kept in fsp-elog.h .
>
> Signed-off-by: Mukesh Ojha <mukesh02 at linux.vnet.ibm.com>
> ---
> Changes in V4:
>   - Changes are rebased on master.
>   - Changes done for core/elog-host moved to core/errorlog.c .
>
> Changes in V3:
>   - No changes.
>
> Changes in V2:
>   - Separates the Bug solve part to patch 3/6.
>
>   core/errorlog.c             | 61 ++++++++++++++++++++++++++++++++-------------
>   hw/fsp/fsp-elog-read.c      | 29 +++------------------
>   include/errorlog.h          |  6 -----
>   include/fsp-elog.h          |  7 ++++++
>   include/platform.h          | 23 +++++++++++++++++
>   platforms/ibm-fsp/apollo.c  |  7 +++++-
>   platforms/ibm-fsp/firenze.c |  7 +++++-
>   7 files changed, 90 insertions(+), 50 deletions(-)
>
> diff --git a/core/errorlog.c b/core/errorlog.c
> index 12dc7a4..5648433 100644
> --- a/core/errorlog.c
> +++ b/core/errorlog.c
> @@ -230,10 +230,11 @@ static inline void opal_elog_write_set_head_state(enum elog_head_state state)
>   	elog_write_to_host_head_state = state;
>   }
>
> -bool opal_elog_info(uint64_t *opal_elog_id, uint64_t *opal_elog_size)
> +static int opal_elog_info(uint64_t *opal_elog_id, uint64_t *opal_elog_size,
> +				uint64_t *elog_type)
>   {
>   	struct errorlog *head;
> -	bool rc = false;
> +	int rc = OPAL_SUCCESS;
>
>   	lock(&elog_write_to_host_lock);
>   	if (elog_write_to_host_head_state == ELOG_STATE_FETCHED_DATA) {
> @@ -249,16 +250,23 @@ bool opal_elog_info(uint64_t *opal_elog_id, uint64_t *opal_elog_size)
>   			prlog(PR_ERR, "%s:Inconsistent internal list state !\n",
>   					__func__);
>   			opal_elog_write_set_head_state(ELOG_STATE_NONE);
> +			unlock(&elog_write_to_host_lock);
> +			return OPAL_INTERNAL_ERROR;
>   		} else {
>   			*opal_elog_id = head->plid;
>   			*opal_elog_size = head->log_size;
>   			opal_elog_write_set_head_state(ELOG_STATE_FETCHED_INFO);
> -			rc = true;
> +			unlock(&elog_write_to_host_lock);
> +			return rc;
>   		}
>   	}
>
>   	unlock(&elog_write_to_host_lock);
> -	return rc;
> +	if (platform.elog_info)
> +		return platform.elog_info(opal_elog_id, opal_elog_size,
> +				elog_type);
> +
> +	return OPAL_PARAMETER;
>   }
>
>   static void opal_commit_elog_in_host(void)
> @@ -279,11 +287,11 @@ static void opal_commit_elog_in_host(void)
>   	unlock(&elog_write_to_host_lock);
>   }
>
> -bool opal_elog_read(uint64_t *buffer, uint64_t opal_elog_size,
> +static int opal_elog_read(uint64_t *buffer, uint64_t opal_elog_size,
>   		uint64_t opal_elog_id)
>   {
>   	struct errorlog *log_data;
> -	bool rc = false;
> +	int rc = OPAL_SUCCESS;
>
>   	lock(&elog_write_to_host_lock);
>   	if (elog_write_to_host_head_state == ELOG_STATE_FETCHED_INFO) {
> @@ -292,13 +300,13 @@ bool opal_elog_read(uint64_t *buffer, uint64_t opal_elog_size,
>   		if (!log_data) {
>   			opal_elog_write_set_head_state(ELOG_STATE_NONE);
>   			unlock(&elog_write_to_host_lock);
> -			return rc;
> +			return OPAL_INTERNAL_ERROR;
>   		}
>
>   		if ((opal_elog_id != log_data->plid) &&
>   				(opal_elog_size != log_data->log_size)) {
>   			unlock(&elog_write_to_host_lock);
> -			return rc;
> +			return OPAL_PARAMETER;
>   		}
>
>   		memcpy((void *)buffer, elog_write_to_host_buffer,
> @@ -306,17 +314,21 @@ bool opal_elog_read(uint64_t *buffer, uint64_t opal_elog_size,
>   		list_del(&log_data->link);
>   		list_add(&elog_write_to_host_processed, &log_data->link);
>   		opal_elog_write_set_head_state(ELOG_STATE_NONE);
> -		rc = true;
> +		unlock(&elog_write_to_host_lock);
> +		opal_commit_elog_in_host();
> +		return rc;
>   	}
>
>   	unlock(&elog_write_to_host_lock);
> -	opal_commit_elog_in_host();
> -	return rc;
> +	if (platform.elog_read)
> +		return platform.elog_read(buffer, opal_elog_size, opal_elog_id);
> +
> +	return OPAL_PARAMETER;
>   }
>
> -bool opal_elog_ack(uint64_t ack_id)
> +static int opal_elog_ack(uint64_t ack_id)
>   {
> -	bool rc = false;
> +	int rc = OPAL_SUCCESS;
>   	struct errorlog *log_data;
>   	struct errorlog *record, *next_record;
>
> @@ -329,7 +341,8 @@ bool opal_elog_ack(uint64_t ack_id)
>
>   			list_del(&record->link);
>   			opal_elog_complete(record, true);
> -			rc = true;
> +			unlock(&elog_write_to_host_lock);
> +			return rc;
>   		}
>   	}
>
> @@ -346,7 +359,6 @@ bool opal_elog_ack(uint64_t ack_id)
>
>   			list_del(&record->link);
>   			opal_elog_complete(record, true);
> -			rc = true;
>   			unlock(&elog_write_to_host_lock);
>   			opal_commit_elog_in_host();
>   			return rc;
> @@ -354,14 +366,20 @@ bool opal_elog_ack(uint64_t ack_id)
>   	}
>
>   	unlock(&elog_write_to_host_lock);
> -	return rc;
> +	if (platform.elog_ack)
> +		return platform.elog_ack(ack_id);
> +
> +	return OPAL_PARAMETER;
>   }
>
> -void opal_resend_pending_logs(void)
> +static void opal_resend_pending_logs(void)
>   {
>   	struct errorlog *record;
>
>   	lock(&elog_write_to_host_lock);
> +	elog_enabled = true;
> +	unlock(&elog_write_to_host_lock);
> +	lock(&elog_write_to_host_lock);

hrm .. unlock followed by lock!

>   	while (!list_empty(&elog_write_to_host_processed)) {
>   		record = list_pop(&elog_write_to_host_processed,
>   				struct errorlog, link);
> @@ -371,6 +389,10 @@ void opal_resend_pending_logs(void)
>   	opal_elog_write_set_head_state(ELOG_STATE_NONE);
>   	unlock(&elog_write_to_host_lock);
>   	opal_commit_elog_in_host();
> +	if (platform.resend_pending_elogs)
> +		platform.resend_pending_elogs();
> +
> +	return;

You don't need return statement for void function.

-Vasant