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