| Message ID | 2418edb8c8c81bc646ce9c508939dc27e848dcd7.1603817008.git.ryder.lee@mediatek.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | pci: mediatek: fix wrong operator used | expand |
$ 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
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;
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(-)