| Message ID | 20150119061243.22912.84269.stgit@localhost.localdomain |
|---|---|
| State | Accepted |
| Headers | show |
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@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? Cheers, Jeremy
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@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.. - Neelesh > > Cheers, > > > Jeremy >
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@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. Regards, Alistair > - Neelesh > > > Cheers, > > > > > > Jeremy
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@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
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);
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@linux.vnet.ibm.com> --- hw/fsp/fsp-rtc.c | 1 + 1 file changed, 1 insertion(+)