This patch set adds a basic implementation of the USB-C connector class for
devices using the ChromeOS EC. On ACPI devices an additional ACPI driver is
necessary to receive USB-C PD host events from the PD EC device "GOOG0003".
Incidentally, this ACPI driver adds notifications for events that
cros-usbpd-charger has been missing, so fix that while we're at it.
Jon Flatley (3):
platform: chrome: Add cros-ec-usbpd-notify driver
power: supply: cros-ec-usbpd-charger: Fix host events
platform: chrome: Added cros-ec-typec driver
drivers/mfd/cros_ec_dev.c | 7 +-
drivers/platform/chrome/Kconfig | 20 +
drivers/platform/chrome/Makefile | 2 +
drivers/platform/chrome/cros_ec_typec.c | 457 ++++++++++++++++++
.../platform/chrome/cros_ec_usbpd_notify.c | 156 ++++++
drivers/power/supply/Kconfig | 2 +-
drivers/power/supply/cros_usbpd-charger.c | 45 +-
.../platform_data/cros_ec_usbpd_notify.h | 40 ++
8 files changed, 696 insertions(+), 33 deletions(-)
create mode 100644 drivers/platform/chrome/cros_ec_typec.c
create mode 100644 drivers/platform/chrome/cros_ec_usbpd_notify.c
create mode 100644 include/linux/platform_data/cros_ec_usbpd_notify.h
--
2.24.0.432.g9d3f5f5b63-goog
ChromiumOS uses ACPI device with HID "GOOG0003" for power delivery
related events. The existing cros-usbpd-charger driver relies on these
events without ever actually receiving them on ACPI platforms. This is
because in the ChromeOS kernel trees, the GOOG0003 device is owned by an
ACPI driver that offers firmware updates to USB-C chargers.
Let's introduce a new platform driver under cros-ec, the ChromeOS
embedded controller, that handles these PD events and dispatches them
appropriately over a notifier chain to all drivers that use them.
Signed-off-by: Jon Flatley <[email protected]>
---
drivers/platform/chrome/Kconfig | 9 +
drivers/platform/chrome/Makefile | 1 +
.../platform/chrome/cros_ec_usbpd_notify.c | 156 ++++++++++++++++++
.../platform_data/cros_ec_usbpd_notify.h | 40 +++++
4 files changed, 206 insertions(+)
create mode 100644 drivers/platform/chrome/cros_ec_usbpd_notify.c
create mode 100644 include/linux/platform_data/cros_ec_usbpd_notify.h
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index ee5f08ea57b6..d4a55b64bc29 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -118,6 +118,15 @@ config CROS_EC_SPI
response time cannot be guaranteed, we support ignoring
'pre-amble' bytes before the response actually starts.
+config CROS_EC_USBPD_NOTIFY
+ tristate "ChromeOS USB-C power delivery event notifier"
+ depends on CROS_EC
+ help
+ If you say Y here, you get support for USB-C PD event notifications
+ from the ChromeOS EC. On ACPI platorms this driver will bind to the
+ GOOG0003 ACPI device, and on non-ACPI platform this driver will match
+ "google,cros-ec-pd-update" in device tree.
+
config CROS_EC_LPC
tristate "ChromeOS Embedded Controller (LPC)"
depends on CROS_EC && ACPI && (X86 || COMPILE_TEST)
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 477ec3d1d1c9..efa355ab526f 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -21,5 +21,6 @@ obj-$(CONFIG_CROS_EC_VBC) += cros_ec_vbc.o
obj-$(CONFIG_CROS_EC_DEBUGFS) += cros_ec_debugfs.o
obj-$(CONFIG_CROS_EC_SYSFS) += cros_ec_sysfs.o
obj-$(CONFIG_CROS_USBPD_LOGGER) += cros_usbpd_logger.o
+obj-$(CONFIG_CROS_EC_USBPD_NOTIFY) += cros_ec_usbpd_notify.o
obj-$(CONFIG_WILCO_EC) += wilco_ec/
diff --git a/drivers/platform/chrome/cros_ec_usbpd_notify.c b/drivers/platform/chrome/cros_ec_usbpd_notify.c
new file mode 100644
index 000000000000..f654586dea2a
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_usbpd_notify.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * cros_ec_usbpd - ChromeOS EC Power Delivery Driver
+ *
+ * Copyright (C) 2019 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.
+ *
+ * This driver serves as the receiver of cros_ec PD host events.
+ */
+
+#include <linux/acpi.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_usbpd_notify.h>
+#include <linux/platform_data/cros_ec_proto.h>
+#include <linux/platform_device.h>
+
+#define DRV_NAME "cros-ec-usbpd-notify"
+#define ACPI_DRV_NAME "GOOG0003"
+
+static BLOCKING_NOTIFIER_HEAD(cros_ec_usbpd_notifier_list);
+
+int cros_ec_usbpd_register_notify(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_register(
+ &cros_ec_usbpd_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(cros_ec_usbpd_register_notify);
+
+void cros_ec_usbpd_unregister_notify(struct notifier_block *nb)
+{
+ blocking_notifier_chain_unregister(&cros_ec_usbpd_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(cros_ec_usbpd_unregister_notify);
+
+static void cros_ec_usbpd_notify(u32 event)
+{
+ blocking_notifier_call_chain(&cros_ec_usbpd_notifier_list, event, NULL);
+}
+
+#ifdef CONFIG_ACPI
+
+static int cros_ec_usbpd_add_acpi(struct acpi_device *adev)
+{
+ return 0;
+}
+
+static int cros_ec_usbpd_remove_acpi(struct acpi_device *adev)
+{
+ return 0;
+}
+static void cros_ec_usbpd_notify_acpi(struct acpi_device *adev, u32 event)
+{
+ cros_ec_usbpd_notify(event);
+}
+
+static const struct acpi_device_id cros_ec_usbpd_acpi_device_ids[] = {
+ { ACPI_DRV_NAME, 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, cros_ec_usbpd_acpi_device_ids);
+
+static struct acpi_driver cros_ec_usbpd_driver = {
+ .name = DRV_NAME,
+ .class = DRV_NAME,
+ .ids = cros_ec_usbpd_acpi_device_ids,
+ .ops = {
+ .add = cros_ec_usbpd_add_acpi,
+ .remove = cros_ec_usbpd_remove_acpi,
+ .notify = cros_ec_usbpd_notify_acpi,
+ },
+};
+module_acpi_driver(cros_ec_usbpd_driver);
+
+#else /* CONFIG_ACPI */
+
+static int cros_ec_usbpd_notify_plat(struct notifier_block *nb,
+ unsigned long queued_during_suspend, void *data)
+{
+ struct cros_ec_device *ec_dev = (struct cros_ec_device *)data;
+ u32 host_event = cros_ec_get_host_event(ec_dev);
+
+ if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_PD_MCU)) {
+ cros_ec_usbpd_notify(host_event);
+ return NOTIFY_OK;
+ } else {
+ return NOTIFY_DONE;
+ }
+
+}
+
+static int cros_ec_usbpd_probe_plat(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct cros_ec_device *ec_dev = dev_get_drvdata(dev->parent);
+ struct notifier_block *nb;
+ int ret;
+
+ nb = devm_kzalloc(dev, sizeof(*nb), GFP_KERNEL);
+ if (!nb)
+ return -ENOMEM;
+
+ nb->notifier_call = cros_ec_usbpd_notify_plat;
+ dev_set_drvdata(dev, nb);
+ ret = blocking_notifier_chain_register(&ec_dev->event_notifier, nb);
+
+ if (ret < 0)
+ dev_warn(dev, "Failed to register notifier\n");
+
+ return 0;
+}
+
+static int cros_ec_usbpd_remove_plat(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct cros_ec_device *ec_dev = dev_get_drvdata(dev->parent);
+ struct notifier_block *nb =
+ (struct notifier_block *)dev_get_drvdata(dev);
+
+ blocking_notifier_chain_unregister(&ec_dev->event_notifier, nb);
+
+ return 0;
+}
+
+static const struct of_device_id cros_ec_usbpd_of_match[] = {
+ { .compatible = "google,cros-ec-pd-update" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, cros_ec_usbpd_of_match);
+
+static struct platform_driver cros_ec_usbpd_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .of_match_table = of_match_ptr(cros_ec_usbpd_of_match),
+ },
+ .probe = cros_ec_usbpd_probe_plat,
+ .remove = cros_ec_usbpd_remove_plat,
+};
+module_platform_driver(cros_ec_usbpd_driver);
+
+#endif /* CONFIG_ACPI */
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("ChromeOS power delivery device");
+MODULE_AUTHOR("Jon Flatley <[email protected]>");
+MODULE_ALIAS("platform:" DRV_NAME);
diff --git a/include/linux/platform_data/cros_ec_usbpd_notify.h b/include/linux/platform_data/cros_ec_usbpd_notify.h
new file mode 100644
index 000000000000..fdcea146b7c4
--- /dev/null
+++ b/include/linux/platform_data/cros_ec_usbpd_notify.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * cros_ec_usbpd - ChromeOS EC Power Delivery Driver
+ *
+ * Copyright (C) 2019 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.
+ */
+
+#ifndef __LINUX_PLATFORM_DATA_CROS_EC_USBPD_NOTIFY_H
+#define __LINUX_PLATFORM_DATA_CROS_EC_USBPD_NOTIFY_H
+
+#include <linux/notifier.h>
+
+/**
+ * cros_ec_usbpd_register_notify - register a notifier callback for USB PD
+ * events. On ACPI platforms this corrisponds to to host events on the ECPD
+ * "GOOG0003" ACPI device. On non-ACPI platforms this will filter mkbp events
+ * for USB PD events.
+ *
+ * @nb: Notifier block pointer to register
+ */
+int cros_ec_usbpd_register_notify(struct notifier_block *nb);
+
+/**
+ * cros_ec_usbpd_unregister_notify - unregister a notifier callback that was
+ * previously registered with cros_ec_usbpd_register_notify().
+ *
+ * @nb: Notifier block pointer to unregister
+ */
+void cros_ec_usbpd_unregister_notify(struct notifier_block *nb);
+
+#endif /* __LINUX_PLATFORM_DATA_CROS_EC_USBPD_NOTIFY_H */
--
2.24.0.432.g9d3f5f5b63-goog
Adds the cros-ec-typec driver for implementing the USB Type-C connector
class for cros-ec devices.
Signed-off-by: Jon Flatley <[email protected]>
---
drivers/mfd/cros_ec_dev.c | 7 +-
drivers/platform/chrome/Kconfig | 11 +
drivers/platform/chrome/Makefile | 1 +
drivers/platform/chrome/cros_ec_typec.c | 457 ++++++++++++++++++++++++
4 files changed, 473 insertions(+), 3 deletions(-)
create mode 100644 drivers/platform/chrome/cros_ec_typec.c
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 6e6dfd6c1871..1136ef986695 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -78,9 +78,10 @@ static const struct mfd_cell cros_ec_rtc_cells[] = {
{ .name = "cros-ec-rtc", },
};
-static const struct mfd_cell cros_usbpd_charger_cells[] = {
+static const struct mfd_cell cros_usbpd_cells[] = {
{ .name = "cros-usbpd-charger", },
{ .name = "cros-usbpd-logger", },
+ { .name = "cros-ec-typec" },
};
static const struct cros_feature_to_cells cros_subdevices[] = {
@@ -96,8 +97,8 @@ static const struct cros_feature_to_cells cros_subdevices[] = {
},
{
.id = EC_FEATURE_USB_PD,
- .mfd_cells = cros_usbpd_charger_cells,
- .num_cells = ARRAY_SIZE(cros_usbpd_charger_cells),
+ .mfd_cells = cros_usbpd_cells,
+ .num_cells = ARRAY_SIZE(cros_usbpd_cells),
},
};
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index d4a55b64bc29..1be5c86f2aad 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -210,6 +210,17 @@ config CROS_EC_SYSFS
To compile this driver as a module, choose M here: the
module will be called cros_ec_sysfs.
+config CROS_EC_TYPEC
+ tristate "ChromeOS Embedded Controller USB Type-C class subdriver"
+ depends on TYPEC && CROS_EC_USBPD_NOTIFY
+ default n
+ help
+ Say Y here to enable the USB Type-C class subdriver for ChromeOS
+ devices.
+
+ To compile this driver as a module, choose M here: the
+ module will be called cros-ec-typec.
+
config CROS_USBPD_LOGGER
tristate "Logging driver for USB PD charger"
depends on CHARGER_CROS_USBPD
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index efa355ab526f..bb298c083f1e 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -22,5 +22,6 @@ obj-$(CONFIG_CROS_EC_DEBUGFS) += cros_ec_debugfs.o
obj-$(CONFIG_CROS_EC_SYSFS) += cros_ec_sysfs.o
obj-$(CONFIG_CROS_USBPD_LOGGER) += cros_usbpd_logger.o
obj-$(CONFIG_CROS_EC_USBPD_NOTIFY) += cros_ec_usbpd_notify.o
+obj-$(CONFIG_CROS_EC_TYPEC) += cros_ec_typec.o
obj-$(CONFIG_WILCO_EC) += wilco_ec/
diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
new file mode 100644
index 000000000000..262b914f0a2e
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -0,0 +1,457 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * cros_ec_usbpd - ChromeOS EC Power Delivery Driver
+ *
+ * Copyright (C) 2019 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/kernel.h>
+#include <linux/kthread.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_usbpd_notify.h>
+#include <linux/platform_data/cros_ec_proto.h>
+#include <linux/usb/typec.h>
+#include <linux/slab.h>
+
+#define DRV_NAME "cros-ec-typec"
+
+struct port_data {
+ int port_num;
+ struct typec_port *port;
+ struct typec_partner *partner;
+ struct usb_pd_identity p_identity;
+ struct typec_data *typec;
+ struct typec_capability caps;
+};
+
+struct typec_data {
+ struct device *dev;
+ struct cros_ec_dev *ec_dev;
+ struct port_data *ports[EC_USB_PD_MAX_PORTS];
+ unsigned int num_ports;
+ struct notifier_block notifier;
+
+ int (*port_update)(struct typec_data *typec, int port_num);
+};
+
+#define caps_to_port_data(_caps_) container_of(_caps_, struct port_data, caps)
+
+static int cros_typec_ec_command(struct typec_data *typec,
+ unsigned int version,
+ unsigned int command,
+ void *outdata,
+ unsigned int outsize,
+ void *indata,
+ unsigned int insize)
+{
+ struct cros_ec_dev *ec_dev = typec->ec_dev;
+ struct cros_ec_command *msg;
+ int ret;
+ char host_cmd[32];
+
+ msg = kzalloc(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;
+
+ if (outsize)
+ memcpy(msg->data, outdata, outsize);
+
+ sprintf(host_cmd, "typec HC 0x%x req: ", command);
+ print_hex_dump(KERN_DEBUG, host_cmd, DUMP_PREFIX_NONE, 16, 1, outdata,
+ outsize, 0);
+
+ ret = cros_ec_cmd_xfer_status(typec->ec_dev->ec_dev, msg);
+ if (ret >= 0 && insize)
+ memcpy(indata, msg->data, insize);
+ sprintf(host_cmd, "typec HC 0x%x res: ", command);
+ print_hex_dump(KERN_DEBUG, host_cmd, DUMP_PREFIX_NONE, 16, 1, indata,
+ insize, 0);
+
+ kfree(msg);
+ return ret;
+}
+
+static int cros_typec_get_cmd_version(struct typec_data *typec, int cmd,
+ uint8_t *version_mask)
+{
+ struct ec_params_get_cmd_versions req_v0;
+ struct ec_params_get_cmd_versions_v1 req_v1;
+ struct ec_response_get_cmd_versions res;
+ int ret;
+
+ req_v1.cmd = cmd;
+ ret = cros_typec_ec_command(typec, 1, EC_CMD_GET_CMD_VERSIONS, &req_v1,
+ sizeof(req_v1), &res, sizeof(res));
+ if (ret < 0) {
+ req_v0.cmd = cmd;
+ ret = cros_typec_ec_command(typec, 0, EC_CMD_GET_CMD_VERSIONS,
+ &req_v0, sizeof(req_v0), &res, sizeof(res));
+ if (ret < 0)
+ return ret;
+ }
+ *version_mask = res.version_mask;
+ dev_dbg(typec->dev, "EC CMD 0x%hhx has version mask 0x%hhx\n", cmd,
+ *version_mask);
+ return 0;
+}
+
+static int cros_typec_query_pd_port_count(struct typec_data *typec)
+{
+ struct ec_response_usb_pd_ports res;
+ int ret;
+
+ ret = cros_typec_ec_command(typec, 0, EC_CMD_USB_PD_PORTS, NULL, 0,
+ &res, sizeof(res));
+ if (ret < 0)
+ return ret;
+ typec->num_ports = res.num_ports;
+ return 0;
+}
+
+static int cros_typec_port_update(struct typec_data *typec,
+ int port_num,
+ struct ec_response_usb_pd_control *res,
+ size_t res_size,
+ int cmd_ver)
+{
+ struct port_data *port;
+ struct ec_params_usb_pd_control req;
+ int ret;
+
+ if (port_num < 0 || port_num >= typec->num_ports) {
+ dev_err(typec->dev, "cannot get status for invalid port %d\n",
+ port_num);
+ return -EPERM;
+ }
+ port = typec->ports[port_num];
+
+ req.port = port_num;
+ req.role = USB_PD_CTRL_ROLE_NO_CHANGE;
+ req.mux = USB_PD_CTRL_MUX_NO_CHANGE;
+ req.swap = USB_PD_CTRL_SWAP_NONE;
+
+ ret = cros_typec_ec_command(typec, cmd_ver, EC_CMD_USB_PD_CONTROL, &req,
+ sizeof(req), res, res_size);
+ if (ret < 0)
+ return ret;
+ dev_dbg(typec->dev, "Enabled %d: 0x%hhx\n", port_num, res->enabled);
+ dev_dbg(typec->dev, "Role %d: 0x%hhx\n", port_num, res->role);
+ dev_dbg(typec->dev, "Polarity %d: 0x%hhx\n", port_num, res->polarity);
+
+ return 0;
+}
+
+static int cros_typec_query_pd_info(struct typec_data *typec, int port_num)
+{
+ struct ec_params_usb_pd_info_request req;
+ struct ec_params_usb_pd_discovery_entry res;
+ int ret;
+
+ req.port = port_num;
+ ret = cros_typec_ec_command(typec, 0, EC_CMD_USB_PD_DISCOVERY, &req,
+ sizeof(req), &res, sizeof(res));
+ if (ret < 0)
+ return ret;
+ /* FIXME Needs the rest of the info in PD Spec 6.4.4.3.1.1 */
+ typec->ports[port_num]->p_identity.id_header = res.vid;
+
+ /* FIXME Needs bcdDevice to match PD Spec 6.4.4.3.1.3 */
+ typec->ports[port_num]->p_identity.product = res.pid << 16;
+ return 0;
+}
+
+static int cros_typec_port_update_v0(struct typec_data *typec, int port_num)
+{
+ struct port_data *port;
+ struct ec_response_usb_pd_control res;
+ enum typec_orientation polarity;
+ int ret;
+
+ port = typec->ports[port_num];
+ ret = cros_typec_port_update(typec, port_num, &res, sizeof(res), 0);
+ if (ret < 0)
+ return ret;
+ dev_dbg(typec->dev, "State %d: %hhx\n", port_num, res.state);
+
+ if (!res.enabled)
+ polarity = TYPEC_ORIENTATION_NONE;
+ else if (!res.polarity)
+ polarity = TYPEC_ORIENTATION_NORMAL;
+ else
+ polarity = TYPEC_ORIENTATION_REVERSE;
+
+ typec_set_pwr_role(port->port, res.role ? TYPEC_SOURCE : TYPEC_SINK);
+ typec_set_orientation(port->port, polarity);
+
+ return 0;
+}
+
+static int cros_typec_add_partner(struct typec_data *typec, int port_num,
+ bool pd_enabled)
+{
+ struct port_data *port;
+ struct typec_partner_desc p_desc;
+ int ret;
+
+ port = typec->ports[port_num];
+ p_desc.usb_pd = pd_enabled;
+ p_desc.identity = &port->p_identity;
+
+ port->partner = typec_register_partner(port->port, &p_desc);
+ if (IS_ERR_OR_NULL(port->partner)) {
+ dev_err(typec->dev, "Port %d partner register failed\n",
+ port_num);
+ port->partner = NULL;
+ return PTR_ERR(port->partner);
+ }
+
+ ret = cros_typec_query_pd_info(typec, port_num);
+ if (ret < 0) {
+ dev_err(typec->dev, "Port %d PD query failed\n", port_num);
+ typec_unregister_partner(port->partner);
+ port->partner = NULL;
+ return ret;
+ }
+
+ ret = typec_partner_set_identity(port->partner);
+ return ret;
+}
+
+static int cros_typec_set_port_params_v1_v2(struct typec_data *typec,
+ int port_num, struct ec_response_usb_pd_control_v1 *res)
+{
+ struct port_data *port;
+ enum typec_orientation polarity;
+ int ret;
+
+ port = typec->ports[port_num];
+ if (!(res->enabled & PD_CTRL_RESP_ENABLED_CONNECTED))
+ polarity = TYPEC_ORIENTATION_NONE;
+ else if (!res->polarity)
+ polarity = TYPEC_ORIENTATION_NORMAL;
+ else
+ polarity = TYPEC_ORIENTATION_REVERSE;
+ typec_set_orientation(port->port, polarity);
+ typec_set_data_role(port->port, res->role & PD_CTRL_RESP_ROLE_DATA ?
+ TYPEC_HOST : TYPEC_DEVICE);
+ typec_set_pwr_role(port->port, res->role & PD_CTRL_RESP_ROLE_POWER ?
+ TYPEC_SOURCE : TYPEC_SINK);
+ typec_set_vconn_role(port->port, res->role & PD_CTRL_RESP_ROLE_VCONN ?
+ TYPEC_SOURCE : TYPEC_SINK);
+
+ if (res->enabled & PD_CTRL_RESP_ENABLED_CONNECTED && !port->partner) {
+ bool pd_enabled =
+ res->enabled & PD_CTRL_RESP_ENABLED_PD_CAPABLE;
+ ret = cros_typec_add_partner(typec, port_num, pd_enabled);
+ if (!ret)
+ return ret;
+ }
+ if (!(res->enabled & PD_CTRL_RESP_ENABLED_CONNECTED) && port->partner) {
+ typec_unregister_partner(port->partner);
+ port->partner = NULL;
+ }
+ return 0;
+}
+
+static int cros_typec_port_update_v1(struct typec_data *typec, int port_num)
+{
+ /*struct port_data *port;*/
+ struct ec_response_usb_pd_control_v1 res;
+ struct ec_response_usb_pd_control *res_v0 =
+ (struct ec_response_usb_pd_control *) &res;
+ int ret;
+
+ ret = cros_typec_port_update(typec, port_num, res_v0, sizeof(res), 1);
+ if (ret < 0)
+ return ret;
+ dev_dbg(typec->dev, "State %d: %s\n", port_num, res.state);
+
+ ret = cros_typec_set_port_params_v1_v2(typec, port_num, &res);
+ return ret;
+}
+
+static int cros_typec_try_role(const struct typec_capability *cap, int role)
+{
+ return 0;
+}
+
+static int cros_typec_dr_set(const struct typec_capability *cap,
+ enum typec_data_role role)
+{
+ return 0;
+}
+
+static int cros_typec_pr_set(const struct typec_capability *cap,
+ enum typec_role role)
+{
+ return 0;
+}
+
+static int cros_typec_vconn_set(const struct typec_capability *cap,
+ enum typec_role role)
+{
+ return 0;
+}
+
+static int cros_typec_ec_event(struct notifier_block *nb,
+ unsigned long queued_during_suspend,
+ void *_notify)
+{
+ struct typec_data *typec;
+ int i;
+
+ typec = container_of(nb, struct typec_data, notifier);
+
+ for (i = 0; i < typec->num_ports; ++i)
+ typec->port_update(typec, i);
+
+ return NOTIFY_DONE;
+
+}
+
+static void cros_typec_unregister_notifier(void *data)
+{
+ struct typec_data *typec = data;
+
+ cros_ec_usbpd_unregister_notify(&typec->notifier);
+}
+
+static int cros_typec_probe(struct platform_device *pd)
+{
+ struct cros_ec_dev *ec_dev = dev_get_drvdata(pd->dev.parent);
+ struct device *dev = &pd->dev;
+ struct typec_data *typec;
+ uint8_t ver_mask;
+ int i;
+ int ret;
+
+ dev_dbg(dev, "Probing Cros EC Type-C device.\n");
+
+ typec = devm_kzalloc(dev, sizeof(*typec), GFP_KERNEL);
+ if (!typec)
+ return -ENOMEM;
+
+ typec->dev = dev;
+ typec->ec_dev = ec_dev;
+
+ platform_set_drvdata(pd, typec);
+
+ ret = cros_typec_query_pd_port_count(typec);
+ if (ret < 0) {
+ dev_err(dev, "Failed to get PD port count from EC\n");
+ return ret;
+ }
+ if (typec->num_ports > EC_USB_PD_MAX_PORTS) {
+ dev_err(dev, "EC reported too many ports. got: %d, max: %d\n",
+ typec->num_ports, EC_USB_PD_MAX_PORTS);
+ return -EOVERFLOW;
+ }
+
+ ret = cros_typec_get_cmd_version(typec, EC_CMD_USB_PD_CONTROL,
+ &ver_mask);
+ if (ret < 0) {
+ dev_err(dev, "Failed to get supported PD command versions\n");
+ return ret;
+ }
+ /* No reason to support EC_CMD_USB_PD_CONTROL v2 as it doesn't add any
+ * useful information
+ */
+ if (ver_mask & EC_VER_MASK(1)) {
+ dev_dbg(dev, "Using PD command ver 1\n");
+ typec->port_update = cros_typec_port_update_v1;
+ } else {
+ dev_dbg(dev, "Using PD command ver 0\n");
+ typec->port_update = cros_typec_port_update_v0;
+ }
+
+ for (i = 0; i < typec->num_ports; ++i) {
+ struct port_data *port;
+
+ port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+ if (!port) {
+ ret = -ENOMEM;
+ goto unregister_ports;
+ }
+ port->typec = typec;
+ port->port_num = i;
+ typec->ports[i] = port;
+
+ /* TODO Make sure these values are accurate */
+ port->caps.type = TYPEC_PORT_DRP;
+ port->caps.data = TYPEC_PORT_DFP;
+ port->caps.prefer_role = TYPEC_SINK;
+
+ port->caps.try_role = cros_typec_try_role;
+ port->caps.dr_set = cros_typec_dr_set;
+ port->caps.pr_set = cros_typec_pr_set;
+ port->caps.vconn_set = cros_typec_vconn_set;
+ port->caps.port_type_set = NULL; /* Not permitted by PD spec */
+
+ port->port = typec_register_port(dev, &port->caps);
+ if (IS_ERR_OR_NULL(port->port)) {
+ dev_err(dev, "Failed to register typec port %d\n", i);
+ ret = PTR_ERR(port->port);
+ goto unregister_ports;
+ }
+
+ ret = typec->port_update(typec, i);
+ if (ret < 0) {
+ dev_err(dev, "Failed to update typec port %d\n", i);
+ goto unregister_ports;
+ }
+ }
+
+ typec->notifier.notifier_call = cros_typec_ec_event;
+ ret = cros_ec_usbpd_register_notify(&typec->notifier);
+ if (ret < 0) {
+ dev_warn(dev, "Failed to register notifier\n");
+ } else {
+ ret = devm_add_action_or_reset(dev,
+ cros_typec_unregister_notifier, typec);
+ if (ret < 0)
+ goto unregister_ports;
+ dev_dbg(dev, "Registered EC notifier\n");
+ }
+
+ return 0;
+
+unregister_ports:
+ for (i = 0; i < typec->num_ports; ++i) {
+ if (typec->ports[i] && typec->ports[i]->port)
+ typec_unregister_port(typec->ports[i]->port);
+ }
+
+ return ret;
+}
+
+static struct platform_driver cros_ec_typec_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ },
+ .probe = cros_typec_probe
+};
+
+module_platform_driver(cros_ec_typec_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("ChromeOS EC USB-C connectors");
+MODULE_AUTHOR("Jon Flatley <[email protected]>");
+MODULE_ALIAS("platform:" DRV_NAME);
--
2.24.0.432.g9d3f5f5b63-goog
Hi Jon,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on ljones-mfd/for-mfd-next]
[cannot apply to v5.4-rc7 next-20191113]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Jon-Flatley/ChromeOS-EC-USB-C-Connector-Class/20191113-193504
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
coccinelle warnings: (new ones prefixed by >>)
>> drivers/platform/chrome/cros_ec_typec.c:223:9-16: ERROR: PTR_ERR applied after initialization to constant on line 222
vim +223 drivers/platform/chrome/cros_ec_typec.c
206
207 static int cros_typec_add_partner(struct typec_data *typec, int port_num,
208 bool pd_enabled)
209 {
210 struct port_data *port;
211 struct typec_partner_desc p_desc;
212 int ret;
213
214 port = typec->ports[port_num];
215 p_desc.usb_pd = pd_enabled;
216 p_desc.identity = &port->p_identity;
217
218 port->partner = typec_register_partner(port->port, &p_desc);
219 if (IS_ERR_OR_NULL(port->partner)) {
220 dev_err(typec->dev, "Port %d partner register failed\n",
221 port_num);
> 222 port->partner = NULL;
> 223 return PTR_ERR(port->partner);
224 }
225
226 ret = cros_typec_query_pd_info(typec, port_num);
227 if (ret < 0) {
228 dev_err(typec->dev, "Port %d PD query failed\n", port_num);
229 typec_unregister_partner(port->partner);
230 port->partner = NULL;
231 return ret;
232 }
233
234 ret = typec_partner_set_identity(port->partner);
235 return ret;
236 }
237
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation
On Wed, Nov 13, 2019 at 4:55 AM kbuild test robot <[email protected]> wrote:
>
> Hi Jon,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on ljones-mfd/for-mfd-next]
> [cannot apply to v5.4-rc7 next-20191113]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url: https://github.com/0day-ci/linux/commits/Jon-Flatley/ChromeOS-EC-USB-C-Connector-Class/20191113-193504
> base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <[email protected]>
>
>
> coccinelle warnings: (new ones prefixed by >>)
>
> >> drivers/platform/chrome/cros_ec_typec.c:223:9-16: ERROR: PTR_ERR applied after initialization to constant on line 222
>
> vim +223 drivers/platform/chrome/cros_ec_typec.c
>
> 206
> 207 static int cros_typec_add_partner(struct typec_data *typec, int port_num,
> 208 bool pd_enabled)
> 209 {
> 210 struct port_data *port;
> 211 struct typec_partner_desc p_desc;
> 212 int ret;
> 213
> 214 port = typec->ports[port_num];
> 215 p_desc.usb_pd = pd_enabled;
> 216 p_desc.identity = &port->p_identity;
> 217
> 218 port->partner = typec_register_partner(port->port, &p_desc);
> 219 if (IS_ERR_OR_NULL(port->partner)) {
> 220 dev_err(typec->dev, "Port %d partner register failed\n",
> 221 port_num);
> > 222 port->partner = NULL;
> > 223 return PTR_ERR(port->partner);
> 224 }
> 225
> 226 ret = cros_typec_query_pd_info(typec, port_num);
> 227 if (ret < 0) {
> 228 dev_err(typec->dev, "Port %d PD query failed\n", port_num);
> 229 typec_unregister_partner(port->partner);
> 230 port->partner = NULL;
> 231 return ret;
> 232 }
> 233
> 234 ret = typec_partner_set_identity(port->partner);
> 235 return ret;
> 236 }
> 237
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation
This patch is based off of the chrome-platform for-next branch.
base: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git
for-next
Is this incorrect? I'm happy to rebase if necessary.
Thanks,
-Jon
Hi Jon,
Thanks for posting this.
Adding Heikki, the typec connector class maintainer, and Enric, co-maintainer
of platform/chrome.
Benson
On Tue, Nov 12, 2019 at 07:10:41PM -0800, Jon Flatley wrote:
> This patch set adds a basic implementation of the USB-C connector class for
> devices using the ChromeOS EC. On ACPI devices an additional ACPI driver is
> necessary to receive USB-C PD host events from the PD EC device "GOOG0003".
> Incidentally, this ACPI driver adds notifications for events that
> cros-usbpd-charger has been missing, so fix that while we're at it.
>
> Jon Flatley (3):
> platform: chrome: Add cros-ec-usbpd-notify driver
> power: supply: cros-ec-usbpd-charger: Fix host events
> platform: chrome: Added cros-ec-typec driver
>
> drivers/mfd/cros_ec_dev.c | 7 +-
> drivers/platform/chrome/Kconfig | 20 +
> drivers/platform/chrome/Makefile | 2 +
> drivers/platform/chrome/cros_ec_typec.c | 457 ++++++++++++++++++
> .../platform/chrome/cros_ec_usbpd_notify.c | 156 ++++++
> drivers/power/supply/Kconfig | 2 +-
> drivers/power/supply/cros_usbpd-charger.c | 45 +-
> .../platform_data/cros_ec_usbpd_notify.h | 40 ++
> 8 files changed, 696 insertions(+), 33 deletions(-)
> create mode 100644 drivers/platform/chrome/cros_ec_typec.c
> create mode 100644 drivers/platform/chrome/cros_ec_usbpd_notify.c
> create mode 100644 include/linux/platform_data/cros_ec_usbpd_notify.h
>
> --
> 2.24.0.432.g9d3f5f5b63-goog
>
--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
[email protected]
Chromium OS Project
[email protected]
Hi guys,
On Wed, Nov 13, 2019 at 09:51:27AM -0800, Benson Leung wrote:
> Hi Jon,
>
> Thanks for posting this.
>
> Adding Heikki, the typec connector class maintainer, and Enric, co-maintainer
> of platform/chrome.
Thanks Benson.
> On Tue, Nov 12, 2019 at 07:10:41PM -0800, Jon Flatley wrote:
> > This patch set adds a basic implementation of the USB-C connector class for
> > devices using the ChromeOS EC. On ACPI devices an additional ACPI driver is
> > necessary to receive USB-C PD host events from the PD EC device "GOOG0003".
> > Incidentally, this ACPI driver adds notifications for events that
> > cros-usbpd-charger has been missing, so fix that while we're at it.
>
> > Jon Flatley (3):
> > platform: chrome: Add cros-ec-usbpd-notify driver
> > power: supply: cros-ec-usbpd-charger: Fix host events
> > platform: chrome: Added cros-ec-typec driver
> >
> > drivers/mfd/cros_ec_dev.c | 7 +-
> > drivers/platform/chrome/Kconfig | 20 +
> > drivers/platform/chrome/Makefile | 2 +
> > drivers/platform/chrome/cros_ec_typec.c | 457 ++++++++++++++++++
> > .../platform/chrome/cros_ec_usbpd_notify.c | 156 ++++++
> > drivers/power/supply/Kconfig | 2 +-
> > drivers/power/supply/cros_usbpd-charger.c | 45 +-
> > .../platform_data/cros_ec_usbpd_notify.h | 40 ++
> > 8 files changed, 696 insertions(+), 33 deletions(-)
> > create mode 100644 drivers/platform/chrome/cros_ec_typec.c
> > create mode 100644 drivers/platform/chrome/cros_ec_usbpd_notify.c
> > create mode 100644 include/linux/platform_data/cros_ec_usbpd_notify.h
I'll go over these tomorrow, but I have one question already. Can you
guys influence what goes to the ACPI tables?
Ideally every Type-C connector is always described in its own ACPI
node (or DT node if DT is used). Otherwise getting the correct
capabilities and especially connections to other devices (like the
muxes) for every port may get difficult.
Br,
--
heikki
On 11/14/19 1:23 AM, Jon Flatley wrote:
> On Wed, Nov 13, 2019 at 4:55 AM kbuild test robot <[email protected]> wrote:
>> Hi Jon,
>>
>> Thank you for the patch! Perhaps something to improve:
>>
>> [auto build test WARNING on ljones-mfd/for-mfd-next]
>> [cannot apply to v5.4-rc7 next-20191113]
>> [if your patch is applied to the wrong git tree, please drop us a note to help
>> improve the system. BTW, we also suggest to use '--base' option to specify the
>> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>>
>> url: https://github.com/0day-ci/linux/commits/Jon-Flatley/ChromeOS-EC-USB-C-Connector-Class/20191113-193504
>> base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
>>
>> If you fix the issue, kindly add following tag
>> Reported-by: kbuild test robot <[email protected]>
>>
>>
>> coccinelle warnings: (new ones prefixed by >>)
>>
>>>> drivers/platform/chrome/cros_ec_typec.c:223:9-16: ERROR: PTR_ERR applied after initialization to constant on line 222
>> vim +223 drivers/platform/chrome/cros_ec_typec.c
>>
>> 206
>> 207 static int cros_typec_add_partner(struct typec_data *typec, int port_num,
>> 208 bool pd_enabled)
>> 209 {
>> 210 struct port_data *port;
>> 211 struct typec_partner_desc p_desc;
>> 212 int ret;
>> 213
>> 214 port = typec->ports[port_num];
>> 215 p_desc.usb_pd = pd_enabled;
>> 216 p_desc.identity = &port->p_identity;
>> 217
>> 218 port->partner = typec_register_partner(port->port, &p_desc);
>> 219 if (IS_ERR_OR_NULL(port->partner)) {
>> 220 dev_err(typec->dev, "Port %d partner register failed\n",
>> 221 port_num);
>> > 222 port->partner = NULL;
>> > 223 return PTR_ERR(port->partner);
>> 224 }
>> 225
>> 226 ret = cros_typec_query_pd_info(typec, port_num);
>> 227 if (ret < 0) {
>> 228 dev_err(typec->dev, "Port %d PD query failed\n", port_num);
>> 229 typec_unregister_partner(port->partner);
>> 230 port->partner = NULL;
>> 231 return ret;
>> 232 }
>> 233
>> 234 ret = typec_partner_set_identity(port->partner);
>> 235 return ret;
>> 236 }
>> 237
>>
>> ---
>> 0-DAY kernel test infrastructure Open Source Technology Center
>> https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation
> This patch is based off of the chrome-platform for-next branch.
>
> base: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git
> for-next
>
> Is this incorrect? I'm happy to rebase if necessary.
>
> Thanks,
> -Jon
Hi Jon,
Thanks for the response, we'll fix the wrong base ASAP.
Best Regards,
Rong Chen
On Wed, Nov 13, 2019 at 9:23 AM Jon Flatley <[email protected]> wrote:
>
> On Wed, Nov 13, 2019 at 4:55 AM kbuild test robot <[email protected]> wrote:
> >
> > Hi Jon,
> >
> > Thank you for the patch! Perhaps something to improve:
> >
> > [auto build test WARNING on ljones-mfd/for-mfd-next]
> > [cannot apply to v5.4-rc7 next-20191113]
> > [if your patch is applied to the wrong git tree, please drop us a note to help
> > improve the system. BTW, we also suggest to use '--base' option to specify the
> > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> >
> > url: https://github.com/0day-ci/linux/commits/Jon-Flatley/ChromeOS-EC-USB-C-Connector-Class/20191113-193504
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
> >
> > If you fix the issue, kindly add following tag
> > Reported-by: kbuild test robot <[email protected]>
> >
> >
> > coccinelle warnings: (new ones prefixed by >>)
> >
> > >> drivers/platform/chrome/cros_ec_typec.c:223:9-16: ERROR: PTR_ERR applied after initialization to constant on line 222
> >
> > vim +223 drivers/platform/chrome/cros_ec_typec.c
> >
> > 206
> > 207 static int cros_typec_add_partner(struct typec_data *typec, int port_num,
> > 208 bool pd_enabled)
> > 209 {
> > 210 struct port_data *port;
> > 211 struct typec_partner_desc p_desc;
> > 212 int ret;
> > 213
> > 214 port = typec->ports[port_num];
> > 215 p_desc.usb_pd = pd_enabled;
> > 216 p_desc.identity = &port->p_identity;
> > 217
> > 218 port->partner = typec_register_partner(port->port, &p_desc);
> > 219 if (IS_ERR_OR_NULL(port->partner)) {
> > 220 dev_err(typec->dev, "Port %d partner register failed\n",
> > 221 port_num);
> > > 222 port->partner = NULL;
> > > 223 return PTR_ERR(port->partner);
> > 224 }
> > 225
> > 226 ret = cros_typec_query_pd_info(typec, port_num);
> > 227 if (ret < 0) {
> > 228 dev_err(typec->dev, "Port %d PD query failed\n", port_num);
> > 229 typec_unregister_partner(port->partner);
> > 230 port->partner = NULL;
> > 231 return ret;
> > 232 }
> > 233
> > 234 ret = typec_partner_set_identity(port->partner);
> > 235 return ret;
> > 236 }
> > 237
> >
> > ---
> > 0-DAY kernel test infrastructure Open Source Technology Center
> > https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation
>
> This patch is based off of the chrome-platform for-next branch.
>
> base: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git
> for-next
>
> Is this incorrect? I'm happy to rebase if necessary.
>
You are missing the problem.
port->partner = NULL;
return PTR_ERR(port->partner);
Does not make sense, and always returns 0 (no error).
Guenter
> Thanks,
> -Jon
On Wed, Nov 13, 2019 at 10:25 AM Heikki Krogerus
<[email protected]> wrote:
>
> Hi guys,
>
> On Wed, Nov 13, 2019 at 09:51:27AM -0800, Benson Leung wrote:
> > Hi Jon,
> >
> > Thanks for posting this.
> >
> > Adding Heikki, the typec connector class maintainer, and Enric, co-maintainer
> > of platform/chrome.
>
> Thanks Benson.
>
> > On Tue, Nov 12, 2019 at 07:10:41PM -0800, Jon Flatley wrote:
> > > This patch set adds a basic implementation of the USB-C connector class for
> > > devices using the ChromeOS EC. On ACPI devices an additional ACPI driver is
> > > necessary to receive USB-C PD host events from the PD EC device "GOOG0003".
> > > Incidentally, this ACPI driver adds notifications for events that
> > > cros-usbpd-charger has been missing, so fix that while we're at it.
> >
> > > Jon Flatley (3):
> > > platform: chrome: Add cros-ec-usbpd-notify driver
> > > power: supply: cros-ec-usbpd-charger: Fix host events
> > > platform: chrome: Added cros-ec-typec driver
> > >
> > > drivers/mfd/cros_ec_dev.c | 7 +-
> > > drivers/platform/chrome/Kconfig | 20 +
> > > drivers/platform/chrome/Makefile | 2 +
> > > drivers/platform/chrome/cros_ec_typec.c | 457 ++++++++++++++++++
> > > .../platform/chrome/cros_ec_usbpd_notify.c | 156 ++++++
> > > drivers/power/supply/Kconfig | 2 +-
> > > drivers/power/supply/cros_usbpd-charger.c | 45 +-
> > > .../platform_data/cros_ec_usbpd_notify.h | 40 ++
> > > 8 files changed, 696 insertions(+), 33 deletions(-)
> > > create mode 100644 drivers/platform/chrome/cros_ec_typec.c
> > > create mode 100644 drivers/platform/chrome/cros_ec_usbpd_notify.c
> > > create mode 100644 include/linux/platform_data/cros_ec_usbpd_notify.h
>
> I'll go over these tomorrow, but I have one question already. Can you
> guys influence what goes to the ACPI tables?
>
> Ideally every Type-C connector is always described in its own ACPI
> node (or DT node if DT is used). Otherwise getting the correct
> capabilities and especially connections to other devices (like the
> muxes) for every port may get difficult.
Hey Heikki, thank you for your quick response!
In general we do have control over the ACPI tables and over DT. The
difference for ChromeOS is that the PD implementation and policy
decisions are handled by the embedded controller. This includes
alternate mode transitions and control over the muxes. I don't believe
there is any information about port capabilities in ACPI or DT, that's
all handled by the EC. With current EC firmware we are mostly limited
to querying the EC for port capabilities and state. There may be some
exceptions to this, such as with Rockchip platforms, but even then the
EC is largely in control.
I think you raise a valid point, but such a change is probably out of
scope for this implementation.
Thanks,
-Jon
>
> Br,
>
> --
> heikki
Hi Jon,
On Wed, Nov 13, 2019 at 05:09:56PM -0800, Jon Flatley wrote:
> > I'll go over these tomorrow, but I have one question already. Can you
> > guys influence what goes to the ACPI tables?
> >
> > Ideally every Type-C connector is always described in its own ACPI
> > node (or DT node if DT is used). Otherwise getting the correct
> > capabilities and especially connections to other devices (like the
> > muxes) for every port may get difficult.
>
> Hey Heikki, thank you for your quick response!
>
> In general we do have control over the ACPI tables and over DT. The
> difference for ChromeOS is that the PD implementation and policy
> decisions are handled by the embedded controller. This includes
> alternate mode transitions and control over the muxes. I don't believe
> there is any information about port capabilities in ACPI or DT, that's
> all handled by the EC. With current EC firmware we are mostly limited
> to querying the EC for port capabilities and state. There may be some
> exceptions to this, such as with Rockchip platforms, but even then the
> EC is largely in control.
The capabilities here mean things like is the port: source, sink or
DRP; host, device or DRD; etc. So static information.
I do understand that the EC is in control of the Port Controller (or
PD controller), the muxes, the policy decisions and what have you, and
that is fine. My point is that the operating system should not have to
get also the hardware description from the EC. That part should always
come from ACPI tables or DT, even when the components are attached to
the EC instead of the host CPU. Otherwise we loose scalability for no
good reason.
Note. The device properties for the port capabilities are already
documented in kernel:
Documentation/devicetree/bindings/connector/usb-connector.txt (the
same properties work in ACPI as well).
> I think you raise a valid point, but such a change is probably out of
> scope for this implementation.
This implementation should already be made so that it works with a
properly prepared ACPI tables or DT. If there are already boards that
don't supply the nodes in ACPI tables for the ports, then software
nodes can be used with those, but all new boards really should have a
real firmware node represeting every Type-C port.
thanks,
--
heikki
On Tue, Nov 12, 2019 at 07:10:44PM -0800, Jon Flatley wrote:
> Adds the cros-ec-typec driver for implementing the USB Type-C connector
> class for cros-ec devices.
>
> Signed-off-by: Jon Flatley <[email protected]>
> ---
> drivers/mfd/cros_ec_dev.c | 7 +-
> drivers/platform/chrome/Kconfig | 11 +
> drivers/platform/chrome/Makefile | 1 +
> drivers/platform/chrome/cros_ec_typec.c | 457 ++++++++++++++++++++++++
> 4 files changed, 473 insertions(+), 3 deletions(-)
> create mode 100644 drivers/platform/chrome/cros_ec_typec.c
>
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index 6e6dfd6c1871..1136ef986695 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -78,9 +78,10 @@ static const struct mfd_cell cros_ec_rtc_cells[] = {
> { .name = "cros-ec-rtc", },
> };
>
> -static const struct mfd_cell cros_usbpd_charger_cells[] = {
> +static const struct mfd_cell cros_usbpd_cells[] = {
> { .name = "cros-usbpd-charger", },
> { .name = "cros-usbpd-logger", },
> + { .name = "cros-ec-typec" },
> };
>
> static const struct cros_feature_to_cells cros_subdevices[] = {
> @@ -96,8 +97,8 @@ static const struct cros_feature_to_cells cros_subdevices[] = {
> },
> {
> .id = EC_FEATURE_USB_PD,
> - .mfd_cells = cros_usbpd_charger_cells,
> - .num_cells = ARRAY_SIZE(cros_usbpd_charger_cells),
> + .mfd_cells = cros_usbpd_cells,
> + .num_cells = ARRAY_SIZE(cros_usbpd_cells),
> },
> };
>
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index d4a55b64bc29..1be5c86f2aad 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -210,6 +210,17 @@ config CROS_EC_SYSFS
> To compile this driver as a module, choose M here: the
> module will be called cros_ec_sysfs.
>
> +config CROS_EC_TYPEC
> + tristate "ChromeOS Embedded Controller USB Type-C class subdriver"
> + depends on TYPEC && CROS_EC_USBPD_NOTIFY
> + default n
> + help
> + Say Y here to enable the USB Type-C class subdriver for ChromeOS
> + devices.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called cros-ec-typec.
> +
> config CROS_USBPD_LOGGER
> tristate "Logging driver for USB PD charger"
> depends on CHARGER_CROS_USBPD
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index efa355ab526f..bb298c083f1e 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -22,5 +22,6 @@ obj-$(CONFIG_CROS_EC_DEBUGFS) += cros_ec_debugfs.o
> obj-$(CONFIG_CROS_EC_SYSFS) += cros_ec_sysfs.o
> obj-$(CONFIG_CROS_USBPD_LOGGER) += cros_usbpd_logger.o
> obj-$(CONFIG_CROS_EC_USBPD_NOTIFY) += cros_ec_usbpd_notify.o
> +obj-$(CONFIG_CROS_EC_TYPEC) += cros_ec_typec.o
>
> obj-$(CONFIG_WILCO_EC) += wilco_ec/
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> new file mode 100644
> index 000000000000..262b914f0a2e
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -0,0 +1,457 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * cros_ec_usbpd - ChromeOS EC Power Delivery Driver
> + *
> + * Copyright (C) 2019 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/kernel.h>
> +#include <linux/kthread.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_usbpd_notify.h>
> +#include <linux/platform_data/cros_ec_proto.h>
> +#include <linux/usb/typec.h>
> +#include <linux/slab.h>
> +
> +#define DRV_NAME "cros-ec-typec"
> +
> +struct port_data {
> + int port_num;
> + struct typec_port *port;
> + struct typec_partner *partner;
> + struct usb_pd_identity p_identity;
> + struct typec_data *typec;
> + struct typec_capability caps;
> +};
> +
> +struct typec_data {
> + struct device *dev;
> + struct cros_ec_dev *ec_dev;
> + struct port_data *ports[EC_USB_PD_MAX_PORTS];
> + unsigned int num_ports;
> + struct notifier_block notifier;
> +
> + int (*port_update)(struct typec_data *typec, int port_num);
> +};
> +
> +#define caps_to_port_data(_caps_) container_of(_caps_, struct port_data, caps)
> +
> +static int cros_typec_ec_command(struct typec_data *typec,
> + unsigned int version,
> + unsigned int command,
> + void *outdata,
> + unsigned int outsize,
> + void *indata,
> + unsigned int insize)
> +{
> + struct cros_ec_dev *ec_dev = typec->ec_dev;
> + struct cros_ec_command *msg;
> + int ret;
> + char host_cmd[32];
> +
> + msg = kzalloc(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;
> +
> + if (outsize)
> + memcpy(msg->data, outdata, outsize);
> +
> + sprintf(host_cmd, "typec HC 0x%x req: ", command);
> + print_hex_dump(KERN_DEBUG, host_cmd, DUMP_PREFIX_NONE, 16, 1, outdata,
> + outsize, 0);
> +
> + ret = cros_ec_cmd_xfer_status(typec->ec_dev->ec_dev, msg);
> + if (ret >= 0 && insize)
> + memcpy(indata, msg->data, insize);
> + sprintf(host_cmd, "typec HC 0x%x res: ", command);
> + print_hex_dump(KERN_DEBUG, host_cmd, DUMP_PREFIX_NONE, 16, 1, indata,
> + insize, 0);
> +
> + kfree(msg);
> + return ret;
> +}
> +
> +static int cros_typec_get_cmd_version(struct typec_data *typec, int cmd,
> + uint8_t *version_mask)
> +{
> + struct ec_params_get_cmd_versions req_v0;
> + struct ec_params_get_cmd_versions_v1 req_v1;
> + struct ec_response_get_cmd_versions res;
> + int ret;
> +
> + req_v1.cmd = cmd;
> + ret = cros_typec_ec_command(typec, 1, EC_CMD_GET_CMD_VERSIONS, &req_v1,
> + sizeof(req_v1), &res, sizeof(res));
> + if (ret < 0) {
> + req_v0.cmd = cmd;
> + ret = cros_typec_ec_command(typec, 0, EC_CMD_GET_CMD_VERSIONS,
> + &req_v0, sizeof(req_v0), &res, sizeof(res));
> + if (ret < 0)
> + return ret;
> + }
> + *version_mask = res.version_mask;
> + dev_dbg(typec->dev, "EC CMD 0x%hhx has version mask 0x%hhx\n", cmd,
> + *version_mask);
> + return 0;
> +}
> +
> +static int cros_typec_query_pd_port_count(struct typec_data *typec)
> +{
> + struct ec_response_usb_pd_ports res;
> + int ret;
> +
> + ret = cros_typec_ec_command(typec, 0, EC_CMD_USB_PD_PORTS, NULL, 0,
> + &res, sizeof(res));
> + if (ret < 0)
> + return ret;
> + typec->num_ports = res.num_ports;
> + return 0;
> +}
> +
> +static int cros_typec_port_update(struct typec_data *typec,
> + int port_num,
> + struct ec_response_usb_pd_control *res,
> + size_t res_size,
> + int cmd_ver)
> +{
> + struct port_data *port;
> + struct ec_params_usb_pd_control req;
> + int ret;
> +
> + if (port_num < 0 || port_num >= typec->num_ports) {
> + dev_err(typec->dev, "cannot get status for invalid port %d\n",
> + port_num);
> + return -EPERM;
> + }
> + port = typec->ports[port_num];
> +
> + req.port = port_num;
> + req.role = USB_PD_CTRL_ROLE_NO_CHANGE;
> + req.mux = USB_PD_CTRL_MUX_NO_CHANGE;
> + req.swap = USB_PD_CTRL_SWAP_NONE;
Unless I'm mistaken, those constants are all 0. How about this:
struct ec_params_usb_pd_control req = {?};
...
req.port = port_num;
> + ret = cros_typec_ec_command(typec, cmd_ver, EC_CMD_USB_PD_CONTROL, &req,
> + sizeof(req), res, res_size);
> + if (ret < 0)
> + return ret;
> + dev_dbg(typec->dev, "Enabled %d: 0x%hhx\n", port_num, res->enabled);
> + dev_dbg(typec->dev, "Role %d: 0x%hhx\n", port_num, res->role);
> + dev_dbg(typec->dev, "Polarity %d: 0x%hhx\n", port_num, res->polarity);
> +
> + return 0;
> +}
> +
> +static int cros_typec_query_pd_info(struct typec_data *typec, int port_num)
> +{
> + struct ec_params_usb_pd_info_request req;
> + struct ec_params_usb_pd_discovery_entry res;
> + int ret;
> +
> + req.port = port_num;
> + ret = cros_typec_ec_command(typec, 0, EC_CMD_USB_PD_DISCOVERY, &req,
> + sizeof(req), &res, sizeof(res));
> + if (ret < 0)
> + return ret;
> + /* FIXME Needs the rest of the info in PD Spec 6.4.4.3.1.1 */
> + typec->ports[port_num]->p_identity.id_header = res.vid;
> +
> + /* FIXME Needs bcdDevice to match PD Spec 6.4.4.3.1.3 */
> + typec->ports[port_num]->p_identity.product = res.pid << 16;
> + return 0;
> +}
> +
> +static int cros_typec_port_update_v0(struct typec_data *typec, int port_num)
> +{
> + struct port_data *port;
> + struct ec_response_usb_pd_control res;
> + enum typec_orientation polarity;
> + int ret;
> +
> + port = typec->ports[port_num];
> + ret = cros_typec_port_update(typec, port_num, &res, sizeof(res), 0);
> + if (ret < 0)
> + return ret;
> + dev_dbg(typec->dev, "State %d: %hhx\n", port_num, res.state);
> +
> + if (!res.enabled)
> + polarity = TYPEC_ORIENTATION_NONE;
> + else if (!res.polarity)
> + polarity = TYPEC_ORIENTATION_NORMAL;
> + else
> + polarity = TYPEC_ORIENTATION_REVERSE;
> +
> + typec_set_pwr_role(port->port, res.role ? TYPEC_SOURCE : TYPEC_SINK);
> + typec_set_orientation(port->port, polarity);
> +
> + return 0;
> +}
> +
> +static int cros_typec_add_partner(struct typec_data *typec, int port_num,
> + bool pd_enabled)
> +{
> + struct port_data *port;
> + struct typec_partner_desc p_desc;
> + int ret;
> +
> + port = typec->ports[port_num];
> + p_desc.usb_pd = pd_enabled;
> + p_desc.identity = &port->p_identity;
> +
> + port->partner = typec_register_partner(port->port, &p_desc);
> + if (IS_ERR_OR_NULL(port->partner)) {
> + dev_err(typec->dev, "Port %d partner register failed\n",
> + port_num);
> + port->partner = NULL;
> + return PTR_ERR(port->partner);
That is the same as:
return PTR_ERR(NULL);
> + }
> +
> + ret = cros_typec_query_pd_info(typec, port_num);
> + if (ret < 0) {
> + dev_err(typec->dev, "Port %d PD query failed\n", port_num);
> + typec_unregister_partner(port->partner);
> + port->partner = NULL;
> + return ret;
> + }
> +
> + ret = typec_partner_set_identity(port->partner);
> + return ret;
> +}
> +
> +static int cros_typec_set_port_params_v1_v2(struct typec_data *typec,
> + int port_num, struct ec_response_usb_pd_control_v1 *res)
> +{
> + struct port_data *port;
> + enum typec_orientation polarity;
> + int ret;
> +
> + port = typec->ports[port_num];
> + if (!(res->enabled & PD_CTRL_RESP_ENABLED_CONNECTED))
> + polarity = TYPEC_ORIENTATION_NONE;
> + else if (!res->polarity)
> + polarity = TYPEC_ORIENTATION_NORMAL;
> + else
> + polarity = TYPEC_ORIENTATION_REVERSE;
> + typec_set_orientation(port->port, polarity);
> + typec_set_data_role(port->port, res->role & PD_CTRL_RESP_ROLE_DATA ?
> + TYPEC_HOST : TYPEC_DEVICE);
> + typec_set_pwr_role(port->port, res->role & PD_CTRL_RESP_ROLE_POWER ?
> + TYPEC_SOURCE : TYPEC_SINK);
> + typec_set_vconn_role(port->port, res->role & PD_CTRL_RESP_ROLE_VCONN ?
> + TYPEC_SOURCE : TYPEC_SINK);
> +
> + if (res->enabled & PD_CTRL_RESP_ENABLED_CONNECTED && !port->partner) {
> + bool pd_enabled =
> + res->enabled & PD_CTRL_RESP_ENABLED_PD_CAPABLE;
> + ret = cros_typec_add_partner(typec, port_num, pd_enabled);
> + if (!ret)
> + return ret;
> + }
> + if (!(res->enabled & PD_CTRL_RESP_ENABLED_CONNECTED) && port->partner) {
> + typec_unregister_partner(port->partner);
> + port->partner = NULL;
> + }
> + return 0;
> +}
> +
> +static int cros_typec_port_update_v1(struct typec_data *typec, int port_num)
> +{
> + /*struct port_data *port;*/
> + struct ec_response_usb_pd_control_v1 res;
> + struct ec_response_usb_pd_control *res_v0 =
> + (struct ec_response_usb_pd_control *) &res;
> + int ret;
> +
> + ret = cros_typec_port_update(typec, port_num, res_v0, sizeof(res), 1);
> + if (ret < 0)
> + return ret;
> + dev_dbg(typec->dev, "State %d: %s\n", port_num, res.state);
> +
> + ret = cros_typec_set_port_params_v1_v2(typec, port_num, &res);
> + return ret;
> +}
> +
> +static int cros_typec_try_role(const struct typec_capability *cap, int role)
> +{
> + return 0;
> +}
> +
> +static int cros_typec_dr_set(const struct typec_capability *cap,
> + enum typec_data_role role)
> +{
> + return 0;
> +}
> +
> +static int cros_typec_pr_set(const struct typec_capability *cap,
> + enum typec_role role)
> +{
> + return 0;
> +}
> +
> +static int cros_typec_vconn_set(const struct typec_capability *cap,
> + enum typec_role role)
> +{
> + return 0;
> +}
Those stubs are not needed.
> +static int cros_typec_ec_event(struct notifier_block *nb,
> + unsigned long queued_during_suspend,
> + void *_notify)
> +{
> + struct typec_data *typec;
> + int i;
> +
> + typec = container_of(nb, struct typec_data, notifier);
> +
> + for (i = 0; i < typec->num_ports; ++i)
> + typec->port_update(typec, i);
> +
> + return NOTIFY_DONE;
> +
> +}
> +
> +static void cros_typec_unregister_notifier(void *data)
> +{
> + struct typec_data *typec = data;
> +
> + cros_ec_usbpd_unregister_notify(&typec->notifier);
> +}
> +
> +static int cros_typec_probe(struct platform_device *pd)
> +{
> + struct cros_ec_dev *ec_dev = dev_get_drvdata(pd->dev.parent);
> + struct device *dev = &pd->dev;
> + struct typec_data *typec;
> + uint8_t ver_mask;
> + int i;
> + int ret;
> +
> + dev_dbg(dev, "Probing Cros EC Type-C device.\n");
> +
> + typec = devm_kzalloc(dev, sizeof(*typec), GFP_KERNEL);
> + if (!typec)
> + return -ENOMEM;
> +
> + typec->dev = dev;
> + typec->ec_dev = ec_dev;
> +
> + platform_set_drvdata(pd, typec);
> +
> + ret = cros_typec_query_pd_port_count(typec);
> + if (ret < 0) {
> + dev_err(dev, "Failed to get PD port count from EC\n");
> + return ret;
> + }
> + if (typec->num_ports > EC_USB_PD_MAX_PORTS) {
> + dev_err(dev, "EC reported too many ports. got: %d, max: %d\n",
> + typec->num_ports, EC_USB_PD_MAX_PORTS);
> + return -EOVERFLOW;
> + }
This step should to be done in the mfd driver. You can then register
a separate device for each port.
For exact hardware description you will need a software node that
represents the "port controller" that you'll give to this device. That
software node will need a child node named "connector" that represents
the Type-C connector. That child node has the actual capability
properties.
I just remembered that the MFD framework does not yet support software
nodes directly, but I have a patch for that. I'm attaching here.
For examples how to use software nodes you can check
drivers/platform/x86/intel_cht_int33fe_typec.c
Or if you prefer, I can also write the mfd patch for you?
> + ret = cros_typec_get_cmd_version(typec, EC_CMD_USB_PD_CONTROL,
> + &ver_mask);
> + if (ret < 0) {
> + dev_err(dev, "Failed to get supported PD command versions\n");
> + return ret;
> + }
> + /* No reason to support EC_CMD_USB_PD_CONTROL v2 as it doesn't add any
> + * useful information
> + */
> + if (ver_mask & EC_VER_MASK(1)) {
> + dev_dbg(dev, "Using PD command ver 1\n");
> + typec->port_update = cros_typec_port_update_v1;
> + } else {
> + dev_dbg(dev, "Using PD command ver 0\n");
> + typec->port_update = cros_typec_port_update_v0;
> + }
> +
> + for (i = 0; i < typec->num_ports; ++i) {
After there is a one device per port, this loop is of course no longer
needed. This driver just needs to know the port number that its port
has, and that it can get also from a device property. That property
you need to give to the parent software node that represents the
"port controller", so not the "connector" child node.
> + struct port_data *port;
> +
> + port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> + if (!port) {
> + ret = -ENOMEM;
> + goto unregister_ports;
> + }
> + port->typec = typec;
> + port->port_num = i;
> + typec->ports[i] = port;
> +
> + /* TODO Make sure these values are accurate */
> + port->caps.type = TYPEC_PORT_DRP;
> + port->caps.data = TYPEC_PORT_DFP;
> + port->caps.prefer_role = TYPEC_SINK;
These are details that we can get from the device properties.
> + port->caps.try_role = cros_typec_try_role;
> + port->caps.dr_set = cros_typec_dr_set;
> + port->caps.pr_set = cros_typec_pr_set;
> + port->caps.vconn_set = cros_typec_vconn_set;
> + port->caps.port_type_set = NULL; /* Not permitted by PD spec */
> +
> + port->port = typec_register_port(dev, &port->caps);
> + if (IS_ERR_OR_NULL(port->port)) {
> + dev_err(dev, "Failed to register typec port %d\n", i);
> + ret = PTR_ERR(port->port);
> + goto unregister_ports;
> + }
> +
> + ret = typec->port_update(typec, i);
> + if (ret < 0) {
> + dev_err(dev, "Failed to update typec port %d\n", i);
> + goto unregister_ports;
> + }
> + }
> +
> + typec->notifier.notifier_call = cros_typec_ec_event;
> + ret = cros_ec_usbpd_register_notify(&typec->notifier);
> + if (ret < 0) {
> + dev_warn(dev, "Failed to register notifier\n");
> + } else {
> + ret = devm_add_action_or_reset(dev,
> + cros_typec_unregister_notifier, typec);
> + if (ret < 0)
> + goto unregister_ports;
> + dev_dbg(dev, "Registered EC notifier\n");
> + }
> +
> + return 0;
> +
> +unregister_ports:
> + for (i = 0; i < typec->num_ports; ++i) {
> + if (typec->ports[i] && typec->ports[i]->port)
> + typec_unregister_port(typec->ports[i]->port);
> + }
> +
> + return ret;
> +}
> +
> +static struct platform_driver cros_ec_typec_driver = {
> + .driver = {
> + .name = DRV_NAME,
> + },
> + .probe = cros_typec_probe
> +};
> +
> +module_platform_driver(cros_ec_typec_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("ChromeOS EC USB-C connectors");
> +MODULE_AUTHOR("Jon Flatley <[email protected]>");
> +MODULE_ALIAS("platform:" DRV_NAME);
thanks,
--
heikki
On Thu, Nov 14, 2019 at 7:24 AM Heikki Krogerus
<[email protected]> wrote:
>
> Hi Jon,
>
> On Wed, Nov 13, 2019 at 05:09:56PM -0800, Jon Flatley wrote:
> > > I'll go over these tomorrow, but I have one question already. Can you
> > > guys influence what goes to the ACPI tables?
> > >
> > > Ideally every Type-C connector is always described in its own ACPI
> > > node (or DT node if DT is used). Otherwise getting the correct
> > > capabilities and especially connections to other devices (like the
> > > muxes) for every port may get difficult.
> >
> > Hey Heikki, thank you for your quick response!
> >
> > In general we do have control over the ACPI tables and over DT. The
> > difference for ChromeOS is that the PD implementation and policy
> > decisions are handled by the embedded controller. This includes
> > alternate mode transitions and control over the muxes. I don't believe
> > there is any information about port capabilities in ACPI or DT, that's
> > all handled by the EC. With current EC firmware we are mostly limited
> > to querying the EC for port capabilities and state. There may be some
> > exceptions to this, such as with Rockchip platforms, but even then the
> > EC is largely in control.
>
> The capabilities here mean things like is the port: source, sink or
> DRP; host, device or DRD; etc. So static information.
>
> I do understand that the EC is in control of the Port Controller (or
> PD controller), the muxes, the policy decisions and what have you, and
> that is fine. My point is that the operating system should not have to
> get also the hardware description from the EC. That part should always
> come from ACPI tables or DT, even when the components are attached to
> the EC instead of the host CPU. Otherwise we loose scalability for no
> good reason.
>
> Note. The device properties for the port capabilities are already
> documented in kernel:
> Documentation/devicetree/bindings/connector/usb-connector.txt (the
> same properties work in ACPI as well).
>
> > I think you raise a valid point, but such a change is probably out of
> > scope for this implementation.
>
> This implementation should already be made so that it works with a
> properly prepared ACPI tables or DT. If there are already boards that
> don't supply the nodes in ACPI tables for the ports, then software
> nodes can be used with those, but all new boards really should have a
> real firmware node represeting every Type-C port.
Hey Heikki,
I spoke with Benson and Prashant and you raise a good point. Moving
forward we should probably be describing these capabilities in ACPI.
We do want to support existing devices, and making changes to the ACPI
tables would mean firmware modifications for each and every one, which
is a complicated process.
To date the port capabilities on all ChromeOS devices have been the
same. I recall now that we don't (and with current firmware can't)
query the port capabilities from the EC; they're just hard coded into
the driver. In the absence of these nodes in the ACPI tables we can
populate these capabilities in software nodes. This would allow us to
support existing systems without the expensive firmware change, and I
think it still provides the scalability you're asking for.
Are you suggesting that every port on the device gets its own ACPI/DT node?
Thanks,
-Jon
>
> thanks,
>
> --
> heikki
Hi Jon,
Many thanks for working on this and push it upstream. Some comments below
Missatge de Jon Flatley <[email protected]> del dia dc., 13 de nov.
2019 a les 4:13:
>
> ChromiumOS uses ACPI device with HID "GOOG0003" for power delivery
> related events. The existing cros-usbpd-charger driver relies on these
> events without ever actually receiving them on ACPI platforms. This is
> because in the ChromeOS kernel trees, the GOOG0003 device is owned by an
> ACPI driver that offers firmware updates to USB-C chargers.
>
> Let's introduce a new platform driver under cros-ec, the ChromeOS
> embedded controller, that handles these PD events and dispatches them
> appropriately over a notifier chain to all drivers that use them.
>
> Signed-off-by: Jon Flatley <[email protected]>
> ---
> drivers/platform/chrome/Kconfig | 9 +
> drivers/platform/chrome/Makefile | 1 +
> .../platform/chrome/cros_ec_usbpd_notify.c | 156 ++++++++++++++++++
> .../platform_data/cros_ec_usbpd_notify.h | 40 +++++
> 4 files changed, 206 insertions(+)
> create mode 100644 drivers/platform/chrome/cros_ec_usbpd_notify.c
> create mode 100644 include/linux/platform_data/cros_ec_usbpd_notify.h
>
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index ee5f08ea57b6..d4a55b64bc29 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -118,6 +118,15 @@ config CROS_EC_SPI
> response time cannot be guaranteed, we support ignoring
> 'pre-amble' bytes before the response actually starts.
>
> +config CROS_EC_USBPD_NOTIFY
> + tristate "ChromeOS USB-C power delivery event notifier"
> + depends on CROS_EC
> + help
> + If you say Y here, you get support for USB-C PD event notifications
> + from the ChromeOS EC. On ACPI platorms this driver will bind to the
> + GOOG0003 ACPI device, and on non-ACPI platform this driver will match
> + "google,cros-ec-pd-update" in device tree.
> +
> config CROS_EC_LPC
> tristate "ChromeOS Embedded Controller (LPC)"
> depends on CROS_EC && ACPI && (X86 || COMPILE_TEST)
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index 477ec3d1d1c9..efa355ab526f 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -21,5 +21,6 @@ obj-$(CONFIG_CROS_EC_VBC) += cros_ec_vbc.o
> obj-$(CONFIG_CROS_EC_DEBUGFS) += cros_ec_debugfs.o
> obj-$(CONFIG_CROS_EC_SYSFS) += cros_ec_sysfs.o
> obj-$(CONFIG_CROS_USBPD_LOGGER) += cros_usbpd_logger.o
> +obj-$(CONFIG_CROS_EC_USBPD_NOTIFY) += cros_ec_usbpd_notify.o
>
> obj-$(CONFIG_WILCO_EC) += wilco_ec/
> diff --git a/drivers/platform/chrome/cros_ec_usbpd_notify.c b/drivers/platform/chrome/cros_ec_usbpd_notify.c
> new file mode 100644
> index 000000000000..f654586dea2a
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_usbpd_notify.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * cros_ec_usbpd - ChromeOS EC Power Delivery Driver
Please remove 'cros_ec_usbpd -'
> + *
> + * Copyright (C) 2019 Google, Inc
> + *
I think it should be
Copyright 2019 Google LLC
> + * 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.
> + *
Remove the license boilerplate, with the SPDX identifier, is enough.
> + * This driver serves as the receiver of cros_ec PD host events.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_usbpd_notify.h>
> +#include <linux/platform_data/cros_ec_proto.h>
> +#include <linux/platform_device.h>
> +
> +#define DRV_NAME "cros-ec-usbpd-notify"
> +#define ACPI_DRV_NAME "GOOG0003"
> +
> +static BLOCKING_NOTIFIER_HEAD(cros_ec_usbpd_notifier_list);
> +
> +int cros_ec_usbpd_register_notify(struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_register(
> + &cros_ec_usbpd_notifier_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(cros_ec_usbpd_register_notify);
> +
> +void cros_ec_usbpd_unregister_notify(struct notifier_block *nb)
> +{
> + blocking_notifier_chain_unregister(&cros_ec_usbpd_notifier_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(cros_ec_usbpd_unregister_notify);
> +
> +static void cros_ec_usbpd_notify(u32 event)
> +{
> + blocking_notifier_call_chain(&cros_ec_usbpd_notifier_list, event, NULL);
> +}
That function is not needed, just call directly blocking_notifier_call_chain.
> +
> +#ifdef CONFIG_ACPI
> +
I think you can create a single platform_driver ACPI and non-ACPI
compatible and get rid of this ifdef
> +static int cros_ec_usbpd_add_acpi(struct acpi_device *adev)
> +{
> + return 0;
> +}
> +
Could be probably be removed
> +static int cros_ec_usbpd_remove_acpi(struct acpi_device *adev)
> +{
> + return 0;
> +}
Could be probably be removed
> +static void cros_ec_usbpd_notify_acpi(struct acpi_device *adev, u32 event)
> +{
> + cros_ec_usbpd_notify(event);
> +}
> +
> +static const struct acpi_device_id cros_ec_usbpd_acpi_device_ids[] = {
> + { ACPI_DRV_NAME, 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, cros_ec_usbpd_acpi_device_ids);
> +
> +static struct acpi_driver cros_ec_usbpd_driver = {
> + .name = DRV_NAME,
> + .class = DRV_NAME,
> + .ids = cros_ec_usbpd_acpi_device_ids,
> + .ops = {
> + .add = cros_ec_usbpd_add_acpi,
> + .remove = cros_ec_usbpd_remove_acpi,
> + .notify = cros_ec_usbpd_notify_acpi,
> + },
> +};
> +module_acpi_driver(cros_ec_usbpd_driver);
> +
> +#else /* CONFIG_ACPI */
> +
> +static int cros_ec_usbpd_notify_plat(struct notifier_block *nb,
> + unsigned long queued_during_suspend, void *data)
> +{
> + struct cros_ec_device *ec_dev = (struct cros_ec_device *)data;
> + u32 host_event = cros_ec_get_host_event(ec_dev);
> +
> + if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_PD_MCU)) {
> + cros_ec_usbpd_notify(host_event);
> + return NOTIFY_OK;
> + } else {
Remove the else and just return NOTIFY_DONE
> + return NOTIFY_DONE;
> + }
> +
Remove the extra blank line.
> +}
> +
> +static int cros_ec_usbpd_probe_plat(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct cros_ec_device *ec_dev = dev_get_drvdata(dev->parent);
> + struct notifier_block *nb;
> + int ret;
> +
> + nb = devm_kzalloc(dev, sizeof(*nb), GFP_KERNEL);
> + if (!nb)
> + return -ENOMEM;
> +
> + nb->notifier_call = cros_ec_usbpd_notify_plat;
> + dev_set_drvdata(dev, nb);
> + ret = blocking_notifier_chain_register(&ec_dev->event_notifier, nb);
> +
Remove the extra blank line
> + if (ret < 0)
> + dev_warn(dev, "Failed to register notifier\n");
Only a warning? shouldn't you return an error?
> +
> + return 0;
> +}
> +
> +static int cros_ec_usbpd_remove_plat(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct cros_ec_device *ec_dev = dev_get_drvdata(dev->parent);
> + struct notifier_block *nb =
> + (struct notifier_block *)dev_get_drvdata(dev);
> +
> + blocking_notifier_chain_unregister(&ec_dev->event_notifier, nb);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id cros_ec_usbpd_of_match[] = {
> + { .compatible = "google,cros-ec-pd-update" },
This should be documented in Documentation/devicetree/bindings
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, cros_ec_usbpd_of_match);
> +
> +static struct platform_driver cros_ec_usbpd_driver = {
> + .driver = {
> + .name = DRV_NAME,
> + .of_match_table = of_match_ptr(cros_ec_usbpd_of_match),
> + },
> + .probe = cros_ec_usbpd_probe_plat,
> + .remove = cros_ec_usbpd_remove_plat,
> +};
> +module_platform_driver(cros_ec_usbpd_driver);
> +
> +#endif /* CONFIG_ACPI */
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("ChromeOS power delivery device");
> +MODULE_AUTHOR("Jon Flatley <[email protected]>");
> +MODULE_ALIAS("platform:" DRV_NAME);
> diff --git a/include/linux/platform_data/cros_ec_usbpd_notify.h b/include/linux/platform_data/cros_ec_usbpd_notify.h
> new file mode 100644
> index 000000000000..fdcea146b7c4
> --- /dev/null
> +++ b/include/linux/platform_data/cros_ec_usbpd_notify.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * cros_ec_usbpd - ChromeOS EC Power Delivery Driver
> + *
Remove 'cros_ec_usbpd -'
> + * Copyright (C) 2019 Google, Inc
> + *
Copyright 2019 Google LLC
> + * 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.
Remove the license boiler plate
> + */
> +
> +#ifndef __LINUX_PLATFORM_DATA_CROS_EC_USBPD_NOTIFY_H
> +#define __LINUX_PLATFORM_DATA_CROS_EC_USBPD_NOTIFY_H
> +
> +#include <linux/notifier.h>
> +
> +/**
> + * cros_ec_usbpd_register_notify - register a notifier callback for USB PD
> + * events. On ACPI platforms this corrisponds to to host events on the ECPD
> + * "GOOG0003" ACPI device. On non-ACPI platforms this will filter mkbp events
> + * for USB PD events.
> + *
> + * @nb: Notifier block pointer to register
> + */
I don't think this is kernel-doc compliant, please check running kernel-doc -v
> +int cros_ec_usbpd_register_notify(struct notifier_block *nb);
> +
> +/**
> + * cros_ec_usbpd_unregister_notify - unregister a notifier callback that was
> + * previously registered with cros_ec_usbpd_register_notify().
> + *
> + * @nb: Notifier block pointer to unregister
> + */
Same here
> +void cros_ec_usbpd_unregister_notify(struct notifier_block *nb);
> +
> +#endif /* __LINUX_PLATFORM_DATA_CROS_EC_USBPD_NOTIFY_H */
> --
> 2.24.0.432.g9d3f5f5b63-goog
>
Thanks,
Enric
Hi,
On Thu, Nov 14, 2019 at 01:00:42PM -0800, Jon Flatley wrote:
> On Thu, Nov 14, 2019 at 7:24 AM Heikki Krogerus
> <[email protected]> wrote:
> >
> > Hi Jon,
> >
> > On Wed, Nov 13, 2019 at 05:09:56PM -0800, Jon Flatley wrote:
> > > > I'll go over these tomorrow, but I have one question already. Can you
> > > > guys influence what goes to the ACPI tables?
> > > >
> > > > Ideally every Type-C connector is always described in its own ACPI
> > > > node (or DT node if DT is used). Otherwise getting the correct
> > > > capabilities and especially connections to other devices (like the
> > > > muxes) for every port may get difficult.
> > >
> > > Hey Heikki, thank you for your quick response!
> > >
> > > In general we do have control over the ACPI tables and over DT. The
> > > difference for ChromeOS is that the PD implementation and policy
> > > decisions are handled by the embedded controller. This includes
> > > alternate mode transitions and control over the muxes. I don't believe
> > > there is any information about port capabilities in ACPI or DT, that's
> > > all handled by the EC. With current EC firmware we are mostly limited
> > > to querying the EC for port capabilities and state. There may be some
> > > exceptions to this, such as with Rockchip platforms, but even then the
> > > EC is largely in control.
> >
> > The capabilities here mean things like is the port: source, sink or
> > DRP; host, device or DRD; etc. So static information.
> >
> > I do understand that the EC is in control of the Port Controller (or
> > PD controller), the muxes, the policy decisions and what have you, and
> > that is fine. My point is that the operating system should not have to
> > get also the hardware description from the EC. That part should always
> > come from ACPI tables or DT, even when the components are attached to
> > the EC instead of the host CPU. Otherwise we loose scalability for no
> > good reason.
> >
> > Note. The device properties for the port capabilities are already
> > documented in kernel:
> > Documentation/devicetree/bindings/connector/usb-connector.txt (the
> > same properties work in ACPI as well).
> >
> > > I think you raise a valid point, but such a change is probably out of
> > > scope for this implementation.
> >
> > This implementation should already be made so that it works with a
> > properly prepared ACPI tables or DT. If there are already boards that
> > don't supply the nodes in ACPI tables for the ports, then software
> > nodes can be used with those, but all new boards really should have a
> > real firmware node represeting every Type-C port.
>
> Hey Heikki,
>
> I spoke with Benson and Prashant and you raise a good point. Moving
> forward we should probably be describing these capabilities in ACPI.
> We do want to support existing devices, and making changes to the ACPI
> tables would mean firmware modifications for each and every one, which
> is a complicated process.
>
> To date the port capabilities on all ChromeOS devices have been the
> same. I recall now that we don't (and with current firmware can't)
> query the port capabilities from the EC; they're just hard coded into
> the driver. In the absence of these nodes in the ACPI tables we can
> populate these capabilities in software nodes. This would allow us to
> support existing systems without the expensive firmware change, and I
> think it still provides the scalability you're asking for.
>
> Are you suggesting that every port on the device gets its own ACPI/DT node?
Yes. Every port (or maybe I should say "connector") should always have
its own ACPI/DT node.
There should also be a separate parent node for the port nodes that
represents the USB Type-C controller part (in EC), meaning you
probable don't want to simply make the port nodes child nodes directly
under the root EC node (the one that has ACPI HID GOOG0004 or
GOOG0008). The controller node is the one that is child of the root EC
node, and the port nodes are children of the controller. The
controller node ideally will have its own ACPI HID, but the port nodes
themselves don't need HIDs. I guess this part is clear..
I proposed in my review (patch 3/3) that there should be actually a
single parent controller node for every port, so that you would have
always a controller node with a single port node for every connector,
but that may be overkill. I was thinking about past problems where it
is no clear which port (number) should be mapped to which port node,
but you do not have that problem.
So I think you only need a single "EC Type-C Controller" node that
is the parent for the port nodes.
I'll put here a short ASL example snippet that has nodes for two
ports, that hopefully helps explain how I think this should be
described in the ACPI tables:
DefinitionBlock ("usbc.aml", "SSDT", 5, "GOOGLE", "USBC", 1)
{
/* I'm assuming here that H_EC is the EC device. */
Scope (\_SB.H_EC)
{
/*
* This is the controller (port parent) device that communicates with
* the EC. It is the node that would create the device instance for the
* driver (drivers/platform/chrome/cros_ec_typec.c).
*/
Device (USBC)
{
Name (_HID, "GOOGXXXX") /* FIXME!!!!!! */
Name (_DDN, "ChromeOS Embedded Controller USB Type-C Control")
/*
* Each connector shall have its own ACPI device entry (node),
* under the actual interface (controller) device.
*/
Device (CON1)
{
/* Port number 1 */
Name (_ADR, One)
/* The port capability device properties. */
Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package() {
Package () {"power-role", "dual"},
Package () {"data-role", "dual"},
},
})
}
Device (CON2)
{
/* Port number 2 */
Name (_ADR, Two)
Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package() {
Package () {"power-role", "dual"},
Package () {"data-role", "dual"},
},
})
}
}
}
}
To play it safe (and be compatible with DT), the port number does not
have to be the node address (_ADR). You can get the port number also
from a device property as well of course.
Br,
--
heikki