* [PATCH] drivers: input: touchscreen: sur40: use static variable instead of stack varialbe for 'packet_id' [not found] ` <20131125011908.GA18921@codeaurora.org> @ 2013-11-27 2:15 ` Chen Gang 2013-11-28 4:07 ` Dmitry Torokhov 0 siblings, 1 reply; 4+ messages in thread From: Chen Gang @ 2013-11-27 2:15 UTC (permalink / raw) To: dmitry.torokhov, floe, rydberg, David Herrmann Cc: rkuo, linux-kernel@vger.kernel.org, linux-input '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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drivers: input: touchscreen: sur40: use static variable instead of stack varialbe for 'packet_id' 2013-11-27 2:15 ` [PATCH] drivers: input: touchscreen: sur40: use static variable instead of stack varialbe for 'packet_id' Chen Gang @ 2013-11-28 4:07 ` Dmitry Torokhov 2013-11-28 4:24 ` Chen Gang 2013-11-28 4:34 ` [PATCH v2] drivers: input: touchscreen: sur40: remove stack variable 'packet_id' from sur40_poll() Chen Gang 0 siblings, 2 replies; 4+ messages in thread From: Dmitry Torokhov @ 2013-11-28 4:07 UTC (permalink / raw) To: Chen Gang Cc: floe, rydberg, David Herrmann, rkuo, linux-kernel@vger.kernel.org, linux-input 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drivers: input: touchscreen: sur40: use static variable instead of stack varialbe for 'packet_id' 2013-11-28 4:07 ` Dmitry Torokhov @ 2013-11-28 4:24 ` Chen Gang 2013-11-28 4:34 ` [PATCH v2] drivers: input: touchscreen: sur40: remove stack variable 'packet_id' from sur40_poll() Chen Gang 1 sibling, 0 replies; 4+ messages in thread From: Chen Gang @ 2013-11-28 4:24 UTC (permalink / raw) To: Dmitry Torokhov Cc: floe, rydberg, David Herrmann, rkuo, linux-kernel@vger.kernel.org, linux-input 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] drivers: input: touchscreen: sur40: remove stack variable 'packet_id' from sur40_poll() 2013-11-28 4:07 ` Dmitry Torokhov 2013-11-28 4:24 ` Chen Gang @ 2013-11-28 4:34 ` Chen Gang 1 sibling, 0 replies; 4+ messages in thread From: Chen Gang @ 2013-11-28 4:34 UTC (permalink / raw) To: Dmitry Torokhov Cc: floe, rydberg, David Herrmann, rkuo, linux-kernel@vger.kernel.org, linux-input 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-11-28 4:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <528AD6B3.1060507@gmail.com>
[not found] ` <20131125011908.GA18921@codeaurora.org>
2013-11-27 2:15 ` [PATCH] drivers: input: touchscreen: sur40: use static variable instead of stack varialbe for 'packet_id' Chen Gang
2013-11-28 4:07 ` Dmitry Torokhov
2013-11-28 4:24 ` Chen Gang
2013-11-28 4:34 ` [PATCH v2] drivers: input: touchscreen: sur40: remove stack variable 'packet_id' from sur40_poll() Chen Gang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).