Re: [PATCH 01/28] OMAP3: PM: GPMC context save/restore

[Nishanth Menon <nm@ti.com>]

Kevin Hilman had written, on 10/05/2009 12:17 PM, the following:

>>> +#ifdef CONFIG_ARCH_OMAP3
>>>   
>> apologies if this is a dumb question - why is this under #ifdef -> if
>> the save save restore structures are not under #ifdef?
> 
> Not a dumb question, good catch.  I'll move the struct inside
> this #ifdef.
Thanks.. would you consider the theoretical issue below?
> 
>>> +void omap3_gpmc_save_context()
>>> +{
>>> +	int i;
>>> +	gpmc_context.sysconfig = gpmc_read_reg(GPMC_SYSCONFIG);
>>> +	gpmc_context.irqenable = gpmc_read_reg(GPMC_IRQENABLE);
>>> +	gpmc_context.timeout_ctrl = gpmc_read_reg(GPMC_TIMEOUT_CONTROL);
>>> +	gpmc_context.config = gpmc_read_reg(GPMC_CONFIG);
>>> +	gpmc_context.prefetch_config1 = gpmc_read_reg(GPMC_PREFETCH_CONFIG1);
>>> +	gpmc_context.prefetch_config2 = gpmc_read_reg(GPMC_PREFETCH_CONFIG2);
>>> +	gpmc_context.prefetch_control = gpmc_read_reg(GPMC_PREFETCH_CONTROL);
>>> +	for (i = 0; i < GPMC_CS_NUM; i++) {
>>> +		gpmc_context.cs_context[i].is_valid =
>>> +				(gpmc_cs_read_reg(i, GPMC_CS_CONFIG7))
>>> +							& GPMC_CONFIG7_CSVALID;
>>> +		if (gpmc_context.cs_context[i].is_valid) {
>>> +			gpmc_context.cs_context[i].config1 =
>>> +				gpmc_cs_read_reg(i, GPMC_CS_CONFIG1);
>>> +			gpmc_context.cs_context[i].config2 =
>>> +				gpmc_cs_read_reg(i, GPMC_CS_CONFIG2);
>>> +			gpmc_context.cs_context[i].config3 =
>>> +				gpmc_cs_read_reg(i, GPMC_CS_CONFIG3);
>>> +			gpmc_context.cs_context[i].config4 =
>>> +				gpmc_cs_read_reg(i, GPMC_CS_CONFIG4);
>>> +			gpmc_context.cs_context[i].config5 =
>>> +				gpmc_cs_read_reg(i, GPMC_CS_CONFIG5);
>>> +			gpmc_context.cs_context[i].config6 =
>>> +				gpmc_cs_read_reg(i, GPMC_CS_CONFIG6);
>>> +			gpmc_context.cs_context[i].config7 =
>>> +				gpmc_cs_read_reg(i, GPMC_CS_CONFIG7);
>>> +		}
>>>   
>> here is a theoretical bug:
>> 1: GPMC, 1, 2, 3 4 5 configured 6 7 not configured.
>> 2. Save and restore 1: save and restore variables which are static will
>> contain 1-5 and not 6&7
>> 3. next I disable 2,3
>> 3. save will save 1,4,5 BUT my variable will contain 1,2,3,4,5 ->
>> restore will rename 2,3 (which I did not intend)..
I wonder if there should be an else with a memset to 0..

-- 
Regards,
Nishanth Menon