On Mon, Jul 30, 2018 at 12:29:06AM -0700, Christoph Hellwig wrote: > On Tue, Jul 24, 2018 at 02:30:52PM +0200, Karel Zak wrote: > > > >> -#if defined(__alpha__) || defined(__sparc__) > > > >> - /* > > > >> - * Not all alpha kernels reject RTC_UIE_ON, but probably > > > >> - * they should. > > > >> - */ > > > >> - rc = -1; > > > >> - errno = EINVAL; > > > >> -#else > > > >> rc = ioctl(rtc_fd, RTC_UIE_ON, 0); > > > >> -#endif > > > > > > > > Are you sure you want to change the code path for SPARC? > > > > > > Hi Berny, > > > > > > After reading linux kernel code, and trying to find references where sparc > > > condition might be originating I could not find good reason why sparc should > > > hard code RTC_UIE_ON not to be supported. There is message from 2005 about > > > chrony having issue with sparc RTC_UIE_ON, but later in that thread Dave > > > Miller tells there is no problem with rtc and sparc. I am fairly confident > > > ioctl(rtc_fd, RTC_UIE_ON, 0) will either do the right thing or return -1. > > > > > > Reference: https://lists.debian.org/debian-sparc/2005/04/msg00115.html > > > > Maybe the best would be to postpone this to v2.34 and in v2.33 just > > inform about this plan. Frankly, I'm not sure where and what is > > supported for alpha... > > Note that for modern kernel RTC support uses a lot more common code > so some of the old quirks should be dead. It might be a better idea > to approach this as dropping support for long obsolete kernels on > not common architectures, although that might require a bit more > research and testing. Yes. Anyway, it seems the patch is wrong. The current kernel still supports RTC_EPOCH_* ioctls, so we need hwclock --{set,get,}epoch commands. The question is RTC_UIE_ON. It seems hwclock tries to be more smart than kernel: #if defined(__alpha__) || defined(__sparc__) /* * Not all alpha kernels reject RTC_UIE_ON, but probably * they should. */ rc = -1; errno = EINVAL; #else rc = ioctl(rtc_fd, RTC_UIE_ON, 0); #endif I don't like such code in userspace. It's kernel business o make decisions about supported/unsupported features... It would be probably better to remove this #ifdef and call the ioctl everywhere and assume EINVAL from kernel rather than play this ifdef userspace game. Karel -- Karel Zak <kzak@xxxxxxxxxx> http://karelzak.blogspot.com -- To unsubscribe from this list: send the line "unsubscribe util-linux" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
