* [PATCH v1 0/6] EC-based USB Power Delivery support for Chrome machines
@ 2016-02-05 13:32 Tomeu Vizoso
2016-02-05 13:32 ` [PATCH v1 1/6] mfd: cros_ec: small kerneldoc fix Tomeu Vizoso
` (5 more replies)
0 siblings, 6 replies; 23+ messages in thread
From: Tomeu Vizoso @ 2016-02-05 13:32 UTC (permalink / raw)
To: linux-kernel
Cc: Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Tomeu Vizoso, linux-pm, Lee Jones, Sebastian Reichel,
Dmitry Eremin-Solenikov, Dmitry Torokhov, David Woodhouse,
linux-input, Olof Johansson
Hi,
this series contains a driver that exposes a power_supply to userspace
representing a port that support USB PD charging.
Allows userspace to display to the user if the machine is charging and
on which port, and if another device is being charged by a port. Also
allows to limit the current draw for thermal reasons.
Thanks,
Tomeu
Benson Leung (1):
power_supply: Add types for USB Type C and PD chargers
Sameer Nanda (1):
power: cros_usbpd-charger: Add EC-based USB PD charger driver
Tomeu Vizoso (2):
mfd: cros_ec: small kerneldoc fix
platform/chrome: Register USB PD charger device
Vic Yang (1):
mfd: cros_ec: Add MKBP event support
Vincent Palatin (1):
platform/chrome: Check the USB PD feature before creating a charger
drivers/input/keyboard/cros_ec_keyb.c | 135 ++---
drivers/mfd/cros_ec.c | 72 ++-
drivers/platform/chrome/cros_ec_dev.c | 72 +++
drivers/platform/chrome/cros_ec_proto.c | 139 +++++
drivers/power/Kconfig | 10 +
drivers/power/Makefile | 1 +
drivers/power/cros_usbpd-charger.c | 907 ++++++++++++++++++++++++++++++++
drivers/power/power_supply_sysfs.c | 3 +-
include/linux/mfd/cros_ec.h | 77 ++-
include/linux/mfd/cros_ec_commands.h | 442 +++++++++++++++-
include/linux/power_supply.h | 3 +
11 files changed, 1747 insertions(+), 114 deletions(-)
create mode 100644 drivers/power/cros_usbpd-charger.c
--
2.5.0
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v1 1/6] mfd: cros_ec: small kerneldoc fix
2016-02-05 13:32 [PATCH v1 0/6] EC-based USB Power Delivery support for Chrome machines Tomeu Vizoso
@ 2016-02-05 13:32 ` Tomeu Vizoso
2016-02-05 18:46 ` Benson Leung
2016-02-10 16:25 ` Lee Jones
2016-02-05 13:32 ` [PATCH v1 2/6] mfd: cros_ec: Add MKBP event support Tomeu Vizoso
` (4 subsequent siblings)
5 siblings, 2 replies; 23+ messages in thread
From: Tomeu Vizoso @ 2016-02-05 13:32 UTC (permalink / raw)
To: linux-kernel
Cc: Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Tomeu Vizoso, Lee Jones
s/cros_ec_register/cros_ec_query_all
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
include/linux/mfd/cros_ec.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 494682ce4bf3..a677c2bd485c 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -245,7 +245,7 @@ int cros_ec_remove(struct cros_ec_device *ec_dev);
int cros_ec_register(struct cros_ec_device *ec_dev);
/**
- * cros_ec_register - Query the protocol version supported by the ChromeOS EC
+ * cros_ec_query_all - Query the protocol version supported by the ChromeOS EC
*
* @ec_dev: Device to register
* @return 0 if ok, -ve on error
--
2.5.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v1 2/6] mfd: cros_ec: Add MKBP event support
2016-02-05 13:32 [PATCH v1 0/6] EC-based USB Power Delivery support for Chrome machines Tomeu Vizoso
2016-02-05 13:32 ` [PATCH v1 1/6] mfd: cros_ec: small kerneldoc fix Tomeu Vizoso
@ 2016-02-05 13:32 ` Tomeu Vizoso
2016-02-10 17:41 ` Gwendal Grignou
[not found] ` <CAMHSBOXRvC7U_KWeg+rX3Qymaos0-3FN6AuaXT2cyP=gAN47mA@mail.gmail.com>
2016-02-05 13:32 ` [PATCH v1 3/6] power_supply: Add types for USB Type C and PD chargers Tomeu Vizoso
` (3 subsequent siblings)
5 siblings, 2 replies; 23+ messages in thread
From: Tomeu Vizoso @ 2016-02-05 13:32 UTC (permalink / raw)
To: linux-kernel
Cc: Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Vic Yang, Tomeu Vizoso, Olof Johansson, linux-input,
Dmitry Torokhov, Lee Jones
From: Vic Yang <victoryang@google.com>
Newer revisions of the ChromeOS EC add more events besides the keyboard
ones. So handle interrupts in the MFD driver and let consumers register
for notifications for the events they might care.
To keep backward compatibility, if the EC doesn't support MKBP event, we
fall back to the old MKBP key matrix host command.
Signed-off-by: Vic Yang <victoryang@chromium.org>
[bleung: fixup some context changes going from v3.14 to v3.18]
Signed-off-by: Benson Leung <bleung@chromium.org>
[tomeu: adapted to changes in mainline (in power-supply and platform/chrome)]
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Randall Spangler <rspangler@chromium.org>
Cc: Vincent Palatin <vpalatin@chromium.org>
---
drivers/input/keyboard/cros_ec_keyb.c | 135 ++++++++------------------------
drivers/mfd/cros_ec.c | 57 +++++++++++++-
drivers/platform/chrome/cros_ec_proto.c | 102 ++++++++++++++++++++++++
include/linux/mfd/cros_ec.h | 44 +++++++++++
include/linux/mfd/cros_ec_commands.h | 34 ++++++++
5 files changed, 265 insertions(+), 107 deletions(-)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index b01966dc7eb3..b891503143dc 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -27,6 +27,7 @@
#include <linux/input.h>
#include <linux/interrupt.h>
#include <linux/kernel.h>
+#include <linux/notifier.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/input/matrix_keypad.h>
@@ -44,6 +45,7 @@
* @dev: Device pointer
* @idev: Input device
* @ec: Top level ChromeOS device to use to talk to EC
+ * @notifier: interrupt event notifier for transport devices
*/
struct cros_ec_keyb {
unsigned int rows;
@@ -57,6 +59,7 @@ struct cros_ec_keyb {
struct device *dev;
struct input_dev *idev;
struct cros_ec_device *ec;
+ struct notifier_block notifier;
};
@@ -146,67 +149,44 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
input_sync(ckdev->idev);
}
-static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
-{
- int ret = 0;
- struct cros_ec_command *msg;
-
- msg = kmalloc(sizeof(*msg) + ckdev->cols, GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
-
- msg->version = 0;
- msg->command = EC_CMD_MKBP_STATE;
- msg->insize = ckdev->cols;
- msg->outsize = 0;
-
- ret = cros_ec_cmd_xfer(ckdev->ec, msg);
- if (ret < 0) {
- dev_err(ckdev->dev, "Error transferring EC message %d\n", ret);
- goto exit;
- }
-
- memcpy(kb_state, msg->data, ckdev->cols);
-exit:
- kfree(msg);
- return ret;
-}
-
-static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
+static int cros_ec_keyb_open(struct input_dev *dev)
{
- struct cros_ec_keyb *ckdev = data;
- struct cros_ec_device *ec = ckdev->ec;
- int ret;
- uint8_t kb_state[ckdev->cols];
-
- if (device_may_wakeup(ec->dev))
- pm_wakeup_event(ec->dev, 0);
-
- ret = cros_ec_keyb_get_state(ckdev, kb_state);
- if (ret >= 0)
- cros_ec_keyb_process(ckdev, kb_state, ret);
- else
- dev_err(ec->dev, "failed to get keyboard state: %d\n", ret);
+ struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
- return IRQ_HANDLED;
+ return blocking_notifier_chain_register(&ckdev->ec->event_notifier,
+ &ckdev->notifier);
}
-static int cros_ec_keyb_open(struct input_dev *dev)
+static void cros_ec_keyb_close(struct input_dev *dev)
{
struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
- struct cros_ec_device *ec = ckdev->ec;
- return request_threaded_irq(ec->irq, NULL, cros_ec_keyb_irq,
- IRQF_TRIGGER_LOW | IRQF_ONESHOT,
- "cros_ec_keyb", ckdev);
+ blocking_notifier_chain_unregister(&ckdev->ec->event_notifier,
+ &ckdev->notifier);
}
-static void cros_ec_keyb_close(struct input_dev *dev)
+static int cros_ec_keyb_work(struct notifier_block *nb,
+ unsigned long queued_during_suspend, void *_notify)
{
- struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
- struct cros_ec_device *ec = ckdev->ec;
+ struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
+ notifier);
- free_irq(ec->irq, ckdev);
+ if (ckdev->ec->event_data.event_type != EC_MKBP_EVENT_KEY_MATRIX)
+ return NOTIFY_DONE;
+ /*
+ * If EC is not the wake source, discard key state changes during
+ * suspend.
+ */
+ if (queued_during_suspend)
+ return NOTIFY_OK;
+ if (ckdev->ec->event_size != ckdev->cols) {
+ dev_err(ckdev->dev,
+ "Discarded incomplete key matrix event.\n");
+ return NOTIFY_OK;
+ }
+ cros_ec_keyb_process(ckdev, ckdev->ec->event_data.data.key_matrix,
+ ckdev->ec->event_size);
+ return NOTIFY_OK;
}
/*
@@ -266,12 +246,8 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
if (!idev)
return -ENOMEM;
- if (!ec->irq) {
- dev_err(dev, "no EC IRQ specified\n");
- return -EINVAL;
- }
-
ckdev->ec = ec;
+ ckdev->notifier.notifier_call = cros_ec_keyb_work;
ckdev->dev = dev;
dev_set_drvdata(&pdev->dev, ckdev);
@@ -312,54 +288,6 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_PM_SLEEP
-/* Clear any keys in the buffer */
-static void cros_ec_keyb_clear_keyboard(struct cros_ec_keyb *ckdev)
-{
- uint8_t old_state[ckdev->cols];
- uint8_t new_state[ckdev->cols];
- unsigned long duration;
- int i, ret;
-
- /*
- * Keep reading until we see that the scan state does not change.
- * That indicates that we are done.
- *
- * Assume that the EC keyscan buffer is at most 32 deep.
- */
- duration = jiffies;
- ret = cros_ec_keyb_get_state(ckdev, new_state);
- for (i = 1; !ret && i < 32; i++) {
- memcpy(old_state, new_state, sizeof(old_state));
- ret = cros_ec_keyb_get_state(ckdev, new_state);
- if (0 == memcmp(old_state, new_state, sizeof(old_state)))
- break;
- }
- duration = jiffies - duration;
- dev_info(ckdev->dev, "Discarded %d keyscan(s) in %dus\n", i,
- jiffies_to_usecs(duration));
-}
-
-static int cros_ec_keyb_resume(struct device *dev)
-{
- struct cros_ec_keyb *ckdev = dev_get_drvdata(dev);
-
- /*
- * When the EC is not a wake source, then it could not have caused the
- * resume, so we clear the EC's key scan buffer. If the EC was a
- * wake source (e.g. the lid is open and the user might press a key to
- * wake) then the key scan buffer should be preserved.
- */
- if (!ckdev->ec->was_wake_device)
- cros_ec_keyb_clear_keyboard(ckdev);
-
- return 0;
-}
-
-#endif
-
-static SIMPLE_DEV_PM_OPS(cros_ec_keyb_pm_ops, NULL, cros_ec_keyb_resume);
-
#ifdef CONFIG_OF
static const struct of_device_id cros_ec_keyb_of_match[] = {
{ .compatible = "google,cros-ec-keyb" },
@@ -373,7 +301,6 @@ static struct platform_driver cros_ec_keyb_driver = {
.driver = {
.name = "cros-ec-keyb",
.of_match_table = of_match_ptr(cros_ec_keyb_of_match),
- .pm = &cros_ec_keyb_pm_ops,
},
};
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 0eee63542038..fbe78b819fdd 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -23,6 +23,7 @@
#include <linux/module.h>
#include <linux/mfd/core.h>
#include <linux/mfd/cros_ec.h>
+#include <asm/unaligned.h>
#define CROS_EC_DEV_EC_INDEX 0
#define CROS_EC_DEV_PD_INDEX 1
@@ -49,11 +50,28 @@ static const struct mfd_cell ec_pd_cell = {
.pdata_size = sizeof(pd_p),
};
+static irqreturn_t ec_irq_thread(int irq, void *data)
+{
+ struct cros_ec_device *ec_dev = data;
+ int ret;
+
+ if (device_may_wakeup(ec_dev->dev))
+ pm_wakeup_event(ec_dev->dev, 0);
+
+ ret = cros_ec_get_next_event(ec_dev);
+ if (ret > 0)
+ blocking_notifier_call_chain(&ec_dev->event_notifier,
+ 0, ec_dev);
+ return IRQ_HANDLED;
+}
+
int cros_ec_register(struct cros_ec_device *ec_dev)
{
struct device *dev = ec_dev->dev;
int err = 0;
+ BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
+
ec_dev->max_request = sizeof(struct ec_params_hello);
ec_dev->max_response = sizeof(struct ec_response_get_protocol_info);
ec_dev->max_passthru = 0;
@@ -70,13 +88,24 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
cros_ec_query_all(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);
+ if (err) {
+ dev_err(dev, "request irq %d: error %d\n",
+ ec_dev->irq, err);
+ return err;
+ }
+ }
+
err = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, &ec_cell, 1,
NULL, ec_dev->irq, NULL);
if (err) {
dev_err(dev,
"Failed to register Embedded Controller subdevice %d\n",
err);
- return err;
+ goto fail_mfd;
}
if (ec_dev->max_passthru) {
@@ -94,7 +123,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
dev_err(dev,
"Failed to register Power Delivery subdevice %d\n",
err);
- return err;
+ goto fail_mfd;
}
}
@@ -103,13 +132,18 @@ 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");
- return err;
+ goto fail_mfd;
}
}
dev_info(dev, "Chrome EC device registered\n");
return 0;
+
+fail_mfd:
+ if (ec_dev->irq)
+ free_irq(ec_dev->irq, ec_dev);
+ return err;
}
EXPORT_SYMBOL(cros_ec_register);
@@ -136,13 +170,30 @@ int cros_ec_suspend(struct cros_ec_device *ec_dev)
}
EXPORT_SYMBOL(cros_ec_suspend);
+static void cros_ec_drain_events(struct cros_ec_device *ec_dev)
+{
+ while (cros_ec_get_next_event(ec_dev) > 0)
+ blocking_notifier_call_chain(&ec_dev->event_notifier,
+ 1, ec_dev);
+}
+
int cros_ec_resume(struct cros_ec_device *ec_dev)
{
enable_irq(ec_dev->irq);
+ /*
+ * In some case, we need to distinguish events that occur during
+ * suspend if the EC is not a wake source. For example, keypresses
+ * during suspend should be discarded if it does not wake the system.
+ *
+ * If the EC is not a wake source, drain the event queue and mark them
+ * as "queued during suspend".
+ */
if (ec_dev->wake_enabled) {
disable_irq_wake(ec_dev->irq);
ec_dev->wake_enabled = 0;
+ } else {
+ cros_ec_drain_events(ec_dev);
}
return 0;
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 990308ca384f..6e138e2333f3 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -19,6 +19,7 @@
#include <linux/device.h>
#include <linux/module.h>
#include <linux/slab.h>
+#include <asm/unaligned.h>
#define EC_COMMAND_RETRIES 50
@@ -234,11 +235,44 @@ static int cros_ec_host_command_proto_query_v2(struct cros_ec_device *ec_dev)
return ret;
}
+static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
+ u16 cmd, u32 *mask)
+{
+ struct ec_params_get_cmd_versions *pver;
+ struct ec_response_get_cmd_versions *rver;
+ struct cros_ec_command *msg;
+ int ret;
+
+ msg = kmalloc(sizeof(*msg) + max(sizeof(rver), sizeof(pver)),
+ GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ msg->version = 0;
+ msg->command = EC_CMD_GET_CMD_VERSIONS;
+ msg->insize = sizeof(rver);
+ msg->outsize = sizeof(pver);
+
+ pver = (struct ec_params_get_cmd_versions *)msg->data;
+ pver->cmd = cmd;
+
+ ret = cros_ec_cmd_xfer(ec_dev, msg);
+ if (ret > 0) {
+ rver = (struct ec_response_get_cmd_versions *)msg->data;
+ *mask = rver->version_mask;
+ }
+
+ kfree(msg);
+
+ return ret;
+}
+
int cros_ec_query_all(struct cros_ec_device *ec_dev)
{
struct device *dev = ec_dev->dev;
struct cros_ec_command *proto_msg;
struct ec_response_get_protocol_info *proto_info;
+ u32 ver_mask = 0;
int ret;
proto_msg = kzalloc(sizeof(*proto_msg) + sizeof(*proto_info),
@@ -328,6 +362,15 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
goto exit;
}
+ /* Probe if MKBP event is supported */
+ ret = cros_ec_get_host_command_version_mask(ec_dev,
+ EC_CMD_GET_NEXT_EVENT,
+ &ver_mask);
+ if (ret < 0 || ver_mask == 0)
+ ec_dev->mkbp_event_supported = 0;
+ else
+ ec_dev->mkbp_event_supported = 1;
+
exit:
kfree(proto_msg);
return ret;
@@ -380,3 +423,62 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
return ret;
}
EXPORT_SYMBOL(cros_ec_cmd_xfer);
+
+static int get_next_event(struct cros_ec_device *ec_dev)
+{
+ struct cros_ec_command *msg;
+ int ret;
+
+ msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->event_data), GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ msg->version = 0;
+ msg->command = EC_CMD_GET_NEXT_EVENT;
+ msg->insize = sizeof(ec_dev->event_data);
+ msg->outsize = 0;
+
+ ret = cros_ec_cmd_xfer(ec_dev, msg);
+ if (ret > 0) {
+ ec_dev->event_size = ret - 1;
+ memcpy(&ec_dev->event_data, msg->data,
+ sizeof(ec_dev->event_data));
+ }
+
+ kfree(msg);
+
+ return ret;
+}
+
+static int get_keyboard_state_event(struct cros_ec_device *ec_dev)
+{
+ struct cros_ec_command *msg;
+
+ msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->event_data.data),
+ GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ msg->version = 0;
+ msg->command = EC_CMD_MKBP_STATE;
+ msg->insize = sizeof(ec_dev->event_data.data);
+ msg->outsize = 0;
+
+ ec_dev->event_size = cros_ec_cmd_xfer(ec_dev, msg);
+ ec_dev->event_data.event_type = EC_MKBP_EVENT_KEY_MATRIX;
+ memcpy(&ec_dev->event_data.data, msg->data,
+ sizeof(ec_dev->event_data.data));
+
+ kfree(msg);
+
+ return ec_dev->event_size;
+}
+
+int cros_ec_get_next_event(struct cros_ec_device *ec_dev)
+{
+ if (ec_dev->mkbp_event_supported)
+ return get_next_event(ec_dev);
+ else
+ return get_keyboard_state_event(ec_dev);
+}
+EXPORT_SYMBOL(cros_ec_get_next_event);
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index a677c2bd485c..8a8fa75178c6 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -72,6 +72,24 @@ struct cros_ec_command {
uint8_t data[0];
};
+/*
+ * event_data is used by keyboard or event notifier:
+ * event_data format:
+ * If MKBP protocol is supported:
+ * 0 1
+ * +-----------+--------------------------------
+ * | type | payload
+ * +-----------+--------------------------------
+ * |HOST_EVENT | EVENT (32 bit)
+ * |KEY_MATRIX | Keyboard keys pressed.
+ * |SENSOR_FIFO| Sensors FIFO information.
+ *
+ * Otherwise:
+ * 0 1
+ * +-----------+--------------------------------
+ * |Unused | Keyboard keys pressed.
+ */
+
/**
* struct cros_ec_device - Information about a ChromeOS EC device
*
@@ -107,6 +125,9 @@ struct cros_ec_command {
* should check msg.result for the EC's result code.
* @pkt_xfer: send packet to EC and get response
* @lock: one transaction at a time
+ * @event_notifier: interrupt event notifier for transport devices.
+ * @event_data: raw payload transferred with the MKBP event.
+ * @event_size: size in bytes of the event data.
*/
struct cros_ec_device {
@@ -135,6 +156,11 @@ struct cros_ec_device {
int (*pkt_xfer)(struct cros_ec_device *ec,
struct cros_ec_command *msg);
struct mutex lock;
+ bool mkbp_event_supported;
+ struct blocking_notifier_head event_notifier;
+
+ struct ec_response_get_next_event event_data;
+ int event_size;
};
/* struct cros_ec_platform - ChromeOS EC platform information
@@ -252,9 +278,27 @@ int cros_ec_register(struct cros_ec_device *ec_dev);
*/
int cros_ec_query_all(struct cros_ec_device *ec_dev);
+/**
+ * cros_ec_get_next_event - Fetch next event from the ChromeOS EC
+ *
+ * @ec_dev: Device to fetch event from
+ * @return 0 if ok, -ve on error
+ */
+int cros_ec_get_next_event(struct cros_ec_device *ec_dev);
+
/* sysfs stuff */
extern struct attribute_group cros_ec_attr_group;
extern struct attribute_group cros_ec_lightbar_attr_group;
extern struct attribute_group cros_ec_vbc_attr_group;
+/**
+ * cros_ec_get_host_event - Return a mask of event set by the EC.
+ *
+ * When MKBP is supported, when the EC raises an interrupt,
+ * We collect the events raised and call the functions in the ec notifier.
+ *
+ * This function is a helper to know which events are raised.
+ */
+uint32_t cros_ec_get_host_event(struct cros_ec_device *ec_dev);
+
#endif /* __LINUX_MFD_CROS_EC_H */
diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index 13b630c10d4c..d86526f0bd8e 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -1762,6 +1762,40 @@ struct ec_result_keyscan_seq_ctrl {
};
} __packed;
+/*
+ * Get the next pending MKBP event.
+ *
+ * Returns EC_RES_UNAVAILABLE if there is no event pending.
+ */
+#define EC_CMD_GET_NEXT_EVENT 0x67
+
+enum ec_mkbp_event {
+ /* Keyboard matrix changed. The event data is the new matrix state. */
+ EC_MKBP_EVENT_KEY_MATRIX = 0,
+
+ /* New host event. The event data is 4 bytes of host event flags. */
+ EC_MKBP_EVENT_HOST_EVENT = 1,
+
+ /* New Sensor FIFO data. The event data is fifo_info structure. */
+ EC_MKBP_EVENT_SENSOR_FIFO = 2,
+
+ /* Number of MKBP events */
+ EC_MKBP_EVENT_COUNT,
+};
+
+union ec_response_get_next_data {
+ uint8_t key_matrix[13];
+
+ /* Unaligned */
+ uint32_t host_event;
+} __packed;
+
+struct ec_response_get_next_event {
+ uint8_t event_type;
+ /* Followed by event data if any */
+ union ec_response_get_next_data data;
+} __packed;
+
/*****************************************************************************/
/* Temperature sensor commands */
--
2.5.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v1 3/6] power_supply: Add types for USB Type C and PD chargers
2016-02-05 13:32 [PATCH v1 0/6] EC-based USB Power Delivery support for Chrome machines Tomeu Vizoso
2016-02-05 13:32 ` [PATCH v1 1/6] mfd: cros_ec: small kerneldoc fix Tomeu Vizoso
2016-02-05 13:32 ` [PATCH v1 2/6] mfd: cros_ec: Add MKBP event support Tomeu Vizoso
@ 2016-02-05 13:32 ` Tomeu Vizoso
2016-02-05 18:38 ` Benson Leung
2016-02-05 13:32 ` [PATCH v1 4/6] power: cros_usbpd-charger: Add EC-based USB PD charger driver Tomeu Vizoso
` (2 subsequent siblings)
5 siblings, 1 reply; 23+ messages in thread
From: Tomeu Vizoso @ 2016-02-05 13:32 UTC (permalink / raw)
To: linux-kernel
Cc: Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Tomeu Vizoso, Sebastian Reichel, Dmitry Eremin-Solenikov,
David Woodhouse, linux-pm
From: Benson Leung <bleung@chromium.org>
This adds power supply types for USB chargers defined in
the USB Type-C Specification 1.1 and in the
USB Power Delivery Specification Revision 2.0 V1.1.
The following are added :
POWER_SUPPLY_TYPE_USB_TYPE_C, /* Type C Port */
POWER_SUPPLY_TYPE_USB_PD, /* Type C Power Delivery Port */
POWER_SUPPLY_TYPE_USB_PD_DRP, /* Type C PD Dual Role Port */
Signed-off-by: Benson Leung <bleung@chromium.org>
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Reviewed-by: Alec Berg <alecaberg@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
Reviewed-by: Todd Broch <tbroch@chromium.org>
---
drivers/power/power_supply_sysfs.c | 3 ++-
include/linux/power_supply.h | 3 +++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
index ed2d7fd0c734..80fed98832f9 100644
--- a/drivers/power/power_supply_sysfs.c
+++ b/drivers/power/power_supply_sysfs.c
@@ -45,7 +45,8 @@ static ssize_t power_supply_show_property(struct device *dev,
char *buf) {
static char *type_text[] = {
"Unknown", "Battery", "UPS", "Mains", "USB",
- "USB_DCP", "USB_CDP", "USB_ACA"
+ "USB_DCP", "USB_CDP", "USB_ACA", "USB_C",
+ "USB_PD", "USB_PD_DRP"
};
static char *status_text[] = {
"Unknown", "Charging", "Discharging", "Not charging", "Full"
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index ef9f1592185d..206c3384c6fa 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -163,6 +163,9 @@ enum power_supply_type {
POWER_SUPPLY_TYPE_USB_DCP, /* Dedicated Charging Port */
POWER_SUPPLY_TYPE_USB_CDP, /* Charging Downstream Port */
POWER_SUPPLY_TYPE_USB_ACA, /* Accessory Charger Adapters */
+ POWER_SUPPLY_TYPE_USB_TYPE_C, /* Type C Port */
+ POWER_SUPPLY_TYPE_USB_PD, /* Type C Power Delivery Port */
+ POWER_SUPPLY_TYPE_USB_PD_DRP, /* Type C PD Dual Role Port */
};
enum power_supply_notifier_events {
--
2.5.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v1 4/6] power: cros_usbpd-charger: Add EC-based USB PD charger driver
2016-02-05 13:32 [PATCH v1 0/6] EC-based USB Power Delivery support for Chrome machines Tomeu Vizoso
` (2 preceding siblings ...)
2016-02-05 13:32 ` [PATCH v1 3/6] power_supply: Add types for USB Type C and PD chargers Tomeu Vizoso
@ 2016-02-05 13:32 ` Tomeu Vizoso
2016-02-10 16:49 ` Lee Jones
2016-02-05 13:33 ` [PATCH v1 5/6] platform/chrome: Register USB PD charger device Tomeu Vizoso
2016-02-05 13:33 ` [PATCH v1 6/6] platform/chrome: Check the USB PD feature before creating a charger Tomeu Vizoso
5 siblings, 1 reply; 23+ messages in thread
From: Tomeu Vizoso @ 2016-02-05 13:32 UTC (permalink / raw)
To: linux-kernel
Cc: Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Tomeu Vizoso, Shawn Nematbakhsh, linux-pm, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Lee Jones,
Olof Johansson
From: Sameer Nanda <snanda@chromium.org>
This driver exposes the charger functionality in the PD EC to userspace.
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Sameer Nanda <snanda@chromium.org>
Cc: Benson Leung <bleung@chromium.org>
Cc: Shawn Nematbakhsh <shawnn@chromium.org>
---
drivers/platform/chrome/cros_ec_proto.c | 37 ++
drivers/power/Kconfig | 10 +
drivers/power/Makefile | 1 +
drivers/power/cros_usbpd-charger.c | 907 ++++++++++++++++++++++++++++++++
include/linux/mfd/cros_ec.h | 28 +
include/linux/mfd/cros_ec_commands.h | 324 +++++++++++-
6 files changed, 1302 insertions(+), 5 deletions(-)
create mode 100644 drivers/power/cros_usbpd-charger.c
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 6e138e2333f3..083a32461f2d 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -482,3 +482,40 @@ int cros_ec_get_next_event(struct cros_ec_device *ec_dev)
return get_keyboard_state_event(ec_dev);
}
EXPORT_SYMBOL(cros_ec_get_next_event);
+
+int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg)
+{
+ int ret = cros_ec_cmd_xfer(ec_dev, msg);
+
+ if (ret < 0)
+ dev_err(ec_dev->dev, "Command xfer error (err:%d)\n", ret);
+ else if (msg->result)
+ return -EECRESULT - msg->result;
+
+ return ret;
+}
+EXPORT_SYMBOL(cros_ec_cmd_xfer_status);
+
+u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev)
+{
+ u32 host_event;
+
+ if (!ec_dev->mkbp_event_supported) {
+ dev_warn(ec_dev->dev,
+ "This EC does not support EC_MKBP_EVENT_HOST_EVENT");
+ return -EPROTONOSUPPORT;
+ }
+
+ if (ec_dev->event_data.event_type != EC_MKBP_EVENT_HOST_EVENT)
+ return 0;
+
+ if (ec_dev->event_size != sizeof(host_event)) {
+ dev_warn(ec_dev->dev, "Invalid host event size\n");
+ return 0;
+ }
+
+ host_event = get_unaligned_le32(&ec_dev->event_data.data.host_event);
+ return host_event;
+}
+EXPORT_SYMBOL(cros_ec_get_host_event);
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 1ddd13cc0c07..26355850d91b 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -502,6 +502,16 @@ config AXP20X_POWER
This driver provides support for the power supply features of
AXP20x PMIC.
+config CHARGER_CROS_USB_PD
+ tristate "Chrome OS EC based USB PD charger"
+ depends on MFD_CROS_EC
+ default y
+ help
+ Say Y here to enable Chrome OS EC based USB PD charger driver. This
+ driver gets various bits of information about what is connected to
+ USB PD ports from the EC and converts that into power_supply
+ properties.
+
endif # POWER_SUPPLY
source "drivers/power/reset/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index 0e4eab55f8d7..b83685bfae61 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -73,3 +73,4 @@ obj-$(CONFIG_CHARGER_TPS65217) += tps65217_charger.o
obj-$(CONFIG_POWER_RESET) += reset/
obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
obj-$(CONFIG_AXP288_CHARGER) += axp288_charger.o
+obj-$(CONFIG_CHARGER_CROS_USB_PD) += cros_usbpd-charger.o
diff --git a/drivers/power/cros_usbpd-charger.c b/drivers/power/cros_usbpd-charger.c
new file mode 100644
index 000000000000..72c427554b56
--- /dev/null
+++ b/drivers/power/cros_usbpd-charger.c
@@ -0,0 +1,907 @@
+/*
+ * Power supply driver for ChromeOS EC based USB PD Charger.
+ *
+ * Copyright (c) 2014 Google, Inc
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/fs.h>
+#include <linux/ktime.h>
+#include <linux/module.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/rtc.h>
+#include <linux/slab.h>
+
+#define CROS_USB_PD_MAX_PORTS 8
+#define CROS_USB_PD_MAX_LOG_ENTRIES 30
+
+#define CROS_USB_PD_LOG_UPDATE_DELAY msecs_to_jiffies(60000)
+#define CROS_USB_PD_CACHE_UPDATE_DELAY msecs_to_jiffies(500)
+
+/* Buffer + macro for building PDLOG string */
+#define BUF_SIZE 80
+#define APPEND_STRING(buf, len, str, ...) ((len) += \
+ snprintf((buf) + (len), max(BUF_SIZE - (len), 0), (str), ##__VA_ARGS__))
+
+#define CHARGER_DIR_NAME "CROS_USB_PD_CHARGER%d"
+#define CHARGER_DIR_NAME_LENGTH sizeof(CHARGER_DIR_NAME)
+
+#define MANUFACTURER_MODEL_LENGTH 32
+
+struct port_data {
+ int port_number;
+ char name[CHARGER_DIR_NAME_LENGTH];
+ char manufacturer[MANUFACTURER_MODEL_LENGTH];
+ char model_name[MANUFACTURER_MODEL_LENGTH];
+ struct power_supply *psy;
+ struct power_supply_desc psy_desc;
+ int psy_type;
+ int psy_online;
+ int psy_status;
+ int psy_current_max;
+ int psy_voltage_max_design;
+ int psy_voltage_now;
+ int psy_power_max;
+ struct charger_data *charger;
+ unsigned long last_update;
+};
+
+struct charger_data {
+ struct device *dev;
+ struct cros_ec_dev *ec_dev;
+ struct cros_ec_device *ec_device;
+ int num_charger_ports;
+ int num_registered_psy;
+ struct port_data *ports[CROS_USB_PD_MAX_PORTS];
+ struct delayed_work log_work;
+ struct workqueue_struct *log_workqueue;
+ struct notifier_block notifier;
+ bool suspended;
+};
+
+#define EC_MAX_IN_SIZE EC_PROTO2_MAX_REQUEST_SIZE
+#define EC_MAX_OUT_SIZE EC_PROTO2_MAX_RESPONSE_SIZE
+uint8_t ec_inbuf[EC_MAX_IN_SIZE];
+uint8_t ec_outbuf[EC_MAX_OUT_SIZE];
+
+static enum power_supply_property cros_usb_pd_charger_props[] = {
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_CURRENT_MAX,
+ POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX,
+ POWER_SUPPLY_PROP_MODEL_NAME,
+ POWER_SUPPLY_PROP_MANUFACTURER,
+};
+
+/* Current external voltage/current limit in mV/mA. Default to none. */
+static uint16_t ext_voltage_lim = EC_POWER_LIMIT_NONE;
+static uint16_t ext_current_lim = EC_POWER_LIMIT_NONE;
+
+static int ec_command(struct cros_ec_dev *ec_dev, int version, int command,
+ uint8_t *outdata, int outsize, uint8_t *indata,
+ int insize)
+{
+ struct cros_ec_device *ec_device = ec_dev->ec_dev;
+ struct cros_ec_command *msg;
+ int ret;
+
+ msg = kmalloc(sizeof(*msg) + max(outsize, insize), GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ msg->version = version;
+ msg->command = ec_dev->cmd_offset + command;
+ msg->outsize = outsize;
+ msg->insize = insize;
+
+ memcpy(msg->data, outdata, outsize);
+ ret = cros_ec_cmd_xfer_status(ec_device, msg);
+ memcpy(indata, msg->data, insize);
+
+ kfree(msg);
+
+ return ret;
+}
+
+static int set_ec_usb_pd_override_ports(struct charger_data *charger,
+ int port_num)
+{
+ struct device *dev = charger->dev;
+ struct ec_params_charge_port_override req;
+ int ret;
+
+ req.override_port = port_num;
+
+ ret = ec_command(charger->ec_dev, 0, EC_CMD_PD_CHARGE_PORT_OVERRIDE,
+ (uint8_t *)&req, sizeof(req),
+ NULL, 0);
+ if (ret < 0) {
+ dev_info(dev, "Port Override command returned 0x%x\n", ret);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int get_ec_num_ports(struct charger_data *charger, int *num_ports)
+{
+ struct device *dev = charger->dev;
+ struct ec_response_usb_pd_ports *resp =
+ (struct ec_response_usb_pd_ports *)ec_inbuf;
+ int ret;
+
+ *num_ports = 0;
+ ret = ec_command(charger->ec_dev, 0, EC_CMD_USB_PD_PORTS,
+ NULL, 0, ec_inbuf, EC_MAX_IN_SIZE);
+ if (ret < 0) {
+ dev_err(dev, "Unable to query PD ports (err:0x%x)\n", ret);
+ return ret;
+ }
+ *num_ports = resp->num_ports;
+ dev_dbg(dev, "Num ports = %d\n", *num_ports);
+
+ return 0;
+}
+
+static int get_ec_usb_pd_discovery_info(struct port_data *port)
+{
+ struct charger_data *charger = port->charger;
+ struct device *dev = charger->dev;
+ struct ec_params_usb_pd_info_request req;
+ struct ec_params_usb_pd_discovery_entry resp;
+ int ret;
+
+ req.port = port->port_number;
+
+ ret = ec_command(charger->ec_dev, 0, EC_CMD_USB_PD_DISCOVERY,
+ (uint8_t *)&req, sizeof(req),
+ (uint8_t *)&resp, sizeof(resp));
+ if (ret < 0) {
+ dev_err(dev, "Unable to query Discovery info (err:0x%x)\n",
+ ret);
+ return -EINVAL;
+ }
+
+ dev_dbg(dev, "Port %d: VID = 0x%x, PID=0x%x, PTYPE=0x%x\n",
+ port->port_number, resp.vid, resp.pid, resp.ptype);
+
+ snprintf(port->manufacturer, MANUFACTURER_MODEL_LENGTH, "%x", resp.vid);
+ snprintf(port->model_name, MANUFACTURER_MODEL_LENGTH, "%x", resp.pid);
+
+ return 0;
+}
+
+static int get_ec_usb_pd_power_info(struct port_data *port)
+{
+ struct charger_data *charger = port->charger;
+ struct device *dev = charger->dev;
+ struct ec_params_usb_pd_power_info req;
+ struct ec_response_usb_pd_power_info resp;
+ char role_str[80];
+ int ret, last_psy_status, last_psy_type;
+
+ req.port = port->port_number;
+ ret = ec_command(charger->ec_dev, 0, EC_CMD_USB_PD_POWER_INFO,
+ (uint8_t *)&req, sizeof(req),
+ (uint8_t *)&resp, sizeof(resp));
+ if (ret < 0) {
+ dev_err(dev, "Unable to query PD power info (err:0x%x)\n", ret);
+ return -EINVAL;
+ }
+
+ last_psy_status = port->psy_status;
+ last_psy_type = port->psy_type;
+
+ switch (resp.role) {
+ case USB_PD_PORT_POWER_DISCONNECTED:
+ snprintf(role_str, sizeof(role_str), "%s", "DISCONNECTED");
+ port->psy_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ port->psy_online = 0;
+ break;
+ case USB_PD_PORT_POWER_SOURCE:
+ snprintf(role_str, sizeof(role_str), "%s", "SOURCE");
+ port->psy_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ port->psy_online = 0;
+ break;
+ case USB_PD_PORT_POWER_SINK:
+ snprintf(role_str, sizeof(role_str), "%s", "SINK");
+ port->psy_status = POWER_SUPPLY_STATUS_CHARGING;
+ port->psy_online = 1;
+ break;
+ case USB_PD_PORT_POWER_SINK_NOT_CHARGING:
+ snprintf(role_str, sizeof(role_str), "%s", "NOT CHARGING");
+ port->psy_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ port->psy_online = 1;
+ break;
+ default:
+ snprintf(role_str, sizeof(role_str), "%s", "UNKNOWN");
+ dev_err(dev, "Unknown role %d\n", resp.role);
+ break;
+ }
+
+ port->psy_voltage_max_design = resp.meas.voltage_max;
+ port->psy_voltage_now = resp.meas.voltage_now;
+ port->psy_current_max = resp.meas.current_max;
+ port->psy_power_max = resp.max_power;
+
+ switch (resp.type) {
+ case USB_CHG_TYPE_BC12_SDP:
+ case USB_CHG_TYPE_VBUS:
+ port->psy_type = POWER_SUPPLY_TYPE_USB;
+ break;
+ case USB_CHG_TYPE_NONE:
+ /*
+ * For dual-role devices when we are a source, the firmware
+ * reports the type as NONE. Report such chargers as type
+ * USB_PD_DRP.
+ */
+ if (resp.role == USB_PD_PORT_POWER_SOURCE && resp.dualrole)
+ port->psy_type = POWER_SUPPLY_TYPE_USB_PD_DRP;
+ else
+ port->psy_type = POWER_SUPPLY_TYPE_USB;
+ break;
+ case USB_CHG_TYPE_PROPRIETARY:
+ port->psy_type = POWER_SUPPLY_TYPE_MAINS;
+ break;
+ case USB_CHG_TYPE_C:
+ port->psy_type = POWER_SUPPLY_TYPE_USB_TYPE_C;
+ break;
+ case USB_CHG_TYPE_BC12_DCP:
+ port->psy_type = POWER_SUPPLY_TYPE_USB_DCP;
+ break;
+ case USB_CHG_TYPE_BC12_CDP:
+ port->psy_type = POWER_SUPPLY_TYPE_USB_CDP;
+ break;
+ case USB_CHG_TYPE_PD:
+ if (resp.dualrole)
+ port->psy_type = POWER_SUPPLY_TYPE_USB_PD_DRP;
+ else
+ port->psy_type = POWER_SUPPLY_TYPE_USB_PD;
+ break;
+ case USB_CHG_TYPE_UNKNOWN:
+ /*
+ * While the EC is trying to determine the type of charger that
+ * has been plugged in, it will report the charger type as
+ * unknown. Treat this case as a dedicated charger since we
+ * don't know any better just yet. Additionally since the power
+ * capabilities are unknown, report the max current and voltage
+ * as zero.
+ */
+ port->psy_type = POWER_SUPPLY_TYPE_MAINS;
+ port->psy_voltage_max_design = 0;
+ port->psy_current_max = 0;
+ break;
+ default:
+ dev_err(dev, "Port %d: default case!\n",
+ port->port_number);
+ port->psy_type = POWER_SUPPLY_TYPE_USB;
+ }
+
+ port->psy_desc.type = port->psy_type;
+
+ dev_dbg(dev,
+ "Port %d: %s type=%d=vmax=%d vnow=%d cmax=%d clim=%d pmax=%d\n",
+ port->port_number, role_str, resp.type,
+ resp.meas.voltage_max, resp.meas.voltage_now,
+ resp.meas.current_max, resp.meas.current_lim,
+ resp.max_power);
+
+ /*
+ * If power supply type or status changed, explicitly call
+ * power_supply_changed. This results in udev event getting generated
+ * and allows user mode apps to react quicker instead of waiting for
+ * their next poll of power supply status.
+ */
+ if (last_psy_type != port->psy_type ||
+ last_psy_status != port->psy_status) {
+ dev_dbg(dev, "Port %d: Calling power_supply_changed\n",
+ port->port_number);
+ power_supply_changed(port->psy);
+ }
+
+ return 0;
+}
+
+static int get_ec_port_status(struct port_data *port, bool ratelimit)
+{
+ int ret;
+
+ if (ratelimit &&
+ time_is_after_jiffies(port->last_update +
+ CROS_USB_PD_CACHE_UPDATE_DELAY))
+ return 0;
+
+ ret = get_ec_usb_pd_power_info(port);
+ if (ret < 0)
+ return ret;
+
+ ret = get_ec_usb_pd_discovery_info(port);
+ port->last_update = jiffies;
+
+ return ret;
+}
+
+static void cros_usb_pd_charger_power_changed(struct power_supply *psy)
+{
+ struct port_data *port = power_supply_get_drvdata(psy);
+ struct charger_data *charger = port->charger;
+ struct device *dev = charger->dev;
+ int i;
+
+ dev_dbg(dev, "cros_usb_pd_charger_power_changed\n");
+ for (i = 0; i < charger->num_registered_psy; i++)
+ get_ec_port_status(charger->ports[i], false);
+}
+
+static int cros_usb_pd_charger_get_prop(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct port_data *port = power_supply_get_drvdata(psy);
+ struct charger_data *charger = port->charger;
+ struct device *dev = charger->dev;
+ int ret;
+
+
+ /* Only refresh ec_port_status for dynamic properties */
+ switch (psp) {
+ case POWER_SUPPLY_PROP_CURRENT_MAX:
+ case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ case POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX:
+ ret = get_ec_port_status(port, true);
+ if (ret < 0) {
+ dev_err(dev, "Failed to get port status (err:0x%x)\n",
+ ret);
+ return -EINVAL;
+ }
+ break;
+ default:
+ break;
+ }
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_ONLINE:
+ val->intval = port->psy_online;
+ break;
+ case POWER_SUPPLY_PROP_STATUS:
+ val->intval = port->psy_status;
+ break;
+ case POWER_SUPPLY_PROP_CURRENT_MAX:
+ val->intval = port->psy_current_max * 1000;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+ val->intval = port->psy_voltage_max_design * 1000;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ val->intval = port->psy_voltage_now * 1000;
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX:
+ /* TODO: send a TBD host command to the EC. */
+ val->intval = 0;
+ break;
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ val->strval = port->model_name;
+ break;
+ case POWER_SUPPLY_PROP_MANUFACTURER:
+ val->strval = port->manufacturer;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int cros_usb_pd_charger_set_prop(struct power_supply *psy,
+ enum power_supply_property psp,
+ const union power_supply_propval *val)
+{
+ struct port_data *port = power_supply_get_drvdata(psy);
+ struct charger_data *charger = port->charger;
+ int port_number;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX:
+ /*
+ * A value of -1 implies switching to battery as the power
+ * source. Any other value implies using this port as the
+ * power source.
+ */
+ port_number = val->intval;
+ if (port_number != -1)
+ port_number = port->port_number;
+ return set_ec_usb_pd_override_ports(charger, port_number);
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int cros_usb_pd_charger_is_writeable(struct power_supply *psy,
+ enum power_supply_property psp)
+{
+ int ret;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX:
+ ret = 1;
+ break;
+ default:
+ ret = 0;
+ }
+
+ return ret;
+}
+
+static void cros_usb_pd_print_log_entry(struct ec_response_pd_log *r,
+ ktime_t tstamp)
+{
+ static const char * const fault_names[] = {
+ "---", "OCP", "fast OCP", "OVP", "Discharge"
+ };
+ static const char * const role_names[] = {
+ "Disconnected", "SRC", "SNK", "SNK (not charging)"
+ };
+ static const char * const chg_type_names[] = {
+ "None", "PD", "Type-C", "Proprietary",
+ "DCP", "CDP", "SDP", "Other", "VBUS"
+ };
+ int i;
+ int role_idx, type_idx;
+ const char *fault, *role, *chg_type;
+ struct usb_chg_measures *meas;
+ struct mcdp_info *minfo;
+ struct rtc_time rt;
+ int len = 0;
+ char buf[BUF_SIZE + 1];
+
+ /* the timestamp is the number of 1024th of seconds in the past */
+ tstamp = ktime_sub_us(tstamp,
+ (uint64_t)r->timestamp << PD_LOG_TIMESTAMP_SHIFT);
+ rt = rtc_ktime_to_tm(tstamp);
+
+ switch (r->type) {
+ case PD_EVENT_MCU_CHARGE:
+ if (r->data & CHARGE_FLAGS_OVERRIDE)
+ APPEND_STRING(buf, len, "override ");
+ if (r->data & CHARGE_FLAGS_DELAYED_OVERRIDE)
+ APPEND_STRING(buf, len, "pending_override ");
+ role_idx = r->data & CHARGE_FLAGS_ROLE_MASK;
+ role = role_idx < ARRAY_SIZE(role_names) ?
+ role_names[role_idx] : "Unknown";
+ type_idx = (r->data & CHARGE_FLAGS_TYPE_MASK)
+ >> CHARGE_FLAGS_TYPE_SHIFT;
+ chg_type = type_idx < ARRAY_SIZE(chg_type_names) ?
+ chg_type_names[type_idx] : "???";
+
+ if ((role_idx == USB_PD_PORT_POWER_DISCONNECTED) ||
+ (role_idx == USB_PD_PORT_POWER_SOURCE)) {
+ APPEND_STRING(buf, len, "%s", role);
+ break;
+ }
+
+ meas = (struct usb_chg_measures *)r->payload;
+ APPEND_STRING(buf, len, "%s %s %s %dmV max %dmV / %dmA", role,
+ r->data & CHARGE_FLAGS_DUAL_ROLE ? "DRP" : "Charger",
+ chg_type,
+ meas->voltage_now,
+ meas->voltage_max,
+ meas->current_max);
+ break;
+ case PD_EVENT_ACC_RW_FAIL:
+ APPEND_STRING(buf, len, "RW signature check failed");
+ break;
+ case PD_EVENT_PS_FAULT:
+ fault = r->data < ARRAY_SIZE(fault_names) ? fault_names[r->data]
+ : "???";
+ APPEND_STRING(buf, len, "Power supply fault: %s", fault);
+ break;
+ case PD_EVENT_VIDEO_DP_MODE:
+ APPEND_STRING(buf, len, "DP mode %sabled",
+ (r->data == 1) ? "en" : "dis");
+ break;
+ case PD_EVENT_VIDEO_CODEC:
+ minfo = (struct mcdp_info *)r->payload;
+ APPEND_STRING(buf, len,
+ "HDMI info: family:%04x chipid:%04x irom:%d.%d.%d fw:%d.%d.%d",
+ MCDP_FAMILY(minfo->family),
+ MCDP_CHIPID(minfo->chipid),
+ minfo->irom.major, minfo->irom.minor,
+ minfo->irom.build, minfo->fw.major,
+ minfo->fw.minor, minfo->fw.build);
+ break;
+ default:
+ APPEND_STRING(buf, len,
+ "Event %02x (%04x) [", r->type, r->data);
+ for (i = 0; i < PD_LOG_SIZE(r->size_port); i++)
+ APPEND_STRING(buf, len, "%02x ", r->payload[i]);
+ APPEND_STRING(buf, len, "]");
+ break;
+ }
+
+ pr_info("PDLOG %d/%02d/%02d %02d:%02d:%02d.%03d P%d %s\n",
+ rt.tm_year + 1900, rt.tm_mon + 1, rt.tm_mday,
+ rt.tm_hour, rt.tm_min, rt.tm_sec,
+ (int)(ktime_to_ms(tstamp) % MSEC_PER_SEC),
+ PD_LOG_PORT(r->size_port), buf);
+}
+
+static void cros_usb_pd_log_check(struct work_struct *work)
+{
+ struct charger_data *charger = container_of(to_delayed_work(work),
+ struct charger_data, log_work);
+ struct device *dev = charger->dev;
+ union {
+ struct ec_response_pd_log r;
+ uint32_t words[8]; /* space for the payload */
+ } u;
+ int ret;
+ int entries = 0;
+ ktime_t now;
+
+ if (charger->suspended) {
+ dev_dbg(dev, "Suspended...bailing out\n");
+ return;
+ }
+
+ while (entries++ < CROS_USB_PD_MAX_LOG_ENTRIES) {
+ ret = ec_command(charger->ec_dev, 0, EC_CMD_PD_GET_LOG_ENTRY,
+ NULL, 0, (uint8_t *)&u, sizeof(u));
+ now = ktime_get_real();
+ if (ret < 0) {
+ dev_dbg(dev, "Cannot get PD log %d\n", ret);
+ break;
+ }
+ if (u.r.type == PD_EVENT_NO_ENTRY)
+ break;
+
+ cros_usb_pd_print_log_entry(&u.r, now);
+ }
+
+ queue_delayed_work(charger->log_workqueue, &charger->log_work,
+ CROS_USB_PD_LOG_UPDATE_DELAY);
+}
+
+static int cros_usb_pd_ec_event(struct notifier_block *nb,
+ unsigned long queued_during_suspend, void *_notify)
+{
+ struct charger_data *charger;
+ struct device *dev;
+ struct cros_ec_device *ec_device;
+ u32 host_event;
+
+ charger = container_of(nb, struct charger_data, notifier);
+ dev = charger->dev;
+ ec_device = charger->ec_device;
+
+ host_event = cros_ec_get_host_event(ec_device);
+ if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_PD_MCU)) {
+ cros_usb_pd_charger_power_changed(charger->ports[0]->psy);
+ return NOTIFY_OK;
+ } else {
+ return NOTIFY_DONE;
+ }
+}
+
+static char *charger_supplied_to[] = {"cros-usb_pd-charger"};
+
+static int cros_usb_pd_charger_probe(struct platform_device *pd)
+{
+ struct device *dev = &pd->dev;
+ struct cros_ec_dev *ec_dev = dev_get_drvdata(pd->dev.parent);
+ struct cros_ec_device *ec_device;
+ struct charger_data *charger;
+ struct port_data *port;
+ struct power_supply_desc *psy_desc;
+ struct power_supply_config psy_cfg = {};
+ int i;
+ int ret = -EINVAL;
+
+ dev_dbg(dev, "cros_usb_pd_charger_probe\n");
+ if (!ec_dev) {
+ WARN(1, "%s: No EC dev found\n", dev_name(dev));
+ return -EINVAL;
+ }
+
+ ec_device = ec_dev->ec_dev;
+ if (!ec_device) {
+ WARN(1, "%s: No EC device found\n", dev_name(dev));
+ return -EINVAL;
+ }
+
+ charger = devm_kzalloc(dev, sizeof(struct charger_data),
+ GFP_KERNEL);
+ if (!charger)
+ return -ENOMEM;
+
+ charger->dev = dev;
+ charger->ec_dev = ec_dev;
+ charger->ec_device = ec_device;
+
+ platform_set_drvdata(pd, charger);
+
+ if (!charger->num_charger_ports) {
+ /*
+ * This can happen on a system that doesn't support USB PD.
+ * Log a message, but no need to warn.
+ */
+ dev_info(dev, "No charging ports found\n");
+ ret = -ENODEV;
+ goto fail_nowarn;
+ }
+
+ for (i = 0; i < charger->num_charger_ports; i++) {
+ port = devm_kzalloc(dev, sizeof(struct port_data), GFP_KERNEL);
+ if (!port) {
+ dev_err(dev, "Failed to alloc port structure\n");
+ ret = -ENOMEM;
+ goto fail;
+ }
+
+ port->charger = charger;
+ port->port_number = i;
+ sprintf(port->name, CHARGER_DIR_NAME, i);
+
+ psy_desc = &port->psy_desc;
+ psy_desc->name = port->name;
+ psy_desc->type = POWER_SUPPLY_TYPE_USB;
+ psy_desc->get_property = cros_usb_pd_charger_get_prop;
+ psy_desc->set_property = cros_usb_pd_charger_set_prop;
+ psy_desc->property_is_writeable =
+ cros_usb_pd_charger_is_writeable;
+ psy_desc->external_power_changed =
+ cros_usb_pd_charger_power_changed;
+ psy_desc->properties = cros_usb_pd_charger_props;
+ psy_desc->num_properties = ARRAY_SIZE(
+ cros_usb_pd_charger_props);
+
+ psy_cfg.drv_data = port;
+ psy_cfg.supplied_to = charger_supplied_to;
+ psy_cfg.num_supplicants = ARRAY_SIZE(charger_supplied_to);
+
+ port->psy = power_supply_register_no_ws(dev, psy_desc,
+ &psy_cfg);
+ if (IS_ERR(port->psy)) {
+ dev_err(dev, "Failed to register power supply: %ld\n",
+ PTR_ERR(port->psy));
+ continue;
+ }
+
+ charger->ports[charger->num_registered_psy++] = port;
+ }
+
+ if (!charger->num_registered_psy) {
+ ret = -ENODEV;
+ dev_err(dev, "No power supplies registered\n");
+ goto fail;
+ }
+
+ if (ec_device->mkbp_event_supported) {
+ /* Get PD events from the EC */
+ charger->notifier.notifier_call = cros_usb_pd_ec_event;
+ ret = blocking_notifier_chain_register(
+ &ec_device->event_notifier, &charger->notifier);
+ if (ret < 0)
+ dev_warn(dev, "failed to register notifier\n");
+ }
+
+ /* Retrieve PD event logs periodically */
+ INIT_DELAYED_WORK(&charger->log_work, cros_usb_pd_log_check);
+ charger->log_workqueue =
+ create_singlethread_workqueue("cros_usb_pd_log");
+ queue_delayed_work(charger->log_workqueue, &charger->log_work,
+ CROS_USB_PD_LOG_UPDATE_DELAY);
+
+ return 0;
+
+fail:
+ WARN(1, "%s: Failing probe (err:0x%x)\n", dev_name(dev), ret);
+
+fail_nowarn:
+ if (charger) {
+ for (i = 0; i < charger->num_registered_psy; i++) {
+ port = charger->ports[i];
+ power_supply_unregister(port->psy);
+ devm_kfree(dev, port);
+ }
+ platform_set_drvdata(pd, NULL);
+ devm_kfree(dev, charger);
+ }
+
+ dev_info(dev, "Failing probe (err:0x%x)\n", ret);
+ return ret;
+}
+
+static int cros_usb_pd_charger_remove(struct platform_device *pd)
+{
+ struct charger_data *charger = platform_get_drvdata(pd);
+ struct device *dev = charger->dev;
+ struct port_data *port;
+ int i;
+
+ if (charger) {
+ for (i = 0; i < charger->num_registered_psy; i++) {
+ port = charger->ports[i];
+ power_supply_unregister(port->psy);
+ devm_kfree(dev, port);
+ }
+ flush_delayed_work(&charger->log_work);
+ platform_set_drvdata(pd, NULL);
+ devm_kfree(dev, charger);
+ }
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int cros_usb_pd_charger_resume(struct device *dev)
+{
+ struct charger_data *charger = dev_get_drvdata(dev);
+ int i;
+
+ if (!charger)
+ return 0;
+
+ charger->suspended = false;
+
+ dev_dbg(dev, "cros_usb_pd_charger_resume: updating power supplies\n");
+ for (i = 0; i < charger->num_registered_psy; i++) {
+ power_supply_changed(charger->ports[i]->psy);
+ charger->ports[i]->last_update =
+ jiffies - CROS_USB_PD_CACHE_UPDATE_DELAY;
+ }
+ queue_delayed_work(charger->log_workqueue, &charger->log_work,
+ CROS_USB_PD_LOG_UPDATE_DELAY);
+
+ return 0;
+}
+
+static int cros_usb_pd_charger_suspend(struct device *dev)
+{
+ struct charger_data *charger = dev_get_drvdata(dev);
+
+ charger->suspended = true;
+
+ if (charger)
+ flush_delayed_work(&charger->log_work);
+ return 0;
+}
+#endif
+
+static int set_ec_ext_power_limit(struct cros_ec_dev *ec,
+ uint16_t current_lim, uint16_t voltage_lim)
+{
+ struct ec_params_external_power_limit_v1 req;
+ int ret;
+
+ req.current_lim = current_lim;
+ req.voltage_lim = voltage_lim;
+
+ ret = ec_command(ec, 0, EC_CMD_EXT_POWER_CURRENT_LIMIT,
+ (uint8_t *)&req, sizeof(req), NULL, 0);
+ if (ret < 0) {
+ dev_warn(ec->dev,
+ "External power limit command returned 0x%x\n",
+ ret);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static ssize_t get_ec_ext_current_lim(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return scnprintf(buf, PAGE_SIZE, "%d\n", ext_current_lim);
+}
+
+static ssize_t set_ec_ext_current_lim(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+ uint16_t tmp_val;
+ struct cros_ec_dev *ec = container_of(
+ dev, struct cros_ec_dev, class_dev);
+
+ if (kstrtou16(buf, 0, &tmp_val) < 0)
+ return -EINVAL;
+
+ ret = set_ec_ext_power_limit(ec, tmp_val, ext_voltage_lim);
+ if (ret < 0)
+ return ret;
+
+ ext_current_lim = tmp_val;
+ if (ext_current_lim == EC_POWER_LIMIT_NONE)
+ dev_info(ec->dev, "External current limit removed\n");
+ else
+ dev_info(ec->dev, "External current limit set to %dmA\n",
+ ext_current_lim);
+
+ return count;
+}
+
+static ssize_t get_ec_ext_voltage_lim(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return scnprintf(buf, PAGE_SIZE, "%d\n", ext_voltage_lim);
+}
+
+static ssize_t set_ec_ext_voltage_lim(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+ uint16_t tmp_val;
+
+ struct cros_ec_dev *ec = container_of(
+ dev, struct cros_ec_dev, class_dev);
+
+ if (kstrtou16(buf, 0, &tmp_val) < 0)
+ return -EINVAL;
+
+ ret = set_ec_ext_power_limit(ec, ext_current_lim, tmp_val);
+ if (ret < 0)
+ return ret;
+
+ ext_voltage_lim = tmp_val;
+ if (ext_voltage_lim == EC_POWER_LIMIT_NONE)
+ dev_info(ec->dev, "External voltage limit removed\n");
+ else
+ dev_info(ec->dev, "External voltage limit set to %dmV\n",
+ ext_voltage_lim);
+
+ return count;
+}
+
+static DEVICE_ATTR(ext_current_lim, S_IWUSR | S_IWGRP | S_IRUGO,
+ get_ec_ext_current_lim,
+ set_ec_ext_current_lim);
+static DEVICE_ATTR(ext_voltage_lim, S_IWUSR | S_IWGRP | S_IRUGO,
+ get_ec_ext_voltage_lim,
+ set_ec_ext_voltage_lim);
+
+static struct attribute *__ext_power_cmds_attrs[] = {
+ &dev_attr_ext_current_lim.attr,
+ &dev_attr_ext_voltage_lim.attr,
+ NULL,
+};
+
+struct attribute_group cros_usb_pd_charger_attr_group = {
+ .name = "usb-pd-charger",
+ .attrs = __ext_power_cmds_attrs,
+};
+
+static SIMPLE_DEV_PM_OPS(cros_usb_pd_charger_pm_ops,
+ cros_usb_pd_charger_suspend, cros_usb_pd_charger_resume);
+
+static struct platform_driver cros_usb_pd_charger_driver = {
+ .driver = {
+ .name = "cros-usb-pd-charger",
+ .owner = THIS_MODULE,
+ .pm = &cros_usb_pd_charger_pm_ops,
+ },
+ .probe = cros_usb_pd_charger_probe,
+ .remove = cros_usb_pd_charger_remove,
+};
+
+module_platform_driver(cros_usb_pd_charger_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Chrome USB PD charger");
+MODULE_ALIAS("power_supply:cros-usb-pd-charger");
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 8a8fa75178c6..c21583411ba0 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -40,6 +40,9 @@
#define EC_MAX_REQUEST_OVERHEAD 1
#define EC_MAX_RESPONSE_OVERHEAD 2
+/* ec_command return value for non-success result from EC */
+#define EECRESULT 1000
+
/*
* Command interface between EC and AP, for LPC, I2C and SPI interfaces.
*/
@@ -250,6 +253,21 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg);
/**
+ * cros_ec_cmd_xfer_status - Send a command to the ChromeOS EC
+ *
+ * This function is identical to cros_ec_cmd_xfer, except it returns succes
+ * status only if both the command was transmitted successfully and the EC
+ * replied with success status. It's not necessary to check msg->result when
+ * using this function.
+ *
+ * @ec_dev: EC device
+ * @msg: Message to write
+ * @return: Num. of bytes transferred on success, <0 on failure
+ */
+int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg);
+
+/**
* cros_ec_remove - Remove a ChromeOS EC
*
* Call this to deregister a ChromeOS EC, then clean up any private data.
@@ -286,6 +304,16 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev);
*/
int cros_ec_get_next_event(struct cros_ec_device *ec_dev);
+/**
+ * cros_ec_get_host_event - Return a mask of event set by the EC.
+ *
+ * Returns the mask of the event that caused the last interrupt to be raised.
+ *
+ * @ec_dev: Device to fetch event from
+ * @return event mask, 0 on error
+ */
+uint32_t cros_ec_get_host_event(struct cros_ec_device *ec_dev);
+
/* sysfs stuff */
extern struct attribute_group cros_ec_attr_group;
extern struct attribute_group cros_ec_lightbar_attr_group;
diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index d86526f0bd8e..4f056d2747ff 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -286,6 +286,9 @@ enum host_event_code {
/* Hang detect logic detected a hang and warm rebooted the AP */
EC_HOST_EVENT_HANG_REBOOT = 21,
+ /* PD MCU triggering host event */
+ EC_HOST_EVENT_PD_MCU = 22,
+
/*
* The high bit of the event mask is not used as a host event code. If
* it reads back as set, then the entire event mask should be
@@ -2273,6 +2276,13 @@ struct ec_params_ext_power_current_limit {
uint32_t limit; /* in mA */
} __packed;
+struct ec_params_external_power_limit_v1 {
+ uint16_t current_lim; /* in mA, or EC_POWER_LIMIT_NONE to clear limit */
+ uint16_t voltage_lim; /* in mV, or EC_POWER_LIMIT_NONE to clear limit */
+} __packed;
+
+#define EC_POWER_LIMIT_NONE 0xffff
+
/*****************************************************************************/
/* Smart battery pass-through */
@@ -2527,8 +2537,6 @@ struct ec_params_reboot_ec {
*/
#define EC_CMD_VERSION0 0xdc
-#endif /* !__ACPI__ */
-
/*****************************************************************************/
/*
* PD commands
@@ -2538,16 +2546,51 @@ struct ec_params_reboot_ec {
/* EC to PD MCU exchange status command */
#define EC_CMD_PD_EXCHANGE_STATUS 0x100
+#define EC_VER_PD_EXCHANGE_STATUS 2
+
+enum pd_charge_state {
+ PD_CHARGE_NO_CHANGE = 0, /* Don't change charge state */
+ PD_CHARGE_NONE, /* No charging allowed */
+ PD_CHARGE_5V, /* 5V charging only */
+ PD_CHARGE_MAX /* Charge at max voltage */
+};
/* Status of EC being sent to PD */
+#define EC_STATUS_HIBERNATING (1 << 0)
+
struct ec_params_pd_status {
- int8_t batt_soc; /* battery state of charge */
+ uint8_t status; /* EC status */
+ int8_t batt_soc; /* battery state of charge */
+ uint8_t charge_state; /* charging state (from enum pd_charge_state) */
} __packed;
/* Status of PD being sent back to EC */
+#define PD_STATUS_HOST_EVENT (1 << 0) /* Forward host event to AP */
+#define PD_STATUS_IN_RW (1 << 1) /* Running RW image */
+#define PD_STATUS_JUMPED_TO_IMAGE (1 << 2) /* Current image was jumped to */
+#define PD_STATUS_TCPC_ALERT_0 (1 << 3) /* Alert active in port 0 TCPC */
+#define PD_STATUS_TCPC_ALERT_1 (1 << 4) /* Alert active in port 1 TCPC */
+#define PD_STATUS_TCPC_ALERT_2 (1 << 5) /* Alert active in port 2 TCPC */
+#define PD_STATUS_TCPC_ALERT_3 (1 << 6) /* Alert active in port 3 TCPC */
+#define PD_STATUS_EC_INT_ACTIVE (PD_STATUS_TCPC_ALERT_0 | \
+ PD_STATUS_TCPC_ALERT_1 | \
+ PD_STATUS_HOST_EVENT)
struct ec_response_pd_status {
- int8_t status; /* PD MCU status */
- uint32_t curr_lim_ma; /* input current limit */
+ uint32_t curr_lim_ma; /* input current limit */
+ uint16_t status; /* PD MCU status */
+ int8_t active_charge_port; /* active charging port */
+} __packed;
+
+/* AP to PD MCU host event status command, cleared on read */
+#define EC_CMD_PD_HOST_EVENT_STATUS 0x104
+
+/* PD MCU host event status bits */
+#define PD_EVENT_UPDATE_DEVICE (1 << 0)
+#define PD_EVENT_POWER_CHANGE (1 << 1)
+#define PD_EVENT_IDENTITY_RECEIVED (1 << 2)
+#define PD_EVENT_DATA_SWAP (1 << 3)
+struct ec_response_host_event_status {
+ uint32_t status; /* PD MCU host event status */
} __packed;
/* Set USB type-C port role and muxes */
@@ -2559,6 +2602,7 @@ enum usb_pd_control_role {
USB_PD_CTRL_ROLE_TOGGLE_OFF = 2,
USB_PD_CTRL_ROLE_FORCE_SINK = 3,
USB_PD_CTRL_ROLE_FORCE_SOURCE = 4,
+ USB_PD_CTRL_ROLE_COUNT
};
enum usb_pd_control_mux {
@@ -2568,14 +2612,284 @@ enum usb_pd_control_mux {
USB_PD_CTRL_MUX_DP = 3,
USB_PD_CTRL_MUX_DOCK = 4,
USB_PD_CTRL_MUX_AUTO = 5,
+ USB_PD_CTRL_MUX_COUNT
+};
+
+enum usb_pd_control_swap {
+ USB_PD_CTRL_SWAP_NONE = 0,
+ USB_PD_CTRL_SWAP_DATA = 1,
+ USB_PD_CTRL_SWAP_POWER = 2,
+ USB_PD_CTRL_SWAP_VCONN = 3,
+ USB_PD_CTRL_SWAP_COUNT
};
struct ec_params_usb_pd_control {
uint8_t port;
uint8_t role;
uint8_t mux;
+ uint8_t swap;
+} __packed;
+
+#define PD_CTRL_RESP_ENABLED_COMMS (1 << 0) /* Communication enabled */
+#define PD_CTRL_RESP_ENABLED_CONNECTED (1 << 1) /* Device connected */
+#define PD_CTRL_RESP_ENABLED_PD_CAPABLE (1 << 2) /* Partner is PD capable */
+
+#define PD_CTRL_RESP_ROLE_POWER (1 << 0) /* 0=SNK/1=SRC */
+#define PD_CTRL_RESP_ROLE_DATA (1 << 1) /* 0=UFP/1=DFP */
+#define PD_CTRL_RESP_ROLE_VCONN (1 << 2) /* Vconn status */
+#define PD_CTRL_RESP_ROLE_DR_POWER (1 << 3) /* Partner is dualrole power */
+#define PD_CTRL_RESP_ROLE_DR_DATA (1 << 4) /* Partner is dualrole data */
+#define PD_CTRL_RESP_ROLE_USB_COMM (1 << 5) /* Partner USB comm capable */
+#define PD_CTRL_RESP_ROLE_EXT_POWERED (1 << 6) /* Partner externally powerd */
+
+struct ec_response_usb_pd_control {
+ uint8_t enabled;
+ uint8_t role;
+ uint8_t polarity;
+ uint8_t state;
+} __packed;
+
+struct ec_response_usb_pd_control_v1 {
+ uint8_t enabled;
+ uint8_t role;
+ uint8_t polarity;
+ char state[32];
+} __packed;
+
+#define EC_CMD_USB_PD_PORTS 0x102
+
+struct ec_response_usb_pd_ports {
+ uint8_t num_ports;
+} __packed;
+
+#define EC_CMD_USB_PD_POWER_INFO 0x103
+
+#define PD_POWER_CHARGING_PORT 0xff
+struct ec_params_usb_pd_power_info {
+ uint8_t port;
+} __packed;
+
+enum usb_chg_type {
+ USB_CHG_TYPE_NONE,
+ USB_CHG_TYPE_PD,
+ USB_CHG_TYPE_C,
+ USB_CHG_TYPE_PROPRIETARY,
+ USB_CHG_TYPE_BC12_DCP,
+ USB_CHG_TYPE_BC12_CDP,
+ USB_CHG_TYPE_BC12_SDP,
+ USB_CHG_TYPE_OTHER,
+ USB_CHG_TYPE_VBUS,
+ USB_CHG_TYPE_UNKNOWN,
+};
+enum usb_power_roles {
+ USB_PD_PORT_POWER_DISCONNECTED,
+ USB_PD_PORT_POWER_SOURCE,
+ USB_PD_PORT_POWER_SINK,
+ USB_PD_PORT_POWER_SINK_NOT_CHARGING,
+};
+
+struct usb_chg_measures {
+ uint16_t voltage_max;
+ uint16_t voltage_now;
+ uint16_t current_max;
+ uint16_t current_lim;
+} __packed;
+
+struct ec_response_usb_pd_power_info {
+ uint8_t role;
+ uint8_t type;
+ uint8_t dualrole;
+ uint8_t reserved1;
+ struct usb_chg_measures meas;
+ uint32_t max_power;
+} __packed;
+
+/* Write USB-PD device FW */
+#define EC_CMD_USB_PD_FW_UPDATE 0x110
+
+enum usb_pd_fw_update_cmds {
+ USB_PD_FW_REBOOT,
+ USB_PD_FW_FLASH_ERASE,
+ USB_PD_FW_FLASH_WRITE,
+ USB_PD_FW_ERASE_SIG,
+};
+
+struct ec_params_usb_pd_fw_update {
+ uint16_t dev_id;
+ uint8_t cmd;
+ uint8_t port;
+ uint32_t size; /* Size to write in bytes */
+ /* Followed by data to write */
+} __packed;
+
+/* Write USB-PD Accessory RW_HASH table entry */
+#define EC_CMD_USB_PD_RW_HASH_ENTRY 0x111
+/* RW hash is first 20 bytes of SHA-256 of RW section */
+#define PD_RW_HASH_SIZE 20
+struct ec_params_usb_pd_rw_hash_entry {
+ uint16_t dev_id;
+ uint8_t dev_rw_hash[PD_RW_HASH_SIZE];
+ uint8_t reserved; /* For alignment of current_image */
+ uint32_t current_image; /* One of ec_current_image */
+} __packed;
+
+/* Read USB-PD Accessory info */
+#define EC_CMD_USB_PD_DEV_INFO 0x112
+
+struct ec_params_usb_pd_info_request {
+ uint8_t port;
+} __packed;
+
+/* Read USB-PD Device discovery info */
+#define EC_CMD_USB_PD_DISCOVERY 0x113
+struct ec_params_usb_pd_discovery_entry {
+ uint16_t vid; /* USB-IF VID */
+ uint16_t pid; /* USB-IF PID */
+ uint8_t ptype; /* product type (hub,periph,cable,ama) */
+} __packed;
+
+/* Override default charge behavior */
+#define EC_CMD_PD_CHARGE_PORT_OVERRIDE 0x114
+
+/* Negative port parameters have special meaning */
+enum usb_pd_override_ports {
+ OVERRIDE_DONT_CHARGE = -2,
+ OVERRIDE_OFF = -1,
+ /* [0, CONFIG_USB_PD_PORT_COUNT): Port# */
+};
+
+struct ec_params_charge_port_override {
+ int16_t override_port; /* Override port# */
+} __packed;
+
+/* Read (and delete) one entry of PD event log */
+#define EC_CMD_PD_GET_LOG_ENTRY 0x115
+
+struct ec_response_pd_log {
+ uint32_t timestamp; /* relative timestamp in milliseconds */
+ uint8_t type; /* event type : see PD_EVENT_xx below */
+ uint8_t size_port; /* [7:5] port number [4:0] payload size in bytes */
+ uint16_t data; /* type-defined data payload */
+ uint8_t payload[0]; /* optional additional data payload: 0..16 bytes */
+} __packed;
+
+
+/* The timestamp is the microsecond counter shifted to get about a ms. */
+#define PD_LOG_TIMESTAMP_SHIFT 10 /* 1 LSB = 1024us */
+
+#define PD_LOG_SIZE_MASK 0x1f
+#define PD_LOG_PORT_MASK 0xe0
+#define PD_LOG_PORT_SHIFT 5
+#define PD_LOG_PORT_SIZE(port, size) (((port) << PD_LOG_PORT_SHIFT) | \
+ ((size) & PD_LOG_SIZE_MASK))
+#define PD_LOG_PORT(size_port) ((size_port) >> PD_LOG_PORT_SHIFT)
+#define PD_LOG_SIZE(size_port) ((size_port) & PD_LOG_SIZE_MASK)
+
+/* PD event log : entry types */
+/* PD MCU events */
+#define PD_EVENT_MCU_BASE 0x00
+#define PD_EVENT_MCU_CHARGE (PD_EVENT_MCU_BASE+0)
+#define PD_EVENT_MCU_CONNECT (PD_EVENT_MCU_BASE+1)
+/* Reserved for custom board event */
+#define PD_EVENT_MCU_BOARD_CUSTOM (PD_EVENT_MCU_BASE+2)
+/* PD generic accessory events */
+#define PD_EVENT_ACC_BASE 0x20
+#define PD_EVENT_ACC_RW_FAIL (PD_EVENT_ACC_BASE+0)
+#define PD_EVENT_ACC_RW_ERASE (PD_EVENT_ACC_BASE+1)
+/* PD power supply events */
+#define PD_EVENT_PS_BASE 0x40
+#define PD_EVENT_PS_FAULT (PD_EVENT_PS_BASE+0)
+/* PD video dongles events */
+#define PD_EVENT_VIDEO_BASE 0x60
+#define PD_EVENT_VIDEO_DP_MODE (PD_EVENT_VIDEO_BASE+0)
+#define PD_EVENT_VIDEO_CODEC (PD_EVENT_VIDEO_BASE+1)
+/* Returned in the "type" field, when there is no entry available */
+#define PD_EVENT_NO_ENTRY 0xff
+
+/*
+ * PD_EVENT_MCU_CHARGE event definition :
+ * the payload is "struct usb_chg_measures"
+ * the data field contains the port state flags as defined below :
+ */
+/* Port partner is a dual role device */
+#define CHARGE_FLAGS_DUAL_ROLE (1 << 15)
+/* Port is the pending override port */
+#define CHARGE_FLAGS_DELAYED_OVERRIDE (1 << 14)
+/* Port is the override port */
+#define CHARGE_FLAGS_OVERRIDE (1 << 13)
+/* Charger type */
+#define CHARGE_FLAGS_TYPE_SHIFT 3
+#define CHARGE_FLAGS_TYPE_MASK (0xf << CHARGE_FLAGS_TYPE_SHIFT)
+/* Power delivery role */
+#define CHARGE_FLAGS_ROLE_MASK (7 << 0)
+
+/*
+ * PD_EVENT_PS_FAULT data field flags definition :
+ */
+#define PS_FAULT_OCP 1
+#define PS_FAULT_FAST_OCP 2
+#define PS_FAULT_OVP 3
+#define PS_FAULT_DISCH 4
+
+/*
+ * PD_EVENT_VIDEO_CODEC payload is "struct mcdp_info".
+ */
+struct mcdp_version {
+ uint8_t major;
+ uint8_t minor;
+ uint16_t build;
} __packed;
+struct mcdp_info {
+ uint8_t family[2];
+ uint8_t chipid[2];
+ struct mcdp_version irom;
+ struct mcdp_version fw;
+} __packed;
+
+/* struct mcdp_info field decoding */
+#define MCDP_CHIPID(chipid) ((chipid[0] << 8) | chipid[1])
+#define MCDP_FAMILY(family) ((family[0] << 8) | family[1])
+
+/* Get/Set USB-PD Alternate mode info */
+#define EC_CMD_USB_PD_GET_AMODE 0x116
+struct ec_params_usb_pd_get_mode_request {
+ uint16_t svid_idx; /* SVID index to get */
+ uint8_t port; /* port */
+} __packed;
+
+struct ec_params_usb_pd_get_mode_response {
+ uint16_t svid; /* SVID */
+ uint16_t opos; /* Object Position */
+ uint32_t vdo[6]; /* Mode VDOs */
+} __packed;
+
+#define EC_CMD_USB_PD_SET_AMODE 0x117
+
+enum pd_mode_cmd {
+ PD_EXIT_MODE = 0,
+ PD_ENTER_MODE = 1,
+ /* Not a command. Do NOT remove. */
+ PD_MODE_CMD_COUNT,
+};
+
+struct ec_params_usb_pd_set_mode_request {
+ uint32_t cmd; /* enum pd_mode_cmd */
+ uint16_t svid; /* SVID to set */
+ uint8_t opos; /* Object Position */
+ uint8_t port; /* port */
+} __packed;
+
+/* Ask the PD MCU to record a log of a requested type */
+#define EC_CMD_PD_WRITE_LOG_ENTRY 0x118
+
+struct ec_params_pd_write_log_entry {
+ uint8_t type; /* event type : see PD_EVENT_xx above */
+ uint8_t port; /* port#, or 0 for events unrelated to a given port */
+} __packed;
+
+#endif /* !__ACPI__ */
+
/*****************************************************************************/
/*
* Passthru commands
--
2.5.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v1 5/6] platform/chrome: Register USB PD charger device
2016-02-05 13:32 [PATCH v1 0/6] EC-based USB Power Delivery support for Chrome machines Tomeu Vizoso
` (3 preceding siblings ...)
2016-02-05 13:32 ` [PATCH v1 4/6] power: cros_usbpd-charger: Add EC-based USB PD charger driver Tomeu Vizoso
@ 2016-02-05 13:33 ` Tomeu Vizoso
2016-02-10 16:46 ` Lee Jones
2016-02-05 13:33 ` [PATCH v1 6/6] platform/chrome: Check the USB PD feature before creating a charger Tomeu Vizoso
5 siblings, 1 reply; 23+ messages in thread
From: Tomeu Vizoso @ 2016-02-05 13:33 UTC (permalink / raw)
To: linux-kernel
Cc: Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Tomeu Vizoso, Olof Johansson, Lee Jones
Check if a EC considers EC_CMD_USB_PD_PORTS a valid command and register
a USB PD charger device if so. This check is needed for older versions
of the ChromeOS EC firmware that don't support the EC_CMD_GET_FEATURES
command.
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
drivers/mfd/cros_ec.c | 15 +++++++++++++++
drivers/platform/chrome/cros_ec_dev.c | 34 ++++++++++++++++++++++++++++++++++
include/linux/mfd/cros_ec.h | 2 ++
3 files changed, 51 insertions(+)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index fbe78b819fdd..20ca9794d2f3 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -202,5 +202,20 @@ EXPORT_SYMBOL(cros_ec_resume);
#endif
+static const struct mfd_cell cros_usb_pd_charger_devs[] = {
+ {
+ .name = "cros-usb-pd-charger",
+ .id = -1,
+ },
+};
+
+int cros_ec_usb_pd_charger_register(struct device *dev)
+{
+ return mfd_add_devices(dev, 0, cros_usb_pd_charger_devs,
+ ARRAY_SIZE(cros_usb_pd_charger_devs),
+ NULL, 0, NULL);
+}
+EXPORT_SYMBOL(cros_ec_usb_pd_charger_register);
+
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("ChromeOS EC core driver");
diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index d45cd254ed1c..7a97cd313c68 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -87,6 +87,29 @@ exit:
return ret;
}
+static int cros_ec_has_cmd_usb_pd_ports(struct cros_ec_dev *ec)
+{
+ struct cros_ec_command *msg;
+ int ret;
+
+ msg = kmalloc(sizeof(*msg) + sizeof(struct ec_response_usb_pd_ports),
+ GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ msg->version = 0;
+ msg->command = EC_CMD_USB_PD_PORTS + ec->cmd_offset;
+ msg->insize = sizeof(struct ec_response_usb_pd_ports);
+ msg->outsize = 0;
+
+ ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+ ret = ret >= 0 && msg->result == EC_RES_SUCCESS;
+
+ kfree(msg);
+
+ return ret;
+}
+
/* Device file ops */
static int ec_device_open(struct inode *inode, struct file *filp)
{
@@ -269,8 +292,19 @@ static int ec_device_probe(struct platform_device *pdev)
goto dev_reg_failed;
}
+ /* check whether this EC instance has the PD charge manager */
+ if (cros_ec_has_cmd_usb_pd_ports(ec)) {
+ retval = cros_ec_usb_pd_charger_register(dev);
+ if (retval) {
+ dev_err(dev, "failed to add usb-pd-charger device\n");
+ goto pd_reg_failed;
+ }
+ }
+
return 0;
+pd_reg_failed:
+ put_device(&ec->class_dev);
dev_reg_failed:
set_named_failed:
dev_set_drvdata(dev, NULL);
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index c21583411ba0..543191f493a9 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -329,4 +329,6 @@ extern struct attribute_group cros_ec_vbc_attr_group;
*/
uint32_t cros_ec_get_host_event(struct cros_ec_device *ec_dev);
+int cros_ec_usb_pd_charger_register(struct device *dev);
+
#endif /* __LINUX_MFD_CROS_EC_H */
--
2.5.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v1 6/6] platform/chrome: Check the USB PD feature before creating a charger
2016-02-05 13:32 [PATCH v1 0/6] EC-based USB Power Delivery support for Chrome machines Tomeu Vizoso
` (4 preceding siblings ...)
2016-02-05 13:33 ` [PATCH v1 5/6] platform/chrome: Register USB PD charger device Tomeu Vizoso
@ 2016-02-05 13:33 ` Tomeu Vizoso
2016-02-10 16:28 ` Lee Jones
5 siblings, 1 reply; 23+ messages in thread
From: Tomeu Vizoso @ 2016-02-05 13:33 UTC (permalink / raw)
To: linux-kernel
Cc: Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Tomeu Vizoso, Olof Johansson, Lee Jones
From: Vincent Palatin <vpalatin@chromium.org>
Use the EC_CMD_GET_FEATURES message to check the supported features for
each MCU before instantied a USB-PD charger.
Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
[tomeu: adapted to changes in mainline]
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
---
drivers/platform/chrome/cros_ec_dev.c | 40 ++++++++++++++++-
include/linux/mfd/cros_ec.h | 1 +
include/linux/mfd/cros_ec_commands.h | 84 +++++++++++++++++++++++++++++++++++
3 files changed, 124 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index 7a97cd313c68..1995cccfc59c 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -87,6 +87,41 @@ exit:
return ret;
}
+static int cros_ec_check_features(struct cros_ec_dev *ec, int feature)
+{
+ struct cros_ec_command *msg;
+ int ret;
+
+ if (ec->features[0] == -1U && ec->features[1] == -1U) {
+ /* features bitmap not read yet */
+
+ msg = kmalloc(sizeof(*msg) + sizeof(ec->features), GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ msg->version = 0;
+ msg->command = EC_CMD_GET_FEATURES + ec->cmd_offset;
+ msg->insize = sizeof(ec->features);
+ msg->outsize = 0;
+
+ ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+ if ((ret < 0) || msg->result != EC_RES_SUCCESS) {
+ dev_warn(ec->dev, "cannot get EC features: %d/%d\n",
+ ret, msg->result);
+ memset(ec->features, 0, sizeof(ec->features));
+ }
+
+ memcpy(ec->features, msg->data, sizeof(ec->features));
+
+ dev_dbg(ec->dev, "EC features %08x %08x\n",
+ ec->features[0], ec->features[1]);
+
+ kfree(msg);
+ }
+
+ return ec->features[feature / 32] & EC_FEATURE_MASK_0(feature);
+}
+
static int cros_ec_has_cmd_usb_pd_ports(struct cros_ec_dev *ec)
{
struct cros_ec_command *msg;
@@ -255,6 +290,8 @@ static int ec_device_probe(struct platform_device *pdev)
ec->ec_dev = dev_get_drvdata(dev->parent);
ec->dev = dev;
ec->cmd_offset = ec_platform->cmd_offset;
+ ec->features[0] = -1U; /* Not cached yet */
+ ec->features[1] = -1U; /* Not cached yet */
device_initialize(&ec->class_dev);
cdev_init(&ec->cdev, &fops);
@@ -293,7 +330,8 @@ static int ec_device_probe(struct platform_device *pdev)
}
/* check whether this EC instance has the PD charge manager */
- if (cros_ec_has_cmd_usb_pd_ports(ec)) {
+ if (cros_ec_check_features(ec, EC_FEATURE_USB_PD) ||
+ cros_ec_has_cmd_usb_pd_ports(ec)) {
retval = cros_ec_usb_pd_charger_register(dev);
if (retval) {
dev_err(dev, "failed to add usb-pd-charger device\n");
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 543191f493a9..02ad92572d5a 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -193,6 +193,7 @@ struct cros_ec_dev {
struct cros_ec_device *ec_dev;
struct device *dev;
u16 cmd_offset;
+ u32 features[2];
};
/**
diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index 4f056d2747ff..cc3a4b1b19e8 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -716,6 +716,90 @@ struct ec_response_get_set_value {
/* More than one command can use these structs to get/set paramters. */
#define EC_CMD_GSV_PAUSE_IN_S5 0x0c
+/*****************************************************************************/
+/* List the features supported by the firmware */
+#define EC_CMD_GET_FEATURES 0x0d
+
+/* Supported features */
+enum ec_feature_code {
+ /*
+ * This image contains a limited set of features. Another image
+ * in RW partition may support more features.
+ */
+ EC_FEATURE_LIMITED = 0,
+ /*
+ * Commands for probing/reading/writing/erasing the flash in the
+ * EC are present.
+ */
+ EC_FEATURE_FLASH = 1,
+ /*
+ * Can control the fan speed directly.
+ */
+ EC_FEATURE_PWM_FAN = 2,
+ /*
+ * Can control the intensity of the keyboard backlight.
+ */
+ EC_FEATURE_PWM_KEYB = 3,
+ /*
+ * Support Google lightbar, introduced on Pixel.
+ */
+ EC_FEATURE_LIGHTBAR = 4,
+ /* Control of LEDs */
+ EC_FEATURE_LED = 5,
+ /* Exposes an interface to control gyro and sensors.
+ * The host goes through the EC to access these sensors.
+ * In addition, the EC may provide composite sensors, like lid angle.
+ */
+ EC_FEATURE_MOTION_SENSE = 6,
+ /* The keyboard is controlled by the EC */
+ EC_FEATURE_KEYB = 7,
+ /* The AP can use part of the EC flash as persistent storage. */
+ EC_FEATURE_PSTORE = 8,
+ /* The EC monitors BIOS port 80h, and can return POST codes. */
+ EC_FEATURE_PORT80 = 9,
+ /*
+ * Thermal management: include TMP specific commands.
+ * Higher level than direct fan control.
+ */
+ EC_FEATURE_THERMAL = 10,
+ /* Can switch the screen backlight on/off */
+ EC_FEATURE_BKLIGHT_SWITCH = 11,
+ /* Can switch the wifi module on/off */
+ EC_FEATURE_WIFI_SWITCH = 12,
+ /* Monitor host events, through for example SMI or SCI */
+ EC_FEATURE_HOST_EVENTS = 13,
+ /* The EC exposes GPIO commands to control/monitor connected devices. */
+ EC_FEATURE_GPIO = 14,
+ /* The EC can send i2c messages to downstream devices. */
+ EC_FEATURE_I2C = 15,
+ /* Command to control charger are included */
+ EC_FEATURE_CHARGER = 16,
+ /* Simple battery support. */
+ EC_FEATURE_BATTERY = 17,
+ /*
+ * Support Smart battery protocol
+ * (Common Smart Battery System Interface Specification)
+ */
+ EC_FEATURE_SMART_BATTERY = 18,
+ /* EC can dectect when the host hangs. */
+ EC_FEATURE_HANG_DETECT = 19,
+ /* Report power information, for pit only */
+ EC_FEATURE_PMU = 20,
+ /* Another Cros EC device is present downstream of this one */
+ EC_FEATURE_SUB_MCU = 21,
+ /* Support USB Power delivery (PD) commands */
+ EC_FEATURE_USB_PD = 22,
+ /* Control USB multiplexer, for audio through USB port for instance. */
+ EC_FEATURE_USB_MUX = 23,
+ /* Motion Sensor code has an internal software FIFO */
+ EC_FEATURE_MOTION_SENSE_FIFO = 24,
+};
+
+#define EC_FEATURE_MASK_0(event_code) (1UL << (event_code % 32))
+#define EC_FEATURE_MASK_1(event_code) (1UL << (event_code - 32))
+struct ec_response_get_features {
+ uint32_t flags[2];
+} __packed;
/*****************************************************************************/
/* Flash commands */
--
2.5.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v1 3/6] power_supply: Add types for USB Type C and PD chargers
2016-02-05 13:32 ` [PATCH v1 3/6] power_supply: Add types for USB Type C and PD chargers Tomeu Vizoso
@ 2016-02-05 18:38 ` Benson Leung
2016-02-11 10:00 ` Tomeu Vizoso
0 siblings, 1 reply; 23+ messages in thread
From: Benson Leung @ 2016-02-05 18:38 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: linux-kernel, Sameer Nanda, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
linux-pm
Thanks for putting this up Tomeu!
On Fri, Feb 5, 2016 at 5:32 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> From: Benson Leung <bleung@chromium.org>
>
> This adds power supply types for USB chargers defined in
> the USB Type-C Specification 1.1 and in the
> USB Power Delivery Specification Revision 2.0 V1.1.
>
> The following are added :
> POWER_SUPPLY_TYPE_USB_TYPE_C, /* Type C Port */
> POWER_SUPPLY_TYPE_USB_PD, /* Type C Power Delivery Port */
> POWER_SUPPLY_TYPE_USB_PD_DRP, /* Type C PD Dual Role Port */
I authored this patch, but in looking back, the comments should be tweaked.
USB_PD and USB_PD_DRP are not necessarily Type-C ports, as you may
have USB_PD ports that are either Type-A or Type-B as well.
Could you change these to be "USB Power Delivery Port" and "USB PD
Dual Role Port"?
>
> Signed-off-by: Benson Leung <bleung@chromium.org>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Reviewed-by: Alec Berg <alecaberg@chromium.org>
> Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
> Reviewed-by: Todd Broch <tbroch@chromium.org>
> ---
>
> drivers/power/power_supply_sysfs.c | 3 ++-
> include/linux/power_supply.h | 3 +++
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
> index ed2d7fd0c734..80fed98832f9 100644
> --- a/drivers/power/power_supply_sysfs.c
> +++ b/drivers/power/power_supply_sysfs.c
> @@ -45,7 +45,8 @@ static ssize_t power_supply_show_property(struct device *dev,
> char *buf) {
> static char *type_text[] = {
> "Unknown", "Battery", "UPS", "Mains", "USB",
> - "USB_DCP", "USB_CDP", "USB_ACA"
> + "USB_DCP", "USB_CDP", "USB_ACA", "USB_C",
> + "USB_PD", "USB_PD_DRP"
> };
> static char *status_text[] = {
> "Unknown", "Charging", "Discharging", "Not charging", "Full"
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index ef9f1592185d..206c3384c6fa 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -163,6 +163,9 @@ enum power_supply_type {
> POWER_SUPPLY_TYPE_USB_DCP, /* Dedicated Charging Port */
> POWER_SUPPLY_TYPE_USB_CDP, /* Charging Downstream Port */
> POWER_SUPPLY_TYPE_USB_ACA, /* Accessory Charger Adapters */
> + POWER_SUPPLY_TYPE_USB_TYPE_C, /* Type C Port */
> + POWER_SUPPLY_TYPE_USB_PD, /* Type C Power Delivery Port */
> + POWER_SUPPLY_TYPE_USB_PD_DRP, /* Type C PD Dual Role Port */
Same change as in commit message. Thanks!
> };
>
> enum power_supply_notifier_events {
--
Benson Leung
Senior Software Engineer, Chrom* OS
bleung@chromium.org
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 1/6] mfd: cros_ec: small kerneldoc fix
2016-02-05 13:32 ` [PATCH v1 1/6] mfd: cros_ec: small kerneldoc fix Tomeu Vizoso
@ 2016-02-05 18:46 ` Benson Leung
2016-02-10 16:25 ` Lee Jones
1 sibling, 0 replies; 23+ messages in thread
From: Benson Leung @ 2016-02-05 18:46 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: linux-kernel, Sameer Nanda, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Lee Jones
On Fri, Feb 5, 2016 at 5:32 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> s/cros_ec_register/cros_ec_query_all
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Reviewed-by: Benson Leung <bleung@chromium.org>
> ---
>
> include/linux/mfd/cros_ec.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 494682ce4bf3..a677c2bd485c 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -245,7 +245,7 @@ int cros_ec_remove(struct cros_ec_device *ec_dev);
> int cros_ec_register(struct cros_ec_device *ec_dev);
>
> /**
> - * cros_ec_register - Query the protocol version supported by the ChromeOS EC
> + * cros_ec_query_all - Query the protocol version supported by the ChromeOS EC
> *
> * @ec_dev: Device to register
> * @return 0 if ok, -ve on error
> --
> 2.5.0
>
--
Benson Leung
Senior Software Engineer, Chrom* OS
bleung@chromium.org
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 1/6] mfd: cros_ec: small kerneldoc fix
2016-02-05 13:32 ` [PATCH v1 1/6] mfd: cros_ec: small kerneldoc fix Tomeu Vizoso
2016-02-05 18:46 ` Benson Leung
@ 2016-02-10 16:25 ` Lee Jones
1 sibling, 0 replies; 23+ messages in thread
From: Lee Jones @ 2016-02-10 16:25 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: linux-kernel, Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou
On Fri, 05 Feb 2016, Tomeu Vizoso wrote:
> s/cros_ec_register/cros_ec_query_all
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> ---
>
> include/linux/mfd/cros_ec.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 494682ce4bf3..a677c2bd485c 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -245,7 +245,7 @@ int cros_ec_remove(struct cros_ec_device *ec_dev);
> int cros_ec_register(struct cros_ec_device *ec_dev);
>
> /**
> - * cros_ec_register - Query the protocol version supported by the ChromeOS EC
> + * cros_ec_query_all - Query the protocol version supported by the ChromeOS EC
> *
> * @ec_dev: Device to register
> * @return 0 if ok, -ve on error
I'll fix the subject line for you this time.
In future please submit using the format set by the subsystem.
`git log --oneline -- <subsystem>` helps with this.
Applied, thanks.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 6/6] platform/chrome: Check the USB PD feature before creating a charger
2016-02-05 13:33 ` [PATCH v1 6/6] platform/chrome: Check the USB PD feature before creating a charger Tomeu Vizoso
@ 2016-02-10 16:28 ` Lee Jones
0 siblings, 0 replies; 23+ messages in thread
From: Lee Jones @ 2016-02-10 16:28 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: linux-kernel, Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Olof Johansson
On Fri, 05 Feb 2016, Tomeu Vizoso wrote:
> From: Vincent Palatin <vpalatin@chromium.org>
>
> Use the EC_CMD_GET_FEATURES message to check the supported features for
> each MCU before instantied a USB-PD charger.
>
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> [tomeu: adapted to changes in mainline]
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
>
> ---
>
> drivers/platform/chrome/cros_ec_dev.c | 40 ++++++++++++++++-
> include/linux/mfd/cros_ec.h | 1 +
> include/linux/mfd/cros_ec_commands.h | 84 +++++++++++++++++++++++++++++++++++
Acked-by: Lee Jones <lee.jones@linaro.org>
> 3 files changed, 124 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
> index 7a97cd313c68..1995cccfc59c 100644
> --- a/drivers/platform/chrome/cros_ec_dev.c
> +++ b/drivers/platform/chrome/cros_ec_dev.c
> @@ -87,6 +87,41 @@ exit:
> return ret;
> }
>
> +static int cros_ec_check_features(struct cros_ec_dev *ec, int feature)
> +{
> + struct cros_ec_command *msg;
> + int ret;
> +
> + if (ec->features[0] == -1U && ec->features[1] == -1U) {
> + /* features bitmap not read yet */
> +
> + msg = kmalloc(sizeof(*msg) + sizeof(ec->features), GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> + msg->version = 0;
> + msg->command = EC_CMD_GET_FEATURES + ec->cmd_offset;
> + msg->insize = sizeof(ec->features);
> + msg->outsize = 0;
> +
> + ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> + if ((ret < 0) || msg->result != EC_RES_SUCCESS) {
> + dev_warn(ec->dev, "cannot get EC features: %d/%d\n",
> + ret, msg->result);
> + memset(ec->features, 0, sizeof(ec->features));
> + }
> +
> + memcpy(ec->features, msg->data, sizeof(ec->features));
> +
> + dev_dbg(ec->dev, "EC features %08x %08x\n",
> + ec->features[0], ec->features[1]);
> +
> + kfree(msg);
> + }
> +
> + return ec->features[feature / 32] & EC_FEATURE_MASK_0(feature);
> +}
> +
> static int cros_ec_has_cmd_usb_pd_ports(struct cros_ec_dev *ec)
> {
> struct cros_ec_command *msg;
> @@ -255,6 +290,8 @@ static int ec_device_probe(struct platform_device *pdev)
> ec->ec_dev = dev_get_drvdata(dev->parent);
> ec->dev = dev;
> ec->cmd_offset = ec_platform->cmd_offset;
> + ec->features[0] = -1U; /* Not cached yet */
> + ec->features[1] = -1U; /* Not cached yet */
> device_initialize(&ec->class_dev);
> cdev_init(&ec->cdev, &fops);
>
> @@ -293,7 +330,8 @@ static int ec_device_probe(struct platform_device *pdev)
> }
>
> /* check whether this EC instance has the PD charge manager */
> - if (cros_ec_has_cmd_usb_pd_ports(ec)) {
> + if (cros_ec_check_features(ec, EC_FEATURE_USB_PD) ||
> + cros_ec_has_cmd_usb_pd_ports(ec)) {
> retval = cros_ec_usb_pd_charger_register(dev);
> if (retval) {
> dev_err(dev, "failed to add usb-pd-charger device\n");
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 543191f493a9..02ad92572d5a 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -193,6 +193,7 @@ struct cros_ec_dev {
> struct cros_ec_device *ec_dev;
> struct device *dev;
> u16 cmd_offset;
> + u32 features[2];
> };
>
> /**
> diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
> index 4f056d2747ff..cc3a4b1b19e8 100644
> --- a/include/linux/mfd/cros_ec_commands.h
> +++ b/include/linux/mfd/cros_ec_commands.h
> @@ -716,6 +716,90 @@ struct ec_response_get_set_value {
> /* More than one command can use these structs to get/set paramters. */
> #define EC_CMD_GSV_PAUSE_IN_S5 0x0c
>
> +/*****************************************************************************/
> +/* List the features supported by the firmware */
> +#define EC_CMD_GET_FEATURES 0x0d
> +
> +/* Supported features */
> +enum ec_feature_code {
> + /*
> + * This image contains a limited set of features. Another image
> + * in RW partition may support more features.
> + */
> + EC_FEATURE_LIMITED = 0,
> + /*
> + * Commands for probing/reading/writing/erasing the flash in the
> + * EC are present.
> + */
> + EC_FEATURE_FLASH = 1,
> + /*
> + * Can control the fan speed directly.
> + */
> + EC_FEATURE_PWM_FAN = 2,
> + /*
> + * Can control the intensity of the keyboard backlight.
> + */
> + EC_FEATURE_PWM_KEYB = 3,
> + /*
> + * Support Google lightbar, introduced on Pixel.
> + */
> + EC_FEATURE_LIGHTBAR = 4,
> + /* Control of LEDs */
> + EC_FEATURE_LED = 5,
> + /* Exposes an interface to control gyro and sensors.
> + * The host goes through the EC to access these sensors.
> + * In addition, the EC may provide composite sensors, like lid angle.
> + */
> + EC_FEATURE_MOTION_SENSE = 6,
> + /* The keyboard is controlled by the EC */
> + EC_FEATURE_KEYB = 7,
> + /* The AP can use part of the EC flash as persistent storage. */
> + EC_FEATURE_PSTORE = 8,
> + /* The EC monitors BIOS port 80h, and can return POST codes. */
> + EC_FEATURE_PORT80 = 9,
> + /*
> + * Thermal management: include TMP specific commands.
> + * Higher level than direct fan control.
> + */
> + EC_FEATURE_THERMAL = 10,
> + /* Can switch the screen backlight on/off */
> + EC_FEATURE_BKLIGHT_SWITCH = 11,
> + /* Can switch the wifi module on/off */
> + EC_FEATURE_WIFI_SWITCH = 12,
> + /* Monitor host events, through for example SMI or SCI */
> + EC_FEATURE_HOST_EVENTS = 13,
> + /* The EC exposes GPIO commands to control/monitor connected devices. */
> + EC_FEATURE_GPIO = 14,
> + /* The EC can send i2c messages to downstream devices. */
> + EC_FEATURE_I2C = 15,
> + /* Command to control charger are included */
> + EC_FEATURE_CHARGER = 16,
> + /* Simple battery support. */
> + EC_FEATURE_BATTERY = 17,
> + /*
> + * Support Smart battery protocol
> + * (Common Smart Battery System Interface Specification)
> + */
> + EC_FEATURE_SMART_BATTERY = 18,
> + /* EC can dectect when the host hangs. */
> + EC_FEATURE_HANG_DETECT = 19,
> + /* Report power information, for pit only */
> + EC_FEATURE_PMU = 20,
> + /* Another Cros EC device is present downstream of this one */
> + EC_FEATURE_SUB_MCU = 21,
> + /* Support USB Power delivery (PD) commands */
> + EC_FEATURE_USB_PD = 22,
> + /* Control USB multiplexer, for audio through USB port for instance. */
> + EC_FEATURE_USB_MUX = 23,
> + /* Motion Sensor code has an internal software FIFO */
> + EC_FEATURE_MOTION_SENSE_FIFO = 24,
> +};
> +
> +#define EC_FEATURE_MASK_0(event_code) (1UL << (event_code % 32))
> +#define EC_FEATURE_MASK_1(event_code) (1UL << (event_code - 32))
> +struct ec_response_get_features {
> + uint32_t flags[2];
> +} __packed;
>
> /*****************************************************************************/
> /* Flash commands */
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 5/6] platform/chrome: Register USB PD charger device
2016-02-05 13:33 ` [PATCH v1 5/6] platform/chrome: Register USB PD charger device Tomeu Vizoso
@ 2016-02-10 16:46 ` Lee Jones
2016-02-12 11:06 ` Tomeu Vizoso
0 siblings, 1 reply; 23+ messages in thread
From: Lee Jones @ 2016-02-10 16:46 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: linux-kernel, Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Olof Johansson
On Fri, 05 Feb 2016, Tomeu Vizoso wrote:
> Check if a EC considers EC_CMD_USB_PD_PORTS a valid command and register
> a USB PD charger device if so. This check is needed for older versions
> of the ChromeOS EC firmware that don't support the EC_CMD_GET_FEATURES
> command.
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> ---
>
> drivers/mfd/cros_ec.c | 15 +++++++++++++++
> drivers/platform/chrome/cros_ec_dev.c | 34 ++++++++++++++++++++++++++++++++++
> include/linux/mfd/cros_ec.h | 2 ++
> 3 files changed, 51 insertions(+)
>
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index fbe78b819fdd..20ca9794d2f3 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -202,5 +202,20 @@ EXPORT_SYMBOL(cros_ec_resume);
>
> #endif
>
> +static const struct mfd_cell cros_usb_pd_charger_devs[] = {
> + {
> + .name = "cros-usb-pd-charger",
> + .id = -1,
> + },
> +};
> +
> +int cros_ec_usb_pd_charger_register(struct device *dev)
> +{
> + return mfd_add_devices(dev, 0, cros_usb_pd_charger_devs,
> + ARRAY_SIZE(cros_usb_pd_charger_devs),
> + NULL, 0, NULL);
> +}
> +EXPORT_SYMBOL(cros_ec_usb_pd_charger_register);
I'm not keen on this idea at all.
You're calling cros_ec_usb_pd_charger_register() from a device you
registered from this same source file. Seems very incestuous and
hacky.
It's bad enough that we're calling into the platform driver from here
already, but seeing as we are, just extend the call to
cros_ec_query_all() to suit your purposes.
> MODULE_LICENSE("GPL");
> MODULE_DESCRIPTION("ChromeOS EC core driver");
> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
> index d45cd254ed1c..7a97cd313c68 100644
> --- a/drivers/platform/chrome/cros_ec_dev.c
> +++ b/drivers/platform/chrome/cros_ec_dev.c
> @@ -87,6 +87,29 @@ exit:
> return ret;
> }
>
> +static int cros_ec_has_cmd_usb_pd_ports(struct cros_ec_dev *ec)
> +{
> + struct cros_ec_command *msg;
> + int ret;
> +
> + msg = kmalloc(sizeof(*msg) + sizeof(struct ec_response_usb_pd_ports),
> + GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> + msg->version = 0;
> + msg->command = EC_CMD_USB_PD_PORTS + ec->cmd_offset;
> + msg->insize = sizeof(struct ec_response_usb_pd_ports);
> + msg->outsize = 0;
> +
> + ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> + ret = ret >= 0 && msg->result == EC_RES_SUCCESS;
> +
> + kfree(msg);
> +
> + return ret;
> +}
> +
> /* Device file ops */
> static int ec_device_open(struct inode *inode, struct file *filp)
> {
> @@ -269,8 +292,19 @@ static int ec_device_probe(struct platform_device *pdev)
> goto dev_reg_failed;
> }
>
> + /* check whether this EC instance has the PD charge manager */
> + if (cros_ec_has_cmd_usb_pd_ports(ec)) {
> + retval = cros_ec_usb_pd_charger_register(dev);
> + if (retval) {
> + dev_err(dev, "failed to add usb-pd-charger device\n");
> + goto pd_reg_failed;
> + }
> + }
> +
> return 0;
>
> +pd_reg_failed:
> + put_device(&ec->class_dev);
> dev_reg_failed:
> set_named_failed:
> dev_set_drvdata(dev, NULL);
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index c21583411ba0..543191f493a9 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -329,4 +329,6 @@ extern struct attribute_group cros_ec_vbc_attr_group;
> */
> uint32_t cros_ec_get_host_event(struct cros_ec_device *ec_dev);
>
> +int cros_ec_usb_pd_charger_register(struct device *dev);
> +
> #endif /* __LINUX_MFD_CROS_EC_H */
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 4/6] power: cros_usbpd-charger: Add EC-based USB PD charger driver
2016-02-05 13:32 ` [PATCH v1 4/6] power: cros_usbpd-charger: Add EC-based USB PD charger driver Tomeu Vizoso
@ 2016-02-10 16:49 ` Lee Jones
2016-02-12 11:07 ` Tomeu Vizoso
0 siblings, 1 reply; 23+ messages in thread
From: Lee Jones @ 2016-02-10 16:49 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: linux-kernel, Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Shawn Nematbakhsh, linux-pm, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Olof Johansson
On Fri, 05 Feb 2016, Tomeu Vizoso wrote:
> From: Sameer Nanda <snanda@chromium.org>
>
> This driver exposes the charger functionality in the PD EC to userspace.
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Sameer Nanda <snanda@chromium.org>
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Shawn Nematbakhsh <shawnn@chromium.org>
> ---
>
> drivers/platform/chrome/cros_ec_proto.c | 37 ++
> drivers/power/Kconfig | 10 +
> drivers/power/Makefile | 1 +
> drivers/power/cros_usbpd-charger.c | 907 ++++++++++++++++++++++++++++++++
> include/linux/mfd/cros_ec.h | 28 +
> include/linux/mfd/cros_ec_commands.h | 324 +++++++++++-
> 6 files changed, 1302 insertions(+), 5 deletions(-)
> create mode 100644 drivers/power/cros_usbpd-charger.c
A 1300 line patch crossing 3 subsystems is not the way you should be
submitting code. Please split these up into separate commits. The
only time you should squash all this functionality into a single patch
is to mitigate otherwise unavoidable build issues. I do not believe
that's the case here.
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index 6e138e2333f3..083a32461f2d 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -482,3 +482,40 @@ int cros_ec_get_next_event(struct cros_ec_device *ec_dev)
> return get_keyboard_state_event(ec_dev);
> }
> EXPORT_SYMBOL(cros_ec_get_next_event);
> +
> +int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
> + struct cros_ec_command *msg)
> +{
> + int ret = cros_ec_cmd_xfer(ec_dev, msg);
> +
> + if (ret < 0)
> + dev_err(ec_dev->dev, "Command xfer error (err:%d)\n", ret);
> + else if (msg->result)
> + return -EECRESULT - msg->result;
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(cros_ec_cmd_xfer_status);
> +
> +u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev)
> +{
> + u32 host_event;
> +
> + if (!ec_dev->mkbp_event_supported) {
> + dev_warn(ec_dev->dev,
> + "This EC does not support EC_MKBP_EVENT_HOST_EVENT");
> + return -EPROTONOSUPPORT;
> + }
> +
> + if (ec_dev->event_data.event_type != EC_MKBP_EVENT_HOST_EVENT)
> + return 0;
> +
> + if (ec_dev->event_size != sizeof(host_event)) {
> + dev_warn(ec_dev->dev, "Invalid host event size\n");
> + return 0;
> + }
> +
> + host_event = get_unaligned_le32(&ec_dev->event_data.data.host_event);
> + return host_event;
> +}
> +EXPORT_SYMBOL(cros_ec_get_host_event);
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 1ddd13cc0c07..26355850d91b 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -502,6 +502,16 @@ config AXP20X_POWER
> This driver provides support for the power supply features of
> AXP20x PMIC.
>
> +config CHARGER_CROS_USB_PD
> + tristate "Chrome OS EC based USB PD charger"
> + depends on MFD_CROS_EC
> + default y
> + help
> + Say Y here to enable Chrome OS EC based USB PD charger driver. This
> + driver gets various bits of information about what is connected to
> + USB PD ports from the EC and converts that into power_supply
> + properties.
> +
> endif # POWER_SUPPLY
>
> source "drivers/power/reset/Kconfig"
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index 0e4eab55f8d7..b83685bfae61 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -73,3 +73,4 @@ obj-$(CONFIG_CHARGER_TPS65217) += tps65217_charger.o
> obj-$(CONFIG_POWER_RESET) += reset/
> obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
> obj-$(CONFIG_AXP288_CHARGER) += axp288_charger.o
> +obj-$(CONFIG_CHARGER_CROS_USB_PD) += cros_usbpd-charger.o
> diff --git a/drivers/power/cros_usbpd-charger.c b/drivers/power/cros_usbpd-charger.c
> new file mode 100644
> index 000000000000..72c427554b56
> --- /dev/null
> +++ b/drivers/power/cros_usbpd-charger.c
> @@ -0,0 +1,907 @@
> +/*
> + * Power supply driver for ChromeOS EC based USB PD Charger.
> + *
> + * Copyright (c) 2014 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/fs.h>
> +#include <linux/ktime.h>
> +#include <linux/module.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/rtc.h>
> +#include <linux/slab.h>
> +
> +#define CROS_USB_PD_MAX_PORTS 8
> +#define CROS_USB_PD_MAX_LOG_ENTRIES 30
> +
> +#define CROS_USB_PD_LOG_UPDATE_DELAY msecs_to_jiffies(60000)
> +#define CROS_USB_PD_CACHE_UPDATE_DELAY msecs_to_jiffies(500)
> +
> +/* Buffer + macro for building PDLOG string */
> +#define BUF_SIZE 80
> +#define APPEND_STRING(buf, len, str, ...) ((len) += \
> + snprintf((buf) + (len), max(BUF_SIZE - (len), 0), (str), ##__VA_ARGS__))
> +
> +#define CHARGER_DIR_NAME "CROS_USB_PD_CHARGER%d"
> +#define CHARGER_DIR_NAME_LENGTH sizeof(CHARGER_DIR_NAME)
> +
> +#define MANUFACTURER_MODEL_LENGTH 32
> +
> +struct port_data {
> + int port_number;
> + char name[CHARGER_DIR_NAME_LENGTH];
> + char manufacturer[MANUFACTURER_MODEL_LENGTH];
> + char model_name[MANUFACTURER_MODEL_LENGTH];
> + struct power_supply *psy;
> + struct power_supply_desc psy_desc;
> + int psy_type;
> + int psy_online;
> + int psy_status;
> + int psy_current_max;
> + int psy_voltage_max_design;
> + int psy_voltage_now;
> + int psy_power_max;
> + struct charger_data *charger;
> + unsigned long last_update;
> +};
> +
> +struct charger_data {
> + struct device *dev;
> + struct cros_ec_dev *ec_dev;
> + struct cros_ec_device *ec_device;
> + int num_charger_ports;
> + int num_registered_psy;
> + struct port_data *ports[CROS_USB_PD_MAX_PORTS];
> + struct delayed_work log_work;
> + struct workqueue_struct *log_workqueue;
> + struct notifier_block notifier;
> + bool suspended;
> +};
> +
> +#define EC_MAX_IN_SIZE EC_PROTO2_MAX_REQUEST_SIZE
> +#define EC_MAX_OUT_SIZE EC_PROTO2_MAX_RESPONSE_SIZE
> +uint8_t ec_inbuf[EC_MAX_IN_SIZE];
> +uint8_t ec_outbuf[EC_MAX_OUT_SIZE];
> +
> +static enum power_supply_property cros_usb_pd_charger_props[] = {
> + POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_CURRENT_MAX,
> + POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
> + POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX,
> + POWER_SUPPLY_PROP_MODEL_NAME,
> + POWER_SUPPLY_PROP_MANUFACTURER,
> +};
> +
> +/* Current external voltage/current limit in mV/mA. Default to none. */
> +static uint16_t ext_voltage_lim = EC_POWER_LIMIT_NONE;
> +static uint16_t ext_current_lim = EC_POWER_LIMIT_NONE;
> +
> +static int ec_command(struct cros_ec_dev *ec_dev, int version, int command,
> + uint8_t *outdata, int outsize, uint8_t *indata,
> + int insize)
> +{
> + struct cros_ec_device *ec_device = ec_dev->ec_dev;
> + struct cros_ec_command *msg;
> + int ret;
> +
> + msg = kmalloc(sizeof(*msg) + max(outsize, insize), GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> + msg->version = version;
> + msg->command = ec_dev->cmd_offset + command;
> + msg->outsize = outsize;
> + msg->insize = insize;
> +
> + memcpy(msg->data, outdata, outsize);
> + ret = cros_ec_cmd_xfer_status(ec_device, msg);
> + memcpy(indata, msg->data, insize);
> +
> + kfree(msg);
> +
> + return ret;
> +}
> +
> +static int set_ec_usb_pd_override_ports(struct charger_data *charger,
> + int port_num)
> +{
> + struct device *dev = charger->dev;
> + struct ec_params_charge_port_override req;
> + int ret;
> +
> + req.override_port = port_num;
> +
> + ret = ec_command(charger->ec_dev, 0, EC_CMD_PD_CHARGE_PORT_OVERRIDE,
> + (uint8_t *)&req, sizeof(req),
> + NULL, 0);
> + if (ret < 0) {
> + dev_info(dev, "Port Override command returned 0x%x\n", ret);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int get_ec_num_ports(struct charger_data *charger, int *num_ports)
> +{
> + struct device *dev = charger->dev;
> + struct ec_response_usb_pd_ports *resp =
> + (struct ec_response_usb_pd_ports *)ec_inbuf;
> + int ret;
> +
> + *num_ports = 0;
> + ret = ec_command(charger->ec_dev, 0, EC_CMD_USB_PD_PORTS,
> + NULL, 0, ec_inbuf, EC_MAX_IN_SIZE);
> + if (ret < 0) {
> + dev_err(dev, "Unable to query PD ports (err:0x%x)\n", ret);
> + return ret;
> + }
> + *num_ports = resp->num_ports;
> + dev_dbg(dev, "Num ports = %d\n", *num_ports);
> +
> + return 0;
> +}
> +
> +static int get_ec_usb_pd_discovery_info(struct port_data *port)
> +{
> + struct charger_data *charger = port->charger;
> + struct device *dev = charger->dev;
> + struct ec_params_usb_pd_info_request req;
> + struct ec_params_usb_pd_discovery_entry resp;
> + int ret;
> +
> + req.port = port->port_number;
> +
> + ret = ec_command(charger->ec_dev, 0, EC_CMD_USB_PD_DISCOVERY,
> + (uint8_t *)&req, sizeof(req),
> + (uint8_t *)&resp, sizeof(resp));
> + if (ret < 0) {
> + dev_err(dev, "Unable to query Discovery info (err:0x%x)\n",
> + ret);
> + return -EINVAL;
> + }
> +
> + dev_dbg(dev, "Port %d: VID = 0x%x, PID=0x%x, PTYPE=0x%x\n",
> + port->port_number, resp.vid, resp.pid, resp.ptype);
> +
> + snprintf(port->manufacturer, MANUFACTURER_MODEL_LENGTH, "%x", resp.vid);
> + snprintf(port->model_name, MANUFACTURER_MODEL_LENGTH, "%x", resp.pid);
> +
> + return 0;
> +}
> +
> +static int get_ec_usb_pd_power_info(struct port_data *port)
> +{
> + struct charger_data *charger = port->charger;
> + struct device *dev = charger->dev;
> + struct ec_params_usb_pd_power_info req;
> + struct ec_response_usb_pd_power_info resp;
> + char role_str[80];
> + int ret, last_psy_status, last_psy_type;
> +
> + req.port = port->port_number;
> + ret = ec_command(charger->ec_dev, 0, EC_CMD_USB_PD_POWER_INFO,
> + (uint8_t *)&req, sizeof(req),
> + (uint8_t *)&resp, sizeof(resp));
> + if (ret < 0) {
> + dev_err(dev, "Unable to query PD power info (err:0x%x)\n", ret);
> + return -EINVAL;
> + }
> +
> + last_psy_status = port->psy_status;
> + last_psy_type = port->psy_type;
> +
> + switch (resp.role) {
> + case USB_PD_PORT_POWER_DISCONNECTED:
> + snprintf(role_str, sizeof(role_str), "%s", "DISCONNECTED");
> + port->psy_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + port->psy_online = 0;
> + break;
> + case USB_PD_PORT_POWER_SOURCE:
> + snprintf(role_str, sizeof(role_str), "%s", "SOURCE");
> + port->psy_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + port->psy_online = 0;
> + break;
> + case USB_PD_PORT_POWER_SINK:
> + snprintf(role_str, sizeof(role_str), "%s", "SINK");
> + port->psy_status = POWER_SUPPLY_STATUS_CHARGING;
> + port->psy_online = 1;
> + break;
> + case USB_PD_PORT_POWER_SINK_NOT_CHARGING:
> + snprintf(role_str, sizeof(role_str), "%s", "NOT CHARGING");
> + port->psy_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + port->psy_online = 1;
> + break;
> + default:
> + snprintf(role_str, sizeof(role_str), "%s", "UNKNOWN");
> + dev_err(dev, "Unknown role %d\n", resp.role);
> + break;
> + }
> +
> + port->psy_voltage_max_design = resp.meas.voltage_max;
> + port->psy_voltage_now = resp.meas.voltage_now;
> + port->psy_current_max = resp.meas.current_max;
> + port->psy_power_max = resp.max_power;
> +
> + switch (resp.type) {
> + case USB_CHG_TYPE_BC12_SDP:
> + case USB_CHG_TYPE_VBUS:
> + port->psy_type = POWER_SUPPLY_TYPE_USB;
> + break;
> + case USB_CHG_TYPE_NONE:
> + /*
> + * For dual-role devices when we are a source, the firmware
> + * reports the type as NONE. Report such chargers as type
> + * USB_PD_DRP.
> + */
> + if (resp.role == USB_PD_PORT_POWER_SOURCE && resp.dualrole)
> + port->psy_type = POWER_SUPPLY_TYPE_USB_PD_DRP;
> + else
> + port->psy_type = POWER_SUPPLY_TYPE_USB;
> + break;
> + case USB_CHG_TYPE_PROPRIETARY:
> + port->psy_type = POWER_SUPPLY_TYPE_MAINS;
> + break;
> + case USB_CHG_TYPE_C:
> + port->psy_type = POWER_SUPPLY_TYPE_USB_TYPE_C;
> + break;
> + case USB_CHG_TYPE_BC12_DCP:
> + port->psy_type = POWER_SUPPLY_TYPE_USB_DCP;
> + break;
> + case USB_CHG_TYPE_BC12_CDP:
> + port->psy_type = POWER_SUPPLY_TYPE_USB_CDP;
> + break;
> + case USB_CHG_TYPE_PD:
> + if (resp.dualrole)
> + port->psy_type = POWER_SUPPLY_TYPE_USB_PD_DRP;
> + else
> + port->psy_type = POWER_SUPPLY_TYPE_USB_PD;
> + break;
> + case USB_CHG_TYPE_UNKNOWN:
> + /*
> + * While the EC is trying to determine the type of charger that
> + * has been plugged in, it will report the charger type as
> + * unknown. Treat this case as a dedicated charger since we
> + * don't know any better just yet. Additionally since the power
> + * capabilities are unknown, report the max current and voltage
> + * as zero.
> + */
> + port->psy_type = POWER_SUPPLY_TYPE_MAINS;
> + port->psy_voltage_max_design = 0;
> + port->psy_current_max = 0;
> + break;
> + default:
> + dev_err(dev, "Port %d: default case!\n",
> + port->port_number);
> + port->psy_type = POWER_SUPPLY_TYPE_USB;
> + }
> +
> + port->psy_desc.type = port->psy_type;
> +
> + dev_dbg(dev,
> + "Port %d: %s type=%d=vmax=%d vnow=%d cmax=%d clim=%d pmax=%d\n",
> + port->port_number, role_str, resp.type,
> + resp.meas.voltage_max, resp.meas.voltage_now,
> + resp.meas.current_max, resp.meas.current_lim,
> + resp.max_power);
> +
> + /*
> + * If power supply type or status changed, explicitly call
> + * power_supply_changed. This results in udev event getting generated
> + * and allows user mode apps to react quicker instead of waiting for
> + * their next poll of power supply status.
> + */
> + if (last_psy_type != port->psy_type ||
> + last_psy_status != port->psy_status) {
> + dev_dbg(dev, "Port %d: Calling power_supply_changed\n",
> + port->port_number);
> + power_supply_changed(port->psy);
> + }
> +
> + return 0;
> +}
> +
> +static int get_ec_port_status(struct port_data *port, bool ratelimit)
> +{
> + int ret;
> +
> + if (ratelimit &&
> + time_is_after_jiffies(port->last_update +
> + CROS_USB_PD_CACHE_UPDATE_DELAY))
> + return 0;
> +
> + ret = get_ec_usb_pd_power_info(port);
> + if (ret < 0)
> + return ret;
> +
> + ret = get_ec_usb_pd_discovery_info(port);
> + port->last_update = jiffies;
> +
> + return ret;
> +}
> +
> +static void cros_usb_pd_charger_power_changed(struct power_supply *psy)
> +{
> + struct port_data *port = power_supply_get_drvdata(psy);
> + struct charger_data *charger = port->charger;
> + struct device *dev = charger->dev;
> + int i;
> +
> + dev_dbg(dev, "cros_usb_pd_charger_power_changed\n");
> + for (i = 0; i < charger->num_registered_psy; i++)
> + get_ec_port_status(charger->ports[i], false);
> +}
> +
> +static int cros_usb_pd_charger_get_prop(struct power_supply *psy,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + struct port_data *port = power_supply_get_drvdata(psy);
> + struct charger_data *charger = port->charger;
> + struct device *dev = charger->dev;
> + int ret;
> +
> +
> + /* Only refresh ec_port_status for dynamic properties */
> + switch (psp) {
> + case POWER_SUPPLY_PROP_CURRENT_MAX:
> + case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> + case POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX:
> + ret = get_ec_port_status(port, true);
> + if (ret < 0) {
> + dev_err(dev, "Failed to get port status (err:0x%x)\n",
> + ret);
> + return -EINVAL;
> + }
> + break;
> + default:
> + break;
> + }
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_ONLINE:
> + val->intval = port->psy_online;
> + break;
> + case POWER_SUPPLY_PROP_STATUS:
> + val->intval = port->psy_status;
> + break;
> + case POWER_SUPPLY_PROP_CURRENT_MAX:
> + val->intval = port->psy_current_max * 1000;
> + break;
> + case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
> + val->intval = port->psy_voltage_max_design * 1000;
> + break;
> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> + val->intval = port->psy_voltage_now * 1000;
> + break;
> + case POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX:
> + /* TODO: send a TBD host command to the EC. */
> + val->intval = 0;
> + break;
> + case POWER_SUPPLY_PROP_MODEL_NAME:
> + val->strval = port->model_name;
> + break;
> + case POWER_SUPPLY_PROP_MANUFACTURER:
> + val->strval = port->manufacturer;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int cros_usb_pd_charger_set_prop(struct power_supply *psy,
> + enum power_supply_property psp,
> + const union power_supply_propval *val)
> +{
> + struct port_data *port = power_supply_get_drvdata(psy);
> + struct charger_data *charger = port->charger;
> + int port_number;
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX:
> + /*
> + * A value of -1 implies switching to battery as the power
> + * source. Any other value implies using this port as the
> + * power source.
> + */
> + port_number = val->intval;
> + if (port_number != -1)
> + port_number = port->port_number;
> + return set_ec_usb_pd_override_ports(charger, port_number);
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int cros_usb_pd_charger_is_writeable(struct power_supply *psy,
> + enum power_supply_property psp)
> +{
> + int ret;
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX:
> + ret = 1;
> + break;
> + default:
> + ret = 0;
> + }
> +
> + return ret;
> +}
> +
> +static void cros_usb_pd_print_log_entry(struct ec_response_pd_log *r,
> + ktime_t tstamp)
> +{
> + static const char * const fault_names[] = {
> + "---", "OCP", "fast OCP", "OVP", "Discharge"
> + };
> + static const char * const role_names[] = {
> + "Disconnected", "SRC", "SNK", "SNK (not charging)"
> + };
> + static const char * const chg_type_names[] = {
> + "None", "PD", "Type-C", "Proprietary",
> + "DCP", "CDP", "SDP", "Other", "VBUS"
> + };
> + int i;
> + int role_idx, type_idx;
> + const char *fault, *role, *chg_type;
> + struct usb_chg_measures *meas;
> + struct mcdp_info *minfo;
> + struct rtc_time rt;
> + int len = 0;
> + char buf[BUF_SIZE + 1];
> +
> + /* the timestamp is the number of 1024th of seconds in the past */
> + tstamp = ktime_sub_us(tstamp,
> + (uint64_t)r->timestamp << PD_LOG_TIMESTAMP_SHIFT);
> + rt = rtc_ktime_to_tm(tstamp);
> +
> + switch (r->type) {
> + case PD_EVENT_MCU_CHARGE:
> + if (r->data & CHARGE_FLAGS_OVERRIDE)
> + APPEND_STRING(buf, len, "override ");
> + if (r->data & CHARGE_FLAGS_DELAYED_OVERRIDE)
> + APPEND_STRING(buf, len, "pending_override ");
> + role_idx = r->data & CHARGE_FLAGS_ROLE_MASK;
> + role = role_idx < ARRAY_SIZE(role_names) ?
> + role_names[role_idx] : "Unknown";
> + type_idx = (r->data & CHARGE_FLAGS_TYPE_MASK)
> + >> CHARGE_FLAGS_TYPE_SHIFT;
> + chg_type = type_idx < ARRAY_SIZE(chg_type_names) ?
> + chg_type_names[type_idx] : "???";
> +
> + if ((role_idx == USB_PD_PORT_POWER_DISCONNECTED) ||
> + (role_idx == USB_PD_PORT_POWER_SOURCE)) {
> + APPEND_STRING(buf, len, "%s", role);
> + break;
> + }
> +
> + meas = (struct usb_chg_measures *)r->payload;
> + APPEND_STRING(buf, len, "%s %s %s %dmV max %dmV / %dmA", role,
> + r->data & CHARGE_FLAGS_DUAL_ROLE ? "DRP" : "Charger",
> + chg_type,
> + meas->voltage_now,
> + meas->voltage_max,
> + meas->current_max);
> + break;
> + case PD_EVENT_ACC_RW_FAIL:
> + APPEND_STRING(buf, len, "RW signature check failed");
> + break;
> + case PD_EVENT_PS_FAULT:
> + fault = r->data < ARRAY_SIZE(fault_names) ? fault_names[r->data]
> + : "???";
> + APPEND_STRING(buf, len, "Power supply fault: %s", fault);
> + break;
> + case PD_EVENT_VIDEO_DP_MODE:
> + APPEND_STRING(buf, len, "DP mode %sabled",
> + (r->data == 1) ? "en" : "dis");
> + break;
> + case PD_EVENT_VIDEO_CODEC:
> + minfo = (struct mcdp_info *)r->payload;
> + APPEND_STRING(buf, len,
> + "HDMI info: family:%04x chipid:%04x irom:%d.%d.%d fw:%d.%d.%d",
> + MCDP_FAMILY(minfo->family),
> + MCDP_CHIPID(minfo->chipid),
> + minfo->irom.major, minfo->irom.minor,
> + minfo->irom.build, minfo->fw.major,
> + minfo->fw.minor, minfo->fw.build);
> + break;
> + default:
> + APPEND_STRING(buf, len,
> + "Event %02x (%04x) [", r->type, r->data);
> + for (i = 0; i < PD_LOG_SIZE(r->size_port); i++)
> + APPEND_STRING(buf, len, "%02x ", r->payload[i]);
> + APPEND_STRING(buf, len, "]");
> + break;
> + }
> +
> + pr_info("PDLOG %d/%02d/%02d %02d:%02d:%02d.%03d P%d %s\n",
> + rt.tm_year + 1900, rt.tm_mon + 1, rt.tm_mday,
> + rt.tm_hour, rt.tm_min, rt.tm_sec,
> + (int)(ktime_to_ms(tstamp) % MSEC_PER_SEC),
> + PD_LOG_PORT(r->size_port), buf);
> +}
> +
> +static void cros_usb_pd_log_check(struct work_struct *work)
> +{
> + struct charger_data *charger = container_of(to_delayed_work(work),
> + struct charger_data, log_work);
> + struct device *dev = charger->dev;
> + union {
> + struct ec_response_pd_log r;
> + uint32_t words[8]; /* space for the payload */
> + } u;
> + int ret;
> + int entries = 0;
> + ktime_t now;
> +
> + if (charger->suspended) {
> + dev_dbg(dev, "Suspended...bailing out\n");
> + return;
> + }
> +
> + while (entries++ < CROS_USB_PD_MAX_LOG_ENTRIES) {
> + ret = ec_command(charger->ec_dev, 0, EC_CMD_PD_GET_LOG_ENTRY,
> + NULL, 0, (uint8_t *)&u, sizeof(u));
> + now = ktime_get_real();
> + if (ret < 0) {
> + dev_dbg(dev, "Cannot get PD log %d\n", ret);
> + break;
> + }
> + if (u.r.type == PD_EVENT_NO_ENTRY)
> + break;
> +
> + cros_usb_pd_print_log_entry(&u.r, now);
> + }
> +
> + queue_delayed_work(charger->log_workqueue, &charger->log_work,
> + CROS_USB_PD_LOG_UPDATE_DELAY);
> +}
> +
> +static int cros_usb_pd_ec_event(struct notifier_block *nb,
> + unsigned long queued_during_suspend, void *_notify)
> +{
> + struct charger_data *charger;
> + struct device *dev;
> + struct cros_ec_device *ec_device;
> + u32 host_event;
> +
> + charger = container_of(nb, struct charger_data, notifier);
> + dev = charger->dev;
> + ec_device = charger->ec_device;
> +
> + host_event = cros_ec_get_host_event(ec_device);
> + if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_PD_MCU)) {
> + cros_usb_pd_charger_power_changed(charger->ports[0]->psy);
> + return NOTIFY_OK;
> + } else {
> + return NOTIFY_DONE;
> + }
> +}
> +
> +static char *charger_supplied_to[] = {"cros-usb_pd-charger"};
> +
> +static int cros_usb_pd_charger_probe(struct platform_device *pd)
> +{
> + struct device *dev = &pd->dev;
> + struct cros_ec_dev *ec_dev = dev_get_drvdata(pd->dev.parent);
> + struct cros_ec_device *ec_device;
> + struct charger_data *charger;
> + struct port_data *port;
> + struct power_supply_desc *psy_desc;
> + struct power_supply_config psy_cfg = {};
> + int i;
> + int ret = -EINVAL;
> +
> + dev_dbg(dev, "cros_usb_pd_charger_probe\n");
> + if (!ec_dev) {
> + WARN(1, "%s: No EC dev found\n", dev_name(dev));
> + return -EINVAL;
> + }
> +
> + ec_device = ec_dev->ec_dev;
> + if (!ec_device) {
> + WARN(1, "%s: No EC device found\n", dev_name(dev));
> + return -EINVAL;
> + }
> +
> + charger = devm_kzalloc(dev, sizeof(struct charger_data),
> + GFP_KERNEL);
> + if (!charger)
> + return -ENOMEM;
> +
> + charger->dev = dev;
> + charger->ec_dev = ec_dev;
> + charger->ec_device = ec_device;
> +
> + platform_set_drvdata(pd, charger);
> +
> + if (!charger->num_charger_ports) {
> + /*
> + * This can happen on a system that doesn't support USB PD.
> + * Log a message, but no need to warn.
> + */
> + dev_info(dev, "No charging ports found\n");
> + ret = -ENODEV;
> + goto fail_nowarn;
> + }
> +
> + for (i = 0; i < charger->num_charger_ports; i++) {
> + port = devm_kzalloc(dev, sizeof(struct port_data), GFP_KERNEL);
> + if (!port) {
> + dev_err(dev, "Failed to alloc port structure\n");
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
> + port->charger = charger;
> + port->port_number = i;
> + sprintf(port->name, CHARGER_DIR_NAME, i);
> +
> + psy_desc = &port->psy_desc;
> + psy_desc->name = port->name;
> + psy_desc->type = POWER_SUPPLY_TYPE_USB;
> + psy_desc->get_property = cros_usb_pd_charger_get_prop;
> + psy_desc->set_property = cros_usb_pd_charger_set_prop;
> + psy_desc->property_is_writeable =
> + cros_usb_pd_charger_is_writeable;
> + psy_desc->external_power_changed =
> + cros_usb_pd_charger_power_changed;
> + psy_desc->properties = cros_usb_pd_charger_props;
> + psy_desc->num_properties = ARRAY_SIZE(
> + cros_usb_pd_charger_props);
> +
> + psy_cfg.drv_data = port;
> + psy_cfg.supplied_to = charger_supplied_to;
> + psy_cfg.num_supplicants = ARRAY_SIZE(charger_supplied_to);
> +
> + port->psy = power_supply_register_no_ws(dev, psy_desc,
> + &psy_cfg);
> + if (IS_ERR(port->psy)) {
> + dev_err(dev, "Failed to register power supply: %ld\n",
> + PTR_ERR(port->psy));
> + continue;
> + }
> +
> + charger->ports[charger->num_registered_psy++] = port;
> + }
> +
> + if (!charger->num_registered_psy) {
> + ret = -ENODEV;
> + dev_err(dev, "No power supplies registered\n");
> + goto fail;
> + }
> +
> + if (ec_device->mkbp_event_supported) {
> + /* Get PD events from the EC */
> + charger->notifier.notifier_call = cros_usb_pd_ec_event;
> + ret = blocking_notifier_chain_register(
> + &ec_device->event_notifier, &charger->notifier);
> + if (ret < 0)
> + dev_warn(dev, "failed to register notifier\n");
> + }
> +
> + /* Retrieve PD event logs periodically */
> + INIT_DELAYED_WORK(&charger->log_work, cros_usb_pd_log_check);
> + charger->log_workqueue =
> + create_singlethread_workqueue("cros_usb_pd_log");
> + queue_delayed_work(charger->log_workqueue, &charger->log_work,
> + CROS_USB_PD_LOG_UPDATE_DELAY);
> +
> + return 0;
> +
> +fail:
> + WARN(1, "%s: Failing probe (err:0x%x)\n", dev_name(dev), ret);
> +
> +fail_nowarn:
> + if (charger) {
> + for (i = 0; i < charger->num_registered_psy; i++) {
> + port = charger->ports[i];
> + power_supply_unregister(port->psy);
> + devm_kfree(dev, port);
> + }
> + platform_set_drvdata(pd, NULL);
> + devm_kfree(dev, charger);
> + }
> +
> + dev_info(dev, "Failing probe (err:0x%x)\n", ret);
> + return ret;
> +}
> +
> +static int cros_usb_pd_charger_remove(struct platform_device *pd)
> +{
> + struct charger_data *charger = platform_get_drvdata(pd);
> + struct device *dev = charger->dev;
> + struct port_data *port;
> + int i;
> +
> + if (charger) {
> + for (i = 0; i < charger->num_registered_psy; i++) {
> + port = charger->ports[i];
> + power_supply_unregister(port->psy);
> + devm_kfree(dev, port);
> + }
> + flush_delayed_work(&charger->log_work);
> + platform_set_drvdata(pd, NULL);
> + devm_kfree(dev, charger);
> + }
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int cros_usb_pd_charger_resume(struct device *dev)
> +{
> + struct charger_data *charger = dev_get_drvdata(dev);
> + int i;
> +
> + if (!charger)
> + return 0;
> +
> + charger->suspended = false;
> +
> + dev_dbg(dev, "cros_usb_pd_charger_resume: updating power supplies\n");
> + for (i = 0; i < charger->num_registered_psy; i++) {
> + power_supply_changed(charger->ports[i]->psy);
> + charger->ports[i]->last_update =
> + jiffies - CROS_USB_PD_CACHE_UPDATE_DELAY;
> + }
> + queue_delayed_work(charger->log_workqueue, &charger->log_work,
> + CROS_USB_PD_LOG_UPDATE_DELAY);
> +
> + return 0;
> +}
> +
> +static int cros_usb_pd_charger_suspend(struct device *dev)
> +{
> + struct charger_data *charger = dev_get_drvdata(dev);
> +
> + charger->suspended = true;
> +
> + if (charger)
> + flush_delayed_work(&charger->log_work);
> + return 0;
> +}
> +#endif
> +
> +static int set_ec_ext_power_limit(struct cros_ec_dev *ec,
> + uint16_t current_lim, uint16_t voltage_lim)
> +{
> + struct ec_params_external_power_limit_v1 req;
> + int ret;
> +
> + req.current_lim = current_lim;
> + req.voltage_lim = voltage_lim;
> +
> + ret = ec_command(ec, 0, EC_CMD_EXT_POWER_CURRENT_LIMIT,
> + (uint8_t *)&req, sizeof(req), NULL, 0);
> + if (ret < 0) {
> + dev_warn(ec->dev,
> + "External power limit command returned 0x%x\n",
> + ret);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static ssize_t get_ec_ext_current_lim(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return scnprintf(buf, PAGE_SIZE, "%d\n", ext_current_lim);
> +}
> +
> +static ssize_t set_ec_ext_current_lim(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int ret;
> + uint16_t tmp_val;
> + struct cros_ec_dev *ec = container_of(
> + dev, struct cros_ec_dev, class_dev);
> +
> + if (kstrtou16(buf, 0, &tmp_val) < 0)
> + return -EINVAL;
> +
> + ret = set_ec_ext_power_limit(ec, tmp_val, ext_voltage_lim);
> + if (ret < 0)
> + return ret;
> +
> + ext_current_lim = tmp_val;
> + if (ext_current_lim == EC_POWER_LIMIT_NONE)
> + dev_info(ec->dev, "External current limit removed\n");
> + else
> + dev_info(ec->dev, "External current limit set to %dmA\n",
> + ext_current_lim);
> +
> + return count;
> +}
> +
> +static ssize_t get_ec_ext_voltage_lim(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return scnprintf(buf, PAGE_SIZE, "%d\n", ext_voltage_lim);
> +}
> +
> +static ssize_t set_ec_ext_voltage_lim(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int ret;
> + uint16_t tmp_val;
> +
> + struct cros_ec_dev *ec = container_of(
> + dev, struct cros_ec_dev, class_dev);
> +
> + if (kstrtou16(buf, 0, &tmp_val) < 0)
> + return -EINVAL;
> +
> + ret = set_ec_ext_power_limit(ec, ext_current_lim, tmp_val);
> + if (ret < 0)
> + return ret;
> +
> + ext_voltage_lim = tmp_val;
> + if (ext_voltage_lim == EC_POWER_LIMIT_NONE)
> + dev_info(ec->dev, "External voltage limit removed\n");
> + else
> + dev_info(ec->dev, "External voltage limit set to %dmV\n",
> + ext_voltage_lim);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(ext_current_lim, S_IWUSR | S_IWGRP | S_IRUGO,
> + get_ec_ext_current_lim,
> + set_ec_ext_current_lim);
> +static DEVICE_ATTR(ext_voltage_lim, S_IWUSR | S_IWGRP | S_IRUGO,
> + get_ec_ext_voltage_lim,
> + set_ec_ext_voltage_lim);
> +
> +static struct attribute *__ext_power_cmds_attrs[] = {
> + &dev_attr_ext_current_lim.attr,
> + &dev_attr_ext_voltage_lim.attr,
> + NULL,
> +};
> +
> +struct attribute_group cros_usb_pd_charger_attr_group = {
> + .name = "usb-pd-charger",
> + .attrs = __ext_power_cmds_attrs,
> +};
> +
> +static SIMPLE_DEV_PM_OPS(cros_usb_pd_charger_pm_ops,
> + cros_usb_pd_charger_suspend, cros_usb_pd_charger_resume);
> +
> +static struct platform_driver cros_usb_pd_charger_driver = {
> + .driver = {
> + .name = "cros-usb-pd-charger",
> + .owner = THIS_MODULE,
> + .pm = &cros_usb_pd_charger_pm_ops,
> + },
> + .probe = cros_usb_pd_charger_probe,
> + .remove = cros_usb_pd_charger_remove,
> +};
> +
> +module_platform_driver(cros_usb_pd_charger_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Chrome USB PD charger");
> +MODULE_ALIAS("power_supply:cros-usb-pd-charger");
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 8a8fa75178c6..c21583411ba0 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -40,6 +40,9 @@
> #define EC_MAX_REQUEST_OVERHEAD 1
> #define EC_MAX_RESPONSE_OVERHEAD 2
>
> +/* ec_command return value for non-success result from EC */
> +#define EECRESULT 1000
> +
> /*
> * Command interface between EC and AP, for LPC, I2C and SPI interfaces.
> */
> @@ -250,6 +253,21 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
> struct cros_ec_command *msg);
>
> /**
> + * cros_ec_cmd_xfer_status - Send a command to the ChromeOS EC
> + *
> + * This function is identical to cros_ec_cmd_xfer, except it returns succes
> + * status only if both the command was transmitted successfully and the EC
> + * replied with success status. It's not necessary to check msg->result when
> + * using this function.
> + *
> + * @ec_dev: EC device
> + * @msg: Message to write
> + * @return: Num. of bytes transferred on success, <0 on failure
> + */
> +int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
> + struct cros_ec_command *msg);
> +
> +/**
> * cros_ec_remove - Remove a ChromeOS EC
> *
> * Call this to deregister a ChromeOS EC, then clean up any private data.
> @@ -286,6 +304,16 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev);
> */
> int cros_ec_get_next_event(struct cros_ec_device *ec_dev);
>
> +/**
> + * cros_ec_get_host_event - Return a mask of event set by the EC.
> + *
> + * Returns the mask of the event that caused the last interrupt to be raised.
> + *
> + * @ec_dev: Device to fetch event from
> + * @return event mask, 0 on error
> + */
> +uint32_t cros_ec_get_host_event(struct cros_ec_device *ec_dev);
> +
> /* sysfs stuff */
> extern struct attribute_group cros_ec_attr_group;
> extern struct attribute_group cros_ec_lightbar_attr_group;
> diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
> index d86526f0bd8e..4f056d2747ff 100644
> --- a/include/linux/mfd/cros_ec_commands.h
> +++ b/include/linux/mfd/cros_ec_commands.h
> @@ -286,6 +286,9 @@ enum host_event_code {
> /* Hang detect logic detected a hang and warm rebooted the AP */
> EC_HOST_EVENT_HANG_REBOOT = 21,
>
> + /* PD MCU triggering host event */
> + EC_HOST_EVENT_PD_MCU = 22,
> +
> /*
> * The high bit of the event mask is not used as a host event code. If
> * it reads back as set, then the entire event mask should be
> @@ -2273,6 +2276,13 @@ struct ec_params_ext_power_current_limit {
> uint32_t limit; /* in mA */
> } __packed;
>
> +struct ec_params_external_power_limit_v1 {
> + uint16_t current_lim; /* in mA, or EC_POWER_LIMIT_NONE to clear limit */
> + uint16_t voltage_lim; /* in mV, or EC_POWER_LIMIT_NONE to clear limit */
> +} __packed;
> +
> +#define EC_POWER_LIMIT_NONE 0xffff
> +
> /*****************************************************************************/
> /* Smart battery pass-through */
>
> @@ -2527,8 +2537,6 @@ struct ec_params_reboot_ec {
> */
> #define EC_CMD_VERSION0 0xdc
>
> -#endif /* !__ACPI__ */
> -
> /*****************************************************************************/
> /*
> * PD commands
> @@ -2538,16 +2546,51 @@ struct ec_params_reboot_ec {
>
> /* EC to PD MCU exchange status command */
> #define EC_CMD_PD_EXCHANGE_STATUS 0x100
> +#define EC_VER_PD_EXCHANGE_STATUS 2
> +
> +enum pd_charge_state {
> + PD_CHARGE_NO_CHANGE = 0, /* Don't change charge state */
> + PD_CHARGE_NONE, /* No charging allowed */
> + PD_CHARGE_5V, /* 5V charging only */
> + PD_CHARGE_MAX /* Charge at max voltage */
> +};
>
> /* Status of EC being sent to PD */
> +#define EC_STATUS_HIBERNATING (1 << 0)
> +
> struct ec_params_pd_status {
> - int8_t batt_soc; /* battery state of charge */
> + uint8_t status; /* EC status */
> + int8_t batt_soc; /* battery state of charge */
> + uint8_t charge_state; /* charging state (from enum pd_charge_state) */
> } __packed;
>
> /* Status of PD being sent back to EC */
> +#define PD_STATUS_HOST_EVENT (1 << 0) /* Forward host event to AP */
> +#define PD_STATUS_IN_RW (1 << 1) /* Running RW image */
> +#define PD_STATUS_JUMPED_TO_IMAGE (1 << 2) /* Current image was jumped to */
> +#define PD_STATUS_TCPC_ALERT_0 (1 << 3) /* Alert active in port 0 TCPC */
> +#define PD_STATUS_TCPC_ALERT_1 (1 << 4) /* Alert active in port 1 TCPC */
> +#define PD_STATUS_TCPC_ALERT_2 (1 << 5) /* Alert active in port 2 TCPC */
> +#define PD_STATUS_TCPC_ALERT_3 (1 << 6) /* Alert active in port 3 TCPC */
> +#define PD_STATUS_EC_INT_ACTIVE (PD_STATUS_TCPC_ALERT_0 | \
> + PD_STATUS_TCPC_ALERT_1 | \
> + PD_STATUS_HOST_EVENT)
> struct ec_response_pd_status {
> - int8_t status; /* PD MCU status */
> - uint32_t curr_lim_ma; /* input current limit */
> + uint32_t curr_lim_ma; /* input current limit */
> + uint16_t status; /* PD MCU status */
> + int8_t active_charge_port; /* active charging port */
> +} __packed;
> +
> +/* AP to PD MCU host event status command, cleared on read */
> +#define EC_CMD_PD_HOST_EVENT_STATUS 0x104
> +
> +/* PD MCU host event status bits */
> +#define PD_EVENT_UPDATE_DEVICE (1 << 0)
> +#define PD_EVENT_POWER_CHANGE (1 << 1)
> +#define PD_EVENT_IDENTITY_RECEIVED (1 << 2)
> +#define PD_EVENT_DATA_SWAP (1 << 3)
> +struct ec_response_host_event_status {
> + uint32_t status; /* PD MCU host event status */
> } __packed;
>
> /* Set USB type-C port role and muxes */
> @@ -2559,6 +2602,7 @@ enum usb_pd_control_role {
> USB_PD_CTRL_ROLE_TOGGLE_OFF = 2,
> USB_PD_CTRL_ROLE_FORCE_SINK = 3,
> USB_PD_CTRL_ROLE_FORCE_SOURCE = 4,
> + USB_PD_CTRL_ROLE_COUNT
> };
>
> enum usb_pd_control_mux {
> @@ -2568,14 +2612,284 @@ enum usb_pd_control_mux {
> USB_PD_CTRL_MUX_DP = 3,
> USB_PD_CTRL_MUX_DOCK = 4,
> USB_PD_CTRL_MUX_AUTO = 5,
> + USB_PD_CTRL_MUX_COUNT
> +};
> +
> +enum usb_pd_control_swap {
> + USB_PD_CTRL_SWAP_NONE = 0,
> + USB_PD_CTRL_SWAP_DATA = 1,
> + USB_PD_CTRL_SWAP_POWER = 2,
> + USB_PD_CTRL_SWAP_VCONN = 3,
> + USB_PD_CTRL_SWAP_COUNT
> };
>
> struct ec_params_usb_pd_control {
> uint8_t port;
> uint8_t role;
> uint8_t mux;
> + uint8_t swap;
> +} __packed;
> +
> +#define PD_CTRL_RESP_ENABLED_COMMS (1 << 0) /* Communication enabled */
> +#define PD_CTRL_RESP_ENABLED_CONNECTED (1 << 1) /* Device connected */
> +#define PD_CTRL_RESP_ENABLED_PD_CAPABLE (1 << 2) /* Partner is PD capable */
> +
> +#define PD_CTRL_RESP_ROLE_POWER (1 << 0) /* 0=SNK/1=SRC */
> +#define PD_CTRL_RESP_ROLE_DATA (1 << 1) /* 0=UFP/1=DFP */
> +#define PD_CTRL_RESP_ROLE_VCONN (1 << 2) /* Vconn status */
> +#define PD_CTRL_RESP_ROLE_DR_POWER (1 << 3) /* Partner is dualrole power */
> +#define PD_CTRL_RESP_ROLE_DR_DATA (1 << 4) /* Partner is dualrole data */
> +#define PD_CTRL_RESP_ROLE_USB_COMM (1 << 5) /* Partner USB comm capable */
> +#define PD_CTRL_RESP_ROLE_EXT_POWERED (1 << 6) /* Partner externally powerd */
> +
> +struct ec_response_usb_pd_control {
> + uint8_t enabled;
> + uint8_t role;
> + uint8_t polarity;
> + uint8_t state;
> +} __packed;
> +
> +struct ec_response_usb_pd_control_v1 {
> + uint8_t enabled;
> + uint8_t role;
> + uint8_t polarity;
> + char state[32];
> +} __packed;
> +
> +#define EC_CMD_USB_PD_PORTS 0x102
> +
> +struct ec_response_usb_pd_ports {
> + uint8_t num_ports;
> +} __packed;
> +
> +#define EC_CMD_USB_PD_POWER_INFO 0x103
> +
> +#define PD_POWER_CHARGING_PORT 0xff
> +struct ec_params_usb_pd_power_info {
> + uint8_t port;
> +} __packed;
> +
> +enum usb_chg_type {
> + USB_CHG_TYPE_NONE,
> + USB_CHG_TYPE_PD,
> + USB_CHG_TYPE_C,
> + USB_CHG_TYPE_PROPRIETARY,
> + USB_CHG_TYPE_BC12_DCP,
> + USB_CHG_TYPE_BC12_CDP,
> + USB_CHG_TYPE_BC12_SDP,
> + USB_CHG_TYPE_OTHER,
> + USB_CHG_TYPE_VBUS,
> + USB_CHG_TYPE_UNKNOWN,
> +};
> +enum usb_power_roles {
> + USB_PD_PORT_POWER_DISCONNECTED,
> + USB_PD_PORT_POWER_SOURCE,
> + USB_PD_PORT_POWER_SINK,
> + USB_PD_PORT_POWER_SINK_NOT_CHARGING,
> +};
> +
> +struct usb_chg_measures {
> + uint16_t voltage_max;
> + uint16_t voltage_now;
> + uint16_t current_max;
> + uint16_t current_lim;
> +} __packed;
> +
> +struct ec_response_usb_pd_power_info {
> + uint8_t role;
> + uint8_t type;
> + uint8_t dualrole;
> + uint8_t reserved1;
> + struct usb_chg_measures meas;
> + uint32_t max_power;
> +} __packed;
> +
> +/* Write USB-PD device FW */
> +#define EC_CMD_USB_PD_FW_UPDATE 0x110
> +
> +enum usb_pd_fw_update_cmds {
> + USB_PD_FW_REBOOT,
> + USB_PD_FW_FLASH_ERASE,
> + USB_PD_FW_FLASH_WRITE,
> + USB_PD_FW_ERASE_SIG,
> +};
> +
> +struct ec_params_usb_pd_fw_update {
> + uint16_t dev_id;
> + uint8_t cmd;
> + uint8_t port;
> + uint32_t size; /* Size to write in bytes */
> + /* Followed by data to write */
> +} __packed;
> +
> +/* Write USB-PD Accessory RW_HASH table entry */
> +#define EC_CMD_USB_PD_RW_HASH_ENTRY 0x111
> +/* RW hash is first 20 bytes of SHA-256 of RW section */
> +#define PD_RW_HASH_SIZE 20
> +struct ec_params_usb_pd_rw_hash_entry {
> + uint16_t dev_id;
> + uint8_t dev_rw_hash[PD_RW_HASH_SIZE];
> + uint8_t reserved; /* For alignment of current_image */
> + uint32_t current_image; /* One of ec_current_image */
> +} __packed;
> +
> +/* Read USB-PD Accessory info */
> +#define EC_CMD_USB_PD_DEV_INFO 0x112
> +
> +struct ec_params_usb_pd_info_request {
> + uint8_t port;
> +} __packed;
> +
> +/* Read USB-PD Device discovery info */
> +#define EC_CMD_USB_PD_DISCOVERY 0x113
> +struct ec_params_usb_pd_discovery_entry {
> + uint16_t vid; /* USB-IF VID */
> + uint16_t pid; /* USB-IF PID */
> + uint8_t ptype; /* product type (hub,periph,cable,ama) */
> +} __packed;
> +
> +/* Override default charge behavior */
> +#define EC_CMD_PD_CHARGE_PORT_OVERRIDE 0x114
> +
> +/* Negative port parameters have special meaning */
> +enum usb_pd_override_ports {
> + OVERRIDE_DONT_CHARGE = -2,
> + OVERRIDE_OFF = -1,
> + /* [0, CONFIG_USB_PD_PORT_COUNT): Port# */
> +};
> +
> +struct ec_params_charge_port_override {
> + int16_t override_port; /* Override port# */
> +} __packed;
> +
> +/* Read (and delete) one entry of PD event log */
> +#define EC_CMD_PD_GET_LOG_ENTRY 0x115
> +
> +struct ec_response_pd_log {
> + uint32_t timestamp; /* relative timestamp in milliseconds */
> + uint8_t type; /* event type : see PD_EVENT_xx below */
> + uint8_t size_port; /* [7:5] port number [4:0] payload size in bytes */
> + uint16_t data; /* type-defined data payload */
> + uint8_t payload[0]; /* optional additional data payload: 0..16 bytes */
> +} __packed;
> +
> +
> +/* The timestamp is the microsecond counter shifted to get about a ms. */
> +#define PD_LOG_TIMESTAMP_SHIFT 10 /* 1 LSB = 1024us */
> +
> +#define PD_LOG_SIZE_MASK 0x1f
> +#define PD_LOG_PORT_MASK 0xe0
> +#define PD_LOG_PORT_SHIFT 5
> +#define PD_LOG_PORT_SIZE(port, size) (((port) << PD_LOG_PORT_SHIFT) | \
> + ((size) & PD_LOG_SIZE_MASK))
> +#define PD_LOG_PORT(size_port) ((size_port) >> PD_LOG_PORT_SHIFT)
> +#define PD_LOG_SIZE(size_port) ((size_port) & PD_LOG_SIZE_MASK)
> +
> +/* PD event log : entry types */
> +/* PD MCU events */
> +#define PD_EVENT_MCU_BASE 0x00
> +#define PD_EVENT_MCU_CHARGE (PD_EVENT_MCU_BASE+0)
> +#define PD_EVENT_MCU_CONNECT (PD_EVENT_MCU_BASE+1)
> +/* Reserved for custom board event */
> +#define PD_EVENT_MCU_BOARD_CUSTOM (PD_EVENT_MCU_BASE+2)
> +/* PD generic accessory events */
> +#define PD_EVENT_ACC_BASE 0x20
> +#define PD_EVENT_ACC_RW_FAIL (PD_EVENT_ACC_BASE+0)
> +#define PD_EVENT_ACC_RW_ERASE (PD_EVENT_ACC_BASE+1)
> +/* PD power supply events */
> +#define PD_EVENT_PS_BASE 0x40
> +#define PD_EVENT_PS_FAULT (PD_EVENT_PS_BASE+0)
> +/* PD video dongles events */
> +#define PD_EVENT_VIDEO_BASE 0x60
> +#define PD_EVENT_VIDEO_DP_MODE (PD_EVENT_VIDEO_BASE+0)
> +#define PD_EVENT_VIDEO_CODEC (PD_EVENT_VIDEO_BASE+1)
> +/* Returned in the "type" field, when there is no entry available */
> +#define PD_EVENT_NO_ENTRY 0xff
> +
> +/*
> + * PD_EVENT_MCU_CHARGE event definition :
> + * the payload is "struct usb_chg_measures"
> + * the data field contains the port state flags as defined below :
> + */
> +/* Port partner is a dual role device */
> +#define CHARGE_FLAGS_DUAL_ROLE (1 << 15)
> +/* Port is the pending override port */
> +#define CHARGE_FLAGS_DELAYED_OVERRIDE (1 << 14)
> +/* Port is the override port */
> +#define CHARGE_FLAGS_OVERRIDE (1 << 13)
> +/* Charger type */
> +#define CHARGE_FLAGS_TYPE_SHIFT 3
> +#define CHARGE_FLAGS_TYPE_MASK (0xf << CHARGE_FLAGS_TYPE_SHIFT)
> +/* Power delivery role */
> +#define CHARGE_FLAGS_ROLE_MASK (7 << 0)
> +
> +/*
> + * PD_EVENT_PS_FAULT data field flags definition :
> + */
> +#define PS_FAULT_OCP 1
> +#define PS_FAULT_FAST_OCP 2
> +#define PS_FAULT_OVP 3
> +#define PS_FAULT_DISCH 4
> +
> +/*
> + * PD_EVENT_VIDEO_CODEC payload is "struct mcdp_info".
> + */
> +struct mcdp_version {
> + uint8_t major;
> + uint8_t minor;
> + uint16_t build;
> } __packed;
>
> +struct mcdp_info {
> + uint8_t family[2];
> + uint8_t chipid[2];
> + struct mcdp_version irom;
> + struct mcdp_version fw;
> +} __packed;
> +
> +/* struct mcdp_info field decoding */
> +#define MCDP_CHIPID(chipid) ((chipid[0] << 8) | chipid[1])
> +#define MCDP_FAMILY(family) ((family[0] << 8) | family[1])
> +
> +/* Get/Set USB-PD Alternate mode info */
> +#define EC_CMD_USB_PD_GET_AMODE 0x116
> +struct ec_params_usb_pd_get_mode_request {
> + uint16_t svid_idx; /* SVID index to get */
> + uint8_t port; /* port */
> +} __packed;
> +
> +struct ec_params_usb_pd_get_mode_response {
> + uint16_t svid; /* SVID */
> + uint16_t opos; /* Object Position */
> + uint32_t vdo[6]; /* Mode VDOs */
> +} __packed;
> +
> +#define EC_CMD_USB_PD_SET_AMODE 0x117
> +
> +enum pd_mode_cmd {
> + PD_EXIT_MODE = 0,
> + PD_ENTER_MODE = 1,
> + /* Not a command. Do NOT remove. */
> + PD_MODE_CMD_COUNT,
> +};
> +
> +struct ec_params_usb_pd_set_mode_request {
> + uint32_t cmd; /* enum pd_mode_cmd */
> + uint16_t svid; /* SVID to set */
> + uint8_t opos; /* Object Position */
> + uint8_t port; /* port */
> +} __packed;
> +
> +/* Ask the PD MCU to record a log of a requested type */
> +#define EC_CMD_PD_WRITE_LOG_ENTRY 0x118
> +
> +struct ec_params_pd_write_log_entry {
> + uint8_t type; /* event type : see PD_EVENT_xx above */
> + uint8_t port; /* port#, or 0 for events unrelated to a given port */
> +} __packed;
> +
> +#endif /* !__ACPI__ */
> +
> /*****************************************************************************/
> /*
> * Passthru commands
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 2/6] mfd: cros_ec: Add MKBP event support
2016-02-05 13:32 ` [PATCH v1 2/6] mfd: cros_ec: Add MKBP event support Tomeu Vizoso
@ 2016-02-10 17:41 ` Gwendal Grignou
2016-02-11 9:15 ` Lee Jones
[not found] ` <CAMHSBOXRvC7U_KWeg+rX3Qymaos0-3FN6AuaXT2cyP=gAN47mA@mail.gmail.com>
1 sibling, 1 reply; 23+ messages in thread
From: Gwendal Grignou @ 2016-02-10 17:41 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: Linux Kernel, Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Vic Yang, Olof Johansson, linux-input, Dmitry Torokhov,
Lee Jones
[Resend[We should not used kmalloc when get events, these functions
are called quite often.
Gwendal.
On Fri, Feb 5, 2016 at 5:32 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> From: Vic Yang <victoryang@google.com>
>
> Newer revisions of the ChromeOS EC add more events besides the keyboard
> ones. So handle interrupts in the MFD driver and let consumers register
> for notifications for the events they might care.
>
> To keep backward compatibility, if the EC doesn't support MKBP event, we
> fall back to the old MKBP key matrix host command.
>
> Signed-off-by: Vic Yang <victoryang@chromium.org>
> [bleung: fixup some context changes going from v3.14 to v3.18]
> Signed-off-by: Benson Leung <bleung@chromium.org>
> [tomeu: adapted to changes in mainline (in power-supply and platform/chrome)]
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Randall Spangler <rspangler@chromium.org>
> Cc: Vincent Palatin <vpalatin@chromium.org>
>
> ---
>
> drivers/input/keyboard/cros_ec_keyb.c | 135 ++++++++------------------------
> drivers/mfd/cros_ec.c | 57 +++++++++++++-
> drivers/platform/chrome/cros_ec_proto.c | 102 ++++++++++++++++++++++++
> include/linux/mfd/cros_ec.h | 44 +++++++++++
> include/linux/mfd/cros_ec_commands.h | 34 ++++++++
> 5 files changed, 265 insertions(+), 107 deletions(-)
>
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index b01966dc7eb3..b891503143dc 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -27,6 +27,7 @@
> #include <linux/input.h>
> #include <linux/interrupt.h>
> #include <linux/kernel.h>
> +#include <linux/notifier.h>
> #include <linux/platform_device.h>
> #include <linux/slab.h>
> #include <linux/input/matrix_keypad.h>
> @@ -44,6 +45,7 @@
> * @dev: Device pointer
> * @idev: Input device
> * @ec: Top level ChromeOS device to use to talk to EC
> + * @notifier: interrupt event notifier for transport devices
> */
> struct cros_ec_keyb {
> unsigned int rows;
> @@ -57,6 +59,7 @@ struct cros_ec_keyb {
> struct device *dev;
> struct input_dev *idev;
> struct cros_ec_device *ec;
> + struct notifier_block notifier;
> };
>
>
> @@ -146,67 +149,44 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
> input_sync(ckdev->idev);
> }
>
> -static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
> -{
> - int ret = 0;
> - struct cros_ec_command *msg;
> -
> - msg = kmalloc(sizeof(*msg) + ckdev->cols, GFP_KERNEL);
> - if (!msg)
> - return -ENOMEM;
> -
> - msg->version = 0;
> - msg->command = EC_CMD_MKBP_STATE;
> - msg->insize = ckdev->cols;
> - msg->outsize = 0;
> -
> - ret = cros_ec_cmd_xfer(ckdev->ec, msg);
> - if (ret < 0) {
> - dev_err(ckdev->dev, "Error transferring EC message %d\n", ret);
> - goto exit;
> - }
> -
> - memcpy(kb_state, msg->data, ckdev->cols);
> -exit:
> - kfree(msg);
> - return ret;
> -}
> -
> -static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
> +static int cros_ec_keyb_open(struct input_dev *dev)
> {
> - struct cros_ec_keyb *ckdev = data;
> - struct cros_ec_device *ec = ckdev->ec;
> - int ret;
> - uint8_t kb_state[ckdev->cols];
> -
> - if (device_may_wakeup(ec->dev))
> - pm_wakeup_event(ec->dev, 0);
> -
> - ret = cros_ec_keyb_get_state(ckdev, kb_state);
> - if (ret >= 0)
> - cros_ec_keyb_process(ckdev, kb_state, ret);
> - else
> - dev_err(ec->dev, "failed to get keyboard state: %d\n", ret);
> + struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
>
> - return IRQ_HANDLED;
> + return blocking_notifier_chain_register(&ckdev->ec->event_notifier,
> + &ckdev->notifier);
> }
>
> -static int cros_ec_keyb_open(struct input_dev *dev)
> +static void cros_ec_keyb_close(struct input_dev *dev)
> {
> struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> - struct cros_ec_device *ec = ckdev->ec;
>
> - return request_threaded_irq(ec->irq, NULL, cros_ec_keyb_irq,
> - IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> - "cros_ec_keyb", ckdev);
> + blocking_notifier_chain_unregister(&ckdev->ec->event_notifier,
> + &ckdev->notifier);
> }
>
> -static void cros_ec_keyb_close(struct input_dev *dev)
> +static int cros_ec_keyb_work(struct notifier_block *nb,
> + unsigned long queued_during_suspend, void *_notify)
> {
> - struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> - struct cros_ec_device *ec = ckdev->ec;
> + struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
> + notifier);
>
> - free_irq(ec->irq, ckdev);
> + if (ckdev->ec->event_data.event_type != EC_MKBP_EVENT_KEY_MATRIX)
> + return NOTIFY_DONE;
> + /*
> + * If EC is not the wake source, discard key state changes during
> + * suspend.
> + */
> + if (queued_during_suspend)
> + return NOTIFY_OK;
> + if (ckdev->ec->event_size != ckdev->cols) {
> + dev_err(ckdev->dev,
> + "Discarded incomplete key matrix event.\n");
> + return NOTIFY_OK;
> + }
> + cros_ec_keyb_process(ckdev, ckdev->ec->event_data.data.key_matrix,
> + ckdev->ec->event_size);
> + return NOTIFY_OK;
> }
>
> /*
> @@ -266,12 +246,8 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
> if (!idev)
> return -ENOMEM;
>
> - if (!ec->irq) {
> - dev_err(dev, "no EC IRQ specified\n");
> - return -EINVAL;
> - }
> -
> ckdev->ec = ec;
> + ckdev->notifier.notifier_call = cros_ec_keyb_work;
> ckdev->dev = dev;
> dev_set_drvdata(&pdev->dev, ckdev);
>
> @@ -312,54 +288,6 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
> return 0;
> }
>
> -#ifdef CONFIG_PM_SLEEP
> -/* Clear any keys in the buffer */
> -static void cros_ec_keyb_clear_keyboard(struct cros_ec_keyb *ckdev)
> -{
> - uint8_t old_state[ckdev->cols];
> - uint8_t new_state[ckdev->cols];
> - unsigned long duration;
> - int i, ret;
> -
> - /*
> - * Keep reading until we see that the scan state does not change.
> - * That indicates that we are done.
> - *
> - * Assume that the EC keyscan buffer is at most 32 deep.
> - */
> - duration = jiffies;
> - ret = cros_ec_keyb_get_state(ckdev, new_state);
> - for (i = 1; !ret && i < 32; i++) {
> - memcpy(old_state, new_state, sizeof(old_state));
> - ret = cros_ec_keyb_get_state(ckdev, new_state);
> - if (0 == memcmp(old_state, new_state, sizeof(old_state)))
> - break;
> - }
> - duration = jiffies - duration;
> - dev_info(ckdev->dev, "Discarded %d keyscan(s) in %dus\n", i,
> - jiffies_to_usecs(duration));
> -}
> -
> -static int cros_ec_keyb_resume(struct device *dev)
> -{
> - struct cros_ec_keyb *ckdev = dev_get_drvdata(dev);
> -
> - /*
> - * When the EC is not a wake source, then it could not have caused the
> - * resume, so we clear the EC's key scan buffer. If the EC was a
> - * wake source (e.g. the lid is open and the user might press a key to
> - * wake) then the key scan buffer should be preserved.
> - */
> - if (!ckdev->ec->was_wake_device)
> - cros_ec_keyb_clear_keyboard(ckdev);
> -
> - return 0;
> -}
> -
> -#endif
> -
> -static SIMPLE_DEV_PM_OPS(cros_ec_keyb_pm_ops, NULL, cros_ec_keyb_resume);
> -
> #ifdef CONFIG_OF
> static const struct of_device_id cros_ec_keyb_of_match[] = {
> { .compatible = "google,cros-ec-keyb" },
> @@ -373,7 +301,6 @@ static struct platform_driver cros_ec_keyb_driver = {
> .driver = {
> .name = "cros-ec-keyb",
> .of_match_table = of_match_ptr(cros_ec_keyb_of_match),
> - .pm = &cros_ec_keyb_pm_ops,
> },
> };
>
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 0eee63542038..fbe78b819fdd 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -23,6 +23,7 @@
> #include <linux/module.h>
> #include <linux/mfd/core.h>
> #include <linux/mfd/cros_ec.h>
> +#include <asm/unaligned.h>
>
> #define CROS_EC_DEV_EC_INDEX 0
> #define CROS_EC_DEV_PD_INDEX 1
> @@ -49,11 +50,28 @@ static const struct mfd_cell ec_pd_cell = {
> .pdata_size = sizeof(pd_p),
> };
>
> +static irqreturn_t ec_irq_thread(int irq, void *data)
> +{
> + struct cros_ec_device *ec_dev = data;
> + int ret;
> +
> + if (device_may_wakeup(ec_dev->dev))
> + pm_wakeup_event(ec_dev->dev, 0);
> +
> + ret = cros_ec_get_next_event(ec_dev);
> + if (ret > 0)
> + blocking_notifier_call_chain(&ec_dev->event_notifier,
> + 0, ec_dev);
> + return IRQ_HANDLED;
> +}
> +
> int cros_ec_register(struct cros_ec_device *ec_dev)
> {
> struct device *dev = ec_dev->dev;
> int err = 0;
>
> + BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
> +
> ec_dev->max_request = sizeof(struct ec_params_hello);
> ec_dev->max_response = sizeof(struct ec_response_get_protocol_info);
> ec_dev->max_passthru = 0;
> @@ -70,13 +88,24 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>
> cros_ec_query_all(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);
> + if (err) {
> + dev_err(dev, "request irq %d: error %d\n",
> + ec_dev->irq, err);
> + return err;
> + }
> + }
> +
> err = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, &ec_cell, 1,
> NULL, ec_dev->irq, NULL);
> if (err) {
> dev_err(dev,
> "Failed to register Embedded Controller subdevice %d\n",
> err);
> - return err;
> + goto fail_mfd;
> }
>
> if (ec_dev->max_passthru) {
> @@ -94,7 +123,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
> dev_err(dev,
> "Failed to register Power Delivery subdevice %d\n",
> err);
> - return err;
> + goto fail_mfd;
> }
> }
>
> @@ -103,13 +132,18 @@ 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");
> - return err;
> + goto fail_mfd;
> }
> }
>
> dev_info(dev, "Chrome EC device registered\n");
>
> return 0;
> +
> +fail_mfd:
> + if (ec_dev->irq)
> + free_irq(ec_dev->irq, ec_dev);
> + return err;
> }
> EXPORT_SYMBOL(cros_ec_register);
>
> @@ -136,13 +170,30 @@ int cros_ec_suspend(struct cros_ec_device *ec_dev)
> }
> EXPORT_SYMBOL(cros_ec_suspend);
>
> +static void cros_ec_drain_events(struct cros_ec_device *ec_dev)
> +{
> + while (cros_ec_get_next_event(ec_dev) > 0)
> + blocking_notifier_call_chain(&ec_dev->event_notifier,
> + 1, ec_dev);
> +}
> +
> int cros_ec_resume(struct cros_ec_device *ec_dev)
> {
> enable_irq(ec_dev->irq);
>
> + /*
> + * In some case, we need to distinguish events that occur during
> + * suspend if the EC is not a wake source. For example, keypresses
> + * during suspend should be discarded if it does not wake the system.
> + *
> + * If the EC is not a wake source, drain the event queue and mark them
> + * as "queued during suspend".
> + */
> if (ec_dev->wake_enabled) {
> disable_irq_wake(ec_dev->irq);
> ec_dev->wake_enabled = 0;
> + } else {
> + cros_ec_drain_events(ec_dev);
> }
>
> return 0;
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index 990308ca384f..6e138e2333f3 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -19,6 +19,7 @@
> #include <linux/device.h>
> #include <linux/module.h>
> #include <linux/slab.h>
> +#include <asm/unaligned.h>
>
> #define EC_COMMAND_RETRIES 50
>
> @@ -234,11 +235,44 @@ static int cros_ec_host_command_proto_query_v2(struct cros_ec_device *ec_dev)
> return ret;
> }
>
> +static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
> + u16 cmd, u32 *mask)
> +{
> + struct ec_params_get_cmd_versions *pver;
> + struct ec_response_get_cmd_versions *rver;
> + struct cros_ec_command *msg;
> + int ret;
> +
> + msg = kmalloc(sizeof(*msg) + max(sizeof(rver), sizeof(pver)),
> + GFP_KERNEL);
Victor's version in https://chromium-review.googlesource.com/272954
looks cleaner: no malloc, no need to cast rver.
> + if (!msg)
> + return -ENOMEM;
> +
> + msg->version = 0;
> + msg->command = EC_CMD_GET_CMD_VERSIONS;
> + msg->insize = sizeof(rver);
> + msg->outsize = sizeof(pver);
> +
> + pver = (struct ec_params_get_cmd_versions *)msg->data;
> + pver->cmd = cmd;
> +
> + ret = cros_ec_cmd_xfer(ec_dev, msg);
> + if (ret > 0) {
> + rver = (struct ec_response_get_cmd_versions *)msg->data;
> + *mask = rver->version_mask;
> + }
> +
> + kfree(msg);
> +
> + return ret;
> +}
> +
> int cros_ec_query_all(struct cros_ec_device *ec_dev)
> {
> struct device *dev = ec_dev->dev;
> struct cros_ec_command *proto_msg;
> struct ec_response_get_protocol_info *proto_info;
> + u32 ver_mask = 0;
> int ret;
>
> proto_msg = kzalloc(sizeof(*proto_msg) + sizeof(*proto_info),
> @@ -328,6 +362,15 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
> goto exit;
> }
>
> + /* Probe if MKBP event is supported */
> + ret = cros_ec_get_host_command_version_mask(ec_dev,
> + EC_CMD_GET_NEXT_EVENT,
> + &ver_mask);
> + if (ret < 0 || ver_mask == 0)
> + ec_dev->mkbp_event_supported = 0;
> + else
> + ec_dev->mkbp_event_supported = 1;
> +
> exit:
> kfree(proto_msg);
> return ret;
> @@ -380,3 +423,62 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
> return ret;
> }
> EXPORT_SYMBOL(cros_ec_cmd_xfer);
> +
> +static int get_next_event(struct cros_ec_device *ec_dev)
> +{
> + struct cros_ec_command *msg;
> + int ret;
> +
> + msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->event_data), GFP_KERNEL);
Same grip about malloc(). This function will be called very often when
sensors are used.
> + if (!msg)
> + return -ENOMEM;
> +
> + msg->version = 0;
> + msg->command = EC_CMD_GET_NEXT_EVENT;
> + msg->insize = sizeof(ec_dev->event_data);
> + msg->outsize = 0;
> +
> + ret = cros_ec_cmd_xfer(ec_dev, msg);
> + if (ret > 0) {
> + ec_dev->event_size = ret - 1;
> + memcpy(&ec_dev->event_data, msg->data,
> + sizeof(ec_dev->event_data));
> + }
> +
> + kfree(msg);
> +
> + return ret;
> +}
> +
> +static int get_keyboard_state_event(struct cros_ec_device *ec_dev)
> +{
> + struct cros_ec_command *msg;
> +
> + msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->event_data.data),
Given this command will be used on every key press, it is better to
pre-allocate data (like in
https://chromium-review.googlesource.com/276768) and use msg on the
stack like Victor's changes.
> + GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> + msg->version = 0;
> + msg->command = EC_CMD_MKBP_STATE;
> + msg->insize = sizeof(ec_dev->event_data.data);
> + msg->outsize = 0;
> +
> + ec_dev->event_size = cros_ec_cmd_xfer(ec_dev, msg);
> + ec_dev->event_data.event_type = EC_MKBP_EVENT_KEY_MATRIX;
> + memcpy(&ec_dev->event_data.data, msg->data,
> + sizeof(ec_dev->event_data.data));
> +
> + kfree(msg);
> +
> + return ec_dev->event_size;
> +}
> +
> +int cros_ec_get_next_event(struct cros_ec_device *ec_dev)
> +{
> + if (ec_dev->mkbp_event_supported)
> + return get_next_event(ec_dev);
> + else
> + return get_keyboard_state_event(ec_dev);
> +}
> +EXPORT_SYMBOL(cros_ec_get_next_event);
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index a677c2bd485c..8a8fa75178c6 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -72,6 +72,24 @@ struct cros_ec_command {
> uint8_t data[0];
> };
>
> +/*
> + * event_data is used by keyboard or event notifier:
> + * event_data format:
> + * If MKBP protocol is supported:
> + * 0 1
> + * +-----------+--------------------------------
> + * | type | payload
> + * +-----------+--------------------------------
> + * |HOST_EVENT | EVENT (32 bit)
> + * |KEY_MATRIX | Keyboard keys pressed.
> + * |SENSOR_FIFO| Sensors FIFO information.
> + *
> + * Otherwise:
> + * 0 1
> + * +-----------+--------------------------------
> + * |Unused | Keyboard keys pressed.
> + */
> +
> /**
> * struct cros_ec_device - Information about a ChromeOS EC device
> *
> @@ -107,6 +125,9 @@ struct cros_ec_command {
> * should check msg.result for the EC's result code.
> * @pkt_xfer: send packet to EC and get response
> * @lock: one transaction at a time
> + * @event_notifier: interrupt event notifier for transport devices.
> + * @event_data: raw payload transferred with the MKBP event.
> + * @event_size: size in bytes of the event data.
> */
> struct cros_ec_device {
>
> @@ -135,6 +156,11 @@ struct cros_ec_device {
> int (*pkt_xfer)(struct cros_ec_device *ec,
> struct cros_ec_command *msg);
> struct mutex lock;
> + bool mkbp_event_supported;
> + struct blocking_notifier_head event_notifier;
> +
> + struct ec_response_get_next_event event_data;
> + int event_size;
> };
>
> /* struct cros_ec_platform - ChromeOS EC platform information
> @@ -252,9 +278,27 @@ int cros_ec_register(struct cros_ec_device *ec_dev);
> */
> int cros_ec_query_all(struct cros_ec_device *ec_dev);
>
> +/**
> + * cros_ec_get_next_event - Fetch next event from the ChromeOS EC
> + *
> + * @ec_dev: Device to fetch event from
> + * @return 0 if ok, -ve on error
> + */
> +int cros_ec_get_next_event(struct cros_ec_device *ec_dev);
> +
> /* sysfs stuff */
> extern struct attribute_group cros_ec_attr_group;
> extern struct attribute_group cros_ec_lightbar_attr_group;
> extern struct attribute_group cros_ec_vbc_attr_group;
>
> +/**
> + * cros_ec_get_host_event - Return a mask of event set by the EC.
> + *
> + * When MKBP is supported, when the EC raises an interrupt,
> + * We collect the events raised and call the functions in the ec notifier.
> + *
> + * This function is a helper to know which events are raised.
> + */
> +uint32_t cros_ec_get_host_event(struct cros_ec_device *ec_dev);
> +
> #endif /* __LINUX_MFD_CROS_EC_H */
> diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
> index 13b630c10d4c..d86526f0bd8e 100644
> --- a/include/linux/mfd/cros_ec_commands.h
> +++ b/include/linux/mfd/cros_ec_commands.h
> @@ -1762,6 +1762,40 @@ struct ec_result_keyscan_seq_ctrl {
> };
> } __packed;
>
> +/*
> + * Get the next pending MKBP event.
> + *
> + * Returns EC_RES_UNAVAILABLE if there is no event pending.
> + */
> +#define EC_CMD_GET_NEXT_EVENT 0x67
> +
> +enum ec_mkbp_event {
> + /* Keyboard matrix changed. The event data is the new matrix state. */
> + EC_MKBP_EVENT_KEY_MATRIX = 0,
> +
> + /* New host event. The event data is 4 bytes of host event flags. */
> + EC_MKBP_EVENT_HOST_EVENT = 1,
> +
> + /* New Sensor FIFO data. The event data is fifo_info structure. */
> + EC_MKBP_EVENT_SENSOR_FIFO = 2,
> +
> + /* Number of MKBP events */
> + EC_MKBP_EVENT_COUNT,
> +};
> +
> +union ec_response_get_next_data {
> + uint8_t key_matrix[13];
> +
> + /* Unaligned */
> + uint32_t host_event;
> +} __packed;
> +
> +struct ec_response_get_next_event {
> + uint8_t event_type;
> + /* Followed by event data if any */
> + union ec_response_get_next_data data;
> +} __packed;
> +
> /*****************************************************************************/
> /* Temperature sensor commands */
>
> --
> 2.5.0
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 2/6] mfd: cros_ec: Add MKBP event support
2016-02-10 17:41 ` Gwendal Grignou
@ 2016-02-11 9:15 ` Lee Jones
0 siblings, 0 replies; 23+ messages in thread
From: Lee Jones @ 2016-02-11 9:15 UTC (permalink / raw)
To: Gwendal Grignou
Cc: Tomeu Vizoso, Linux Kernel, Sameer Nanda, Benson Leung,
Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Vic Yang,
Olof Johansson, linux-input, Dmitry Torokhov
On Wed, 10 Feb 2016, Gwendal Grignou wrote:
> [Resend[We should not used kmalloc when get events, these functions
> are called quite often.
> Gwendal.
Why are you re-sending [again]? Why are you top posting? Why aren't
you cutting the 370 lines before your first comment and the 150 lines
after your last?
> On Fri, Feb 5, 2016 at 5:32 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> > From: Vic Yang <victoryang@google.com>
> >
> > Newer revisions of the ChromeOS EC add more events besides the keyboard
> > ones. So handle interrupts in the MFD driver and let consumers register
> > for notifications for the events they might care.
> >
> > To keep backward compatibility, if the EC doesn't support MKBP event, we
> > fall back to the old MKBP key matrix host command.
> >
> > Signed-off-by: Vic Yang <victoryang@chromium.org>
> > [bleung: fixup some context changes going from v3.14 to v3.18]
> > Signed-off-by: Benson Leung <bleung@chromium.org>
> > [tomeu: adapted to changes in mainline (in power-supply and platform/chrome)]
> > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> > Cc: Randall Spangler <rspangler@chromium.org>
> > Cc: Vincent Palatin <vpalatin@chromium.org>
> >
> > ---
> >
> > drivers/input/keyboard/cros_ec_keyb.c | 135 ++++++++------------------------
> > drivers/mfd/cros_ec.c | 57 +++++++++++++-
> > drivers/platform/chrome/cros_ec_proto.c | 102 ++++++++++++++++++++++++
> > include/linux/mfd/cros_ec.h | 44 +++++++++++
> > include/linux/mfd/cros_ec_commands.h | 34 ++++++++
> > 5 files changed, 265 insertions(+), 107 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> > index b01966dc7eb3..b891503143dc 100644
> > --- a/drivers/input/keyboard/cros_ec_keyb.c
> > +++ b/drivers/input/keyboard/cros_ec_keyb.c
> > @@ -27,6 +27,7 @@
> > #include <linux/input.h>
> > #include <linux/interrupt.h>
> > #include <linux/kernel.h>
> > +#include <linux/notifier.h>
> > #include <linux/platform_device.h>
> > #include <linux/slab.h>
> > #include <linux/input/matrix_keypad.h>
> > @@ -44,6 +45,7 @@
> > * @dev: Device pointer
> > * @idev: Input device
> > * @ec: Top level ChromeOS device to use to talk to EC
> > + * @notifier: interrupt event notifier for transport devices
> > */
> > struct cros_ec_keyb {
> > unsigned int rows;
> > @@ -57,6 +59,7 @@ struct cros_ec_keyb {
> > struct device *dev;
> > struct input_dev *idev;
> > struct cros_ec_device *ec;
> > + struct notifier_block notifier;
> > };
> >
> >
> > @@ -146,67 +149,44 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
> > input_sync(ckdev->idev);
> > }
> >
> > -static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
> > -{
> > - int ret = 0;
> > - struct cros_ec_command *msg;
> > -
> > - msg = kmalloc(sizeof(*msg) + ckdev->cols, GFP_KERNEL);
> > - if (!msg)
> > - return -ENOMEM;
> > -
> > - msg->version = 0;
> > - msg->command = EC_CMD_MKBP_STATE;
> > - msg->insize = ckdev->cols;
> > - msg->outsize = 0;
> > -
> > - ret = cros_ec_cmd_xfer(ckdev->ec, msg);
> > - if (ret < 0) {
> > - dev_err(ckdev->dev, "Error transferring EC message %d\n", ret);
> > - goto exit;
> > - }
> > -
> > - memcpy(kb_state, msg->data, ckdev->cols);
> > -exit:
> > - kfree(msg);
> > - return ret;
> > -}
> > -
> > -static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
> > +static int cros_ec_keyb_open(struct input_dev *dev)
> > {
> > - struct cros_ec_keyb *ckdev = data;
> > - struct cros_ec_device *ec = ckdev->ec;
> > - int ret;
> > - uint8_t kb_state[ckdev->cols];
> > -
> > - if (device_may_wakeup(ec->dev))
> > - pm_wakeup_event(ec->dev, 0);
> > -
> > - ret = cros_ec_keyb_get_state(ckdev, kb_state);
> > - if (ret >= 0)
> > - cros_ec_keyb_process(ckdev, kb_state, ret);
> > - else
> > - dev_err(ec->dev, "failed to get keyboard state: %d\n", ret);
> > + struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> >
> > - return IRQ_HANDLED;
> > + return blocking_notifier_chain_register(&ckdev->ec->event_notifier,
> > + &ckdev->notifier);
> > }
> >
> > -static int cros_ec_keyb_open(struct input_dev *dev)
> > +static void cros_ec_keyb_close(struct input_dev *dev)
> > {
> > struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> > - struct cros_ec_device *ec = ckdev->ec;
> >
> > - return request_threaded_irq(ec->irq, NULL, cros_ec_keyb_irq,
> > - IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> > - "cros_ec_keyb", ckdev);
> > + blocking_notifier_chain_unregister(&ckdev->ec->event_notifier,
> > + &ckdev->notifier);
> > }
> >
> > -static void cros_ec_keyb_close(struct input_dev *dev)
> > +static int cros_ec_keyb_work(struct notifier_block *nb,
> > + unsigned long queued_during_suspend, void *_notify)
> > {
> > - struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> > - struct cros_ec_device *ec = ckdev->ec;
> > + struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
> > + notifier);
> >
> > - free_irq(ec->irq, ckdev);
> > + if (ckdev->ec->event_data.event_type != EC_MKBP_EVENT_KEY_MATRIX)
> > + return NOTIFY_DONE;
> > + /*
> > + * If EC is not the wake source, discard key state changes during
> > + * suspend.
> > + */
> > + if (queued_during_suspend)
> > + return NOTIFY_OK;
> > + if (ckdev->ec->event_size != ckdev->cols) {
> > + dev_err(ckdev->dev,
> > + "Discarded incomplete key matrix event.\n");
> > + return NOTIFY_OK;
> > + }
> > + cros_ec_keyb_process(ckdev, ckdev->ec->event_data.data.key_matrix,
> > + ckdev->ec->event_size);
> > + return NOTIFY_OK;
> > }
> >
> > /*
> > @@ -266,12 +246,8 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
> > if (!idev)
> > return -ENOMEM;
> >
> > - if (!ec->irq) {
> > - dev_err(dev, "no EC IRQ specified\n");
> > - return -EINVAL;
> > - }
> > -
> > ckdev->ec = ec;
> > + ckdev->notifier.notifier_call = cros_ec_keyb_work;
> > ckdev->dev = dev;
> > dev_set_drvdata(&pdev->dev, ckdev);
> >
> > @@ -312,54 +288,6 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
> > return 0;
> > }
> >
> > -#ifdef CONFIG_PM_SLEEP
> > -/* Clear any keys in the buffer */
> > -static void cros_ec_keyb_clear_keyboard(struct cros_ec_keyb *ckdev)
> > -{
> > - uint8_t old_state[ckdev->cols];
> > - uint8_t new_state[ckdev->cols];
> > - unsigned long duration;
> > - int i, ret;
> > -
> > - /*
> > - * Keep reading until we see that the scan state does not change.
> > - * That indicates that we are done.
> > - *
> > - * Assume that the EC keyscan buffer is at most 32 deep.
> > - */
> > - duration = jiffies;
> > - ret = cros_ec_keyb_get_state(ckdev, new_state);
> > - for (i = 1; !ret && i < 32; i++) {
> > - memcpy(old_state, new_state, sizeof(old_state));
> > - ret = cros_ec_keyb_get_state(ckdev, new_state);
> > - if (0 == memcmp(old_state, new_state, sizeof(old_state)))
> > - break;
> > - }
> > - duration = jiffies - duration;
> > - dev_info(ckdev->dev, "Discarded %d keyscan(s) in %dus\n", i,
> > - jiffies_to_usecs(duration));
> > -}
> > -
> > -static int cros_ec_keyb_resume(struct device *dev)
> > -{
> > - struct cros_ec_keyb *ckdev = dev_get_drvdata(dev);
> > -
> > - /*
> > - * When the EC is not a wake source, then it could not have caused the
> > - * resume, so we clear the EC's key scan buffer. If the EC was a
> > - * wake source (e.g. the lid is open and the user might press a key to
> > - * wake) then the key scan buffer should be preserved.
> > - */
> > - if (!ckdev->ec->was_wake_device)
> > - cros_ec_keyb_clear_keyboard(ckdev);
> > -
> > - return 0;
> > -}
> > -
> > -#endif
> > -
> > -static SIMPLE_DEV_PM_OPS(cros_ec_keyb_pm_ops, NULL, cros_ec_keyb_resume);
> > -
> > #ifdef CONFIG_OF
> > static const struct of_device_id cros_ec_keyb_of_match[] = {
> > { .compatible = "google,cros-ec-keyb" },
> > @@ -373,7 +301,6 @@ static struct platform_driver cros_ec_keyb_driver = {
> > .driver = {
> > .name = "cros-ec-keyb",
> > .of_match_table = of_match_ptr(cros_ec_keyb_of_match),
> > - .pm = &cros_ec_keyb_pm_ops,
> > },
> > };
> >
> > diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> > index 0eee63542038..fbe78b819fdd 100644
> > --- a/drivers/mfd/cros_ec.c
> > +++ b/drivers/mfd/cros_ec.c
> > @@ -23,6 +23,7 @@
> > #include <linux/module.h>
> > #include <linux/mfd/core.h>
> > #include <linux/mfd/cros_ec.h>
> > +#include <asm/unaligned.h>
> >
> > #define CROS_EC_DEV_EC_INDEX 0
> > #define CROS_EC_DEV_PD_INDEX 1
> > @@ -49,11 +50,28 @@ static const struct mfd_cell ec_pd_cell = {
> > .pdata_size = sizeof(pd_p),
> > };
> >
> > +static irqreturn_t ec_irq_thread(int irq, void *data)
> > +{
> > + struct cros_ec_device *ec_dev = data;
> > + int ret;
> > +
> > + if (device_may_wakeup(ec_dev->dev))
> > + pm_wakeup_event(ec_dev->dev, 0);
> > +
> > + ret = cros_ec_get_next_event(ec_dev);
> > + if (ret > 0)
> > + blocking_notifier_call_chain(&ec_dev->event_notifier,
> > + 0, ec_dev);
> > + return IRQ_HANDLED;
> > +}
> > +
> > int cros_ec_register(struct cros_ec_device *ec_dev)
> > {
> > struct device *dev = ec_dev->dev;
> > int err = 0;
> >
> > + BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
> > +
> > ec_dev->max_request = sizeof(struct ec_params_hello);
> > ec_dev->max_response = sizeof(struct ec_response_get_protocol_info);
> > ec_dev->max_passthru = 0;
> > @@ -70,13 +88,24 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
> >
> > cros_ec_query_all(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);
> > + if (err) {
> > + dev_err(dev, "request irq %d: error %d\n",
> > + ec_dev->irq, err);
> > + return err;
> > + }
> > + }
> > +
> > err = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, &ec_cell, 1,
> > NULL, ec_dev->irq, NULL);
> > if (err) {
> > dev_err(dev,
> > "Failed to register Embedded Controller subdevice %d\n",
> > err);
> > - return err;
> > + goto fail_mfd;
> > }
> >
> > if (ec_dev->max_passthru) {
> > @@ -94,7 +123,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
> > dev_err(dev,
> > "Failed to register Power Delivery subdevice %d\n",
> > err);
> > - return err;
> > + goto fail_mfd;
> > }
> > }
> >
> > @@ -103,13 +132,18 @@ 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");
> > - return err;
> > + goto fail_mfd;
> > }
> > }
> >
> > dev_info(dev, "Chrome EC device registered\n");
> >
> > return 0;
> > +
> > +fail_mfd:
> > + if (ec_dev->irq)
> > + free_irq(ec_dev->irq, ec_dev);
> > + return err;
> > }
> > EXPORT_SYMBOL(cros_ec_register);
> >
> > @@ -136,13 +170,30 @@ int cros_ec_suspend(struct cros_ec_device *ec_dev)
> > }
> > EXPORT_SYMBOL(cros_ec_suspend);
> >
> > +static void cros_ec_drain_events(struct cros_ec_device *ec_dev)
> > +{
> > + while (cros_ec_get_next_event(ec_dev) > 0)
> > + blocking_notifier_call_chain(&ec_dev->event_notifier,
> > + 1, ec_dev);
> > +}
> > +
> > int cros_ec_resume(struct cros_ec_device *ec_dev)
> > {
> > enable_irq(ec_dev->irq);
> >
> > + /*
> > + * In some case, we need to distinguish events that occur during
> > + * suspend if the EC is not a wake source. For example, keypresses
> > + * during suspend should be discarded if it does not wake the system.
> > + *
> > + * If the EC is not a wake source, drain the event queue and mark them
> > + * as "queued during suspend".
> > + */
> > if (ec_dev->wake_enabled) {
> > disable_irq_wake(ec_dev->irq);
> > ec_dev->wake_enabled = 0;
> > + } else {
> > + cros_ec_drain_events(ec_dev);
> > }
> >
> > return 0;
> > diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> > index 990308ca384f..6e138e2333f3 100644
> > --- a/drivers/platform/chrome/cros_ec_proto.c
> > +++ b/drivers/platform/chrome/cros_ec_proto.c
> > @@ -19,6 +19,7 @@
> > #include <linux/device.h>
> > #include <linux/module.h>
> > #include <linux/slab.h>
> > +#include <asm/unaligned.h>
> >
> > #define EC_COMMAND_RETRIES 50
> >
> > @@ -234,11 +235,44 @@ static int cros_ec_host_command_proto_query_v2(struct cros_ec_device *ec_dev)
> > return ret;
> > }
> >
> > +static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
> > + u16 cmd, u32 *mask)
> > +{
> > + struct ec_params_get_cmd_versions *pver;
> > + struct ec_response_get_cmd_versions *rver;
> > + struct cros_ec_command *msg;
> > + int ret;
> > +
> > + msg = kmalloc(sizeof(*msg) + max(sizeof(rver), sizeof(pver)),
> > + GFP_KERNEL);
> Victor's version in https://chromium-review.googlesource.com/272954
> looks cleaner: no malloc, no need to cast rver.
> > + if (!msg)
> > + return -ENOMEM;
> > +
> > + msg->version = 0;
> > + msg->command = EC_CMD_GET_CMD_VERSIONS;
> > + msg->insize = sizeof(rver);
> > + msg->outsize = sizeof(pver);
> > +
> > + pver = (struct ec_params_get_cmd_versions *)msg->data;
> > + pver->cmd = cmd;
> > +
> > + ret = cros_ec_cmd_xfer(ec_dev, msg);
> > + if (ret > 0) {
> > + rver = (struct ec_response_get_cmd_versions *)msg->data;
> > + *mask = rver->version_mask;
> > + }
> > +
> > + kfree(msg);
> > +
> > + return ret;
> > +}
> > +
> > int cros_ec_query_all(struct cros_ec_device *ec_dev)
> > {
> > struct device *dev = ec_dev->dev;
> > struct cros_ec_command *proto_msg;
> > struct ec_response_get_protocol_info *proto_info;
> > + u32 ver_mask = 0;
> > int ret;
> >
> > proto_msg = kzalloc(sizeof(*proto_msg) + sizeof(*proto_info),
> > @@ -328,6 +362,15 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
> > goto exit;
> > }
> >
> > + /* Probe if MKBP event is supported */
> > + ret = cros_ec_get_host_command_version_mask(ec_dev,
> > + EC_CMD_GET_NEXT_EVENT,
> > + &ver_mask);
> > + if (ret < 0 || ver_mask == 0)
> > + ec_dev->mkbp_event_supported = 0;
> > + else
> > + ec_dev->mkbp_event_supported = 1;
> > +
> > exit:
> > kfree(proto_msg);
> > return ret;
> > @@ -380,3 +423,62 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
> > return ret;
> > }
> > EXPORT_SYMBOL(cros_ec_cmd_xfer);
> > +
> > +static int get_next_event(struct cros_ec_device *ec_dev)
> > +{
> > + struct cros_ec_command *msg;
> > + int ret;
> > +
> > + msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->event_data), GFP_KERNEL);
> Same grip about malloc(). This function will be called very often when
> sensors are used.
> > + if (!msg)
> > + return -ENOMEM;
> > +
> > + msg->version = 0;
> > + msg->command = EC_CMD_GET_NEXT_EVENT;
> > + msg->insize = sizeof(ec_dev->event_data);
> > + msg->outsize = 0;
> > +
> > + ret = cros_ec_cmd_xfer(ec_dev, msg);
> > + if (ret > 0) {
> > + ec_dev->event_size = ret - 1;
> > + memcpy(&ec_dev->event_data, msg->data,
> > + sizeof(ec_dev->event_data));
> > + }
> > +
> > + kfree(msg);
> > +
> > + return ret;
> > +}
> > +
> > +static int get_keyboard_state_event(struct cros_ec_device *ec_dev)
> > +{
> > + struct cros_ec_command *msg;
> > +
> > + msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->event_data.data),
> Given this command will be used on every key press, it is better to
> pre-allocate data (like in
> https://chromium-review.googlesource.com/276768) and use msg on the
> stack like Victor's changes.
> > + GFP_KERNEL);
> > + if (!msg)
> > + return -ENOMEM;
> > +
> > + msg->version = 0;
> > + msg->command = EC_CMD_MKBP_STATE;
> > + msg->insize = sizeof(ec_dev->event_data.data);
> > + msg->outsize = 0;
> > +
> > + ec_dev->event_size = cros_ec_cmd_xfer(ec_dev, msg);
> > + ec_dev->event_data.event_type = EC_MKBP_EVENT_KEY_MATRIX;
> > + memcpy(&ec_dev->event_data.data, msg->data,
> > + sizeof(ec_dev->event_data.data));
> > +
> > + kfree(msg);
> > +
> > + return ec_dev->event_size;
> > +}
> > +
> > +int cros_ec_get_next_event(struct cros_ec_device *ec_dev)
> > +{
> > + if (ec_dev->mkbp_event_supported)
> > + return get_next_event(ec_dev);
> > + else
> > + return get_keyboard_state_event(ec_dev);
> > +}
> > +EXPORT_SYMBOL(cros_ec_get_next_event);
> > diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> > index a677c2bd485c..8a8fa75178c6 100644
> > --- a/include/linux/mfd/cros_ec.h
> > +++ b/include/linux/mfd/cros_ec.h
> > @@ -72,6 +72,24 @@ struct cros_ec_command {
> > uint8_t data[0];
> > };
> >
> > +/*
> > + * event_data is used by keyboard or event notifier:
> > + * event_data format:
> > + * If MKBP protocol is supported:
> > + * 0 1
> > + * +-----------+--------------------------------
> > + * | type | payload
> > + * +-----------+--------------------------------
> > + * |HOST_EVENT | EVENT (32 bit)
> > + * |KEY_MATRIX | Keyboard keys pressed.
> > + * |SENSOR_FIFO| Sensors FIFO information.
> > + *
> > + * Otherwise:
> > + * 0 1
> > + * +-----------+--------------------------------
> > + * |Unused | Keyboard keys pressed.
> > + */
> > +
> > /**
> > * struct cros_ec_device - Information about a ChromeOS EC device
> > *
> > @@ -107,6 +125,9 @@ struct cros_ec_command {
> > * should check msg.result for the EC's result code.
> > * @pkt_xfer: send packet to EC and get response
> > * @lock: one transaction at a time
> > + * @event_notifier: interrupt event notifier for transport devices.
> > + * @event_data: raw payload transferred with the MKBP event.
> > + * @event_size: size in bytes of the event data.
> > */
> > struct cros_ec_device {
> >
> > @@ -135,6 +156,11 @@ struct cros_ec_device {
> > int (*pkt_xfer)(struct cros_ec_device *ec,
> > struct cros_ec_command *msg);
> > struct mutex lock;
> > + bool mkbp_event_supported;
> > + struct blocking_notifier_head event_notifier;
> > +
> > + struct ec_response_get_next_event event_data;
> > + int event_size;
> > };
> >
> > /* struct cros_ec_platform - ChromeOS EC platform information
> > @@ -252,9 +278,27 @@ int cros_ec_register(struct cros_ec_device *ec_dev);
> > */
> > int cros_ec_query_all(struct cros_ec_device *ec_dev);
> >
> > +/**
> > + * cros_ec_get_next_event - Fetch next event from the ChromeOS EC
> > + *
> > + * @ec_dev: Device to fetch event from
> > + * @return 0 if ok, -ve on error
> > + */
> > +int cros_ec_get_next_event(struct cros_ec_device *ec_dev);
> > +
> > /* sysfs stuff */
> > extern struct attribute_group cros_ec_attr_group;
> > extern struct attribute_group cros_ec_lightbar_attr_group;
> > extern struct attribute_group cros_ec_vbc_attr_group;
> >
> > +/**
> > + * cros_ec_get_host_event - Return a mask of event set by the EC.
> > + *
> > + * When MKBP is supported, when the EC raises an interrupt,
> > + * We collect the events raised and call the functions in the ec notifier.
> > + *
> > + * This function is a helper to know which events are raised.
> > + */
> > +uint32_t cros_ec_get_host_event(struct cros_ec_device *ec_dev);
> > +
> > #endif /* __LINUX_MFD_CROS_EC_H */
> > diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
> > index 13b630c10d4c..d86526f0bd8e 100644
> > --- a/include/linux/mfd/cros_ec_commands.h
> > +++ b/include/linux/mfd/cros_ec_commands.h
> > @@ -1762,6 +1762,40 @@ struct ec_result_keyscan_seq_ctrl {
> > };
> > } __packed;
> >
> > +/*
> > + * Get the next pending MKBP event.
> > + *
> > + * Returns EC_RES_UNAVAILABLE if there is no event pending.
> > + */
> > +#define EC_CMD_GET_NEXT_EVENT 0x67
> > +
> > +enum ec_mkbp_event {
> > + /* Keyboard matrix changed. The event data is the new matrix state. */
> > + EC_MKBP_EVENT_KEY_MATRIX = 0,
> > +
> > + /* New host event. The event data is 4 bytes of host event flags. */
> > + EC_MKBP_EVENT_HOST_EVENT = 1,
> > +
> > + /* New Sensor FIFO data. The event data is fifo_info structure. */
> > + EC_MKBP_EVENT_SENSOR_FIFO = 2,
> > +
> > + /* Number of MKBP events */
> > + EC_MKBP_EVENT_COUNT,
> > +};
> > +
> > +union ec_response_get_next_data {
> > + uint8_t key_matrix[13];
> > +
> > + /* Unaligned */
> > + uint32_t host_event;
> > +} __packed;
> > +
> > +struct ec_response_get_next_event {
> > + uint8_t event_type;
> > + /* Followed by event data if any */
> > + union ec_response_get_next_data data;
> > +} __packed;
> > +
> > /*****************************************************************************/
> > /* Temperature sensor commands */
> >
> >
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 2/6] mfd: cros_ec: Add MKBP event support
@ 2016-02-11 9:15 ` Lee Jones
0 siblings, 0 replies; 23+ messages in thread
From: Lee Jones @ 2016-02-11 9:15 UTC (permalink / raw)
To: Gwendal Grignou
Cc: Tomeu Vizoso, Linux Kernel, Sameer Nanda, Benson Leung,
Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Vic Yang,
Olof Johansson, linux-input, Dmitry Torokhov
On Wed, 10 Feb 2016, Gwendal Grignou wrote:
> [Resend[We should not used kmalloc when get events, these functions
> are called quite often.
> Gwendal.
Why are you re-sending [again]? Why are you top posting? Why aren't
you cutting the 370 lines before your first comment and the 150 lines
after your last?
> On Fri, Feb 5, 2016 at 5:32 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> > From: Vic Yang <victoryang@google.com>
> >
> > Newer revisions of the ChromeOS EC add more events besides the keyboard
> > ones. So handle interrupts in the MFD driver and let consumers register
> > for notifications for the events they might care.
> >
> > To keep backward compatibility, if the EC doesn't support MKBP event, we
> > fall back to the old MKBP key matrix host command.
> >
> > Signed-off-by: Vic Yang <victoryang@chromium.org>
> > [bleung: fixup some context changes going from v3.14 to v3.18]
> > Signed-off-by: Benson Leung <bleung@chromium.org>
> > [tomeu: adapted to changes in mainline (in power-supply and platform/chrome)]
> > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> > Cc: Randall Spangler <rspangler@chromium.org>
> > Cc: Vincent Palatin <vpalatin@chromium.org>
> >
> > ---
> >
> > drivers/input/keyboard/cros_ec_keyb.c | 135 ++++++++------------------------
> > drivers/mfd/cros_ec.c | 57 +++++++++++++-
> > drivers/platform/chrome/cros_ec_proto.c | 102 ++++++++++++++++++++++++
> > include/linux/mfd/cros_ec.h | 44 +++++++++++
> > include/linux/mfd/cros_ec_commands.h | 34 ++++++++
> > 5 files changed, 265 insertions(+), 107 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> > index b01966dc7eb3..b891503143dc 100644
> > --- a/drivers/input/keyboard/cros_ec_keyb.c
> > +++ b/drivers/input/keyboard/cros_ec_keyb.c
> > @@ -27,6 +27,7 @@
> > #include <linux/input.h>
> > #include <linux/interrupt.h>
> > #include <linux/kernel.h>
> > +#include <linux/notifier.h>
> > #include <linux/platform_device.h>
> > #include <linux/slab.h>
> > #include <linux/input/matrix_keypad.h>
> > @@ -44,6 +45,7 @@
> > * @dev: Device pointer
> > * @idev: Input device
> > * @ec: Top level ChromeOS device to use to talk to EC
> > + * @notifier: interrupt event notifier for transport devices
> > */
> > struct cros_ec_keyb {
> > unsigned int rows;
> > @@ -57,6 +59,7 @@ struct cros_ec_keyb {
> > struct device *dev;
> > struct input_dev *idev;
> > struct cros_ec_device *ec;
> > + struct notifier_block notifier;
> > };
> >
> >
> > @@ -146,67 +149,44 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
> > input_sync(ckdev->idev);
> > }
> >
> > -static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
> > -{
> > - int ret = 0;
> > - struct cros_ec_command *msg;
> > -
> > - msg = kmalloc(sizeof(*msg) + ckdev->cols, GFP_KERNEL);
> > - if (!msg)
> > - return -ENOMEM;
> > -
> > - msg->version = 0;
> > - msg->command = EC_CMD_MKBP_STATE;
> > - msg->insize = ckdev->cols;
> > - msg->outsize = 0;
> > -
> > - ret = cros_ec_cmd_xfer(ckdev->ec, msg);
> > - if (ret < 0) {
> > - dev_err(ckdev->dev, "Error transferring EC message %d\n", ret);
> > - goto exit;
> > - }
> > -
> > - memcpy(kb_state, msg->data, ckdev->cols);
> > -exit:
> > - kfree(msg);
> > - return ret;
> > -}
> > -
> > -static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
> > +static int cros_ec_keyb_open(struct input_dev *dev)
> > {
> > - struct cros_ec_keyb *ckdev = data;
> > - struct cros_ec_device *ec = ckdev->ec;
> > - int ret;
> > - uint8_t kb_state[ckdev->cols];
> > -
> > - if (device_may_wakeup(ec->dev))
> > - pm_wakeup_event(ec->dev, 0);
> > -
> > - ret = cros_ec_keyb_get_state(ckdev, kb_state);
> > - if (ret >= 0)
> > - cros_ec_keyb_process(ckdev, kb_state, ret);
> > - else
> > - dev_err(ec->dev, "failed to get keyboard state: %d\n", ret);
> > + struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> >
> > - return IRQ_HANDLED;
> > + return blocking_notifier_chain_register(&ckdev->ec->event_notifier,
> > + &ckdev->notifier);
> > }
> >
> > -static int cros_ec_keyb_open(struct input_dev *dev)
> > +static void cros_ec_keyb_close(struct input_dev *dev)
> > {
> > struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> > - struct cros_ec_device *ec = ckdev->ec;
> >
> > - return request_threaded_irq(ec->irq, NULL, cros_ec_keyb_irq,
> > - IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> > - "cros_ec_keyb", ckdev);
> > + blocking_notifier_chain_unregister(&ckdev->ec->event_notifier,
> > + &ckdev->notifier);
> > }
> >
> > -static void cros_ec_keyb_close(struct input_dev *dev)
> > +static int cros_ec_keyb_work(struct notifier_block *nb,
> > + unsigned long queued_during_suspend, void *_notify)
> > {
> > - struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> > - struct cros_ec_device *ec = ckdev->ec;
> > + struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
> > + notifier);
> >
> > - free_irq(ec->irq, ckdev);
> > + if (ckdev->ec->event_data.event_type != EC_MKBP_EVENT_KEY_MATRIX)
> > + return NOTIFY_DONE;
> > + /*
> > + * If EC is not the wake source, discard key state changes during
> > + * suspend.
> > + */
> > + if (queued_during_suspend)
> > + return NOTIFY_OK;
> > + if (ckdev->ec->event_size != ckdev->cols) {
> > + dev_err(ckdev->dev,
> > + "Discarded incomplete key matrix event.\n");
> > + return NOTIFY_OK;
> > + }
> > + cros_ec_keyb_process(ckdev, ckdev->ec->event_data.data.key_matrix,
> > + ckdev->ec->event_size);
> > + return NOTIFY_OK;
> > }
> >
> > /*
> > @@ -266,12 +246,8 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
> > if (!idev)
> > return -ENOMEM;
> >
> > - if (!ec->irq) {
> > - dev_err(dev, "no EC IRQ specified\n");
> > - return -EINVAL;
> > - }
> > -
> > ckdev->ec = ec;
> > + ckdev->notifier.notifier_call = cros_ec_keyb_work;
> > ckdev->dev = dev;
> > dev_set_drvdata(&pdev->dev, ckdev);
> >
> > @@ -312,54 +288,6 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
> > return 0;
> > }
> >
> > -#ifdef CONFIG_PM_SLEEP
> > -/* Clear any keys in the buffer */
> > -static void cros_ec_keyb_clear_keyboard(struct cros_ec_keyb *ckdev)
> > -{
> > - uint8_t old_state[ckdev->cols];
> > - uint8_t new_state[ckdev->cols];
> > - unsigned long duration;
> > - int i, ret;
> > -
> > - /*
> > - * Keep reading until we see that the scan state does not change.
> > - * That indicates that we are done.
> > - *
> > - * Assume that the EC keyscan buffer is at most 32 deep.
> > - */
> > - duration = jiffies;
> > - ret = cros_ec_keyb_get_state(ckdev, new_state);
> > - for (i = 1; !ret && i < 32; i++) {
> > - memcpy(old_state, new_state, sizeof(old_state));
> > - ret = cros_ec_keyb_get_state(ckdev, new_state);
> > - if (0 == memcmp(old_state, new_state, sizeof(old_state)))
> > - break;
> > - }
> > - duration = jiffies - duration;
> > - dev_info(ckdev->dev, "Discarded %d keyscan(s) in %dus\n", i,
> > - jiffies_to_usecs(duration));
> > -}
> > -
> > -static int cros_ec_keyb_resume(struct device *dev)
> > -{
> > - struct cros_ec_keyb *ckdev = dev_get_drvdata(dev);
> > -
> > - /*
> > - * When the EC is not a wake source, then it could not have caused the
> > - * resume, so we clear the EC's key scan buffer. If the EC was a
> > - * wake source (e.g. the lid is open and the user might press a key to
> > - * wake) then the key scan buffer should be preserved.
> > - */
> > - if (!ckdev->ec->was_wake_device)
> > - cros_ec_keyb_clear_keyboard(ckdev);
> > -
> > - return 0;
> > -}
> > -
> > -#endif
> > -
> > -static SIMPLE_DEV_PM_OPS(cros_ec_keyb_pm_ops, NULL, cros_ec_keyb_resume);
> > -
> > #ifdef CONFIG_OF
> > static const struct of_device_id cros_ec_keyb_of_match[] = {
> > { .compatible = "google,cros-ec-keyb" },
> > @@ -373,7 +301,6 @@ static struct platform_driver cros_ec_keyb_driver = {
> > .driver = {
> > .name = "cros-ec-keyb",
> > .of_match_table = of_match_ptr(cros_ec_keyb_of_match),
> > - .pm = &cros_ec_keyb_pm_ops,
> > },
> > };
> >
> > diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> > index 0eee63542038..fbe78b819fdd 100644
> > --- a/drivers/mfd/cros_ec.c
> > +++ b/drivers/mfd/cros_ec.c
> > @@ -23,6 +23,7 @@
> > #include <linux/module.h>
> > #include <linux/mfd/core.h>
> > #include <linux/mfd/cros_ec.h>
> > +#include <asm/unaligned.h>
> >
> > #define CROS_EC_DEV_EC_INDEX 0
> > #define CROS_EC_DEV_PD_INDEX 1
> > @@ -49,11 +50,28 @@ static const struct mfd_cell ec_pd_cell = {
> > .pdata_size = sizeof(pd_p),
> > };
> >
> > +static irqreturn_t ec_irq_thread(int irq, void *data)
> > +{
> > + struct cros_ec_device *ec_dev = data;
> > + int ret;
> > +
> > + if (device_may_wakeup(ec_dev->dev))
> > + pm_wakeup_event(ec_dev->dev, 0);
> > +
> > + ret = cros_ec_get_next_event(ec_dev);
> > + if (ret > 0)
> > + blocking_notifier_call_chain(&ec_dev->event_notifier,
> > + 0, ec_dev);
> > + return IRQ_HANDLED;
> > +}
> > +
> > int cros_ec_register(struct cros_ec_device *ec_dev)
> > {
> > struct device *dev = ec_dev->dev;
> > int err = 0;
> >
> > + BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
> > +
> > ec_dev->max_request = sizeof(struct ec_params_hello);
> > ec_dev->max_response = sizeof(struct ec_response_get_protocol_info);
> > ec_dev->max_passthru = 0;
> > @@ -70,13 +88,24 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
> >
> > cros_ec_query_all(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);
> > + if (err) {
> > + dev_err(dev, "request irq %d: error %d\n",
> > + ec_dev->irq, err);
> > + return err;
> > + }
> > + }
> > +
> > err = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, &ec_cell, 1,
> > NULL, ec_dev->irq, NULL);
> > if (err) {
> > dev_err(dev,
> > "Failed to register Embedded Controller subdevice %d\n",
> > err);
> > - return err;
> > + goto fail_mfd;
> > }
> >
> > if (ec_dev->max_passthru) {
> > @@ -94,7 +123,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
> > dev_err(dev,
> > "Failed to register Power Delivery subdevice %d\n",
> > err);
> > - return err;
> > + goto fail_mfd;
> > }
> > }
> >
> > @@ -103,13 +132,18 @@ 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");
> > - return err;
> > + goto fail_mfd;
> > }
> > }
> >
> > dev_info(dev, "Chrome EC device registered\n");
> >
> > return 0;
> > +
> > +fail_mfd:
> > + if (ec_dev->irq)
> > + free_irq(ec_dev->irq, ec_dev);
> > + return err;
> > }
> > EXPORT_SYMBOL(cros_ec_register);
> >
> > @@ -136,13 +170,30 @@ int cros_ec_suspend(struct cros_ec_device *ec_dev)
> > }
> > EXPORT_SYMBOL(cros_ec_suspend);
> >
> > +static void cros_ec_drain_events(struct cros_ec_device *ec_dev)
> > +{
> > + while (cros_ec_get_next_event(ec_dev) > 0)
> > + blocking_notifier_call_chain(&ec_dev->event_notifier,
> > + 1, ec_dev);
> > +}
> > +
> > int cros_ec_resume(struct cros_ec_device *ec_dev)
> > {
> > enable_irq(ec_dev->irq);
> >
> > + /*
> > + * In some case, we need to distinguish events that occur during
> > + * suspend if the EC is not a wake source. For example, keypresses
> > + * during suspend should be discarded if it does not wake the system.
> > + *
> > + * If the EC is not a wake source, drain the event queue and mark them
> > + * as "queued during suspend".
> > + */
> > if (ec_dev->wake_enabled) {
> > disable_irq_wake(ec_dev->irq);
> > ec_dev->wake_enabled = 0;
> > + } else {
> > + cros_ec_drain_events(ec_dev);
> > }
> >
> > return 0;
> > diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> > index 990308ca384f..6e138e2333f3 100644
> > --- a/drivers/platform/chrome/cros_ec_proto.c
> > +++ b/drivers/platform/chrome/cros_ec_proto.c
> > @@ -19,6 +19,7 @@
> > #include <linux/device.h>
> > #include <linux/module.h>
> > #include <linux/slab.h>
> > +#include <asm/unaligned.h>
> >
> > #define EC_COMMAND_RETRIES 50
> >
> > @@ -234,11 +235,44 @@ static int cros_ec_host_command_proto_query_v2(struct cros_ec_device *ec_dev)
> > return ret;
> > }
> >
> > +static int cros_ec_get_host_command_version_mask(struct cros_ec_device *ec_dev,
> > + u16 cmd, u32 *mask)
> > +{
> > + struct ec_params_get_cmd_versions *pver;
> > + struct ec_response_get_cmd_versions *rver;
> > + struct cros_ec_command *msg;
> > + int ret;
> > +
> > + msg = kmalloc(sizeof(*msg) + max(sizeof(rver), sizeof(pver)),
> > + GFP_KERNEL);
> Victor's version in https://chromium-review.googlesource.com/272954
> looks cleaner: no malloc, no need to cast rver.
> > + if (!msg)
> > + return -ENOMEM;
> > +
> > + msg->version = 0;
> > + msg->command = EC_CMD_GET_CMD_VERSIONS;
> > + msg->insize = sizeof(rver);
> > + msg->outsize = sizeof(pver);
> > +
> > + pver = (struct ec_params_get_cmd_versions *)msg->data;
> > + pver->cmd = cmd;
> > +
> > + ret = cros_ec_cmd_xfer(ec_dev, msg);
> > + if (ret > 0) {
> > + rver = (struct ec_response_get_cmd_versions *)msg->data;
> > + *mask = rver->version_mask;
> > + }
> > +
> > + kfree(msg);
> > +
> > + return ret;
> > +}
> > +
> > int cros_ec_query_all(struct cros_ec_device *ec_dev)
> > {
> > struct device *dev = ec_dev->dev;
> > struct cros_ec_command *proto_msg;
> > struct ec_response_get_protocol_info *proto_info;
> > + u32 ver_mask = 0;
> > int ret;
> >
> > proto_msg = kzalloc(sizeof(*proto_msg) + sizeof(*proto_info),
> > @@ -328,6 +362,15 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
> > goto exit;
> > }
> >
> > + /* Probe if MKBP event is supported */
> > + ret = cros_ec_get_host_command_version_mask(ec_dev,
> > + EC_CMD_GET_NEXT_EVENT,
> > + &ver_mask);
> > + if (ret < 0 || ver_mask == 0)
> > + ec_dev->mkbp_event_supported = 0;
> > + else
> > + ec_dev->mkbp_event_supported = 1;
> > +
> > exit:
> > kfree(proto_msg);
> > return ret;
> > @@ -380,3 +423,62 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
> > return ret;
> > }
> > EXPORT_SYMBOL(cros_ec_cmd_xfer);
> > +
> > +static int get_next_event(struct cros_ec_device *ec_dev)
> > +{
> > + struct cros_ec_command *msg;
> > + int ret;
> > +
> > + msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->event_data), GFP_KERNEL);
> Same grip about malloc(). This function will be called very often when
> sensors are used.
> > + if (!msg)
> > + return -ENOMEM;
> > +
> > + msg->version = 0;
> > + msg->command = EC_CMD_GET_NEXT_EVENT;
> > + msg->insize = sizeof(ec_dev->event_data);
> > + msg->outsize = 0;
> > +
> > + ret = cros_ec_cmd_xfer(ec_dev, msg);
> > + if (ret > 0) {
> > + ec_dev->event_size = ret - 1;
> > + memcpy(&ec_dev->event_data, msg->data,
> > + sizeof(ec_dev->event_data));
> > + }
> > +
> > + kfree(msg);
> > +
> > + return ret;
> > +}
> > +
> > +static int get_keyboard_state_event(struct cros_ec_device *ec_dev)
> > +{
> > + struct cros_ec_command *msg;
> > +
> > + msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->event_data.data),
> Given this command will be used on every key press, it is better to
> pre-allocate data (like in
> https://chromium-review.googlesource.com/276768) and use msg on the
> stack like Victor's changes.
> > + GFP_KERNEL);
> > + if (!msg)
> > + return -ENOMEM;
> > +
> > + msg->version = 0;
> > + msg->command = EC_CMD_MKBP_STATE;
> > + msg->insize = sizeof(ec_dev->event_data.data);
> > + msg->outsize = 0;
> > +
> > + ec_dev->event_size = cros_ec_cmd_xfer(ec_dev, msg);
> > + ec_dev->event_data.event_type = EC_MKBP_EVENT_KEY_MATRIX;
> > + memcpy(&ec_dev->event_data.data, msg->data,
> > + sizeof(ec_dev->event_data.data));
> > +
> > + kfree(msg);
> > +
> > + return ec_dev->event_size;
> > +}
> > +
> > +int cros_ec_get_next_event(struct cros_ec_device *ec_dev)
> > +{
> > + if (ec_dev->mkbp_event_supported)
> > + return get_next_event(ec_dev);
> > + else
> > + return get_keyboard_state_event(ec_dev);
> > +}
> > +EXPORT_SYMBOL(cros_ec_get_next_event);
> > diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> > index a677c2bd485c..8a8fa75178c6 100644
> > --- a/include/linux/mfd/cros_ec.h
> > +++ b/include/linux/mfd/cros_ec.h
> > @@ -72,6 +72,24 @@ struct cros_ec_command {
> > uint8_t data[0];
> > };
> >
> > +/*
> > + * event_data is used by keyboard or event notifier:
> > + * event_data format:
> > + * If MKBP protocol is supported:
> > + * 0 1
> > + * +-----------+--------------------------------
> > + * | type | payload
> > + * +-----------+--------------------------------
> > + * |HOST_EVENT | EVENT (32 bit)
> > + * |KEY_MATRIX | Keyboard keys pressed.
> > + * |SENSOR_FIFO| Sensors FIFO information.
> > + *
> > + * Otherwise:
> > + * 0 1
> > + * +-----------+--------------------------------
> > + * |Unused | Keyboard keys pressed.
> > + */
> > +
> > /**
> > * struct cros_ec_device - Information about a ChromeOS EC device
> > *
> > @@ -107,6 +125,9 @@ struct cros_ec_command {
> > * should check msg.result for the EC's result code.
> > * @pkt_xfer: send packet to EC and get response
> > * @lock: one transaction at a time
> > + * @event_notifier: interrupt event notifier for transport devices.
> > + * @event_data: raw payload transferred with the MKBP event.
> > + * @event_size: size in bytes of the event data.
> > */
> > struct cros_ec_device {
> >
> > @@ -135,6 +156,11 @@ struct cros_ec_device {
> > int (*pkt_xfer)(struct cros_ec_device *ec,
> > struct cros_ec_command *msg);
> > struct mutex lock;
> > + bool mkbp_event_supported;
> > + struct blocking_notifier_head event_notifier;
> > +
> > + struct ec_response_get_next_event event_data;
> > + int event_size;
> > };
> >
> > /* struct cros_ec_platform - ChromeOS EC platform information
> > @@ -252,9 +278,27 @@ int cros_ec_register(struct cros_ec_device *ec_dev);
> > */
> > int cros_ec_query_all(struct cros_ec_device *ec_dev);
> >
> > +/**
> > + * cros_ec_get_next_event - Fetch next event from the ChromeOS EC
> > + *
> > + * @ec_dev: Device to fetch event from
> > + * @return 0 if ok, -ve on error
> > + */
> > +int cros_ec_get_next_event(struct cros_ec_device *ec_dev);
> > +
> > /* sysfs stuff */
> > extern struct attribute_group cros_ec_attr_group;
> > extern struct attribute_group cros_ec_lightbar_attr_group;
> > extern struct attribute_group cros_ec_vbc_attr_group;
> >
> > +/**
> > + * cros_ec_get_host_event - Return a mask of event set by the EC.
> > + *
> > + * When MKBP is supported, when the EC raises an interrupt,
> > + * We collect the events raised and call the functions in the ec notifier.
> > + *
> > + * This function is a helper to know which events are raised.
> > + */
> > +uint32_t cros_ec_get_host_event(struct cros_ec_device *ec_dev);
> > +
> > #endif /* __LINUX_MFD_CROS_EC_H */
> > diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
> > index 13b630c10d4c..d86526f0bd8e 100644
> > --- a/include/linux/mfd/cros_ec_commands.h
> > +++ b/include/linux/mfd/cros_ec_commands.h
> > @@ -1762,6 +1762,40 @@ struct ec_result_keyscan_seq_ctrl {
> > };
> > } __packed;
> >
> > +/*
> > + * Get the next pending MKBP event.
> > + *
> > + * Returns EC_RES_UNAVAILABLE if there is no event pending.
> > + */
> > +#define EC_CMD_GET_NEXT_EVENT 0x67
> > +
> > +enum ec_mkbp_event {
> > + /* Keyboard matrix changed. The event data is the new matrix state. */
> > + EC_MKBP_EVENT_KEY_MATRIX = 0,
> > +
> > + /* New host event. The event data is 4 bytes of host event flags. */
> > + EC_MKBP_EVENT_HOST_EVENT = 1,
> > +
> > + /* New Sensor FIFO data. The event data is fifo_info structure. */
> > + EC_MKBP_EVENT_SENSOR_FIFO = 2,
> > +
> > + /* Number of MKBP events */
> > + EC_MKBP_EVENT_COUNT,
> > +};
> > +
> > +union ec_response_get_next_data {
> > + uint8_t key_matrix[13];
> > +
> > + /* Unaligned */
> > + uint32_t host_event;
> > +} __packed;
> > +
> > +struct ec_response_get_next_event {
> > + uint8_t event_type;
> > + /* Followed by event data if any */
> > + union ec_response_get_next_data data;
> > +} __packed;
> > +
> > /*****************************************************************************/
> > /* Temperature sensor commands */
> >
> >
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 3/6] power_supply: Add types for USB Type C and PD chargers
2016-02-05 18:38 ` Benson Leung
@ 2016-02-11 10:00 ` Tomeu Vizoso
2016-02-11 15:05 ` Benson Leung
0 siblings, 1 reply; 23+ messages in thread
From: Tomeu Vizoso @ 2016-02-11 10:00 UTC (permalink / raw)
To: Benson Leung
Cc: linux-kernel, Sameer Nanda, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
linux-pm
On 5 February 2016 at 19:38, Benson Leung <bleung@chromium.org> wrote:
> Thanks for putting this up Tomeu!
No problem, thanks for the encouragement.
> On Fri, Feb 5, 2016 at 5:32 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>> From: Benson Leung <bleung@chromium.org>
>>
>> This adds power supply types for USB chargers defined in
>> the USB Type-C Specification 1.1 and in the
>> USB Power Delivery Specification Revision 2.0 V1.1.
>>
>> The following are added :
>> POWER_SUPPLY_TYPE_USB_TYPE_C, /* Type C Port */
>> POWER_SUPPLY_TYPE_USB_PD, /* Type C Power Delivery Port */
>> POWER_SUPPLY_TYPE_USB_PD_DRP, /* Type C PD Dual Role Port */
>
> I authored this patch, but in looking back, the comments should be tweaked.
> USB_PD and USB_PD_DRP are not necessarily Type-C ports, as you may
> have USB_PD ports that are either Type-A or Type-B as well.
>
> Could you change these to be "USB Power Delivery Port" and "USB PD
> Dual Role Port"?
I have gone with omitting the USB prefix, following the lead of the
existing entries, do you agree with that?
Thanks,
Tomeu
>> Signed-off-by: Benson Leung <bleung@chromium.org>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> Reviewed-by: Alec Berg <alecaberg@chromium.org>
>> Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
>> Reviewed-by: Todd Broch <tbroch@chromium.org>
>> ---
>>
>> drivers/power/power_supply_sysfs.c | 3 ++-
>> include/linux/power_supply.h | 3 +++
>> 2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
>> index ed2d7fd0c734..80fed98832f9 100644
>> --- a/drivers/power/power_supply_sysfs.c
>> +++ b/drivers/power/power_supply_sysfs.c
>> @@ -45,7 +45,8 @@ static ssize_t power_supply_show_property(struct device *dev,
>> char *buf) {
>> static char *type_text[] = {
>> "Unknown", "Battery", "UPS", "Mains", "USB",
>> - "USB_DCP", "USB_CDP", "USB_ACA"
>> + "USB_DCP", "USB_CDP", "USB_ACA", "USB_C",
>> + "USB_PD", "USB_PD_DRP"
>> };
>> static char *status_text[] = {
>> "Unknown", "Charging", "Discharging", "Not charging", "Full"
>> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
>> index ef9f1592185d..206c3384c6fa 100644
>> --- a/include/linux/power_supply.h
>> +++ b/include/linux/power_supply.h
>> @@ -163,6 +163,9 @@ enum power_supply_type {
>> POWER_SUPPLY_TYPE_USB_DCP, /* Dedicated Charging Port */
>> POWER_SUPPLY_TYPE_USB_CDP, /* Charging Downstream Port */
>> POWER_SUPPLY_TYPE_USB_ACA, /* Accessory Charger Adapters */
>> + POWER_SUPPLY_TYPE_USB_TYPE_C, /* Type C Port */
>> + POWER_SUPPLY_TYPE_USB_PD, /* Type C Power Delivery Port */
>> + POWER_SUPPLY_TYPE_USB_PD_DRP, /* Type C PD Dual Role Port */
>
> Same change as in commit message. Thanks!
>
>> };
>>
>> enum power_supply_notifier_events {
>
>
>
>
> --
> Benson Leung
> Senior Software Engineer, Chrom* OS
> bleung@chromium.org
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 2/6] mfd: cros_ec: Add MKBP event support
[not found] ` <CAMHSBOXRvC7U_KWeg+rX3Qymaos0-3FN6AuaXT2cyP=gAN47mA@mail.gmail.com>
@ 2016-02-11 14:52 ` Tomeu Vizoso
0 siblings, 0 replies; 23+ messages in thread
From: Tomeu Vizoso @ 2016-02-11 14:52 UTC (permalink / raw)
To: gwendal
Cc: Linux Kernel, Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Vic Yang, Olof Johansson, linux-input, Dmitry Torokhov,
Lee Jones
On 02/05/2016 06:24 PM, Gwendal Grignou wrote:
> We should not used kmalloc when get events, these functions are called
> quite often.
> Gwendal.
>
> On Fri, Feb 5, 2016 at 5:32 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com
> <mailto:tomeu.vizoso@collabora.com>> wrote:
>
> +static int cros_ec_get_host_command_version_mask(struct
> cros_ec_device *ec_dev,
> + u16 cmd, u32 *mask)
> +{
> + struct ec_params_get_cmd_versions *pver;
> + struct ec_response_get_cmd_versions *rver;
> + struct cros_ec_command *msg;
> + int ret;
> +
> + msg = kmalloc(sizeof(*msg) + max(sizeof(rver), sizeof(pver)),
> + GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
>
> Victor's version in https://chromium-review.googlesource.com/272954
> looks cleaner: no malloc, no need to cast rver.
I agree that it looks cleaner, but how would you allocate the payload at
build time if it has to be max(sizeof(*pver), sizeof(*rver))?
> +static int get_next_event(struct cros_ec_device *ec_dev)
> +{
> + struct cros_ec_command *msg;
> + int ret;
> +
> + msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->event_data),
> GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
>
> Same grip about malloc(). This function will be called very often when
> sensors are used.
Ok.
> +static int get_keyboard_state_event(struct cros_ec_device *ec_dev)
> +{
> + struct cros_ec_command *msg;
> +
> + msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->event_data.data),
> + GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
>
> Given this command will be used on every key press, it is better to
> pre-allocate data (like
> in https://chromium-review.googlesource.com/276768) and use msg on the
> stack like Victor's changes.
Ok.
Thanks,
Tomeu
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 2/6] mfd: cros_ec: Add MKBP event support
@ 2016-02-11 14:52 ` Tomeu Vizoso
0 siblings, 0 replies; 23+ messages in thread
From: Tomeu Vizoso @ 2016-02-11 14:52 UTC (permalink / raw)
Cc: Linux Kernel, Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Vic Yang, Olof Johansson, linux-input, Dmitry Torokhov,
Lee Jones
On 02/05/2016 06:24 PM, Gwendal Grignou wrote:
> We should not used kmalloc when get events, these functions are called
> quite often.
> Gwendal.
>
> On Fri, Feb 5, 2016 at 5:32 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com
> <mailto:tomeu.vizoso@collabora.com>> wrote:
>
> +static int cros_ec_get_host_command_version_mask(struct
> cros_ec_device *ec_dev,
> + u16 cmd, u32 *mask)
> +{
> + struct ec_params_get_cmd_versions *pver;
> + struct ec_response_get_cmd_versions *rver;
> + struct cros_ec_command *msg;
> + int ret;
> +
> + msg = kmalloc(sizeof(*msg) + max(sizeof(rver), sizeof(pver)),
> + GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
>
> Victor's version in https://chromium-review.googlesource.com/272954
> looks cleaner: no malloc, no need to cast rver.
I agree that it looks cleaner, but how would you allocate the payload at
build time if it has to be max(sizeof(*pver), sizeof(*rver))?
> +static int get_next_event(struct cros_ec_device *ec_dev)
> +{
> + struct cros_ec_command *msg;
> + int ret;
> +
> + msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->event_data),
> GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
>
> Same grip about malloc(). This function will be called very often when
> sensors are used.
Ok.
> +static int get_keyboard_state_event(struct cros_ec_device *ec_dev)
> +{
> + struct cros_ec_command *msg;
> +
> + msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->event_data.data),
> + GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
>
> Given this command will be used on every key press, it is better to
> pre-allocate data (like
> in https://chromium-review.googlesource.com/276768) and use msg on the
> stack like Victor's changes.
Ok.
Thanks,
Tomeu
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 3/6] power_supply: Add types for USB Type C and PD chargers
2016-02-11 10:00 ` Tomeu Vizoso
@ 2016-02-11 15:05 ` Benson Leung
0 siblings, 0 replies; 23+ messages in thread
From: Benson Leung @ 2016-02-11 15:05 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: linux-kernel, Sameer Nanda, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
linux-pm
On Thu, Feb 11, 2016 at 2:00 AM, Tomeu Vizoso
<tomeu.vizoso@collabora.com> wrote:
>> Could you change these to be "USB Power Delivery Port" and "USB PD
>> Dual Role Port"?
>
> I have gone with omitting the USB prefix, following the lead of the
> existing entries, do you agree with that?
Sounds good to me!
Thanks,
Benson
--
Benson Leung
Senior Software Engineer, Chrom* OS
bleung@chromium.org
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 2/6] mfd: cros_ec: Add MKBP event support
2016-02-11 14:52 ` Tomeu Vizoso
(?)
@ 2016-02-11 18:49 ` Gwendal Grignou
-1 siblings, 0 replies; 23+ messages in thread
From: Gwendal Grignou @ 2016-02-11 18:49 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: Gwendal Grignou, Linux Kernel, Sameer Nanda, Benson Leung,
Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Vic Yang,
Olof Johansson, linux-input, Dmitry Torokhov, Lee Jones
On Thu, Feb 11, 2016 at 6:52 AM, Tomeu Vizoso
<tomeu.vizoso@collabora.com> wrote:
>
> On 02/05/2016 06:24 PM, Gwendal Grignou wrote:
> > We should not used kmalloc when get events, these functions are called
> > quite often.
> > Gwendal.
> >
> > On Fri, Feb 5, 2016 at 5:32 AM, Tomeu Vizoso <tomeu.vizoso@collabora.com
> > <mailto:tomeu.vizoso@collabora.com>> wrote:
> >
> > +static int cros_ec_get_host_command_version_mask(struct
> > cros_ec_device *ec_dev,
> > + u16 cmd, u32 *mask)
> > +{
> > + struct ec_params_get_cmd_versions *pver;
> > + struct ec_response_get_cmd_versions *rver;
> > + struct cros_ec_command *msg;
> > + int ret;
> > +
> > + msg = kmalloc(sizeof(*msg) + max(sizeof(rver), sizeof(pver)),
> > + GFP_KERNEL);
> > + if (!msg)
> > + return -ENOMEM;
> >
> > Victor's version in https://chromium-review.googlesource.com/272954
> > looks cleaner: no malloc, no need to cast rver.
>
> I agree that it looks cleaner, but how would you allocate the payload at
> build time if it has to be max(sizeof(*pver), sizeof(*rver))?
For this one, given cros_ec_get_host_command_version_mask() is called
once, a kmalloc is fine.
Gwendal.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 5/6] platform/chrome: Register USB PD charger device
2016-02-10 16:46 ` Lee Jones
@ 2016-02-12 11:06 ` Tomeu Vizoso
0 siblings, 0 replies; 23+ messages in thread
From: Tomeu Vizoso @ 2016-02-12 11:06 UTC (permalink / raw)
To: Lee Jones
Cc: linux-kernel, Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Olof Johansson
On 10 February 2016 at 17:46, Lee Jones <lee.jones@linaro.org> wrote:
> On Fri, 05 Feb 2016, Tomeu Vizoso wrote:
>
>> Check if a EC considers EC_CMD_USB_PD_PORTS a valid command and register
>> a USB PD charger device if so. This check is needed for older versions
>> of the ChromeOS EC firmware that don't support the EC_CMD_GET_FEATURES
>> command.
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> ---
>>
>> drivers/mfd/cros_ec.c | 15 +++++++++++++++
>> drivers/platform/chrome/cros_ec_dev.c | 34 ++++++++++++++++++++++++++++++++++
>> include/linux/mfd/cros_ec.h | 2 ++
>> 3 files changed, 51 insertions(+)
>>
>> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
>> index fbe78b819fdd..20ca9794d2f3 100644
>> --- a/drivers/mfd/cros_ec.c
>> +++ b/drivers/mfd/cros_ec.c
>> @@ -202,5 +202,20 @@ EXPORT_SYMBOL(cros_ec_resume);
>>
>> #endif
>>
>> +static const struct mfd_cell cros_usb_pd_charger_devs[] = {
>> + {
>> + .name = "cros-usb-pd-charger",
>> + .id = -1,
>> + },
>> +};
>> +
>> +int cros_ec_usb_pd_charger_register(struct device *dev)
>> +{
>> + return mfd_add_devices(dev, 0, cros_usb_pd_charger_devs,
>> + ARRAY_SIZE(cros_usb_pd_charger_devs),
>> + NULL, 0, NULL);
>> +}
>> +EXPORT_SYMBOL(cros_ec_usb_pd_charger_register);
>
> I'm not keen on this idea at all.
>
> You're calling cros_ec_usb_pd_charger_register() from a device you
> registered from this same source file. Seems very incestuous and
> hacky.
>
> It's bad enough that we're calling into the platform driver from here
> already, but seeing as we are, just extend the call to
> cros_ec_query_all() to suit your purposes.
But cros_ec_query_all() is querying the EC that is directly accessible
from the AP, while the EC that takes care of PD is the one registered
from ec_pd_cell (it sits behind the other EC).
Maybe it's just that cros_ec_usb_pd_charger_register() belongs to cros_ec_dev.c?
Thanks,
Tomeu
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 4/6] power: cros_usbpd-charger: Add EC-based USB PD charger driver
2016-02-10 16:49 ` Lee Jones
@ 2016-02-12 11:07 ` Tomeu Vizoso
0 siblings, 0 replies; 23+ messages in thread
From: Tomeu Vizoso @ 2016-02-12 11:07 UTC (permalink / raw)
To: Lee Jones
Cc: linux-kernel, Sameer Nanda, Benson Leung, Enric Balletbò,
Vic Yang, Vincent Palatin, Randall Spangler, Gwendal Grignou,
Shawn Nematbakhsh, linux-pm, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Olof Johansson
On 10 February 2016 at 17:49, Lee Jones <lee.jones@linaro.org> wrote:
> On Fri, 05 Feb 2016, Tomeu Vizoso wrote:
>
>> From: Sameer Nanda <snanda@chromium.org>
>>
>> This driver exposes the charger functionality in the PD EC to userspace.
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> Cc: Sameer Nanda <snanda@chromium.org>
>> Cc: Benson Leung <bleung@chromium.org>
>> Cc: Shawn Nematbakhsh <shawnn@chromium.org>
>> ---
>>
>> drivers/platform/chrome/cros_ec_proto.c | 37 ++
>> drivers/power/Kconfig | 10 +
>> drivers/power/Makefile | 1 +
>> drivers/power/cros_usbpd-charger.c | 907 ++++++++++++++++++++++++++++++++
>> include/linux/mfd/cros_ec.h | 28 +
>> include/linux/mfd/cros_ec_commands.h | 324 +++++++++++-
>> 6 files changed, 1302 insertions(+), 5 deletions(-)
>> create mode 100644 drivers/power/cros_usbpd-charger.c
>
> A 1300 line patch crossing 3 subsystems is not the way you should be
> submitting code. Please split these up into separate commits. The
> only time you should squash all this functionality into a single patch
> is to mitigate otherwise unavoidable build issues. I do not believe
> that's the case here.
Agreed, sorry about that.
Thanks,
Tomeu
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2016-02-12 11:07 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-05 13:32 [PATCH v1 0/6] EC-based USB Power Delivery support for Chrome machines Tomeu Vizoso
2016-02-05 13:32 ` [PATCH v1 1/6] mfd: cros_ec: small kerneldoc fix Tomeu Vizoso
2016-02-05 18:46 ` Benson Leung
2016-02-10 16:25 ` Lee Jones
2016-02-05 13:32 ` [PATCH v1 2/6] mfd: cros_ec: Add MKBP event support Tomeu Vizoso
2016-02-10 17:41 ` Gwendal Grignou
2016-02-11 9:15 ` Lee Jones
2016-02-11 9:15 ` Lee Jones
[not found] ` <CAMHSBOXRvC7U_KWeg+rX3Qymaos0-3FN6AuaXT2cyP=gAN47mA@mail.gmail.com>
2016-02-11 14:52 ` Tomeu Vizoso
2016-02-11 14:52 ` Tomeu Vizoso
2016-02-11 18:49 ` Gwendal Grignou
2016-02-05 13:32 ` [PATCH v1 3/6] power_supply: Add types for USB Type C and PD chargers Tomeu Vizoso
2016-02-05 18:38 ` Benson Leung
2016-02-11 10:00 ` Tomeu Vizoso
2016-02-11 15:05 ` Benson Leung
2016-02-05 13:32 ` [PATCH v1 4/6] power: cros_usbpd-charger: Add EC-based USB PD charger driver Tomeu Vizoso
2016-02-10 16:49 ` Lee Jones
2016-02-12 11:07 ` Tomeu Vizoso
2016-02-05 13:33 ` [PATCH v1 5/6] platform/chrome: Register USB PD charger device Tomeu Vizoso
2016-02-10 16:46 ` Lee Jones
2016-02-12 11:06 ` Tomeu Vizoso
2016-02-05 13:33 ` [PATCH v1 6/6] platform/chrome: Check the USB PD feature before creating a charger Tomeu Vizoso
2016-02-10 16:28 ` Lee Jones
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.