linux-kernel - Re: [PATCH v5 2/2] usb: dwc3: eic7700: Add EIC7700 USB driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20251105234102.aet5y4qrennagpkg@synopsys.com>
Date: Wed, 5 Nov 2025 23:41:18 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Hang Cao <caohang@...incomputing.com>
CC: Krzysztof Kozlowski <krzk@...nel.org>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "robh@...nel.org" <robh@...nel.org>,
        "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
        "conor+dt@...nel.org" <conor+dt@...nel.org>,
        "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "ningyu@...incomputing.com" <ningyu@...incomputing.com>,
        "linmin@...incomputing.com" <linmin@...incomputing.com>,
        "pinkesh.vaghela@...fochips.com" <pinkesh.vaghela@...fochips.com>,
        Senchuan Zhang <zhangsenchuan@...incomputing.com>
Subject: Re: [PATCH v5 2/2] usb: dwc3: eic7700: Add EIC7700 USB driver

Hi Hang,

On Wed, Nov 05, 2025, Hang Cao wrote:
> Hi, Krzysztof and Thinh:
> > 
> > > +
> > >  static int dwc3_generic_probe(struct platform_device *pdev)
> > >  {
> > >  	const struct dwc3_properties *properties;
> > > @@ -83,6 +119,12 @@ static int dwc3_generic_probe(struct platform_device *pdev)
> > >  	else
> > >  		probe_data.properties = DWC3_DEFAULT_PROPERTIES;
> > >  
> > > +	if (of_device_is_compatible(dev->of_node, "eswin,eic7700-dwc3")) {
> > 
> > No, you have driver match data for that.
> > 
> We implemented it with driver match data in v4 patch.
> https://urldefense.com/v3/__https://lore.kernel.org/all/20251016222713.d2sutc7tyf2idbkv@synopsys.com/__;!!A4F2R9G_pg!bnU1bsEHivo93LGmhV68yL2IecKrcNCZfKvMRBDFZsvpkszq5uEaNlbvSmSjjgk_riIJALl21T0yq5yhBn9G0gCeWPQ$ 
> 
> However, Thinh suggested using direct function calls instead, noting 
> that this is a function call rather than data. 
> We are not sure if we’ve fully understood his feedback correctly.
> 
> Our driver requires special handling for USB bus initialization, which does 
> involve function calls.
> 
> So we’d really appreciate it if you and Thinh could share further thoughts 
> on which approach would be more suitable for our driver’s needs.
> 

I wanted to avoid modifying the struct dwc3_probe_data. It's only meant
to be used for passing data to the core. If we want to get the custom
init function through the driver match data within the same glue driver,
we can just create a new struct to pass that. This keeps the code a bit
cleaner.

You can try the following (not tested):

diff --git a/drivers/usb/dwc3/dwc3-generic-plat.c b/drivers/usb/dwc3/dwc3-generic-plat.c
index e869c7de7bc8..c3541fefe959 100644
--- a/drivers/usb/dwc3/dwc3-generic-plat.c
+++ b/drivers/usb/dwc3/dwc3-generic-plat.c
@@ -20,6 +20,11 @@ struct dwc3_generic {
 	struct reset_control	*resets;
 };
 
+struct dwc3_plat_config {
+	int (*init)(struct dwc3_generic *dwc3g);
+	struct dwc3_properties properties;
+};
+
 #define to_dwc3_generic(d) container_of((d), struct dwc3_generic, dwc)
 
 static void dwc3_generic_reset_control_assert(void *data)
@@ -27,9 +32,15 @@ static void dwc3_generic_reset_control_assert(void *data)
 	reset_control_assert(data);
 }
 
+static int dwc3_eic7700_init(struct dwc3_generic *dwc3g)
+{
+	/* init code goes here */
+	return 0;
+}
+
 static int dwc3_generic_probe(struct platform_device *pdev)
 {
-	const struct dwc3_properties *properties;
+	const struct dwc3_plat_config *plat_config;
 	struct dwc3_probe_data probe_data = {};
 	struct device *dev = &pdev->dev;
 	struct dwc3_generic *dwc3g;
@@ -77,12 +88,21 @@ static int dwc3_generic_probe(struct platform_device *pdev)
 	probe_data.res = res;
 	probe_data.ignore_clocks_and_resets = true;
 
-	properties = of_device_get_match_data(dev);
-	if (properties)
-		probe_data.properties = *properties;
-	else
+	plat_config = of_device_get_match_data(dev);
+	if (!plat_config) {
 		probe_data.properties = DWC3_DEFAULT_PROPERTIES;
+		goto core_probe;
+	}
+
+	probe_data.properties = plat_config->properties;
+
+	if (plat_config->init) {
+		ret = plat_config->init(dwc3g);
+		if (ret)
+			return dev_err_probe(dev, ret, "platform init failed\n");
+	}
 
+core_probe:
 	ret = dwc3_core_probe(&probe_data);
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to register DWC3 Core\n");
@@ -150,13 +170,19 @@ static const struct dev_pm_ops dwc3_generic_dev_pm_ops = {
 		       dwc3_generic_runtime_idle)
 };
 
-static const struct dwc3_properties fsl_ls1028_dwc3 = {
-	.gsbuscfg0_reqinfo = 0x2222,
+static const struct dwc3_plat_config fsl_ls1028_dwc3 = {
+	.properties.gsbuscfg0_reqinfo = 0x2222,
+};
+
+static const struct dwc3_plat_config eic7700_dwc3 = {
+	.init = dwc3_eic7700_init,
+	.properties = DWC3_DEFAULT_PROPERTIES,
 };
 
 static const struct of_device_id dwc3_generic_of_match[] = {
 	{ .compatible = "spacemit,k1-dwc3", },
 	{ .compatible = "fsl,ls1028a-dwc3", &fsl_ls1028_dwc3},
+	{ .compatible = "eswin,eic7700-dwc3", &eic7700_dwc3},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, dwc3_generic_of_match);

---

Side note, can you provide more context in the commit message of this
new device? (e.g. DWC_usb3 IP, device mode only, highspeed etc.). It'd
help whenever I need to revisit or considering new changes.

Thanks,
Thinh