[PATCH v5 0/3] platform/chrome: Support for cros_ec_accel_legacy driver

[PATCH v5 0/3] platform/chrome: Support for cros_ec_accel_legacy driver

[Thierry Escande]

Hi,

This series enables the legacy accelerometer driver used on Chromebook
devices with older EC firmware.

The cros_ec_accel_legacy driver itself is no more part of this patchset
as it has already been merged in mainline. This patchset still contains
the patch that registers the cros_ec_accel_legacy driver if the usual
way of registering the croc_ec sensors hub fails. This patch has been
rebased on top of [1].

In addition to the cros_ec_accel_legacy driver, this series contains a
fix that registers the cros_ec_lpc driver on Chromebook devices that
does not have the GOOG0004 ACPI entry. In such case, the driver register
the device itself. This series also adds support to the cros_ec_lpc
driver for Glimmer based devices (Lenovo Yoga 11e).

[1] https://lkml.org/lkml/2017/11/20/408

Changes in v5:
- Remove the driver which is already merged
- Rebased on top of patchset [1]

Changes in v4:
- Use correct bitmask for scale channel specification
- Use precomputed scale factor value

Changes in v3:
- Use kernel-doc notation for function headers
- Add more comment to sensor ID sysfs attribute documentation
- Restore accidentally deleted comment for IIO_CHAN_INFO_CALIBBIAS handling in
  iio_info write_raw() callback

Changes in v2:
- Reorganize code to avoid forward declarations
- Simplify capture buffer declaration (also helps to remove forward
  declarations)
- Make use of iio_push_to_buffers_with_timestamp() and let the
  framework handle timestamp copy into capture buffer
- Added missing sysfs attribute documentation
- Few cosmetic changes here and there


Enric Balletbo i Serra (1):
  platform/chrome: cros_ec_lpc: Register the driver if ACPI entry is
    missing.

Thierry Escande (2):
  platform/chrome: cros_ec_lpc: Add support for Google Glimmer
  platform/chrome: Register cros_ec_accel_legacy driver

 drivers/mfd/cros_ec_dev.c             | 53 +++++++++++++++++++++++++++++++++++
 drivers/platform/chrome/cros_ec_lpc.c | 41 ++++++++++++++++++++++++++-
 2 files changed, 93 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH v5 1/3] platform/chrome: cros_ec_lpc: Register the driver if ACPI entry is missing.

[Thierry Escande]

From: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Commit 12278dc7c572 ("platform/chrome: cros_ec_lpc: Add support for
GOOG004 ACPI device") added support when the firmware reports the ACPI
device, there are some firmwares though that doesn't report this device
but have it. In such cases we need to instantiate the driver explicitly
if it is not instantiated through ACPI.

Fixes: 12278dc7c572 ("platform/chrome: cros_ec_lpc: Add support for GOOG004 ACPI device")
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/platform/chrome/cros_ec_lpc.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 1baf720..0b26a09 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -35,6 +35,9 @@
 #define DRV_NAME "cros_ec_lpcs"
 #define ACPI_DRV_NAME "GOOG0004"
 
+/* True if ACPI device is present */
+static bool cros_ec_lpc_acpi_device_found;
+
 static int ec_response_timed_out(void)
 {
 	unsigned long one_second = jiffies + HZ;
@@ -396,9 +399,21 @@ static struct platform_driver cros_ec_lpc_driver = {
 	.remove = cros_ec_lpc_remove,
 };
 
+static struct platform_device cros_ec_lpc_device = {
+	.name = DRV_NAME
+};
+
+static acpi_status cros_ec_lpc_parse_device(acpi_handle handle, u32 level,
+					    void *context, void **retval)
+{
+	*(bool *)context = true;
+	return AE_CTRL_TERMINATE;
+}
+
 static int __init cros_ec_lpc_init(void)
 {
 	int ret;
+	acpi_status status;
 
 	if (!dmi_check_system(cros_ec_lpc_dmi_table)) {
 		pr_err(DRV_NAME ": unsupported system.\n");
@@ -415,11 +430,28 @@ static int __init cros_ec_lpc_init(void)
 		return ret;
 	}
 
-	return 0;
+	status = acpi_get_devices(ACPI_DRV_NAME, cros_ec_lpc_parse_device,
+				  &cros_ec_lpc_acpi_device_found, NULL);
+	if (ACPI_FAILURE(status))
+		pr_warn(DRV_NAME ": Looking for %s failed\n", ACPI_DRV_NAME);
+
+	if (!cros_ec_lpc_acpi_device_found) {
+		/* Register the device, and it'll get hooked up automatically */
+		ret = platform_device_register(&cros_ec_lpc_device);
+		if (ret) {
+			pr_err(DRV_NAME ": can't register device: %d\n", ret);
+			platform_driver_unregister(&cros_ec_lpc_driver);
+			cros_ec_lpc_reg_destroy();
+		}
+	}
+
+	return ret;
 }
 
 static void __exit cros_ec_lpc_exit(void)
 {
+	if (!cros_ec_lpc_acpi_device_found)
+		platform_device_unregister(&cros_ec_lpc_device);
 	platform_driver_unregister(&cros_ec_lpc_driver);
 	cros_ec_lpc_reg_destroy();
 }
-- 
2.7.4

[PATCH v5 2/3] platform/chrome: cros_ec_lpc: Add support for Google Glimmer

[Thierry Escande]

This patch adds device information to the DMI table of the cros_ec_lpc
driver for Google Glimmer devices. Since Google BIOS does not enumerate
devices in the LPC bus, the cros_ec_lpc driver checks for system
compatibility and registers the cros_ec device itself.

Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/platform/chrome/cros_ec_lpc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 0b26a09..4a2fc55 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -365,6 +365,13 @@ static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Peppy"),
 		},
 	},
+	{
+		/* x86-glimmer, the Lenovo Thinkpad Yoga 11e. */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Glimmer"),
+		},
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(dmi, cros_ec_lpc_dmi_table);
-- 
2.7.4

[PATCH v5 3/3] platform/chrome: Register cros_ec_accel_legacy driver

[Thierry Escande]

With this patch, the cros_ec_ctl driver will register the legacy
accelerometer driver (named cros_ec_accel_legacy) if it fails to
register sensors through the usual path cros_ec_sensors_register().
This legacy device is present on Chromebook devices with older EC
firmware only supporting deprecated EC commands (Glimmer based devices).

Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/mfd/cros_ec_dev.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 3c4859d..1914c6e 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -389,6 +389,56 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
 	kfree(msg);
 }
 
+#define CROS_EC_SENSOR_LEGACY_NUM 2
+static struct mfd_cell cros_ec_accel_legacy_cells[CROS_EC_SENSOR_LEGACY_NUM];
+
+static void cros_ec_accel_legacy_register(struct cros_ec_dev *ec)
+{
+	struct cros_ec_device *ec_dev = ec->ec_dev;
+	u8 status;
+	int i, ret;
+	struct cros_ec_sensor_platform
+		sensor_platforms[CROS_EC_SENSOR_LEGACY_NUM];
+
+	/*
+	 * Check if EC supports direct memory reads and if EC has
+	 * accelerometers.
+	 */
+	if (!ec_dev->cmd_readmem)
+		return;
+
+	ret = ec_dev->cmd_readmem(ec_dev, EC_MEMMAP_ACC_STATUS, 1, &status);
+	if (ret < 0) {
+		dev_warn(ec->dev, "EC does not support direct reads.\n");
+		return;
+	}
+
+	/* Check if EC has accelerometers. */
+	if (!(status & EC_MEMMAP_ACC_STATUS_PRESENCE_BIT)) {
+		dev_info(ec->dev, "EC does not have accelerometers.\n");
+		return;
+	}
+
+	/*
+	 * Register 2 accelerometers
+	 */
+	for (i = 0; i < CROS_EC_SENSOR_LEGACY_NUM; i++) {
+		cros_ec_accel_legacy_cells[i].name = "cros-ec-accel-legacy";
+		sensor_platforms[i].sensor_num = i;
+		cros_ec_accel_legacy_cells[i].id = i;
+		cros_ec_accel_legacy_cells[i].platform_data =
+			&sensor_platforms[i];
+		cros_ec_accel_legacy_cells[i].pdata_size =
+			sizeof(struct cros_ec_sensor_platform);
+	}
+	ret = mfd_add_devices(ec->dev, PLATFORM_DEVID_AUTO,
+			      cros_ec_accel_legacy_cells,
+			      CROS_EC_SENSOR_LEGACY_NUM,
+			      NULL, 0, NULL);
+	if (ret)
+		dev_err(ec_dev->dev, "failed to add EC sensors\n");
+}
+
 static int ec_device_probe(struct platform_device *pdev)
 {
 	int retval = -ENOMEM;
@@ -436,6 +486,9 @@ static int ec_device_probe(struct platform_device *pdev)
 	/* check whether this EC is a sensor hub. */
 	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
 		cros_ec_sensors_register(ec);
+	else
+		/* Workaroud for older EC firmware */
+		cros_ec_accel_legacy_register(ec);
 
 	/* Take control of the lightbar from the EC. */
 	lb_manual_suspend_ctrl(ec, 1);
-- 
2.7.4

Re: [PATCH v5 1/3] platform/chrome: cros_ec_lpc: Register the driver if ACPI entry is missing.

[Gwendal Grignou]

Checked against 7412f0a0d90ee6ddbad4cde794f88f1489422f3a (CHROMIUM:
platform/chrome: Support MKBP protocol over ACPI)
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>

On Fri, Dec 1, 2017 at 5:42 AM, Thierry Escande
<thierry.escande@collabora.com> wrote:
> From: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>
> Commit 12278dc7c572 ("platform/chrome: cros_ec_lpc: Add support for
> GOOG004 ACPI device") added support when the firmware reports the ACPI
> device, there are some firmwares though that doesn't report this device
> but have it. In such cases we need to instantiate the driver explicitly
> if it is not instantiated through ACPI.
>
> Fixes: 12278dc7c572 ("platform/chrome: cros_ec_lpc: Add support for GOOG004 ACPI device")
> Signed-off-by: Guenter Roeck <groeck@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> ---
>  drivers/platform/chrome/cros_ec_lpc.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 1baf720..0b26a09 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -35,6 +35,9 @@
>  #define DRV_NAME "cros_ec_lpcs"
>  #define ACPI_DRV_NAME "GOOG0004"
>
> +/* True if ACPI device is present */
> +static bool cros_ec_lpc_acpi_device_found;
> +
>  static int ec_response_timed_out(void)
>  {
>         unsigned long one_second = jiffies + HZ;
> @@ -396,9 +399,21 @@ static struct platform_driver cros_ec_lpc_driver = {
>         .remove = cros_ec_lpc_remove,
>  };
>
> +static struct platform_device cros_ec_lpc_device = {
> +       .name = DRV_NAME
> +};
> +
> +static acpi_status cros_ec_lpc_parse_device(acpi_handle handle, u32 level,
> +                                           void *context, void **retval)
> +{
> +       *(bool *)context = true;
> +       return AE_CTRL_TERMINATE;
> +}
> +
>  static int __init cros_ec_lpc_init(void)
>  {
>         int ret;
> +       acpi_status status;
>
>         if (!dmi_check_system(cros_ec_lpc_dmi_table)) {
>                 pr_err(DRV_NAME ": unsupported system.\n");
> @@ -415,11 +430,28 @@ static int __init cros_ec_lpc_init(void)
>                 return ret;
>         }
>
> -       return 0;
> +       status = acpi_get_devices(ACPI_DRV_NAME, cros_ec_lpc_parse_device,
> +                                 &cros_ec_lpc_acpi_device_found, NULL);
> +       if (ACPI_FAILURE(status))
> +               pr_warn(DRV_NAME ": Looking for %s failed\n", ACPI_DRV_NAME);
> +
> +       if (!cros_ec_lpc_acpi_device_found) {
> +               /* Register the device, and it'll get hooked up automatically */
> +               ret = platform_device_register(&cros_ec_lpc_device);
> +               if (ret) {
> +                       pr_err(DRV_NAME ": can't register device: %d\n", ret);
> +                       platform_driver_unregister(&cros_ec_lpc_driver);
> +                       cros_ec_lpc_reg_destroy();
> +               }
> +       }
> +
> +       return ret;
>  }
>
>  static void __exit cros_ec_lpc_exit(void)
>  {
> +       if (!cros_ec_lpc_acpi_device_found)
> +               platform_device_unregister(&cros_ec_lpc_device);
>         platform_driver_unregister(&cros_ec_lpc_driver);
>         cros_ec_lpc_reg_destroy();
>  }
> --
> 2.7.4
>

Re: [PATCH v5 2/3] platform/chrome: cros_ec_lpc: Add support for Google Glimmer

[Gwendal Grignou]

This is not required.
Looking with dmidecode, Glimmer reports:
...
BIOS Information
        Vendor: coreboot
        Version: Google_Glimmer.5216.198.19
...

Therefore, the first entry of cros_ec_lpc_dmi_table will match.

Gwendal.

On Fri, Dec 1, 2017 at 5:42 AM, Thierry Escande
<thierry.escande@collabora.com> wrote:
> This patch adds device information to the DMI table of the cros_ec_lpc
> driver for Google Glimmer devices. Since Google BIOS does not enumerate
> devices in the LPC bus, the cros_ec_lpc driver checks for system
> compatibility and registers the cros_ec device itself.
>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> ---
>  drivers/platform/chrome/cros_ec_lpc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 0b26a09..4a2fc55 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -365,6 +365,13 @@ static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
>                         DMI_MATCH(DMI_PRODUCT_NAME, "Peppy"),
>                 },
>         },
> +       {
> +               /* x86-glimmer, the Lenovo Thinkpad Yoga 11e. */
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Glimmer"),
> +               },
> +       },
>         { /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(dmi, cros_ec_lpc_dmi_table);
> --
> 2.7.4
>

Re: [PATCH v5 3/3] platform/chrome: Register cros_ec_accel_legacy driver

[Gwendal Grignou]

Checked against 3bf98755f9c670c5c10ca05cba22848d65117cb2 (CHROMIUM:
iio: accel: Add cros_ec_accel_legacy driver)

Fixup f2b141a242e59017dbc774dc916748670a41da0b (FIXUP: CHROMIUM: iio:
accel: Add cros_ec_accel_legacy driver) is required for devices with
secondary Embedded Controllers.

Gwendal.

On Fri, Dec 1, 2017 at 5:42 AM, Thierry Escande
<thierry.escande@collabora.com> wrote:
> With this patch, the cros_ec_ctl driver will register the legacy
> accelerometer driver (named cros_ec_accel_legacy) if it fails to
> register sensors through the usual path cros_ec_sensors_register().
> This legacy device is present on Chromebook devices with older EC
> firmware only supporting deprecated EC commands (Glimmer based devices).
>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> ---
>  drivers/mfd/cros_ec_dev.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index 3c4859d..1914c6e 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -389,6 +389,56 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
>         kfree(msg);
>  }
>
> +#define CROS_EC_SENSOR_LEGACY_NUM 2
> +static struct mfd_cell cros_ec_accel_legacy_cells[CROS_EC_SENSOR_LEGACY_NUM];
> +
> +static void cros_ec_accel_legacy_register(struct cros_ec_dev *ec)
> +{
> +       struct cros_ec_device *ec_dev = ec->ec_dev;
> +       u8 status;
> +       int i, ret;
> +       struct cros_ec_sensor_platform
> +               sensor_platforms[CROS_EC_SENSOR_LEGACY_NUM];
> +
> +       /*
> +        * Check if EC supports direct memory reads and if EC has
> +        * accelerometers.
> +        */
> +       if (!ec_dev->cmd_readmem)
> +               return;
> +
> +       ret = ec_dev->cmd_readmem(ec_dev, EC_MEMMAP_ACC_STATUS, 1, &status);
> +       if (ret < 0) {
> +               dev_warn(ec->dev, "EC does not support direct reads.\n");
> +               return;
> +       }
> +
> +       /* Check if EC has accelerometers. */
> +       if (!(status & EC_MEMMAP_ACC_STATUS_PRESENCE_BIT)) {
> +               dev_info(ec->dev, "EC does not have accelerometers.\n");
> +               return;
> +       }
> +
> +       /*
> +        * Register 2 accelerometers
> +        */
> +       for (i = 0; i < CROS_EC_SENSOR_LEGACY_NUM; i++) {
> +               cros_ec_accel_legacy_cells[i].name = "cros-ec-accel-legacy";
> +               sensor_platforms[i].sensor_num = i;
> +               cros_ec_accel_legacy_cells[i].id = i;
> +               cros_ec_accel_legacy_cells[i].platform_data =
> +                       &sensor_platforms[i];
> +               cros_ec_accel_legacy_cells[i].pdata_size =
> +                       sizeof(struct cros_ec_sensor_platform);
> +       }
> +       ret = mfd_add_devices(ec->dev, PLATFORM_DEVID_AUTO,
> +                             cros_ec_accel_legacy_cells,
> +                             CROS_EC_SENSOR_LEGACY_NUM,
> +                             NULL, 0, NULL);
> +       if (ret)
> +               dev_err(ec_dev->dev, "failed to add EC sensors\n");
> +}
> +
>  static int ec_device_probe(struct platform_device *pdev)
>  {
>         int retval = -ENOMEM;
> @@ -436,6 +486,9 @@ static int ec_device_probe(struct platform_device *pdev)
>         /* check whether this EC is a sensor hub. */
>         if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
>                 cros_ec_sensors_register(ec);
> +       else
> +               /* Workaroud for older EC firmware */
> +               cros_ec_accel_legacy_register(ec);
>
>         /* Take control of the lightbar from the EC. */
>         lb_manual_suspend_ctrl(ec, 1);
> --
> 2.7.4
>

Re: [PATCH v5 2/3] platform/chrome: cros_ec_lpc: Add support for Google Glimmer

[Thierry Escande]

Hi Gwendal,

On 01/12/2017 20:54, Gwendal Grignou wrote:
> This is not required.
> Looking with dmidecode, Glimmer reports:
> ...
> BIOS Information
>          Vendor: coreboot
>          Version: Google_Glimmer.5216.198.19
> ...
> 
> Therefore, the first entry of cros_ec_lpc_dmi_table will match.

These DMI vendor/version strings are not exposed when booting in legacy 
mode using SeaBIOS on the Yoga 11e. Instead it matches with the pair 
GOOGLE/Glimmer. So this patch is needed for booting a vanilla kernel in 
legacy mode.

Regards,
  Thierry

> 
> Gwendal.
> 
> On Fri, Dec 1, 2017 at 5:42 AM, Thierry Escande
> <thierry.escande@collabora.com> wrote:
>> This patch adds device information to the DMI table of the cros_ec_lpc
>> driver for Google Glimmer devices. Since Google BIOS does not enumerate
>> devices in the LPC bus, the cros_ec_lpc driver checks for system
>> compatibility and registers the cros_ec device itself.
>>
>> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
>> ---
>>   drivers/platform/chrome/cros_ec_lpc.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
>> index 0b26a09..4a2fc55 100644
>> --- a/drivers/platform/chrome/cros_ec_lpc.c
>> +++ b/drivers/platform/chrome/cros_ec_lpc.c
>> @@ -365,6 +365,13 @@ static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
>>                          DMI_MATCH(DMI_PRODUCT_NAME, "Peppy"),
>>                  },
>>          },
>> +       {
>> +               /* x86-glimmer, the Lenovo Thinkpad Yoga 11e. */
>> +               .matches = {
>> +                       DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
>> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Glimmer"),
>> +               },
>> +       },
>>          { /* sentinel */ }
>>   };
>>   MODULE_DEVICE_TABLE(dmi, cros_ec_lpc_dmi_table);
>> --
>> 2.7.4
>>

Re: [PATCH v5 2/3] platform/chrome: cros_ec_lpc: Add support for Google Glimmer

[Gwendal Grignou]

On Tue, Dec 5, 2017 at 1:55 PM, Thierry Escande
<thierry.escande@collabora.com> wrote:
> Hi Gwendal,
>
> On 01/12/2017 20:54, Gwendal Grignou wrote:
>>
>> This is not required.
>> Looking with dmidecode, Glimmer reports:
>> ...
>> BIOS Information
>>          Vendor: coreboot
>>          Version: Google_Glimmer.5216.198.19
>> ...
>>
>> Therefore, the first entry of cros_ec_lpc_dmi_table will match.
>
>
> These DMI vendor/version strings are not exposed when booting in legacy mode
> using SeaBIOS on the Yoga 11e. Instead it matches with the pair
> GOOGLE/Glimmer. So this patch is needed for booting a vanilla kernel in
> legacy mode.
Good point. Given we will have the same issue with TianoCore, we have
to use a DMI product match. Newer ACPI based chromebooks registers
their EC dynamically , but looking at cros_ec_lpc.c, all registrations
are gated on a dmi table match. This is only required for older
devices.

In the meantime,
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>

> Regards,
>  Thierry
>
>
>>
>> Gwendal.
>>
>> On Fri, Dec 1, 2017 at 5:42 AM, Thierry Escande
>> <thierry.escande@collabora.com> wrote:
>>>
>>> This patch adds device information to the DMI table of the cros_ec_lpc
>>> driver for Google Glimmer devices. Since Google BIOS does not enumerate
>>> devices in the LPC bus, the cros_ec_lpc driver checks for system
>>> compatibility and registers the cros_ec device itself.
>>>
>>> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
>>> ---
>>>   drivers/platform/chrome/cros_ec_lpc.c | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/platform/chrome/cros_ec_lpc.c
>>> b/drivers/platform/chrome/cros_ec_lpc.c
>>> index 0b26a09..4a2fc55 100644
>>> --- a/drivers/platform/chrome/cros_ec_lpc.c
>>> +++ b/drivers/platform/chrome/cros_ec_lpc.c
>>> @@ -365,6 +365,13 @@ static const struct dmi_system_id
>>> cros_ec_lpc_dmi_table[] __initconst = {
>>>                          DMI_MATCH(DMI_PRODUCT_NAME, "Peppy"),
>>>                  },
>>>          },
>>> +       {
>>> +               /* x86-glimmer, the Lenovo Thinkpad Yoga 11e. */
>>> +               .matches = {
>>> +                       DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
>>> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Glimmer"),
>>> +               },
>>> +       },
>>>          { /* sentinel */ }
>>>   };
>>>   MODULE_DEVICE_TABLE(dmi, cros_ec_lpc_dmi_table);
>>> --
>>> 2.7.4
>>>
>

Re: [PATCH v5 3/3] platform/chrome: Register cros_ec_accel_legacy driver

[Lee Jones]

On Fri, 01 Dec 2017, Gwendal Grignou wrote:

> Checked against 3bf98755f9c670c5c10ca05cba22848d65117cb2 (CHROMIUM:
> iio: accel: Add cros_ec_accel_legacy driver)
> 
> Fixup f2b141a242e59017dbc774dc916748670a41da0b (FIXUP: CHROMIUM: iio:
> accel: Add cros_ec_accel_legacy driver) is required for devices with
> secondary Embedded Controllers.

Is that an Ack or a NAck?

And pleeeeeease don't top post.

> On Fri, Dec 1, 2017 at 5:42 AM, Thierry Escande
> <thierry.escande@collabora.com> wrote:
> > With this patch, the cros_ec_ctl driver will register the legacy
> > accelerometer driver (named cros_ec_accel_legacy) if it fails to
> > register sensors through the usual path cros_ec_sensors_register().
> > This legacy device is present on Chromebook devices with older EC
> > firmware only supporting deprecated EC commands (Glimmer based devices).
> >
> > Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> > ---
> >  drivers/mfd/cros_ec_dev.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 53 insertions(+)

[...]

-- 
Lee Jones
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v5 1/3] platform/chrome: cros_ec_lpc: Register the driver if ACPI entry is missing.

[Benson Leung]

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

Hi Thierry,

Sorry for the delay in processing these.

On Fri, Dec 01, 2017 at 02:42:21PM +0100, Thierry Escande wrote:
> From: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> 
> Commit 12278dc7c572 ("platform/chrome: cros_ec_lpc: Add support for
> GOOG004 ACPI device") added support when the firmware reports the ACPI
> device, there are some firmwares though that doesn't report this device
> but have it. In such cases we need to instantiate the driver explicitly
> if it is not instantiated through ACPI.
> 
> Fixes: 12278dc7c572 ("platform/chrome: cros_ec_lpc: Add support for GOOG004 ACPI device")
> Signed-off-by: Guenter Roeck <groeck@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>

Applied for v4.16.

Thanks,
Benson

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v5 1/3] platform/chrome: cros_ec_lpc: Register the driver if ACPI entry is missing.

[Benson Leung]

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

Hi Thierry,

On Fri, Dec 01, 2017 at 02:42:21PM +0100, Thierry Escande wrote:
> From: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> 
> Commit 12278dc7c572 ("platform/chrome: cros_ec_lpc: Add support for
> GOOG004 ACPI device") added support when the firmware reports the ACPI
> device, there are some firmwares though that doesn't report this device
> but have it. In such cases we need to instantiate the driver explicitly
> if it is not instantiated through ACPI.
> 
> Fixes: 12278dc7c572 ("platform/chrome: cros_ec_lpc: Add support for GOOG004 ACPI device")
> Signed-off-by: Guenter Roeck <groeck@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>

Applied for v4.16.

> ---
>  drivers/platform/chrome/cros_ec_lpc.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 1baf720..0b26a09 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -35,6 +35,9 @@
>  #define DRV_NAME "cros_ec_lpcs"
>  #define ACPI_DRV_NAME "GOOG0004"
>  
> +/* True if ACPI device is present */
> +static bool cros_ec_lpc_acpi_device_found;
> +
>  static int ec_response_timed_out(void)
>  {
>  	unsigned long one_second = jiffies + HZ;
> @@ -396,9 +399,21 @@ static struct platform_driver cros_ec_lpc_driver = {
>  	.remove = cros_ec_lpc_remove,
>  };
>  
> +static struct platform_device cros_ec_lpc_device = {
> +	.name = DRV_NAME
> +};
> +
> +static acpi_status cros_ec_lpc_parse_device(acpi_handle handle, u32 level,
> +					    void *context, void **retval)
> +{
> +	*(bool *)context = true;
> +	return AE_CTRL_TERMINATE;
> +}
> +
>  static int __init cros_ec_lpc_init(void)
>  {
>  	int ret;
> +	acpi_status status;
>  
>  	if (!dmi_check_system(cros_ec_lpc_dmi_table)) {
>  		pr_err(DRV_NAME ": unsupported system.\n");
> @@ -415,11 +430,28 @@ static int __init cros_ec_lpc_init(void)
>  		return ret;
>  	}
>  
> -	return 0;
> +	status = acpi_get_devices(ACPI_DRV_NAME, cros_ec_lpc_parse_device,
> +				  &cros_ec_lpc_acpi_device_found, NULL);
> +	if (ACPI_FAILURE(status))
> +		pr_warn(DRV_NAME ": Looking for %s failed\n", ACPI_DRV_NAME);
> +
> +	if (!cros_ec_lpc_acpi_device_found) {
> +		/* Register the device, and it'll get hooked up automatically */
> +		ret = platform_device_register(&cros_ec_lpc_device);
> +		if (ret) {
> +			pr_err(DRV_NAME ": can't register device: %d\n", ret);
> +			platform_driver_unregister(&cros_ec_lpc_driver);
> +			cros_ec_lpc_reg_destroy();
> +		}
> +	}
> +
> +	return ret;
>  }
>  
>  static void __exit cros_ec_lpc_exit(void)
>  {
> +	if (!cros_ec_lpc_acpi_device_found)
> +		platform_device_unregister(&cros_ec_lpc_device);
>  	platform_driver_unregister(&cros_ec_lpc_driver);
>  	cros_ec_lpc_reg_destroy();
>  }
> -- 
> 2.7.4
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v5 2/3] platform/chrome: cros_ec_lpc: Add support for Google Glimmer

[Benson Leung]

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

Hi Thierry,

On Fri, Dec 01, 2017 at 02:42:22PM +0100, Thierry Escande wrote:
> This patch adds device information to the DMI table of the cros_ec_lpc
> driver for Google Glimmer devices. Since Google BIOS does not enumerate
> devices in the LPC bus, the cros_ec_lpc driver checks for system
> compatibility and registers the cros_ec device itself.
> 
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>

Applied. Thanks.

> ---
>  drivers/platform/chrome/cros_ec_lpc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 0b26a09..4a2fc55 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -365,6 +365,13 @@ static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
>  			DMI_MATCH(DMI_PRODUCT_NAME, "Peppy"),
>  		},
>  	},
> +	{
> +		/* x86-glimmer, the Lenovo Thinkpad Yoga 11e. */
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Glimmer"),
> +		},
> +	},
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(dmi, cros_ec_lpc_dmi_table);
> -- 
> 2.7.4
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v5 1/3] platform/chrome: cros_ec_lpc: Register the driver if ACPI entry is missing.

[Benson Leung]

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

On Fri, Dec 01, 2017 at 11:49:54AM -0800, Gwendal Grignou wrote:
> Checked against 7412f0a0d90ee6ddbad4cde794f88f1489422f3a (CHROMIUM:
> platform/chrome: Support MKBP protocol over ACPI)
> Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
> 

Thanks Gwendal.

(ps, don't top post.)

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v5 3/3] platform/chrome: Register cros_ec_accel_legacy driver

[Gwendal Grignou]

On Thu, Dec 7, 2017 at 1:31 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Fri, 01 Dec 2017, Gwendal Grignou wrote:
>
>> Checked against 3bf98755f9c670c5c10ca05cba22848d65117cb2 (CHROMIUM:
>> iio: accel: Add cros_ec_accel_legacy driver)
>>
>> Fixup f2b141a242e59017dbc774dc916748670a41da0b (FIXUP: CHROMIUM: iio:
>> accel: Add cros_ec_accel_legacy driver) is required for devices with
>> secondary Embedded Controllers.
>
> Is that an Ack or a NAck?
My response was incomplete: it is a nack.
Thierry's patch does match 3bf98755f9, but patch f2b141a242e should be
merged in.
>
> And pleeeeeease don't top post.
Sorry,

Gwendal.
>
>> On Fri, Dec 1, 2017 at 5:42 AM, Thierry Escande
>> <thierry.escande@collabora.com> wrote:
>> > With this patch, the cros_ec_ctl driver will register the legacy
>> > accelerometer driver (named cros_ec_accel_legacy) if it fails to
>> > register sensors through the usual path cros_ec_sensors_register().
>> > This legacy device is present on Chromebook devices with older EC
>> > firmware only supporting deprecated EC commands (Glimmer based devices).
>> >
>> > Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
>> > ---
>> >  drivers/mfd/cros_ec_dev.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 53 insertions(+)
>
> [...]
>
> --
> Lee Jones
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog