[PATCH v5 0/8] usb: typec: Implement UCSI driver for ChromeOS

[PATCH v5 0/8] usb: typec: Implement UCSI driver for ChromeOS

[Łukasz Bartosik]

This series implements a UCSI ChromeOS EC transport driver. 
The ChromeOS EC is expected to implement UCSI Platform Policy
Manager (PPM).

---
Changes in v5:
- Increased WRITE_TMO_MS to 5000.
- Replaced DRV_NAME with KBUILD_MODNAME.
- Added comments for WRITE_TMO_MS and MAX_EC_DATA_SIZE defines.
- Refactored cros_ucsi_async_control() to dynamically allocate memory
for a message to EC instead of allocating the message on stack.
- Replaced type names uint*_t with u*.
- Removed ret variable in cros_ucsi_work().
- Updated ucsi_operations interface to align with changes introduced in
  v6.11.
- Replaced test_bit() with test_and_clear_bit() in cros_ucsi_work().
- Added trace events in commit "usb: typec: cros_ec_ucsi: Add trace
  events".
- Added netlink in commit "usb: typec: cros_ec_ucsi: Add netlink"
for debugging and testing puropses. 
- Link to v4: https://lore.kernel.org/all/CAB2FV=6We88NrvN8NZYt8NkMFH9N_ZBGyUWVUpGwPdja2X_+NA@mail.gmail.com/T/

Changes in v4:
- Setup notifications before calling ucsi_register.
- Cancel work before destroying driver data.
- Link to v3: https://lore.kernel.org/r/20240403-public-ucsi-h-v3-0-f848e18c8ed2@chromium.org

Changes in v3:
- Moved driver from platform/chrome to usb/typec/ucsi.
- Used id_table instead of MODULE_ALIAS.
- Split EC header changes into seperate commit.
- Fixes from additional internal reviews and kernel bot warnings.
- Link to v2: https://lore.kernel.org/r/20240325-public-ucsi-h-v2-0-a6d716968bb1@chromium.org

Changes in v2:
- No code or commit message changes.
- Added drivers/platform/chrome maintainers for review.
- Link to v1: https://lore.kernel.org/r/20240325-public-ucsi-h-v1-0-7c7e888edc0a@chromium.org

Abhishek Pandit-Subedi (2):
  usb: typec: cros_ec_ucsi: Use complete instead of resume
  mfd: cros_ec: Don't load charger with UCSI

Pavan Holla (4):
  platform/chrome: Update ChromeOS EC header for UCSI
  platform/chrome: Update EC feature flags
  usb: typec: ucsi: Implement ChromeOS UCSI driver
  mfd: cros_ec: Load cros_ec_ucsi on supported ECs

Łukasz Bartosik (2):
  usb: typec: cros_ec_ucsi: Add trace events
  usb: typec: cros_ec_ucsi: Add netlink

 MAINTAINERS                                   |  10 +
 drivers/mfd/cros_ec_dev.c                     |  25 +-
 drivers/usb/typec/ucsi/Kconfig                |  13 +
 drivers/usb/typec/ucsi/Makefile               |   3 +
 drivers/usb/typec/ucsi/cros_ec_ucsi_main.c    | 351 ++++++++++++++++++
 drivers/usb/typec/ucsi/cros_ec_ucsi_nl.c      |  87 +++++
 drivers/usb/typec/ucsi/cros_ec_ucsi_nl.h      |  52 +++
 drivers/usb/typec/ucsi/cros_ec_ucsi_trace.h   |  92 +++++
 .../linux/platform_data/cros_ec_commands.h    |  54 ++-
 9 files changed, 683 insertions(+), 4 deletions(-)
 create mode 100644 drivers/usb/typec/ucsi/cros_ec_ucsi_main.c
 create mode 100644 drivers/usb/typec/ucsi/cros_ec_ucsi_nl.c
 create mode 100644 drivers/usb/typec/ucsi/cros_ec_ucsi_nl.h
 create mode 100644 drivers/usb/typec/ucsi/cros_ec_ucsi_trace.h

-- 
2.46.0.469.g59c65b2a67-goog

[PATCH v5 1/8] platform/chrome: Update ChromeOS EC header for UCSI

[Łukasz Bartosik]

From: Pavan Holla <pholla@chromium.org>

Add EC host commands for reading and writing UCSI structures
in the EC. The corresponding kernel driver is cros-ec-ucsi.

Also update PD events supported by the EC.

Signed-off-by: Pavan Holla <pholla@chromium.org>
---
 .../linux/platform_data/cros_ec_commands.h    | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
index e574b790be6f..08b6c98ed890 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -5012,8 +5012,10 @@ struct ec_response_pd_status {
 #define PD_EVENT_POWER_CHANGE      BIT(1)
 #define PD_EVENT_IDENTITY_RECEIVED BIT(2)
 #define PD_EVENT_DATA_SWAP         BIT(3)
+#define PD_EVENT_TYPEC             BIT(4)
+#define PD_EVENT_PPM               BIT(5)
 struct ec_response_host_event_status {
-	uint32_t status;      /* PD MCU host event status */
+	u32 status;      /* PD MCU host event status */
 } __ec_align4;
 
 /* Set USB type-C port role and muxes */
@@ -6073,6 +6075,24 @@ struct ec_response_typec_vdm_response {
 
 #undef VDO_MAX_SIZE
 
+/*
+ * Read/write interface for UCSI OPM <-> PPM communication.
+ */
+#define EC_CMD_UCSI_PPM_SET 0x0140
+
+/* The data size is stored in the host command protocol header. */
+struct ec_params_ucsi_ppm_set {
+	u16 offset;
+	u8 data[];
+} __ec_align2;
+
+#define EC_CMD_UCSI_PPM_GET 0x0141
+
+struct ec_params_ucsi_ppm_get {
+	u16 offset;
+	u8 size;
+} __ec_align2;
+
 /*****************************************************************************/
 /* The command range 0x200-0x2FF is reserved for Rotor. */
 
-- 
2.46.0.469.g59c65b2a67-goog

[PATCH v5 2/8] platform/chrome: Update EC feature flags

[Łukasz Bartosik]

From: Pavan Holla <pholla@chromium.org>

Define EC_FEATURE_UCSI_PPM to enable usage of the cros_ec_ucsi
driver. Also, add any feature flags that are implemented by the EC
but are missing in the kernel header.

Signed-off-by: Pavan Holla <pholla@chromium.org>
---
 .../linux/platform_data/cros_ec_commands.h    | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
index 08b6c98ed890..2998d956ba14 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -1312,6 +1312,38 @@ enum ec_feature_code {
 	 * The EC supports the AP composing VDMs for us to send.
 	 */
 	EC_FEATURE_TYPEC_AP_VDM_SEND = 46,
+	/*
+	 * The EC supports system safe mode panic recovery.
+	 */
+	EC_FEATURE_SYSTEM_SAFE_MODE = 47,
+	/*
+	 * The EC will reboot on runtime assertion failures.
+	 */
+	EC_FEATURE_ASSERT_REBOOTS = 48,
+	/*
+	 * The EC image is built with tokenized logging enabled.
+	 */
+	EC_FEATURE_TOKENIZED_LOGGING = 49,
+	/*
+	 * The EC supports triggering an STB dump.
+	 */
+	EC_FEATURE_AMD_STB_DUMP = 50,
+	/*
+	 * The EC supports memory dump commands.
+	 */
+	EC_FEATURE_MEMORY_DUMP = 51,
+	/*
+	 * The EC supports DP2.1 capability
+	 */
+	EC_FEATURE_TYPEC_DP2_1 = 52,
+	/*
+	 * The MCU is System Companion Processor Core 1
+	 */
+	EC_FEATURE_SCP_C1 = 53,
+	/*
+	 * The EC supports UCSI PPM.
+	 */
+	EC_FEATURE_UCSI_PPM = 54,
 };
 
 #define EC_FEATURE_MASK_0(event_code) BIT(event_code % 32)
-- 
2.46.0.469.g59c65b2a67-goog

[PATCH v5 3/8] usb: typec: ucsi: Implement ChromeOS UCSI driver

[Łukasz Bartosik]

From: Pavan Holla <pholla@chromium.org>

Implementation of a UCSI transport driver for ChromeOS.
This driver will be loaded if the ChromeOS EC implements a PPM.

Signed-off-by: Pavan Holla <pholla@chromium.org>
Co-developed-by: Łukasz Bartosik <ukaszb@chromium.org>
Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
---
 MAINTAINERS                           |   7 +
 drivers/usb/typec/ucsi/Kconfig        |  13 ++
 drivers/usb/typec/ucsi/Makefile       |   1 +
 drivers/usb/typec/ucsi/cros_ec_ucsi.c | 278 ++++++++++++++++++++++++++
 4 files changed, 299 insertions(+)
 create mode 100644 drivers/usb/typec/ucsi/cros_ec_ucsi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index fe83ba7194ea..8c030ea0b503 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5300,6 +5300,13 @@ L:	chrome-platform@lists.linux.dev
 S:	Maintained
 F:	drivers/watchdog/cros_ec_wdt.c
 
+CHROMEOS UCSI DRIVER
+M:	Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
+M:	Łukasz Bartosik <ukaszb@chromium.org>
+L:	chrome-platform@lists.linux.dev
+S:	Maintained
+F:	drivers/usb/typec/ucsi/cros_ec_ucsi.c
+
 CHRONTEL CH7322 CEC DRIVER
 M:	Joe Tessler <jrt@google.com>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
index 680e1b87b152..75559601fe8f 100644
--- a/drivers/usb/typec/ucsi/Kconfig
+++ b/drivers/usb/typec/ucsi/Kconfig
@@ -69,6 +69,19 @@ config UCSI_PMIC_GLINK
 	  To compile the driver as a module, choose M here: the module will be
 	  called ucsi_glink.
 
+config CROS_EC_UCSI
+	tristate "UCSI Driver for ChromeOS EC"
+	depends on MFD_CROS_EC_DEV
+	depends on CROS_USBPD_NOTIFY
+	depends on !EXTCON_TCSS_CROS_EC
+	default MFD_CROS_EC_DEV
+	help
+	  This driver enables UCSI support for a ChromeOS EC. The EC is
+	  expected to implement a PPM.
+
+	  To compile the driver as a module, choose M here: the module
+	  will be called cros_ec_ucsi.
+
 config UCSI_LENOVO_YOGA_C630
 	tristate "UCSI Interface Driver for Lenovo Yoga C630"
 	depends on EC_LENOVO_YOGA_C630
diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
index aed41d23887b..be98a879104d 100644
--- a/drivers/usb/typec/ucsi/Makefile
+++ b/drivers/usb/typec/ucsi/Makefile
@@ -21,4 +21,5 @@ obj-$(CONFIG_UCSI_ACPI)			+= ucsi_acpi.o
 obj-$(CONFIG_UCSI_CCG)			+= ucsi_ccg.o
 obj-$(CONFIG_UCSI_STM32G0)		+= ucsi_stm32g0.o
 obj-$(CONFIG_UCSI_PMIC_GLINK)		+= ucsi_glink.o
+obj-$(CONFIG_CROS_EC_UCSI)		+= cros_ec_ucsi.o
 obj-$(CONFIG_UCSI_LENOVO_YOGA_C630)	+= ucsi_yoga_c630.o
diff --git a/drivers/usb/typec/ucsi/cros_ec_ucsi.c b/drivers/usb/typec/ucsi/cros_ec_ucsi.c
new file mode 100644
index 000000000000..6b9dc05a4960
--- /dev/null
+++ b/drivers/usb/typec/ucsi/cros_ec_ucsi.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * UCSI driver for ChromeOS EC
+ *
+ * Copyright 2024 Google LLC.
+ */
+
+#include <linux/container_of.h>
+#include <linux/dev_printk.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_usbpd_notify.h>
+#include <linux/platform_data/cros_ec_proto.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/wait.h>
+
+#include "ucsi.h"
+
+/*
+ * Maximum size in bytes of a UCSI message between AP and EC
+ */
+#define MAX_EC_DATA_SIZE	256
+
+/*
+ * Maximum time in miliseconds the cros_ec_ucsi driver
+ * will wait for a response to a command or and ack.
+ */
+#define WRITE_TMO_MS		5000
+
+struct cros_ucsi_data {
+	struct device *dev;
+	struct ucsi *ucsi;
+
+	struct cros_ec_device *ec;
+	struct notifier_block nb;
+	struct work_struct work;
+
+	struct completion complete;
+	unsigned long flags;
+};
+
+static int cros_ucsi_read(struct ucsi *ucsi, unsigned int offset, void *val,
+			  size_t val_len)
+{
+	struct cros_ucsi_data *udata = ucsi_get_drvdata(ucsi);
+	struct ec_params_ucsi_ppm_get req = {
+		.offset = offset,
+		.size = val_len,
+	};
+	int ret;
+
+	if (val_len > MAX_EC_DATA_SIZE) {
+		dev_err(udata->dev, "Can't read %zu bytes. Too big.", val_len);
+		return -EINVAL;
+	}
+
+	ret = cros_ec_cmd(udata->ec, 0, EC_CMD_UCSI_PPM_GET,
+			  &req, sizeof(req), val, val_len);
+	if (ret < 0) {
+		dev_warn(udata->dev, "Failed to send EC message UCSI_PPM_GET: error=%d", ret);
+		return ret;
+	}
+	return 0;
+}
+
+static int cros_ucsi_read_version(struct ucsi *ucsi, u16 *version)
+{
+	return cros_ucsi_read(ucsi, UCSI_VERSION, version, sizeof(*version));
+}
+
+static int cros_ucsi_read_cci(struct ucsi *ucsi, u32 *cci)
+{
+	return cros_ucsi_read(ucsi, UCSI_CCI, cci, sizeof(*cci));
+}
+
+static int cros_ucsi_read_message_in(struct ucsi *ucsi, void *val,
+				     size_t val_len)
+{
+	return cros_ucsi_read(ucsi, UCSI_MESSAGE_IN, val, val_len);
+}
+
+static int cros_ucsi_async_control(struct ucsi *ucsi, u64 cmd)
+{
+	struct cros_ucsi_data *udata = ucsi_get_drvdata(ucsi);
+	struct ec_params_ucsi_ppm_set *req;
+	size_t req_len;
+	int ret;
+
+	req_len = sizeof(struct ec_params_ucsi_ppm_set) + sizeof(cmd);
+	req = kzalloc(req_len, GFP_KERNEL);
+	if (!req)
+		return -ENOMEM;
+
+	req->offset = UCSI_CONTROL;
+	memcpy(req->data, &cmd, sizeof(cmd));
+	ret = cros_ec_cmd(udata->ec, 0, EC_CMD_UCSI_PPM_SET,
+			  req, req_len, NULL, 0);
+	if (ret < 0) {
+		dev_warn(udata->dev, "Failed to send EC message UCSI_PPM_SET: error=%d", ret);
+		return ret;
+	}
+	return 0;
+}
+
+static int cros_ucsi_sync_control(struct ucsi *ucsi, u64 cmd)
+{
+	struct cros_ucsi_data *udata = ucsi_get_drvdata(ucsi);
+	bool ack = UCSI_COMMAND(cmd) == UCSI_ACK_CC_CI;
+	int ret;
+
+	if (ack)
+		set_bit(ACK_PENDING, &udata->flags);
+	else
+		set_bit(COMMAND_PENDING, &udata->flags);
+
+	ret = cros_ucsi_async_control(ucsi, cmd);
+	if (ret)
+		goto out;
+
+	if (!wait_for_completion_timeout(&udata->complete, WRITE_TMO_MS))
+		ret = -ETIMEDOUT;
+
+out:
+	if (ack)
+		clear_bit(ACK_PENDING, &udata->flags);
+	else
+		clear_bit(COMMAND_PENDING, &udata->flags);
+	return ret;
+}
+
+struct ucsi_operations cros_ucsi_ops = {
+	.read_version = cros_ucsi_read_version,
+	.read_cci = cros_ucsi_read_cci,
+	.read_message_in = cros_ucsi_read_message_in,
+	.async_control = cros_ucsi_async_control,
+	.sync_control = cros_ucsi_sync_control,
+};
+
+static void cros_ucsi_work(struct work_struct *work)
+{
+	struct cros_ucsi_data *udata = container_of(work, struct cros_ucsi_data, work);
+	u32 cci;
+
+	if (cros_ucsi_read(udata->ucsi, UCSI_CCI, &cci, sizeof(cci)))
+		return;
+
+	if (UCSI_CCI_CONNECTOR(cci))
+		ucsi_connector_change(udata->ucsi, UCSI_CCI_CONNECTOR(cci));
+
+	if (cci & UCSI_CCI_ACK_COMPLETE &&
+	    test_and_clear_bit(ACK_PENDING, &udata->flags))
+		complete(&udata->complete);
+	if (cci & UCSI_CCI_COMMAND_COMPLETE &&
+	    test_and_clear_bit(COMMAND_PENDING, &udata->flags))
+		complete(&udata->complete);
+}
+
+static int cros_ucsi_event(struct notifier_block *nb,
+			   unsigned long host_event, void *_notify)
+{
+	struct cros_ucsi_data *udata = container_of(nb, struct cros_ucsi_data, nb);
+
+	if (!(host_event & PD_EVENT_PPM))
+		return NOTIFY_OK;
+
+	dev_dbg(udata->dev, "UCSI notification received");
+	flush_work(&udata->work);
+	schedule_work(&udata->work);
+
+	return NOTIFY_OK;
+}
+
+static void cros_ucsi_destroy(struct cros_ucsi_data *udata)
+{
+	cros_usbpd_unregister_notify(&udata->nb);
+	cancel_work_sync(&udata->work);
+	ucsi_destroy(udata->ucsi);
+}
+
+static int cros_ucsi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct cros_ec_dev *ec_data = dev_get_drvdata(dev->parent);
+	struct cros_ucsi_data *udata;
+	int ret;
+
+	udata = devm_kzalloc(dev, sizeof(*udata), GFP_KERNEL);
+	if (!udata)
+		return -ENOMEM;
+
+	udata->dev = dev;
+
+	udata->ec = ec_data->ec_dev;
+	if (!udata->ec) {
+		dev_err(dev, "couldn't find parent EC device");
+		return -ENODEV;
+	}
+
+	platform_set_drvdata(pdev, udata);
+
+	INIT_WORK(&udata->work, cros_ucsi_work);
+	init_completion(&udata->complete);
+
+	udata->ucsi = ucsi_create(dev, &cros_ucsi_ops);
+	if (IS_ERR(udata->ucsi)) {
+		dev_err(dev, "failed to allocate UCSI instance");
+		return PTR_ERR(udata->ucsi);
+	}
+
+	ucsi_set_drvdata(udata->ucsi, udata);
+
+	udata->nb.notifier_call = cros_ucsi_event;
+	ret = cros_usbpd_register_notify(&udata->nb);
+	if (ret) {
+		dev_err(dev, "failed to register notifier: error=%d", ret);
+		ucsi_destroy(udata->ucsi);
+		return ret;
+	}
+
+	ret = ucsi_register(udata->ucsi);
+	if (ret) {
+		dev_err(dev, "failed to register UCSI: error=%d", ret);
+		cros_ucsi_destroy(udata);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void cros_ucsi_remove(struct platform_device *dev)
+{
+	struct cros_ucsi_data *udata = platform_get_drvdata(dev);
+
+	ucsi_unregister(udata->ucsi);
+	cros_ucsi_destroy(udata);
+}
+
+static int __maybe_unused cros_ucsi_suspend(struct device *dev)
+{
+	struct cros_ucsi_data *udata = dev_get_drvdata(dev);
+
+	cancel_work_sync(&udata->work);
+
+	return 0;
+}
+
+static int __maybe_unused cros_ucsi_resume(struct device *dev)
+{
+	struct cros_ucsi_data *udata = dev_get_drvdata(dev);
+
+	return ucsi_resume(udata->ucsi);
+}
+
+static SIMPLE_DEV_PM_OPS(cros_ucsi_pm_ops, cros_ucsi_suspend,
+			 cros_ucsi_resume);
+
+static const struct platform_device_id cros_ucsi_id[] = {
+	{ KBUILD_MODNAME, 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(platform, cros_ucsi_id);
+
+static struct platform_driver cros_ucsi_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.pm = &cros_ucsi_pm_ops,
+	},
+	.id_table = cros_ucsi_id,
+	.probe = cros_ucsi_probe,
+	.remove = cros_ucsi_remove,
+};
+
+module_platform_driver(cros_ucsi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("UCSI driver for ChromeOS EC");
-- 
2.46.0.469.g59c65b2a67-goog

[PATCH v5 4/8] usb: typec: cros_ec_ucsi: Use complete instead of resume

[Łukasz Bartosik]

From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

On platforms using cros_ec_lpc, resume is split into .resume_early and
.complete. To avoid missing EC events, use .complete to schedule work
when resuming.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---
 drivers/usb/typec/ucsi/cros_ec_ucsi.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/typec/ucsi/cros_ec_ucsi.c b/drivers/usb/typec/ucsi/cros_ec_ucsi.c
index 6b9dc05a4960..4a3369c649bf 100644
--- a/drivers/usb/typec/ucsi/cros_ec_ucsi.c
+++ b/drivers/usb/typec/ucsi/cros_ec_ucsi.c
@@ -246,15 +246,18 @@ static int __maybe_unused cros_ucsi_suspend(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused cros_ucsi_resume(struct device *dev)
+static void __maybe_unused cros_ucsi_complete(struct device *dev)
 {
 	struct cros_ucsi_data *udata = dev_get_drvdata(dev);
-
-	return ucsi_resume(udata->ucsi);
+	ucsi_resume(udata->ucsi);
 }
 
-static SIMPLE_DEV_PM_OPS(cros_ucsi_pm_ops, cros_ucsi_suspend,
-			 cros_ucsi_resume);
+static const struct dev_pm_ops cros_ucsi_pm_ops = {
+#ifdef CONFIG_PM_SLEEP
+	.suspend = cros_ucsi_suspend,
+	.complete = cros_ucsi_complete,
+#endif
+};
 
 static const struct platform_device_id cros_ucsi_id[] = {
 	{ KBUILD_MODNAME, 0 },
-- 
2.46.0.469.g59c65b2a67-goog

[PATCH v5 5/8] usb: typec: cros_ec_ucsi: Add trace events

[Łukasz Bartosik]

Add trace events to ChromeOS UCSI driver to enable debugging.

Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
---
 MAINTAINERS                                 |  1 +
 drivers/usb/typec/ucsi/cros_ec_ucsi.c       |  8 ++
 drivers/usb/typec/ucsi/cros_ec_ucsi_trace.h | 92 +++++++++++++++++++++
 3 files changed, 101 insertions(+)
 create mode 100644 drivers/usb/typec/ucsi/cros_ec_ucsi_trace.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 8c030ea0b503..d084f32208f0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5306,6 +5306,7 @@ M:	Łukasz Bartosik <ukaszb@chromium.org>
 L:	chrome-platform@lists.linux.dev
 S:	Maintained
 F:	drivers/usb/typec/ucsi/cros_ec_ucsi.c
+F:	drivers/usb/typec/ucsi/cros_ec_ucsi_trace.h
 
 CHRONTEL CH7322 CEC DRIVER
 M:	Joe Tessler <jrt@google.com>
diff --git a/drivers/usb/typec/ucsi/cros_ec_ucsi.c b/drivers/usb/typec/ucsi/cros_ec_ucsi.c
index 4a3369c649bf..6e020b7ed352 100644
--- a/drivers/usb/typec/ucsi/cros_ec_ucsi.c
+++ b/drivers/usb/typec/ucsi/cros_ec_ucsi.c
@@ -16,7 +16,9 @@
 #include <linux/slab.h>
 #include <linux/wait.h>
 
+#define CREATE_TRACE_POINTS
 #include "ucsi.h"
+#include "cros_ec_ucsi_trace.h"
 
 /*
  * Maximum size in bytes of a UCSI message between AP and EC
@@ -62,6 +64,8 @@ static int cros_ucsi_read(struct ucsi *ucsi, unsigned int offset, void *val,
 		dev_warn(udata->dev, "Failed to send EC message UCSI_PPM_GET: error=%d", ret);
 		return ret;
 	}
+
+	trace_cros_ec_opm_to_ppm_rd(offset, val, val_len);
 	return 0;
 }
 
@@ -101,6 +105,8 @@ static int cros_ucsi_async_control(struct ucsi *ucsi, u64 cmd)
 		dev_warn(udata->dev, "Failed to send EC message UCSI_PPM_SET: error=%d", ret);
 		return ret;
 	}
+
+	trace_cros_ec_opm_to_ppm_wr(req->offset, &cmd, sizeof(cmd));
 	return 0;
 }
 
@@ -143,6 +149,8 @@ static void cros_ucsi_work(struct work_struct *work)
 	struct cros_ucsi_data *udata = container_of(work, struct cros_ucsi_data, work);
 	u32 cci;
 
+	trace_cros_ec_ppm_to_opm(0);
+
 	if (cros_ucsi_read(udata->ucsi, UCSI_CCI, &cci, sizeof(cci)))
 		return;
 
diff --git a/drivers/usb/typec/ucsi/cros_ec_ucsi_trace.h b/drivers/usb/typec/ucsi/cros_ec_ucsi_trace.h
new file mode 100644
index 000000000000..b765ef5c8236
--- /dev/null
+++ b/drivers/usb/typec/ucsi/cros_ec_ucsi_trace.h
@@ -0,0 +1,92 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM cros_ec_ucsi
+
+#if !defined(__CROS_EC_UCSI_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define __CROS_EC_UCSI_TRACE_H
+
+#include <linux/tracepoint.h>
+
+#define decode_cmd(cmd)									\
+	__print_symbolic(cmd,								\
+		{ 0,				"Unknown command"		},	\
+		{ UCSI_PPM_RESET,		"PPM_RESET"			},	\
+		{ UCSI_CONNECTOR_RESET,		"CONNECTOR_RESET,"		},	\
+		{ UCSI_ACK_CC_CI,		"ACK_CC_CI"			},	\
+		{ UCSI_SET_NOTIFICATION_ENABLE,	"SET_NOTIFICATION_ENABLE"	},	\
+		{ UCSI_GET_CAPABILITY,		"GET_CAPABILITY"		},	\
+		{ UCSI_GET_CONNECTOR_CAPABILITY, "GET_CONNECTOR_CAPABILITY"	},	\
+		{ UCSI_SET_UOM,			"SET_UOM"			},	\
+		{ UCSI_SET_UOR,			"SET_UOR"			},	\
+		{ UCSI_SET_PDM,			"SET_PDM"			},	\
+		{ UCSI_SET_PDR,			"SET_PDR"			},	\
+		{ UCSI_GET_ALTERNATE_MODES,	"GET_ALTERNATE_MODES"		},	\
+		{ UCSI_GET_CAM_SUPPORTED,	"GET_CAM_SUPPORTED"		},	\
+		{ UCSI_GET_CURRENT_CAM,		"GET_CURRENT_CAM"		},	\
+		{ UCSI_SET_NEW_CAM,		"SET_NEW_CAM"			},	\
+		{ UCSI_GET_PDOS,		"GET_PDOS"			},	\
+		{ UCSI_GET_CABLE_PROPERTY,	"GET_CABLE_PROPERTY"		},	\
+		{ UCSI_GET_CONNECTOR_STATUS,	"GET_CONNECTOR_STATUS"		},	\
+		{ UCSI_GET_ERROR_STATUS,	"GET_ERROR_STATUS"		})
+
+#define decode_offset(offset)					\
+	__print_symbolic(offset,				\
+		{ UCSI_VERSION,		"VER"		},	\
+		{ UCSI_CCI,		"CCI"		},	\
+		{ UCSI_CONTROL,		"CTRL"		},	\
+		{ UCSI_MESSAGE_IN,	"MSG_IN"	},	\
+		{ UCSI_MESSAGE_OUT,	"MSG_OUT"	},	\
+		{ UCSIv2_MESSAGE_OUT,	"MSG_OUTv2"	})
+
+DECLARE_EVENT_CLASS(cros_ec_opm_to_ppm,
+	TP_PROTO(u16 offset, const void *value, size_t length),
+	TP_ARGS(offset, value, length),
+	TP_STRUCT__entry(
+		__field(u8, cmd)
+		__field(u16, offset)
+		__field(size_t, length)
+		__dynamic_array(char, msg, length)
+	),
+	TP_fast_assign(
+		__entry->cmd = *((u64 *) value + 3);
+		__entry->offset = offset;
+		__entry->length = length;
+		memcpy(__get_dynamic_array(msg), value, length);
+	),
+	TP_printk("(%s) %s: %s",
+		decode_offset(__entry->offset),
+		__entry->offset == UCSI_CONTROL ?
+		decode_cmd(__entry->cmd) : "",
+		__print_hex(__get_dynamic_array(msg), __entry->length))
+);
+
+DEFINE_EVENT(cros_ec_opm_to_ppm, cros_ec_opm_to_ppm_rd,
+	TP_PROTO(u16 offset, const void *value, size_t length),
+	TP_ARGS(offset, value, length)
+);
+
+DEFINE_EVENT(cros_ec_opm_to_ppm, cros_ec_opm_to_ppm_wr,
+	TP_PROTO(u16 offset, const void *value, size_t length),
+	TP_ARGS(offset, value, length)
+);
+
+TRACE_EVENT(cros_ec_ppm_to_opm,
+	TP_PROTO(int x),
+	TP_ARGS(x),
+	TP_STRUCT__entry(__array(char, x, 0)),
+	TP_fast_assign((void)x),
+	TP_printk("notification%s", "")
+);
+
+#endif /* __CROS_EC_UCSI_TRACE_H */
+
+/* This part must be outside protection */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE cros_ec_ucsi_trace
+
+#include <trace/define_trace.h>
-- 
2.46.0.469.g59c65b2a67-goog

[PATCH v5 6/8] usb: typec: cros_ec_ucsi: Add netlink

[Łukasz Bartosik]

Add netlink to ChromeOS UCSI driver to allow forwarding
of UCSI messages to userspace for debugging and testing
purposes.

Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
---
 MAINTAINERS                                   |  4 +-
 drivers/usb/typec/ucsi/Makefile               |  4 +-
 .../{cros_ec_ucsi.c => cros_ec_ucsi_main.c}   | 66 +++++++++++++-
 drivers/usb/typec/ucsi/cros_ec_ucsi_nl.c      | 87 +++++++++++++++++++
 drivers/usb/typec/ucsi/cros_ec_ucsi_nl.h      | 52 +++++++++++
 5 files changed, 209 insertions(+), 4 deletions(-)
 rename drivers/usb/typec/ucsi/{cros_ec_ucsi.c => cros_ec_ucsi_main.c} (79%)
 create mode 100644 drivers/usb/typec/ucsi/cros_ec_ucsi_nl.c
 create mode 100644 drivers/usb/typec/ucsi/cros_ec_ucsi_nl.h

diff --git a/MAINTAINERS b/MAINTAINERS
index d084f32208f0..2afb406a24ce 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5305,7 +5305,9 @@ M:	Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
 M:	Łukasz Bartosik <ukaszb@chromium.org>
 L:	chrome-platform@lists.linux.dev
 S:	Maintained
-F:	drivers/usb/typec/ucsi/cros_ec_ucsi.c
+F:	drivers/usb/typec/ucsi/cros_ec_ucsi_main.c
+F:	drivers/usb/typec/ucsi/cros_ec_ucsi_nl.c
+F:	drivers/usb/typec/ucsi/cros_ec_ucsi_nl.h
 F:	drivers/usb/typec/ucsi/cros_ec_ucsi_trace.h
 
 CHRONTEL CH7322 CEC DRIVER
diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
index be98a879104d..82d960394c39 100644
--- a/drivers/usb/typec/ucsi/Makefile
+++ b/drivers/usb/typec/ucsi/Makefile
@@ -21,5 +21,7 @@ obj-$(CONFIG_UCSI_ACPI)			+= ucsi_acpi.o
 obj-$(CONFIG_UCSI_CCG)			+= ucsi_ccg.o
 obj-$(CONFIG_UCSI_STM32G0)		+= ucsi_stm32g0.o
 obj-$(CONFIG_UCSI_PMIC_GLINK)		+= ucsi_glink.o
-obj-$(CONFIG_CROS_EC_UCSI)		+= cros_ec_ucsi.o
 obj-$(CONFIG_UCSI_LENOVO_YOGA_C630)	+= ucsi_yoga_c630.o
+
+obj-$(CONFIG_CROS_EC_UCSI)		+= cros_ec_ucsi.o
+cros_ec_ucsi-y				:= cros_ec_ucsi_main.o cros_ec_ucsi_nl.o
diff --git a/drivers/usb/typec/ucsi/cros_ec_ucsi.c b/drivers/usb/typec/ucsi/cros_ec_ucsi_main.c
similarity index 79%
rename from drivers/usb/typec/ucsi/cros_ec_ucsi.c
rename to drivers/usb/typec/ucsi/cros_ec_ucsi_main.c
index 6e020b7ed352..85edfa95782a 100644
--- a/drivers/usb/typec/ucsi/cros_ec_ucsi.c
+++ b/drivers/usb/typec/ucsi/cros_ec_ucsi_main.c
@@ -19,6 +19,7 @@
 #define CREATE_TRACE_POINTS
 #include "ucsi.h"
 #include "cros_ec_ucsi_trace.h"
+#include "cros_ec_ucsi_nl.h"
 
 /*
  * Maximum size in bytes of a UCSI message between AP and EC
@@ -43,6 +44,43 @@ struct cros_ucsi_data {
 	unsigned long flags;
 };
 
+/*
+ * When set to true the cros_ec_ucsi driver will forward all UCSI messages
+ * exchanged between OPM <-> PPM to userspace through netlink
+ */
+static bool is_ap_sniffer_en;
+
+static ssize_t enable_ap_sniffer_show(struct device *dev,
+				      struct device_attribute *attr,
+				      char *buf)
+{
+	return sprintf(buf, "%d\n", is_ap_sniffer_en);
+}
+
+static ssize_t enable_ap_sniffer_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t count)
+{
+	u8 value;
+
+	if (kstrtou8(buf, 0, &value))
+		return -EINVAL;
+
+	is_ap_sniffer_en = value ? 1 : 0;
+	return count;
+}
+
+static DEVICE_ATTR_RW(enable_ap_sniffer);
+
+static struct attribute *cros_ec_ucsi_attrs[] = {
+	&dev_attr_enable_ap_sniffer.attr,
+	NULL
+};
+
+static const struct attribute_group cros_ec_ucsi_attrs_grp = {
+	.attrs = cros_ec_ucsi_attrs,
+};
+
 static int cros_ucsi_read(struct ucsi *ucsi, unsigned int offset, void *val,
 			  size_t val_len)
 {
@@ -65,6 +103,9 @@ static int cros_ucsi_read(struct ucsi *ucsi, unsigned int offset, void *val,
 		return ret;
 	}
 
+	if (is_ap_sniffer_en)
+		nl_cros_ec_bcast_msg(NL_CROS_EC_TO_PPM, NL_CROS_EC_RD, offset,
+				     val, val_len);
 	trace_cros_ec_opm_to_ppm_rd(offset, val, val_len);
 	return 0;
 }
@@ -106,6 +147,9 @@ static int cros_ucsi_async_control(struct ucsi *ucsi, u64 cmd)
 		return ret;
 	}
 
+	if (is_ap_sniffer_en)
+		nl_cros_ec_bcast_msg(NL_CROS_EC_TO_PPM, NL_CROS_EC_WR,
+				     req->offset, (u8 *) &cmd, sizeof(cmd));
 	trace_cros_ec_opm_to_ppm_wr(req->offset, &cmd, sizeof(cmd));
 	return 0;
 }
@@ -149,6 +193,8 @@ static void cros_ucsi_work(struct work_struct *work)
 	struct cros_ucsi_data *udata = container_of(work, struct cros_ucsi_data, work);
 	u32 cci;
 
+	if (is_ap_sniffer_en)
+		nl_cros_ec_bcast_msg(NL_CROS_EC_TO_OPM, 0, 0, NULL, 0);
 	trace_cros_ec_ppm_to_opm(0);
 
 	if (cros_ucsi_read(udata->ucsi, UCSI_CCI, &cci, sizeof(cci)))
@@ -234,13 +280,29 @@ static int cros_ucsi_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	ret = nl_cros_ec_register();
+	if (ret) {
+		dev_err(dev, "failed to register netlink: error=%d", ret);
+		cros_ucsi_destroy(udata);
+		return ret;
+	}
+
+	ret = sysfs_create_group(&dev->kobj, &cros_ec_ucsi_attrs_grp);
+	if (ret) {
+		dev_err(dev, "failed to register sysfs group: error=%d", ret);
+		cros_ucsi_destroy(udata);
+		return ret;
+	}
+
 	return 0;
 }
 
-static void cros_ucsi_remove(struct platform_device *dev)
+static void cros_ucsi_remove(struct platform_device *pdev)
 {
-	struct cros_ucsi_data *udata = platform_get_drvdata(dev);
+	struct cros_ucsi_data *udata = platform_get_drvdata(pdev);
 
+	sysfs_remove_group(&pdev->dev.kobj, &cros_ec_ucsi_attrs_grp);
+	nl_cros_ec_unregister();
 	ucsi_unregister(udata->ucsi);
 	cros_ucsi_destroy(udata);
 }
diff --git a/drivers/usb/typec/ucsi/cros_ec_ucsi_nl.c b/drivers/usb/typec/ucsi/cros_ec_ucsi_nl.c
new file mode 100644
index 000000000000..360568044891
--- /dev/null
+++ b/drivers/usb/typec/ucsi/cros_ec_ucsi_nl.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <net/genetlink.h>
+#include "cros_ec_ucsi_nl.h"
+
+static const struct genl_multicast_group nl_mc_grps[] = {
+	{ .name = NL_CROS_EC_MC_GRP_NAME },
+};
+
+static struct genl_family genl_fam = {
+	.name	  = NL_CROS_EC_NAME,
+	.version  = NL_CROS_EC_VER,
+	.maxattr  = NL_CROS_EC_A_MAX,
+	.mcgrps	  = nl_mc_grps,
+	.n_mcgrps = ARRAY_SIZE(nl_mc_grps),
+};
+
+int nl_cros_ec_register(void)
+{
+	return genl_register_family(&genl_fam);
+}
+
+void nl_cros_ec_unregister(void)
+{
+	genl_unregister_family(&genl_fam);
+}
+
+int nl_cros_ec_bcast_msg(enum nl_cros_ec_msg_dir dir,
+			 enum nl_cros_ec_cmd_type cmd_type,
+			 u16 offset, const u8 *payload, size_t msg_size)
+{
+	struct timespec64 ts;
+	struct sk_buff *skb;
+	int ret = -ENOMEM;
+	void *hdr;
+
+	skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	hdr = genlmsg_put(skb, 0, 0, &genl_fam, 0, NL_CROS_EC_C_UCSI);
+	if (!hdr)
+		goto free_mem;
+
+	ret = nla_put_u8(skb, NL_CROS_EC_A_SRC, NL_CROS_EC_AP);
+	if (ret)
+		goto cancel;
+
+	ret = nla_put_u8(skb, NL_CROS_EC_A_DIR, dir);
+	if (ret)
+		goto cancel;
+
+	ret = nla_put_u16(skb, NL_CROS_EC_A_OFFSET, offset);
+	if (ret)
+		goto cancel;
+
+	ret = nla_put_u8(skb, NL_CROS_EC_A_CMD_TYPE, cmd_type);
+	if (ret)
+		goto cancel;
+
+	ktime_get_ts64(&ts);
+	ret = nla_put_u32(skb, NL_CROS_EC_A_TSTAMP_SEC, (u32)ts.tv_sec);
+	if (ret)
+		goto cancel;
+
+	ret = nla_put_u32(skb, NL_CROS_EC_A_TSTAMP_USEC,
+			  (u32)(ts.tv_nsec/1000));
+	if (ret)
+		goto cancel;
+
+	ret = nla_put(skb, NL_CROS_EC_A_PAYLOAD, msg_size, payload);
+	if (ret)
+		goto cancel;
+
+	genlmsg_end(skb, hdr);
+
+	ret = genlmsg_multicast(&genl_fam, skb, 0, 0, GFP_KERNEL);
+	if (ret && ret != -ESRCH)
+		goto free_mem;
+
+	return 0;
+cancel:
+	genlmsg_cancel(skb, hdr);
+free_mem:
+	nlmsg_free(skb);
+	return ret;
+}
diff --git a/drivers/usb/typec/ucsi/cros_ec_ucsi_nl.h b/drivers/usb/typec/ucsi/cros_ec_ucsi_nl.h
new file mode 100644
index 000000000000..c6192d8ace56
--- /dev/null
+++ b/drivers/usb/typec/ucsi/cros_ec_ucsi_nl.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __DRIVER_USB_TYPEC_CROS_EC_UCSI_NL_H
+#define __DRIVER_USB_TYPEC_CROS_EC_UCSI_NL_H
+
+#define NL_CROS_EC_NAME "cros_ec_ucsi"
+#define NL_CROS_EC_VER 1
+#define NL_CROS_EC_MC_GRP_NAME "cros_ec_ucsi_mc"
+
+/* attributes */
+enum nl_cros_ec_attrs {
+	NL_CROS_EC_A_SRC,
+	NL_CROS_EC_A_DIR,
+	NL_CROS_EC_A_OFFSET,
+	NL_CROS_EC_A_CMD_TYPE,
+	NL_CROS_EC_A_TSTAMP_SEC,
+	NL_CROS_EC_A_TSTAMP_USEC,
+	NL_CROS_EC_A_PAYLOAD,
+	NL_CROS_EC_A_MAX
+};
+
+enum nl_cros_ec_cmds {
+	NL_CROS_EC_C_UCSI,
+	NL_CROS_EC_C_MAX
+};
+
+/* where message was captured - EC or AP */
+enum nl_cros_ec_src {
+	NL_CROS_EC_AP,
+	NL_CROS_EC_EC
+};
+
+/* message destination */
+enum nl_cros_ec_msg_dir {
+	NL_CROS_EC_TO_PPM,
+	NL_CROS_EC_TO_OPM,
+	NL_CROS_EC_TO_LPM
+};
+
+/* command type - read or write */
+enum nl_cros_ec_cmd_type {
+	NL_CROS_EC_RD,
+	NL_CROS_EC_WR
+};
+
+int nl_cros_ec_register(void);
+void nl_cros_ec_unregister(void);
+int nl_cros_ec_bcast_msg(enum nl_cros_ec_msg_dir dir,
+			 enum nl_cros_ec_cmd_type cmd_type,
+			 u16 offset, const u8 *payload, size_t msg_size);
+
+#endif /* __DRIVER_USB_TYPEC_CROS_EC_UCSI_NL_H */
-- 
2.46.0.469.g59c65b2a67-goog

[PATCH v5 7/8] mfd: cros_ec: Load cros_ec_ucsi on supported ECs

[Łukasz Bartosik]

From: Pavan Holla <pholla@chromium.org>

Load cros_ec_ucsi driver if the ChromeOS EC implements
UCSI Platform Policy Manager (PPM).

Signed-off-by: Pavan Holla <pholla@chromium.org>
---
 drivers/mfd/cros_ec_dev.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index e2aae8918679..d5d63df7fcbd 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -108,6 +108,10 @@ static const struct mfd_cell cros_ec_keyboard_leds_cells[] = {
 	{ .name = "cros-keyboard-leds", },
 };
 
+static const struct mfd_cell cros_ec_ucsi_cells[] = {
+	{ .name = "cros_ec_ucsi", },
+};
+
 static const struct cros_feature_to_cells cros_subdevices[] = {
 	{
 		.id		= EC_FEATURE_CEC,
@@ -124,6 +128,11 @@ static const struct cros_feature_to_cells cros_subdevices[] = {
 		.mfd_cells	= cros_ec_rtc_cells,
 		.num_cells	= ARRAY_SIZE(cros_ec_rtc_cells),
 	},
+	{
+		.id		= EC_FEATURE_UCSI_PPM,
+		.mfd_cells	= cros_ec_ucsi_cells,
+		.num_cells	= ARRAY_SIZE(cros_ec_ucsi_cells),
+	},
 	{
 		.id		= EC_FEATURE_USB_PD,
 		.mfd_cells	= cros_usbpd_charger_cells,
-- 
2.46.0.469.g59c65b2a67-goog

[PATCH v5 8/8] mfd: cros_ec: Don't load charger with UCSI

[Łukasz Bartosik]

From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

When UCSI is enabled, don't load cros_usbpd_charger and cros_usbpd_logger
drivers. Charger functionality is provided by the UCSI driver already and
logging will need to be added.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---
 drivers/mfd/cros_ec_dev.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index d5d63df7fcbd..bc083c7b21de 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -133,11 +133,6 @@ static const struct cros_feature_to_cells cros_subdevices[] = {
 		.mfd_cells	= cros_ec_ucsi_cells,
 		.num_cells	= ARRAY_SIZE(cros_ec_ucsi_cells),
 	},
-	{
-		.id		= EC_FEATURE_USB_PD,
-		.mfd_cells	= cros_usbpd_charger_cells,
-		.num_cells	= ARRAY_SIZE(cros_usbpd_charger_cells),
-	},
 	{
 		.id		= EC_FEATURE_HANG_DETECT,
 		.mfd_cells	= cros_ec_wdt_cells,
@@ -261,6 +256,21 @@ static int ec_device_probe(struct platform_device *pdev)
 		}
 	}
 
+	/*
+	 * UCSI provides power supply information so we don't need to separately
+	 * load the cros_usbpd_charger driver.
+	 */
+	if (cros_ec_check_features(ec, EC_FEATURE_USB_PD) &&
+	    !cros_ec_check_features(ec, EC_FEATURE_UCSI_PPM)) {
+		retval = mfd_add_hotplug_devices(ec->dev,
+						 cros_usbpd_charger_cells,
+						 ARRAY_SIZE(cros_usbpd_charger_cells));
+
+		if (retval)
+			dev_warn(ec->dev, "failed to add usbpd-charger: %d\n",
+				 retval);
+	}
+
 	/*
 	 * Lightbar is a special case. Newer devices support autodetection,
 	 * but older ones do not.
-- 
2.46.0.469.g59c65b2a67-goog

Re: [PATCH v5 3/8] usb: typec: ucsi: Implement ChromeOS UCSI driver

[Heikki Krogerus]

On Tue, Sep 03, 2024 at 04:30:28PM +0000, Łukasz Bartosik wrote:
> From: Pavan Holla <pholla@chromium.org>
> 
> Implementation of a UCSI transport driver for ChromeOS.
> This driver will be loaded if the ChromeOS EC implements a PPM.
> 
> Signed-off-by: Pavan Holla <pholla@chromium.org>
> Co-developed-by: Łukasz Bartosik <ukaszb@chromium.org>
> Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  MAINTAINERS                           |   7 +
>  drivers/usb/typec/ucsi/Kconfig        |  13 ++
>  drivers/usb/typec/ucsi/Makefile       |   1 +
>  drivers/usb/typec/ucsi/cros_ec_ucsi.c | 278 ++++++++++++++++++++++++++
>  4 files changed, 299 insertions(+)
>  create mode 100644 drivers/usb/typec/ucsi/cros_ec_ucsi.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fe83ba7194ea..8c030ea0b503 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5300,6 +5300,13 @@ L:	chrome-platform@lists.linux.dev
>  S:	Maintained
>  F:	drivers/watchdog/cros_ec_wdt.c
>  
> +CHROMEOS UCSI DRIVER
> +M:	Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> +M:	Łukasz Bartosik <ukaszb@chromium.org>
> +L:	chrome-platform@lists.linux.dev
> +S:	Maintained
> +F:	drivers/usb/typec/ucsi/cros_ec_ucsi.c
> +
>  CHRONTEL CH7322 CEC DRIVER
>  M:	Joe Tessler <jrt@google.com>
>  L:	linux-media@vger.kernel.org
> diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
> index 680e1b87b152..75559601fe8f 100644
> --- a/drivers/usb/typec/ucsi/Kconfig
> +++ b/drivers/usb/typec/ucsi/Kconfig
> @@ -69,6 +69,19 @@ config UCSI_PMIC_GLINK
>  	  To compile the driver as a module, choose M here: the module will be
>  	  called ucsi_glink.
>  
> +config CROS_EC_UCSI
> +	tristate "UCSI Driver for ChromeOS EC"
> +	depends on MFD_CROS_EC_DEV
> +	depends on CROS_USBPD_NOTIFY
> +	depends on !EXTCON_TCSS_CROS_EC
> +	default MFD_CROS_EC_DEV
> +	help
> +	  This driver enables UCSI support for a ChromeOS EC. The EC is
> +	  expected to implement a PPM.
> +
> +	  To compile the driver as a module, choose M here: the module
> +	  will be called cros_ec_ucsi.
> +
>  config UCSI_LENOVO_YOGA_C630
>  	tristate "UCSI Interface Driver for Lenovo Yoga C630"
>  	depends on EC_LENOVO_YOGA_C630
> diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
> index aed41d23887b..be98a879104d 100644
> --- a/drivers/usb/typec/ucsi/Makefile
> +++ b/drivers/usb/typec/ucsi/Makefile
> @@ -21,4 +21,5 @@ obj-$(CONFIG_UCSI_ACPI)			+= ucsi_acpi.o
>  obj-$(CONFIG_UCSI_CCG)			+= ucsi_ccg.o
>  obj-$(CONFIG_UCSI_STM32G0)		+= ucsi_stm32g0.o
>  obj-$(CONFIG_UCSI_PMIC_GLINK)		+= ucsi_glink.o
> +obj-$(CONFIG_CROS_EC_UCSI)		+= cros_ec_ucsi.o
>  obj-$(CONFIG_UCSI_LENOVO_YOGA_C630)	+= ucsi_yoga_c630.o
> diff --git a/drivers/usb/typec/ucsi/cros_ec_ucsi.c b/drivers/usb/typec/ucsi/cros_ec_ucsi.c
> new file mode 100644
> index 000000000000..6b9dc05a4960
> --- /dev/null
> +++ b/drivers/usb/typec/ucsi/cros_ec_ucsi.c
> @@ -0,0 +1,278 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * UCSI driver for ChromeOS EC
> + *
> + * Copyright 2024 Google LLC.
> + */
> +
> +#include <linux/container_of.h>
> +#include <linux/dev_printk.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_usbpd_notify.h>
> +#include <linux/platform_data/cros_ec_proto.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/wait.h>
> +
> +#include "ucsi.h"
> +
> +/*
> + * Maximum size in bytes of a UCSI message between AP and EC
> + */
> +#define MAX_EC_DATA_SIZE	256
> +
> +/*
> + * Maximum time in miliseconds the cros_ec_ucsi driver
> + * will wait for a response to a command or and ack.
> + */
> +#define WRITE_TMO_MS		5000
> +
> +struct cros_ucsi_data {
> +	struct device *dev;
> +	struct ucsi *ucsi;
> +
> +	struct cros_ec_device *ec;
> +	struct notifier_block nb;
> +	struct work_struct work;
> +
> +	struct completion complete;
> +	unsigned long flags;
> +};
> +
> +static int cros_ucsi_read(struct ucsi *ucsi, unsigned int offset, void *val,
> +			  size_t val_len)
> +{
> +	struct cros_ucsi_data *udata = ucsi_get_drvdata(ucsi);
> +	struct ec_params_ucsi_ppm_get req = {
> +		.offset = offset,
> +		.size = val_len,
> +	};
> +	int ret;
> +
> +	if (val_len > MAX_EC_DATA_SIZE) {
> +		dev_err(udata->dev, "Can't read %zu bytes. Too big.", val_len);
> +		return -EINVAL;
> +	}
> +
> +	ret = cros_ec_cmd(udata->ec, 0, EC_CMD_UCSI_PPM_GET,
> +			  &req, sizeof(req), val, val_len);
> +	if (ret < 0) {
> +		dev_warn(udata->dev, "Failed to send EC message UCSI_PPM_GET: error=%d", ret);
> +		return ret;
> +	}
> +	return 0;
> +}
> +
> +static int cros_ucsi_read_version(struct ucsi *ucsi, u16 *version)
> +{
> +	return cros_ucsi_read(ucsi, UCSI_VERSION, version, sizeof(*version));
> +}
> +
> +static int cros_ucsi_read_cci(struct ucsi *ucsi, u32 *cci)
> +{
> +	return cros_ucsi_read(ucsi, UCSI_CCI, cci, sizeof(*cci));
> +}
> +
> +static int cros_ucsi_read_message_in(struct ucsi *ucsi, void *val,
> +				     size_t val_len)
> +{
> +	return cros_ucsi_read(ucsi, UCSI_MESSAGE_IN, val, val_len);
> +}
> +
> +static int cros_ucsi_async_control(struct ucsi *ucsi, u64 cmd)
> +{
> +	struct cros_ucsi_data *udata = ucsi_get_drvdata(ucsi);
> +	struct ec_params_ucsi_ppm_set *req;
> +	size_t req_len;
> +	int ret;
> +
> +	req_len = sizeof(struct ec_params_ucsi_ppm_set) + sizeof(cmd);
> +	req = kzalloc(req_len, GFP_KERNEL);
> +	if (!req)
> +		return -ENOMEM;
> +
> +	req->offset = UCSI_CONTROL;
> +	memcpy(req->data, &cmd, sizeof(cmd));
> +	ret = cros_ec_cmd(udata->ec, 0, EC_CMD_UCSI_PPM_SET,
> +			  req, req_len, NULL, 0);
> +	if (ret < 0) {
> +		dev_warn(udata->dev, "Failed to send EC message UCSI_PPM_SET: error=%d", ret);
> +		return ret;
> +	}
> +	return 0;
> +}
> +
> +static int cros_ucsi_sync_control(struct ucsi *ucsi, u64 cmd)
> +{
> +	struct cros_ucsi_data *udata = ucsi_get_drvdata(ucsi);
> +	bool ack = UCSI_COMMAND(cmd) == UCSI_ACK_CC_CI;
> +	int ret;
> +
> +	if (ack)
> +		set_bit(ACK_PENDING, &udata->flags);
> +	else
> +		set_bit(COMMAND_PENDING, &udata->flags);
> +
> +	ret = cros_ucsi_async_control(ucsi, cmd);
> +	if (ret)
> +		goto out;
> +
> +	if (!wait_for_completion_timeout(&udata->complete, WRITE_TMO_MS))
> +		ret = -ETIMEDOUT;
> +
> +out:
> +	if (ack)
> +		clear_bit(ACK_PENDING, &udata->flags);
> +	else
> +		clear_bit(COMMAND_PENDING, &udata->flags);
> +	return ret;
> +}
> +
> +struct ucsi_operations cros_ucsi_ops = {
> +	.read_version = cros_ucsi_read_version,
> +	.read_cci = cros_ucsi_read_cci,
> +	.read_message_in = cros_ucsi_read_message_in,
> +	.async_control = cros_ucsi_async_control,
> +	.sync_control = cros_ucsi_sync_control,
> +};
> +
> +static void cros_ucsi_work(struct work_struct *work)
> +{
> +	struct cros_ucsi_data *udata = container_of(work, struct cros_ucsi_data, work);
> +	u32 cci;
> +
> +	if (cros_ucsi_read(udata->ucsi, UCSI_CCI, &cci, sizeof(cci)))
> +		return;
> +
> +	if (UCSI_CCI_CONNECTOR(cci))
> +		ucsi_connector_change(udata->ucsi, UCSI_CCI_CONNECTOR(cci));
> +
> +	if (cci & UCSI_CCI_ACK_COMPLETE &&
> +	    test_and_clear_bit(ACK_PENDING, &udata->flags))
> +		complete(&udata->complete);
> +	if (cci & UCSI_CCI_COMMAND_COMPLETE &&
> +	    test_and_clear_bit(COMMAND_PENDING, &udata->flags))
> +		complete(&udata->complete);
> +}
> +
> +static int cros_ucsi_event(struct notifier_block *nb,
> +			   unsigned long host_event, void *_notify)
> +{
> +	struct cros_ucsi_data *udata = container_of(nb, struct cros_ucsi_data, nb);
> +
> +	if (!(host_event & PD_EVENT_PPM))
> +		return NOTIFY_OK;
> +
> +	dev_dbg(udata->dev, "UCSI notification received");
> +	flush_work(&udata->work);
> +	schedule_work(&udata->work);
> +
> +	return NOTIFY_OK;
> +}
> +
> +static void cros_ucsi_destroy(struct cros_ucsi_data *udata)
> +{
> +	cros_usbpd_unregister_notify(&udata->nb);
> +	cancel_work_sync(&udata->work);
> +	ucsi_destroy(udata->ucsi);
> +}
> +
> +static int cros_ucsi_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct cros_ec_dev *ec_data = dev_get_drvdata(dev->parent);
> +	struct cros_ucsi_data *udata;
> +	int ret;
> +
> +	udata = devm_kzalloc(dev, sizeof(*udata), GFP_KERNEL);
> +	if (!udata)
> +		return -ENOMEM;
> +
> +	udata->dev = dev;
> +
> +	udata->ec = ec_data->ec_dev;
> +	if (!udata->ec) {
> +		dev_err(dev, "couldn't find parent EC device");
> +		return -ENODEV;
> +	}
> +
> +	platform_set_drvdata(pdev, udata);
> +
> +	INIT_WORK(&udata->work, cros_ucsi_work);
> +	init_completion(&udata->complete);
> +
> +	udata->ucsi = ucsi_create(dev, &cros_ucsi_ops);
> +	if (IS_ERR(udata->ucsi)) {
> +		dev_err(dev, "failed to allocate UCSI instance");
> +		return PTR_ERR(udata->ucsi);
> +	}
> +
> +	ucsi_set_drvdata(udata->ucsi, udata);
> +
> +	udata->nb.notifier_call = cros_ucsi_event;
> +	ret = cros_usbpd_register_notify(&udata->nb);
> +	if (ret) {
> +		dev_err(dev, "failed to register notifier: error=%d", ret);
> +		ucsi_destroy(udata->ucsi);
> +		return ret;
> +	}
> +
> +	ret = ucsi_register(udata->ucsi);
> +	if (ret) {
> +		dev_err(dev, "failed to register UCSI: error=%d", ret);
> +		cros_ucsi_destroy(udata);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void cros_ucsi_remove(struct platform_device *dev)
> +{
> +	struct cros_ucsi_data *udata = platform_get_drvdata(dev);
> +
> +	ucsi_unregister(udata->ucsi);
> +	cros_ucsi_destroy(udata);
> +}
> +
> +static int __maybe_unused cros_ucsi_suspend(struct device *dev)
> +{
> +	struct cros_ucsi_data *udata = dev_get_drvdata(dev);
> +
> +	cancel_work_sync(&udata->work);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused cros_ucsi_resume(struct device *dev)
> +{
> +	struct cros_ucsi_data *udata = dev_get_drvdata(dev);
> +
> +	return ucsi_resume(udata->ucsi);
> +}
> +
> +static SIMPLE_DEV_PM_OPS(cros_ucsi_pm_ops, cros_ucsi_suspend,
> +			 cros_ucsi_resume);
> +
> +static const struct platform_device_id cros_ucsi_id[] = {
> +	{ KBUILD_MODNAME, 0 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(platform, cros_ucsi_id);
> +
> +static struct platform_driver cros_ucsi_driver = {
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +		.pm = &cros_ucsi_pm_ops,
> +	},
> +	.id_table = cros_ucsi_id,
> +	.probe = cros_ucsi_probe,
> +	.remove = cros_ucsi_remove,
> +};
> +
> +module_platform_driver(cros_ucsi_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("UCSI driver for ChromeOS EC");
> -- 
> 2.46.0.469.g59c65b2a67-goog

-- 
heikki

Re: [PATCH v5 1/8] platform/chrome: Update ChromeOS EC header for UCSI

[Tzung-Bi Shih]

On Tue, Sep 03, 2024 at 04:30:26PM +0000, Łukasz Bartosik wrote:
> From: Pavan Holla <pholla@chromium.org>
> 
> Add EC host commands for reading and writing UCSI structures
> in the EC. The corresponding kernel driver is cros-ec-ucsi.
> 
> Also update PD events supported by the EC.
> 
> Signed-off-by: Pavan Holla <pholla@chromium.org>

It needs your S-o-b tag at the end.  See [1].

[1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

> --- a/include/linux/platform_data/cros_ec_commands.h
> +++ b/include/linux/platform_data/cros_ec_commands.h
> @@ -5012,8 +5012,10 @@ struct ec_response_pd_status {
[...]
>  struct ec_response_host_event_status {
> -	uint32_t status;      /* PD MCU host event status */
> +	u32 status;      /* PD MCU host event status */

Even though ./scripts/checkpatch.pl dislikes it, but please don't do that.
The header is a re-import from EC's repo.  We should try not to be divergent
from the origin too much.

> +/*
> + * Read/write interface for UCSI OPM <-> PPM communication.
> + */

Same reason: it'd be better if it can align to [2].

[2]: https://crrev.com/1454f2e8dac20ca37428744345c1bb4fdec30255/include/ec_commands.h#8055

> +#define EC_CMD_UCSI_PPM_SET 0x0140
> +
> +/* The data size is stored in the host command protocol header. */
> +struct ec_params_ucsi_ppm_set {
> +	u16 offset;
> +	u8 data[];

Same for the u16 and u8.

> +struct ec_params_ucsi_ppm_get {
> +	u16 offset;
> +	u8 size;

Same here.

Re: [PATCH v5 2/8] platform/chrome: Update EC feature flags

[Tzung-Bi Shih]

On Tue, Sep 03, 2024 at 04:30:27PM +0000, Łukasz Bartosik wrote:
> From: Pavan Holla <pholla@chromium.org>
> 
> Define EC_FEATURE_UCSI_PPM to enable usage of the cros_ec_ucsi
> driver. Also, add any feature flags that are implemented by the EC
> but are missing in the kernel header.
> 
> Signed-off-by: Pavan Holla <pholla@chromium.org>

Same as previous patch, it needs your S-o-b tag at the end.  See [1].

[1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

Re: [PATCH v5 1/8] platform/chrome: Update ChromeOS EC header for UCSI

[Łukasz Bartosik]

On Fri, Sep 6, 2024 at 10:44 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Tue, Sep 03, 2024 at 04:30:26PM +0000, Łukasz Bartosik wrote:
> > From: Pavan Holla <pholla@chromium.org>
> >
> > Add EC host commands for reading and writing UCSI structures
> > in the EC. The corresponding kernel driver is cros-ec-ucsi.
> >
> > Also update PD events supported by the EC.
> >
> > Signed-off-by: Pavan Holla <pholla@chromium.org>
>
> It needs your S-o-b tag at the end.  See [1].
>
> [1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
>
> > --- a/include/linux/platform_data/cros_ec_commands.h
> > +++ b/include/linux/platform_data/cros_ec_commands.h
> > @@ -5012,8 +5012,10 @@ struct ec_response_pd_status {
> [...]
> >  struct ec_response_host_event_status {
> > -     uint32_t status;      /* PD MCU host event status */
> > +     u32 status;      /* PD MCU host event status */
>
> Even though ./scripts/checkpatch.pl dislikes it, but please don't do that.
> The header is a re-import from EC's repo.  We should try not to be divergent
> from the origin too much.
>
> > +/*
> > + * Read/write interface for UCSI OPM <-> PPM communication.
> > + */
>
> Same reason: it'd be better if it can align to [2].
>
> [2]: https://crrev.com/1454f2e8dac20ca37428744345c1bb4fdec30255/include/ec_commands.h#8055
>

I will do.

> > +#define EC_CMD_UCSI_PPM_SET 0x0140
> > +
> > +/* The data size is stored in the host command protocol header. */
> > +struct ec_params_ucsi_ppm_set {
> > +     u16 offset;
> > +     u8 data[];
>
> Same for the u16 and u8.
>
> > +struct ec_params_ucsi_ppm_get {
> > +     u16 offset;
> > +     u8 size;
>
> Same here.

It was a comment from Greg no to use uint*_t types but I agree the
changes I made are inconsistent with the rest of cros_ec_commands.h
file.

Greg would you be ok to stay with uint*_t types in cros_ec_commands.h
to be consistent with the rest of the file ?

Thanks,
Lukasz

Re: [PATCH v5 2/8] platform/chrome: Update EC feature flags

[Łukasz Bartosik]

On Fri, Sep 6, 2024 at 10:44 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Tue, Sep 03, 2024 at 04:30:27PM +0000, Łukasz Bartosik wrote:
> > From: Pavan Holla <pholla@chromium.org>
> >
> > Define EC_FEATURE_UCSI_PPM to enable usage of the cros_ec_ucsi
> > driver. Also, add any feature flags that are implemented by the EC
> > but are missing in the kernel header.
> >
> > Signed-off-by: Pavan Holla <pholla@chromium.org>
>
> Same as previous patch, it needs your S-o-b tag at the end.  See [1].
>
> [1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

I have not added any modifications to this patch. I understand that my
S-o-b tag is not needed in such a case. Is that not correct ?

Thanks,
Lukasz

Re: [PATCH v5 2/8] platform/chrome: Update EC feature flags

[Greg Kroah-Hartman]

On Fri, Sep 06, 2024 at 03:53:06PM +0200, Łukasz Bartosik wrote:
> On Fri, Sep 6, 2024 at 10:44 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > On Tue, Sep 03, 2024 at 04:30:27PM +0000, Łukasz Bartosik wrote:
> > > From: Pavan Holla <pholla@chromium.org>
> > >
> > > Define EC_FEATURE_UCSI_PPM to enable usage of the cros_ec_ucsi
> > > driver. Also, add any feature flags that are implemented by the EC
> > > but are missing in the kernel header.
> > >
> > > Signed-off-by: Pavan Holla <pholla@chromium.org>
> >
> > Same as previous patch, it needs your S-o-b tag at the end.  See [1].
> >
> > [1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
> 
> I have not added any modifications to this patch. I understand that my
> S-o-b tag is not needed in such a case. Is that not correct ?

If you are forwarding on patches from someone else, yes, you HAVE TO
sign off on it as well saying that you are allowed to do the forwarding.

thanks,

greg k-h

Re: [PATCH v5 3/8] usb: typec: ucsi: Implement ChromeOS UCSI driver

[Heikki Krogerus]

Hi,

Sorry to go back on this, but I noticed something..

On Tue, Sep 03, 2024 at 04:30:28PM +0000, Łukasz Bartosik wrote:
> +static int cros_ucsi_async_control(struct ucsi *ucsi, u64 cmd)
> +{
> +	struct cros_ucsi_data *udata = ucsi_get_drvdata(ucsi);
> +	struct ec_params_ucsi_ppm_set *req;
> +	size_t req_len;
> +	int ret;
> +
> +	req_len = sizeof(struct ec_params_ucsi_ppm_set) + sizeof(cmd);
> +	req = kzalloc(req_len, GFP_KERNEL);
> +	if (!req)
> +		return -ENOMEM;

Where is the memory for req released?

First I though that cros_ec_cmd() does it, but it's actually
allocating it's own cros_ec_command, and then releasing that in the
end, so I just got confused about it.

Is this a memory leak, or am I missing something?

> +
> +	req->offset = UCSI_CONTROL;
> +	memcpy(req->data, &cmd, sizeof(cmd));
> +	ret = cros_ec_cmd(udata->ec, 0, EC_CMD_UCSI_PPM_SET,
> +			  req, req_len, NULL, 0);
> +	if (ret < 0) {
> +		dev_warn(udata->dev, "Failed to send EC message UCSI_PPM_SET: error=%d", ret);
> +		return ret;
> +	}
> +	return 0;
> +}

thanks,

-- 
heikki

Re: [PATCH v5 2/8] platform/chrome: Update EC feature flags

[Łukasz Bartosik]

On Fri, Sep 6, 2024 at 4:07 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Sep 06, 2024 at 03:53:06PM +0200, Łukasz Bartosik wrote:
> > On Fri, Sep 6, 2024 at 10:44 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> > >
> > > On Tue, Sep 03, 2024 at 04:30:27PM +0000, Łukasz Bartosik wrote:
> > > > From: Pavan Holla <pholla@chromium.org>
> > > >
> > > > Define EC_FEATURE_UCSI_PPM to enable usage of the cros_ec_ucsi
> > > > driver. Also, add any feature flags that are implemented by the EC
> > > > but are missing in the kernel header.
> > > >
> > > > Signed-off-by: Pavan Holla <pholla@chromium.org>
> > >
> > > Same as previous patch, it needs your S-o-b tag at the end.  See [1].
> > >
> > > [1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
> >
> > I have not added any modifications to this patch. I understand that my
> > S-o-b tag is not needed in such a case. Is that not correct ?
>
> If you are forwarding on patches from someone else, yes, you HAVE TO
> sign off on it as well saying that you are allowed to do the forwarding.
>

Thanks for the clarification Greg.
I will add S-o-b then.

Thanks,
Lukasz

> thanks,
>
> greg k-h
>

Re: [PATCH v5 3/8] usb: typec: ucsi: Implement ChromeOS UCSI driver

[Łukasz Bartosik]

On Fri, Sep 6, 2024 at 4:20 PM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> Hi,
>
> Sorry to go back on this, but I noticed something..
>
> On Tue, Sep 03, 2024 at 04:30:28PM +0000, Łukasz Bartosik wrote:
> > +static int cros_ucsi_async_control(struct ucsi *ucsi, u64 cmd)
> > +{
> > +     struct cros_ucsi_data *udata = ucsi_get_drvdata(ucsi);
> > +     struct ec_params_ucsi_ppm_set *req;
> > +     size_t req_len;
> > +     int ret;
> > +
> > +     req_len = sizeof(struct ec_params_ucsi_ppm_set) + sizeof(cmd);
> > +     req = kzalloc(req_len, GFP_KERNEL);
> > +     if (!req)
> > +             return -ENOMEM;
>
> Where is the memory for req released?
>
> First I though that cros_ec_cmd() does it, but it's actually
> allocating it's own cros_ec_command, and then releasing that in the
> end, so I just got confused about it.
>
> Is this a memory leak, or am I missing something?
>

Yes, you are right this is a memory leak.
Thanks for catching this. I will fix it in the next version.

Regards,
Lukasz

> > +
> > +     req->offset = UCSI_CONTROL;
> > +     memcpy(req->data, &cmd, sizeof(cmd));
> > +     ret = cros_ec_cmd(udata->ec, 0, EC_CMD_UCSI_PPM_SET,
> > +                       req, req_len, NULL, 0);
> > +     if (ret < 0) {
> > +             dev_warn(udata->dev, "Failed to send EC message UCSI_PPM_SET: error=%d", ret);
> > +             return ret;
> > +     }
> > +     return 0;
> > +}
>
> thanks,
>
> --
> heikki