[PATCH 0/2] platform: chrome: Simplify interrupt path

[PATCH 0/2] platform: chrome: Simplify interrupt path

[Gwendal Grignou]

Irrespective of the transport (i2c, spi, ish, rpmsg), have all cros ec
interrupt stack call the threaded part (bottom half) of the interrupt
handler.
Fix an issue where EC could be stuck if it sends a message while the AP
is not powered on.

Gwendal Grignou (2):
  platform: cros_ec: Call interrupt bottom half in ISH or RPMSG mode
  platform: cros_ec: Call interrupt bottom half at probe time

 drivers/platform/chrome/cros_ec.c           | 33 +++++++++++++++++----
 drivers/platform/chrome/cros_ec_ishtp.c     |  6 +---
 drivers/platform/chrome/cros_ec_rpmsg.c     |  6 +---
 include/linux/platform_data/cros_ec_proto.h |  3 +-
 4 files changed, 31 insertions(+), 17 deletions(-)

-- 
2.29.2.576.ga3fc446d84-goog

[PATCH 1/2] platform: cros_ec: Call interrupt bottom half in ISH or RPMSG mode

[Gwendal Grignou]

Call the same bottom half for all EC protocols (threaded code).

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
 drivers/platform/chrome/cros_ec.c           | 26 ++++++++++++++++-----
 drivers/platform/chrome/cros_ec_ishtp.c     |  6 +----
 drivers/platform/chrome/cros_ec_rpmsg.c     |  6 +----
 include/linux/platform_data/cros_ec_proto.h |  3 ++-
 4 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index 6d6ce86a1408a..4ac33491d0d18 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -31,7 +31,14 @@ static struct cros_ec_platform pd_p = {
 	.cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX),
 };
 
-static irqreturn_t ec_irq_handler(int irq, void *data)
+/**
+ * cros_ec_irq_handler: top half part of the interrupt handler
+ * @irq: IRQ id
+ * @data: (ec_dev) Device with events to process.
+ *
+ * Return: Wakeup the bottom half
+ */
+static irqreturn_t cros_ec_irq_handler(int irq, void *data)
 {
 	struct cros_ec_device *ec_dev = data;
 
@@ -50,7 +57,7 @@ static irqreturn_t ec_irq_handler(int irq, void *data)
  * Return: true if more events are still pending and this function should be
  * called again.
  */
-bool cros_ec_handle_event(struct cros_ec_device *ec_dev)
+static bool cros_ec_handle_event(struct cros_ec_device *ec_dev)
 {
 	bool wake_event;
 	bool ec_has_more_events;
@@ -72,9 +79,15 @@ bool cros_ec_handle_event(struct cros_ec_device *ec_dev)
 
 	return ec_has_more_events;
 }
-EXPORT_SYMBOL(cros_ec_handle_event);
 
-static irqreturn_t ec_irq_thread(int irq, void *data)
+/**
+ * cros_ec_irq_thread: bottom half part of the interrupt handler
+ * @irq: IRQ id
+ * @data: (ec_dev) Device with events to process.
+ *
+ * Return: Interrupt handled.
+ */
+irqreturn_t cros_ec_irq_thread(int irq, void *data)
 {
 	struct cros_ec_device *ec_dev = data;
 	bool ec_has_more_events;
@@ -85,6 +98,7 @@ static irqreturn_t ec_irq_thread(int irq, void *data)
 
 	return IRQ_HANDLED;
 }
+EXPORT_SYMBOL(cros_ec_irq_thread);
 
 static int cros_ec_sleep_event(struct cros_ec_device *ec_dev, u8 sleep_event)
 {
@@ -175,8 +189,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 
 	if (ec_dev->irq > 0) {
 		err = devm_request_threaded_irq(dev, ec_dev->irq,
-						ec_irq_handler,
-						ec_irq_thread,
+						cros_ec_irq_handler,
+						cros_ec_irq_thread,
 						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
 						"chromeos-ec", ec_dev);
 		if (err) {
diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c
index 316cb4bee80d3..62c03be0183a2 100644
--- a/drivers/platform/chrome/cros_ec_ishtp.c
+++ b/drivers/platform/chrome/cros_ec_ishtp.c
@@ -138,12 +138,8 @@ static void ish_evt_handler(struct work_struct *work)
 {
 	struct ishtp_cl_data *client_data =
 		container_of(work, struct ishtp_cl_data, work_ec_evt);
-	struct cros_ec_device *ec_dev = client_data->ec_dev;
-	bool ec_has_more_events;
 
-	do {
-		ec_has_more_events = cros_ec_handle_event(ec_dev);
-	} while (ec_has_more_events);
+	cros_ec_irq_thread(0, client_data->ec_dev);
 }
 
 /**
diff --git a/drivers/platform/chrome/cros_ec_rpmsg.c b/drivers/platform/chrome/cros_ec_rpmsg.c
index 1b38bc8e164c3..12ddc5a4729b7 100644
--- a/drivers/platform/chrome/cros_ec_rpmsg.c
+++ b/drivers/platform/chrome/cros_ec_rpmsg.c
@@ -144,12 +144,8 @@ cros_ec_rpmsg_host_event_function(struct work_struct *host_event_work)
 	struct cros_ec_rpmsg *ec_rpmsg = container_of(host_event_work,
 						      struct cros_ec_rpmsg,
 						      host_event_work);
-	struct cros_ec_device *ec_dev = dev_get_drvdata(&ec_rpmsg->rpdev->dev);
-	bool ec_has_more_events;
 
-	do {
-		ec_has_more_events = cros_ec_handle_event(ec_dev);
-	} while (ec_has_more_events);
+	cros_ec_irq_thread(0, dev_get_drvdata(&ec_rpmsg->rpdev->dev));
 }
 
 static int cros_ec_rpmsg_callback(struct rpmsg_device *rpdev, void *data,
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
index 8776041d5cead..37cfd05d30a0b 100644
--- a/include/linux/platform_data/cros_ec_proto.h
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -9,6 +9,7 @@
 #define __LINUX_CROS_EC_PROTO_H
 
 #include <linux/device.h>
+#include <linux/interrupt.h>
 #include <linux/mutex.h>
 #include <linux/notifier.h>
 #include <linux/power_supply.h>
@@ -241,7 +242,7 @@ int cros_ec_check_features(struct cros_ec_dev *ec, int feature);
 
 int cros_ec_get_sensor_count(struct cros_ec_dev *ec);
 
-bool cros_ec_handle_event(struct cros_ec_device *ec_dev);
+irqreturn_t cros_ec_irq_thread(int irq, void *data);
 
 /**
  * cros_ec_get_time_ns() - Return time in ns.
-- 
2.29.2.576.ga3fc446d84-goog

[PATCH 2/2] platform: cros_ec: Call interrupt bottom half at probe time

[Gwendal Grignou]

While the AP was powered off, the EC may have send messages.
If the message is not serviced within 3s, the EC stops sending message.
Unlock the EC by purging stale messages at probe time.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
 drivers/platform/chrome/cros_ec.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index 4ac33491d0d18..a45d6a6640d50 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -252,6 +252,13 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 
 	dev_info(dev, "Chrome EC device registered\n");
 
+	/*
+	 * Unlock EC that may be waiting for AP to process MKBP events.
+	 * If the AP takes to long to answer, the EC would stop sending events.
+	 */
+	if (ec_dev->mkbp_event_supported)
+		cros_ec_irq_thread(0, ec_dev);
+
 	return 0;
 }
 EXPORT_SYMBOL(cros_ec_register);
-- 
2.29.2.576.ga3fc446d84-goog

Re: [PATCH 1/2] platform: cros_ec: Call interrupt bottom half in ISH or RPMSG mode

[Randy Dunlap]

Hi--

Please use correct kernel-doc notation. See below:


On 12/10/20 2:58 PM, Gwendal Grignou wrote:
> Call the same bottom half for all EC protocols (threaded code).
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---
>  drivers/platform/chrome/cros_ec.c           | 26 ++++++++++++++++-----
>  drivers/platform/chrome/cros_ec_ishtp.c     |  6 +----
>  drivers/platform/chrome/cros_ec_rpmsg.c     |  6 +----
>  include/linux/platform_data/cros_ec_proto.h |  3 ++-
>  4 files changed, 24 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
> index 6d6ce86a1408a..4ac33491d0d18 100644
> --- a/drivers/platform/chrome/cros_ec.c
> +++ b/drivers/platform/chrome/cros_ec.c
> @@ -31,7 +31,14 @@ static struct cros_ec_platform pd_p = {
>  	.cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX),
>  };
>  
> -static irqreturn_t ec_irq_handler(int irq, void *data)
> +/**
> + * cros_ec_irq_handler: top half part of the interrupt handler

 * cros_ec_irq_handler - top half part of the interrupt handler

> + * @irq: IRQ id
> + * @data: (ec_dev) Device with events to process.
> + *
> + * Return: Wakeup the bottom half
> + */
> +static irqreturn_t cros_ec_irq_handler(int irq, void *data)
>  {
>  	struct cros_ec_device *ec_dev = data;
>  

> @@ -72,9 +79,15 @@ bool cros_ec_handle_event(struct cros_ec_device *ec_dev)
>  
>  	return ec_has_more_events;
>  }
> -EXPORT_SYMBOL(cros_ec_handle_event);
>  
> -static irqreturn_t ec_irq_thread(int irq, void *data)
> +/**
> + * cros_ec_irq_thread: bottom half part of the interrupt handler

 * cros_ec_irq_thread - bottom half part of the interrupt handler

> + * @irq: IRQ id
> + * @data: (ec_dev) Device with events to process.
> + *
> + * Return: Interrupt handled.
> + */
> +irqreturn_t cros_ec_irq_thread(int irq, void *data)
>  {
>  	struct cros_ec_device *ec_dev = data;
>  	bool ec_has_more_events;




thanks.
-- 
~Randy