Re: [PATCH] hid: hid-chicony: fix switch case indentation

From: Joe Perches
Date: Sat May 29 2021 - 04:06:38 EST


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:

> diff --git a/drivers/hid/hid-chicony.c b/drivers/hid/hid-chicony.c
[]
> @@ -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;
}