[PATCH v4 1/1] Staging: comedi: replace timeval with ktime_t

64 views
Skip to first unread message

Somya Anand

unread,
Oct 20, 2014, 3:50:14 AM10/20/14
to ar...@arndb.de, opw-k...@googlegroups.com, somyaa...@gmail.com
'struct timeval last' is used for recording last time interrupt.
'struct timeval now' is used for calculating elapsed time.

32-bit systems using 'struct timeval' will break in the year 2038,
so we have to replace that code with more appropriate types.
This patch changes the comedi driver to use ktime_t.

*ktime_get() is better than using do_gettimeofday(),
because it uses the monotonic clock.

---
Changes since v3:
* Reword commit message

Changes since v2:
* devpriv->usec_current = (ktime_to_us(devpriv->last) % USEC_PER_SEC)
% devpriv->usec_period;
is replaced by

devpriv->usec_current = ((u32)ktime_to_us(devpriv->last))
% devpriv->usec_period;

Signed-off-by: Somya Anand <somyaa...@gmail.com>
---
drivers/staging/comedi/drivers/comedi_test.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c b/drivers/staging/comedi/drivers/comedi_test.c
index 00c03df..d73a151 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -52,13 +52,14 @@ zero volts).

#include "comedi_fc.h"
#include <linux/timer.h>
+#include <linux/ktime.h>

#define N_CHANS 8

/* Data unique to this driver */
struct waveform_private {
struct timer_list timer;
- struct timeval last; /* time last timer interrupt occurred */
+ ktime_t last; /* time last timer interrupt occurred */
unsigned int uvolt_amplitude; /* waveform amplitude in microvolts */
unsigned long usec_period; /* waveform period in microseconds */
unsigned long usec_current; /* current time (mod waveform period) */
@@ -170,14 +171,12 @@ static void waveform_ai_interrupt(unsigned long arg)
/* all times in microsec */
unsigned long elapsed_time;
unsigned int num_scans;
- struct timeval now;
+ ktime_t now;
bool stopping = false;

- do_gettimeofday(&now);
+ now = ktime_get();

- elapsed_time =
- 1000000 * (now.tv_sec - devpriv->last.tv_sec) + now.tv_usec -
- devpriv->last.tv_usec;
+ elapsed_time = ktime_to_us(ktime_sub(now, devpriv->last));
devpriv->last = now;
num_scans =
(devpriv->usec_remainder + elapsed_time) / devpriv->scan_period;
@@ -316,8 +315,9 @@ static int waveform_ai_cmd(struct comedi_device *dev,
else /* TRIG_TIMER */
devpriv->convert_period = cmd->convert_arg / nano_per_micro;

- do_gettimeofday(&devpriv->last);
- devpriv->usec_current = devpriv->last.tv_usec % devpriv->usec_period;
+ devpriv->last = ktime_get();
+ devpriv->usec_current =
+ ((u32)ktime_to_us(devpriv->last)) % devpriv->usec_period;
devpriv->usec_remainder = 0;

devpriv->timer.expires = jiffies + 1;
--
1.9.1

Arnd Bergmann

unread,
Oct 20, 2014, 9:46:08 AM10/20/14
to Somya Anand, opw-k...@googlegroups.com
On Monday 20 October 2014 13:19:54 Somya Anand wrote:
> 'struct timeval last' is used for recording last time interrupt.
> 'struct timeval now' is used for calculating elapsed time.
>
> 32-bit systems using 'struct timeval' will break in the year 2038,
> so we have to replace that code with more appropriate types.
> This patch changes the comedi driver to use ktime_t.
>
> *ktime_get() is better than using do_gettimeofday(),
> because it uses the monotonic clock.

Please describe why it is better here, it's clearly not always
better.

> ---
> Changes since v3:
> * Reword commit message

Better also mention the exact comedi driver that is affected.
You have now added an extraneous whitespace character in
each line above. Looks good otherwise,


>
> Changes since v2:
> * devpriv->usec_current = (ktime_to_us(devpriv->last) % USEC_PER_SEC)
> % devpriv->usec_period;
> is replaced by
>
> devpriv->usec_current = ((u32)ktime_to_us(devpriv->last))
> % devpriv->usec_period;

This also really needs to be described. Why is this safe to do (I explained
it to you, but you apparently forgot to mention it here)?

Arnd

Somya Anand

unread,
Oct 20, 2014, 12:22:43 PM10/20/14
to ar...@arndb.de, opw-k...@googlegroups.com, somyaa...@gmail.com
'struct timeval last' is used for recording last time interrupt.
'struct timeval now' is used for calculating elapsed time.

32-bit systems using 'struct timeval' will break in the year 2038,
so we have to replace that code with more appropriate types.
This patch changes the comedi driver to use ktime_t.

Since this code doesn't communicate the time values
to the outside (user space, file system, network).Thus ktime_get()
is a better than using do_gettimeofday() as it uses monotonic
clock.

ktime_to_us() returns an 's64', and using the '%' operator on that requires
doing a 64-bit division which needs an expensive library function call.
Therefore:
devpriv->usec_current = (ktime_to_us(devpriv->last) % USEC_PER_SEC)
% devpriv->usec_period;
is replaced by

devpriv->usec_current = ((u32)ktime_to_us(devpriv->last))
% devpriv->usec_period;

---
Changes since v4:
* Reword the commit message.

Somya Anand

unread,
Oct 20, 2014, 1:11:07 PM10/20/14
to ar...@arndb.de, opw-k...@googlegroups.com, somyaa...@gmail.com
'struct timeval last' is used for recording last time interrupt.
'struct timeval now' is used for calculating elapsed time.

32-bit systems using 'struct timeval' will break in the year 2038,
so we have to replace that code with more appropriate types.
This patch changes the comedi driver to use ktime_t.

Since this code doesn't communicate the time values
to the outside (user space, file system, network).Thus ktime_get()
is a better than using do_gettimeofday() as it uses monotonic
clock.

ktime_to_us() returns an 's64', and using the '%' operator on that requires
doing a 64-bit division which needs an expensive library function call.
Therefore:
devpriv->usec_current = (ktime_to_us(devpriv->last) % USEC_PER_SEC)
% devpriv->usec_period;
is replaced by

devpriv->usec_current = ((u32)ktime_to_us(devpriv->last))
% devpriv->usec_period;

---
Signed-off-by: Somya Anand <somyaa...@gmail.com>
Changes since v4:
* Reword the commit message.

Arnd Bergmann

unread,
Oct 20, 2014, 1:31:01 PM10/20/14
to Somya Anand, opw-k...@googlegroups.com
On Monday 20 October 2014 21:51:53 Somya Anand wrote:
>
> ktime_to_us() returns an 's64', and using the '%' operator on that requires
> doing a 64-bit division which needs an expensive library function call.
> Therefore:
> devpriv->usec_current = (ktime_to_us(devpriv->last) % USEC_PER_SEC)
> % devpriv->usec_period;
> is replaced by
>
> devpriv->usec_current = ((u32)ktime_to_us(devpriv->last))
> % devpriv->usec_period;

You still don't explain why it's safe to do this, as the resulting
usec_current value is different, and rather random now.


Arnd

Somya Anand

unread,
Oct 20, 2014, 3:02:21 PM10/20/14
to ar...@arndb.de, opw-k...@googlegroups.com, somyaa...@gmail.com
'struct timeval last' is used for recording last time interrupt.
'struct timeval now' is used for calculating elapsed time.

32-bit systems using 'struct timeval' will break in the year 2038,
so we have to replace that code with more appropriate types.
This patch changes the comedi driver to use ktime_t.

Since this code doesn't communicate the time values
to the outside (user space, file system, network).Thus ktime_get()
is a better than using do_gettimeofday() as it uses monotonic
clock.

ktime_to_us() returns an 's64', and using the '%' operator on that requires
doing a 64-bit division which needs an expensive library function call
the specific value of usec_current does not actually matter although it might
matter that it's not always the same
which will start with the offset from the lower 32 bit of the microsecond.
Therefore:
devpriv->usec_current = (ktime_to_us(devpriv->last) % USEC_PER_SEC)
% devpriv->usec_period;
is replaced by

devpriv->usec_current = ((u32)ktime_to_us(devpriv->last))
% devpriv->usec_period;

Signed-off-by: Somya Anand <somyaa...@gmail.com>
Changes since v4:
* Reword the commit message.

Signed-off-by: Somya Anand <somyaa...@gmail.com>
---

Julia Lawall

unread,
Oct 20, 2014, 3:20:30 PM10/20/14
to Somya Anand, ar...@arndb.de, opw-k...@googlegroups.com


On Tue, 21 Oct 2014, Somya Anand wrote:

> 'struct timeval last' is used for recording last time interrupt.
> 'struct timeval now' is used for calculating elapsed time.
>
> 32-bit systems using 'struct timeval' will break in the year 2038,
> so we have to replace that code with more appropriate types.
> This patch changes the comedi driver to use ktime_t.
>
> Since this code doesn't communicate the time values
> to the outside (user space, file system, network).Thus ktime_get()
> is a better than using do_gettimeofday() as it uses monotonic
> clock.
>
> ktime_to_us() returns an 's64', and using the '%' operator on that requires
> doing a 64-bit division which needs an expensive library function call
> the specific value of usec_current does not actually matter although it might
> matter that it's not always the same
> which will start with the offset from the lower 32 bit of the microsecond.
> Therefore:
> devpriv->usec_current = (ktime_to_us(devpriv->last) % USEC_PER_SEC)
> % devpriv->usec_period;
> is replaced by
>
> devpriv->usec_current = ((u32)ktime_to_us(devpriv->last))
> % devpriv->usec_period;

You shoulld have only one Signed off by. Below that should be ---. Below
that should be the description of the changes since the previous version.

julia
> --
> You received this message because you are subscribed to the Google Groups "opw-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to opw-kernel+...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

Arnd Bergmann

unread,
Oct 20, 2014, 4:06:14 PM10/20/14
to opw-k...@googlegroups.com, Somya Anand
On Tuesday 21 October 2014 00:31:16 Somya Anand wrote:
> 'struct timeval last' is used for recording last time interrupt.
> 'struct timeval now' is used for calculating elapsed time.
>
> 32-bit systems using 'struct timeval' will break in the year 2038,
> so we have to replace that code with more appropriate types.
> This patch changes the comedi driver to use ktime_t.
>
> Since this code doesn't communicate the time values
> to the outside (user space, file system, network).Thus ktime_get()
> is a better than using do_gettimeofday() as it uses monotonic
> clock.
>
> ktime_to_us() returns an 's64', and using the '%' operator on that requires
> doing a 64-bit division which needs an expensive library function call
> the specific value of usec_current does not actually matter although it might
> matter that it's not always the same
> which will start with the offset from the lower 32 bit of the microsecond.
> Therefore:
> devpriv->usec_current = (ktime_to_us(devpriv->last) % USEC_PER_SEC)
> % devpriv->usec_period;
> is replaced by
>
> devpriv->usec_current = ((u32)ktime_to_us(devpriv->last))
> % devpriv->usec_period;
>
> Signed-off-by: Somya Anand <somyaa...@gmail.com>

Reviewed-by: Arnd Bergmann <ar...@arndb.de>

(once you fix the issue that Julia pointed out)

Somya Anand

unread,
Oct 22, 2014, 7:53:53 AM10/22/14
to opw-k...@googlegroups.com, Somya Anand
---
Changes since v6:
* Reword commit message

Arnd Bergmann

unread,
Oct 23, 2014, 3:28:54 PM10/23/14
to Greg Kroah-Hartman, opw-kernel, Tapasweni Pathak, Somya Anand, Ebru Akagunduz, Arnd Bergmann
From: Somya Anand <somyaa...@gmail.com>

'struct timeval last' is used for recording last time interrupt.
'struct timeval now' is used for calculating elapsed time.

32-bit systems using 'struct timeval' will break in the year 2038,
so we have to replace that code with more appropriate types.
This patch changes the comedi driver to use ktime_t.

Since this code doesn't communicate the time values
to the outside (user space, file system, network).Thus ktime_get()
is a better than using do_gettimeofday() as it uses monotonic
clock.

ktime_to_us() returns an 's64', and using the '%' operator on that requires
doing a 64-bit division which needs an expensive library function call
the specific value of usec_current does not actually matter although it might
matter that it's not always the same
which will start with the offset from the lower 32 bit of the microsecond.
Therefore:
devpriv->usec_current = (ktime_to_us(devpriv->last) % USEC_PER_SEC)
% devpriv->usec_period;
is replaced by

devpriv->usec_current = ((u32)ktime_to_us(devpriv->last))
% devpriv->usec_period;

Signed-off-by: Somya Anand <somyaa...@gmail.com>
Signed-off-by: Arnd Bergmann <ar...@arndb.de>
---
drivers/staging/comedi/drivers/comedi_test.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c b/drivers/staging/comedi/drivers/comedi_test.c
index 8845075cd3cc..4d4358ce1710 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -52,13 +52,14 @@ zero volts).

#include "comedi_fc.h"
#include <linux/timer.h>
+#include <linux/ktime.h>

#define N_CHANS 8

/* Data unique to this driver */
struct waveform_private {
struct timer_list timer;
- struct timeval last; /* time last timer interrupt occurred */
+ ktime_t last; /* time last timer interrupt occurred */
unsigned int uvolt_amplitude; /* waveform amplitude in microvolts */
unsigned long usec_period; /* waveform period in microseconds */
unsigned long usec_current; /* current time (mod waveform period) */
@@ -171,14 +172,12 @@ static void waveform_ai_interrupt(unsigned long arg)
/* all times in microsec */
unsigned long elapsed_time;
unsigned int num_scans;
- struct timeval now;
+ ktime_t now;
bool stopping = false;

- do_gettimeofday(&now);
+ now = ktime_get();

- elapsed_time =
- 1000000 * (now.tv_sec - devpriv->last.tv_sec) + now.tv_usec -
- devpriv->last.tv_usec;
+ elapsed_time = ktime_to_us(ktime_sub(now, devpriv->last));
devpriv->last = now;
num_scans =
(devpriv->usec_remainder + elapsed_time) / devpriv->scan_period;
@@ -317,8 +316,9 @@ static int waveform_ai_cmd(struct comedi_device *dev,
else /* TRIG_TIMER */
devpriv->convert_period = cmd->convert_arg / nano_per_micro;

- do_gettimeofday(&devpriv->last);
- devpriv->usec_current = devpriv->last.tv_usec % devpriv->usec_period;
+ devpriv->last = ktime_get();
+ devpriv->usec_current =
+ ((u32)ktime_to_us(devpriv->last)) % devpriv->usec_period;
devpriv->usec_remainder = 0;

devpriv->timer.expires = jiffies + 1;
--
2.1.0.rc2

Arnd Bergmann

unread,
Oct 23, 2014, 3:28:55 PM10/23/14
to Greg Kroah-Hartman, opw-kernel, Tapasweni Pathak, Somya Anand, Ebru Akagunduz, Arnd Bergmann
From: Tapasweni Pathak <tapaswe...@gmail.com>

struct sdu_stamp in the gdm_sdio.h is a timeval type.
'struct timeval now' is used for calculating elapsed time.

32-bit systems using 'struct timeval' will break in the year 2038,
so we have to replace that code with more appropriate types.
This patch changes the gdm72xx driver to use ktime_t.

ktime_get() is better than using do_gettimeofday(),
because it uses the monotonic clock. ktime_sub
are used to subtract two ktime variables.

Signed-off-by: Tapasweni Pathak <tapaswe...@gmail.com>
Signed-off-by: Arnd Bergmann <ar...@arndb.de>
---
drivers/staging/gdm72xx/gdm_sdio.c | 11 +++++------
drivers/staging/gdm72xx/gdm_sdio.h | 4 ++--
2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/gdm72xx/gdm_sdio.c b/drivers/staging/gdm72xx/gdm_sdio.c
index 7a0a0f221418..68cc57dc25ca 100644
--- a/drivers/staging/gdm72xx/gdm_sdio.c
+++ b/drivers/staging/gdm72xx/gdm_sdio.c
@@ -36,7 +36,7 @@
#define RX_BUF_SIZE (25*1024)

#define TX_HZ 2000
-#define TX_INTERVAL (1000000/TX_HZ)
+#define TX_INTERVAL (NSEC_PER_SEC/TX_HZ)

static struct sdio_tx *alloc_tx_struct(struct tx_cxt *tx)
{
@@ -305,7 +305,7 @@ static void send_sdu(struct sdio_func *func, struct tx_cxt *tx)
put_tx_struct(t->tx_cxt, t);
}

- do_gettimeofday(&tx->sdu_stamp);
+ tx->sdu_stamp = ktime_get();
spin_unlock_irqrestore(&tx->lock, flags);
}

@@ -332,7 +332,7 @@ static void do_tx(struct work_struct *work)
struct sdio_func *func = sdev->func;
struct tx_cxt *tx = &sdev->tx;
struct sdio_tx *t = NULL;
- struct timeval now, *before;
+ ktime_t now, *before;
int is_sdu = 0;
long diff;
unsigned long flags;
@@ -348,11 +348,10 @@ static void do_tx(struct work_struct *work)
list_del(&t->list);
is_sdu = 0;
} else if (!tx->stop_sdu_tx && !list_empty(&tx->sdu_list)) {
- do_gettimeofday(&now);
+ now = ktime_get();
before = &tx->sdu_stamp;

- diff = (now.tv_sec - before->tv_sec) * 1000000 +
- (now.tv_usec - before->tv_usec);
+ diff = ktime_to_ns(ktime_sub(now, before));
if (diff >= 0 && diff < TX_INTERVAL) {
schedule_work(&sdev->ws);
spin_unlock_irqrestore(&tx->lock, flags);
diff --git a/drivers/staging/gdm72xx/gdm_sdio.h b/drivers/staging/gdm72xx/gdm_sdio.h
index 77ad9d686f8e..aa7dad22a219 100644
--- a/drivers/staging/gdm72xx/gdm_sdio.h
+++ b/drivers/staging/gdm72xx/gdm_sdio.h
@@ -15,7 +15,7 @@
#define __GDM72XX_GDM_SDIO_H__

#include <linux/types.h>
-#include <linux/time.h>
+#include <linux/ktime.h>

#define MAX_NR_SDU_BUF 64

@@ -32,7 +32,7 @@ struct tx_cxt {
struct list_head free_list;
struct list_head sdu_list;
struct list_head hci_list;
- struct timeval sdu_stamp;
+ ktime_t sdu_stamp;
u8 *sdu_buf;
spinlock_t lock;
int can_send;
--
2.1.0.rc2

Arnd Bergmann

unread,
Oct 23, 2014, 3:28:56 PM10/23/14
to Greg Kroah-Hartman, opw-kernel, Tapasweni Pathak, Somya Anand, Ebru Akagunduz, Arnd Bergmann
From: Somya Anand <somyaa...@gmail.com>

'struct timeval t' is used to return remaining time in milliseconds.

32-bit systems using 'struct timeval' will break in the year 2038,
so we have to replace that code with more appropriate types.
This patch changes the android driver to use ktime_t.

Signed-off-by: Somya Anand <somyaa...@gmail.com>
Signed-off-by: Arnd Bergmann <ar...@arndb.de>
---
drivers/staging/android/timed_gpio.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/android/timed_gpio.c b/drivers/staging/android/timed_gpio.c
index 8fa4758517c0..12d55148bafc 100644
--- a/drivers/staging/android/timed_gpio.c
+++ b/drivers/staging/android/timed_gpio.c
@@ -20,6 +20,7 @@
#include <linux/hrtimer.h>
#include <linux/err.h>
#include <linux/gpio.h>
+#include <linux/ktime.h>

#include "timed_output.h"
#include "timed_gpio.h"
@@ -46,16 +47,16 @@ static enum hrtimer_restart gpio_timer_func(struct hrtimer *timer)
static int gpio_get_time(struct timed_output_dev *dev)
{
struct timed_gpio_data *data;
- struct timeval t;
+ ktime_t t;

data = container_of(dev, struct timed_gpio_data, dev);

if (!hrtimer_active(&data->timer))
return 0;

- t = ktime_to_timeval(hrtimer_get_remaining(&data->timer));
+ t = hrtimer_get_remaining(&data->timer);

- return t.tv_sec * 1000 + t.tv_usec / 1000;
+ return ktime_to_ms(t);
}

static void gpio_enable(struct timed_output_dev *dev, int value)
--
2.1.0.rc2

Arnd Bergmann

unread,
Oct 23, 2014, 3:28:57 PM10/23/14
to Greg Kroah-Hartman, opw-kernel, Tapasweni Pathak, Somya Anand, Ebru Akagunduz, Arnd Bergmann
From: Tapasweni Pathak <tapaswe...@gmail.com>

'timeval' is used to print timestamps in seconds and millisecond.

32-bit systems using 'struct timeval' will break in the year 2038,
So we have to replace that code with more appropriate types.

This patch changes the android driver to use timespec64.
Since this is a staging driver and the output is only used for a
debugfs file, it is fine to slightly change the output and
use nanoseconds instead. ktime_to_timespec64, is used which will return
seconds and nanoseconds.

Signed-off-by: Tapasweni Pathak <tapaswe...@gmail.com>
Signed-off-by: Arnd Bergmann <ar...@arndb.de>
---
drivers/staging/android/sync_debug.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c
index 257fc91bf02b..9af56fd7fad3 100644
--- a/drivers/staging/android/sync_debug.c
+++ b/drivers/staging/android/sync_debug.c
@@ -25,6 +25,7 @@
#include <linux/slab.h>
#include <linux/uaccess.h>
#include <linux/anon_inodes.h>
+#include <linux/time64.h>
#include "sync.h"

#ifdef CONFIG_DEBUG_FS
@@ -95,9 +96,9 @@ static void sync_print_pt(struct seq_file *s, struct sync_pt *pt, bool fence)
sync_status_str(status));

if (status <= 0) {
- struct timeval tv = ktime_to_timeval(pt->base.timestamp);
+ struct timespec64 ts64 = ktime_to_timespec64(pt->base.timestamp);

- seq_printf(s, "@%ld.%06ld", tv.tv_sec, tv.tv_usec);
+ seq_printf(s, "@%lld.%09ld", ts64.tv_sec, ts64.tv_nsec);
}

if (parent->ops->timeline_value_str &&
--
2.1.0.rc2

Arnd Bergmann

unread,
Oct 23, 2014, 3:28:58 PM10/23/14
to Greg Kroah-Hartman, opw-kernel, Tapasweni Pathak, Somya Anand, Ebru Akagunduz, Arnd Bergmann
Hi Greg,

I've gone through my sent-mail folder now and found four patches
for staging that I have reviewed and considered good. Please apply.

OPW applicants,

It's quite possible that I missed some other patch for the y2038
work that is already complete. If so, please resend the latest
patch. Somya had one or two more patches that no longer apply
because the bcm driver was deleted, so I couldn't send that.
There is also one patch from Ebru for drivers/power/. I will
check if that has made it into next, and if not, I'll start a
new git tree for that and have Stephen add it to linux-next.

Thanks,

Arnd

Somya Anand (2):
Staging: comedi: replace timeval with ktime_t
Staging: android: Replace timeval with ktime_t in timed_gpio.c

Tapasweni Pathak (2):
staging: gdm72xx: Replace timeval with ktime_t
staging: android: Replace timeval with timespec64

drivers/staging/android/sync_debug.c | 5 +++--
drivers/staging/android/timed_gpio.c | 7 ++++---
drivers/staging/comedi/drivers/comedi_test.c | 16 ++++++++--------
drivers/staging/gdm72xx/gdm_sdio.c | 11 +++++------
drivers/staging/gdm72xx/gdm_sdio.h | 4 ++--
5 files changed, 22 insertions(+), 21 deletions(-)

--
2.1.0.rc2

Greg Kroah-Hartman

unread,
Oct 24, 2014, 11:27:33 PM10/24/14
to Arnd Bergmann, opw-kernel, Tapasweni Pathak, Somya Anand, Ebru Akagunduz
This causes a build warning:

drivers/staging/android/sync_debug.c: In function ‘sync_print_pt’:
drivers/staging/android/sync_debug.c:101:3: warning: format ‘%lld’ expects argument of type ‘long long int’, but argument 3 has type ‘__kernel_time_t’ [-Wformat=]
seq_printf(s, "@%lld.%09ld", ts64.tv_sec, ts64.tv_nsec);
^

So I can't take it, sorry.

greg k-h

Tapasweni Pathak

unread,
Oct 25, 2014, 12:03:10 AM10/25/14
to Greg Kroah-Hartman, Arnd Bergmann, opw-kernel, Somya Anand, Ebru Akagunduz

Greg Kroah-Hartman

unread,
Oct 27, 2014, 6:26:12 AM10/27/14
to Tapasweni Pathak, Arnd Bergmann, opw-kernel, Somya Anand, Ebru Akagunduz
You are probably running in a 32bit system, not a 64 bit kernel, right?

thanks,

greg k-h

Tapasweni Pathak

unread,
Oct 27, 2014, 6:33:01 AM10/27/14
to Greg Kroah-Hartman, Arnd Bergmann, opw-kernel, Somya Anand, Ebru Akagunduz
No, I am running a 64 bit kernel.
uname -m gives me
x86_64

still I did not got the warning. :(
It compiled without any warning.  Is the gcc version can cause such a problem?


Greg Kroah-Hartman

unread,
Oct 27, 2014, 6:41:11 AM10/27/14
to Tapasweni Pathak, Arnd Bergmann, opw-kernel, Somya Anand, Ebru Akagunduz
I really do not know, sorry.

Arnd Bergmann

unread,
Oct 27, 2014, 7:57:06 AM10/27/14
to opw-k...@googlegroups.com, Greg Kroah-Hartman, Tapasweni Pathak, Somya Anand, Ebru Akagunduz
> > It compiled without any warning. Is the gcc version can cause such a problem?
> >
> >
>
> I really do not know, sorry.

The question here is whether CONFIG_32BIT was set in the kernel. Sometimes
warnings only show up either with or without this option, and it's independent
of what kernel you are running.

Version 4 of the patch (from yesterday) fixes the problem for all I can tell.

Arnd

Tapasweni Pathak

unread,
Oct 27, 2014, 9:13:51 AM10/27/14
to Arnd Bergmann, opw-kernel, Greg Kroah-Hartman, Somya Anand, Ebru Akagunduz
Hi arnd,

I have CONFIG_64BIT set in my .config as CONFIG_64BIT=y. 

I grep CONFIG_32BIT .config it returned none, when grep BIT .config, gave me, http://pastebin.com/aGPxz6Bx.

When I previously compiled the changes (in version 3 of this patch), where Greg reported the warning, I got this, http://gyazo.com/c262514006c4436dab55963e9b0bc7cd, no warnings.

So after checking the 32 bit or 64 bit build thing, I thought to revert the change of casting it to (s64), and expected the same thing as I got previously. But I got the warning, now (http://gyazo.com/fbb04bcc73102ee7ab2d742e51d6be1b).

After correcting it again (casting it to s64), I got this (http://gyazo.com/11a5d367be0afe5dd5b09b7ebd3dbfb3). No warnings.

But there is a difference between the log that after the v3 of this patch, and today when I checked it. In the previous compilation(when Greg reported the warning), no sync_debug.o was created(that means it was not compiling this file)? but today it is.

I have recently used
make allyesconfig(as I wanted to compile media: lirc and I wanted to set LIRC_STAGING, and I was not able to find anything under device drivers->staging drivers->media staging driver to set it).

But just to check that may be it set some config symbol, I again did cp /boot/config-`uname -r`* .config . and the process. This gave me the same thing as I got just before copying the config again.

Then why I didn't got any warning for the v3 of this patch or why sync_debug was not compiled? Please correct me if I am wrong somewhere. There must be something wrong at my part.

I hope I was able to explain my confusion.

Thank you so much
Tapasweni Pathak

Arnd Bergmann

unread,
Oct 27, 2014, 9:53:09 AM10/27/14
to Tapasweni Pathak, opw-kernel, Greg Kroah-Hartman, Somya Anand, Ebru Akagunduz
Right, the sync_debug.o does not show up in the two build logs above, which
means it was not built back then.

> I have recently used
> make allyesconfig(as I wanted to compile media: lirc and I wanted to
> set LIRC_STAGING, and I was not able to find anything under device
> drivers->staging drivers->media staging driver to set it).
>
> But just to check that may be it set some config symbol, I again did cp
> /boot/config-`uname -r`* .config . and the process. This gave me the same
> thing as I got just before copying the config again.
>
> Then why I didn't got any warning for the v3 of this patch or why
> sync_debug was not compiled? Please correct me if I am wrong somewhere.
> There must be something wrong at my part.
>
> I hope I was able to explain my confusion.

It depends on the initial configuration you were using. If you never
ran 'make allmodconfig' or something like that, the kernel will pick
the configuration from the running system as a start, and this file is
normally turned off.

You should look at the Makefile in the directory of the driver to find
which Kconfig symbol is needed (CONFIG_SYNC in this case) and then
find that symbol in the Kconfig file and .config to see how to turn
it on if necessary.

Arnd
Reply all
Reply to author
Forward
0 new messages