* Re: [PATCH] PCI: designware-ep: Fix the Header Type check
2020-08-14 8:08 [PATCH] PCI: designware-ep: Fix the Header Type check Zhiqiang Hou
@ 2020-08-14 15:50 ` Rob Herring
2020-08-16 16:35 ` Z.q. Hou
2020-08-16 3:55 ` kernel test robot
2020-08-17 12:04 ` kernel test robot
2 siblings, 1 reply; 5+ messages in thread
From: Rob Herring @ 2020-08-14 15:50 UTC (permalink / raw)
To: Zhiqiang Hou
Cc: linux-kernel, PCI, Lorenzo Pieralisi, Bjorn Helgaas,
Andrew Murray, Jingoo Han, Gustavo Pimentel
On Fri, Aug 14, 2020 at 2:15 AM Zhiqiang Hou <Zhiqiang.Hou@nxp.com> wrote:
>
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>
> The current check will result in the multiple function device
> fails to initialize. So fix the check by masking out the
> multiple function bit.
>
> Fixes: 0b24134f7888 ("PCI: dwc: Add validation that PCIe core is set to correct mode")
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
> drivers/pci/controller/dwc/pcie-designware-ep.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 4680a51c49c0..4b7abfb1e669 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -654,7 +654,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
> int i;
>
> hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
> - if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
> + if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
Should have () around 'hdr_type & 0x7f'.
> dev_err(pci->dev,
> "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
> hdr_type);
However, shouldn't the printed value be masked too? I'd just do:
hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) & 0x7f;
Perhaps add a #define too. '0x7f' is used in several places.
Rob
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH] PCI: designware-ep: Fix the Header Type check
2020-08-14 15:50 ` Rob Herring
@ 2020-08-16 16:35 ` Z.q. Hou
0 siblings, 0 replies; 5+ messages in thread
From: Z.q. Hou @ 2020-08-16 16:35 UTC (permalink / raw)
To: Rob Herring
Cc: linux-kernel, PCI, Lorenzo Pieralisi, Bjorn Helgaas,
Andrew Murray, Jingoo Han, Gustavo Pimentel
Hi Rob,
Thanks a lot for your comments!
> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: 2020年8月14日 23:51
> To: Z.q. Hou <zhiqiang.hou@nxp.com>
> Cc: linux-kernel@vger.kernel.org; PCI <linux-pci@vger.kernel.org>; Lorenzo
> Pieralisi <lorenzo.pieralisi@arm.com>; Bjorn Helgaas
> <bhelgaas@google.com>; Andrew Murray <amurray@thegoodpenguin.co.uk>;
> Jingoo Han <jingoohan1@gmail.com>; Gustavo Pimentel
> <gustavo.pimentel@synopsys.com>
> Subject: Re: [PATCH] PCI: designware-ep: Fix the Header Type check
>
> On Fri, Aug 14, 2020 at 2:15 AM Zhiqiang Hou <Zhiqiang.Hou@nxp.com>
> wrote:
> >
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > The current check will result in the multiple function device fails to
> > initialize. So fix the check by masking out the multiple function bit.
> >
> > Fixes: 0b24134f7888 ("PCI: dwc: Add validation that PCIe core is set
> > to correct mode")
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> > drivers/pci/controller/dwc/pcie-designware-ep.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 4680a51c49c0..4b7abfb1e669 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -654,7 +654,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep
> *ep)
> > int i;
> >
> > hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
> > - if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
> > + if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
>
> Should have () around 'hdr_type & 0x7f'.
>
> > dev_err(pci->dev,
> > "PCIe controller is not set to EP mode
> (hdr_type:0x%x)!\n",
> > hdr_type);
>
> However, shouldn't the printed value be masked too? I'd just do:
>
> hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE) & 0x7f;
>
> Perhaps add a #define too. '0x7f' is used in several places.
All these are good suggestions, will make it in next version.
Thanks,
Zhiqiang
>
> Rob
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PCI: designware-ep: Fix the Header Type check
2020-08-14 8:08 [PATCH] PCI: designware-ep: Fix the Header Type check Zhiqiang Hou
2020-08-14 15:50 ` Rob Herring
@ 2020-08-16 3:55 ` kernel test robot
2020-08-17 12:04 ` kernel test robot
2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-08-16 3:55 UTC (permalink / raw)
To: Zhiqiang Hou, linux-kernel, linux-pci, lorenzo.pieralisi, robh,
bhelgaas, amurray, jingoohan1, gustavo.pimentel
Cc: kbuild-all, Hou Zhiqiang
[-- Attachment #1: Type: text/plain, Size: 2884 bytes --]
Hi Zhiqiang,
I love your patch! Perhaps something to improve:
[auto build test WARNING on pci/next]
[also build test WARNING on linux/master linus/master v5.8 next-20200814]
[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/Zhiqiang-Hou/PCI-designware-ep-Fix-the-Header-Type-check/20200814-161623
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
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
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/pci/controller/dwc/pcie-designware-ep.c: In function 'dw_pcie_ep_init_complete':
>> drivers/pci/controller/dwc/pcie-designware-ep.c:509:15: warning: suggest parentheses around comparison in operand of '&' [-Wparentheses]
509 | if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
| ^
vim +509 drivers/pci/controller/dwc/pcie-designware-ep.c
498
499 int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
500 {
501 struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
502 unsigned int offset;
503 unsigned int nbars;
504 u8 hdr_type;
505 u32 reg;
506 int i;
507
508 hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
> 509 if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
510 dev_err(pci->dev,
511 "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
512 hdr_type);
513 return -EIO;
514 }
515
516 ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
517
518 ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
519
520 offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
521 if (offset) {
522 reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
523 nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
524 PCI_REBAR_CTRL_NBAR_SHIFT;
525
526 dw_pcie_dbi_ro_wr_en(pci);
527 for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
528 dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
529 dw_pcie_dbi_ro_wr_dis(pci);
530 }
531
532 dw_pcie_setup(pci);
533
534 return 0;
535 }
536 EXPORT_SYMBOL_GPL(dw_pcie_ep_init_complete);
537
---
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: 65036 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] PCI: designware-ep: Fix the Header Type check
2020-08-14 8:08 [PATCH] PCI: designware-ep: Fix the Header Type check Zhiqiang Hou
2020-08-14 15:50 ` Rob Herring
2020-08-16 3:55 ` kernel test robot
@ 2020-08-17 12:04 ` kernel test robot
2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2020-08-17 12:04 UTC (permalink / raw)
To: Zhiqiang Hou, linux-kernel, linux-pci, lorenzo.pieralisi, robh,
bhelgaas, amurray, jingoohan1, gustavo.pimentel
Cc: kbuild-all, clang-built-linux, Hou Zhiqiang
[-- Attachment #1: Type: text/plain, Size: 3850 bytes --]
Hi Zhiqiang,
I love your patch! Perhaps something to improve:
[auto build test WARNING on pci/next]
[also build test WARNING on linux/master linus/master v5.9-rc1 next-20200817]
[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/Zhiqiang-Hou/PCI-designware-ep-Fix-the-Header-Type-check/20200814-161623
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-randconfig-r013-20200817 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project de71b46a519db014ce906a39f8a0e1b235ef1568)
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 warnings (new ones prefixed by >>):
>> drivers/pci/controller/dwc/pcie-designware-ep.c:509:15: warning: & has lower precedence than !=; != will be evaluated first [-Wparentheses]
if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/dwc/pcie-designware-ep.c:509:15: note: place parentheses around the '!=' expression to silence this warning
if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/dwc/pcie-designware-ep.c:509:15: note: place parentheses around the & expression to evaluate it first
if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
^
( )
1 warning generated.
# https://github.com/0day-ci/linux/commit/819b7848e81d962a90377b2988abf5862ab35d01
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Zhiqiang-Hou/PCI-designware-ep-Fix-the-Header-Type-check/20200814-161623
git checkout 819b7848e81d962a90377b2988abf5862ab35d01
vim +509 drivers/pci/controller/dwc/pcie-designware-ep.c
498
499 int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
500 {
501 struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
502 unsigned int offset;
503 unsigned int nbars;
504 u8 hdr_type;
505 u32 reg;
506 int i;
507
508 hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
> 509 if (hdr_type & 0x7f != PCI_HEADER_TYPE_NORMAL) {
510 dev_err(pci->dev,
511 "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
512 hdr_type);
513 return -EIO;
514 }
515
516 ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
517
518 ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
519
520 offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
521 if (offset) {
522 reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
523 nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
524 PCI_REBAR_CTRL_NBAR_SHIFT;
525
526 dw_pcie_dbi_ro_wr_en(pci);
527 for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
528 dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
529 dw_pcie_dbi_ro_wr_dis(pci);
530 }
531
532 dw_pcie_setup(pci);
533
534 return 0;
535 }
536 EXPORT_SYMBOL_GPL(dw_pcie_ep_init_complete);
537
---
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: 33375 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread