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:
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-Postpone... 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
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-Postpone... 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
Hi
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag fixing commit: 117777b2c3bb ("usb: dwc2: Move gadget probe function into platform code").
The bot has tested the following trees: v5.6.15, v5.4.43, v4.19.125, v4.14.182, v4.9.225, v4.4.225.
v5.6.15: Build failed! Errors: drivers/usb/dwc2/platform.c:515:4: error: label ‘error_init’ used but not defined
v5.4.43: Build failed! Errors: drivers/usb/dwc2/platform.c:515:4: error: label ‘error_init’ used but not defined
v4.19.125: Build failed! Errors: drivers/usb/dwc2/platform.c:500:4: error: label ‘error_init’ used but not defined
v4.14.182: Build failed! Errors: drivers/usb/dwc2/platform.c:460:4: error: label ‘error_init’ used but not defined
v4.9.225: Build failed! Errors: drivers/usb/dwc2/platform.c:669:4: error: label ‘error_init’ used but not defined
v4.4.225: Build failed! Errors: drivers/usb/dwc2/platform.c:460:4: error: label ‘error_init’ used but not defined
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
linux-stable-mirror@lists.linaro.org
-
kbuild test robot -
Minas Harutyunyan -
Sasha Levin