Re: [PATCH v3 2/2] usb: typec: ucsi: add ITE885x I2C transport driver

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Sat, Aug 29, 2026 at 03:55:16PM +0100, Edward Blair wrote:
> Add a UCSI transport driver for ITE8853 and ITE8800 through ITE8805
> USB Type-C controllers found on desktop motherboards.
> 
> These controllers expose CCI, MESSAGE_IN and CONTROL at ITE-specific I2C
> offsets and signal UCSI and vendor events through a shared interrupt
> status register. Read and cache each complete UCSI event before
> acknowledging it so command data remains coherent between the interrupt
> handler and UCSI core.
> 
> The interface does not expose a VERSION register and does not accept
> PPM_RESET over I2C. Report UCSI 1.0, limit MESSAGE_IN to its 16-byte
> window and handle PPM_RESET locally, matching the vendor driver's
> behavior.
> 
> Signed-off-by: Edward Blair <edward.blair@gmail.com>
> ---
>  drivers/usb/typec/ucsi/Kconfig    |  11 +
>  drivers/usb/typec/ucsi/Makefile   |   1 +
>  drivers/usb/typec/ucsi/ucsi_ite.c | 395 ++++++++++++++++++++++++++++++
>  3 files changed, 407 insertions(+)
>  create mode 100644 drivers/usb/typec/ucsi/ucsi_ite.c
> 
> diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
> index 87dd992a4..3819c4f73 100644
> --- a/drivers/usb/typec/ucsi/Kconfig
> +++ b/drivers/usb/typec/ucsi/Kconfig
> @@ -104,4 +104,15 @@ config UCSI_HUAWEI_GAOKUN
>  	  To compile the driver as a module, choose M here: the module will be
>  	  called ucsi_huawei_gaokun.
>  
> +config UCSI_ITE
> +	tristate "UCSI Interface Driver for ITE885x"
> +	depends on ACPI && I2C
> +	help
> +	  This driver enables UCSI support on platforms that expose an ITE8853
> +	  or ITE8800-ITE8805 USB Type-C controller over I2C, commonly found
> +	  on ASUS Z690/Z790/X670E motherboards.
> +
> +	  To compile the driver as a module, choose M here: the module will be
> +	  called ucsi_ite.
> +
>  endif
> diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
> index c7e38bf01..9bc1d6bbb 100644
> --- a/drivers/usb/typec/ucsi/Makefile
> +++ b/drivers/usb/typec/ucsi/Makefile
> @@ -28,3 +28,4 @@ obj-$(CONFIG_UCSI_PMIC_GLINK)		+= ucsi_glink.o
>  obj-$(CONFIG_CROS_EC_UCSI)		+= cros_ec_ucsi.o
>  obj-$(CONFIG_UCSI_LENOVO_YOGA_C630)	+= ucsi_yoga_c630.o
>  obj-$(CONFIG_UCSI_HUAWEI_GAOKUN)	+= ucsi_huawei_gaokun.o
> +obj-$(CONFIG_UCSI_ITE)			+= ucsi_ite.o
> diff --git a/drivers/usb/typec/ucsi/ucsi_ite.c b/drivers/usb/typec/ucsi/ucsi_ite.c
> new file mode 100644
> index 000000000..16b22c77f
> --- /dev/null
> +++ b/drivers/usb/typec/ucsi/ucsi_ite.c
> @@ -0,0 +1,395 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * UCSI I2C transport driver for ITE885x USB-C controllers
> + *
> + * ITE8853/ITE8800-ITE8805 are UCSI-compliant USB-C controllers found on
> + * desktop motherboards. They communicate over I2C using UCSI registers at
> + * ITE-specific offsets and signal events through a vendor interrupt register.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>

We need to include everything instead of relying on nested headers
nowadays.

        linux/device.h
        linux/device/devres.h
        linux/dev_printk.h
        linux/pm.h
        linux/string.h

> +static int ucsi_ite_process_event(struct ucsi_ite *ite, u32 *cci)
> +{
> +	u8 message_in[ITE_MESSAGE_IN_MAX_LEN] = {};
> +	__le32 raw_cci;
> +	u8 status;
> +	u8 len = 0;
> +	int event;
> +	int err = 0;
> +	int ret;
> +
> +	mutex_lock(&ite->event_lock);

guard(mutex)(@ite->event_lock);

> +	ret = ucsi_ite_read(ite, ITE_REG_INT_STATUS, &status, sizeof(status));
> +	if (ret)
> +		goto out_unlock;
> +
> +	status &= ITE_INT_MASK;
> +	if (!status) {
> +		mutex_lock(&ite->received_lock);
> +		*cci = ite->cci;
> +		mutex_unlock(&ite->received_lock);
> +		ret = ITE_EVENT_NONE;
> +		goto out_unlock;
> +	}
> +
> +	if (status & ITE_INT_CCI) {
> +		err = ucsi_ite_read(ite, ITE_REG_CCI, &raw_cci,
> +				    sizeof(raw_cci));
> +		if (!err) {
> +			*cci = le32_to_cpu(raw_cci);
> +			len = UCSI_CCI_LENGTH(*cci);
> +
> +			if (len > sizeof(message_in)) {
> +				len = sizeof(message_in);
> +				*cci &= ~GENMASK(15, 8);
> +				*cci |= UCSI_SET_CCI_LENGTH(len);
> +			}
> +			if (len) {
> +				err = ucsi_ite_read(ite, ITE_REG_MESSAGE_IN,
> +						    message_in, len);
> +			}
> +		}
> +	}
> +
> +	/* Acknowledge each latched event with the value expected by the PPM. */
> +	if (status & ITE_INT_VENDOR_ALERT) {
> +		u8 ack = ITE_INT_VENDOR_ALERT;
> +
> +		ret = ucsi_ite_write(ite, ITE_REG_INT_ACK, &ack, sizeof(ack));
> +		if (ret)
> +			goto out_unlock;
> +	}
> +
> +	if ((status & ITE_INT_CCI) && !err) {
> +		u8 ack = ITE_INT_CCI;
> +
> +		ret = ucsi_ite_write(ite, ITE_REG_INT_ACK, &ack, sizeof(ack));
> +		if (ret)
> +			goto out_unlock;
> +	}
> +
> +	if (err) {
> +		ret = err;
> +		goto out_unlock;
> +	}
> +
> +	if (status & ITE_INT_CCI) {
> +		mutex_lock(&ite->received_lock);
> +		ite->cci = *cci;
> +		memset(ite->message_in, 0, sizeof(ite->message_in));
> +		memcpy(ite->message_in, message_in, len);
> +		mutex_unlock(&ite->received_lock);
> +		event = ITE_EVENT_CCI;
> +	} else {
> +		mutex_lock(&ite->received_lock);
> +		*cci = ite->cci;
> +		mutex_unlock(&ite->received_lock);
> +		event = ITE_EVENT_VENDOR;
> +	}
> +
> +	ret = event;
> +
> +out_unlock:
> +	mutex_unlock(&ite->event_lock);
> +	return ret;
> +}

> +static int ucsi_ite_suspend(struct device *dev)
> +{
> +	struct ucsi_ite *ite = dev_get_drvdata(dev);
> +	int ret;
> +
> +	disable_irq(ite->client->irq);
> +	ret = ucsi_suspend(ite->ucsi);
> +	if (ret)
> +		enable_irq(ite->client->irq);
> +
> +	return ret;
> +}
> +
> +static int ucsi_ite_resume(struct device *dev)
> +{
> +	struct ucsi_ite *ite = dev_get_drvdata(dev);
> +
> +	enable_irq(ite->client->irq);
> +	return ucsi_resume(ite->ucsi);
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(ucsi_ite_pm, ucsi_ite_suspend,
> +				ucsi_ite_resume);

Is the enable/disable_irq really necessary - couldn't you just use
NOIRQ version of the PM operations (DEFINE_NOIRQ_DEV_PM_OPS)?

Thanks,

-- 
heikki