* [PATCH] PCI: brcmstb: Fix control flow issue @ 2024-09-11 2:50 Qianqiang Liu 2024-09-11 13:44 ` Bjorn Helgaas 0 siblings, 1 reply; 4+ messages in thread From: Qianqiang Liu @ 2024-09-11 2:50 UTC (permalink / raw) To: bcm-kernel-feedback-list, florian.fainelli, james.quinlan Cc: linux-pci, linux-kernel, Qianqiang Liu The type of "num_inbound_wins" is "u8", so the less-than-zero comparison of an unsigned value is never true. Signed-off-by: Qianqiang Liu <qianqiang.liu@163.com> --- drivers/pci/controller/pcie-brcmstb.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c index 55311dc47615..3e4572c3eeb1 100644 --- a/drivers/pci/controller/pcie-brcmstb.c +++ b/drivers/pci/controller/pcie-brcmstb.c @@ -1090,9 +1090,11 @@ 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: brcmstb: Fix control flow issue 2024-09-11 2:50 [PATCH] PCI: brcmstb: Fix control flow issue Qianqiang Liu @ 2024-09-11 13:44 ` Bjorn Helgaas 2024-09-11 13:56 ` Qianqiang Liu 0 siblings, 1 reply; 4+ messages in thread From: Bjorn Helgaas @ 2024-09-11 13:44 UTC (permalink / raw) To: Qianqiang Liu Cc: bcm-kernel-feedback-list, florian.fainelli, james.quinlan, linux-pci, linux-kernel On Wed, Sep 11, 2024 at 10:50:59AM +0800, Qianqiang Liu wrote: > The type of "num_inbound_wins" is "u8", so the less-than-zero > comparison of an unsigned value is never true. I think this was fixed slightly differently but with the same effect; please check this to make sure: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/tree/drivers/pci/controller/pcie-brcmstb.c?h=controller/brcmstb#n1034 > --- > drivers/pci/controller/pcie-brcmstb.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > index 55311dc47615..3e4572c3eeb1 100644 > --- a/drivers/pci/controller/pcie-brcmstb.c > +++ b/drivers/pci/controller/pcie-brcmstb.c > @@ -1090,9 +1090,11 @@ 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 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: brcmstb: Fix control flow issue 2024-09-11 13:44 ` Bjorn Helgaas @ 2024-09-11 13:56 ` Qianqiang Liu 2024-09-11 15:02 ` Jim Quinlan 0 siblings, 1 reply; 4+ messages in thread From: Qianqiang Liu @ 2024-09-11 13:56 UTC (permalink / raw) To: Bjorn Helgaas Cc: bcm-kernel-feedback-list, florian.fainelli, james.quinlan, linux-pci, linux-kernel On Wed, Sep 11, 2024 at 08:44:36AM -0500, Bjorn Helgaas wrote: > On Wed, Sep 11, 2024 at 10:50:59AM +0800, Qianqiang Liu wrote: > > The type of "num_inbound_wins" is "u8", so the less-than-zero > > comparison of an unsigned value is never true. > > I think this was fixed slightly differently but with the same effect; > please check this to make sure: > > https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/tree/drivers/pci/controller/pcie-brcmstb.c?h=controller/brcmstb#n1034 > > > --- > > drivers/pci/controller/pcie-brcmstb.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > > index 55311dc47615..3e4572c3eeb1 100644 > > --- a/drivers/pci/controller/pcie-brcmstb.c > > +++ b/drivers/pci/controller/pcie-brcmstb.c > > @@ -1090,9 +1090,11 @@ 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 > > Yes, they have the same effect. -- Best, Qianqiang Liu ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: brcmstb: Fix control flow issue 2024-09-11 13:56 ` Qianqiang Liu @ 2024-09-11 15:02 ` Jim Quinlan 0 siblings, 0 replies; 4+ messages in thread From: Jim Quinlan @ 2024-09-11 15:02 UTC (permalink / raw) To: Qianqiang Liu Cc: Bjorn Helgaas, bcm-kernel-feedback-list, florian.fainelli, linux-pci, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2099 bytes --] On Wed, Sep 11, 2024 at 9:56 AM Qianqiang Liu <qianqiang.liu@163.com> wrote: > > On Wed, Sep 11, 2024 at 08:44:36AM -0500, Bjorn Helgaas wrote: > > On Wed, Sep 11, 2024 at 10:50:59AM +0800, Qianqiang Liu wrote: > > > The type of "num_inbound_wins" is "u8", so the less-than-zero > > > comparison of an unsigned value is never true. > > > > I think this was fixed slightly differently but with the same effect; > > please check this to make sure: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/tree/drivers/pci/controller/pcie-brcmstb.c?h=controller/brcmstb#n1034 > > > > > --- > > > drivers/pci/controller/pcie-brcmstb.c | 8 +++++--- > > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > > > index 55311dc47615..3e4572c3eeb1 100644 > > > --- a/drivers/pci/controller/pcie-brcmstb.c > > > +++ b/drivers/pci/controller/pcie-brcmstb.c > > > @@ -1090,9 +1090,11 @@ 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 > > > > > Yes, they have the same effect. Qianqiang's fixup has the slight advantage of keeping the var a u8, which is what a reviewer wanted in the first place. I found it surprising that "if (u8var < 0)" does not cause a compiler warning, but it appears that W=1 does not include "-Wtype-limits". That's no excuse for this mistake, of course :-( Thanks and regards, Jim Quinlan Broadcom STB/CM > > -- > Best, > Qianqiang Liu > [-- Attachment #2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 4210 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-09-11 15:02 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-11 2:50 [PATCH] PCI: brcmstb: Fix control flow issue Qianqiang Liu 2024-09-11 13:44 ` Bjorn Helgaas 2024-09-11 13:56 ` Qianqiang Liu 2024-09-11 15:02 ` Jim Quinlan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.