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

[Madhavan Srinivasan]

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;
>> +}
>> +
>