linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: dwc2: Postponed gadget registration to the udc class driver
@ 2020-05-29 10:12 Minas Harutyunyan
  2020-05-31 13:33 ` kbuild test robot
  2020-05-31 15:45 ` kbuild test robot
  0 siblings, 2 replies; 3+ messages in thread
From: Minas Harutyunyan @ 2020-05-29 10:12 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Minas Harutyunyan,
	Paul Zimmerman, linux-usb, Felipe Balbi, Dinh Nguyen
  Cc: John Youn, stable, Marek Vasut

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(-)

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:
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] usb: dwc2: Postponed gadget registration to the udc class driver
  2020-05-29 10:12 [PATCH] usb: dwc2: Postponed gadget registration to the udc class driver Minas Harutyunyan
@ 2020-05-31 13:33 ` kbuild test robot
  2020-05-31 15:45 ` kbuild test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2020-05-31 13:33 UTC (permalink / raw)
  To: Minas Harutyunyan, Felipe Balbi, Greg Kroah-Hartman,
	Paul Zimmerman, linux-usb, Dinh Nguyen
  Cc: kbuild-all, John Youn, stable, Marek Vasut

[-- Attachment #1: Type: text/plain, Size: 7845 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28220 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] usb: dwc2: Postponed gadget registration to the udc class driver
  2020-05-29 10:12 [PATCH] usb: dwc2: Postponed gadget registration to the udc class driver Minas Harutyunyan
  2020-05-31 13:33 ` kbuild test robot
@ 2020-05-31 15:45 ` kbuild test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2020-05-31 15:45 UTC (permalink / raw)
  To: Minas Harutyunyan, Felipe Balbi, Greg Kroah-Hartman,
	Paul Zimmerman, linux-usb, Dinh Nguyen
  Cc: kbuild-all, clang-built-linux, John Youn, stable, Marek Vasut

[-- Attachment #1: Type: text/plain, Size: 8221 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 41336 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-05-31 15:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-29 10:12 [PATCH] usb: dwc2: Postponed gadget registration to the udc class driver Minas Harutyunyan
2020-05-31 13:33 ` kbuild test robot
2020-05-31 15:45 ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).