Re: [PATCH v3 2/2] usb: typec: ucsi: Add ITE IT885x Type-C PD controller driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The I2C read buffer uses an unaligned pointer `buf + 1`, which violates DMA API constraints and can cause warnings or memory corruption if the I2C master uses DMA.
- [Medium] Un-ratelimited dev_err() in I2C accessors defeats the rate-limiting strategy in the IRQ handler, leading to a log storm during persistent I2C failures.
--

--- Patch [2]: [PATCH v3 2/2] usb: typec: ucsi: Add ITE IT885x Type-C PD controller driver ---
commit 1c3b27c040c8a8858b0730cebf73ad5bfc62f517
Author: Amber Kao <amber.kao@ite.com.tw>

usb: typec: ucsi: Add ITE IT885x Type-C PD controller driver

This patch adds core UCSI support for the ITE IT885x USB Type-C Power Delivery
controller over I2C.

Link: https://lore.kernel.org/all/20260710-ucsi-itepd-feature-v2-0-41943fd5df38@ite.com.tw/
Link: https://lore.kernel.org/all/20260615-ucsi-itepd-feature-v1-0-a826cfd0df6a@ite.com.tw/

> diff --git a/drivers/usb/typec/ucsi/ucsi_itepd.c b/drivers/usb/typec/ucsi/ucsi_itepd.c
> new file mode 100644
> index 0000000000000..539818869ab73
> --- /dev/null
> +++ b/drivers/usb/typec/ucsi/ucsi_itepd.c

[ ... ]

> +static int itepd_read_reg(struct itepd *itepd, u8 reg, void *data, u32 len)
> +{
> +	struct i2c_client *client = itepd->client;
> +	struct i2c_msg msg[2];
> +	u8 *buf;
> +	int ret;
> +
> +	/* I2C buffers must be DMA-safe, so no stack memory here. */
> +	buf = kzalloc(len + 1, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	buf[0] = reg;
> +
> +	msg[0].addr     = client->addr;
> +	msg[0].flags    = 0;
> +	msg[0].len      = 1;
> +	msg[0].buf      = buf;
> +
> +	msg[1].addr     = client->addr;
> +	msg[1].flags    = I2C_M_RD;
> +	msg[1].len      = len;
> +	msg[1].buf      = buf + 1;
                        ^^^^^^^

[Severity: High]
Does this violate DMA alignment requirements?

Since buf is allocated with kzalloc(), it is aligned by the slab allocator,
but buf + 1 is unaligned. If the I2C host controller uses DMA for transfers,
mapping an unaligned address can cause memory corruption, data shifts, or
trigger warnings in dma_map_single() on architectures with strict DMA
alignment requirements.

> +
> +	mutex_lock(&itepd->i2c_lock);
> +	ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> +	mutex_unlock(&itepd->i2c_lock);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "reg 0x%02x read failed: %d\n", reg, ret);

[Severity: Medium]
Can this cause a log storm during an I2C failure?

If the I2C bus fails and the IRQ line remains asserted, itepd_irq_process()
will repeatedly call this function. Because this error print is not
rate-limited, it defeats the dev_err_ratelimited() check in the caller and
will spam the console before the core IRQ subsystem has a chance to disable
the spurious interrupt.

> +		goto out_free;
> +	}

[ ... ]

> +static int itepd_write_reg(struct itepd *itepd, u8 reg, const void *data, u32 len)
> +{

[ ... ]

> +	mutex_lock(&itepd->i2c_lock);
> +	ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> +	mutex_unlock(&itepd->i2c_lock);
> +
> +	if (ret < 0) {
> +		dev_err(&client->dev, "reg 0x%02x write failed: %d\n", reg, ret);

[Severity: Medium]
Similarly, can this also cause a log storm if the I2C bus fails during the
write in the out_clear path of itepd_irq_process()?

> +		goto out_free;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-ucsi-itepd-feature-v3-0-8523e1e1c47a@ite.com.tw?part=2