diff mbox series

usb: dwc2: Postponed gadget registration to the udc class driver

Message ID 137e787bf7c7935bda3358c8f07230d3f4998fad.1590745119.git.hminas@synopsys.com (mailing list archive)
State Superseded
Headers show
Series usb: dwc2: Postponed gadget registration to the udc class driver | expand

Commit Message

Minas Harutyunyan May 29, 2020, 10:12 a.m. UTC 
During dwc2 driver probe, after gadget registration to the udc class
driver, if exist any builtin function driver it immediately bound to
dwc2 and after init host side (dwc2_hcd_init()) stucked in host mode.
Patch postpone gadget registration after host side initialization done.

Cc: stable@vger.kernel.org
Fixes: 117777b2c3bb9 ("usb: dwc2: Move gadget probe function into
platform code")
Tested-by: Marek Vasut <marex@denx.de>

Signed-off-by: Minas Harutyunyan <hminas@synopsys.com>
---
 drivers/usb/dwc2/gadget.c   | 6 ------
 drivers/usb/dwc2/platform.c | 9 +++++++++
 2 files changed, 9 insertions(+), 6 deletions(-)

Comments

kernel test robot May 31, 2020, 1:33 p.m. UTC | #1 
Hi Minas,

I love your patch! Yet something to improve:

[auto build test ERROR on balbi-usb/testing/next]
[also build test ERROR on usb/usb-testing peter.chen-usb/ci-for-usb-next v5.7-rc7 next-20200529]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Minas-Harutyunyan/usb-dwc2-Postponed-gadget-registration-to-the-udc-class-driver/20200531-074103
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next
config: c6x-randconfig-c022-20200531 (attached as .config)
compiler: c6x-elf-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

drivers/usb/dwc2/platform.c: In function 'dwc2_driver_probe':
>> drivers/usb/dwc2/platform.c:580:49: error: 'struct dwc2_hsotg' has no member named 'gadget'
580 |   retval = usb_add_gadget_udc(hsotg->dev, &hsotg->gadget);
|                                                 ^~

vim +580 drivers/usb/dwc2/platform.c

   395	
   396	/**
   397	 * dwc2_driver_probe() - Called when the DWC_otg core is bound to the DWC_otg
   398	 * driver
   399	 *
   400	 * @dev: Platform device
   401	 *
   402	 * This routine creates the driver components required to control the device
   403	 * (core, HCD, and PCD) and initializes the device. The driver components are
   404	 * stored in a dwc2_hsotg structure. A reference to the dwc2_hsotg is saved
   405	 * in the device private data. This allows the driver to access the dwc2_hsotg
   406	 * structure on subsequent calls to driver methods for this device.
   407	 */
   408	static int dwc2_driver_probe(struct platform_device *dev)
   409	{
   410		struct dwc2_hsotg *hsotg;
   411		struct resource *res;
   412		int retval;
   413	
   414		hsotg = devm_kzalloc(&dev->dev, sizeof(*hsotg), GFP_KERNEL);
   415		if (!hsotg)
   416			return -ENOMEM;
   417	
   418		hsotg->dev = &dev->dev;
   419	
   420		/*
   421		 * Use reasonable defaults so platforms don't have to provide these.
   422		 */
   423		if (!dev->dev.dma_mask)
   424			dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
   425		retval = dma_set_coherent_mask(&dev->dev, DMA_BIT_MASK(32));
   426		if (retval) {
   427			dev_err(&dev->dev, "can't set coherent DMA mask: %d\n", retval);
   428			return retval;
   429		}
   430	
   431		hsotg->regs = devm_platform_get_and_ioremap_resource(dev, 0, &res);
   432		if (IS_ERR(hsotg->regs))
   433			return PTR_ERR(hsotg->regs);
   434	
   435		dev_dbg(&dev->dev, "mapped PA %08lx to VA %p\n",
   436			(unsigned long)res->start, hsotg->regs);
   437	
   438		retval = dwc2_lowlevel_hw_init(hsotg);
   439		if (retval)
   440			return retval;
   441	
   442		spin_lock_init(&hsotg->lock);
   443	
   444		hsotg->irq = platform_get_irq(dev, 0);
   445		if (hsotg->irq < 0)
   446			return hsotg->irq;
   447	
   448		dev_dbg(hsotg->dev, "registering common handler for irq%d\n",
   449			hsotg->irq);
   450		retval = devm_request_irq(hsotg->dev, hsotg->irq,
   451					  dwc2_handle_common_intr, IRQF_SHARED,
   452					  dev_name(hsotg->dev), hsotg);
   453		if (retval)
   454			return retval;
   455	
   456		hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
   457		if (IS_ERR(hsotg->vbus_supply)) {
   458			retval = PTR_ERR(hsotg->vbus_supply);
   459			hsotg->vbus_supply = NULL;
   460			if (retval != -ENODEV)
   461				return retval;
   462		}
   463	
   464		retval = dwc2_lowlevel_hw_enable(hsotg);
   465		if (retval)
   466			return retval;
   467	
   468		hsotg->needs_byte_swap = dwc2_check_core_endianness(hsotg);
   469	
   470		retval = dwc2_get_dr_mode(hsotg);
   471		if (retval)
   472			goto error;
   473	
   474		hsotg->need_phy_for_wake =
   475			of_property_read_bool(dev->dev.of_node,
   476					      "snps,need-phy-for-wake");
   477	
   478		/*
   479		 * Before performing any core related operations
   480		 * check core version.
   481		 */
   482		retval = dwc2_check_core_version(hsotg);
   483		if (retval)
   484			goto error;
   485	
   486		/*
   487		 * Reset before dwc2_get_hwparams() then it could get power-on real
   488		 * reset value form registers.
   489		 */
   490		retval = dwc2_core_reset(hsotg, false);
   491		if (retval)
   492			goto error;
   493	
   494		/* Detect config values from hardware */
   495		retval = dwc2_get_hwparams(hsotg);
   496		if (retval)
   497			goto error;
   498	
   499		/*
   500		 * For OTG cores, set the force mode bits to reflect the value
   501		 * of dr_mode. Force mode bits should not be touched at any
   502		 * other time after this.
   503		 */
   504		dwc2_force_dr_mode(hsotg);
   505	
   506		retval = dwc2_init_params(hsotg);
   507		if (retval)
   508			goto error;
   509	
   510		if (hsotg->params.activate_stm_id_vb_detection) {
   511			u32 ggpio;
   512	
   513			hsotg->usb33d = devm_regulator_get(hsotg->dev, "usb33d");
   514			if (IS_ERR(hsotg->usb33d)) {
   515				retval = PTR_ERR(hsotg->usb33d);
   516				if (retval != -EPROBE_DEFER)
   517					dev_err(hsotg->dev,
   518						"failed to request usb33d supply: %d\n",
   519						retval);
   520				goto error;
   521			}
   522			retval = regulator_enable(hsotg->usb33d);
   523			if (retval) {
   524				dev_err(hsotg->dev,
   525					"failed to enable usb33d supply: %d\n", retval);
   526				goto error;
   527			}
   528	
   529			ggpio = dwc2_readl(hsotg, GGPIO);
   530			ggpio |= GGPIO_STM32_OTG_GCCFG_IDEN;
   531			ggpio |= GGPIO_STM32_OTG_GCCFG_VBDEN;
   532			dwc2_writel(hsotg, ggpio, GGPIO);
   533		}
   534	
   535		if (hsotg->dr_mode != USB_DR_MODE_HOST) {
   536			retval = dwc2_gadget_init(hsotg);
   537			if (retval)
   538				goto error_init;
   539			hsotg->gadget_enabled = 1;
   540		}
   541	
   542		/*
   543		 * If we need PHY for wakeup we must be wakeup capable.
   544		 * When we have a device that can wake without the PHY we
   545		 * can adjust this condition.
   546		 */
   547		if (hsotg->need_phy_for_wake)
   548			device_set_wakeup_capable(&dev->dev, true);
   549	
   550		hsotg->reset_phy_on_wake =
   551			of_property_read_bool(dev->dev.of_node,
   552					      "snps,reset-phy-on-wake");
   553		if (hsotg->reset_phy_on_wake && !hsotg->phy) {
   554			dev_warn(hsotg->dev,
   555				 "Quirk reset-phy-on-wake only supports generic PHYs\n");
   556			hsotg->reset_phy_on_wake = false;
   557		}
   558	
   559		if (hsotg->dr_mode != USB_DR_MODE_PERIPHERAL) {
   560			retval = dwc2_hcd_init(hsotg);
   561			if (retval) {
   562				if (hsotg->gadget_enabled)
   563					dwc2_hsotg_remove(hsotg);
   564				goto error_init;
   565			}
   566			hsotg->hcd_enabled = 1;
   567		}
   568	
   569		platform_set_drvdata(dev, hsotg);
   570		hsotg->hibernated = 0;
   571	
   572		dwc2_debugfs_init(hsotg);
   573	
   574		/* Gadget code manages lowlevel hw on its own */
   575		if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL)
   576			dwc2_lowlevel_hw_disable(hsotg);
   577	
   578		/* Postponed adding a new gadget to the udc class driver list */
   579		if (hsotg->gadget_enabled) {
 > 580			retval = usb_add_gadget_udc(hsotg->dev, &hsotg->gadget);
   581			if (retval) {
   582				dwc2_hsotg_remove(hsotg);
   583				goto error_init;
   584			}
   585		}
   586	
   587		return 0;
   588	
   589	error_init:
   590		if (hsotg->params.activate_stm_id_vb_detection)
   591			regulator_disable(hsotg->usb33d);
   592	error:
   593		dwc2_lowlevel_hw_disable(hsotg);
   594		return retval;
   595	}
   596	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot May 31, 2020, 3:45 p.m. UTC | #2 
Hi Minas,

I love your patch! Yet something to improve:

[auto build test ERROR on balbi-usb/testing/next]
[also build test ERROR on usb/usb-testing v5.7-rc7 next-20200529]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Minas-Harutyunyan/usb-dwc2-Postponed-gadget-registration-to-the-udc-class-driver/20200531-074103
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next
config: x86_64-randconfig-a005-20200531 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 2388a096e7865c043e83ece4e26654bd3d1a20d5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> drivers/usb/dwc2/platform.c:580:51: error: no member named 'gadget' in 'struct dwc2_hsotg'
retval = usb_add_gadget_udc(hsotg->dev, &hsotg->gadget);
~~~~~  ^
1 error generated.

vim +580 drivers/usb/dwc2/platform.c

   395	
   396	/**
   397	 * dwc2_driver_probe() - Called when the DWC_otg core is bound to the DWC_otg
   398	 * driver
   399	 *
   400	 * @dev: Platform device
   401	 *
   402	 * This routine creates the driver components required to control the device
   403	 * (core, HCD, and PCD) and initializes the device. The driver components are
   404	 * stored in a dwc2_hsotg structure. A reference to the dwc2_hsotg is saved
   405	 * in the device private data. This allows the driver to access the dwc2_hsotg
   406	 * structure on subsequent calls to driver methods for this device.
   407	 */
   408	static int dwc2_driver_probe(struct platform_device *dev)
   409	{
   410		struct dwc2_hsotg *hsotg;
   411		struct resource *res;
   412		int retval;
   413	
   414		hsotg = devm_kzalloc(&dev->dev, sizeof(*hsotg), GFP_KERNEL);
   415		if (!hsotg)
   416			return -ENOMEM;
   417	
   418		hsotg->dev = &dev->dev;
   419	
   420		/*
   421		 * Use reasonable defaults so platforms don't have to provide these.
   422		 */
   423		if (!dev->dev.dma_mask)
   424			dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
   425		retval = dma_set_coherent_mask(&dev->dev, DMA_BIT_MASK(32));
   426		if (retval) {
   427			dev_err(&dev->dev, "can't set coherent DMA mask: %d\n", retval);
   428			return retval;
   429		}
   430	
   431		hsotg->regs = devm_platform_get_and_ioremap_resource(dev, 0, &res);
   432		if (IS_ERR(hsotg->regs))
   433			return PTR_ERR(hsotg->regs);
   434	
   435		dev_dbg(&dev->dev, "mapped PA %08lx to VA %p\n",
   436			(unsigned long)res->start, hsotg->regs);
   437	
   438		retval = dwc2_lowlevel_hw_init(hsotg);
   439		if (retval)
   440			return retval;
   441	
   442		spin_lock_init(&hsotg->lock);
   443	
   444		hsotg->irq = platform_get_irq(dev, 0);
   445		if (hsotg->irq < 0)
   446			return hsotg->irq;
   447	
   448		dev_dbg(hsotg->dev, "registering common handler for irq%d\n",
   449			hsotg->irq);
   450		retval = devm_request_irq(hsotg->dev, hsotg->irq,
   451					  dwc2_handle_common_intr, IRQF_SHARED,
   452					  dev_name(hsotg->dev), hsotg);
   453		if (retval)
   454			return retval;
   455	
   456		hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
   457		if (IS_ERR(hsotg->vbus_supply)) {
   458			retval = PTR_ERR(hsotg->vbus_supply);
   459			hsotg->vbus_supply = NULL;
   460			if (retval != -ENODEV)
   461				return retval;
   462		}
   463	
   464		retval = dwc2_lowlevel_hw_enable(hsotg);
   465		if (retval)
   466			return retval;
   467	
   468		hsotg->needs_byte_swap = dwc2_check_core_endianness(hsotg);
   469	
   470		retval = dwc2_get_dr_mode(hsotg);
   471		if (retval)
   472			goto error;
   473	
   474		hsotg->need_phy_for_wake =
   475			of_property_read_bool(dev->dev.of_node,
   476					      "snps,need-phy-for-wake");
   477	
   478		/*
   479		 * Before performing any core related operations
   480		 * check core version.
   481		 */
   482		retval = dwc2_check_core_version(hsotg);
   483		if (retval)
   484			goto error;
   485	
   486		/*
   487		 * Reset before dwc2_get_hwparams() then it could get power-on real
   488		 * reset value form registers.
   489		 */
   490		retval = dwc2_core_reset(hsotg, false);
   491		if (retval)
   492			goto error;
   493	
   494		/* Detect config values from hardware */
   495		retval = dwc2_get_hwparams(hsotg);
   496		if (retval)
   497			goto error;
   498	
   499		/*
   500		 * For OTG cores, set the force mode bits to reflect the value
   501		 * of dr_mode. Force mode bits should not be touched at any
   502		 * other time after this.
   503		 */
   504		dwc2_force_dr_mode(hsotg);
   505	
   506		retval = dwc2_init_params(hsotg);
   507		if (retval)
   508			goto error;
   509	
   510		if (hsotg->params.activate_stm_id_vb_detection) {
   511			u32 ggpio;
   512	
   513			hsotg->usb33d = devm_regulator_get(hsotg->dev, "usb33d");
   514			if (IS_ERR(hsotg->usb33d)) {
   515				retval = PTR_ERR(hsotg->usb33d);
   516				if (retval != -EPROBE_DEFER)
   517					dev_err(hsotg->dev,
   518						"failed to request usb33d supply: %d\n",
   519						retval);
   520				goto error;
   521			}
   522			retval = regulator_enable(hsotg->usb33d);
   523			if (retval) {
   524				dev_err(hsotg->dev,
   525					"failed to enable usb33d supply: %d\n", retval);
   526				goto error;
   527			}
   528	
   529			ggpio = dwc2_readl(hsotg, GGPIO);
   530			ggpio |= GGPIO_STM32_OTG_GCCFG_IDEN;
   531			ggpio |= GGPIO_STM32_OTG_GCCFG_VBDEN;
   532			dwc2_writel(hsotg, ggpio, GGPIO);
   533		}
   534	
   535		if (hsotg->dr_mode != USB_DR_MODE_HOST) {
   536			retval = dwc2_gadget_init(hsotg);
   537			if (retval)
   538				goto error_init;
   539			hsotg->gadget_enabled = 1;
   540		}
   541	
   542		/*
   543		 * If we need PHY for wakeup we must be wakeup capable.
   544		 * When we have a device that can wake without the PHY we
   545		 * can adjust this condition.
   546		 */
   547		if (hsotg->need_phy_for_wake)
   548			device_set_wakeup_capable(&dev->dev, true);
   549	
   550		hsotg->reset_phy_on_wake =
   551			of_property_read_bool(dev->dev.of_node,
   552					      "snps,reset-phy-on-wake");
   553		if (hsotg->reset_phy_on_wake && !hsotg->phy) {
   554			dev_warn(hsotg->dev,
   555				 "Quirk reset-phy-on-wake only supports generic PHYs\n");
   556			hsotg->reset_phy_on_wake = false;
   557		}
   558	
   559		if (hsotg->dr_mode != USB_DR_MODE_PERIPHERAL) {
   560			retval = dwc2_hcd_init(hsotg);
   561			if (retval) {
   562				if (hsotg->gadget_enabled)
   563					dwc2_hsotg_remove(hsotg);
   564				goto error_init;
   565			}
   566			hsotg->hcd_enabled = 1;
   567		}
   568	
   569		platform_set_drvdata(dev, hsotg);
   570		hsotg->hibernated = 0;
   571	
   572		dwc2_debugfs_init(hsotg);
   573	
   574		/* Gadget code manages lowlevel hw on its own */
   575		if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL)
   576			dwc2_lowlevel_hw_disable(hsotg);
   577	
   578		/* Postponed adding a new gadget to the udc class driver list */
   579		if (hsotg->gadget_enabled) {
 > 580			retval = usb_add_gadget_udc(hsotg->dev, &hsotg->gadget);
   581			if (retval) {
   582				dwc2_hsotg_remove(hsotg);
   583				goto error_init;
   584			}
   585		}
   586	
   587		return 0;
   588	
   589	error_init:
   590		if (hsotg->params.activate_stm_id_vb_detection)
   591			regulator_disable(hsotg->usb33d);
   592	error:
   593		dwc2_lowlevel_hw_disable(hsotg);
   594		return retval;
   595	}
   596	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 12b98b466287..7faf5f8c056d 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -4920,12 +4920,6 @@  int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
 					  epnum, 0);
 	}
 
-	ret = usb_add_gadget_udc(dev, &hsotg->gadget);
-	if (ret) {
-		dwc2_hsotg_ep_free_request(&hsotg->eps_out[0]->ep,
-					   hsotg->ctrl_req);
-		return ret;
-	}
 	dwc2_hsotg_dump(hsotg);
 
 	return 0;
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index e571c8ae65ec..6b4043117e97 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -575,6 +575,15 @@  static int dwc2_driver_probe(struct platform_device *dev)
 	if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL)
 		dwc2_lowlevel_hw_disable(hsotg);
 
+	/* Postponed adding a new gadget to the udc class driver list */
+	if (hsotg->gadget_enabled) {
+		retval = usb_add_gadget_udc(hsotg->dev, &hsotg->gadget);
+		if (retval) {
+			dwc2_hsotg_remove(hsotg);
+			goto error_init;
+		}
+	}
+
 	return 0;
 
 error_init: