Message ID | 20160413060916.GA21184@localhost |
---|---|
State | Awaiting Upstream, archived |
Delegated to: | David Miller |
Headers | show |
> 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.
> '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
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 */
'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(-) -- 2.8.0.rc3.226.g39d4020