diff mbox series

[v2,3/3] platform/chrome: cros_ec_ucsi: Implement UCSI PDC driver

Message ID 20240325-public-ucsi-h-v2-3-a6d716968bb1@chromium.org
State Superseded
Headers show
Series [v2,1/3] usb: typec: ucsi: Provide interface for UCSI transport | expand

Commit Message

Pavan Holla March 25, 2024, 11:42 p.m. UTC
Implementation of transport driver for UCSI. This driver will be used
if the ChromeOS EC implements a PPM.

Signed-off-by: Pavan Holla <pholla@chromium.org>
---
 drivers/platform/chrome/Kconfig                |  14 ++
 drivers/platform/chrome/Makefile               |   1 +
 drivers/platform/chrome/cros_ec_ucsi.c         | 247 +++++++++++++++++++++++++
 include/linux/platform_data/cros_ec_commands.h |  19 ++
 4 files changed, 281 insertions(+)

Comments

Dmitry Baryshkov March 28, 2024, 3:32 p.m. UTC | #1
On 26/03/2024 01:42, Pavan Holla wrote:
> Implementation of transport driver for UCSI. This driver will be used
> if the ChromeOS EC implements a PPM.
> 
> Signed-off-by: Pavan Holla <pholla@chromium.org>
> ---
>   drivers/platform/chrome/Kconfig                |  14 ++
>   drivers/platform/chrome/Makefile               |   1 +
>   drivers/platform/chrome/cros_ec_ucsi.c         | 247 +++++++++++++++++++++++++
>   include/linux/platform_data/cros_ec_commands.h |  19 ++

While it's fine to use platform/chrome for platform drivers, please 
place drivers which have a subsystem into the subsystem dir. I think we 
don't want to hunt UCSI implementations all over the codebase. Please 
use drivers/usb/typec/ucsi/ location for your driver. This also removes 
a need for a global header.
Pavan Holla March 29, 2024, 3:08 p.m. UTC | #2
Hi Dmitry,

Thanks for the review.

On Thu, Mar 28, 2024 at 8:32 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
> While it's fine to use platform/chrome for platform drivers, please
> place drivers which have a subsystem into the subsystem dir. I think we
> don't want to hunt UCSI implementations all over the codebase. Please
> use drivers/usb/typec/ucsi/ location for your driver. This also removes
> a need for a global header.

I agree with your assessment that drivers/usb/typec/ucsi/ is a good
location for the driver currently. However, the driver is still in
early stages of development. We may have to account for PDC/ EC quirks
(we have multiple vendors), add FW update functionality outside the
UCSI spec, or add PDC logging functionality. While I'd like to write
separate drivers for those, some of this functionality will need to
acquire a lock over communication to the PDC. Even if I were to write
separate drivers for new functionality related to the PDC, maybe it's
better for all ChromeOS PDC related drivers to reside in the same
location. I am not sure what form this driver will take in the near
future, thus I would prefer to keep it in platform/chrome. Maybe
cros_ec_ucsi isn't the best name here, cros_ec_pdc probably conveys
the intention better.

Thanks,
Pavan
Dmitry Baryshkov March 29, 2024, 3:13 p.m. UTC | #3
On Fri, 29 Mar 2024 at 17:09, Pavan Holla <pholla@chromium.org> wrote:
>
> Hi Dmitry,
>
> Thanks for the review.
>
> On Thu, Mar 28, 2024 at 8:32 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> > While it's fine to use platform/chrome for platform drivers, please
> > place drivers which have a subsystem into the subsystem dir. I think we
> > don't want to hunt UCSI implementations all over the codebase. Please
> > use drivers/usb/typec/ucsi/ location for your driver. This also removes
> > a need for a global header.
>
> I agree with your assessment that drivers/usb/typec/ucsi/ is a good
> location for the driver currently. However, the driver is still in
> early stages of development. We may have to account for PDC/ EC quirks
> (we have multiple vendors), add FW update functionality outside the
> UCSI spec, or add PDC logging functionality. While I'd like to write
> separate drivers for those, some of this functionality will need to
> acquire a lock over communication to the PDC. Even if I were to write
> separate drivers for new functionality related to the PDC, maybe it's
> better for all ChromeOS PDC related drivers to reside in the same
> location. I am not sure what form this driver will take in the near
> future, thus I would prefer to keep it in platform/chrome. Maybe
> cros_ec_ucsi isn't the best name here, cros_ec_pdc probably conveys
> the intention better.

In such a case please consider using auxilliary device bus or MFD
cells to describe pdc / ucsi relationship. See how this is handled for
pmic-glink / ucsi_glink.
The drivers/platform should really be limited to simple drivers, which
don't have a better place. Otherwise it becomes a nightmare to handle
driver changes (this applies not only to the UCSI but also to other
drivers which have their own subsystem tree).
Pavan Holla April 1, 2024, 8:32 p.m. UTC | #4
On Fri, Mar 29, 2024 at 8:13 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Fri, 29 Mar 2024 at 17:09, Pavan Holla <pholla@chromium.org> wrote:
> >
> > Hi Dmitry,
> >
> > Thanks for the review.
> >
> > On Thu, Mar 28, 2024 at 8:32 AM Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> > > While it's fine to use platform/chrome for platform drivers, please
> > > place drivers which have a subsystem into the subsystem dir. I think we
> > > don't want to hunt UCSI implementations all over the codebase. Please
> > > use drivers/usb/typec/ucsi/ location for your driver. This also removes
> > > a need for a global header.
> >
> > I agree with your assessment that drivers/usb/typec/ucsi/ is a good
> > location for the driver currently. However, the driver is still in
> > early stages of development. We may have to account for PDC/ EC quirks
> > (we have multiple vendors), add FW update functionality outside the
> > UCSI spec, or add PDC logging functionality. While I'd like to write
> > separate drivers for those, some of this functionality will need to
> > acquire a lock over communication to the PDC. Even if I were to write
> > separate drivers for new functionality related to the PDC, maybe it's
> > better for all ChromeOS PDC related drivers to reside in the same
> > location. I am not sure what form this driver will take in the near
> > future, thus I would prefer to keep it in platform/chrome. Maybe
> > cros_ec_ucsi isn't the best name here, cros_ec_pdc probably conveys
> > the intention better.
>
> In such a case please consider using auxilliary device bus or MFD
> cells to describe pdc / ucsi relationship. See how this is handled for
> pmic-glink / ucsi_glink.
> The drivers/platform should really be limited to simple drivers, which
> don't have a better place. Otherwise it becomes a nightmare to handle
> driver changes (this applies not only to the UCSI but also to other
> drivers which have their own subsystem tree).

Thanks for the pointers. I will move the driver to drivers/usb/typec/ucsi/
diff mbox series

Patch

diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 7a83346bfa53..67ab79a6815b 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -238,6 +238,20 @@  config CROS_EC_TYPEC
 	  To compile this driver as a module, choose M here: the module will be
 	  called cros-ec-typec.
 
+config CROS_EC_UCSI
+	tristate "UCSI Driver for ChromeOS EC"
+	depends on MFD_CROS_EC_DEV
+	depends on CROS_USBPD_NOTIFY
+	depends on TYPEC_UCSI
+	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 CROS_HPS_I2C
 	tristate "ChromeOS HPS device"
 	depends on HID && I2C && PM
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 2dcc6ccc2302..85964e165a70 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -19,6 +19,7 @@  obj-$(CONFIG_CROS_EC_UART)		+= cros_ec_uart.o
 cros_ec_lpcs-objs			:= cros_ec_lpc.o cros_ec_lpc_mec.o
 cros-ec-typec-objs			:= cros_ec_typec.o cros_typec_vdm.o
 obj-$(CONFIG_CROS_EC_TYPEC)		+= cros-ec-typec.o
+obj-$(CONFIG_CROS_EC_UCSI)		+= cros_ec_ucsi.o
 obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpcs.o
 obj-$(CONFIG_CROS_EC_PROTO)		+= cros_ec_proto.o cros_ec_trace.o
 obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT)	+= cros_kbd_led_backlight.o
diff --git a/drivers/platform/chrome/cros_ec_ucsi.c b/drivers/platform/chrome/cros_ec_ucsi.c
new file mode 100644
index 000000000000..9fee300a3d93
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_ucsi.c
@@ -0,0 +1,247 @@ 
+// 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/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/usb/ucsi.h>
+#include <linux/wait.h>
+
+#define DRV_NAME "cros-ec-ucsi"
+
+#define MAX_EC_DATA_SIZE 256
+#define WRITE_TMO_MS 500
+
+#define COMMAND_PENDING 1
+#define ACK_PENDING     2
+
+#define UCSI_COMMAND(_cmd_) ((_cmd_) & 0xff)
+#define UCSI_ACK_CC_CI 0x04
+
+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_async_write(struct ucsi *ucsi, unsigned int offset,
+				 const void *val, size_t val_len)
+{
+	struct cros_ucsi_data *udata = ucsi_get_drvdata(ucsi);
+	uint8_t ec_buffer[MAX_EC_DATA_SIZE + sizeof(struct ec_params_ucsi_ppm_set)];
+	struct ec_params_ucsi_ppm_set *req = (struct ec_params_ucsi_ppm_set *)ec_buffer;
+	int ret = 0;
+
+	if (val_len > MAX_EC_DATA_SIZE) {
+		dev_err(udata->dev, "Can't write %zu bytes. Too big.", val_len);
+		return -EINVAL;
+	}
+
+	memset(req, 0, sizeof(ec_buffer));
+	req->offset = offset;
+	memcpy(req->data, val, val_len);
+	ret = cros_ec_cmd(udata->ec, 0, EC_CMD_UCSI_PPM_SET,
+			  req, sizeof(struct ec_params_ucsi_ppm_set) + val_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_write(struct ucsi *ucsi, unsigned int offset,
+				const void *val, size_t val_len)
+{
+	struct cros_ucsi_data *udata = ucsi_get_drvdata(ucsi);
+	bool ack = UCSI_COMMAND(*(u64 *)val) == 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_write(ucsi, offset, val, val_len);
+	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 = cros_ucsi_read,
+	.async_write = cros_ucsi_async_write,
+	.sync_write = cros_ucsi_sync_write,
+};
+
+static void cros_ucsi_work(struct work_struct *work)
+{
+	struct cros_ucsi_data *udata = container_of(work, struct cros_ucsi_data, work);
+	u32 cci;
+	int ret;
+
+	ret = cros_ucsi_read(udata->ucsi, UCSI_CCI, &cci, sizeof(cci));
+	if (ret)
+		return;
+
+	if (UCSI_CCI_CONNECTOR(cci))
+		ucsi_connector_change(udata->ucsi, UCSI_CCI_CONNECTOR(cci));
+
+	if (cci & UCSI_CCI_ACK_COMPLETE && test_bit(ACK_PENDING, &udata->flags))
+		complete(&udata->complete);
+	if (cci & UCSI_CCI_COMMAND_COMPLETE &&
+	    test_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 int cros_ucsi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct cros_ucsi_data *udata;
+	int ret;
+
+	udata = devm_kzalloc(dev, sizeof(*udata), GFP_KERNEL);
+	if (!udata)
+		return -ENOMEM;
+
+	udata->dev = dev;
+
+	udata->ec = ((struct cros_ec_dev *) dev_get_drvdata(pdev->dev.parent))->ec_dev;
+	if (!udata->ec) {
+		dev_err(dev, "couldn't find parent EC device\n");
+		return -ENODEV;
+	}
+
+	platform_set_drvdata(pdev, udata);
+
+	INIT_WORK(&udata->work, cros_ucsi_work);
+	init_completion(&udata->complete);
+
+	udata->ucsi = ucsi_create(udata->dev, &cros_ucsi_ops);
+	if (IS_ERR(udata->ucsi)) {
+		dev_err(dev, "failed to allocate UCSI instance\n");
+		return PTR_ERR(udata->ucsi);
+	}
+
+	ucsi_set_drvdata(udata->ucsi, udata);
+
+	ret = ucsi_register(udata->ucsi);
+	if (ret) {
+		ucsi_destroy(udata->ucsi);
+		udata->ucsi = NULL;
+		return ret;
+	}
+
+	udata->nb.notifier_call = cros_ucsi_event;
+	ret = cros_usbpd_register_notify(&udata->nb);
+	return ret;
+}
+
+static int cros_ucsi_remove(struct platform_device *dev)
+{
+	struct cros_ucsi_data *udata = platform_get_drvdata(dev);
+
+	if (udata->ucsi) {
+		ucsi_unregister(udata->ucsi);
+		ucsi_destroy(udata->ucsi);
+		udata->ucsi = NULL;
+	}
+	return 0;
+}
+
+static int cros_ucsi_suspend(struct device *dev)
+{
+	struct cros_ucsi_data *udata = dev_get_drvdata(dev);
+
+	cancel_work_sync(&udata->work);
+
+	return 0;
+}
+
+static int 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 struct platform_driver cros_ucsi_driver = {
+	.driver = {
+		.name = DRV_NAME,
+		.pm = &cros_ucsi_pm_ops,
+	},
+	.probe = cros_ucsi_probe,
+	.remove = cros_ucsi_remove,
+};
+
+module_platform_driver(cros_ucsi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("UCSI driver for ChromeOS EC.");
+MODULE_ALIAS("platform:" DRV_NAME);
diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
index ecc47d5fe239..39f6f54bbe4c 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -4933,6 +4933,7 @@  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_PPM               BIT(5)
 struct ec_response_host_event_status {
 	uint32_t status;      /* PD MCU host event status */
 } __ec_align4;
@@ -5994,6 +5995,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 {
+	uint16_t offset;
+	uint8_t data[];
+} __ec_align2;
+
+#define EC_CMD_UCSI_PPM_GET 0x0141
+
+struct ec_params_ucsi_ppm_get {
+	uint16_t offset;
+	uint8_t size;
+} __ec_align2;
+
 /*****************************************************************************/
 /* The command range 0x200-0x2FF is reserved for Rotor. */