Re: [PATCH v1 3/3] usb: dwc3-generic: Add Starfive JHB100 USB 2.0 device controller and adapter

[Thinh Nguyen]

(sorry for the delay)

On Fri, Sep 18, 2026, Minda Chen wrote:
> Add PHY or PHY adapter iomem field in struct dwc3_gerneric for
> dwc3 generic platform to add PHY/PHY adapter setting.
> 
> Also Add Starfive JHB100 USB 2.0 device controller generic init
> function to remap PHY adapter io memory region and add adapter
> pre-run/stop function.
> 
> JHB100 BMC soc contains Synopsys dwc3 multi-port host controller.
> And export its xhci controller to host server through PCIe. xhci
> contains 8 ports and each connect with one dwc3 device controller
> through an PHY adapter. Server host USB subsystem includes
> one multi-port dwc3 USB host, multiple dwc3 device controllers
> and their adapters.
> 
> While USB adapter 2.0 enable bit is set, the connect status bit of
> xhci port stat register will be asserted. If the usb enumeration is
> not ready for USB gadget, The xhci will fail with enumerate usb device.
> So The JHB100 USB device controller adapter require to set/clear
> USB 2.0 port enable while usb gadget start/stop usb device enumeration.
> This can be treated as a hardware quirk.
> 
> Signed-off-by: Minda Chen <minda.chen@xxxxxxxxxxxxxxxx>
> ---
>  drivers/usb/dwc3/dwc3-generic-plat.c | 47 +++++++++++++++++++++++++++-
>  1 file changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-generic-plat.c b/drivers/usb/dwc3/dwc3-generic-plat.c
> index ca69ac0eb07c..e3394205a519 100644
> --- a/drivers/usb/dwc3/dwc3-generic-plat.c
> +++ b/drivers/usb/dwc3/dwc3-generic-plat.c
> @@ -22,12 +22,15 @@
>  #define EIC7700_HSP_AXI_LP_XM_CSYSREQ	BIT(0)
>  #define EIC7700_HSP_AXI_LP_XS_CSYSREQ	BIT(16)
>  
> +#define JHB100_ADAPTER_PORT_EN		BIT(20)
> +
>  struct dwc3_generic {
>  	struct device		*dev;
>  	struct dwc3		dwc;
>  	struct clk_bulk_data	*clks;
>  	int			num_clocks;
>  	struct reset_control	*resets;
> +	void __iomem		*phy_regs; /* PHY or PHY adapter regs */

The name should be generic. Can we just use "regs" and drop the comment?

>  };
>  
>  struct dwc3_generic_config {
> @@ -42,6 +45,43 @@ static void dwc3_generic_reset_control_assert(void *data)
>  	reset_control_assert(data);
>  }
>  
> +static void dwc3_jhb100_pre_run_stop(struct dwc3 *dwc3, bool start)

Can we keep the dwc parameter name consistent with the rest of the dwc3
code.

> +{
> +	struct dwc3_generic *data = to_dwc3_generic(dwc3);
> +	u32	val;
> +
> +	if (start) {
> +		val = readl(data->phy_regs);
> +		val |= JHB100_ADAPTER_PORT_EN;
> +		writel(val, data->phy_regs);
> +	} else {
> +		val = readl(data->phy_regs);
> +		val &= ~JHB100_ADAPTER_PORT_EN;
> +		writel(val, data->phy_regs);
> +	}

Minor nit, simplify to this:
	val = readl(data->phy_regs);
	if (start)
		val |= JHB100_ADAPTER_PORT_EN;
	else
		val &= ~JHB100_ADAPTER_PORT_EN;

	writel(val, data->phy_regs);

> +}
> +
> +struct dwc3_glue_ops jhb100_glue_ops = {

Should this be static?

static struct dwc3_glue_ops jhb100_glue_ops

> +	.pre_run_stop = dwc3_jhb100_pre_run_stop,
> +};
> +
> +static int dwc3_jhb100_init(struct dwc3_generic *dwc3g)
> +{
> +	struct platform_device *pdev = to_platform_device(dwc3g->dev);
> +
> +	dwc3g->phy_regs = devm_platform_ioremap_resource_byname(pdev, "adapter");
> +	if (IS_ERR(dwc3g->phy_regs)) {

What if this fails for reasons other than resource not found? Please
properly check for error code.

> +		dwc3g->phy_regs = NULL;
> +		/* Generic bmc USB with PHY*/

Fix the comment missing space.

> +		return 0;
> +	}
> +
> +	/* usb device controller with adapter */
> +	dwc3g->dwc.glue_ops = &jhb100_glue_ops;
> +
> +	return 0;
> +}
> +
>  static int dwc3_eic7700_init(struct dwc3_generic *dwc3g)
>  {
>  	struct device *dev = dwc3g->dev;
> @@ -231,12 +271,17 @@ static const struct dwc3_generic_config eic7700_dwc3 =  {
>  	.properties = DWC3_DEFAULT_PROPERTIES,
>  };
>  
> +static const struct dwc3_generic_config jhb100_dwc3 =  {
> +	.init = dwc3_jhb100_init,
> +	.properties = DWC3_DEFAULT_PROPERTIES,
> +};
> +
>  static const struct of_device_id dwc3_generic_of_match[] = {
>  	{ .compatible = "spacemit,k1-dwc3", &spacemit_k1_dwc3},
>  	{ .compatible = "spacemit,k3-dwc3", },
>  	{ .compatible = "fsl,ls1028a-dwc3", &fsl_ls1028_dwc3},
>  	{ .compatible = "eswin,eic7700-dwc3", &eic7700_dwc3},
> -	{ .compatible = "starfive,jhb100-dwc3", },
> +	{ .compatible = "starfive,jhb100-dwc3", &jhb100_dwc3},
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, dwc3_generic_of_match);
> -- 
> 2.17.1
> 

The rest looks good to me.

Thanks,
Thinh