[PATCH] arch: hexagon: kernel: add export symbol function __delay()

[PATCH] arch: hexagon: kernel: add export symbol function __delay()

[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]


Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 arch/hexagon/include/asm/delay.h |    1 +
 arch/hexagon/kernel/time.c       |    9 +++++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/hexagon/include/asm/delay.h b/arch/hexagon/include/asm/delay.h
index 5307971..8933b9b1 100644
--- a/arch/hexagon/include/asm/delay.h
+++ b/arch/hexagon/include/asm/delay.h
@@ -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))
diff --git a/arch/hexagon/kernel/time.c b/arch/hexagon/kernel/time.c
index d0c4f5a..17fbf45 100644
--- a/arch/hexagon/kernel/time.c
+++ b/arch/hexagon/kernel/time.c
@@ -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.
-- 
1.7.7.6

Re: [PATCH] arch: hexagon: kernel: add export symbol function __delay()

[rkuo]

On Tue, Nov 19, 2013 at 11:10:43AM +0800, Chen Gang wrote:
> 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 <gang.chen.5i5j@gmail.com>
> ---
>  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 <rkuo@codeaurora.org>



-- 

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

Re: [PATCH] arch: hexagon: kernel: add export symbol function __delay()

[Chen Gang]

On 11/25/2013 09:19 AM, rkuo wrote:
> On Tue, Nov 19, 2013 at 11:10:43AM +0800, Chen Gang wrote:
>> 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 <gang.chen.5i5j@gmail.com>
>> ---
>>  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. :-)

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

> 
> 
> Acked-by: Richard Kuo <rkuo@codeaurora.org>
> 

Thanks.
-- 
Chen Gang

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

[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 <gang.chen.5i5j@gmail.com>
---
 drivers/input/touchscreen/sur40.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index cfd1b7e..5dfd01a 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -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];
-- 
1.7.7.6

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

[Dmitry Torokhov]

Hi Chen,

On Wed, Nov 27, 2013 at 10:15:33AM +0800, Chen Gang wrote:
> '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 <gang.chen.5i5j@gmail.com>
> ---
>  drivers/input/touchscreen/sur40.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index cfd1b7e..5dfd01a 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -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.

-- 
Dmitry

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

[Chen Gang]

On 11/28/2013 12:07 PM, Dmitry Torokhov wrote:
> Hi Chen,
> 
> On Wed, Nov 27, 2013 at 10:15:33AM +0800, Chen Gang wrote:
>> > '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 <gang.chen.5i5j@gmail.com>
>> > ---
>> >  drivers/input/touchscreen/sur40.c |    2 +-
>> >  1 files changed, 1 insertions(+), 1 deletions(-)
>> > 
>> > diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
>> > index cfd1b7e..5dfd01a 100644
>> > --- a/drivers/input/touchscreen/sur40.c
>> > +++ b/drivers/input/touchscreen/sur40.c
>> > @@ -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.

-- 
Chen Gang

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

[Chen Gang]

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 <gang.chen.5i5j@gmail.com>
---
 drivers/input/touchscreen/sur40.c |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index cfd1b7e..2ca32cb 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -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);
 
-- 
1.7.7.6