Message-ID: <e7esebdau6ebhym7m5ce5r6eraobgto4chklvmoezaw2b4wp56@svv73ahucaqo>
Date: Wed, 11 Feb 2026 18:33:30 +0800
From: Xu Yang <xu.yang_2@....com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>, 
	Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>, 
	Fabio Estevam <festevam@...il.com>, Frank Li <frank.li@....com>, Li Jun <jun.li@....com>, 
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>, "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, 
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>, 
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 3/3] usb: dwc3: introduce flatten model driver of i.MX
 Soc

On Tue, Feb 10, 2026 at 02:34:50AM +0000, Thinh Nguyen wrote:
> On Fri, Feb 06, 2026, Xu Yang wrote:
> > The i.MX USB glue and DWC3 core are closely coupled. Describe the i.MX
> > USB block in a single block will bring more benefits than a parent-
> > child relation. To support the flatten model devicetree, DWC3 USB core
> > driver already support to directly register and initialize the core in
> > glue layer using one device. And many notification can be received in
> > glue layer timely and proper actions can be executed accordingly.
> > 
> > To align with mainstream, introduce a new driver to support flatten dwc3
> > devicetree model for i.MX Soc. Besides this driver disabling wakeup irq
> > when system is active, no other function change in this version compared
> > to dwc3-imx8mp.c
> 
> Going forward, what will happen to the legacy driver? Can we only expect
> maintenance fixes rather than new feature development? Should we
> document this transition policy in the commit message?

OK, I'll add more such information in the commit message.

> 
> > 
> > Signed-off-by: Xu Yang <xu.yang_2@....com>
> > 
> > ---
> > Changes in v3:
> >  - update compatible as nxp,imx8mp-dwc3
> > Changes in v2:
> >  - improve commit message
> >  - fix code style
> >  - add IRQF_NO_AUTOEN
> >  - remove pm_runtime_* in dwc3_imx_remove(), dwc3_core_remove()
> >    will do that
> > ---
> >  drivers/usb/dwc3/Kconfig    |  12 ++
> >  drivers/usb/dwc3/Makefile   |   1 +
> >  drivers/usb/dwc3/dwc3-imx.c | 429 ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 442 insertions(+)
> > 
> > diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> > index 240b15bc52cb..18169727a413 100644
> > --- a/drivers/usb/dwc3/Kconfig
> > +++ b/drivers/usb/dwc3/Kconfig
> > @@ -150,6 +150,18 @@ config USB_DWC3_IMX8MP
> >  	  functionality.
> >  	  Say 'Y' or 'M' if you have one such device.
> >  
> > +config USB_DWC3_IMX
> > +	tristate "NXP iMX Platform"
> > +	depends on OF && COMMON_CLK
> > +	depends on (ARCH_MXC && ARM64) || COMPILE_TEST
> > +	default USB_DWC3
> > +	help
> > +	  NXP iMX SoC use DesignWare Core IP for USB2/3
> > +	  functionality.
> > +	  This driver also handles the wakeup feature outside
> > +	  of DesignWare Core.
> > +	  Say 'Y' or 'M' if you have one such device.
> > +
> >  config USB_DWC3_XILINX
> >  	tristate "Xilinx Platforms"
> >  	depends on (ARCH_ZYNQMP || COMPILE_TEST) && OF
> > diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> > index 073bef5309b5..f37971197203 100644
> > --- a/drivers/usb/dwc3/Makefile
> > +++ b/drivers/usb/dwc3/Makefile
> > @@ -55,6 +55,7 @@ obj-$(CONFIG_USB_DWC3_ST)		+= dwc3-st.o
> >  obj-$(CONFIG_USB_DWC3_QCOM)		+= dwc3-qcom.o
> >  obj-$(CONFIG_USB_DWC3_QCOM)		+= dwc3-qcom-legacy.o
> >  obj-$(CONFIG_USB_DWC3_IMX8MP)		+= dwc3-imx8mp.o
> > +obj-$(CONFIG_USB_DWC3_IMX)		+= dwc3-imx.o
> >  obj-$(CONFIG_USB_DWC3_XILINX)		+= dwc3-xilinx.o
> >  obj-$(CONFIG_USB_DWC3_OCTEON)		+= dwc3-octeon.o
> >  obj-$(CONFIG_USB_DWC3_RTK)		+= dwc3-rtk.o
> > diff --git a/drivers/usb/dwc3/dwc3-imx.c b/drivers/usb/dwc3/dwc3-imx.c
> > new file mode 100644
> > index 000000000000..3b154d075bcf
> > --- /dev/null
> > +++ b/drivers/usb/dwc3/dwc3-imx.c
> > @@ -0,0 +1,429 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * dwc3-imx.c - NXP i.MX Soc USB3 Specific Glue layer
> > + *
> > + * Copyright 2026 NXP
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +
> > +#include "core.h"
> > +#include "glue.h"
> > +
> > +/* USB wakeup registers */
> > +#define USB_WAKEUP_CTRL			0x00
> > +
> > +/* Global wakeup interrupt enable, also used to clear interrupt */
> > +#define USB_WAKEUP_EN			BIT(31)
> > +/* Wakeup from connect or disconnect, only for superspeed */
> > +#define USB_WAKEUP_SS_CONN		BIT(5)
> > +/* 0 select vbus_valid, 1 select sessvld */
> > +#define USB_WAKEUP_VBUS_SRC_SESS_VAL	BIT(4)
> > +/* Enable signal for wake up from u3 state */
> > +#define USB_WAKEUP_U3_EN		BIT(3)
> > +/* Enable signal for wake up from id change */
> > +#define USB_WAKEUP_ID_EN		BIT(2)
> > +/* Enable signal for wake up from vbus change */
> > +#define	USB_WAKEUP_VBUS_EN		BIT(1)
> > +/* Enable signal for wake up from dp/dm change */
> > +#define USB_WAKEUP_DPDM_EN		BIT(0)
> > +
> > +#define USB_WAKEUP_EN_MASK		GENMASK(5, 0)
> > +
> > +/* USB glue registers */
> > +#define USB_CTRL0		0x00
> > +#define USB_CTRL1		0x04
> > +
> > +#define USB_CTRL0_PORTPWR_EN	BIT(12) /* 1 - PPC enabled (default) */
> > +#define USB_CTRL0_USB3_FIXED	BIT(22) /* 1 - USB3 permanent attached */
> > +#define USB_CTRL0_USB2_FIXED	BIT(23) /* 1 - USB2 permanent attached */
> > +
> > +#define USB_CTRL1_OC_POLARITY	BIT(16) /* 0 - HIGH / 1 - LOW */
> > +#define USB_CTRL1_PWR_POLARITY	BIT(17) /* 0 - HIGH / 1 - LOW */
> > +
> > +struct dwc3_imx {
> > +	struct dwc3	dwc;
> > +	struct device	*dev;
> > +	void __iomem	*blkctl_base;
> > +	void __iomem	*glue_base;
> > +	struct clk	*hsio_clk;
> > +	struct clk	*suspend_clk;
> > +	int		irq;
> > +	bool		pm_suspended;
> > +	bool		wakeup_pending;
> > +};
> > +
> > +#define to_dwc3_imx(d) container_of((d), struct dwc3_imx, dwc)
> > +
> > +static void dwc3_imx_configure_glue(struct dwc3_imx *dwc_imx)
> > +{
> > +	struct device	*dev = dwc_imx->dev;
> > +	u32		value;
> > +
> > +	if (!dwc_imx->glue_base)
> > +		return;
> > +
> > +	value = readl(dwc_imx->glue_base + USB_CTRL0);
> > +
> > +	if (device_property_read_bool(dev, "fsl,permanently-attached"))
> > +		value |= USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED;
> > +	else
> > +		value &= ~(USB_CTRL0_USB2_FIXED | USB_CTRL0_USB3_FIXED);
> > +
> > +	if (device_property_read_bool(dev, "fsl,disable-port-power-control"))
> > +		value &= ~USB_CTRL0_PORTPWR_EN;
> > +	else
> > +		value |= USB_CTRL0_PORTPWR_EN;
> > +
> > +	writel(value, dwc_imx->glue_base + USB_CTRL0);
> > +
> > +	value = readl(dwc_imx->glue_base + USB_CTRL1);
> > +	if (device_property_read_bool(dev, "fsl,over-current-active-low"))
> > +		value |= USB_CTRL1_OC_POLARITY;
> > +	else
> > +		value &= ~USB_CTRL1_OC_POLARITY;
> > +
> > +	if (device_property_read_bool(dev, "fsl,power-active-low"))
> > +		value |= USB_CTRL1_PWR_POLARITY;
> > +	else
> > +		value &= ~USB_CTRL1_PWR_POLARITY;
> > +
> > +	writel(value, dwc_imx->glue_base + USB_CTRL1);
> > +}
> > +
> > +static void dwc3_imx_wakeup_enable(struct dwc3_imx *dwc_imx, pm_message_t msg)
> > +{
> > +	struct dwc3	*dwc = &dwc_imx->dwc;
> > +	u32		val;
> > +
> > +	val = readl(dwc_imx->blkctl_base + USB_WAKEUP_CTRL);
> > +
> > +	if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST && dwc->xhci) {
> > +		val |= USB_WAKEUP_EN | USB_WAKEUP_DPDM_EN;
> > +		if (PMSG_IS_AUTO(msg))
> > +			val |= USB_WAKEUP_SS_CONN | USB_WAKEUP_U3_EN;
> > +	} else {
> > +		val |= USB_WAKEUP_EN | USB_WAKEUP_VBUS_EN |
> > +		       USB_WAKEUP_VBUS_SRC_SESS_VAL;
> > +	}
> > +
> > +	writel(val, dwc_imx->blkctl_base + USB_WAKEUP_CTRL);
> > +}
> > +
> > +static void dwc3_imx_wakeup_disable(struct dwc3_imx *dwc_imx)
> > +{
> > +	u32	val;
> > +
> > +	val = readl(dwc_imx->blkctl_base + USB_WAKEUP_CTRL);
> > +	val &= ~(USB_WAKEUP_EN | USB_WAKEUP_EN_MASK);
> > +	writel(val, dwc_imx->blkctl_base + USB_WAKEUP_CTRL);
> > +}
> > +
> > +static irqreturn_t dwc3_imx_interrupt(int irq, void *data)
> > +{
> > +	struct dwc3_imx	*dwc_imx = data;
> > +	struct dwc3	*dwc = &dwc_imx->dwc;
> > +
> > +	if (!dwc_imx->pm_suspended)
> > +		return IRQ_HANDLED;
> > +
> > +	disable_irq_nosync(dwc_imx->irq);
> > +	dwc_imx->wakeup_pending = true;
> > +
> > +	if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST && dwc->xhci)
> > +		pm_runtime_resume(&dwc->xhci->dev);
> > +	else if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_DEVICE)
> > +		pm_runtime_get(dwc->dev);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static void dwc3_imx_pre_set_role(struct dwc3 *dwc, enum usb_role role)
> > +{
> > +	if (role == USB_ROLE_HOST)
> > +		/*
> > +		 * For xhci host, we need disable dwc core auto
> > +		 * suspend, because during this auto suspend delay(5s),
> > +		 * xhci host RUN_STOP is cleared and wakeup is not
> > +		 * enabled, if device is inserted, xhci host can't
> > +		 * response the connection.
> > +		 */
> > +		pm_runtime_dont_use_autosuspend(dwc->dev);
> > +	else
> > +		pm_runtime_use_autosuspend(dwc->dev);
> > +}
> > +
> > +static struct dwc3_glue_ops dwc3_imx_glue_ops = {
> > +	.pre_set_role = dwc3_imx_pre_set_role,
> > +};
> > +
> > +static const struct property_entry dwc3_imx_properties[] = {
> > +	PROPERTY_ENTRY_BOOL("xhci-missing-cas-quirk"),
> > +	PROPERTY_ENTRY_BOOL("xhci-skip-phy-init-quirk"),
> > +	{},
> > +};
> > +
> > +static const struct software_node dwc3_imx_swnode = {
> > +	.properties = dwc3_imx_properties,
> > +};
> > +
> > +static int dwc3_imx_probe(struct platform_device *pdev)
> > +{
> > +	struct device		*dev = &pdev->dev;
> > +	struct dwc3_imx		*dwc_imx;
> > +	struct dwc3		*dwc;
> > +	struct resource		*res;
> > +	const char		*irq_name;
> > +	struct dwc3_probe_data	probe_data = {};
> > +	int			ret, irq;
> > +
> > +	dwc_imx = devm_kzalloc(dev, sizeof(*dwc_imx), GFP_KERNEL);
> > +	if (!dwc_imx)
> > +		return -ENOMEM;
> > +
> > +	platform_set_drvdata(pdev, dwc_imx);
> > +	dwc_imx->dev = dev;
> > +
> > +	dwc_imx->blkctl_base = devm_platform_ioremap_resource_byname(pdev, "blkctl");
> > +	if (IS_ERR(dwc_imx->blkctl_base))
> > +		return PTR_ERR(dwc_imx->blkctl_base);
> > +
> > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "glue");
> > +	if (!res) {
> > +		dev_warn(dev, "Base address for glue layer missing\n");
> > +	} else {
> > +		dwc_imx->glue_base = devm_ioremap_resource(dev, res);
> > +		if (IS_ERR(dwc_imx->glue_base))
> > +			return PTR_ERR(dwc_imx->glue_base);
> > +	}
> > +
> > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "core");
> > +	if (!res)
> > +		return dev_err_probe(dev, -ENODEV, "missing core memory resource\n");
> > +
> > +	dwc_imx->hsio_clk = devm_clk_get_enabled(dev, "hsio");
> > +	if (IS_ERR(dwc_imx->hsio_clk))
> > +		return dev_err_probe(dev, PTR_ERR(dwc_imx->hsio_clk),
> > +				     "Failed to get hsio clk\n");
> > +
> > +	dwc_imx->suspend_clk = devm_clk_get_enabled(dev, "suspend");
> > +	if (IS_ERR(dwc_imx->suspend_clk))
> > +		return dev_err_probe(dev, PTR_ERR(dwc_imx->suspend_clk),
> > +				     "Failed to get suspend clk\n");
> > +
> > +	irq = platform_get_irq_byname(pdev, "wakeup");
> > +	if (irq < 0)
> > +		return irq;
> > +	dwc_imx->irq = irq;
> > +
> > +	irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s:wakeup", dev_name(dev));
> > +	if (!irq_name)
> > +		return dev_err_probe(dev, -ENOMEM, "failed to create irq_name\n");
> > +
> > +	ret = devm_request_threaded_irq(dev, irq, NULL, dwc3_imx_interrupt,
> > +					IRQF_ONESHOT | IRQF_NO_AUTOEN,
> > +					irq_name, dwc_imx);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "failed to request IRQ #%d\n", irq);
> > +
> > +	ret = device_add_software_node(dev, &dwc3_imx_swnode);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "failed to add software node\n");
> > +
> > +	dwc = &dwc_imx->dwc;
> > +	dwc->dev = dev;
> > +	dwc->glue_ops = &dwc3_imx_glue_ops;
> > +
> > +	probe_data.res = res;
> > +	probe_data.dwc = dwc;
> > +	probe_data.properties = DWC3_DEFAULT_PROPERTIES;
> > +	probe_data.properties.core_may_lose_power = true;
> > +
> > +	ret = dwc3_core_probe(&probe_data);
> > +	if (ret) {
> > +		device_remove_software_node(dev);
> > +		return ret;
> > +	}
> > +
> > +	device_set_wakeup_capable(dev, true);
> > +	return 0;
> > +}
> > +
> > +static void dwc3_imx_remove(struct platform_device *pdev)
> > +{
> > +	struct device	*dev = &pdev->dev;
> > +	struct dwc3	*dwc = dev_get_drvdata(dev);
> > +
> > +	dwc3_core_remove(dwc);
> > +	device_remove_software_node(dev);
> > +}
> > +
> > +static void dwc3_imx_suspend(struct dwc3_imx *dwc_imx, pm_message_t msg)
> > +{
> > +	if (PMSG_IS_AUTO(msg) || device_may_wakeup(dwc_imx->dev))
> > +		dwc3_imx_wakeup_enable(dwc_imx, msg);
> > +
> > +	enable_irq(dwc_imx->irq);
> > +	dwc_imx->pm_suspended = true;
> > +}
> > +
> > +static void dwc3_imx_resume(struct dwc3_imx *dwc_imx, pm_message_t msg)
> > +{
> > +	struct dwc3	*dwc = &dwc_imx->dwc;
> > +
> > +	dwc_imx->pm_suspended = false;
> > +	if (!dwc_imx->wakeup_pending)
> > +		disable_irq_nosync(dwc_imx->irq);
> > +
> > +	dwc3_imx_wakeup_disable(dwc_imx);
> > +
> > +	/* Upon power loss any previous configuration is lost, restore it */
> > +	dwc3_imx_configure_glue(dwc_imx);
> > +
> > +	if (dwc_imx->wakeup_pending) {
> > +		dwc_imx->wakeup_pending = false;
> > +		if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_DEVICE)
> > +			pm_runtime_put_autosuspend(dwc->dev);
> > +		else
> > +			/*
> > +			 * Add wait for xhci switch from suspend
> > +			 * clock to normal clock to detect connection.
> > +			 */
> > +			usleep_range(9000, 10000);
> > +	}
> > +}
> > +
> > +static int dwc3_imx_runtime_suspend(struct device *dev)
> > +{
> > +	struct dwc3	*dwc = dev_get_drvdata(dev);
> > +	struct dwc3_imx	*dwc_imx = to_dwc3_imx(dwc);
> > +	int		ret;
> > +
> > +	dev_dbg(dev, "at %s\n", __func__);
> 
> This debug message doesn't fit here. It should only belongs during
> development.

OK. I will remove them.

Thanks,
Xu Yang

Powered by blists - more mailing lists