Re: [PATCH v4 2/2] usb: dwc3: dwc3-generic-plat: Add ST STM32MP2 DWC3 xHCI USB controller glue

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] STM32MP25 USB3 syscfg register configuration is permanently lost after a suspend/resume cycle.
--

commit e3287a65c05b7a190b49e2a633f70e39f4bce365
Author: Marek Vasut <marex@nabladev.com>

usb: dwc3: dwc3-generic-plat: Add ST STM32MP2 DWC3 xHCI USB controller glue
    
The ST STM32MP2 SoC contains one instance of DWC3 USB controller with
a glue logic wrapper around it controlled by syscon. This commit extends
the generic DWC3 platform driver with ST STM32MP2 glue logic specifics.

> diff --git a/drivers/usb/dwc3/dwc3-generic-plat.c b/drivers/usb/dwc3/dwc3-generic-plat.c
> index ca69ac0eb07ce..bb2be75a82875 100644
> --- a/drivers/usb/dwc3/dwc3-generic-plat.c
> +++ b/drivers/usb/dwc3/dwc3-generic-plat.c

[ ... ]

> @@ -85,6 +92,38 @@ static int dwc3_spacemit_k1_init(struct dwc3_generic *dwc3g)
>  	return 0;
>  }
>  
> +static int dwc3_stm32mp25_init(struct dwc3_generic *dwc3g)
> +{
> +	struct device *dev = dwc3g->dev;
> +	bool ovrcur_polarity_low = device_property_read_bool(dev, "over-current-active-low");
> +	bool prt_pwr_ctrl = device_property_read_bool(dev, "st,enable-port-power-control");
> +	bool usb2only_conf = device_property_match_string(dev, "phy-names", "usb3-phy") < 0;
> +	bool vbusen_polarity_low = device_property_read_bool(dev, "st,vbusen-active-low");
> +	struct regmap *regmap;
> +	u32 drcr;
> +
> +	regmap = syscon_regmap_lookup_by_phandle_args(dev->of_node, "st,syscfg", 1, &drcr);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap), "No st,syscfg phandle specified\n");
> +
> +	return regmap_update_bits(regmap, drcr,
> +				  STM32MP2_USB3DRCR_HOST_PORT_POWER_CONTROL_PRESENT |
> +				  STM32MP2_USB3DRCR_OVRCUR_POLARITY |
> +				  STM32MP2_USB3DRCR_VBUSEN_POLARITY |
> +				  STM32MP2_USB3DRCR_USB2ONLYD |
> +				  STM32MP2_USB3DRCR_USB2ONLYH,
> +				  FIELD_PREP(STM32MP2_USB3DRCR_HOST_PORT_POWER_CONTROL_PRESENT,
> +					     prt_pwr_ctrl) |
> +				  FIELD_PREP(STM32MP2_USB3DRCR_OVRCUR_POLARITY,
> +					     ovrcur_polarity_low) |
> +				  FIELD_PREP(STM32MP2_USB3DRCR_VBUSEN_POLARITY,
> +					     vbusen_polarity_low) |
> +				  FIELD_PREP(STM32MP2_USB3DRCR_USB2ONLYD,
> +					     !!usb2only_conf) |
> +				  FIELD_PREP(STM32MP2_USB3DRCR_USB2ONLYH,
> +					     !!usb2only_conf));

[Severity: Medium]
Will this STM32MP25 USB3 syscfg register configuration be permanently lost
after a system suspend and resume cycle?

This function configures vital USB hardware behaviors (over-current polarity,
VBUS polarity, power control, USB2-only mode) via a syscfg regmap update
during probe.

However, dwc3_generic_resume() does not appear to re-invoke the initialization
callback or provide a mechanism to restore this state on resume:

static int dwc3_generic_resume(struct device *dev)
{
...
	ret = clk_bulk_prepare_enable(dwc3g->num_clocks, dwc3g->clks);
...
	ret = dwc3_pm_resume(dwc);
...
}

If this configuration is lost during deep sleep, could this regression
cause the USB port to fail to function correctly after resuming?

> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828150937.174735-1-marex@nabladev.com?part=2