Message ID | 20200923231517.221310-4-refactormyself@gmail.com |
---|---|
State | New |
Headers | show |
Series | PCI: Move some ASPM info to struct pci_dev | expand |
Hi "Saheed, Thank you for the patch! Yet something to improve: [auto build test ERROR on pci/next] [also build test ERROR on v5.9-rc6 next-20200924] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Saheed-O-Bolarinwa/PCI-Move-some-ASPM-info-to-struct-pci_dev/20200924-081626 base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next config: x86_64-randconfig-r024-20200924 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project d6ac649ccda289ecc2d2c0cb51892d57e8ec328c) 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: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> drivers/pci/pcie/aspm.c:542:2: error: void function 'aspm_support' should not return a value [-Wreturn-type] return (pdev->lnkcap & PCI_EXP_LNKCAP_ASPMS) >> 10; ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/pci/pcie/aspm.c:566:29: error: invalid operands to binary expression ('void' and 'void') if (!(aspm_support(parent) & aspm_support(child))) ~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~ drivers/pci/pcie/aspm.c:586:27: error: invalid operands to binary expression ('void' and 'void') if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L0S) ~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~ drivers/pci/pcie/aspm.c:597:27: error: invalid operands to binary expression ('void' and 'void') if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L1) ~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~ 4 errors generated. # https://github.com/0day-ci/linux/commit/835c34b3f165061415e22e546de958901adca123 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Saheed-O-Bolarinwa/PCI-Move-some-ASPM-info-to-struct-pci_dev/20200924-081626 git checkout 835c34b3f165061415e22e546de958901adca123 vim +/aspm_support +542 drivers/pci/pcie/aspm.c 539 540 static void aspm_support(struct pci_dev *pdev) 541 { > 542 return (pdev->lnkcap & PCI_EXP_LNKCAP_ASPMS) >> 10; 543 } 544 545 static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist) 546 { 547 struct pci_dev *child = link->downstream, *parent = link->pdev; 548 struct pci_bus *linkbus = parent->subordinate; 549 struct aspm_register_info upreg, dwreg; 550 551 if (blacklist) { 552 /* Set enabled/disable so that we will disable ASPM later */ 553 link->aspm_enabled = ASPM_STATE_ALL; 554 link->aspm_disable = ASPM_STATE_ALL; 555 return; 556 } 557 558 /* Get upstream/downstream components' register state */ 559 pcie_get_aspm_reg(parent, &upreg); 560 pcie_get_aspm_reg(child, &dwreg); 561 562 /* 563 * If ASPM not supported, don't mess with the clocks and link, 564 * bail out now. 565 */ > 566 if (!(aspm_support(parent) & aspm_support(child))) 567 return; 568 569 /* Configure common clock before checking latencies */ 570 pcie_aspm_configure_common_clock(link); 571 572 /* 573 * Re-read upstream/downstream components' register state 574 * after clock configuration 575 */ 576 pcie_get_aspm_reg(parent, &upreg); 577 pcie_get_aspm_reg(child, &dwreg); 578 579 /* 580 * Setup L0s state 581 * 582 * Note that we must not enable L0s in either direction on a 583 * given link unless components on both sides of the link each 584 * support L0s. 585 */ 586 if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L0S) 587 link->aspm_support |= ASPM_STATE_L0S; 588 589 if (dwreg.enabled & PCIE_LINK_STATE_L0S) 590 link->aspm_enabled |= ASPM_STATE_L0S_UP; 591 if (upreg.enabled & PCIE_LINK_STATE_L0S) 592 link->aspm_enabled |= ASPM_STATE_L0S_DW; 593 link->latency_up.l0s = calc_l0s_latency(parent); 594 link->latency_dw.l0s = calc_l0s_latency(child); 595 596 /* Setup L1 state */ 597 if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L1) 598 link->aspm_support |= ASPM_STATE_L1; 599 600 if (upreg.enabled & dwreg.enabled & PCIE_LINK_STATE_L1) 601 link->aspm_enabled |= ASPM_STATE_L1; 602 link->latency_up.l1 = calc_l1_latency(parent); 603 link->latency_dw.l1 = calc_l1_latency(child); 604 605 /* Setup L1 substate */ 606 if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_ASPM_L1_1) 607 link->aspm_support |= ASPM_STATE_L1_1; 608 if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_ASPM_L1_2) 609 link->aspm_support |= ASPM_STATE_L1_2; 610 if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_1) 611 link->aspm_support |= ASPM_STATE_L1_1_PCIPM; 612 if (upreg.l1ss_cap & dwreg.l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_2) 613 link->aspm_support |= ASPM_STATE_L1_2_PCIPM; 614 615 if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_1) 616 link->aspm_enabled |= ASPM_STATE_L1_1; 617 if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_2) 618 link->aspm_enabled |= ASPM_STATE_L1_2; 619 if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_1) 620 link->aspm_enabled |= ASPM_STATE_L1_1_PCIPM; 621 if (upreg.l1ss_ctl1 & dwreg.l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_2) 622 link->aspm_enabled |= ASPM_STATE_L1_2_PCIPM; 623 624 if (link->aspm_support & ASPM_STATE_L1SS) 625 aspm_calc_l1ss_info(link, &upreg, &dwreg); 626 627 /* Save default state */ 628 link->aspm_default = link->aspm_enabled; 629 630 /* Setup initial capable state. Will be updated later */ 631 link->aspm_capable = link->aspm_support; 632 633 /* Get and check endpoint acceptable latencies */ 634 list_for_each_entry(child, &linkbus->devices, bus_list) { 635 u32 reg32, encoding; 636 struct aspm_latency *acceptable = 637 &link->acceptable[PCI_FUNC(child->devfn)]; 638 639 if (pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT && 640 pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END) 641 continue; 642 643 pcie_capability_read_dword(child, PCI_EXP_DEVCAP, ®32); 644 /* Calculate endpoint L0s acceptable latency */ 645 encoding = (reg32 & PCI_EXP_DEVCAP_L0S) >> 6; 646 acceptable->l0s = calc_l0s_acceptable(encoding); 647 /* Calculate endpoint L1 acceptable latency */ 648 encoding = (reg32 & PCI_EXP_DEVCAP_L1) >> 9; 649 acceptable->l1 = calc_l1_acceptable(encoding); 650 651 pcie_aspm_check_latency(child); 652 } 653 } 654 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 5f7cf47b6a40..321b328347c1 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -383,7 +383,6 @@ static void encode_l12_threshold(u32 threshold_us, u32 *scale, u32 *value) } struct aspm_register_info { - u32 support:2; u32 enabled:2; /* L1 substates */ @@ -396,12 +395,10 @@ struct aspm_register_info { static void pcie_get_aspm_reg(struct pci_dev *pdev, struct aspm_register_info *info) { - u16 reg16; - u32 reg32 = pdev->lnkcap; + u16 ctl; - info->support = (reg32 & PCI_EXP_LNKCAP_ASPMS) >> 10; - pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, ®16); - info->enabled = reg16 & PCI_EXP_LNKCTL_ASPMC; + pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &ctl); + info->enabled = ctl & PCI_EXP_LNKCTL_ASPMC; /* Read L1 PM substate capabilities */ info->l1ss_cap = info->l1ss_ctl1 = info->l1ss_ctl2 = 0; @@ -540,6 +537,11 @@ static void aspm_calc_l1ss_info(struct pcie_link_state *link, link->l1ss.ctl1 |= t_common_mode << 8 | scale << 29 | value << 16; } +static void aspm_support(struct pci_dev *pdev) +{ + return (pdev->lnkcap & PCI_EXP_LNKCAP_ASPMS) >> 10; +} + static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist) { struct pci_dev *child = link->downstream, *parent = link->pdev; @@ -561,7 +563,7 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist) * If ASPM not supported, don't mess with the clocks and link, * bail out now. */ - if (!(upreg.support & dwreg.support)) + if (!(aspm_support(parent) & aspm_support(child))) return; /* Configure common clock before checking latencies */ @@ -581,8 +583,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist) * given link unless components on both sides of the link each * support L0s. */ - if (dwreg.support & upreg.support & PCIE_LINK_STATE_L0S) + if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L0S) link->aspm_support |= ASPM_STATE_L0S; + if (dwreg.enabled & PCIE_LINK_STATE_L0S) link->aspm_enabled |= ASPM_STATE_L0S_UP; if (upreg.enabled & PCIE_LINK_STATE_L0S) @@ -591,8 +594,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist) link->latency_dw.l0s = calc_l0s_latency(child); /* Setup L1 state */ - if (upreg.support & dwreg.support & PCIE_LINK_STATE_L1) + if (aspm_support(parent) & aspm_support(child) & PCIE_LINK_STATE_L1) link->aspm_support |= ASPM_STATE_L1; + if (upreg.enabled & dwreg.enabled & PCIE_LINK_STATE_L1) link->aspm_enabled |= ASPM_STATE_L1; link->latency_up.l1 = calc_l1_latency(parent);
- Calculate aspm_register_info.support inside aspm_support() - Replace references to aspm_register_info.support with aspm_support(). - In pcie_get_aspm_reg() remove assignment to aspm_register_info.support - Remove aspm_register_info.support Signed-off-by: Saheed O. Bolarinwa <refactormyself@gmail.com> --- drivers/pci/pcie/aspm.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)