* [PATCH] Input: mousedev - fix implicit conversion warning @ 2017-05-26 5:22 Nick Desaulniers 2017-05-26 15:34 ` [PATCH v2] " Nick Desaulniers 0 siblings, 1 reply; 13+ messages in thread From: Nick Desaulniers @ 2017-05-26 5:22 UTC (permalink / raw) Cc: gregkh, Nick Desaulniers, Dmitry Torokhov, linux-input, linux-kernel Clang warns: drivers/input/mousedev.c:653:63: error: implicit conversion from 'int' to 'signed char' changes value from 200 to -56 [-Wconstant-conversion] client->ps2[1] = 0x60; client->ps2[2] = 3; client->ps2[3] = 200; ~ ^~~ As far as I can tell, from http://www.computer-engineering.org/ps2mouse/ Under "Command Set" > "0xE9 (Status Request)" the value 200 is a valid sample rate. An explicit cast silences this warning. Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> --- drivers/input/mousedev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c index 0e0ff84088fd..816e2431bba8 100644 --- a/drivers/input/mousedev.c +++ b/drivers/input/mousedev.c @@ -650,7 +650,9 @@ static void mousedev_generate_response(struct mousedev_client *client, break; case 0xe9: /* Get info */ - client->ps2[1] = 0x60; client->ps2[2] = 3; client->ps2[3] = 200; + client->ps2[1] = 0x60; + client->ps2[2] = 3; + client->ps2[3] = (char) 200; client->bufsiz = 4; break; -- 2.11.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2] Input: mousedev - fix implicit conversion warning 2017-05-26 5:22 [PATCH] Input: mousedev - fix implicit conversion warning Nick Desaulniers @ 2017-05-26 15:34 ` Nick Desaulniers 2017-05-26 15:40 ` [PATCH v3] " Nick Desaulniers 0 siblings, 1 reply; 13+ messages in thread From: Nick Desaulniers @ 2017-05-26 15:34 UTC (permalink / raw) Cc: gregkh, Nick Desaulniers, Dmitry Torokhov, linux-input, linux-kernel Clang warns: drivers/input/mousedev.c:653:63: error: implicit conversion from 'int' to 'signed char' changes value from 200 to -56 [-Wconstant-conversion] client->ps2[1] = 0x60; client->ps2[2] = 3; client->ps2[3] = 200; ~ ^~~ As far as I can tell, from http://www.computer-engineering.org/ps2mouse/ Under "Command Set" > "0xE9 (Status Request)" the value 200 is a valid sample rate. Using unsigned char, rather than signed char, for client->ps2 silences this warning. Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> --- * Changed from using an explicit cast to changing the signedness of the declaration. drivers/input/mousedev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c index 0e0ff84088fd..c83688eb2ef4 100644 --- a/drivers/input/mousedev.c +++ b/drivers/input/mousedev.c @@ -103,7 +103,7 @@ struct mousedev_client { spinlock_t packet_lock; int pos_x, pos_y; - signed char ps2[6]; + unsigned char ps2[6]; unsigned char ready, buffer, bufsiz; unsigned char imexseq, impsseq; enum mousedev_emul mode; -- 2.11.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v3] Input: mousedev - fix implicit conversion warning 2017-05-26 15:34 ` [PATCH v2] " Nick Desaulniers @ 2017-05-26 15:40 ` Nick Desaulniers 2017-05-26 17:07 ` Dmitry Torokhov 2017-05-30 5:41 ` [PATCH v4] " Nick Desaulniers 0 siblings, 2 replies; 13+ messages in thread From: Nick Desaulniers @ 2017-05-26 15:40 UTC (permalink / raw) Cc: gregkh, Nick Desaulniers, Dmitry Torokhov, linux-input, linux-kernel Clang warns: drivers/input/mousedev.c:653:63: error: implicit conversion from 'int' to 'signed char' changes value from 200 to -56 [-Wconstant-conversion] client->ps2[1] = 0x60; client->ps2[2] = 3; client->ps2[3] = 200; ~ ^~~ As far as I can tell, from http://www.computer-engineering.org/ps2mouse/ Under "Command Set" > "0xE9 (Status Request)" the value 200 is a valid sample rate. Using unsigned char, rather than signed char, for client->ps2 silences this warning. Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> --- * Additionally change function signature for the lone function dealing with ps2 data. drivers/input/mousedev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c index 0e0ff84088fd..0e31a109b1b4 100644 --- a/drivers/input/mousedev.c +++ b/drivers/input/mousedev.c @@ -103,7 +103,7 @@ struct mousedev_client { spinlock_t packet_lock; int pos_x, pos_y; - signed char ps2[6]; + unsigned char ps2[6]; unsigned char ready, buffer, bufsiz; unsigned char imexseq, impsseq; enum mousedev_emul mode; @@ -577,7 +577,7 @@ static inline int mousedev_limit_delta(int delta, int limit) } static void mousedev_packet(struct mousedev_client *client, - signed char *ps2_data) + unsigned char *ps2_data) { struct mousedev_motion *p = &client->packets[client->tail]; -- 2.11.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3] Input: mousedev - fix implicit conversion warning 2017-05-26 15:40 ` [PATCH v3] " Nick Desaulniers @ 2017-05-26 17:07 ` Dmitry Torokhov 2017-05-29 19:22 ` Nick Desaulniers 2017-05-30 5:41 ` [PATCH v4] " Nick Desaulniers 1 sibling, 1 reply; 13+ messages in thread From: Dmitry Torokhov @ 2017-05-26 17:07 UTC (permalink / raw) To: Nick Desaulniers; +Cc: gregkh, linux-input, linux-kernel On Fri, May 26, 2017 at 08:40:28AM -0700, Nick Desaulniers wrote: > Clang warns: > > drivers/input/mousedev.c:653:63: error: implicit conversion from 'int' > to 'signed char' changes value from 200 to -56 > [-Wconstant-conversion] > client->ps2[1] = 0x60; client->ps2[2] = 3; client->ps2[3] = 200; > ~ ^~~ > > As far as I can tell, from > > http://www.computer-engineering.org/ps2mouse/ > > Under "Command Set" > "0xE9 (Status Request)" > > the value 200 is a valid sample rate. Using unsigned char, rather than > signed char, for client->ps2 silences this warning. > > Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> > --- > * Additionally change function signature for the lone function dealing > with ps2 data. > > drivers/input/mousedev.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c > index 0e0ff84088fd..0e31a109b1b4 100644 > --- a/drivers/input/mousedev.c > +++ b/drivers/input/mousedev.c > @@ -103,7 +103,7 @@ struct mousedev_client { > spinlock_t packet_lock; > int pos_x, pos_y; > > - signed char ps2[6]; > + unsigned char ps2[6]; > unsigned char ready, buffer, bufsiz; > unsigned char imexseq, impsseq; > enum mousedev_emul mode; > @@ -577,7 +577,7 @@ static inline int mousedev_limit_delta(int delta, int limit) > } > > static void mousedev_packet(struct mousedev_client *client, > - signed char *ps2_data) > + unsigned char *ps2_data) > { > struct mousedev_motion *p = &client->packets[client->tail]; If you look at the code of this fucntion we really use ps2_data as signed in calculations, and this change would break that. While making ps2_data u8 might be beneficial we'd need to rework mousedev_packet() to use signed temporaries for dx, dy and dz before stufifng them into ps2_data. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3] Input: mousedev - fix implicit conversion warning 2017-05-26 17:07 ` Dmitry Torokhov @ 2017-05-29 19:22 ` Nick Desaulniers 2017-05-30 2:44 ` Dmitry Torokhov 0 siblings, 1 reply; 13+ messages in thread From: Nick Desaulniers @ 2017-05-29 19:22 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: gregkh, linux-input, linux-kernel On Fri, May 26, 2017 at 10:07:46AM -0700, Dmitry Torokhov wrote: > If you look at the code of this fucntion we really use ps2_data as > signed in calculations, and this change would break that. While making > ps2_data u8 might be beneficial we'd need to rework mousedev_packet() to > use signed temporaries for dx, dy and dz before stufifng them into > ps2_data. Is your recommendation then to stick with the simple cast as in V1 of this patch, or stick with u8 and rework mousedev_packet()? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3] Input: mousedev - fix implicit conversion warning 2017-05-29 19:22 ` Nick Desaulniers @ 2017-05-30 2:44 ` Dmitry Torokhov 0 siblings, 0 replies; 13+ messages in thread From: Dmitry Torokhov @ 2017-05-30 2:44 UTC (permalink / raw) To: Nick Desaulniers; +Cc: gregkh, linux-input, linux-kernel On Mon, May 29, 2017 at 12:22:52PM -0700, Nick Desaulniers wrote: > On Fri, May 26, 2017 at 10:07:46AM -0700, Dmitry Torokhov wrote: > > If you look at the code of this fucntion we really use ps2_data as > > signed in calculations, and this change would break that. While making > > ps2_data u8 might be beneficial we'd need to rework mousedev_packet() to > > use signed temporaries for dx, dy and dz before stufifng them into > > ps2_data. > > Is your recommendation then to stick with the simple cast as in V1 of > this patch, or stick with u8 and rework mousedev_packet()? I think casts are often mysterious, so, unless we'll end up with worse object code, I'd switch to u8 and temps. -- Dmitry ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4] Input: mousedev - fix implicit conversion warning 2017-05-26 15:40 ` [PATCH v3] " Nick Desaulniers 2017-05-26 17:07 ` Dmitry Torokhov @ 2017-05-30 5:41 ` Nick Desaulniers 2017-06-06 16:18 ` Nick Desaulniers ` (2 more replies) 1 sibling, 3 replies; 13+ messages in thread From: Nick Desaulniers @ 2017-05-30 5:41 UTC (permalink / raw) Cc: Nick Desaulniers, Dmitry Torokhov, linux-input, linux-kernel Clang warns: drivers/input/mousedev.c:653:63: error: implicit conversion from 'int' to 'signed char' changes value from 200 to -56 [-Wconstant-conversion] client->ps2[1] = 0x60; client->ps2[2] = 3; client->ps2[3] = 200; ~ ^~~ As far as I can tell, from http://www.computer-engineering.org/ps2mouse/ Under "Command Set" > "0xE9 (Status Request)" the value 200 is a valid sample rate. Using unsigned char [], rather than signed char [], for client->ps2 silences this warning. Also moves some reused logic into a helper function. Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> --- What's new in v4: * replace mousedev_limit_delta() with update_clamped() that also updates the ps2_data and delta values. The use of the temporary val should avoid integral conversion and promotion issues. drivers/input/mousedev.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c index 0e0ff84088fd..e645b8c6f2cb 100644 --- a/drivers/input/mousedev.c +++ b/drivers/input/mousedev.c @@ -103,7 +103,7 @@ struct mousedev_client { spinlock_t packet_lock; int pos_x, pos_y; - signed char ps2[6]; + unsigned char ps2[6]; unsigned char ready, buffer, bufsiz; unsigned char imexseq, impsseq; enum mousedev_emul mode; @@ -571,27 +571,27 @@ static int mousedev_open(struct inode *inode, struct file *file) return error; } -static inline int mousedev_limit_delta(int delta, int limit) +static inline void +update_clamped(unsigned char *ps2_data, int *delta, int limit) { - return delta > limit ? limit : (delta < -limit ? -limit : delta); + int val = *delta > limit ? limit : (*delta < -limit ? -limit : *delta); + *ps2_data = (unsigned char) val; + *delta -= val; } static void mousedev_packet(struct mousedev_client *client, - signed char *ps2_data) + unsigned char *ps2_data) { struct mousedev_motion *p = &client->packets[client->tail]; ps2_data[0] = 0x08 | ((p->dx < 0) << 4) | ((p->dy < 0) << 5) | (p->buttons & 0x07); - ps2_data[1] = mousedev_limit_delta(p->dx, 127); - ps2_data[2] = mousedev_limit_delta(p->dy, 127); - p->dx -= ps2_data[1]; - p->dy -= ps2_data[2]; + update_clamped(&ps2_data[1], &p->dx, 127); + update_clamped(&ps2_data[2], &p->dy, 127); switch (client->mode) { case MOUSEDEV_EMUL_EXPS: - ps2_data[3] = mousedev_limit_delta(p->dz, 7); - p->dz -= ps2_data[3]; + update_clamped(&ps2_data[3], &p->dz, 7); ps2_data[3] = (ps2_data[3] & 0x0f) | ((p->buttons & 0x18) << 1); client->bufsiz = 4; break; @@ -599,8 +599,7 @@ static void mousedev_packet(struct mousedev_client *client, case MOUSEDEV_EMUL_IMPS: ps2_data[0] |= ((p->buttons & 0x10) >> 3) | ((p->buttons & 0x08) >> 1); - ps2_data[3] = mousedev_limit_delta(p->dz, 127); - p->dz -= ps2_data[3]; + update_clamped(&ps2_data[3], &p->dz, 127); client->bufsiz = 4; break; -- 2.11.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4] Input: mousedev - fix implicit conversion warning 2017-05-30 5:41 ` [PATCH v4] " Nick Desaulniers @ 2017-06-06 16:18 ` Nick Desaulniers 2017-06-23 4:16 ` Nick Desaulniers 2017-06-25 18:06 ` Dmitry Torokhov 2 siblings, 0 replies; 13+ messages in thread From: Nick Desaulniers @ 2017-06-06 16:18 UTC (permalink / raw) To: Dmitry Torokhov, linux-input, linux-kernel ping for re-review ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] Input: mousedev - fix implicit conversion warning 2017-05-30 5:41 ` [PATCH v4] " Nick Desaulniers 2017-06-06 16:18 ` Nick Desaulniers @ 2017-06-23 4:16 ` Nick Desaulniers 2017-06-25 18:06 ` Dmitry Torokhov 2 siblings, 0 replies; 13+ messages in thread From: Nick Desaulniers @ 2017-06-23 4:16 UTC (permalink / raw) To: Dmitry Torokhov, linux-input, linux-kernel Hi Dmitry, did you have more feedback for this patch? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] Input: mousedev - fix implicit conversion warning 2017-05-30 5:41 ` [PATCH v4] " Nick Desaulniers 2017-06-06 16:18 ` Nick Desaulniers 2017-06-23 4:16 ` Nick Desaulniers @ 2017-06-25 18:06 ` Dmitry Torokhov 2017-06-27 1:39 ` Nick Desaulniers 2017-07-12 6:57 ` Nick Desaulniers 2 siblings, 2 replies; 13+ messages in thread From: Dmitry Torokhov @ 2017-06-25 18:06 UTC (permalink / raw) To: Nick Desaulniers; +Cc: linux-input, linux-kernel Hi Nick, On Mon, May 29, 2017 at 10:41:51PM -0700, Nick Desaulniers wrote: > Clang warns: > > drivers/input/mousedev.c:653:63: error: implicit conversion from 'int' > to 'signed char' changes value from 200 to -56 > [-Wconstant-conversion] > client->ps2[1] = 0x60; client->ps2[2] = 3; client->ps2[3] = 200; > ~ ^~~ > > As far as I can tell, from > > http://www.computer-engineering.org/ps2mouse/ > > Under "Command Set" > "0xE9 (Status Request)" > > the value 200 is a valid sample rate. Using unsigned char [], rather than > signed char [], for client->ps2 silences this warning. > > Also moves some reused logic into a helper function. > > Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> > --- > What's new in v4: > * replace mousedev_limit_delta() with update_clamped() that also updates > the ps2_data and delta values. The use of the temporary val should > avoid integral conversion and promotion issues. > > drivers/input/mousedev.c | 23 +++++++++++------------ > 1 file changed, 11 insertions(+), 12 deletions(-) > > diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c > index 0e0ff84088fd..e645b8c6f2cb 100644 > --- a/drivers/input/mousedev.c > +++ b/drivers/input/mousedev.c > @@ -103,7 +103,7 @@ struct mousedev_client { > spinlock_t packet_lock; > int pos_x, pos_y; > > - signed char ps2[6]; > + unsigned char ps2[6]; > unsigned char ready, buffer, bufsiz; > unsigned char imexseq, impsseq; > enum mousedev_emul mode; > @@ -571,27 +571,27 @@ static int mousedev_open(struct inode *inode, struct file *file) > return error; > } > > -static inline int mousedev_limit_delta(int delta, int limit) > +static inline void > +update_clamped(unsigned char *ps2_data, int *delta, int limit) > { > - return delta > limit ? limit : (delta < -limit ? -limit : delta); > + int val = *delta > limit ? limit : (*delta < -limit ? -limit : *delta); > + *ps2_data = (unsigned char) val; > + *delta -= val; Since the time the code was written we got nice helpers to clamp the values. Does the following work for you or it still leaves clang unhappy? Thanks. -- Dmitry Input: mousedev - fix implicit conversion warning From: Nick Desaulniers <nick.desaulniers@gmail.com> Clang warns: drivers/input/mousedev.c:653:63: error: implicit conversion from 'int' to 'signed char' changes value from 200 to -56 [-Wconstant-conversion] client->ps2[1] = 0x60; client->ps2[2] = 3; client->ps2[3] = 200; ~ ^~~ As the PS2 data is really a stream of bytes, let's switch to using u8 type for it, which silences this warning. Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> Patchwork-Id: 9753771 Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- drivers/input/mousedev.c | 60 +++++++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c index 0e0ff84088fd..6ef0c5314f69 100644 --- a/drivers/input/mousedev.c +++ b/drivers/input/mousedev.c @@ -15,6 +15,7 @@ #define MOUSEDEV_MINORS 31 #define MOUSEDEV_MIX 63 +#include <linux/bitops.h> #include <linux/sched.h> #include <linux/slab.h> #include <linux/poll.h> @@ -103,7 +104,7 @@ struct mousedev_client { spinlock_t packet_lock; int pos_x, pos_y; - signed char ps2[6]; + u8 ps2[6]; unsigned char ready, buffer, bufsiz; unsigned char imexseq, impsseq; enum mousedev_emul mode; @@ -291,11 +292,10 @@ static void mousedev_notify_readers(struct mousedev *mousedev, } client->pos_x += packet->dx; - client->pos_x = client->pos_x < 0 ? - 0 : (client->pos_x >= xres ? xres : client->pos_x); + client->pos_x = clamp_val(client->pos_x, 0, xres); + client->pos_y += packet->dy; - client->pos_y = client->pos_y < 0 ? - 0 : (client->pos_y >= yres ? yres : client->pos_y); + client->pos_y = clamp_val(client->pos_y, 0, yres); p->dx += packet->dx; p->dy += packet->dy; @@ -571,44 +571,48 @@ static int mousedev_open(struct inode *inode, struct file *file) return error; } -static inline int mousedev_limit_delta(int delta, int limit) -{ - return delta > limit ? limit : (delta < -limit ? -limit : delta); -} - -static void mousedev_packet(struct mousedev_client *client, - signed char *ps2_data) +static void mousedev_packet(struct mousedev_client *client, u8 *ps2_data) { struct mousedev_motion *p = &client->packets[client->tail]; + s8 dx, dy, dz; + + dx = clamp_val(p->dx, -127, 127); + p->dx -= dx; + + dy = clamp_val(p->dy, -127, 127); + p->dy -= dy; - ps2_data[0] = 0x08 | - ((p->dx < 0) << 4) | ((p->dy < 0) << 5) | (p->buttons & 0x07); - ps2_data[1] = mousedev_limit_delta(p->dx, 127); - ps2_data[2] = mousedev_limit_delta(p->dy, 127); - p->dx -= ps2_data[1]; - p->dy -= ps2_data[2]; + ps2_data[0] = BIT(3); + ps2_data[0] |= ((dx & BIT(7)) >> 3) | ((dy & BIT(7)) >> 2); + ps2_data[0] |= p->buttons & 0x07; switch (client->mode) { case MOUSEDEV_EMUL_EXPS: - ps2_data[3] = mousedev_limit_delta(p->dz, 7); - p->dz -= ps2_data[3]; - ps2_data[3] = (ps2_data[3] & 0x0f) | ((p->buttons & 0x18) << 1); + dz = clamp_val(p->dz, -7, 7); + p->dz -= dz; + + ps2_data[3] = (dz & 0x0f) | ((p->buttons & 0x18) << 1); client->bufsiz = 4; break; case MOUSEDEV_EMUL_IMPS: - ps2_data[0] |= - ((p->buttons & 0x10) >> 3) | ((p->buttons & 0x08) >> 1); - ps2_data[3] = mousedev_limit_delta(p->dz, 127); - p->dz -= ps2_data[3]; + dz = clamp_val(p->dz, -127, 127); + p->dz -= dz; + + ps2_data[0] |= ((p->buttons & 0x10) >> 3) | + ((p->buttons & 0x08) >> 1); + ps2_data[3] = dz; + client->bufsiz = 4; break; case MOUSEDEV_EMUL_PS2: default: - ps2_data[0] |= - ((p->buttons & 0x10) >> 3) | ((p->buttons & 0x08) >> 1); p->dz = 0; + + ps2_data[0] |= ((p->buttons & 0x10) >> 3) | + ((p->buttons & 0x08) >> 1); + client->bufsiz = 3; break; } @@ -714,7 +718,7 @@ static ssize_t mousedev_read(struct file *file, char __user *buffer, { struct mousedev_client *client = file->private_data; struct mousedev *mousedev = client->mousedev; - signed char data[sizeof(client->ps2)]; + u8 data[sizeof(client->ps2)]; int retval = 0; if (!client->ready && !client->buffer && mousedev->exist && ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4] Input: mousedev - fix implicit conversion warning 2017-06-25 18:06 ` Dmitry Torokhov @ 2017-06-27 1:39 ` Nick Desaulniers 2017-07-12 6:57 ` Nick Desaulniers 1 sibling, 0 replies; 13+ messages in thread From: Nick Desaulniers @ 2017-06-27 1:39 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-input, linux-kernel On Sun, Jun 25, 2017 at 11:06:09AM -0700, Dmitry Torokhov wrote: > Hi Nick, > > Since the time the code was written we got nice helpers to clamp the > values. Does the following work for you or it still leaves clang > unhappy? LGTM, no more warnings with Clang. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] Input: mousedev - fix implicit conversion warning 2017-06-25 18:06 ` Dmitry Torokhov 2017-06-27 1:39 ` Nick Desaulniers @ 2017-07-12 6:57 ` Nick Desaulniers 2017-07-12 18:19 ` Dmitry Torokhov 1 sibling, 1 reply; 13+ messages in thread From: Nick Desaulniers @ 2017-07-12 6:57 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-input, linux-kernel Hi Dmitry, did you get a chance to merge the sugguested revision, yet? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] Input: mousedev - fix implicit conversion warning 2017-07-12 6:57 ` Nick Desaulniers @ 2017-07-12 18:19 ` Dmitry Torokhov 0 siblings, 0 replies; 13+ messages in thread From: Dmitry Torokhov @ 2017-07-12 18:19 UTC (permalink / raw) To: Nick Desaulniers; +Cc: linux-input, linux-kernel On Tue, Jul 11, 2017 at 11:57:10PM -0700, Nick Desaulniers wrote: > Hi Dmitry, did you get a chance to merge the sugguested revision, yet? Sorry, no, as it was broken (we lost the assignment to the bytes 2 and 3 of the PS/2 data stream). Here is the version that seems to work, but at this time I guess I'll push it out to 4.14. -- Dmitry Input: mousedev - fix implicit conversion warning From: Nick Desaulniers <nick.desaulniers@gmail.com> Clang warns: drivers/input/mousedev.c:653:63: error: implicit conversion from 'int' to 'signed char' changes value from 200 to -56 [-Wconstant-conversion] client->ps2[1] = 0x60; client->ps2[2] = 3; client->ps2[3] = 200; ~ ^~~ As the PS2 data is really a stream of bytes, let's switch to using u8 type for it, which silences this warning. Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> Patchwork-Id: 9753771 Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- drivers/input/mousedev.c | 62 +++++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c index 0e0ff84088fd..114730a0c3fa 100644 --- a/drivers/input/mousedev.c +++ b/drivers/input/mousedev.c @@ -15,6 +15,7 @@ #define MOUSEDEV_MINORS 31 #define MOUSEDEV_MIX 63 +#include <linux/bitops.h> #include <linux/sched.h> #include <linux/slab.h> #include <linux/poll.h> @@ -103,7 +104,7 @@ struct mousedev_client { spinlock_t packet_lock; int pos_x, pos_y; - signed char ps2[6]; + u8 ps2[6]; unsigned char ready, buffer, bufsiz; unsigned char imexseq, impsseq; enum mousedev_emul mode; @@ -291,11 +292,10 @@ static void mousedev_notify_readers(struct mousedev *mousedev, } client->pos_x += packet->dx; - client->pos_x = client->pos_x < 0 ? - 0 : (client->pos_x >= xres ? xres : client->pos_x); + client->pos_x = clamp_val(client->pos_x, 0, xres); + client->pos_y += packet->dy; - client->pos_y = client->pos_y < 0 ? - 0 : (client->pos_y >= yres ? yres : client->pos_y); + client->pos_y = clamp_val(client->pos_y, 0, yres); p->dx += packet->dx; p->dy += packet->dy; @@ -571,44 +571,50 @@ static int mousedev_open(struct inode *inode, struct file *file) return error; } -static inline int mousedev_limit_delta(int delta, int limit) -{ - return delta > limit ? limit : (delta < -limit ? -limit : delta); -} - -static void mousedev_packet(struct mousedev_client *client, - signed char *ps2_data) +static void mousedev_packet(struct mousedev_client *client, u8 *ps2_data) { struct mousedev_motion *p = &client->packets[client->tail]; + s8 dx, dy, dz; + + dx = clamp_val(p->dx, -127, 127); + p->dx -= dx; + + dy = clamp_val(p->dy, -127, 127); + p->dy -= dy; - ps2_data[0] = 0x08 | - ((p->dx < 0) << 4) | ((p->dy < 0) << 5) | (p->buttons & 0x07); - ps2_data[1] = mousedev_limit_delta(p->dx, 127); - ps2_data[2] = mousedev_limit_delta(p->dy, 127); - p->dx -= ps2_data[1]; - p->dy -= ps2_data[2]; + ps2_data[0] = BIT(3); + ps2_data[0] |= ((dx & BIT(7)) >> 3) | ((dy & BIT(7)) >> 2); + ps2_data[0] |= p->buttons & 0x07; + ps2_data[1] = dx; + ps2_data[2] = dy; switch (client->mode) { case MOUSEDEV_EMUL_EXPS: - ps2_data[3] = mousedev_limit_delta(p->dz, 7); - p->dz -= ps2_data[3]; - ps2_data[3] = (ps2_data[3] & 0x0f) | ((p->buttons & 0x18) << 1); + dz = clamp_val(p->dz, -7, 7); + p->dz -= dz; + + ps2_data[3] = (dz & 0x0f) | ((p->buttons & 0x18) << 1); client->bufsiz = 4; break; case MOUSEDEV_EMUL_IMPS: - ps2_data[0] |= - ((p->buttons & 0x10) >> 3) | ((p->buttons & 0x08) >> 1); - ps2_data[3] = mousedev_limit_delta(p->dz, 127); - p->dz -= ps2_data[3]; + dz = clamp_val(p->dz, -127, 127); + p->dz -= dz; + + ps2_data[0] |= ((p->buttons & 0x10) >> 3) | + ((p->buttons & 0x08) >> 1); + ps2_data[3] = dz; + client->bufsiz = 4; break; case MOUSEDEV_EMUL_PS2: default: - ps2_data[0] |= - ((p->buttons & 0x10) >> 3) | ((p->buttons & 0x08) >> 1); p->dz = 0; + + ps2_data[0] |= ((p->buttons & 0x10) >> 3) | + ((p->buttons & 0x08) >> 1); + client->bufsiz = 3; break; } @@ -714,7 +720,7 @@ static ssize_t mousedev_read(struct file *file, char __user *buffer, { struct mousedev_client *client = file->private_data; struct mousedev *mousedev = client->mousedev; - signed char data[sizeof(client->ps2)]; + u8 data[sizeof(client->ps2)]; int retval = 0; if (!client->ready && !client->buffer && mousedev->exist && ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-07-12 18:20 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-26 5:22 [PATCH] Input: mousedev - fix implicit conversion warning Nick Desaulniers 2017-05-26 15:34 ` [PATCH v2] " Nick Desaulniers 2017-05-26 15:40 ` [PATCH v3] " Nick Desaulniers 2017-05-26 17:07 ` Dmitry Torokhov 2017-05-29 19:22 ` Nick Desaulniers 2017-05-30 2:44 ` Dmitry Torokhov 2017-05-30 5:41 ` [PATCH v4] " Nick Desaulniers 2017-06-06 16:18 ` Nick Desaulniers 2017-06-23 4:16 ` Nick Desaulniers 2017-06-25 18:06 ` Dmitry Torokhov 2017-06-27 1:39 ` Nick Desaulniers 2017-07-12 6:57 ` Nick Desaulniers 2017-07-12 18:19 ` Dmitry Torokhov
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).