Re: [PATCH v2 2/2] usb: typec: Add onsemi FUSB15201 driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unrecoverable interrupt storm due to masked write-1-to-clear operations.
--

commit f97eba3512034404133da1fbd6a5cac5d95da491
Author: Shawn Guo <shengchao.guo@oss.qualcomm.com>

usb: typec: Add onsemi FUSB15201 driver

This commit adds the driver for the onsemi FUSB15201 dual-port USB Type-C 
and Power Delivery controller.

Link: https://lore.kernel.org/r/20260902045938.184221-3-shengchao.guo@oss.qualcomm.com

> diff --git a/drivers/usb/typec/fusb15201.c b/drivers/usb/typec/fusb15201.c
> new file mode 100644
> index 0000000000000..897b2fabc449e
> --- /dev/null
> +++ b/drivers/usb/typec/fusb15201.c

[ ... ]

> +static irqreturn_t fusb15201_irq(int irq, void *data)
> +{
> +	struct fusb15201 *chip = data;
> +	irqreturn_t ret = IRQ_NONE;
> +	unsigned int i;
> +
> +	guard(mutex)(&chip->lock);
> +
> +	for (i = 0; i < FUSB15201_NUM_PORTS; i++) {
> +		struct fusb15201_port *port = &chip->ports[i];
> +		unsigned int pending;
> +
> +		if (!port->port)
> +			continue;
> +
> +		if (regmap_read(chip->regmap, FUSB15201_REG_INTERRUPT(i),
> +				&pending))
> +			continue;
> +
> +		pending &= FUSB15201_INT_ALL;
> +		if (!pending)
> +			continue;
> +
> +		/* Write one to clear */
> +		regmap_write(chip->regmap, FUSB15201_REG_INTERRUPT(i), pending);

[Severity: High]
Can this lead to an unrecoverable interrupt loop if the hardware sets an
unexpected bit?

By applying a bitmask (pending &= FUSB15201_INT_ALL) before writing back to
the W1C register, any interrupt bit outside this mask will never be cleared.

[ ... ]

> +static int fusb15201_hw_init(struct fusb15201 *chip)
> +{
> +	bool source_only = true;
> +	unsigned int i;
> +	int ret;
> +
> +	for (i = 0; i < FUSB15201_NUM_PORTS; i++) {
> +		struct fusb15201_port *port = &chip->ports[i];
> +
> +		if (!port->port) {
> +			/* Nothing describes this port, keep it quiet. */
> +			ret = regmap_write(chip->regmap,
> +					   FUSB15201_REG_INT_MASK(i),
> +					   FUSB15201_INT_ALL);

[Severity: High]
Does this leave bits 3-7 unmasked on unused ports?

Since FUSB15201_INT_ALL is 0x07, writing it to the mask register might
keep bits 0-2 masked, but leaves the upper bits completely unmasked.

> +			if (ret)
> +				return ret;
> +
> +			continue;
> +		}
> +
> +		if (port->cap.type != TYPEC_PORT_SRC)
> +			source_only = false;
> +
> +		ret = regmap_write(chip->regmap, FUSB15201_REG_INT_MASK(i), 0);

[Severity: High]
Does writing 0 here unmask all interrupt bits on the port, including ones
not covered by FUSB15201_INT_ALL?

Combined with the clearing logic in fusb15201_irq() that only clears bits
defined in FUSB15201_INT_ALL, if any of the upper bits trigger an
interrupt, they won't be cleared. This would cause the IRQ handler to
immediately re-trigger in a continuous storm.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910091751.819264-1-shengchao.guo@oss.qualcomm.com?part=2