On 12/12/2012 09:21 PM, Jason Gunthorpe wrote:
On Wed, Dec 12, 2012 at 04:18:31PM -0800, John Stultz wrote:
I do, although again, in the case where the arch specific
implementation is "better", we end up losing granularity (s390 is
the specific example I'm thinking of), since this prefers the RTC
implementation over the arch specific one. So maybe I'd suggest
switching it so we prefer the arch specific one, and then remove the
arch specific implementations where they are inferior to the RTC
one.
Unfortunately I put rtc_update_persistent_clock first because it can
have sensible error reporting. update_persistent_clock will return 0
if there is no RTC device available, or if the RTC was successfully
updated.
Hrm.. Maybe update_persistent_clock() should be changed to return an error?
I can make rtc_update_persistent_clock return -ENODEV.
As long as we have a clear iterative path forward, with a solution
for the cases where the arch method is actually preferred, I think
it sounds like a reasonable approach.
I think it is fine to leave it as a configure option, archs can
default it to yes when it is appropriate for them.
A quick scan through the 3.7 tree for update_persistent_clock::
alpha - single non class RTC clock scheme
cris - printk's and does nothing
mips - weak function rtc_mips_set_time, all implementations are
non class rtc
mn10300 - single non class RTC clock scheme
powerpc - indirects through ppc_md.set_rtc_time, all implementations
do not use class RTC
sh - indirects through rtc_sh_set_time, two implementations, neither
use class RTC
sparc - Seems to be class rtc converted. Already implements this
patch's functionality in an arch specific file
x86 - All the functions under the set_wallclock indirection seem
to be non class RTC cases
No other arches seem to have update_persistent_clock in them.
I think the s390 functionality you are refering to is in its
read_persistant_clock, which looks like it has ns resolution.
That is also fine because s390 does not use class rtc so there is no
duplicate path to the 'tod' code either through
rtc_update_persistent_clock or through rtc_hctosys.
Basically, as far as I can tell, if rtc_update_persistent_clock
succeeds then update_persistent_clock is a nop. I can't find any case
where *both* could succeed. Thus trying rtc_update_persistent_clock
first is OK.
Ok then. I think part of my confusion is that on the
read_persistent_clock/rtc_hctosys side of things, we are careful to
prioritize the read_persistent_clock implementation (since it has the
additional requirement to be safe to call in irq context, it allows us
to update the system time at resume earlier, avoiding time error). So I
guess I just naturally expect the same prioritization on the write side.
So yea, I guess your approach will work. Although I get the suspicion it
will require further cleanups down the road (just to help make the
various arches more consistent).
Want to resend with the changes you suggested, and I'll take another
look at it?
thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
