[OPW,kernel] Staging: speakup: Using another if instead of jiffies comparison
diff mbox

Message ID 20131017145437.GA4310@Ashvini
State Rejected
Headers show

Commit Message

Ashvini Varatharaj Oct. 17, 2013, 2:54 p.m. UTC
Fix chechpatch warning: WARNING: Comparing jiffies is almost always
wrong; prefer time_after, time_before and friends

Signed-off-by: Ashvini Varatharaj <ashvinivaratharaj@gmail.com>
---
 drivers/staging/speakup/Module.symvers |   25 +++++++++++++++++++++++++
 drivers/staging/speakup/main.c         |   14 +++++++-------
 2 files changed, 32 insertions(+), 7 deletions(-)
 create mode 100644 drivers/staging/speakup/Module.symvers

Comments

Greg Kroah-Hartman Oct. 17, 2013, 4:15 p.m. UTC | #1
On Thu, Oct 17, 2013 at 08:24:37PM +0530, Ashvini Varatharaj wrote:
> Fix chechpatch warning: WARNING: Comparing jiffies is almost always
> wrong; prefer time_after, time_before and friends
> 
> Signed-off-by: Ashvini Varatharaj <ashvinivaratharaj@gmail.com>
> ---
>  drivers/staging/speakup/Module.symvers |   25 +++++++++++++++++++++++++

PLEASE look at your patches to verify that you really are including only
what you need to include in them.  Again, this file should never be in a
patch, and in fact, if it is, something is really broken in the process
you are using to create a patch.  Git should be ignoring this type of
file, which makes me curious as to how this patch was even created in
the first place?

> --- a/drivers/staging/speakup/main.c
> +++ b/drivers/staging/speakup/main.c
> @@ -2068,14 +2068,14 @@ speakup_key(struct vc_data *vc, int shift_state, int keycode, u_short keysym,
>  			}
>  			if (up_flag)
>  				goto out;
> -			if (last_keycode == keycode &&
> -			    last_spk_jiffy + MAX_DELAY > jiffies) {
> -				spk_close_press = 1;
> -				offset = spk_shift_table[shift_info + 32];
> +	if (last_keycode == keycode)
> +		if (time_after((last_spk_jiffy + MAX_DELAY), jiffies)) {
> +			spk_close_press = 1;
> +			offset = spk_shift_table[shift_info + 32];
>  				/* double press? */
> -				if (offset && key_info[offset])
> -					new_key = key_info[offset];
> -			}
> +			if (offset && key_info[offset])
> +				new_key = key_info[offset];
> +		}

It's odd that checkpatch.pl didn't complain about this, but please, look
at the code and see if what you did makes sense with the indentation...

thanks,

greg k-h

Patch
diff mbox

diff --git a/drivers/staging/speakup/Module.symvers b/drivers/staging/speakup/Module.symvers
new file mode 100644
index 0000000..0eb706a
--- /dev/null
+++ b/drivers/staging/speakup/Module.symvers
@@ -0,0 +1,25 @@ 
+0xa8d01f22	synth_remove	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0xdd825f1a	synth_add	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0xe7e810f8	spk_serial_in	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0x475e158a	synth_request_region	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0x541ad416	spk_var_show	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0xbbd15a51	speakup_start_ttys	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0xc766ae09	synth_printf	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0x4449e1dd	synth_buffer_clear	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0x9a888082	synth_buffer_getc	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0xde7ae2e8	speakup_info	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0x37cdb397	spk_var_store	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0x0bfad5d7	spk_synth_is_alive_restart	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0x0e81c76d	spk_serial_out	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0x14f06fe7	spk_serial_in_nowait	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0xcda38867	speakup_event	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0x8e146195	synth_release_region	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0xf0d524fb	spk_do_catch_up	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0x41a160e5	synth_buffer_empty	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0x0dee3a00	spk_synth_is_alive_nop	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0xb48956f8	synth_buffer_peek	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0x8d66d3f3	spk_synth_flush	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0xb67b6d52	spk_serial_synth_probe	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0xce6ebc6c	spk_synth_immediate	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0xd1b551a2	spk_get_var	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
+0xe7cd4558	spk_serial_release	drivers/staging/speakup/speakup	EXPORT_SYMBOL_GPL
diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index 47502fa..1836aa1 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -2068,14 +2068,14 @@  speakup_key(struct vc_data *vc, int shift_state, int keycode, u_short keysym,
 			}
 			if (up_flag)
 				goto out;
-			if (last_keycode == keycode &&
-			    last_spk_jiffy + MAX_DELAY > jiffies) {
-				spk_close_press = 1;
-				offset = spk_shift_table[shift_info + 32];
+	if (last_keycode == keycode)
+		if (time_after((last_spk_jiffy + MAX_DELAY), jiffies)) {
+			spk_close_press = 1;
+			offset = spk_shift_table[shift_info + 32];
 				/* double press? */
-				if (offset && key_info[offset])
-					new_key = key_info[offset];
-			}
+			if (offset && key_info[offset])
+				new_key = key_info[offset];
+		}
 			last_keycode = keycode;
 			last_spk_jiffy = jiffies;
 			type = KT_SPKUP;