Re: [PATCH][next] PCI: brcmstb: fix a missing if statement on a return error check
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 wrote:
>>
>> On 9/21/20 7:40 AM, Colin King wrote:
>>> From: Colin Ian King
>>>
>>> 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
>>> ---
>>> 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
Re: [PATCH][next] PCI: brcmstb: fix a missing if statement on a return error check
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 wrote:
>
> On 9/21/20 7:40 AM, Colin King wrote:
> > From: Colin Ian King
> >
> > 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
> > ---
> > 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
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH][next] PCI: brcmstb: fix a missing if statement on a return error check
On 9/21/20 7:40 AM, Colin King wrote:
> From: Colin Ian King
>
> 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
> ---
> 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
[PATCH][next] PCI: brcmstb: fix a missing if statement on a return error check
From: Colin Ian King
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
---
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
