All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] platform/x86: x86-android-tablets: Add support for more then 1 gpio_key
@ 2023-05-05 20:59 Hans de Goede
  2023-05-05 20:59 ` [PATCH 2/2] platform/x86: x86-android-tablets: Add support for extra buttons on Cyberbook T116 Hans de Goede
  2023-05-08  7:38 ` [PATCH 1/2] platform/x86: x86-android-tablets: Add support for more then 1 gpio_key Ilpo Järvinen
  0 siblings, 2 replies; 5+ messages in thread
From: Hans de Goede @ 2023-05-05 20:59 UTC (permalink / raw
  To: Ilpo Järvinen, Andy Shevchenko; +Cc: Hans de Goede, platform-driver-x86

Modify the gpio_keys support in x86_android_tablet_init() for
tablets which have more then 1 key/button which needs to be handled
by the gpio_keys driver.

This requires copying over the struct gpio_keys_button from
the x86_gpio_button struct array to a new gpio_keys_button struct array,
as an added benefit this allows marking the per model x86_gpio_button
arrays __initconst so that they all can be freed after module init().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../platform/x86/x86-android-tablets/asus.c   |  4 ++-
 .../platform/x86/x86-android-tablets/core.c   | 33 ++++++++++++-------
 .../platform/x86/x86-android-tablets/lenovo.c |  6 ++--
 .../platform/x86/x86-android-tablets/other.c  |  6 ++--
 .../x86-android-tablets/x86-android-tablets.h |  3 +-
 5 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/x86/x86-android-tablets/asus.c b/drivers/platform/x86/x86-android-tablets/asus.c
index 2aca91678219..f9c4083be86d 100644
--- a/drivers/platform/x86/x86-android-tablets/asus.c
+++ b/drivers/platform/x86/x86-android-tablets/asus.c
@@ -24,7 +24,7 @@ static struct gpiod_lookup_table int3496_gpo2_pin22_gpios = {
 	},
 };
 
-static struct x86_gpio_button asus_me176c_tf103c_lid = {
+static const struct x86_gpio_button asus_me176c_tf103c_lid __initconst = {
 	.button = {
 		.code = SW_LID,
 		.active_low = true,
@@ -175,6 +175,7 @@ const struct x86_dev_info asus_me176c_info __initconst = {
 	.serdev_info = asus_me176c_serdevs,
 	.serdev_count = ARRAY_SIZE(asus_me176c_serdevs),
 	.gpio_button = &asus_me176c_tf103c_lid,
+	.gpio_button_count = 1,
 	.gpiod_lookup_tables = asus_me176c_gpios,
 	.bat_swnode = &generic_lipo_hv_4v35_battery_node,
 	.modules = bq24190_modules,
@@ -317,6 +318,7 @@ const struct x86_dev_info asus_tf103c_info __initconst = {
 	.pdev_info = int3496_pdevs,
 	.pdev_count = 1,
 	.gpio_button = &asus_me176c_tf103c_lid,
+	.gpio_button_count = 1,
 	.gpiod_lookup_tables = asus_tf103c_gpios,
 	.bat_swnode = &asus_tf103c_battery_node,
 	.modules = bq24190_modules,
diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
index 245167674aa2..0c7142f4eccc 100644
--- a/drivers/platform/x86/x86-android-tablets/core.c
+++ b/drivers/platform/x86/x86-android-tablets/core.c
@@ -124,6 +124,7 @@ static int serdev_count;
 static struct i2c_client **i2c_clients;
 static struct platform_device **pdevs;
 static struct serdev_device **serdevs;
+static struct gpio_keys_button *buttons;
 static struct gpiod_lookup_table * const *gpiod_lookup_tables;
 static const struct software_node *bat_swnode;
 static void (*exit_handler)(void);
@@ -238,6 +239,7 @@ static void x86_android_tablet_cleanup(void)
 		platform_device_unregister(pdevs[i]);
 
 	kfree(pdevs);
+	kfree(buttons);
 
 	for (i = 0; i < i2c_client_count; i++)
 		i2c_unregister_device(i2c_clients[i]);
@@ -353,22 +355,31 @@ static __init int x86_android_tablet_init(void)
 		}
 	}
 
-	if (dev_info->gpio_button) {
-		struct gpio_keys_platform_data pdata = {
-			.buttons = &dev_info->gpio_button->button,
-			.nbuttons = 1,
-		};
+	if (dev_info->gpio_button_count) {
+		struct gpio_keys_platform_data pdata = { };
 		struct gpio_desc *gpiod;
 
-		/* Get GPIO for the gpio-button */
-		ret = x86_android_tablet_get_gpiod(dev_info->gpio_button->chip,
-						   dev_info->gpio_button->pin, &gpiod);
-		if (ret < 0) {
+		buttons = kcalloc(dev_info->gpio_button_count, sizeof(*buttons), GFP_KERNEL);
+		if (!buttons) {
 			x86_android_tablet_cleanup();
-			return ret;
+			return -ENOMEM;
+		}
+
+		for (i = 0; i < dev_info->gpio_button_count; i++) {
+			buttons[i] = dev_info->gpio_button[i].button;
+			/* Get GPIO for the gpio-button */
+			ret = x86_android_tablet_get_gpiod(dev_info->gpio_button[i].chip,
+							   dev_info->gpio_button[i].pin, &gpiod);
+			if (ret < 0) {
+				x86_android_tablet_cleanup();
+				return ret;
+			}
+
+			buttons[i].gpio = desc_to_gpio(gpiod);
 		}
 
-		dev_info->gpio_button->button.gpio = desc_to_gpio(gpiod);
+		pdata.buttons = buttons;
+		pdata.nbuttons = dev_info->gpio_button_count;
 
 		pdevs[pdev_count] = platform_device_register_data(NULL, "gpio-keys",
 								  PLATFORM_DEVID_AUTO,
diff --git a/drivers/platform/x86/x86-android-tablets/lenovo.c b/drivers/platform/x86/x86-android-tablets/lenovo.c
index 50031e902a2c..26a4ef670ad7 100644
--- a/drivers/platform/x86/x86-android-tablets/lenovo.c
+++ b/drivers/platform/x86/x86-android-tablets/lenovo.c
@@ -160,7 +160,7 @@ static const struct x86_serdev_info lenovo_yb1_x90_serdevs[] __initconst = {
 	},
 };
 
-static struct x86_gpio_button lenovo_yb1_x90_lid = {
+static const struct x86_gpio_button lenovo_yb1_x90_lid __initconst = {
 	.button = {
 		.code = SW_LID,
 		.active_low = true,
@@ -232,6 +232,7 @@ const struct x86_dev_info lenovo_yogabook_x90_info __initconst = {
 	.serdev_info = lenovo_yb1_x90_serdevs,
 	.serdev_count = ARRAY_SIZE(lenovo_yb1_x90_serdevs),
 	.gpio_button = &lenovo_yb1_x90_lid,
+	.gpio_button_count = 1,
 	.gpiod_lookup_tables = lenovo_yb1_x90_gpios,
 	.init = lenovo_yb1_x90_init,
 };
@@ -268,7 +269,7 @@ static const struct software_node lenovo_yoga_tab2_830_1050_bq24190_node = {
 	.properties = lenovo_yoga_tab2_830_1050_bq24190_props,
 };
 
-static struct x86_gpio_button lenovo_yoga_tab2_830_1050_lid = {
+static const struct x86_gpio_button lenovo_yoga_tab2_830_1050_lid __initconst = {
 	.button = {
 		.code = SW_LID,
 		.active_low = true,
@@ -394,6 +395,7 @@ const struct x86_dev_info lenovo_yoga_tab2_830_1050_info __initconst = {
 	.pdev_info = int3496_pdevs,
 	.pdev_count = 1,
 	.gpio_button = &lenovo_yoga_tab2_830_1050_lid,
+	.gpio_button_count = 1,
 	.gpiod_lookup_tables = lenovo_yoga_tab2_830_1050_gpios,
 	.bat_swnode = &generic_lipo_hv_4v35_battery_node,
 	.modules = bq24190_modules,
diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
index 3754d2453cdb..4d54c89e6ca2 100644
--- a/drivers/platform/x86/x86-android-tablets/other.c
+++ b/drivers/platform/x86/x86-android-tablets/other.c
@@ -94,7 +94,7 @@ const struct x86_dev_info acer_b1_750_info __initconst = {
  * which is not described in the ACPI tables in anyway.
  * Use the x86-android-tablets infra to create a gpio-button device for this.
  */
-static struct x86_gpio_button advantech_mica_071_button = {
+static const struct x86_gpio_button advantech_mica_071_button __initconst = {
 	.button = {
 		.code = KEY_PROG1,
 		.active_low = true,
@@ -109,6 +109,7 @@ static struct x86_gpio_button advantech_mica_071_button = {
 
 const struct x86_dev_info advantech_mica_071_info __initconst = {
 	.gpio_button = &advantech_mica_071_button,
+	.gpio_button_count = 1,
 };
 
 /*
@@ -449,7 +450,7 @@ const struct x86_dev_info nextbook_ares8a_info __initconst = {
  * This button has a WMI interface, but that is broken. Instead of trying to
  * use the broken WMI interface, instantiate a gpio_keys device for this.
  */
-static struct x86_gpio_button peaq_c1010_button = {
+static const struct x86_gpio_button peaq_c1010_button __initconst = {
 	.button = {
 		.code = KEY_SOUND,
 		.active_low = true,
@@ -464,6 +465,7 @@ static struct x86_gpio_button peaq_c1010_button = {
 
 const struct x86_dev_info peaq_c1010_info __initconst = {
 	.gpio_button = &peaq_c1010_button,
+	.gpio_button_count = 1,
 	/*
 	 * Move the ACPI event handler used by the broken WMI interface out of
 	 * the way. This is the only event handler on INT33FC:00.
diff --git a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
index 8e9f7238015c..8f04a052eada 100644
--- a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
+++ b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
@@ -73,10 +73,11 @@ struct x86_dev_info {
 	const struct x86_i2c_client_info *i2c_client_info;
 	const struct platform_device_info *pdev_info;
 	const struct x86_serdev_info *serdev_info;
-	struct x86_gpio_button *gpio_button;
+	const struct x86_gpio_button *gpio_button;
 	int i2c_client_count;
 	int pdev_count;
 	int serdev_count;
+	int gpio_button_count;
 	int (*init)(void);
 	void (*exit)(void);
 };
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] platform/x86: x86-android-tablets: Add support for extra buttons on Cyberbook T116
  2023-05-05 20:59 [PATCH 1/2] platform/x86: x86-android-tablets: Add support for more then 1 gpio_key Hans de Goede
@ 2023-05-05 20:59 ` Hans de Goede
  2023-05-06 11:40   ` Andy Shevchenko
  2023-05-08  7:38 ` [PATCH 1/2] platform/x86: x86-android-tablets: Add support for more then 1 gpio_key Ilpo Järvinen
  1 sibling, 1 reply; 5+ messages in thread
From: Hans de Goede @ 2023-05-05 20:59 UTC (permalink / raw
  To: Ilpo Järvinen, Andy Shevchenko; +Cc: Hans de Goede, platform-driver-x86

The Cyberbook T116 rugged tablet comes in both Windows and Android versions
and even on the Android version the DSDT is mostly sane. This tablet has
2 extra general purpose buttons in the row with the power + volume-buttons,
labeled P and F.

Use the x86-android-tablets infra to create a gpio-button device for these
2 extra buttons.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../platform/x86/x86-android-tablets/dmi.c    | 11 ++++++
 .../platform/x86/x86-android-tablets/other.c  | 39 +++++++++++++++++++
 .../x86-android-tablets/x86-android-tablets.h |  1 +
 3 files changed, 51 insertions(+)

diff --git a/drivers/platform/x86/x86-android-tablets/dmi.c b/drivers/platform/x86/x86-android-tablets/dmi.c
index 07acf0978325..2bda152b4a11 100644
--- a/drivers/platform/x86/x86-android-tablets/dmi.c
+++ b/drivers/platform/x86/x86-android-tablets/dmi.c
@@ -58,6 +58,17 @@ const struct dmi_system_id x86_android_tablet_ids[] __initconst = {
 		},
 		.driver_data = (void *)&chuwi_hi8_info,
 	},
+	{
+		/* Cyberbook T116 Android version */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "Default string"),
+			DMI_MATCH(DMI_BOARD_NAME, "Cherry Trail CR"),
+			/* Above strings are much too generic, also match on SKU + BIOS date */
+			DMI_MATCH(DMI_PRODUCT_SKU, "20170531"),
+			DMI_MATCH(DMI_BIOS_DATE, "07/12/2017"),
+		},
+		.driver_data = (void *)&cyberbook_t116_info,
+	},
 	{
 		/* CZC P10T */
 		.ident = "CZC ODEON TPC-10 (\"P10T\")",
diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
index 4d54c89e6ca2..e79549c6aae1 100644
--- a/drivers/platform/x86/x86-android-tablets/other.c
+++ b/drivers/platform/x86/x86-android-tablets/other.c
@@ -197,6 +197,45 @@ const struct x86_dev_info chuwi_hi8_info __initconst = {
 	.init = chuwi_hi8_init,
 };
 
+/*
+ * Cyberbook T116 Android version
+ * This comes in both Windows and Android versions and even on Android
+ * the DSDT is mostly sane. This tablet has 2 extra general purpose buttons
+ * in the button row with the power + volume-buttons labeled P and F.
+ * Use the x86-android-tablets infra to create a gpio-button device for these.
+ */
+static const struct x86_gpio_button cyberbook_t116_buttons[] __initconst = {
+	{
+		.button = {
+			.code = KEY_PROG1,
+			.active_low = true,
+			.desc = "prog1_key",
+			.type = EV_KEY,
+			.wakeup = false,
+			.debounce_interval = 50,
+		},
+		.chip = "INT33FF:00",
+		.pin = 30,
+	},
+	{
+		.button = {
+			.code = KEY_PROG2,
+			.active_low = true,
+			.desc = "prog2_key",
+			.type = EV_KEY,
+			.wakeup = false,
+			.debounce_interval = 50,
+		},
+		.chip = "INT33FF:03",
+		.pin = 48,
+	},
+};
+
+const struct x86_dev_info cyberbook_t116_info __initconst = {
+	.gpio_button = cyberbook_t116_buttons,
+	.gpio_button_count = ARRAY_SIZE(cyberbook_t116_buttons),
+};
+
 #define CZC_EC_EXTRA_PORT	0x68
 #define CZC_EC_ANDROID_KEYS	0x63
 
diff --git a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
index 8f04a052eada..e46e1128acc8 100644
--- a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
+++ b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
@@ -94,6 +94,7 @@ extern const struct x86_dev_info advantech_mica_071_info;
 extern const struct x86_dev_info asus_me176c_info;
 extern const struct x86_dev_info asus_tf103c_info;
 extern const struct x86_dev_info chuwi_hi8_info;
+extern const struct x86_dev_info cyberbook_t116_info;
 extern const struct x86_dev_info czc_p10t;
 extern const struct x86_dev_info lenovo_yogabook_x90_info;
 extern const struct x86_dev_info lenovo_yogabook_x91_info;
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] platform/x86: x86-android-tablets: Add support for extra buttons on Cyberbook T116
  2023-05-05 20:59 ` [PATCH 2/2] platform/x86: x86-android-tablets: Add support for extra buttons on Cyberbook T116 Hans de Goede
@ 2023-05-06 11:40   ` Andy Shevchenko
  0 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2023-05-06 11:40 UTC (permalink / raw
  To: Hans de Goede; +Cc: Ilpo Järvinen, Andy Shevchenko, platform-driver-x86

On Fri, May 5, 2023 at 11:59 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> The Cyberbook T116 rugged tablet comes in both Windows and Android versions
> and even on the Android version the DSDT is mostly sane. This tablet has
> 2 extra general purpose buttons in the row with the power + volume-buttons,
> labeled P and F.
>
> Use the x86-android-tablets infra to create a gpio-button device for these
> 2 extra buttons.

For both
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  .../platform/x86/x86-android-tablets/dmi.c    | 11 ++++++
>  .../platform/x86/x86-android-tablets/other.c  | 39 +++++++++++++++++++
>  .../x86-android-tablets/x86-android-tablets.h |  1 +
>  3 files changed, 51 insertions(+)
>
> diff --git a/drivers/platform/x86/x86-android-tablets/dmi.c b/drivers/platform/x86/x86-android-tablets/dmi.c
> index 07acf0978325..2bda152b4a11 100644
> --- a/drivers/platform/x86/x86-android-tablets/dmi.c
> +++ b/drivers/platform/x86/x86-android-tablets/dmi.c
> @@ -58,6 +58,17 @@ const struct dmi_system_id x86_android_tablet_ids[] __initconst = {
>                 },
>                 .driver_data = (void *)&chuwi_hi8_info,
>         },
> +       {
> +               /* Cyberbook T116 Android version */
> +               .matches = {
> +                       DMI_MATCH(DMI_BOARD_VENDOR, "Default string"),
> +                       DMI_MATCH(DMI_BOARD_NAME, "Cherry Trail CR"),
> +                       /* Above strings are much too generic, also match on SKU + BIOS date */
> +                       DMI_MATCH(DMI_PRODUCT_SKU, "20170531"),
> +                       DMI_MATCH(DMI_BIOS_DATE, "07/12/2017"),
> +               },
> +               .driver_data = (void *)&cyberbook_t116_info,
> +       },
>         {
>                 /* CZC P10T */
>                 .ident = "CZC ODEON TPC-10 (\"P10T\")",
> diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
> index 4d54c89e6ca2..e79549c6aae1 100644
> --- a/drivers/platform/x86/x86-android-tablets/other.c
> +++ b/drivers/platform/x86/x86-android-tablets/other.c
> @@ -197,6 +197,45 @@ const struct x86_dev_info chuwi_hi8_info __initconst = {
>         .init = chuwi_hi8_init,
>  };
>
> +/*
> + * Cyberbook T116 Android version
> + * This comes in both Windows and Android versions and even on Android
> + * the DSDT is mostly sane. This tablet has 2 extra general purpose buttons
> + * in the button row with the power + volume-buttons labeled P and F.
> + * Use the x86-android-tablets infra to create a gpio-button device for these.
> + */
> +static const struct x86_gpio_button cyberbook_t116_buttons[] __initconst = {
> +       {
> +               .button = {
> +                       .code = KEY_PROG1,
> +                       .active_low = true,
> +                       .desc = "prog1_key",
> +                       .type = EV_KEY,
> +                       .wakeup = false,
> +                       .debounce_interval = 50,
> +               },
> +               .chip = "INT33FF:00",
> +               .pin = 30,
> +       },
> +       {
> +               .button = {
> +                       .code = KEY_PROG2,
> +                       .active_low = true,
> +                       .desc = "prog2_key",
> +                       .type = EV_KEY,
> +                       .wakeup = false,
> +                       .debounce_interval = 50,
> +               },
> +               .chip = "INT33FF:03",
> +               .pin = 48,
> +       },
> +};
> +
> +const struct x86_dev_info cyberbook_t116_info __initconst = {
> +       .gpio_button = cyberbook_t116_buttons,
> +       .gpio_button_count = ARRAY_SIZE(cyberbook_t116_buttons),
> +};
> +
>  #define CZC_EC_EXTRA_PORT      0x68
>  #define CZC_EC_ANDROID_KEYS    0x63
>
> diff --git a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
> index 8f04a052eada..e46e1128acc8 100644
> --- a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
> +++ b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
> @@ -94,6 +94,7 @@ extern const struct x86_dev_info advantech_mica_071_info;
>  extern const struct x86_dev_info asus_me176c_info;
>  extern const struct x86_dev_info asus_tf103c_info;
>  extern const struct x86_dev_info chuwi_hi8_info;
> +extern const struct x86_dev_info cyberbook_t116_info;
>  extern const struct x86_dev_info czc_p10t;
>  extern const struct x86_dev_info lenovo_yogabook_x90_info;
>  extern const struct x86_dev_info lenovo_yogabook_x91_info;
> --
> 2.40.1
>


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] platform/x86: x86-android-tablets: Add support for more then 1 gpio_key
  2023-05-05 20:59 [PATCH 1/2] platform/x86: x86-android-tablets: Add support for more then 1 gpio_key Hans de Goede
  2023-05-05 20:59 ` [PATCH 2/2] platform/x86: x86-android-tablets: Add support for extra buttons on Cyberbook T116 Hans de Goede
@ 2023-05-08  7:38 ` Ilpo Järvinen
  2023-05-09  9:33   ` Hans de Goede
  1 sibling, 1 reply; 5+ messages in thread
From: Ilpo Järvinen @ 2023-05-08  7:38 UTC (permalink / raw
  To: Hans de Goede; +Cc: Andy Shevchenko, platform-driver-x86

[-- Attachment #1: Type: text/plain, Size: 8834 bytes --]

On Fri, 5 May 2023, Hans de Goede wrote:

> Modify the gpio_keys support in x86_android_tablet_init() for
> tablets which have more then 1 key/button which needs to be handled
> by the gpio_keys driver.
> 
> This requires copying over the struct gpio_keys_button from
> the x86_gpio_button struct array to a new gpio_keys_button struct array,
> as an added benefit this allows marking the per model x86_gpio_button
> arrays __initconst so that they all can be freed after module init().
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  .../platform/x86/x86-android-tablets/asus.c   |  4 ++-
>  .../platform/x86/x86-android-tablets/core.c   | 33 ++++++++++++-------
>  .../platform/x86/x86-android-tablets/lenovo.c |  6 ++--
>  .../platform/x86/x86-android-tablets/other.c  |  6 ++--
>  .../x86-android-tablets/x86-android-tablets.h |  3 +-
>  5 files changed, 35 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/platform/x86/x86-android-tablets/asus.c b/drivers/platform/x86/x86-android-tablets/asus.c
> index 2aca91678219..f9c4083be86d 100644
> --- a/drivers/platform/x86/x86-android-tablets/asus.c
> +++ b/drivers/platform/x86/x86-android-tablets/asus.c
> @@ -24,7 +24,7 @@ static struct gpiod_lookup_table int3496_gpo2_pin22_gpios = {
>  	},
>  };
>  
> -static struct x86_gpio_button asus_me176c_tf103c_lid = {
> +static const struct x86_gpio_button asus_me176c_tf103c_lid __initconst = {
>  	.button = {
>  		.code = SW_LID,
>  		.active_low = true,
> @@ -175,6 +175,7 @@ const struct x86_dev_info asus_me176c_info __initconst = {
>  	.serdev_info = asus_me176c_serdevs,
>  	.serdev_count = ARRAY_SIZE(asus_me176c_serdevs),
>  	.gpio_button = &asus_me176c_tf103c_lid,
> +	.gpio_button_count = 1,
>  	.gpiod_lookup_tables = asus_me176c_gpios,
>  	.bat_swnode = &generic_lipo_hv_4v35_battery_node,
>  	.modules = bq24190_modules,
> @@ -317,6 +318,7 @@ const struct x86_dev_info asus_tf103c_info __initconst = {
>  	.pdev_info = int3496_pdevs,
>  	.pdev_count = 1,
>  	.gpio_button = &asus_me176c_tf103c_lid,
> +	.gpio_button_count = 1,
>  	.gpiod_lookup_tables = asus_tf103c_gpios,
>  	.bat_swnode = &asus_tf103c_battery_node,
>  	.modules = bq24190_modules,
> diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
> index 245167674aa2..0c7142f4eccc 100644
> --- a/drivers/platform/x86/x86-android-tablets/core.c
> +++ b/drivers/platform/x86/x86-android-tablets/core.c
> @@ -124,6 +124,7 @@ static int serdev_count;
>  static struct i2c_client **i2c_clients;
>  static struct platform_device **pdevs;
>  static struct serdev_device **serdevs;
> +static struct gpio_keys_button *buttons;
>  static struct gpiod_lookup_table * const *gpiod_lookup_tables;
>  static const struct software_node *bat_swnode;
>  static void (*exit_handler)(void);
> @@ -238,6 +239,7 @@ static void x86_android_tablet_cleanup(void)
>  		platform_device_unregister(pdevs[i]);
>  
>  	kfree(pdevs);
> +	kfree(buttons);
>  
>  	for (i = 0; i < i2c_client_count; i++)
>  		i2c_unregister_device(i2c_clients[i]);
> @@ -353,22 +355,31 @@ static __init int x86_android_tablet_init(void)
>  		}
>  	}
>  
> -	if (dev_info->gpio_button) {
> -		struct gpio_keys_platform_data pdata = {
> -			.buttons = &dev_info->gpio_button->button,
> -			.nbuttons = 1,
> -		};
> +	if (dev_info->gpio_button_count) {
> +		struct gpio_keys_platform_data pdata = { };
>  		struct gpio_desc *gpiod;
>  
> -		/* Get GPIO for the gpio-button */
> -		ret = x86_android_tablet_get_gpiod(dev_info->gpio_button->chip,
> -						   dev_info->gpio_button->pin, &gpiod);
> -		if (ret < 0) {
> +		buttons = kcalloc(dev_info->gpio_button_count, sizeof(*buttons), GFP_KERNEL);
> +		if (!buttons) {
>  			x86_android_tablet_cleanup();
> -			return ret;
> +			return -ENOMEM;
> +		}
> +
> +		for (i = 0; i < dev_info->gpio_button_count; i++) {
> +			buttons[i] = dev_info->gpio_button[i].button;
> +			/* Get GPIO for the gpio-button */
> +			ret = x86_android_tablet_get_gpiod(dev_info->gpio_button[i].chip,
> +							   dev_info->gpio_button[i].pin, &gpiod);

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

With the note that the comment seems to just spell out what the code does 
so it feels useless.

-- 
 i.

> +			if (ret < 0) {
> +				x86_android_tablet_cleanup();
> +				return ret;
> +			}
> +
> +			buttons[i].gpio = desc_to_gpio(gpiod);
>  		}
>  
> -		dev_info->gpio_button->button.gpio = desc_to_gpio(gpiod);
> +		pdata.buttons = buttons;
> +		pdata.nbuttons = dev_info->gpio_button_count;
>  
>  		pdevs[pdev_count] = platform_device_register_data(NULL, "gpio-keys",
>  								  PLATFORM_DEVID_AUTO,
> diff --git a/drivers/platform/x86/x86-android-tablets/lenovo.c b/drivers/platform/x86/x86-android-tablets/lenovo.c
> index 50031e902a2c..26a4ef670ad7 100644
> --- a/drivers/platform/x86/x86-android-tablets/lenovo.c
> +++ b/drivers/platform/x86/x86-android-tablets/lenovo.c
> @@ -160,7 +160,7 @@ static const struct x86_serdev_info lenovo_yb1_x90_serdevs[] __initconst = {
>  	},
>  };
>  
> -static struct x86_gpio_button lenovo_yb1_x90_lid = {
> +static const struct x86_gpio_button lenovo_yb1_x90_lid __initconst = {
>  	.button = {
>  		.code = SW_LID,
>  		.active_low = true,
> @@ -232,6 +232,7 @@ const struct x86_dev_info lenovo_yogabook_x90_info __initconst = {
>  	.serdev_info = lenovo_yb1_x90_serdevs,
>  	.serdev_count = ARRAY_SIZE(lenovo_yb1_x90_serdevs),
>  	.gpio_button = &lenovo_yb1_x90_lid,
> +	.gpio_button_count = 1,
>  	.gpiod_lookup_tables = lenovo_yb1_x90_gpios,
>  	.init = lenovo_yb1_x90_init,
>  };
> @@ -268,7 +269,7 @@ static const struct software_node lenovo_yoga_tab2_830_1050_bq24190_node = {
>  	.properties = lenovo_yoga_tab2_830_1050_bq24190_props,
>  };
>  
> -static struct x86_gpio_button lenovo_yoga_tab2_830_1050_lid = {
> +static const struct x86_gpio_button lenovo_yoga_tab2_830_1050_lid __initconst = {
>  	.button = {
>  		.code = SW_LID,
>  		.active_low = true,
> @@ -394,6 +395,7 @@ const struct x86_dev_info lenovo_yoga_tab2_830_1050_info __initconst = {
>  	.pdev_info = int3496_pdevs,
>  	.pdev_count = 1,
>  	.gpio_button = &lenovo_yoga_tab2_830_1050_lid,
> +	.gpio_button_count = 1,
>  	.gpiod_lookup_tables = lenovo_yoga_tab2_830_1050_gpios,
>  	.bat_swnode = &generic_lipo_hv_4v35_battery_node,
>  	.modules = bq24190_modules,
> diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
> index 3754d2453cdb..4d54c89e6ca2 100644
> --- a/drivers/platform/x86/x86-android-tablets/other.c
> +++ b/drivers/platform/x86/x86-android-tablets/other.c
> @@ -94,7 +94,7 @@ const struct x86_dev_info acer_b1_750_info __initconst = {
>   * which is not described in the ACPI tables in anyway.
>   * Use the x86-android-tablets infra to create a gpio-button device for this.
>   */
> -static struct x86_gpio_button advantech_mica_071_button = {
> +static const struct x86_gpio_button advantech_mica_071_button __initconst = {
>  	.button = {
>  		.code = KEY_PROG1,
>  		.active_low = true,
> @@ -109,6 +109,7 @@ static struct x86_gpio_button advantech_mica_071_button = {
>  
>  const struct x86_dev_info advantech_mica_071_info __initconst = {
>  	.gpio_button = &advantech_mica_071_button,
> +	.gpio_button_count = 1,
>  };
>  
>  /*
> @@ -449,7 +450,7 @@ const struct x86_dev_info nextbook_ares8a_info __initconst = {
>   * This button has a WMI interface, but that is broken. Instead of trying to
>   * use the broken WMI interface, instantiate a gpio_keys device for this.
>   */
> -static struct x86_gpio_button peaq_c1010_button = {
> +static const struct x86_gpio_button peaq_c1010_button __initconst = {
>  	.button = {
>  		.code = KEY_SOUND,
>  		.active_low = true,
> @@ -464,6 +465,7 @@ static struct x86_gpio_button peaq_c1010_button = {
>  
>  const struct x86_dev_info peaq_c1010_info __initconst = {
>  	.gpio_button = &peaq_c1010_button,
> +	.gpio_button_count = 1,
>  	/*
>  	 * Move the ACPI event handler used by the broken WMI interface out of
>  	 * the way. This is the only event handler on INT33FC:00.
> diff --git a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
> index 8e9f7238015c..8f04a052eada 100644
> --- a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
> +++ b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
> @@ -73,10 +73,11 @@ struct x86_dev_info {
>  	const struct x86_i2c_client_info *i2c_client_info;
>  	const struct platform_device_info *pdev_info;
>  	const struct x86_serdev_info *serdev_info;
> -	struct x86_gpio_button *gpio_button;
> +	const struct x86_gpio_button *gpio_button;
>  	int i2c_client_count;
>  	int pdev_count;
>  	int serdev_count;
> +	int gpio_button_count;
>  	int (*init)(void);
>  	void (*exit)(void);
>  };
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] platform/x86: x86-android-tablets: Add support for more then 1 gpio_key
  2023-05-08  7:38 ` [PATCH 1/2] platform/x86: x86-android-tablets: Add support for more then 1 gpio_key Ilpo Järvinen
@ 2023-05-09  9:33   ` Hans de Goede
  0 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2023-05-09  9:33 UTC (permalink / raw
  To: Ilpo Järvinen; +Cc: Andy Shevchenko, platform-driver-x86

Hi,

On 5/8/23 09:38, Ilpo Järvinen wrote:
> On Fri, 5 May 2023, Hans de Goede wrote:
> 
>> Modify the gpio_keys support in x86_android_tablet_init() for
>> tablets which have more then 1 key/button which needs to be handled
>> by the gpio_keys driver.
>>
>> This requires copying over the struct gpio_keys_button from
>> the x86_gpio_button struct array to a new gpio_keys_button struct array,
>> as an added benefit this allows marking the per model x86_gpio_button
>> arrays __initconst so that they all can be freed after module init().
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  .../platform/x86/x86-android-tablets/asus.c   |  4 ++-
>>  .../platform/x86/x86-android-tablets/core.c   | 33 ++++++++++++-------
>>  .../platform/x86/x86-android-tablets/lenovo.c |  6 ++--
>>  .../platform/x86/x86-android-tablets/other.c  |  6 ++--
>>  .../x86-android-tablets/x86-android-tablets.h |  3 +-
>>  5 files changed, 35 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/platform/x86/x86-android-tablets/asus.c b/drivers/platform/x86/x86-android-tablets/asus.c
>> index 2aca91678219..f9c4083be86d 100644
>> --- a/drivers/platform/x86/x86-android-tablets/asus.c
>> +++ b/drivers/platform/x86/x86-android-tablets/asus.c
>> @@ -24,7 +24,7 @@ static struct gpiod_lookup_table int3496_gpo2_pin22_gpios = {
>>  	},
>>  };
>>  
>> -static struct x86_gpio_button asus_me176c_tf103c_lid = {
>> +static const struct x86_gpio_button asus_me176c_tf103c_lid __initconst = {
>>  	.button = {
>>  		.code = SW_LID,
>>  		.active_low = true,
>> @@ -175,6 +175,7 @@ const struct x86_dev_info asus_me176c_info __initconst = {
>>  	.serdev_info = asus_me176c_serdevs,
>>  	.serdev_count = ARRAY_SIZE(asus_me176c_serdevs),
>>  	.gpio_button = &asus_me176c_tf103c_lid,
>> +	.gpio_button_count = 1,
>>  	.gpiod_lookup_tables = asus_me176c_gpios,
>>  	.bat_swnode = &generic_lipo_hv_4v35_battery_node,
>>  	.modules = bq24190_modules,
>> @@ -317,6 +318,7 @@ const struct x86_dev_info asus_tf103c_info __initconst = {
>>  	.pdev_info = int3496_pdevs,
>>  	.pdev_count = 1,
>>  	.gpio_button = &asus_me176c_tf103c_lid,
>> +	.gpio_button_count = 1,
>>  	.gpiod_lookup_tables = asus_tf103c_gpios,
>>  	.bat_swnode = &asus_tf103c_battery_node,
>>  	.modules = bq24190_modules,
>> diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
>> index 245167674aa2..0c7142f4eccc 100644
>> --- a/drivers/platform/x86/x86-android-tablets/core.c
>> +++ b/drivers/platform/x86/x86-android-tablets/core.c
>> @@ -124,6 +124,7 @@ static int serdev_count;
>>  static struct i2c_client **i2c_clients;
>>  static struct platform_device **pdevs;
>>  static struct serdev_device **serdevs;
>> +static struct gpio_keys_button *buttons;
>>  static struct gpiod_lookup_table * const *gpiod_lookup_tables;
>>  static const struct software_node *bat_swnode;
>>  static void (*exit_handler)(void);
>> @@ -238,6 +239,7 @@ static void x86_android_tablet_cleanup(void)
>>  		platform_device_unregister(pdevs[i]);
>>  
>>  	kfree(pdevs);
>> +	kfree(buttons);
>>  
>>  	for (i = 0; i < i2c_client_count; i++)
>>  		i2c_unregister_device(i2c_clients[i]);
>> @@ -353,22 +355,31 @@ static __init int x86_android_tablet_init(void)
>>  		}
>>  	}
>>  
>> -	if (dev_info->gpio_button) {
>> -		struct gpio_keys_platform_data pdata = {
>> -			.buttons = &dev_info->gpio_button->button,
>> -			.nbuttons = 1,
>> -		};
>> +	if (dev_info->gpio_button_count) {
>> +		struct gpio_keys_platform_data pdata = { };
>>  		struct gpio_desc *gpiod;
>>  
>> -		/* Get GPIO for the gpio-button */
>> -		ret = x86_android_tablet_get_gpiod(dev_info->gpio_button->chip,
>> -						   dev_info->gpio_button->pin, &gpiod);
>> -		if (ret < 0) {
>> +		buttons = kcalloc(dev_info->gpio_button_count, sizeof(*buttons), GFP_KERNEL);
>> +		if (!buttons) {
>>  			x86_android_tablet_cleanup();
>> -			return ret;
>> +			return -ENOMEM;
>> +		}
>> +
>> +		for (i = 0; i < dev_info->gpio_button_count; i++) {
>> +			buttons[i] = dev_info->gpio_button[i].button;
>> +			/* Get GPIO for the gpio-button */
>> +			ret = x86_android_tablet_get_gpiod(dev_info->gpio_button[i].chip,
>> +							   dev_info->gpio_button[i].pin, &gpiod);
> 
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

Thank you.

> With the note that the comment seems to just spell out what the code does 
> so it feels useless.

That is a valid note. I've dropped the comment while merging this.

I've added this series to my review-hans (soon to be for-next) branch now.

Regards,

Hans




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-05-09  9:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-05 20:59 [PATCH 1/2] platform/x86: x86-android-tablets: Add support for more then 1 gpio_key Hans de Goede
2023-05-05 20:59 ` [PATCH 2/2] platform/x86: x86-android-tablets: Add support for extra buttons on Cyberbook T116 Hans de Goede
2023-05-06 11:40   ` Andy Shevchenko
2023-05-08  7:38 ` [PATCH 1/2] platform/x86: x86-android-tablets: Add support for more then 1 gpio_key Ilpo Järvinen
2023-05-09  9:33   ` Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.