Re: [PATCH] hid: hid-chicony: fix switch case indentation
From: Joe Perches <joe@perches.com>
Date: 2021-05-29 07:57:33
Also in:
lkml
Subsystem:
hid core layer, the rest · Maintainers:
Jiri Kosina, Benjamin Tissoires, Linus Torvalds
On Sat, 2021-05-29 at 12:48 +0530, Navin Sankar Velliangiri wrote:
fixed switch case indentation.
Please try not to merely fix checkpatch warnings. Instead try to improve the code. And there's nothing _really_ wrong with the existing code but:
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-chicony.c b/drivers/hid/hid-chicony.c
[]
quoted hunk ↗ jump to hunk
@@ -65,26 +65,61 @@ static int ch_input_mapping(struct hid_device *hdev, struct hid_input *hi,set_bit(EV_REP, hi->input->evbit); switch (usage->hid & HID_USAGE) { - case 0xff01: ch_map_key_clear(BTN_1); break; - case 0xff02: ch_map_key_clear(BTN_2); break;
[...]
+ case 0xff01: + ch_map_key_clear(BTN_1); + break; + case 0xff02: + ch_map_key_clear(BTN_2); + break;
[...]
default: return 0; } + return 1;
IMO: This might be (umm) clearer with a separate function. A lot smaller code too. $ size drivers/hid/hid-chicony.o* text data bss dec hex filename 1886 392 0 2278 8e6 drivers/hid/hid-chicony.o.new 3329 392 0 3721 e89 drivers/hid/hid-chicony.o.old Something like: --- drivers/hid/hid-chicony.c | 52 +++++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 22 deletions(-)
diff --git a/drivers/hid/hid-chicony.c b/drivers/hid/hid-chicony.c
index ca556d39da2ae..03e9a1d943d96 100644
--- a/drivers/hid/hid-chicony.c
+++ b/drivers/hid/hid-chicony.c@@ -54,37 +54,45 @@ static int ch_raw_event(struct hid_device *hdev, return 0; } -#define ch_map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max, \ - EV_KEY, (c)) +static int map_use_to_btn(int use) +{ + switch (use) { + case 0xff01: return BTN_1; + case 0xff02: return BTN_2; + case 0xff03: return BTN_3; + case 0xff04: return BTN_4; + case 0xff05: return BTN_5; + case 0xff06: return BTN_6; + case 0xff07: return BTN_7; + case 0xff08: return BTN_8; + case 0xff09: return BTN_9; + case 0xff0a: return BTN_A; + case 0xff0b: return BTN_B; + case 0x00f1: return KEY_WLAN; + case 0x00f2: return KEY_BRIGHTNESSDOWN; + case 0x00f3: return KEY_BRIGHTNESSUP; + case 0x00f4: return KEY_DISPLAY_OFF; + case 0x00f7: return KEY_CAMERA; + case 0x00f8: return KEY_PROG1; + } + return 0; +} + static int ch_input_mapping(struct hid_device *hdev, struct hid_input *hi, struct hid_field *field, struct hid_usage *usage, unsigned long **bit, int *max) { + int btn; + if ((usage->hid & HID_USAGE_PAGE) != HID_UP_MSVENDOR) return 0; set_bit(EV_REP, hi->input->evbit); - switch (usage->hid & HID_USAGE) { - case 0xff01: ch_map_key_clear(BTN_1); break; - case 0xff02: ch_map_key_clear(BTN_2); break; - case 0xff03: ch_map_key_clear(BTN_3); break; - case 0xff04: ch_map_key_clear(BTN_4); break; - case 0xff05: ch_map_key_clear(BTN_5); break; - case 0xff06: ch_map_key_clear(BTN_6); break; - case 0xff07: ch_map_key_clear(BTN_7); break; - case 0xff08: ch_map_key_clear(BTN_8); break; - case 0xff09: ch_map_key_clear(BTN_9); break; - case 0xff0a: ch_map_key_clear(BTN_A); break; - case 0xff0b: ch_map_key_clear(BTN_B); break; - case 0x00f1: ch_map_key_clear(KEY_WLAN); break; - case 0x00f2: ch_map_key_clear(KEY_BRIGHTNESSDOWN); break; - case 0x00f3: ch_map_key_clear(KEY_BRIGHTNESSUP); break; - case 0x00f4: ch_map_key_clear(KEY_DISPLAY_OFF); break; - case 0x00f7: ch_map_key_clear(KEY_CAMERA); break; - case 0x00f8: ch_map_key_clear(KEY_PROG1); break; - default: + btn = map_use_to_btn(usage->hid & HID_USAGE); + if (!btn) return 0; - } + + hid_map_usage_clear(hi, usage, bit, max, EV_KEY, btn); return 1; }