| Message ID | 20240906110932.299689-1-usama.anjum@collabora.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | PCI: brcmstb: Correctly store and use the output value | expand |
On Fri, Sep 6, 2024 at 7:10 AM Muhammad Usama Anjum <usama.anjum@collabora.com> wrote: > > brcm_pcie_get_inbound_wins() can return negative error. As > num_inbound_wins is unsigned, we'll be unable to recognize the error. > Hence store return value of brcm_pcie_get_inbound_wins() in ret which is > signed and store result back to num_inbound_wins after confirming that > it isn't negative. Hello Muhammad, You are correct -- I was asked to make a few variables to be of the type u8, but I missed having an int (ret) hold the resultof that call. I believe I am still in the process of submitting this commit series -- V7 is coming next -- so I will take your email as a review instead of adding a fixup commit. Unless Bjorn says that V6 was applied. Thanks and regards, Jim Quinlan Broadcom STB/CM > > > Fixes: 46c981fd60de ("PCI: brcmstb: Refactor for chips with many regular inbound windows") > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > drivers/pci/controller/pcie-brcmstb.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > index 55311dc47615d..054810d7962d7 100644 > --- a/drivers/pci/controller/pcie-brcmstb.c > +++ b/drivers/pci/controller/pcie-brcmstb.c > @@ -1090,9 +1090,10 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie) > u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE_MASK); > writel(tmp, base + PCIE_MISC_MISC_CTRL); > > - num_inbound_wins = brcm_pcie_get_inbound_wins(pcie, inbound_wins); > - if (num_inbound_wins < 0) > - return num_inbound_wins; > + ret = brcm_pcie_get_inbound_wins(pcie, inbound_wins); > + if (ret < 0) > + return ret; > + num_inbound_wins = (u8)ret; > > set_inbound_win_registers(pcie, inbound_wins, num_inbound_wins); > > -- > 2.39.2 >
On 9/6/24 11:20, Jim Quinlan wrote: > On Fri, Sep 6, 2024 at 7:10 AM Muhammad Usama Anjum > <usama.anjum@collabora.com> wrote: >> >> brcm_pcie_get_inbound_wins() can return negative error. As >> num_inbound_wins is unsigned, we'll be unable to recognize the error. >> Hence store return value of brcm_pcie_get_inbound_wins() in ret which is >> signed and store result back to num_inbound_wins after confirming that >> it isn't negative. > > > Hello Muhammad, > You are correct -- I was asked to make a few variables to be of the > type u8, but I missed having an int (ret) hold the > resultof that call. I believe I am still in the process of submitting > this commit series -- V7 is coming next -- so I will > take your email as a review instead of adding a fixup commit. > > Unless Bjorn says that V6 was applied. A similar patch has already been submitted to address this in a slightly different way: https://lore.kernel.org/all/20240904161953.46790-2-riyandhiman14@gmail.com/
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c index 55311dc47615d..054810d7962d7 100644 --- a/drivers/pci/controller/pcie-brcmstb.c +++ b/drivers/pci/controller/pcie-brcmstb.c @@ -1090,9 +1090,10 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie) u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE_MASK); writel(tmp, base + PCIE_MISC_MISC_CTRL); - num_inbound_wins = brcm_pcie_get_inbound_wins(pcie, inbound_wins); - if (num_inbound_wins < 0) - return num_inbound_wins; + ret = brcm_pcie_get_inbound_wins(pcie, inbound_wins); + if (ret < 0) + return ret; + num_inbound_wins = (u8)ret; set_inbound_win_registers(pcie, inbound_wins, num_inbound_wins);
brcm_pcie_get_inbound_wins() can return negative error. As num_inbound_wins is unsigned, we'll be unable to recognize the error. Hence store return value of brcm_pcie_get_inbound_wins() in ret which is signed and store result back to num_inbound_wins after confirming that it isn't negative. Fixes: 46c981fd60de ("PCI: brcmstb: Refactor for chips with many regular inbound windows") Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> --- drivers/pci/controller/pcie-brcmstb.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)