[PATCH v2 0/8] mfd: cros_ec: add subdevices and fixes

[PATCH v2 0/8] mfd: cros_ec: add subdevices and fixes

[Enric Balletbo i Serra]

Hi,

This is a second version of a patchset that collects some patches
already send but that needed some rework due the patchset to split the
cros_ec_devs modules in 2 parts [1]. This patchset contains some of
imrpovements and also some fixes. They can be applied independently
and I think that all can go through the MFD tree.

Best regards,
 Enric

Changes in v2:
- [2/8] Remove the free_irq in cros_ec_remove.
- [3/8] That patch is new in this series.
- [4/8] Add the Reviewed-by Gwendal.
- [5/8] Add the Reviewed-by Gwendal.
- [6/8] This patch is new in this series.
- [7/8] Add the Reviewed-by Gwendal.
- [8/8] This patch is new in this series replacing [5/6] of v1.

Daniel Hung-yu Wu (1):
  mfd: cros_ec_dev: register shutdown function for debugfs

Douglas Anderson (1):
  mfd: cros_ec: Don't try to grab log when suspended

Enric Balletbo i Serra (1):
  mfd: cros_ec_dev: Register cros-ec-rtc driver as a subdevice.

Joseph Lo (1):
  mfd: cros_ec_i2c: moving the system sleep pm ops to late

Thierry Escande (1):
  mfd: cros_ec_dev: Register cros_ec_accel_legacy driver as a subdevice.

Vincent Palatin (2):
  mfd: cros_ec: fail early if we cannot identify the EC
  mfd: cros_ec: free IRQ automatically

Wei-Ning Huang (1):
  mfd: cros_ec_i2c: add ACPI module device table

 drivers/mfd/cros_ec.c                     | 26 ++++-----
 drivers/mfd/cros_ec_dev.c                 | 87 +++++++++++++++++++++++++++++++
 drivers/mfd/cros_ec_i2c.c                 | 17 +++++-
 drivers/platform/chrome/cros_ec_debugfs.c | 20 +++++++
 include/linux/mfd/cros_ec.h               |  2 +
 5 files changed, 135 insertions(+), 17 deletions(-)

-- 
2.16.1

[PATCH v2 1/8] mfd: cros_ec: fail early if we cannot identify the EC

[Enric Balletbo i Serra]

From: Vincent Palatin <vpalatin@chromium.org>

If we cannot communicate with the EC chip to detect the protocol version
and its features, it's very likely useless to continue. Else we will
commit all kind of uninformed mistakes (using the wrong protocol, the
wrong buffer size, mixing the EC with other chips).

Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
Acked-by: Benson Leung <bleung@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
---

Changes in v2: None

 drivers/mfd/cros_ec.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index d61024141e2b..74780f2964a1 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -112,7 +112,11 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 
 	mutex_init(&ec_dev->lock);
 
-	cros_ec_query_all(ec_dev);
+	err = cros_ec_query_all(ec_dev);
+	if (err) {
+		dev_err(dev, "Cannot identify the EC: error %d\n", err);
+		return err;
+	}
 
 	if (ec_dev->irq) {
 		err = request_threaded_irq(ec_dev->irq, NULL, ec_irq_thread,
-- 
2.16.1

[PATCH v2 2/8] mfd: cros_ec: free IRQ automatically

[Enric Balletbo i Serra]

From: Vincent Palatin <vpalatin@chromium.org>

Free the IRQ we might have requested when removing the cros_ec device,
so we can unload and reload the driver properly.

Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

---

Changes in v2:
- [2/8] Remove the free_irq in cros_ec_remove.

 drivers/mfd/cros_ec.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 74780f2964a1..ea8aa704b61e 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -119,9 +119,9 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 	}
 
 	if (ec_dev->irq) {
-		err = request_threaded_irq(ec_dev->irq, NULL, ec_irq_thread,
-					   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
-					   "chromeos-ec", ec_dev);
+		err = devm_request_threaded_irq(dev, ec_dev->irq, NULL,
+				ec_irq_thread, IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+				"chromeos-ec", ec_dev);
 		if (err) {
 			dev_err(dev, "Failed to request IRQ %d: %d",
 				ec_dev->irq, err);
@@ -135,7 +135,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 		dev_err(dev,
 			"Failed to register Embedded Controller subdevice %d\n",
 			err);
-		goto fail_mfd;
+		return err;
 	}
 
 	if (ec_dev->max_passthru) {
@@ -153,7 +153,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 			dev_err(dev,
 				"Failed to register Power Delivery subdevice %d\n",
 				err);
-			goto fail_mfd;
+			return err;
 		}
 	}
 
@@ -162,7 +162,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 		if (err) {
 			mfd_remove_devices(dev);
 			dev_err(dev, "Failed to register sub-devices\n");
-			goto fail_mfd;
+			return err;
 		}
 	}
 
@@ -180,11 +180,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 	cros_ec_acpi_install_gpe_handler(dev);
 
 	return 0;
-
-fail_mfd:
-	if (ec_dev->irq)
-		free_irq(ec_dev->irq, ec_dev);
-	return err;
 }
 EXPORT_SYMBOL(cros_ec_register);
 
@@ -194,9 +189,6 @@ int cros_ec_remove(struct cros_ec_device *ec_dev)
 
 	cros_ec_acpi_remove_gpe_handler();
 
-	if (ec_dev->irq)
-		free_irq(ec_dev->irq, ec_dev);
-
 	return 0;
 }
 EXPORT_SYMBOL(cros_ec_remove);
-- 
2.16.1

[PATCH v2 3/8] mfd: cros_ec: Don't try to grab log when suspended

[Enric Balletbo i Serra]

From: Douglas Anderson <dianders@chromium.org>

We should stop our worker thread while we're suspended.  If we don't
then we'll get messages like:

  cros-ec-spi spi5.0: spi transfer failed: -108
  cros-ec-spi spi5.0: cs-deassert spi transfer failed: -108
  cros-ec-ctl cros-ec-ctl.0.auto: EC communication failed

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v2:
- [3/8] That patch is new in this series.

 drivers/mfd/cros_ec_dev.c                 |  4 ++++
 drivers/platform/chrome/cros_ec_debugfs.c | 20 ++++++++++++++++++++
 include/linux/mfd/cros_ec.h               |  2 ++
 3 files changed, 26 insertions(+)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index e4fafdd96e5e..f98e5beffca6 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -471,6 +471,8 @@ static __maybe_unused int ec_device_suspend(struct device *dev)
 {
 	struct cros_ec_dev *ec = dev_get_drvdata(dev);
 
+	cros_ec_debugfs_suspend(ec);
+
 	lb_suspend(ec);
 
 	return 0;
@@ -480,6 +482,8 @@ static __maybe_unused int ec_device_resume(struct device *dev)
 {
 	struct cros_ec_dev *ec = dev_get_drvdata(dev);
 
+	cros_ec_debugfs_resume(ec);
+
 	lb_resume(ec);
 
 	return 0;
diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index 0e88e18362c1..0328856ec3a2 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -398,3 +398,23 @@ void cros_ec_debugfs_remove(struct cros_ec_dev *ec)
 	cros_ec_cleanup_console_log(ec->debug_info);
 }
 EXPORT_SYMBOL(cros_ec_debugfs_remove);
+
+void cros_ec_debugfs_suspend(struct cros_ec_dev *ec)
+{
+	/*
+	 * cros_ec_debugfs_init() failures are non-fatal; it's also possible
+	 * that we initted things but decided that console log wasn't supported.
+	 * We'll use the same set of checks that cros_ec_debugfs_remove() +
+	 * cros_ec_cleanup_console_log() end up using to handle those cases.
+	 */
+	if (ec->debug_info && ec->debug_info->log_buffer.buf)
+		cancel_delayed_work_sync(&ec->debug_info->log_poll_work);
+}
+EXPORT_SYMBOL(cros_ec_debugfs_suspend);
+
+void cros_ec_debugfs_resume(struct cros_ec_dev *ec)
+{
+	if (ec->debug_info && ec->debug_info->log_buffer.buf)
+		schedule_delayed_work(&ec->debug_info->log_poll_work, 0);
+}
+EXPORT_SYMBOL(cros_ec_debugfs_resume);
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index c61535979b8f..804b3ddbf819 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -325,6 +325,8 @@ extern struct attribute_group cros_ec_vbc_attr_group;
 /* debugfs stuff */
 int cros_ec_debugfs_init(struct cros_ec_dev *ec);
 void cros_ec_debugfs_remove(struct cros_ec_dev *ec);
+void cros_ec_debugfs_suspend(struct cros_ec_dev *ec);
+void cros_ec_debugfs_resume(struct cros_ec_dev *ec);
 
 /* ACPI GPE handler */
 #ifdef CONFIG_ACPI
-- 
2.16.1

[PATCH v2 4/8] mfd: cros_ec_dev: Register cros-ec-rtc driver as a subdevice.

[Enric Balletbo i Serra]

Check whether this EC instance has RTC host command support and instatiate
the RTC driver as a subdevice in such case.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
---

Changes in v2:
- [4/8] Add the Reviewed-by Gwendal.

 drivers/mfd/cros_ec_dev.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index f98e5beffca6..33fe1b439ee2 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -389,6 +389,10 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
 	kfree(msg);
 }
 
+static const struct mfd_cell cros_ec_rtc_cell = {
+	.name = "cros-ec-rtc",
+};
+
 static int ec_device_probe(struct platform_device *pdev)
 {
 	int retval = -ENOMEM;
@@ -437,6 +441,16 @@ static int ec_device_probe(struct platform_device *pdev)
 	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
 		cros_ec_sensors_register(ec);
 
+	/* check whether this EC instance has RTC host command support */
+	if (cros_ec_check_features(ec, EC_FEATURE_RTC)) {
+		retval = mfd_add_devices(ec->dev, PLATFORM_DEVID_AUTO,
+					 &cros_ec_rtc_cell, 1, NULL, 0, NULL);
+		if (retval)
+			dev_err(ec->dev,
+				"failed to add cros-ec-rtc device: %d\n",
+				retval);
+	}
+
 	/* Take control of the lightbar from the EC. */
 	lb_manual_suspend_ctrl(ec, 1);
 
-- 
2.16.1

Re: [PATCH v2 2/8] mfd: cros_ec: free IRQ automatically

[Vincent Palatin]

On Fri, Feb 23, 2018 at 4:05 PM, Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> From: Vincent Palatin <vpalatin@chromium.org>
>
> Free the IRQ we might have requested when removing the cros_ec device,
> so we can unload and reload the driver properly.
>
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>
> ---
>
> Changes in v2:
> - [2/8] Remove the free_irq in cros_ec_remove.
>
>  drivers/mfd/cros_ec.c | 20 ++++++--------------
>  1 file changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 74780f2964a1..ea8aa704b61e 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -119,9 +119,9 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>         }
>
>         if (ec_dev->irq) {
> -               err = request_threaded_irq(ec_dev->irq, NULL, ec_irq_thread,
> -                                          IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> -                                          "chromeos-ec", ec_dev);
> +               err = devm_request_threaded_irq(dev, ec_dev->irq, NULL,
> +                               ec_irq_thread, IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +                               "chromeos-ec", ec_dev);
>                 if (err) {
>                         dev_err(dev, "Failed to request IRQ %d: %d",
>                                 ec_dev->irq, err);
> @@ -135,7 +135,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>                 dev_err(dev,
>                         "Failed to register Embedded Controller subdevice %d\n",
>                         err);
> -               goto fail_mfd;
> +               return err;
>         }
>
>         if (ec_dev->max_passthru) {
> @@ -153,7 +153,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>                         dev_err(dev,
>                                 "Failed to register Power Delivery subdevice %d\n",
>                                 err);
> -                       goto fail_mfd;
> +                       return err;
>                 }
>         }
>
> @@ -162,7 +162,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>                 if (err) {
>                         mfd_remove_devices(dev);
>                         dev_err(dev, "Failed to register sub-devices\n");
> -                       goto fail_mfd;
> +                       return err;
>                 }
>         }
>
> @@ -180,11 +180,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>         cros_ec_acpi_install_gpe_handler(dev);
>
>         return 0;
> -
> -fail_mfd:
> -       if (ec_dev->irq)
> -               free_irq(ec_dev->irq, ec_dev);
> -       return err;
>  }
>  EXPORT_SYMBOL(cros_ec_register);
>
> @@ -194,9 +189,6 @@ int cros_ec_remove(struct cros_ec_device *ec_dev)
>
>         cros_ec_acpi_remove_gpe_handler();
>
> -       if (ec_dev->irq)
> -               free_irq(ec_dev->irq, ec_dev);
> -


Acked-by: Vincent Palatin <vpalatin@chromium.org>


>
>         return 0;
>  }
>  EXPORT_SYMBOL(cros_ec_remove);
> --
> 2.16.1
>

Re: [PATCH v2 0/8] mfd: cros_ec: add subdevices and fixes

[Andy Shevchenko]

On Fri, Feb 23, 2018 at 5:05 PM, Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
> Hi,
>
> This is a second version of a patchset that collects some patches
> already send but that needed some rework due the patchset to split the
> cros_ec_devs modules in 2 parts [1]. This patchset contains some of
> imrpovements and also some fixes. They can be applied independently
> and I think that all can go through the MFD tree.

After addressing the minor issues I found, feel free to add

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

>
> Best regards,
>  Enric
>
> Changes in v2:
> - [2/8] Remove the free_irq in cros_ec_remove.
> - [3/8] That patch is new in this series.
> - [4/8] Add the Reviewed-by Gwendal.
> - [5/8] Add the Reviewed-by Gwendal.
> - [6/8] This patch is new in this series.
> - [7/8] Add the Reviewed-by Gwendal.
> - [8/8] This patch is new in this series replacing [5/6] of v1.
>
> Daniel Hung-yu Wu (1):
>   mfd: cros_ec_dev: register shutdown function for debugfs
>
> Douglas Anderson (1):
>   mfd: cros_ec: Don't try to grab log when suspended
>
> Enric Balletbo i Serra (1):
>   mfd: cros_ec_dev: Register cros-ec-rtc driver as a subdevice.
>
> Joseph Lo (1):
>   mfd: cros_ec_i2c: moving the system sleep pm ops to late
>
> Thierry Escande (1):
>   mfd: cros_ec_dev: Register cros_ec_accel_legacy driver as a subdevice.
>
> Vincent Palatin (2):
>   mfd: cros_ec: fail early if we cannot identify the EC
>   mfd: cros_ec: free IRQ automatically
>
> Wei-Ning Huang (1):
>   mfd: cros_ec_i2c: add ACPI module device table
>
>  drivers/mfd/cros_ec.c                     | 26 ++++-----
>  drivers/mfd/cros_ec_dev.c                 | 87 +++++++++++++++++++++++++++++++
>  drivers/mfd/cros_ec_i2c.c                 | 17 +++++-
>  drivers/platform/chrome/cros_ec_debugfs.c | 20 +++++++
>  include/linux/mfd/cros_ec.h               |  2 +
>  5 files changed, 135 insertions(+), 17 deletions(-)
>
> --
> 2.16.1
>



-- 
With Best Regards,
Andy Shevchenko