diff mbox series

[v2,1/4] HID: apple: Use common table for MacBook Pro fn mapping

Message ID CEFE855F-CC63-4361-8ABD-875BD5662294@live.com
State New
Headers show
Series [v2,1/4] HID: apple: Use common table for MacBook Pro fn mapping | expand

Commit Message

Aditya Garg Feb. 14, 2025, 10:05 a.m. UTC 
From: Aditya Garg <gargaditya08@live.com>

The only difference between the fn mapping of the MacBook Pros with esc key
and those without is of the presence of KEY_GRAVE in the translation table.

We can easily use a flag instead of writing the whole table again to omit
it from the models that have an esc key.

Additionally, APPLE_IGNORE_MOUSE quirk was unused in this driver, so has
been removed in this commit.

Signed-off-by: Aditya Garg <gargaditya08@live.com>
---
drivers/hid/hid-apple.c | 72 ++++++++++++++++-------------------------
1 file changed, 27 insertions(+), 45 deletions(-)

Comments

Jiri Kosina Feb. 18, 2025, 7:59 p.m. UTC | #1 
On Fri, 14 Feb 2025, Aditya Garg wrote:

> From: Aditya Garg <gargaditya08@live.com>
> 
> The only difference between the fn mapping of the MacBook Pros with esc key
> and those without is of the presence of KEY_GRAVE in the translation table.
> 
> We can easily use a flag instead of writing the whole table again to omit
> it from the models that have an esc key.
> 
> Additionally, APPLE_IGNORE_MOUSE quirk was unused in this driver, so has
> been removed in this commit.
> 
> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> ---

Why am I getting v1, [RESEND v1] and v2 in the timespan of 1 day without 
any documentation whatsoever what are the changes between the individual 
submission and why do we have so many of them?

Thanks in advance for clarification,
Aditya Garg Feb. 18, 2025, 8:08 p.m. UTC | #2 
Hi Jiri

> On 19 Feb 2025, at 1:29 AM, Jiri Kosina <jikos@kernel.org> wrote:
> 
> On Fri, 14 Feb 2025, Aditya Garg wrote:
> 
>> From: Aditya Garg <gargaditya08@live.com>
>> 
>> The only difference between the fn mapping of the MacBook Pros with esc key
>> and those without is of the presence of KEY_GRAVE in the translation table.
>> 
>> We can easily use a flag instead of writing the whole table again to omit
>> it from the models that have an esc key.
>> 
>> Additionally, APPLE_IGNORE_MOUSE quirk was unused in this driver, so has
>> been removed in this commit.
>> 
>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
>> ---
> 
> Why am I getting v1, [RESEND v1] and v2 in the timespan of 1 day without 
> any documentation whatsoever what are the changes between the individual 
> submission and why do we have so many of them?

I'll make sure changelog is provided next time.

RESEND was done because I forgot to Cc the mailing lists. Its my fault here

V2: I was suggested to use switch case by a colleague later that day, so added 4/4

> 
> Thanks in advance for clarification,
> 
> -- 
> Jiri Kosina
> SUSE Labs
Aditya Garg Feb. 19, 2025, 4:43 p.m. UTC | #3 
Hi Jiri

I would also like to mention that these patches were made from the for-next branch of your hid tree.

> On 19 Feb 2025, at 1:38 AM, Aditya Garg <gargaditya08@live.com> wrote:
> 
> Hi Jiri
> 
>> On 19 Feb 2025, at 1:29 AM, Jiri Kosina <jikos@kernel.org> wrote:
>> 
>>> On Fri, 14 Feb 2025, Aditya Garg wrote:
>>> 
>>> From: Aditya Garg <gargaditya08@live.com>
>>> 
>>> The only difference between the fn mapping of the MacBook Pros with esc key
>>> and those without is of the presence of KEY_GRAVE in the translation table.
>>> 
>>> We can easily use a flag instead of writing the whole table again to omit
>>> it from the models that have an esc key.
>>> 
>>> Additionally, APPLE_IGNORE_MOUSE quirk was unused in this driver, so has
>>> been removed in this commit.
>>> 
>>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
>>> ---
>> 
>> Why am I getting v1, [RESEND v1] and v2 in the timespan of 1 day without
>> any documentation whatsoever what are the changes between the individual
>> submission and why do we have so many of them?
> 
> I'll make sure changelog is provided next time.
> 
> RESEND was done because I forgot to Cc the mailing lists. Its my fault here
> 
> V2: I was suggested to use switch case by a colleague later that day, so added 4/4
> 
>> 
>> Thanks in advance for clarification,
>> 
>> --
>> Jiri Kosina
>> SUSE Labs
> 
>
Jiri Kosina March 4, 2025, 8:23 p.m. UTC | #4 
On Fri, 14 Feb 2025, Aditya Garg wrote:

> From: Aditya Garg <gargaditya08@live.com>
> 
> The only difference between the fn mapping of the MacBook Pros with esc key
> and those without is of the presence of KEY_GRAVE in the translation table.
> 
> We can easily use a flag instead of writing the whole table again to omit
> it from the models that have an esc key.
> 
> Additionally, APPLE_IGNORE_MOUSE quirk was unused in this driver, so has
> been removed in this commit.
> 
> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> ---
> drivers/hid/hid-apple.c | 72 ++++++++++++++++-------------------------
> 1 file changed, 27 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> index 49812a76b..e31c9e8e8 100644
> --- a/drivers/hid/hid-apple.c
> +++ b/drivers/hid/hid-apple.c
> @@ -30,7 +30,7 @@
> #include "hid-ids.h"
> 
> #define APPLE_RDESC_JIS		BIT(0)
> -#define APPLE_IGNORE_MOUSE	BIT(1)
> +/* BIT(1) reserved, was: APPLE_IGNORE_MOUSE */
> #define APPLE_HAS_FN		BIT(2)
> /* BIT(3) reserved, was: APPLE_HIDDEV */
> #define APPLE_ISO_TILDE_QUIRK	BIT(4)
> @@ -43,7 +43,8 @@
> #define APPLE_IS_NON_APPLE	BIT(11)
> #define APPLE_MAGIC_BACKLIGHT	BIT(12)

This patch is corrupt -- the context lines are missing the leading space. 
For some reason, it's only the 1/4 which is corrupted, the rest is fine.

Can you please look into this and resubmit?

Thanks,
Aditya Garg March 5, 2025, 3:33 a.m. UTC | #5 
Hi Jiri
> 
> This patch is corrupt -- the context lines are missing the leading space. 
> For some reason, it's only the 1/4 which is corrupted, the rest is fine.
> 
> Can you please look into this and resubmit?


I sent another patchset yesterday since common tables were becoming confusing for people, so dropped those patches.

Please review this patchset:

https://lore.kernel.org/linux-input/F4DF8D56-7095-43AE-A788-F10B8CE477B0@live.com/
Aditya Garg March 5, 2025, 4:11 a.m. UTC | #6 
> On 5 Mar 2025, at 9:03 AM, Aditya Garg <gargaditya08@live.com> wrote:
> 
> Hi Jiri
>> 
>> This patch is corrupt -- the context lines are missing the leading space. 
>> For some reason, it's only the 1/4 which is corrupted, the rest is fine.
>> 
>> Can you please look into this and resubmit?
> 
> 
> I sent another patchset yesterday since common tables were becoming confusing for people, so dropped those patches.
> 
> Please review this patchset:
> 
> https://lore.kernel.org/linux-input/F4DF8D56-7095-43AE-A788-F10B8CE477B0@live.com/


You may also want to review this small patch, which is related:

https://lore.kernel.org/linux-input/4CBC715A-59C2-4815-8D90-62683850E176@live.com/T/#u
diff mbox series

Patch

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 49812a76b..e31c9e8e8 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -30,7 +30,7 @@ 
#include "hid-ids.h"

#define APPLE_RDESC_JIS		BIT(0)
-#define APPLE_IGNORE_MOUSE	BIT(1)
+/* BIT(1) reserved, was: APPLE_IGNORE_MOUSE */
#define APPLE_HAS_FN		BIT(2)
/* BIT(3) reserved, was: APPLE_HIDDEV */
#define APPLE_ISO_TILDE_QUIRK	BIT(4)
@@ -43,7 +43,8 @@ 
#define APPLE_IS_NON_APPLE	BIT(11)
#define APPLE_MAGIC_BACKLIGHT	BIT(12)

-#define APPLE_FLAG_FKEY		0x01
+#define APPLE_FLAG_FKEY			0x01
+#define APPLE_FLAG_DONT_TRANSLATE	0x02

#define HID_COUNTRY_INTERNATIONAL_ISO	13
#define APPLE_BATTERY_TIMEOUT_MS	60000
@@ -89,6 +90,19 @@  struct apple_sc_backlight {
	struct hid_device *hdev;
};

+struct apple_backlight_config_report {
+	u8 report_id;
+	u8 version;
+	u16 backlight_off, backlight_on_min, backlight_on_max;
+};
+
+struct apple_backlight_set_report {
+	u8 report_id;
+	u8 version;
+	u16 backlight;
+	u16 rate;
+};
+
struct apple_magic_backlight {
	struct led_classdev cdev;
	struct hid_report *brightness;
@@ -152,20 +166,6 @@  static const struct apple_key_translation magic_keyboard_2015_fn_keys[] = {
	{ }
};

-struct apple_backlight_config_report {
-	u8 report_id;
-	u8 version;
-	u16 backlight_off, backlight_on_min, backlight_on_max;
-};
-
-struct apple_backlight_set_report {
-	u8 report_id;
-	u8 version;
-	u16 backlight;
-	u16 rate;
-};
-
-
static const struct apple_key_translation apple2021_fn_keys[] = {
	{ KEY_BACKSPACE, KEY_DELETE },
	{ KEY_ENTER,	KEY_INSERT },
@@ -209,32 +209,10 @@  static const struct apple_key_translation macbookair_fn_keys[] = {
	{ }
};

-static const struct apple_key_translation macbookpro_no_esc_fn_keys[] = {
-	{ KEY_BACKSPACE, KEY_DELETE },
-	{ KEY_ENTER,	KEY_INSERT },
-	{ KEY_GRAVE,	KEY_ESC },
-	{ KEY_1,	KEY_F1 },
-	{ KEY_2,	KEY_F2 },
-	{ KEY_3,	KEY_F3 },
-	{ KEY_4,	KEY_F4 },
-	{ KEY_5,	KEY_F5 },
-	{ KEY_6,	KEY_F6 },
-	{ KEY_7,	KEY_F7 },
-	{ KEY_8,	KEY_F8 },
-	{ KEY_9,	KEY_F9 },
-	{ KEY_0,	KEY_F10 },
-	{ KEY_MINUS,	KEY_F11 },
-	{ KEY_EQUAL,	KEY_F12 },
-	{ KEY_UP,	KEY_PAGEUP },
-	{ KEY_DOWN,	KEY_PAGEDOWN },
-	{ KEY_LEFT,	KEY_HOME },
-	{ KEY_RIGHT,	KEY_END },
-	{ }
-};
-
-static const struct apple_key_translation macbookpro_dedicated_esc_fn_keys[] = {
+static const struct apple_key_translation macbookpro_fn_keys[] = {
	{ KEY_BACKSPACE, KEY_DELETE },
	{ KEY_ENTER,	KEY_INSERT },
+	{ KEY_GRAVE,	KEY_ESC, APPLE_FLAG_DONT_TRANSLATE },
	{ KEY_1,	KEY_F1 },
	{ KEY_2,	KEY_F2 },
	{ KEY_3,	KEY_F3 },
@@ -415,6 +393,7 @@  static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
	struct apple_sc *asc = hid_get_drvdata(hid);
	const struct apple_key_translation *trans, *table;
	bool do_translate;
+	bool dont_translate_flagged_key = false;
	u16 code = usage->code;
	unsigned int real_fnmode;

@@ -481,14 +460,14 @@  static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
		else if (hid->product == USB_DEVICE_ID_APPLE_WELLSPRINGT2_J132 ||
			 hid->product == USB_DEVICE_ID_APPLE_WELLSPRINGT2_J680 ||
			 hid->product == USB_DEVICE_ID_APPLE_WELLSPRINGT2_J213)
-				table = macbookpro_no_esc_fn_keys;
+			table = macbookpro_fn_keys;
		else if (hid->product == USB_DEVICE_ID_APPLE_WELLSPRINGT2_J214K ||
			 hid->product == USB_DEVICE_ID_APPLE_WELLSPRINGT2_J223 ||
			 hid->product == USB_DEVICE_ID_APPLE_WELLSPRINGT2_J152F)
-				table = macbookpro_dedicated_esc_fn_keys;
+			table = macbookpro_fn_keys, dont_translate_flagged_key = true;
		else if (hid->product == USB_DEVICE_ID_APPLE_WELLSPRINGT2_J140K ||
			 hid->product == USB_DEVICE_ID_APPLE_WELLSPRINGT2_J230K)
-				table = apple_fn_keys;
+			table = apple_fn_keys;
		else if (hid->product >= USB_DEVICE_ID_APPLE_WELLSPRING4_ANSI &&
				hid->product <= USB_DEVICE_ID_APPLE_WELLSPRING4A_JIS)
			table = macbookair_fn_keys;
@@ -525,6 +504,10 @@  static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
					do_translate = asc->fn_on;
				}

+				if (dont_translate_flagged_key &&
+						trans->flags & APPLE_FLAG_DONT_TRANSLATE)
+					do_translate = false;
+
				if (do_translate)
					code = trans->to;
			}
@@ -680,8 +663,7 @@  static void apple_setup_input(struct input_dev *input)
	apple_setup_key_translation(input, magic_keyboard_alu_fn_keys);
	apple_setup_key_translation(input, magic_keyboard_2015_fn_keys);
	apple_setup_key_translation(input, apple2021_fn_keys);
-	apple_setup_key_translation(input, macbookpro_no_esc_fn_keys);
-	apple_setup_key_translation(input, macbookpro_dedicated_esc_fn_keys);
+	apple_setup_key_translation(input, macbookpro_fn_keys);
}

static int apple_input_mapping(struct hid_device *hdev, struct hid_input *hi,