diff mbox series

[3/5] usb: typec: ucsi: add Huawei Matebook E Go (sc8280xp) ucsi driver

Message ID 20241227171353.404432-4-mitltlatltl@gmail.com (mailing list archive)
State New
Headers show
Series platform: arm64: Huawei Matebook E Go embedded controller | expand

Commit Message

Pengyu Luo Dec. 27, 2024, 5:13 p.m. UTC
The Huawei Matebook E Go (sc8280xp) tablet provides implements UCSI
interface in the onboard EC. Add the glue driver to interface the
platform's UCSI implementation.

Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
---
 drivers/usb/typec/ucsi/Kconfig              |   9 +
 drivers/usb/typec/ucsi/Makefile             |   1 +
 drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c | 481 ++++++++++++++++++++
 3 files changed, 491 insertions(+)
 create mode 100644 drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c

Comments

Bryan O'Donoghue Dec. 28, 2024, 1:06 p.m. UTC | #1
On 27/12/2024 17:13, Pengyu Luo wrote:
> The Huawei Matebook E Go (sc8280xp) tablet provides implements UCSI
> interface in the onboard EC. Add the glue driver to interface the
> platform's UCSI implementation.
> 
> Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> ---
>   drivers/usb/typec/ucsi/Kconfig              |   9 +
>   drivers/usb/typec/ucsi/Makefile             |   1 +
>   drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c | 481 ++++++++++++++++++++
>   3 files changed, 491 insertions(+)
>   create mode 100644 drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
> 
> diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
> index 680e1b87b..0d0f07488 100644
> --- a/drivers/usb/typec/ucsi/Kconfig
> +++ b/drivers/usb/typec/ucsi/Kconfig
> @@ -78,4 +78,13 @@ config UCSI_LENOVO_YOGA_C630
>   	  To compile the driver as a module, choose M here: the module will be
>   	  called ucsi_yoga_c630.
>   
> +config UCSI_HUAWEI_GAOKUN
> +	tristate "UCSI Interface Driver for Huawei Matebook E Go (sc8280xp)"
> +	depends on EC_HUAWEI_GAOKUN
> +	help
> +	  This driver enables UCSI support on the Huawei Matebook E Go tablet.
> +
> +	  To compile the driver as a module, choose M here: the module will be
> +	  called ucsi_huawei_gaokun.
> +
>   endif
> diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
> index aed41d238..0b400122b 100644
> --- a/drivers/usb/typec/ucsi/Makefile
> +++ b/drivers/usb/typec/ucsi/Makefile
> @@ -22,3 +22,4 @@ obj-$(CONFIG_UCSI_CCG)			+= ucsi_ccg.o
>   obj-$(CONFIG_UCSI_STM32G0)		+= ucsi_stm32g0.o
>   obj-$(CONFIG_UCSI_PMIC_GLINK)		+= ucsi_glink.o
>   obj-$(CONFIG_UCSI_LENOVO_YOGA_C630)	+= ucsi_yoga_c630.o
> +obj-$(CONFIG_UCSI_HUAWEI_GAOKUN)	+= ucsi_huawei_gaokun.o
> diff --git a/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
> new file mode 100644
> index 000000000..84ed0407d
> --- /dev/null
> +++ b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
> @@ -0,0 +1,481 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ucsi-huawei-gaokun - A UCSI driver for HUAWEI Matebook E Go (sc8280xp)
> + *
> + * reference: drivers/usb/typec/ucsi/ucsi_yoga_c630.c
> + *            drivers/usb/typec/ucsi/ucsi_glink.c
> + *            drivers/soc/qcom/pmic_glink_altmode.c
> + *
> + * Copyright (C) 2024 Pengyu Luo <mitltlatltl@gmail.com>
> + */
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/bitops.h>
> +#include <linux/completion.h>
> +#include <linux/container_of.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +#include <linux/of.h>
> +#include <linux/string.h>
> +#include <linux/workqueue_types.h>
> +
> +#include <linux/usb/pd_vdo.h>
> +#include <drm/bridge/aux-bridge.h

Is there a reason you don't have strict include alphanumeric ordering here ?

>
> +
> +#include "ucsi.h"
> +#include <linux/platform_data/huawei-gaokun-ec.h>
> +
> +
> +#define EC_EVENT_UCSI	0x21
> +#define EC_EVENT_USB	0x22
> +
> +#define GAOKUN_CCX_MASK		GENMASK(1, 0)
> +#define GAOKUN_MUX_MASK		GENMASK(3, 2)
> +
> +#define GAOKUN_DPAM_MASK	GENMASK(3, 0)
> +#define GAOKUN_HPD_STATE_MASK	BIT(4)
> +#define GAOKUN_HPD_IRQ_MASK	BIT(5)
> +
> +#define CCX_TO_ORI(ccx) (++ccx % 3)

Why do you increment the value of the enum ?
Seems strange.

> +
> +#define GET_IDX(updt) (ffs(updt) - 1)
> +
> +/* Configuration Channel Extension */
> +enum gaokun_ucsi_ccx {
> +	USBC_CCX_NORMAL,
> +	USBC_CCX_REVERSE,
> +	USBC_CCX_NONE,
> +};
> +
> +enum gaokun_ucsi_mux {
> +	USBC_MUX_NONE,
> +	USBC_MUX_USB_2L,
> +	USBC_MUX_DP_4L,
> +	USBC_MUX_USB_DP,
> +};
> +
> +struct gaokun_ucsi_reg {
> +	u8 port_num;
> +	u8 port_updt;
> +	u8 port_data[4];
> +	u8 checksum;
> +	u8 reserved;
> +} __packed;
> +
> +struct gaokun_ucsi_port {
> +	struct completion usb_ack;
> +	spinlock_t lock;
> +
> +	struct gaokun_ucsi *ucsi;
> +	struct auxiliary_device *bridge;
> +
> +	int idx;
> +	enum gaokun_ucsi_ccx ccx;
> +	enum gaokun_ucsi_mux mux;
> +	u8 mode;
> +	u16 svid;
> +	u8 hpd_state;
> +	u8 hpd_irq;
> +};
> +
> +struct gaokun_ucsi {
> +	struct gaokun_ec *ec;
> +	struct ucsi *ucsi;
> +	struct gaokun_ucsi_port *ports;
> +	struct device *dev;
> +	struct work_struct work;
> +	struct notifier_block nb;
> +	u16 version;
> +	u8 port_num;
> +};
> +
> +/* -------------------------------------------------------------------------- */
> +/* For UCSI */
> +
> +static int gaokun_ucsi_read_version(struct ucsi *ucsi, u16 *version)
> +{
> +	struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> +
> +	*version = uec->version;
> +
> +	return 0;
> +}
> +
> +static int gaokun_ucsi_read_cci(struct ucsi *ucsi, u32 *cci)
> +{
> +	struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> +	u8 buf[GAOKUN_UCSI_READ_SIZE];
> +	int ret;
> +
> +	ret = gaokun_ec_ucsi_read(uec->ec, buf);
> +	if (ret)
> +		return ret;
> +
> +	memcpy(cci, buf, sizeof(*cci));
> +
> +	return 0;
> +}
> +
> +static int gaokun_ucsi_read_message_in(struct ucsi *ucsi,
> +				       void *val, size_t val_len)
> +{
> +	struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> +	u8 buf[GAOKUN_UCSI_READ_SIZE];
> +	int ret;
> +
> +	ret = gaokun_ec_ucsi_read(uec->ec, buf);
> +	if (ret)
> +		return ret;
> +
> +	memcpy(val, buf + GAOKUN_UCSI_CCI_SIZE,
> +	       min(val_len, GAOKUN_UCSI_DATA_SIZE));
> +
> +	return 0;
> +}
> +
> +static int gaokun_ucsi_async_control(struct ucsi *ucsi, u64 command)
> +{
> +	struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> +	u8 buf[GAOKUN_UCSI_WRITE_SIZE] = {};
> +
> +	memcpy(buf, &command, sizeof(command));
> +
> +	return gaokun_ec_ucsi_write(uec->ec, buf);
> +}
> +
> +static void gaokun_ucsi_update_connector(struct ucsi_connector *con)
> +{
> +	struct gaokun_ucsi *uec = ucsi_get_drvdata(con->ucsi);
> +
> +	if (con->num > uec->port_num)
> +		return;
> +
> +	con->typec_cap.orientation_aware = true;
> +}
> +
> +static void gaokun_set_orientation(struct ucsi_connector *con,
> +				   struct gaokun_ucsi_port *port)
> +{
> +	enum gaokun_ucsi_ccx ccx;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&port->lock, flags);
> +	ccx = port->ccx;
> +	spin_unlock_irqrestore(&port->lock, flags);
> +
> +	typec_set_orientation(con->port, CCX_TO_ORI(ccx));
> +}
> +
> +static void gaokun_ucsi_connector_status(struct ucsi_connector *con)
> +{
> +	struct gaokun_ucsi *uec = ucsi_get_drvdata(con->ucsi);
> +	int idx;
> +
> +	idx = con->num - 1;
> +	if (con->num > uec->port_num) {
> +		dev_warn(uec->ucsi->dev, "set orientation out of range: con%d\n", idx);
> +		return;
> +	}
> +
> +	gaokun_set_orientation(con, &uec->ports[idx]);
> +}
> +
> +const struct ucsi_operations gaokun_ucsi_ops = {
> +	.read_version = gaokun_ucsi_read_version,
> +	.read_cci = gaokun_ucsi_read_cci,
> +	.read_message_in = gaokun_ucsi_read_message_in,
> +	.sync_control = ucsi_sync_control_common,
> +	.async_control = gaokun_ucsi_async_control,
> +	.update_connector = gaokun_ucsi_update_connector,
> +	.connector_status = gaokun_ucsi_connector_status,
> +};
> +
> +/* -------------------------------------------------------------------------- */
> +/* For Altmode */
> +
> +static void gaokun_ucsi_port_update(struct gaokun_ucsi_port *port,
> +				    const u8 *port_data)
> +{
> +	unsigned long flags;
> +	u8 dcc, ddi;
> +	int offset = port->idx * 2; /* every port has 2 Bytes data */
> +
> +	dcc = port_data[offset];
> +	ddi = port_data[offset + 1];
> +
> +	spin_lock_irqsave(&port->lock, flags);
> +
> +	port->ccx = FIELD_GET(GAOKUN_CCX_MASK, dcc);
> +	port->mux = FIELD_GET(GAOKUN_MUX_MASK, dcc);
> +	port->mode = FIELD_GET(GAOKUN_DPAM_MASK, ddi);
> +	port->hpd_state = FIELD_GET(GAOKUN_HPD_STATE_MASK, ddi);
> +	port->hpd_irq = FIELD_GET(GAOKUN_HPD_IRQ_MASK, ddi);
> +
> +	switch (port->mux) {
> +	case USBC_MUX_NONE:
> +		port->svid = 0;
> +		break;
> +	case USBC_MUX_USB_2L:
> +		port->svid = USB_SID_PD;
> +		break;
> +	case USBC_MUX_DP_4L:
> +	case USBC_MUX_USB_DP:
> +		port->svid = USB_SID_DISPLAYPORT;
> +		if (port->ccx == USBC_CCX_REVERSE)
> +			port->mode -= 6;

why minus six ?
needs a comment.

> +		break;
> +	default:
> +		break;
> +	}
> +
> +	spin_unlock_irqrestore(&port->lock, flags);
> +}
> +
> +static int gaokun_ucsi_refresh(struct gaokun_ucsi *uec)
> +{
> +	struct gaokun_ucsi_reg ureg;
> +	int ret, idx;
> +
> +	ret = gaokun_ec_ucsi_get_reg(uec->ec, (u8 *)&ureg);
> +	if (ret)
> +		return -EIO;
> +
> +	uec->port_num = ureg.port_num;
> +	idx = GET_IDX(ureg.port_updt);
> +
> +	if (idx >= 0 && idx < ureg.port_num)
> +		gaokun_ucsi_port_update(&uec->ports[idx], ureg.port_data);

Since you are checking the validity of the index, you should -EINVAL if 
the index is out of range.

> +
> +	return idx;
> +}
> +
> +static void gaokun_ucsi_handle_altmode(struct gaokun_ucsi_port *port)
> +{
> +	struct gaokun_ucsi *uec = port->ucsi;
> +	int idx = port->idx;
> +
> +	if (idx >= uec->ucsi->cap.num_connectors || !uec->ucsi->connector) {
> +		dev_warn(uec->ucsi->dev, "altmode port out of range: %d\n", idx);
> +		return;
> +	}
> +
> +	/* UCSI callback .connector_status() have set orientation */
> +	if (port->bridge)
> +		drm_aux_hpd_bridge_notify(&port->bridge->dev,
> +					  port->hpd_state ?
> +					  connector_status_connected :
> +					  connector_status_disconnected);
> +
> +	gaokun_ec_ucsi_pan_ack(uec->ec, port->idx);
> +}
> +
> +static void gaokun_ucsi_altmode_notify_ind(struct gaokun_ucsi *uec)
> +{
> +	int idx;
> +
> +	idx = gaokun_ucsi_refresh(uec);
> +	if (idx < 0)
> +		gaokun_ec_ucsi_pan_ack(uec->ec, idx);
> +	else
> +		gaokun_ucsi_handle_altmode(&uec->ports[idx]);
> +}
> +
> +/*
> + * USB event is necessary for enabling altmode, the event should follow
> + * UCSI event, if not after timeout(this notify may be disabled somehow),
> + * then force to enable altmode.
> + */
> +static void gaokun_ucsi_handle_no_usb_event(struct gaokun_ucsi *uec, int idx)
> +{
> +	struct gaokun_ucsi_port *port;
> +
> +	port = &uec->ports[idx];
> +	if (!wait_for_completion_timeout(&port->usb_ack, 2 * HZ)) {
> +		dev_warn(uec->dev, "No USB EVENT, triggered by UCSI EVENT");
> +		gaokun_ucsi_altmode_notify_ind(uec);
> +	}
> +}
> +
> +static int gaokun_ucsi_notify(struct notifier_block *nb,
> +			      unsigned long action, void *data)
> +{
> +	u32 cci;
> +	struct gaokun_ucsi *uec = container_of(nb, struct gaokun_ucsi, nb);
> +
> +	switch (action) {
> +	case EC_EVENT_USB:
> +		gaokun_ucsi_altmode_notify_ind(uec);
> +		return NOTIFY_OK;
> +
> +	case EC_EVENT_UCSI:
> +		uec->ucsi->ops->read_cci(uec->ucsi, &cci);
> +		ucsi_notify_common(uec->ucsi, cci);
> +		if (UCSI_CCI_CONNECTOR(cci))
> +			gaokun_ucsi_handle_no_usb_event(uec, UCSI_CCI_CONNECTOR(cci) - 1);
> +
> +		return NOTIFY_OK;
> +
> +	default:
> +		return NOTIFY_DONE;
> +	}
> +}
> +
> +static int gaokun_ucsi_get_port_num(struct gaokun_ucsi *uec)
> +{
> +	struct gaokun_ucsi_reg ureg;
> +	int ret;
> +
> +	ret = gaokun_ec_ucsi_get_reg(uec->ec, (u8 *)&ureg);
> +
> +	return ret ? 0 : ureg.port_num;
> +}
> +
> +static int gaokun_ucsi_ports_init(struct gaokun_ucsi *uec)
> +{
> +	u32 port;
> +	int i, ret, port_num;
> +	struct device *dev = uec->dev;
> +	struct gaokun_ucsi_port *ucsi_port;
> +	struct fwnode_handle *fwnode;
> +
> +	port_num = gaokun_ucsi_get_port_num(uec);
> +	uec->port_num = port_num;
> +
> +	uec->ports = devm_kzalloc(dev, port_num * sizeof(*(uec->ports)),
> +				  GFP_KERNEL);
> +	if (!uec->ports)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < port_num; ++i) {
> +		ucsi_port = &uec->ports[i];
> +		ucsi_port->ccx = USBC_CCX_NONE;
> +		ucsi_port->idx = i;
> +		ucsi_port->ucsi = uec;
> +		init_completion(&ucsi_port->usb_ack);
> +		spin_lock_init(&ucsi_port->lock);
> +	}
> +
> +	device_for_each_child_node(dev, fwnode) {
> +		ret = fwnode_property_read_u32(fwnode, "reg", &port);
> +		if (ret < 0) {
> +			dev_err(dev, "missing reg property of %pOFn\n", fwnode);
> +			fwnode_handle_put(fwnode);
> +			return ret;
> +		}
> +
> +		if (port >= port_num) {
> +			dev_warn(dev, "invalid connector number %d, ignoring\n", port);
> +			continue;
> +		}
> +
> +		ucsi_port = &uec->ports[port];
> +		ucsi_port->bridge = devm_drm_dp_hpd_bridge_alloc(dev, to_of_node(fwnode));
> +		if (IS_ERR(ucsi_port->bridge)) {
> +			fwnode_handle_put(fwnode);
> +			return PTR_ERR(ucsi_port->bridge);
> +		}
> +	}
> +
> +	for (i = 0; i < port_num; i++) {
> +		if (!uec->ports[i].bridge)
> +			continue;
> +
> +		ret = devm_drm_dp_hpd_bridge_add(dev, uec->ports[i].bridge);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void gaokun_ucsi_register_worker(struct work_struct *work)
> +{
> +	struct gaokun_ucsi *uec;
> +	struct ucsi *ucsi;
> +	int ret;
> +
> +	uec = container_of(work, struct gaokun_ucsi, work);
> +	ucsi = uec->ucsi;
> +
> +	ucsi->quirks = UCSI_NO_PARTNER_PDOS | UCSI_DELAY_DEVICE_PDOS;
> +
> +	ssleep(3); /* EC can't handle UCSI properly in the early stage */

Could you not schedule work for + 3 seconds instead of sleeping here - 
representing the required stall time in some sort of state machine ?

3 seconds is an incredibly long time for a computer to sleep.

> +
> +	ret = gaokun_ec_register_notify(uec->ec, &uec->nb);
> +	if (ret) {
> +		dev_err_probe(ucsi->dev, ret, "notifier register failed\n");
> +		return;
> +	}
> +
> +	ret = ucsi_register(ucsi);
> +	if (ret)
> +		dev_err_probe(ucsi->dev, ret, "ucsi register failed\n");
> +}
> +
> +static int gaokun_ucsi_register(struct gaokun_ucsi *uec)
> +{
> +	schedule_work(&uec->work);
> +
> +	return 0;
> +}
> +
> +static int gaokun_ucsi_probe(struct auxiliary_device *adev,
> +			     const struct auxiliary_device_id *id)
> +{
> +	struct gaokun_ec *ec = adev->dev.platform_data;
> +	struct device *dev = &adev->dev;
> +	struct gaokun_ucsi *uec;
> +	int ret;
> +
> +	uec = devm_kzalloc(dev, sizeof(*uec), GFP_KERNEL);
> +	if (!uec)
> +		return -ENOMEM;
> +
> +	uec->ec = ec;
> +	uec->dev = dev;
> +	uec->version = 0x0100;
> +	uec->nb.notifier_call = gaokun_ucsi_notify;
> +
> +	INIT_WORK(&uec->work, gaokun_ucsi_register_worker);
> +
> +	ret = gaokun_ucsi_ports_init(uec);
> +	if (ret)
> +		return ret;
> +
> +	uec->ucsi = ucsi_create(dev, &gaokun_ucsi_ops);
> +	if (IS_ERR(uec->ucsi))
> +		return PTR_ERR(uec->ucsi);
> +
> +	ucsi_set_drvdata(uec->ucsi, uec);
> +	auxiliary_set_drvdata(adev, uec);
> +
> +	return gaokun_ucsi_register(uec);
> +}
> +
> +static void gaokun_ucsi_remove(struct auxiliary_device *adev)
> +{
> +	struct gaokun_ucsi *uec = auxiliary_get_drvdata(adev);
> +
> +	gaokun_ec_unregister_notify(uec->ec, &uec->nb);
> +	ucsi_unregister(uec->ucsi);
> +	ucsi_destroy(uec->ucsi);
> +}
> +
> +static const struct auxiliary_device_id gaokun_ucsi_id_table[] = {
> +	{ .name = GAOKUN_MOD_NAME "." GAOKUN_DEV_UCSI, },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(auxiliary, gaokun_ucsi_id_table);
> +
> +static struct auxiliary_driver gaokun_ucsi_driver = {
> +	.name = GAOKUN_DEV_UCSI,
> +	.id_table = gaokun_ucsi_id_table,
> +	.probe = gaokun_ucsi_probe,
> +	.remove = gaokun_ucsi_remove,
> +};
> +
> +module_auxiliary_driver(gaokun_ucsi_driver);
> +
> +MODULE_DESCRIPTION("HUAWEI Matebook E Go UCSI driver");
> +MODULE_LICENSE("GPL");
Pengyu Luo Dec. 28, 2024, 2:38 p.m. UTC | #2
On Sat, Dec 28, 2024 at 9:06 PM Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
> On 27/12/2024 17:13, Pengyu Luo wrote:
> > The Huawei Matebook E Go (sc8280xp) tablet provides implements UCSI
> > interface in the onboard EC. Add the glue driver to interface the
> > platform's UCSI implementation.
> >
> > Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> > ---
> >   drivers/usb/typec/ucsi/Kconfig              |   9 +
> >   drivers/usb/typec/ucsi/Makefile             |   1 +
> >   drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c | 481 ++++++++++++++++++++
> >   3 files changed, 491 insertions(+)
> >   create mode 100644 drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
> >
> > diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
> > index 680e1b87b..0d0f07488 100644
> > --- a/drivers/usb/typec/ucsi/Kconfig
> > +++ b/drivers/usb/typec/ucsi/Kconfig
> > @@ -78,4 +78,13 @@ config UCSI_LENOVO_YOGA_C630
> >         To compile the driver as a module, choose M here: the module will be
> >         called ucsi_yoga_c630.
> >
> > +config UCSI_HUAWEI_GAOKUN
> > +     tristate "UCSI Interface Driver for Huawei Matebook E Go (sc8280xp)"
> > +     depends on EC_HUAWEI_GAOKUN
> > +     help
> > +       This driver enables UCSI support on the Huawei Matebook E Go tablet.
> > +
> > +       To compile the driver as a module, choose M here: the module will be
> > +       called ucsi_huawei_gaokun.
> > +
> >   endif
> > diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
> > index aed41d238..0b400122b 100644
> > --- a/drivers/usb/typec/ucsi/Makefile
> > +++ b/drivers/usb/typec/ucsi/Makefile
> > @@ -22,3 +22,4 @@ obj-$(CONFIG_UCSI_CCG)                      += ucsi_ccg.o
> >   obj-$(CONFIG_UCSI_STM32G0)          += ucsi_stm32g0.o
> >   obj-$(CONFIG_UCSI_PMIC_GLINK)               += ucsi_glink.o
> >   obj-$(CONFIG_UCSI_LENOVO_YOGA_C630) += ucsi_yoga_c630.o
> > +obj-$(CONFIG_UCSI_HUAWEI_GAOKUN)     += ucsi_huawei_gaokun.o
> > diff --git a/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
> > new file mode 100644
> > index 000000000..84ed0407d
> > --- /dev/null
> > +++ b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
> > @@ -0,0 +1,481 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * ucsi-huawei-gaokun - A UCSI driver for HUAWEI Matebook E Go (sc8280xp)
> > + *
> > + * reference: drivers/usb/typec/ucsi/ucsi_yoga_c630.c
> > + *            drivers/usb/typec/ucsi/ucsi_glink.c
> > + *            drivers/soc/qcom/pmic_glink_altmode.c
> > + *
> > + * Copyright (C) 2024 Pengyu Luo <mitltlatltl@gmail.com>
> > + */
> > +
> > +#include <linux/auxiliary_bus.h>
> > +#include <linux/bitops.h>
> > +#include <linux/completion.h>
> > +#include <linux/container_of.h>
> > +#include <linux/delay.h>
> > +#include <linux/module.h>
> > +#include <linux/notifier.h>
> > +#include <linux/of.h>
> > +#include <linux/string.h>
> > +#include <linux/workqueue_types.h>
> > +
> > +#include <linux/usb/pd_vdo.h>
> > +#include <drm/bridge/aux-bridge.h
>
> Is there a reason you don't have strict include alphanumeric ordering here ?
>

These two is dp/alt mode related, so listing them out. Above of them are
general things.

> >
> > +
> > +#include "ucsi.h"
> > +#include <linux/platform_data/huawei-gaokun-ec.h>
> > +
> > +
> > +#define EC_EVENT_UCSI        0x21
> > +#define EC_EVENT_USB 0x22
> > +
> > +#define GAOKUN_CCX_MASK              GENMASK(1, 0)
> > +#define GAOKUN_MUX_MASK              GENMASK(3, 2)
> > +
> > +#define GAOKUN_DPAM_MASK     GENMASK(3, 0)
> > +#define GAOKUN_HPD_STATE_MASK        BIT(4)
> > +#define GAOKUN_HPD_IRQ_MASK  BIT(5)
> > +
> > +#define CCX_TO_ORI(ccx) (++ccx % 3)
>
> Why do you increment the value of the enum ?
> Seems strange.
>

EC's logic, it is just a trick. Qualcomm maps
0 1 2 to normal, reverse, none(no device insert)
typec lib maps 1 2 0 to that.

> > +
> > +#define GET_IDX(updt) (ffs(updt) - 1)
> > +
> > +/* Configuration Channel Extension */
> > +enum gaokun_ucsi_ccx {
> > +     USBC_CCX_NORMAL,
> > +     USBC_CCX_REVERSE,
> > +     USBC_CCX_NONE,
> > +};
> > +
> > +enum gaokun_ucsi_mux {
> > +     USBC_MUX_NONE,
> > +     USBC_MUX_USB_2L,
> > +     USBC_MUX_DP_4L,
> > +     USBC_MUX_USB_DP,
> > +};
> > +
> > +struct gaokun_ucsi_reg {
> > +     u8 port_num;
> > +     u8 port_updt;
> > +     u8 port_data[4];
> > +     u8 checksum;
> > +     u8 reserved;
> > +} __packed;
> > +
> > +struct gaokun_ucsi_port {
> > +     struct completion usb_ack;
> > +     spinlock_t lock;
> > +
> > +     struct gaokun_ucsi *ucsi;
> > +     struct auxiliary_device *bridge;
> > +
> > +     int idx;
> > +     enum gaokun_ucsi_ccx ccx;
> > +     enum gaokun_ucsi_mux mux;
> > +     u8 mode;
> > +     u16 svid;
> > +     u8 hpd_state;
> > +     u8 hpd_irq;
> > +};
> > +
> > +struct gaokun_ucsi {
> > +     struct gaokun_ec *ec;
> > +     struct ucsi *ucsi;
> > +     struct gaokun_ucsi_port *ports;
> > +     struct device *dev;
> > +     struct work_struct work;
> > +     struct notifier_block nb;
> > +     u16 version;
> > +     u8 port_num;
> > +};
> > +
> > +/* -------------------------------------------------------------------------- */
> > +/* For UCSI */
> > +
> > +static int gaokun_ucsi_read_version(struct ucsi *ucsi, u16 *version)
> > +{
> > +     struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> > +
> > +     *version = uec->version;
> > +
> > +     return 0;
> > +}
> > +
> > +static int gaokun_ucsi_read_cci(struct ucsi *ucsi, u32 *cci)
> > +{
> > +     struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> > +     u8 buf[GAOKUN_UCSI_READ_SIZE];
> > +     int ret;
> > +
> > +     ret = gaokun_ec_ucsi_read(uec->ec, buf);
> > +     if (ret)
> > +             return ret;
> > +
> > +     memcpy(cci, buf, sizeof(*cci));
> > +
> > +     return 0;
> > +}
> > +
> > +static int gaokun_ucsi_read_message_in(struct ucsi *ucsi,
> > +                                    void *val, size_t val_len)
> > +{
> > +     struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> > +     u8 buf[GAOKUN_UCSI_READ_SIZE];
> > +     int ret;
> > +
> > +     ret = gaokun_ec_ucsi_read(uec->ec, buf);
> > +     if (ret)
> > +             return ret;
> > +
> > +     memcpy(val, buf + GAOKUN_UCSI_CCI_SIZE,
> > +            min(val_len, GAOKUN_UCSI_DATA_SIZE));
> > +
> > +     return 0;
> > +}
> > +
> > +static int gaokun_ucsi_async_control(struct ucsi *ucsi, u64 command)
> > +{
> > +     struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> > +     u8 buf[GAOKUN_UCSI_WRITE_SIZE] = {};
> > +
> > +     memcpy(buf, &command, sizeof(command));
> > +
> > +     return gaokun_ec_ucsi_write(uec->ec, buf);
> > +}
> > +
> > +static void gaokun_ucsi_update_connector(struct ucsi_connector *con)
> > +{
> > +     struct gaokun_ucsi *uec = ucsi_get_drvdata(con->ucsi);
> > +
> > +     if (con->num > uec->port_num)
> > +             return;
> > +
> > +     con->typec_cap.orientation_aware = true;
> > +}
> > +
> > +static void gaokun_set_orientation(struct ucsi_connector *con,
> > +                                struct gaokun_ucsi_port *port)
> > +{
> > +     enum gaokun_ucsi_ccx ccx;
> > +     unsigned long flags;
> > +
> > +     spin_lock_irqsave(&port->lock, flags);
> > +     ccx = port->ccx;
> > +     spin_unlock_irqrestore(&port->lock, flags);
> > +
> > +     typec_set_orientation(con->port, CCX_TO_ORI(ccx));
> > +}
> > +
> > +static void gaokun_ucsi_connector_status(struct ucsi_connector *con)
> > +{
> > +     struct gaokun_ucsi *uec = ucsi_get_drvdata(con->ucsi);
> > +     int idx;
> > +
> > +     idx = con->num - 1;
> > +     if (con->num > uec->port_num) {
> > +             dev_warn(uec->ucsi->dev, "set orientation out of range: con%d\n", idx);
> > +             return;
> > +     }
> > +
> > +     gaokun_set_orientation(con, &uec->ports[idx]);
> > +}
> > +
> > +const struct ucsi_operations gaokun_ucsi_ops = {
> > +     .read_version = gaokun_ucsi_read_version,
> > +     .read_cci = gaokun_ucsi_read_cci,
> > +     .read_message_in = gaokun_ucsi_read_message_in,
> > +     .sync_control = ucsi_sync_control_common,
> > +     .async_control = gaokun_ucsi_async_control,
> > +     .update_connector = gaokun_ucsi_update_connector,
> > +     .connector_status = gaokun_ucsi_connector_status,
> > +};
> > +
> > +/* -------------------------------------------------------------------------- */
> > +/* For Altmode */
> > +
> > +static void gaokun_ucsi_port_update(struct gaokun_ucsi_port *port,
> > +                                 const u8 *port_data)
> > +{
> > +     unsigned long flags;
> > +     u8 dcc, ddi;
> > +     int offset = port->idx * 2; /* every port has 2 Bytes data */
> > +
> > +     dcc = port_data[offset];
> > +     ddi = port_data[offset + 1];
> > +
> > +     spin_lock_irqsave(&port->lock, flags);
> > +
> > +     port->ccx = FIELD_GET(GAOKUN_CCX_MASK, dcc);
> > +     port->mux = FIELD_GET(GAOKUN_MUX_MASK, dcc);
> > +     port->mode = FIELD_GET(GAOKUN_DPAM_MASK, ddi);
> > +     port->hpd_state = FIELD_GET(GAOKUN_HPD_STATE_MASK, ddi);
> > +     port->hpd_irq = FIELD_GET(GAOKUN_HPD_IRQ_MASK, ddi);
> > +
> > +     switch (port->mux) {
> > +     case USBC_MUX_NONE:
> > +             port->svid = 0;
> > +             break;
> > +     case USBC_MUX_USB_2L:
> > +             port->svid = USB_SID_PD;
> > +             break;
> > +     case USBC_MUX_DP_4L:
> > +     case USBC_MUX_USB_DP:
> > +             port->svid = USB_SID_DISPLAYPORT;
> > +             if (port->ccx == USBC_CCX_REVERSE)
> > +                     port->mode -= 6;
>
> why minus six ?
> needs a comment.
>

EC's logic. I don't know why, it is a quirk from Qualcomm or Huawei.
I will mention this.

> > +             break;
> > +     default:
> > +             break;
> > +     }
> > +
> > +     spin_unlock_irqrestore(&port->lock, flags);
> > +}
> > +
> > +static int gaokun_ucsi_refresh(struct gaokun_ucsi *uec)
> > +{
> > +     struct gaokun_ucsi_reg ureg;
> > +     int ret, idx;
> > +
> > +     ret = gaokun_ec_ucsi_get_reg(uec->ec, (u8 *)&ureg);
> > +     if (ret)
> > +             return -EIO;
> > +
> > +     uec->port_num = ureg.port_num;
> > +     idx = GET_IDX(ureg.port_updt);
> > +
> > +     if (idx >= 0 && idx < ureg.port_num)
> > +             gaokun_ucsi_port_update(&uec->ports[idx], ureg.port_data);
>
> Since you are checking the validity of the index, you should -EINVAL if
> the index is out of range.
>

EC / pmic glink encode every port in a bit
0/1/2/4/... => ???/left/right/some port

I remap it to -1/0/1/2, to access specific port exceptions(-1) are not
harmful, later in gaokun_ucsi_altmode_notify_ind

	if (idx < 0)
		gaokun_ec_ucsi_pan_ack(uec->ec, idx);
	else
		gaokun_ucsi_handle_altmode(&uec->ports[idx]);

gaokun_ec_ucsi_pan_ack can handle exceptions.

> > +
> > +     return idx;
> > +}
> > +
> > +static void gaokun_ucsi_handle_altmode(struct gaokun_ucsi_port *port)
> > +{
> > +     struct gaokun_ucsi *uec = port->ucsi;
> > +     int idx = port->idx;
> > +
> > +     if (idx >= uec->ucsi->cap.num_connectors || !uec->ucsi->connector) {
> > +             dev_warn(uec->ucsi->dev, "altmode port out of range: %d\n", idx);
> > +             return;
> > +     }
> > +
> > +     /* UCSI callback .connector_status() have set orientation */
> > +     if (port->bridge)
> > +             drm_aux_hpd_bridge_notify(&port->bridge->dev,
> > +                                       port->hpd_state ?
> > +                                       connector_status_connected :
> > +                                       connector_status_disconnected);
> > +
> > +     gaokun_ec_ucsi_pan_ack(uec->ec, port->idx);
> > +}
> > +
> > +static void gaokun_ucsi_altmode_notify_ind(struct gaokun_ucsi *uec)
> > +{
> > +     int idx;
> > +
> > +     idx = gaokun_ucsi_refresh(uec);
> > +     if (idx < 0)
> > +             gaokun_ec_ucsi_pan_ack(uec->ec, idx);
> > +     else
> > +             gaokun_ucsi_handle_altmode(&uec->ports[idx]);
> > +}
> > +
> > +/*
> > + * USB event is necessary for enabling altmode, the event should follow
> > + * UCSI event, if not after timeout(this notify may be disabled somehow),
> > + * then force to enable altmode.
> > + */
> > +static void gaokun_ucsi_handle_no_usb_event(struct gaokun_ucsi *uec, int idx)
> > +{
> > +     struct gaokun_ucsi_port *port;
> > +
> > +     port = &uec->ports[idx];
> > +     if (!wait_for_completion_timeout(&port->usb_ack, 2 * HZ)) {
> > +             dev_warn(uec->dev, "No USB EVENT, triggered by UCSI EVENT");
> > +             gaokun_ucsi_altmode_notify_ind(uec);
> > +     }
> > +}
> > +
> > +static int gaokun_ucsi_notify(struct notifier_block *nb,
> > +                           unsigned long action, void *data)
> > +{
> > +     u32 cci;
> > +     struct gaokun_ucsi *uec = container_of(nb, struct gaokun_ucsi, nb);
> > +
> > +     switch (action) {
> > +     case EC_EVENT_USB:
> > +             gaokun_ucsi_altmode_notify_ind(uec);
> > +             return NOTIFY_OK;
> > +
> > +     case EC_EVENT_UCSI:
> > +             uec->ucsi->ops->read_cci(uec->ucsi, &cci);
> > +             ucsi_notify_common(uec->ucsi, cci);
> > +             if (UCSI_CCI_CONNECTOR(cci))
> > +                     gaokun_ucsi_handle_no_usb_event(uec, UCSI_CCI_CONNECTOR(cci) - 1);
> > +
> > +             return NOTIFY_OK;
> > +
> > +     default:
> > +             return NOTIFY_DONE;
> > +     }
> > +}
> > +
> > +static int gaokun_ucsi_get_port_num(struct gaokun_ucsi *uec)
> > +{
> > +     struct gaokun_ucsi_reg ureg;
> > +     int ret;
> > +
> > +     ret = gaokun_ec_ucsi_get_reg(uec->ec, (u8 *)&ureg);
> > +
> > +     return ret ? 0 : ureg.port_num;
> > +}
> > +
> > +static int gaokun_ucsi_ports_init(struct gaokun_ucsi *uec)
> > +{
> > +     u32 port;
> > +     int i, ret, port_num;
> > +     struct device *dev = uec->dev;
> > +     struct gaokun_ucsi_port *ucsi_port;
> > +     struct fwnode_handle *fwnode;
> > +
> > +     port_num = gaokun_ucsi_get_port_num(uec);
> > +     uec->port_num = port_num;
> > +
> > +     uec->ports = devm_kzalloc(dev, port_num * sizeof(*(uec->ports)),
> > +                               GFP_KERNEL);
> > +     if (!uec->ports)
> > +             return -ENOMEM;
> > +
> > +     for (i = 0; i < port_num; ++i) {
> > +             ucsi_port = &uec->ports[i];
> > +             ucsi_port->ccx = USBC_CCX_NONE;
> > +             ucsi_port->idx = i;
> > +             ucsi_port->ucsi = uec;
> > +             init_completion(&ucsi_port->usb_ack);
> > +             spin_lock_init(&ucsi_port->lock);
> > +     }
> > +
> > +     device_for_each_child_node(dev, fwnode) {
> > +             ret = fwnode_property_read_u32(fwnode, "reg", &port);
> > +             if (ret < 0) {
> > +                     dev_err(dev, "missing reg property of %pOFn\n", fwnode);
> > +                     fwnode_handle_put(fwnode);
> > +                     return ret;
> > +             }
> > +
> > +             if (port >= port_num) {
> > +                     dev_warn(dev, "invalid connector number %d, ignoring\n", port);
> > +                     continue;
> > +             }
> > +
> > +             ucsi_port = &uec->ports[port];
> > +             ucsi_port->bridge = devm_drm_dp_hpd_bridge_alloc(dev, to_of_node(fwnode));
> > +             if (IS_ERR(ucsi_port->bridge)) {
> > +                     fwnode_handle_put(fwnode);
> > +                     return PTR_ERR(ucsi_port->bridge);
> > +             }
> > +     }
> > +
> > +     for (i = 0; i < port_num; i++) {
> > +             if (!uec->ports[i].bridge)
> > +                     continue;
> > +
> > +             ret = devm_drm_dp_hpd_bridge_add(dev, uec->ports[i].bridge);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static void gaokun_ucsi_register_worker(struct work_struct *work)
> > +{
> > +     struct gaokun_ucsi *uec;
> > +     struct ucsi *ucsi;
> > +     int ret;
> > +
> > +     uec = container_of(work, struct gaokun_ucsi, work);
> > +     ucsi = uec->ucsi;
> > +
> > +     ucsi->quirks = UCSI_NO_PARTNER_PDOS | UCSI_DELAY_DEVICE_PDOS;
> > +
> > +     ssleep(3); /* EC can't handle UCSI properly in the early stage */
>
> Could you not schedule work for + 3 seconds instead of sleeping here -
> representing the required stall time in some sort of state machine ?
>

I see, I will check work schedule interface.

> 3 seconds is an incredibly long time for a computer to sleep.
>

This module will be loaded at about 5th second after power up, if not
sleep, we will receive something disharmonious, sleeping for 3 seconds is
a hack.

> > +
> > +     ret = gaokun_ec_register_notify(uec->ec, &uec->nb);
> > +     if (ret) {
> > +             dev_err_probe(ucsi->dev, ret, "notifier register failed\n");
> > +             return;
> > +     }
> > +
> > +     ret = ucsi_register(ucsi);
> > +     if (ret)
> > +             dev_err_probe(ucsi->dev, ret, "ucsi register failed\n");
> > +}
> > +
> > +static int gaokun_ucsi_register(struct gaokun_ucsi *uec)
> > +{
> > +     schedule_work(&uec->work);
> > +
> > +     return 0;
> > +}
> > +
> > +static int gaokun_ucsi_probe(struct auxiliary_device *adev,
> > +                          const struct auxiliary_device_id *id)
> > +{
> > +     struct gaokun_ec *ec = adev->dev.platform_data;
> > +     struct device *dev = &adev->dev;
> > +     struct gaokun_ucsi *uec;
> > +     int ret;
> > +
> > +     uec = devm_kzalloc(dev, sizeof(*uec), GFP_KERNEL);
> > +     if (!uec)
> > +             return -ENOMEM;
> > +
> > +     uec->ec = ec;
> > +     uec->dev = dev;
> > +     uec->version = 0x0100;
> > +     uec->nb.notifier_call = gaokun_ucsi_notify;
> > +
> > +     INIT_WORK(&uec->work, gaokun_ucsi_register_worker);
> > +
> > +     ret = gaokun_ucsi_ports_init(uec);
> > +     if (ret)
> > +             return ret;
> > +
> > +     uec->ucsi = ucsi_create(dev, &gaokun_ucsi_ops);
> > +     if (IS_ERR(uec->ucsi))
> > +             return PTR_ERR(uec->ucsi);
> > +
> > +     ucsi_set_drvdata(uec->ucsi, uec);
> > +     auxiliary_set_drvdata(adev, uec);
> > +
> > +     return gaokun_ucsi_register(uec);
> > +}
> > +
> > +static void gaokun_ucsi_remove(struct auxiliary_device *adev)
> > +{
> > +     struct gaokun_ucsi *uec = auxiliary_get_drvdata(adev);
> > +
> > +     gaokun_ec_unregister_notify(uec->ec, &uec->nb);
> > +     ucsi_unregister(uec->ucsi);
> > +     ucsi_destroy(uec->ucsi);
> > +}
> > +
> > +static const struct auxiliary_device_id gaokun_ucsi_id_table[] = {
> > +     { .name = GAOKUN_MOD_NAME "." GAOKUN_DEV_UCSI, },
> > +     {}
> > +};
> > +MODULE_DEVICE_TABLE(auxiliary, gaokun_ucsi_id_table);
> > +
> > +static struct auxiliary_driver gaokun_ucsi_driver = {
> > +     .name = GAOKUN_DEV_UCSI,
> > +     .id_table = gaokun_ucsi_id_table,
> > +     .probe = gaokun_ucsi_probe,
> > +     .remove = gaokun_ucsi_remove,
> > +};
> > +
> > +module_auxiliary_driver(gaokun_ucsi_driver);
> > +
> > +MODULE_DESCRIPTION("HUAWEI Matebook E Go UCSI driver");
> > +MODULE_LICENSE("GPL");
>


Best Wishes,
Pengyu
Dmitry Baryshkov Dec. 29, 2024, 4:40 a.m. UTC | #3
On Sat, Dec 28, 2024 at 01:13:51AM +0800, Pengyu Luo wrote:
> The Huawei Matebook E Go (sc8280xp) tablet provides implements UCSI
> interface in the onboard EC. Add the glue driver to interface the
> platform's UCSI implementation.
> 
> Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> ---
>  drivers/usb/typec/ucsi/Kconfig              |   9 +
>  drivers/usb/typec/ucsi/Makefile             |   1 +
>  drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c | 481 ++++++++++++++++++++
>  3 files changed, 491 insertions(+)
>  create mode 100644 drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
> 
> diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
> index 680e1b87b..0d0f07488 100644
> --- a/drivers/usb/typec/ucsi/Kconfig
> +++ b/drivers/usb/typec/ucsi/Kconfig
> @@ -78,4 +78,13 @@ config UCSI_LENOVO_YOGA_C630
>  	  To compile the driver as a module, choose M here: the module will be
>  	  called ucsi_yoga_c630.
>  
> +config UCSI_HUAWEI_GAOKUN
> +	tristate "UCSI Interface Driver for Huawei Matebook E Go (sc8280xp)"
> +	depends on EC_HUAWEI_GAOKUN
> +	help
> +	  This driver enables UCSI support on the Huawei Matebook E Go tablet.
> +
> +	  To compile the driver as a module, choose M here: the module will be
> +	  called ucsi_huawei_gaokun.
> +
>  endif
> diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
> index aed41d238..0b400122b 100644
> --- a/drivers/usb/typec/ucsi/Makefile
> +++ b/drivers/usb/typec/ucsi/Makefile
> @@ -22,3 +22,4 @@ obj-$(CONFIG_UCSI_CCG)			+= ucsi_ccg.o
>  obj-$(CONFIG_UCSI_STM32G0)		+= ucsi_stm32g0.o
>  obj-$(CONFIG_UCSI_PMIC_GLINK)		+= ucsi_glink.o
>  obj-$(CONFIG_UCSI_LENOVO_YOGA_C630)	+= ucsi_yoga_c630.o
> +obj-$(CONFIG_UCSI_HUAWEI_GAOKUN)	+= ucsi_huawei_gaokun.o
> diff --git a/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
> new file mode 100644
> index 000000000..84ed0407d
> --- /dev/null
> +++ b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
> @@ -0,0 +1,481 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ucsi-huawei-gaokun - A UCSI driver for HUAWEI Matebook E Go (sc8280xp)
> + *
> + * reference: drivers/usb/typec/ucsi/ucsi_yoga_c630.c
> + *            drivers/usb/typec/ucsi/ucsi_glink.c
> + *            drivers/soc/qcom/pmic_glink_altmode.c
> + *
> + * Copyright (C) 2024 Pengyu Luo <mitltlatltl@gmail.com>
> + */
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/bitops.h>
> +#include <linux/completion.h>
> +#include <linux/container_of.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +#include <linux/of.h>
> +#include <linux/string.h>
> +#include <linux/workqueue_types.h>
> +
> +#include <linux/usb/pd_vdo.h>
> +#include <drm/bridge/aux-bridge.h>
> +
> +#include "ucsi.h"
> +#include <linux/platform_data/huawei-gaokun-ec.h>
> +
> +
> +#define EC_EVENT_UCSI	0x21
> +#define EC_EVENT_USB	0x22
> +
> +#define GAOKUN_CCX_MASK		GENMASK(1, 0)
> +#define GAOKUN_MUX_MASK		GENMASK(3, 2)
> +
> +#define GAOKUN_DPAM_MASK	GENMASK(3, 0)
> +#define GAOKUN_HPD_STATE_MASK	BIT(4)
> +#define GAOKUN_HPD_IRQ_MASK	BIT(5)
> +
> +#define CCX_TO_ORI(ccx) (++ccx % 3)
> +
> +#define GET_IDX(updt) (ffs(updt) - 1)
> +
> +/* Configuration Channel Extension */
> +enum gaokun_ucsi_ccx {
> +	USBC_CCX_NORMAL,
> +	USBC_CCX_REVERSE,
> +	USBC_CCX_NONE,
> +};
> +
> +enum gaokun_ucsi_mux {
> +	USBC_MUX_NONE,
> +	USBC_MUX_USB_2L,
> +	USBC_MUX_DP_4L,
> +	USBC_MUX_USB_DP,
> +};
> +
> +struct gaokun_ucsi_reg {
> +	u8 port_num;
> +	u8 port_updt;
> +	u8 port_data[4];
> +	u8 checksum;
> +	u8 reserved;
> +} __packed;
> +
> +struct gaokun_ucsi_port {
> +	struct completion usb_ack;
> +	spinlock_t lock;
> +
> +	struct gaokun_ucsi *ucsi;
> +	struct auxiliary_device *bridge;
> +
> +	int idx;
> +	enum gaokun_ucsi_ccx ccx;
> +	enum gaokun_ucsi_mux mux;
> +	u8 mode;
> +	u16 svid;
> +	u8 hpd_state;
> +	u8 hpd_irq;
> +};
> +
> +struct gaokun_ucsi {
> +	struct gaokun_ec *ec;
> +	struct ucsi *ucsi;
> +	struct gaokun_ucsi_port *ports;
> +	struct device *dev;
> +	struct work_struct work;
> +	struct notifier_block nb;
> +	u16 version;
> +	u8 port_num;
> +};
> +
> +/* -------------------------------------------------------------------------- */
> +/* For UCSI */
> +
> +static int gaokun_ucsi_read_version(struct ucsi *ucsi, u16 *version)
> +{
> +	struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> +
> +	*version = uec->version;
> +
> +	return 0;
> +}
> +
> +static int gaokun_ucsi_read_cci(struct ucsi *ucsi, u32 *cci)
> +{
> +	struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> +	u8 buf[GAOKUN_UCSI_READ_SIZE];
> +	int ret;
> +
> +	ret = gaokun_ec_ucsi_read(uec->ec, buf);
> +	if (ret)
> +		return ret;
> +
> +	memcpy(cci, buf, sizeof(*cci));
> +
> +	return 0;
> +}
> +
> +static int gaokun_ucsi_read_message_in(struct ucsi *ucsi,
> +				       void *val, size_t val_len)
> +{
> +	struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> +	u8 buf[GAOKUN_UCSI_READ_SIZE];
> +	int ret;
> +
> +	ret = gaokun_ec_ucsi_read(uec->ec, buf);
> +	if (ret)
> +		return ret;
> +
> +	memcpy(val, buf + GAOKUN_UCSI_CCI_SIZE,
> +	       min(val_len, GAOKUN_UCSI_DATA_SIZE));
> +
> +	return 0;
> +}
> +
> +static int gaokun_ucsi_async_control(struct ucsi *ucsi, u64 command)
> +{
> +	struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> +	u8 buf[GAOKUN_UCSI_WRITE_SIZE] = {};
> +
> +	memcpy(buf, &command, sizeof(command));
> +
> +	return gaokun_ec_ucsi_write(uec->ec, buf);
> +}
> +
> +static void gaokun_ucsi_update_connector(struct ucsi_connector *con)
> +{
> +	struct gaokun_ucsi *uec = ucsi_get_drvdata(con->ucsi);
> +
> +	if (con->num > uec->port_num)
> +		return;
> +
> +	con->typec_cap.orientation_aware = true;
> +}
> +
> +static void gaokun_set_orientation(struct ucsi_connector *con,
> +				   struct gaokun_ucsi_port *port)
> +{
> +	enum gaokun_ucsi_ccx ccx;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&port->lock, flags);
> +	ccx = port->ccx;
> +	spin_unlock_irqrestore(&port->lock, flags);
> +
> +	typec_set_orientation(con->port, CCX_TO_ORI(ccx));
> +}
> +
> +static void gaokun_ucsi_connector_status(struct ucsi_connector *con)
> +{
> +	struct gaokun_ucsi *uec = ucsi_get_drvdata(con->ucsi);
> +	int idx;
> +
> +	idx = con->num - 1;
> +	if (con->num > uec->port_num) {
> +		dev_warn(uec->ucsi->dev, "set orientation out of range: con%d\n", idx);
> +		return;
> +	}
> +
> +	gaokun_set_orientation(con, &uec->ports[idx]);
> +}
> +
> +const struct ucsi_operations gaokun_ucsi_ops = {
> +	.read_version = gaokun_ucsi_read_version,
> +	.read_cci = gaokun_ucsi_read_cci,
> +	.read_message_in = gaokun_ucsi_read_message_in,
> +	.sync_control = ucsi_sync_control_common,
> +	.async_control = gaokun_ucsi_async_control,
> +	.update_connector = gaokun_ucsi_update_connector,
> +	.connector_status = gaokun_ucsi_connector_status,
> +};
> +
> +/* -------------------------------------------------------------------------- */
> +/* For Altmode */
> +
> +static void gaokun_ucsi_port_update(struct gaokun_ucsi_port *port,
> +				    const u8 *port_data)
> +{
> +	unsigned long flags;
> +	u8 dcc, ddi;
> +	int offset = port->idx * 2; /* every port has 2 Bytes data */
> +
> +	dcc = port_data[offset];
> +	ddi = port_data[offset + 1];

What is dcc and ddi? Are those just names from the DSDT?

> +
> +	spin_lock_irqsave(&port->lock, flags);
> +
> +	port->ccx = FIELD_GET(GAOKUN_CCX_MASK, dcc);
> +	port->mux = FIELD_GET(GAOKUN_MUX_MASK, dcc);
> +	port->mode = FIELD_GET(GAOKUN_DPAM_MASK, ddi);
> +	port->hpd_state = FIELD_GET(GAOKUN_HPD_STATE_MASK, ddi);
> +	port->hpd_irq = FIELD_GET(GAOKUN_HPD_IRQ_MASK, ddi);
> +
> +	switch (port->mux) {
> +	case USBC_MUX_NONE:
> +		port->svid = 0;
> +		break;
> +	case USBC_MUX_USB_2L:
> +		port->svid = USB_SID_PD;
> +		break;
> +	case USBC_MUX_DP_4L:
> +	case USBC_MUX_USB_DP:
> +		port->svid = USB_SID_DISPLAYPORT;
> +		if (port->ccx == USBC_CCX_REVERSE)
> +			port->mode -= 6;

I'd prefer it this were more explicit about what is happening.

> +		break;
> +	default:
> +		break;
> +	}
> +
> +	spin_unlock_irqrestore(&port->lock, flags);
> +}
> +
> +static int gaokun_ucsi_refresh(struct gaokun_ucsi *uec)
> +{
> +	struct gaokun_ucsi_reg ureg;
> +	int ret, idx;
> +
> +	ret = gaokun_ec_ucsi_get_reg(uec->ec, (u8 *)&ureg);
> +	if (ret)
> +		return -EIO;
> +
> +	uec->port_num = ureg.port_num;
> +	idx = GET_IDX(ureg.port_updt);
> +
> +	if (idx >= 0 && idx < ureg.port_num)
> +		gaokun_ucsi_port_update(&uec->ports[idx], ureg.port_data);
> +
> +	return idx;
> +}
> +
> +static void gaokun_ucsi_handle_altmode(struct gaokun_ucsi_port *port)
> +{
> +	struct gaokun_ucsi *uec = port->ucsi;
> +	int idx = port->idx;
> +
> +	if (idx >= uec->ucsi->cap.num_connectors || !uec->ucsi->connector) {
> +		dev_warn(uec->ucsi->dev, "altmode port out of range: %d\n", idx);
> +		return;
> +	}
> +
> +	/* UCSI callback .connector_status() have set orientation */
> +	if (port->bridge)
> +		drm_aux_hpd_bridge_notify(&port->bridge->dev,
> +					  port->hpd_state ?
> +					  connector_status_connected :
> +					  connector_status_disconnected);

Does your platform report any altmodes? What do you see in
/sys/class/typec/port0/port0.*/ ?

> +
> +	gaokun_ec_ucsi_pan_ack(uec->ec, port->idx);
> +}
> +
> +static void gaokun_ucsi_altmode_notify_ind(struct gaokun_ucsi *uec)
> +{
> +	int idx;
> +
> +	idx = gaokun_ucsi_refresh(uec);
> +	if (idx < 0)
> +		gaokun_ec_ucsi_pan_ack(uec->ec, idx);
> +	else
> +		gaokun_ucsi_handle_altmode(&uec->ports[idx]);
> +}
> +
> +/*
> + * USB event is necessary for enabling altmode, the event should follow
> + * UCSI event, if not after timeout(this notify may be disabled somehow),
> + * then force to enable altmode.
> + */
> +static void gaokun_ucsi_handle_no_usb_event(struct gaokun_ucsi *uec, int idx)
> +{
> +	struct gaokun_ucsi_port *port;
> +
> +	port = &uec->ports[idx];
> +	if (!wait_for_completion_timeout(&port->usb_ack, 2 * HZ)) {
> +		dev_warn(uec->dev, "No USB EVENT, triggered by UCSI EVENT");
> +		gaokun_ucsi_altmode_notify_ind(uec);
> +	}
> +}
> +
> +static int gaokun_ucsi_notify(struct notifier_block *nb,
> +			      unsigned long action, void *data)
> +{
> +	u32 cci;
> +	struct gaokun_ucsi *uec = container_of(nb, struct gaokun_ucsi, nb);
> +
> +	switch (action) {
> +	case EC_EVENT_USB:
> +		gaokun_ucsi_altmode_notify_ind(uec);
> +		return NOTIFY_OK;
> +
> +	case EC_EVENT_UCSI:
> +		uec->ucsi->ops->read_cci(uec->ucsi, &cci);
> +		ucsi_notify_common(uec->ucsi, cci);
> +		if (UCSI_CCI_CONNECTOR(cci))
> +			gaokun_ucsi_handle_no_usb_event(uec, UCSI_CCI_CONNECTOR(cci) - 1);
> +
> +		return NOTIFY_OK;
> +
> +	default:
> +		return NOTIFY_DONE;
> +	}
> +}
> +
> +static int gaokun_ucsi_get_port_num(struct gaokun_ucsi *uec)
> +{
> +	struct gaokun_ucsi_reg ureg;
> +	int ret;
> +
> +	ret = gaokun_ec_ucsi_get_reg(uec->ec, (u8 *)&ureg);
> +
> +	return ret ? 0 : ureg.port_num;
> +}
> +
> +static int gaokun_ucsi_ports_init(struct gaokun_ucsi *uec)
> +{
> +	u32 port;
> +	int i, ret, port_num;
> +	struct device *dev = uec->dev;
> +	struct gaokun_ucsi_port *ucsi_port;
> +	struct fwnode_handle *fwnode;
> +
> +	port_num = gaokun_ucsi_get_port_num(uec);
> +	uec->port_num = port_num;
> +
> +	uec->ports = devm_kzalloc(dev, port_num * sizeof(*(uec->ports)),
> +				  GFP_KERNEL);
> +	if (!uec->ports)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < port_num; ++i) {
> +		ucsi_port = &uec->ports[i];
> +		ucsi_port->ccx = USBC_CCX_NONE;
> +		ucsi_port->idx = i;
> +		ucsi_port->ucsi = uec;
> +		init_completion(&ucsi_port->usb_ack);
> +		spin_lock_init(&ucsi_port->lock);
> +	}
> +
> +	device_for_each_child_node(dev, fwnode) {
> +		ret = fwnode_property_read_u32(fwnode, "reg", &port);
> +		if (ret < 0) {
> +			dev_err(dev, "missing reg property of %pOFn\n", fwnode);
> +			fwnode_handle_put(fwnode);
> +			return ret;
> +		}
> +
> +		if (port >= port_num) {
> +			dev_warn(dev, "invalid connector number %d, ignoring\n", port);
> +			continue;
> +		}
> +
> +		ucsi_port = &uec->ports[port];
> +		ucsi_port->bridge = devm_drm_dp_hpd_bridge_alloc(dev, to_of_node(fwnode));
> +		if (IS_ERR(ucsi_port->bridge)) {
> +			fwnode_handle_put(fwnode);
> +			return PTR_ERR(ucsi_port->bridge);
> +		}
> +	}
> +
> +	for (i = 0; i < port_num; i++) {
> +		if (!uec->ports[i].bridge)
> +			continue;
> +
> +		ret = devm_drm_dp_hpd_bridge_add(dev, uec->ports[i].bridge);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void gaokun_ucsi_register_worker(struct work_struct *work)
> +{
> +	struct gaokun_ucsi *uec;
> +	struct ucsi *ucsi;
> +	int ret;
> +
> +	uec = container_of(work, struct gaokun_ucsi, work);
> +	ucsi = uec->ucsi;
> +
> +	ucsi->quirks = UCSI_NO_PARTNER_PDOS | UCSI_DELAY_DEVICE_PDOS;

Does it crash in the same way as GLINK crashes (as you've set
UCSI_NO_PARTNER_PDOS)?

> +
> +	ssleep(3); /* EC can't handle UCSI properly in the early stage */
> +
> +	ret = gaokun_ec_register_notify(uec->ec, &uec->nb);
> +	if (ret) {
> +		dev_err_probe(ucsi->dev, ret, "notifier register failed\n");
> +		return;
> +	}
> +
> +	ret = ucsi_register(ucsi);
> +	if (ret)
> +		dev_err_probe(ucsi->dev, ret, "ucsi register failed\n");
> +}
> +
> +static int gaokun_ucsi_register(struct gaokun_ucsi *uec)

Please inline

> +{
> +	schedule_work(&uec->work);
> +
> +	return 0;
> +}
> +
> +static int gaokun_ucsi_probe(struct auxiliary_device *adev,
> +			     const struct auxiliary_device_id *id)
> +{
> +	struct gaokun_ec *ec = adev->dev.platform_data;
> +	struct device *dev = &adev->dev;
> +	struct gaokun_ucsi *uec;
> +	int ret;
> +
> +	uec = devm_kzalloc(dev, sizeof(*uec), GFP_KERNEL);
> +	if (!uec)
> +		return -ENOMEM;
> +
> +	uec->ec = ec;
> +	uec->dev = dev;
> +	uec->version = 0x0100;
> +	uec->nb.notifier_call = gaokun_ucsi_notify;
> +
> +	INIT_WORK(&uec->work, gaokun_ucsi_register_worker);
> +
> +	ret = gaokun_ucsi_ports_init(uec);
> +	if (ret)
> +		return ret;
> +
> +	uec->ucsi = ucsi_create(dev, &gaokun_ucsi_ops);
> +	if (IS_ERR(uec->ucsi))
> +		return PTR_ERR(uec->ucsi);
> +
> +	ucsi_set_drvdata(uec->ucsi, uec);
> +	auxiliary_set_drvdata(adev, uec);
> +
> +	return gaokun_ucsi_register(uec);
> +}
> +
> +static void gaokun_ucsi_remove(struct auxiliary_device *adev)
> +{
> +	struct gaokun_ucsi *uec = auxiliary_get_drvdata(adev);
> +
> +	gaokun_ec_unregister_notify(uec->ec, &uec->nb);
> +	ucsi_unregister(uec->ucsi);
> +	ucsi_destroy(uec->ucsi);
> +}
> +
> +static const struct auxiliary_device_id gaokun_ucsi_id_table[] = {
> +	{ .name = GAOKUN_MOD_NAME "." GAOKUN_DEV_UCSI, },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(auxiliary, gaokun_ucsi_id_table);
> +
> +static struct auxiliary_driver gaokun_ucsi_driver = {
> +	.name = GAOKUN_DEV_UCSI,
> +	.id_table = gaokun_ucsi_id_table,
> +	.probe = gaokun_ucsi_probe,
> +	.remove = gaokun_ucsi_remove,
> +};
> +
> +module_auxiliary_driver(gaokun_ucsi_driver);
> +
> +MODULE_DESCRIPTION("HUAWEI Matebook E Go UCSI driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.47.1
>
Pengyu Luo Dec. 29, 2024, 9:05 a.m. UTC | #4
On Sun, Dec 29, 2024 at 12:40 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote:
> On Sat, Dec 28, 2024 at 01:13:51AM +0800, Pengyu Luo wrote:
> > The Huawei Matebook E Go (sc8280xp) tablet provides implements UCSI
> > interface in the onboard EC. Add the glue driver to interface the
> > platform's UCSI implementation.
> >
> > Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> > ---
> >  drivers/usb/typec/ucsi/Kconfig              |   9 +
> >  drivers/usb/typec/ucsi/Makefile             |   1 +
> >  drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c | 481 ++++++++++++++++++++
> >  3 files changed, 491 insertions(+)
> >  create mode 100644 drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
> >
> > diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
> > index 680e1b87b..0d0f07488 100644
> > --- a/drivers/usb/typec/ucsi/Kconfig
> > +++ b/drivers/usb/typec/ucsi/Kconfig
> > @@ -78,4 +78,13 @@ config UCSI_LENOVO_YOGA_C630
> >         To compile the driver as a module, choose M here: the module will be
> >         called ucsi_yoga_c630.
> >
> > +config UCSI_HUAWEI_GAOKUN
> > +     tristate "UCSI Interface Driver for Huawei Matebook E Go (sc8280xp)"
> > +     depends on EC_HUAWEI_GAOKUN
> > +     help
> > +       This driver enables UCSI support on the Huawei Matebook E Go tablet.
> > +
> > +       To compile the driver as a module, choose M here: the module will be
> > +       called ucsi_huawei_gaokun.
> > +
> >  endif
> > diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
> > index aed41d238..0b400122b 100644
> > --- a/drivers/usb/typec/ucsi/Makefile
> > +++ b/drivers/usb/typec/ucsi/Makefile
> > @@ -22,3 +22,4 @@ obj-$(CONFIG_UCSI_CCG)                      += ucsi_ccg.o
> >  obj-$(CONFIG_UCSI_STM32G0)           += ucsi_stm32g0.o
> >  obj-$(CONFIG_UCSI_PMIC_GLINK)                += ucsi_glink.o
> >  obj-$(CONFIG_UCSI_LENOVO_YOGA_C630)  += ucsi_yoga_c630.o
> > +obj-$(CONFIG_UCSI_HUAWEI_GAOKUN)     += ucsi_huawei_gaokun.o
> > diff --git a/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
> > new file mode 100644
> > index 000000000..84ed0407d
> > --- /dev/null
> > +++ b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
> > @@ -0,0 +1,481 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * ucsi-huawei-gaokun - A UCSI driver for HUAWEI Matebook E Go (sc8280xp)
> > + *
> > + * reference: drivers/usb/typec/ucsi/ucsi_yoga_c630.c
> > + *            drivers/usb/typec/ucsi/ucsi_glink.c
> > + *            drivers/soc/qcom/pmic_glink_altmode.c
> > + *
> > + * Copyright (C) 2024 Pengyu Luo <mitltlatltl@gmail.com>
> > + */
> > +
> > +#include <linux/auxiliary_bus.h>
> > +#include <linux/bitops.h>
> > +#include <linux/completion.h>
> > +#include <linux/container_of.h>
> > +#include <linux/delay.h>
> > +#include <linux/module.h>
> > +#include <linux/notifier.h>
> > +#include <linux/of.h>
> > +#include <linux/string.h>
> > +#include <linux/workqueue_types.h>
> > +
> > +#include <linux/usb/pd_vdo.h>
> > +#include <drm/bridge/aux-bridge.h>
> > +
> > +#include "ucsi.h"
> > +#include <linux/platform_data/huawei-gaokun-ec.h>
> > +
> > +
> > +#define EC_EVENT_UCSI        0x21
> > +#define EC_EVENT_USB 0x22
> > +
> > +#define GAOKUN_CCX_MASK              GENMASK(1, 0)
> > +#define GAOKUN_MUX_MASK              GENMASK(3, 2)
> > +
> > +#define GAOKUN_DPAM_MASK     GENMASK(3, 0)
> > +#define GAOKUN_HPD_STATE_MASK        BIT(4)
> > +#define GAOKUN_HPD_IRQ_MASK  BIT(5)
> > +
> > +#define CCX_TO_ORI(ccx) (++ccx % 3)
> > +
> > +#define GET_IDX(updt) (ffs(updt) - 1)
> > +
> > +/* Configuration Channel Extension */
> > +enum gaokun_ucsi_ccx {
> > +     USBC_CCX_NORMAL,
> > +     USBC_CCX_REVERSE,
> > +     USBC_CCX_NONE,
> > +};
> > +
> > +enum gaokun_ucsi_mux {
> > +     USBC_MUX_NONE,
> > +     USBC_MUX_USB_2L,
> > +     USBC_MUX_DP_4L,
> > +     USBC_MUX_USB_DP,
> > +};
> > +
> > +struct gaokun_ucsi_reg {
> > +     u8 port_num;
> > +     u8 port_updt;
> > +     u8 port_data[4];
> > +     u8 checksum;
> > +     u8 reserved;
> > +} __packed;
> > +
> > +struct gaokun_ucsi_port {
> > +     struct completion usb_ack;
> > +     spinlock_t lock;
> > +
> > +     struct gaokun_ucsi *ucsi;
> > +     struct auxiliary_device *bridge;
> > +
> > +     int idx;
> > +     enum gaokun_ucsi_ccx ccx;
> > +     enum gaokun_ucsi_mux mux;
> > +     u8 mode;
> > +     u16 svid;
> > +     u8 hpd_state;
> > +     u8 hpd_irq;
> > +};
> > +
> > +struct gaokun_ucsi {
> > +     struct gaokun_ec *ec;
> > +     struct ucsi *ucsi;
> > +     struct gaokun_ucsi_port *ports;
> > +     struct device *dev;
> > +     struct work_struct work;
> > +     struct notifier_block nb;
> > +     u16 version;
> > +     u8 port_num;
> > +};
> > +
> > +/* -------------------------------------------------------------------------- */
> > +/* For UCSI */
> > +
> > +static int gaokun_ucsi_read_version(struct ucsi *ucsi, u16 *version)
> > +{
> > +     struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> > +
> > +     *version = uec->version;
> > +
> > +     return 0;
> > +}
> > +
> > +static int gaokun_ucsi_read_cci(struct ucsi *ucsi, u32 *cci)
> > +{
> > +     struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> > +     u8 buf[GAOKUN_UCSI_READ_SIZE];
> > +     int ret;
> > +
> > +     ret = gaokun_ec_ucsi_read(uec->ec, buf);
> > +     if (ret)
> > +             return ret;
> > +
> > +     memcpy(cci, buf, sizeof(*cci));
> > +
> > +     return 0;
> > +}
> > +
> > +static int gaokun_ucsi_read_message_in(struct ucsi *ucsi,
> > +                                    void *val, size_t val_len)
> > +{
> > +     struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> > +     u8 buf[GAOKUN_UCSI_READ_SIZE];
> > +     int ret;
> > +
> > +     ret = gaokun_ec_ucsi_read(uec->ec, buf);
> > +     if (ret)
> > +             return ret;
> > +
> > +     memcpy(val, buf + GAOKUN_UCSI_CCI_SIZE,
> > +            min(val_len, GAOKUN_UCSI_DATA_SIZE));
> > +
> > +     return 0;
> > +}
> > +
> > +static int gaokun_ucsi_async_control(struct ucsi *ucsi, u64 command)
> > +{
> > +     struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
> > +     u8 buf[GAOKUN_UCSI_WRITE_SIZE] = {};
> > +
> > +     memcpy(buf, &command, sizeof(command));
> > +
> > +     return gaokun_ec_ucsi_write(uec->ec, buf);
> > +}
> > +
> > +static void gaokun_ucsi_update_connector(struct ucsi_connector *con)
> > +{
> > +     struct gaokun_ucsi *uec = ucsi_get_drvdata(con->ucsi);
> > +
> > +     if (con->num > uec->port_num)
> > +             return;
> > +
> > +     con->typec_cap.orientation_aware = true;
> > +}
> > +
> > +static void gaokun_set_orientation(struct ucsi_connector *con,
> > +                                struct gaokun_ucsi_port *port)
> > +{
> > +     enum gaokun_ucsi_ccx ccx;
> > +     unsigned long flags;
> > +
> > +     spin_lock_irqsave(&port->lock, flags);
> > +     ccx = port->ccx;
> > +     spin_unlock_irqrestore(&port->lock, flags);
> > +
> > +     typec_set_orientation(con->port, CCX_TO_ORI(ccx));
> > +}
> > +
> > +static void gaokun_ucsi_connector_status(struct ucsi_connector *con)
> > +{
> > +     struct gaokun_ucsi *uec = ucsi_get_drvdata(con->ucsi);
> > +     int idx;
> > +
> > +     idx = con->num - 1;
> > +     if (con->num > uec->port_num) {
> > +             dev_warn(uec->ucsi->dev, "set orientation out of range: con%d\n", idx);
> > +             return;
> > +     }
> > +
> > +     gaokun_set_orientation(con, &uec->ports[idx]);
> > +}
> > +
> > +const struct ucsi_operations gaokun_ucsi_ops = {
> > +     .read_version = gaokun_ucsi_read_version,
> > +     .read_cci = gaokun_ucsi_read_cci,
> > +     .read_message_in = gaokun_ucsi_read_message_in,
> > +     .sync_control = ucsi_sync_control_common,
> > +     .async_control = gaokun_ucsi_async_control,
> > +     .update_connector = gaokun_ucsi_update_connector,
> > +     .connector_status = gaokun_ucsi_connector_status,
> > +};
> > +
> > +/* -------------------------------------------------------------------------- */
> > +/* For Altmode */
> > +
> > +static void gaokun_ucsi_port_update(struct gaokun_ucsi_port *port,
> > +                                 const u8 *port_data)
> > +{
> > +     unsigned long flags;
> > +     u8 dcc, ddi;
> > +     int offset = port->idx * 2; /* every port has 2 Bytes data */
> > +
> > +     dcc = port_data[offset];
> > +     ddi = port_data[offset + 1];
>
> What is dcc and ddi? Are those just names from the DSDT?
>

Yes, DSDT's inventions. Huawei one uses that.

Some additional information, you can check the following in sc8280xp or
xelite based dsdt.

In UPAN(usbc pinassignment notification), PBUF carries a pan info, which
is a 8B data, {BPID, BORI, BMUX, BVID(2B), BSID(2B), BSSD} which stands for
port_id, orientation of port, mux state, USB-IF vendor id, USB-IF standard id,
I don't know the BSSD, (if linaro know something?)
but according to drivers/soc/qcom/pmic_glink_altmode.c
BSSD is related to pin assignment(mode field), hpd_state, hpd_irq, ddi is
something equivalent to BSSD. dcc is something equivalent to BORI and BMUX.


> > +
> > +     spin_lock_irqsave(&port->lock, flags);
> > +
> > +     port->ccx = FIELD_GET(GAOKUN_CCX_MASK, dcc);
> > +     port->mux = FIELD_GET(GAOKUN_MUX_MASK, dcc);
> > +     port->mode = FIELD_GET(GAOKUN_DPAM_MASK, ddi);
> > +     port->hpd_state = FIELD_GET(GAOKUN_HPD_STATE_MASK, ddi);
> > +     port->hpd_irq = FIELD_GET(GAOKUN_HPD_IRQ_MASK, ddi);
> > +
> > +     switch (port->mux) {
> > +     case USBC_MUX_NONE:
> > +             port->svid = 0;
> > +             break;
> > +     case USBC_MUX_USB_2L:
> > +             port->svid = USB_SID_PD;
> > +             break;
> > +     case USBC_MUX_DP_4L:
> > +     case USBC_MUX_USB_DP:
> > +             port->svid = USB_SID_DISPLAYPORT;
> > +             if (port->ccx == USBC_CCX_REVERSE)
> > +                     port->mode -= 6;
>
> I'd prefer it this were more explicit about what is happening.
>

If orientation is reverse, then we should minus 6, EC's logic.
I will add a comment for it. Actually, this field is unused, I don't
find the mux yet, so I cannot set it with this field. But I don't want
to make things imcomplete, so keep it.

Let me go off the topic, on my device, I can just use drm_aux_hpd_bridge_notify
to enable altmode, usb functions well after I pluged out, I don't need set mode
switch(orientation switch is required if orientation is reverse), which is quiet
similar to Acer aspire 1. Is mux controlled also by QMP combo phy(see [1])?

> > +             break;
> > +     default:
> > +             break;
> > +     }
> > +
> > +     spin_unlock_irqrestore(&port->lock, flags);
> > +}
> > +
> > +static int gaokun_ucsi_refresh(struct gaokun_ucsi *uec)
> > +{
> > +     struct gaokun_ucsi_reg ureg;
> > +     int ret, idx;
> > +
> > +     ret = gaokun_ec_ucsi_get_reg(uec->ec, (u8 *)&ureg);
> > +     if (ret)
> > +             return -EIO;
> > +
> > +     uec->port_num = ureg.port_num;
> > +     idx = GET_IDX(ureg.port_updt);
> > +
> > +     if (idx >= 0 && idx < ureg.port_num)
> > +             gaokun_ucsi_port_update(&uec->ports[idx], ureg.port_data);
> > +
> > +     return idx;
> > +}
> > +
> > +static void gaokun_ucsi_handle_altmode(struct gaokun_ucsi_port *port)
> > +{
> > +     struct gaokun_ucsi *uec = port->ucsi;
> > +     int idx = port->idx;
> > +
> > +     if (idx >= uec->ucsi->cap.num_connectors || !uec->ucsi->connector) {
> > +             dev_warn(uec->ucsi->dev, "altmode port out of range: %d\n", idx);
> > +             return;
> > +     }
> > +
> > +     /* UCSI callback .connector_status() have set orientation */
> > +     if (port->bridge)
> > +             drm_aux_hpd_bridge_notify(&port->bridge->dev,
> > +                                       port->hpd_state ?
> > +                                       connector_status_connected :
> > +                                       connector_status_disconnected);
>
> Does your platform report any altmodes? What do you see in
> /sys/class/typec/port0/port0.*/ ?
>

/sys/class/typec/port0/port0.0:
active  mode  mode1  power  svid  uevent  vdo

/sys/class/typec/port0/port0.1:
active  mode  mode1  power  svid  uevent  vdo

/sys/class/typec/port0/port0.2:
active  mode  mode1  power  svid  uevent  vdo

/sys/class/typec/port0/port0.3:
active  mode  mode2  power  svid  uevent  vdo

/sys/class/typec/port0/port0.4:
active  mode  mode3  power  svid  uevent  vdo


> > +
> > +     gaokun_ec_ucsi_pan_ack(uec->ec, port->idx);
> > +}
> > +
> > +static void gaokun_ucsi_altmode_notify_ind(struct gaokun_ucsi *uec)
> > +{
> > +     int idx;
> > +
> > +     idx = gaokun_ucsi_refresh(uec);
> > +     if (idx < 0)
> > +             gaokun_ec_ucsi_pan_ack(uec->ec, idx);
> > +     else
> > +             gaokun_ucsi_handle_altmode(&uec->ports[idx]);
> > +}
> > +
> > +/*
> > + * USB event is necessary for enabling altmode, the event should follow
> > + * UCSI event, if not after timeout(this notify may be disabled somehow),
> > + * then force to enable altmode.
> > + */
> > +static void gaokun_ucsi_handle_no_usb_event(struct gaokun_ucsi *uec, int idx)
> > +{
> > +     struct gaokun_ucsi_port *port;
> > +
> > +     port = &uec->ports[idx];
> > +     if (!wait_for_completion_timeout(&port->usb_ack, 2 * HZ)) {
> > +             dev_warn(uec->dev, "No USB EVENT, triggered by UCSI EVENT");
> > +             gaokun_ucsi_altmode_notify_ind(uec);
> > +     }
> > +}
> > +
> > +static int gaokun_ucsi_notify(struct notifier_block *nb,
> > +                           unsigned long action, void *data)
> > +{
> > +     u32 cci;
> > +     struct gaokun_ucsi *uec = container_of(nb, struct gaokun_ucsi, nb);
> > +
> > +     switch (action) {
> > +     case EC_EVENT_USB:
> > +             gaokun_ucsi_altmode_notify_ind(uec);
> > +             return NOTIFY_OK;
> > +
> > +     case EC_EVENT_UCSI:
> > +             uec->ucsi->ops->read_cci(uec->ucsi, &cci);
> > +             ucsi_notify_common(uec->ucsi, cci);
> > +             if (UCSI_CCI_CONNECTOR(cci))
> > +                     gaokun_ucsi_handle_no_usb_event(uec, UCSI_CCI_CONNECTOR(cci) - 1);
> > +
> > +             return NOTIFY_OK;
> > +
> > +     default:
> > +             return NOTIFY_DONE;
> > +     }
> > +}
> > +
> > +static int gaokun_ucsi_get_port_num(struct gaokun_ucsi *uec)
> > +{
> > +     struct gaokun_ucsi_reg ureg;
> > +     int ret;
> > +
> > +     ret = gaokun_ec_ucsi_get_reg(uec->ec, (u8 *)&ureg);
> > +
> > +     return ret ? 0 : ureg.port_num;
> > +}
> > +
> > +static int gaokun_ucsi_ports_init(struct gaokun_ucsi *uec)
> > +{
> > +     u32 port;
> > +     int i, ret, port_num;
> > +     struct device *dev = uec->dev;
> > +     struct gaokun_ucsi_port *ucsi_port;
> > +     struct fwnode_handle *fwnode;
> > +
> > +     port_num = gaokun_ucsi_get_port_num(uec);
> > +     uec->port_num = port_num;
> > +
> > +     uec->ports = devm_kzalloc(dev, port_num * sizeof(*(uec->ports)),
> > +                               GFP_KERNEL);
> > +     if (!uec->ports)
> > +             return -ENOMEM;
> > +
> > +     for (i = 0; i < port_num; ++i) {
> > +             ucsi_port = &uec->ports[i];
> > +             ucsi_port->ccx = USBC_CCX_NONE;
> > +             ucsi_port->idx = i;
> > +             ucsi_port->ucsi = uec;
> > +             init_completion(&ucsi_port->usb_ack);
> > +             spin_lock_init(&ucsi_port->lock);
> > +     }
> > +
> > +     device_for_each_child_node(dev, fwnode) {
> > +             ret = fwnode_property_read_u32(fwnode, "reg", &port);
> > +             if (ret < 0) {
> > +                     dev_err(dev, "missing reg property of %pOFn\n", fwnode);
> > +                     fwnode_handle_put(fwnode);
> > +                     return ret;
> > +             }
> > +
> > +             if (port >= port_num) {
> > +                     dev_warn(dev, "invalid connector number %d, ignoring\n", port);
> > +                     continue;
> > +             }
> > +
> > +             ucsi_port = &uec->ports[port];
> > +             ucsi_port->bridge = devm_drm_dp_hpd_bridge_alloc(dev, to_of_node(fwnode));
> > +             if (IS_ERR(ucsi_port->bridge)) {
> > +                     fwnode_handle_put(fwnode);
> > +                     return PTR_ERR(ucsi_port->bridge);
> > +             }
> > +     }
> > +
> > +     for (i = 0; i < port_num; i++) {
> > +             if (!uec->ports[i].bridge)
> > +                     continue;
> > +
> > +             ret = devm_drm_dp_hpd_bridge_add(dev, uec->ports[i].bridge);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static void gaokun_ucsi_register_worker(struct work_struct *work)
> > +{
> > +     struct gaokun_ucsi *uec;
> > +     struct ucsi *ucsi;
> > +     int ret;
> > +
> > +     uec = container_of(work, struct gaokun_ucsi, work);
> > +     ucsi = uec->ucsi;
> > +
> > +     ucsi->quirks = UCSI_NO_PARTNER_PDOS | UCSI_DELAY_DEVICE_PDOS;
>
> Does it crash in the same way as GLINK crashes (as you've set
> UCSI_NO_PARTNER_PDOS)?
>

Yes, no partner can be detected, I checked. I think it is also handled by
the firmware As you said in [2]
> In some obscure cases (Qualcomm PMIC Glink) altmode is completely
> handled by the firmware. Linux does not get proper partner altmode info.

> > +
> > +     ssleep(3); /* EC can't handle UCSI properly in the early stage */
> > +
> > +     ret = gaokun_ec_register_notify(uec->ec, &uec->nb);
> > +     if (ret) {
> > +             dev_err_probe(ucsi->dev, ret, "notifier register failed\n");
> > +             return;
> > +     }
> > +
> > +     ret = ucsi_register(ucsi);
> > +     if (ret)
> > +             dev_err_probe(ucsi->dev, ret, "ucsi register failed\n");
> > +}
> > +
> > +static int gaokun_ucsi_register(struct gaokun_ucsi *uec)
>
> Please inline
>

I see.

Best wishes
Pengyu

[1] https://elixir.bootlin.com/linux/v6.12.5/source/drivers/phy/qualcomm/phy-qcom-qmp-combo.c#L2679
[2] https://lore.kernel.org/lkml/20240416-ucsi-glink-altmode-v1-0-890db00877ac@linaro.org
Bryan O'Donoghue Dec. 29, 2024, 2:51 p.m. UTC | #5
On 28/12/2024 14:38, Pengyu Luo wrote:
> On Sat, Dec 28, 2024 at 9:06 PM Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
>> On 27/12/2024 17:13, Pengyu Luo wrote:
>>> The Huawei Matebook E Go (sc8280xp) tablet provides implements UCSI
>>> interface in the onboard EC. Add the glue driver to interface the
>>> platform's UCSI implementation.
>>>
>>> Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
>>> ---
>>>    drivers/usb/typec/ucsi/Kconfig              |   9 +
>>>    drivers/usb/typec/ucsi/Makefile             |   1 +
>>>    drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c | 481 ++++++++++++++++++++
>>>    3 files changed, 491 insertions(+)
>>>    create mode 100644 drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
>>>
>>> diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
>>> index 680e1b87b..0d0f07488 100644
>>> --- a/drivers/usb/typec/ucsi/Kconfig
>>> +++ b/drivers/usb/typec/ucsi/Kconfig
>>> @@ -78,4 +78,13 @@ config UCSI_LENOVO_YOGA_C630
>>>          To compile the driver as a module, choose M here: the module will be
>>>          called ucsi_yoga_c630.
>>>
>>> +config UCSI_HUAWEI_GAOKUN
>>> +     tristate "UCSI Interface Driver for Huawei Matebook E Go (sc8280xp)"
>>> +     depends on EC_HUAWEI_GAOKUN
>>> +     help
>>> +       This driver enables UCSI support on the Huawei Matebook E Go tablet.
>>> +
>>> +       To compile the driver as a module, choose M here: the module will be
>>> +       called ucsi_huawei_gaokun.
>>> +
>>>    endif
>>> diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
>>> index aed41d238..0b400122b 100644
>>> --- a/drivers/usb/typec/ucsi/Makefile
>>> +++ b/drivers/usb/typec/ucsi/Makefile
>>> @@ -22,3 +22,4 @@ obj-$(CONFIG_UCSI_CCG)                      += ucsi_ccg.o
>>>    obj-$(CONFIG_UCSI_STM32G0)          += ucsi_stm32g0.o
>>>    obj-$(CONFIG_UCSI_PMIC_GLINK)               += ucsi_glink.o
>>>    obj-$(CONFIG_UCSI_LENOVO_YOGA_C630) += ucsi_yoga_c630.o
>>> +obj-$(CONFIG_UCSI_HUAWEI_GAOKUN)     += ucsi_huawei_gaokun.o
>>> diff --git a/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
>>> new file mode 100644
>>> index 000000000..84ed0407d
>>> --- /dev/null
>>> +++ b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
>>> @@ -0,0 +1,481 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * ucsi-huawei-gaokun - A UCSI driver for HUAWEI Matebook E Go (sc8280xp)
>>> + *
>>> + * reference: drivers/usb/typec/ucsi/ucsi_yoga_c630.c
>>> + *            drivers/usb/typec/ucsi/ucsi_glink.c
>>> + *            drivers/soc/qcom/pmic_glink_altmode.c
>>> + *
>>> + * Copyright (C) 2024 Pengyu Luo <mitltlatltl@gmail.com>
>>> + */
>>> +
>>> +#include <linux/auxiliary_bus.h>
>>> +#include <linux/bitops.h>
>>> +#include <linux/completion.h>
>>> +#include <linux/container_of.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/module.h>
>>> +#include <linux/notifier.h>
>>> +#include <linux/of.h>
>>> +#include <linux/string.h>
>>> +#include <linux/workqueue_types.h>
>>> +
>>> +#include <linux/usb/pd_vdo.h>
>>> +#include <drm/bridge/aux-bridge.h
>>
>> Is there a reason you don't have strict include alphanumeric ordering here ?
>>
> 
> These two is dp/alt mode related, so listing them out. Above of them are
> general things.

OK. Unless there's an include dependency reason you need to, please just 
sort the headers alphanumerically in order

#include <globals_first>
#include <globals_first_alpha>

#include "locals_next"
#include "locals_next_alpha_also"

>>>
>>> +
>>> +#include "ucsi.h"
>>> +#include <linux/platform_data/huawei-gaokun-ec.h>
>>> +
>>> +
>>> +#define EC_EVENT_UCSI        0x21
>>> +#define EC_EVENT_USB 0x22
>>> +
>>> +#define GAOKUN_CCX_MASK              GENMASK(1, 0)
>>> +#define GAOKUN_MUX_MASK              GENMASK(3, 2)
>>> +
>>> +#define GAOKUN_DPAM_MASK     GENMASK(3, 0)
>>> +#define GAOKUN_HPD_STATE_MASK        BIT(4)
>>> +#define GAOKUN_HPD_IRQ_MASK  BIT(5)
>>> +
>>> +#define CCX_TO_ORI(ccx) (++ccx % 3)
>>
>> Why do you increment the value of the enum ?
>> Seems strange.
>>
> 
> EC's logic, it is just a trick. Qualcomm maps
> 0 1 2 to normal, reverse, none(no device insert)
> typec lib maps 1 2 0 to that.

I'd suggest making the trick much more obvious.

Either with a comment or just mapping 1:1 between EC and Linux' view of 
this data.

The main reason for that is to make it easier to 
read/understand/maintain/debug.



>>> +             port->svid = USB_SID_DISPLAYPORT;
>>> +             if (port->ccx == USBC_CCX_REVERSE)
>>> +                     port->mode -= 6;
>>
>> why minus six ?
>> needs a comment.
>>
> 
> EC's logic. I don't know why, it is a quirk from Qualcomm or Huawei.
> I will mention this.

Instead of hard-coding a mapping between the EC's mode and Linux' UCSI 
enum - just make a define or an inline, ideally something with

switch(port->mode)
case GOAKUN_MODE_0:
	val = LINUX_UCSI_MODE_X;
case GOAKUN_MODE_1:
	val = LINUX_UCSI_MODE_Y;
}

That will make the mapping obvious and also ensure both to yourself and 
to your reviewers that you have accounted for _all_ of the potential 
mappings and if those mappings don't exist then the default: of your 
switch statement should make some noise about it

dev_err(dev, "GOKUN UCSI mode %d unmapped\n"); or something like that.


> 
>>> +             break;
>>> +     default:
>>> +             break;
>>> +     }
>>> +
>>> +     spin_unlock_irqrestore(&port->lock, flags);
>>> +}
>>> +
>>> +static int gaokun_ucsi_refresh(struct gaokun_ucsi *uec)
>>> +{
>>> +     struct gaokun_ucsi_reg ureg;
>>> +     int ret, idx;
>>> +
>>> +     ret = gaokun_ec_ucsi_get_reg(uec->ec, (u8 *)&ureg);
>>> +     if (ret)
>>> +             return -EIO;
>>> +
>>> +     uec->port_num = ureg.port_num;
>>> +     idx = GET_IDX(ureg.port_updt);
>>> +
>>> +     if (idx >= 0 && idx < ureg.port_num)
>>> +             gaokun_ucsi_port_update(&uec->ports[idx], ureg.port_data);
>>
>> Since you are checking the validity of the index, you should -EINVAL if
>> the index is out of range.
>>
> 
> EC / pmic glink encode every port in a bit
> 0/1/2/4/... => ???/left/right/some port
> 
> I remap it to -1/0/1/2, to access specific port exceptions(-1) are not
> harmful, later in gaokun_ucsi_altmode_notify_ind
> 
> 	if (idx < 0)
> 		gaokun_ec_ucsi_pan_ack(uec->ec, idx);
> 	else
> 		gaokun_ucsi_handle_altmode(&uec->ports[idx]);
> 
> gaokun_ec_ucsi_pan_ack can handle exceptions.
> 

gaokun_ucsi_refresh() can return

-EIO
-1
 >=0

Where -EIO and -1 both trigger gaokun_ec_ucsi_pan_ack() in 
gaokun_ucsi_altmode_notify_ind()

So if the index doesn't matter and < 0 => pan_ack() is OK or -EIO is not 
returning meaningful error.

Either way strongly advise against mixing a negative index as having a 
valid meaning with actual -E error codes...

As a reviewer doing a fist-pass this looks suspicous and implies more 
thought/refinement should be done to the flow.


>>> +
>>> +     ucsi->quirks = UCSI_NO_PARTNER_PDOS | UCSI_DELAY_DEVICE_PDOS;
>>> +
>>> +     ssleep(3); /* EC can't handle UCSI properly in the early stage */
>>
>> Could you not schedule work for + 3 seconds instead of sleeping here -
>> representing the required stall time in some sort of state machine ?
>>
> 
> I see, I will check work schedule interface.
> 
>> 3 seconds is an incredibly long time for a computer to sleep.
>>
> 
> This module will be loaded at about 5th second after power up, if not
> sleep, we will receive something disharmonious, sleeping for 3 seconds is
> a hack.

Yes it is. You could schedule some work to complete three seconds from 
here instead of doing this long sleep here.

In fact you are registering a worker here right ?

In which case its pretty trivial to schedule some work on that worker 
for three seconds hence ..

Please investigate.

---
bod
Markus Elfring Dec. 29, 2024, 4:15 p.m. UTC | #6
> The Huawei Matebook E Go (sc8280xp) tablet provides implements UCSI

                                                      Omit this verb here?


> interface in the onboard EC. …


> +++ b/drivers/platform/arm64/huawei-gaokun-ec.c
> @@ -0,0 +1,598 @@> +static void gaokun_set_orientation(struct ucsi_connector *con,
> +				   struct gaokun_ucsi_port *port)
> +{> +	spin_lock_irqsave(&port->lock, flags);
> +	ccx = port->ccx;
> +	spin_unlock_irqrestore(&port->lock, flags);
> +
> +	typec_set_orientation(con->port, CCX_TO_ORI(ccx));
> +}
…

Under which circumstances would you become interested to apply a statement
like “guard(spinlock_irqsave)(&port->lock);”?
https://elixir.bootlin.com/linux/v6.13-rc3/source/include/linux/spinlock.h#L572

Regards,
Markus
Pengyu Luo Dec. 29, 2024, 4:25 p.m. UTC | #7
On Sun, Dec 29, 2024 at 10:52 PM Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
> On 28/12/2024 14:38, Pengyu Luo wrote:
> > On Sat, Dec 28, 2024 at 9:06 PM Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:
> >> On 27/12/2024 17:13, Pengyu Luo wrote:
> >>> The Huawei Matebook E Go (sc8280xp) tablet provides implements UCSI
> >>> interface in the onboard EC. Add the glue driver to interface the
> >>> platform's UCSI implementation.
> >>>
> >>> Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> >>> ---
> >>>    drivers/usb/typec/ucsi/Kconfig              |   9 +
> >>>    drivers/usb/typec/ucsi/Makefile             |   1 +
> >>>    drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c | 481 ++++++++++++++++++++
> >>>    3 files changed, 491 insertions(+)
> >>>    create mode 100644 drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
> >>>
> >>> diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
> >>> index 680e1b87b..0d0f07488 100644
> >>> --- a/drivers/usb/typec/ucsi/Kconfig
> >>> +++ b/drivers/usb/typec/ucsi/Kconfig
> >>> @@ -78,4 +78,13 @@ config UCSI_LENOVO_YOGA_C630
> >>>          To compile the driver as a module, choose M here: the module will be
> >>>          called ucsi_yoga_c630.
> >>>
> >>> +config UCSI_HUAWEI_GAOKUN
> >>> +     tristate "UCSI Interface Driver for Huawei Matebook E Go (sc8280xp)"
> >>> +     depends on EC_HUAWEI_GAOKUN
> >>> +     help
> >>> +       This driver enables UCSI support on the Huawei Matebook E Go tablet.
> >>> +
> >>> +       To compile the driver as a module, choose M here: the module will be
> >>> +       called ucsi_huawei_gaokun.
> >>> +
> >>>    endif
> >>> diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
> >>> index aed41d238..0b400122b 100644
> >>> --- a/drivers/usb/typec/ucsi/Makefile
> >>> +++ b/drivers/usb/typec/ucsi/Makefile
> >>> @@ -22,3 +22,4 @@ obj-$(CONFIG_UCSI_CCG)                      += ucsi_ccg.o
> >>>    obj-$(CONFIG_UCSI_STM32G0)          += ucsi_stm32g0.o
> >>>    obj-$(CONFIG_UCSI_PMIC_GLINK)               += ucsi_glink.o
> >>>    obj-$(CONFIG_UCSI_LENOVO_YOGA_C630) += ucsi_yoga_c630.o
> >>> +obj-$(CONFIG_UCSI_HUAWEI_GAOKUN)     += ucsi_huawei_gaokun.o
> >>> diff --git a/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
> >>> new file mode 100644
> >>> index 000000000..84ed0407d
> >>> --- /dev/null
> >>> +++ b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
> >>> @@ -0,0 +1,481 @@
> >>> +// SPDX-License-Identifier: GPL-2.0-only
> >>> +/*
> >>> + * ucsi-huawei-gaokun - A UCSI driver for HUAWEI Matebook E Go (sc8280xp)
> >>> + *
> >>> + * reference: drivers/usb/typec/ucsi/ucsi_yoga_c630.c
> >>> + *            drivers/usb/typec/ucsi/ucsi_glink.c
> >>> + *            drivers/soc/qcom/pmic_glink_altmode.c
> >>> + *
> >>> + * Copyright (C) 2024 Pengyu Luo <mitltlatltl@gmail.com>
> >>> + */
> >>> +
> >>> +#include <linux/auxiliary_bus.h>
> >>> +#include <linux/bitops.h>
> >>> +#include <linux/completion.h>
> >>> +#include <linux/container_of.h>
> >>> +#include <linux/delay.h>
> >>> +#include <linux/module.h>
> >>> +#include <linux/notifier.h>
> >>> +#include <linux/of.h>
> >>> +#include <linux/string.h>
> >>> +#include <linux/workqueue_types.h>
> >>> +
> >>> +#include <linux/usb/pd_vdo.h>
> >>> +#include <drm/bridge/aux-bridge.h
> >>
> >> Is there a reason you don't have strict include alphanumeric ordering here ?
> >>
> >
> > These two is dp/alt mode related, so listing them out. Above of them are
> > general things.
>
> OK. Unless there's an include dependency reason you need to, please just
> sort the headers alphanumerically in order
>
> #include <globals_first>
> #include <globals_first_alpha>
>
> #include "locals_next"
> #include "locals_next_alpha_also"
>

I will follow this in v2.

> >>>
> >>> +
> >>> +#include "ucsi.h"
> >>> +#include <linux/platform_data/huawei-gaokun-ec.h>
> >>> +
> >>> +
> >>> +#define EC_EVENT_UCSI        0x21
> >>> +#define EC_EVENT_USB 0x22
> >>> +
> >>> +#define GAOKUN_CCX_MASK              GENMASK(1, 0)
> >>> +#define GAOKUN_MUX_MASK              GENMASK(3, 2)
> >>> +
> >>> +#define GAOKUN_DPAM_MASK     GENMASK(3, 0)
> >>> +#define GAOKUN_HPD_STATE_MASK        BIT(4)
> >>> +#define GAOKUN_HPD_IRQ_MASK  BIT(5)
> >>> +
> >>> +#define CCX_TO_ORI(ccx) (++ccx % 3)
> >>
> >> Why do you increment the value of the enum ?
> >> Seems strange.
> >>
> >
> > EC's logic, it is just a trick. Qualcomm maps
> > 0 1 2 to normal, reverse, none(no device insert)
> > typec lib maps 1 2 0 to that.
>
> I'd suggest making the trick much more obvious.
>
> Either with a comment or just mapping 1:1 between EC and Linux' view of
> this data.
>
> The main reason for that is to make it easier to
> read/understand/maintain/debug.
>

I got it

>
>
> >>> +             port->svid = USB_SID_DISPLAYPORT;
> >>> +             if (port->ccx == USBC_CCX_REVERSE)
> >>> +                     port->mode -= 6;
> >>
> >> why minus six ?
> >> needs a comment.
> >>
> >
> > EC's logic. I don't know why, it is a quirk from Qualcomm or Huawei.
> > I will mention this.
>
> Instead of hard-coding a mapping between the EC's mode and Linux' UCSI
> enum - just make a define or an inline, ideally something with
>
> switch(port->mode)
> case GOAKUN_MODE_0:
> 	val = LINUX_UCSI_MODE_X;
> case GOAKUN_MODE_1:
> 	val = LINUX_UCSI_MODE_Y;
> }
>
> That will make the mapping obvious and also ensure both to yourself and
> to your reviewers that you have accounted for _all_ of the potential
> mappings and if those mappings don't exist then the default: of your
> switch statement should make some noise about it
>
> dev_err(dev, "GOKUN UCSI mode %d unmapped\n"); or something like that.
>

Makes sense

>
> >
> >>> +             break;
> >>> +     default:
> >>> +             break;
> >>> +     }
> >>> +
> >>> +     spin_unlock_irqrestore(&port->lock, flags);
> >>> +}
> >>> +
> >>> +static int gaokun_ucsi_refresh(struct gaokun_ucsi *uec)
> >>> +{
> >>> +     struct gaokun_ucsi_reg ureg;
> >>> +     int ret, idx;
> >>> +
> >>> +     ret = gaokun_ec_ucsi_get_reg(uec->ec, (u8 *)&ureg);
> >>> +     if (ret)
> >>> +             return -EIO;
> >>> +
> >>> +     uec->port_num = ureg.port_num;
> >>> +     idx = GET_IDX(ureg.port_updt);
> >>> +
> >>> +     if (idx >= 0 && idx < ureg.port_num)
> >>> +             gaokun_ucsi_port_update(&uec->ports[idx], ureg.port_data);
> >>
> >> Since you are checking the validity of the index, you should -EINVAL if
> >> the index is out of range.
> >>
> >
> > EC / pmic glink encode every port in a bit
> > 0/1/2/4/... => ???/left/right/some port
> >
> > I remap it to -1/0/1/2, to access specific port exceptions(-1) are not
> > harmful, later in gaokun_ucsi_altmode_notify_ind
> >
> >       if (idx < 0)
> >               gaokun_ec_ucsi_pan_ack(uec->ec, idx);
> >       else
> >               gaokun_ucsi_handle_altmode(&uec->ports[idx]);
> >
> > gaokun_ec_ucsi_pan_ack can handle exceptions.
> >
>
> gaokun_ucsi_refresh() can return
>
> -EIO
> -1
> >=0
>
> Where -EIO and -1 both trigger gaokun_ec_ucsi_pan_ack() in
> gaokun_ucsi_altmode_notify_ind()
>
> So if the index doesn't matter and < 0 => pan_ack() is OK or -EIO is not
> returning meaningful error.
>
> Either way strongly advise against mixing a negative index as having a
> valid meaning with actual -E error codes...
>
> As a reviewer doing a fist-pass this looks suspicous and implies more
> thought/refinement should be done to the flow.
>

Agree, I will drop -EIO to use -1 instead, and define -1 as NO_UPDATE.
It is also resonable when there is a EC transaction error.

Best wishes,
Pengyu
diff mbox series

Patch

diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
index 680e1b87b..0d0f07488 100644
--- a/drivers/usb/typec/ucsi/Kconfig
+++ b/drivers/usb/typec/ucsi/Kconfig
@@ -78,4 +78,13 @@  config UCSI_LENOVO_YOGA_C630
 	  To compile the driver as a module, choose M here: the module will be
 	  called ucsi_yoga_c630.
 
+config UCSI_HUAWEI_GAOKUN
+	tristate "UCSI Interface Driver for Huawei Matebook E Go (sc8280xp)"
+	depends on EC_HUAWEI_GAOKUN
+	help
+	  This driver enables UCSI support on the Huawei Matebook E Go tablet.
+
+	  To compile the driver as a module, choose M here: the module will be
+	  called ucsi_huawei_gaokun.
+
 endif
diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
index aed41d238..0b400122b 100644
--- a/drivers/usb/typec/ucsi/Makefile
+++ b/drivers/usb/typec/ucsi/Makefile
@@ -22,3 +22,4 @@  obj-$(CONFIG_UCSI_CCG)			+= ucsi_ccg.o
 obj-$(CONFIG_UCSI_STM32G0)		+= ucsi_stm32g0.o
 obj-$(CONFIG_UCSI_PMIC_GLINK)		+= ucsi_glink.o
 obj-$(CONFIG_UCSI_LENOVO_YOGA_C630)	+= ucsi_yoga_c630.o
+obj-$(CONFIG_UCSI_HUAWEI_GAOKUN)	+= ucsi_huawei_gaokun.o
diff --git a/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
new file mode 100644
index 000000000..84ed0407d
--- /dev/null
+++ b/drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c
@@ -0,0 +1,481 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ucsi-huawei-gaokun - A UCSI driver for HUAWEI Matebook E Go (sc8280xp)
+ *
+ * reference: drivers/usb/typec/ucsi/ucsi_yoga_c630.c
+ *            drivers/usb/typec/ucsi/ucsi_glink.c
+ *            drivers/soc/qcom/pmic_glink_altmode.c
+ *
+ * Copyright (C) 2024 Pengyu Luo <mitltlatltl@gmail.com>
+ */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/bitops.h>
+#include <linux/completion.h>
+#include <linux/container_of.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/of.h>
+#include <linux/string.h>
+#include <linux/workqueue_types.h>
+
+#include <linux/usb/pd_vdo.h>
+#include <drm/bridge/aux-bridge.h>
+
+#include "ucsi.h"
+#include <linux/platform_data/huawei-gaokun-ec.h>
+
+
+#define EC_EVENT_UCSI	0x21
+#define EC_EVENT_USB	0x22
+
+#define GAOKUN_CCX_MASK		GENMASK(1, 0)
+#define GAOKUN_MUX_MASK		GENMASK(3, 2)
+
+#define GAOKUN_DPAM_MASK	GENMASK(3, 0)
+#define GAOKUN_HPD_STATE_MASK	BIT(4)
+#define GAOKUN_HPD_IRQ_MASK	BIT(5)
+
+#define CCX_TO_ORI(ccx) (++ccx % 3)
+
+#define GET_IDX(updt) (ffs(updt) - 1)
+
+/* Configuration Channel Extension */
+enum gaokun_ucsi_ccx {
+	USBC_CCX_NORMAL,
+	USBC_CCX_REVERSE,
+	USBC_CCX_NONE,
+};
+
+enum gaokun_ucsi_mux {
+	USBC_MUX_NONE,
+	USBC_MUX_USB_2L,
+	USBC_MUX_DP_4L,
+	USBC_MUX_USB_DP,
+};
+
+struct gaokun_ucsi_reg {
+	u8 port_num;
+	u8 port_updt;
+	u8 port_data[4];
+	u8 checksum;
+	u8 reserved;
+} __packed;
+
+struct gaokun_ucsi_port {
+	struct completion usb_ack;
+	spinlock_t lock;
+
+	struct gaokun_ucsi *ucsi;
+	struct auxiliary_device *bridge;
+
+	int idx;
+	enum gaokun_ucsi_ccx ccx;
+	enum gaokun_ucsi_mux mux;
+	u8 mode;
+	u16 svid;
+	u8 hpd_state;
+	u8 hpd_irq;
+};
+
+struct gaokun_ucsi {
+	struct gaokun_ec *ec;
+	struct ucsi *ucsi;
+	struct gaokun_ucsi_port *ports;
+	struct device *dev;
+	struct work_struct work;
+	struct notifier_block nb;
+	u16 version;
+	u8 port_num;
+};
+
+/* -------------------------------------------------------------------------- */
+/* For UCSI */
+
+static int gaokun_ucsi_read_version(struct ucsi *ucsi, u16 *version)
+{
+	struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
+
+	*version = uec->version;
+
+	return 0;
+}
+
+static int gaokun_ucsi_read_cci(struct ucsi *ucsi, u32 *cci)
+{
+	struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
+	u8 buf[GAOKUN_UCSI_READ_SIZE];
+	int ret;
+
+	ret = gaokun_ec_ucsi_read(uec->ec, buf);
+	if (ret)
+		return ret;
+
+	memcpy(cci, buf, sizeof(*cci));
+
+	return 0;
+}
+
+static int gaokun_ucsi_read_message_in(struct ucsi *ucsi,
+				       void *val, size_t val_len)
+{
+	struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
+	u8 buf[GAOKUN_UCSI_READ_SIZE];
+	int ret;
+
+	ret = gaokun_ec_ucsi_read(uec->ec, buf);
+	if (ret)
+		return ret;
+
+	memcpy(val, buf + GAOKUN_UCSI_CCI_SIZE,
+	       min(val_len, GAOKUN_UCSI_DATA_SIZE));
+
+	return 0;
+}
+
+static int gaokun_ucsi_async_control(struct ucsi *ucsi, u64 command)
+{
+	struct gaokun_ucsi *uec = ucsi_get_drvdata(ucsi);
+	u8 buf[GAOKUN_UCSI_WRITE_SIZE] = {};
+
+	memcpy(buf, &command, sizeof(command));
+
+	return gaokun_ec_ucsi_write(uec->ec, buf);
+}
+
+static void gaokun_ucsi_update_connector(struct ucsi_connector *con)
+{
+	struct gaokun_ucsi *uec = ucsi_get_drvdata(con->ucsi);
+
+	if (con->num > uec->port_num)
+		return;
+
+	con->typec_cap.orientation_aware = true;
+}
+
+static void gaokun_set_orientation(struct ucsi_connector *con,
+				   struct gaokun_ucsi_port *port)
+{
+	enum gaokun_ucsi_ccx ccx;
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+	ccx = port->ccx;
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	typec_set_orientation(con->port, CCX_TO_ORI(ccx));
+}
+
+static void gaokun_ucsi_connector_status(struct ucsi_connector *con)
+{
+	struct gaokun_ucsi *uec = ucsi_get_drvdata(con->ucsi);
+	int idx;
+
+	idx = con->num - 1;
+	if (con->num > uec->port_num) {
+		dev_warn(uec->ucsi->dev, "set orientation out of range: con%d\n", idx);
+		return;
+	}
+
+	gaokun_set_orientation(con, &uec->ports[idx]);
+}
+
+const struct ucsi_operations gaokun_ucsi_ops = {
+	.read_version = gaokun_ucsi_read_version,
+	.read_cci = gaokun_ucsi_read_cci,
+	.read_message_in = gaokun_ucsi_read_message_in,
+	.sync_control = ucsi_sync_control_common,
+	.async_control = gaokun_ucsi_async_control,
+	.update_connector = gaokun_ucsi_update_connector,
+	.connector_status = gaokun_ucsi_connector_status,
+};
+
+/* -------------------------------------------------------------------------- */
+/* For Altmode */
+
+static void gaokun_ucsi_port_update(struct gaokun_ucsi_port *port,
+				    const u8 *port_data)
+{
+	unsigned long flags;
+	u8 dcc, ddi;
+	int offset = port->idx * 2; /* every port has 2 Bytes data */
+
+	dcc = port_data[offset];
+	ddi = port_data[offset + 1];
+
+	spin_lock_irqsave(&port->lock, flags);
+
+	port->ccx = FIELD_GET(GAOKUN_CCX_MASK, dcc);
+	port->mux = FIELD_GET(GAOKUN_MUX_MASK, dcc);
+	port->mode = FIELD_GET(GAOKUN_DPAM_MASK, ddi);
+	port->hpd_state = FIELD_GET(GAOKUN_HPD_STATE_MASK, ddi);
+	port->hpd_irq = FIELD_GET(GAOKUN_HPD_IRQ_MASK, ddi);
+
+	switch (port->mux) {
+	case USBC_MUX_NONE:
+		port->svid = 0;
+		break;
+	case USBC_MUX_USB_2L:
+		port->svid = USB_SID_PD;
+		break;
+	case USBC_MUX_DP_4L:
+	case USBC_MUX_USB_DP:
+		port->svid = USB_SID_DISPLAYPORT;
+		if (port->ccx == USBC_CCX_REVERSE)
+			port->mode -= 6;
+		break;
+	default:
+		break;
+	}
+
+	spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static int gaokun_ucsi_refresh(struct gaokun_ucsi *uec)
+{
+	struct gaokun_ucsi_reg ureg;
+	int ret, idx;
+
+	ret = gaokun_ec_ucsi_get_reg(uec->ec, (u8 *)&ureg);
+	if (ret)
+		return -EIO;
+
+	uec->port_num = ureg.port_num;
+	idx = GET_IDX(ureg.port_updt);
+
+	if (idx >= 0 && idx < ureg.port_num)
+		gaokun_ucsi_port_update(&uec->ports[idx], ureg.port_data);
+
+	return idx;
+}
+
+static void gaokun_ucsi_handle_altmode(struct gaokun_ucsi_port *port)
+{
+	struct gaokun_ucsi *uec = port->ucsi;
+	int idx = port->idx;
+
+	if (idx >= uec->ucsi->cap.num_connectors || !uec->ucsi->connector) {
+		dev_warn(uec->ucsi->dev, "altmode port out of range: %d\n", idx);
+		return;
+	}
+
+	/* UCSI callback .connector_status() have set orientation */
+	if (port->bridge)
+		drm_aux_hpd_bridge_notify(&port->bridge->dev,
+					  port->hpd_state ?
+					  connector_status_connected :
+					  connector_status_disconnected);
+
+	gaokun_ec_ucsi_pan_ack(uec->ec, port->idx);
+}
+
+static void gaokun_ucsi_altmode_notify_ind(struct gaokun_ucsi *uec)
+{
+	int idx;
+
+	idx = gaokun_ucsi_refresh(uec);
+	if (idx < 0)
+		gaokun_ec_ucsi_pan_ack(uec->ec, idx);
+	else
+		gaokun_ucsi_handle_altmode(&uec->ports[idx]);
+}
+
+/*
+ * USB event is necessary for enabling altmode, the event should follow
+ * UCSI event, if not after timeout(this notify may be disabled somehow),
+ * then force to enable altmode.
+ */
+static void gaokun_ucsi_handle_no_usb_event(struct gaokun_ucsi *uec, int idx)
+{
+	struct gaokun_ucsi_port *port;
+
+	port = &uec->ports[idx];
+	if (!wait_for_completion_timeout(&port->usb_ack, 2 * HZ)) {
+		dev_warn(uec->dev, "No USB EVENT, triggered by UCSI EVENT");
+		gaokun_ucsi_altmode_notify_ind(uec);
+	}
+}
+
+static int gaokun_ucsi_notify(struct notifier_block *nb,
+			      unsigned long action, void *data)
+{
+	u32 cci;
+	struct gaokun_ucsi *uec = container_of(nb, struct gaokun_ucsi, nb);
+
+	switch (action) {
+	case EC_EVENT_USB:
+		gaokun_ucsi_altmode_notify_ind(uec);
+		return NOTIFY_OK;
+
+	case EC_EVENT_UCSI:
+		uec->ucsi->ops->read_cci(uec->ucsi, &cci);
+		ucsi_notify_common(uec->ucsi, cci);
+		if (UCSI_CCI_CONNECTOR(cci))
+			gaokun_ucsi_handle_no_usb_event(uec, UCSI_CCI_CONNECTOR(cci) - 1);
+
+		return NOTIFY_OK;
+
+	default:
+		return NOTIFY_DONE;
+	}
+}
+
+static int gaokun_ucsi_get_port_num(struct gaokun_ucsi *uec)
+{
+	struct gaokun_ucsi_reg ureg;
+	int ret;
+
+	ret = gaokun_ec_ucsi_get_reg(uec->ec, (u8 *)&ureg);
+
+	return ret ? 0 : ureg.port_num;
+}
+
+static int gaokun_ucsi_ports_init(struct gaokun_ucsi *uec)
+{
+	u32 port;
+	int i, ret, port_num;
+	struct device *dev = uec->dev;
+	struct gaokun_ucsi_port *ucsi_port;
+	struct fwnode_handle *fwnode;
+
+	port_num = gaokun_ucsi_get_port_num(uec);
+	uec->port_num = port_num;
+
+	uec->ports = devm_kzalloc(dev, port_num * sizeof(*(uec->ports)),
+				  GFP_KERNEL);
+	if (!uec->ports)
+		return -ENOMEM;
+
+	for (i = 0; i < port_num; ++i) {
+		ucsi_port = &uec->ports[i];
+		ucsi_port->ccx = USBC_CCX_NONE;
+		ucsi_port->idx = i;
+		ucsi_port->ucsi = uec;
+		init_completion(&ucsi_port->usb_ack);
+		spin_lock_init(&ucsi_port->lock);
+	}
+
+	device_for_each_child_node(dev, fwnode) {
+		ret = fwnode_property_read_u32(fwnode, "reg", &port);
+		if (ret < 0) {
+			dev_err(dev, "missing reg property of %pOFn\n", fwnode);
+			fwnode_handle_put(fwnode);
+			return ret;
+		}
+
+		if (port >= port_num) {
+			dev_warn(dev, "invalid connector number %d, ignoring\n", port);
+			continue;
+		}
+
+		ucsi_port = &uec->ports[port];
+		ucsi_port->bridge = devm_drm_dp_hpd_bridge_alloc(dev, to_of_node(fwnode));
+		if (IS_ERR(ucsi_port->bridge)) {
+			fwnode_handle_put(fwnode);
+			return PTR_ERR(ucsi_port->bridge);
+		}
+	}
+
+	for (i = 0; i < port_num; i++) {
+		if (!uec->ports[i].bridge)
+			continue;
+
+		ret = devm_drm_dp_hpd_bridge_add(dev, uec->ports[i].bridge);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static void gaokun_ucsi_register_worker(struct work_struct *work)
+{
+	struct gaokun_ucsi *uec;
+	struct ucsi *ucsi;
+	int ret;
+
+	uec = container_of(work, struct gaokun_ucsi, work);
+	ucsi = uec->ucsi;
+
+	ucsi->quirks = UCSI_NO_PARTNER_PDOS | UCSI_DELAY_DEVICE_PDOS;
+
+	ssleep(3); /* EC can't handle UCSI properly in the early stage */
+
+	ret = gaokun_ec_register_notify(uec->ec, &uec->nb);
+	if (ret) {
+		dev_err_probe(ucsi->dev, ret, "notifier register failed\n");
+		return;
+	}
+
+	ret = ucsi_register(ucsi);
+	if (ret)
+		dev_err_probe(ucsi->dev, ret, "ucsi register failed\n");
+}
+
+static int gaokun_ucsi_register(struct gaokun_ucsi *uec)
+{
+	schedule_work(&uec->work);
+
+	return 0;
+}
+
+static int gaokun_ucsi_probe(struct auxiliary_device *adev,
+			     const struct auxiliary_device_id *id)
+{
+	struct gaokun_ec *ec = adev->dev.platform_data;
+	struct device *dev = &adev->dev;
+	struct gaokun_ucsi *uec;
+	int ret;
+
+	uec = devm_kzalloc(dev, sizeof(*uec), GFP_KERNEL);
+	if (!uec)
+		return -ENOMEM;
+
+	uec->ec = ec;
+	uec->dev = dev;
+	uec->version = 0x0100;
+	uec->nb.notifier_call = gaokun_ucsi_notify;
+
+	INIT_WORK(&uec->work, gaokun_ucsi_register_worker);
+
+	ret = gaokun_ucsi_ports_init(uec);
+	if (ret)
+		return ret;
+
+	uec->ucsi = ucsi_create(dev, &gaokun_ucsi_ops);
+	if (IS_ERR(uec->ucsi))
+		return PTR_ERR(uec->ucsi);
+
+	ucsi_set_drvdata(uec->ucsi, uec);
+	auxiliary_set_drvdata(adev, uec);
+
+	return gaokun_ucsi_register(uec);
+}
+
+static void gaokun_ucsi_remove(struct auxiliary_device *adev)
+{
+	struct gaokun_ucsi *uec = auxiliary_get_drvdata(adev);
+
+	gaokun_ec_unregister_notify(uec->ec, &uec->nb);
+	ucsi_unregister(uec->ucsi);
+	ucsi_destroy(uec->ucsi);
+}
+
+static const struct auxiliary_device_id gaokun_ucsi_id_table[] = {
+	{ .name = GAOKUN_MOD_NAME "." GAOKUN_DEV_UCSI, },
+	{}
+};
+MODULE_DEVICE_TABLE(auxiliary, gaokun_ucsi_id_table);
+
+static struct auxiliary_driver gaokun_ucsi_driver = {
+	.name = GAOKUN_DEV_UCSI,
+	.id_table = gaokun_ucsi_id_table,
+	.probe = gaokun_ucsi_probe,
+	.remove = gaokun_ucsi_remove,
+};
+
+module_auxiliary_driver(gaokun_ucsi_driver);
+
+MODULE_DESCRIPTION("HUAWEI Matebook E Go UCSI driver");
+MODULE_LICENSE("GPL");