* [PATCH][next] PCI: brcmstb: fix a missing if statement on a return error check
@ 2020-09-21 14:40 Colin King
2020-09-21 19:42 ` Florian Fainelli
0 siblings, 1 reply; 4+ messages in thread
From: Colin King @ 2020-09-21 14:40 UTC (permalink / raw)
To: Nicolas Saenz Julienne, Lorenzo Pieralisi, Rob Herring,
Bjorn Helgaas, Florian Fainelli, bcm-kernel-feedback-list,
Jim Quinlan, linux-rpi-kernel, linux-arm-kernel, linux-pci
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The error return ret is not being check with an if statement and
currently the code always returns leaving the following code as
dead code. Fix this by adding in the missing if statement.
Addresses-Coverity: ("Structurally dead code")
Fixes: ad3d29c77e1e ("PCI: brcmstb: Add control of rescal reset")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/pci/controller/pcie-brcmstb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 7a3ff4632e7c..cb0c11b7308e 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -1154,6 +1154,7 @@ static int brcm_pcie_resume(struct device *dev)
clk_prepare_enable(pcie->clk);
ret = brcm_phy_start(pcie);
+ if (ret)
return ret;
/* Take bridge out of reset so we can access the SERDES reg */
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH][next] PCI: brcmstb: fix a missing if statement on a return error check
2020-09-21 14:40 [PATCH][next] PCI: brcmstb: fix a missing if statement on a return error check Colin King
@ 2020-09-21 19:42 ` Florian Fainelli
2020-09-21 20:53 ` Jim Quinlan
0 siblings, 1 reply; 4+ messages in thread
From: Florian Fainelli @ 2020-09-21 19:42 UTC (permalink / raw)
To: Colin King, Nicolas Saenz Julienne, Lorenzo Pieralisi,
Rob Herring, Bjorn Helgaas, bcm-kernel-feedback-list,
Jim Quinlan, linux-rpi-kernel, linux-arm-kernel, linux-pci
Cc: kernel-janitors, linux-kernel
On 9/21/20 7:40 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The error return ret is not being check with an if statement and
> currently the code always returns leaving the following code as
> dead code. Fix this by adding in the missing if statement.
>
> Addresses-Coverity: ("Structurally dead code")
> Fixes: ad3d29c77e1e ("PCI: brcmstb: Add control of rescal reset")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/pci/controller/pcie-brcmstb.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 7a3ff4632e7c..cb0c11b7308e 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -1154,6 +1154,7 @@ static int brcm_pcie_resume(struct device *dev)
> clk_prepare_enable(pcie->clk);
>
> ret = brcm_phy_start(pcie);
> + if (ret)
> return ret;
Maybe this should also disable the clock if we failed to start the PHY
somehow.
--
Florian
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH][next] PCI: brcmstb: fix a missing if statement on a return error check
2020-09-21 19:42 ` Florian Fainelli
@ 2020-09-21 20:53 ` Jim Quinlan
2020-09-21 21:03 ` Colin Ian King
0 siblings, 1 reply; 4+ messages in thread
From: Jim Quinlan @ 2020-09-21 20:53 UTC (permalink / raw)
To: Florian Fainelli
Cc: Colin King, Nicolas Saenz Julienne, Lorenzo Pieralisi,
Rob Herring, Bjorn Helgaas,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Jim Quinlan,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
kernel-janitors, open list
[-- Attachment #1: Type: text/plain, Size: 1620 bytes --]
Hello,
I am fine with Colin's suggested change or Florians as well:
ret = brcm_phy_start(pcie);
+ if (ret) {
+ clk_disable_unprepare(pcie->clk);
return ret;
+ }
Currently, our STB upstream suspend/resume is not functional yet and
this is how this omission slipped by testing.
Thanks,
Jim Quinlan
Broadcom STB
On Mon, Sep 21, 2020 at 3:43 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> On 9/21/20 7:40 AM, Colin King wrote:
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > The error return ret is not being check with an if statement and
> > currently the code always returns leaving the following code as
> > dead code. Fix this by adding in the missing if statement.
> >
> > Addresses-Coverity: ("Structurally dead code")
> > Fixes: ad3d29c77e1e ("PCI: brcmstb: Add control of rescal reset")
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
> > ---
> > drivers/pci/controller/pcie-brcmstb.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> > index 7a3ff4632e7c..cb0c11b7308e 100644
> > --- a/drivers/pci/controller/pcie-brcmstb.c
> > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > @@ -1154,6 +1154,7 @@ static int brcm_pcie_resume(struct device *dev)
> > clk_prepare_enable(pcie->clk);
> >
> > ret = brcm_phy_start(pcie);
> > + if (ret)
> > return ret;
>
> Maybe this should also disable the clock if we failed to start the PHY
> somehow.
Hi Florian,
I'm fine with Colin's change as
>
> --
> Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4167 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH][next] PCI: brcmstb: fix a missing if statement on a return error check
2020-09-21 20:53 ` Jim Quinlan
@ 2020-09-21 21:03 ` Colin Ian King
0 siblings, 0 replies; 4+ messages in thread
From: Colin Ian King @ 2020-09-21 21:03 UTC (permalink / raw)
To: Jim Quinlan, Florian Fainelli
Cc: Nicolas Saenz Julienne, Lorenzo Pieralisi, Rob Herring,
Bjorn Helgaas, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
Jim Quinlan,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
kernel-janitors, open list
On 21/09/2020 21:53, Jim Quinlan wrote:
> Hello,
> I am fine with Colin's suggested change or Florians as well:
>
> ret = brcm_phy_start(pcie);
> + if (ret) {
> + clk_disable_unprepare(pcie->clk);
> return ret;
> + }
>
> Currently, our STB upstream suspend/resume is not functional yet and
> this is how this omission slipped by testing.
>
> Thanks,
> Jim Quinlan
> Broadcom STB
>
> On Mon, Sep 21, 2020 at 3:43 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>> On 9/21/20 7:40 AM, Colin King wrote:
>>> From: Colin Ian King <colin.king@canonical.com>
>>>
>>> The error return ret is not being check with an if statement and
>>> currently the code always returns leaving the following code as
>>> dead code. Fix this by adding in the missing if statement.
>>>
>>> Addresses-Coverity: ("Structurally dead code")
>>> Fixes: ad3d29c77e1e ("PCI: brcmstb: Add control of rescal reset")
>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>> ---
>>> drivers/pci/controller/pcie-brcmstb.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
>>> index 7a3ff4632e7c..cb0c11b7308e 100644
>>> --- a/drivers/pci/controller/pcie-brcmstb.c
>>> +++ b/drivers/pci/controller/pcie-brcmstb.c
>>> @@ -1154,6 +1154,7 @@ static int brcm_pcie_resume(struct device *dev)
>>> clk_prepare_enable(pcie->clk);
>>>
>>> ret = brcm_phy_start(pcie);
>>> + if (ret)
>>> return ret;
>>
>> Maybe this should also disable the clock if we failed to start the PHY
>> somehow.
>
> Hi Florian,
>
> I'm fine with Colin's change as
>
I'll send a V2 in a short while
>
>>
>> --
>> Florian
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-09-21 21:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-21 14:40 [PATCH][next] PCI: brcmstb: fix a missing if statement on a return error check Colin King
2020-09-21 19:42 ` Florian Fainelli
2020-09-21 20:53 ` Jim Quinlan
2020-09-21 21:03 ` Colin Ian King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).