[RESEND,v1,08/11] ibm-fsp/firenze: Nest Alink unit support

Patch adds support for Nest Alink unit. Alinks connects sockets.

Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
 hw/nest.c      | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 include/nest.h |  3 +++
 2 files changed, 50 insertions(+)

On Sun, 2015-07-05 at 22:25 +0530, Madhavan Srinivasan wrote:
> diff --git a/hw/nest.c b/hw/nest.c
> index bc87ec0..df93601 100644
> --- a/hw/nest.c
> +++ b/hw/nest.c
> @@ -151,6 +151,49 @@ int dt_create_ima_chip_powerbus_type( struct dt_node *ima,
>  	return 0;
>  }
>  
> +int dt_create_ima_chip_alink_event(struct dt_node *ima,
> +					int idx, u32 offset)
> +{
> +	char ev_name[MAX_NAME_SIZE];
> +	const char *unit = "MiB", *scale = "7.629e-6";
> +
> +	memset((void *)ev_name, '\0', MAX_NAME_SIZE);
> +	snprintf(ev_name, MAX_NAME_SIZE, "Alink%d", idx);

No need to memset, snprintf will add a trailing null for you.

> +	dt_add_property_cells(ima, ev_name, offset);
> +
> +	memset((void *)ev_name, '\0', MAX_NAME_SIZE);
> +	snprintf(ev_name, MAX_NAME_SIZE, "unit.Alink%d.unit", idx);

Again.

> +	dt_add_property_string(ima, ev_name, unit);
> +
> +	memset((void *)ev_name, '\0', MAX_NAME_SIZE);
> +	snprintf(ev_name, MAX_NAME_SIZE, "scale.Alink%d.scale", idx);

Again.

This code seems to be very similar to patch 7. Could you merge
dt_create_ima_chip_powerbus_event and dt_create_ima_chip_alink_event?

> +	dt_add_property_string(ima, ev_name, scale);
> +
> +	return 0;
> +}
> +
> +int dt_create_ima_chip_alink_type( struct dt_node *ima,
> +			struct ima_catalogue_group_data *gptr, char *name)
> +{
> +	struct dt_node *type;
> +	int idx;
> +	u32 offset;
> +
> +	type = dt_new(ima, name);
> +	if (!type) {
> +		prlog(PR_DEBUG, "ima %s type creation failed \n", name);
> +		return -1;
> +	}
> +
> +	dt_add_property_string(type, "device_type", "nest-ima-unit");
> +	for(idx=0;idx < 3; idx++) {

we normally space things like this, and there's no harm in using i for
your counter:

 for (i = 0; i < 3; i++)

> +		offset = get_chip_event_offset(gptr->event_index[idx]);
> +		dt_create_ima_chip_alink_event(type, idx, offset);
> +	}
> +
> +	return 0;
> +}
> +

On Thursday 16 July 2015 07:23 AM, Joel Stanley wrote:
> On Sun, 2015-07-05 at 22:25 +0530, Madhavan Srinivasan wrote:
>> diff --git a/hw/nest.c b/hw/nest.c
>> index bc87ec0..df93601 100644
>> --- a/hw/nest.c
>> +++ b/hw/nest.c
>> @@ -151,6 +151,49 @@ int dt_create_ima_chip_powerbus_type( struct dt_node *ima,
>>  	return 0;
>>  }
>>  
>> +int dt_create_ima_chip_alink_event(struct dt_node *ima,
>> +					int idx, u32 offset)
>> +{
>> +	char ev_name[MAX_NAME_SIZE];
>> +	const char *unit = "MiB", *scale = "7.629e-6";
>> +
>> +	memset((void *)ev_name, '\0', MAX_NAME_SIZE);
>> +	snprintf(ev_name, MAX_NAME_SIZE, "Alink%d", idx);
> No need to memset, snprintf will add a trailing null for you.
>
>> +	dt_add_property_cells(ima, ev_name, offset);
>> +
>> +	memset((void *)ev_name, '\0', MAX_NAME_SIZE);
>> +	snprintf(ev_name, MAX_NAME_SIZE, "unit.Alink%d.unit", idx);
> Again.
>
>> +	dt_add_property_string(ima, ev_name, unit);
>> +
>> +	memset((void *)ev_name, '\0', MAX_NAME_SIZE);
>> +	snprintf(ev_name, MAX_NAME_SIZE, "scale.Alink%d.scale", idx);
> Again.
>
> This code seems to be very similar to patch 7. Could you merge
> dt_create_ima_chip_powerbus_event and dt_create_ima_chip_alink_event?

Will remove the memset.
No. would prefer to have separate function that way,
it is easier to handle changes specific to that unit in
the meta-data file.

>> +	dt_add_property_string(ima, ev_name, scale);
>> +
>> +	return 0;
>> +}
>> +
>> +int dt_create_ima_chip_alink_type( struct dt_node *ima,
>> +			struct ima_catalogue_group_data *gptr, char *name)
>> +{
>> +	struct dt_node *type;
>> +	int idx;
>> +	u32 offset;
>> +
>> +	type = dt_new(ima, name);
>> +	if (!type) {
>> +		prlog(PR_DEBUG, "ima %s type creation failed \n", name);
>> +		return -1;
>> +	}
>> +
>> +	dt_add_property_string(type, "device_type", "nest-ima-unit");
>> +	for(idx=0;idx < 3; idx++) {
> we normally space things like this, and there's no harm in using i for
> your counter:

Will fix the spacing. Thanks

>  for (i = 0; i < 3; i++)
>
>> +		offset = get_chip_event_offset(gptr->event_index[idx]);
>> +		dt_create_ima_chip_alink_event(type, idx, offset);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>