From: Xu Yilun <yilun.xu@intel.com> To: Tom Rix <trix@redhat.com> Cc: mdf@kernel.org, linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org, lgoncalv@redhat.com Subject: Re: [RESEND PATCH 1/2] fpga: dfl: pci: reduce the scope of variable 'ret' Date: Fri, 10 Jul 2020 13:16:00 +0800 [thread overview] Message-ID: <20200710051600.GA8689@yilunxu-OptiPlex-7050> (raw) In-Reply-To: <ef506ec6-d857-736b-e9e2-46077ab7d778@redhat.com> On Thu, Jul 09, 2020 at 06:18:18AM -0700, Tom Rix wrote: > I think a better change is to use the ret variable, like this > > --- a/drivers/fpga/dfl-pci.c > +++ b/drivers/fpga/dfl-pci.c > @@ -312,7 +312,7 @@ static int cci_pci_sriov_configure(struct pci_dev *pcidev, int num_vfs) > } > } > > - return num_vfs; > + return ret; > } > > The existing use of returning num_vfs is not right, the function should return 0/err not num_vfs. currently it is reusing the 0 passed in with num_vfs to mean disable as the 0 return status. it should be properly returning ret. The sriov_configure callback should return negative value for error, and return num_vfs if success. See the Documentation/PCI/pci-iov-howto.rst also in drivers/pci/iov.c: static ssize_t sriov_numvfs_store(struct device *dev, ...) { ... ret = pdev->driver->sriov_configure(pdev, num_vfs); if (ret < 0) goto exit; if (ret != num_vfs) pci_warn(pdev, "%d VFs requested; only %d enabled\n", num_vfs, ret); ... } > > Tom > > On 7/9/20 1:12 AM, Xu Yilun wrote: > > This is to fix lkp cppcheck warnings: > > > > drivers/fpga/dfl-pci.c:230:6: warning: The scope of the variable 'ret' can be reduced. [variableScope] > > int ret = 0; > > ^ > > > > drivers/fpga/dfl-pci.c:230:10: warning: Variable 'ret' is assigned a value that is never used. [unreadVariable] > > int ret = 0; > > ^ > > > > Fixes: 3c2760b78f90 ("fpga: dfl: pci: fix return value of cci_pci_sriov_configure") > > Reported-by: kbuild test robot <lkp@intel.com> > > Signed-off-by: Xu Yilun <yilun.xu@intel.com> > > Acked-by: Wu Hao <hao.wu@intel.com> > > --- > > drivers/fpga/dfl-pci.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c > > index 4a14a24..73b5153 100644 > > --- a/drivers/fpga/dfl-pci.c > > +++ b/drivers/fpga/dfl-pci.c > > @@ -285,7 +285,6 @@ static int cci_pci_sriov_configure(struct pci_dev *pcidev, int num_vfs) > > { > > struct cci_drvdata *drvdata = pci_get_drvdata(pcidev); > > struct dfl_fpga_cdev *cdev = drvdata->cdev; > > - int ret = 0; > > > > if (!num_vfs) { > > /* > > @@ -297,6 +296,8 @@ static int cci_pci_sriov_configure(struct pci_dev *pcidev, int num_vfs) > > dfl_fpga_cdev_config_ports_pf(cdev); > > > > } else { > > + int ret; > > + > > /* > > * before enable SRIOV, put released ports into VF access mode > > * first of all.
WARNING: multiple messages have this Message-ID (diff)
From: Xu Yilun <yilun.xu@intel.com> To: Tom Rix <trix@redhat.com> Cc: mdf@kernel.org, linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org, lgoncalv@redhat.com Subject: Re: [RESEND PATCH 1/2] fpga: dfl: pci: reduce the scope of variable 'ret' Date: Fri, 10 Jul 2020 13:16:00 +0800 [thread overview] Message-ID: <20200710051600.GA8689@yilunxu-OptiPlex-7050> (raw) In-Reply-To: <ef506ec6-d857-736b-e9e2-46077ab7d778@redhat.com> On Thu, Jul 09, 2020 at 06:18:18AM -0700, Tom Rix wrote: > I think a better change is to use the ret variable, like this > > --- a/drivers/fpga/dfl-pci.c > +++ b/drivers/fpga/dfl-pci.c > @@ -312,7 +312,7 @@ static int cci_pci_sriov_configure(struct pci_dev *pcidev, int num_vfs) > ��������������� } > ������� } > � > -������ return num_vfs; > +������ return ret; > �} > > The existing use of returning num_vfs is not right, the function should return 0/err not num_vfs. currently it is reusing the 0 passed in with num_vfs to mean disable as the 0 return status.� it should be properly returning ret. The sriov_configure callback should return negative value for error, and return num_vfs if success. See the Documentation/PCI/pci-iov-howto.rst also in drivers/pci/iov.c: static ssize_t sriov_numvfs_store(struct device *dev, ...) { ... ret = pdev->driver->sriov_configure(pdev, num_vfs); if (ret < 0) goto exit; if (ret != num_vfs) pci_warn(pdev, "%d VFs requested; only %d enabled\n", num_vfs, ret); ... } > > Tom > > On 7/9/20 1:12 AM, Xu Yilun wrote: > > This is to fix lkp cppcheck warnings: > > > > drivers/fpga/dfl-pci.c:230:6: warning: The scope of the variable 'ret' can be reduced. [variableScope] > > int ret = 0; > > ^ > > > > drivers/fpga/dfl-pci.c:230:10: warning: Variable 'ret' is assigned a value that is never used. [unreadVariable] > > int ret = 0; > > ^ > > > > Fixes: 3c2760b78f90 ("fpga: dfl: pci: fix return value of cci_pci_sriov_configure") > > Reported-by: kbuild test robot <lkp@intel.com> > > Signed-off-by: Xu Yilun <yilun.xu@intel.com> > > Acked-by: Wu Hao <hao.wu@intel.com> > > --- > > drivers/fpga/dfl-pci.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c > > index 4a14a24..73b5153 100644 > > --- a/drivers/fpga/dfl-pci.c > > +++ b/drivers/fpga/dfl-pci.c > > @@ -285,7 +285,6 @@ static int cci_pci_sriov_configure(struct pci_dev *pcidev, int num_vfs) > > { > > struct cci_drvdata *drvdata = pci_get_drvdata(pcidev); > > struct dfl_fpga_cdev *cdev = drvdata->cdev; > > - int ret = 0; > > > > if (!num_vfs) { > > /* > > @@ -297,6 +296,8 @@ static int cci_pci_sriov_configure(struct pci_dev *pcidev, int num_vfs) > > dfl_fpga_cdev_config_ports_pf(cdev); > > > > } else { > > + int ret; > > + > > /* > > * before enable SRIOV, put released ports into VF access mode > > * first of all.
next prev parent reply other threads:[~2020-07-10 5:22 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-09 8:12 [RESEND PATCH 0/2] Bug fixes for FPGA DFL Xu Yilun 2020-07-09 8:12 ` [RESEND PATCH 1/2] fpga: dfl: pci: reduce the scope of variable 'ret' Xu Yilun 2020-07-09 13:18 ` Tom Rix 2020-07-10 5:16 ` Xu Yilun [this message] 2020-07-10 5:16 ` Xu Yilun 2020-07-11 16:21 ` Tom Rix 2020-07-09 8:12 ` [RESEND PATCH 2/2] fpga: dfl: fix bug in port reset handshake Xu Yilun 2020-07-09 13:20 ` Tom Rix
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200710051600.GA8689@yilunxu-OptiPlex-7050 \ --to=yilun.xu@intel.com \ --cc=lgoncalv@redhat.com \ --cc=linux-fpga@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mdf@kernel.org \ --cc=trix@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.