arch: hexagon: kernel: add export symbol function __delay()

PATCH In review Chen Gang 3 participants 7 messages Nov 19, 2013 – Nov 28, 2013

Subsystems (3)

HID Input drivers Qualcomm hexagon architecture

lore

Share & tools

lore

Chen Gang

author

2013-11-19 03:08 UTC #0 lore raw

PATCH 1/1 arch: hexagon: kernel: add export symbol function __delay()

Need add __delay() implementation, or can not pass allmodconfig in next-20131118 tree.

The related error:

CC kernel/locking/spinlock_debug.o
kernel/locking/spinlock_debug.c: In function '__spin_lock_debug':
kernel/locking/spinlock_debug.c:114:3: error: implicit declaration of function '__delay' [-Werror=implicit-function-declaration]

Signed-off-by Chen Gang <[email protected]>

---
 arch/hexagon/include/asm/delay.h |    1 +
 arch/hexagon/kernel/time.c       |    9 +++++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

arch/hexagon/include/asm/delay.h +1 -0

index 5307971..8933b9b1 100644
@@ -21,6 +21,7 @@
 
 #include <asm/param.h>
 
+extern void __delay(unsigned long cycles);
 extern void __udelay(unsigned long usecs);
 
 #define udelay(usecs) __udelay((usecs))

arch/hexagon/kernel/time.c +9 -0

index d0c4f5a..17fbf45 100644
@@ -229,6 +229,15 @@ void __init time_init(void)
 	late_time_init = time_init_deferred;
 }
 
+void __delay(unsigned long cycles)
+{
+	unsigned long long start = __vmgettime();
+
+	while ((__vmgettime() - start) < cycles)
+		cpu_relax();
+}
+EXPORT_SYMBOL(__delay);
+
 /*
  * This could become parametric or perhaps even computed at run-time,
  * but for now we take the observed simulator jitter.

Signature · 1 line

1.7.7.6

rkuo

Acked-by on v1 1/1

2013-11-25 01:19 UTC #1 lore raw

Quoted 10 lines · Chen Gang

Need add __delay() implementation, or can not pass allmodconfig in next-20131118 tree.
The related error:
CC kernel/locking/spinlock_debug.o
kernel/locking/spinlock_debug.c: In function '__spin_lock_debug':
kernel/locking/spinlock_debug.c:114:3: error: implicit declaration of function '__delay' [-Werror=implicit-function-declaration]

Quoted 5 lines

Signed-off-by Chen Gang <[email protected]>

---
 arch/hexagon/include/asm/delay.h |    1 +
 arch/hexagon/kernel/time.c       |    9 +++++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

Thanks again for all the cleanups. I've tested this and the rest of your patches on my internal tree and everything checks out.

Also just to let you know, I'm still waiting to hear back on that compiler bug.

Acked-by Richard Kuo <[email protected]>

Signature · 3 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Chen Gang

author

2013-11-25 01:48 UTC #2 lore raw

Quoted 11 lines · rkuo

Quoted 10 lines · Chen Gang
Need add __delay() implementation, or can not pass allmodconfig in next-20131118 tree.
The related error:
CC kernel/locking/spinlock_debug.o
kernel/locking/spinlock_debug.c: In function '__spin_lock_debug':
kernel/locking/spinlock_debug.c:114:3: error: implicit declaration of function '__delay' [-Werror=implicit-function-declaration]

Quoted 9 lines

Quoted 5 lines
Signed-off-by Chen Gang <[email protected]>
---
 arch/hexagon/include/asm/delay.h |    1 +
 arch/hexagon/kernel/time.c       |    9 +++++++++
 2 files changed, 10 insertions(+), 0 deletions(-)
Thanks again for all the cleanups. I've tested this and the rest of your patches on my internal tree and everything checks out.

OK, thanks. I will/should continue. :-)

Quoted 3 lines

Also just to let you know, I'm still waiting to hear back on that compiler bug.

If necessary/suitable, can let me join to the related communication.

Quoted 4 lines

Acked-by Richard Kuo <[email protected]>

Thanks.

Signature · 1 line

Chen Gang

Chen Gang

author

2013-11-27 02:13 UTC #3 lore raw

PATCH 1/1 drivers: input: touchscreen: sur40: use static variable instead of stack varialbe for 'packet_id'

In reply to rkuo

'packet_id' is used for checking sequence whether in order, it need be static variable independent from sur40_poll().

The related warning (with allmodconfig under hexagon):

drivers/input/touchscreen/sur40.c: In function 'sur40_poll':
drivers/input/touchscreen/sur40.c:297:6: warning: 'packet_id' may be used uninitialized in this function [-Wuninitialized]

Signed-off-by Chen Gang <[email protected]>

---
 drivers/input/touchscreen/sur40.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/input/touchscreen/sur40.c +1 -1

index cfd1b7e..5dfd01a 100644
@@ -251,7 +251,7 @@ static void sur40_poll(struct input_polled_dev *polldev)
 	struct sur40_state *sur40 = polldev->private;
 	struct input_dev *input = polldev->input;
 	int result, bulk_read, need_blobs, packet_blobs, i;
-	u32 packet_id;
+	static u32 packet_id;
 
 	struct sur40_header *header = &sur40->bulk_in_buffer->header;
 	struct sur40_blob *inblob = &sur40->bulk_in_buffer->blobs[0];

Signature · 1 line

1.7.7.6

Dmitry Torokhov

maintainer

2013-11-28 04:07 UTC #4 lore raw

Re: [PATCH] drivers: input: touchscreen: sur40: use static variable instead of stack varialbe for 'packet_id'

Hi Chen,

Quoted 19 lines · Chen Gang

'packet_id' is used for checking sequence whether in order, it need be static variable independent from sur40_poll().
The related warning (with allmodconfig under hexagon):
drivers/input/touchscreen/sur40.c: In function 'sur40_poll':
drivers/input/touchscreen/sur40.c:297:6: warning: 'packet_id' may be used uninitialized in this function [-Wuninitialized]
Signed-off-by Chen Gang <[email protected]>
---
 drivers/input/touchscreen/sur40.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/input/touchscreen/sur40.c +0 -0
index cfd1b7e..5dfd01a 100644

drivers/input/touchscreen/sur40.c static void sur40_poll(struct input_polled_dev *polldev) · line 254

Quoted 5 lines

drivers/input/touchscreen/sur40.c +1 -1

@@ -251,7 +251,7 @@ static void sur40_poll(struct input_polled_dev *polldev)
 	struct sur40_state *sur40 = polldev->private;
 	struct input_dev *input = polldev->input;
 	int result, bulk_read, need_blobs, packet_blobs, i;
-	u32 packet_id;
+	static u32 packet_id;

It is usually not a good idea to use statics in device drivers as it does not work well when you have several devices of the same type present in a system. Also, we process all blobs in one pass so there is no need to preserve value of packet_id between calls to sur40_poll().

Thanks.

Signature · 1 line

Dmitry

Chen Gang

author

2013-11-28 04:22 UTC #5 lore raw

Re: [PATCH] drivers: input: touchscreen: sur40: use static variable instead of stack varialbe for 'packet_id'

Quoted 17 lines · Dmitry Torokhov

Hi Chen,
Quoted 14 lines · Chen Gang
Quoted 14 lines
'packet_id' is used for checking sequence whether in order, it need be static variable independent from sur40_poll().
The related warning (with allmodconfig under hexagon):
drivers/input/touchscreen/sur40.c: In function 'sur40_poll':
drivers/input/touchscreen/sur40.c:297:6: warning: 'packet_id' may be used uninitialized in this function [-Wuninitialized]
Signed-off-by Chen Gang <[email protected]>
---
 drivers/input/touchscreen/sur40.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Quoted 14 lines

Quoted 10 lines
drivers/input/touchscreen/sur40.c static void sur40_poll(struct input_polled_dev *polldev) · line 254
Quoted 10 lines
drivers/input/touchscreen/sur40.c +1 -1
index cfd1b7e..5dfd01a 100644
@@ -251,7 +251,7 @@ static void sur40_poll(struct input_polled_dev *polldev)
 	struct sur40_state *sur40 = polldev->private;
 	struct input_dev *input = polldev->input;
 	int result, bulk_read, need_blobs, packet_blobs, i;
-	u32 packet_id;
+	static u32 packet_id;
It is usually not a good idea to use statics in device drivers as it does not work well when you have several devices of the same type present in a system. Also, we process all blobs in one pass so there is no need to preserve value of packet_id between calls to sur40_poll().

OK, thanks, I will/should send patch v2 for it.

Signature · 1 line

Chen Gang

Chen Gang

author

2013-11-28 04:31 UTC #6 lore raw

PATCH v2 1/1 drivers: input: touchscreen: sur40: remove stack variable 'packet_id' from sur40_poll()

In reply to Dmitry Torokhov

The drivers process all blobs in one pass, so there is no need to preserve value of packet_id between calls to sur40_poll().

And the original implementation may cause 'packet_id' uninitialized, the related warning (with allmodconfig under hexagon):

drivers/input/touchscreen/sur40.c: In function 'sur40_poll':
drivers/input/touchscreen/sur40.c:297:6: warning: 'packet_id' may be used uninitialized in this function [-Wuninitialized]

Signed-off-by Chen Gang <[email protected]>

---
 drivers/input/touchscreen/sur40.c |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

drivers/input/touchscreen/sur40.c +0 ~~-10~~

index cfd1b7e..2ca32cb 100644
@@ -251,7 +251,6 @@ static void sur40_poll(struct input_polled_dev *polldev)
 	struct sur40_state *sur40 = polldev->private;
 	struct input_dev *input = polldev->input;
 	int result, bulk_read, need_blobs, packet_blobs, i;
-	u32 packet_id;
 
 	struct sur40_header *header = &sur40->bulk_in_buffer->header;
 	struct sur40_blob *inblob = &sur40->bulk_in_buffer->blobs[0];

@@ -286,17 +285,8 @@ static void sur40_poll(struct input_polled_dev *polldev)
 		if (need_blobs == -1) {
 			need_blobs = le16_to_cpu(header->count);
 			dev_dbg(sur40->dev, "need %d blobs\n", need_blobs);
-			packet_id = header->packet_id;
 		}
 
-		/*
-		 * Sanity check. when video data is also being retrieved, the
-		 * packet ID will usually increase in the middle of a series
-		 * instead of at the end.
-		 */
-		if (packet_id != header->packet_id)
-			dev_warn(sur40->dev, "packet ID mismatch\n");
-
 		packet_blobs = result / sizeof(struct sur40_blob);
 		dev_dbg(sur40->dev, "received %d blobs\n", packet_blobs);

Signature · 1 line

1.7.7.6