'struct timeval' uses a 32-bit seconds field which will overflow in year 2038 and beyond. This patch is part of a larger effort to remove all instances of 'struct timeval' from the kernel and replace them with 64-bit timekeeping variables. The patch also fixes the debug printf specifier to avoid the seconds value being truncated. The patch was build-tested / debugged by removing the "if VERBOSE > SHOW_ERROR_MESSAGES" guards.
Signed-off-by: Tina Ruchandani ruchandani.tina@gmail.com Suggested-by: Arnd Bergmann arnd@arndb.de -- Changes in v3: Fix commit message Changes in v2: Changed printf specifier as suggested by Arnd Bergmann to avoid truncation. --- drivers/net/wireless/intersil/prism54/isl_38xx.c | 35 ++++++++++++------------ 1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/intersil/prism54/isl_38xx.c b/drivers/net/wireless/intersil/prism54/isl_38xx.c index 333c1a2..6700387 100644 --- a/drivers/net/wireless/intersil/prism54/isl_38xx.c +++ b/drivers/net/wireless/intersil/prism54/isl_38xx.c @@ -19,6 +19,7 @@ #include <linux/module.h> #include <linux/types.h> #include <linux/delay.h> +#include <linux/ktime.h>
#include <asm/uaccess.h> #include <asm/io.h> @@ -113,7 +114,7 @@ isl38xx_trigger_device(int asleep, void __iomem *device_base)
#if VERBOSE > SHOW_ERROR_MESSAGES u32 counter = 0; - struct timeval current_time; + struct timespec64 current_ts64; DEBUG(SHOW_FUNCTION_CALLS, "isl38xx trigger device\n"); #endif
@@ -121,22 +122,22 @@ isl38xx_trigger_device(int asleep, void __iomem *device_base) if (asleep) { /* device is in powersave, trigger the device for wakeup */ #if VERBOSE > SHOW_ERROR_MESSAGES - do_gettimeofday(¤t_time); - DEBUG(SHOW_TRACING, "%08li.%08li Device wakeup triggered\n", - current_time.tv_sec, (long)current_time.tv_usec); + ktime_get_real_ts64(¤t_ts64); + DEBUG(SHOW_TRACING, "%lld.%09ld Device wakeup triggered\n", + (s64)current_ts64.tv_sec, current_ts64.tv_nsec);
- DEBUG(SHOW_TRACING, "%08li.%08li Device register read %08x\n", - current_time.tv_sec, (long)current_time.tv_usec, + DEBUG(SHOW_TRACING, "%lld.%09ld Device register read %08x\n", + (s64)current_ts64.tv_sec, current_ts64.tv_nsec, readl(device_base + ISL38XX_CTRL_STAT_REG)); #endif
reg = readl(device_base + ISL38XX_INT_IDENT_REG); if (reg == 0xabadface) { #if VERBOSE > SHOW_ERROR_MESSAGES - do_gettimeofday(¤t_time); + ktime_get_real_ts64(¤t_ts64); DEBUG(SHOW_TRACING, - "%08li.%08li Device register abadface\n", - current_time.tv_sec, (long)current_time.tv_usec); + "%lld.%09ld Device register abadface\n", + (s64)current_ts64.tv_sec, current_ts64.tv_nsec); #endif /* read the Device Status Register until Sleepmode bit is set */ while (reg = readl(device_base + ISL38XX_CTRL_STAT_REG), @@ -149,13 +150,13 @@ isl38xx_trigger_device(int asleep, void __iomem *device_base)
#if VERBOSE > SHOW_ERROR_MESSAGES DEBUG(SHOW_TRACING, - "%08li.%08li Device register read %08x\n", - current_time.tv_sec, (long)current_time.tv_usec, + "%lld.%09ld Device register read %08x\n", + (s64)current_ts64.tv_sec, current_ts64.tv_nsec, readl(device_base + ISL38XX_CTRL_STAT_REG)); - do_gettimeofday(¤t_time); + ktime_get_real_ts64(¤t_ts64); DEBUG(SHOW_TRACING, - "%08li.%08li Device asleep counter %i\n", - current_time.tv_sec, (long)current_time.tv_usec, + "%lld.%09ld Device asleep counter %i\n", + (s64)current_ts64.tv_sec, current_ts64.tv_nsec, counter); #endif } @@ -168,9 +169,9 @@ isl38xx_trigger_device(int asleep, void __iomem *device_base)
/* perform another read on the Device Status Register */ reg = readl(device_base + ISL38XX_CTRL_STAT_REG); - do_gettimeofday(¤t_time); - DEBUG(SHOW_TRACING, "%08li.%08li Device register read %08x\n", - current_time.tv_sec, (long)current_time.tv_usec, reg); + ktime_get_real_ts64(¤t_ts64); + DEBUG(SHOW_TRACING, "%lld.%00ld Device register read %08x\n", + (s64)current_ts64.tv_sec, current_ts64.tv_nsec, reg); #endif } else { /* device is (still) awake */ -- 2.8.0.rc3.226.g39d4020
The patch was build-tested / debugged by removing the "if VERBOSE > SHOW_ERROR_MESSAGES" guards.
Stands to reason that we should just remove the (more or less) dead code, since I don't think anyone really ever touches this driver any more or will ever again ...
johannes
On 13-4-2016 10:38, Johannes Berg wrote:
The patch was build-tested / debugged by removing the "if VERBOSE > SHOW_ERROR_MESSAGES" guards.
Stands to reason that we should just remove the (more or less) dead code, since I don't think anyone really ever touches this driver any more or will ever again ...
It does bring back memories from my Intersil/Globespan/Conexant day(s), but not sentimental enough to touch the prism54 driver.
Gr. AvS
johannes
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 13 April 2016 10:38:26 Johannes Berg wrote:
The patch was build-tested / debugged by removing the "if VERBOSE > SHOW_ERROR_MESSAGES" guards.
Stands to reason that we should just remove the (more or less) dead code, since I don't think anyone really ever touches this driver any more or will ever again ...
Do you mean removing all DEBUG() statements from the driver, or removing the entire driver?
Arnd
On Sun, 2016-04-17 at 01:34 +0200, Arnd Bergmann wrote:
On Wednesday 13 April 2016 10:38:26 Johannes Berg wrote:
The patch was build-tested / debugged by removing the "if VERBOSE > SHOW_ERROR_MESSAGES" guards.
Stands to reason that we should just remove the (more or less) dead code, since I don't think anyone really ever touches this driver any more or will ever again ...
Do you mean removing all DEBUG() statements from the driver, or removing the entire driver?
We tried removing the driver once, since p54 supposedly drives the same hardware, but some people had certain use cases that didn't work there, apparently.
I was thinking more restrictively of just the stuff that can't even be built without modifying the sources - like the "#if VERBOSE" thing.
johannes
On Sunday 17 April 2016 14:42:33 Johannes Berg wrote:
I was thinking more restrictively of just the stuff that can't even be built without modifying the sources - like the "#if VERBOSE" thing.
All the DEBUG() statements are inside of this kind of check, so if we remove the #ifdefs, it would be logical to remove the rest of the debugging infrastructure (DEBUG() macros, SHOW_*, pc_debug, maybe more) as well.
Arnd
On Mon, 2016-04-18 at 00:10 +0200, Arnd Bergmann wrote:
On Sunday 17 April 2016 14:42:33 Johannes Berg wrote:
I was thinking more restrictively of just the stuff that can't even be built without modifying the sources - like the "#if VERBOSE" thing.
All the DEBUG() statements are inside of this kind of check, so if we remove the #ifdefs, it would be logical to remove the rest of the debugging infrastructure (DEBUG() macros, SHOW_*, pc_debug, maybe more) as well.
Seems reasonable.
Maybe we should Cc the maintainer, but I suspect that since the driver is marked Obsolete anyway Luis won't care either :)
johannes
Johannes Berg johannes@sipsolutions.net writes:
On Mon, 2016-04-18 at 00:10 +0200, Arnd Bergmann wrote:
On Sunday 17 April 2016 14:42:33 Johannes Berg wrote:
I was thinking more restrictively of just the stuff that can't even be built without modifying the sources - like the "#if VERBOSE" thing.
All the DEBUG() statements are inside of this kind of check, so if we remove the #ifdefs, it would be logical to remove the rest of the debugging infrastructure (DEBUG() macros, SHOW_*, pc_debug, maybe more) as well.
Seems reasonable.
Maybe we should Cc the maintainer, but I suspect that since the driver is marked Obsolete anyway Luis won't care either :)
I'm planning to apply this patch anyway, the debugging infrastructure removal can be a followup patch. But please let me know if I should drop this instead.
On Apr 22, 2016 8:09 PM, "Kalle Valo" kvalo@codeaurora.org wrote:
Johannes Berg johannes@sipsolutions.net writes:
On Mon, 2016-04-18 at 00:10 +0200, Arnd Bergmann wrote:
On Sunday 17 April 2016 14:42:33 Johannes Berg wrote:
I was thinking more restrictively of just the stuff that can't even be built without modifying the sources - like the "#if VERBOSE" thing.
All the DEBUG() statements are inside of this kind of check, so if we remove the #ifdefs, it would be logical to remove the rest of the debugging infrastructure (DEBUG() macros, SHOW_*, pc_debug, maybe more) as well.
Seems reasonable.
Maybe we should Cc the maintainer, but I suspect that since the driver is marked Obsolete anyway Luis won't care either :)
I'm planning to apply this patch anyway, the debugging infrastructure removal can be a followup patch. But please let me know if I should drop this instead.
I'd say let's bury the driver now. We have a process to stage large chunks of poo now through staging, let's use that to shaft prism54 there now and give it a few cycles to let people call bloody murder before finally removing. We need to do better at deleting ancient legacy shit. I'll stand behind this one. Sue me if needed.
Luis
'struct timeval' uses a 32-bit seconds field which will overflow in year 2038 and beyond. This patch is part of a larger effort to remove all instances of 'struct timeval' from the kernel and replace them with 64-bit timekeeping variables. The patch also fixes the debug printf specifier to avoid the seconds value being truncated. The patch was build-tested / debugged by removing the "if VERBOSE > SHOW_ERROR_MESSAGES" guards.
Signed-off-by: Tina Ruchandani ruchandani.tina@gmail.com Suggested-by: Arnd Bergmann arnd@arndb.de
Thanks, applied to wireless-drivers-next.git.
Kalle Valo
-
Arend Van Spriel -
Arnd Bergmann -
Johannes Berg -
Kalle Valo -
Luis R. Rodriguez -
Tina Ruchandani