Re: [PATCH 4/5] ARM: dts: ti: cpcap-mapphone: use charger detection interrupt for CPCAP USB PHY

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Backward Compatibility Breakage in DT interrupt name change
- [High] Missing Cleanup / Error Handling causes permanent sysrq disablement
- [Medium] Data Race on `cpcap_enable_uart` module parameter leads to mixed hardware state
--

commit 657e07c7931cc99eefeab1abc1d21d20f3271aa2
Author: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>

ARM: dts: ti: cpcap-mapphone: use charger detection interrupt for CPCAP USB PHY

This commit updates the CPCAP USB PHY interrupt mapping in the device tree
to use the charger detection interrupt. It replaces the "se1" interrupt name
with "chrg_det", which matches driver and binding updates for DCP detection.

> diff --git a/arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi b/arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi
> index 73e27b95105e1..f3ec85d73a16f 100644
> --- a/arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi
> +++ b/arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi
[ ... ]
> @@ -106,11 +106,11 @@ cpcap_usb2_phy: phy {
>  			interrupts-extended =
>  				<&cpcap 15 0>, <&cpcap 14 0>, <&cpcap 28 0>,
>  				<&cpcap 19 0>, <&cpcap 18 0>, <&cpcap 17 0>,
> -				<&cpcap 16 0>, <&cpcap 49 0>, <&cpcap 48 0>;
> +				<&cpcap 13 0>, <&cpcap 49 0>, <&cpcap 48 0>;
>  			interrupt-names =
>  				"id_ground", "id_float", "se0conn",
>  				"vbusvld", "sessvld", "sessend",
> -				"se1", "dm", "dp";
> +				"chrg_det", "dm", "dp";

[Severity: High]
Does replacing the "se1" interrupt name outright instead of appending it
break backward compatibility?

If a new kernel boots with an older device tree (which provides "se1"
instead of "chrg_det"), probe aborts. The driver strictly requires all
interrupts to be present:

drivers/phy/motorola/phy-cpcap-usb.c:cpcap_phy_irqs:
	/* REG_INT1 */
	"se0conn", "vbusvld", "sessvld", "sessend", "chrg_det",

And in cpcap_usb_init_irq():
	irq = platform_get_irq_byname(pdev, name);
	if (irq < 0)
		return -ENODEV;

Probe is aborted if "chrg_det" is not found, without attempting to fall
back to "se1".

[Severity: High]
Can a transient error during PHY mode switching in the driver permanently
disable sysrq?

In drivers/phy/motorola/phy-cpcap-usb.c:cpcap_usb_set_idle_mode(), sysrq is
disabled at the start:

	sysrq = sysrq_mask();
	if (sysrq & 1)
		sysrq_toggle_support(sysrq & ~1);

	error = cpcap_usb_gpio_set_mode(ddata, CPCAP_UNKNOWN_DISABLED);
	if (error)
		goto out_err;

If hardware configuration fails, the function skips the restoration block:

	if (sysrq & 1)
		sysrq_toggle_support(sysrq);

	return 0;

out_err:
	dev_err(ddata->dev, "%s failed with %i\n", __func__, error);
	return error;

Does this silently strip the system of its diagnostic capabilities on a
single I2C/SPI bus error?

[Severity: Medium]
Could a concurrent modification to the cpcap_enable_uart module parameter
cause a mixed hardware state?

In drivers/phy/motorola/phy-cpcap-usb.c:cpcap_usb_set_idle_mode(), the global
cpcap_enable_uart is queried independently multiple times:

	if (cpcap_enable_uart && ddata->pins_uart) {
		error = pinctrl_select_state(ddata->pins,  ddata->pins_uart);
...
	if (cpcap_enable_uart)
		val |= (CPCAP_BIT_UARTMUX0 | CPCAP_BIT_EMUMODE0);
...
	if (!cpcap_enable_uart)
		val |= CPCAP_BIT_SUSPEND_SPI;

If modified during execution, could this read true during the pinctrl check
and false during the regmap bitmask construction, leaving the hardware in an
inconsistent configuration?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704073843.1750458-1-ivo.g.dimitrov.75@gmail.com?part=4