This patch replaces timeval with timespec64 as 32 bit 'struct timeval' will not give current time beyond 2038.
This also changes the code to use ktime_get_real_ts64() which returns a 'struct timespec64' instead of do_gettimeofday() which returns a 'struct timeval'
Signed-off-by: Amitoj Kaur Chawla amitoj1606@gmail.com --- drivers/misc/ibmasm/ibmasm.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/ibmasm/ibmasm.h b/drivers/misc/ibmasm/ibmasm.h index 9b08344..0c7bb6d 100644 --- a/drivers/misc/ibmasm/ibmasm.h +++ b/drivers/misc/ibmasm/ibmasm.h @@ -34,6 +34,7 @@ #include <linux/kref.h> #include <linux/device.h> #include <linux/input.h> +#include <linux/time64.h>
/* Driver identification */ #define DRIVER_NAME "ibmasm" @@ -53,9 +54,9 @@ extern int ibmasm_debug;
static inline char *get_timestamp(char *buf) { - struct timeval now; - do_gettimeofday(&now); - sprintf(buf, "%lu.%lu", now.tv_sec, now.tv_usec); + struct timespec64 now; + ktime_get_real_ts64(&now); + sprintf(buf, "%lu.%lu", now.tv_sec, now.tv_nsec / NSEC_PER_USEC); return buf; }
On Thursday 22 October 2015 23:09:20 Amitoj Kaur Chawla wrote:
This patch replaces timeval with timespec64 as 32 bit 'struct timeval' will not give current time beyond 2038.
This also changes the code to use ktime_get_real_ts64() which returns a 'struct timespec64' instead of do_gettimeofday() which returns a 'struct timeval'
Signed-off-by: Amitoj Kaur Chawla amitoj1606@gmail.com
Looks almost right, except for one bit:
drivers/misc/ibmasm/ibmasm.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/ibmasm/ibmasm.h b/drivers/misc/ibmasm/ibmasm.h index 9b08344..0c7bb6d 100644 --- a/drivers/misc/ibmasm/ibmasm.h +++ b/drivers/misc/ibmasm/ibmasm.h @@ -34,6 +34,7 @@ #include <linux/kref.h> #include <linux/device.h> #include <linux/input.h> +#include <linux/time64.h> /* Driver identification */ #define DRIVER_NAME "ibmasm" @@ -53,9 +54,9 @@ extern int ibmasm_debug; static inline char *get_timestamp(char *buf) {
struct timeval now;do_gettimeofday(&now);sprintf(buf, "%lu.%lu", now.tv_sec, now.tv_usec);
struct timespec64 now;ktime_get_real_ts64(&now);sprintf(buf, "%lu.%lu", now.tv_sec, now.tv_nsec / NSEC_PER_USEC); return buf;
now.tv_sec is using a 'time_t' a.k.a. 'long' on 64-bit architectures, but a 'time64_t' a.k.a. 'long long' on 32-bit architectures. This is a bit nonintuitive, but please change the format string to %llu for the first argument and add a corresponding cast to 's64' or 'long long', so we actually print the correct data and do not get a compiler warning.
The tv_nsec argument is always 'long', on both 32-bit and 64-bit architectures, so there is no warning and we correctly print the number. However, there is a preexisting bug in the format string that you should fix while you are here: %lu drops leading zeroes, which results in something like "12345678.1234" where we want to see "1234567.001234". Please change the format string to add the correct number of leading zeroes here.
As you seem to generally be doing well with the trivial patches, please move on to the 'simple' category next. I should also add a few more 'medium' level tasks but need to pick them carefully first.
Arnd
On Fri, Oct 23, 2015 at 12:45 AM, Arnd Bergmann arnd@arndb.de wrote:
On Thursday 22 October 2015 23:09:20 Amitoj Kaur Chawla wrote:
This patch replaces timeval with timespec64 as 32 bit 'struct timeval' will not give current time beyond 2038.
This also changes the code to use ktime_get_real_ts64() which returns a 'struct timespec64' instead of do_gettimeofday() which returns a 'struct timeval'
Signed-off-by: Amitoj Kaur Chawla amitoj1606@gmail.com
Looks almost right, except for one bit:
drivers/misc/ibmasm/ibmasm.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/ibmasm/ibmasm.h b/drivers/misc/ibmasm/ibmasm.h index 9b08344..0c7bb6d 100644 --- a/drivers/misc/ibmasm/ibmasm.h +++ b/drivers/misc/ibmasm/ibmasm.h @@ -34,6 +34,7 @@ #include <linux/kref.h> #include <linux/device.h> #include <linux/input.h> +#include <linux/time64.h>
/* Driver identification */ #define DRIVER_NAME "ibmasm" @@ -53,9 +54,9 @@ extern int ibmasm_debug;
static inline char *get_timestamp(char *buf) {
struct timeval now;do_gettimeofday(&now);sprintf(buf, "%lu.%lu", now.tv_sec, now.tv_usec);
struct timespec64 now;ktime_get_real_ts64(&now);sprintf(buf, "%lu.%lu", now.tv_sec, now.tv_nsec / NSEC_PER_USEC); return buf;now.tv_sec is using a 'time_t' a.k.a. 'long' on 64-bit architectures, but a 'time64_t' a.k.a. 'long long' on 32-bit architectures. This is a bit nonintuitive, but please change the format string to %llu for the first argument and add a corresponding cast to 's64' or 'long long', so we actually print the correct data and do not get a compiler warning.
The tv_nsec argument is always 'long', on both 32-bit and 64-bit architectures, so there is no warning and we correctly print the number. However, there is a preexisting bug in the format string that you should fix while you are here: %lu drops leading zeroes, which results in something like "12345678.1234" where we want to see "1234567.001234". Please change the format string to add the correct number of leading zeroes here.
Just to clarify, I should change this to sprintf(buf, "%llu.%8.8lu", now.tv_sec, now.tv_nsec / NSEC_PER_USEC);
As you seem to generally be doing well with the trivial patches, please move on to the 'simple' category next. I should also add a few more 'medium' level tasks but need to pick them carefully first.
Okay! Next patches will be the simple tasks. Also, I couldn't find the tutorial you mentioned for compiling, can you help me out some there?
Thanks,
On Fri, Oct 23, 2015 at 7:18 AM, Amitoj Kaur Chawla amitoj1606@gmail.com wrote:
On Fri, Oct 23, 2015 at 12:45 AM, Arnd Bergmann arnd@arndb.de wrote:
On Thursday 22 October 2015 23:09:20 Amitoj Kaur Chawla wrote:
This patch replaces timeval with timespec64 as 32 bit 'struct timeval' will not give current time beyond 2038.
This also changes the code to use ktime_get_real_ts64() which returns a 'struct timespec64' instead of do_gettimeofday() which returns a 'struct timeval'
Signed-off-by: Amitoj Kaur Chawla amitoj1606@gmail.com
Looks almost right, except for one bit:
drivers/misc/ibmasm/ibmasm.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/ibmasm/ibmasm.h b/drivers/misc/ibmasm/ibmasm.h index 9b08344..0c7bb6d 100644 --- a/drivers/misc/ibmasm/ibmasm.h +++ b/drivers/misc/ibmasm/ibmasm.h @@ -34,6 +34,7 @@ #include <linux/kref.h> #include <linux/device.h> #include <linux/input.h> +#include <linux/time64.h>
/* Driver identification */ #define DRIVER_NAME "ibmasm" @@ -53,9 +54,9 @@ extern int ibmasm_debug;
static inline char *get_timestamp(char *buf) {
struct timeval now;do_gettimeofday(&now);sprintf(buf, "%lu.%lu", now.tv_sec, now.tv_usec);
struct timespec64 now;ktime_get_real_ts64(&now);sprintf(buf, "%lu.%lu", now.tv_sec, now.tv_nsec / NSEC_PER_USEC); return buf;now.tv_sec is using a 'time_t' a.k.a. 'long' on 64-bit architectures, but a 'time64_t' a.k.a. 'long long' on 32-bit architectures. This is a bit nonintuitive, but please change the format string to %llu for the first argument and add a corresponding cast to 's64' or 'long long', so we actually print the correct data and do not get a compiler warning.
The tv_nsec argument is always 'long', on both 32-bit and 64-bit architectures, so there is no warning and we correctly print the number. However, there is a preexisting bug in the format string that you should fix while you are here: %lu drops leading zeroes, which results in something like "12345678.1234" where we want to see "1234567.001234". Please change the format string to add the correct number of leading zeroes here.
Just to clarify, I should change this to sprintf(buf, "%llu.%8.8lu", now.tv_sec, now.tv_nsec / NSEC_PER_USEC);
Sorry, it should be changed to sprintf(buf, "%llu.%.08lu", (long long)now.tv_sec, now.tv_nsec / NSEC_PER_USEC);
As you seem to generally be doing well with the trivial patches, please move on to the 'simple' category next. I should also add a few more 'medium' level tasks but need to pick them carefully first.
Okay! Next patches will be the simple tasks. Also, I couldn't find the tutorial you mentioned for compiling, can you help me out some there?
Thanks,
Amitoj
On Friday 23 October 2015 08:55:22 Amitoj Kaur Chawla wrote:
Just to clarify, I should change this to sprintf(buf, "%llu.%8.8lu", now.tv_sec, now.tv_nsec / NSEC_PER_USEC);
Sorry, it should be changed to sprintf(buf, "%llu.%.08lu", (long long)now.tv_sec, now.tv_nsec / NSEC_PER_USEC);
Yes, the last version of that looks correct to me.
Arnd
Hi Amitoj,
On Fri, Oct 23, 2015 at 3:48 AM, Amitoj Kaur Chawla amitoj1606@gmail.com wrote:
On Fri, Oct 23, 2015 at 12:45 AM, Arnd Bergmann arnd@arndb.de wrote:
On Thursday 22 October 2015 23:09:20 Amitoj Kaur Chawla wrote:
This patch replaces timeval with timespec64 as 32 bit 'struct timeval' will not give current time beyond 2038.
This also changes the code to use ktime_get_real_ts64() which returns a 'struct timespec64' instead of do_gettimeofday() which returns a 'struct timeval'
Signed-off-by: Amitoj Kaur Chawla amitoj1606@gmail.com
Looks almost right, except for one bit:
drivers/misc/ibmasm/ibmasm.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/ibmasm/ibmasm.h b/drivers/misc/ibmasm/ibmasm.h index 9b08344..0c7bb6d 100644 --- a/drivers/misc/ibmasm/ibmasm.h +++ b/drivers/misc/ibmasm/ibmasm.h @@ -34,6 +34,7 @@ #include <linux/kref.h> #include <linux/device.h> #include <linux/input.h> +#include <linux/time64.h>
/* Driver identification */ #define DRIVER_NAME "ibmasm" @@ -53,9 +54,9 @@ extern int ibmasm_debug;
static inline char *get_timestamp(char *buf) {
struct timeval now;do_gettimeofday(&now);sprintf(buf, "%lu.%lu", now.tv_sec, now.tv_usec);
struct timespec64 now;ktime_get_real_ts64(&now);sprintf(buf, "%lu.%lu", now.tv_sec, now.tv_nsec / NSEC_PER_USEC); return buf;now.tv_sec is using a 'time_t' a.k.a. 'long' on 64-bit architectures, but a 'time64_t' a.k.a. 'long long' on 32-bit architectures. This is a bit nonintuitive, but please change the format string to %llu for the first argument and add a corresponding cast to 's64' or 'long long', so we actually print the correct data and do not get a compiler warning.
The tv_nsec argument is always 'long', on both 32-bit and 64-bit architectures, so there is no warning and we correctly print the number. However, there is a preexisting bug in the format string that you should fix while you are here: %lu drops leading zeroes, which results in something like "12345678.1234" where we want to see "1234567.001234". Please change the format string to add the correct number of leading zeroes here.
Just to clarify, I should change this to sprintf(buf, "%llu.%8.8lu", now.tv_sec, now.tv_nsec / NSEC_PER_USEC);
As you seem to generally be doing well with the trivial patches, please move on to the 'simple' category next. I should also add a few more 'medium' level tasks but need to pick them carefully first.
Okay! Next patches will be the simple tasks. Also, I couldn't find the tutorial you mentioned for compiling, can you help me out some there?
I think the tutorial mentioned is kernel newbies First patch tutorial and under section Make a driver change: http://kernelnewbies.org/Outreachyfirstpatch#head-816e6eef0008340eab7d542c3f... there is instruction how to compile your changes:
Compile your changes:
Recompile your kernel, by running make (with an optional -jN flag): make -j2
Okay! Next patches will be the simple tasks. Also, I couldn't find the tutorial you mentioned for compiling, can you help me out some there?
I think the tutorial mentioned is kernel newbies First patch tutorial and under section Make a driver change: http://kernelnewbies.org/Outreachyfirstpatch#head-816e6eef0008340eab7d542c3f... there is instruction how to compile your changes:
Compile your changes:
Recompile your kernel, by running make (with an optional -jN flag): make -j2
Hi Ksenija,
Thanks for the help. I'm already doing this, I was under the impression that Arnd was mentioning something other than this. I could be wrong.
On Fri, 23 Oct 2015, Amitoj Kaur Chawla wrote:
Okay! Next patches will be the simple tasks. Also, I couldn't find the tutorial you mentioned for compiling, can you help me out some there?
I think the tutorial mentioned is kernel newbies First patch tutorial and under section Make a driver change: http://kernelnewbies.org/Outreachyfirstpatch#head-816e6eef0008340eab7d542c3f... there is instruction how to compile your changes:
Compile your changes:
Recompile your kernel, by running make (with an optional -jN flag): make -j2
Hi Ksenija,
Thanks for the help. I'm already doing this, I was under the impression that Arnd was mentioning something other than this. I could be wrong.
Are you sure that you have a .o file for the code you are working on? And if you do, are you compiling the whole kernel? Maybe the problem is related to linking?
julia
On Fri, Oct 23, 2015 at 5:55 PM, Julia Lawall julia.lawall@lip6.fr wrote:
On Fri, 23 Oct 2015, Amitoj Kaur Chawla wrote:
Okay! Next patches will be the simple tasks. Also, I couldn't find the tutorial you mentioned for compiling, can you help me out some there?
I think the tutorial mentioned is kernel newbies First patch tutorial
and under
section Make a driver change:
http://kernelnewbies.org/Outreachyfirstpatch#head-816e6eef0008340eab7d542c3f...
there is instruction how to compile your changes:
Compile your changes:
Recompile your kernel, by running make (with an optional -jN flag): make -j2
Hi Ksenija,
Thanks for the help. I'm already doing this, I was under the impression that Arnd was mentioning something other than this. I could be wrong.
Are you sure that you have a .o file for the code you are working on? And if you do, are you compiling the whole kernel? Maybe the problem is related to linking?
julia
Yep, found the issue.
For reference of other applicants in case they get stuck in a similar issue, I'll clarify what happened.
I had initially modified only staging drivers to be compilable as a module. But here I was working on other drivers. Hence the issue.
The Outreachyfirstpatch http://kernelnewbies.org/Outreachyfirstpatch page specifies the process for staging drivers. For other drivers, the Kconfig file can be referred to for finding the driver to make it compilable as a module.
Thank you Julia for helping out!
-
Amitoj Kaur Chawla -
Arnd Bergmann -
Julia Lawall -
Ksenija Stanojević