Re: [PATCH v4] ARM: omap: edma: add suspend suspend/resume hooks

[Joel Fernandes <joelf@ti.com>]

On 11/07/2013 10:34 AM, Joel Fernandes wrote:
> Hi Daniel,
> 
> Thanks for your followup patch on this. It looks much better now using existing
> functions to save/restore the state.
> 
> On 10/30/2013 03:21 PM, Daniel Mack wrote:
>> This patch makes the edma driver resume correctly after suspend. Tested
>> on an AM33xx platform with cyclic audio streams and omap_hsmmc.
>>
>> All information can be reconstructed by already known runtime
>> information.
>>
>> As we now use some functions that were previously only used from __init
>> context, annotations had to be dropped.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>> ---
>> There was actually only a v3 ever, I made a mistake when formating the
>> first version of this patch. To prevent confusion though, I named this
>> one v4.
>>
>> v3 -> v4:
>> 	* dropped extra allocations, and reconstruct register values
>> 	  from already known driver states.
>>
>>
>>
>> Hi Joel, Gururaja, Balaji,
>>
>> thanks a lot for your feedback. I successfully tested this version with
>> davinci mcasp as well as omap_hsmmc. I'd appreciate another round of
>> reviews :)
>>
>>
>> Thanks,
>> Daniel
>>
>>  arch/arm/common/edma.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 79 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
> [..]
>> +static int edma_pm_resume(struct device *dev)
>> +{
>> +	int i, j;
>> +
>> +	pm_runtime_get_sync(dev);
>> +
>> +	for (j = 0; j < arch_num_cc; j++) {
>> +		struct edma *cc = edma_cc[j];
>> +
>> +		s8 (*queue_priority_mapping)[2];
>> +		s8 (*queue_tc_mapping)[2];
>> +
>> +		queue_tc_mapping = cc->info->queue_tc_mapping;
>> +		queue_priority_mapping = cc->info->queue_priority_mapping;
>> +
>> +		/* Event queue to TC mapping */
>> +		for (i = 0; queue_tc_mapping[i][0] != -1; i++)
>> +			map_queue_tc(j, queue_tc_mapping[i][0],
>> +					queue_tc_mapping[i][1]);
>> +
>> +		/* Event queue priority mapping */
>> +		for (i = 0; queue_priority_mapping[i][0] != -1; i++)
>> +			assign_priority_to_queue(j,
>> +						queue_priority_mapping[i][0],
>> +						queue_priority_mapping[i][1]);
> 
> I know ti,edma-regions property is not currently being used, but we should
> future proof this by setting up DRAE for like done in probe:
> 
>                 for (i = 0; i < info[j]->n_region; i++) {
>                         edma_write_array2(j, EDMA_DRAE, i, 0, 0x0);
>                         edma_write_array2(j, EDMA_DRAE, i, 1, 0x0);
>                         edma_write_array(j, EDMA_QRAE, i, 0x0);
>                 }

Please ignore this comment I posted earlier. That is not all required to do.
Should've looked closer. Sorry about it.

cheers,

-Joel