diff mbox series

[v2,4/7] usb: dwc3: core: Expose core driver as library

Message ID 20240811-dwc3-refactor-v2-4-91f370d61ad2@quicinc.com (mailing list archive)
State New
Headers show
Series usb: dwc3: qcom: Flatten dwc3 structure | expand

Commit Message

Bjorn Andersson Aug. 12, 2024, 3:12 a.m. UTC
From: Bjorn Andersson <quic_bjorande@quicinc.com>

The DWC3 IP block is handled by three distinct device drivers: XHCI,
DWC3 core and a platform specific (optional) DWC3 glue driver.

This has resulted in, at least in the case of the Qualcomm glue, the
presence of a number of layering violations, where the glue code either
can't handle, or has to work around, the fact that core might not probe
deterministically.

An example of this is that the suspend path should operate slightly
different depending on the device operating in host or peripheral mode,
and the only way to determine the operating state is to peek into the
core's drvdata.

The Qualcomm glue driver is expected to make updates in the qscratch
register region (the "glue" region) during role switch events, but with
the glue and core split using the driver model, there is no reasonable
way to introduce listeners for mode changes.

Split the dwc3 core platform_driver callbacks and their implementation
and export the implementation, to make it possible to deterministically
instantiate the dwc3 core as part of the dwc3 glue drivers and to
allow flattening of the DeviceTree representation.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 drivers/usb/dwc3/core.c | 169 +++++++++++++++++++++++++++++++-----------------
 drivers/usb/dwc3/core.h |   3 +
 2 files changed, 114 insertions(+), 58 deletions(-)

Comments

kernel test robot Aug. 12, 2024, 12:21 p.m. UTC | #1
Hi Bjorn,

kernel test robot noticed the following build errors:

[auto build test ERROR on 864b1099d16fc7e332c3ad7823058c65f890486c]

url:    https://github.com/intel-lab-lkp/linux/commits/Bjorn-Andersson/dt-bindings-usb-snps-dwc3-Split-core-description/20240812-111102
base:   864b1099d16fc7e332c3ad7823058c65f890486c
patch link:    https://lore.kernel.org/r/20240811-dwc3-refactor-v2-4-91f370d61ad2%40quicinc.com
patch subject: [PATCH v2 4/7] usb: dwc3: core: Expose core driver as library
config: i386-buildonly-randconfig-005-20240812 (https://download.01.org/0day-ci/archive/20240812/202408122011.adSQGOUp-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240812/202408122011.adSQGOUp-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408122011.adSQGOUp-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/usb/dwc3/core.c:39:10: fatal error: glue.h: No such file or directory
      39 | #include "glue.h"
         |          ^~~~~~~~
   compilation terminated.


vim +39 drivers/usb/dwc3/core.c

    36	
    37	#include "core.h"
    38	#include "gadget.h"
  > 39	#include "glue.h"
    40	#include "io.h"
    41
kernel test robot Aug. 12, 2024, 12:21 p.m. UTC | #2
Hi Bjorn,

kernel test robot noticed the following build errors:

[auto build test ERROR on 864b1099d16fc7e332c3ad7823058c65f890486c]

url:    https://github.com/intel-lab-lkp/linux/commits/Bjorn-Andersson/dt-bindings-usb-snps-dwc3-Split-core-description/20240812-111102
base:   864b1099d16fc7e332c3ad7823058c65f890486c
patch link:    https://lore.kernel.org/r/20240811-dwc3-refactor-v2-4-91f370d61ad2%40quicinc.com
patch subject: [PATCH v2 4/7] usb: dwc3: core: Expose core driver as library
config: i386-buildonly-randconfig-002-20240812 (https://download.01.org/0day-ci/archive/20240812/202408122054.h09BWBT2-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240812/202408122054.h09BWBT2-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408122054.h09BWBT2-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/usb/dwc3/core.c:39:10: fatal error: 'glue.h' file not found
      39 | #include "glue.h"
         |          ^~~~~~~~
   1 error generated.


vim +39 drivers/usb/dwc3/core.c

    36	
    37	#include "core.h"
    38	#include "gadget.h"
  > 39	#include "glue.h"
    40	#include "io.h"
    41
Bjorn Andersson Aug. 12, 2024, 7 p.m. UTC | #3
On Mon, Aug 12, 2024 at 08:21:54PM GMT, kernel test robot wrote:
> Hi Bjorn,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on 864b1099d16fc7e332c3ad7823058c65f890486c]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Bjorn-Andersson/dt-bindings-usb-snps-dwc3-Split-core-description/20240812-111102
> base:   864b1099d16fc7e332c3ad7823058c65f890486c
> patch link:    https://lore.kernel.org/r/20240811-dwc3-refactor-v2-4-91f370d61ad2%40quicinc.com
> patch subject: [PATCH v2 4/7] usb: dwc3: core: Expose core driver as library
> config: i386-buildonly-randconfig-005-20240812 (https://download.01.org/0day-ci/archive/20240812/202408122011.adSQGOUp-lkp@intel.com/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240812/202408122011.adSQGOUp-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202408122011.adSQGOUp-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
> >> drivers/usb/dwc3/core.c:39:10: fatal error: glue.h: No such file or directory
>       39 | #include "glue.h"
>          |          ^~~~~~~~
>    compilation terminated.

Sorry about that, seems I missed the git add on the header file where I
declare the exported symbols.

Regards,
Bjorn

> 
> 
> vim +39 drivers/usb/dwc3/core.c
> 
>     36	
>     37	#include "core.h"
>     38	#include "gadget.h"
>   > 39	#include "glue.h"
>     40	#include "io.h"
>     41	
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
Frank Li Aug. 13, 2024, 6:15 p.m. UTC | #4
On Sun, Aug 11, 2024 at 08:12:01PM -0700, Bjorn Andersson wrote:
> From: Bjorn Andersson <quic_bjorande@quicinc.com>
>
> The DWC3 IP block is handled by three distinct device drivers: XHCI,
> DWC3 core and a platform specific (optional) DWC3 glue driver.
>
> This has resulted in, at least in the case of the Qualcomm glue, the
> presence of a number of layering violations, where the glue code either
> can't handle, or has to work around, the fact that core might not probe
> deterministically.
>
> An example of this is that the suspend path should operate slightly
> different depending on the device operating in host or peripheral mode,
> and the only way to determine the operating state is to peek into the
> core's drvdata.
>
> The Qualcomm glue driver is expected to make updates in the qscratch
> register region (the "glue" region) during role switch events, but with
> the glue and core split using the driver model, there is no reasonable
> way to introduce listeners for mode changes.
>
> Split the dwc3 core platform_driver callbacks and their implementation
> and export the implementation, to make it possible to deterministically
> instantiate the dwc3 core as part of the dwc3 glue drivers and to
> allow flattening of the DeviceTree representation.
>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  drivers/usb/dwc3/core.c | 169 +++++++++++++++++++++++++++++++-----------------
>  drivers/usb/dwc3/core.h |   3 +
>  2 files changed, 114 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 734de2a8bd21..6addb3c367e6 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -36,6 +36,7 @@
>
>  #include "core.h"
>  #include "gadget.h"
> +#include "glue.h"
>  #include "io.h"
>
>  #include "debug.h"
> @@ -2076,10 +2077,11 @@ static int dwc3_get_num_ports(struct dwc3 *dwc)
>  	return 0;
>  }
>
> -static int dwc3_probe(struct platform_device *pdev)
> +struct dwc3 *dwc3_probe(struct platform_device *pdev, struct resource *res,
> +			bool ignore_clocks_and_resets, void *glue)
>  {
>  	struct device		*dev = &pdev->dev;
> -	struct resource		*res, dwc_res;
> +	struct resource		dwc_res;
>  	unsigned int		hw_mode;
>  	void __iomem		*regs;
>  	struct dwc3		*dwc;
> @@ -2087,15 +2089,10 @@ static int dwc3_probe(struct platform_device *pdev)
>
>  	dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL);
>  	if (!dwc)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
>
>  	dwc->dev = dev;
> -
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!res) {
> -		dev_err(dev, "missing memory resource\n");
> -		return -ENODEV;
> -	}
> +	dwc->glue = glue;
>
>  	dwc->xhci_resources[0].start = res->start;
>  	dwc->xhci_resources[0].end = dwc->xhci_resources[0].start +
> @@ -2123,7 +2120,7 @@ static int dwc3_probe(struct platform_device *pdev)
>
>  	regs = devm_ioremap_resource(dev, &dwc_res);
>  	if (IS_ERR(regs))
> -		return PTR_ERR(regs);
> +		return ERR_CAST(regs);
>
>  	dwc->regs	= regs;
>  	dwc->regs_size	= resource_size(&dwc_res);
> @@ -2132,15 +2129,17 @@ static int dwc3_probe(struct platform_device *pdev)
>
>  	dwc3_get_software_properties(dwc);
>
> -	dwc->reset = devm_reset_control_array_get_optional_shared(dev);
> -	if (IS_ERR(dwc->reset)) {
> -		ret = PTR_ERR(dwc->reset);
> -		goto err_put_psy;
> -	}
> +	if (!ignore_clocks_and_resets) {
> +		dwc->reset = devm_reset_control_array_get_optional_shared(dev);
> +		if (IS_ERR(dwc->reset)) {
> +			ret = PTR_ERR(dwc->reset);
> +			goto err_put_psy;
> +		}
>
> -	ret = dwc3_get_clocks(dwc);
> -	if (ret)
> -		goto err_put_psy;
> +		ret = dwc3_get_clocks(dwc);
> +		if (ret)
> +			goto err_put_psy;
> +	}
>
>  	ret = reset_control_deassert(dwc->reset);
>  	if (ret)
> @@ -2225,7 +2224,7 @@ static int dwc3_probe(struct platform_device *pdev)
>
>  	dma_set_max_seg_size(dev, UINT_MAX);
>
> -	return 0;
> +	return dwc;
>
>  err_exit_debugfs:
>  	dwc3_debugfs_exit(dwc);
> @@ -2249,14 +2248,33 @@ static int dwc3_probe(struct platform_device *pdev)
>  	if (dwc->usb_psy)
>  		power_supply_put(dwc->usb_psy);
>
> -	return ret;
> +	return ERR_PTR(ret);
>  }
> +EXPORT_SYMBOL_GPL(dwc3_probe);
>
> -static void dwc3_remove(struct platform_device *pdev)
> +static int dwc3_plat_probe(struct platform_device *pdev)
>  {
> -	struct dwc3	*dwc = platform_get_drvdata(pdev);
> +	struct resource *res;
> +	struct dwc3 *dwc;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "missing memory resource\n");
> +		return -ENODEV;
> +	}
>
> -	pm_runtime_get_sync(&pdev->dev);
> +	dwc = dwc3_probe(pdev, res, false, NULL);
> +	if (IS_ERR(dwc))
> +		return PTR_ERR(dwc);
> +
> +	platform_set_drvdata(pdev, dwc);
> +
> +	return 0;
> +}
> +
> +void dwc3_remove(struct dwc3 *dwc)
> +{
> +	pm_runtime_get_sync(dwc->dev);
>
>  	dwc3_core_exit_mode(dwc);
>  	dwc3_debugfs_exit(dwc);
> @@ -2264,22 +2282,28 @@ static void dwc3_remove(struct platform_device *pdev)
>  	dwc3_core_exit(dwc);
>  	dwc3_ulpi_exit(dwc);
>
> -	pm_runtime_allow(&pdev->dev);
> -	pm_runtime_disable(&pdev->dev);
> -	pm_runtime_dont_use_autosuspend(&pdev->dev);
> -	pm_runtime_put_noidle(&pdev->dev);
> +	pm_runtime_allow(dwc->dev);
> +	pm_runtime_disable(dwc->dev);
> +	pm_runtime_dont_use_autosuspend(dwc->dev);
> +	pm_runtime_put_noidle(dwc->dev);
>  	/*
>  	 * HACK: Clear the driver data, which is currently accessed by parent
>  	 * glue drivers, before allowing the parent to suspend.
>  	 */
> -	platform_set_drvdata(pdev, NULL);
> -	pm_runtime_set_suspended(&pdev->dev);
> +	dev_set_drvdata(dwc->dev, NULL);
> +	pm_runtime_set_suspended(dwc->dev);
>
>  	dwc3_free_event_buffers(dwc);
>
>  	if (dwc->usb_psy)
>  		power_supply_put(dwc->usb_psy);
>  }
> +EXPORT_SYMBOL_GPL(dwc3_remove);
> +
> +static void dwc3_plat_remove(struct platform_device *pdev)
> +{
> +	dwc3_remove(platform_get_drvdata(pdev));
> +}
>
>  #ifdef CONFIG_PM
>  static int dwc3_core_init_for_resume(struct dwc3 *dwc)
> @@ -2450,9 +2474,8 @@ static int dwc3_runtime_checks(struct dwc3 *dwc)
>  	return 0;
>  }
>
> -static int dwc3_runtime_suspend(struct device *dev)
> +int dwc3_runtime_suspend(struct dwc3 *dwc)
>  {
> -	struct dwc3     *dwc = dev_get_drvdata(dev);
>  	int		ret;
>
>  	if (dwc3_runtime_checks(dwc))
> @@ -2464,10 +2487,10 @@ static int dwc3_runtime_suspend(struct device *dev)
>
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwc3_runtime_suspend);
>
> -static int dwc3_runtime_resume(struct device *dev)
> +int dwc3_runtime_resume(struct dwc3 *dwc)
>  {
> -	struct dwc3     *dwc = dev_get_drvdata(dev);
>  	int		ret;
>
>  	ret = dwc3_resume_common(dwc, PMSG_AUTO_RESUME);
> @@ -2484,15 +2507,14 @@ static int dwc3_runtime_resume(struct device *dev)
>  		break;
>  	}
>
> -	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_mark_last_busy(dwc->dev);
>
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwc3_runtime_resume);
>
> -static int dwc3_runtime_idle(struct device *dev)
> +int dwc3_runtime_idle(struct dwc3 *dwc)
>  {
> -	struct dwc3     *dwc = dev_get_drvdata(dev);
> -
>  	switch (dwc->current_dr_role) {
>  	case DWC3_GCTL_PRTCAP_DEVICE:
>  		if (dwc3_runtime_checks(dwc))
> @@ -2504,52 +2526,67 @@ static int dwc3_runtime_idle(struct device *dev)
>  		break;
>  	}
>
> -	pm_runtime_mark_last_busy(dev);
> -	pm_runtime_autosuspend(dev);
> +	pm_runtime_mark_last_busy(dwc->dev);
> +	pm_runtime_autosuspend(dwc->dev);
>
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwc3_runtime_idle);
> +
> +static int dwc3_plat_runtime_suspend(struct device *dev)
> +{
> +	return dwc3_runtime_suspend(dev_get_drvdata(dev));
> +}
> +
> +static int dwc3_plat_runtime_resume(struct device *dev)
> +{
> +	return dwc3_runtime_resume(dev_get_drvdata(dev));
> +}
> +
> +static int dwc3_plat_runtime_idle(struct device *dev)
> +{
> +	return dwc3_runtime_idle(dev_get_drvdata(dev));
> +}
>  #endif /* CONFIG_PM */
>
>  #ifdef CONFIG_PM_SLEEP
> -static int dwc3_suspend(struct device *dev)
> +int dwc3_suspend(struct dwc3 *dwc)
>  {
> -	struct dwc3	*dwc = dev_get_drvdata(dev);
>  	int		ret;
>
>  	ret = dwc3_suspend_common(dwc, PMSG_SUSPEND);
>  	if (ret)
>  		return ret;
>
> -	pinctrl_pm_select_sleep_state(dev);
> +	pinctrl_pm_select_sleep_state(dwc->dev);
>
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwc3_suspend);
>
> -static int dwc3_resume(struct device *dev)
> +int dwc3_resume(struct dwc3 *dwc)
>  {
> -	struct dwc3	*dwc = dev_get_drvdata(dev);
>  	int		ret;
>
> -	pinctrl_pm_select_default_state(dev);
> +	pinctrl_pm_select_default_state(dwc->dev);
>
> -	pm_runtime_disable(dev);
> -	pm_runtime_set_active(dev);
> +	pm_runtime_disable(dwc->dev);
> +	pm_runtime_set_active(dwc->dev);
>
>  	ret = dwc3_resume_common(dwc, PMSG_RESUME);
>  	if (ret) {
> -		pm_runtime_set_suspended(dev);
> +		pm_runtime_set_suspended(dwc->dev);
>  		return ret;
>  	}
>
> -	pm_runtime_enable(dev);
> +	pm_runtime_enable(dwc->dev);
>
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwc3_resume);
>
> -static void dwc3_complete(struct device *dev)
> +void dwc3_complete(struct dwc3 *dwc)
>  {
> -	struct dwc3	*dwc = dev_get_drvdata(dev);
>  	u32		reg;
>
>  	if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST &&
> @@ -2559,15 +2596,31 @@ static void dwc3_complete(struct device *dev)
>  		dwc3_writel(dwc->regs, DWC3_GUCTL3, reg);
>  	}
>  }
> +EXPORT_SYMBOL_GPL(dwc3_complete);
> +
> +static int dwc3_plat_suspend(struct device *dev)
> +{
> +	return dwc3_suspend(dev_get_drvdata(dev));
> +}
> +
> +static int dwc3_plat_resume(struct device *dev)
> +{
> +	return dwc3_resume(dev_get_drvdata(dev));
> +}
> +
> +static void dwc3_plat_complete(struct device *dev)
> +{
> +	dwc3_complete(dev_get_drvdata(dev));
> +}
>  #else
> -#define dwc3_complete NULL
> +#define dwc3_plat_complete NULL
>  #endif /* CONFIG_PM_SLEEP */
>
>  static const struct dev_pm_ops dwc3_dev_pm_ops = {
> -	SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
> -	.complete = dwc3_complete,
> -	SET_RUNTIME_PM_OPS(dwc3_runtime_suspend, dwc3_runtime_resume,
> -			dwc3_runtime_idle)
> +	SET_SYSTEM_SLEEP_PM_OPS(dwc3_plat_suspend, dwc3_plat_resume)

since you touch this line,
suggest use new SYSTEM_SLEEP_PM_OPS() and RUNTIME_PM_OPS() help macro.
also CONFIG_PM_SLEEP can be removed.

Frank

> +	.complete = dwc3_plat_complete,
> +	SET_RUNTIME_PM_OPS(dwc3_plat_runtime_suspend, dwc3_plat_runtime_resume,
> +			   dwc3_plat_runtime_idle)
>  };
>
>  #ifdef CONFIG_OF
> @@ -2595,8 +2648,8 @@ MODULE_DEVICE_TABLE(acpi, dwc3_acpi_match);
>  #endif
>
>  static struct platform_driver dwc3_driver = {
> -	.probe		= dwc3_probe,
> -	.remove_new	= dwc3_remove,
> +	.probe		= dwc3_plat_probe,
> +	.remove_new	= dwc3_plat_remove,
>  	.driver		= {
>  		.name	= "dwc3",
>  		.of_match_table	= of_match_ptr(of_dwc3_match),
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 1e561fd8b86e..4a0ee9ef72e2 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1160,6 +1160,7 @@ struct dwc3_scratchpad_array {
>   * @gsbuscfg0_reqinfo: store GSBUSCFG0.DATRDREQINFO, DESRDREQINFO,
>   *		       DATWRREQINFO, and DESWRREQINFO value passed from
>   *		       glue driver.
> + * @glue: private reference to any glue context
>   */
>  struct dwc3 {
>  	struct work_struct	drd_work;
> @@ -1388,6 +1389,8 @@ struct dwc3 {
>  	int			num_ep_resized;
>  	struct dentry		*debug_root;
>  	u32			gsbuscfg0_reqinfo;
> +
> +	void			*glue;
>  };
>
>  #define INCRX_BURST_MODE 0
>
> --
> 2.45.2
>
Thinh Nguyen Aug. 14, 2024, 12:56 a.m. UTC | #5
On Sun, Aug 11, 2024, Bjorn Andersson wrote:
> From: Bjorn Andersson <quic_bjorande@quicinc.com>
> 
> The DWC3 IP block is handled by three distinct device drivers: XHCI,
> DWC3 core and a platform specific (optional) DWC3 glue driver.
> 
> This has resulted in, at least in the case of the Qualcomm glue, the
> presence of a number of layering violations, where the glue code either
> can't handle, or has to work around, the fact that core might not probe
> deterministically.
> 
> An example of this is that the suspend path should operate slightly
> different depending on the device operating in host or peripheral mode,
> and the only way to determine the operating state is to peek into the
> core's drvdata.
> 
> The Qualcomm glue driver is expected to make updates in the qscratch
> register region (the "glue" region) during role switch events, but with
> the glue and core split using the driver model, there is no reasonable
> way to introduce listeners for mode changes.
> 
> Split the dwc3 core platform_driver callbacks and their implementation
> and export the implementation, to make it possible to deterministically
> instantiate the dwc3 core as part of the dwc3 glue drivers and to
> allow flattening of the DeviceTree representation.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  drivers/usb/dwc3/core.c | 169 +++++++++++++++++++++++++++++++-----------------
>  drivers/usb/dwc3/core.h |   3 +
>  2 files changed, 114 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 734de2a8bd21..6addb3c367e6 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -36,6 +36,7 @@
>  
>  #include "core.h"
>  #include "gadget.h"
> +#include "glue.h"
>  #include "io.h"
>  
>  #include "debug.h"
> @@ -2076,10 +2077,11 @@ static int dwc3_get_num_ports(struct dwc3 *dwc)
>  	return 0;
>  }
>  
> -static int dwc3_probe(struct platform_device *pdev)
> +struct dwc3 *dwc3_probe(struct platform_device *pdev, struct resource *res,
> +			bool ignore_clocks_and_resets, void *glue)

Perhaps create a wrapper struct to hold the parameters above. I can see
we may expand this in the future.

>  {
>  	struct device		*dev = &pdev->dev;
> -	struct resource		*res, dwc_res;
> +	struct resource		dwc_res;
>  	unsigned int		hw_mode;
>  	void __iomem		*regs;
>  	struct dwc3		*dwc;
> @@ -2087,15 +2089,10 @@ static int dwc3_probe(struct platform_device *pdev)
>  
>  	dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL);
>  	if (!dwc)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
>  
>  	dwc->dev = dev;
> -
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!res) {
> -		dev_err(dev, "missing memory resource\n");
> -		return -ENODEV;
> -	}
> +	dwc->glue = glue;
>  
>  	dwc->xhci_resources[0].start = res->start;
>  	dwc->xhci_resources[0].end = dwc->xhci_resources[0].start +
> @@ -2123,7 +2120,7 @@ static int dwc3_probe(struct platform_device *pdev)
>  
>  	regs = devm_ioremap_resource(dev, &dwc_res);
>  	if (IS_ERR(regs))
> -		return PTR_ERR(regs);
> +		return ERR_CAST(regs);
>  
>  	dwc->regs	= regs;
>  	dwc->regs_size	= resource_size(&dwc_res);
> @@ -2132,15 +2129,17 @@ static int dwc3_probe(struct platform_device *pdev)
>  
>  	dwc3_get_software_properties(dwc);
>  
> -	dwc->reset = devm_reset_control_array_get_optional_shared(dev);
> -	if (IS_ERR(dwc->reset)) {
> -		ret = PTR_ERR(dwc->reset);
> -		goto err_put_psy;
> -	}
> +	if (!ignore_clocks_and_resets) {

This seems to be specific change for your platform. Let's keep this
change separated from this patch.

Thanks,
Thinh

> +		dwc->reset = devm_reset_control_array_get_optional_shared(dev);
> +		if (IS_ERR(dwc->reset)) {
> +			ret = PTR_ERR(dwc->reset);
> +			goto err_put_psy;
> +		}
>  
> -	ret = dwc3_get_clocks(dwc);
> -	if (ret)
> -		goto err_put_psy;
> +		ret = dwc3_get_clocks(dwc);
> +		if (ret)
> +			goto err_put_psy;
> +	}
>  
>  	ret = reset_control_deassert(dwc->reset);
>  	if (ret)
> @@ -2225,7 +2224,7 @@ static int dwc3_probe(struct platform_device *pdev)
>  
>  	dma_set_max_seg_size(dev, UINT_MAX);
>  
> -	return 0;
> +	return dwc;
>  
>  err_exit_debugfs:
>  	dwc3_debugfs_exit(dwc);
> @@ -2249,14 +2248,33 @@ static int dwc3_probe(struct platform_device *pdev)
>  	if (dwc->usb_psy)
>  		power_supply_put(dwc->usb_psy);
>  
> -	return ret;
> +	return ERR_PTR(ret);
>  }
> +EXPORT_SYMBOL_GPL(dwc3_probe);
>  
> -static void dwc3_remove(struct platform_device *pdev)
> +static int dwc3_plat_probe(struct platform_device *pdev)
>  {
> -	struct dwc3	*dwc = platform_get_drvdata(pdev);
> +	struct resource *res;
> +	struct dwc3 *dwc;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "missing memory resource\n");
> +		return -ENODEV;
> +	}
>  
> -	pm_runtime_get_sync(&pdev->dev);
> +	dwc = dwc3_probe(pdev, res, false, NULL);
> +	if (IS_ERR(dwc))
> +		return PTR_ERR(dwc);
> +
> +	platform_set_drvdata(pdev, dwc);
> +
> +	return 0;
> +}
> +
> +void dwc3_remove(struct dwc3 *dwc)
> +{
> +	pm_runtime_get_sync(dwc->dev);
>  
>  	dwc3_core_exit_mode(dwc);
>  	dwc3_debugfs_exit(dwc);
> @@ -2264,22 +2282,28 @@ static void dwc3_remove(struct platform_device *pdev)
>  	dwc3_core_exit(dwc);
>  	dwc3_ulpi_exit(dwc);
>  
> -	pm_runtime_allow(&pdev->dev);
> -	pm_runtime_disable(&pdev->dev);
> -	pm_runtime_dont_use_autosuspend(&pdev->dev);
> -	pm_runtime_put_noidle(&pdev->dev);
> +	pm_runtime_allow(dwc->dev);
> +	pm_runtime_disable(dwc->dev);
> +	pm_runtime_dont_use_autosuspend(dwc->dev);
> +	pm_runtime_put_noidle(dwc->dev);
>  	/*
>  	 * HACK: Clear the driver data, which is currently accessed by parent
>  	 * glue drivers, before allowing the parent to suspend.
>  	 */
> -	platform_set_drvdata(pdev, NULL);
> -	pm_runtime_set_suspended(&pdev->dev);
> +	dev_set_drvdata(dwc->dev, NULL);
> +	pm_runtime_set_suspended(dwc->dev);
>  
>  	dwc3_free_event_buffers(dwc);
>  
>  	if (dwc->usb_psy)
>  		power_supply_put(dwc->usb_psy);
>  }
> +EXPORT_SYMBOL_GPL(dwc3_remove);
> +
> +static void dwc3_plat_remove(struct platform_device *pdev)
> +{
> +	dwc3_remove(platform_get_drvdata(pdev));
> +}
>  
>  #ifdef CONFIG_PM
>  static int dwc3_core_init_for_resume(struct dwc3 *dwc)
> @@ -2450,9 +2474,8 @@ static int dwc3_runtime_checks(struct dwc3 *dwc)
>  	return 0;
>  }
>  
> -static int dwc3_runtime_suspend(struct device *dev)
> +int dwc3_runtime_suspend(struct dwc3 *dwc)
>  {
> -	struct dwc3     *dwc = dev_get_drvdata(dev);
>  	int		ret;
>  
>  	if (dwc3_runtime_checks(dwc))
> @@ -2464,10 +2487,10 @@ static int dwc3_runtime_suspend(struct device *dev)
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwc3_runtime_suspend);
>  
> -static int dwc3_runtime_resume(struct device *dev)
> +int dwc3_runtime_resume(struct dwc3 *dwc)
>  {
> -	struct dwc3     *dwc = dev_get_drvdata(dev);
>  	int		ret;
>  
>  	ret = dwc3_resume_common(dwc, PMSG_AUTO_RESUME);
> @@ -2484,15 +2507,14 @@ static int dwc3_runtime_resume(struct device *dev)
>  		break;
>  	}
>  
> -	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_mark_last_busy(dwc->dev);
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwc3_runtime_resume);
>  
> -static int dwc3_runtime_idle(struct device *dev)
> +int dwc3_runtime_idle(struct dwc3 *dwc)
>  {
> -	struct dwc3     *dwc = dev_get_drvdata(dev);
> -
>  	switch (dwc->current_dr_role) {
>  	case DWC3_GCTL_PRTCAP_DEVICE:
>  		if (dwc3_runtime_checks(dwc))
> @@ -2504,52 +2526,67 @@ static int dwc3_runtime_idle(struct device *dev)
>  		break;
>  	}
>  
> -	pm_runtime_mark_last_busy(dev);
> -	pm_runtime_autosuspend(dev);
> +	pm_runtime_mark_last_busy(dwc->dev);
> +	pm_runtime_autosuspend(dwc->dev);
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwc3_runtime_idle);
> +
> +static int dwc3_plat_runtime_suspend(struct device *dev)
> +{
> +	return dwc3_runtime_suspend(dev_get_drvdata(dev));
> +}
> +
> +static int dwc3_plat_runtime_resume(struct device *dev)
> +{
> +	return dwc3_runtime_resume(dev_get_drvdata(dev));
> +}
> +
> +static int dwc3_plat_runtime_idle(struct device *dev)
> +{
> +	return dwc3_runtime_idle(dev_get_drvdata(dev));
> +}
>  #endif /* CONFIG_PM */
>  
>  #ifdef CONFIG_PM_SLEEP
> -static int dwc3_suspend(struct device *dev)
> +int dwc3_suspend(struct dwc3 *dwc)
>  {
> -	struct dwc3	*dwc = dev_get_drvdata(dev);
>  	int		ret;
>  
>  	ret = dwc3_suspend_common(dwc, PMSG_SUSPEND);
>  	if (ret)
>  		return ret;
>  
> -	pinctrl_pm_select_sleep_state(dev);
> +	pinctrl_pm_select_sleep_state(dwc->dev);
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwc3_suspend);
>  
> -static int dwc3_resume(struct device *dev)
> +int dwc3_resume(struct dwc3 *dwc)
>  {
> -	struct dwc3	*dwc = dev_get_drvdata(dev);
>  	int		ret;
>  
> -	pinctrl_pm_select_default_state(dev);
> +	pinctrl_pm_select_default_state(dwc->dev);
>  
> -	pm_runtime_disable(dev);
> -	pm_runtime_set_active(dev);
> +	pm_runtime_disable(dwc->dev);
> +	pm_runtime_set_active(dwc->dev);
>  
>  	ret = dwc3_resume_common(dwc, PMSG_RESUME);
>  	if (ret) {
> -		pm_runtime_set_suspended(dev);
> +		pm_runtime_set_suspended(dwc->dev);
>  		return ret;
>  	}
>  
> -	pm_runtime_enable(dev);
> +	pm_runtime_enable(dwc->dev);
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwc3_resume);
>  
> -static void dwc3_complete(struct device *dev)
> +void dwc3_complete(struct dwc3 *dwc)
>  {
> -	struct dwc3	*dwc = dev_get_drvdata(dev);
>  	u32		reg;
>  
>  	if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST &&
> @@ -2559,15 +2596,31 @@ static void dwc3_complete(struct device *dev)
>  		dwc3_writel(dwc->regs, DWC3_GUCTL3, reg);
>  	}
>  }
> +EXPORT_SYMBOL_GPL(dwc3_complete);
> +
> +static int dwc3_plat_suspend(struct device *dev)
> +{
> +	return dwc3_suspend(dev_get_drvdata(dev));
> +}
> +
> +static int dwc3_plat_resume(struct device *dev)
> +{
> +	return dwc3_resume(dev_get_drvdata(dev));
> +}
> +
> +static void dwc3_plat_complete(struct device *dev)
> +{
> +	dwc3_complete(dev_get_drvdata(dev));
> +}
>  #else
> -#define dwc3_complete NULL
> +#define dwc3_plat_complete NULL
>  #endif /* CONFIG_PM_SLEEP */
>  
>  static const struct dev_pm_ops dwc3_dev_pm_ops = {
> -	SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
> -	.complete = dwc3_complete,
> -	SET_RUNTIME_PM_OPS(dwc3_runtime_suspend, dwc3_runtime_resume,
> -			dwc3_runtime_idle)
> +	SET_SYSTEM_SLEEP_PM_OPS(dwc3_plat_suspend, dwc3_plat_resume)
> +	.complete = dwc3_plat_complete,
> +	SET_RUNTIME_PM_OPS(dwc3_plat_runtime_suspend, dwc3_plat_runtime_resume,
> +			   dwc3_plat_runtime_idle)
>  };
>  
>  #ifdef CONFIG_OF
> @@ -2595,8 +2648,8 @@ MODULE_DEVICE_TABLE(acpi, dwc3_acpi_match);
>  #endif
>  
>  static struct platform_driver dwc3_driver = {
> -	.probe		= dwc3_probe,
> -	.remove_new	= dwc3_remove,
> +	.probe		= dwc3_plat_probe,
> +	.remove_new	= dwc3_plat_remove,
>  	.driver		= {
>  		.name	= "dwc3",
>  		.of_match_table	= of_match_ptr(of_dwc3_match),
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 1e561fd8b86e..4a0ee9ef72e2 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1160,6 +1160,7 @@ struct dwc3_scratchpad_array {
>   * @gsbuscfg0_reqinfo: store GSBUSCFG0.DATRDREQINFO, DESRDREQINFO,
>   *		       DATWRREQINFO, and DESWRREQINFO value passed from
>   *		       glue driver.
> + * @glue: private reference to any glue context
>   */
>  struct dwc3 {
>  	struct work_struct	drd_work;
> @@ -1388,6 +1389,8 @@ struct dwc3 {
>  	int			num_ep_resized;
>  	struct dentry		*debug_root;
>  	u32			gsbuscfg0_reqinfo;
> +
> +	void			*glue;
>  };
>  
>  #define INCRX_BURST_MODE 0
> 
> -- 
> 2.45.2
>
Thinh Nguyen Aug. 14, 2024, 1:01 a.m. UTC | #6
On Tue, Aug 13, 2024, Frank Li wrote:
> On Sun, Aug 11, 2024 at 08:12:01PM -0700, Bjorn Andersson wrote:
> > From: Bjorn Andersson <quic_bjorande@quicinc.com>

...

> > +{
> > +	dwc3_complete(dev_get_drvdata(dev));
> > +}
> >  #else
> > -#define dwc3_complete NULL
> > +#define dwc3_plat_complete NULL
> >  #endif /* CONFIG_PM_SLEEP */
> >
> >  static const struct dev_pm_ops dwc3_dev_pm_ops = {
> > -	SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
> > -	.complete = dwc3_complete,
> > -	SET_RUNTIME_PM_OPS(dwc3_runtime_suspend, dwc3_runtime_resume,
> > -			dwc3_runtime_idle)
> > +	SET_SYSTEM_SLEEP_PM_OPS(dwc3_plat_suspend, dwc3_plat_resume)
> 
> since you touch this line,
> suggest use new SYSTEM_SLEEP_PM_OPS() and RUNTIME_PM_OPS() help macro.
> also CONFIG_PM_SLEEP can be removed.
> 

If we want to make that change, please keep that change separate from
this patch/series.

Thanks,
Thinh
Krishna Kurapati Aug. 19, 2024, 5:48 p.m. UTC | #7
On 8/12/2024 8:42 AM, Bjorn Andersson wrote:
> From: Bjorn Andersson <quic_bjorande@quicinc.com>
> 
> The DWC3 IP block is handled by three distinct device drivers: XHCI,
> DWC3 core and a platform specific (optional) DWC3 glue driver.
> 
> This has resulted in, at least in the case of the Qualcomm glue, the
> presence of a number of layering violations, where the glue code either
> can't handle, or has to work around, the fact that core might not probe
> deterministically.
> 
> An example of this is that the suspend path should operate slightly
> different depending on the device operating in host or peripheral mode,
> and the only way to determine the operating state is to peek into the
> core's drvdata.
> 
> The Qualcomm glue driver is expected to make updates in the qscratch
> register region (the "glue" region) during role switch events, but with
> the glue and core split using the driver model, there is no reasonable
> way to introduce listeners for mode changes.
> 
> Split the dwc3 core platform_driver callbacks and their implementation
> and export the implementation, to make it possible to deterministically
> instantiate the dwc3 core as part of the dwc3 glue drivers and to
> allow flattening of the DeviceTree representation.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>

...

> -static int dwc3_probe(struct platform_device *pdev)
> +struct dwc3 *dwc3_probe(struct platform_device *pdev, struct resource *res,
> +			bool ignore_clocks_and_resets, void *glue)
>   {
>   	struct device		*dev = &pdev->dev;
> -	struct resource		*res, dwc_res;
> +	struct resource		dwc_res;
>   	unsigned int		hw_mode;
>   	void __iomem		*regs;
>   	struct dwc3		*dwc;
> @@ -2087,15 +2089,10 @@ static int dwc3_probe(struct platform_device *pdev)
>   
>   	dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL);
>   	if (!dwc)
> -		return -ENOMEM;
> +		return ERR_PTR(-ENOMEM);
>   
>   	dwc->dev = dev;
> -
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!res) {
> -		dev_err(dev, "missing memory resource\n");
> -		return -ENODEV;
> -	}

...

> +static int dwc3_plat_probe(struct platform_device *pdev)
>   {
> -	struct dwc3	*dwc = platform_get_drvdata(pdev);
> +	struct resource *res;
> +	struct dwc3 *dwc;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&pdev->dev, "missing memory resource\n");
> +		return -ENODEV;
> +	}
>   
> -	pm_runtime_get_sync(&pdev->dev);
> +	dwc = dwc3_probe(pdev, res, false, NULL);
> +	if (IS_ERR(dwc))
> +		return PTR_ERR(dwc);
> +
> +	platform_set_drvdata(pdev, dwc);

This setting of platform drvdata is redundant I believe. We already do 
it in dwc3_probe.

> +
> +	return 0;
> +}
> +
Bjorn Andersson Aug. 19, 2024, 9:14 p.m. UTC | #8
On Wed, Aug 14, 2024 at 12:56:44AM +0000, Thinh Nguyen wrote:
> On Sun, Aug 11, 2024, Bjorn Andersson wrote:
> > From: Bjorn Andersson <quic_bjorande@quicinc.com>
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
[..]
> > @@ -2076,10 +2077,11 @@ static int dwc3_get_num_ports(struct dwc3 *dwc)
> >  	return 0;
> >  }
> >  
> > -static int dwc3_probe(struct platform_device *pdev)
> > +struct dwc3 *dwc3_probe(struct platform_device *pdev, struct resource *res,
> > +			bool ignore_clocks_and_resets, void *glue)
> 
> Perhaps create a wrapper struct to hold the parameters above. I can see
> we may expand this in the future.
> 

Sounds good.

There are a few cases where e.g. the Qualcomm glue needs to react to
role switching in the core, so we already know that we want to add some
callbacks here.

> >  {
> >  	struct device		*dev = &pdev->dev;
> > -	struct resource		*res, dwc_res;
> > +	struct resource		dwc_res;
> >  	unsigned int		hw_mode;
> >  	void __iomem		*regs;
> >  	struct dwc3		*dwc;
> > @@ -2087,15 +2089,10 @@ static int dwc3_probe(struct platform_device *pdev)
> >  
> >  	dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL);
> >  	if (!dwc)
> > -		return -ENOMEM;
> > +		return ERR_PTR(-ENOMEM);
> >  
> >  	dwc->dev = dev;
> > -
> > -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -	if (!res) {
> > -		dev_err(dev, "missing memory resource\n");
> > -		return -ENODEV;
> > -	}
> > +	dwc->glue = glue;
> >  
> >  	dwc->xhci_resources[0].start = res->start;
> >  	dwc->xhci_resources[0].end = dwc->xhci_resources[0].start +
> > @@ -2123,7 +2120,7 @@ static int dwc3_probe(struct platform_device *pdev)
> >  
> >  	regs = devm_ioremap_resource(dev, &dwc_res);
> >  	if (IS_ERR(regs))
> > -		return PTR_ERR(regs);
> > +		return ERR_CAST(regs);
> >  
> >  	dwc->regs	= regs;
> >  	dwc->regs_size	= resource_size(&dwc_res);
> > @@ -2132,15 +2129,17 @@ static int dwc3_probe(struct platform_device *pdev)
> >  
> >  	dwc3_get_software_properties(dwc);
> >  
> > -	dwc->reset = devm_reset_control_array_get_optional_shared(dev);
> > -	if (IS_ERR(dwc->reset)) {
> > -		ret = PTR_ERR(dwc->reset);
> > -		goto err_put_psy;
> > -	}
> > +	if (!ignore_clocks_and_resets) {
> 
> This seems to be specific change for your platform. Let's keep this
> change separated from this patch.
> 

The primary need here is that the glue code needs to be able to access
the hardware before the core's resume and after suspend. I'd expect
other glue implementations will have the same need.

But moving this change to a separate change allows us to reason about
that separately, so I like the suggestion.

Thanks,
Bjorn

> Thanks,
> Thinh
>
Bjorn Andersson Aug. 19, 2024, 9:35 p.m. UTC | #9
On Tue, Aug 13, 2024 at 02:15:35PM -0400, Frank Li wrote:
> On Sun, Aug 11, 2024 at 08:12:01PM -0700, Bjorn Andersson wrote:
> > From: Bjorn Andersson <quic_bjorande@quicinc.com>
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
[..]
> >  static const struct dev_pm_ops dwc3_dev_pm_ops = {
> > -	SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
> > -	.complete = dwc3_complete,
> > -	SET_RUNTIME_PM_OPS(dwc3_runtime_suspend, dwc3_runtime_resume,
> > -			dwc3_runtime_idle)
> > +	SET_SYSTEM_SLEEP_PM_OPS(dwc3_plat_suspend, dwc3_plat_resume)
> 
> since you touch this line,
> suggest use new SYSTEM_SLEEP_PM_OPS() and RUNTIME_PM_OPS() help macro.
> also CONFIG_PM_SLEEP can be removed.
> 

I'd be happy to follow up with such cleanups after we've concluded this,
I did spot a few other things that would benefit from some TLC.

Regards,
Bjorn
Bjorn Andersson Aug. 19, 2024, 9:36 p.m. UTC | #10
On Mon, Aug 19, 2024 at 11:18:15PM +0530, Krishna Kurapati wrote:
> 
> 
> On 8/12/2024 8:42 AM, Bjorn Andersson wrote:
> > From: Bjorn Andersson <quic_bjorande@quicinc.com>
> > 
> > The DWC3 IP block is handled by three distinct device drivers: XHCI,
> > DWC3 core and a platform specific (optional) DWC3 glue driver.
> > 
> > This has resulted in, at least in the case of the Qualcomm glue, the
> > presence of a number of layering violations, where the glue code either
> > can't handle, or has to work around, the fact that core might not probe
> > deterministically.
> > 
> > An example of this is that the suspend path should operate slightly
> > different depending on the device operating in host or peripheral mode,
> > and the only way to determine the operating state is to peek into the
> > core's drvdata.
> > 
> > The Qualcomm glue driver is expected to make updates in the qscratch
> > register region (the "glue" region) during role switch events, but with
> > the glue and core split using the driver model, there is no reasonable
> > way to introduce listeners for mode changes.
> > 
> > Split the dwc3 core platform_driver callbacks and their implementation
> > and export the implementation, to make it possible to deterministically
> > instantiate the dwc3 core as part of the dwc3 glue drivers and to
> > allow flattening of the DeviceTree representation.
> > 
> > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> 
> ...
> 
> > -static int dwc3_probe(struct platform_device *pdev)
> > +struct dwc3 *dwc3_probe(struct platform_device *pdev, struct resource *res,
> > +			bool ignore_clocks_and_resets, void *glue)
> >   {
> >   	struct device		*dev = &pdev->dev;
> > -	struct resource		*res, dwc_res;
> > +	struct resource		dwc_res;
> >   	unsigned int		hw_mode;
> >   	void __iomem		*regs;
> >   	struct dwc3		*dwc;
> > @@ -2087,15 +2089,10 @@ static int dwc3_probe(struct platform_device *pdev)
> >   	dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL);
> >   	if (!dwc)
> > -		return -ENOMEM;
> > +		return ERR_PTR(-ENOMEM);
> >   	dwc->dev = dev;
> > -
> > -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -	if (!res) {
> > -		dev_err(dev, "missing memory resource\n");
> > -		return -ENODEV;
> > -	}
> 
> ...
> 
> > +static int dwc3_plat_probe(struct platform_device *pdev)
> >   {
> > -	struct dwc3	*dwc = platform_get_drvdata(pdev);
> > +	struct resource *res;
> > +	struct dwc3 *dwc;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (!res) {
> > +		dev_err(&pdev->dev, "missing memory resource\n");
> > +		return -ENODEV;
> > +	}
> > -	pm_runtime_get_sync(&pdev->dev);
> > +	dwc = dwc3_probe(pdev, res, false, NULL);
> > +	if (IS_ERR(dwc))
> > +		return PTR_ERR(dwc);
> > +
> > +	platform_set_drvdata(pdev, dwc);
> 
> This setting of platform drvdata is redundant I believe. We already do it in
> dwc3_probe.
> 

Good catch, you're certainly right.

Regards,
Bjorn

> > +
> > +	return 0;
> > +}
> > +
Masahiro Yamada Sept. 12, 2024, 10:21 p.m. UTC | #11
On Mon, Aug 12, 2024 at 12:07 PM Bjorn Andersson <andersson@kernel.org> wrote:
>
> From: Bjorn Andersson <quic_bjorande@quicinc.com>
>
> The DWC3 IP block is handled by three distinct device drivers: XHCI,
> DWC3 core and a platform specific (optional) DWC3 glue driver.
>
> This has resulted in, at least in the case of the Qualcomm glue, the
> presence of a number of layering violations, where the glue code either
> can't handle, or has to work around, the fact that core might not probe
> deterministically.
>
> An example of this is that the suspend path should operate slightly
> different depending on the device operating in host or peripheral mode,
> and the only way to determine the operating state is to peek into the
> core's drvdata.
>
> The Qualcomm glue driver is expected to make updates in the qscratch
> register region (the "glue" region) during role switch events, but with
> the glue and core split using the driver model, there is no reasonable
> way to introduce listeners for mode changes.
>
> Split the dwc3 core platform_driver callbacks and their implementation
> and export the implementation, to make it possible to deterministically
> instantiate the dwc3 core as part of the dwc3 glue drivers and to
> allow flattening of the DeviceTree representation.
>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  drivers/usb/dwc3/core.c | 169 +++++++++++++++++++++++++++++++-----------------
>  drivers/usb/dwc3/core.h |   3 +
>  2 files changed, 114 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 734de2a8bd21..6addb3c367e6 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -36,6 +36,7 @@
>
>  #include "core.h"
>  #include "gadget.h"
> +#include "glue.h"
>  #include "io.h"
>
>  #include "debug.h"
> @@ -2076,10 +2077,11 @@ static int dwc3_get_num_ports(struct dwc3 *dwc)
>         return 0;
>  }
>
> -static int dwc3_probe(struct platform_device *pdev)
> +struct dwc3 *dwc3_probe(struct platform_device *pdev, struct resource *res,
> +                       bool ignore_clocks_and_resets, void *glue)
>  {
>         struct device           *dev = &pdev->dev;
> -       struct resource         *res, dwc_res;
> +       struct resource         dwc_res;
>         unsigned int            hw_mode;
>         void __iomem            *regs;
>         struct dwc3             *dwc;
> @@ -2087,15 +2089,10 @@ static int dwc3_probe(struct platform_device *pdev)
>
>         dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL);
>         if (!dwc)
> -               return -ENOMEM;
> +               return ERR_PTR(-ENOMEM);
>
>         dwc->dev = dev;
> -
> -       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -       if (!res) {
> -               dev_err(dev, "missing memory resource\n");
> -               return -ENODEV;
> -       }
> +       dwc->glue = glue;
>
>         dwc->xhci_resources[0].start = res->start;
>         dwc->xhci_resources[0].end = dwc->xhci_resources[0].start +
> @@ -2123,7 +2120,7 @@ static int dwc3_probe(struct platform_device *pdev)
>
>         regs = devm_ioremap_resource(dev, &dwc_res);
>         if (IS_ERR(regs))
> -               return PTR_ERR(regs);
> +               return ERR_CAST(regs);
>
>         dwc->regs       = regs;
>         dwc->regs_size  = resource_size(&dwc_res);
> @@ -2132,15 +2129,17 @@ static int dwc3_probe(struct platform_device *pdev)
>
>         dwc3_get_software_properties(dwc);
>
> -       dwc->reset = devm_reset_control_array_get_optional_shared(dev);
> -       if (IS_ERR(dwc->reset)) {
> -               ret = PTR_ERR(dwc->reset);
> -               goto err_put_psy;
> -       }
> +       if (!ignore_clocks_and_resets) {
> +               dwc->reset = devm_reset_control_array_get_optional_shared(dev);
> +               if (IS_ERR(dwc->reset)) {
> +                       ret = PTR_ERR(dwc->reset);
> +                       goto err_put_psy;
> +               }
>
> -       ret = dwc3_get_clocks(dwc);
> -       if (ret)
> -               goto err_put_psy;
> +               ret = dwc3_get_clocks(dwc);
> +               if (ret)
> +                       goto err_put_psy;
> +       }
>
>         ret = reset_control_deassert(dwc->reset);
>         if (ret)
> @@ -2225,7 +2224,7 @@ static int dwc3_probe(struct platform_device *pdev)
>
>         dma_set_max_seg_size(dev, UINT_MAX);
>
> -       return 0;
> +       return dwc;
>
>  err_exit_debugfs:
>         dwc3_debugfs_exit(dwc);
> @@ -2249,14 +2248,33 @@ static int dwc3_probe(struct platform_device *pdev)
>         if (dwc->usb_psy)
>                 power_supply_put(dwc->usb_psy);
>
> -       return ret;
> +       return ERR_PTR(ret);
>  }
> +EXPORT_SYMBOL_GPL(dwc3_probe);
>
> -static void dwc3_remove(struct platform_device *pdev)
> +static int dwc3_plat_probe(struct platform_device *pdev)
>  {
> -       struct dwc3     *dwc = platform_get_drvdata(pdev);
> +       struct resource *res;
> +       struct dwc3 *dwc;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res) {
> +               dev_err(&pdev->dev, "missing memory resource\n");
> +               return -ENODEV;
> +       }
>
> -       pm_runtime_get_sync(&pdev->dev);
> +       dwc = dwc3_probe(pdev, res, false, NULL);
> +       if (IS_ERR(dwc))
> +               return PTR_ERR(dwc);
> +
> +       platform_set_drvdata(pdev, dwc);
> +
> +       return 0;
> +}
> +
> +void dwc3_remove(struct dwc3 *dwc)
> +{
> +       pm_runtime_get_sync(dwc->dev);
>
>         dwc3_core_exit_mode(dwc);
>         dwc3_debugfs_exit(dwc);
> @@ -2264,22 +2282,28 @@ static void dwc3_remove(struct platform_device *pdev)
>         dwc3_core_exit(dwc);
>         dwc3_ulpi_exit(dwc);
>
> -       pm_runtime_allow(&pdev->dev);
> -       pm_runtime_disable(&pdev->dev);
> -       pm_runtime_dont_use_autosuspend(&pdev->dev);
> -       pm_runtime_put_noidle(&pdev->dev);
> +       pm_runtime_allow(dwc->dev);
> +       pm_runtime_disable(dwc->dev);
> +       pm_runtime_dont_use_autosuspend(dwc->dev);
> +       pm_runtime_put_noidle(dwc->dev);
>         /*
>          * HACK: Clear the driver data, which is currently accessed by parent
>          * glue drivers, before allowing the parent to suspend.
>          */
> -       platform_set_drvdata(pdev, NULL);
> -       pm_runtime_set_suspended(&pdev->dev);
> +       dev_set_drvdata(dwc->dev, NULL);
> +       pm_runtime_set_suspended(dwc->dev);
>
>         dwc3_free_event_buffers(dwc);
>
>         if (dwc->usb_psy)
>                 power_supply_put(dwc->usb_psy);
>  }
> +EXPORT_SYMBOL_GPL(dwc3_remove);
> +
> +static void dwc3_plat_remove(struct platform_device *pdev)
> +{
> +       dwc3_remove(platform_get_drvdata(pdev));
> +}
>
>  #ifdef CONFIG_PM
>  static int dwc3_core_init_for_resume(struct dwc3 *dwc)
> @@ -2450,9 +2474,8 @@ static int dwc3_runtime_checks(struct dwc3 *dwc)
>         return 0;
>  }
>
> -static int dwc3_runtime_suspend(struct device *dev)
> +int dwc3_runtime_suspend(struct dwc3 *dwc)
>  {
> -       struct dwc3     *dwc = dev_get_drvdata(dev);
>         int             ret;
>
>         if (dwc3_runtime_checks(dwc))
> @@ -2464,10 +2487,10 @@ static int dwc3_runtime_suspend(struct device *dev)
>
>         return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwc3_runtime_suspend);
>
> -static int dwc3_runtime_resume(struct device *dev)
> +int dwc3_runtime_resume(struct dwc3 *dwc)
>  {
> -       struct dwc3     *dwc = dev_get_drvdata(dev);
>         int             ret;
>
>         ret = dwc3_resume_common(dwc, PMSG_AUTO_RESUME);
> @@ -2484,15 +2507,14 @@ static int dwc3_runtime_resume(struct device *dev)
>                 break;
>         }
>
> -       pm_runtime_mark_last_busy(dev);
> +       pm_runtime_mark_last_busy(dwc->dev);
>
>         return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwc3_runtime_resume);
>
> -static int dwc3_runtime_idle(struct device *dev)
> +int dwc3_runtime_idle(struct dwc3 *dwc)
>  {
> -       struct dwc3     *dwc = dev_get_drvdata(dev);
> -
>         switch (dwc->current_dr_role) {
>         case DWC3_GCTL_PRTCAP_DEVICE:
>                 if (dwc3_runtime_checks(dwc))
> @@ -2504,52 +2526,67 @@ static int dwc3_runtime_idle(struct device *dev)
>                 break;
>         }
>
> -       pm_runtime_mark_last_busy(dev);
> -       pm_runtime_autosuspend(dev);
> +       pm_runtime_mark_last_busy(dwc->dev);
> +       pm_runtime_autosuspend(dwc->dev);
>
>         return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwc3_runtime_idle);
> +
> +static int dwc3_plat_runtime_suspend(struct device *dev)
> +{
> +       return dwc3_runtime_suspend(dev_get_drvdata(dev));
> +}
> +
> +static int dwc3_plat_runtime_resume(struct device *dev)
> +{
> +       return dwc3_runtime_resume(dev_get_drvdata(dev));
> +}
> +
> +static int dwc3_plat_runtime_idle(struct device *dev)
> +{
> +       return dwc3_runtime_idle(dev_get_drvdata(dev));
> +}
>  #endif /* CONFIG_PM */
>
>  #ifdef CONFIG_PM_SLEEP
> -static int dwc3_suspend(struct device *dev)
> +int dwc3_suspend(struct dwc3 *dwc)
>  {
> -       struct dwc3     *dwc = dev_get_drvdata(dev);
>         int             ret;
>
>         ret = dwc3_suspend_common(dwc, PMSG_SUSPEND);
>         if (ret)
>                 return ret;
>
> -       pinctrl_pm_select_sleep_state(dev);
> +       pinctrl_pm_select_sleep_state(dwc->dev);
>
>         return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwc3_suspend);
>
> -static int dwc3_resume(struct device *dev)
> +int dwc3_resume(struct dwc3 *dwc)
>  {
> -       struct dwc3     *dwc = dev_get_drvdata(dev);
>         int             ret;
>
> -       pinctrl_pm_select_default_state(dev);
> +       pinctrl_pm_select_default_state(dwc->dev);
>
> -       pm_runtime_disable(dev);
> -       pm_runtime_set_active(dev);
> +       pm_runtime_disable(dwc->dev);
> +       pm_runtime_set_active(dwc->dev);
>
>         ret = dwc3_resume_common(dwc, PMSG_RESUME);
>         if (ret) {
> -               pm_runtime_set_suspended(dev);
> +               pm_runtime_set_suspended(dwc->dev);
>                 return ret;
>         }
>
> -       pm_runtime_enable(dev);
> +       pm_runtime_enable(dwc->dev);
>
>         return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwc3_resume);
>
> -static void dwc3_complete(struct device *dev)
> +void dwc3_complete(struct dwc3 *dwc)
>  {
> -       struct dwc3     *dwc = dev_get_drvdata(dev);
>         u32             reg;
>
>         if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST &&
> @@ -2559,15 +2596,31 @@ static void dwc3_complete(struct device *dev)
>                 dwc3_writel(dwc->regs, DWC3_GUCTL3, reg);
>         }
>  }
> +EXPORT_SYMBOL_GPL(dwc3_complete);
> +
> +static int dwc3_plat_suspend(struct device *dev)
> +{
> +       return dwc3_suspend(dev_get_drvdata(dev));
> +}
> +
> +static int dwc3_plat_resume(struct device *dev)
> +{
> +       return dwc3_resume(dev_get_drvdata(dev));
> +}
> +
> +static void dwc3_plat_complete(struct device *dev)
> +{
> +       dwc3_complete(dev_get_drvdata(dev));
> +}
>  #else
> -#define dwc3_complete NULL
> +#define dwc3_plat_complete NULL
>  #endif /* CONFIG_PM_SLEEP */
>
>  static const struct dev_pm_ops dwc3_dev_pm_ops = {
> -       SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
> -       .complete = dwc3_complete,
> -       SET_RUNTIME_PM_OPS(dwc3_runtime_suspend, dwc3_runtime_resume,
> -                       dwc3_runtime_idle)
> +       SET_SYSTEM_SLEEP_PM_OPS(dwc3_plat_suspend, dwc3_plat_resume)
> +       .complete = dwc3_plat_complete,
> +       SET_RUNTIME_PM_OPS(dwc3_plat_runtime_suspend, dwc3_plat_runtime_resume,
> +                          dwc3_plat_runtime_idle)
>  };
>
>  #ifdef CONFIG_OF
> @@ -2595,8 +2648,8 @@ MODULE_DEVICE_TABLE(acpi, dwc3_acpi_match);
>  #endif
>
>  static struct platform_driver dwc3_driver = {
> -       .probe          = dwc3_probe,
> -       .remove_new     = dwc3_remove,
> +       .probe          = dwc3_plat_probe,
> +       .remove_new     = dwc3_plat_remove,
>         .driver         = {
>                 .name   = "dwc3",
>                 .of_match_table = of_match_ptr(of_dwc3_match),
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 1e561fd8b86e..4a0ee9ef72e2 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1160,6 +1160,7 @@ struct dwc3_scratchpad_array {
>   * @gsbuscfg0_reqinfo: store GSBUSCFG0.DATRDREQINFO, DESRDREQINFO,
>   *                    DATWRREQINFO, and DESWRREQINFO value passed from
>   *                    glue driver.
> + * @glue: private reference to any glue context
>   */
>  struct dwc3 {
>         struct work_struct      drd_work;
> @@ -1388,6 +1389,8 @@ struct dwc3 {
>         int                     num_ep_resized;
>         struct dentry           *debug_root;
>         u32                     gsbuscfg0_reqinfo;
> +
> +       void                    *glue;


This is ugly.


'struct dwc3_qcom' can wrap 'struct dwc3'
instead of having two separate structures
pointing at each other.


You can use container_of() to
convert (struct dwc3 *) to (struct dwc3_qcom *).


I attached a diff to delete the reverse ->glue pointer.

I did not compile-test it (I cannot due to missing glue.h
anyway), but you will understand my suggestion.
Bjorn Andersson Sept. 13, 2024, 2:34 a.m. UTC | #12
On Fri, Sep 13, 2024 at 07:21:35AM +0900, Masahiro Yamada wrote:
> On Mon, Aug 12, 2024 at 12:07 PM Bjorn Andersson <andersson@kernel.org> wrote:
[..]
> > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> > index 1e561fd8b86e..4a0ee9ef72e2 100644
> > --- a/drivers/usb/dwc3/core.h
> > +++ b/drivers/usb/dwc3/core.h
> > @@ -1160,6 +1160,7 @@ struct dwc3_scratchpad_array {
> >   * @gsbuscfg0_reqinfo: store GSBUSCFG0.DATRDREQINFO, DESRDREQINFO,
> >   *                    DATWRREQINFO, and DESWRREQINFO value passed from
> >   *                    glue driver.
> > + * @glue: private reference to any glue context
> >   */
> >  struct dwc3 {
> >         struct work_struct      drd_work;
> > @@ -1388,6 +1389,8 @@ struct dwc3 {
> >         int                     num_ep_resized;
> >         struct dentry           *debug_root;
> >         u32                     gsbuscfg0_reqinfo;
> > +
> > +       void                    *glue;
> 
> 
> This is ugly.
> 
> 
> 'struct dwc3_qcom' can wrap 'struct dwc3'
> instead of having two separate structures
> pointing at each other.
> 
> 
> You can use container_of() to
> convert (struct dwc3 *) to (struct dwc3_qcom *).
> 
> 
> I attached a diff to delete the reverse ->glue pointer.
> 
> I did not compile-test it (I cannot due to missing glue.h
> anyway), but you will understand my suggestion.
> 

Thanks for your suggestion, Yamada-san.
I agree, that will look better.

Regards,
Bjorn
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 734de2a8bd21..6addb3c367e6 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -36,6 +36,7 @@ 
 
 #include "core.h"
 #include "gadget.h"
+#include "glue.h"
 #include "io.h"
 
 #include "debug.h"
@@ -2076,10 +2077,11 @@  static int dwc3_get_num_ports(struct dwc3 *dwc)
 	return 0;
 }
 
-static int dwc3_probe(struct platform_device *pdev)
+struct dwc3 *dwc3_probe(struct platform_device *pdev, struct resource *res,
+			bool ignore_clocks_and_resets, void *glue)
 {
 	struct device		*dev = &pdev->dev;
-	struct resource		*res, dwc_res;
+	struct resource		dwc_res;
 	unsigned int		hw_mode;
 	void __iomem		*regs;
 	struct dwc3		*dwc;
@@ -2087,15 +2089,10 @@  static int dwc3_probe(struct platform_device *pdev)
 
 	dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL);
 	if (!dwc)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	dwc->dev = dev;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		dev_err(dev, "missing memory resource\n");
-		return -ENODEV;
-	}
+	dwc->glue = glue;
 
 	dwc->xhci_resources[0].start = res->start;
 	dwc->xhci_resources[0].end = dwc->xhci_resources[0].start +
@@ -2123,7 +2120,7 @@  static int dwc3_probe(struct platform_device *pdev)
 
 	regs = devm_ioremap_resource(dev, &dwc_res);
 	if (IS_ERR(regs))
-		return PTR_ERR(regs);
+		return ERR_CAST(regs);
 
 	dwc->regs	= regs;
 	dwc->regs_size	= resource_size(&dwc_res);
@@ -2132,15 +2129,17 @@  static int dwc3_probe(struct platform_device *pdev)
 
 	dwc3_get_software_properties(dwc);
 
-	dwc->reset = devm_reset_control_array_get_optional_shared(dev);
-	if (IS_ERR(dwc->reset)) {
-		ret = PTR_ERR(dwc->reset);
-		goto err_put_psy;
-	}
+	if (!ignore_clocks_and_resets) {
+		dwc->reset = devm_reset_control_array_get_optional_shared(dev);
+		if (IS_ERR(dwc->reset)) {
+			ret = PTR_ERR(dwc->reset);
+			goto err_put_psy;
+		}
 
-	ret = dwc3_get_clocks(dwc);
-	if (ret)
-		goto err_put_psy;
+		ret = dwc3_get_clocks(dwc);
+		if (ret)
+			goto err_put_psy;
+	}
 
 	ret = reset_control_deassert(dwc->reset);
 	if (ret)
@@ -2225,7 +2224,7 @@  static int dwc3_probe(struct platform_device *pdev)
 
 	dma_set_max_seg_size(dev, UINT_MAX);
 
-	return 0;
+	return dwc;
 
 err_exit_debugfs:
 	dwc3_debugfs_exit(dwc);
@@ -2249,14 +2248,33 @@  static int dwc3_probe(struct platform_device *pdev)
 	if (dwc->usb_psy)
 		power_supply_put(dwc->usb_psy);
 
-	return ret;
+	return ERR_PTR(ret);
 }
+EXPORT_SYMBOL_GPL(dwc3_probe);
 
-static void dwc3_remove(struct platform_device *pdev)
+static int dwc3_plat_probe(struct platform_device *pdev)
 {
-	struct dwc3	*dwc = platform_get_drvdata(pdev);
+	struct resource *res;
+	struct dwc3 *dwc;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "missing memory resource\n");
+		return -ENODEV;
+	}
 
-	pm_runtime_get_sync(&pdev->dev);
+	dwc = dwc3_probe(pdev, res, false, NULL);
+	if (IS_ERR(dwc))
+		return PTR_ERR(dwc);
+
+	platform_set_drvdata(pdev, dwc);
+
+	return 0;
+}
+
+void dwc3_remove(struct dwc3 *dwc)
+{
+	pm_runtime_get_sync(dwc->dev);
 
 	dwc3_core_exit_mode(dwc);
 	dwc3_debugfs_exit(dwc);
@@ -2264,22 +2282,28 @@  static void dwc3_remove(struct platform_device *pdev)
 	dwc3_core_exit(dwc);
 	dwc3_ulpi_exit(dwc);
 
-	pm_runtime_allow(&pdev->dev);
-	pm_runtime_disable(&pdev->dev);
-	pm_runtime_dont_use_autosuspend(&pdev->dev);
-	pm_runtime_put_noidle(&pdev->dev);
+	pm_runtime_allow(dwc->dev);
+	pm_runtime_disable(dwc->dev);
+	pm_runtime_dont_use_autosuspend(dwc->dev);
+	pm_runtime_put_noidle(dwc->dev);
 	/*
 	 * HACK: Clear the driver data, which is currently accessed by parent
 	 * glue drivers, before allowing the parent to suspend.
 	 */
-	platform_set_drvdata(pdev, NULL);
-	pm_runtime_set_suspended(&pdev->dev);
+	dev_set_drvdata(dwc->dev, NULL);
+	pm_runtime_set_suspended(dwc->dev);
 
 	dwc3_free_event_buffers(dwc);
 
 	if (dwc->usb_psy)
 		power_supply_put(dwc->usb_psy);
 }
+EXPORT_SYMBOL_GPL(dwc3_remove);
+
+static void dwc3_plat_remove(struct platform_device *pdev)
+{
+	dwc3_remove(platform_get_drvdata(pdev));
+}
 
 #ifdef CONFIG_PM
 static int dwc3_core_init_for_resume(struct dwc3 *dwc)
@@ -2450,9 +2474,8 @@  static int dwc3_runtime_checks(struct dwc3 *dwc)
 	return 0;
 }
 
-static int dwc3_runtime_suspend(struct device *dev)
+int dwc3_runtime_suspend(struct dwc3 *dwc)
 {
-	struct dwc3     *dwc = dev_get_drvdata(dev);
 	int		ret;
 
 	if (dwc3_runtime_checks(dwc))
@@ -2464,10 +2487,10 @@  static int dwc3_runtime_suspend(struct device *dev)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dwc3_runtime_suspend);
 
-static int dwc3_runtime_resume(struct device *dev)
+int dwc3_runtime_resume(struct dwc3 *dwc)
 {
-	struct dwc3     *dwc = dev_get_drvdata(dev);
 	int		ret;
 
 	ret = dwc3_resume_common(dwc, PMSG_AUTO_RESUME);
@@ -2484,15 +2507,14 @@  static int dwc3_runtime_resume(struct device *dev)
 		break;
 	}
 
-	pm_runtime_mark_last_busy(dev);
+	pm_runtime_mark_last_busy(dwc->dev);
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dwc3_runtime_resume);
 
-static int dwc3_runtime_idle(struct device *dev)
+int dwc3_runtime_idle(struct dwc3 *dwc)
 {
-	struct dwc3     *dwc = dev_get_drvdata(dev);
-
 	switch (dwc->current_dr_role) {
 	case DWC3_GCTL_PRTCAP_DEVICE:
 		if (dwc3_runtime_checks(dwc))
@@ -2504,52 +2526,67 @@  static int dwc3_runtime_idle(struct device *dev)
 		break;
 	}
 
-	pm_runtime_mark_last_busy(dev);
-	pm_runtime_autosuspend(dev);
+	pm_runtime_mark_last_busy(dwc->dev);
+	pm_runtime_autosuspend(dwc->dev);
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dwc3_runtime_idle);
+
+static int dwc3_plat_runtime_suspend(struct device *dev)
+{
+	return dwc3_runtime_suspend(dev_get_drvdata(dev));
+}
+
+static int dwc3_plat_runtime_resume(struct device *dev)
+{
+	return dwc3_runtime_resume(dev_get_drvdata(dev));
+}
+
+static int dwc3_plat_runtime_idle(struct device *dev)
+{
+	return dwc3_runtime_idle(dev_get_drvdata(dev));
+}
 #endif /* CONFIG_PM */
 
 #ifdef CONFIG_PM_SLEEP
-static int dwc3_suspend(struct device *dev)
+int dwc3_suspend(struct dwc3 *dwc)
 {
-	struct dwc3	*dwc = dev_get_drvdata(dev);
 	int		ret;
 
 	ret = dwc3_suspend_common(dwc, PMSG_SUSPEND);
 	if (ret)
 		return ret;
 
-	pinctrl_pm_select_sleep_state(dev);
+	pinctrl_pm_select_sleep_state(dwc->dev);
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dwc3_suspend);
 
-static int dwc3_resume(struct device *dev)
+int dwc3_resume(struct dwc3 *dwc)
 {
-	struct dwc3	*dwc = dev_get_drvdata(dev);
 	int		ret;
 
-	pinctrl_pm_select_default_state(dev);
+	pinctrl_pm_select_default_state(dwc->dev);
 
-	pm_runtime_disable(dev);
-	pm_runtime_set_active(dev);
+	pm_runtime_disable(dwc->dev);
+	pm_runtime_set_active(dwc->dev);
 
 	ret = dwc3_resume_common(dwc, PMSG_RESUME);
 	if (ret) {
-		pm_runtime_set_suspended(dev);
+		pm_runtime_set_suspended(dwc->dev);
 		return ret;
 	}
 
-	pm_runtime_enable(dev);
+	pm_runtime_enable(dwc->dev);
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dwc3_resume);
 
-static void dwc3_complete(struct device *dev)
+void dwc3_complete(struct dwc3 *dwc)
 {
-	struct dwc3	*dwc = dev_get_drvdata(dev);
 	u32		reg;
 
 	if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST &&
@@ -2559,15 +2596,31 @@  static void dwc3_complete(struct device *dev)
 		dwc3_writel(dwc->regs, DWC3_GUCTL3, reg);
 	}
 }
+EXPORT_SYMBOL_GPL(dwc3_complete);
+
+static int dwc3_plat_suspend(struct device *dev)
+{
+	return dwc3_suspend(dev_get_drvdata(dev));
+}
+
+static int dwc3_plat_resume(struct device *dev)
+{
+	return dwc3_resume(dev_get_drvdata(dev));
+}
+
+static void dwc3_plat_complete(struct device *dev)
+{
+	dwc3_complete(dev_get_drvdata(dev));
+}
 #else
-#define dwc3_complete NULL
+#define dwc3_plat_complete NULL
 #endif /* CONFIG_PM_SLEEP */
 
 static const struct dev_pm_ops dwc3_dev_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
-	.complete = dwc3_complete,
-	SET_RUNTIME_PM_OPS(dwc3_runtime_suspend, dwc3_runtime_resume,
-			dwc3_runtime_idle)
+	SET_SYSTEM_SLEEP_PM_OPS(dwc3_plat_suspend, dwc3_plat_resume)
+	.complete = dwc3_plat_complete,
+	SET_RUNTIME_PM_OPS(dwc3_plat_runtime_suspend, dwc3_plat_runtime_resume,
+			   dwc3_plat_runtime_idle)
 };
 
 #ifdef CONFIG_OF
@@ -2595,8 +2648,8 @@  MODULE_DEVICE_TABLE(acpi, dwc3_acpi_match);
 #endif
 
 static struct platform_driver dwc3_driver = {
-	.probe		= dwc3_probe,
-	.remove_new	= dwc3_remove,
+	.probe		= dwc3_plat_probe,
+	.remove_new	= dwc3_plat_remove,
 	.driver		= {
 		.name	= "dwc3",
 		.of_match_table	= of_match_ptr(of_dwc3_match),
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 1e561fd8b86e..4a0ee9ef72e2 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1160,6 +1160,7 @@  struct dwc3_scratchpad_array {
  * @gsbuscfg0_reqinfo: store GSBUSCFG0.DATRDREQINFO, DESRDREQINFO,
  *		       DATWRREQINFO, and DESWRREQINFO value passed from
  *		       glue driver.
+ * @glue: private reference to any glue context
  */
 struct dwc3 {
 	struct work_struct	drd_work;
@@ -1388,6 +1389,8 @@  struct dwc3 {
 	int			num_ep_resized;
 	struct dentry		*debug_root;
 	u32			gsbuscfg0_reqinfo;
+
+	void			*glue;
 };
 
 #define INCRX_BURST_MODE 0