Re: [PATCH v2] powerpc/powernv: Platform dump interface

[Vasant Hegde <hegdevasant@linux.vnet.ibm.com>]

On 02/09/2014 02:50 AM, Anton Blanchard wrote:
>
> Hi Vasant,
>
>> +static void free_dump_sg_list(struct opal_sg_list *list)
>> +{
>> +	struct opal_sg_list *sg1;
>> +	while (list) {
>> +		sg1 = list->next;
>> +		kfree(list);
>> +		list = sg1;
>> +	}
>> +	list = NULL;
>> +}
>> +
>> +/*
>> + * Build dump buffer scatter gather list
>> + */
>> +static struct opal_sg_list *dump_data_to_sglist(void)
>> +{
>> +	struct opal_sg_list *sg1, *list = NULL;
>> +	void *addr;
>> +	int64_t size;
>> +
>> +	addr = dump_record.buffer;
>> +	size = dump_record.size;
>> +
>> +	sg1 = kzalloc(PAGE_SIZE, GFP_KERNEL);
>> +	if (!sg1)
>> +		goto nomem;
>> +
>> +	list = sg1;
>> +	sg1->num_entries = 0;
>> +	while (size > 0) {
>> +		/* Translate virtual address to physical address */
>> +		sg1->entry[sg1->num_entries].data =
>> +			(void *)(vmalloc_to_pfn(addr) << PAGE_SHIFT);
>> +
>> +		if (size > PAGE_SIZE)
>> +			sg1->entry[sg1->num_entries].length =
>> PAGE_SIZE;
>> +		else
>> +			sg1->entry[sg1->num_entries].length = size;
>> +
>> +		sg1->num_entries++;
>> +		if (sg1->num_entries >= SG_ENTRIES_PER_NODE) {
>> +			sg1->next = kzalloc(PAGE_SIZE, GFP_KERNEL);
>> +			if (!sg1->next)
>> +				goto nomem;
>> +
>> +			sg1 = sg1->next;
>> +			sg1->num_entries = 0;
>> +		}
>> +		addr += PAGE_SIZE;
>> +		size -= PAGE_SIZE;
>> +	}
>> +	return list;
>> +
>> +nomem:
>> +	pr_err("%s : Failed to allocate memory\n", __func__);
>> +	free_dump_sg_list(list);
>> +	return NULL;
>> +}
>> +
>> +/*
>> + * Translate sg list address to absolute
>> + */
>> +static void sglist_to_phy_addr(struct opal_sg_list *list)
>> +{
>> +	struct opal_sg_list *sg, *next;
>> +
>> +	for (sg = list; sg; sg = next) {
>> +		next = sg->next;
>> +		/* Don't translate NULL pointer for last entry */
>> +		if (sg->next)
>> +			sg->next = (struct opal_sg_list
>> *)__pa(sg->next);
>> +		else
>> +			sg->next = NULL;
>> +
>> +		/* Convert num_entries to length */
>> +		sg->num_entries =
>> +			sg->num_entries * sizeof(struct
>> opal_sg_entry) + 16;
>> +	}
>> +}
>> +
>> +static void free_dump_data_buf(void)
>> +{
>> +	vfree(dump_record.buffer);
>> +	dump_record.size = 0;
>> +}
>

Anton,

> This looks identical to the code in opal-flash.c. Considering how
> complicated it is, can we put it somewhere common?

Thanks for the review.. Will look into it next week.

-Vasant

>
> Anton
>