Re: [PATCH 2/2] Input: atmel_mxt_ts - require device properties present when probing

From: Benson Leung
Date: Wed May 23 2018 - 15:10:28 EST


On Thu, May 03, 2018 at 05:41:35PM -0700, Dmitry Torokhov wrote:
> The driver needs help determining whether it is dealing with a touchscreen
> or a touchpad, and with button mapping. Previously we supported passing
> this data via device properties, and also had DMI lists for Chromebooks
> that specified Atmel devices in ACPI, but did not provide enough data
> there. Now that chromeos_laptop driver is adjusted to supply necessary
> device properties even for ACPI devices, we can drop the DMI tables and
> refuse to probe if device properties are not attached to the device.
>
> We use presence of "compatible" property to determine if device properties
> are attached to the device or not and rely on chromeos_laptop to re-probe
> the device after attaching missing device properties to it.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>

Looks good with the chromeos_laptop patch that preceeds this.
Reviewed-by: Benson Leung <bleung@xxxxxxxxxxxx>

> ---
> drivers/input/touchscreen/atmel_mxt_ts.c | 152 ++---------------------
> 1 file changed, 12 insertions(+), 140 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 09194721aed2d..4ac49c6c00060 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -2999,142 +2999,6 @@ static int mxt_parse_device_properties(struct mxt_data *data)
> return 0;
> }
>
> -#ifdef CONFIG_ACPI
> -
> -struct mxt_acpi_platform_data {
> - const char *hid;
> - const struct property_entry *props;
> -};
> -
> -static unsigned int samus_touchpad_buttons[] = {
> - KEY_RESERVED,
> - KEY_RESERVED,
> - KEY_RESERVED,
> - BTN_LEFT
> -};
> -
> -static const struct property_entry samus_touchpad_props[] = {
> - PROPERTY_ENTRY_U32_ARRAY("linux,gpio-keymap", samus_touchpad_buttons),
> - { }
> -};
> -
> -static struct mxt_acpi_platform_data samus_platform_data[] = {
> - {
> - /* Touchpad */
> - .hid = "ATML0000",
> - .props = samus_touchpad_props,
> - },
> - {
> - /* Touchscreen */
> - .hid = "ATML0001",
> - },
> - { }
> -};
> -
> -static unsigned int chromebook_tp_buttons[] = {
> - KEY_RESERVED,
> - KEY_RESERVED,
> - KEY_RESERVED,
> - KEY_RESERVED,
> - KEY_RESERVED,
> - BTN_LEFT
> -};
> -
> -static const struct property_entry chromebook_tp_props[] = {
> - PROPERTY_ENTRY_U32_ARRAY("linux,gpio-keymap", chromebook_tp_buttons),
> - { }
> -};
> -
> -static struct mxt_acpi_platform_data chromebook_platform_data[] = {
> - {
> - /* Touchpad */
> - .hid = "ATML0000",
> - .props = chromebook_tp_props,
> - },
> - {
> - /* Touchscreen */
> - .hid = "ATML0001",
> - },
> - { }
> -};
> -
> -static const struct dmi_system_id mxt_dmi_table[] = {
> - {
> - /* 2015 Google Pixel */
> - .ident = "Chromebook Pixel 2",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
> - DMI_MATCH(DMI_PRODUCT_NAME, "Samus"),
> - },
> - .driver_data = samus_platform_data,
> - },
> - {
> - /* Samsung Chromebook Pro */
> - .ident = "Samsung Chromebook Pro",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "Google"),
> - DMI_MATCH(DMI_PRODUCT_NAME, "Caroline"),
> - },
> - .driver_data = samus_platform_data,
> - },
> - {
> - /* Other Google Chromebooks */
> - .ident = "Chromebook",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
> - },
> - .driver_data = chromebook_platform_data,
> - },
> - { }
> -};
> -
> -static int mxt_prepare_acpi_properties(struct i2c_client *client)
> -{
> - struct acpi_device *adev;
> - const struct dmi_system_id *system_id;
> - const struct mxt_acpi_platform_data *acpi_pdata;
> -
> - adev = ACPI_COMPANION(&client->dev);
> - if (!adev)
> - return -ENOENT;
> -
> - system_id = dmi_first_match(mxt_dmi_table);
> - if (!system_id)
> - return -ENOENT;
> -
> - acpi_pdata = system_id->driver_data;
> - if (!acpi_pdata)
> - return -ENOENT;
> -
> - while (acpi_pdata->hid) {
> - if (!strcmp(acpi_device_hid(adev), acpi_pdata->hid)) {
> - /*
> - * Remove previously installed properties if we
> - * are probing this device not for the very first
> - * time.
> - */
> - device_remove_properties(&client->dev);
> -
> - /*
> - * Now install the platform-specific properties
> - * that are missing from ACPI.
> - */
> - device_add_properties(&client->dev, acpi_pdata->props);
> - break;
> - }
> -
> - acpi_pdata++;
> - }
> -
> - return 0;
> -}
> -#else
> -static int mxt_prepare_acpi_properties(struct i2c_client *client)
> -{
> - return -ENOENT;
> -}
> -#endif
> -
> static const struct dmi_system_id chromebook_T9_suspend_dmi[] = {
> {
> .matches = {
> @@ -3155,6 +3019,18 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
> struct mxt_data *data;
> int error;
>
> + /*
> + * Ignore devices that do not have device properties attached to
> + * them, as we need help determining whether we are dealing with
> + * touch screen or touchpad.
> + *
> + * So far on x86 the only users of Atmel touch controllers are
> + * Chromebooks, and chromeos_laptop driver will ensure that
> + * necessary properties are provided (if firmware does not do that).
> + */
> + if (!device_property_present(&client->dev, "compatible"))
> + return -ENXIO;
> +
> /*
> * Ignore ACPI devices representing bootloader mode.
> *
> @@ -3186,10 +3062,6 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
> data->suspend_mode = dmi_check_system(chromebook_T9_suspend_dmi) ?
> MXT_SUSPEND_T9_CTRL : MXT_SUSPEND_DEEP_SLEEP;
>
> - error = mxt_prepare_acpi_properties(client);
> - if (error && error != -ENOENT)
> - return error;
> -
> error = mxt_parse_device_properties(data);
> if (error)
> return error;
> --
> 2.17.0.441.gb46fe60e1d-goog
>

--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@xxxxxxxxxx
Chromium OS Project
bleung@xxxxxxxxxxxx

Attachment: signature.asc
Description: PGP signature