linux-kernel - Re: [PATCH v6 2/2] usb: dwc3: add generic driver to support flattened

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: <aH7cpr0faRPVnxXL@pie>
Date: Tue, 22 Jul 2025 00:34:46 +0000
From: Yao Zi <ziyao@...root.org>
To: Ze Huang <huang.ze@...ux.dev>, Alex Elder <elder@...e.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Yixun Lan <dlan@...too.org>,
	Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
	Philipp Zabel <p.zabel@...gutronix.de>
Cc: linux-usb@...r.kernel.org, devicetree@...r.kernel.org,
	linux-riscv@...ts.infradead.org, spacemit@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 2/2] usb: dwc3: add generic driver to support flattened

On Mon, Jul 21, 2025 at 08:08:06PM +0800, Ze Huang wrote:
> On Sun, Jul 20, 2025 at 02:34:07PM +0800, Ze Huang wrote:
> > On Tue, Jul 15, 2025 at 03:50:54PM -0500, Alex Elder wrote:
> > > On 7/12/25 2:49 AM, Ze Huang wrote:
> > > > To support flattened dwc3 dt model and drop the glue layer, introduce the
> > > > `dwc3-generic` driver. This enables direct binding of the DWC3 core driver
> > > > and offers an alternative to the existing glue driver `dwc3-of-simple`.
> > > 
> > > I'm not familiar with dwc-of-simple.c, and won't comment on
> > > how this differs from that (or does not).
> > > 
> > > Given you're implementing an alternative though, can you explain
> > > in a little more detail what's different between the two?  Why
> > > would someone choose to use this driver rather than the other one?
> > 
> > They are basically the same.
> > 
> > dwc-generic use a plain dt node while dwc-of-simple will nest the dwc3
> > node as its child.
> > 
> > Both will use dwc3_core_probe() to finish the probe process. But now we
> > can simplify the process by just calling it, instead of calling
> > of_platform_populate() and create another snps,dwc3 device driver.
> 
> [...]
> 
> > > > +	ret = reset_control_assert(dwc3->resets);
> > > > +	if (ret)
> > > > +		return dev_err_probe(dev, ret, "failed to assert resets\n");
> > > > +
> > > > +	ret = devm_add_action_or_reset(dev, dwc3_generic_reset_control_assert, dwc3->resets);
> > > > +	if (ret)
> > > > +		return ret;
> > > 
> > > The re-assert shouldn't be set up unless the deassert below
> > > succeeds.
> > > 
> > 
> > Will move behind the deassert.
> > 
> > > > +	usleep_range(10, 1000);
> > > 
> > > This seems like a large range.  You could just do msleep(1);
> > > Also, can you add a comment explaining why a delay is needed,
> > > and why 1 millisecond is the right amount of time to sleep?
> > > 
> > 
> > I will check the range with spacemit and reply soon.
> > 
> 
> the resets are asynchronous with no strict timing. But to be safe, each
> reset should stay active for at least 1 µs. I’ll switch to a udelay(2)
> and add comment accordingly.

This may be a little farsight: do you think it's better to make the
reset timing part of the of_match_data? This is more flexible and
reduces future burden when introducing a new platform that comes with a
different reset timing, which is a very likely case we'll face since
it's a "generic" driver.

> > > > +	ret = reset_control_deassert(dwc3->resets);
> > > > +	if (ret)
> > > > +		return dev_err_probe(dev, ret, "failed to deassert resets\n");
> > > > +
> > > > +	ret = devm_clk_bulk_get_all(dwc3->dev, &dwc3->clks);
> > > > +	if (ret < 0)
> > > > +		return dev_err_probe(dev, ret, "failed to get clocks\n");
> > > 
> > > Call devm_clk_bulk_get_all_enabled() instead of doing the two
> > > steps separately here.
> > > 
> > 
> > Will do, thanks.
> > 
> > > 					-Alex
> 

Regards,
Yao Zi