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

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

On Fri, Aug 28, 2026, Marek Vasut wrote:
> The ST STM32MP2 SoC contains one instance of DWC3 USB controller with
> glue logic wrapper around it controlled by syscon. Extend the generic
> DWC3 platform driver with ST STM32MP2 glue logic specifics.
> 
> Signed-off-by: Marek Vasut <marex@nabladev.com>
> ---
> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Marek Vasut <marex@nabladev.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> Cc: devicetree@vger.kernel.org
> Cc: kernel@dh-electronics.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> Cc: linux-usb@vger.kernel.org
> ---
> V2: No change
> V3: - Use generic over-current-active-low
> V4: - Include bitfield.h
>     - Split this from series arm64: dts: phy: st: usb: Add STM32MP2 USB support
>       https://urldefense.com/v3/__https://lore.kernel.org/all/20260822074816.548662-1-marex@nabladev.com/__;!!A4F2R9G_pg!YlDeUOOZwFCheRmIge9bWFuXVhAXksoyGKgQWm0l0Kh_GhAze-2XweqeU0l3QsxZhXtx0XUGAdIEiXBk14Tq$ 
> ---
>  drivers/usb/dwc3/dwc3-generic-plat.c | 45 ++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> 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
> @@ -7,6 +7,7 @@
>   * Inspired by dwc3-qcom.c and dwc3-of-simple.c
>   */
>  
> +#include <linux/bitfield.h>
>  #include <linux/clk.h>
>  #include <linux/platform_device.h>
>  #include <linux/reset.h>
> @@ -22,6 +23,12 @@
>  #define EIC7700_HSP_AXI_LP_XM_CSYSREQ	BIT(0)
>  #define EIC7700_HSP_AXI_LP_XS_CSYSREQ	BIT(16)
>  
> +#define STM32MP2_USB3DRCR_HOST_PORT_POWER_CONTROL_PRESENT	BIT(0)
> +#define STM32MP2_USB3DRCR_OVRCUR_POLARITY			BIT(1)
> +#define STM32MP2_USB3DRCR_VBUSEN_POLARITY			BIT(2)
> +#define STM32MP2_USB3DRCR_USB2ONLYH				BIT(3)
> +#define STM32MP2_USB3DRCR_USB2ONLYD				BIT(4)
> +
>  struct dwc3_generic {
>  	struct device		*dev;
>  	struct dwc3		dwc;
> @@ -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;

Minor nit: this is hard to see when buried among the others.
Can we split this out? e.g.:

	bool usb2only_conf = false;

	if (device_property_match_string(dev, "phy-names", "usb3-phy") < 0)
		usb2only_conf = true;

> +	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));

Minor nit: usb2only_conf is already a bool, so the "!!" is redundant.
Can you remove it like the other 3 you have right above?

The rest looks fine to me.

You can add this after the minor nits:

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh