2020-12-11 22:19:36

by Gwendal Grignou

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

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


2020-12-11 22:19:37

by Gwendal Grignou

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

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

Signed-off-by: Gwendal Grignou <[email protected]>
---
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

2020-12-11 23:07:58

by Randy Dunlap

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

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 <[email protected]>
> ---
> 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