Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] pci: mediatek: fix wrong operator used
@ 2020-10-27 16:51 Ryder Lee
  2020-11-03 21:53 ` Bjorn Helgaas
  0 siblings, 1 reply; 2+ messages in thread
From: Ryder Lee @ 2020-10-27 16:51 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci, linux-mediatek, Ryder Lee

Fix the issue reported by Coverity:
Wrong operator used (CONSTANT_EXPRESSION_RESULT) operator_confusion:
(port->slot << 3) & 7 is always 0 regardless of the values of its operands.
This occurs as an initializer.

Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index cf4c18f0c25a..1980b01cee35 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -760,7 +760,7 @@ static struct pci_ops mtk_pcie_ops = {
 static int mtk_pcie_startup_port(struct mtk_pcie_port *port)
 {
 	struct mtk_pcie *pcie = port->pcie;
-	u32 func = PCI_FUNC(port->slot << 3);
+	u32 func = PCI_FUNC(port->slot);
 	u32 slot = PCI_SLOT(port->slot << 3);
 	u32 val;
 	int err;
-- 
2.18.0

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

* Re: [PATCH] pci: mediatek: fix wrong operator used
  2020-10-27 16:51 [PATCH] pci: mediatek: fix wrong operator used Ryder Lee
@ 2020-11-03 21:53 ` Bjorn Helgaas
  0 siblings, 0 replies; 2+ messages in thread
From: Bjorn Helgaas @ 2020-11-03 21:53 UTC (permalink / raw)
  To: Ryder Lee; +Cc: Bjorn Helgaas, linux-pci, linux-mediatek

  $ git log --oneline drivers/pci/controller/pcie-mediatek.c
  ...
  0cccd42e6193 ("PCI: mediatek: Add controller support for MT7629")
  6be22343cc54 ("PCI: mediatek: Get optional clocks with devm_clk_get_optional()")
  ff7a5a0a8562 ("PCI: mediatek: Fix a leaked reference by adding missing of_node_put()")
  cbe3a7728c7a ("PCI: mediatek: Enlarge PCIe2AHB window size to support 4GB DRAM")

Make yours match in capitalization and sentence structure, e.g.,

  PCI: mediatek: Fix ...

On Wed, Oct 28, 2020 at 12:51:48AM +0800, Ryder Lee wrote:
> Fix the issue reported by Coverity:
> Wrong operator used (CONSTANT_EXPRESSION_RESULT) operator_confusion:
> (port->slot << 3) & 7 is always 0 regardless of the values of its operands.
> This occurs as an initializer.

The important thing here is *not* fixing the Coverity warning.  The
important thing is fixing the *bug*.

The bug is that we computed "func" incorrectly.  The commit log should
mention what bad things happened because "func" was incorrect.

  #define PCI_FUNC(devfn)         ((devfn) & 0x07)

  func = PCI_FUNC(port->slot << 3);

So "func" was always 0 before, and from the code, it looks like that
means we only configured FC credits and FTS for function 0, so any
other functions may not have been configured correctly.

And it looks like this only affects MT2701 and MT7623, since those are
the only devices that use mtk_pcie_startup_port().

This is all info that should be in the commit log so users can tell
whether they are affected.

It's nice to still *mention* Coverity since it gave us useful
information, but all we need is a reference like this:

  Addresses-Coverity-ID: 1437218 ("Wrong operator used")

> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index cf4c18f0c25a..1980b01cee35 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -760,7 +760,7 @@ static struct pci_ops mtk_pcie_ops = {
>  static int mtk_pcie_startup_port(struct mtk_pcie_port *port)
>  {
>  	struct mtk_pcie *pcie = port->pcie;
> -	u32 func = PCI_FUNC(port->slot << 3);
> +	u32 func = PCI_FUNC(port->slot);
>  	u32 slot = PCI_SLOT(port->slot << 3);
>  	u32 val;
>  	int err;
> -- 
> 2.18.0

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

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-27 16:51 [PATCH] pci: mediatek: fix wrong operator used Ryder Lee
2020-11-03 21:53 ` Bjorn Helgaas

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org
	public-inbox-index linux-pci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git