Skip to contentSkip to conversation

prism54: isl_38xx: Replace 'struct timeval'

PATCH Applied Tina Ruchandani 6 participants 11 messages Apr 13, 2016 – Apr 26, 2016

Subsystems (3)
Networking Network drivers P54 wireless driver
lore
Share & tools
lore

Tina Ruchandani

author
#0 lore raw

PATCH v3 1/1 prism54: isl_38xx: Replace 'struct timeval'

'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 <[email protected]>
Suggested-by Arnd Bergmann <[email protected]>

--
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(-)
drivers/net/wireless/intersil/prism54/isl_38xx.c +18 -17
index 333c1a2..6700387 100644
@@ -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(&current_time);
-		DEBUG(SHOW_TRACING, "%08li.%08li Device wakeup triggered\n",
-		      current_time.tv_sec, (long)current_time.tv_usec);
+		ktime_get_real_ts64(&current_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(&current_time);
+			ktime_get_real_ts64(&current_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(&current_time);
+			ktime_get_real_ts64(&current_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(&current_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(&current_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

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#1 lore raw
Quoted 3 lines

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
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038

#2 lore raw
Quoted 7 lines · Johannes Berg
Quoted 3 lines

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

Quoted 6 lines

johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038

#3 lore raw
In reply to Johannes Berg
Quoted 6 lines · On Wednesday 13 April 2016 10:38:26 Johannes Berg wrote:
Quoted 2 lines

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
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038

#4 lore raw
Quoted 12 lines · Arnd Bergmann
Quoted 8 lines · On Wednesday 13 April 2016 10:38:26 Johannes Berg wrote:
Quoted 4 lines

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
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038

#5 lore raw
Quoted 3 lines · 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
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038

#6 lore raw
Quoted 11 lines · Arnd Bergmann
Quoted 5 lines · 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
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038

#7 lore raw
Quoted 12 lines · Johannes Berg
Quoted 10 lines · Arnd Bergmann
Quoted 4 lines · 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.

Quoted 4 lines

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.

Signature · 5 lines
Kalle Valo
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038
#8 lore raw
Quoted 15 lines · Kalle Valo
Quoted 12 lines · Johannes Berg
Quoted 10 lines · Arnd Bergmann
Quoted 4 lines · 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.

Quoted 8 lines
Quoted 4 lines

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
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038

Applied
#9 lore raw
In reply to Tina Ruchandani
Quoted 10 lines

'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.

Quoted 2 lines
Signed-off-by Tina Ruchandani <[email protected]>
Suggested-by Arnd Bergmann <[email protected]>

Thanks, applied to wireless-drivers-next.git.

Kalle Valo

Applied
#10 lore raw
In reply to Tina Ruchandani
Quoted 10 lines

'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.

Quoted 2 lines
Signed-off-by Tina Ruchandani <[email protected]>
Suggested-by Arnd Bergmann <[email protected]>

Thanks, applied to wireless-drivers-next.git.

Kalle Valo
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038