Re: [PATCH 1/2] x86: Conditional init of pcengines leds/keys gpios

From: Enrico Weigelt, metux IT consult
Date: Thu Oct 22 2020 - 05:22:47 EST

On 21.10.20 23:41, Ed Wildgoose wrote:

Hi,

> The pcengines bios/firmware includes ACPI tables (since 4.10.0.1) which
> will cause the kernel to automatically create led + gpio_key devices for
> the platform. This means that the platform setup now creates duplicates
> of all these led/key devices.
>
> Driver conditionally initialises leds/keys only for older bios.

still breaks existing userlands as device names change - LEDs as well as
keys, and keycodes also change.

IOW: userland needs to be depending on specific BIOS versions.


--mtx


>
> Signed-off-by: Ed Wildgoose <lists@xxxxxxxxxxxxxx>
> ---
> drivers/platform/x86/pcengines-apuv2.c | 115 +++++++++++++++++++------
> 1 file changed, 90 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/platform/x86/pcengines-apuv2.c b/drivers/platform/x86/pcengines-apuv2.c
> index c37349f97..45f7a89de 100644
> --- a/drivers/platform/x86/pcengines-apuv2.c
> +++ b/drivers/platform/x86/pcengines-apuv2.c
> @@ -128,6 +128,18 @@ static struct gpiod_lookup_table gpios_key_table = {
>
> /* Board setup */
>
> +struct apu_driver_data {
> + const struct amd_fch_gpio_pdata *board_data;
> + const struct gpio_keys_platform_data *apu_keys_pdata;
> + const struct gpio_led_platform_data *apu_leds_pdata;
> +};
> +
> +static struct apu_driver_data apu2_driver_data = {
> + .board_data = &board_apu2,
> + .apu_keys_pdata = &apu2_keys_pdata,
> + .apu_leds_pdata = &apu2_leds_pdata
> +};
> +
> /* Note: matching works on string prefix, so "apu2" must come before "apu" */
> static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
>
> @@ -138,7 +150,7 @@ static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
> DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
> DMI_MATCH(DMI_BOARD_NAME, "APU2")
> },
> - .driver_data = (void *)&board_apu2,
> + .driver_data = (void *)&apu2_driver_data,
> },
> /* APU2 w/ legacy BIOS >= 4.0.8 */
> {
> @@ -147,7 +159,7 @@ static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
> DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
> DMI_MATCH(DMI_BOARD_NAME, "apu2")
> },
> - .driver_data = (void *)&board_apu2,
> + .driver_data = (void *)&apu2_driver_data,
> },
> /* APU2 w/ mainline BIOS */
> {
> @@ -156,7 +168,7 @@ static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
> DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
> DMI_MATCH(DMI_BOARD_NAME, "PC Engines apu2")
> },
> - .driver_data = (void *)&board_apu2,
> + .driver_data = (void *)&apu2_driver_data,
> },
>
> /* APU3 w/ legacy BIOS < 4.0.8 */
> @@ -166,7 +178,7 @@ static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
> DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
> DMI_MATCH(DMI_BOARD_NAME, "APU3")
> },
> - .driver_data = (void *)&board_apu2,
> + .driver_data = (void *)&apu2_driver_data,
> },
> /* APU3 w/ legacy BIOS >= 4.0.8 */
> {
> @@ -175,7 +187,7 @@ static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
> DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
> DMI_MATCH(DMI_BOARD_NAME, "apu3")
> },
> - .driver_data = (void *)&board_apu2,
> + .driver_data = (void *)&apu2_driver_data,
> },
> /* APU3 w/ mainline BIOS */
> {
> @@ -184,7 +196,7 @@ static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
> DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
> DMI_MATCH(DMI_BOARD_NAME, "PC Engines apu3")
> },
> - .driver_data = (void *)&board_apu2,
> + .driver_data = (void *)&apu2_driver_data,
> },
> /* APU4 w/ legacy BIOS < 4.0.8 */
> {
> @@ -193,7 +205,7 @@ static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
> DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
> DMI_MATCH(DMI_BOARD_NAME, "APU4")
> },
> - .driver_data = (void *)&board_apu2,
> + .driver_data = (void *)&apu2_driver_data,
> },
> /* APU4 w/ legacy BIOS >= 4.0.8 */
> {
> @@ -202,7 +214,7 @@ static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
> DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
> DMI_MATCH(DMI_BOARD_NAME, "apu4")
> },
> - .driver_data = (void *)&board_apu2,
> + .driver_data = (void *)&apu2_driver_data,
> },
> /* APU4 w/ mainline BIOS */
> {
> @@ -211,11 +223,42 @@ static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
> DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
> DMI_MATCH(DMI_BOARD_NAME, "PC Engines apu4")
> },
> - .driver_data = (void *)&board_apu2,
> + .driver_data = (void *)&apu2_driver_data,
> },
> {}
> };
>
> +int cmp_version(const char *base, const char *test)
> +{
> + int i;
> + int oct_base[4], oct_test[4];
> + int matched_base, matched;
> +
> + matched_base = sscanf(base, "v%4d.%4d.%4d.%4d",
> + &oct_base[0], &oct_base[1],
> + &oct_base[2], &oct_base[3]);
> + matched = sscanf(test, "v%4d.%4d.%4d.%4d",
> + &oct_test[0], &oct_test[1],
> + &oct_test[2], &oct_test[3]);
> +
> + /* opinionated: match as lower if we can't parse test version */
> + if (matched == 0)
> + return -1;
> +
> + if (matched_base < matched)
> + matched = matched_base;
> +
> + for (i = 0; i < matched; i++) {
> + if (oct_test[i] > oct_base[i])
> + return 1;
> + else if (oct_test[i] < oct_base[i])
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> static struct platform_device *apu_gpio_pdev;
> static struct platform_device *apu_leds_pdev;
> static struct platform_device *apu_keys_pdev;
> @@ -244,41 +287,63 @@ static struct platform_device * __init apu_create_pdev(
> static int __init apu_board_init(void)
> {
> const struct dmi_system_id *id;
> + const struct apu_driver_data *driver_data;
> + char const *bios_ver;
> +
> + apu_leds_pdev = apu_keys_pdev = NULL;
>
> id = dmi_first_match(apu_gpio_dmi_table);
> if (!id) {
> pr_err("failed to detect APU board via DMI\n");
> return -ENODEV;
> + } else {
> + pr_info("Detected APU board: %s - BIOS: %s\n",
> + dmi_get_system_info(DMI_BOARD_NAME),
> + dmi_get_system_info(DMI_BIOS_VERSION));
> }
>
> - gpiod_add_lookup_table(&gpios_led_table);
> - gpiod_add_lookup_table(&gpios_key_table);
> + driver_data = id->driver_data;
> + bios_ver = dmi_get_system_info(DMI_BIOS_VERSION);
>
> apu_gpio_pdev = apu_create_pdev(
> AMD_FCH_GPIO_DRIVER_NAME,
> - id->driver_data,
> + driver_data->board_data,
> sizeof(struct amd_fch_gpio_pdata));
>
> - apu_leds_pdev = apu_create_pdev(
> - "leds-gpio",
> - &apu2_leds_pdata,
> - sizeof(apu2_leds_pdata));
> -
> - apu_keys_pdev = apu_create_pdev(
> - "gpio-keys-polled",
> - &apu2_keys_pdata,
> - sizeof(apu2_keys_pdata));
> + if (cmp_version("v4.10", bios_ver) < 0) {
> + /* Newer bios configure LEDs/keys via ACPI */
> +
> + if (driver_data->apu_keys_pdata) {
> + gpiod_add_lookup_table(&gpios_key_table);
> + apu_keys_pdev = apu_create_pdev(
> + "gpio-keys-polled",
> + driver_data->apu_keys_pdata,
> + sizeof(struct gpio_keys_platform_data));
> + }
> +
> + if (driver_data->apu_leds_pdata) {
> + gpiod_add_lookup_table(&gpios_led_table);
> + apu_leds_pdev = apu_create_pdev(
> + "leds-gpio",
> + driver_data->apu_leds_pdata,
> + sizeof(struct gpio_led_platform_data));
> + }
> + }
>
> return 0;
> }
>
> static void __exit apu_board_exit(void)
> {
> - gpiod_remove_lookup_table(&gpios_led_table);
> - gpiod_remove_lookup_table(&gpios_key_table);
> + if (apu_keys_pdev) {
> + gpiod_remove_lookup_table(&gpios_key_table);
> + platform_device_unregister(apu_keys_pdev);
> + }
> + if (apu_leds_pdev) {
> + gpiod_remove_lookup_table(&gpios_led_table);
> + platform_device_unregister(apu_leds_pdev);
> + }
>
> - platform_device_unregister(apu_keys_pdev);
> - platform_device_unregister(apu_leds_pdev);
> platform_device_unregister(apu_gpio_pdev);
> }
>
>

--
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@xxxxxxxxx -- +49-151-27565287