[Skiboot] [PATCH 2/2] rtc: rtc tod state need to be updated on success

[Neelesh Gupta]

On 01/20/2015 12:12 PM, Alistair Popple wrote:
> On Tue, 20 Jan 2015 11:43:56 Neelesh Gupta wrote:
>> On 01/20/2015 05:45 AM, Jeremy Kerr wrote:
>>> Hi Neelesh & Alistair,
>>>
>>>> The OPAL rtc read interface currently fails as the tod state is
>>>> not getting updated in the callback. The patch fixes this issue.
>>>>
>>>> Signed-off-by: Neelesh Gupta <neelegup at linux.vnet.ibm.com>
>>>> ---
>>>>
>>>>    hw/fsp/fsp-rtc.c |    1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/hw/fsp/fsp-rtc.c b/hw/fsp/fsp-rtc.c
>>>> index f60d2f3..b83bb2e 100644
>>>> --- a/hw/fsp/fsp-rtc.c
>>>> +++ b/hw/fsp/fsp-rtc.c
>>>> @@ -158,6 +158,7 @@ static void fsp_rtc_process_read(struct fsp_msg
>>>> *read_resp)>>
>>>>    	case FSP_STATUS_SUCCESS:
>>>>    		/* Save the read RTC value in our cache */
>>>>
>>>> +		rtc_tod_state = RTC_TOD_VALID;
>>>>
>>>>    		datetime_to_tm(read_resp->data.words[0],
>>>>    		
>>>>    			       (u64) read_resp->data.words[1] << 32, &tm);
>>>>    		
>>>>    		rtc_cache_update(&tm);
>>> We seem to have this state in the rtc core code too
>>> (rtc_tod_cache.valid) - do we need both?
>> 'rtc_tod_state' is required indeed, it is more discrete while
>> 'rtc_tod_cache.valid'
>> is a bool. So, I think both are required, but I also see that
>> 'rtc_tod_cahce.valid'
>> doesn't get updated, it maintains a 'true' value all the time.. probably
>> that
>> should be fixed separately..
> rtc_tod_state is really tracking the state of the OPAL call rather than the
> state of the RTC cache. For example it is used to make sure that the RTC cache
> is updated from the FSP every time OPAL_RTC_READ is called.
>
> rtc_tod_cache.valid is used to track if a valid time has been written into
> rtc_tod_cache.tm, which is unnecessary because it could just assume validity
> if rtc_tod_cache.tm != 0.
>
> My eventual plan was to make a platform hook to get the current rtc value and
> implement a more fully featured cache that would keep itself updated according
> to some kind of policy rather than just updating it occasionally when
> OPAL_RTC_READ is called.

Yeah, second that. There need to be platform hook to read the rtc 
time/state and
core rtc uses that to keep its cache updated.

- Neelesh

>
> Regards,
>
> Alistair
>
>> - Neelesh
>>
>>> Cheers,
>>>
>>>
>>> Jeremy