From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Subject: Re: [PATCH v3] platform/x86: add support for devices with Silead touchscreens Date: Sun, 22 Jan 2017 21:05:10 +0100 Message-ID: References: <20170122152551.10423-1-hdegoede@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.redhat.com ([209.132.183.28]:50788 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751190AbdAVUFT (ORCPT ); Sun, 22 Jan 2017 15:05:19 -0500 In-Reply-To: Sender: linux-input-owner@vger.kernel.org List-Id: linux-input@vger.kernel.org To: Andy Shevchenko Cc: Dmitry Torokhov , Darren Hart , "russianneuromancer @ ya . ru" , Gregor Riepl , linux-input , Platform Driver Hi, On 22-01-17 17:49, Andy Shevchenko wrote: > On Sun, Jan 22, 2017 at 5:25 PM, Hans de Goede wrote: >> On ACPI based tablets, the ACPI touchscreen node only contains info on >> the gpio and the irq, and is missing any info on the axis. This info is >> expected to be built into the tablet model specific version of the driver >> shipped with the os-image for the device. >> >> Add support for getting the missing info from a table built into the >> driver, using dmi data to identify which entry of the table to use and >> add info for the CUBE iwork8 Air and Jumper EZpad mini3 tablets on which >> this code was tested / developed. >> >> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=187531 >> Signed-off-by: Hans de Goede >> [dmitry.torokhov@gmail.com: Move to platform/x86] >> Signed-off-by: Dmitry Torokhov > > Few nitpicks below. After addressing them > Acked-by: Andy Shevchenko Thank you. > (in case Dmitry would like to push this via input subsystem) I've addressed all your remarks, I will send a v4 shortly. Regards, Hans > >> --- >> Changes in v2: >> -Put the dmi code in a separate silead_dmi.c file >> -Use device_add_properties to add the info >> Changes in v3 (Dmitry Torokhov): >> -Move the silead_dmi code to drivers/platform/x86 >> --- >> MAINTAINERS | 8 +++ >> drivers/platform/x86/Kconfig | 11 +++ >> drivers/platform/x86/Makefile | 1 + >> drivers/platform/x86/silead_dmi.c | 136 ++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 156 insertions(+) >> create mode 100644 drivers/platform/x86/silead_dmi.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index bdc4843..5591ac3 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -11255,6 +11255,14 @@ F: drivers/media/usb/siano/ >> F: drivers/media/usb/siano/ >> F: drivers/media/mmc/siano/ >> >> +SILEAD TOUCHSCREEN DRIVER >> +M: Hans de Goede >> +L: linux-input@vger.kernel.org >> +L: platform-driver-x86@vger.kernel.org >> +S: Maintained >> +F: drivers/input/touchscreen/silead.c >> +F: drivers/platform/x86/silead_dmi.c >> + >> SIMPLEFB FB DRIVER >> M: Hans de Goede >> L: linux-fbdev@vger.kernel.org >> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig >> index 59aa8e3..60be7bc 100644 >> --- a/drivers/platform/x86/Kconfig >> +++ b/drivers/platform/x86/Kconfig >> @@ -1076,4 +1076,15 @@ config MLX_CPLD_PLATFORM >> This driver handles hot-plug events for the power suppliers, power >> cables and fans on the wide range Mellanox IB and Ethernet systems. >> >> +config SILEAD_DMI >> + bool "Tablets with Silead touchscreens" >> + depends on ACPI && DMI && I2C && INPUT >> + ---help--- >> + Certain ACPI based tablets with Silead touchscreens do not have >> + enough data in ACPI tables for the touchscreen driver to handle >> + the touchscreen properly, as OEMs expected the data to be baked >> + into the tablet model specific version of the driver shipped >> + with the os-image for the device. This option supplies the missing > > os -> OS > >> + information. Enable this for x86 tablets with Silead touchscreens. >> + >> endif # X86_PLATFORM_DEVICES >> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile >> index d4111f0..840b07b 100644 >> --- a/drivers/platform/x86/Makefile >> +++ b/drivers/platform/x86/Makefile >> @@ -76,3 +76,4 @@ obj-$(CONFIG_INTEL_TELEMETRY) += intel_telemetry_core.o \ >> obj-$(CONFIG_INTEL_PMC_CORE) += intel_pmc_core.o >> obj-$(CONFIG_MLX_PLATFORM) += mlx-platform.o >> obj-$(CONFIG_MLX_CPLD_PLATFORM) += mlxcpld-hotplug.o >> +obj-$(CONFIG_SILEAD_DMI) += silead_dmi.o >> diff --git a/drivers/platform/x86/silead_dmi.c b/drivers/platform/x86/silead_dmi.c >> new file mode 100644 >> index 0000000..11b0a46 >> --- /dev/null >> +++ b/drivers/platform/x86/silead_dmi.c >> @@ -0,0 +1,136 @@ >> +/* >> + * Silead touchscreen driver DMI based configuration code >> + * >> + * Copyright (c) 2017 Red Hat Inc. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * Red Hat authors: >> + * Hans de Goede >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +struct silead_ts_dmi_data { >> + const char *acpi_name; >> + struct property_entry *properties; >> +}; >> + >> +static struct property_entry cube_iwork8_air_props[] = { >> + PROPERTY_ENTRY_U32("touchscreen-size-x", 1660), >> + PROPERTY_ENTRY_U32("touchscreen-size-y", 900), >> + PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"), >> + PROPERTY_ENTRY_STRING("firmware-name", "gsl3670-cube-iwork8-air.fw"), >> + PROPERTY_ENTRY_U32("silead,max-fingers", 10), >> + { } >> +}; >> + >> +static const struct silead_ts_dmi_data cube_iwork8_air_data = { >> + .acpi_name = "MSSL1680:00", >> + .properties = cube_iwork8_air_props, > > I hope indentation is correct and uses tabs. > >> +}; >> + >> +static struct property_entry jumper_ezpad_mini3_props[] = { >> + PROPERTY_ENTRY_U32("touchscreen-size-x", 1700), >> + PROPERTY_ENTRY_U32("touchscreen-size-y", 1150), >> + PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"), >> + PROPERTY_ENTRY_STRING("firmware-name", "gsl3676-jumper-ezpad-mini3.fw"), >> + PROPERTY_ENTRY_U32("silead,max-fingers", 10), >> + { } >> +}; >> + >> +static const struct silead_ts_dmi_data jumper_ezpad_mini3_data = { >> + .acpi_name = "MSSL1680:00", >> + .properties = jumper_ezpad_mini3_props, > > Ditto. > >> +}; >> + >> +static const struct dmi_system_id silead_ts_dmi_table[] = { >> + { >> + .ident = "CUBE iwork8 Air", >> + .driver_data = (void *)&cube_iwork8_air_data, >> + .matches = { >> + DMI_MATCH(DMI_SYS_VENDOR, "cube"), >> + DMI_MATCH(DMI_PRODUCT_NAME, "i1-TF"), >> + DMI_MATCH(DMI_BOARD_NAME, "Cherry Trail CR"), >> + }, >> + }, >> + { >> + .ident = "Jumper EZpad mini3", >> + .driver_data = (void *)&jumper_ezpad_mini3_data, >> + .matches = { >> + DMI_MATCH(DMI_SYS_VENDOR, "Insyde"), >> + /* jumperx.T87.KFBNEEA02 with the version-nr dropped */ >> + DMI_MATCH(DMI_BIOS_VERSION, "jumperx.T87.KFBNEEA"), >> + }, >> + }, >> + { }, >> +}; >> + >> +static void silead_ts_dmi_add_props(struct device *dev) >> +{ >> + struct i2c_client *client = to_i2c_client(dev); >> + const struct dmi_system_id *dmi_id; >> + const struct silead_ts_dmi_data *ts_data; >> + int error; >> + >> + dmi_id = dmi_first_match(silead_ts_dmi_table); >> + if (dmi_id) { > > if (!dmi_id) > return; > ? > >> + ts_data = dmi_id->driver_data; >> + if (ACPI_COMPANION(dev) && > > has_acpi_companion() > >> + !strncmp(ts_data->acpi_name, client->name, I2C_NAME_SIZE)) { >> + error = device_add_properties(dev, ts_data->properties); >> + if (error) >> + dev_err(dev, "failed to add properties: %d\n", >> + error); > > And perhaps use ret or err and put this to a single line. > >> + } >> + } >> +} >> + >> +static int silead_ts_dmi_notifier_call(struct notifier_block *nb, >> + unsigned long action, void *data) >> +{ >> + struct device *dev = data; >> + >> + switch (action) { >> + case BUS_NOTIFY_ADD_DEVICE: >> + silead_ts_dmi_add_props(dev); >> + break; >> + >> + default: >> + break; >> + } >> + >> + return 0; >> +} >> + >> +static struct notifier_block silead_ts_dmi_notifier = { >> + .notifier_call = silead_ts_dmi_notifier_call, >> +}; >> + >> +static int __init silead_ts_dmi_init(void) >> +{ >> + int error; >> + >> + error = bus_register_notifier(&i2c_bus_type, &silead_ts_dmi_notifier); >> + if (error) >> + pr_err("%s: failed to register i2c bus notifier: %d\n", >> + __func__, error); >> + >> + return error; > > If above variable would be renamed this would follow. > >> +} >> + >> +/* >> + * We are registering out notifier after i2c core is initialized and i2c bus >> + * itself is ready (which happens at postcore initcall level), but before >> + * ACPI starts enumerating devices (at subsys initcall level). >> + */ >> +arch_initcall(silead_ts_dmi_init); >> -- >> 2.9.3 >> > > >