From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Hang Cao <caohang@eswincomputing.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>,
Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"robh@kernel.org" <robh@kernel.org>,
"krzk+dt@kernel.org" <krzk+dt@kernel.org>,
"conor+dt@kernel.org" <conor+dt@kernel.org>,
"p.zabel@pengutronix.de" <p.zabel@pengutronix.de>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"ningyu@eswincomputing.com" <ningyu@eswincomputing.com>,
"linmin@eswincomputing.com" <linmin@eswincomputing.com>,
"pinkesh.vaghela@einfochips.com" <pinkesh.vaghela@einfochips.com>,
Senchuan Zhang <zhangsenchuan@eswincomputing.com>
Subject: Re: [PATCH v5 2/2] usb: dwc3: eic7700: Add EIC7700 USB driver
Date: Wed, 5 Nov 2025 23:41:18 +0000 [thread overview]
Message-ID: <20251105234102.aet5y4qrennagpkg@synopsys.com> (raw)
In-Reply-To: <9e50dfa.f63.19a53fcd7ec.Coremail.caohang@eswincomputing.com>
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
prev parent reply other threads:[~2025-11-05 23:41 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-04 6:50 [PATCH v5 0/2] Add driver support for ESWIN EIC7700 SoC USB controller caohang
2025-11-04 6:52 ` [PATCH v5 1/2] dt-bindings: usb: Add ESWIN EIC7700 " caohang
2025-11-04 8:26 ` Rob Herring (Arm)
2025-11-04 6:52 ` [PATCH v5 2/2] usb: dwc3: eic7700: Add EIC7700 USB driver caohang
2025-11-04 7:59 ` Krzysztof Kozlowski
2025-11-05 12:27 ` Hang Cao
2025-11-05 23:41 ` Thinh Nguyen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251105234102.aet5y4qrennagpkg@synopsys.com \
--to=thinh.nguyen@synopsys.com \
--cc=caohang@eswincomputing.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=krzk+dt@kernel.org \
--cc=krzk@kernel.org \
--cc=linmin@eswincomputing.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=ningyu@eswincomputing.com \
--cc=p.zabel@pengutronix.de \
--cc=pinkesh.vaghela@einfochips.com \
--cc=robh@kernel.org \
--cc=zhangsenchuan@eswincomputing.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox